arm64: dts: r8a7795: Increase the size of GIC-400 mapped registers

From: Pooya Keshavarzi <[email protected]>

There are some requirements about the GIC-400 memory layout and its mapping if using 64k aligned base addresses like on r8a7795.

See e.g.

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Map the whole memory range instead of only 0x2000. This will fix the issue that some hypervisors, e.g. Xen, fail to handle the interrupts correctly.

Signed-off-by Pooya Keshavarzi <[email protected]>

Signed-off-by Dirk Behme <[email protected]>

---
Note: This patch is against renesas-drivers-2016-04-12-v4.6-rc3

 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/boot/dts/renesas/r8a7795.dtsi +2 -2

index 8be9424..d880fd4 100644
@@ -160,9 +160,9 @@
 			#address-cells = <0>;
 			interrupt-controller;
 			reg = <0x0 0xf1010000 0 0x1000>,
-			      <0x0 0xf1020000 0 0x2000>,
+			      <0x0 0xf1020000 0 0x20000>,
 			      <0x0 0xf1040000 0 0x20000>,
-			      <0x0 0xf1060000 0 0x2000>;
+			      <0x0 0xf1060000 0 0x20000>;
 			interrupts = <GIC_PPI 9
 					(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		};

Signature · 1 line

2.8.0

Hi Dirk,

I understand that there is an issue here but I'm not yet able to convince myself that this is the correct solution.

In revision r0p1 of the CoreLink GIC-400 Generic Interrupt Controller Technical Reference Manual[1] I see in Section 3.2. "GIC-400 register map" that the size of both the CPU interfaces and Virtual CPU interfaces are 0x2000 bytes. And assuming that the hardware follows the specification it appears that DT is correctly describing the hardware.

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0471b/index.html

Quoted 40 lines · Dirk Behme

From: Pooya Keshavarzi <[email protected]>

There are some requirements about the GIC-400 memory layout and its mapping if using 64k aligned base addresses like on r8a7795.

See e.g.

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Map the whole memory range instead of only 0x2000. This will fix the issue that some hypervisors, e.g. Xen, fail to handle the interrupts correctly.

Signed-off-by Pooya Keshavarzi <[email protected]>

Signed-off-by Dirk Behme <[email protected]>

---
Note: This patch is against renesas-drivers-2016-04-12-v4.6-rc3

  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/boot/dts/renesas/r8a7795.dtsi +2 -3

index 8be9424..d880fd4 100644
@@ -160,9 +160,9 @@
  			#address-cells = <0>;
  			interrupt-controller;
  			reg = <0x0 0xf1010000 0 0x1000>,
-			      <0x0 0xf1020000 0 0x2000>,
+			      <0x0 0xf1020000 0 0x20000>,
  			      <0x0 0xf1040000 0 0x20000>,
-			      <0x0 0xf1060000 0 0x2000>;
+			      <0x0 0xf1060000 0 0x20000>;
  			interrupts = <GIC_PPI 9
  					(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
  		};
--

2.8.0

Hi Simon,

Quoted 10 lines · Simon Horman

Hi Dirk,

I understand that there is an issue here but I'm not yet able to convince myself that this is the correct solution.

In revision r0p1 of the CoreLink GIC-400 Generic Interrupt Controller Technical Reference Manual[1] I see in Section 3.2. "GIC-400 register map" that the size of both the CPU interfaces and Virtual CPU interfaces are 0x2000 bytes. And assuming that the hardware follows the specification it appears that DT is correctly describing the hardware.

I think you are missing the details described by ARM in

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Maybe Julien could help if you have some more doubts?

Best regards

Dirk

Quoted 44 lines

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0471b/index.html

Quoted 40 lines · Dirk Behme

From: Pooya Keshavarzi <[email protected]>

There are some requirements about the GIC-400 memory layout and its mapping if using 64k aligned base addresses like on r8a7795.

See e.g.

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Map the whole memory range instead of only 0x2000. This will fix the issue that some hypervisors, e.g. Xen, fail to handle the interrupts correctly.

Signed-off-by Pooya Keshavarzi <[email protected]>

Signed-off-by Dirk Behme <[email protected]>

---
Note: This patch is against renesas-drivers-2016-04-12-v4.6-rc3

 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/boot/dts/renesas/r8a7795.dtsi +2 -3

