Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On 24/02/15 06:34, Pranavkumar Sawargaonkar wrote: > Hi Rob, > > On Mon, Feb 23, 2015 at 10:09 PM, Rob Herring wrote: >> On Mon, Feb 23, 2015 at 6:07 AM, Christoffer Dall >> wrote: >>> On Sat, Feb 21, 2015 at 03:56:17PM -0600, Rob Herring wrote: On Thu, Feb 19, 2015 at 1:03 PM, Christoffer Dall wrote: > On Thu, Feb 19, 2015 at 12:23:15PM -0600, Rob Herring wrote: >> On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar >> wrote: >>> 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). >> > 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. > 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. >>> 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 = <0x0 >> 0xe111 0 0x1000>, >> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 >> 0xe112f000 0 0x2000>, >> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 >> 0xe114 0 0x1>, >> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 >> 0xe116 0 0x1>; >> >> arch/arm64/boot/dts/arm/juno.dts- reg = <0x0 0x2c01 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. > > 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. I've had a look at my X-Gene board, and noticed the following thing: Mustang# md.l 0x7802 0x10 7802: 01e7 0080 0002 03ff 78020010: 00ff 03ff 0003 78020020: 03ff 03ff 78020030: Mustang# md.l 0x7802f000 0x10 7802f000: 01e7 0080 0002 03ff 7802f010: 00ff 03ff 0003 7802f020: 03ff 03ff 7802f030: 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
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
[adding RobH to the CC list, as he was commenting on the subject earlier] Hi Pranav, On 12/03/15 03:52, Pranavkumar Sawargaonkar wrote: > Hi Marc, > > On Wed, Mar 11, 2015 at 11:47 PM, Marc Zyngier wrote: >> On 11/03/15 17:57, Feng Kan wrote: >>> On Wed, Mar 11, 2015 at 10:31 AM, Marc Zyngier wrote: On 11/03/15 17:19, Feng Kan wrote: > On Wed, Mar 11, 2015 at 7:53 AM, Marc Zyngier > wrote: >> On 27/01/15 07:03, Pranavkumar Sawargaonkar wrote: >>> 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. 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. >>> 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 >> >> 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). > 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@0x1, size 128kB (X-Gene) - GICC_CTLR@0, GICC_DIR@0x1000, size 64kB (Seattle) - GICC_CTL@0xF000, GICC_DIR@0x1, 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 = 0x1 For the two first braindead systems above: - ctlr-offset = 0, dir-offset = 0x1 (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. -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
Hi Marc, On Wed, Mar 11, 2015 at 11:47 PM, Marc Zyngier wrote: > On 11/03/15 17:57, Feng Kan wrote: >> On Wed, Mar 11, 2015 at 10:31 AM, Marc Zyngier wrote: >>> On 11/03/15 17:19, Feng Kan wrote: On Wed, Mar 11, 2015 at 7:53 AM, Marc Zyngier wrote: > On 27/01/15 07:03, Pranavkumar Sawargaonkar wrote: >> 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. >>> >>> 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. >> >>> >>> 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 > > 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. 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 > > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On 11/03/15 17:57, Feng Kan wrote: > On Wed, Mar 11, 2015 at 10:31 AM, Marc Zyngier wrote: >> On 11/03/15 17:19, Feng Kan wrote: >>> On Wed, Mar 11, 2015 at 7:53 AM, Marc Zyngier wrote: On 27/01/15 07:03, Pranavkumar Sawargaonkar wrote: > 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. >> >> 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. > >> >> 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 Yeah. This is as wrong as it can possibly be. Oh well... 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On Wed, Mar 11, 2015 at 10:31 AM, Marc Zyngier wrote: > On 11/03/15 17:19, Feng Kan wrote: >> On Wed, Mar 11, 2015 at 7:53 AM, Marc Zyngier wrote: >>> On 27/01/15 07:03, Pranavkumar Sawargaonkar wrote: 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. > > 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. > > 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 > > Thanks, > > M. > >>> >>> Another system I can remove from my 64k-capable list. >>> >>> M. >>> CC: linux-arm-ker...@lists.infradead.org CC: kvm...@lists.cs.columbia.edu CC: a...@arndb.de CC: marc.zyng...@arm.com CC: christoffer.d...@linaro.org CC: j...@redhat.com Signed-off-by: Pranavkumar Sawargaonkar Signed-off-by: Tushar Jagad Signed-off-by: Feng Kan --- arch/arm64/boot/dts/apm/apm-storm.dtsi |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi index f1ad9c2..65f0e6d 100644 --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi @@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x7801 0x0 0x1000>, /* GIC Dist */ - <0x0 0x7802 0x0 0x1000>, /* GIC CPU */ - <0x0 0x7804 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x7806 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x7801 0x0 0x1>, /* GIC Dist */ + <0x0 0x7802 0x0 0x2>, /* GIC CPU */ + <0x0 0x7804 0x0 0x1>, /* GIC VCPU Control */ + <0x0 0x7806 0x0 0x2>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ }; -- 1.7.9.5 >>> >>> >>> -- >>> Jazz is not dead. It just smells funny... >> > > > -- > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On 11/03/15 17:19, Feng Kan wrote: > On Wed, Mar 11, 2015 at 7:53 AM, Marc Zyngier wrote: >> On 27/01/15 07:03, Pranavkumar Sawargaonkar wrote: >>> 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. 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. >> >> Another system I can remove from my 64k-capable list. >> >> M. >> >>> CC: linux-arm-ker...@lists.infradead.org >>> CC: kvm...@lists.cs.columbia.edu >>> CC: a...@arndb.de >>> CC: marc.zyng...@arm.com >>> CC: christoffer.d...@linaro.org >>> CC: j...@redhat.com >>> Signed-off-by: Pranavkumar Sawargaonkar >>> Signed-off-by: Tushar Jagad >>> Signed-off-by: Feng Kan >>> --- >>> arch/arm64/boot/dts/apm/apm-storm.dtsi |8 >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi >>> b/arch/arm64/boot/dts/apm/apm-storm.dtsi >>> index f1ad9c2..65f0e6d 100644 >>> --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi >>> +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi >>> @@ -81,10 +81,10 @@ >>> compatible = "arm,cortex-a15-gic"; >>> #interrupt-cells = <3>; >>> interrupt-controller; >>> - reg = <0x0 0x7801 0x0 0x1000>, /* GIC Dist */ >>> - <0x0 0x7802 0x0 0x1000>, /* GIC CPU */ >>> - <0x0 0x7804 0x0 0x2000>, /* GIC VCPU Control */ >>> - <0x0 0x7806 0x0 0x2000>; /* GIC VCPU */ >>> + reg = <0x0 0x7801 0x0 0x1>, /* GIC Dist */ >>> + <0x0 0x7802 0x0 0x2>, /* GIC CPU */ >>> + <0x0 0x7804 0x0 0x1>, /* GIC VCPU Control */ >>> + <0x0 0x7806 0x0 0x2>; /* GIC VCPU */ >>> interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ >>> }; >>> >>> -- >>> 1.7.9.5 >>> >>> >> >> >> -- >> Jazz is not dead. It just smells funny... > -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On Wed, Mar 11, 2015 at 7:53 AM, Marc Zyngier wrote: > On 27/01/15 07:03, Pranavkumar Sawargaonkar wrote: >> 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. > > Another system I can remove from my 64k-capable list. > > M. > >> CC: linux-arm-ker...@lists.infradead.org >> CC: kvm...@lists.cs.columbia.edu >> CC: a...@arndb.de >> CC: marc.zyng...@arm.com >> CC: christoffer.d...@linaro.org >> CC: j...@redhat.com >> Signed-off-by: Pranavkumar Sawargaonkar >> Signed-off-by: Tushar Jagad >> Signed-off-by: Feng Kan >> --- >> arch/arm64/boot/dts/apm/apm-storm.dtsi |8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi >> b/arch/arm64/boot/dts/apm/apm-storm.dtsi >> index f1ad9c2..65f0e6d 100644 >> --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi >> +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi >> @@ -81,10 +81,10 @@ >> compatible = "arm,cortex-a15-gic"; >> #interrupt-cells = <3>; >> interrupt-controller; >> - reg = <0x0 0x7801 0x0 0x1000>, /* GIC Dist */ >> - <0x0 0x7802 0x0 0x1000>, /* GIC CPU */ >> - <0x0 0x7804 0x0 0x2000>, /* GIC VCPU Control */ >> - <0x0 0x7806 0x0 0x2000>; /* GIC VCPU */ >> + reg = <0x0 0x7801 0x0 0x1>, /* GIC Dist */ >> + <0x0 0x7802 0x0 0x2>, /* GIC CPU */ >> + <0x0 0x7804 0x0 0x1>, /* GIC VCPU Control */ >> + <0x0 0x7806 0x0 0x2>; /* GIC VCPU */ >> interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ >> }; >> >> -- >> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On 27/01/15 07:03, Pranavkumar Sawargaonkar wrote: > 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. > CC: linux-arm-ker...@lists.infradead.org > CC: kvm...@lists.cs.columbia.edu > CC: a...@arndb.de > CC: marc.zyng...@arm.com > CC: christoffer.d...@linaro.org > CC: j...@redhat.com > Signed-off-by: Pranavkumar Sawargaonkar > Signed-off-by: Tushar Jagad > Signed-off-by: Feng Kan > --- > arch/arm64/boot/dts/apm/apm-storm.dtsi |8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi > b/arch/arm64/boot/dts/apm/apm-storm.dtsi > index f1ad9c2..65f0e6d 100644 > --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi > +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi > @@ -81,10 +81,10 @@ > compatible = "arm,cortex-a15-gic"; > #interrupt-cells = <3>; > interrupt-controller; > - reg = <0x0 0x7801 0x0 0x1000>, /* GIC Dist */ > - <0x0 0x7802 0x0 0x1000>, /* GIC CPU */ > - <0x0 0x7804 0x0 0x2000>, /* GIC VCPU Control */ > - <0x0 0x7806 0x0 0x2000>; /* GIC VCPU */ > + reg = <0x0 0x7801 0x0 0x1>, /* GIC Dist */ > + <0x0 0x7802 0x0 0x2>, /* GIC CPU */ > + <0x0 0x7804 0x0 0x1>, /* GIC VCPU Control */ > + <0x0 0x7806 0x0 0x2>; /* GIC VCPU */ > interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ > }; > > -- > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
Hi Rob, On Tue, Feb 24, 2015 at 8:00 PM, Rob Herring wrote: > On Tue, Feb 24, 2015 at 12:34 AM, Pranavkumar Sawargaonkar > wrote: >> Hi Rob, >> >> On Mon, Feb 23, 2015 at 10:09 PM, Rob Herring wrote: >>> On Mon, Feb 23, 2015 at 6:07 AM, Christoffer Dall >>> wrote: On Sat, Feb 21, 2015 at 03:56:17PM -0600, Rob Herring wrote: > On Thu, Feb 19, 2015 at 1:03 PM, Christoffer Dall > wrote: > > On Thu, Feb 19, 2015 at 12:23:15PM -0600, Rob Herring wrote: > >> On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar > >> wrote: > >> > 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). > >> > > 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. > > > 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. > > 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 = <0x0 >>> 0xe111 0 0x1000>, >>> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 >>> 0xe112f000 0 0x2000>, >>> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 >>> 0xe114 0 0x1>, >>> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 >>> 0xe116 0 0x1>; >>> >>> arch/arm64/boot/dts/arm/juno.dts- reg = <0x0 0x2c01 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. >> >> 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. > > 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. > >> 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. > > 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
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On Tue, Feb 24, 2015 at 12:34 AM, Pranavkumar Sawargaonkar wrote: > Hi Rob, > > On Mon, Feb 23, 2015 at 10:09 PM, Rob Herring wrote: >> On Mon, Feb 23, 2015 at 6:07 AM, Christoffer Dall >> wrote: >>> On Sat, Feb 21, 2015 at 03:56:17PM -0600, Rob Herring wrote: On Thu, Feb 19, 2015 at 1:03 PM, Christoffer Dall wrote: > On Thu, Feb 19, 2015 at 12:23:15PM -0600, Rob Herring wrote: >> On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar >> wrote: >> > 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). >> > 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. > 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. >>> 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 = <0x0 >> 0xe111 0 0x1000>, >> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 >> 0xe112f000 0 0x2000>, >> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 >> 0xe114 0 0x1>, >> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 >> 0xe116 0 0x1>; >> >> arch/arm64/boot/dts/arm/juno.dts- reg = <0x0 0x2c01 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. > > 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. 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. > 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. No, let's get this right now and not keep changing the dts. Rob > >> >> 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 messa
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
Hi Rob, On Mon, Feb 23, 2015 at 10:09 PM, Rob Herring wrote: > On Mon, Feb 23, 2015 at 6:07 AM, Christoffer Dall > wrote: >> On Sat, Feb 21, 2015 at 03:56:17PM -0600, Rob Herring wrote: >>> On Thu, Feb 19, 2015 at 1:03 PM, Christoffer Dall >>> wrote: >>> > On Thu, Feb 19, 2015 at 12:23:15PM -0600, Rob Herring wrote: >>> >> On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar >>> >> wrote: >>> >> > 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). >>> >> >>> > 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. >>> >>> > 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. >> >>> >>> 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 = <0x0 > 0xe111 0 0x1000>, > arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 > 0xe112f000 0 0x2000>, > arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 > 0xe114 0 0x1>, > arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 > 0xe116 0 0x1>; > > arch/arm64/boot/dts/arm/juno.dts- reg = <0x0 0x2c01 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. 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. > > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On Mon, Feb 23, 2015 at 6:07 AM, Christoffer Dall wrote: > On Sat, Feb 21, 2015 at 03:56:17PM -0600, Rob Herring wrote: >> On Thu, Feb 19, 2015 at 1:03 PM, Christoffer Dall >> wrote: >> > On Thu, Feb 19, 2015 at 12:23:15PM -0600, Rob Herring wrote: >> >> On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar >> >> wrote: >> >> > 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). >> >> >> > 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. >> >> > 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. > >> >> 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 = <0x0 0xe111 0 0x1000>, arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 0xe112f000 0 0x2000>, arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 0xe114 0 0x1>, arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi- <0x0 0xe116 0 0x1>; arch/arm64/boot/dts/arm/juno.dts- reg = <0x0 0x2c01 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
Sorry about top post. Quick comment in reply to Seattle point below - yes, it does indeed support 64K and already has the right DTS. -- Computer Architect | Sent from my #ARM Powered Mobile Device On Feb 23, 2015 4:14 AM, Christoffer Dall wrote: > > On Sat, Feb 21, 2015 at 03:56:17PM -0600, Rob Herring wrote: > > On Thu, Feb 1On Sat, Feb 21, 2015 at 03:56:17PM -0600, Rob Herring wrote: > On Thu, Feb 19, 2015 at 1:03 PM, Christoffer Dall > wrote: > > On Thu, Feb 19, 2015 at 12:23:15PM -0600, Rob Herring wrote: > >> On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar > >> wrote: > >> > 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). > >> > > 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. > > > 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. > > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On Sat, Feb 21, 2015 at 03:56:17PM -0600, Rob Herring wrote: > On Thu, Feb 19, 2015 at 1:03 PM, Christoffer Dall > wrote: > > On Thu, Feb 19, 2015 at 12:23:15PM -0600, Rob Herring wrote: > >> On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar > >> wrote: > >> > 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). > >> > > 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. > > > 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. > > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On 22 February 2015 at 06:56, Rob Herring wrote: > On Thu, Feb 19, 2015 at 1:03 PM, Christoffer Dall > wrote: >> 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. > >> 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 :-) > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On Thu, Feb 19, 2015 at 1:03 PM, Christoffer Dall wrote: > On Thu, Feb 19, 2015 at 12:23:15PM -0600, Rob Herring wrote: >> On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar >> wrote: >> > 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). >> > 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. > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On Thu, Feb 19, 2015 at 12:23:15PM -0600, Rob Herring wrote: > On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar > wrote: > > 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). > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On Tue, Jan 27, 2015 at 1:03 AM, Pranavkumar Sawargaonkar wrote: > 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 > > CC: linux-arm-ker...@lists.infradead.org > CC: kvm...@lists.cs.columbia.edu > CC: a...@arndb.de > CC: marc.zyng...@arm.com > CC: christoffer.d...@linaro.org > CC: j...@redhat.com > Signed-off-by: Pranavkumar Sawargaonkar > Signed-off-by: Tushar Jagad > Signed-off-by: Feng Kan > --- > arch/arm64/boot/dts/apm/apm-storm.dtsi |8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi > b/arch/arm64/boot/dts/apm/apm-storm.dtsi > index f1ad9c2..65f0e6d 100644 > --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi > +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi > @@ -81,10 +81,10 @@ > compatible = "arm,cortex-a15-gic"; > #interrupt-cells = <3>; > interrupt-controller; > - reg = <0x0 0x7801 0x0 0x1000>, /* GIC Dist */ > - <0x0 0x7802 0x0 0x1000>, /* GIC CPU */ > - <0x0 0x7804 0x0 0x2000>, /* GIC VCPU Control */ > - <0x0 0x7806 0x0 0x2000>; /* GIC VCPU */ > + reg = <0x0 0x7801 0x0 0x1>, /* GIC Dist */ > + <0x0 0x7802 0x0 0x2>, /* GIC CPU */ > + <0x0 0x7804 0x0 0x1>, /* GIC VCPU Control */ > + <0x0 0x7806 0x0 0x2>; /* GIC VCPU */ > interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ > }; > > -- > 1.7.9.5 > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On Tue, Jan 27, 2015 at 12:33:26PM +0530, Pranavkumar Sawargaonkar wrote: > 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-ker...@lists.infradead.org > CC: kvm...@lists.cs.columbia.edu > CC: a...@arndb.de > CC: marc.zyng...@arm.com > CC: christoffer.d...@linaro.org > CC: j...@redhat.com > Signed-off-by: Pranavkumar Sawargaonkar > Signed-off-by: Tushar Jagad > Signed-off-by: Feng Kan > --- > arch/arm64/boot/dts/apm/apm-storm.dtsi |8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi > b/arch/arm64/boot/dts/apm/apm-storm.dtsi > index f1ad9c2..65f0e6d 100644 > --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi > +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi > @@ -81,10 +81,10 @@ > compatible = "arm,cortex-a15-gic"; > #interrupt-cells = <3>; > interrupt-controller; > - reg = <0x0 0x7801 0x0 0x1000>, /* GIC Dist */ > - <0x0 0x7802 0x0 0x1000>, /* GIC CPU */ > - <0x0 0x7804 0x0 0x2000>, /* GIC VCPU Control */ > - <0x0 0x7806 0x0 0x2000>; /* GIC VCPU */ > + reg = <0x0 0x7801 0x0 0x1>, /* GIC Dist */ > + <0x0 0x7802 0x0 0x2>, /* GIC CPU */ > + <0x0 0x7804 0x0 0x1>, /* GIC VCPU Control */ > + <0x0 0x7806 0x0 0x2>; /* GIC VCPU */ > interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ > }; > > -- > 1.7.9.5 > This looks good to me and works with 4K and 64K pages on the mustang I have at hand. Acked-by: Christoffer Dall -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
Hi, On Tue, Jan 27, 2015 at 3:02 PM, Jon Masters wrote: > On 01/27/2015 02:03 AM, Pranavkumar Sawargaonkar wrote: >> 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-ker...@lists.infradead.org >> CC: kvm...@lists.cs.columbia.edu >> CC: a...@arndb.de >> CC: marc.zyng...@arm.com >> CC: christoffer.d...@linaro.org >> CC: j...@redhat.com >> Signed-off-by: Pranavkumar Sawargaonkar >> Signed-off-by: Tushar Jagad >> Signed-off-by: Feng Kan >> --- >> arch/arm64/boot/dts/apm/apm-storm.dtsi |8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi >> b/arch/arm64/boot/dts/apm/apm-storm.dtsi >> index f1ad9c2..65f0e6d 100644 >> --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi >> +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi >> @@ -81,10 +81,10 @@ >> compatible = "arm,cortex-a15-gic"; >> #interrupt-cells = <3>; >> interrupt-controller; >> - reg = <0x0 0x7801 0x0 0x1000>, /* GIC Dist */ >> - <0x0 0x7802 0x0 0x1000>, /* GIC CPU */ >> - <0x0 0x7804 0x0 0x2000>, /* GIC VCPU Control */ >> - <0x0 0x7806 0x0 0x2000>; /* GIC VCPU */ >> + reg = <0x0 0x7801 0x0 0x1>, /* GIC Dist */ >> + <0x0 0x7802 0x0 0x2>, /* GIC CPU */ >> + <0x0 0x7804 0x0 0x1>, /* GIC VCPU Control */ >> + <0x0 0x7806 0x0 0x2>; /* GIC VCPU */ >> interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ >> }; Any comments on this patch ? > > Thanks. I confirm that we have tested this. > > Jon. > > Thanks, Pranav -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: dts: Fix GIC reg sizes for APM X-Gene
On 01/27/2015 02:03 AM, Pranavkumar Sawargaonkar wrote: > 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-ker...@lists.infradead.org > CC: kvm...@lists.cs.columbia.edu > CC: a...@arndb.de > CC: marc.zyng...@arm.com > CC: christoffer.d...@linaro.org > CC: j...@redhat.com > Signed-off-by: Pranavkumar Sawargaonkar > Signed-off-by: Tushar Jagad > Signed-off-by: Feng Kan > --- > arch/arm64/boot/dts/apm/apm-storm.dtsi |8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi > b/arch/arm64/boot/dts/apm/apm-storm.dtsi > index f1ad9c2..65f0e6d 100644 > --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi > +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi > @@ -81,10 +81,10 @@ > compatible = "arm,cortex-a15-gic"; > #interrupt-cells = <3>; > interrupt-controller; > - reg = <0x0 0x7801 0x0 0x1000>, /* GIC Dist */ > - <0x0 0x7802 0x0 0x1000>, /* GIC CPU */ > - <0x0 0x7804 0x0 0x2000>, /* GIC VCPU Control */ > - <0x0 0x7806 0x0 0x2000>; /* GIC VCPU */ > + reg = <0x0 0x7801 0x0 0x1>, /* GIC Dist */ > + <0x0 0x7802 0x0 0x2>, /* GIC CPU */ > + <0x0 0x7804 0x0 0x1>, /* GIC VCPU Control */ > + <0x0 0x7806 0x0 0x2>; /* GIC VCPU */ > interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ > }; Thanks. I confirm that we have tested this. Jon. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] 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. CC: linux-arm-ker...@lists.infradead.org CC: kvm...@lists.cs.columbia.edu CC: a...@arndb.de CC: marc.zyng...@arm.com CC: christoffer.d...@linaro.org CC: j...@redhat.com Signed-off-by: Pranavkumar Sawargaonkar Signed-off-by: Tushar Jagad Signed-off-by: Feng Kan --- arch/arm64/boot/dts/apm/apm-storm.dtsi |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm64/boot/dts/apm/apm-storm.dtsi b/arch/arm64/boot/dts/apm/apm-storm.dtsi index f1ad9c2..65f0e6d 100644 --- a/arch/arm64/boot/dts/apm/apm-storm.dtsi +++ b/arch/arm64/boot/dts/apm/apm-storm.dtsi @@ -81,10 +81,10 @@ compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; interrupt-controller; - reg = <0x0 0x7801 0x0 0x1000>, /* GIC Dist */ - <0x0 0x7802 0x0 0x1000>, /* GIC CPU */ - <0x0 0x7804 0x0 0x2000>, /* GIC VCPU Control */ - <0x0 0x7806 0x0 0x2000>; /* GIC VCPU */ + reg = <0x0 0x7801 0x0 0x1>, /* GIC Dist */ + <0x0 0x7802 0x0 0x2>, /* GIC CPU */ + <0x0 0x7804 0x0 0x1>, /* GIC VCPU Control */ + <0x0 0x7806 0x0 0x2>; /* GIC VCPU */ interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ }; -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
