From: Jacob Pan <jacob.jun.pan@linux.intel.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: "Raj, Ashok" <ashok.raj@intel.com>,
Jean-Philippe Brucker <jean-philippe@linaro.com>,
LKML <linux-kernel@vger.kernel.org>,
"iommu@lists.linux-foundation.org"
<iommu@lists.linux-foundation.org>,
Alex Williamson <alex.williamson@redhat.com>,
David Woodhouse <dwmw2@infradead.org>,
Jonathan Cameron <jic23@kernel.org>
Subject: Re: [PATCH 10/10] iommu/vt-d: Register PASID notifier for status change
Date: Fri, 27 Mar 2020 16:47:01 -0700 [thread overview]
Message-ID: <20200327164701.5cfee1c3@jacob-builder> (raw)
In-Reply-To: <AADFC41AFE54684AB9EE6CBC0274A5D19D7ED65A@SHSMSX104.ccr.corp.intel.com>
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]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-03-27 23:41 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-25 17:55 [PATCH 00/10] IOASID extensions for guest SVA Jacob Pan
2020-03-25 17:55 ` [PATCH 01/10] iommu/ioasid: Introduce system-wide capacity Jacob Pan
2020-03-27 8:07 ` Tian, Kevin
2020-03-27 16:08 ` Jacob Pan
2020-04-01 13:45 ` Jean-Philippe Brucker
2020-04-01 22:50 ` Jacob Pan
2020-03-25 17:55 ` [PATCH 02/10] iommu/vt-d: Set IOASID capacity when SVM is enabled Jacob Pan
2020-03-27 8:08 ` Tian, Kevin
2020-03-25 17:55 ` [PATCH 03/10] iommu/ioasid: Introduce per set allocation APIs Jacob Pan
2020-03-26 2:12 ` Lu Baolu
2020-03-26 21:30 ` Jacob Pan
2020-03-27 8:38 ` Tian, Kevin
2020-03-27 16:59 ` Jacob Pan
2020-03-28 6:32 ` Tian, Kevin
2020-04-01 13:47 ` Jean-Philippe Brucker
2020-04-06 20:02 ` Jacob Pan
2020-04-07 11:01 ` Jean-Philippe Brucker
2020-04-21 21:51 ` Jacob Pan
2020-03-25 17:55 ` [PATCH 04/10] iommu/ioasid: Rename ioasid_set_data to avoid confusion with ioasid_set Jacob Pan
2020-03-27 9:35 ` Tian, Kevin
2020-03-25 17:55 ` [PATCH 05/10] iommu/ioasid: Create an IOASID set for host SVA use Jacob Pan
2020-03-27 9:41 ` Tian, Kevin
2020-03-27 17:28 ` Jacob Pan
2020-03-28 6:33 ` Tian, Kevin
2020-04-01 13:53 ` Jean-Philippe Brucker
2020-04-06 15:33 ` Jacob Pan
2020-04-07 11:01 ` Jean-Philippe Brucker
2020-04-13 22:06 ` Jacob Pan
2020-04-15 15:10 ` Jean-Philippe Brucker
2020-03-25 17:55 ` [PATCH 06/10] iommu/ioasid: Convert to set aware allocations Jacob Pan
2020-03-27 9:54 ` Tian, Kevin
2020-03-27 17:41 ` Jacob Pan
2020-03-28 6:40 ` Tian, Kevin
2020-04-06 20:07 ` Jacob Pan
2020-04-01 13:55 ` Jean-Philippe Brucker
2020-04-01 22:45 ` Jacob Pan
2020-03-25 17:55 ` [PATCH 07/10] iommu/ioasid: Use mutex instead of spinlock Jacob Pan
2020-03-27 9:55 ` Tian, Kevin
2020-04-01 13:58 ` Jean-Philippe Brucker
2020-03-25 17:55 ` [PATCH 08/10] iommu/ioasid: Introduce notifier APIs Jacob Pan
2020-03-27 10:03 ` Tian, Kevin
2020-03-27 18:36 ` Jacob Pan
2020-03-28 6:43 ` Tian, Kevin
2020-03-31 15:13 ` Jacob Pan
2020-04-01 14:00 ` Jean-Philippe Brucker
2020-04-10 15:43 ` Jacob Pan
2020-03-25 17:55 ` [PATCH 09/10] iommu/ioasid: Support ioasid_set quota adjustment Jacob Pan
2020-03-27 10:09 ` Tian, Kevin
2020-03-27 23:30 ` Jacob Pan
2020-03-28 6:44 ` Tian, Kevin
2020-03-25 17:55 ` [PATCH 10/10] iommu/vt-d: Register PASID notifier for status change Jacob Pan
2020-03-27 10:22 ` Tian, Kevin
2020-03-27 23:47 ` Jacob Pan [this message]
2020-04-01 14:03 ` [PATCH 00/10] IOASID extensions for guest SVA Jean-Philippe Brucker
2020-04-01 23:38 ` Jacob Pan
2020-04-02 12:26 ` Jean-Philippe Brucker
2020-04-02 16:09 ` Jacob Pan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200327164701.5cfee1c3@jacob-builder \
--to=jacob.jun.pan@linux.intel.com \
--cc=alex.williamson@redhat.com \
--cc=ashok.raj@intel.com \
--cc=dwmw2@infradead.org \
--cc=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.com \
--cc=jic23@kernel.org \
--cc=kevin.tian@intel.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).