Re: [PATCH v4 15/22] iommu/vt-d: Replace Intel specific PASID allocator with IOASID
On Thu, 27 Jun 2019 09:53:11 +0800
Lu Baolu wrote:
> Hi Jacob,
>
> On 6/9/19 9:44 PM, Jacob Pan wrote:
> > Make use of generic IOASID code to manage PASID allocation,
> > free, and lookup. Replace Intel specific code.
> >
> > Signed-off-by: Jacob Pan
> > ---
> > 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);
> > + 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(>lock);
> > spin_unlock_irqrestore(_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(_lock);
> > - ret = idr_alloc(_idr, ptr, min, max, GFP_ATOMIC);
> > - spin_unlock(_lock);
> > - idr_preload_end();
> > -
> > - return ret;
> > -}
> > -
> > -void intel_pasid_free_id(int pasid)
> > -{
> > - spin_lock(_lock);
> > - idr_remove(_idr, pasid);
> > - spin_unlock(_lock);
> > -}
> > -
> > -void *intel_pasid_lookup_id(int pasid)
> > -{
> > - void *p;
> > -
> > - spin_lock(_lock);
> > - p = idr_find(_idr, pasid);
> > - spin_unlock(_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
> > #include
> > #include
> > +#include
> > #include
> >
> > #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 = _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(>notifier, mm); if (ret) {
> > - intel_pasid_free_id(svm->pasid);
> > +
Re: [PATCH v4 15/22] iommu/vt-d: Replace Intel specific PASID allocator with IOASID
Hi Jacob,
On 6/9/19 9:44 PM, Jacob Pan wrote:
Make use of generic IOASID code to manage PASID allocation,
free, and lookup. Replace Intel specific code.
Signed-off-by: Jacob Pan
---
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);
+ 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(>lock);
spin_unlock_irqrestore(_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(_lock);
- ret = idr_alloc(_idr, ptr, min, max, GFP_ATOMIC);
- spin_unlock(_lock);
- idr_preload_end();
-
- return ret;
-}
-
-void intel_pasid_free_id(int pasid)
-{
- spin_lock(_lock);
- idr_remove(_idr, pasid);
- spin_unlock(_lock);
-}
-
-void *intel_pasid_lookup_id(int pasid)
-{
- void *p;
-
- spin_lock(_lock);
- p = idr_find(_idr, pasid);
- spin_unlock(_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
#include
#include
+#include
#include
#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 = _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(>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
Re: [PATCH v4 15/22] iommu/vt-d: Replace Intel specific PASID allocator with IOASID
On Tue, 18 Jun 2019 16:57:09 +0100
Jonathan Cameron wrote:
> On Sun, 9 Jun 2019 06:44:15 -0700
> Jacob Pan wrote:
>
> > Make use of generic IOASID code to manage PASID allocation,
> > free, and lookup. Replace Intel specific code.
> >
> > Signed-off-by: Jacob Pan
> Hi Jacob,
>
> One question inline.
>
Hi Jonathan,
Sorry, I got distracted and missed your review. My answer
below.
Thanks!
Jacob
> 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?
>
Good point, I agree. There is no need for passing the domain. aux domain
and its defaul_pasid has 1:1 mapping.
> It is also rather confusing as the same driver stashes two different
> types of data in the same xarray.
On VT-d, there should be only one type of data for all PASID users that
needs private data. Whether it is native SVA or guest SVA. Only an
internal flag will tell them apart.
Re: [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 wrote:
> Make use of generic IOASID code to manage PASID allocation,
> free, and lookup. Replace Intel specific code.
>
> Signed-off-by: Jacob Pan
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(>lock);
> spin_unlock_irqrestore(_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(_lock);
> - ret = idr_alloc(_idr, ptr, min, max, GFP_ATOMIC);
> - spin_unlock(_lock);
> - idr_preload_end();
> -
> - return ret;
> -}
> -
> -void intel_pasid_free_id(int pasid)
> -{
> - spin_lock(_lock);
> - idr_remove(_idr, pasid);
> - spin_unlock(_lock);
> -}
> -
> -void *intel_pasid_lookup_id(int pasid)
> -{
> - void *p;
> -
> - spin_lock(_lock);
> - p = idr_find(_idr, pasid);
> - spin_unlock(_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
> #include
> #include
> +#include
> #include
>
> #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 = _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) {
>
[PATCH v4 15/22] iommu/vt-d: Replace Intel specific PASID allocator with IOASID
Make use of generic IOASID code to manage PASID allocation,
free, and lookup. Replace Intel specific code.
Signed-off-by: Jacob Pan
---
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);
+ 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(>lock);
spin_unlock_irqrestore(_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(_lock);
- ret = idr_alloc(_idr, ptr, min, max, GFP_ATOMIC);
- spin_unlock(_lock);
- idr_preload_end();
-
- return ret;
-}
-
-void intel_pasid_free_id(int pasid)
-{
- spin_lock(_lock);
- idr_remove(_idr, pasid);
- spin_unlock(_lock);
-}
-
-void *intel_pasid_lookup_id(int pasid)
-{
- void *p;
-
- spin_lock(_lock);
- p = idr_find(_idr, pasid);
- spin_unlock(_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
#include
#include
+#include
#include
#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 = _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(>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)
