* [PATCH v4 1/8] iommu/arm-smmu-v3: Put iopf enablement in the domain attach path
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
@ 2025-03-13 5:19 ` Lu Baolu
2025-03-13 5:19 ` [PATCH v4 2/8] iommu: Remove IOMMU_DEV_FEAT_SVA Lu Baolu
` (9 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Lu Baolu @ 2025-03-13 5:19 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel, Jason Gunthorpe, Lu Baolu
From: Jason Gunthorpe <jgg@nvidia.com>
SMMUv3 co-mingles FEAT_IOPF and FEAT_SVA behaviors so that fault reporting
doesn't work unless both are enabled. This is not correct and causes
problems for iommufd which does not enable FEAT_SVA for it's fault capable
domains.
These APIs are both obsolete, update SMMUv3 to use the new method like AMD
implements.
A driver should enable iopf support when a domain with an iopf_handler is
attached, and disable iopf support when the domain is removed.
Move the fault support logic to sva domain allocation and to domain
attach, refusing to create or attach fault capable domains if the HW
doesn't support it.
Move all the logic for controlling the iopf queue under
arm_smmu_attach_prepare(). Keep track of the number of domains on the
master (over all the SSIDs) that require iopf. When the first domain
requiring iopf is attached create the iopf queue, when the last domain is
detached destroy it.
Turn FEAT_IOPF and FEAT_SVA into no ops.
Remove the sva_lock, this is all protected by the group mutex.
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Acked-by: Will Deacon <will@kernel.org>
---
.../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 86 +-------------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 105 +++++++++++++-----
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 39 ++-----
3 files changed, 91 insertions(+), 139 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 9ba596430e7c..605d1dd0e1cc 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -13,8 +13,6 @@
#include "arm-smmu-v3.h"
#include "../../io-pgtable-arm.h"
-static DEFINE_MUTEX(sva_lock);
-
static void __maybe_unused
arm_smmu_update_s1_domain_cd_entry(struct arm_smmu_domain *smmu_domain)
{
@@ -257,84 +255,6 @@ bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
return true;
}
-bool arm_smmu_master_iopf_supported(struct arm_smmu_master *master)
-{
- /* We're not keeping track of SIDs in fault events */
- if (master->num_streams != 1)
- return false;
-
- return master->stall_enabled;
-}
-
-bool arm_smmu_master_sva_supported(struct arm_smmu_master *master)
-{
- if (!(master->smmu->features & ARM_SMMU_FEAT_SVA))
- return false;
-
- /* SSID support is mandatory for the moment */
- return master->ssid_bits;
-}
-
-bool arm_smmu_master_sva_enabled(struct arm_smmu_master *master)
-{
- bool enabled;
-
- mutex_lock(&sva_lock);
- enabled = master->sva_enabled;
- mutex_unlock(&sva_lock);
- return enabled;
-}
-
-static int arm_smmu_master_sva_enable_iopf(struct arm_smmu_master *master)
-{
- struct device *dev = master->dev;
-
- /*
- * Drivers for devices supporting PRI or stall should enable IOPF first.
- * Others have device-specific fault handlers and don't need IOPF.
- */
- if (!arm_smmu_master_iopf_supported(master))
- return 0;
-
- if (!master->iopf_enabled)
- return -EINVAL;
-
- return iopf_queue_add_device(master->smmu->evtq.iopf, dev);
-}
-
-static void arm_smmu_master_sva_disable_iopf(struct arm_smmu_master *master)
-{
- struct device *dev = master->dev;
-
- if (!master->iopf_enabled)
- return;
-
- iopf_queue_remove_device(master->smmu->evtq.iopf, dev);
-}
-
-int arm_smmu_master_enable_sva(struct arm_smmu_master *master)
-{
- int ret;
-
- mutex_lock(&sva_lock);
- ret = arm_smmu_master_sva_enable_iopf(master);
- if (!ret)
- master->sva_enabled = true;
- mutex_unlock(&sva_lock);
-
- return ret;
-}
-
-int arm_smmu_master_disable_sva(struct arm_smmu_master *master)
-{
- mutex_lock(&sva_lock);
- arm_smmu_master_sva_disable_iopf(master);
- master->sva_enabled = false;
- mutex_unlock(&sva_lock);
-
- return 0;
-}
-
void arm_smmu_sva_notifier_synchronize(void)
{
/*
@@ -353,6 +273,9 @@ static int arm_smmu_sva_set_dev_pasid(struct iommu_domain *domain,
struct arm_smmu_cd target;
int ret;
+ if (!(master->smmu->features & ARM_SMMU_FEAT_SVA))
+ return -EOPNOTSUPP;
+
/* Prevent arm_smmu_mm_release from being called while we are attaching */
if (!mmget_not_zero(domain->mm))
return -EINVAL;
@@ -406,6 +329,9 @@ struct iommu_domain *arm_smmu_sva_domain_alloc(struct device *dev,
u32 asid;
int ret;
+ if (!(master->smmu->features & ARM_SMMU_FEAT_SVA))
+ return ERR_PTR(-EOPNOTSUPP);
+
smmu_domain = arm_smmu_domain_alloc();
if (IS_ERR(smmu_domain))
return ERR_CAST(smmu_domain);
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 358072b4e293..a8b1b8b17992 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2712,6 +2712,7 @@ static void arm_smmu_disable_pasid(struct arm_smmu_master *master)
static struct arm_smmu_master_domain *
arm_smmu_find_master_domain(struct arm_smmu_domain *smmu_domain,
+ struct iommu_domain *domain,
struct arm_smmu_master *master,
ioasid_t ssid, bool nested_ats_flush)
{
@@ -2722,6 +2723,7 @@ arm_smmu_find_master_domain(struct arm_smmu_domain *smmu_domain,
list_for_each_entry(master_domain, &smmu_domain->devices,
devices_elm) {
if (master_domain->master == master &&
+ master_domain->domain == domain &&
master_domain->ssid == ssid &&
master_domain->nested_ats_flush == nested_ats_flush)
return master_domain;
@@ -2748,6 +2750,58 @@ to_smmu_domain_devices(struct iommu_domain *domain)
return NULL;
}
+static int arm_smmu_enable_iopf(struct arm_smmu_master *master,
+ struct arm_smmu_master_domain *master_domain)
+{
+ int ret;
+
+ iommu_group_mutex_assert(master->dev);
+
+ if (!IS_ENABLED(CONFIG_ARM_SMMU_V3_SVA))
+ return -EOPNOTSUPP;
+
+ /*
+ * Drivers for devices supporting PRI or stall require iopf others have
+ * device-specific fault handlers and don't need IOPF, so this is not a
+ * failure.
+ */
+ if (!master->stall_enabled)
+ return 0;
+
+ /* We're not keeping track of SIDs in fault events */
+ if (master->num_streams != 1)
+ return -EOPNOTSUPP;
+
+ if (master->iopf_refcount) {
+ master->iopf_refcount++;
+ master_domain->using_iopf = true;
+ return 0;
+ }
+
+ ret = iopf_queue_add_device(master->smmu->evtq.iopf, master->dev);
+ if (ret)
+ return ret;
+ master->iopf_refcount = 1;
+ master_domain->using_iopf = true;
+ return 0;
+}
+
+static void arm_smmu_disable_iopf(struct arm_smmu_master *master,
+ struct arm_smmu_master_domain *master_domain)
+{
+ iommu_group_mutex_assert(master->dev);
+
+ if (!IS_ENABLED(CONFIG_ARM_SMMU_V3_SVA))
+ return;
+
+ if (!master_domain || !master_domain->using_iopf)
+ return;
+
+ master->iopf_refcount--;
+ if (master->iopf_refcount == 0)
+ iopf_queue_remove_device(master->smmu->evtq.iopf, master->dev);
+}
+
static void arm_smmu_remove_master_domain(struct arm_smmu_master *master,
struct iommu_domain *domain,
ioasid_t ssid)
@@ -2764,15 +2818,17 @@ static void arm_smmu_remove_master_domain(struct arm_smmu_master *master,
nested_ats_flush = to_smmu_nested_domain(domain)->enable_ats;
spin_lock_irqsave(&smmu_domain->devices_lock, flags);
- master_domain = arm_smmu_find_master_domain(smmu_domain, master, ssid,
- nested_ats_flush);
+ master_domain = arm_smmu_find_master_domain(smmu_domain, domain, master,
+ ssid, nested_ats_flush);
if (master_domain) {
list_del(&master_domain->devices_elm);
- kfree(master_domain);
if (master->ats_enabled)
atomic_dec(&smmu_domain->nr_ats_masters);
}
spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+
+ arm_smmu_disable_iopf(master, master_domain);
+ kfree(master_domain);
}
/*
@@ -2803,6 +2859,7 @@ int arm_smmu_attach_prepare(struct arm_smmu_attach_state *state,
struct arm_smmu_domain *smmu_domain =
to_smmu_domain_devices(new_domain);
unsigned long flags;
+ int ret;
/*
* arm_smmu_share_asid() must not see two domains pointing to the same
@@ -2835,12 +2892,19 @@ int arm_smmu_attach_prepare(struct arm_smmu_attach_state *state,
master_domain = kzalloc(sizeof(*master_domain), GFP_KERNEL);
if (!master_domain)
return -ENOMEM;
+ master_domain->domain = new_domain;
master_domain->master = master;
master_domain->ssid = state->ssid;
if (new_domain->type == IOMMU_DOMAIN_NESTED)
master_domain->nested_ats_flush =
to_smmu_nested_domain(new_domain)->enable_ats;
+ if (new_domain->iopf_handler) {
+ ret = arm_smmu_enable_iopf(master, master_domain);
+ if (ret)
+ goto err_free_master_domain;
+ }
+
/*
* During prepare we want the current smmu_domain and new
* smmu_domain to be in the devices list before we change any
@@ -2860,8 +2924,8 @@ int arm_smmu_attach_prepare(struct arm_smmu_attach_state *state,
!arm_smmu_master_canwbs(master)) {
spin_unlock_irqrestore(&smmu_domain->devices_lock,
flags);
- kfree(master_domain);
- return -EINVAL;
+ ret = -EINVAL;
+ goto err_iopf;
}
if (state->ats_enabled)
@@ -2880,6 +2944,12 @@ int arm_smmu_attach_prepare(struct arm_smmu_attach_state *state,
wmb();
}
return 0;
+
+err_iopf:
+ arm_smmu_disable_iopf(master, master_domain);
+err_free_master_domain:
+ kfree(master_domain);
+ return ret;
}
/*
@@ -3475,8 +3545,7 @@ static void arm_smmu_release_device(struct device *dev)
{
struct arm_smmu_master *master = dev_iommu_priv_get(dev);
- if (WARN_ON(arm_smmu_master_sva_enabled(master)))
- iopf_queue_remove_device(master->smmu->evtq.iopf, dev);
+ WARN_ON(master->iopf_refcount);
/* Put the STE back to what arm_smmu_init_strtab() sets */
if (dev->iommu->require_direct)
@@ -3561,18 +3630,8 @@ static int arm_smmu_dev_enable_feature(struct device *dev,
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
- if (!arm_smmu_master_iopf_supported(master))
- return -EINVAL;
- if (master->iopf_enabled)
- return -EBUSY;
- master->iopf_enabled = true;
- return 0;
case IOMMU_DEV_FEAT_SVA:
- if (!arm_smmu_master_sva_supported(master))
- return -EINVAL;
- if (arm_smmu_master_sva_enabled(master))
- return -EBUSY;
- return arm_smmu_master_enable_sva(master);
+ return 0;
default:
return -EINVAL;
}
@@ -3588,16 +3647,8 @@ static int arm_smmu_dev_disable_feature(struct device *dev,
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
- if (!master->iopf_enabled)
- return -EINVAL;
- if (master->sva_enabled)
- return -EBUSY;
- master->iopf_enabled = false;
- return 0;
case IOMMU_DEV_FEAT_SVA:
- if (!arm_smmu_master_sva_enabled(master))
- return -EINVAL;
- return arm_smmu_master_disable_sva(master);
+ return 0;
default:
return -EINVAL;
}
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index bd9d7c85576a..fe6b88affa4a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -830,9 +830,8 @@ struct arm_smmu_master {
bool ats_enabled : 1;
bool ste_ats_enabled : 1;
bool stall_enabled;
- bool sva_enabled;
- bool iopf_enabled;
unsigned int ssid_bits;
+ unsigned int iopf_refcount;
};
/* SMMU private data for an IOMMU domain */
@@ -908,8 +907,14 @@ void arm_smmu_make_sva_cd(struct arm_smmu_cd *target,
struct arm_smmu_master_domain {
struct list_head devices_elm;
struct arm_smmu_master *master;
+ /*
+ * For nested domains the master_domain is threaded onto the S2 parent,
+ * this points to the IOMMU_DOMAIN_NESTED to disambiguate the masters.
+ */
+ struct iommu_domain *domain;
ioasid_t ssid;
bool nested_ats_flush : 1;
+ bool using_iopf : 1;
};
static inline struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
@@ -987,11 +992,6 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
#ifdef CONFIG_ARM_SMMU_V3_SVA
bool arm_smmu_sva_supported(struct arm_smmu_device *smmu);
-bool arm_smmu_master_sva_supported(struct arm_smmu_master *master);
-bool arm_smmu_master_sva_enabled(struct arm_smmu_master *master);
-int arm_smmu_master_enable_sva(struct arm_smmu_master *master);
-int arm_smmu_master_disable_sva(struct arm_smmu_master *master);
-bool arm_smmu_master_iopf_supported(struct arm_smmu_master *master);
void arm_smmu_sva_notifier_synchronize(void);
struct iommu_domain *arm_smmu_sva_domain_alloc(struct device *dev,
struct mm_struct *mm);
@@ -1001,31 +1001,6 @@ static inline bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
return false;
}
-static inline bool arm_smmu_master_sva_supported(struct arm_smmu_master *master)
-{
- return false;
-}
-
-static inline bool arm_smmu_master_sva_enabled(struct arm_smmu_master *master)
-{
- return false;
-}
-
-static inline int arm_smmu_master_enable_sva(struct arm_smmu_master *master)
-{
- return -ENODEV;
-}
-
-static inline int arm_smmu_master_disable_sva(struct arm_smmu_master *master)
-{
- return -ENODEV;
-}
-
-static inline bool arm_smmu_master_iopf_supported(struct arm_smmu_master *master)
-{
- return false;
-}
-
static inline void arm_smmu_sva_notifier_synchronize(void) {}
#define arm_smmu_sva_domain_alloc NULL
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v4 2/8] iommu: Remove IOMMU_DEV_FEAT_SVA
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
2025-03-13 5:19 ` [PATCH v4 1/8] iommu/arm-smmu-v3: Put iopf enablement in the domain attach path Lu Baolu
@ 2025-03-13 5:19 ` Lu Baolu
2025-03-13 5:19 ` [PATCH v4 3/8] iommu/vt-d: Put iopf enablement in domain attach path Lu Baolu
` (8 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Lu Baolu @ 2025-03-13 5:19 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel, Jason Gunthorpe, Lu Baolu, Yi Liu
From: Jason Gunthorpe <jgg@nvidia.com>
None of the drivers implement anything here anymore, remove the dead code.
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
drivers/accel/amdxdna/aie2_pci.c | 13 ++-----------
drivers/dma/idxd/init.c | 8 +-------
drivers/iommu/amd/iommu.c | 2 --
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 --
drivers/iommu/intel/iommu.c | 6 ------
drivers/iommu/iommu-sva.c | 3 ---
drivers/misc/uacce/uacce.c | 9 ---------
include/linux/iommu.h | 9 +--------
8 files changed, 4 insertions(+), 48 deletions(-)
diff --git a/drivers/accel/amdxdna/aie2_pci.c b/drivers/accel/amdxdna/aie2_pci.c
index 5a058e565b01..c6cf7068d23c 100644
--- a/drivers/accel/amdxdna/aie2_pci.c
+++ b/drivers/accel/amdxdna/aie2_pci.c
@@ -512,12 +512,6 @@ static int aie2_init(struct amdxdna_dev *xdna)
goto release_fw;
}
- ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
- if (ret) {
- XDNA_ERR(xdna, "Enable PASID failed, ret %d", ret);
- goto free_irq;
- }
-
psp_conf.fw_size = fw->size;
psp_conf.fw_buf = fw->data;
for (i = 0; i < PSP_MAX_REGS; i++)
@@ -526,14 +520,14 @@ static int aie2_init(struct amdxdna_dev *xdna)
if (!ndev->psp_hdl) {
XDNA_ERR(xdna, "failed to create psp");
ret = -ENOMEM;
- goto disable_sva;
+ goto free_irq;
}
xdna->dev_handle = ndev;
ret = aie2_hw_start(xdna);
if (ret) {
XDNA_ERR(xdna, "start npu failed, ret %d", ret);
- goto disable_sva;
+ goto free_irq;
}
ret = aie2_mgmt_fw_query(ndev);
@@ -584,8 +578,6 @@ static int aie2_init(struct amdxdna_dev *xdna)
aie2_error_async_events_free(ndev);
stop_hw:
aie2_hw_stop(xdna);
-disable_sva:
- iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
free_irq:
pci_free_irq_vectors(pdev);
release_fw:
@@ -601,7 +593,6 @@ static void aie2_fini(struct amdxdna_dev *xdna)
aie2_hw_stop(xdna);
aie2_error_async_events_free(ndev);
- iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
pci_free_irq_vectors(pdev);
}
diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index b946f78f85e1..1e5038cca22c 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -633,17 +633,11 @@ static int idxd_enable_sva(struct pci_dev *pdev)
ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
if (ret)
return ret;
-
- ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
- if (ret)
- iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
-
- return ret;
+ return 0;
}
static void idxd_disable_sva(struct pci_dev *pdev)
{
- iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
}
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 6e4fd6351646..25449a564804 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2987,7 +2987,6 @@ static int amd_iommu_dev_enable_feature(struct device *dev,
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
- case IOMMU_DEV_FEAT_SVA:
break;
default:
ret = -EINVAL;
@@ -3003,7 +3002,6 @@ static int amd_iommu_dev_disable_feature(struct device *dev,
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
- case IOMMU_DEV_FEAT_SVA:
break;
default:
ret = -EINVAL;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index a8b1b8b17992..60c8f1fa5bf0 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3630,7 +3630,6 @@ static int arm_smmu_dev_enable_feature(struct device *dev,
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
- case IOMMU_DEV_FEAT_SVA:
return 0;
default:
return -EINVAL;
@@ -3647,7 +3646,6 @@ static int arm_smmu_dev_disable_feature(struct device *dev,
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
- case IOMMU_DEV_FEAT_SVA:
return 0;
default:
return -EINVAL;
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 85aa66ef4d61..8d557d04a273 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3945,9 +3945,6 @@ intel_iommu_dev_enable_feat(struct device *dev, enum iommu_dev_features feat)
case IOMMU_DEV_FEAT_IOPF:
return intel_iommu_enable_iopf(dev);
- case IOMMU_DEV_FEAT_SVA:
- return 0;
-
default:
return -ENODEV;
}
@@ -3961,9 +3958,6 @@ intel_iommu_dev_disable_feat(struct device *dev, enum iommu_dev_features feat)
intel_iommu_disable_iopf(dev);
return 0;
- case IOMMU_DEV_FEAT_SVA:
- return 0;
-
default:
return -ENODEV;
}
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index 503c5d23c1ea..331be2761a75 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -63,9 +63,6 @@ static struct iommu_mm_data *iommu_alloc_mm_data(struct mm_struct *mm, struct de
* reference is taken. Caller must call iommu_sva_unbind_device()
* to release each reference.
*
- * iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) must be called first, to
- * initialize the required SVA features.
- *
* On error, returns an ERR_PTR value.
*/
struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
index bdc2e6fda782..2a1db2abeeca 100644
--- a/drivers/misc/uacce/uacce.c
+++ b/drivers/misc/uacce/uacce.c
@@ -479,14 +479,6 @@ static unsigned int uacce_enable_sva(struct device *parent, unsigned int flags)
dev_err(parent, "failed to enable IOPF feature! ret = %pe\n", ERR_PTR(ret));
return flags;
}
-
- ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA);
- if (ret) {
- dev_err(parent, "failed to enable SVA feature! ret = %pe\n", ERR_PTR(ret));
- iommu_dev_disable_feature(parent, IOMMU_DEV_FEAT_IOPF);
- return flags;
- }
-
return flags | UACCE_DEV_SVA;
}
@@ -495,7 +487,6 @@ static void uacce_disable_sva(struct uacce_device *uacce)
if (!(uacce->flags & UACCE_DEV_SVA))
return;
- iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA);
iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF);
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index cf8c16ba04a0..1308c8f85591 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -323,18 +323,11 @@ struct iommu_iort_rmr_data {
/**
* enum iommu_dev_features - Per device IOMMU features
- * @IOMMU_DEV_FEAT_SVA: Shared Virtual Addresses
- * @IOMMU_DEV_FEAT_IOPF: I/O Page Faults such as PRI or Stall. Generally
- * enabling %IOMMU_DEV_FEAT_SVA requires
- * %IOMMU_DEV_FEAT_IOPF, but some devices manage I/O Page
- * Faults themselves instead of relying on the IOMMU. When
- * supported, this feature must be enabled before and
- * disabled after %IOMMU_DEV_FEAT_SVA.
+ * @IOMMU_DEV_FEAT_IOPF: I/O Page Faults such as PRI or Stall.
*
* Device drivers enable a feature using iommu_dev_enable_feature().
*/
enum iommu_dev_features {
- IOMMU_DEV_FEAT_SVA,
IOMMU_DEV_FEAT_IOPF,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v4 3/8] iommu/vt-d: Put iopf enablement in domain attach path
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
2025-03-13 5:19 ` [PATCH v4 1/8] iommu/arm-smmu-v3: Put iopf enablement in the domain attach path Lu Baolu
2025-03-13 5:19 ` [PATCH v4 2/8] iommu: Remove IOMMU_DEV_FEAT_SVA Lu Baolu
@ 2025-03-13 5:19 ` Lu Baolu
2025-03-18 9:58 ` Yi Liu
2025-03-13 5:19 ` [PATCH v4 4/8] iommufd/selftest: " Lu Baolu
` (7 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Lu Baolu @ 2025-03-13 5:19 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel, Lu Baolu
Update iopf enablement in the driver to use the new method, similar to
the arm-smmu-v3 driver. Enable iopf support when any domain with an
iopf_handler is attached, and disable it when the domain is removed.
Place all the logic for controlling the PRI and iopf queue in the domain
set/remove/replace paths. Keep track of the number of domains set to the
device and PASIDs that require iopf. When the first domain requiring iopf
is attached, add the device to the iopf queue and enable PRI. When the
last domain is removed, remove it from the iopf queue and disable PRI.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
drivers/iommu/intel/iommu.c | 42 ++++++++++++++++++++++++++++++------
drivers/iommu/intel/iommu.h | 33 ++++++++++++++++++++++++++++
drivers/iommu/intel/nested.c | 16 ++++++++++++--
drivers/iommu/intel/svm.c | 9 ++++++--
4 files changed, 90 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 8d557d04a273..227cbe7a66bc 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3280,6 +3280,9 @@ void device_block_translation(struct device *dev)
static int blocking_domain_attach_dev(struct iommu_domain *domain,
struct device *dev)
{
+ struct device_domain_info *info = dev_iommu_priv_get(dev);
+
+ iopf_for_domain_remove(info->domain ? &info->domain->domain : NULL, dev);
device_block_translation(dev);
return 0;
}
@@ -3489,7 +3492,15 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
if (ret)
return ret;
- return dmar_domain_attach_device(to_dmar_domain(domain), dev);
+ ret = iopf_for_domain_set(domain, dev);
+ if (ret)
+ return ret;
+
+ ret = dmar_domain_attach_device(to_dmar_domain(domain), dev);
+ if (ret)
+ iopf_for_domain_remove(domain, dev);
+
+ return ret;
}
static int intel_iommu_map(struct iommu_domain *domain,
@@ -3910,6 +3921,8 @@ int intel_iommu_enable_iopf(struct device *dev)
if (!info->pri_enabled)
return -ENODEV;
+ /* pri_enabled is protected by the group mutex. */
+ iommu_group_mutex_assert(dev);
if (info->iopf_refcount) {
info->iopf_refcount++;
return 0;
@@ -3932,6 +3945,7 @@ void intel_iommu_disable_iopf(struct device *dev)
if (WARN_ON(!info->pri_enabled || !info->iopf_refcount))
return;
+ iommu_group_mutex_assert(dev);
if (--info->iopf_refcount)
return;
@@ -3943,8 +3957,7 @@ intel_iommu_dev_enable_feat(struct device *dev, enum iommu_dev_features feat)
{
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
- return intel_iommu_enable_iopf(dev);
-
+ return 0;
default:
return -ENODEV;
}
@@ -3955,7 +3968,6 @@ intel_iommu_dev_disable_feat(struct device *dev, enum iommu_dev_features feat)
{
switch (feat) {
case IOMMU_DEV_FEAT_IOPF:
- intel_iommu_disable_iopf(dev);
return 0;
default:
@@ -4036,6 +4048,7 @@ static int blocking_domain_set_dev_pasid(struct iommu_domain *domain,
{
struct device_domain_info *info = dev_iommu_priv_get(dev);
+ iopf_for_domain_remove(old, dev);
intel_pasid_tear_down_entry(info->iommu, dev, pasid, false);
domain_remove_dev_pasid(old, dev, pasid);
@@ -4109,6 +4122,10 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
if (IS_ERR(dev_pasid))
return PTR_ERR(dev_pasid);
+ ret = iopf_for_domain_replace(domain, old, dev);
+ if (ret)
+ goto out_remove_dev_pasid;
+
if (dmar_domain->use_first_level)
ret = domain_setup_first_level(iommu, dmar_domain,
dev, pasid, old);
@@ -4116,7 +4133,7 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
ret = domain_setup_second_level(iommu, dmar_domain,
dev, pasid, old);
if (ret)
- goto out_remove_dev_pasid;
+ goto out_unwind_iopf;
domain_remove_dev_pasid(old, dev, pasid);
@@ -4124,6 +4141,8 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
return 0;
+out_unwind_iopf:
+ iopf_for_domain_replace(old, domain, dev);
out_remove_dev_pasid:
domain_remove_dev_pasid(domain, dev, pasid);
return ret;
@@ -4338,6 +4357,11 @@ static int identity_domain_attach_dev(struct iommu_domain *domain, struct device
if (dev_is_real_dma_subdevice(dev))
return 0;
+ /*
+ * No PRI support with the global identity domain. No need to enable or
+ * disable PRI in this path as the iommu has been put in the blocking
+ * state.
+ */
if (sm_supported(iommu))
ret = intel_pasid_setup_pass_through(iommu, dev, IOMMU_NO_PASID);
else
@@ -4357,9 +4381,15 @@ static int identity_domain_set_dev_pasid(struct iommu_domain *domain,
if (!pasid_supported(iommu) || dev_is_real_dma_subdevice(dev))
return -EOPNOTSUPP;
+ ret = iopf_for_domain_replace(domain, old, dev);
+ if (ret)
+ return ret;
+
ret = domain_setup_passthrough(iommu, dev, pasid, old);
- if (ret)
+ if (ret) {
+ iopf_for_domain_replace(old, domain, dev);
return ret;
+ }
domain_remove_dev_pasid(old, dev, pasid);
return 0;
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index c4916886da5a..02649eafcc24 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -1298,6 +1298,39 @@ void intel_iommu_drain_pasid_prq(struct device *dev, u32 pasid);
int intel_iommu_enable_iopf(struct device *dev);
void intel_iommu_disable_iopf(struct device *dev);
+static inline int iopf_for_domain_set(struct iommu_domain *domain,
+ struct device *dev)
+{
+ if (!domain || !domain->iopf_handler)
+ return 0;
+
+ return intel_iommu_enable_iopf(dev);
+}
+
+static inline void iopf_for_domain_remove(struct iommu_domain *domain,
+ struct device *dev)
+{
+ if (!domain || !domain->iopf_handler)
+ return;
+
+ intel_iommu_disable_iopf(dev);
+}
+
+static inline int iopf_for_domain_replace(struct iommu_domain *new,
+ struct iommu_domain *old,
+ struct device *dev)
+{
+ int ret;
+
+ ret = iopf_for_domain_set(new, dev);
+ if (ret)
+ return ret;
+
+ iopf_for_domain_remove(old, dev);
+
+ return 0;
+}
+
#ifdef CONFIG_INTEL_IOMMU_SVM
void intel_svm_check(struct intel_iommu *iommu);
struct iommu_domain *intel_svm_domain_alloc(struct device *dev,
diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c
index aba92c00b427..ad307248bcae 100644
--- a/drivers/iommu/intel/nested.c
+++ b/drivers/iommu/intel/nested.c
@@ -56,10 +56,14 @@ static int intel_nested_attach_dev(struct iommu_domain *domain,
if (ret)
goto detach_iommu;
+ ret = iopf_for_domain_set(domain, dev);
+ if (ret)
+ goto unassign_tag;
+
ret = intel_pasid_setup_nested(iommu, dev,
IOMMU_NO_PASID, dmar_domain);
if (ret)
- goto unassign_tag;
+ goto disable_iopf;
info->domain = dmar_domain;
spin_lock_irqsave(&dmar_domain->lock, flags);
@@ -67,6 +71,8 @@ static int intel_nested_attach_dev(struct iommu_domain *domain,
spin_unlock_irqrestore(&dmar_domain->lock, flags);
return 0;
+disable_iopf:
+ iopf_for_domain_remove(domain, dev);
unassign_tag:
cache_tag_unassign_domain(dmar_domain, dev, IOMMU_NO_PASID);
detach_iommu:
@@ -166,14 +172,20 @@ static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
if (IS_ERR(dev_pasid))
return PTR_ERR(dev_pasid);
- ret = domain_setup_nested(iommu, dmar_domain, dev, pasid, old);
+ ret = iopf_for_domain_replace(domain, old, dev);
if (ret)
goto out_remove_dev_pasid;
+ ret = domain_setup_nested(iommu, dmar_domain, dev, pasid, old);
+ if (ret)
+ goto out_unwind_iopf;
+
domain_remove_dev_pasid(old, dev, pasid);
return 0;
+out_unwind_iopf:
+ iopf_for_domain_replace(old, domain, dev);
out_remove_dev_pasid:
domain_remove_dev_pasid(domain, dev, pasid);
return ret;
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index ba93123cb4eb..f3da596410b5 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -164,18 +164,23 @@ static int intel_svm_set_dev_pasid(struct iommu_domain *domain,
if (IS_ERR(dev_pasid))
return PTR_ERR(dev_pasid);
+ ret = iopf_for_domain_replace(domain, old, dev);
+ if (ret)
+ goto out_remove_dev_pasid;
+
/* Setup the pasid table: */
sflags = cpu_feature_enabled(X86_FEATURE_LA57) ? PASID_FLAG_FL5LP : 0;
ret = __domain_setup_first_level(iommu, dev, pasid,
FLPT_DEFAULT_DID, mm->pgd,
sflags, old);
if (ret)
- goto out_remove_dev_pasid;
+ goto out_unwind_iopf;
domain_remove_dev_pasid(old, dev, pasid);
return 0;
-
+out_unwind_iopf:
+ iopf_for_domain_replace(old, domain, dev);
out_remove_dev_pasid:
domain_remove_dev_pasid(domain, dev, pasid);
return ret;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 3/8] iommu/vt-d: Put iopf enablement in domain attach path
2025-03-13 5:19 ` [PATCH v4 3/8] iommu/vt-d: Put iopf enablement in domain attach path Lu Baolu
@ 2025-03-18 9:58 ` Yi Liu
2025-03-18 13:19 ` Baolu Lu
0 siblings, 1 reply; 23+ messages in thread
From: Yi Liu @ 2025-03-18 9:58 UTC (permalink / raw)
To: Lu Baolu, Joerg Roedel, Will Deacon, Robin Murphy,
Jason Gunthorpe, Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel
On 2025/3/13 13:19, Lu Baolu wrote:
> Update iopf enablement in the driver to use the new method, similar to
> the arm-smmu-v3 driver. Enable iopf support when any domain with an
> iopf_handler is attached, and disable it when the domain is removed.
>
> Place all the logic for controlling the PRI and iopf queue in the domain
> set/remove/replace paths. Keep track of the number of domains set to the
> device and PASIDs that require iopf. When the first domain requiring iopf
> is attached, add the device to the iopf queue and enable PRI. When the
> last domain is removed, remove it from the iopf queue and disable PRI.
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
a nit. It appears to me the PRI cap and IOMMU PRI enable bit is set in the
probe_device() now after the below patch. This commit now is more dealing
with iopf_refcount and adding the device to the iopf queue.
https://lore.kernel.org/linux-iommu/20250228092631.3425464-6-baolu.lu@linux.intel.com/
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 3/8] iommu/vt-d: Put iopf enablement in domain attach path
2025-03-18 9:58 ` Yi Liu
@ 2025-03-18 13:19 ` Baolu Lu
0 siblings, 0 replies; 23+ messages in thread
From: Baolu Lu @ 2025-03-18 13:19 UTC (permalink / raw)
To: Yi Liu, Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian
Cc: baolu.lu, Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao,
Zhou Wang, iommu, linux-kernel
On 3/18/2025 5:58 PM, Yi Liu wrote:
> On 2025/3/13 13:19, Lu Baolu wrote:
>> Update iopf enablement in the driver to use the new method, similar to
>> the arm-smmu-v3 driver. Enable iopf support when any domain with an
>> iopf_handler is attached, and disable it when the domain is removed.
>>
>> Place all the logic for controlling the PRI and iopf queue in the domain
>> set/remove/replace paths. Keep track of the number of domains set to the
>> device and PASIDs that require iopf. When the first domain requiring iopf
>> is attached, add the device to the iopf queue and enable PRI. When the
>> last domain is removed, remove it from the iopf queue and disable PRI.
>
> Reviewed-by: Yi Liu <yi.l.liu@intel.com>
>
> a nit. It appears to me the PRI cap and IOMMU PRI enable bit is set in the
> probe_device() now after the below patch. This commit now is more dealing
> with iopf_refcount and adding the device to the iopf queue.
>
> https://lore.kernel.org/linux-iommu/20250228092631.3425464-6-
> baolu.lu@linux.intel.com/
Yes. With the device not adding to the iommu iopf queue, the IOMMU core
will respond to the device with IOMMU_PAGE_RESP_INVALID.
Thanks,
baolu
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 4/8] iommufd/selftest: Put iopf enablement in domain attach path
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
` (2 preceding siblings ...)
2025-03-13 5:19 ` [PATCH v4 3/8] iommu/vt-d: Put iopf enablement in domain attach path Lu Baolu
@ 2025-03-13 5:19 ` Lu Baolu
2025-03-18 10:25 ` Yi Liu
2025-03-18 19:50 ` Nicolin Chen
2025-03-13 5:19 ` [PATCH v4 5/8] dmaengine: idxd: Remove unnecessary IOMMU_DEV_FEAT_IOPF Lu Baolu
` (6 subsequent siblings)
10 siblings, 2 replies; 23+ messages in thread
From: Lu Baolu @ 2025-03-13 5:19 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel, Lu Baolu
Update iopf enablement in the iommufd mock device driver to use the new
method, similar to the arm-smmu-v3 driver. Enable iopf support when any
domain with an iopf_handler is attached, and disable it when the domain
is removed.
Add a refcount in the mock device state structure to keep track of the
number of domains set to the device and PASIDs that require iopf.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
drivers/iommu/iommufd/selftest.c | 64 ++++++++++++++++++++++++++------
1 file changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index d40deb0a4f06..471af81e2617 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -58,6 +58,9 @@ enum {
MOCK_PFN_HUGE_IOVA = _MOCK_PFN_START << 2,
};
+static int mock_dev_enable_iopf(struct device *dev, struct iommu_domain *domain);
+static void mock_dev_disable_iopf(struct device *dev, struct iommu_domain *domain);
+
/*
* Syzkaller has trouble randomizing the correct iova to use since it is linked
* to the map ioctl's output, and it has no ide about that. So, simplify things.
@@ -164,6 +167,8 @@ struct mock_dev {
unsigned long flags;
int id;
u32 cache[MOCK_DEV_CACHE_NUM];
+ unsigned int iopf_refcount;
+ struct iommu_domain *domain;
};
static inline struct mock_dev *to_mock_dev(struct device *dev)
@@ -193,15 +198,34 @@ static int mock_domain_nop_attach(struct iommu_domain *domain,
struct device *dev)
{
struct mock_dev *mdev = to_mock_dev(dev);
+ int ret;
if (domain->dirty_ops && (mdev->flags & MOCK_FLAGS_DEVICE_NO_DIRTY))
return -EINVAL;
+ ret = mock_dev_enable_iopf(dev, domain);
+ if (ret)
+ return ret;
+
+ mock_dev_disable_iopf(dev, mdev->domain);
+ mdev->domain = domain;
+
+ return 0;
+}
+
+static int mock_domain_blocking_attach(struct iommu_domain *domain,
+ struct device *dev)
+{
+ struct mock_dev *mdev = to_mock_dev(dev);
+
+ mock_dev_disable_iopf(dev, mdev->domain);
+ mdev->domain = NULL;
+
return 0;
}
static const struct iommu_domain_ops mock_blocking_ops = {
- .attach_dev = mock_domain_nop_attach,
+ .attach_dev = mock_domain_blocking_attach,
};
static struct iommu_domain mock_blocking_domain = {
@@ -549,22 +573,42 @@ static void mock_domain_page_response(struct device *dev, struct iopf_fault *evt
{
}
-static int mock_dev_enable_feat(struct device *dev, enum iommu_dev_features feat)
+static int mock_dev_enable_iopf(struct device *dev, struct iommu_domain *domain)
{
- if (feat != IOMMU_DEV_FEAT_IOPF || !mock_iommu_iopf_queue)
+ struct mock_dev *mdev = to_mock_dev(dev);
+ int ret;
+
+ if (!domain || !domain->iopf_handler)
+ return 0;
+
+ if (!mock_iommu_iopf_queue)
return -ENODEV;
- return iopf_queue_add_device(mock_iommu_iopf_queue, dev);
+ if (mdev->iopf_refcount) {
+ mdev->iopf_refcount++;
+ return 0;
+ }
+
+ ret = iopf_queue_add_device(mock_iommu_iopf_queue, dev);
+ if (ret)
+ return ret;
+
+ mdev->iopf_refcount = 1;
+
+ return 0;
}
-static int mock_dev_disable_feat(struct device *dev, enum iommu_dev_features feat)
+static void mock_dev_disable_iopf(struct device *dev, struct iommu_domain *domain)
{
- if (feat != IOMMU_DEV_FEAT_IOPF || !mock_iommu_iopf_queue)
- return -ENODEV;
+ struct mock_dev *mdev = to_mock_dev(dev);
+
+ if (!domain || !domain->iopf_handler)
+ return;
+
+ if (--mdev->iopf_refcount)
+ return;
iopf_queue_remove_device(mock_iommu_iopf_queue, dev);
-
- return 0;
}
static void mock_viommu_destroy(struct iommufd_viommu *viommu)
@@ -709,8 +753,6 @@ static const struct iommu_ops mock_ops = {
.device_group = generic_device_group,
.probe_device = mock_probe_device,
.page_response = mock_domain_page_response,
- .dev_enable_feat = mock_dev_enable_feat,
- .dev_disable_feat = mock_dev_disable_feat,
.user_pasid_table = true,
.viommu_alloc = mock_viommu_alloc,
.default_domain_ops =
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 4/8] iommufd/selftest: Put iopf enablement in domain attach path
2025-03-13 5:19 ` [PATCH v4 4/8] iommufd/selftest: " Lu Baolu
@ 2025-03-18 10:25 ` Yi Liu
2025-03-18 19:50 ` Nicolin Chen
1 sibling, 0 replies; 23+ messages in thread
From: Yi Liu @ 2025-03-18 10:25 UTC (permalink / raw)
To: Lu Baolu, Joerg Roedel, Will Deacon, Robin Murphy,
Jason Gunthorpe, Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel
On 2025/3/13 13:19, Lu Baolu wrote:
> Update iopf enablement in the iommufd mock device driver to use the new
> method, similar to the arm-smmu-v3 driver. Enable iopf support when any
> domain with an iopf_handler is attached, and disable it when the domain
> is removed.
>
> Add a refcount in the mock device state structure to keep track of the
> number of domains set to the device and PASIDs that require iopf.
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> ---
> drivers/iommu/iommufd/selftest.c | 64 ++++++++++++++++++++++++++------
> 1 file changed, 53 insertions(+), 11 deletions(-)
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 4/8] iommufd/selftest: Put iopf enablement in domain attach path
2025-03-13 5:19 ` [PATCH v4 4/8] iommufd/selftest: " Lu Baolu
2025-03-18 10:25 ` Yi Liu
@ 2025-03-18 19:50 ` Nicolin Chen
1 sibling, 0 replies; 23+ messages in thread
From: Nicolin Chen @ 2025-03-18 19:50 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian, Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao,
Zhou Wang, iommu, linux-kernel
On Thu, Mar 13, 2025 at 01:19:49PM +0800, Lu Baolu wrote:
> @@ -164,6 +167,8 @@ struct mock_dev {
> unsigned long flags;
> int id;
> u32 cache[MOCK_DEV_CACHE_NUM];
> + unsigned int iopf_refcount;
> + struct iommu_domain *domain;
Assuming both are protected by the core group->mutex, maybe add
lockdep_assert_held() in mock_dev_enable/disable_iopf()?
> +static int mock_domain_blocking_attach(struct iommu_domain *domain,
> + struct device *dev)
> +{
> + struct mock_dev *mdev = to_mock_dev(dev);
> +
> + mock_dev_disable_iopf(dev, mdev->domain);
> + mdev->domain = NULL;
The input domain is valid. Maybe better to rename mdev->domain
to mdev->iopf_domain?
With that,
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 5/8] dmaengine: idxd: Remove unnecessary IOMMU_DEV_FEAT_IOPF
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
` (3 preceding siblings ...)
2025-03-13 5:19 ` [PATCH v4 4/8] iommufd/selftest: " Lu Baolu
@ 2025-03-13 5:19 ` Lu Baolu
2025-03-18 10:20 ` Yi Liu
2025-03-13 5:19 ` [PATCH v4 6/8] uacce: " Lu Baolu
` (5 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Lu Baolu @ 2025-03-13 5:19 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel, Lu Baolu, Jason Gunthorpe
The IOMMU_DEV_FEAT_IOPF implementation in the iommu driver is just a no-op.
It will also be removed from the iommu driver in the subsequent patch.
Remove it to avoid dead code.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
drivers/dma/idxd/init.c | 37 ++++++-------------------------------
1 file changed, 6 insertions(+), 31 deletions(-)
diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 1e5038cca22c..d44944195807 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -626,21 +626,6 @@ static void idxd_disable_system_pasid(struct idxd_device *idxd)
idxd->pasid = IOMMU_PASID_INVALID;
}
-static int idxd_enable_sva(struct pci_dev *pdev)
-{
- int ret;
-
- ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
- if (ret)
- return ret;
- return 0;
-}
-
-static void idxd_disable_sva(struct pci_dev *pdev)
-{
- iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
-}
-
static int idxd_probe(struct idxd_device *idxd)
{
struct pci_dev *pdev = idxd->pdev;
@@ -655,17 +640,13 @@ static int idxd_probe(struct idxd_device *idxd)
dev_dbg(dev, "IDXD reset complete\n");
if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM) && sva) {
- if (idxd_enable_sva(pdev)) {
- dev_warn(dev, "Unable to turn on user SVA feature.\n");
- } else {
- set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
+ set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
- rc = idxd_enable_system_pasid(idxd);
- if (rc)
- dev_warn(dev, "No in-kernel DMA with PASID. %d\n", rc);
- else
- set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
- }
+ rc = idxd_enable_system_pasid(idxd);
+ if (rc)
+ dev_warn(dev, "No in-kernel DMA with PASID. %d\n", rc);
+ else
+ set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
} else if (!sva) {
dev_warn(dev, "User forced SVA off via module param.\n");
}
@@ -703,8 +684,6 @@ static int idxd_probe(struct idxd_device *idxd)
err:
if (device_pasid_enabled(idxd))
idxd_disable_system_pasid(idxd);
- if (device_user_pasid_enabled(idxd))
- idxd_disable_sva(pdev);
return rc;
}
@@ -715,8 +694,6 @@ static void idxd_cleanup(struct idxd_device *idxd)
idxd_cleanup_internals(idxd);
if (device_pasid_enabled(idxd))
idxd_disable_system_pasid(idxd);
- if (device_user_pasid_enabled(idxd))
- idxd_disable_sva(idxd->pdev);
}
/*
@@ -1248,8 +1225,6 @@ static void idxd_remove(struct pci_dev *pdev)
free_irq(irq_entry->vector, irq_entry);
pci_free_irq_vectors(pdev);
pci_iounmap(pdev, idxd->reg_base);
- if (device_user_pasid_enabled(idxd))
- idxd_disable_sva(pdev);
pci_disable_device(pdev);
destroy_workqueue(idxd->wq);
perfmon_pmu_remove(idxd);
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 5/8] dmaengine: idxd: Remove unnecessary IOMMU_DEV_FEAT_IOPF
2025-03-13 5:19 ` [PATCH v4 5/8] dmaengine: idxd: Remove unnecessary IOMMU_DEV_FEAT_IOPF Lu Baolu
@ 2025-03-18 10:20 ` Yi Liu
0 siblings, 0 replies; 23+ messages in thread
From: Yi Liu @ 2025-03-18 10:20 UTC (permalink / raw)
To: Lu Baolu, Joerg Roedel, Will Deacon, Robin Murphy,
Jason Gunthorpe, Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel, Jason Gunthorpe
On 2025/3/13 13:19, Lu Baolu wrote:
> The IOMMU_DEV_FEAT_IOPF implementation in the iommu driver is just a no-op.
> It will also be removed from the iommu driver in the subsequent patch.
> Remove it to avoid dead code.
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Acked-by: Vinod Koul <vkoul@kernel.org>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> ---
> drivers/dma/idxd/init.c | 37 ++++++-------------------------------
> 1 file changed, 6 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 1e5038cca22c..d44944195807 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -626,21 +626,6 @@ static void idxd_disable_system_pasid(struct idxd_device *idxd)
> idxd->pasid = IOMMU_PASID_INVALID;
> }
>
> -static int idxd_enable_sva(struct pci_dev *pdev)
> -{
> - int ret;
> -
> - ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
> - if (ret)
> - return ret;
> - return 0;
> -}
> -
> -static void idxd_disable_sva(struct pci_dev *pdev)
> -{
> - iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
> -}
> -
> static int idxd_probe(struct idxd_device *idxd)
> {
> struct pci_dev *pdev = idxd->pdev;
> @@ -655,17 +640,13 @@ static int idxd_probe(struct idxd_device *idxd)
> dev_dbg(dev, "IDXD reset complete\n");
>
> if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM) && sva) {
> - if (idxd_enable_sva(pdev)) {
> - dev_warn(dev, "Unable to turn on user SVA feature.\n");
> - } else {
> - set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
> + set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
a nit. The IDXD_FLAG_USER_PASID_ENABLED becomes almost equivalent to the
sva option. Is it ok?
This patch itself LGTM.
Reviewed-by: Yi Liu <yi.l.liu@intel.com>
--
Regards,
Yi Liu
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 6/8] uacce: Remove unnecessary IOMMU_DEV_FEAT_IOPF
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
` (4 preceding siblings ...)
2025-03-13 5:19 ` [PATCH v4 5/8] dmaengine: idxd: Remove unnecessary IOMMU_DEV_FEAT_IOPF Lu Baolu
@ 2025-03-13 5:19 ` Lu Baolu
2025-03-13 5:19 ` [PATCH v4 7/8] iommufd: " Lu Baolu
` (4 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Lu Baolu @ 2025-03-13 5:19 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel, Lu Baolu, Jason Gunthorpe
None of the drivers implement anything for IOMMU_DEV_FEAT_IOPF anymore,
remove it to avoid dead code.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Acked-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
drivers/misc/uacce/uacce.c | 31 -------------------------------
1 file changed, 31 deletions(-)
diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
index 2a1db2abeeca..42e7d2a2a90c 100644
--- a/drivers/misc/uacce/uacce.c
+++ b/drivers/misc/uacce/uacce.c
@@ -465,31 +465,6 @@ static void uacce_release(struct device *dev)
kfree(uacce);
}
-static unsigned int uacce_enable_sva(struct device *parent, unsigned int flags)
-{
- int ret;
-
- if (!(flags & UACCE_DEV_SVA))
- return flags;
-
- flags &= ~UACCE_DEV_SVA;
-
- ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF);
- if (ret) {
- dev_err(parent, "failed to enable IOPF feature! ret = %pe\n", ERR_PTR(ret));
- return flags;
- }
- return flags | UACCE_DEV_SVA;
-}
-
-static void uacce_disable_sva(struct uacce_device *uacce)
-{
- if (!(uacce->flags & UACCE_DEV_SVA))
- return;
-
- iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF);
-}
-
/**
* uacce_alloc() - alloc an accelerator
* @parent: pointer of uacce parent device
@@ -509,8 +484,6 @@ struct uacce_device *uacce_alloc(struct device *parent,
if (!uacce)
return ERR_PTR(-ENOMEM);
- flags = uacce_enable_sva(parent, flags);
-
uacce->parent = parent;
uacce->flags = flags;
uacce->ops = interface->ops;
@@ -533,7 +506,6 @@ struct uacce_device *uacce_alloc(struct device *parent,
return uacce;
err_with_uacce:
- uacce_disable_sva(uacce);
kfree(uacce);
return ERR_PTR(ret);
}
@@ -596,9 +568,6 @@ void uacce_remove(struct uacce_device *uacce)
unmap_mapping_range(q->mapping, 0, 0, 1);
}
- /* disable sva now since no opened queues */
- uacce_disable_sva(uacce);
-
if (uacce->cdev)
cdev_device_del(uacce->cdev, &uacce->dev);
xa_erase(&uacce_xa, uacce->dev_id);
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v4 7/8] iommufd: Remove unnecessary IOMMU_DEV_FEAT_IOPF
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
` (5 preceding siblings ...)
2025-03-13 5:19 ` [PATCH v4 6/8] uacce: " Lu Baolu
@ 2025-03-13 5:19 ` Lu Baolu
2025-03-13 5:19 ` [PATCH v4 8/8] iommu: Remove iommu_dev_enable/disable_feature() Lu Baolu
` (3 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Lu Baolu @ 2025-03-13 5:19 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel, Lu Baolu, Jason Gunthorpe
The iopf enablement has been moved to the iommu drivers. It is unnecessary
for iommufd to handle iopf enablement. Remove the iopf enablement logic to
avoid duplication.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
drivers/iommu/iommufd/device.c | 64 +++++++++++++------------
drivers/iommu/iommufd/fault.c | 45 -----------------
drivers/iommu/iommufd/iommufd_private.h | 5 --
3 files changed, 33 insertions(+), 81 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 4e107f69f951..9c488680871f 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -6,6 +6,8 @@
#include <linux/slab.h>
#include <uapi/linux/iommufd.h>
#include <linux/msi.h>
+#include <linux/pci.h>
+#include <linux/pci-ats.h>
#include "../iommu-priv.h"
#include "io_pagetable.h"
@@ -216,7 +218,6 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
refcount_inc(&idev->obj.users);
/* igroup refcount moves into iommufd_device */
idev->igroup = igroup;
- mutex_init(&idev->iopf_lock);
/*
* If the caller fails after this success it must call
@@ -471,6 +472,25 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
/* The device attach/detach/replace helpers for attach_handle */
+static bool iommufd_hwpt_compatible_device(struct iommufd_hw_pagetable *hwpt,
+ struct iommufd_device *idev)
+{
+ struct pci_dev *pdev;
+
+ if (!hwpt->fault || !dev_is_pci(idev->dev))
+ return true;
+
+ /*
+ * Once we turn on PCI/PRI support for VF, the response failure code
+ * should not be forwarded to the hardware due to PRI being a shared
+ * resource between PF and VFs. There is no coordination for this
+ * shared capability. This waits for a vPRI reset to recover.
+ */
+ pdev = to_pci_dev(idev->dev);
+
+ return (!pdev->is_virtfn || !pci_pri_supported(pdev));
+}
+
static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev)
{
@@ -479,30 +499,22 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
lockdep_assert_held(&idev->igroup->lock);
+ if (!iommufd_hwpt_compatible_device(hwpt, idev))
+ return -EINVAL;
+
handle = kzalloc(sizeof(*handle), GFP_KERNEL);
if (!handle)
return -ENOMEM;
- if (hwpt->fault) {
- rc = iommufd_fault_iopf_enable(idev);
- if (rc)
- goto out_free_handle;
- }
-
handle->idev = idev;
rc = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,
&handle->handle);
- if (rc)
- goto out_disable_iopf;
+ if (rc) {
+ kfree(handle);
+ return rc;
+ }
return 0;
-
-out_disable_iopf:
- if (hwpt->fault)
- iommufd_fault_iopf_disable(idev);
-out_free_handle:
- kfree(handle);
- return rc;
}
static struct iommufd_attach_handle *
@@ -526,10 +538,8 @@ static void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,
handle = iommufd_device_get_attach_handle(idev);
iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
- if (hwpt->fault) {
+ if (hwpt->fault)
iommufd_auto_response_faults(hwpt, handle);
- iommufd_fault_iopf_disable(idev);
- }
kfree(handle);
}
@@ -541,34 +551,26 @@ static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
iommufd_device_get_attach_handle(idev);
int rc;
+ if (!iommufd_hwpt_compatible_device(hwpt, idev))
+ return -EINVAL;
+
handle = kzalloc(sizeof(*handle), GFP_KERNEL);
if (!handle)
return -ENOMEM;
- if (hwpt->fault && !old->fault) {
- rc = iommufd_fault_iopf_enable(idev);
- if (rc)
- goto out_free_handle;
- }
-
handle->idev = idev;
rc = iommu_replace_group_handle(idev->igroup->group, hwpt->domain,
&handle->handle);
if (rc)
- goto out_disable_iopf;
+ goto out_free_handle;
if (old->fault) {
iommufd_auto_response_faults(hwpt, old_handle);
- if (!hwpt->fault)
- iommufd_fault_iopf_disable(idev);
}
kfree(old_handle);
return 0;
-out_disable_iopf:
- if (hwpt->fault && !old->fault)
- iommufd_fault_iopf_disable(idev);
out_free_handle:
kfree(handle);
return rc;
diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c
index c48d72c9668c..46f17eb20ee2 100644
--- a/drivers/iommu/iommufd/fault.c
+++ b/drivers/iommu/iommufd/fault.c
@@ -9,57 +9,12 @@
#include <linux/iommufd.h>
#include <linux/module.h>
#include <linux/mutex.h>
-#include <linux/pci.h>
-#include <linux/pci-ats.h>
#include <linux/poll.h>
#include <uapi/linux/iommufd.h>
#include "../iommu-priv.h"
#include "iommufd_private.h"
-int iommufd_fault_iopf_enable(struct iommufd_device *idev)
-{
- struct device *dev = idev->dev;
- int ret;
-
- /*
- * Once we turn on PCI/PRI support for VF, the response failure code
- * should not be forwarded to the hardware due to PRI being a shared
- * resource between PF and VFs. There is no coordination for this
- * shared capability. This waits for a vPRI reset to recover.
- */
- if (dev_is_pci(dev)) {
- struct pci_dev *pdev = to_pci_dev(dev);
-
- if (pdev->is_virtfn && pci_pri_supported(pdev))
- return -EINVAL;
- }
-
- mutex_lock(&idev->iopf_lock);
- /* Device iopf has already been on. */
- if (++idev->iopf_enabled > 1) {
- mutex_unlock(&idev->iopf_lock);
- return 0;
- }
-
- ret = iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_IOPF);
- if (ret)
- --idev->iopf_enabled;
- mutex_unlock(&idev->iopf_lock);
-
- return ret;
-}
-
-void iommufd_fault_iopf_disable(struct iommufd_device *idev)
-{
- mutex_lock(&idev->iopf_lock);
- if (!WARN_ON(idev->iopf_enabled == 0)) {
- if (--idev->iopf_enabled == 0)
- iommu_dev_disable_feature(idev->dev, IOMMU_DEV_FEAT_IOPF);
- }
- mutex_unlock(&idev->iopf_lock);
-}
-
void iommufd_auto_response_faults(struct iommufd_hw_pagetable *hwpt,
struct iommufd_attach_handle *handle)
{
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 246297452a44..dcab49f79e7a 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -420,9 +420,6 @@ struct iommufd_device {
/* always the physical device */
struct device *dev;
bool enforce_cache_coherency;
- /* protect iopf_enabled counter */
- struct mutex iopf_lock;
- unsigned int iopf_enabled;
};
static inline struct iommufd_device *
@@ -517,8 +514,6 @@ int iommufd_fault_alloc(struct iommufd_ucmd *ucmd);
void iommufd_fault_destroy(struct iommufd_object *obj);
int iommufd_fault_iopf_handler(struct iopf_group *group);
-int iommufd_fault_iopf_enable(struct iommufd_device *idev);
-void iommufd_fault_iopf_disable(struct iommufd_device *idev);
void iommufd_auto_response_faults(struct iommufd_hw_pagetable *hwpt,
struct iommufd_attach_handle *handle);
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v4 8/8] iommu: Remove iommu_dev_enable/disable_feature()
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
` (6 preceding siblings ...)
2025-03-13 5:19 ` [PATCH v4 7/8] iommufd: " Lu Baolu
@ 2025-03-13 5:19 ` Lu Baolu
2025-03-13 9:51 ` [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Zhangfei Gao
` (2 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Lu Baolu @ 2025-03-13 5:19 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian
Cc: Dave Jiang, Vinod Koul, Fenghua Yu, Zhangfei Gao, Zhou Wang,
iommu, linux-kernel, Lu Baolu, Jason Gunthorpe
No external drivers use these interfaces anymore. Furthermore, no existing
iommu drivers implement anything in the callbacks. Remove them to avoid
dead code.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Tested-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
drivers/iommu/amd/iommu.c | 32 -------------------
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 34 ---------------------
drivers/iommu/intel/iommu.c | 25 ---------------
drivers/iommu/iommu.c | 32 -------------------
include/linux/iommu.h | 28 -----------------
5 files changed, 151 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 25449a564804..16892bff1eaa 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2980,36 +2980,6 @@ static const struct iommu_dirty_ops amd_dirty_ops = {
.read_and_clear_dirty = amd_iommu_read_and_clear_dirty,
};
-static int amd_iommu_dev_enable_feature(struct device *dev,
- enum iommu_dev_features feat)
-{
- int ret = 0;
-
- switch (feat) {
- case IOMMU_DEV_FEAT_IOPF:
- break;
- default:
- ret = -EINVAL;
- break;
- }
- return ret;
-}
-
-static int amd_iommu_dev_disable_feature(struct device *dev,
- enum iommu_dev_features feat)
-{
- int ret = 0;
-
- switch (feat) {
- case IOMMU_DEV_FEAT_IOPF:
- break;
- default:
- ret = -EINVAL;
- break;
- }
- return ret;
-}
-
const struct iommu_ops amd_iommu_ops = {
.capable = amd_iommu_capable,
.blocked_domain = &blocked_domain,
@@ -3023,8 +2993,6 @@ const struct iommu_ops amd_iommu_ops = {
.get_resv_regions = amd_iommu_get_resv_regions,
.is_attach_deferred = amd_iommu_is_attach_deferred,
.def_domain_type = amd_iommu_def_domain_type,
- .dev_enable_feat = amd_iommu_dev_enable_feature,
- .dev_disable_feat = amd_iommu_dev_disable_feature,
.page_response = amd_iommu_page_response,
.default_domain_ops = &(const struct iommu_domain_ops) {
.attach_dev = amd_iommu_attach_device,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 60c8f1fa5bf0..fc6943b2665a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3620,38 +3620,6 @@ static void arm_smmu_get_resv_regions(struct device *dev,
iommu_dma_get_resv_regions(dev, head);
}
-static int arm_smmu_dev_enable_feature(struct device *dev,
- enum iommu_dev_features feat)
-{
- struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-
- if (!master)
- return -ENODEV;
-
- switch (feat) {
- case IOMMU_DEV_FEAT_IOPF:
- return 0;
- default:
- return -EINVAL;
- }
-}
-
-static int arm_smmu_dev_disable_feature(struct device *dev,
- enum iommu_dev_features feat)
-{
- struct arm_smmu_master *master = dev_iommu_priv_get(dev);
-
- if (!master)
- return -EINVAL;
-
- switch (feat) {
- case IOMMU_DEV_FEAT_IOPF:
- return 0;
- default:
- return -EINVAL;
- }
-}
-
/*
* HiSilicon PCIe tune and trace device can be used to trace TLP headers on the
* PCIe link and save the data to memory by DMA. The hardware is restricted to
@@ -3684,8 +3652,6 @@ static struct iommu_ops arm_smmu_ops = {
.device_group = arm_smmu_device_group,
.of_xlate = arm_smmu_of_xlate,
.get_resv_regions = arm_smmu_get_resv_regions,
- .dev_enable_feat = arm_smmu_dev_enable_feature,
- .dev_disable_feat = arm_smmu_dev_disable_feature,
.page_response = arm_smmu_page_response,
.def_domain_type = arm_smmu_def_domain_type,
.viommu_alloc = arm_vsmmu_alloc,
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 227cbe7a66bc..9f14195e7781 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3952,29 +3952,6 @@ void intel_iommu_disable_iopf(struct device *dev)
iopf_queue_remove_device(iommu->iopf_queue, dev);
}
-static int
-intel_iommu_dev_enable_feat(struct device *dev, enum iommu_dev_features feat)
-{
- switch (feat) {
- case IOMMU_DEV_FEAT_IOPF:
- return 0;
- default:
- return -ENODEV;
- }
-}
-
-static int
-intel_iommu_dev_disable_feat(struct device *dev, enum iommu_dev_features feat)
-{
- switch (feat) {
- case IOMMU_DEV_FEAT_IOPF:
- return 0;
-
- default:
- return -ENODEV;
- }
-}
-
static bool intel_iommu_is_attach_deferred(struct device *dev)
{
struct device_domain_info *info = dev_iommu_priv_get(dev);
@@ -4416,8 +4393,6 @@ const struct iommu_ops intel_iommu_ops = {
.release_device = intel_iommu_release_device,
.get_resv_regions = intel_iommu_get_resv_regions,
.device_group = intel_iommu_device_group,
- .dev_enable_feat = intel_iommu_dev_enable_feat,
- .dev_disable_feat = intel_iommu_dev_disable_feat,
.is_attach_deferred = intel_iommu_is_attach_deferred,
.def_domain_type = device_def_domain_type,
.pgsize_bitmap = SZ_4K,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index c283721579b3..f3e70b146826 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2884,38 +2884,6 @@ int iommu_fwspec_add_ids(struct device *dev, const u32 *ids, int num_ids)
}
EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids);
-/*
- * Per device IOMMU features.
- */
-int iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
-{
- if (dev_has_iommu(dev)) {
- const struct iommu_ops *ops = dev_iommu_ops(dev);
-
- if (ops->dev_enable_feat)
- return ops->dev_enable_feat(dev, feat);
- }
-
- return -ENODEV;
-}
-EXPORT_SYMBOL_GPL(iommu_dev_enable_feature);
-
-/*
- * The device drivers should do the necessary cleanups before calling this.
- */
-int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat)
-{
- if (dev_has_iommu(dev)) {
- const struct iommu_ops *ops = dev_iommu_ops(dev);
-
- if (ops->dev_disable_feat)
- return ops->dev_disable_feat(dev, feat);
- }
-
- return -EBUSY;
-}
-EXPORT_SYMBOL_GPL(iommu_dev_disable_feature);
-
/**
* iommu_setup_default_domain - Set the default_domain for the group
* @group: Group to change
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 1308c8f85591..e6c21e15c74f 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -321,16 +321,6 @@ struct iommu_iort_rmr_data {
u32 num_sids;
};
-/**
- * enum iommu_dev_features - Per device IOMMU features
- * @IOMMU_DEV_FEAT_IOPF: I/O Page Faults such as PRI or Stall.
- *
- * Device drivers enable a feature using iommu_dev_enable_feature().
- */
-enum iommu_dev_features {
- IOMMU_DEV_FEAT_IOPF,
-};
-
#define IOMMU_NO_PASID (0U) /* Reserved for DMA w/o PASID */
#define IOMMU_FIRST_GLOBAL_PASID (1U) /*starting range for allocation */
#define IOMMU_PASID_INVALID (-1U)
@@ -650,9 +640,6 @@ struct iommu_ops {
bool (*is_attach_deferred)(struct device *dev);
/* Per device IOMMU features */
- int (*dev_enable_feat)(struct device *dev, enum iommu_dev_features f);
- int (*dev_disable_feat)(struct device *dev, enum iommu_dev_features f);
-
void (*page_response)(struct device *dev, struct iopf_fault *evt,
struct iommu_page_response *msg);
@@ -1121,9 +1108,6 @@ void dev_iommu_priv_set(struct device *dev, void *priv);
extern struct mutex iommu_probe_device_lock;
int iommu_probe_device(struct device *dev);
-int iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features f);
-int iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features f);
-
int iommu_device_use_default_domain(struct device *dev);
void iommu_device_unuse_default_domain(struct device *dev);
@@ -1408,18 +1392,6 @@ static inline int iommu_fwspec_add_ids(struct device *dev, u32 *ids,
return -ENODEV;
}
-static inline int
-iommu_dev_enable_feature(struct device *dev, enum iommu_dev_features feat)
-{
- return -ENODEV;
-}
-
-static inline int
-iommu_dev_disable_feature(struct device *dev, enum iommu_dev_features feat)
-{
- return -ENODEV;
-}
-
static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
{
return NULL;
--
2.43.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
` (7 preceding siblings ...)
2025-03-13 5:19 ` [PATCH v4 8/8] iommu: Remove iommu_dev_enable/disable_feature() Lu Baolu
@ 2025-03-13 9:51 ` Zhangfei Gao
2025-03-13 10:57 ` Zhangfei Gao
2025-03-14 8:11 ` Zhangfei Gao
2025-04-16 7:29 ` Zhangfei Gao
10 siblings, 1 reply; 23+ messages in thread
From: Zhangfei Gao @ 2025-03-13 9:51 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian, Dave Jiang, Vinod Koul, Fenghua Yu, Zhou Wang, iommu,
linux-kernel, Jason Gunthorpe
Hi, Baolu
On Thu, 13 Mar 2025 at 13:19, Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
> The new method for driver fault reporting support relies on the domain
> to specify a iopf_handler. The driver should detect this and setup the
> HW when fault capable domains are attached.
>
> Move SMMUv3 to use this method and have VT-D validate support during
> attach so that all three fault capable drivers have a no-op FEAT_SVA and
> _IOPF. Then remove them.
>
> This was initiated by Jason. I'm following up to remove FEAT_IOPF and
> further clean up.
>
> The whole series is also available at github:
> https://github.com/LuBaolu/intel-iommu/commits/iommu_no_feat-v4
I got an issue on this branch.
Linux 6.14-rc4 + iommu_no_feat-v2
drivers/pci/quirks.c
quirk_huawei_pcie_sva will set dma-can-stall first
arm_smmu_probe_device will check dma-can-stall and set stall_enabled
accordingly.
This branch
arm_smmu_probe_device happens first, when dma-can-stall = 0, so
stall_enabled =0.
Then drivers/pci/quirks.c: quirk_xxx happens
Still in checking.
And this branch does not need these two patches, right?
c7b1397bef3c iommu/arm-smmu-v3: Implement arm_smmu_get_msi_mapping_domain
5cd34634a73e iommu/dma: Support MSIs through nested domains
Thanks
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF
2025-03-13 9:51 ` [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Zhangfei Gao
@ 2025-03-13 10:57 ` Zhangfei Gao
2025-03-13 11:20 ` Baolu Lu
0 siblings, 1 reply; 23+ messages in thread
From: Zhangfei Gao @ 2025-03-13 10:57 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian, Dave Jiang, Vinod Koul, Fenghua Yu, Zhou Wang, iommu,
linux-kernel, Jason Gunthorpe
On Thu, 13 Mar 2025 at 17:51, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:
>
> Hi, Baolu
>
> On Thu, 13 Mar 2025 at 13:19, Lu Baolu <baolu.lu@linux.intel.com> wrote:
> >
> > The new method for driver fault reporting support relies on the domain
> > to specify a iopf_handler. The driver should detect this and setup the
> > HW when fault capable domains are attached.
> >
> > Move SMMUv3 to use this method and have VT-D validate support during
> > attach so that all three fault capable drivers have a no-op FEAT_SVA and
> > _IOPF. Then remove them.
> >
> > This was initiated by Jason. I'm following up to remove FEAT_IOPF and
> > further clean up.
> >
> > The whole series is also available at github:
> > https://github.com/LuBaolu/intel-iommu/commits/iommu_no_feat-v4
>
> I got an issue on this branch.
>
> Linux 6.14-rc4 + iommu_no_feat-v2
> drivers/pci/quirks.c
> quirk_huawei_pcie_sva will set dma-can-stall first
> arm_smmu_probe_device will check dma-can-stall and set stall_enabled
> accordingly.
This working branch arm_smmu_probe_device is called from pci_bus_add_device
So pci_fixup_device is called first
[ 1121.314405] arm_smmu_probe_device+0x48/0x450
[ 1121.314410] __iommu_probe_device+0xc4/0x3c8
[ 1121.314412] iommu_probe_device+0x40/0x90
[ 1121.314414] acpi_dma_configure_id+0xb4/0x100
[ 1121.314417] pci_dma_configure+0xf8/0x108
[ 1121.314421] really_probe+0x78/0x278
[ 1121.314425] __driver_probe_device+0x80/0x140
[ 1121.314427] driver_probe_device+0x48/0x130
[ 1121.314430] __device_attach_driver+0xc0/0x108
[ 1121.314432] bus_for_each_drv+0x8c/0xf8
[ 1121.314435] __device_attach+0x104/0x1a0
[ 1121.314437] device_attach+0x1c/0x30
[ 1121.314440] pci_bus_add_device+0xb8/0x1f0
[ 1121.314442] pci_iov_add_virtfn+0x2ac/0x300
[ 1121.314446] sriov_enable+0x204/0x468
[ 1121.314447] pci_enable_sriov+0x20/0x40
>
> This branch
> arm_smmu_probe_device happens first, when dma-can-stall = 0, so
> stall_enabled =0.
> Then drivers/pci/quirks.c: quirk_xxx happens
This not working branch: Linux 6.14-rc6 + iommu_no_feat-v4
arm_smmu_probe_device is called by pci_device_add
Then call pci_bus_add_device -> pci_fixup_device
215.072859] arm_smmu_probe_device+0x48/0x450
[ 215.072871] __iommu_probe_device+0xc0/0x468
[ 215.072875] iommu_probe_device+0x40/0x90
[ 215.072877] iommu_bus_notifier+0x38/0x68
[ 215.072879] notifier_call_chain+0x80/0x148
[ 215.072886] blocking_notifier_call_chain+0x50/0x80
[ 215.072889] bus_notify+0x44/0x68
[ 215.072896] device_add+0x580/0x768
[ 215.072898] pci_device_add+0x1e8/0x568
[ 215.072906] pci_iov_add_virtfn+0x198/0x300
[ 215.072910] sriov_enable+0x204/0x468
[ 215.072912] pci_enable_sriov+0x20/0x40
pci_iov_add_virtfn:
pci_device_add(virtfn, virtfn->bus);
pci_bus_add_device(virtfn); -> pci_fixup_device(pci_fixup_final, dev);
>
> Still in checking.
>
> And this branch does not need these two patches, right?
> c7b1397bef3c iommu/arm-smmu-v3: Implement arm_smmu_get_msi_mapping_domain
> 5cd34634a73e iommu/dma: Support MSIs through nested domains
>
> Thanks
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF
2025-03-13 10:57 ` Zhangfei Gao
@ 2025-03-13 11:20 ` Baolu Lu
2025-03-13 11:37 ` Zhangfei Gao
0 siblings, 1 reply; 23+ messages in thread
From: Baolu Lu @ 2025-03-13 11:20 UTC (permalink / raw)
To: Zhangfei Gao
Cc: baolu.lu, Joerg Roedel, Will Deacon, Robin Murphy,
Jason Gunthorpe, Kevin Tian, Dave Jiang, Vinod Koul, Fenghua Yu,
Zhou Wang, iommu, linux-kernel, Jason Gunthorpe
On 2025/3/13 18:57, Zhangfei Gao wrote:
> On Thu, 13 Mar 2025 at 17:51, Zhangfei Gao<zhangfei.gao@linaro.org> wrote:
>> Hi, Baolu
>>
>> On Thu, 13 Mar 2025 at 13:19, Lu Baolu<baolu.lu@linux.intel.com> wrote:
>>> The new method for driver fault reporting support relies on the domain
>>> to specify a iopf_handler. The driver should detect this and setup the
>>> HW when fault capable domains are attached.
>>>
>>> Move SMMUv3 to use this method and have VT-D validate support during
>>> attach so that all three fault capable drivers have a no-op FEAT_SVA and
>>> _IOPF. Then remove them.
>>>
>>> This was initiated by Jason. I'm following up to remove FEAT_IOPF and
>>> further clean up.
>>>
>>> The whole series is also available at github:
>>> https://github.com/LuBaolu/intel-iommu/commits/iommu_no_feat-v4
>> I got an issue on this branch.
>>
>> Linux 6.14-rc4 + iommu_no_feat-v2
>> drivers/pci/quirks.c
>> quirk_huawei_pcie_sva will set dma-can-stall first
>> arm_smmu_probe_device will check dma-can-stall and set stall_enabled
>> accordingly.
> This working branch arm_smmu_probe_device is called from pci_bus_add_device
> So pci_fixup_device is called first
>
> [ 1121.314405] arm_smmu_probe_device+0x48/0x450
> [ 1121.314410] __iommu_probe_device+0xc4/0x3c8
> [ 1121.314412] iommu_probe_device+0x40/0x90
> [ 1121.314414] acpi_dma_configure_id+0xb4/0x100
> [ 1121.314417] pci_dma_configure+0xf8/0x108
> [ 1121.314421] really_probe+0x78/0x278
> [ 1121.314425] __driver_probe_device+0x80/0x140
> [ 1121.314427] driver_probe_device+0x48/0x130
> [ 1121.314430] __device_attach_driver+0xc0/0x108
> [ 1121.314432] bus_for_each_drv+0x8c/0xf8
> [ 1121.314435] __device_attach+0x104/0x1a0
> [ 1121.314437] device_attach+0x1c/0x30
> [ 1121.314440] pci_bus_add_device+0xb8/0x1f0
> [ 1121.314442] pci_iov_add_virtfn+0x2ac/0x300
> [ 1121.314446] sriov_enable+0x204/0x468
> [ 1121.314447] pci_enable_sriov+0x20/0x40
>
>
>> This branch
>> arm_smmu_probe_device happens first, when dma-can-stall = 0, so
>> stall_enabled =0.
>> Then drivers/pci/quirks.c: quirk_xxx happens
> This not working branch: Linux 6.14-rc6 + iommu_no_feat-v4
> arm_smmu_probe_device is called by pci_device_add
> Then call pci_bus_add_device -> pci_fixup_device
>
> 215.072859] arm_smmu_probe_device+0x48/0x450
> [ 215.072871] __iommu_probe_device+0xc0/0x468
> [ 215.072875] iommu_probe_device+0x40/0x90
> [ 215.072877] iommu_bus_notifier+0x38/0x68
> [ 215.072879] notifier_call_chain+0x80/0x148
> [ 215.072886] blocking_notifier_call_chain+0x50/0x80
> [ 215.072889] bus_notify+0x44/0x68
> [ 215.072896] device_add+0x580/0x768
> [ 215.072898] pci_device_add+0x1e8/0x568
> [ 215.072906] pci_iov_add_virtfn+0x198/0x300
> [ 215.072910] sriov_enable+0x204/0x468
> [ 215.072912] pci_enable_sriov+0x20/0x40
>
> pci_iov_add_virtfn:
> pci_device_add(virtfn, virtfn->bus);
> pci_bus_add_device(virtfn); -> pci_fixup_device(pci_fixup_final, dev);
This probably is not caused by this patch series. Can you please have a
try with the next branch of iommu tree? Or the latest linux-next tree?
https://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git
Thanks,
baolu
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF
2025-03-13 11:20 ` Baolu Lu
@ 2025-03-13 11:37 ` Zhangfei Gao
2025-03-14 4:04 ` Zhangfei Gao
0 siblings, 1 reply; 23+ messages in thread
From: Zhangfei Gao @ 2025-03-13 11:37 UTC (permalink / raw)
To: Baolu Lu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian, Dave Jiang, Vinod Koul, Fenghua Yu, Zhou Wang, iommu,
linux-kernel, Jason Gunthorpe
On Thu, 13 Mar 2025 at 19:20, Baolu Lu <baolu.lu@linux.intel.com> wrote:
>
> On 2025/3/13 18:57, Zhangfei Gao wrote:
> > On Thu, 13 Mar 2025 at 17:51, Zhangfei Gao<zhangfei.gao@linaro.org> wrote:
> >> Hi, Baolu
> >>
> >> On Thu, 13 Mar 2025 at 13:19, Lu Baolu<baolu.lu@linux.intel.com> wrote:
> >>> The new method for driver fault reporting support relies on the domain
> >>> to specify a iopf_handler. The driver should detect this and setup the
> >>> HW when fault capable domains are attached.
> >>>
> >>> Move SMMUv3 to use this method and have VT-D validate support during
> >>> attach so that all three fault capable drivers have a no-op FEAT_SVA and
> >>> _IOPF. Then remove them.
> >>>
> >>> This was initiated by Jason. I'm following up to remove FEAT_IOPF and
> >>> further clean up.
> >>>
> >>> The whole series is also available at github:
> >>> https://github.com/LuBaolu/intel-iommu/commits/iommu_no_feat-v4
> >> I got an issue on this branch.
> >>
> >> Linux 6.14-rc4 + iommu_no_feat-v2
> >> drivers/pci/quirks.c
> >> quirk_huawei_pcie_sva will set dma-can-stall first
> >> arm_smmu_probe_device will check dma-can-stall and set stall_enabled
> >> accordingly.
> > This working branch arm_smmu_probe_device is called from pci_bus_add_device
> > So pci_fixup_device is called first
> >
> > [ 1121.314405] arm_smmu_probe_device+0x48/0x450
> > [ 1121.314410] __iommu_probe_device+0xc4/0x3c8
> > [ 1121.314412] iommu_probe_device+0x40/0x90
> > [ 1121.314414] acpi_dma_configure_id+0xb4/0x100
> > [ 1121.314417] pci_dma_configure+0xf8/0x108
> > [ 1121.314421] really_probe+0x78/0x278
> > [ 1121.314425] __driver_probe_device+0x80/0x140
> > [ 1121.314427] driver_probe_device+0x48/0x130
> > [ 1121.314430] __device_attach_driver+0xc0/0x108
> > [ 1121.314432] bus_for_each_drv+0x8c/0xf8
> > [ 1121.314435] __device_attach+0x104/0x1a0
> > [ 1121.314437] device_attach+0x1c/0x30
> > [ 1121.314440] pci_bus_add_device+0xb8/0x1f0
> > [ 1121.314442] pci_iov_add_virtfn+0x2ac/0x300
> > [ 1121.314446] sriov_enable+0x204/0x468
> > [ 1121.314447] pci_enable_sriov+0x20/0x40
> >
> >
> >> This branch
> >> arm_smmu_probe_device happens first, when dma-can-stall = 0, so
> >> stall_enabled =0.
> >> Then drivers/pci/quirks.c: quirk_xxx happens
> > This not working branch: Linux 6.14-rc6 + iommu_no_feat-v4
> > arm_smmu_probe_device is called by pci_device_add
> > Then call pci_bus_add_device -> pci_fixup_device
> >
> > 215.072859] arm_smmu_probe_device+0x48/0x450
> > [ 215.072871] __iommu_probe_device+0xc0/0x468
> > [ 215.072875] iommu_probe_device+0x40/0x90
> > [ 215.072877] iommu_bus_notifier+0x38/0x68
> > [ 215.072879] notifier_call_chain+0x80/0x148
> > [ 215.072886] blocking_notifier_call_chain+0x50/0x80
> > [ 215.072889] bus_notify+0x44/0x68
> > [ 215.072896] device_add+0x580/0x768
> > [ 215.072898] pci_device_add+0x1e8/0x568
> > [ 215.072906] pci_iov_add_virtfn+0x198/0x300
> > [ 215.072910] sriov_enable+0x204/0x468
> > [ 215.072912] pci_enable_sriov+0x20/0x40
> >
> > pci_iov_add_virtfn:
> > pci_device_add(virtfn, virtfn->bus);
> > pci_bus_add_device(virtfn); -> pci_fixup_device(pci_fixup_final, dev);
>
> This probably is not caused by this patch series. Can you please have a
> try with the next branch of iommu tree? Or the latest linux-next tree?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git
Ok, will try.
Though still not finding which patch caused the problem, will do more
investigation.
The issue is arm_smmu_probe_device is changed
from
pci_bus_add_device(virtfn); -> pci_fixup_device(pci_fixup_final, dev);
to
pci_device_add(virtfn, virtfn->bus) -> pci_fixup_device(pci_fixup_header, dev);
And the issue can be solved by
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
Thanks
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF
2025-03-13 11:37 ` Zhangfei Gao
@ 2025-03-14 4:04 ` Zhangfei Gao
0 siblings, 0 replies; 23+ messages in thread
From: Zhangfei Gao @ 2025-03-14 4:04 UTC (permalink / raw)
To: Baolu Lu, Robin Murphy
Cc: Joerg Roedel, Will Deacon, Jason Gunthorpe, Kevin Tian,
Dave Jiang, Vinod Koul, Fenghua Yu, Zhou Wang, iommu,
linux-kernel, Jason Gunthorpe
On Thu, 13 Mar 2025 at 19:37, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:
>
> On Thu, 13 Mar 2025 at 19:20, Baolu Lu <baolu.lu@linux.intel.com> wrote:
> >
> > On 2025/3/13 18:57, Zhangfei Gao wrote:
> > > On Thu, 13 Mar 2025 at 17:51, Zhangfei Gao<zhangfei.gao@linaro.org> wrote:
> > >> Hi, Baolu
> > >>
> > >> On Thu, 13 Mar 2025 at 13:19, Lu Baolu<baolu.lu@linux.intel.com> wrote:
> > >>> The new method for driver fault reporting support relies on the domain
> > >>> to specify a iopf_handler. The driver should detect this and setup the
> > >>> HW when fault capable domains are attached.
> > >>>
> > >>> Move SMMUv3 to use this method and have VT-D validate support during
> > >>> attach so that all three fault capable drivers have a no-op FEAT_SVA and
> > >>> _IOPF. Then remove them.
> > >>>
> > >>> This was initiated by Jason. I'm following up to remove FEAT_IOPF and
> > >>> further clean up.
> > >>>
> > >>> The whole series is also available at github:
> > >>> https://github.com/LuBaolu/intel-iommu/commits/iommu_no_feat-v4
> > >> I got an issue on this branch.
> > >>
> > >> Linux 6.14-rc4 + iommu_no_feat-v2
> > >> drivers/pci/quirks.c
> > >> quirk_huawei_pcie_sva will set dma-can-stall first
> > >> arm_smmu_probe_device will check dma-can-stall and set stall_enabled
> > >> accordingly.
> > > This working branch arm_smmu_probe_device is called from pci_bus_add_device
> > > So pci_fixup_device is called first
> > >
> > > [ 1121.314405] arm_smmu_probe_device+0x48/0x450
> > > [ 1121.314410] __iommu_probe_device+0xc4/0x3c8
> > > [ 1121.314412] iommu_probe_device+0x40/0x90
> > > [ 1121.314414] acpi_dma_configure_id+0xb4/0x100
> > > [ 1121.314417] pci_dma_configure+0xf8/0x108
> > > [ 1121.314421] really_probe+0x78/0x278
> > > [ 1121.314425] __driver_probe_device+0x80/0x140
> > > [ 1121.314427] driver_probe_device+0x48/0x130
> > > [ 1121.314430] __device_attach_driver+0xc0/0x108
> > > [ 1121.314432] bus_for_each_drv+0x8c/0xf8
> > > [ 1121.314435] __device_attach+0x104/0x1a0
> > > [ 1121.314437] device_attach+0x1c/0x30
> > > [ 1121.314440] pci_bus_add_device+0xb8/0x1f0
> > > [ 1121.314442] pci_iov_add_virtfn+0x2ac/0x300
> > > [ 1121.314446] sriov_enable+0x204/0x468
> > > [ 1121.314447] pci_enable_sriov+0x20/0x40
> > >
> > >
> > >> This branch
> > >> arm_smmu_probe_device happens first, when dma-can-stall = 0, so
> > >> stall_enabled =0.
> > >> Then drivers/pci/quirks.c: quirk_xxx happens
> > > This not working branch: Linux 6.14-rc6 + iommu_no_feat-v4
> > > arm_smmu_probe_device is called by pci_device_add
> > > Then call pci_bus_add_device -> pci_fixup_device
> > >
> > > 215.072859] arm_smmu_probe_device+0x48/0x450
> > > [ 215.072871] __iommu_probe_device+0xc0/0x468
> > > [ 215.072875] iommu_probe_device+0x40/0x90
> > > [ 215.072877] iommu_bus_notifier+0x38/0x68
> > > [ 215.072879] notifier_call_chain+0x80/0x148
> > > [ 215.072886] blocking_notifier_call_chain+0x50/0x80
> > > [ 215.072889] bus_notify+0x44/0x68
> > > [ 215.072896] device_add+0x580/0x768
> > > [ 215.072898] pci_device_add+0x1e8/0x568
> > > [ 215.072906] pci_iov_add_virtfn+0x198/0x300
> > > [ 215.072910] sriov_enable+0x204/0x468
> > > [ 215.072912] pci_enable_sriov+0x20/0x40
> > >
> > > pci_iov_add_virtfn:
> > > pci_device_add(virtfn, virtfn->bus);
> > > pci_bus_add_device(virtfn); -> pci_fixup_device(pci_fixup_final, dev);
> >
> > This probably is not caused by this patch series. Can you please have a
> > try with the next branch of iommu tree? Or the latest linux-next tree?
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git
>
>
> Ok, will try.
>
> Though still not finding which patch caused the problem, will do more
> investigation.
>
> The issue is arm_smmu_probe_device is changed
> from
> pci_bus_add_device(virtfn); -> pci_fixup_device(pci_fixup_final, dev);
> to
> pci_device_add(virtfn, virtfn->bus) -> pci_fixup_device(pci_fixup_header, dev);
Update:
This sequence change is caused by
bcb81ac6ae3c iommu: Get DT/ACPI parsing into the proper probe path
The probe is put in earlier so we have to make fixup earlier as well.
DECLARE_PCI_FIXUP_FINAL -> DECLARE_PCI_FIXUP_HEADER
Thanks
>
> And the issue can be solved by
> -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
>
> Thanks
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
` (8 preceding siblings ...)
2025-03-13 9:51 ` [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Zhangfei Gao
@ 2025-03-14 8:11 ` Zhangfei Gao
2025-03-17 14:15 ` Jason Gunthorpe
2025-04-16 7:29 ` Zhangfei Gao
10 siblings, 1 reply; 23+ messages in thread
From: Zhangfei Gao @ 2025-03-14 8:11 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian, Dave Jiang, Vinod Koul, Fenghua Yu, Zhou Wang, iommu,
linux-kernel, Jason Gunthorpe
Hi, Baolu
On Thu, 13 Mar 2025 at 13:19, Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
> The new method for driver fault reporting support relies on the domain
> to specify a iopf_handler. The driver should detect this and setup the
> HW when fault capable domains are attached.
>
> Move SMMUv3 to use this method and have VT-D validate support during
> attach so that all three fault capable drivers have a no-op FEAT_SVA and
> _IOPF. Then remove them.
>
> This was initiated by Jason. I'm following up to remove FEAT_IOPF and
> further clean up.
>
> The whole series is also available at github:
> https://github.com/LuBaolu/intel-iommu/commits/iommu_no_feat-v4
I retested this branch, and it works
https://github.com/Linaro/linux-kernel-uadk/tree/iommu_no_feat-v4
Thanks
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF
2025-03-14 8:11 ` Zhangfei Gao
@ 2025-03-17 14:15 ` Jason Gunthorpe
0 siblings, 0 replies; 23+ messages in thread
From: Jason Gunthorpe @ 2025-03-17 14:15 UTC (permalink / raw)
To: Zhangfei Gao
Cc: Lu Baolu, Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian,
Dave Jiang, Vinod Koul, Fenghua Yu, Zhou Wang, iommu,
linux-kernel
On Fri, Mar 14, 2025 at 04:11:56PM +0800, Zhangfei Gao wrote:
> Hi, Baolu
>
> On Thu, 13 Mar 2025 at 13:19, Lu Baolu <baolu.lu@linux.intel.com> wrote:
> >
> > The new method for driver fault reporting support relies on the domain
> > to specify a iopf_handler. The driver should detect this and setup the
> > HW when fault capable domains are attached.
> >
> > Move SMMUv3 to use this method and have VT-D validate support during
> > attach so that all three fault capable drivers have a no-op FEAT_SVA and
> > _IOPF. Then remove them.
> >
> > This was initiated by Jason. I'm following up to remove FEAT_IOPF and
> > further clean up.
> >
> > The whole series is also available at github:
> > https://github.com/LuBaolu/intel-iommu/commits/iommu_no_feat-v4
>
> I retested this branch, and it works
>
> https://github.com/Linaro/linux-kernel-uadk/tree/iommu_no_feat-v4
Thanks!
Jason
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF
2025-03-13 5:19 [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF Lu Baolu
` (9 preceding siblings ...)
2025-03-14 8:11 ` Zhangfei Gao
@ 2025-04-16 7:29 ` Zhangfei Gao
2025-04-16 7:33 ` Baolu Lu
10 siblings, 1 reply; 23+ messages in thread
From: Zhangfei Gao @ 2025-04-16 7:29 UTC (permalink / raw)
To: Lu Baolu
Cc: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe,
Kevin Tian, Dave Jiang, Vinod Koul, Fenghua Yu, Zhou Wang, iommu,
linux-kernel, Jason Gunthorpe
Hi, Baolu
On Thu, 13 Mar 2025 at 13:19, Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
> The new method for driver fault reporting support relies on the domain
> to specify a iopf_handler. The driver should detect this and setup the
> HW when fault capable domains are attached.
>
> Move SMMUv3 to use this method and have VT-D validate support during
> attach so that all three fault capable drivers have a no-op FEAT_SVA and
> _IOPF. Then remove them.
>
> This was initiated by Jason. I'm following up to remove FEAT_IOPF and
> further clean up.
>
> The whole series is also available at github:
> https://github.com/LuBaolu/intel-iommu/commits/iommu_no_feat-v4
>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>
Will you re-spin this patchset, it is not in 6.15-rc.
Thanks
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v4 0/8] iommu: Remove IOMMU_DEV_FEAT_SVA/_IOPF
2025-04-16 7:29 ` Zhangfei Gao
@ 2025-04-16 7:33 ` Baolu Lu
0 siblings, 0 replies; 23+ messages in thread
From: Baolu Lu @ 2025-04-16 7:33 UTC (permalink / raw)
To: Zhangfei Gao
Cc: baolu.lu, Joerg Roedel, Will Deacon, Robin Murphy,
Jason Gunthorpe, Kevin Tian, Dave Jiang, Vinod Koul, Fenghua Yu,
Zhou Wang, iommu, linux-kernel, Jason Gunthorpe
On 4/16/2025 3:29 PM, Zhangfei Gao wrote:
> On Thu, 13 Mar 2025 at 13:19, Lu Baolu<baolu.lu@linux.intel.com> wrote:
>> The new method for driver fault reporting support relies on the domain
>> to specify a iopf_handler. The driver should detect this and setup the
>> HW when fault capable domains are attached.
>>
>> Move SMMUv3 to use this method and have VT-D validate support during
>> attach so that all three fault capable drivers have a no-op FEAT_SVA and
>> _IOPF. Then remove them.
>>
>> This was initiated by Jason. I'm following up to remove FEAT_IOPF and
>> further clean up.
>>
>> The whole series is also available at github:
>> https://github.com/LuBaolu/intel-iommu/commits/iommu_no_feat-v4
>>
>> Signed-off-by: Jason Gunthorpe<jgg@nvidia.com>
>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>>
> Will you re-spin this patchset, it is not in 6.15-rc.
Yes. I will rebase the patches and post a new version for v6.16 merge.
Thanks,
baolu
^ permalink raw reply [flat|nested] 23+ messages in thread