* APM smmu implementation @ 2017-01-06 23:21 Feng Kan 2017-01-09 10:54 ` Will Deacon 2017-01-09 11:34 ` Robin Murphy 0 siblings, 2 replies; 14+ messages in thread From: Feng Kan @ 2017-01-06 23:21 UTC (permalink / raw) To: linux-arm-kernel Hi Will/Robin: The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. However, our internal bus is only 42 bits wide. Our IAS field is coded as 48 bits, which cause IPA to truncated to 42 bits on the physical bus. In order for our system to work with the arm-smmu.c, there needs to be a way to force the ipa_size to 42. The current internal solution is to use the cpuid, but that is quite ugly. I was thinking of using the model as indication to right the ipa_size, but I am not too sure of the ACPI side. Would it be okay to add an APM MMU500 variant? I would also appreciated it if you guys have any alternate solutions. Thanks Feng Kan ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-01-06 23:21 APM smmu implementation Feng Kan @ 2017-01-09 10:54 ` Will Deacon 2017-01-09 17:37 ` Feng Kan 2017-01-09 11:34 ` Robin Murphy 1 sibling, 1 reply; 14+ messages in thread From: Will Deacon @ 2017-01-09 10:54 UTC (permalink / raw) To: linux-arm-kernel On Fri, Jan 06, 2017 at 03:21:34PM -0800, Feng Kan wrote: > The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. > However, our internal bus is only 42 bits wide. Our IAS field is coded > as 48 bits, which cause IPA to truncated to 42 bits on the physical > bus. In order for our system to work with the arm-smmu.c, there needs > to be a way to force the ipa_size to 42. The current internal solution > is to use the cpuid, but that is quite ugly. I was thinking of using > the model Just so I understand, what are the UBS and OAS values on your part? > as indication to right the ipa_size, but I am not too sure of the ACPI > side. Would it be okay to add an APM MMU500 variant? I would also > appreciated it if you guys have any alternate solutions. For this sort of implementation erratum, I think using the model field is the right thing to do. However, you'll need to get your model registered with IORT, so I've added Charles to cc since he maintains that document. Also, please add a device-tree property to override the IAS once you start writing patches. Will ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-01-09 10:54 ` Will Deacon @ 2017-01-09 17:37 ` Feng Kan 0 siblings, 0 replies; 14+ messages in thread From: Feng Kan @ 2017-01-09 17:37 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 9, 2017 at 2:54 AM, Will Deacon <will.deacon@arm.com> wrote: > On Fri, Jan 06, 2017 at 03:21:34PM -0800, Feng Kan wrote: >> The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. >> However, our internal bus is only 42 bits wide. Our IAS field is coded >> as 48 bits, which cause IPA to truncated to 42 bits on the physical >> bus. In order for our system to work with the arm-smmu.c, there needs >> to be a way to force the ipa_size to 42. The current internal solution >> is to use the cpuid, but that is quite ugly. I was thinking of using >> the model > > Just so I understand, what are the UBS and OAS values on your part? UBS/OAS/IAS are all default to 5. > >> as indication to right the ipa_size, but I am not too sure of the ACPI >> side. Would it be okay to add an APM MMU500 variant? I would also >> appreciated it if you guys have any alternate solutions. > > For this sort of implementation erratum, I think using the model field > is the right thing to do. However, you'll need to get your model registered > with IORT, so I've added Charles to cc since he maintains that document. > > Also, please add a device-tree property to override the IAS once you > start writing patches. > > Will ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-01-06 23:21 APM smmu implementation Feng Kan 2017-01-09 10:54 ` Will Deacon @ 2017-01-09 11:34 ` Robin Murphy 2017-01-09 12:03 ` Will Deacon 1 sibling, 1 reply; 14+ messages in thread From: Robin Murphy @ 2017-01-09 11:34 UTC (permalink / raw) To: linux-arm-kernel On 06/01/17 23:21, Feng Kan wrote: > Hi Will/Robin: > > The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. "Mostly"? Have APM actually modified it (which I strongly doubt) or do you mean it's simply been integrated with the upper address lines tied off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave interfaces; that's all there is to it. Whether or not you use all of those bits is up to you as a system integrator. > However, our internal bus is only 42 bits wide. Our IAS field is coded > as 48 bits, which cause IPA to truncated to 42 bits on the physical > bus. In order for our system to work with the arm-smmu.c, there needs > to be a way to force the ipa_size to 42. The current internal solution > is to use the cpuid, but that is quite ugly. I was thinking of using > the model > as indication to right the ipa_size, but I am not too sure of the ACPI > side. Would it be okay to add an APM MMU500 variant? I would also > appreciated it if you guys have any alternate solutions. This is something we've been axpecting to run into for a while now - the appropriate solution is to use a "dma-ranges" property on the master device(s) to describe that they have 42 bits of address wired up, from which they will then inherit the appropriate DMA mask. The outstanding issue which remains is that we're still missing some way of preventing drivers simply clobbering that with a 64-bit mask later, but that's a more general problem[1]. Robin. [1]:https://www.mail-archive.com/linux-renesas-soc at vger.kernel.org/msg10209.html > > Thanks > Feng Kan > ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-01-09 11:34 ` Robin Murphy @ 2017-01-09 12:03 ` Will Deacon 2017-01-09 17:49 ` Feng Kan 2017-01-09 17:55 ` Robin Murphy 0 siblings, 2 replies; 14+ messages in thread From: Will Deacon @ 2017-01-09 12:03 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote: > On 06/01/17 23:21, Feng Kan wrote: > > The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. > > "Mostly"? Have APM actually modified it (which I strongly doubt) or do > you mean it's simply been integrated with the upper address lines tied > off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave > interfaces; that's all there is to it. Whether or not you use all of > those bits is up to you as a system integrator. That's a good point; MMU-500 doesn't appear to let you change the IAS anyway. That should also mean that UBS and OAS are unchanged. > > However, our internal bus is only 42 bits wide. Our IAS field is coded > > as 48 bits, which cause IPA to truncated to 42 bits on the physical > > bus. In order for our system to work with the arm-smmu.c, there needs > > to be a way to force the ipa_size to 42. The current internal solution > > is to use the cpuid, but that is quite ugly. I was thinking of using > > the model > > as indication to right the ipa_size, but I am not too sure of the ACPI > > side. Would it be okay to add an APM MMU500 variant? I would also > > appreciated it if you guys have any alternate solutions. > > This is something we've been axpecting to run into for a while now - the > appropriate solution is to use a "dma-ranges" property on the master > device(s) to describe that they have 42 bits of address wired up, from > which they will then inherit the appropriate DMA mask. The outstanding > issue which remains is that we're still missing some way of preventing > drivers simply clobbering that with a 64-bit mask later, but that's a > more general problem[1]. I wonder if the driver is actually using IAS, OAS and UBS incorrectly. We're using them to parameterise the DMA aperture, which is then used to size the IOVA domain, but that's wrong because the IAS, OAS and UBS are upper bounds and we can still end up allocating unusable/unreachable addresses. So I do think that this should be fixed on the SMMU firmware node, rather than restricting the range of each master device. Will ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-01-09 12:03 ` Will Deacon @ 2017-01-09 17:49 ` Feng Kan 2017-01-09 17:55 ` Robin Murphy 1 sibling, 0 replies; 14+ messages in thread From: Feng Kan @ 2017-01-09 17:49 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 9, 2017 at 4:03 AM, Will Deacon <will.deacon@arm.com> wrote: > On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote: >> On 06/01/17 23:21, Feng Kan wrote: >> > The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. >> >> "Mostly"? Have APM actually modified it (which I strongly doubt) or do >> you mean it's simply been integrated with the upper address lines tied >> off? Yes, and that we only do stage 2 translation. MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave >> interfaces; that's all there is to it. Whether or not you use all of >> those bits is up to you as a system integrator. > > That's a good point; MMU-500 doesn't appear to let you change the IAS > anyway. That should also mean that UBS and OAS are unchanged. > >> > However, our internal bus is only 42 bits wide. Our IAS field is coded >> > as 48 bits, which cause IPA to truncated to 42 bits on the physical >> > bus. In order for our system to work with the arm-smmu.c, there needs >> > to be a way to force the ipa_size to 42. The current internal solution >> > is to use the cpuid, but that is quite ugly. I was thinking of using >> > the model >> > as indication to right the ipa_size, but I am not too sure of the ACPI >> > side. Would it be okay to add an APM MMU500 variant? I would also >> > appreciated it if you guys have any alternate solutions. >> >> This is something we've been axpecting to run into for a while now - the >> appropriate solution is to use a "dma-ranges" property on the master >> device(s) to describe that they have 42 bits of address wired up, from >> which they will then inherit the appropriate DMA mask. The outstanding >> issue which remains is that we're still missing some way of preventing >> drivers simply clobbering that with a 64-bit mask later, but that's a >> more general problem[1]. Thanks, I have thought about this as well. As you have stated, it simply does not work for all at this point. > > I wonder if the driver is actually using IAS, OAS and UBS incorrectly. > We're using them to parameterise the DMA aperture, which is then used > to size the IOVA domain, but that's wrong because the IAS, OAS and UBS > are upper bounds and we can still end up allocating unusable/unreachable > addresses. > > So I do think that this should be fixed on the SMMU firmware node, rather > than restricting the range of each master device. I am more partial to the Will's idea, adding an override attribute would be the simplest at this point. > > Will ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-01-09 12:03 ` Will Deacon 2017-01-09 17:49 ` Feng Kan @ 2017-01-09 17:55 ` Robin Murphy 2017-07-12 21:07 ` Feng Kan 1 sibling, 1 reply; 14+ messages in thread From: Robin Murphy @ 2017-01-09 17:55 UTC (permalink / raw) To: linux-arm-kernel On 09/01/17 12:03, Will Deacon wrote: > On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote: >> On 06/01/17 23:21, Feng Kan wrote: >>> The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. >> >> "Mostly"? Have APM actually modified it (which I strongly doubt) or do >> you mean it's simply been integrated with the upper address lines tied >> off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave >> interfaces; that's all there is to it. Whether or not you use all of >> those bits is up to you as a system integrator. > > That's a good point; MMU-500 doesn't appear to let you change the IAS > anyway. That should also mean that UBS and OAS are unchanged. > >>> However, our internal bus is only 42 bits wide. Our IAS field is coded >>> as 48 bits, which cause IPA to truncated to 42 bits on the physical >>> bus. In order for our system to work with the arm-smmu.c, there needs >>> to be a way to force the ipa_size to 42. The current internal solution >>> is to use the cpuid, but that is quite ugly. I was thinking of using >>> the model >>> as indication to right the ipa_size, but I am not too sure of the ACPI >>> side. Would it be okay to add an APM MMU500 variant? I would also >>> appreciated it if you guys have any alternate solutions. >> >> This is something we've been axpecting to run into for a while now - the >> appropriate solution is to use a "dma-ranges" property on the master >> device(s) to describe that they have 42 bits of address wired up, from >> which they will then inherit the appropriate DMA mask. The outstanding >> issue which remains is that we're still missing some way of preventing >> drivers simply clobbering that with a 64-bit mask later, but that's a >> more general problem[1]. > > I wonder if the driver is actually using IAS, OAS and UBS incorrectly. > We're using them to parameterise the DMA aperture, which is then used > to size the IOVA domain, but that's wrong because the IAS, OAS and UBS > are upper bounds and we can still end up allocating unusable/unreachable > addresses. It's not incorrect, it's simply all we know at that point. From inside the SMMU, we can't tell how many of the bits we have are actually wired up externally, which is why we always take the intersection of the DMA aperture and the given device's DMA mask at the point of IOVA allocation. You can reproduce much the same thing on your Juno if you fancy - just make the HDLCD or PL330 driver set a DMA mask larger than the default 32 bits and the top 8 bits of the MMU-401's 40-bit input being tied off to 0 will become apparent (the only difference being there's not actually anything at the master end they could be wired to either). > So I do think that this should be fixed on the SMMU firmware node, rather > than restricting the range of each master device. In general, it's a per-master thing which "dma-ranges" is exactly the correct tool to describe - a property on the SMMU would just be a weird nonstandard shorthand for a very particular case (it breaks as soon as you have some *more* limited, e.g. 32-bit, device in the same system). The fact that every master in this case apparently has the same capability is just happenstance. Robin. > > Will > ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-01-09 17:55 ` Robin Murphy @ 2017-07-12 21:07 ` Feng Kan 2017-07-12 23:28 ` Al Stone 0 siblings, 1 reply; 14+ messages in thread From: Feng Kan @ 2017-07-12 21:07 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 9, 2017 at 9:55 AM, Robin Murphy <robin.murphy@arm.com> wrote: > On 09/01/17 12:03, Will Deacon wrote: >> On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote: >>> On 06/01/17 23:21, Feng Kan wrote: >>>> The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. >>> >>> "Mostly"? Have APM actually modified it (which I strongly doubt) or do >>> you mean it's simply been integrated with the upper address lines tied >>> off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave >>> interfaces; that's all there is to it. Whether or not you use all of >>> those bits is up to you as a system integrator. >> >> That's a good point; MMU-500 doesn't appear to let you change the IAS >> anyway. That should also mean that UBS and OAS are unchanged. >> >>>> However, our internal bus is only 42 bits wide. Our IAS field is coded >>>> as 48 bits, which cause IPA to truncated to 42 bits on the physical >>>> bus. In order for our system to work with the arm-smmu.c, there needs >>>> to be a way to force the ipa_size to 42. The current internal solution >>>> is to use the cpuid, but that is quite ugly. I was thinking of using >>>> the model >>>> as indication to right the ipa_size, but I am not too sure of the ACPI >>>> side. Would it be okay to add an APM MMU500 variant? I would also >>>> appreciated it if you guys have any alternate solutions. >>> >>> This is something we've been axpecting to run into for a while now - the >>> appropriate solution is to use a "dma-ranges" property on the master >>> device(s) to describe that they have 42 bits of address wired up, from >>> which they will then inherit the appropriate DMA mask. The outstanding >>> issue which remains is that we're still missing some way of preventing >>> drivers simply clobbering that with a 64-bit mask later, but that's a >>> more general problem[1]. >> >> I wonder if the driver is actually using IAS, OAS and UBS incorrectly. >> We're using them to parameterise the DMA aperture, which is then used >> to size the IOVA domain, but that's wrong because the IAS, OAS and UBS >> are upper bounds and we can still end up allocating unusable/unreachable >> addresses. > > It's not incorrect, it's simply all we know at that point. From inside > the SMMU, we can't tell how many of the bits we have are actually wired > up externally, which is why we always take the intersection of the DMA > aperture and the given device's DMA mask at the point of IOVA allocation. > > You can reproduce much the same thing on your Juno if you fancy - just > make the HDLCD or PL330 driver set a DMA mask larger than the default 32 > bits and the top 8 bits of the MMU-401's 40-bit input being tied off to > 0 will become apparent (the only difference being there's not actually > anything at the master end they could be wired to either). > >> So I do think that this should be fixed on the SMMU firmware node, rather >> than restricting the range of each master device. > > In general, it's a per-master thing which "dma-ranges" is exactly the > correct tool to describe - a property on the SMMU would just be a weird > nonstandard shorthand for a very particular case (it breaks as soon as > you have some *more* limited, e.g. 32-bit, device in the same system). > The fact that every master in this case apparently has the same > capability is just happenstance. I know this thread is pretty old, so sorry about that. Given that using dma-range is not an option in our system since it uses ACPI tables. There doesn't seem to be any more activity on this. I see that the IORT table has added support for CAVIUM_SMMUV2, would it be possible to add APM_SMMUV2 and key the ipa_size to less or equal to 42 bits. This way, we can fix the problem in the SMMUv2 driver and not worry about the greater DMA layer changes that could affect everyone. > > Robin. > >> >> Will >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-07-12 21:07 ` Feng Kan @ 2017-07-12 23:28 ` Al Stone 2017-07-12 23:31 ` Feng Kan 0 siblings, 1 reply; 14+ messages in thread From: Al Stone @ 2017-07-12 23:28 UTC (permalink / raw) To: linux-arm-kernel On 07/12/2017 03:07 PM, Feng Kan wrote: > On Mon, Jan 9, 2017 at 9:55 AM, Robin Murphy <robin.murphy@arm.com> wrote: >> On 09/01/17 12:03, Will Deacon wrote: >>> On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote: >>>> On 06/01/17 23:21, Feng Kan wrote: >>>>> The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. >>>> >>>> "Mostly"? Have APM actually modified it (which I strongly doubt) or do >>>> you mean it's simply been integrated with the upper address lines tied >>>> off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave >>>> interfaces; that's all there is to it. Whether or not you use all of >>>> those bits is up to you as a system integrator. >>> >>> That's a good point; MMU-500 doesn't appear to let you change the IAS >>> anyway. That should also mean that UBS and OAS are unchanged. >>> >>>>> However, our internal bus is only 42 bits wide. Our IAS field is coded >>>>> as 48 bits, which cause IPA to truncated to 42 bits on the physical >>>>> bus. In order for our system to work with the arm-smmu.c, there needs >>>>> to be a way to force the ipa_size to 42. The current internal solution >>>>> is to use the cpuid, but that is quite ugly. I was thinking of using >>>>> the model >>>>> as indication to right the ipa_size, but I am not too sure of the ACPI >>>>> side. Would it be okay to add an APM MMU500 variant? I would also >>>>> appreciated it if you guys have any alternate solutions. >>>> >>>> This is something we've been axpecting to run into for a while now - the >>>> appropriate solution is to use a "dma-ranges" property on the master >>>> device(s) to describe that they have 42 bits of address wired up, from >>>> which they will then inherit the appropriate DMA mask. The outstanding >>>> issue which remains is that we're still missing some way of preventing >>>> drivers simply clobbering that with a 64-bit mask later, but that's a >>>> more general problem[1]. >>> >>> I wonder if the driver is actually using IAS, OAS and UBS incorrectly. >>> We're using them to parameterise the DMA aperture, which is then used >>> to size the IOVA domain, but that's wrong because the IAS, OAS and UBS >>> are upper bounds and we can still end up allocating unusable/unreachable >>> addresses. >> >> It's not incorrect, it's simply all we know at that point. From inside >> the SMMU, we can't tell how many of the bits we have are actually wired >> up externally, which is why we always take the intersection of the DMA >> aperture and the given device's DMA mask at the point of IOVA allocation. >> >> You can reproduce much the same thing on your Juno if you fancy - just >> make the HDLCD or PL330 driver set a DMA mask larger than the default 32 >> bits and the top 8 bits of the MMU-401's 40-bit input being tied off to >> 0 will become apparent (the only difference being there's not actually >> anything at the master end they could be wired to either). >> >>> So I do think that this should be fixed on the SMMU firmware node, rather >>> than restricting the range of each master device. >> >> In general, it's a per-master thing which "dma-ranges" is exactly the >> correct tool to describe - a property on the SMMU would just be a weird >> nonstandard shorthand for a very particular case (it breaks as soon as >> you have some *more* limited, e.g. 32-bit, device in the same system). >> The fact that every master in this case apparently has the same >> capability is just happenstance. > I know this thread is pretty old, so sorry about that. Given that > using dma-range is > not an option in our system since it uses ACPI tables. There doesn't seem to be > any more activity on this. I see that the IORT table has added support > for CAVIUM_SMMUV2, > would it be possible to add APM_SMMUV2 and key the ipa_size to less or equal > to 42 bits. This way, we can fix the problem in the SMMUv2 driver and not worry > about the greater DMA layer changes that could affect everyone. Adding Charles to the To: list -- he's the one you'll need to contact about modifying IORT content since he controls the document. I would guess it's possible to add an entry for APM but I don't get to decide that, Charles does. Contacting him directly would be your best bet, though. >> >> Robin. >> >>> >>> Will >>> >> > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- ciao, al ----------------------------------- Al Stone Software Engineer Red Hat, Inc. ahs3 at redhat.com ----------------------------------- ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-07-12 23:28 ` Al Stone @ 2017-07-12 23:31 ` Feng Kan 2017-07-13 10:03 ` Robin Murphy 0 siblings, 1 reply; 14+ messages in thread From: Feng Kan @ 2017-07-12 23:31 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jul 12, 2017 at 4:28 PM, Al Stone <ahs3@redhat.com> wrote: > On 07/12/2017 03:07 PM, Feng Kan wrote: >> On Mon, Jan 9, 2017 at 9:55 AM, Robin Murphy <robin.murphy@arm.com> wrote: >>> On 09/01/17 12:03, Will Deacon wrote: >>>> On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote: >>>>> On 06/01/17 23:21, Feng Kan wrote: >>>>>> The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. >>>>> >>>>> "Mostly"? Have APM actually modified it (which I strongly doubt) or do >>>>> you mean it's simply been integrated with the upper address lines tied >>>>> off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave >>>>> interfaces; that's all there is to it. Whether or not you use all of >>>>> those bits is up to you as a system integrator. >>>> >>>> That's a good point; MMU-500 doesn't appear to let you change the IAS >>>> anyway. That should also mean that UBS and OAS are unchanged. >>>> >>>>>> However, our internal bus is only 42 bits wide. Our IAS field is coded >>>>>> as 48 bits, which cause IPA to truncated to 42 bits on the physical >>>>>> bus. In order for our system to work with the arm-smmu.c, there needs >>>>>> to be a way to force the ipa_size to 42. The current internal solution >>>>>> is to use the cpuid, but that is quite ugly. I was thinking of using >>>>>> the model >>>>>> as indication to right the ipa_size, but I am not too sure of the ACPI >>>>>> side. Would it be okay to add an APM MMU500 variant? I would also >>>>>> appreciated it if you guys have any alternate solutions. >>>>> >>>>> This is something we've been axpecting to run into for a while now - the >>>>> appropriate solution is to use a "dma-ranges" property on the master >>>>> device(s) to describe that they have 42 bits of address wired up, from >>>>> which they will then inherit the appropriate DMA mask. The outstanding >>>>> issue which remains is that we're still missing some way of preventing >>>>> drivers simply clobbering that with a 64-bit mask later, but that's a >>>>> more general problem[1]. >>>> >>>> I wonder if the driver is actually using IAS, OAS and UBS incorrectly. >>>> We're using them to parameterise the DMA aperture, which is then used >>>> to size the IOVA domain, but that's wrong because the IAS, OAS and UBS >>>> are upper bounds and we can still end up allocating unusable/unreachable >>>> addresses. >>> >>> It's not incorrect, it's simply all we know at that point. From inside >>> the SMMU, we can't tell how many of the bits we have are actually wired >>> up externally, which is why we always take the intersection of the DMA >>> aperture and the given device's DMA mask at the point of IOVA allocation. >>> >>> You can reproduce much the same thing on your Juno if you fancy - just >>> make the HDLCD or PL330 driver set a DMA mask larger than the default 32 >>> bits and the top 8 bits of the MMU-401's 40-bit input being tied off to >>> 0 will become apparent (the only difference being there's not actually >>> anything at the master end they could be wired to either). >>> >>>> So I do think that this should be fixed on the SMMU firmware node, rather >>>> than restricting the range of each master device. >>> >>> In general, it's a per-master thing which "dma-ranges" is exactly the >>> correct tool to describe - a property on the SMMU would just be a weird >>> nonstandard shorthand for a very particular case (it breaks as soon as >>> you have some *more* limited, e.g. 32-bit, device in the same system). >>> The fact that every master in this case apparently has the same >>> capability is just happenstance. >> I know this thread is pretty old, so sorry about that. Given that >> using dma-range is >> not an option in our system since it uses ACPI tables. There doesn't seem to be >> any more activity on this. I see that the IORT table has added support >> for CAVIUM_SMMUV2, >> would it be possible to add APM_SMMUV2 and key the ipa_size to less or equal >> to 42 bits. This way, we can fix the problem in the SMMUv2 driver and not worry >> about the greater DMA layer changes that could affect everyone. > > Adding Charles to the To: list -- he's the one you'll need to contact > about modifying IORT content since he controls the document. I would > guess it's possible to add an entry for APM but I don't get to decide > that, Charles does. Contacting him directly would be your best bet, > though. Thanks Al, I already send Charles an email. However, I think the SMMU driver maintainers at ARM should have the first say in if this change would be okay to do. > >>> >>> Robin. >>> >>>> >>>> Will >>>> >>> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel at lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> > > > -- > ciao, > al > ----------------------------------- > Al Stone > Software Engineer > Red Hat, Inc. > ahs3 at redhat.com > ----------------------------------- ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-07-12 23:31 ` Feng Kan @ 2017-07-13 10:03 ` Robin Murphy 2017-07-13 16:08 ` Feng Kan 0 siblings, 1 reply; 14+ messages in thread From: Robin Murphy @ 2017-07-13 10:03 UTC (permalink / raw) To: linux-arm-kernel On 13/07/17 00:31, Feng Kan wrote: > On Wed, Jul 12, 2017 at 4:28 PM, Al Stone <ahs3@redhat.com> wrote: >> On 07/12/2017 03:07 PM, Feng Kan wrote: >>> On Mon, Jan 9, 2017 at 9:55 AM, Robin Murphy <robin.murphy@arm.com> wrote: >>>> On 09/01/17 12:03, Will Deacon wrote: >>>>> On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote: >>>>>> On 06/01/17 23:21, Feng Kan wrote: >>>>>>> The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. >>>>>> >>>>>> "Mostly"? Have APM actually modified it (which I strongly doubt) or do >>>>>> you mean it's simply been integrated with the upper address lines tied >>>>>> off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave >>>>>> interfaces; that's all there is to it. Whether or not you use all of >>>>>> those bits is up to you as a system integrator. >>>>> >>>>> That's a good point; MMU-500 doesn't appear to let you change the IAS >>>>> anyway. That should also mean that UBS and OAS are unchanged. >>>>> >>>>>>> However, our internal bus is only 42 bits wide. Our IAS field is coded >>>>>>> as 48 bits, which cause IPA to truncated to 42 bits on the physical >>>>>>> bus. In order for our system to work with the arm-smmu.c, there needs >>>>>>> to be a way to force the ipa_size to 42. The current internal solution >>>>>>> is to use the cpuid, but that is quite ugly. I was thinking of using >>>>>>> the model >>>>>>> as indication to right the ipa_size, but I am not too sure of the ACPI >>>>>>> side. Would it be okay to add an APM MMU500 variant? I would also >>>>>>> appreciated it if you guys have any alternate solutions. >>>>>> >>>>>> This is something we've been axpecting to run into for a while now - the >>>>>> appropriate solution is to use a "dma-ranges" property on the master >>>>>> device(s) to describe that they have 42 bits of address wired up, from >>>>>> which they will then inherit the appropriate DMA mask. The outstanding >>>>>> issue which remains is that we're still missing some way of preventing >>>>>> drivers simply clobbering that with a 64-bit mask later, but that's a >>>>>> more general problem[1]. >>>>> >>>>> I wonder if the driver is actually using IAS, OAS and UBS incorrectly. >>>>> We're using them to parameterise the DMA aperture, which is then used >>>>> to size the IOVA domain, but that's wrong because the IAS, OAS and UBS >>>>> are upper bounds and we can still end up allocating unusable/unreachable >>>>> addresses. >>>> >>>> It's not incorrect, it's simply all we know at that point. From inside >>>> the SMMU, we can't tell how many of the bits we have are actually wired >>>> up externally, which is why we always take the intersection of the DMA >>>> aperture and the given device's DMA mask at the point of IOVA allocation. >>>> >>>> You can reproduce much the same thing on your Juno if you fancy - just >>>> make the HDLCD or PL330 driver set a DMA mask larger than the default 32 >>>> bits and the top 8 bits of the MMU-401's 40-bit input being tied off to >>>> 0 will become apparent (the only difference being there's not actually >>>> anything at the master end they could be wired to either). >>>> >>>>> So I do think that this should be fixed on the SMMU firmware node, rather >>>>> than restricting the range of each master device. >>>> >>>> In general, it's a per-master thing which "dma-ranges" is exactly the >>>> correct tool to describe - a property on the SMMU would just be a weird >>>> nonstandard shorthand for a very particular case (it breaks as soon as >>>> you have some *more* limited, e.g. 32-bit, device in the same system). >>>> The fact that every master in this case apparently has the same >>>> capability is just happenstance. >>> I know this thread is pretty old, so sorry about that. Given that >>> using dma-range is >>> not an option in our system since it uses ACPI tables. There doesn't seem to be >>> any more activity on this. I see that the IORT table has added support >>> for CAVIUM_SMMUV2, >>> would it be possible to add APM_SMMUV2 and key the ipa_size to less or equal >>> to 42 bits. This way, we can fix the problem in the SMMUv2 driver and not worry >>> about the greater DMA layer changes that could affect everyone. >> >> Adding Charles to the To: list -- he's the one you'll need to contact >> about modifying IORT content since he controls the document. I would >> guess it's possible to add an entry for APM but I don't get to decide >> that, Charles does. Contacting him directly would be your best bet, >> though. > Thanks Al, I already send Charles an email. However, I think the SMMU driver > maintainers at ARM should have the first say in if this change would > be okay to do. Cavium are not using MMU-500 - the ThunderX SMMU is their own in-house microarchitecture (with its own bugs and foibles), hence it rightly gets its own implementation identifier. I think Lorenzo has plans to wire up support for the IORT "Device memory address size limit" field, which is the correct way for ACPI to describe the upstream bus width of your MMU-500 integration (and all the other IOMMU integrations facing the same issue). Robin. ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-07-13 10:03 ` Robin Murphy @ 2017-07-13 16:08 ` Feng Kan 2017-07-13 16:41 ` Edward Gasiorowski 2017-07-13 16:52 ` Lorenzo Pieralisi 0 siblings, 2 replies; 14+ messages in thread From: Feng Kan @ 2017-07-13 16:08 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 13, 2017 at 3:03 AM, Robin Murphy <robin.murphy@arm.com> wrote: > On 13/07/17 00:31, Feng Kan wrote: >> On Wed, Jul 12, 2017 at 4:28 PM, Al Stone <ahs3@redhat.com> wrote: >>> On 07/12/2017 03:07 PM, Feng Kan wrote: >>>> On Mon, Jan 9, 2017 at 9:55 AM, Robin Murphy <robin.murphy@arm.com> wrote: >>>>> On 09/01/17 12:03, Will Deacon wrote: >>>>>> On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote: >>>>>>> On 06/01/17 23:21, Feng Kan wrote: >>>>>>>> The APM IOMMU implementation is mostly just the ARM SMMU 500 variant. >>>>>>> >>>>>>> "Mostly"? Have APM actually modified it (which I strongly doubt) or do >>>>>>> you mean it's simply been integrated with the upper address lines tied >>>>>>> off? MMU-500 reports a 48-bit IAS because MMU-500 has 48-bit-wide slave >>>>>>> interfaces; that's all there is to it. Whether or not you use all of >>>>>>> those bits is up to you as a system integrator. >>>>>> >>>>>> That's a good point; MMU-500 doesn't appear to let you change the IAS >>>>>> anyway. That should also mean that UBS and OAS are unchanged. >>>>>> >>>>>>>> However, our internal bus is only 42 bits wide. Our IAS field is coded >>>>>>>> as 48 bits, which cause IPA to truncated to 42 bits on the physical >>>>>>>> bus. In order for our system to work with the arm-smmu.c, there needs >>>>>>>> to be a way to force the ipa_size to 42. The current internal solution >>>>>>>> is to use the cpuid, but that is quite ugly. I was thinking of using >>>>>>>> the model >>>>>>>> as indication to right the ipa_size, but I am not too sure of the ACPI >>>>>>>> side. Would it be okay to add an APM MMU500 variant? I would also >>>>>>>> appreciated it if you guys have any alternate solutions. >>>>>>> >>>>>>> This is something we've been axpecting to run into for a while now - the >>>>>>> appropriate solution is to use a "dma-ranges" property on the master >>>>>>> device(s) to describe that they have 42 bits of address wired up, from >>>>>>> which they will then inherit the appropriate DMA mask. The outstanding >>>>>>> issue which remains is that we're still missing some way of preventing >>>>>>> drivers simply clobbering that with a 64-bit mask later, but that's a >>>>>>> more general problem[1]. >>>>>> >>>>>> I wonder if the driver is actually using IAS, OAS and UBS incorrectly. >>>>>> We're using them to parameterise the DMA aperture, which is then used >>>>>> to size the IOVA domain, but that's wrong because the IAS, OAS and UBS >>>>>> are upper bounds and we can still end up allocating unusable/unreachable >>>>>> addresses. >>>>> >>>>> It's not incorrect, it's simply all we know at that point. From inside >>>>> the SMMU, we can't tell how many of the bits we have are actually wired >>>>> up externally, which is why we always take the intersection of the DMA >>>>> aperture and the given device's DMA mask at the point of IOVA allocation. >>>>> >>>>> You can reproduce much the same thing on your Juno if you fancy - just >>>>> make the HDLCD or PL330 driver set a DMA mask larger than the default 32 >>>>> bits and the top 8 bits of the MMU-401's 40-bit input being tied off to >>>>> 0 will become apparent (the only difference being there's not actually >>>>> anything at the master end they could be wired to either). >>>>> >>>>>> So I do think that this should be fixed on the SMMU firmware node, rather >>>>>> than restricting the range of each master device. >>>>> >>>>> In general, it's a per-master thing which "dma-ranges" is exactly the >>>>> correct tool to describe - a property on the SMMU would just be a weird >>>>> nonstandard shorthand for a very particular case (it breaks as soon as >>>>> you have some *more* limited, e.g. 32-bit, device in the same system). >>>>> The fact that every master in this case apparently has the same >>>>> capability is just happenstance. >>>> I know this thread is pretty old, so sorry about that. Given that >>>> using dma-range is >>>> not an option in our system since it uses ACPI tables. There doesn't seem to be >>>> any more activity on this. I see that the IORT table has added support >>>> for CAVIUM_SMMUV2, >>>> would it be possible to add APM_SMMUV2 and key the ipa_size to less or equal >>>> to 42 bits. This way, we can fix the problem in the SMMUv2 driver and not worry >>>> about the greater DMA layer changes that could affect everyone. >>> >>> Adding Charles to the To: list -- he's the one you'll need to contact >>> about modifying IORT content since he controls the document. I would >>> guess it's possible to add an entry for APM but I don't get to decide >>> that, Charles does. Contacting him directly would be your best bet, >>> though. >> Thanks Al, I already send Charles an email. However, I think the SMMU driver >> maintainers at ARM should have the first say in if this change would >> be okay to do. > > Cavium are not using MMU-500 - the ThunderX SMMU is their own in-house > microarchitecture (with its own bugs and foibles), hence it rightly gets > its own implementation identifier. I think Lorenzo has plans to wire up > support for the IORT "Device memory address size limit" field, which is > the correct way for ACPI to describe the upstream bus width of your > MMU-500 integration (and all the other IOMMU integrations facing the > same issue). This is great, thanks. Lorenzo, May I ask when will this be ready. We would very much like this change to push out into the latest CentOS release. > > Robin. ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-07-13 16:08 ` Feng Kan @ 2017-07-13 16:41 ` Edward Gasiorowski 2017-07-13 16:52 ` Lorenzo Pieralisi 1 sibling, 0 replies; 14+ messages in thread From: Edward Gasiorowski @ 2017-07-13 16:41 UTC (permalink / raw) To: linux-arm-kernel Lorenzo, Would also like to get this into RHEL-7.4.. -----Original Message----- From: Feng Kan [mailto:fkan at apm.com] Sent: Thursday, July 13, 2017 9:09 AM To: Robin Murphy Cc: Al Stone; Mark Salter; Jeff Underhill; Ed Gasiorowski (egasiorowski at apm.com) (egasiorowski at apm.com); Charles Garcia-Tobin; Will Deacon; linux-arm-kernel at lists.infradead.org; Lorenzo Pieralisi Subject: Re: APM smmu implementation On Thu, Jul 13, 2017 at 3:03 AM, Robin Murphy <robin.murphy@arm.com> wrote: > On 13/07/17 00:31, Feng Kan wrote: >> On Wed, Jul 12, 2017 at 4:28 PM, Al Stone <ahs3@redhat.com> wrote: >>> On 07/12/2017 03:07 PM, Feng Kan wrote: >>>> On Mon, Jan 9, 2017 at 9:55 AM, Robin Murphy <robin.murphy@arm.com> >>>> wrote: >>>>> On 09/01/17 12:03, Will Deacon wrote: >>>>>> On Mon, Jan 09, 2017 at 11:34:42AM +0000, Robin Murphy wrote: >>>>>>> On 06/01/17 23:21, Feng Kan wrote: >>>>>>>> The APM IOMMU implementation is mostly just the ARM SMMU 500 >>>>>>>> variant. >>>>>>> >>>>>>> "Mostly"? Have APM actually modified it (which I strongly doubt) >>>>>>> or do you mean it's simply been integrated with the upper >>>>>>> address lines tied off? MMU-500 reports a 48-bit IAS because >>>>>>> MMU-500 has 48-bit-wide slave interfaces; that's all there is to >>>>>>> it. Whether or not you use all of those bits is up to you as a >>>>>>> system integrator. >>>>>> >>>>>> That's a good point; MMU-500 doesn't appear to let you change the >>>>>> IAS anyway. That should also mean that UBS and OAS are unchanged. >>>>>> >>>>>>>> However, our internal bus is only 42 bits wide. Our IAS field >>>>>>>> is coded as 48 bits, which cause IPA to truncated to 42 bits on >>>>>>>> the physical bus. In order for our system to work with the >>>>>>>> arm-smmu.c, there needs to be a way to force the ipa_size to >>>>>>>> 42. The current internal solution is to use the cpuid, but that >>>>>>>> is quite ugly. I was thinking of using the model as indication >>>>>>>> to right the ipa_size, but I am not too sure of the ACPI side. >>>>>>>> Would it be okay to add an APM MMU500 variant? I would also >>>>>>>> appreciated it if you guys have any alternate solutions. >>>>>>> >>>>>>> This is something we've been axpecting to run into for a while >>>>>>> now - the appropriate solution is to use a "dma-ranges" property >>>>>>> on the master >>>>>>> device(s) to describe that they have 42 bits of address wired >>>>>>> up, from which they will then inherit the appropriate DMA mask. >>>>>>> The outstanding issue which remains is that we're still missing >>>>>>> some way of preventing drivers simply clobbering that with a >>>>>>> 64-bit mask later, but that's a more general problem[1]. >>>>>> >>>>>> I wonder if the driver is actually using IAS, OAS and UBS >>>>>> incorrectly. >>>>>> We're using them to parameterise the DMA aperture, which is then >>>>>> used to size the IOVA domain, but that's wrong because the IAS, >>>>>> OAS and UBS are upper bounds and we can still end up allocating >>>>>> unusable/unreachable addresses. >>>>> >>>>> It's not incorrect, it's simply all we know at that point. From >>>>> inside the SMMU, we can't tell how many of the bits we have are >>>>> actually wired up externally, which is why we always take the >>>>> intersection of the DMA aperture and the given device's DMA mask at >>>>> the point of IOVA allocation. >>>>> >>>>> You can reproduce much the same thing on your Juno if you fancy - >>>>> just make the HDLCD or PL330 driver set a DMA mask larger than the >>>>> default 32 bits and the top 8 bits of the MMU-401's 40-bit input >>>>> being tied off to >>>>> 0 will become apparent (the only difference being there's not >>>>> actually anything at the master end they could be wired to either). >>>>> >>>>>> So I do think that this should be fixed on the SMMU firmware >>>>>> node, rather than restricting the range of each master device. >>>>> >>>>> In general, it's a per-master thing which "dma-ranges" is exactly >>>>> the correct tool to describe - a property on the SMMU would just >>>>> be a weird nonstandard shorthand for a very particular case (it >>>>> breaks as soon as you have some *more* limited, e.g. 32-bit, device in >>>>> the same system). >>>>> The fact that every master in this case apparently has the same >>>>> capability is just happenstance. >>>> I know this thread is pretty old, so sorry about that. Given that >>>> using dma-range is not an option in our system since it uses ACPI >>>> tables. There doesn't seem to be any more activity on this. I see >>>> that the IORT table has added support for CAVIUM_SMMUV2, would it >>>> be possible to add APM_SMMUV2 and key the ipa_size to less or equal >>>> to 42 bits. This way, we can fix the problem in the SMMUv2 driver >>>> and not worry about the greater DMA layer changes that could affect >>>> everyone. >>> >>> Adding Charles to the To: list -- he's the one you'll need to >>> contact about modifying IORT content since he controls the document. >>> I would guess it's possible to add an entry for APM but I don't get >>> to decide that, Charles does. Contacting him directly would be your >>> best bet, though. >> Thanks Al, I already send Charles an email. However, I think the SMMU >> driver maintainers at ARM should have the first say in if this change >> would be okay to do. > > Cavium are not using MMU-500 - the ThunderX SMMU is their own in-house > microarchitecture (with its own bugs and foibles), hence it rightly > gets its own implementation identifier. I think Lorenzo has plans to > wire up support for the IORT "Device memory address size limit" field, > which is the correct way for ACPI to describe the upstream bus width > of your > MMU-500 integration (and all the other IOMMU integrations facing the > same issue). This is great, thanks. Lorenzo, May I ask when will this be ready. We would very much like this change to push out into the latest CentOS release. > > Robin. -- CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to Applied Micro Circuits Corporation or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message. ^ permalink raw reply [flat|nested] 14+ messages in thread
* APM smmu implementation 2017-07-13 16:08 ` Feng Kan 2017-07-13 16:41 ` Edward Gasiorowski @ 2017-07-13 16:52 ` Lorenzo Pieralisi 1 sibling, 0 replies; 14+ messages in thread From: Lorenzo Pieralisi @ 2017-07-13 16:52 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jul 13, 2017 at 09:08:38AM -0700, Feng Kan wrote: [...] > > Cavium are not using MMU-500 - the ThunderX SMMU is their own in-house > > microarchitecture (with its own bugs and foibles), hence it rightly gets > > its own implementation identifier. I think Lorenzo has plans to wire up > > support for the IORT "Device memory address size limit" field, which is > > the correct way for ACPI to describe the upstream bus width of your > > MMU-500 integration (and all the other IOMMU integrations facing the > > same issue). > This is great, thanks. > > Lorenzo, May I ask when will this be ready. We would very much like > this change to push out into the latest CentOS release. I should be able to post a patch at v4.13-rc1 aiming for v4.14. Lorenzo ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-07-13 16:52 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-06 23:21 APM smmu implementation Feng Kan 2017-01-09 10:54 ` Will Deacon 2017-01-09 17:37 ` Feng Kan 2017-01-09 11:34 ` Robin Murphy 2017-01-09 12:03 ` Will Deacon 2017-01-09 17:49 ` Feng Kan 2017-01-09 17:55 ` Robin Murphy 2017-07-12 21:07 ` Feng Kan 2017-07-12 23:28 ` Al Stone 2017-07-12 23:31 ` Feng Kan 2017-07-13 10:03 ` Robin Murphy 2017-07-13 16:08 ` Feng Kan 2017-07-13 16:41 ` Edward Gasiorowski 2017-07-13 16:52 ` Lorenzo Pieralisi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).