index 8be9424..d880fd4 100644
@@ -160,9 +160,9 @@
 			#address-cells = <0>;
 			interrupt-controller;
 			reg = <0x0 0xf1010000 0 0x1000>,
-			      <0x0 0xf1020000 0 0x2000>,
+			      <0x0 0xf1020000 0 0x20000>,
 			      <0x0 0xf1040000 0 0x20000>,
-			      <0x0 0xf1060000 0 0x2000>;
+			      <0x0 0xf1060000 0 0x20000>;
 			interrupts = <GIC_PPI 9
 					(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		};
--

2.8.0

[Cc Mark Zyngier, linux-arm-kernel]

Hi Dirk,

Quoted 20 lines · Dirk Behme

Hi Simon,

Quoted 10 lines · Simon Horman

Hi Dirk,

I understand that there is an issue here but I'm not yet able to convince myself that this is the correct solution.

In revision r0p1 of the CoreLink GIC-400 Generic Interrupt Controller Technical Reference Manual[1] I see in Section 3.2. "GIC-400 register map" that the size of both the CPU interfaces and Virtual CPU interfaces are 0x2000 bytes. And assuming that the hardware follows the specification it appears that DT is correctly describing the hardware.

I think you are missing the details described by ARM in

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Quoted 1 line

Maybe Julien could help if you have some more doubts?

I guess I am confused.

I see that there is now handling of the case where the region size is 128Kbytes. But I'm still not seeing the bit which describes that the GIC-400 has a region size of 128Kbytes. Perhaps the later is somehow implied by the former. Or perhaps I need to check with the hw team.

Quoted 25 lines · On Fri, 29 Apr 2016 09:43:45 +1000 Simon Horman <[email protected]> wrote:

[Cc Mark Zyngier, linux-arm-kernel]

Hi Dirk,

Quoted 20 lines · Dirk Behme

Hi Simon,

Quoted 10 lines · Simon Horman

Hi Dirk,

I understand that there is an issue here but I'm not yet able to convince myself that this is the correct solution.

In revision r0p1 of the CoreLink GIC-400 Generic Interrupt Controller Technical Reference Manual[1] I see in Section 3.2. "GIC-400 register map" that the size of both the CPU interfaces and Virtual CPU interfaces are 0x2000 bytes. And assuming that the hardware follows the specification it appears that DT is correctly describing the hardware.

I think you are missing the details described by ARM in

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Quoted 8 lines

Quoted 1 line

Maybe Julien could help if you have some more doubts?

I guess I am confused.

I see that there is now handling of the case where the region size is 128Kbytes. But I'm still not seeing the bit which describes that the GIC-400 has a region size of 128Kbytes. Perhaps the later is somehow implied by the former. Or perhaps I need to check with the hw team.

Please have a look at the SBSA document, and in particular the Appendix-F (registration and selling your soul required - only kidding):

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0029/index.html

This requires that, in order for the two halves of GICV to be trappable *separately* by a hypervisor using 64kB pages at Stage-2, the two 4kB pages that describe that region are aliased as such:
- the first 4kB page is aliased 16 times over a 64kB region
- the second 4kB page is aliased 16 times over another contiguous 64kB region

This means that your GIC is indeed covering a 128kB region, with the mapping corresponding to the GICv2 memory map located at offset 0xf000 from the base of that 128kB region. Also, this GICV requirement also applies to GICC (most likely because the two regions use the same decoding logic).

The OS must of course be aware of this (see gic_check_eoimode in the GIC driver). Of course, almost nobody got that right (I only know of the APM Xgene-1 so far). If you actually did, great!

Also, the ACPI spec fails to recognize this by not providing the length of the region, meaning that those who got it right with DT are likely to get it wrong with ACPI, and vice-versa.

It's a wonderful world.

Hi Simon,

Quoted 59 lines · Marc Zyngier

Quoted 25 lines · On Fri, 29 Apr 2016 09:43:45 +1000 Simon Horman <[email protected]> wrote:

[Cc Mark Zyngier, linux-arm-kernel]

Hi Dirk,

Quoted 20 lines · Dirk Behme

Hi Simon,

Quoted 10 lines · Simon Horman

Hi Dirk,

