[RFC,16/42] iommufd: Enable device feature IOPF during device attachment to KVM HWPT

Message ID 20231202092311.14392-1-yan.y.zhao@intel.com
State New
Headers
Series Sharing KVM TDP to IOMMU |

Commit Message

Yan Zhao Dec. 2, 2023, 9:23 a.m. UTC
  Enable device feature IOPF during device attachment to KVM HWPT and abort
the attachment if feature enabling is failed.

"pin" is not done by KVM HWPT. If VMM wants to create KVM HWPT, it must
know that all devices attached to this HWPT support IOPF so that pin-all
is skipped.

Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 drivers/iommu/iommufd/device.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
  

Comments

Jason Gunthorpe Dec. 4, 2023, 6:36 p.m. UTC | #1
On Sat, Dec 02, 2023 at 05:23:11PM +0800, Yan Zhao wrote:
> Enable device feature IOPF during device attachment to KVM HWPT and abort
> the attachment if feature enabling is failed.
> 
> "pin" is not done by KVM HWPT. If VMM wants to create KVM HWPT, it must
> know that all devices attached to this HWPT support IOPF so that pin-all
> is skipped.
> 
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> ---
>  drivers/iommu/iommufd/device.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 83af6b7e2784b..4ea447e052ce1 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -381,10 +381,26 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
>  			goto err_unresv;
>  		idev->igroup->hwpt = hwpt;
>  	}
> +	if (hwpt_is_kvm(hwpt)) {
> +		/*
> +		 * Feature IOPF requires ats is enabled which is true only
> +		 * after device is attached to iommu domain.
> +		 * So enable dev feature IOPF after iommu_attach_group().
> +		 * -EBUSY will be returned if feature IOPF is already on.
> +		 */
> +		rc = iommu_dev_enable_feature(idev->dev, IOMMU_DEV_FEAT_IOPF);
> +		if (rc && rc != -EBUSY)
> +			goto err_detach;

I would like to remove IOMMU_DEV_FEAT_IOPF completely please

Jason
  
Yan Zhao Dec. 5, 2023, 7:14 a.m. UTC | #2
On Mon, Dec 04, 2023 at 02:36:03PM -0400, Jason Gunthorpe wrote:
> On Sat, Dec 02, 2023 at 05:23:11PM +0800, Yan Zhao wrote:
> > Enable device feature IOPF during device attachment to KVM HWPT and abort
> > the attachment if feature enabling is failed.
> > 
> > "pin" is not done by KVM HWPT. If VMM wants to create KVM HWPT, it must
> > know that all devices attached to this HWPT support IOPF so that pin-all
> > is skipped.
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > ---
> >  drivers/iommu/iommufd/device.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> > index 83af6b7e2784b..4ea447e052ce1 100644
> > --- a/drivers/iommu/iommufd/device.c
> > +++ b/drivers/iommu/iommufd/device.c
> > @@ -381,10 +381,26 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
> >  			goto err_unresv;
> >  		idev->igroup->hwpt = hwpt;
> >  	}
> > +	if (hwpt_is_kvm(hwpt)) {
> > +		/*
> > +		 * Feature IOPF requires ats is enabled which is true only
> > +		 * after device is attached to iommu domain.
> > +		 * So enable dev feature IOPF after iommu_attach_group().
> > +		 * -EBUSY will be returned if feature IOPF is already on.
> > +		 */
> > +		rc = iommu_dev_enable_feature(idev->dev, IOMMU_DEV_FEAT_IOPF);
> > +		if (rc && rc != -EBUSY)
> > +			goto err_detach;
> 
> I would like to remove IOMMU_DEV_FEAT_IOPF completely please

So, turn on device PRI during device attachment in IOMMU vendor driver?
  
Jason Gunthorpe Dec. 5, 2023, 2:53 p.m. UTC | #3
On Tue, Dec 05, 2023 at 03:14:09PM +0800, Yan Zhao wrote:

> > I would like to remove IOMMU_DEV_FEAT_IOPF completely please
> 
> So, turn on device PRI during device attachment in IOMMU vendor driver?

If a fault requesting domain is attached then PRI should just be
enabled in the driver

Jason
  
Yan Zhao Dec. 6, 2023, 12:55 a.m. UTC | #4
On Tue, Dec 05, 2023 at 10:53:41AM -0400, Jason Gunthorpe wrote:
> On Tue, Dec 05, 2023 at 03:14:09PM +0800, Yan Zhao wrote:
> 
> > > I would like to remove IOMMU_DEV_FEAT_IOPF completely please
> > 
> > So, turn on device PRI during device attachment in IOMMU vendor driver?
> 
> If a fault requesting domain is attached then PRI should just be
> enabled in the driver
>
Right, it makes sense!
  

Patch

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 83af6b7e2784b..4ea447e052ce1 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -381,10 +381,26 @@  int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
 			goto err_unresv;
 		idev->igroup->hwpt = hwpt;
 	}
+	if (hwpt_is_kvm(hwpt)) {
+		/*
+		 * Feature IOPF requires ats is enabled which is true only
+		 * after device is attached to iommu domain.
+		 * So enable dev feature IOPF after iommu_attach_group().
+		 * -EBUSY will be returned if feature IOPF is already on.
+		 */
+		rc = iommu_dev_enable_feature(idev->dev, IOMMU_DEV_FEAT_IOPF);
+		if (rc && rc != -EBUSY)
+			goto err_detach;
+	}
 	refcount_inc(&hwpt->obj.users);
 	list_add_tail(&idev->group_item, &idev->igroup->device_list);
 	mutex_unlock(&idev->igroup->lock);
 	return 0;
+err_detach:
+	if (list_empty(&idev->igroup->device_list)) {
+		iommu_detach_group(hwpt->domain, idev->igroup->group);
+		idev->igroup->hwpt = NULL;
+	}
 err_unresv:
 	if (hwpt_is_paging(hwpt))
 		iopt_remove_reserved_iova(&to_hwpt_paging(hwpt)->ioas->iopt,
@@ -408,6 +424,8 @@  iommufd_hw_pagetable_detach(struct iommufd_device *idev)
 	if (hwpt_is_paging(hwpt))
 		iopt_remove_reserved_iova(&to_hwpt_paging(hwpt)->ioas->iopt,
 					  idev->dev);
+	if (hwpt_is_kvm(hwpt))
+		iommu_dev_disable_feature(idev->dev, IOMMU_DEV_FEAT_IOPF);
 	mutex_unlock(&idev->igroup->lock);
 
 	/* Caller must destroy hwpt */