Re: [RFC PATCH] x86: Allow IOMMU drivers to Hook arch_setup_dma_ops()

[David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2581 bytes --]

On Mon, 2016-10-31 at 11:45 +0000, Robin Murphy wrote:
> Hi David,
> 
> On 30/10/16 19:14, David Woodhouse wrote:
> > 
> > The Intel IOMMU code registers a notifier and plays nasty tricks to fill
> > device pointers in with its DMAR scope tables when devices are detected,
> > then later compares the device pointers in those tables when it needs to
> > find the IOMMU unit for a given device.
> > 
> > If we let it use arch_setup_dma_ops() to match the device to an IOMMU
> > unit at device_register() time, this gets a whole lot saner.
>
> I like the sound of this, but do beware that there are plans afoot to
> move the arch_setup_dma_ops() call later, to driver probe time[1], 

That's OK. I only need the arch_setup_dma_ops() call to happen *after*
the initial parsing of the DMAR tables (which is really early), and
*before* anyone actually tries to set up DMA for a given device.

There's a special case for RMRRs where we have to set up the 1:1
mapping before we enable the IOMMU, but that needs work anyway — it
currently doesn't cope with non-existent devices (the HP horridness
where a device is doing DMA and we don't even know it *exists*).

So I want to fix the RMRR thing to be entirely decoupled from the
actual *devices* anyway.

> FWIW, arm64 is probably the nicer example. The arch side of the DMA ops
> is purely cache maintenance, and the rest is just funnelled through the
> generic dma-iommu.c glue layer.

Right, that's exactly what we're looking for.

So dma_ops can go back to being a platform-wide thing; it's only the
IOMMU ops which are different per device.

> > And IOMMU ops should be per-device of course, not per-bus. But this is
> > a start...
> As it happens, that was one of the additional motivations for
> introducing the new iommu_fwspec - see [3] for my take on the matter.

Right... in my case I don't actually need iommu_ops to change. We have
multiple IOMMUs of the same type, and we don't have a *separate* ops
structure for each of them.

What we *do* need to change is the iommu-private data pointer, which
indicates specifically which DMAR unit is being used.

That's currently kept in dev->archdata but with assumptions that the
device->IOMMU mapping is only done when the domain is first set up, so
it needs to be redone either with a new field in dev->archdata or some
additional complexity.

Rather than a new field in archdata.... should we make this a generic
iommu_priv pointer living next to iommu_ops in the device structure?

Could you use that?

-- 
dwmw2


[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5760 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]