I understand that there is an issue here but I'm not yet able to convince myself that this is the correct solution.

In revision r0p1 of the CoreLink GIC-400 Generic Interrupt Controller Technical Reference Manual[1] I see in Section 3.2. "GIC-400 register map" that the size of both the CPU interfaces and Virtual CPU interfaces are 0x2000 bytes. And assuming that the hardware follows the specification it appears that DT is correctly describing the hardware.

I think you are missing the details described by ARM in

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Quoted 8 lines

Quoted 1 line

Maybe Julien could help if you have some more doubts?

I guess I am confused.

I see that there is now handling of the case where the region size is 128Kbytes. But I'm still not seeing the bit which describes that the GIC-400 has a region size of 128Kbytes. Perhaps the later is somehow implied by the former. Or perhaps I need to check with the hw team.

Please have a look at the SBSA document, and in particular the Appendix-F (registration and selling your soul required - only kidding):

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0029/index.html

This requires that, in order for the two halves of GICV to be trappable *separately* by a hypervisor using 64kB pages at Stage-2, the two 4kB pages that describe that region are aliased as such:
- the first 4kB page is aliased 16 times over a 64kB region
- the second 4kB page is aliased 16 times over another contiguous 64kB region

This means that your GIC is indeed covering a 128kB region, with the mapping corresponding to the GICv2 memory map located at offset 0xf000 from the base of that 128kB region. Also, this GICV requirement also applies to GICC (most likely because the two regions use the same decoding logic).

The OS must of course be aware of this (see gic_check_eoimode in the GIC driver). Of course, almost nobody got that right (I only know of the APM Xgene-1 so far). If you actually did, great!

Quoted 5 lines

Also, the ACPI spec fails to recognize this by not providing the length of the region, meaning that those who got it right with DT are likely to get it wrong with ACPI, and vice-versa.

It's a wonderful world.

Could this patch be applied, then?

Hi Dirk,

Quoted 66 lines · Dirk Behme

Hi Simon,

Quoted 63 lines · Marc Zyngier

Quoted 25 lines · On Fri, 29 Apr 2016 09:43:45 +1000 Simon Horman <[email protected]> wrote:

[Cc Mark Zyngier, linux-arm-kernel]

Hi Dirk,

Quoted 20 lines · Dirk Behme

Hi Simon,

Quoted 10 lines · Simon Horman

Hi Dirk,

I understand that there is an issue here but I'm not yet able to convince myself that this is the correct solution.

In revision r0p1 of the CoreLink GIC-400 Generic Interrupt Controller Technical Reference Manual[1] I see in Section 3.2. "GIC-400 register map" that the size of both the CPU interfaces and Virtual CPU interfaces are 0x2000 bytes. And assuming that the hardware follows the specification it appears that DT is correctly describing the hardware.

I think you are missing the details described by ARM in

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Quoted 8 lines

Quoted 1 line

Maybe Julien could help if you have some more doubts?

I guess I am confused.

I see that there is now handling of the case where the region size is 128Kbytes. But I'm still not seeing the bit which describes that the GIC-400 has a region size of 128Kbytes. Perhaps the later is somehow implied by the former. Or perhaps I need to check with the hw team.

Please have a look at the SBSA document, and in particular the Appendix-F (registration and selling your soul required - only kidding):

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0029/index.html

This requires that, in order for the two halves of GICV to be trappable *separately* by a hypervisor using 64kB pages at Stage-2, the two 4kB pages that describe that region are aliased as such:
- the first 4kB page is aliased 16 times over a 64kB region
- the second 4kB page is aliased 16 times over another contiguous 64kB region

This means that your GIC is indeed covering a 128kB region, with the mapping corresponding to the GICv2 memory map located at offset 0xf000 from the base of that 128kB region. Also, this GICV requirement also applies to GICC (most likely because the two regions use the same decoding logic).

The OS must of course be aware of this (see gic_check_eoimode in the GIC driver). Of course, almost nobody got that right (I only know of the APM Xgene-1 so far). If you actually did, great!

Also, the ACPI spec fails to recognize this by not providing the length of the region, meaning that those who got it right with DT are likely to get it wrong with ACPI, and vice-versa.

Quoted 4 lines

Quoted 1 line

It's a wonderful world.

