lkml.org 
[lkml]   [2020]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 10/10] iommu/vt-d: Register PASID notifier for status change
On Fri, 27 Mar 2020 10:22:57 +0000
"Tian, Kevin" <kevin.tian@intel.com> wrote:

> > From: Jacob Pan
> > Sent: Thursday, March 26, 2020 1:56 AM
> >
> > In bare metal SVA, IOMMU driver ensures that IOASID free call
> > always comes after IOASID unbind operation.
> >
> > However, for guest SVA the unbind and free call come from user space
> > via VFIO, which could be out of order. This patch registers a
> > notifier block in case IOASID free() comes before unbind such that
> > VT-d driver can take action to clean up PASID context and data.
>
> clearly the patch includes more than above usage. It also handles the
> bind usage to notify KVM for setting PASID translation table.
>
> >
> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > ---
> > drivers/iommu/intel-svm.c | 68
> > ++++++++++++++++++++++++++++++++++++++++++++-
> > include/linux/intel-iommu.h | 1 +
> > 2 files changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> > index f511855d187b..779dd2c6f9e1 100644
> > --- a/drivers/iommu/intel-svm.c
> > +++ b/drivers/iommu/intel-svm.c
> > @@ -23,6 +23,7 @@
> > #include "intel-pasid.h"
> >
> > static irqreturn_t prq_event_thread(int irq, void *d);
> > +static DEFINE_MUTEX(pasid_mutex);
> >
> > #define PRQ_ORDER 0
> >
> > @@ -92,6 +93,65 @@ static inline bool intel_svm_capable(struct
> > intel_iommu *iommu)
> > return iommu->flags & VTD_FLAG_SVM_CAPABLE;
> > }
> >
> > +#define pasid_lock_held() lock_is_held(&pasid_mutex.dep_map)
> > +
> > +static int pasid_status_change(struct notifier_block *nb,
> > + unsigned long code, void *data)
> > +{
> > + struct ioasid_nb_args *args = (struct ioasid_nb_args
> > *)data;
> > + struct intel_svm_dev *sdev;
> > + struct intel_svm *svm;
> > + int ret = NOTIFY_DONE;
> > +
> > + if (code == IOASID_FREE) {
> > + /*
> > + * Unbind all devices associated with this PASID
> > which is
> > + * being freed by other users such as VFIO.
> > + */
> > + mutex_lock(&pasid_mutex);
> > + svm = ioasid_find(INVALID_IOASID_SET, args->id,
> > NULL);
> > + if (!svm || !svm->iommu)
> > + goto done_unlock;
>
> should we treat !svm->iommu as an error condition? if not, do you have
> an example when it may occur in normal situation?
>
Right, should be an error. Initially, unbind could retrieve iommu from
struct device, but with notifier we have to set svm->iommu all the time.

> > +
> > + if (IS_ERR(svm)) {
> > + ret = NOTIFY_BAD;
> > + goto done_unlock;
> > + }
>
> svm->iommu should be referenced after IS_ERR check
>
Good point, will move up.
> > +
> > + list_for_each_entry_rcu(sdev, &svm->devs, list,
> > pasid_lock_held()) {
> > + /* Does not poison forward pointer */
> > + list_del_rcu(&sdev->list);
> > + intel_pasid_tear_down_entry(svm->iommu,
> > sdev-
> > >dev,
> > + svm->pasid);
> > + kfree_rcu(sdev, rcu);
> > +
> > + /*
> > + * Free before unbind only happens with
> > guest usaged
> > + * host PASIDs. IOASID free will detach
> > private data
> > + * and free the IOASID entry.
>
> "guest usaged host PASIDs"?
>
I mean VM used PASID, will change to
/*
* Free before unbind only happens with PASIDs used
* by VM. IOASID free will detach private data
* and free the IOASID entry.
*/

> > + */
> > + if (list_empty(&svm->devs))
> > + kfree(svm);
> > + }
> > + mutex_unlock(&pasid_mutex);
> > +
> > + return NOTIFY_OK;
> > + }
> > +
> > +done_unlock:
> > + mutex_unlock(&pasid_mutex);
> > + return ret;
> > +}
> > +
> > +static struct notifier_block pasid_nb = {
> > + .notifier_call = pasid_status_change,
> > +};
> > +
> > +void intel_svm_add_pasid_notifier(void)
> > +{
> > + ioasid_add_notifier(&pasid_nb);
> > +}
> > +
> > void intel_svm_check(struct intel_iommu *iommu)
> > {
> > if (!pasid_supported(iommu))
> > @@ -219,7 +279,6 @@ static const struct mmu_notifier_ops
> > intel_mmuops = {
> > .invalidate_range = intel_invalidate_range,
> > };
> >
> > -static DEFINE_MUTEX(pasid_mutex);
> > static LIST_HEAD(global_svm_list);
> >
> > #define for_each_svm_dev(sdev, svm, d) \
> > @@ -319,6 +378,7 @@ int intel_svm_bind_gpasid(struct iommu_domain
> > *domain,
> > svm->gpasid = data->gpasid;
> > svm->flags |= SVM_FLAG_GUEST_PASID;
> > }
> > + svm->iommu = iommu;
>
> ah, it's interesting to see we have a field defined before but never
> used. 😊
>
The unbind call can retrieve iommu from struct device.

> >
> > ioasid_attach_data(data->hpasid, svm);
> > INIT_LIST_HEAD_RCU(&svm->devs);
> > @@ -383,6 +443,11 @@ int intel_svm_bind_gpasid(struct iommu_domain
> > *domain,
> > }
> > svm->flags |= SVM_FLAG_GUEST_MODE;
> >
> > + /*
> > + * Notify KVM new host-guest PASID bind is ready. KVM will
> > set up
> > + * PASID translation table to support guest ENQCMD.
> > + */
>
> should take it as an example instead of the only possible usage.
>
True, we don;t know who is getting notified.

> > + ioasid_notify(data->hpasid, IOASID_BIND);
> > init_rcu_head(&sdev->rcu);
> > list_add_rcu(&sdev->list, &svm->devs);
> > out:
> > @@ -440,6 +505,7 @@ int intel_svm_unbind_gpasid(struct device *dev,
> > int pasid)
> > * used by another.
> > */
> > ioasid_attach_data(pasid, NULL);
> > + ioasid_notify(pasid,
> > IOASID_UNBIND); kfree(svm);
> > }
> > }
> > diff --git a/include/linux/intel-iommu.h
> > b/include/linux/intel-iommu.h index f8504a980981..64799067ea58
> > 100644 --- a/include/linux/intel-iommu.h
> > +++ b/include/linux/intel-iommu.h
> > @@ -708,6 +708,7 @@ extern struct iommu_sva *
> > intel_svm_bind(struct device *dev, struct mm_struct *mm, void
> > *drvdata); extern void intel_svm_unbind(struct iommu_sva *handle);
> > extern int intel_svm_get_pasid(struct iommu_sva *handle);
> > +extern void intel_svm_add_pasid_notifier(void);
> >
> > struct svm_dev_ops;
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu

[Jacob Pan]

\
 
 \ /
  Last update: 2020-03-28 00:42    [W:0.072 / U:0.724 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site