* REGRESSION: iommu fails to take address limit into account @ 2018-05-25 9:48 Ard Biesheuvel 2018-05-25 10:34 ` Robin Murphy 2018-05-25 10:35 ` Ard Biesheuvel 0 siblings, 2 replies; 5+ messages in thread From: Ard Biesheuvel @ 2018-05-25 9:48 UTC (permalink / raw) To: linux-arm-kernel Hello all, I am looking into an issue where a platform device is wired to a MMU-500, and for some reason (which is under investigation) the platform device can not drive all address bits. I can work around this by limiting the DMA mask to 40 bits in the driver. However, the IORT table allows me to set the address limit as well, and so I was expecting this to be taken into account by the SMMU driver. When the iort/iommu layer sets up the DMA operations, iommu_dma_init_domain() is entered with the expected values: base == 0 size == 0x100_0000_0000 However, the iommu layer ends up generating IOVA addresses that have bits [47:40] set (which is what the MMU-500 supports). Looking closer, this is not surprising, given that the end_pfn variable that is calculated in iommu_dma_init_domain() is no longer used after Zhen's patch aa3ac9469c185 ("iommu/iova: Make dma_32bit_pfn implicit") was applied. So effectively, this is a regression, and I would like your help figuring out how to go about fixing this. Thanks, Ard. ^ permalink raw reply [flat|nested] 5+ messages in thread
* REGRESSION: iommu fails to take address limit into account 2018-05-25 9:48 REGRESSION: iommu fails to take address limit into account Ard Biesheuvel @ 2018-05-25 10:34 ` Robin Murphy 2018-05-25 10:35 ` Ard Biesheuvel 1 sibling, 0 replies; 5+ messages in thread From: Robin Murphy @ 2018-05-25 10:34 UTC (permalink / raw) To: linux-arm-kernel Hi Ard, On 25/05/18 10:48, Ard Biesheuvel wrote: > Hello all, > > I am looking into an issue where a platform device is wired to a > MMU-500, and for some reason (which is under investigation) the > platform device can not drive all address bits. I can work around this > by limiting the DMA mask to 40 bits in the driver. However, the IORT > table allows me to set the address limit as well, and so I was > expecting this to be taken into account by the SMMU driver. > > When the iort/iommu layer sets up the DMA operations, > iommu_dma_init_domain() is entered with the expected values: > > base == 0 > size == 0x100_0000_0000 > > However, the iommu layer ends up generating IOVA addresses that have > bits [47:40] set (which is what the MMU-500 supports). Looking closer, > this is not surprising, given that the end_pfn variable that is > calculated in iommu_dma_init_domain() is no longer used after Zhen's > patch aa3ac9469c185 ("iommu/iova: Make dma_32bit_pfn implicit") was > applied. > > So effectively, this is a regression, and I would like your help > figuring out how to go about fixing this. Note that the size passed to iommu_dma_init_domain() has *never* been any kind of enforced limit, so nothing is actually regressing here. If the device master interface is natively >40 bits wide (such that the driver would expect to set a larger DMA mask) and the restriction is purely in the interconnect between device and SMMU, then that's the case which has always been broken. I do have a long-standing plan of attack for fixing the underlying issue (of actually enforcing any dma mask restriction described by IORT or DT "dma-ranges"), but it's one of those things that's tied to a whole bunch of other rework and fixes in order to be viable. I'm about to go off on holiday for a while but I should hopefully have the bandwidth to start looking at this seriously once I get back (frankly it's been getting bumped down my to-do list for far too long now...) Robin. ^ permalink raw reply [flat|nested] 5+ messages in thread
* REGRESSION: iommu fails to take address limit into account 2018-05-25 9:48 REGRESSION: iommu fails to take address limit into account Ard Biesheuvel 2018-05-25 10:34 ` Robin Murphy @ 2018-05-25 10:35 ` Ard Biesheuvel 2018-05-25 10:49 ` Robin Murphy 1 sibling, 1 reply; 5+ messages in thread From: Ard Biesheuvel @ 2018-05-25 10:35 UTC (permalink / raw) To: linux-arm-kernel On 25 May 2018 at 11:48, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > Hello all, > > I am looking into an issue where a platform device is wired to a > MMU-500, and for some reason (which is under investigation) the > platform device can not drive all address bits. I can work around this > by limiting the DMA mask to 40 bits in the driver. However, the IORT > table allows me to set the address limit as well, and so I was > expecting this to be taken into account by the SMMU driver. > > When the iort/iommu layer sets up the DMA operations, > iommu_dma_init_domain() is entered with the expected values: > > base == 0 > size == 0x100_0000_0000 > > However, the iommu layer ends up generating IOVA addresses that have > bits [47:40] set (which is what the MMU-500 supports). Looking closer, > this is not surprising, given that the end_pfn variable that is > calculated in iommu_dma_init_domain() is no longer used after Zhen's > patch aa3ac9469c185 ("iommu/iova: Make dma_32bit_pfn implicit") was > applied. > > So effectively, this is a regression, and I would like your help > figuring out how to go about fixing this. > I have narrowed it down a bit further: even though the IOVA range ignores the IORT address limit, the device's DMA mask is set correctly. The only problem is that the driver (like all drivers afaict) does not take into account the fact that its DMA mask has already been set by the bus layer before its probe function is called. I could add something like this - if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) + if (dma_get_mask(&pdev->dev) == DMA_BIT_MASK(32) && + dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) dev_warn(&pdev->dev, "Failed to enable 64-bit DMA\n"); to only override the mask if it is not at its default value, but it feels like papering over the problem given that most drivers appear to ignore the preset mask as well. ^ permalink raw reply [flat|nested] 5+ messages in thread
* REGRESSION: iommu fails to take address limit into account 2018-05-25 10:35 ` Ard Biesheuvel @ 2018-05-25 10:49 ` Robin Murphy 2018-05-25 10:52 ` Ard Biesheuvel 0 siblings, 1 reply; 5+ messages in thread From: Robin Murphy @ 2018-05-25 10:49 UTC (permalink / raw) To: linux-arm-kernel On 25/05/18 11:35, Ard Biesheuvel wrote: > On 25 May 2018 at 11:48, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> Hello all, >> >> I am looking into an issue where a platform device is wired to a >> MMU-500, and for some reason (which is under investigation) the >> platform device can not drive all address bits. I can work around this >> by limiting the DMA mask to 40 bits in the driver. However, the IORT >> table allows me to set the address limit as well, and so I was >> expecting this to be taken into account by the SMMU driver. >> >> When the iort/iommu layer sets up the DMA operations, >> iommu_dma_init_domain() is entered with the expected values: >> >> base == 0 >> size == 0x100_0000_0000 >> >> However, the iommu layer ends up generating IOVA addresses that have >> bits [47:40] set (which is what the MMU-500 supports). Looking closer, >> this is not surprising, given that the end_pfn variable that is >> calculated in iommu_dma_init_domain() is no longer used after Zhen's >> patch aa3ac9469c185 ("iommu/iova: Make dma_32bit_pfn implicit") was >> applied. >> >> So effectively, this is a regression, and I would like your help >> figuring out how to go about fixing this. >> > > I have narrowed it down a bit further: even though the IOVA range > ignores the IORT address limit, the device's DMA mask is set > correctly. The only problem is that the driver (like all drivers > afaict) does not take into account the fact that its DMA mask has > already been set by the bus layer before its probe function is called. > > I could add something like this > > - if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) > + if (dma_get_mask(&pdev->dev) == DMA_BIT_MASK(32) && > + dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) > dev_warn(&pdev->dev, "Failed to enable 64-bit DMA\n"); > > to only override the mask if it is not at its default value, but it > feels like papering over the problem given that most drivers appear to > ignore the preset mask as well. Yup, that's the crux of it - if, say, the firmware describes an explicit restriction to 32 bits, then drivers have no way to tell the difference between that and the default mask initialised by the bus code (which they *should* be able to widen), and at the moment neither does arch code or anyone else (and the current interface between arch code and firmware code does not help matters). Robin. ^ permalink raw reply [flat|nested] 5+ messages in thread
* REGRESSION: iommu fails to take address limit into account 2018-05-25 10:49 ` Robin Murphy @ 2018-05-25 10:52 ` Ard Biesheuvel 0 siblings, 0 replies; 5+ messages in thread From: Ard Biesheuvel @ 2018-05-25 10:52 UTC (permalink / raw) To: linux-arm-kernel On 25 May 2018 at 12:49, Robin Murphy <robin.murphy@arm.com> wrote: > On 25/05/18 11:35, Ard Biesheuvel wrote: >> >> On 25 May 2018 at 11:48, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >>> >>> Hello all, >>> >>> I am looking into an issue where a platform device is wired to a >>> MMU-500, and for some reason (which is under investigation) the >>> platform device can not drive all address bits. I can work around this >>> by limiting the DMA mask to 40 bits in the driver. However, the IORT >>> table allows me to set the address limit as well, and so I was >>> expecting this to be taken into account by the SMMU driver. >>> >>> When the iort/iommu layer sets up the DMA operations, >>> iommu_dma_init_domain() is entered with the expected values: >>> >>> base == 0 >>> size == 0x100_0000_0000 >>> >>> However, the iommu layer ends up generating IOVA addresses that have >>> bits [47:40] set (which is what the MMU-500 supports). Looking closer, >>> this is not surprising, given that the end_pfn variable that is >>> calculated in iommu_dma_init_domain() is no longer used after Zhen's >>> patch aa3ac9469c185 ("iommu/iova: Make dma_32bit_pfn implicit") was >>> applied. >>> >>> So effectively, this is a regression, and I would like your help >>> figuring out how to go about fixing this. >>> >> >> I have narrowed it down a bit further: even though the IOVA range >> ignores the IORT address limit, the device's DMA mask is set >> correctly. The only problem is that the driver (like all drivers >> afaict) does not take into account the fact that its DMA mask has >> already been set by the bus layer before its probe function is called. >> >> I could add something like this >> >> - if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) >> + if (dma_get_mask(&pdev->dev) == DMA_BIT_MASK(32) && >> + dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) >> dev_warn(&pdev->dev, "Failed to enable 64-bit DMA\n"); >> >> to only override the mask if it is not at its default value, but it >> feels like papering over the problem given that most drivers appear to >> ignore the preset mask as well. > > > Yup, that's the crux of it - if, say, the firmware describes an explicit > restriction to 32 bits, then drivers have no way to tell the difference > between that and the default mask initialised by the bus code (which they > *should* be able to widen), and at the moment neither does arch code or > anyone else (and the current interface between arch code and firmware code > does not help matters). > OK, so I guess we need an additional bus limit field that is initialized to ~0 and can only be narrowed, and take that into account when dma_set_mask_and_coherent() is called. Not pretty, but at least we won't have to fix all the drivers. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-25 10:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-25 9:48 REGRESSION: iommu fails to take address limit into account Ard Biesheuvel 2018-05-25 10:34 ` Robin Murphy 2018-05-25 10:35 ` Ard Biesheuvel 2018-05-25 10:49 ` Robin Murphy 2018-05-25 10:52 ` Ard Biesheuvel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.