Re: [PATCH 06/10] iommu/ioasid: Convert to set aware allocations
On Sat, 28 Mar 2020 06:40:58 + "Tian, Kevin" wrote: > > From: Jacob Pan > > Sent: Saturday, March 28, 2020 1:42 AM > > > > On Fri, 27 Mar 2020 09:54:11 + > > "Tian, Kevin" wrote: > > > > > > From: Jacob Pan > > > > Sent: Thursday, March 26, 2020 1:55 AM > > > > > > > > The current ioasid_alloc function takes a token/ioasid_set then > > > > record it on the IOASID being allocated. There is no alloc/free > > > > on the ioasid_set. > > > > > > > > With the IOASID set APIs, callers must allocate an ioasid_set > > > > before allocate IOASIDs within the set. Quota and other > > > > ioasid_set level activities can then be enforced. > > > > > > > > This patch converts existing API to the new ioasid_set model. > > > > > > > > Signed-off-by: Liu Yi L > > > > Signed-off-by: Jacob Pan > > > > --- > > > > drivers/iommu/intel-iommu.c | 10 +++--- > > > > drivers/iommu/intel-svm.c | 10 +++--- > > > > drivers/iommu/ioasid.c | 78 > > > > +--- - > > > > include/linux/ioasid.h | 11 +++ > > > > 4 files changed, 72 insertions(+), 37 deletions(-) > > > > > > > > diff --git a/drivers/iommu/intel-iommu.c > > > > b/drivers/iommu/intel-iommu.c index af7a1ef7b31e..c571cc8d9e57 > > > > 100644 --- a/drivers/iommu/intel-iommu.c > > > > +++ b/drivers/iommu/intel-iommu.c > > > > @@ -3323,11 +3323,11 @@ static void intel_ioasid_free(ioasid_t > > > > ioasid, void *data) > > > > if (!iommu) > > > > return; > > > > /* > > > > -* Sanity check the ioasid owner is done at upper > > > > layer, e.g. VFIO > > > > -* We can only free the PASID when all the devices are > > > > unbound. > > > > +* In the guest, all IOASIDs belong to the > > > > system_ioasid set. > > > > +* Sanity check against the system set. > > > > > > below code has nothing to deal with guest, then why putting the > > > comment specifically for guest? > > > > > intel_ioasid_alloc/free() is the custom IOASID allocator only > > registered when running in the guest. > > in that case may be rename the functions to > intel_guest_ioasid_alloc/free would avoid similar confusion as I had? > Sounds good. > > > > The custom allocator calls virtual command. Since we don't support > > nested guest, all IOASIDs belong to the system ioasid_set. > > could you put no support of nested guest in the comment, so later > when people want to add nested support they will know some > additional work required here? > will do. > > > > > > */ > > > > - if (ioasid_find(NULL, ioasid, NULL)) { > > > > - pr_alert("Cannot free active IOASID %d\n", > > > > ioasid); > > > > + if (IS_ERR(ioasid_find(system_ioasid_sid, ioasid, > > > > NULL))) { > > > > + pr_err("Cannot free IOASID %d, not in system > > > > set\n", ioasid); return; > > > > } > > > > vcmd_free_pasid(iommu, ioasid); > > > > @@ -5541,7 +5541,7 @@ static int aux_domain_add_dev(struct > > > > dmar_domain *domain, > > > > int pasid; > > > > > > > > /* No private data needed for the default > > > > pasid */ > > > > - pasid = ioasid_alloc(NULL, PASID_MIN, > > > > + pasid = ioasid_alloc(system_ioasid_sid, > > > > PASID_MIN, pci_max_pasids(to_pci_dev(dev)) > > > > - 1, NULL); > > > > if (pasid == INVALID_IOASID) { > > > > diff --git a/drivers/iommu/intel-svm.c > > > > b/drivers/iommu/intel-svm.c index 1991587fd3fd..f511855d187b > > > > 100644 --- a/drivers/iommu/intel-svm.c > > > > +++ b/drivers/iommu/intel-svm.c > > > > @@ -268,7 +268,7 @@ int intel_svm_bind_gpasid(struct > > > > iommu_domain *domain, > > > > } > > > > > > > > mutex_lock(_mutex); > > > > - svm = ioasid_find(NULL, data->hpasid, NULL); > > > > + svm = ioasid_find(INVALID_IOASID_SET, data->hpasid, > > > > NULL); if (IS_ERR(svm)) { > > > > ret = PTR_ERR(svm); > > > > goto out; > > > > @@ -401,7 +401,7 @@ int intel_svm_unbind_gpasid(struct device > > > > *dev, int pasid) > > > > return -EINVAL; > > > > > > > > mutex_lock(_mutex); > > > > - svm = ioasid_find(NULL, pasid, NULL); > > > > + svm = ioasid_find(INVALID_IOASID_SET, pasid, NULL); > > > > if (!svm) { > > > > ret = -EINVAL; > > > > goto out; > > > > @@ -559,7 +559,7 @@ static int intel_svm_bind_mm(struct device > > > > *dev, int flags, struct svm_dev_ops * > > > > pasid_max = intel_pasid_max_id; > > > > > > > > /* Do not use PASID 0, reserved for RID to > > > > PASID */ > > > > - svm->pasid = ioasid_alloc(NULL, PASID_MIN, > > > > + svm->pasid = ioasid_alloc(system_ioasid_sid, > > > > PASID_MIN, pasid_max - 1, svm); > > > > if (svm->pasid == INVALID_IOASID) { > > > > kfree(svm); > > > > @@ -642,7 +642,7
Re: [PATCH 06/10] iommu/ioasid: Convert to set aware allocations
On Wed, 1 Apr 2020 15:55:25 +0200 Jean-Philippe Brucker wrote: > On Wed, Mar 25, 2020 at 10:55:27AM -0700, Jacob Pan wrote: > > The current ioasid_alloc function takes a token/ioasid_set then > > record it on the IOASID being allocated. There is no alloc/free on > > the ioasid_set. > > > > With the IOASID set APIs, callers must allocate an ioasid_set before > > allocate IOASIDs within the set. Quota and other ioasid_set level > > activities can then be enforced. > > > > This patch converts existing API to the new ioasid_set model. > > > > Signed-off-by: Liu Yi L > > Signed-off-by: Jacob Pan > > [...] > > > @@ -379,6 +391,9 @@ ioasid_t ioasid_alloc(struct ioasid_set *set, > > ioasid_t min, ioasid_t max, } > > data->id = id; > > > > + /* Store IOASID in the per set data */ > > + xa_store(>xa, id, data, GFP_KERNEL); > > I couldn't figure out why you're maintaining an additional xarray for > each set. We're already storing that data in active_allocator->xa, > why the duplication? If it's for the gPASID -> hPASID translation > mentioned by the cover letter, maybe you could add this xa when > introducing that change? > Sounds good. I will add that later. I was hoping to get common code in place. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 06/10] iommu/ioasid: Convert to set aware allocations
On Wed, Mar 25, 2020 at 10:55:27AM -0700, Jacob Pan wrote: > The current ioasid_alloc function takes a token/ioasid_set then record it > on the IOASID being allocated. There is no alloc/free on the ioasid_set. > > With the IOASID set APIs, callers must allocate an ioasid_set before > allocate IOASIDs within the set. Quota and other ioasid_set level > activities can then be enforced. > > This patch converts existing API to the new ioasid_set model. > > Signed-off-by: Liu Yi L > Signed-off-by: Jacob Pan [...] > @@ -379,6 +391,9 @@ ioasid_t ioasid_alloc(struct ioasid_set *set, ioasid_t > min, ioasid_t max, > } > data->id = id; > > + /* Store IOASID in the per set data */ > + xa_store(>xa, id, data, GFP_KERNEL); I couldn't figure out why you're maintaining an additional xarray for each set. We're already storing that data in active_allocator->xa, why the duplication? If it's for the gPASID -> hPASID translation mentioned by the cover letter, maybe you could add this xa when introducing that change? Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 06/10] iommu/ioasid: Convert to set aware allocations
> From: Jacob Pan > Sent: Saturday, March 28, 2020 1:42 AM > > On Fri, 27 Mar 2020 09:54:11 + > "Tian, Kevin" wrote: > > > > From: Jacob Pan > > > Sent: Thursday, March 26, 2020 1:55 AM > > > > > > The current ioasid_alloc function takes a token/ioasid_set then > > > record it on the IOASID being allocated. There is no alloc/free on > > > the ioasid_set. > > > > > > With the IOASID set APIs, callers must allocate an ioasid_set before > > > allocate IOASIDs within the set. Quota and other ioasid_set level > > > activities can then be enforced. > > > > > > This patch converts existing API to the new ioasid_set model. > > > > > > Signed-off-by: Liu Yi L > > > Signed-off-by: Jacob Pan > > > --- > > > drivers/iommu/intel-iommu.c | 10 +++--- > > > drivers/iommu/intel-svm.c | 10 +++--- > > > drivers/iommu/ioasid.c | 78 > > > +--- - > > > include/linux/ioasid.h | 11 +++ > > > 4 files changed, 72 insertions(+), 37 deletions(-) > > > > > > diff --git a/drivers/iommu/intel-iommu.c > > > b/drivers/iommu/intel-iommu.c index af7a1ef7b31e..c571cc8d9e57 > > > 100644 --- a/drivers/iommu/intel-iommu.c > > > +++ b/drivers/iommu/intel-iommu.c > > > @@ -3323,11 +3323,11 @@ static void intel_ioasid_free(ioasid_t > > > ioasid, void *data) > > > if (!iommu) > > > return; > > > /* > > > - * Sanity check the ioasid owner is done at upper layer, > > > e.g. VFIO > > > - * We can only free the PASID when all the devices are > > > unbound. > > > + * In the guest, all IOASIDs belong to the system_ioasid > > > set. > > > + * Sanity check against the system set. > > > > below code has nothing to deal with guest, then why putting the > > comment specifically for guest? > > > intel_ioasid_alloc/free() is the custom IOASID allocator only > registered when running in the guest. in that case may be rename the functions to intel_guest_ioasid_alloc/free would avoid similar confusion as I had? > > The custom allocator calls virtual command. Since we don't support > nested guest, all IOASIDs belong to the system ioasid_set. could you put no support of nested guest in the comment, so later when people want to add nested support they will know some additional work required here? > > > >*/ > > > - if (ioasid_find(NULL, ioasid, NULL)) { > > > - pr_alert("Cannot free active IOASID %d\n", ioasid); > > > + if (IS_ERR(ioasid_find(system_ioasid_sid, ioasid, NULL))) { > > > + pr_err("Cannot free IOASID %d, not in system > > > set\n", ioasid); return; > > > } > > > vcmd_free_pasid(iommu, ioasid); > > > @@ -5541,7 +5541,7 @@ static int aux_domain_add_dev(struct > > > dmar_domain *domain, > > > int pasid; > > > > > > /* No private data needed for the default pasid */ > > > - pasid = ioasid_alloc(NULL, PASID_MIN, > > > + pasid = ioasid_alloc(system_ioasid_sid, PASID_MIN, > > >pci_max_pasids(to_pci_dev(dev)) > > > - 1, NULL); > > > if (pasid == INVALID_IOASID) { > > > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c > > > index 1991587fd3fd..f511855d187b 100644 > > > --- a/drivers/iommu/intel-svm.c > > > +++ b/drivers/iommu/intel-svm.c > > > @@ -268,7 +268,7 @@ int intel_svm_bind_gpasid(struct iommu_domain > > > *domain, > > > } > > > > > > mutex_lock(_mutex); > > > - svm = ioasid_find(NULL, data->hpasid, NULL); > > > + svm = ioasid_find(INVALID_IOASID_SET, data->hpasid, NULL); > > > if (IS_ERR(svm)) { > > > ret = PTR_ERR(svm); > > > goto out; > > > @@ -401,7 +401,7 @@ int intel_svm_unbind_gpasid(struct device *dev, > > > int pasid) > > > return -EINVAL; > > > > > > mutex_lock(_mutex); > > > - svm = ioasid_find(NULL, pasid, NULL); > > > + svm = ioasid_find(INVALID_IOASID_SET, pasid, NULL); > > > if (!svm) { > > > ret = -EINVAL; > > > goto out; > > > @@ -559,7 +559,7 @@ static int intel_svm_bind_mm(struct device > > > *dev, int flags, struct svm_dev_ops * > > > pasid_max = intel_pasid_max_id; > > > > > > /* Do not use PASID 0, reserved for RID to PASID */ > > > - svm->pasid = ioasid_alloc(NULL, PASID_MIN, > > > + svm->pasid = ioasid_alloc(system_ioasid_sid, > > > PASID_MIN, pasid_max - 1, svm); > > > if (svm->pasid == INVALID_IOASID) { > > > kfree(svm); > > > @@ -642,7 +642,7 @@ int intel_svm_unbind_mm(struct device *dev, int > > > pasid) > > > if (!iommu) > > > goto out; > > > > > > - svm = ioasid_find(NULL, pasid, NULL); > > > + svm = ioasid_find(system_ioasid_sid, pasid, NULL); > > > if (!svm) > > > goto out; > > > > > > @@ -778,7 +778,7 @@ static irqreturn_t prq_event_thread(int irq, > > > void *d) > > > > > > if (!svm || svm->pasid != req->pasid) { > > > rcu_read_lock(); > > > - svm = ioasid_find(NULL, req->pasid, NULL); > > > + svm
Re: [PATCH 06/10] iommu/ioasid: Convert to set aware allocations
On Fri, 27 Mar 2020 09:54:11 + "Tian, Kevin" wrote: > > From: Jacob Pan > > Sent: Thursday, March 26, 2020 1:55 AM > > > > The current ioasid_alloc function takes a token/ioasid_set then > > record it on the IOASID being allocated. There is no alloc/free on > > the ioasid_set. > > > > With the IOASID set APIs, callers must allocate an ioasid_set before > > allocate IOASIDs within the set. Quota and other ioasid_set level > > activities can then be enforced. > > > > This patch converts existing API to the new ioasid_set model. > > > > Signed-off-by: Liu Yi L > > Signed-off-by: Jacob Pan > > --- > > drivers/iommu/intel-iommu.c | 10 +++--- > > drivers/iommu/intel-svm.c | 10 +++--- > > drivers/iommu/ioasid.c | 78 > > +--- - > > include/linux/ioasid.h | 11 +++ > > 4 files changed, 72 insertions(+), 37 deletions(-) > > > > diff --git a/drivers/iommu/intel-iommu.c > > b/drivers/iommu/intel-iommu.c index af7a1ef7b31e..c571cc8d9e57 > > 100644 --- a/drivers/iommu/intel-iommu.c > > +++ b/drivers/iommu/intel-iommu.c > > @@ -3323,11 +3323,11 @@ static void intel_ioasid_free(ioasid_t > > ioasid, void *data) > > if (!iommu) > > return; > > /* > > -* Sanity check the ioasid owner is done at upper layer, > > e.g. VFIO > > -* We can only free the PASID when all the devices are > > unbound. > > +* In the guest, all IOASIDs belong to the system_ioasid > > set. > > +* Sanity check against the system set. > > below code has nothing to deal with guest, then why putting the > comment specifically for guest? > intel_ioasid_alloc/free() is the custom IOASID allocator only registered when running in the guest. The custom allocator calls virtual command. Since we don't support nested guest, all IOASIDs belong to the system ioasid_set. > > */ > > - if (ioasid_find(NULL, ioasid, NULL)) { > > - pr_alert("Cannot free active IOASID %d\n", ioasid); > > + if (IS_ERR(ioasid_find(system_ioasid_sid, ioasid, NULL))) { > > + pr_err("Cannot free IOASID %d, not in system > > set\n", ioasid); return; > > } > > vcmd_free_pasid(iommu, ioasid); > > @@ -5541,7 +5541,7 @@ static int aux_domain_add_dev(struct > > dmar_domain *domain, > > int pasid; > > > > /* No private data needed for the default pasid */ > > - pasid = ioasid_alloc(NULL, PASID_MIN, > > + pasid = ioasid_alloc(system_ioasid_sid, PASID_MIN, > > pci_max_pasids(to_pci_dev(dev)) > > - 1, NULL); > > if (pasid == INVALID_IOASID) { > > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c > > index 1991587fd3fd..f511855d187b 100644 > > --- a/drivers/iommu/intel-svm.c > > +++ b/drivers/iommu/intel-svm.c > > @@ -268,7 +268,7 @@ int intel_svm_bind_gpasid(struct iommu_domain > > *domain, > > } > > > > mutex_lock(_mutex); > > - svm = ioasid_find(NULL, data->hpasid, NULL); > > + svm = ioasid_find(INVALID_IOASID_SET, data->hpasid, NULL); > > if (IS_ERR(svm)) { > > ret = PTR_ERR(svm); > > goto out; > > @@ -401,7 +401,7 @@ int intel_svm_unbind_gpasid(struct device *dev, > > int pasid) > > return -EINVAL; > > > > mutex_lock(_mutex); > > - svm = ioasid_find(NULL, pasid, NULL); > > + svm = ioasid_find(INVALID_IOASID_SET, pasid, NULL); > > if (!svm) { > > ret = -EINVAL; > > goto out; > > @@ -559,7 +559,7 @@ static int intel_svm_bind_mm(struct device > > *dev, int flags, struct svm_dev_ops * > > pasid_max = intel_pasid_max_id; > > > > /* Do not use PASID 0, reserved for RID to PASID */ > > - svm->pasid = ioasid_alloc(NULL, PASID_MIN, > > + svm->pasid = ioasid_alloc(system_ioasid_sid, > > PASID_MIN, pasid_max - 1, svm); > > if (svm->pasid == INVALID_IOASID) { > > kfree(svm); > > @@ -642,7 +642,7 @@ int intel_svm_unbind_mm(struct device *dev, int > > pasid) > > if (!iommu) > > goto out; > > > > - svm = ioasid_find(NULL, pasid, NULL); > > + svm = ioasid_find(system_ioasid_sid, pasid, NULL); > > if (!svm) > > goto out; > > > > @@ -778,7 +778,7 @@ static irqreturn_t prq_event_thread(int irq, > > void *d) > > > > if (!svm || svm->pasid != req->pasid) { > > rcu_read_lock(); > > - svm = ioasid_find(NULL, req->pasid, NULL); > > + svm = ioasid_find(INVALID_IOASID_SET, > > req->pasid, NULL); > > is there a criteria when INVALID_IOASID_SET should be used? > Two use cases for INVALID_IOASID_SET: 1. a hint to ioasid_find to do global search, ignore set ownership check 2. cannot find a set ID for a given ioasid_find_sid() You brought up a good point, I missed the second use case. > > /* It *can't* go away, because the driver > > is not permitted > >
RE: [PATCH 06/10] iommu/ioasid: Convert to set aware allocations
> From: Jacob Pan > Sent: Thursday, March 26, 2020 1:55 AM > > The current ioasid_alloc function takes a token/ioasid_set then record it > on the IOASID being allocated. There is no alloc/free on the ioasid_set. > > With the IOASID set APIs, callers must allocate an ioasid_set before > allocate IOASIDs within the set. Quota and other ioasid_set level > activities can then be enforced. > > This patch converts existing API to the new ioasid_set model. > > Signed-off-by: Liu Yi L > Signed-off-by: Jacob Pan > --- > drivers/iommu/intel-iommu.c | 10 +++--- > drivers/iommu/intel-svm.c | 10 +++--- > drivers/iommu/ioasid.c | 78 +--- > - > include/linux/ioasid.h | 11 +++ > 4 files changed, 72 insertions(+), 37 deletions(-) > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index af7a1ef7b31e..c571cc8d9e57 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -3323,11 +3323,11 @@ static void intel_ioasid_free(ioasid_t ioasid, void > *data) > if (!iommu) > return; > /* > - * Sanity check the ioasid owner is done at upper layer, e.g. VFIO > - * We can only free the PASID when all the devices are unbound. > + * In the guest, all IOASIDs belong to the system_ioasid set. > + * Sanity check against the system set. below code has nothing to deal with guest, then why putting the comment specifically for guest? >*/ > - if (ioasid_find(NULL, ioasid, NULL)) { > - pr_alert("Cannot free active IOASID %d\n", ioasid); > + if (IS_ERR(ioasid_find(system_ioasid_sid, ioasid, NULL))) { > + pr_err("Cannot free IOASID %d, not in system set\n", ioasid); > return; > } > vcmd_free_pasid(iommu, ioasid); > @@ -5541,7 +5541,7 @@ static int aux_domain_add_dev(struct > dmar_domain *domain, > int pasid; > > /* No private data needed for the default pasid */ > - pasid = ioasid_alloc(NULL, PASID_MIN, > + pasid = ioasid_alloc(system_ioasid_sid, PASID_MIN, >pci_max_pasids(to_pci_dev(dev)) - 1, >NULL); > if (pasid == INVALID_IOASID) { > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c > index 1991587fd3fd..f511855d187b 100644 > --- a/drivers/iommu/intel-svm.c > +++ b/drivers/iommu/intel-svm.c > @@ -268,7 +268,7 @@ int intel_svm_bind_gpasid(struct iommu_domain > *domain, > } > > mutex_lock(_mutex); > - svm = ioasid_find(NULL, data->hpasid, NULL); > + svm = ioasid_find(INVALID_IOASID_SET, data->hpasid, NULL); > if (IS_ERR(svm)) { > ret = PTR_ERR(svm); > goto out; > @@ -401,7 +401,7 @@ int intel_svm_unbind_gpasid(struct device *dev, int > pasid) > return -EINVAL; > > mutex_lock(_mutex); > - svm = ioasid_find(NULL, pasid, NULL); > + svm = ioasid_find(INVALID_IOASID_SET, pasid, NULL); > if (!svm) { > ret = -EINVAL; > goto out; > @@ -559,7 +559,7 @@ static int intel_svm_bind_mm(struct device *dev, int > flags, struct svm_dev_ops * > pasid_max = intel_pasid_max_id; > > /* Do not use PASID 0, reserved for RID to PASID */ > - svm->pasid = ioasid_alloc(NULL, PASID_MIN, > + svm->pasid = ioasid_alloc(system_ioasid_sid, PASID_MIN, > pasid_max - 1, svm); > if (svm->pasid == INVALID_IOASID) { > kfree(svm); > @@ -642,7 +642,7 @@ int intel_svm_unbind_mm(struct device *dev, int > pasid) > if (!iommu) > goto out; > > - svm = ioasid_find(NULL, pasid, NULL); > + svm = ioasid_find(system_ioasid_sid, pasid, NULL); > if (!svm) > goto out; > > @@ -778,7 +778,7 @@ static irqreturn_t prq_event_thread(int irq, void *d) > > if (!svm || svm->pasid != req->pasid) { > rcu_read_lock(); > - svm = ioasid_find(NULL, req->pasid, NULL); > + svm = ioasid_find(INVALID_IOASID_SET, req->pasid, > NULL); is there a criteria when INVALID_IOASID_SET should be used? > /* 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. */ > diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c > index 9135af171a7c..f89a595f6978 100644 > --- a/drivers/iommu/ioasid.c > +++ b/drivers/iommu/ioasid.c > @@ -31,7 +31,7 @@ struct ioasid_set_data { > > struct ioasid_data { > ioasid_t id; > - struct ioasid_set *set; > + struct ioasid_set_data *sdata; > void *private; > struct rcu_head rcu; > }; > @@ -334,7 +334,7 @@
[PATCH 06/10] iommu/ioasid: Convert to set aware allocations
The current ioasid_alloc function takes a token/ioasid_set then record it on the IOASID being allocated. There is no alloc/free on the ioasid_set. With the IOASID set APIs, callers must allocate an ioasid_set before allocate IOASIDs within the set. Quota and other ioasid_set level activities can then be enforced. This patch converts existing API to the new ioasid_set model. Signed-off-by: Liu Yi L Signed-off-by: Jacob Pan --- drivers/iommu/intel-iommu.c | 10 +++--- drivers/iommu/intel-svm.c | 10 +++--- drivers/iommu/ioasid.c | 78 + include/linux/ioasid.h | 11 +++ 4 files changed, 72 insertions(+), 37 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index af7a1ef7b31e..c571cc8d9e57 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -3323,11 +3323,11 @@ static void intel_ioasid_free(ioasid_t ioasid, void *data) if (!iommu) return; /* -* Sanity check the ioasid owner is done at upper layer, e.g. VFIO -* We can only free the PASID when all the devices are unbound. +* In the guest, all IOASIDs belong to the system_ioasid set. +* Sanity check against the system set. */ - if (ioasid_find(NULL, ioasid, NULL)) { - pr_alert("Cannot free active IOASID %d\n", ioasid); + if (IS_ERR(ioasid_find(system_ioasid_sid, ioasid, NULL))) { + pr_err("Cannot free IOASID %d, not in system set\n", ioasid); return; } vcmd_free_pasid(iommu, ioasid); @@ -5541,7 +5541,7 @@ static int aux_domain_add_dev(struct dmar_domain *domain, int pasid; /* No private data needed for the default pasid */ - pasid = ioasid_alloc(NULL, PASID_MIN, + pasid = ioasid_alloc(system_ioasid_sid, PASID_MIN, pci_max_pasids(to_pci_dev(dev)) - 1, NULL); if (pasid == INVALID_IOASID) { diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c index 1991587fd3fd..f511855d187b 100644 --- a/drivers/iommu/intel-svm.c +++ b/drivers/iommu/intel-svm.c @@ -268,7 +268,7 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, } mutex_lock(_mutex); - svm = ioasid_find(NULL, data->hpasid, NULL); + svm = ioasid_find(INVALID_IOASID_SET, data->hpasid, NULL); if (IS_ERR(svm)) { ret = PTR_ERR(svm); goto out; @@ -401,7 +401,7 @@ int intel_svm_unbind_gpasid(struct device *dev, int pasid) return -EINVAL; mutex_lock(_mutex); - svm = ioasid_find(NULL, pasid, NULL); + svm = ioasid_find(INVALID_IOASID_SET, pasid, NULL); if (!svm) { ret = -EINVAL; goto out; @@ -559,7 +559,7 @@ static int intel_svm_bind_mm(struct device *dev, int flags, struct svm_dev_ops * pasid_max = intel_pasid_max_id; /* Do not use PASID 0, reserved for RID to PASID */ - svm->pasid = ioasid_alloc(NULL, PASID_MIN, + svm->pasid = ioasid_alloc(system_ioasid_sid, PASID_MIN, pasid_max - 1, svm); if (svm->pasid == INVALID_IOASID) { kfree(svm); @@ -642,7 +642,7 @@ int intel_svm_unbind_mm(struct device *dev, int pasid) if (!iommu) goto out; - svm = ioasid_find(NULL, pasid, NULL); + svm = ioasid_find(system_ioasid_sid, pasid, NULL); if (!svm) goto out; @@ -778,7 +778,7 @@ static irqreturn_t prq_event_thread(int irq, void *d) if (!svm || svm->pasid != req->pasid) { rcu_read_lock(); - svm = ioasid_find(NULL, req->pasid, NULL); + svm = ioasid_find(INVALID_IOASID_SET, 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. */ diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c index 9135af171a7c..f89a595f6978 100644 --- a/drivers/iommu/ioasid.c +++ b/drivers/iommu/ioasid.c @@ -31,7 +31,7 @@ struct ioasid_set_data { struct ioasid_data { ioasid_t id; - struct ioasid_set *set; + struct ioasid_set_data *sdata; void *private; struct rcu_head rcu; }; @@ -334,7 +334,7 @@ EXPORT_SYMBOL_GPL(ioasid_attach_data); /** * ioasid_alloc - Allocate an IOASID - * @set: the IOASID set + * @sid: the IOASID set ID * @min: the minimum ID (inclusive) * @max: the maximum ID (inclusive) * @private: data private to the caller @@ -344,18 +344,30 @@ EXPORT_SYMBOL_GPL(ioasid_attach_data); * * Return: the allocated