RE: [PATCH 07/14] vfio: simplify iommu group allocation for mediated devices

From: "Tian, Kevin" <kevin.tian@intel.com>
To: Christoph Hellwig <hch@lst.de>,
	Alex Williamson <alex.williamson@redhat.com>
Cc: Diana Craciun <diana.craciun@oss.nxp.com>,
	Cornelia Huck <cohuck@redhat.com>,
	Kirti Wankhede <kwankhede@nvidia.com>,
	Eric Auger <eric.auger@redhat.com>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: RE: [PATCH 07/14] vfio: simplify iommu group allocation for mediated devices
Date: Thu, 26 Aug 2021 03:10:27 +0000	[thread overview]
Message-ID: <BN9PR11MB5433EAD76B636CD7148F06628CC79@BN9PR11MB5433.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210825161916.50393-8-hch@lst.de>

> From: Christoph Hellwig <hch@lst.de>
> Sent: Thursday, August 26, 2021 12:19 AM
> 
[...]
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 71e0d3c4f1ac08..6bdfcb9264458c 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -67,6 +67,9 @@ struct vfio_unbound_dev {
>  	struct list_head		unbound_next;
>  };
> 
> +#define VFIO_EMULATED_IOMMU	(1 << 0)
> +#define VFIO_NO_IOMMU		(1 << 1)

it'd be helpful to have some comment here, as emulated iommu
is easy to be mixed with virtual iommu. At least here so-called iommu
doesn't refer to the exact iommu hardware. It's more about the
capability of DMA isolation and how it is implemented.

[...]
> @@ -391,8 +394,8 @@ static struct vfio_group *vfio_create_group(struct
> iommu_group *iommu_group,
>  	}
> 
>  	dev = device_create(vfio.class, NULL,
> -			    MKDEV(MAJOR(vfio.group_devt), minor),
> -			    group, "%s%d", group->noiommu ? "noiommu-" :
> "",
> +			    MKDEV(MAJOR(vfio.group_devt), minor), group,
> "%s%d",
> +			    (group->flags & VFIO_NO_IOMMU) ? "noiommu-" :
> "",

what about introducing a group_no_iommu(group), given many duplicated
checks in this patch?

Thanks
Kevin