On 5/25/23 9:02 PM, Liu, Yi L wrote:
It's possible that requirement
might be relaxed in the new DMA ownership model, but as it is right
now, the code enforces that requirement and any new discussion about
what makes hot-reset available should note both the ownership and
dev_set requirement. Thanks,
I think your point is that if an iommufd_ctx has acquired DMA ownerhisp
of an iommu_group, it means the device is owned. And it should not
matter whether all the devices in the iommu_group is present in the
dev_set. It is allowed that some devices are bound to pci-stub or
pcieport driver. Is it?
Actually I have a doubt on it. IIUC, the above requirement on dev_set
is to ensure the reset to the devices are protected by the dev_set->lock.
So that either the reset issued by driver itself or a hot reset request
from user, there is no race. But if a device is not in the dev_set, then
hot reset request from user might race with the bound driver. DMA ownership
only guarantees the drivers won't handle DMA via DMA API which would have
conflict with DMA mappings from user. I'm not sure if it is able to
guarantee reset is exclusive as well. I see pci-stub and pcieport driver
are the only two drivers that set the driver_managed_dma flag besides the
vfio drivers. pci-stub may be fine. not sure about pcieport driver.
commit c7d469849747 ("PCI: portdrv: Set driver_managed_dma") described
the criteria of adding driver_managed_dma to the pcieport driver.
"
We achieve this by setting ".driver_managed_dma = true" in pci_driver
structure. It is safe because the portdrv driver meets below criteria:
- This driver doesn't use DMA, as you can't find any related calls like
pci_set_master() or any kernel DMA API (dma_map_*() and etc.).
- It doesn't use MMIO as you can't find ioremap() or similar calls. It's
tolerant to userspace possibly also touching the same MMIO registers
via P2P DMA access.
"
pci_rest_device() definitely shouldn't be done by the kernel drivers
that have driver_managed_dma set.
# line filename / context / line
1 39 drivers/pci/pci-stub.c <<GLOBAL>>
.driver_managed_dma = true,
2 796 drivers/pci/pcie/portdrv.c <<GLOBAL>>
.driver_managed_dma = true,
3 607 drivers/vfio/fsl-mc/vfio_fsl_mc.c <<GLOBAL>>
.driver_managed_dma = true,
4 1459 drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c <<GLOBAL>>
.driver_managed_dma = true,
5 1374 drivers/vfio/pci/mlx5/main.c <<GLOBAL>>
.driver_managed_dma = true,
6 203 drivers/vfio/pci/vfio_pci.c <<GLOBAL>>
.driver_managed_dma = true,
7 139 drivers/vfio/platform/vfio_amba.c <<GLOBAL>>
.driver_managed_dma = true,
8 120 drivers/vfio/platform/vfio_platform.c <<GLOBAL>>
.driver_managed_dma = true,
Anyhow, I think this is not a must so far. is it? Even doable, it shall
be done in the future. 😄
Perhaps we can take it in this way: it's a bug if any driver sets its
driver_managed_dma but still resets the hardware during it's life cycle?
Best regards,
baolu
