| Message ID | 1453929121-12171-12-git-send-email-Suravee.Suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 27, 2016 at 03:11:59PM -0600, Suravee Suthikulpanit wrote: > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > Add PCIe SMMU device tree node for AMD Seattle SOC. > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > --- > arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > index a7fc059..bfccfea 100644 > --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > @@ -210,6 +210,7 @@ > device_type = "pci"; > bus-range = <0 0x7f>; > msi-parent = <&v2m0>; > + #stream-id-cells = <16>; > reg = <0 0xf0000000 0 0x10000000>; > > interrupt-map-mask = <0xf800 0x0 0x0 0x7>; > @@ -230,6 +231,28 @@ > <0x03000000 0x01 0x00000000 0x01 0x00000000 0x7f 0x00000000>; > }; > > + pcie0_smmu: smmu@e0a00000 { > + compatible = "arm,mmu-401"; > + reg = <0 0xe0a00000 0 0x10000>; > + #global-interrupts = <1>; > + interrupts = /* Uses combined intr for both > + * global and context > + */ > + <0 333 4>, > + <0 333 4>; > + /* Note: > + * SID[2:0] = PCIe function number > + * SID[7:3] = PCIe device number > + * SID[14:8] = PCIe bus number > + */ > + mmu-masters = <&pcie0 > + /* 1:00:[0,3] */ 256 257 258 259 > + /* 2:00:[0,3] */ 512 513 514 515 > + /* 3:00:[0,3] */ 768 769 770 771 > + /* 4:00:[0,3] */ 1024 1025 1026 1027 > + >; > + }; This doesn't look right to me. I didn't think that RID->SID mapping was actually defined by any binding, so (how) are these numbers used? I'm uncomfortable with this, given we should be moving towards the generic IOMMU binding (and then we'd use the iommu-map binding [1] for this). Will, Robin, thoughts? Mark. [1] https://lkml.org/lkml/2015/7/23/561
On Thu, Jan 28, 2016 at 11:14:53AM +0000, Mark Rutland wrote: > On Wed, Jan 27, 2016 at 03:11:59PM -0600, Suravee Suthikulpanit wrote: > > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > > > Add PCIe SMMU device tree node for AMD Seattle SOC. > > > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > --- > > arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 23 +++++++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > > index a7fc059..bfccfea 100644 > > --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > > +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > > @@ -210,6 +210,7 @@ > > device_type = "pci"; > > bus-range = <0 0x7f>; > > msi-parent = <&v2m0>; > > + #stream-id-cells = <16>; > > reg = <0 0xf0000000 0 0x10000000>; > > > > interrupt-map-mask = <0xf800 0x0 0x0 0x7>; > > @@ -230,6 +231,28 @@ > > <0x03000000 0x01 0x00000000 0x01 0x00000000 0x7f 0x00000000>; > > }; > > > > + pcie0_smmu: smmu@e0a00000 { > > + compatible = "arm,mmu-401"; > > + reg = <0 0xe0a00000 0 0x10000>; > > + #global-interrupts = <1>; > > + interrupts = /* Uses combined intr for both > > + * global and context > > + */ > > + <0 333 4>, > > + <0 333 4>; > > + /* Note: > > + * SID[2:0] = PCIe function number > > + * SID[7:3] = PCIe device number > > + * SID[14:8] = PCIe bus number > > + */ > > + mmu-masters = <&pcie0 > > + /* 1:00:[0,3] */ 256 257 258 259 > > + /* 2:00:[0,3] */ 512 513 514 515 > > + /* 3:00:[0,3] */ 768 769 770 771 > > + /* 4:00:[0,3] */ 1024 1025 1026 1027 > > + >; > > + }; > > This doesn't look right to me. > > I didn't think that RID->SID mapping was actually defined by any > binding, so (how) are these numbers used? > > I'm uncomfortable with this, given we should be moving towards the > generic IOMMU binding (and then we'd use the iommu-map binding [1] for > this). > > Will, Robin, thoughts? The driver currently assumes a 1:1 RID:SID mapping when it sees a PCI device, so those numbers should be ignored. Will
On Thu, Jan 28, 2016 at 11:17:39AM +0000, Will Deacon wrote: > On Thu, Jan 28, 2016 at 11:14:53AM +0000, Mark Rutland wrote: > > On Wed, Jan 27, 2016 at 03:11:59PM -0600, Suravee Suthikulpanit wrote: > > > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > > > > > Add PCIe SMMU device tree node for AMD Seattle SOC. > > > > > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > > --- > > > arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 23 +++++++++++++++++++++++ > > > 1 file changed, 23 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > > > index a7fc059..bfccfea 100644 > > > --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > > > +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > > > @@ -210,6 +210,7 @@ > > > device_type = "pci"; > > > bus-range = <0 0x7f>; > > > msi-parent = <&v2m0>; > > > + #stream-id-cells = <16>; > > > reg = <0 0xf0000000 0 0x10000000>; > > > > > > interrupt-map-mask = <0xf800 0x0 0x0 0x7>; > > > @@ -230,6 +231,28 @@ > > > <0x03000000 0x01 0x00000000 0x01 0x00000000 0x7f 0x00000000>; > > > }; > > > > > > + pcie0_smmu: smmu@e0a00000 { > > > + compatible = "arm,mmu-401"; > > > + reg = <0 0xe0a00000 0 0x10000>; > > > + #global-interrupts = <1>; > > > + interrupts = /* Uses combined intr for both > > > + * global and context > > > + */ > > > + <0 333 4>, > > > + <0 333 4>; > > > + /* Note: > > > + * SID[2:0] = PCIe function number > > > + * SID[7:3] = PCIe device number > > > + * SID[14:8] = PCIe bus number > > > + */ > > > + mmu-masters = <&pcie0 > > > + /* 1:00:[0,3] */ 256 257 258 259 > > > + /* 2:00:[0,3] */ 512 513 514 515 > > > + /* 3:00:[0,3] */ 768 769 770 771 > > > + /* 4:00:[0,3] */ 1024 1025 1026 1027 > > > + >; > > > + }; > > > > This doesn't look right to me. > > > > I didn't think that RID->SID mapping was actually defined by any > > binding, so (how) are these numbers used? > > > > I'm uncomfortable with this, given we should be moving towards the > > generic IOMMU binding (and then we'd use the iommu-map binding [1] for > > this). > > > > Will, Robin, thoughts? > > The driver currently assumes a 1:1 RID:SID mapping when it sees a PCI > device, so those numbers should be ignored. Given that, they shouldn't be in the DT, then? Mark.
On Thu, Jan 28, 2016 at 11:18:19AM +0000, Mark Rutland wrote: > On Thu, Jan 28, 2016 at 11:17:39AM +0000, Will Deacon wrote: > > On Thu, Jan 28, 2016 at 11:14:53AM +0000, Mark Rutland wrote: > > > On Wed, Jan 27, 2016 at 03:11:59PM -0600, Suravee Suthikulpanit wrote: > > > > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > > > > > > > Add PCIe SMMU device tree node for AMD Seattle SOC. > > > > > > > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > > > --- > > > > arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 23 +++++++++++++++++++++++ > > > > 1 file changed, 23 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > > > > index a7fc059..bfccfea 100644 > > > > --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > > > > +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi > > > > @@ -210,6 +210,7 @@ > > > > device_type = "pci"; > > > > bus-range = <0 0x7f>; > > > > msi-parent = <&v2m0>; > > > > + #stream-id-cells = <16>; > > > > reg = <0 0xf0000000 0 0x10000000>; > > > > > > > > interrupt-map-mask = <0xf800 0x0 0x0 0x7>; > > > > @@ -230,6 +231,28 @@ > > > > <0x03000000 0x01 0x00000000 0x01 0x00000000 0x7f 0x00000000>; > > > > }; > > > > > > > > + pcie0_smmu: smmu@e0a00000 { > > > > + compatible = "arm,mmu-401"; > > > > + reg = <0 0xe0a00000 0 0x10000>; > > > > + #global-interrupts = <1>; > > > > + interrupts = /* Uses combined intr for both > > > > + * global and context > > > > + */ > > > > + <0 333 4>, > > > > + <0 333 4>; > > > > + /* Note: > > > > + * SID[2:0] = PCIe function number > > > > + * SID[7:3] = PCIe device number > > > > + * SID[14:8] = PCIe bus number > > > > + */ > > > > + mmu-masters = <&pcie0 > > > > + /* 1:00:[0,3] */ 256 257 258 259 > > > > + /* 2:00:[0,3] */ 512 513 514 515 > > > > + /* 3:00:[0,3] */ 768 769 770 771 > > > > + /* 4:00:[0,3] */ 1024 1025 1026 1027 > > > > + >; > > > > + }; > > > > > > This doesn't look right to me. > > > > > > I didn't think that RID->SID mapping was actually defined by any > > > binding, so (how) are these numbers used? > > > > > > I'm uncomfortable with this, given we should be moving towards the > > > generic IOMMU binding (and then we'd use the iommu-map binding [1] for > > > this). > > > > > > Will, Robin, thoughts? > > > > The driver currently assumes a 1:1 RID:SID mapping when it sees a PCI > > device, so those numbers should be ignored. > > Given that, they shouldn't be in the DT, then? Not unless there's a patch extending the driver/binding so that they do something useful, no. Will
On 28/01/16 11:14, Mark Rutland wrote: > On Wed, Jan 27, 2016 at 03:11:59PM -0600, Suravee Suthikulpanit wrote: >> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >> >> Add PCIe SMMU device tree node for AMD Seattle SOC. >> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> >> --- >> arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi >> index a7fc059..bfccfea 100644 >> --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi >> +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi >> @@ -210,6 +210,7 @@ >> device_type = "pci"; >> bus-range = <0 0x7f>; >> msi-parent = <&v2m0>; >> + #stream-id-cells = <16>; >> reg = <0 0xf0000000 0 0x10000000>; >> >> interrupt-map-mask = <0xf800 0x0 0x0 0x7>; >> @@ -230,6 +231,28 @@ >> <0x03000000 0x01 0x00000000 0x01 0x00000000 0x7f 0x00000000>; >> }; >> >> + pcie0_smmu: smmu@e0a00000 { >> + compatible = "arm,mmu-401"; >> + reg = <0 0xe0a00000 0 0x10000>; >> + #global-interrupts = <1>; >> + interrupts = /* Uses combined intr for both >> + * global and context >> + */ >> + <0 333 4>, >> + <0 333 4>; >> + /* Note: >> + * SID[2:0] = PCIe function number >> + * SID[7:3] = PCIe device number >> + * SID[14:8] = PCIe bus number >> + */ >> + mmu-masters = <&pcie0 >> + /* 1:00:[0,3] */ 256 257 258 259 >> + /* 2:00:[0,3] */ 512 513 514 515 >> + /* 3:00:[0,3] */ 768 769 770 771 >> + /* 4:00:[0,3] */ 1024 1025 1026 1027 >> + >; >> + }; > > This doesn't look right to me. > > I didn't think that RID->SID mapping was actually defined by any > binding, so (how) are these numbers used? > > I'm uncomfortable with this, given we should be moving towards the > generic IOMMU binding (and then we'd use the iommu-map binding [1] for > this). > > Will, Robin, thoughts? Any IDs specified here would only apply to DMA by the "platform device" side of the host controller itself (as would an equivalent "iommus" property on pcie0 once I finish the SMMUv2 generic binding support I'm working on). In terms of PCI devices, the "mmu-masters" property is overloaded such that only its existence matters, to identify that there _is_ a relationship between the SMMU and the PCI bus(es) behind that host controller. Really, the only binding that's going to make sense for that context is "iommu-map". I think that should be pretty straightforward to implement entirely in core OF/IOMMU code - we can already figure out a device's DMA alias as the host controller sees it, we just need the IOMMU drivers to be able to then run that through an additional downstream translation (which would be identity-mapped by default). Robin. > > Mark. > > [1] https://lkml.org/lkml/2015/7/23/561 >
On Thursday 28 January 2016 12:20:58 Robin Murphy wrote: > > > > Will, Robin, thoughts? > > Any IDs specified here would only apply to DMA by the "platform device" > side of the host controller itself (as would an equivalent "iommus" > property on pcie0 once I finish the SMMUv2 generic binding support I'm > working on). In terms of PCI devices, the "mmu-masters" property is > overloaded such that only its existence matters, to identify that there > _is_ a relationship between the SMMU and the PCI bus(es) behind that > host controller. I wasn't aware that this was actually still specified. I had hoped we were getting rid of mmu-masters before anyone actually started using it, but now I see it in ns2.dtsi and fsl-ls2080a.dtsi. Does anyone know what happened to the plan to use the iommu DT binding for the ARM SMMU instead? Do we now have to support both ways indefinitely? Arnd
On Thu, Jan 28, 2016 at 03:17:33PM +0100, Arnd Bergmann wrote: > On Thursday 28 January 2016 12:20:58 Robin Murphy wrote: > > > > > > Will, Robin, thoughts? > > > > Any IDs specified here would only apply to DMA by the "platform device" > > side of the host controller itself (as would an equivalent "iommus" > > property on pcie0 once I finish the SMMUv2 generic binding support I'm > > working on). In terms of PCI devices, the "mmu-masters" property is > > overloaded such that only its existence matters, to identify that there > > _is_ a relationship between the SMMU and the PCI bus(es) behind that > > host controller. > > I wasn't aware that this was actually still specified. I had hoped > we were getting rid of mmu-masters before anyone actually started > using it, but now I see it in ns2.dtsi and fsl-ls2080a.dtsi. > > Does anyone know what happened to the plan to use the iommu DT binding > for the ARM SMMU instead? Do we now have to support both ways indefinitely? We always did -- Seattle used the mmu-masters binding before the generic binding even existed. Robin has been working on patches to get of_xlate up and running, but it got held up by Laurent's series which didn't end up going anywhere. Will
Dear all, On 01/28/2016 03:27 PM, Will Deacon wrote: > On Thu, Jan 28, 2016 at 03:17:33PM +0100, Arnd Bergmann wrote: >> On Thursday 28 January 2016 12:20:58 Robin Murphy wrote: >>>> >>>> Will, Robin, thoughts? >>> >>> Any IDs specified here would only apply to DMA by the "platform device" >>> side of the host controller itself (as would an equivalent "iommus" >>> property on pcie0 once I finish the SMMUv2 generic binding support I'm >>> working on). In terms of PCI devices, the "mmu-masters" property is >>> overloaded such that only its existence matters, to identify that there >>> _is_ a relationship between the SMMU and the PCI bus(es) behind that >>> host controller. >> >> I wasn't aware that this was actually still specified. I had hoped >> we were getting rid of mmu-masters before anyone actually started >> using it, but now I see it in ns2.dtsi and fsl-ls2080a.dtsi. >> >> Does anyone know what happened to the plan to use the iommu DT binding >> for the ARM SMMU instead? Do we now have to support both ways indefinitely? > > We always did -- Seattle used the mmu-masters binding before the generic > binding even existed. Robin has been working on patches to get of_xlate > up and running, but it got held up by Laurent's series which didn't end > up going anywhere. > > Will Up to now I have used the PCI smmu description as described in Suravee's patch and this does not work anymore with 4.6-rc1 since the default domain was introduced. So now I see 2 SMRs matching a single streamid (in my case 256, one steming from the "platform device" side of the host controller and one steming from the PCI device) and this causes SMCF (stream match conflict fault). So PCIe PF does not work. I observe the fault only when I override the PCIe ACS property in my case which was pretty confusing (each PF/VF is put in a separate group). What is the correct syntax then: mmu-masters = <&pcie0>? instead of + mmu-masters = <&pcie0 + /* 1:00:[0,3] */ 256 257 258 259 + /* 2:00:[0,3] */ 512 513 514 515 + /* 3:00:[0,3] */ 768 769 770 771 + /* 4:00:[0,3] */ 1024 1025 1026 1027> Is there a plan to update/upstream the dt description for PCIe smmu. Thank you in advance Best Regards Eric > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
Hi Eric, On Wed, Mar 30, 2016 at 05:37:27PM +0200, Eric Auger wrote: > On 01/28/2016 03:27 PM, Will Deacon wrote: > > On Thu, Jan 28, 2016 at 03:17:33PM +0100, Arnd Bergmann wrote: > >> On Thursday 28 January 2016 12:20:58 Robin Murphy wrote: > >>>> > >>> Any IDs specified here would only apply to DMA by the "platform device" > >>> side of the host controller itself (as would an equivalent "iommus" > >>> property on pcie0 once I finish the SMMUv2 generic binding support I'm > >>> working on). In terms of PCI devices, the "mmu-masters" property is > >>> overloaded such that only its existence matters, to identify that there > >>> _is_ a relationship between the SMMU and the PCI bus(es) behind that > >>> host controller. > >> > >> I wasn't aware that this was actually still specified. I had hoped > >> we were getting rid of mmu-masters before anyone actually started > >> using it, but now I see it in ns2.dtsi and fsl-ls2080a.dtsi. > >> > >> Does anyone know what happened to the plan to use the iommu DT binding > >> for the ARM SMMU instead? Do we now have to support both ways indefinitely? > > > > We always did -- Seattle used the mmu-masters binding before the generic > > binding even existed. Robin has been working on patches to get of_xlate > > up and running, but it got held up by Laurent's series which didn't end > > up going anywhere. > > > Up to now I have used the PCI smmu description as described in Suravee's > patch and this does not work anymore with 4.6-rc1 since the default > domain was introduced. So now I see 2 SMRs matching a single streamid > (in my case 256, one steming from the "platform device" side of the host > controller and one steming from the PCI device) and this causes SMCF > (stream match conflict fault). So PCIe PF does not work. Sorry about that, it wasn't intentional. In fact, I wrote commit cbf8277ef456 ("iommu/arm-smmu: Treat IOMMU_DOMAIN_DMA as bypass for now") specifically to avoid this breakage, after seeing it myself with VFIO and an S2CR-based configuration. It looks like the check just needs moving higher up (i.e. before we initialise the SMRs). Does that fix it for you? Will
Hi Will, On 03/30/2016 05:45 PM, Will Deacon wrote: > Hi Eric, > > On Wed, Mar 30, 2016 at 05:37:27PM +0200, Eric Auger wrote: >> On 01/28/2016 03:27 PM, Will Deacon wrote: >>> On Thu, Jan 28, 2016 at 03:17:33PM +0100, Arnd Bergmann wrote: >>>> On Thursday 28 January 2016 12:20:58 Robin Murphy wrote: >>>>>> >>>>> Any IDs specified here would only apply to DMA by the "platform device" >>>>> side of the host controller itself (as would an equivalent "iommus" >>>>> property on pcie0 once I finish the SMMUv2 generic binding support I'm >>>>> working on). In terms of PCI devices, the "mmu-masters" property is >>>>> overloaded such that only its existence matters, to identify that there >>>>> _is_ a relationship between the SMMU and the PCI bus(es) behind that >>>>> host controller. >>>> >>>> I wasn't aware that this was actually still specified. I had hoped >>>> we were getting rid of mmu-masters before anyone actually started >>>> using it, but now I see it in ns2.dtsi and fsl-ls2080a.dtsi. >>>> >>>> Does anyone know what happened to the plan to use the iommu DT binding >>>> for the ARM SMMU instead? Do we now have to support both ways indefinitely? >>> >>> We always did -- Seattle used the mmu-masters binding before the generic >>> binding even existed. Robin has been working on patches to get of_xlate >>> up and running, but it got held up by Laurent's series which didn't end >>> up going anywhere. >>> >> Up to now I have used the PCI smmu description as described in Suravee's >> patch and this does not work anymore with 4.6-rc1 since the default >> domain was introduced. So now I see 2 SMRs matching a single streamid >> (in my case 256, one steming from the "platform device" side of the host >> controller and one steming from the PCI device) and this causes SMCF >> (stream match conflict fault). So PCIe PF does not work. > > Sorry about that, it wasn't intentional. In fact, I wrote commit > cbf8277ef456 ("iommu/arm-smmu: Treat IOMMU_DOMAIN_DMA as bypass for now") > specifically to avoid this breakage, after seeing it myself with VFIO > and an S2CR-based configuration. It looks like the check just needs moving > higher up (i.e. before we initialise the SMRs). > > Does that fix it for you? Yes this fixes the issue for me, thanks! I guess you will send that patch? So eventually what is the right way to describe the smmu-masters (~ future of that patch)? Best Regards Eric > > Will >
On Wed, Mar 30, 2016 at 05:57:08PM +0200, Eric Auger wrote: > On 03/30/2016 05:45 PM, Will Deacon wrote: > > On Wed, Mar 30, 2016 at 05:37:27PM +0200, Eric Auger wrote: > >> On 01/28/2016 03:27 PM, Will Deacon wrote: > >>> On Thu, Jan 28, 2016 at 03:17:33PM +0100, Arnd Bergmann wrote: > >>>> On Thursday 28 January 2016 12:20:58 Robin Murphy wrote: > >>>>>> > >>>>> Any IDs specified here would only apply to DMA by the "platform device" > >>>>> side of the host controller itself (as would an equivalent "iommus" > >>>>> property on pcie0 once I finish the SMMUv2 generic binding support I'm > >>>>> working on). In terms of PCI devices, the "mmu-masters" property is > >>>>> overloaded such that only its existence matters, to identify that there > >>>>> _is_ a relationship between the SMMU and the PCI bus(es) behind that > >>>>> host controller. > >>>> > >>>> I wasn't aware that this was actually still specified. I had hoped > >>>> we were getting rid of mmu-masters before anyone actually started > >>>> using it, but now I see it in ns2.dtsi and fsl-ls2080a.dtsi. > >>>> > >>>> Does anyone know what happened to the plan to use the iommu DT binding > >>>> for the ARM SMMU instead? Do we now have to support both ways indefinitely? > >>> > >>> We always did -- Seattle used the mmu-masters binding before the generic > >>> binding even existed. Robin has been working on patches to get of_xlate > >>> up and running, but it got held up by Laurent's series which didn't end > >>> up going anywhere. > >>> > >> Up to now I have used the PCI smmu description as described in Suravee's > >> patch and this does not work anymore with 4.6-rc1 since the default > >> domain was introduced. So now I see 2 SMRs matching a single streamid > >> (in my case 256, one steming from the "platform device" side of the host > >> controller and one steming from the PCI device) and this causes SMCF > >> (stream match conflict fault). So PCIe PF does not work. > > > > Sorry about that, it wasn't intentional. In fact, I wrote commit > > cbf8277ef456 ("iommu/arm-smmu: Treat IOMMU_DOMAIN_DMA as bypass for now") > > specifically to avoid this breakage, after seeing it myself with VFIO > > and an S2CR-based configuration. It looks like the check just needs moving > > higher up (i.e. before we initialise the SMRs). > > > > Does that fix it for you? > Yes this fixes the issue for me, thanks! I guess you will send that patch? I need to check that it doesn't break rebinding to the host after VFIO has been used for passthrough, first. Does your devicetree explicitly assign a StreamID to the platform device for the host controller? We should probably be handling this, since it will crop us as an issue again once we decide to enable the default domain properly. > So eventually what is the right way to describe the smmu-masters (~ > future of that patch)? Using the generic iommu binding (Documentation/devicetree/bindings/iommu/iommu.txt), which will be required for of_xlate-based probing. The old binding should still continue to function as it always has, though. Will
Hi Will, On 03/30/2016 07:24 PM, Will Deacon wrote: > On Wed, Mar 30, 2016 at 05:57:08PM +0200, Eric Auger wrote: >> On 03/30/2016 05:45 PM, Will Deacon wrote: >>> On Wed, Mar 30, 2016 at 05:37:27PM +0200, Eric Auger wrote: >>>> On 01/28/2016 03:27 PM, Will Deacon wrote: >>>>> On Thu, Jan 28, 2016 at 03:17:33PM +0100, Arnd Bergmann wrote: >>>>>> On Thursday 28 January 2016 12:20:58 Robin Murphy wrote: >>>>>>>> >>>>>>> Any IDs specified here would only apply to DMA by the "platform device" >>>>>>> side of the host controller itself (as would an equivalent "iommus" >>>>>>> property on pcie0 once I finish the SMMUv2 generic binding support I'm >>>>>>> working on). In terms of PCI devices, the "mmu-masters" property is >>>>>>> overloaded such that only its existence matters, to identify that there >>>>>>> _is_ a relationship between the SMMU and the PCI bus(es) behind that >>>>>>> host controller. >>>>>> >>>>>> I wasn't aware that this was actually still specified. I had hoped >>>>>> we were getting rid of mmu-masters before anyone actually started >>>>>> using it, but now I see it in ns2.dtsi and fsl-ls2080a.dtsi. >>>>>> >>>>>> Does anyone know what happened to the plan to use the iommu DT binding >>>>>> for the ARM SMMU instead? Do we now have to support both ways indefinitely? >>>>> >>>>> We always did -- Seattle used the mmu-masters binding before the generic >>>>> binding even existed. Robin has been working on patches to get of_xlate >>>>> up and running, but it got held up by Laurent's series which didn't end >>>>> up going anywhere. >>>>> >>>> Up to now I have used the PCI smmu description as described in Suravee's >>>> patch and this does not work anymore with 4.6-rc1 since the default >>>> domain was introduced. So now I see 2 SMRs matching a single streamid >>>> (in my case 256, one steming from the "platform device" side of the host >>>> controller and one steming from the PCI device) and this causes SMCF >>>> (stream match conflict fault). So PCIe PF does not work. >>> >>> Sorry about that, it wasn't intentional. In fact, I wrote commit >>> cbf8277ef456 ("iommu/arm-smmu: Treat IOMMU_DOMAIN_DMA as bypass for now") >>> specifically to avoid this breakage, after seeing it myself with VFIO >>> and an S2CR-based configuration. It looks like the check just needs moving >>> higher up (i.e. before we initialise the SMRs). >>> >>> Does that fix it for you? >> Yes this fixes the issue for me, thanks! I guess you will send that patch? > > I need to check that it doesn't break rebinding to the host after VFIO > has been used for passthrough, first. OK. I can help testing too since I am currently working on PCIe passthrough respin. > > Does your devicetree explicitly assign a StreamID to the platform device > for the host controller? We should probably be handling this, since it > will crop us as an issue again once we decide to enable the default > domain properly. Yes it does. My dt description currently matches the one found in this patch from Suravee (old binding style). Hence my question. Shall I remove this and let PCIe register their RID=SID automatically? > >> So eventually what is the right way to describe the smmu-masters (~ >> future of that patch)? > > Using the generic iommu binding > (Documentation/devicetree/bindings/iommu/iommu.txt), which will be required > for of_xlate-based probing. The old binding should still continue to > function as it always has, though. OK thanks Eric > > Will >
On Thu, Mar 31, 2016 at 09:39:38AM +0200, Eric Auger wrote: > On 03/30/2016 07:24 PM, Will Deacon wrote: > > On Wed, Mar 30, 2016 at 05:57:08PM +0200, Eric Auger wrote: > >> On 03/30/2016 05:45 PM, Will Deacon wrote: > >>> On Wed, Mar 30, 2016 at 05:37:27PM +0200, Eric Auger wrote: > >>>> On 01/28/2016 03:27 PM, Will Deacon wrote: > >>>>> On Thu, Jan 28, 2016 at 03:17:33PM +0100, Arnd Bergmann wrote: > >>>>>> On Thursday 28 January 2016 12:20:58 Robin Murphy wrote: > >>>>>>>> > >>>>>>> Any IDs specified here would only apply to DMA by the "platform device" > >>>>>>> side of the host controller itself (as would an equivalent "iommus" > >>>>>>> property on pcie0 once I finish the SMMUv2 generic binding support I'm > >>>>>>> working on). In terms of PCI devices, the "mmu-masters" property is > >>>>>>> overloaded such that only its existence matters, to identify that there > >>>>>>> _is_ a relationship between the SMMU and the PCI bus(es) behind that > >>>>>>> host controller. > >>>>>> > >>>>>> I wasn't aware that this was actually still specified. I had hoped > >>>>>> we were getting rid of mmu-masters before anyone actually started > >>>>>> using it, but now I see it in ns2.dtsi and fsl-ls2080a.dtsi. > >>>>>> > >>>>>> Does anyone know what happened to the plan to use the iommu DT binding > >>>>>> for the ARM SMMU instead? Do we now have to support both ways indefinitely? > >>>>> > >>>>> We always did -- Seattle used the mmu-masters binding before the generic > >>>>> binding even existed. Robin has been working on patches to get of_xlate > >>>>> up and running, but it got held up by Laurent's series which didn't end > >>>>> up going anywhere. > >>>>> > >>>> Up to now I have used the PCI smmu description as described in Suravee's > >>>> patch and this does not work anymore with 4.6-rc1 since the default > >>>> domain was introduced. So now I see 2 SMRs matching a single streamid > >>>> (in my case 256, one steming from the "platform device" side of the host > >>>> controller and one steming from the PCI device) and this causes SMCF > >>>> (stream match conflict fault). So PCIe PF does not work. > >>> > >>> Sorry about that, it wasn't intentional. In fact, I wrote commit > >>> cbf8277ef456 ("iommu/arm-smmu: Treat IOMMU_DOMAIN_DMA as bypass for now") > >>> specifically to avoid this breakage, after seeing it myself with VFIO > >>> and an S2CR-based configuration. It looks like the check just needs moving > >>> higher up (i.e. before we initialise the SMRs). > >>> > >>> Does that fix it for you? > >> Yes this fixes the issue for me, thanks! I guess you will send that patch? > > > > I need to check that it doesn't break rebinding to the host after VFIO > > has been used for passthrough, first. > > OK. I can help testing too since I am currently working on PCIe > passthrough respin. Thanks, that would be really helpful if you have the time. Will
diff --git a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi index a7fc059..bfccfea 100644 --- a/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi +++ b/arch/arm64/boot/dts/amd/amd-seattle-soc.dtsi @@ -210,6 +210,7 @@ device_type = "pci"; bus-range = <0 0x7f>; msi-parent = <&v2m0>; + #stream-id-cells = <16>; reg = <0 0xf0000000 0 0x10000000>; interrupt-map-mask = <0xf800 0x0 0x0 0x7>; @@ -230,6 +231,28 @@ <0x03000000 0x01 0x00000000 0x01 0x00000000 0x7f 0x00000000>; }; + pcie0_smmu: smmu@e0a00000 { + compatible = "arm,mmu-401"; + reg = <0 0xe0a00000 0 0x10000>; + #global-interrupts = <1>; + interrupts = /* Uses combined intr for both + * global and context + */ + <0 333 4>, + <0 333 4>; + /* Note: + * SID[2:0] = PCIe function number + * SID[7:3] = PCIe device number + * SID[14:8] = PCIe bus number + */ + mmu-masters = <&pcie0 + /* 1:00:[0,3] */ 256 257 258 259 + /* 2:00:[0,3] */ 512 513 514 515 + /* 3:00:[0,3] */ 768 769 770 771 + /* 4:00:[0,3] */ 1024 1025 1026 1027 + >; + }; + /* Perf CCN504 PMU */ ccn: ccn@0xe8000000 { compatible = "arm,ccn-504";