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

[Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>]

On 31/10/16 14:34, David Woodhouse wrote:
> 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.

Cool, sounds like there shouldn't be any harm in going ahead with this
implementation then. By my reckoning IORT and DMAR (at least the
DRHD/Device Scope parts of it) seem broadly similar, so there might even
be scope to drive a bit more convergence in the ACPI code in future once
the dust settles.

> 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.

Well, in our case we'll probably never escape the kind of nutty SoC
topologies where only some devices are behind IOMMUs (which might
themselves be heterogeneous), only some are coherent, etc., but if you
don't have to deal with that then dma_ops indeed become nice and easy.

>>> 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?

As far as I can tell from looking through the x86 drivers, you should
already be good to go to convert the likes of device_to_iommu() and the
equivalent AMD lookup tables over to a one-off initialisation of
dev->iommu_fwspec and stashing stuff in iommu_fwspec::iommu_priv and
iommu_fwspec::ids as appropriate. I just didn't dare try writing the
patches myself...

Robin.

> 
> -- 
> dwmw2
> 

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu