Re: [PATCH v4 02/13] driver core: Set DMA ownership during driver bind/unbind

From: Lu Baolu
Date: Wed Dec 22 2021 - 21:08:46 EST


Hi Greg,

On 12/22/21 8:47 PM, Greg Kroah-Hartman wrote:
Which one will actually care about the iommu_device_set_dma_owner()
call? All of them? None of them? Some of them?

Again, why can't this just happen in the (very few) bus callbacks that
care about this? In following patches in this series, you turn off this
for the pci_dma_configure users, so what is left? 3 odd bus types that
are not used often. How well did you test devices of those types with
this patchset?

It's fine to have "suppress" fields when they are the minority, but here
it's a_very_ tiny tiny number of actual devices in a system that will
ever get the chance to have this check happen for them and trigger,
right?

Thank you for your comments. Current VFIO implementation supports
devices on pci/platform/amba/fls-mc buses for user-space DMA. So only
those buses need to call iommu_device_set/release_dma_owner() in their
dma_configure/cleanup() callbacks.

The "suppress" field is only for a few device drivers (not devices), for
example,

- vfio-pci, a PCI device driver used to bind to a PCI device so that it
could be assigned for user-space DMA.

Other similar drivers in drivers/vfio are vfio-fsl-mc, vfio-amba and
vfio-platform. These drivers will call
iommu_device_set/release_dma_owner(DMA_OWNER_USER) explicitly when the
device is assigned to user.

The logic is that on the affected buses (pci/platform/amba/fls-mc),

- for non-vfio drivers, bus dma_configure/cleanup() will automatically
call iommu_device_set_dma_owner(KERNEL) for the device; [This is the
majority cases.]

- for vfio drivers, the auto-call will be suppressed, and the vfio
drivers are supposed to call iommu_device_set_dma_owner(USER) before
device is assigned to the userspace. [This is the rare case.]

The KERNEL and USER conflict will be detected in
iommu_device_set_dma_owner() with a -EBUSY return value. In that case,
the driver binding or device assignment should be aborted.

Best regards,
baolu