2023-01-09 02:11:40

by Lu Baolu

[permalink] [raw]
Subject: [PATCH 0/4] iommu/vt-d: SVM implementation cleanup

This is a cleanup series after the merge of commit be51b1d6bbff
("iommu/sva: Refactoring iommu_sva_bind/unbind_device()"). It removes
the unused/duplicate data and code.

Lu Baolu (4):
iommu/vt-d: Remove include/linux/intel-svm.h
iommu/vt-d: Remove unused fields in svm structures
iommu/vt-d: Remove users from intel_svm_dev
iommu/vt-d: Remove sva from intel_svm_dev

include/linux/intel-svm.h | 16 -------
drivers/iommu/intel/iommu.h | 11 +++--
drivers/iommu/intel/iommu.c | 1 -
drivers/iommu/intel/svm.c | 88 +++++++++++++++----------------------
MAINTAINERS | 1 -
5 files changed, 41 insertions(+), 76 deletions(-)
delete mode 100644 include/linux/intel-svm.h

--
2.34.1


2023-01-09 02:18:03

by Lu Baolu

[permalink] [raw]
Subject: [PATCH 3/4] iommu/vt-d: Remove users from intel_svm_dev

It was used as a reference counter of an existing bond between device
and user application memory address. Commit be51b1d6bbff ("iommu/sva:
Refactoring iommu_sva_bind/unbind_device()") has added this in iommu
core. Remove it to avoid duplicate code.

Signed-off-by: Lu Baolu <[email protected]>
---
drivers/iommu/intel/iommu.h | 1 -
drivers/iommu/intel/svm.c | 62 ++++++++++++++++---------------------
2 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index e7c732979364..2a1619ff0d79 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -762,7 +762,6 @@ struct intel_svm_dev {
struct device *dev;
struct intel_iommu *iommu;
struct iommu_sva sva;
- int users;
u16 did;
u16 sid, qdep;
};
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index d1e445f03aa6..c7dc53e40c26 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -333,13 +333,6 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
}
}

