Hi Robin, Shimoda-san, everyone, On Thu, Jan 26, 2017 at 1:27 AM, Robin Murphy <robin.murphy@xxxxxxx> wrote: > On 25/01/17 12:53, Yoshihiro Shimoda wrote: >> From: Magnus Damm <damm+renesas@xxxxxxxxxxxxx> >> >> To track mapped iova for a workaround code in the future. >> >> Signed-off-by: Magnus Damm <damm+renesas@xxxxxxxxxxxxx> >> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> >> --- >> drivers/iommu/dma-iommu.c | 29 +++++++++++++++++++++++------ >> include/linux/iommu.h | 2 ++ > > So what's being added here is a counter of allocations within the > iova_domain held by an iommu_dma_cookie? Then why is it all the way down > in the iommu_domain and not in the cookie? That's needlessly invasive - > it would be almost understandable (but still horrible) if you needed to > refer to it directly from the IOMMU driver, but as far as I can see you > don't. You are very correct about all points. =) As you noticed, the counter is kept per-domain. History is a bit blurry but I think the reason for my abstraction-breaking is that i decided to implement the simplest possible code to get the reset callback to near the iova code and the domain level seemed suitable as a first step. Moving it to a different level is of course no problem. My main concern for this series is however if a subsystem-level intrusive workaround like this ever could be beaten into acceptable form for upstream merge.The code is pretty simple - the main portion of the workaround is this counter that used to detect when the number of mapped pages drops back to zero together with the callback. But unless the code can be made pluggable it introduces overhead for all users. In your other email you asked what happens if the counter never drops to zero. You are right that this workaround is not a general-purpose fix suitable for all kinds of devices. Because of that the workaround is designed to be enabled on only some of the IPMMU instances on the system. It is also most likely an errata workaround for a particular ES version, so we would most likely have to enable it with soc_device_match(). I'll reply to patch 3/4 with some more details. Thanks! / magnus
