Re: [PATCH v3 1/8] iommu: Add ops entry for supported default domain type

From: Lu Baolu
Date: Fri May 10 2019 - 01:37:19 EST


Hi Robin,

On 5/10/19 12:11 AM, Robin Murphy wrote:
On 09/05/2019 03:30, Lu Baolu wrote:
Hi Robin,

On 5/7/19 6:28 PM, Robin Murphy wrote:
On 06/05/2019 16:32, Tom Murphy via iommu wrote:
The AMD driver already solves this problem and uses the generic
iommu_request_dm_for_dev function. It seems like both drivers have the
same problem and could use the same solution. Is there any reason we
can't have use the same solution for the intel and amd driver?

Could we just copy the implementation of the AMD driver? It would be
nice to have the same behavior across both drivers especially as we
move to make both drivers use more generic code.

TBH I don't think the API really needs to be involved at all here. Drivers can already not provide the requested default domain type if they don't support it, so as long as the driver can ensure that the device ends up with IOMMU or direct DMA ops as appropriate, I don't see any great problem with drivers just returning a passthrough domain when a DMA domain was requested, or vice versa (and logging a message that the requested type was overridden). The only type that we really do have to honour strictly is non-default (i.e. unmanaged) domains.

I agree with you that we only have to honor strictly the non-default
domains. But domain type saved in iommu_domain is consumed in iommu.c
and exposed to user through sysfs. It's not clean if the iommu driver
silently replace the default domain.

Right, I did get a bit ahead of myself there - the implicit step before that is to fix default domain allocation so that the core actually passes the relevant device which it has to hand, such that the IOMMU drivers *can* make the right decision up-front.


Yes, passing the relevant device when allocating the default domain so
that the IOMMU driver could make right decision seems to be a better
solution. Somebody can come up with a patch set to bring this up for
discussion. I won't include this in this patch set since it's not for
that purpose. I will follow the existing mechanism that is using on amd
and other iommu drivers.

Best regards,
Lu Baolu