Re: [RFC PATCH v2 00/10] vfio/mdev: IOMMU aware mediated device

[Lu Baolu]

Hi,

On 09/06/2018 03:15 AM, Alex Williamson wrote:
> On Wed, 5 Sep 2018 03:01:39 +0000
> "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote:
>
> > > From: Lu Baolu [mailto:baolu.lu@xxxxxxxxxxxxxxx]
> > > Sent: Thursday, August 30, 2018 12:09 PM
> > >    
> > [...]
> > > In order to distinguish the IOMMU-capable mediated devices from those
> > > which still need to rely on parent devices, this patch set adds a
> > > domain type attribute to each mdev.
> > >
> > > enum mdev_domain_type {
> > > 	DOMAIN_TYPE_NO_IOMMU,	/* Don't need any IOMMU support.
> > > 				 * All isolation and protection
> > > 				 * are handled by the parent
> > > 				 * device driver with a device
> > > 				 * specific mechanism.
> > > 				 */
> > > 	DOMAIN_TYPE_ATTACH_PARENT, /* IOMMU can isolate and
> > > protect
> > > 				    * the mdev, and the isolation
> > > 				    * domain should be attaced with
> > > 				    * the parent device.
> > > 				    */
> > > };
> > >    
> >
> > ATTACH_PARENT is not like a good counterpart to NO_IOMMU.
>
> Please do not use NO_IOMMU, we already have a thing called
> vfio-noiommu, enabled through CONFIG_VFIO_NOIOMMU and module parameter
> enable_unsafe_noiommu_mode.  This is much, much too similar and will
> generate confusion.

Sure. Will remove this confusion.

> > what about DOMAIN_TYPE_NO_IOMMU/DOMAIN_TYPE_IOMMU? whether
> > to attach parent device is just internal logic.
> >
> > Alternatively DOMAIN_TYPE_SOFTWARE/DOMAIN_TYPE_HARDWARE,
> > where software means iommu_domain is managed by software while
> > the other means managed by hardware.
>
> I haven't gotten deep enough into the series to see how it's used, but
> my gut reaction is that we don't need an enum, we just need some sort
> of pointer on the mdev that points to an iommu_parent, which indicates
> the root of our IOMMU based isolation, or is NULL, which indicates we
> use vendor defined isolation as we have now.

It works as long as we can distinguish IOMMU based isolation and the
vendor defined isolation.

How about making the iommu_parent points the device structure who
created the mdev? If this pointer is NOT NULL we will bind the domain
to the device pointed to by it, otherwise, handle it in the vendor
defined way?

Best regards,
Lu Baolu

> > One side note to Alex - with multiple domain extension in IOMMU layer,
> > this version combines IOMMU-capable usages in VFIO: PASID-based (as
> > in scalable iov) and RID-based (as the usage of mdev wrapper on any
> > device). Both cases share the common path - just binding the domain to the
> > parent device of mdev. IOMMU layer will handle two cases differently later.
>
> Good, I'm glad you've considered the regular (RID) IOMMU domain and not
> just the new aux domain.  Thanks,
>
> Alex