Messages in this thread | | | Date | Sat, 07 Aug 2021 13:47:21 +0200 | From | "Sven Peter" <> | Subject | Re: [RFC PATCH 2/3] iommu/dma-iommu: Support iovad->granule > PAGE SIZE |
| |
On Fri, Aug 6, 2021, at 20:04, Robin Murphy wrote: > On 2021-08-06 16:55, Sven Peter via iommu wrote: > > @@ -1006,6 +1019,31 @@ static int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, > > if (dev_is_untrusted(dev)) > > return iommu_dma_map_sg_swiotlb(dev, sg, nents, dir, attrs); > > > > + /* > > + * If the IOMMU pagesize is larger than the CPU pagesize we will > > + * very likely run into sgs with a physical address that is not aligned > > + * to an IOMMU page boundary. Fall back to just mapping every entry > > + * independently with __iommu_dma_map then. > > Scatterlist segments often don't have nicely aligned ends, which is why > we already align things to IOVA granules in main loop here. I think in > principle we'd just need to move the non-IOVA-aligned part of the > address from sg->page to sg->offset in the temporary transformation for > the rest of the assumptions to hold. I don't blame you for being timid > about touching that, though - it took me 3 tries to get right when I > first wrote it... >
I've spent some time with that code now and I think we cannot use it but have to fall back to iommu_dma_map_sg_swiotlb (even though that swiotlb part is a lie then):
When we have sg_phys(s) = 0x802e65000 with s->offset = 0 the paddr is aligned to PAGE_SIZE but has an offset of 0x1000 from something the IOMMU can map. Now this would result in s->offset = -0x1000 which is already weird enough. Offset is unsigned (and 32bit) so this will actually look like s->offset = 0xfffff000 then, which isn't much better. And then sg_phys(s) = 0x902e64000 (instead of 0x802e64000) and we'll map some random memory in iommu_map_sg_atomic and a little bit later everything explodes.
Now I could probably adjust the phys addr backwards and make sure offset is always positive (and possibly larger than PAGE_SIZE) and later restore it in __finalise_sg then but I feel like that's pushing this a little bit too far.
Sven
| |