| Message ID | 588759E0.1010804@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,1/4] irqchip, gicv3-its: Add device tree binding for hisilicon 161010801 erratum | expand |
Hi Mark, > -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: Tuesday, January 24, 2017 1:52 PM > To: Shameerali Kolothum Thodi > Cc: marc.zyngier@arm.com; will.deacon@arm.com; linux- > kernel@vger.kernel.org; Linuxarm; devicetree@vger.kernel.org; John > Garry; Guohanjun (Hanjun Guo) > Subject: Re: [RFC 1/4] irqchip, gicv3-its: Add device tree binding for > hisilicon 161010801 erratum > > Hi, > > I see this wasn't Cc'd to LAKML, unlike the cover letter, and patch 3 > (which isn't threaded against the cover letter). > > Please use a consistent Cc list, with patches in-reply to the cover > letter. Apologies for the inconsistencies. > On Tue, Jan 24, 2017 at 01:42:56PM +0000, Shameerali Kolothum Thodi > wrote: > > This erratum describes the limitation of certain HiSilicon platforms > > to support the SMMU mappings for MSI transactions and on those > > platforms the MSI transactions has to be bypassed by SMMU. The IIDR > > register of the > > GICv3 ITS on these platforms are not properly populated to > > differentiate the hardware, hence describe it in device tree. > > > > Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com> > > --- > > .../devicetree/bindings/interrupt-controller/arm,gic-v3.txt > | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/interrupt-controller/arm,gic- > v3.tx > > t > > b/Documentation/devicetree/bindings/interrupt-controller/arm,gic- > v3.tx > > t > > index 4c29cda..84af301 100644 > > --- > > a/Documentation/devicetree/bindings/interrupt-controller/arm,gic- > v3.tx > > t > > +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic- > v > > +++ 3.txt > > @@ -75,6 +75,12 @@ These nodes must have the following properties: > > - reg: Specifies the base physical address and size of the ITS > > registers. > > > > +Optional > > +- hisilicon,erratum-161010801 : A boolean property. Indicates the > > +presence of > > + erratum 161010801, which says that these platforms doesn't support > > +SMMU > > + mapping for MSI transactions and those transactions has to be > > +bypassed > > + by SMMU. > > What exactly is meant by "doesn't support SMMU mapping" here? What > precisely is the problem in HW? On this platforms the ITS doorbell deviceID information is embedded in the MSI payload. To do this, the PCIe controller differentiates the MSI payload and DMA payload and modifies the MSI payload to add the deviceID information. The way it modifies this is by comparing against a SYS_CTRL register which is configured by UEFI with the ITS doorbell phys address. Hence if EP is configured with a SMMU translated Doorbell address, the PCIe RC cannot differentiate the MSI payload and ITS will fail to generate the Interrupt. Hope I am clear. Thanks, Shameer -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 24, 2017 at 02:00:30PM +0000, Shameerali Kolothum Thodi wrote: > > -----Original Message----- > > From: Mark Rutland [mailto:mark.rutland@arm.com] > > On Tue, Jan 24, 2017 at 01:42:56PM +0000, Shameerali Kolothum Thodi > > wrote: > > > +Optional > > > +- hisilicon,erratum-161010801 : A boolean property. Indicates the > > > +presence of > > > + erratum 161010801, which says that these platforms doesn't support > > > +SMMU > > > + mapping for MSI transactions and those transactions has to be > > > +bypassed > > > + by SMMU. > > > > What exactly is meant by "doesn't support SMMU mapping" here? What > > precisely is the problem in HW? > > On this platforms the ITS doorbell deviceID information is embedded in the MSI > payload. To do this, the PCIe controller differentiates the MSI payload and > DMA payload and modifies the MSI payload to add the deviceID information. > The way it modifies this is by comparing against a SYS_CTRL register which > is configured by UEFI with the ITS doorbell phys address. Ok. Some part of this will need to go in the binding description. How does this interact with translations via the SMMU? Do writes matching this address: (a) always bypass translation. (b) get translated after modification. (c) other? Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: Tuesday, January 24, 2017 2:29 PM > To: Shameerali Kolothum Thodi > Cc: marc.zyngier@arm.com; will.deacon@arm.com; linux- > kernel@vger.kernel.org; Linuxarm; devicetree@vger.kernel.org; John > Garry; Guohanjun (Hanjun Guo) > Subject: Re: [RFC 1/4] irqchip, gicv3-its: Add device tree binding for > hisilicon 161010801 erratum > > On Tue, Jan 24, 2017 at 02:00:30PM +0000, Shameerali Kolothum Thodi > wrote: > > > -----Original Message----- > > > From: Mark Rutland [mailto:mark.rutland@arm.com] > > > > On Tue, Jan 24, 2017 at 01:42:56PM +0000, Shameerali Kolothum Thodi > > > wrote: > > > > > +Optional > > > > +- hisilicon,erratum-161010801 : A boolean property. Indicates > the > > > > +presence of > > > > + erratum 161010801, which says that these platforms doesn't > > > > +support SMMU > > > > + mapping for MSI transactions and those transactions has to be > > > > +bypassed > > > > + by SMMU. > > > > > > What exactly is meant by "doesn't support SMMU mapping" here? What > > > precisely is the problem in HW? > > > > On this platforms the ITS doorbell deviceID information is embedded > in > > the MSI payload. To do this, the PCIe controller differentiates the > > MSI payload and DMA payload and modifies the MSI payload to add the > deviceID information. > > The way it modifies this is by comparing against a SYS_CTRL register > > which is configured by UEFI with the ITS doorbell phys address. > > Ok. Some part of this will need to go in the binding description. > > How does this interact with translations via the SMMU? > > Do writes matching this address: > > (a) always bypass translation. > (b) get translated after modification. > (c) other? PCIe RC has a configuration setting to enable/disable SMMU bypass for PCIe MSI write and with this patch series we are using the disable mode. So it bypasses SMMU always for MSI but not for DMA. As per our SoC engineers this implementation seems to be based on an earlier version of GIC spec earlier version the GIC spec(Document number:PRD03-GENC-010745 18.0) where it says: "Implementations may choose to transform writes to GITS_TRANSLATER by either: -multiplexing the device ID onto the address bus (which is what GIC-500 provides a mechanism for), or -extending the data value to 64 bits, providing the device ID in the upper bits, and transforming the access to become a 64-bit write" Though I can't find the same in latest GIC spec. Thanks, Shameer -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 24/01/17 15:13, Shameerali Kolothum Thodi wrote: > > >> -----Original Message----- >> From: Mark Rutland [mailto:mark.rutland@arm.com] >> Sent: Tuesday, January 24, 2017 2:29 PM >> To: Shameerali Kolothum Thodi >> Cc: marc.zyngier@arm.com; will.deacon@arm.com; linux- >> kernel@vger.kernel.org; Linuxarm; devicetree@vger.kernel.org; John >> Garry; Guohanjun (Hanjun Guo) >> Subject: Re: [RFC 1/4] irqchip, gicv3-its: Add device tree binding for >> hisilicon 161010801 erratum >> >> On Tue, Jan 24, 2017 at 02:00:30PM +0000, Shameerali Kolothum Thodi >> wrote: >>>> -----Original Message----- >>>> From: Mark Rutland [mailto:mark.rutland@arm.com] >> >>>> On Tue, Jan 24, 2017 at 01:42:56PM +0000, Shameerali Kolothum Thodi >>>> wrote: >> >>>>> +Optional >>>>> +- hisilicon,erratum-161010801 : A boolean property. Indicates >> the >>>>> +presence of >>>>> + erratum 161010801, which says that these platforms doesn't >>>>> +support SMMU >>>>> + mapping for MSI transactions and those transactions has to be >>>>> +bypassed >>>>> + by SMMU. >>>> >>>> What exactly is meant by "doesn't support SMMU mapping" here? What >>>> precisely is the problem in HW? >>> >>> On this platforms the ITS doorbell deviceID information is embedded >> in >>> the MSI payload. To do this, the PCIe controller differentiates the >>> MSI payload and DMA payload and modifies the MSI payload to add the >> deviceID information. >>> The way it modifies this is by comparing against a SYS_CTRL register >>> which is configured by UEFI with the ITS doorbell phys address. >> >> Ok. Some part of this will need to go in the binding description. >> >> How does this interact with translations via the SMMU? >> >> Do writes matching this address: >> >> (a) always bypass translation. >> (b) get translated after modification. >> (c) other? > > PCIe RC has a configuration setting to enable/disable SMMU > bypass for PCIe MSI write and with this patch series we > are using the disable mode. So it bypasses SMMU always for > MSI but not for DMA. > > As per our SoC engineers this implementation seems to be based on an earlier > version of GIC spec earlier version the GIC spec(Document > number:PRD03-GENC-010745 18.0) where it says: > > "Implementations may choose to transform writes to GITS_TRANSLATER by either: > -multiplexing the device ID onto the address bus (which is what GIC-500 provides > a mechanism for), or > -extending the data value to 64 bits, providing the device ID in the upper bits, > and transforming the access to become a 64-bit write" Crucially, that should be done by performing the up-scaling just as the write reaches the ITS translation register, and *not* when the write leaves the RC. If you up-scale it early, you end-up in this silly situation. > Though I can't find the same in latest GIC spec. Because that's not an architecture feature, but an implement decision. And whatever the implementation does, it should be invisible to SW. Unfortunately, bypassing the SMMU is not exactly invisible... 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt index 4c29cda..84af301 100644 --- a/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt +++ b/Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt @@ -75,6 +75,12 @@ These nodes must have the following properties: - reg: Specifies the base physical address and size of the ITS registers. +Optional +- hisilicon,erratum-161010801 : A boolean property. Indicates the presence of + erratum 161010801, which says that these platforms doesn't support SMMU + mapping for MSI transactions and those transactions has to be bypassed + by SMMU. + The main GIC node must contain the appropriate #address-cells, #size-cells and ranges properties for the reg property of all ITS nodes.
This erratum describes the limitation of certain HiSilicon platforms to support the SMMU mappings for MSI transactions and on those platforms the MSI transactions has to be bypassed by SMMU. The IIDR register of the GICv3 ITS on these platforms are not properly populated to differentiate the hardware, hence describe it in device tree. Signed-off-by: shameer <shameerali.kolothum.thodi@huawei.com> --- .../devicetree/bindings/interrupt-controller/arm,gic-v3.txt | 6 ++++++ 1 file changed, 6 insertions(+) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html