On 05/10/18 08:19, Christoph Hellwig wrote:
On Thu, Oct 04, 2018 at 04:47:37PM +0100, Robin Murphy wrote:
The original motivation for iommu_map_sg() was to give IOMMU drivers the
chance to map an IOVA-contiguous scatterlist as efficiently as they
could. It turns out that there isn't really much driver-specific
business involved there, so now that the default implementation is
mandatory let's just improve that - the main thing we're after is to use
larger pages wherever possible, and as long as domain->pgsize_bitmap
reflects reality, iommu_map() can already do that in a generic way. All
we need to do is detect physically-contiguous segments and batch them
into a single map operation, since whatever we do here is transparent to
our caller and not bound by any segment-length restrictions on the list
itself.
Speaking of efficiency, there's really very little point in duplicating
the checks that iommu_map() is going to do anyway, so those get cleared
up in the process.
Signed-off-by: Robin Murphy <[email protected]>
I like the idea, but I find the goto usage to jump back into the just
terminated loop highly confusing. Would it be that much worse to simply
duplicate the iommu_map call?
Yeah, I fiddled around for ages trying to find the cleanest approach,
but really there just doesn't seem to be one - I'd say the worst bit of
that goto is the even-more-subtle need for the explicit break. FWIW the
naive diff below is only actually +2 source lines, but the duplication
does also carry through to the object code (at least for my arm64 GCC7
build). I might have a quick hack around to see if I can do any better
with a do...while loop - of course what I *really* want is nested
function definitions, but that might just be brain damage from doing too
much MATLAB in the past ;)
Robin.
----->8-----
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8b22e0502349..4d43146720e9 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1686,15 +1686,12 @@ size_t iommu_map_sg(struct iommu_domain *domain,
unsigned long iova,
phys_addr_t s_phys = sg_phys(s);
if (len && s_phys != start + len) {
-do_map:
ret = iommu_map(domain, iova + mapped, start, len,
prot);
if (ret)
goto out_err;
mapped += len;
len = 0;
- if (!s)
- break;
}
if (len) {
@@ -1704,8 +1701,13 @@ size_t iommu_map_sg(struct iommu_domain *domain,
unsigned long iova,
start = s_phys;
}
}
- if (len)
- goto do_map;
+ if (len) {
+ ret = iommu_map(domain, iova + mapped, start, len, prot);
+ if (ret)
+ goto out_err;
+
+ mapped += len;
+ }
return mapped;
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