Could this patch be applied, then?

Yes I have now done so :)

In parallel, a r8a7796.dtsi was added where this change is missing, now:

https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/tree/arch/arm64/boot/dts/renesas/r8a7796.dtsi?h=topic/gen3-latest

Do you need a new patch to add this to the r8a7796.dtsi, too? Or could you apply

https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/commit/arch/arm64/boot/dts/renesas?h=topic/gen3-latest&id=f7c7e97097915643a86051fe557b41a87c837172

to the r8a7796.dtsi, too?

Hi Geert and Simon,

Quoted 69 lines · Simon Horman

Hi Dirk,

Quoted 66 lines · Dirk Behme

Hi Simon,

Quoted 63 lines · Marc Zyngier

Quoted 25 lines · On Fri, 29 Apr 2016 09:43:45 +1000 Simon Horman <[email protected]> wrote:

[Cc Mark Zyngier, linux-arm-kernel]

Hi Dirk,

Quoted 20 lines · Dirk Behme

Hi Simon,

Quoted 10 lines · Simon Horman

Hi Dirk,

I understand that there is an issue here but I'm not yet able to convince myself that this is the correct solution.

In revision r0p1 of the CoreLink GIC-400 Generic Interrupt Controller Technical Reference Manual[1] I see in Section 3.2. "GIC-400 register map" that the size of both the CPU interfaces and Virtual CPU interfaces are 0x2000 bytes. And assuming that the hardware follows the specification it appears that DT is correctly describing the hardware.

I think you are missing the details described by ARM in

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Quoted 8 lines

Quoted 1 line

Maybe Julien could help if you have some more doubts?

I guess I am confused.

I see that there is now handling of the case where the region size is 128Kbytes. But I'm still not seeing the bit which describes that the GIC-400 has a region size of 128Kbytes. Perhaps the later is somehow implied by the former. Or perhaps I need to check with the hw team.

Please have a look at the SBSA document, and in particular the Appendix-F (registration and selling your soul required - only kidding):

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0029/index.html

This requires that, in order for the two halves of GICV to be trappable *separately* by a hypervisor using 64kB pages at Stage-2, the two 4kB pages that describe that region are aliased as such:
- the first 4kB page is aliased 16 times over a 64kB region
- the second 4kB page is aliased 16 times over another contiguous 64kB region

This means that your GIC is indeed covering a 128kB region, with the mapping corresponding to the GICv2 memory map located at offset 0xf000 from the base of that 128kB region. Also, this GICV requirement also applies to GICC (most likely because the two regions use the same decoding logic).

The OS must of course be aware of this (see gic_check_eoimode in the GIC driver). Of course, almost nobody got that right (I only know of the APM Xgene-1 so far). If you actually did, great!

Also, the ACPI spec fails to recognize this by not providing the length of the region, meaning that those who got it right with DT are likely to get it wrong with ACPI, and vice-versa.

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

It's a wonderful world.

Could this patch be applied, then?

Yes I have now done so :)

In parallel, a r8a7796.dtsi was added where this change is missing, now:

https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/tree/arch/arm64/boot/dts/renesas/r8a7796.dtsi?h=topic/gen3-latest

Do you need a new patch to add this to the r8a7796.dtsi, too? Or could you apply

https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/commit/arch/arm64/boot/dts/renesas?h=topic/gen3-latest&id=f7c7e97097915643a86051fe557b41a87c837172

to the r8a7796.dtsi, too?

Quoted 82 lines · Dirk Behme

Hi Geert and Simon,

Quoted 69 lines · Simon Horman

Hi Dirk,

Quoted 66 lines · Dirk Behme

Hi Simon,

Quoted 63 lines · Marc Zyngier

Quoted 25 lines · On Fri, 29 Apr 2016 09:43:45 +1000 Simon Horman <[email protected]> wrote:

[Cc Mark Zyngier, linux-arm-kernel]

Hi Dirk,

Quoted 20 lines · Dirk Behme

Hi Simon,

Quoted 10 lines · Simon Horman

Hi Dirk,

I understand that there is an issue here but I'm not yet able to convince myself that this is the correct solution.

