Re: DMA error when sg->offset value is greater than PAGE_SIZE in Intel IOMMU

From: Casey Leedom
Date: Tue Sep 26 2017 - 16:50:13 EST


So just to be 100% sure I understand the patch you're proposing, you got
the first use of VTD_PAGE_SHIFT wrong; it should have been VTD_PAGE_MASK? I.e.

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 6784a05..d43b566 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2254,10 +2254,13 @@ static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
uint64_t tmp;

if (!sg_res) {
+ size_t off = sg->offset & ~VTD_PAGE_MASK;
sg_res = aligned_nrpages(sg->offset, sg->length);
- sg->dma_address = ((dma_addr_t)iov_pfn << VTD_PAGE_SHIFT) + sg->offset;
+ sg->dma_address = ((dma_addr_t)
+ (iov_pfn + sg->offset) << VTD_PAGE_SHIFT) + off;
sg->dma_length = sg->length;
- pteval = page_to_phys(sg_page(sg)) | prot;
+ pteval = (page_to_phys(sg_page(sg)) +
+ (sg->offset << VTD_PAGE_SHIFT)) | prot;
phys_pfn = pteval >> VTD_PAGE_SHIFT;
}

??? And I'm still confused about this portion:

- sg->dma_address = ((dma_addr_t)iov_pfn << VTD_PAGE_SHIFT) + sg->offset;
+ sg->dma_address = ((dma_addr_t)
+ (iov_pfn + sg->offset) << VTD_PAGE_SHIFT) + off;

Isn't iov_pfn a Physical Page Number and we're adding a Byte Offset
to that? I would have though that it would be more like:

size_t page_off = sg->offset & ~VTD_PAGE_MASK;
unsigned long pfn_off = sg->offset >> VTD_PAGE_MASK;
...
sg->dma_address = ((dma_addr_t)
(iov_pfn + pfn_off) << VTD_PAGE_SHIFT) + page_off;

I want to be sure that Harsh has a concrete patch to work with when he
wakes up.

How about it Robin, Dan, David, Herbert, what do you guys think of Raj's
proposed patch?

Casey