arm64: dts: Fix GIC reg sizes for APM X-Gene
PATCH 1/1 arm64: dts: Fix GIC reg sizes for APM X-Gene
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
--- arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/arm64/boot/dts/apm/apm-storm.dtsi +4 -4
@@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x78010000 0x0 0x1000>, /* GIC Dist */ - <0x0 0x78020000 0x0 0x1000>, /* GIC CPU */ - <0x0 0x78040000 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x78060000 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x78010000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x78020000 0x0 0x20000>, /* GIC CPU */ + <0x0 0x78040000 0x0 0x10000>, /* GIC VCPU Control */ + <0x0 0x78060000 0x0 0x20000>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ };
--
1.7.9.5
PATCH 1/1 arm64: dts: Fix GIC reg sizes for APM X-Gene
Quoted 28 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
CC linux-arm-kernel at lists.infradead.orgCC kvmarm at lists.cs.columbia.eduCC arnd at arndb.deCC marc.zyngier at arm.comCC christoffer.dall at linaro.orgCC jcm at redhat.comSigned-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Tushar Jagad <[email protected]>Signed-off-by Feng Kan <[email protected]>--- arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm64/boot/dts/apm/apm-storm.dtsi +0
-0@@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller;
arch/arm64/boot/dts/apm/apm-storm.dtsi line 89
Quoted 10 lines
arch/arm64/boot/dts/apm/apm-storm.dtsi +4
-4- reg = <0x0 0x78010000 0x0 0x1000>, /* GIC Dist */ - <0x0 0x78020000 0x0 0x1000>, /* GIC CPU */ - <0x0 0x78040000 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x78060000 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x78010000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x78020000 0x0 0x20000>, /* GIC CPU */ + <0x0 0x78040000 0x0 0x10000>, /* GIC VCPU Control */ + <0x0 0x78060000 0x0 0x20000>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ };
Thanks. I confirm that we have tested this.
Jon.
PATCH 1/1 arm64: dts: Fix GIC reg sizes for APM X-Gene
Hi,
Quoted 31 lines · Jon Masters
Quoted 30 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
CC linux-arm-kernel at lists.infradead.orgCC kvmarm at lists.cs.columbia.eduCC arnd at arndb.deCC marc.zyngier at arm.comCC christoffer.dall at linaro.orgCC jcm at redhat.comSigned-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Tushar Jagad <[email protected]>Signed-off-by Feng Kan <[email protected]>--- arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm64/boot/dts/apm/apm-storm.dtsi +0
-2@@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x78010000 0x0 0x1000>, /* GIC Dist */ - <0x0 0x78020000 0x0 0x1000>, /* GIC CPU */
Quoted 8 lines
arch/arm64/boot/dts/apm/apm-storm.dtsi line 89
Quoted 8 lines
arch/arm64/boot/dts/apm/apm-storm.dtsi +4
-2- <0x0 0x78040000 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x78060000 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x78010000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x78020000 0x0 0x20000>, /* GIC CPU */ + <0x0 0x78040000 0x0 0x10000>, /* GIC VCPU Control */ + <0x0 0x78060000 0x0 0x20000>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ };
Any comments on this patch ?
Quoted 6 lines
Thanks. I confirm that we have tested this.
Jon.
Thanks,
Pranav
Quoted 40 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Tushar Jagad <[email protected]>Signed-off-by Feng Kan <[email protected]>--- arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm64/boot/dts/apm/apm-storm.dtsi +4
-5@@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x78010000 0x0 0x1000>, /* GIC Dist */ - <0x0 0x78020000 0x0 0x1000>, /* GIC CPU */ - <0x0 0x78040000 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x78060000 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x78010000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x78020000 0x0 0x20000>, /* GIC CPU */ + <0x0 0x78040000 0x0 0x10000>, /* GIC VCPU Control */ + <0x0 0x78060000 0x0 0x20000>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ }; --
Quoted 2 lines
1.7.9.5
This looks good to me and works with 4K and 64K pages on the mustang I have at hand.
A message that is not in the archive (sent off-list or not yet received): 1422342206-4750-1-git-send-email-psawargaonkar-qTEPVZfXA3Y@public.gmane.org
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
Rob
Quoted 37 lines
CC linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.orgSigned-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Tushar Jagad <[email protected]>Signed-off-by Feng Kan <[email protected]>--- arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm64/boot/dts/apm/apm-storm.dtsi +4
-5@@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x78010000 0x0 0x1000>, /* GIC Dist */ - <0x0 0x78020000 0x0 0x1000>, /* GIC CPU */ - <0x0 0x78040000 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x78060000 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x78010000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x78020000 0x0 0x20000>, /* GIC CPU */ + <0x0 0x78040000 0x0 0x10000>, /* GIC VCPU Control */ + <0x0 0x78060000 0x0 0x20000>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ }; --1.7.9.5
Quoted 4 lines
_______________________________________________
linux-arm-kernel mailing list linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): CAL_JsqJQcuX2cp50oHod-QAbhdMg48TaRP+gLGEO2kbFnQ3B+A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 15 lines · Rob Herring
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
Quoted 4 lines
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
How else should this be enforced?
-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 16 lines · Christoffer Dall
Quoted 15 lines · Rob Herring
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
Quoted 8 lines
Quoted 4 lines
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
Good point.
Quoted 1 line
How else should this be enforced?
Rely on correct h/w design? You'll have to repeat this every time you want to do pass-thru of a device.
What do you do if 64K mapping is not supported? Fallback to emulation of the CPU interface?
Are there other DTSs that need to be fixed?
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): CAL_JsqLcBOC+AnVe7oATjg2g6Fz2vqwacu8QzS4tXMaxxOP_Xg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 7 lines · Rob Herring
Quoted 4 lines · Christoffer Dall
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
Quoted 6 lines
Good point.
Quoted 1 line
How else should this be enforced?
Rely on correct h/w design? You'll have to repeat this every time you want to do pass-thru of a device.
The idea is that the device tree lets us describe whether the hardware design is correct or not :-)
Quoted 2 lines
What do you do if 64K mapping is not supported? Fallback to emulation of the CPU interface?
In this case KVM can't be supported -- use 4K host pages with this hardware, or don't use KVM...
-- PMM
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 29 lines · Rob Herring
Quoted 16 lines · Christoffer Dall
Quoted 15 lines · Rob Herring
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
Quoted 8 lines
Quoted 4 lines
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
Good point.
Quoted 7 lines
Quoted 1 line
How else should this be enforced?
Rely on correct h/w design? You'll have to repeat this every time you want to do pass-thru of a device.
What do you do if 64K mapping is not supported? Fallback to emulation of the CPU interface?
Agree with Peter on these two points.
Quoted 3 lines
Are there other DTSs that need to be fixed?
Not sure really, AMD Seattle works with 64K pages IIRC.
Thanks,
-Christoffer
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Sorry about top post. Quick comment in reply to Seattle point below - yes, it does indeed support 64K and already has the right DTS.
Signature · 1 line
Computer Architect | Sent from my #ARM Powered Mobile Device
Quoted 32 lines · Christoffer Dall
Quoted 1 line · Rob Herring
On Thu, Feb 1On Sat, Feb 21, 2015 at 03:56:17PM -0600, Rob Herring wrote:
Quoted 16 lines · Christoffer Dall
Quoted 15 lines · Rob Herring
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
Quoted 8 lines
Quoted 4 lines
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
Good point.
Quoted 7 lines
Quoted 1 line
How else should this be enforced?
Rely on correct h/w design? You'll have to repeat this every time you want to do pass-thru of a device.
What do you do if 64K mapping is not supported? Fallback to emulation of the CPU interface?
Agree with Peter on these two points.
Quoted 3 lines
Are there other DTSs that need to be fixed?
Not sure really, AMD Seattle works with 64K pages IIRC.
Thanks,
-Christoffer
Quoted 38 lines · Christoffer Dall
Quoted 29 lines · Rob Herring
Quoted 16 lines · Christoffer Dall
Quoted 15 lines · Rob Herring
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
Quoted 8 lines
Quoted 4 lines
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
Good point.
Quoted 7 lines
Quoted 1 line
How else should this be enforced?
Rely on correct h/w design? You'll have to repeat this every time you want to do pass-thru of a device.
What do you do if 64K mapping is not supported? Fallback to emulation of the CPU interface?
Quoted 6 lines
Agree with Peter on these two points.
Quoted 3 lines
Are there other DTSs that need to be fixed?
Not sure really, AMD Seattle works with 64K pages IIRC.
Well, looks we have been inconsistent here:
arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- reg = <0x00xe1110000 0 0x1000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x00xe112f000 0 0x2000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x00xe1140000 0 0x10000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x00xe1160000 0 0x10000>;
arch/arm64/boot/dts/arm/juno.dts- reg = <0x0 0x2c010000 0 0x1000>,
arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c02f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c04f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c06f000 0 0x2000>;
If we are going to use 64K sizes, can we have some consistency here please. Which ranges really need 64KB sizes? It should only be the VCPU interface. right? Why does XGene need 128K? If XGene is doing address swizzling, then the CPU and VCPU base addresses are wrong. Seattle is also wrong for the VCPU, but no one has noticed because we don't use the DIR register IIRC.
XGene should also add an "arm,gic-400" compatible string or something XGene specific if in fact it is not GIC-400.
I think perhaps we need a specific compatible property to indicate a GIC-400 with address swizzling. While we could get away with using the aliased addresses, that seems to be hard to get right and we may regret not doing it in the long term. It would indicate at least it is 64K page safe for example.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rob,
Quoted 70 lines · Rob Herring
Quoted 38 lines · Christoffer Dall
Quoted 29 lines · Rob Herring
Quoted 16 lines · Christoffer Dall
Quoted 15 lines · Rob Herring
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
Quoted 8 lines
Quoted 4 lines
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
Good point.
Quoted 7 lines
Quoted 1 line
How else should this be enforced?
Rely on correct h/w design? You'll have to repeat this every time you want to do pass-thru of a device.
What do you do if 64K mapping is not supported? Fallback to emulation of the CPU interface?
Quoted 6 lines
Agree with Peter on these two points.
Quoted 3 lines
Are there other DTSs that need to be fixed?
Not sure really, AMD Seattle works with 64K pages IIRC.
Well, looks we have been inconsistent here:
arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-reg = <0x00xe111000000x1000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe112f00000x2000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe114000000x10000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe116000000x10000>;
arch/arm64/boot/dts/arm/juno.dts-reg = <0x00x2c01000000x1000>,arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c02f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c04f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c06f000 0 0x2000>;If we are going to use 64K sizes, can we have some consistency here please. Which ranges really need 64KB sizes? It should only be the VCPU interface. right? Why does XGene need 128K? If XGene is doing address swizzling, then the CPU and VCPU base addresses are wrong. Seattle is also wrong for the VCPU, but no one has noticed because we don't use the DIR register IIRC.
Quoted 2 lines
XGene should also add an "arm,gic-400" compatible string or something XGene specific if in fact it is not GIC-400.
X-Gene has gic-400 as an interrupt controller.
Only thing is GIC pages are mapped at 64K boundary (with 64K page size) Hence CPU, VCPU interfaces has a size of 128K (2GIC pages) Regarding GICC_DIR, yes there is a problem which needs to be solved since the first page size is 64K.
In XEN we already have a small fix to access GICC_DIR with 64K page offset instead of standard 4K.
I remember a small discussion in this regard in past
(http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266468.html)
which was deferred at that time.
Once this patch is accepted we can post RFC patch to address GICC_DIR and discuss further.
Quoted 8 lines
I think perhaps we need a specific compatible property to indicate a GIC-400 with address swizzling. While we could get away with using the aliased addresses, that seems to be hard to get right and we may regret not doing it in the long term. It would indicate at least it is 64K page safe for example.
Rob
Thanks,
Pranav
A message that is not in the archive (sent off-list or not yet received): CANFfpkQF-8Kzq-UoP=xLpkTafGn6ScyiEb6oCs-Lxygb+ummLA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 76 lines · Pranavkumar Sawargaonkar
Hi Rob,
Quoted 70 lines · Rob Herring
Quoted 38 lines · Christoffer Dall
Quoted 29 lines · Rob Herring
Quoted 16 lines · Christoffer Dall
Quoted 15 lines · Rob Herring
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
Quoted 8 lines
Quoted 4 lines
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
Good point.
Quoted 7 lines
Quoted 1 line
How else should this be enforced?
Rely on correct h/w design? You'll have to repeat this every time you want to do pass-thru of a device.
What do you do if 64K mapping is not supported? Fallback to emulation of the CPU interface?
Quoted 6 lines
Agree with Peter on these two points.
Quoted 3 lines
Are there other DTSs that need to be fixed?
Not sure really, AMD Seattle works with 64K pages IIRC.
Well, looks we have been inconsistent here:
arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-reg = <0x00xe111000000x1000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe112f00000x2000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe114000000x10000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe116000000x10000>;
arch/arm64/boot/dts/arm/juno.dts-reg = <0x00x2c01000000x1000>,arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c02f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c04f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c06f000 0 0x2000>;If we are going to use 64K sizes, can we have some consistency here please. Which ranges really need 64KB sizes? It should only be the VCPU interface. right? Why does XGene need 128K? If XGene is doing address swizzling, then the CPU and VCPU base addresses are wrong. Seattle is also wrong for the VCPU, but no one has noticed because we don't use the DIR register IIRC.
Quoted 2 lines
XGene should also add an "arm,gic-400" compatible string or something XGene specific if in fact it is not GIC-400.
Quoted 7 lines
X-Gene has gic-400 as an interrupt controller.
Only thing is GIC pages are mapped at 64K boundary (with 64K page size) Hence CPU, VCPU interfaces has a size of 128K (2GIC pages) RegardingGICC_DIR, yes there is a problem which needs to be solved since the first page size is 64K.
In XEN we already have a small fix to accessGICC_DIRwith 64K page offset instead of standard 4K.
Right, and in order for this to work, you should use the last 4K alias for the cpu interface(s). This is why other platforms use xxxf000 as their cpu interface base.
It is of course possible that xgene does not properly do the address swizzling and therefore you have to use 64K aligned addresses. But in that case you need a unique compatible string.
Quoted 5 lines
I remember a small discussion in this regard in past
(http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266468.html)
which was deferred at that time.
Once this patch is accepted we can post RFC patch to addressGICC_DIRand discuss further.
No, let's get this right now and not keep changing the dts.
Rob
Quoted 8 lines
Quoted 8 lines
I think perhaps we need a specific compatible property to indicate a GIC-400 with address swizzling. While we could get away with using the aliased addresses, that seems to be hard to get right and we may regret not doing it in the long term. It would indicate at least it is 64K page safe for example.
Quoted 4 lines
Quoted 1 line
Rob
Thanks,
Pranav
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): CAL_JsqLs4HdT6N=Vb4s--x3ugXKbWYQ5R2WGbiWFhhnYxnK-xw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Hi Rob,
Quoted 94 lines · Rob Herring
Quoted 76 lines · Pranavkumar Sawargaonkar
Hi Rob,
Quoted 70 lines · Rob Herring
Quoted 38 lines · Christoffer Dall
Quoted 29 lines · Rob Herring
Quoted 16 lines · Christoffer Dall
Quoted 15 lines · Rob Herring
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
Quoted 8 lines
Quoted 4 lines
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
Good point.
Quoted 7 lines
Quoted 1 line
How else should this be enforced?
Rely on correct h/w design? You'll have to repeat this every time you want to do pass-thru of a device.
What do you do if 64K mapping is not supported? Fallback to emulation of the CPU interface?
Quoted 6 lines
Agree with Peter on these two points.
Quoted 3 lines
Are there other DTSs that need to be fixed?
Not sure really, AMD Seattle works with 64K pages IIRC.
Well, looks we have been inconsistent here:
arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-reg = <0x00xe111000000x1000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe112f00000x2000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe114000000x10000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe116000000x10000>;
arch/arm64/boot/dts/arm/juno.dts-reg = <0x00x2c01000000x1000>,arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c02f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c04f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c06f000 0 0x2000>;If we are going to use 64K sizes, can we have some consistency here please. Which ranges really need 64KB sizes? It should only be the VCPU interface. right? Why does XGene need 128K? If XGene is doing address swizzling, then the CPU and VCPU base addresses are wrong. Seattle is also wrong for the VCPU, but no one has noticed because we don't use the DIR register IIRC.
Quoted 2 lines
XGene should also add an "arm,gic-400" compatible string or something XGene specific if in fact it is not GIC-400.
Quoted 7 lines
X-Gene has gic-400 as an interrupt controller.
Only thing is GIC pages are mapped at 64K boundary (with 64K page size) Hence CPU, VCPU interfaces has a size of 128K (2GIC pages) RegardingGICC_DIR, yes there is a problem which needs to be solved since the first page size is 64K.
In XEN we already have a small fix to accessGICC_DIRwith 64K page offset instead of standard 4K.Right, and in order for this to work, you should use the last 4K alias for the cpu interface(s). This is why other platforms use xxxf000 as their cpu interface base.
It is of course possible that xgene does not properly do the address swizzling and therefore you have to use 64K aligned addresses. But in that case you need a unique compatible string.
Quoted 8 lines
Quoted 5 lines
I remember a small discussion in this regard in past
(http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266468.html)
which was deferred at that time.
Once this patch is accepted we can post RFC patch to addressGICC_DIRand discuss further.No, let's get this right now and not keep changing the dts.
So should we add some string specific to apm/xgnene (something like apm,cortex-a15-gic) or specific to 64K GIC page size (arm,cortex-a15-gic-64Kpg) ?
Also till 3.19, I am not sure if any code is accessing GICC_DIR so for now only thing which seems to be needed is a new dt string for 64K gic pages.
Thanks,
Pranav
Quoted 14 lines
Rob
Quoted 12 lines
Quoted 9 lines
I think perhaps we need a specific compatible property to indicate a GIC-400 with address swizzling. While we could get away with using the aliased addresses, that seems to be hard to get right and we may regret not doing it in the long term. It would indicate at least it is 64K page safe for example.
Rob
Thanks,
Pranav
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 76 lines · Pranavkumar Sawargaonkar
Hi Rob,
Quoted 70 lines · Rob Herring
Quoted 38 lines · Christoffer Dall
Quoted 29 lines · Rob Herring
Quoted 16 lines · Christoffer Dall
Quoted 15 lines · Rob Herring
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This does not make sense. The GIC regions are still only 4 or 8KB and the h/w description should reflect that. For implementations using gic-400 and the addressing decode trick, the rest of the register range is also not safe to access given it is multiple mapped. Also, this wastes virtual space, but I guess we don't care on 64-bit.
Quoted 8 lines
Quoted 4 lines
KVM should be fixed to only check base address alignment. Size alignment does not matter (if it does, then you need to fix all register blocks).
It matters if you want to ensure that the 64K page you are assigning to a guest for the GIC virtual CPU interface contains only GIC virtual CPU mappings, and not other random stuff that the guest is not allowed to touch.
Good point.
Quoted 7 lines
Quoted 1 line
How else should this be enforced?
Rely on correct h/w design? You'll have to repeat this every time you want to do pass-thru of a device.
What do you do if 64K mapping is not supported? Fallback to emulation of the CPU interface?
Quoted 6 lines
Agree with Peter on these two points.
Quoted 3 lines
Are there other DTSs that need to be fixed?
Not sure really, AMD Seattle works with 64K pages IIRC.
Well, looks we have been inconsistent here:
arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-reg = <0x00xe111000000x1000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe112f00000x2000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe114000000x10000>,arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi-<0x00xe116000000x10000>;
arch/arm64/boot/dts/arm/juno.dts-reg = <0x00x2c01000000x1000>,arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c02f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c04f000 0 0x2000>, arch/arm64/boot/dts/arm/juno.dts- <0x0 0x2c06f000 0 0x2000>;If we are going to use 64K sizes, can we have some consistency here please. Which ranges really need 64KB sizes? It should only be the VCPU interface. right? Why does XGene need 128K? If XGene is doing address swizzling, then the CPU and VCPU base addresses are wrong. Seattle is also wrong for the VCPU, but no one has noticed because we don't use the DIR register IIRC.
Quoted 2 lines
XGene should also add an "arm,gic-400" compatible string or something XGene specific if in fact it is not GIC-400.
Quoted 12 lines
X-Gene has gic-400 as an interrupt controller.
Only thing is GIC pages are mapped at 64K boundary (with 64K page size) Hence CPU, VCPU interfaces has a size of 128K (2GIC pages) RegardingGICC_DIR, yes there is a problem which needs to be solved since the first page size is 64K.
In XEN we already have a small fix to accessGICC_DIRwith 64K page offset instead of standard 4K.
I remember a small discussion in this regard in past
(http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/266468.html)
which was deferred at that time.
Once this patch is accepted we can post RFC patch to addressGICC_DIRand discuss further.
I've had a look at my X-Gene board, and noticed the following thing:
Mustang# md.l 0x78020000 0x10
78020000: 000001e7 00000080 00000002 000003ff ................ 78020010: 00000000 000000ff 000003ff 00000003 ................ 78020020: 000003ff 00000000 000003ff 00000000 ................ 78020030: 00000000 00000000 00000000 00000000 ................ Mustang# md.l 0x7802f000 0x10 7802f000: 000001e7 00000080 00000002 000003ff ................ 7802f010: 00000000 000000ff 000003ff 00000003 ................ 7802f020: 000003ff 00000000 000003ff 00000000 ................ 7802f030: 00000000 00000000 00000000 00000000 ................
Notice a pattern? Yes, X-Gene is aliasing the GICC region over the 64k page, repeated 16 times, as it should.
So either the DT is fixed to show GICC and GICV at offset 0xf000 (and we don't support 64k pages with it), or we extend the binding to present an offset inside the 128K region.
My preference goes of course to the latter, because for once we seem to have something designed as expected.
Rob, what do you think?
M.
Signature · 5 lines
Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This patch may be correct, but it is useless. The firmware on my APM system (some version of u-boot) repaints the DT at boot time, negating the effect of this patch.
Another system I can remove from my 64k-capable list.
M.
Quoted 36 lines
CC linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.orgSigned-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Tushar Jagad <[email protected]>Signed-off-by Feng Kan <[email protected]>--- arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm64/boot/dts/apm/apm-storm.dtsi +4
-5@@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x78010000 0x0 0x1000>, /* GIC Dist */ - <0x0 0x78020000 0x0 0x1000>, /* GIC CPU */ - <0x0 0x78040000 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x78060000 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x78010000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x78020000 0x0 0x20000>, /* GIC CPU */ + <0x0 0x78040000 0x0 0x10000>, /* GIC VCPU Control */ + <0x0 0x78060000 0x0 0x20000>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ }; --1.7.9.5
Signature · 5 lines
Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 11 lines · Marc Zyngier
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This patch may be correct, but it is useless. The firmware on my APM system (some version of u-boot) repaints the DT at boot time, negating the effect of this patch.
We have updated u-boot to reflect this change. I can supply you with a updated image if you wish.
Quoted 37 lines
Another system I can remove from my 64k-capable list.
M.Quoted 32 lines
CC linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.orgSigned-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Tushar Jagad <[email protected]>Signed-off-by Feng Kan <[email protected]>--- arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm64/boot/dts/apm/apm-storm.dtsi +4
-4@@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x78010000 0x0 0x1000>, /* GIC Dist */ - <0x0 0x78020000 0x0 0x1000>, /* GIC CPU */ - <0x0 0x78040000 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x78060000 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x78010000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x78020000 0x0 0x20000>, /* GIC CPU */ + <0x0 0x78040000 0x0 0x10000>, /* GIC VCPU Control */ + <0x0 0x78060000 0x0 0x20000>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ };
Quoted 8 lines
Quoted 4 lines
--
1.7.9.5--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): CAL85gmCuB4LfNp+6B8cL9+emFqWPM6W9gevTzibyaAzL+7jVdg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 7 lines · Feng Kan
Quoted 6 lines · Marc Zyngier
Quoted 5 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
This patch corrects the sizes to be inline with the hardware spec.
This patch may be correct, but it is useless. The firmware on my APM system (some version of u-boot) repaints the DT at boot time, negating the effect of this patch.
We have updated u-boot to reflect this change. I can supply you with a updated image if you wish.
That would be useful, thanks.
But more importantly, why bother upstreaming your DT into the kernel tree if your firmware is going to overwrite whatever we provide?
Either the firmware let the user provide its own DT (and doesn't touch it other than to change the CPU enable method, insert a /memreserve/ or similar things), or the firmware always provide its own DT, and doesn't let the user provide its own. Corrupting the user DT is a disaster, as we just found.
Thanks,
M.
Quoted 46 lines
Quoted 45 lines
Another system I can remove from my 64k-capable list.
M.Quoted 33 lines
CC linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.orgSigned-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Tushar Jagad <[email protected]>Signed-off-by Feng Kan <[email protected]>--- arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm64/boot/dts/apm/apm-storm.dtsi +4
-5@@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x78010000 0x0 0x1000>, /* GIC Dist */ - <0x0 0x78020000 0x0 0x1000>, /* GIC CPU */ - <0x0 0x78040000 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x78060000 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x78010000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x78020000 0x0 0x20000>, /* GIC CPU */ + <0x0 0x78040000 0x0 0x10000>, /* GIC VCPU Control */ + <0x0 0x78060000 0x0 0x20000>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ }; --Quoted 3 lines
1.7.9.5
--
Jazz is not dead. It just smells funny...
Signature · 5 lines
Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 13 lines · Marc Zyngier
Quoted 12 lines · Feng Kan
Quoted 11 lines · Marc Zyngier
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This patch may be correct, but it is useless. The firmware on my APM system (some version of u-boot) repaints the DT at boot time, negating the effect of this patch.
Quoted 7 lines
Quoted 2 lines
We have updated u-boot to reflect this change. I can supply you with a updated image if you wish.
That would be useful, thanks.
But more importantly, why bother upstreaming your DT into the kernel tree if your firmware is going to overwrite whatever we provide?
We did tried to submit a version upstream but was rejected.
Quoted 6 lines
Either the firmware let the user provide its own DT (and doesn't touch it other than to change the CPU enable method, insert a /memreserve/ or similar things), or the firmware always provide its own DT, and doesn't let the user provide its own. Corrupting the user DT is a disaster, as we just found.
Yes, the intent of the change is listed in the link below. It is not a justification by any means,
just the effects of things appearing in layers.
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/288574.html
Quoted 55 lines
Thanks,
M.Quoted 37 lines
Quoted 37 lines
Another system I can remove from my 64k-capable list.
M.Quoted 32 lines
Signed-off-by Pranavkumar Sawargaonkar <[email protected]>Signed-off-by Tushar Jagad <[email protected]>Signed-off-by Feng Kan <[email protected]>--- arch/arm64/boot/dts/apm/apm-storm.dtsi | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)arch/arm64/boot/dts/apm/apm-storm.dtsi +4
-4@@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x78010000 0x0 0x1000>, /* GIC Dist */ - <0x0 0x78020000 0x0 0x1000>, /* GIC CPU */ - <0x0 0x78040000 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x78060000 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x78010000 0x0 0x10000>, /* GIC Dist */ + <0x0 0x78020000 0x0 0x20000>, /* GIC CPU */ + <0x0 0x78040000 0x0 0x10000>, /* GIC VCPU Control */ + <0x0 0x78060000 0x0 0x20000>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ };Quoted 9 lines
Quoted 8 lines
Quoted 4 lines
--
1.7.9.5--
Jazz is not dead. It just smells funny...--
Jazz is not dead. It just smells funny...
A message that is not in the archive (sent off-list or not yet received): CAL85gmDVfop1_roHyLTRzFY3BahzTWs7nwbpmZD7emFKcFHyLg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 29 lines · Feng Kan
Quoted 13 lines · Marc Zyngier
Quoted 12 lines · Feng Kan
Quoted 11 lines · Marc Zyngier
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This patch may be correct, but it is useless. The firmware on my APM system (some version of u-boot) repaints the DT at boot time, negating the effect of this patch.
Quoted 7 lines
Quoted 2 lines
We have updated u-boot to reflect this change. I can supply you with a updated image if you wish.
That would be useful, thanks.
But more importantly, why bother upstreaming your DT into the kernel tree if your firmware is going to overwrite whatever we provide?
We did tried to submit a version upstream but was rejected.
Quoted 6 lines
Either the firmware let the user provide its own DT (and doesn't touch it other than to change the CPU enable method, insert a /memreserve/ or similar things), or the firmware always provide its own DT, and doesn't let the user provide its own. Corrupting the user DT is a disaster, as we just found.
Quoted 4 lines
Yes, the intent of the change is listed in the link below. It is not a justification by any means,
just the effects of things appearing in layers.
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/288574.html
Yeah. This is as wrong as it can possibly be. Oh well...
Thanks,
M.
Signature · 5 lines
Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi Marc,
Quoted 30 lines · Marc Zyngier
Quoted 29 lines · Feng Kan
Quoted 13 lines · Marc Zyngier
Quoted 12 lines · Feng Kan
Quoted 11 lines · Marc Zyngier
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This patch may be correct, but it is useless. The firmware on my APM system (some version of u-boot) repaints the DT at boot time, negating the effect of this patch.
Quoted 7 lines
Quoted 2 lines
We have updated u-boot to reflect this change. I can supply you with a updated image if you wish.
That would be useful, thanks.
But more importantly, why bother upstreaming your DT into the kernel tree if your firmware is going to overwrite whatever we provide?
We did tried to submit a version upstream but was rejected.
Quoted 6 lines
Either the firmware let the user provide its own DT (and doesn't touch it other than to change the CPU enable method, insert a /memreserve/ or similar things), or the firmware always provide its own DT, and doesn't let the user provide its own. Corrupting the user DT is a disaster, as we just found.
Quoted 6 lines
Quoted 4 lines
Yes, the intent of the change is listed in the link below. It is not a justification by any means,
just the effects of things appearing in layers.
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/288574.htmlYeah. This is as wrong as it can possibly be. Oh well...
Yes there is an issue with u-boot patching the dt for end user who wants his DT to be used, for this we can (in fact we should) provide an option in u-boot (may be setting some environment variable) which will make sure end user's DT does not get modified (apart from standard things like patching mac-addresses) by u-boot.
Another point I want to reopen here is the how to handle 64K GIC page size pointed out in this thread, what would be the best way to tackle this (adding a new DT string or any other way) ?
Thanks,
Pranav
Quoted 6 lines
Thanks,
M.--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): CANFfpkQ1QyWEGwwQx1g1By=uXvctJjF8AOO+uA5CFUgO4v9DFg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
[adding RobH to the CC list, as he was commenting on the subject earlier]
Hi Pranav,
Quoted 38 lines · Pranavkumar Sawargaonkar
Hi Marc,
Quoted 35 lines · Marc Zyngier
Quoted 33 lines · Feng Kan
Quoted 13 lines · Marc Zyngier
Quoted 12 lines · Feng Kan
Quoted 11 lines · Marc Zyngier
Quoted 6 lines · Pranavkumar Sawargaonkar
In APM X-Gene, GIC register space is 64K aligned while the sizes mentioned in the dt are 4K aligned. This breaks KVM when kernel is built with 64K page size due to size alignment checking in vgic driver for VCPU Control and VCPU register.
This patch corrects the sizes to be inline with the hardware spec.
This patch may be correct, but it is useless. The firmware on my APM system (some version of u-boot) repaints the DT at boot time, negating the effect of this patch.
Quoted 7 lines
Quoted 2 lines
We have updated u-boot to reflect this change. I can supply you with a updated image if you wish.
That would be useful, thanks.
But more importantly, why bother upstreaming your DT into the kernel tree if your firmware is going to overwrite whatever we provide?
We did tried to submit a version upstream but was rejected.
Quoted 6 lines
Either the firmware let the user provide its own DT (and doesn't touch it other than to change the CPU enable method, insert a /memreserve/ or similar things), or the firmware always provide its own DT, and doesn't let the user provide its own. Corrupting the user DT is a disaster, as we just found.
Yes, the intent of the change is listed in the link below. It is not a justification by any means,
just the effects of things appearing in layers.
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/288574.html
Quoted 7 lines
Quoted 1 line
Yeah. This is as wrong as it can possibly be. Oh well...
Yes there is an issue with u-boot patching the dt for end user who wants his DT to be used, for this we can (in fact we should) provide an option in u-boot (may be setting some environment variable) which will make sure end user's DT does not get modified (apart from standard things like patching mac-addresses) by u-boot.
Definitely. I would even argue that not overwriting the DT should be the default behaviour, and that you could have a compatibility mode that repaint ancient DTs if you want to. But at least having something would be good. At the moment, my X-Gene board is screwed, as I don't have any way to fix the bootloader (or even to download a binary).
Quoted 3 lines
Another point I want to reopen here is the how to handle 64K GIC page size pointed out in this thread, what would be the best way to tackle this (adding a new DT string or any other way) ?
The main problem is that there is several flavours of brokenness:
- GICC_CTLR@0, GICC_DIR@0x10000, size 128kB (X-Gene)
- GICC_CTLR@0, GICC_DIR@0x1000, size 64kB (Seattle)
- GICC_CTL@0xF000, GICC_DIR@0x10000, size 8kB (Juno)
- GICC_CTL@0, GICC_DIR@0x1000, size 8kB (HiKey)
Yes, they all have a GIC400, and yet they are all irritatingly non compliant with SBSA. As far as I can tell, nobody has correctly implemented the expected aliasing that would have made it work.
So either we add new compatibility strings describing all the possible way people can break things, or we introduce something like dir-offset and ctlr-offset that would tell the driver where the two sub-regions are placed. The default values would be:
- When size is < 8kB -> invalid configuration, this is not a GIC400.
- When size is = 8kB -> ctlr-offset = 0, dir-offset = 0x1000
- When size is = 128kB -> ctlr-offset = 0xf000, dir-offset = 0x10000
For the two first braindead systems above:
- ctlr-offset = 0, dir-offset = 0x10000 (X-Gene)
- ctlr-offset = 0, dir-offset = 0x1000; (Seattle)
and that's just enough to get bare metal going.
When it comes to virtualization, this is hell:
- We need an API to be able to expose these various offsets to userspace, so that QEMU/kvmtool can place the GICV region at the right location (offset within a page).
- We will also miss the capability to trap GICV_DIR independently from the rest of the VCPU interface on systems like Seattle, which is rather bad.
- Systems that look like Juno or HiKey cannot use virtualization with 64k pages, end of story.
After writing this, I'm feeling slightly depressed...
M.
Signature · 5 lines
Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html