- /* Find the matching device in svm list */
- sdev = svm_lookup_device_by_dev(svm, dev);
- if (sdev) {
- sdev->users++;
- goto success;
- }
-
sdev = kzalloc(sizeof(*sdev), GFP_KERNEL);
if (!sdev) {
ret = -ENOMEM;
@@ -350,7 +343,6 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
sdev->iommu = iommu;
sdev->did = FLPT_DEFAULT_DID;
sdev->sid = PCI_DEVID(info->bus, info->devfn);
- sdev->users = 1;
sdev->sva.dev = dev;
init_rcu_head(&sdev->rcu);
if (info->ats_enabled) {
@@ -367,7 +359,7 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
goto free_sdev;

list_add_rcu(&sdev->list, &svm->devs);
-success:
+
return &sdev->sva;

free_sdev:
@@ -401,32 +393,32 @@ static int intel_svm_unbind_mm(struct device *dev, u32 pasid)
mm = svm->mm;

if (sdev) {
- sdev->users--;
- if (!sdev->users) {
- list_del_rcu(&sdev->list);
- /* Flush the PASID cache and IOTLB for this device.
- * Note that we do depend on the hardware *not* using
- * the PASID any more. Just as we depend on other
- * devices never using PASIDs that they have no right
- * to use. We have a *shared* PASID table, because it's
- * large and has to be physically contiguous. So it's
- * hard to be as defensive as we might like. */
- intel_pasid_tear_down_entry(iommu, dev,
- svm->pasid, false);
- intel_svm_drain_prq(dev, svm->pasid);
- kfree_rcu(sdev, rcu);
-
- if (list_empty(&svm->devs)) {
- if (svm->notifier.ops)
- mmu_notifier_unregister(&svm->notifier, mm);
- pasid_private_remove(svm->pasid);
- /* We mandate that no page faults may be outstanding
- * for the PASID when intel_svm_unbind_mm() is called.
- * If that is not obeyed, subtle errors will happen.
- * Let's make them less subtle... */
- memset(svm, 0x6b, sizeof(*svm));
- kfree(svm);
- }
+ list_del_rcu(&sdev->list);
+ /*
+ * Flush the PASID cache and IOTLB for this device.
+ * Note that we do depend on the hardware *not* using
+ * the PASID any more. Just as we depend on other
+ * devices never using PASIDs that they have no right
+ * to use. We have a *shared* PASID table, because it's
+ * large and has to be physically contiguous. So it's
+ * hard to be as defensive as we might like.
+ */
+ intel_pasid_tear_down_entry(iommu, dev, svm->pasid, false);
+ intel_svm_drain_prq(dev, svm->pasid);
+ kfree_rcu(sdev, rcu);
+
+ if (list_empty(&svm->devs)) {
+ if (svm->notifier.ops)
+ mmu_notifier_unregister(&svm->notifier, mm);
+ pasid_private_remove(svm->pasid);
+ /*
+ * We mandate that no page faults may be outstanding
+ * for the PASID when intel_svm_unbind_mm() is called.
+ * If that is not obeyed, subtle errors will happen.
+ * Let's make them less subtle...
+ */
+ memset(svm, 0x6b, sizeof(*svm));
+ kfree(svm);
}
}
out:
--
2.34.1

2023-01-09 03:05:20

by Lu Baolu

[permalink] [raw]
Subject: [PATCH 4/4] iommu/vt-d: Remove sva from intel_svm_dev

After commit be51b1d6bbff ("iommu/sva: Refactoring
iommu_sva_bind/unbind_device()"), the iommu driver doesn't need to
return an iommu_sva pointer anymore. This removes the sva field
from intel_svm_dev and cleanups the code accordingly.

Signed-off-by: Lu Baolu <[email protected]>
---
drivers/iommu/intel/iommu.h | 1 -
drivers/iommu/intel/svm.c | 23 +++++++++--------------
2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 2a1619ff0d79..f0222ad38757 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -761,7 +761,6 @@ struct intel_svm_dev {
struct rcu_head rcu;
struct device *dev;
struct intel_iommu *iommu;
- struct iommu_sva sva;
u16 did;
u16 sid, qdep;
};
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index c7dc53e40c26..e7b9bedebcc0 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -298,9 +298,8 @@ static int pasid_to_svm_sdev(struct device *dev, unsigned int pasid,
return 0;
}

-static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
- struct device *dev,
- struct mm_struct *mm)
+static int intel_svm_bind_mm(struct intel_iommu *iommu, struct device *dev,
+ struct mm_struct *mm)
{
struct device_domain_info *info = dev_iommu_priv_get(dev);
struct intel_svm_dev *sdev;
@@ -312,7 +311,7 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
if (!svm) {
svm = kzalloc(sizeof(*svm), GFP_KERNEL);
if (!svm)
- return ERR_PTR(-ENOMEM);
+ return -ENOMEM;

svm->pasid = mm->pasid;
svm->mm = mm;
@@ -322,14 +321,14 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
ret = mmu_notifier_register(&svm->notifier, mm);
if (ret) {
kfree(svm);
- return ERR_PTR(ret);
+ return ret;
}

ret = pasid_private_add(svm->pasid, svm);
if (ret) {
mmu_notifier_unregister(&svm->notifier, mm);
kfree(svm);
- return ERR_PTR(ret);
+ return ret;
}
}

@@ -343,7 +342,6 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
sdev->iommu = iommu;
sdev->did = FLPT_DEFAULT_DID;
sdev->sid = PCI_DEVID(info->bus, info->devfn);
- sdev->sva.dev = dev;
init_rcu_head(&sdev->rcu);
if (info->ats_enabled) {
sdev->qdep = info->ats_qdep;
@@ -360,7 +358,7 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,

list_add_rcu(&sdev->list, &svm->devs);

- return &sdev->sva;
+ return 0;

free_sdev:
kfree(sdev);
@@ -371,7 +369,7 @@ static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
kfree(svm);
}

- return ERR_PTR(ret);
+ return ret;
}

/* Caller must hold pasid_mutex */
@@ -843,13 +841,10 @@ static int intel_svm_set_dev_pasid(struct iommu_domain *domain,
struct device_domain_info *info = dev_iommu_priv_get(dev);
struct intel_iommu *iommu = info->iommu;
struct mm_struct *mm = domain->mm;
- struct iommu_sva *sva;
- int ret = 0;
+ int ret;

mutex_lock(&pasid_mutex);
- sva = intel_svm_bind_mm(iommu, dev, mm);
- if (IS_ERR(sva))
- ret = PTR_ERR(sva);
+ ret = intel_svm_bind_mm(iommu, dev, mm);
mutex_unlock(&pasid_mutex);

return ret;
--
2.34.1

2023-01-10 03:04:39

by Tian, Kevin

[permalink] [raw]
Subject: RE: [PATCH 0/4] iommu/vt-d: SVM implementation cleanup

> From: Lu Baolu <[email protected]>
> Sent: Monday, January 9, 2023 9:50 AM
>
> This is a cleanup series after the merge of commit be51b1d6bbff
> ("iommu/sva: Refactoring iommu_sva_bind/unbind_device()"). It removes
> the unused/duplicate data and code.
>
> Lu Baolu (4):
> iommu/vt-d: Remove include/linux/intel-svm.h
> iommu/vt-d: Remove unused fields in svm structures
> iommu/vt-d: Remove users from intel_svm_dev
> iommu/vt-d: Remove sva from intel_svm_dev
>

For the series:

Reviewed-by: Kevin Tian <[email protected]>

2023-01-31 07:51:39

by Lu Baolu

[permalink] [raw]
Subject: Re: [PATCH 0/4] iommu/vt-d: SVM implementation cleanup

On 2023/1/9 9:49, Lu Baolu wrote:
> This is a cleanup series after the merge of commit be51b1d6bbff
> ("iommu/sva: Refactoring iommu_sva_bind/unbind_device()"). It removes
> the unused/duplicate data and code.
>
> Lu Baolu (4):
> iommu/vt-d: Remove include/linux/intel-svm.h
> iommu/vt-d: Remove unused fields in svm structures
> iommu/vt-d: Remove users from intel_svm_dev
> iommu/vt-d: Remove sva from intel_svm_dev

Patches queued for v6.3.

https://lore.kernel.org/linux-iommu/[email protected]/

Best regards,
baolu