In revision r0p1 of the CoreLink GIC-400 Generic Interrupt Controller Technical Reference Manual[1] I see in Section 3.2. "GIC-400 register map" that the size of both the CPU interfaces and Virtual CPU interfaces are 0x2000 bytes. And assuming that the hardware follows the specification it appears that DT is correctly describing the hardware.

I think you are missing the details described by ARM in

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Quoted 8 lines

Quoted 1 line

Maybe Julien could help if you have some more doubts?

I guess I am confused.

I see that there is now handling of the case where the region size is 128Kbytes. But I'm still not seeing the bit which describes that the GIC-400 has a region size of 128Kbytes. Perhaps the later is somehow implied by the former. Or perhaps I need to check with the hw team.

Please have a look at the SBSA document, and in particular the Appendix-F (registration and selling your soul required - only kidding):

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0029/index.html

This requires that, in order for the two halves of GICV to be trappable *separately* by a hypervisor using 64kB pages at Stage-2, the two 4kB pages that describe that region are aliased as such:
- the first 4kB page is aliased 16 times over a 64kB region
- the second 4kB page is aliased 16 times over another contiguous 64kB region

This means that your GIC is indeed covering a 128kB region, with the mapping corresponding to the GICv2 memory map located at offset 0xf000 from the base of that 128kB region. Also, this GICV requirement also applies to GICC (most likely because the two regions use the same decoding logic).

The OS must of course be aware of this (see gic_check_eoimode in the GIC driver). Of course, almost nobody got that right (I only know of the APM Xgene-1 so far). If you actually did, great!

Also, the ACPI spec fails to recognize this by not providing the length of the region, meaning that those who got it right with DT are likely to get it wrong with ACPI, and vice-versa.

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

It's a wonderful world.

Could this patch be applied, then?

Yes I have now done so :)

In parallel, a r8a7796.dtsi was added where this change is missing, now:

Quoted 8 lines

https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/tree/arch/arm64/boot/dts/renesas/r8a7796.dtsi?h=topic/gen3-latest

Do you need a new patch to add this to the r8a7796.dtsi, too? Or could you apply

https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/commit/arch/arm64/boot/dts/renesas?h=topic/gen3-latest&id=f7c7e97097915643a86051fe557b41a87c837172

to the r8a7796.dtsi, too?

Hi Dirk,

The copy of r8a7796.dtsi in that tree is from a topic branch and it is not targeted at upstream in that form.

I am working on patches to add r8a7796.dtsi amongst other things and will include this change there.

From: Pooya Keshavarzi <[email protected]>

There are some requirements about the GIC-400 memory layout and its mapping if using 64k aligned base addresses like on r8a7795.

See e.g.

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Map the whole memory range instead of only 0x2000. This will fix the issue that some hypervisors, e.g. Xen, fail to handle the interrupts correctly.

Signed-off-by Pooya Keshavarzi <[email protected]>

Signed-off-by Dirk Behme <[email protected]>

---
Note: This patch is against renesas-drivers-2016-04-12-v4.6-rc3

 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/boot/dts/renesas/r8a7795.dtsi +0 -0

index 8be9424..d880fd4 100644
@@ -160,9 +160,9 @@
                        #address-cells = <0>;

CC Marc, lakml

Quoted 7 lines · Dirk Behme

From: Pooya Keshavarzi <[email protected]>

There are some requirements about the GIC-400 memory layout and its mapping if using 64k aligned base addresses like on r8a7795.

See e.g.

Quoted 8 lines

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Map the whole memory range instead of only 0x2000. This will fix the issue that some hypervisors, e.g. Xen, fail to handle the interrupts correctly.

Signed-off-by Pooya Keshavarzi <[email protected]>

Signed-off-by Dirk Behme <[email protected]>

Based on my understanding below

Acked-by Geert Uytterhoeven <[email protected]>

Quoted 12 lines

---
Note: This patch is against renesas-drivers-2016-04-12-v4.6-rc3

 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/boot/dts/renesas/r8a7795.dtsi +0 -0

index 8be9424..d880fd4 100644
@@ -160,9 +160,9 @@
                        #address-cells = <0>;

arch/arm64/boot/dts/renesas/r8a7795.dtsi line 168

Quoted 10 lines

arch/arm64/boot/dts/renesas/r8a7795.dtsi +2 -2

                        interrupt-controller;
                        reg = <0x0 0xf1010000 0 0x1000>,
