RE: [PATCH v6 09/10] vfio/pci: Extend VFIO_DEVICE_GET_PCI_HOT_RESET_INFO for vfio device cdev

Liu, Yi L
2023-06-01T06:06:27Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, June 1, 2023 3:00 AM
>
> On Fri, May 26, 2023 at 10:04:27AM +0800, Baolu Lu wrote:
> > 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.
>
> Right
>
> The only time it is safe to reset is if you know there is no attached
> driver or you know VFIO is the attached driver and the caller owns the
> VFIO too.
>
> We haven't done a no attached driver test due to races.

Ok. @Alex, should we relax the above dev_set requirement now or should
be in a separate series?

Regards,
Yi Liu