lkml.org 
[lkml]   [2019]   [Jun]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 15/22] iommu/vt-d: Replace Intel specific PASID allocator with IOASID
    On Sun, 9 Jun 2019 06:44:15 -0700
    Jacob Pan <jacob.jun.pan@linux.intel.com> wrote:

    > Make use of generic IOASID code to manage PASID allocation,
    > free, and lookup. Replace Intel specific code.
    >
    > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
    Hi Jacob,

    One question inline.

    Jonathan

    > ---
    > drivers/iommu/intel-iommu.c | 11 +++++------
    > drivers/iommu/intel-pasid.c | 36 ------------------------------------
    > drivers/iommu/intel-svm.c | 37 +++++++++++++++++++++----------------
    > 3 files changed, 26 insertions(+), 58 deletions(-)
    >
    > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
    > index 5b84994..39b63fe 100644
    > --- a/drivers/iommu/intel-iommu.c
    > +++ b/drivers/iommu/intel-iommu.c
    > @@ -5167,7 +5167,7 @@ static void auxiliary_unlink_device(struct dmar_domain *domain,
    > domain->auxd_refcnt--;
    >
    > if (!domain->auxd_refcnt && domain->default_pasid > 0)
    > - intel_pasid_free_id(domain->default_pasid);
    > + ioasid_free(domain->default_pasid);
    > }
    >
    > static int aux_domain_add_dev(struct dmar_domain *domain,
    > @@ -5185,10 +5185,9 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
    > if (domain->default_pasid <= 0) {
    > int pasid;
    >
    > - pasid = intel_pasid_alloc_id(domain, PASID_MIN,
    > - pci_max_pasids(to_pci_dev(dev)),
    > - GFP_KERNEL);
    > - if (pasid <= 0) {
    > + pasid = ioasid_alloc(NULL, PASID_MIN, pci_max_pasids(to_pci_dev(dev)) - 1,
    > + domain);

    Is there any point in passing the domain in as the private pointer here?
    I can't immediately see anywhere it is read back?

    It is also rather confusing as the same driver stashes two different types of data
    in the same xarray.

    > + if (pasid == INVALID_IOASID) {
    > pr_err("Can't allocate default pasid\n");
    > return -ENODEV;
    > }
    > @@ -5224,7 +5223,7 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
    > spin_unlock(&iommu->lock);
    > spin_unlock_irqrestore(&device_domain_lock, flags);
    > if (!domain->auxd_refcnt && domain->default_pasid > 0)
    > - intel_pasid_free_id(domain->default_pasid);
    > + ioasid_free(domain->default_pasid);
    >
    > return ret;
    > }
    > diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
    > index 69fddd3..1e25539 100644
    > --- a/drivers/iommu/intel-pasid.c
    > +++ b/drivers/iommu/intel-pasid.c
    > @@ -26,42 +26,6 @@
    > */
    > static DEFINE_SPINLOCK(pasid_lock);
    > u32 intel_pasid_max_id = PASID_MAX;
    > -static DEFINE_IDR(pasid_idr);
    > -
    > -int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp)
    > -{
    > - int ret, min, max;
    > -
    > - min = max_t(int, start, PASID_MIN);
    > - max = min_t(int, end, intel_pasid_max_id);
    > -
    > - WARN_ON(in_interrupt());
    > - idr_preload(gfp);
    > - spin_lock(&pasid_lock);
    > - ret = idr_alloc(&pasid_idr, ptr, min, max, GFP_ATOMIC);
    > - spin_unlock(&pasid_lock);
    > - idr_preload_end();
    > -
    > - return ret;
    > -}
    > -
    > -void intel_pasid_free_id(int pasid)
    > -{
    > - spin_lock(&pasid_lock);
    > - idr_remove(&pasid_idr, pasid);
    > - spin_unlock(&pasid_lock);
    > -}
    > -
    > -void *intel_pasid_lookup_id(int pasid)
    > -{
    > - void *p;
    > -
    > - spin_lock(&pasid_lock);
    > - p = idr_find(&pasid_idr, pasid);
    > - spin_unlock(&pasid_lock);
    > -
    > - return p;
    > -}
    >
    > int vcmd_alloc_pasid(struct intel_iommu *iommu, unsigned int *pasid)
    > {
    > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
    > index 8f87304..9cbcc1f 100644
    > --- a/drivers/iommu/intel-svm.c
    > +++ b/drivers/iommu/intel-svm.c
    > @@ -25,6 +25,7 @@
    > #include <linux/dmar.h>
    > #include <linux/interrupt.h>
    > #include <linux/mm_types.h>
    > +#include <linux/ioasid.h>
    > #include <asm/page.h>
    >
    > #include "intel-pasid.h"
    > @@ -332,16 +333,15 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
    > if (pasid_max > intel_pasid_max_id)
    > pasid_max = intel_pasid_max_id;
    >
    > - /* Do not use PASID 0 in caching mode (virtualised IOMMU) */
    > - ret = intel_pasid_alloc_id(svm,
    > - !!cap_caching_mode(iommu->cap),
    > - pasid_max - 1, GFP_KERNEL);
    > - if (ret < 0) {
    > + /* Do not use PASID 0, reserved for RID to PASID */
    > + svm->pasid = ioasid_alloc(NULL, PASID_MIN,
    > + pasid_max - 1, svm);
    > + if (svm->pasid == INVALID_IOASID) {
    > kfree(svm);
    > kfree(sdev);
    > + ret = ENOSPC;
    > goto out;
    > }
    > - svm->pasid = ret;
    > svm->notifier.ops = &intel_mmuops;
    > svm->mm = mm;
    > svm->flags = flags;
    > @@ -351,7 +351,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
    > if (mm) {
    > ret = mmu_notifier_register(&svm->notifier, mm);
    > if (ret) {
    > - intel_pasid_free_id(svm->pasid);
    > + ioasid_free(svm->pasid);
    > kfree(svm);
    > kfree(sdev);
    > goto out;
    > @@ -367,7 +367,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
    > if (ret) {
    > if (mm)
    > mmu_notifier_unregister(&svm->notifier, mm);
    > - intel_pasid_free_id(svm->pasid);
    > + ioasid_free(svm->pasid);
    > kfree(svm);
    > kfree(sdev);
    > goto out;
    > @@ -400,7 +400,12 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
    > if (!iommu)
    > goto out;
    >
    > - svm = intel_pasid_lookup_id(pasid);
    > + svm = ioasid_find(NULL, pasid, NULL);
    > + if (IS_ERR(svm)) {
    > + ret = PTR_ERR(svm);
    > + goto out;
    > + }
    > +
    > if (!svm)
    > goto out;
    >
    > @@ -422,7 +427,7 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
    > kfree_rcu(sdev, rcu);
    >
    > if (list_empty(&svm->devs)) {
    > - intel_pasid_free_id(svm->pasid);
    > + ioasid_free(svm->pasid);
    > if (svm->mm)
    > mmu_notifier_unregister(&svm->notifier, svm->mm);
    >
    > @@ -457,10 +462,11 @@ int intel_svm_is_pasid_valid(struct device *dev, int pasid)
    > if (!iommu)
    > goto out;
    >
    > - svm = intel_pasid_lookup_id(pasid);
    > - if (!svm)
    > + svm = ioasid_find(NULL, pasid, NULL);
    > + if (IS_ERR(svm)) {
    > + ret = PTR_ERR(svm);
    > goto out;
    > -
    > + }
    > /* init_mm is used in this case */
    > if (!svm->mm)
    > ret = 1;
    > @@ -567,13 +573,12 @@ static irqreturn_t prq_event_thread(int irq, void *d)
    >
    > if (!svm || svm->pasid != req->pasid) {
    > rcu_read_lock();
    > - svm = intel_pasid_lookup_id(req->pasid);
    > + svm = ioasid_find(NULL, req->pasid, NULL);
    > /* It *can't* go away, because the driver is not permitted
    > * to unbind the mm while any page faults are outstanding.
    > * So we only need RCU to protect the internal idr code. */
    > rcu_read_unlock();
    > -
    > - if (!svm) {
    > + if (IS_ERR(svm) || !svm) {
    > pr_err("%s: Page request for invalid PASID %d: %08llx %08llx\n",
    > iommu->name, req->pasid, ((unsigned long long *)req)[0],
    > ((unsigned long long *)req)[1]);


    \
     
     \ /
      Last update: 2019-06-18 17:58    [W:4.371 / U:0.292 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site