-                             <0x0 0xf1020000 0 0x2000>,
+                             <0x0 0xf1020000 0 0x20000>,
                              <0x0 0xf1040000 0 0x20000>,
-                             <0x0 0xf1060000 0 0x2000>;
+                             <0x0 0xf1060000 0 0x20000>;
                        interrupts = <GIC_PPI 9
                                        (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
                };

Region 0:
4 KiB-pages 0xf1011000-0xf101ffff are aliased to 0xf1010000-0xf1010fff, but we need the first 4 KiB only.

Region 1:

    4 KiB-pages 0xf1021000-0xf102ffff are aliased to 0xf1020000-0xf1020fff,
    4 KiB-pages 0xf1030000-0xf103ffff are all zeroes, probably due to
    non-secure mode?

Region 2:

    4 KiB-pages 0xf1041000-0xf104ffff are aliased to 0xf1040000-0xf1040fff,
    4 KiB-pages 0xf1050000-0xf105ffff are all zeroes, probably due to
    non-secure mode?

Region 3:

    4 KiB-pages 0xf1061000-0xf106ffff are aliased to 0xf1060000-0xf1060fff,
    4 KiB-pages 0xf1070000-0xf107ffff are all zeroes, probably due to
    non-secure mode?

Region 2 already had a 128 KiB size before, which allowed to use 8 KiB at 0xf104f000.

An 8 KiB size for regions 1 and 3 indeed didn't make much sense, as this covered two identical (aliased) 4 KiB pages, instead of two different pages at offset 0xf000.

Quoted 49 lines · Geert Uytterhoeven

CC Marc, lakml

Quoted 7 lines · Dirk Behme

From: Pooya Keshavarzi <[email protected]>

There are some requirements about the GIC-400 memory layout and its mapping if using 64k aligned base addresses like on r8a7795.

See e.g.

Quoted 8 lines

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Map the whole memory range instead of only 0x2000. This will fix the issue that some hypervisors, e.g. Xen, fail to handle the interrupts correctly.

Signed-off-by Pooya Keshavarzi <[email protected]>

Signed-off-by Dirk Behme <[email protected]>

Based on my understanding below

Acked-by Geert Uytterhoeven <[email protected]>

Quoted 12 lines

---
Note: This patch is against renesas-drivers-2016-04-12-v4.6-rc3

  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/boot/dts/renesas/r8a7795.dtsi +0 -0

index 8be9424..d880fd4 100644
@@ -160,9 +160,9 @@
                         #address-cells = <0>;

arch/arm64/boot/dts/renesas/r8a7795.dtsi line 168

Quoted 10 lines

arch/arm64/boot/dts/renesas/r8a7795.dtsi +2 -2

                         interrupt-controller;
                         reg = <0x0 0xf1010000 0 0x1000>,
-                             <0x0 0xf1020000 0 0x2000>,
+                             <0x0 0xf1020000 0 0x20000>,
                               <0x0 0xf1040000 0 0x20000>,
-                             <0x0 0xf1060000 0 0x2000>;
+                             <0x0 0xf1060000 0 0x20000>;
                         interrupts = <GIC_PPI 9
                                         (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
                 };

Region 0:
4 KiB-pages 0xf1011000-0xf101ffff are aliased to 0xf1010000-0xf1010fff, but we need the first 4 KiB only.

Quoted 4 lines

Region 1:

     4 KiB-pages 0xf1021000-0xf102ffff are aliased to 0xf1020000-0xf1020fff,
     4 KiB-pages 0xf1030000-0xf103ffff are all zeroes, probably due to
     non-secure mode?

No. This 4kB page only contain a single register (GICC_DIR), which is WO/RAZ.

Quoted 5 lines

Region 2:

     4 KiB-pages 0xf1041000-0xf104ffff are aliased to 0xf1040000-0xf1040fff,
     4 KiB-pages 0xf1050000-0xf105ffff are all zeroes, probably due to
     non-secure mode?

Neither. The aliases are an unused feature of GIC400 exposing the other CPUs view of the same registers...

Quoted 5 lines

Region 3:

     4 KiB-pages 0xf1061000-0xf106ffff are aliased to 0xf1060000-0xf1060fff,
     4 KiB-pages 0xf1070000-0xf107ffff are all zeroes, probably due to
     non-secure mode?

