Re: [PATCH v2 07/11] vfio-iommufd: Support iommufd for physical VFIO devices

[Yi Liu <yi.l.liu@xxxxxxxxx>]

On 2022/11/8 08:52, Jason Gunthorpe wrote:
> This creates the iommufd_device for the physical VFIO drivers. These are
> all the drivers that are calling vfio_register_group_dev() and expect the
> type1 code to setup a real iommu_domain against their parent struct
> device.
>
> The design gives the driver a choice in how it gets connected to iommufd
> by providing bind_iommufd/unbind_iommufd/attach_ioas callbacks to
> implement as required. The core code provides three default callbacks for
> physical mode using a real iommu_domain. This is suitable for drivers
> using vfio_register_group_dev()
>
> Tested-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> ---
>   drivers/vfio/Makefile                         |  1 +
>   drivers/vfio/fsl-mc/vfio_fsl_mc.c             |  3 +
>   drivers/vfio/iommufd.c                        | 99 +++++++++++++++++++
>   .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    |  6 ++
>   drivers/vfio/pci/mlx5/main.c                  |  3 +
>   drivers/vfio/pci/vfio_pci.c                   |  3 +
>   drivers/vfio/platform/vfio_amba.c             |  3 +
>   drivers/vfio/platform/vfio_platform.c         |  3 +
>   drivers/vfio/vfio.h                           | 15 +++
>   drivers/vfio/vfio_main.c                      | 13 ++-
>   include/linux/vfio.h                          | 25 +++++
>   11 files changed, 172 insertions(+), 2 deletions(-)
>   create mode 100644 drivers/vfio/iommufd.c
>
> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> index b693a1169286f8..3863922529ef20 100644
> --- a/drivers/vfio/Makefile
> +++ b/drivers/vfio/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_VFIO) += vfio.o
>   vfio-y += vfio_main.o \
>   	  iova_bitmap.o \
>   	  container.o
> +vfio-$(CONFIG_IOMMUFD) += iommufd.o
>   
>   obj-$(CONFIG_VFIO_VIRQFD) += vfio_virqfd.o
>   obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index b16874e913e4f5..5cd4bb47644039 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -592,6 +592,9 @@ static const struct vfio_device_ops vfio_fsl_mc_ops = {
>   	.read		= vfio_fsl_mc_read,
>   	.write		= vfio_fsl_mc_write,
>   	.mmap		= vfio_fsl_mc_mmap,
> +	.bind_iommufd	= vfio_iommufd_physical_bind,
> +	.unbind_iommufd	= vfio_iommufd_physical_unbind,
> +	.attach_ioas	= vfio_iommufd_physical_attach_ioas,
>   };
>   
>   static struct fsl_mc_driver vfio_fsl_mc_driver = {
> diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c
> new file mode 100644
> index 00000000000000..bf755d0f375c5d
> --- /dev/null
> +++ b/drivers/vfio/iommufd.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2021-2022, NVIDIA CORPORATION & AFFILIATES
> + */
> +#include <linux/vfio.h>
> +#include <linux/iommufd.h>
> +
> +#include "vfio.h"
> +
> +MODULE_IMPORT_NS(IOMMUFD);
> +MODULE_IMPORT_NS(IOMMUFD_VFIO);
> +
> +int vfio_iommufd_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx)
> +{
> +	u32 ioas_id;
> +	u32 device_id;
> +	int ret;
> +
> +	lockdep_assert_held(&vdev->dev_set->lock);
> +
> +	/*
> +	 * If the driver doesn't provide this op then it means the device does
> +	 * not do DMA at all. So nothing to do.
> +	 */
> +	if (!vdev->ops->bind_iommufd)
> +		return 0;
> +
> +	ret = vdev->ops->bind_iommufd(vdev, ictx, &device_id);
> +	if (ret)
> +		return ret;
> +
> +	ret = iommufd_vfio_compat_ioas_id(ictx, &ioas_id);
> +	if (ret)
> +		goto err_unbind;
> +	ret = vdev->ops->attach_ioas(vdev, &ioas_id);
> +	if (ret)
> +		goto err_unbind;
> +	vdev->iommufd_attached = true;

it's better to set this bool in vfio_iommufd_physical_attach_ioas() as
the emulated devices uses iommufd_access instead. is it? or you mean this
flag to cover both cases?
 --
Regards,
Yi Liu