Re: [PATCH v5 09/19] vfio/pci: Allow passing zero-length fd array in VFIO_DEVICE_PCI_HOT_RESET

Jason Gunthorpe
2023-02-27T18:22:53Z

On Mon, Feb 27, 2023 at 03:11:25AM -0800, Yi Liu wrote:
> to indicate kernel to use the device's bound iommufd_ctx for the device
> ownership check. Kernel should loop all the opened devices in the dev_set,
> and check if they are bound to the same iommufd_ctx. For the devices that
> has not been opened yet but affected, they can be reset by the current
> users as they cannot be opened by any other user. This applies to the
> existing group/container path as well.
>
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/vfio/pci/vfio_pci_core.c | 111 +++++++++++++++++++++++--------
> drivers/vfio/vfio.h | 11 +++
> include/uapi/linux/vfio.h | 16 +++++
> 3 files changed, 109 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 1bf54beeaef2..e0ebe55b4df0 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -27,11 +27,13 @@
> #include <linux/vgaarb.h>
> #include <linux/nospec.h>
> #include <linux/sched/mm.h>
> +#include <linux/iommufd.h>

Is this needed anymore?

> #if IS_ENABLED(CONFIG_EEH)
> #include <asm/eeh.h>
> #endif
>
> #include "vfio_pci_priv.h"
> +#include "../vfio.h"

Don't do this, put vfio_device_iommufd() in the normal public header

> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 0552e8dcf0cb..4bf11ee8de53 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -673,6 +673,22 @@ struct vfio_pci_hot_reset_info {
> * VFIO_DEVICE_PCI_HOT_RESET - _IOW(VFIO_TYPE, VFIO_BASE + 13,
> * struct vfio_pci_hot_reset)
> *
> + * Userspace requests hot reset for the devices it uses. Due to the
> + * underlying topology, multiple devices may be affected in the reset.
> + * The affected devices may have been opened by the user or by other
> + * users or not opened yet. Only when all the affected devices are
> + * either opened by the current user or not opened by any user, should
> + * the reset request be allowed. Otherwise, this request is expected
> + * to return error.
> + *
> + * If the user uses group and container interface, it should pass down
> + * a set of group fds for ownership check. If the user uses iommufd, it
> + * should pass down a zero-length group_fds array to indicate the kernel
> + * to use the bound iommufd for the ownership check. User that uses the
> + * vfio iommufd compatible mode can also pass down a zero-length group_fds
> + * array as this mode uses iommufd in kernel, and there is no reason to
> + * forbide it.

'forbid'

Rest looks good

Thanks,
Jason