Same as region 1.

Quoted 3 lines

Region 2 already had a 128 KiB size before, which allowed to use 8 KiB at 0xf104f000.

No. This region (GICH) only needs the first 256 bytes or so. The rest is either RAZ/WI or useless stuff.

Quoted 1 line

Quoted 3 lines

An 8 KiB size for regions 1 and 3 indeed didn't make much sense, as this covered two identical (aliased) 4 KiB pages, instead of two different pages at offset 0xf000.

While we're at it, adding a pointer to the documentation (GIC400 and SBSA) would be tremendously useful, as it'd avoid misinterpreting the various bits.

Quoted 87 lines · Marc Zyngier

Quoted 49 lines · Geert Uytterhoeven

CC Marc, lakml

Quoted 7 lines · Dirk Behme

From: Pooya Keshavarzi <[email protected]>

There are some requirements about the GIC-400 memory layout and its mapping if using 64k aligned base addresses like on r8a7795.

See e.g.

Quoted 8 lines

http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=21550029f709072aacf3b9

Map the whole memory range instead of only 0x2000. This will fix the issue that some hypervisors, e.g. Xen, fail to handle the interrupts correctly.

Signed-off-by Pooya Keshavarzi <[email protected]>

Signed-off-by Dirk Behme <[email protected]>

Based on my understanding below

Acked-by Geert Uytterhoeven <[email protected]>

Quoted 12 lines

---
Note: This patch is against renesas-drivers-2016-04-12-v4.6-rc3

  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/boot/dts/renesas/r8a7795.dtsi +0 -0

index 8be9424..d880fd4 100644
@@ -160,9 +160,9 @@
                         #address-cells = <0>;

arch/arm64/boot/dts/renesas/r8a7795.dtsi line 168

Quoted 10 lines

arch/arm64/boot/dts/renesas/r8a7795.dtsi +2 -2

                         interrupt-controller;
                         reg = <0x0 0xf1010000 0 0x1000>,
-                             <0x0 0xf1020000 0 0x2000>,
+                             <0x0 0xf1020000 0 0x20000>,
                               <0x0 0xf1040000 0 0x20000>,
-                             <0x0 0xf1060000 0 0x2000>;
+                             <0x0 0xf1060000 0 0x20000>;
                         interrupts = <GIC_PPI 9
                                         (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
                 };

Region 0:
4 KiB-pages 0xf1011000-0xf101ffff are aliased to 0xf1010000-0xf1010fff, but we need the first 4 KiB only.

Quoted 4 lines

Region 1:

     4 KiB-pages 0xf1021000-0xf102ffff are aliased to 0xf1020000-0xf1020fff,
     4 KiB-pages 0xf1030000-0xf103ffff are all zeroes, probably due to
     non-secure mode?

No. This 4kB page only contain a single register (GICC_DIR), which is WO/RAZ.

Quoted 5 lines

Region 2:

     4 KiB-pages 0xf1041000-0xf104ffff are aliased to 0xf1040000-0xf1040fff,
     4 KiB-pages 0xf1050000-0xf105ffff are all zeroes, probably due to
     non-secure mode?

Neither. The aliases are an unused feature of GIC400 exposing the other CPUs view of the same registers...

Quoted 5 lines

Region 3:

     4 KiB-pages 0xf1061000-0xf106ffff are aliased to 0xf1060000-0xf1060fff,
     4 KiB-pages 0xf1070000-0xf107ffff are all zeroes, probably due to
     non-secure mode?

Same as region 1.

Quoted 3 lines

Region 2 already had a 128 KiB size before, which allowed to use 8 KiB at 0xf104f000.

No. This region (GICH) only needs the first 256 bytes or so. The rest is either RAZ/WI or useless stuff.

Quoted 4 lines

An 8 KiB size for regions 1 and 3 indeed didn't make much sense, as this covered two identical (aliased) 4 KiB pages, instead of two different pages at offset 0xf000.

Quoted 3 lines

While we're at it, adding a pointer to the documentation (GIC400 and SBSA) would be tremendously useful, as it'd avoid misinterpreting the various bits.

If anybody could give a short description I could copy & paste into the commit message that would be quite helpful :)