Skip to contentSkip to conversation

Remove VT-d virtual command interface and IOASID

PATCH Superseded Jacob Pan 7 participants 34 messages Mar 1, 2023 – Mar 20, 2023

Subsystems (8)

v4 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Jacob Pan

author
#0 lore raw

v4 cover Remove VT-d virtual command interface and IOASID

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Hi all,

This patch set removes unused VT-d virtual command interface followed by removal of the IOASID infrastructure.

This has only been tested on x86 platforms, need help with testing on ARM SMMU and other architectures.

Thanks,

Jacob

ChangeLog:
v4:
- keep mm_pasid helpers inline as much as we can for fork performance
- separate GFP_ATOMIC to GFP_KERNEL change for bisectability

v3:
- moved helper functions for PASID under SVA code, avoided circular inclusion between mm.h and iommu.h
- deleted makefiles
- put rename under a different patch

Jacob Pan (4):
iommu/vt-d: Remove virtual command interface
iommu/sva: Move PASID helpers to sva code
iommu/sva: Use GFP_KERNEL for pasid allocation
iommu/ioasid: Rename INVALID_IOASID

Jason Gunthorpe (2):
iommu/sva: Stop using ioasid_set for SVA
iommu: Remove ioasid infrastructure

 Documentation/x86/sva.rst       |   2 +-
 arch/x86/kernel/traps.c         |   5 +-
 drivers/dma/idxd/device.c       |   8 +-
 drivers/dma/idxd/idxd.h         |   2 +-
 drivers/dma/idxd/init.c         |   2 +-
 drivers/dma/idxd/irq.c          |   2 +-
 drivers/iommu/Kconfig           |   5 -
 drivers/iommu/Makefile          |   1 -
 drivers/iommu/intel/cap_audit.c |   2 -
 drivers/iommu/intel/dmar.c      |   6 +-
 drivers/iommu/intel/iommu.c     |  87 +------
 drivers/iommu/intel/iommu.h     |   9 -
 drivers/iommu/intel/svm.c       |   3 +-
 drivers/iommu/ioasid.c          | 422 --------------------------------
 drivers/iommu/iommu-sva.c       |  62 ++---
 drivers/iommu/iommu-sva.h       |   4 -
 include/linux/ioasid.h          |  83 -------
 include/linux/iommu-helper.h    |   1 +
 include/linux/iommu.h           |   8 +-
 include/linux/sched/mm.h        |  20 +-
 mm/init-mm.c                    |   4 +-
 21 files changed, 46 insertions(+), 692 deletions(-)
 delete mode 100644 drivers/iommu/ioasid.c
 delete mode 100644 include/linux/ioasid.h
Signature · 1 line
2.25.1
author
#1 lore raw

PATCH v4 1/6 iommu/vt-d: Remove virtual command interface

Virtual command interface was introduced to allow using host PASIDs inside VMs. It is unused and abandoned due to architectural change.

With this patch, we can safely remove this feature and the related helpers.

---
 drivers/iommu/intel/cap_audit.c |  2 -
 drivers/iommu/intel/dmar.c      |  2 -
 drivers/iommu/intel/iommu.c     | 85 ---------------------------------
 drivers/iommu/intel/iommu.h     |  8 ----
 4 files changed, 97 deletions(-)
drivers/iommu/intel/cap_audit.c +0 -2
index 806986696841..9862dc20b35e 100644
@@ -54,7 +54,6 @@ static inline void check_dmar_capabilities(struct intel_iommu *a,
 	CHECK_FEATURE_MISMATCH(a, b, ecap, slts, ECAP_SLTS_MASK);
 	CHECK_FEATURE_MISMATCH(a, b, ecap, nwfs, ECAP_NWFS_MASK);
 	CHECK_FEATURE_MISMATCH(a, b, ecap, slads, ECAP_SLADS_MASK);
-	CHECK_FEATURE_MISMATCH(a, b, ecap, vcs, ECAP_VCS_MASK);
 	CHECK_FEATURE_MISMATCH(a, b, ecap, smts, ECAP_SMTS_MASK);
 	CHECK_FEATURE_MISMATCH(a, b, ecap, pds, ECAP_PDS_MASK);
 	CHECK_FEATURE_MISMATCH(a, b, ecap, dit, ECAP_DIT_MASK);
@@ -101,7 +100,6 @@ static int cap_audit_hotplug(struct intel_iommu *iommu, enum cap_audit_type type
 	CHECK_FEATURE_MISMATCH_HOTPLUG(iommu, ecap, slts, ECAP_SLTS_MASK);
 	CHECK_FEATURE_MISMATCH_HOTPLUG(iommu, ecap, nwfs, ECAP_NWFS_MASK);
 	CHECK_FEATURE_MISMATCH_HOTPLUG(iommu, ecap, slads, ECAP_SLADS_MASK);
-	CHECK_FEATURE_MISMATCH_HOTPLUG(iommu, ecap, vcs, ECAP_VCS_MASK);
 	CHECK_FEATURE_MISMATCH_HOTPLUG(iommu, ecap, smts, ECAP_SMTS_MASK);
 	CHECK_FEATURE_MISMATCH_HOTPLUG(iommu, ecap, pds, ECAP_PDS_MASK);
 	CHECK_FEATURE_MISMATCH_HOTPLUG(iommu, ecap, dit, ECAP_DIT_MASK);
drivers/iommu/intel/dmar.c +0 -2
index b00a0ceb2d13..bf0bfe5ba7a7 100644
@@ -989,8 +989,6 @@ static int map_iommu(struct intel_iommu *iommu, u64 phys_addr)
 		warn_invalid_dmar(phys_addr, " returns all ones");
 		goto unmap;
 	}
-	if (ecap_vcs(iommu->ecap))
-		iommu->vccap = dmar_readq(iommu->reg + DMAR_VCCAP_REG);
 
 	/* the registers might be more than one page */
 	map_size = max_t(int, ecap_max_iotlb_offset(iommu->ecap),
drivers/iommu/intel/iommu.c +0 -85

103 lines not on this page · raw message

drivers/iommu/intel/iommu.h +0 -8
index 06e61e474856..6bdfbead82c4 100644
@@ -184,7 +184,6 @@
 #define ecap_flts(e)		(((e) >> 47) & 0x1)
 #define ecap_slts(e)		(((e) >> 46) & 0x1)
 #define ecap_slads(e)		(((e) >> 45) & 0x1)
-#define ecap_vcs(e)		(((e) >> 44) & 0x1)
 #define ecap_smts(e)		(((e) >> 43) & 0x1)
 #define ecap_dit(e)		(((e) >> 41) & 0x1)
 #define ecap_pds(e)		(((e) >> 42) & 0x1)
@@ -210,9 +209,6 @@
 #define ecap_max_handle_mask(e) (((e) >> 20) & 0xf)
 #define ecap_sc_support(e)	(((e) >> 7) & 0x1) /* Snooping Control */
 
-/* Virtual command interface capability */
-#define vccap_pasid(v)		(((v) & DMA_VCS_PAS)) /* PASID allocation */
-
 /* IOTLB_REG */
 #define DMA_TLB_FLUSH_GRANU_OFFSET  60
 #define DMA_TLB_GLOBAL_FLUSH (((u64)1) << 60)
@@ -307,8 +303,6 @@
 #define DMA_PRS_PPR	((u32)1)
 #define DMA_PRS_PRO	((u32)2)
 
-#define DMA_VCS_PAS	((u64)1)
-
 #define IOMMU_WAIT_OP(iommu, offset, op, cond, sts)			\
 do {									\
 	cycles_t start_time = get_cycles();				\
@@ -560,7 +554,6 @@ struct intel_iommu {
 	u64		reg_size; /* size of hw register set */
 	u64		cap;
 	u64		ecap;
-	u64		vccap;
 	u32		gcmd; /* Holds TE, EAFL. Don't need SRTP, SFL, WBF */
 	raw_spinlock_t	register_lock; /* protect register handling */
 	int		seq_id;	/* sequence id of the iommu */
@@ -583,7 +576,6 @@ struct intel_iommu {
 	unsigned char prq_name[16];    /* Name for PRQ interrupt */
 	unsigned long prq_seq_number;
 	struct completion prq_complete;
-	struct ioasid_allocator_ops pasid_allocator; /* Custom allocator for PASIDs */
 #endif
 	struct iopf_queue *iopf_queue;
 	unsigned char iopfq_name[16];
Signature · 1 line
2.25.1
author
#2 lore raw

PATCH v4 2/6 iommu/sva: Move PASID helpers to sva code

In reply to Jacob Pan

Preparing to remove IOASID infrastructure, PASID management will be under SVA code. Decouple mm code from IOASID. Use iommu-help.h instead of iommu.h to prevent circular inclusion.

---
v4: (Jason's comments)
- delete and open code mm_set_pasid
- keep mm_init_pasid() as inline for fork performance

---
 drivers/iommu/iommu-sva.c    | 10 +++++++++-
 include/linux/ioasid.h       |  2 +-
 include/linux/iommu-helper.h |  1 +
 include/linux/sched/mm.h     | 18 ++----------------
 4 files changed, 13 insertions(+), 18 deletions(-)
drivers/iommu/iommu-sva.c +9 -1
index 24bf9b2b58aa..376b2a9e2543 100644
@@ -44,7 +44,7 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
 	if (!pasid_valid(pasid))
 		ret = -ENOMEM;
 	else
-		mm_pasid_set(mm, pasid);
+		mm->pasid = ret;
 out:
 	mutex_unlock(&iommu_sva_lock);
 	return ret;
@@ -238,3 +238,11 @@ iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
 
 	return status;
 }
+
+void mm_pasid_drop(struct mm_struct *mm)
+{
+	if (pasid_valid(mm->pasid)) {
+		ioasid_free(mm->pasid);
+		mm->pasid = INVALID_IOASID;
+	}
+}
include/linux/ioasid.h +1 -1
index af1c9d62e642..2c502e77ee78 100644
@@ -4,8 +4,8 @@
 
 #include <linux/types.h>
 #include <linux/errno.h>
+#include <linux/iommu-helper.h>
 
-#define INVALID_IOASID ((ioasid_t)-1)
 typedef unsigned int ioasid_t;
 typedef ioasid_t (*ioasid_alloc_fn_t)(ioasid_t min, ioasid_t max, void *data);
 typedef void (*ioasid_free_fn_t)(ioasid_t ioasid, void *data);
include/linux/iommu-helper.h +1 -0
index 74be34f3a20a..0aa922f6bfad 100644
@@ -40,5 +40,6 @@ static inline unsigned long iommu_num_pages(unsigned long addr,
 
 	return DIV_ROUND_UP(size, io_page_size);
 }
+#define INVALID_IOASID	(-1U)
 
 #endif
include/linux/sched/mm.h +2 -16
index 2a243616f222..ae5a3f16b321 100644
@@ -8,7 +8,7 @@
 #include <linux/mm_types.h>
 #include <linux/gfp.h>
 #include <linux/sync_core.h>
-#include <linux/ioasid.h>
+#include <linux/iommu-helper.h>
 
 /*
  * Routines for handling mm_structs
@@ -456,23 +456,9 @@ static inline void mm_pasid_init(struct mm_struct *mm)
 {
 	mm->pasid = INVALID_IOASID;
 }
-
-/* Associate a PASID with an mm_struct: */
-static inline void mm_pasid_set(struct mm_struct *mm, u32 pasid)
-{
-	mm->pasid = pasid;
-}
-
-static inline void mm_pasid_drop(struct mm_struct *mm)
-{
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
-}
+void mm_pasid_drop(struct mm_struct *mm);
 #else
 static inline void mm_pasid_init(struct mm_struct *mm) {}
-static inline void mm_pasid_set(struct mm_struct *mm, u32 pasid) {}
 static inline void mm_pasid_drop(struct mm_struct *mm) {}
 #endif
 
Signature · 1 line
2.25.1
#3 lore raw

RE: [PATCH v4 2/6] iommu/sva: Move PASID helpers to sva code

include/linux/sched/mm.h static inline void mm_pasid_init(struct mm_struct *mm) · line 459

Quoted 11 lines

From: Jacob Pan <[email protected]>
Sent: Thursday, March 2, 2023 7:57 AM

include/linux/sched/mm.h +1 -7
-static inline void mm_pasid_drop(struct mm_struct *mm)
-{
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
-}
+void mm_pasid_drop(struct mm_struct *mm);

Is it good to have a function declared in a header file of one subsystem while being implemented in another subsystem?

author
#4 lore raw

Re: [PATCH v4 2/6] iommu/sva: Move PASID helpers to sva code

Hi Kevin,

Quoted 14 lines · Kevin

include/linux/sched/mm.h static inline void mm_pasid_init(struct mm_struct *mm) · line 459

Quoted 11 lines

From: Jacob Pan <[email protected]>
Sent: Thursday, March 2, 2023 7:57 AM

include/linux/sched/mm.h +1 -7
-static inline void mm_pasid_drop(struct mm_struct *mm)
-{
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
-}
+void mm_pasid_drop(struct mm_struct *mm);  

Is it good to have a function declared in a header file of one subsystem while being implemented in another subsystem?

Good point! I will move it to iommu_helper.h

Thanks,

Jacob

maintainer
#5 lore raw

Re: [PATCH v4 2/6] iommu/sva: Move PASID helpers to sva code

In reply to Jacob Pan
Quoted 21 lines · Jacob Pan

Preparing to remove IOASID infrastructure, PASID management will be under SVA code. Decouple mm code from IOASID. Use iommu-help.h instead of iommu.h to prevent circular inclusion.

---
v4: (Jason's comments)
- delete and open code mm_set_pasid
- keep mm_init_pasid() as inline for fork performance

---
  drivers/iommu/iommu-sva.c    | 10 +++++++++-
  include/linux/ioasid.h       |  2 +-
  include/linux/iommu-helper.h |  1 +
  include/linux/sched/mm.h     | 18 ++----------------
  4 files changed, 13 insertions(+), 18 deletions(-)
drivers/iommu/iommu-sva.c +0 -0
index 24bf9b2b58aa..376b2a9e2543 100644

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) · line 47

Quoted 5 lines
drivers/iommu/iommu-sva.c +1 -1
@@ -44,7 +44,7 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
  	if (!pasid_valid(pasid))
  		ret = -ENOMEM;
  	else
-		mm_pasid_set(mm, pasid);
+		mm->pasid = ret;

This seems obviously incorrect.

		mm->pasid = pasid;

?

Quoted 66 lines
drivers/iommu/iommu-sva.c +8 -0
  out:
  	mutex_unlock(&iommu_sva_lock);
  	return ret;
@@ -238,3 +238,11 @@ iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
 
  	return status;
  }
+
+void mm_pasid_drop(struct mm_struct *mm)
+{
+	if (pasid_valid(mm->pasid)) {
+		ioasid_free(mm->pasid);
+		mm->pasid = INVALID_IOASID;
+	}
+}
include/linux/ioasid.h +1 -1
index af1c9d62e642..2c502e77ee78 100644
@@ -4,8 +4,8 @@
 
  #include <linux/types.h>
  #include <linux/errno.h>
+#include <linux/iommu-helper.h>
 
-#define INVALID_IOASID ((ioasid_t)-1)
  typedef unsigned int ioasid_t;
  typedef ioasid_t (*ioasid_alloc_fn_t)(ioasid_t min, ioasid_t max, void *data);
  typedef void (*ioasid_free_fn_t)(ioasid_t ioasid, void *data);
include/linux/iommu-helper.h +1 -0
index 74be34f3a20a..0aa922f6bfad 100644
@@ -40,5 +40,6 @@ static inline unsigned long iommu_num_pages(unsigned long addr,
 
  	return DIV_ROUND_UP(size, io_page_size);
  }
+#define INVALID_IOASID	(-1U)
 
  #endif
include/linux/sched/mm.h +1 -10
index 2a243616f222..ae5a3f16b321 100644
@@ -8,7 +8,7 @@
  #include <linux/mm_types.h>
  #include <linux/gfp.h>
  #include <linux/sync_core.h>
-#include <linux/ioasid.h>
+#include <linux/iommu-helper.h>
 
  /*
   * Routines for handling mm_structs
@@ -456,23 +456,9 @@ static inline void mm_pasid_init(struct mm_struct *mm)
  {
  	mm->pasid = INVALID_IOASID;
  }
-
-/* Associate a PASID with an mm_struct: */
-static inline void mm_pasid_set(struct mm_struct *mm, u32 pasid)
-{
-	mm->pasid = pasid;
-}
-
-static inline void mm_pasid_drop(struct mm_struct *mm)
-{

include/linux/sched/mm.h static inline void mm_pasid_init(struct mm_struct *mm) · line 462

Quoted 10 lines
include/linux/sched/mm.h +1 -6
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
-}
+void mm_pasid_drop(struct mm_struct *mm);
  #else
  static inline void mm_pasid_init(struct mm_struct *mm) {}
-static inline void mm_pasid_set(struct mm_struct *mm, u32 pasid) {}
  static inline void mm_pasid_drop(struct mm_struct *mm) {}

Above mm_pasid_drop() should also be removed.

include/linux/sched/mm.h static inline void mm_pasid_init(struct mm_struct *mm) · line 464

Quoted 2 lines
include/linux/sched/mm.h +0 -0
  #endif
 

Best regards,
baolu

author
#6 lore raw

Re: [PATCH v4 2/6] iommu/sva: Move PASID helpers to sva code

Hi Baolu,

Quoted 30 lines · Baolu Lu
Quoted 21 lines · Jacob Pan

Preparing to remove IOASID infrastructure, PASID management will be under SVA code. Decouple mm code from IOASID. Use iommu-help.h instead of iommu.h to prevent circular inclusion.

---
v4: (Jason's comments)
- delete and open code mm_set_pasid
- keep mm_init_pasid() as inline for fork performance

---
  drivers/iommu/iommu-sva.c    | 10 +++++++++-
  include/linux/ioasid.h       |  2 +-
  include/linux/iommu-helper.h |  1 +
  include/linux/sched/mm.h     | 18 ++----------------
  4 files changed, 13 insertions(+), 18 deletions(-)
drivers/iommu/iommu-sva.c +0 -0
index 24bf9b2b58aa..376b2a9e2543 100644

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, · line 47

Quoted 5 lines
drivers/iommu/iommu-sva.c +1 -1
@@ -44,7 +44,7 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm,
 ioasid_t min, ioasid_t max) if (!pasid_valid(pasid))
  		ret = -ENOMEM;
  	else
-		mm_pasid_set(mm, pasid);
+		mm->pasid = ret;  

This seems obviously incorrect.

Quoted 2 lines
		mm->pasid = pasid;

you are right, it got messed up while partitioning. the final was correct.

Quoted 71 lines
Quoted 71 lines
drivers/iommu/iommu-sva.c +8 -0
  out:
  	mutex_unlock(&iommu_sva_lock);
  	return ret;
@@ -238,3 +238,11 @@ iommu_sva_handle_iopf(struct iommu_fault *fault,
 void *data) 
  	return status;
  }
+
+void mm_pasid_drop(struct mm_struct *mm)
+{
+	if (pasid_valid(mm->pasid)) {
+		ioasid_free(mm->pasid);
+		mm->pasid = INVALID_IOASID;
+	}
+}
include/linux/ioasid.h +1 -1
index af1c9d62e642..2c502e77ee78 100644
@@ -4,8 +4,8 @@
 
  #include <linux/types.h>
  #include <linux/errno.h>
+#include <linux/iommu-helper.h>
 
-#define INVALID_IOASID ((ioasid_t)-1)
  typedef unsigned int ioasid_t;
  typedef ioasid_t (*ioasid_alloc_fn_t)(ioasid_t min, ioasid_t max,

void *data); typedef void (*ioasid_free_fn_t)(ioasid_t ioasid, void *data); diff --git a/include/linux/iommu-helper.h b/include/linux/iommu-helper.h index 74be34f3a20a..0aa922f6bfad 100644

include/linux/iommu-helper.h +1 -0
@@ -40,5 +40,6 @@ static inline unsigned long iommu_num_pages(unsigned
 long addr, 
  	return DIV_ROUND_UP(size, io_page_size);
  }
+#define INVALID_IOASID	(-1U)
 
  #endif
include/linux/sched/mm.h +1 -14
index 2a243616f222..ae5a3f16b321 100644
@@ -8,7 +8,7 @@
  #include <linux/mm_types.h>
  #include <linux/gfp.h>
  #include <linux/sync_core.h>
-#include <linux/ioasid.h>
+#include <linux/iommu-helper.h>
 
  /*
   * Routines for handling mm_structs
@@ -456,23 +456,9 @@ static inline void mm_pasid_init(struct mm_struct
 *mm) {
  	mm->pasid = INVALID_IOASID;
  }
-
-/* Associate a PASID with an mm_struct: */
-static inline void mm_pasid_set(struct mm_struct *mm, u32 pasid)
-{
-	mm->pasid = pasid;
-}
-
-static inline void mm_pasid_drop(struct mm_struct *mm)
-{
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
Quoted 8 lines

include/linux/sched/mm.h static inline void mm_pasid_init(struct mm_struct *mm) · line 462

Quoted 6 lines
include/linux/sched/mm.h +1 -2
-}
+void mm_pasid_drop(struct mm_struct *mm);
  #else
  static inline void mm_pasid_init(struct mm_struct *mm) {}
-static inline void mm_pasid_set(struct mm_struct *mm, u32 pasid) {}
  static inline void mm_pasid_drop(struct mm_struct *mm) {}  

Above mm_pasid_drop() should also be removed.

will do,

Thanks,

Jacob

author
#7 lore raw

PATCH v4 3/6 iommu/sva: Stop using ioasid_set for SVA

In reply to Jacob Pan

From: Jason Gunthorpe <[email protected]>

Instead SVA drivers can use a simple global IDA to allocate PASIDs for each mm_struct.

Future work would be to allow drivers using the SVA APIs to reserve global PASIDs from this IDA for their internal use, eg with the DMA API PASID support.

---
v4:
- Keep GFP_ATOMIC flag for PASID allocation, will changed to GFP_KERNEL in a separate patch.

---
 drivers/iommu/iommu-sva.c | 62 ++++++++++-----------------------------
 drivers/iommu/iommu-sva.h |  3 --
 2 files changed, 15 insertions(+), 50 deletions(-)
drivers/iommu/iommu-sva.c +15 -47
index 376b2a9e2543..297852ae5e7c 100644
@@ -9,26 +9,13 @@
 #include "iommu-sva.h"
 
 static DEFINE_MUTEX(iommu_sva_lock);
-static DECLARE_IOASID_SET(iommu_sva_pasid);
+static DEFINE_IDA(iommu_global_pasid_ida);
 
-/**
- * iommu_sva_alloc_pasid - Allocate a PASID for the mm
- * @mm: the mm
- * @min: minimum PASID value (inclusive)
- * @max: maximum PASID value (inclusive)
- *
- * Try to allocate a PASID for this mm, or take a reference to the existing one
- * provided it fits within the [@min, @max] range. On success the PASID is
- * available in mm->pasid and will be available for the lifetime of the mm.
- *
- * Returns 0 on success and < 0 on error.
- */
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
+static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
 {
-	int ret = 0;
-	ioasid_t pasid;
+	int ret;
 
-	if (min == INVALID_IOASID || max == INVALID_IOASID ||
+	if (min == IOMMU_PASID_INVALID || max == IOMMU_PASID_INVALID ||
 	    min == 0 || max < min)
 		return -EINVAL;
 
@@ -37,39 +24,20 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
 	if (pasid_valid(mm->pasid)) {
 		if (mm->pasid < min || mm->pasid >= max)
 			ret = -EOVERFLOW;
+		else
+			ret = 0;
 		goto out;
 	}
 
-	pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm);
-	if (!pasid_valid(pasid))
-		ret = -ENOMEM;
-	else
-		mm->pasid = ret;
+	ret = ida_alloc_range(&iommu_global_pasid_ida, min, max, GFP_ATOMIC);
+	if (ret < min)
+		goto out;
+	mm->pasid = ret;
+	ret = 0;
 out:
 	mutex_unlock(&iommu_sva_lock);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(iommu_sva_alloc_pasid);
-
-/* ioasid_find getter() requires a void * argument */
-static bool __mmget_not_zero(void *mm)
-{
-	return mmget_not_zero(mm);
-}
-
-/**
- * iommu_sva_find() - Find mm associated to the given PASID
- * @pasid: Process Address Space ID assigned to the mm
- *
- * On success a reference to the mm is taken, and must be released with mmput().
- *
- * Returns the mm corresponding to this PASID, or an error if not found.
- */
-struct mm_struct *iommu_sva_find(ioasid_t pasid)
-{
-	return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero);
-}
-EXPORT_SYMBOL_GPL(iommu_sva_find);
 
 /**
  * iommu_sva_bind_device() - Bind a process address space to a device
@@ -241,8 +209,8 @@ iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
 
 void mm_pasid_drop(struct mm_struct *mm)
 {
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
+	if (likely(!pasid_valid(mm->pasid)))
+		return;
+
+	ida_free(&iommu_global_pasid_ida, mm->pasid);
 }
drivers/iommu/iommu-sva.h +0 -3
index 7215a761b962..c22d0174ad61 100644
@@ -8,9 +8,6 @@
 #include <linux/ioasid.h>
 #include <linux/mm_types.h>
 
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max);
-struct mm_struct *iommu_sva_find(ioasid_t pasid);
-
 /* I/O Page fault */
 struct device;
 struct iommu_fault;
Signature · 1 line
2.25.1
Reviewed-by on v4 3/6
#8 lore raw

RE: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

drivers/iommu/iommu-sva.c line 20

Quoted 10 lines

From: Jacob Pan <[email protected]>
Sent: Thursday, March 2, 2023 7:57 AM

(quoted diff) +1 -1
-	if (min == INVALID_IOASID || max == INVALID_IOASID ||
+	if (min == IOMMU_PASID_INVALID || max ==

IOMMU_PASID_INVALID ||

drivers/iommu/iommu-sva.c +0 -0
 	    min == 0 || max < min)
 		return -EINVAL;

if (!pasid_valid(min) || !pasid_valid(max) || ...)

with that,

author
#9 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Hi Kevin,

Quoted 4 lines · Kevin
Quoted 4 lines

From: Jacob Pan <[email protected]>
Sent: Thursday, March 2, 2023 7:57 AM

Quoted 9 lines

drivers/iommu/iommu-sva.c line 20

Quoted 6 lines
(quoted diff) +1 -1
-	if (min == INVALID_IOASID || max == INVALID_IOASID ||
+	if (min == IOMMU_PASID_INVALID || max ==

IOMMU_PASID_INVALID ||

drivers/iommu/iommu-sva.c +0 -0
 	    min == 0 || max < min)
 		return -EINVAL;

if (!pasid_valid(min) || !pasid_valid(max) || ...)

will do

Quoted 4 lines

with that,

Thanks,

Jacob

maintainer
#10 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

In reply to Jacob Pan
Quoted 43 lines · Jacob Pan

From: Jason Gunthorpe <[email protected]>

Instead SVA drivers can use a simple global IDA to allocate PASIDs for each mm_struct.

Future work would be to allow drivers using the SVA APIs to reserve global PASIDs from this IDA for their internal use, eg with the DMA API PASID support.

---
v4:
- Keep GFP_ATOMIC flag for PASID allocation, will changed to GFP_KERNEL in a separate patch.

---
  drivers/iommu/iommu-sva.c | 62 ++++++++++-----------------------------
  drivers/iommu/iommu-sva.h |  3 --
  2 files changed, 15 insertions(+), 50 deletions(-)
drivers/iommu/iommu-sva.c +1 -13
index 376b2a9e2543..297852ae5e7c 100644
@@ -9,26 +9,13 @@
  #include "iommu-sva.h"
 
  static DEFINE_MUTEX(iommu_sva_lock);
-static DECLARE_IOASID_SET(iommu_sva_pasid);
+static DEFINE_IDA(iommu_global_pasid_ida);
 
-/**
- * iommu_sva_alloc_pasid - Allocate a PASID for the mm
- * @mm: the mm
- * @min: minimum PASID value (inclusive)
- * @max: maximum PASID value (inclusive)
- *
- * Try to allocate a PASID for this mm, or take a reference to the existing one
- * provided it fits within the [@min, @max] range. On success the PASID is
- * available in mm->pasid and will be available for the lifetime of the mm.
- *
- * Returns 0 on success and < 0 on error.
- */

drivers/iommu/iommu-sva.c line 19

Quoted 10 lines
drivers/iommu/iommu-sva.c +3 -4
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
+static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
  {
-	int ret = 0;
-	ioasid_t pasid;
+	int ret;
 
-	if (min == INVALID_IOASID || max == INVALID_IOASID ||
+	if (min == IOMMU_PASID_INVALID || max == IOMMU_PASID_INVALID ||
  	    min == 0 || max < min)

It's irrelevant to this patch. Just out of curiosity, why do we need to exclude PASID 0 here? I just had a quick look at PCI spec section 6.20. The spec does not state that PASID 0 is invalid.

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) · line 28

Quoted 8 lines
drivers/iommu/iommu-sva.c +2 -0
  		return -EINVAL;
 
@@ -37,39 +24,20 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
  	if (pasid_valid(mm->pasid)) {
  		if (mm->pasid < min || mm->pasid >= max)
  			ret = -EOVERFLOW;
+		else
+			ret = 0;

Nit:

If you didn't change "int ret = 0" to "int ret", we don't need above two lines. Did I miss anything?

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) · line 33

Quoted 10 lines
drivers/iommu/iommu-sva.c +2 -5
  		goto out;
  	}
 
-	pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm);
-	if (!pasid_valid(pasid))
-		ret = -ENOMEM;
-	else
-		mm->pasid = ret;
+	ret = ida_alloc_range(&iommu_global_pasid_ida, min, max, GFP_ATOMIC);
+	if (ret < min)

Nit:

	    ret < 0?

ida_alloc_range() returns negative error number on failure.

Quoted 18 lines
drivers/iommu/iommu-sva.c +3 -11
+		goto out;
+	mm->pasid = ret;
+	ret = 0;
  out:
  	mutex_unlock(&iommu_sva_lock);
  	return ret;
  }
-EXPORT_SYMBOL_GPL(iommu_sva_alloc_pasid);
-
-/* ioasid_find getter() requires a void * argument */
-static bool __mmget_not_zero(void *mm)
-{
-	return mmget_not_zero(mm);
-}
-
-/**
- * iommu_sva_find() - Find mm associated to the given PASID
- * @pasid: Process Address Space ID assigned to the mm

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) · line 72

Quoted 10 lines
drivers/iommu/iommu-sva.c +0 -10
- *
- * On success a reference to the mm is taken, and must be released with mmput().
- *
- * Returns the mm corresponding to this PASID, or an error if not found.
- */
-struct mm_struct *iommu_sva_find(ioasid_t pasid)
-{
-	return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero);
-}
-EXPORT_SYMBOL_GPL(iommu_sva_find);

Removing iommu_sva_find() has nothing to do with the intention of this patch. Perhaps make it in a separated patch?

drivers/iommu/iommu-sva.c iommu_sva_handle_iopf(struct iommu_fault *fault, void *data) · line 212

Quoted 12 lines
drivers/iommu/iommu-sva.c +1 -4
  /**
   * iommu_sva_bind_device() - Bind a process address space to a device
@@ -241,8 +209,8 @@ iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
 
  void mm_pasid_drop(struct mm_struct *mm)
  {
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
+	if (likely(!pasid_valid(mm->pasid)))

Why is this a likely?

Quoted 9 lines
drivers/iommu/iommu-sva.c +3 -0
+		return;
+
+	ida_free(&iommu_global_pasid_ida, mm->pasid);
  }
drivers/iommu/iommu-sva.h +0 -0
index 7215a761b962..c22d0174ad61 100644

drivers/iommu/iommu-sva.h line 13

Quoted 9 lines
drivers/iommu/iommu-sva.h +0 -3
@@ -8,9 +8,6 @@
  #include <linux/ioasid.h>
  #include <linux/mm_types.h>
 
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max);
-struct mm_struct *iommu_sva_find(ioasid_t pasid);
-
  /* I/O Page fault */
  struct device;
  struct iommu_fault;

Best regards,
baolu

author
#11 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Hi Baolu,

Quoted 54 lines · Baolu Lu
Quoted 53 lines · Jacob Pan

From: Jason Gunthorpe <[email protected]>

Instead SVA drivers can use a simple global IDA to allocate PASIDs for each mm_struct.

Future work would be to allow drivers using the SVA APIs to reserve global PASIDs from this IDA for their internal use, eg with the DMA API PASID support.

---
v4:
- Keep GFP_ATOMIC flag for PASID allocation, will changed to GFP_KERNEL in a separate patch.

---
  drivers/iommu/iommu-sva.c | 62 ++++++++++-----------------------------
  drivers/iommu/iommu-sva.h |  3 --
  2 files changed, 15 insertions(+), 50 deletions(-)
drivers/iommu/iommu-sva.c +1 -8
index 376b2a9e2543..297852ae5e7c 100644
@@ -9,26 +9,13 @@
  #include "iommu-sva.h"
 
  static DEFINE_MUTEX(iommu_sva_lock);
-static DECLARE_IOASID_SET(iommu_sva_pasid);
+static DEFINE_IDA(iommu_global_pasid_ida);
 
-/**
- * iommu_sva_alloc_pasid - Allocate a PASID for the mm
- * @mm: the mm
- * @min: minimum PASID value (inclusive)
- * @max: maximum PASID value (inclusive)
- *
- * Try to allocate a PASID for this mm, or take a reference to the

existing one
- * provided it fits within the [@min, @max] range. On success the PASID is
- * available in mm->pasid and will be available for the lifetime of the mm.
- *
- * Returns 0 on success and < 0 on error.
- */
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) +static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) {

drivers/iommu/iommu-sva.c +1 -2
-	int ret = 0;
-	ioasid_t pasid;
+	int ret;
Quoted 8 lines

drivers/iommu/iommu-sva.c line 19

Quoted 3 lines
drivers/iommu/iommu-sva.c +1 -1
-	if (min == INVALID_IOASID || max == INVALID_IOASID ||
+	if (min == IOMMU_PASID_INVALID || max == IOMMU_PASID_INVALID ||
  	    min == 0 || max < min)  

It's irrelevant to this patch. Just out of curiosity, why do we need to exclude PASID 0 here? I just had a quick look at PCI spec section 6.20. The spec does not state that PASID 0 is invalid.

my understanding is that ARM reserves PASID0, unlike VT-d where RID_PASID is programmable.

Quoted 11 lines

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, · line 28

Quoted 8 lines

return -EINVAL;

drivers/iommu/iommu-sva.c +2 -0
@@ -37,39 +24,20 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm,
 ioasid_t min, ioasid_t max) if (pasid_valid(mm->pasid)) {
  		if (mm->pasid < min || mm->pasid >= max)
  			ret = -EOVERFLOW;
+		else
+			ret = 0;  

Nit:

Quoted 3 lines

If you didn't change "int ret = 0" to "int ret", we don't need above two lines. Did I miss anything?

you are right

Quoted 13 lines

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) · line 33

Quoted 11 lines
drivers/iommu/iommu-sva.c +2 -5
  		goto out;
  	}
 
-	pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm);
-	if (!pasid_valid(pasid))
-		ret = -ENOMEM;
-	else
-		mm->pasid = ret;
+	ret = ida_alloc_range(&iommu_global_pasid_ida, min, max,
 GFP_ATOMIC);
+	if (ret < min)  

Nit:

Quoted 1 line
	    ret < 0?

will do

Quoted 27 lines

ida_alloc_range() returns negative error number on failure.

Quoted 25 lines
drivers/iommu/iommu-sva.c +3 -13
+		goto out;
+	mm->pasid = ret;
+	ret = 0;
  out:
  	mutex_unlock(&iommu_sva_lock);
  	return ret;
  }
-EXPORT_SYMBOL_GPL(iommu_sva_alloc_pasid);
-
-/* ioasid_find getter() requires a void * argument */
-static bool __mmget_not_zero(void *mm)
-{
-	return mmget_not_zero(mm);
-}
-
-/**
- * iommu_sva_find() - Find mm associated to the given PASID
- * @pasid: Process Address Space ID assigned to the mm
- *
- * On success a reference to the mm is taken, and must be released

with mmput().
- *
- * Returns the mm corresponding to this PASID, or an error if not found.
- */

Quoted 8 lines

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) · line 72

Quoted 5 lines
drivers/iommu/iommu-sva.c +0 -5
-struct mm_struct *iommu_sva_find(ioasid_t pasid)
-{
-	return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero);
-}
-EXPORT_SYMBOL_GPL(iommu_sva_find);  

Removing iommu_sva_find() has nothing to do with the intention of this patch. Perhaps make it in a separated patch?

will do

Quoted 14 lines

drivers/iommu/iommu-sva.c iommu_sva_handle_iopf(struct iommu_fault *fault, · line 212

Quoted 12 lines

/**
* iommu_sva_bind_device() - Bind a process address space to a device

drivers/iommu/iommu-sva.c +1 -4
@@ -241,8 +209,8 @@ iommu_sva_handle_iopf(struct iommu_fault *fault,
 void *data) 
  void mm_pasid_drop(struct mm_struct *mm)
  {
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
+	if (likely(!pasid_valid(mm->pasid)))  

Why is this a likely?

most mm does not have a PASID, thus initialized with invalid ioasid during fork. This function is called for every mm.

Quoted 12 lines
Quoted 12 lines
drivers/iommu/iommu-sva.c +3 -0
+		return;
+
+	ida_free(&iommu_global_pasid_ida, mm->pasid);
  }
drivers/iommu/iommu-sva.h +0 -0
index 7215a761b962..c22d0174ad61 100644
@@ -8,9 +8,6 @@
  #include <linux/ioasid.h>
  #include <linux/mm_types.h>
 
Quoted 9 lines

drivers/iommu/iommu-sva.h line 13

Quoted 6 lines
drivers/iommu/iommu-sva.h +0 -2
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t
 max); -struct mm_struct *iommu_sva_find(ioasid_t pasid);
-
  /* I/O Page fault */
  struct device;
  struct iommu_fault;  

Best regards,
baolu

Thanks,

Jacob

maintainer
#12 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Quoted 67 lines · Jacob Pan

Hi Baolu,

Quoted 54 lines · Baolu Lu
Quoted 53 lines · Jacob Pan

From: Jason Gunthorpe <[email protected]>

Instead SVA drivers can use a simple global IDA to allocate PASIDs for each mm_struct.

Future work would be to allow drivers using the SVA APIs to reserve global PASIDs from this IDA for their internal use, eg with the DMA API PASID support.

---
v4:
- Keep GFP_ATOMIC flag for PASID allocation, will changed to GFP_KERNEL in a separate patch.

---
   drivers/iommu/iommu-sva.c | 62 ++++++++++-----------------------------
   drivers/iommu/iommu-sva.h |  3 --
   2 files changed, 15 insertions(+), 50 deletions(-)
drivers/iommu/iommu-sva.c +1 -8
index 376b2a9e2543..297852ae5e7c 100644
@@ -9,26 +9,13 @@
   #include "iommu-sva.h"
 
   static DEFINE_MUTEX(iommu_sva_lock);
-static DECLARE_IOASID_SET(iommu_sva_pasid);
+static DEFINE_IDA(iommu_global_pasid_ida);
 
-/**
- * iommu_sva_alloc_pasid - Allocate a PASID for the mm
- * @mm: the mm
- * @min: minimum PASID value (inclusive)
- * @max: maximum PASID value (inclusive)
- *
- * Try to allocate a PASID for this mm, or take a reference to the

existing one
- * provided it fits within the [@min, @max] range. On success the PASID is
- * available in mm->pasid and will be available for the lifetime of the mm.
- *
- * Returns 0 on success and < 0 on error.
- */
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) +static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) {

drivers/iommu/iommu-sva.c +1 -2
-	int ret = 0;
-	ioasid_t pasid;
+	int ret;
Quoted 8 lines

drivers/iommu/iommu-sva.c line 19

Quoted 3 lines
drivers/iommu/iommu-sva.c +1 -1
-	if (min == INVALID_IOASID || max == INVALID_IOASID ||
+	if (min == IOMMU_PASID_INVALID || max == IOMMU_PASID_INVALID ||
   	    min == 0 || max < min)

It's irrelevant to this patch. Just out of curiosity, why do we need to exclude PASID 0 here? I just had a quick look at PCI spec section 6.20. The spec does not state that PASID 0 is invalid.

Quoted 2 lines

my understanding is that ARM reserves PASID0, unlike VT-d where RID_PASID is programmable.

I suppose the common thing is reserving some kind of special PASIDs.

Quoted 84 lines
Quoted 12 lines

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, · line 28

Quoted 9 lines
   		return -EINVAL;
drivers/iommu/iommu-sva.c +2 -0
@@ -37,39 +24,20 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm,
 ioasid_t min, ioasid_t max) if (pasid_valid(mm->pasid)) {
   		if (mm->pasid < min || mm->pasid >= max)
   			ret = -EOVERFLOW;
+		else
+			ret = 0;

Nit:

Quoted 3 lines

If you didn't change "int ret = 0" to "int ret", we don't need above two lines. Did I miss anything?

you are right

Quoted 13 lines

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) · line 33

Quoted 11 lines
drivers/iommu/iommu-sva.c +2 -5
   		goto out;
   	}
 
-	pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm);
-	if (!pasid_valid(pasid))
-		ret = -ENOMEM;
-	else
-		mm->pasid = ret;
+	ret = ida_alloc_range(&iommu_global_pasid_ida, min, max,
 GFP_ATOMIC);
+	if (ret < min)

Nit:

Quoted 1 line
	    ret < 0?

will do

Quoted 27 lines

ida_alloc_range() returns negative error number on failure.

Quoted 25 lines
drivers/iommu/iommu-sva.c +3 -13
+		goto out;
+	mm->pasid = ret;
+	ret = 0;
   out:
   	mutex_unlock(&iommu_sva_lock);
   	return ret;
   }
-EXPORT_SYMBOL_GPL(iommu_sva_alloc_pasid);
-
-/* ioasid_find getter() requires a void * argument */
-static bool __mmget_not_zero(void *mm)
-{
-	return mmget_not_zero(mm);
-}
-
-/**
- * iommu_sva_find() - Find mm associated to the given PASID
- * @pasid: Process Address Space ID assigned to the mm
- *
- * On success a reference to the mm is taken, and must be released

with mmput().
- *
- * Returns the mm corresponding to this PASID, or an error if not found.
- */

Quoted 8 lines

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) · line 72

Quoted 5 lines
drivers/iommu/iommu-sva.c +0 -5
-struct mm_struct *iommu_sva_find(ioasid_t pasid)
-{
-	return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero);
-}
-EXPORT_SYMBOL_GPL(iommu_sva_find);

Removing iommu_sva_find() has nothing to do with the intention of this patch. Perhaps make it in a separated patch?

will do

Quoted 14 lines

drivers/iommu/iommu-sva.c iommu_sva_handle_iopf(struct iommu_fault *fault, · line 212

Quoted 12 lines

/**
* iommu_sva_bind_device() - Bind a process address space to a device

drivers/iommu/iommu-sva.c +1 -4
@@ -241,8 +209,8 @@ iommu_sva_handle_iopf(struct iommu_fault *fault,
 void *data)
   void mm_pasid_drop(struct mm_struct *mm)
   {
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
+	if (likely(!pasid_valid(mm->pasid)))

Why is this a likely?

Quoted 2 lines

most mm does not have a PASID, thus initialized with invalid ioasid during fork. This function is called for every mm.

Make sense.

Quoted 24 lines
Quoted 13 lines
Quoted 13 lines
drivers/iommu/iommu-sva.c +3 -0
+		return;
+
+	ida_free(&iommu_global_pasid_ida, mm->pasid);
   }
drivers/iommu/iommu-sva.h +0 -0
index 7215a761b962..c22d0174ad61 100644
@@ -8,9 +8,6 @@
   #include <linux/ioasid.h>
   #include <linux/mm_types.h>
 
Quoted 9 lines

drivers/iommu/iommu-sva.h line 13

Quoted 6 lines
drivers/iommu/iommu-sva.h +0 -2
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t
 max); -struct mm_struct *iommu_sva_find(ioasid_t pasid);
-
   /* I/O Page fault */
   struct device;
   struct iommu_fault;

Best regards,
baolu

Quoted 3 lines

Thanks,

Jacob

Best regards,
baolu

#13 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Quoted 14 lines · Baolu Lu
Quoted 14 lines
Quoted 14 lines

drivers/iommu/iommu-sva.c line 19

Quoted 9 lines

-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) +static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) {

drivers/iommu/iommu-sva.c +2 -3
-	int ret = 0;
-	ioasid_t pasid;
+	int ret;
-	if (min == INVALID_IOASID || max == INVALID_IOASID ||
+	if (min == IOMMU_PASID_INVALID || max == IOMMU_PASID_INVALID ||
   	    min == 0 || max < min)

It's irrelevant to this patch. Just out of curiosity, why do we need to exclude PASID 0 here? I just had a quick look at PCI spec section 6.20. The spec does not state that PASID 0 is invalid.

Quoted 2 lines
Quoted 2 lines

my understanding is that ARM reserves PASID0, unlike VT-d where RID_PASID is programmable.

It does, but that's specific to the IOMMU driver so we shouldn't check it here.

Quoted 1 line

I suppose the common thing is reserving some kind of special PASIDs.

Are you planning to use RID_PASID != 0 in VT-d? Otherwise we could just communicate min_pasid from the IOMMU driver the same way we do max_pasid.

Otherwise I guess re-introduce a lighter ioasid_alloc() that the IOMMU driver calls to reserve PASID0/RID_PASID.

Thanks,
Jean

maintainer
#14 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Quoted 6 lines · Jean-Philippe Brucker
Quoted 1 line

I suppose the common thing is reserving some kind of special PASIDs.

Are you planning to use RID_PASID != 0 in VT-d? Otherwise we could just communicate min_pasid from the IOMMU driver the same way we do max_pasid.

Otherwise I guess re-introduce a lighter ioasid_alloc() that the IOMMU driver calls to reserve PASID0/RID_PASID.

Yes. We probably will use a non-zero RID_PASID in the future. An interface to reserve (or allocate) a PASID from iommu_global_pasid_ida should work then.

Best regards,
baolu

#15 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Quoted 11 lines · Baolu Lu
Quoted 6 lines · Jean-Philippe Brucker
Quoted 1 line

I suppose the common thing is reserving some kind of special PASIDs.

Are you planning to use RID_PASID != 0 in VT-d? Otherwise we could just communicate min_pasid from the IOMMU driver the same way we do max_pasid.

Otherwise I guess re-introduce a lighter ioasid_alloc() that the IOMMU driver calls to reserve PASID0/RID_PASID.

Yes. We probably will use a non-zero RID_PASID in the future. An interface to reserve (or allocate) a PASID from iommu_global_pasid_ida should work then.

Just allowing the driver to store XA_ZERO_ENTRY would be fine

Jason

author
#16 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Hi Jason,

Quoted 8 lines · Jason Gunthorpe
Quoted 7 lines · Baolu Lu
Quoted 6 lines · Jean-Philippe Brucker
Quoted 2 lines

I suppose the common thing is reserving some kind of special PASIDs.

Are you planning to use RID_PASID != 0 in VT-d? Otherwise we could just communicate min_pasid from the IOMMU driver the same way we do max_pasid.

Quoted 9 lines
Quoted 6 lines
Quoted 2 lines

Otherwise I guess re-introduce a lighter ioasid_alloc() that the IOMMU driver calls to reserve PASID0/RID_PASID.

Yes. We probably will use a non-zero RID_PASID in the future. An interface to reserve (or allocate) a PASID from iommu_global_pasid_ida should work then.

Just allowing the driver to store XA_ZERO_ENTRY would be fine

So we provide APIs for both?
1. alloc a global PASID, returned by this API
2. try to reserve a global PASID given by the driver, i.e.

	xa_cmpxchg(&iommu_global_pasid_ida.xa, 2, NULL, XA_ZERO_ENTRY,
			 GFP_KERNEL);

seems #1 is sufficient.

Thanks,

Jacob

#17 lore raw

RE: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Quoted 26 lines

From: Jacob Pan <[email protected]>
Sent: Wednesday, March 8, 2023 1:42 AM

Hi Jason,

Quoted 6 lines · Jason Gunthorpe
Quoted 5 lines · Baolu Lu
Quoted 4 lines · Jean-Philippe Brucker
Quoted 2 lines

I suppose the common thing is reserving some kind of special PASIDs.

Are you planning to use RID_PASID != 0 in VT-d? Otherwise we could just communicate min_pasid from the IOMMU driver the same way we

do

Quoted 11 lines
Quoted 8 lines
Quoted 4 lines

max_pasid.

Otherwise I guess re-introduce a lighter ioasid_alloc() that the IOMMU driver calls to reserve PASID0/RID_PASID.

Yes. We probably will use a non-zero RID_PASID in the future. An interface to reserve (or allocate) a PASID from iommu_global_pasid_ida should work then.

Just allowing the driver to store XA_ZERO_ENTRY would be fine

Quoted 8 lines

So we provide APIs for both?
1. alloc a global PASID, returned by this API
2. try to reserve a global PASID given by the driver, i.e. xa_cmpxchg(&iommu_global_pasid_ida.xa, 2, NULL,
XA_ZERO_ENTRY,

			 GFP_KERNEL);

seems #1 is sufficient.

No need for both. There will be on-demand allocation on this global space so a reservation interface doesn't make sense.

author
#18 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

In reply to Jason Gunthorpe

Hi Jason,

Quoted 8 lines · Jason Gunthorpe
Quoted 7 lines · Baolu Lu
Quoted 6 lines · Jean-Philippe Brucker
Quoted 2 lines

I suppose the common thing is reserving some kind of special PASIDs.

Are you planning to use RID_PASID != 0 in VT-d? Otherwise we could just communicate min_pasid from the IOMMU driver the same way we do max_pasid.

Quoted 9 lines
Quoted 6 lines
Quoted 2 lines

Otherwise I guess re-introduce a lighter ioasid_alloc() that the IOMMU driver calls to reserve PASID0/RID_PASID.

Yes. We probably will use a non-zero RID_PASID in the future. An interface to reserve (or allocate) a PASID from iommu_global_pasid_ida should work then.

Just allowing the driver to store XA_ZERO_ENTRY would be fine

It looks like there are incoming users of iommu_sva_find()
https://lore.kernel.org/lkml/[email protected]/T/#m1fc97725a0e56ea269c8bdabacee447070d51846
Should we keep the xa here instead of the global ida?

Thanks,

Jacob

#19 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Quoted 19 lines · Jacob Pan

Hi Jason,

Quoted 15 lines · Jason Gunthorpe
Quoted 13 lines · Baolu Lu
Quoted 8 lines · Jean-Philippe Brucker
Quoted 2 lines

I suppose the common thing is reserving some kind of special PASIDs.

Are you planning to use RID_PASID != 0 in VT-d? Otherwise we could just communicate min_pasid from the IOMMU driver the same way we do max_pasid.

Otherwise I guess re-introduce a lighter ioasid_alloc() that the IOMMU driver calls to reserve PASID0/RID_PASID.

Yes. We probably will use a non-zero RID_PASID in the future. An interface to reserve (or allocate) a PASID from iommu_global_pasid_ida should work then.

Quoted 5 lines
Quoted 2 lines

Just allowing the driver to store XA_ZERO_ENTRY would be fine

It looks like there are incoming users of iommu_sva_find()
https://lore.kernel.org/lkml/[email protected]/T/#m1fc97725a0e56ea269c8bdabacee447070d51846
Should we keep the xa here instead of the global ida?

I'm not sure this should be in the iommu core, it is really gross.

I would expect IDXD to keep track of the PASID's and mms it is using and do this kind of stuff itself.

And why is this using access_remote_vm anyhow? If you know you are in a kthread then kthread_use_mm() is probably better anyhow.

In any event we don't need a iommu_sva_find() function to wrapper xa_load for another function inside the same .c file.

Jason

#20 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Hi, Jason and Jacob,

Quoted 28 lines · Jason Gunthorpe
Quoted 19 lines · Jacob Pan

Hi Jason,

Quoted 15 lines · Jason Gunthorpe
Quoted 13 lines · Baolu Lu
Quoted 8 lines · Jean-Philippe Brucker
Quoted 2 lines

I suppose the common thing is reserving some kind of special PASIDs.

Are you planning to use RID_PASID != 0 in VT-d? Otherwise we could just communicate min_pasid from the IOMMU driver the same way we do max_pasid.

Otherwise I guess re-introduce a lighter ioasid_alloc() that the IOMMU driver calls to reserve PASID0/RID_PASID.

Yes. We probably will use a non-zero RID_PASID in the future. An interface to reserve (or allocate) a PASID from iommu_global_pasid_ida should work then.

Quoted 5 lines
Quoted 2 lines

Just allowing the driver to store XA_ZERO_ENTRY would be fine

It looks like there are incoming users of iommu_sva_find()
https://lore.kernel.org/lkml/[email protected]/T/#m1fc97725a0e56ea269c8bdabacee447070d51846
Should we keep the xa here instead of the global ida?

I'm not sure this should be in the iommu core, it is really gross.

Quoted 8 lines

I would expect IDXD to keep track of the PASID's and mms it is using and do this kind of stuff itself.

And why is this using access_remote_vm anyhow? If you know you are in a kthread then kthread_use_mm() is probably better anyhow.

In any event we don't need a iommu_sva_find() function to wrapper xa_load for another function inside the same .c file.

Ok. I will maintain mm and find mm from PASID inside IDXD driver. And will implement accessing the remote mm inside IDXD driver although the implementation will have duplicate code as access_remote_vm().

Thanks.

-Fenghua

#21 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Quoted 3 lines · Fenghua Yu

Ok. I will maintain mm and find mm from PASID inside IDXD driver. And will implement accessing the remote mm inside IDXD driver although the implementation will have duplicate code as access_remote_vm().

If you really need it and it really makes sense, then export it

Jason

#22 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

In reply to Jacob Pan

Hi,

Quoted 56 lines · Jacob Pan

From: Jason Gunthorpe <[email protected]>

Instead SVA drivers can use a simple global IDA to allocate PASIDs for each mm_struct.

Future work would be to allow drivers using the SVA APIs to reserve global PASIDs from this IDA for their internal use, eg with the DMA API PASID support.

---
v4:
- Keep GFP_ATOMIC flag for PASID allocation, will changed to GFP_KERNEL in a separate patch.

---
  drivers/iommu/iommu-sva.c | 62 ++++++++++-----------------------------
  drivers/iommu/iommu-sva.h |  3 --
  2 files changed, 15 insertions(+), 50 deletions(-)
drivers/iommu/iommu-sva.c +4 -17
index 376b2a9e2543..297852ae5e7c 100644
@@ -9,26 +9,13 @@
  #include "iommu-sva.h"
 
  static DEFINE_MUTEX(iommu_sva_lock);
-static DECLARE_IOASID_SET(iommu_sva_pasid);
+static DEFINE_IDA(iommu_global_pasid_ida);
 
-/**
- * iommu_sva_alloc_pasid - Allocate a PASID for the mm
- * @mm: the mm
- * @min: minimum PASID value (inclusive)
- * @max: maximum PASID value (inclusive)
- *
- * Try to allocate a PASID for this mm, or take a reference to the existing one
- * provided it fits within the [@min, @max] range. On success the PASID is
- * available in mm->pasid and will be available for the lifetime of the mm.
- *
- * Returns 0 on success and < 0 on error.
- */
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
+static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
  {
-	int ret = 0;
-	ioasid_t pasid;
+	int ret;
 
-	if (min == INVALID_IOASID || max == INVALID_IOASID ||
+	if (min == IOMMU_PASID_INVALID || max == IOMMU_PASID_INVALID ||
  	    min == 0 || max < min)
  		return -EINVAL;
 

drivers/iommu/iommu-sva.c int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max) · line 25

Quoted 2 lines
drivers/iommu/iommu-sva.c +0 -0
@@ -37,39 +24,20 @@ int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
  	if (pasid_valid(mm->pasid)) {
  		if (mm->pasid < min || mm->pasid >= max)

Here seems not right, since the valid range is defined [min, max]. Shouldn't the invalid range be:

		if (mm->pasid < min || mm->pasid > max)

Regards,
-Tina

Quoted 71 lines
drivers/iommu/iommu-sva.c +11 -30
  			ret = -EOVERFLOW;
+		else
+			ret = 0;
  		goto out;
  	}
 
-	pasid = ioasid_alloc(&iommu_sva_pasid, min, max, mm);
-	if (!pasid_valid(pasid))
-		ret = -ENOMEM;
-	else
-		mm->pasid = ret;
+	ret = ida_alloc_range(&iommu_global_pasid_ida, min, max, GFP_ATOMIC);
+	if (ret < min)
+		goto out;
+	mm->pasid = ret;
+	ret = 0;
  out:
  	mutex_unlock(&iommu_sva_lock);
  	return ret;
  }
-EXPORT_SYMBOL_GPL(iommu_sva_alloc_pasid);
-
-/* ioasid_find getter() requires a void * argument */
-static bool __mmget_not_zero(void *mm)
-{
-	return mmget_not_zero(mm);
-}
-
-/**
- * iommu_sva_find() - Find mm associated to the given PASID
- * @pasid: Process Address Space ID assigned to the mm
- *
- * On success a reference to the mm is taken, and must be released with mmput().
- *
- * Returns the mm corresponding to this PASID, or an error if not found.
- */
-struct mm_struct *iommu_sva_find(ioasid_t pasid)
-{
-	return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero);
-}
-EXPORT_SYMBOL_GPL(iommu_sva_find);
 
  /**
   * iommu_sva_bind_device() - Bind a process address space to a device
@@ -241,8 +209,8 @@ iommu_sva_handle_iopf(struct iommu_fault *fault, void *data)
 
  void mm_pasid_drop(struct mm_struct *mm)
  {
-	if (pasid_valid(mm->pasid)) {
-		ioasid_free(mm->pasid);
-		mm->pasid = INVALID_IOASID;
-	}
+	if (likely(!pasid_valid(mm->pasid)))
+		return;
+
+	ida_free(&iommu_global_pasid_ida, mm->pasid);
  }
drivers/iommu/iommu-sva.h +0 -3
index 7215a761b962..c22d0174ad61 100644
@@ -8,9 +8,6 @@
  #include <linux/ioasid.h>
  #include <linux/mm_types.h>
 
-int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max);
-struct mm_struct *iommu_sva_find(ioasid_t pasid);
-
  /* I/O Page fault */
  struct device;
  struct iommu_fault;
author
#23 lore raw

Re: [PATCH v4 3/6] iommu/sva: Stop using ioasid_set for SVA

Hi Tina,

Quoted 4 lines · Tina Zhang
Quoted 1 line
  		if (mm->pasid < min || mm->pasid >= max)  

Here seems not right, since the valid range is defined [min, max]. Shouldn't the invalid range be:

		if (mm->pasid < min || mm->pasid > max)

yes it is better to be consistent even if we removed the inclusive requirements in the previous comments.

Thanks,

Jacob

author
#24 lore raw

PATCH v4 4/6 iommu/sva: Use GFP_KERNEL for pasid allocation

In reply to Jacob Pan

We’re not using spinlock-protected IOASID allocation anymore, there’s no need for GFP_ATOMIC.

---
 drivers/iommu/iommu-sva.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/iommu-sva.c +1 -1
index 297852ae5e7c..8c92a145e15d 100644
@@ -29,7 +29,7 @@ static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t ma
 		goto out;
 	}
 
-	ret = ida_alloc_range(&iommu_global_pasid_ida, min, max, GFP_ATOMIC);
+	ret = ida_alloc_range(&iommu_global_pasid_ida, min, max, GFP_KERNEL);
 	if (ret < min)
 		goto out;
 	mm->pasid = ret;
Signature · 1 line
2.25.1
author
#27 lore raw

PATCH v4 5/6 iommu/ioasid: Rename INVALID_IOASID

In reply to Jacob Pan

INVALID_IOASID and IOMMU_PASID_INVALID are duplicated. Rename INVALID_IOASID and consolidate since we are moving away from IOASID infrastructure.

---
 Documentation/x86/sva.rst    | 2 +-
 arch/x86/kernel/traps.c      | 5 ++++-
 drivers/dma/idxd/device.c    | 8 ++++----
 drivers/dma/idxd/idxd.h      | 1 +
 drivers/dma/idxd/init.c      | 2 +-
 drivers/dma/idxd/irq.c       | 2 +-
 drivers/iommu/intel/dmar.c   | 4 ++--
 drivers/iommu/intel/iommu.c  | 2 +-
 drivers/iommu/intel/svm.c    | 2 +-
 include/linux/ioasid.h       | 5 +----
 include/linux/iommu-helper.h | 2 +-
 include/linux/iommu.h        | 7 +++++--
 include/linux/sched/mm.h     | 2 +-
 mm/init-mm.c                 | 4 ++--
 14 files changed, 26 insertions(+), 22 deletions(-)
Documentation/x86/sva.rst +1 -1
index 2e9b8b0f9a0f..33cb05005982 100644
@@ -107,7 +107,7 @@ process share the same page tables, thus the same MSR value.
 PASID Life Cycle Management
 ===========================
 
-PASID is initialized as INVALID_IOASID (-1) when a process is created.
+PASID is initialized as IOMMU_PASID_INVALID (-1) when a process is created.
 
 Only processes that access SVA-capable devices need to have a PASID
 allocated. This allocation happens when a process opens/binds an SVA-capable
arch/x86/kernel/traps.c +4 -1
index d317dc3d06a3..d6fb03ebf548 100644
@@ -40,7 +40,10 @@
 #include <linux/io.h>
 #include <linux/hardirq.h>
 #include <linux/atomic.h>
-#include <linux/ioasid.h>
+
+#ifdef CONFIG_IOMMU_SVA
+#include <linux/iommu.h>
+#endif
 
 #include <asm/stacktrace.h>
 #include <asm/processor.h>
drivers/dma/idxd/device.c +4 -4
index 29dbb0f52e18..125652a8bb29 100644
@@ -1194,7 +1194,7 @@ static void idxd_device_set_perm_entry(struct idxd_device *idxd,
 {
 	union msix_perm mperm;
 
-	if (ie->pasid == INVALID_IOASID)
+	if (ie->pasid == IOMMU_PASID_INVALID)
 		return;
 
 	mperm.bits = 0;
@@ -1224,7 +1224,7 @@ void idxd_wq_free_irq(struct idxd_wq *wq)
 	idxd_device_clear_perm_entry(idxd, ie);
 	ie->vector = -1;
 	ie->int_handle = INVALID_INT_HANDLE;
-	ie->pasid = INVALID_IOASID;
+	ie->pasid = IOMMU_PASID_INVALID;
 }
 
 int idxd_wq_request_irq(struct idxd_wq *wq)
@@ -1240,7 +1240,7 @@ int idxd_wq_request_irq(struct idxd_wq *wq)
 
 	ie = &wq->ie;
 	ie->vector = pci_irq_vector(pdev, ie->id);
-	ie->pasid = device_pasid_enabled(idxd) ? idxd->pasid : INVALID_IOASID;
+	ie->pasid = device_pasid_enabled(idxd) ? idxd->pasid : IOMMU_PASID_INVALID;
 	idxd_device_set_perm_entry(idxd, ie);
 
 	rc = request_threaded_irq(ie->vector, NULL, idxd_wq_thread, 0, "idxd-portal", ie);
@@ -1265,7 +1265,7 @@ int idxd_wq_request_irq(struct idxd_wq *wq)
 	free_irq(ie->vector, ie);
 err_irq:
 	idxd_device_clear_perm_entry(idxd, ie);
-	ie->pasid = INVALID_IOASID;
+	ie->pasid = IOMMU_PASID_INVALID;
 	return rc;
 }
 
drivers/dma/idxd/idxd.h +1 -0
index 7ced8d283d98..417e602a46b6 100644
@@ -13,6 +13,7 @@
 #include <linux/ioasid.h>
 #include <linux/bitmap.h>
 #include <linux/perf_event.h>
+#include <linux/iommu.h>
 #include <uapi/linux/idxd.h>
 #include "registers.h"
 
drivers/dma/idxd/init.c +1 -1
index 529ea09c9094..f30eef701970 100644
@@ -105,7 +105,7 @@ static int idxd_setup_interrupts(struct idxd_device *idxd)
 		ie = idxd_get_ie(idxd, msix_idx);
 		ie->id = msix_idx;
 		ie->int_handle = INVALID_INT_HANDLE;
-		ie->pasid = INVALID_IOASID;
+		ie->pasid = IOMMU_PASID_INVALID;
 
 		spin_lock_init(&ie->list_lock);
 		init_llist_head(&ie->pending_llist);
drivers/dma/idxd/irq.c +1 -1
index aa314ebec587..242f1f0b9f09 100644
@@ -80,7 +80,7 @@ static void idxd_int_handle_revoke_drain(struct idxd_irq_entry *ie)
 	desc.opcode = DSA_OPCODE_DRAIN;
 	desc.priv = 1;
 
-	if (ie->pasid != INVALID_IOASID)
+	if (ie->pasid != IOMMU_PASID_INVALID)
 		desc.pasid = ie->pasid;
 	desc.int_handle = ie->int_handle;
 	portal = idxd_wq_portal_addr(wq);
drivers/iommu/intel/dmar.c +2 -2
index bf0bfe5ba7a7..c567f94b66c7 100644
@@ -1933,7 +1933,7 @@ static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
 		return 0;
 	}
 
-	if (pasid == INVALID_IOASID)
+	if (pasid == IOMMU_PASID_INVALID)
 		pr_err("[%s NO_PASID] Request device [%02x:%02x.%d] fault addr 0x%llx [fault reason 0x%02x] %s\n",
 		       type ? "DMA Read" : "DMA Write",
 		       source_id >> 8, PCI_SLOT(source_id & 0xFF),
@@ -2014,7 +2014,7 @@ irqreturn_t dmar_fault(int irq, void *dev_id)
 		if (!ratelimited)
 			/* Using pasid -1 if pasid is not present */
 			dmar_fault_do_one(iommu, type, fault_reason,
-					  pasid_present ? pasid : INVALID_IOASID,
+					  pasid_present ? pasid : IOMMU_PASID_INVALID,
 					  source_id, guest_addr);
 
 		fault_index++;
drivers/iommu/intel/iommu.c +1 -1
index a295e80fdfe8..10f657828d3a 100644
@@ -876,7 +876,7 @@ void dmar_fault_dump_ptes(struct intel_iommu *iommu, u16 source_id,
 		return;
 	}
 	/* For request-without-pasid, get the pasid from context entry */
-	if (intel_iommu_sm && pasid == INVALID_IOASID)
+	if (intel_iommu_sm && pasid == IOMMU_PASID_INVALID)
 		pasid = PASID_RID2PASID;
 
 	dir_index = pasid >> PASID_PDE_SHIFT;
drivers/iommu/intel/svm.c +1 -1
index c76b66263467..be98af2fce06 100644
@@ -274,7 +274,7 @@ static int pasid_to_svm_sdev(struct device *dev, unsigned int pasid,
 	if (WARN_ON(!mutex_is_locked(&pasid_mutex)))
 		return -EINVAL;
 
-	if (pasid == INVALID_IOASID || pasid >= PASID_MAX)
+	if (pasid == IOMMU_PASID_INVALID || pasid >= PASID_MAX)
 		return -EINVAL;
 
 	svm = pasid_private_find(pasid);
include/linux/ioasid.h +1 -4
index 2c502e77ee78..665a3773a409 100644
@@ -7,6 +7,7 @@
 #include <linux/iommu-helper.h>
 
 typedef unsigned int ioasid_t;
+#define INVALID_IOASID ((ioasid_t)-1)
 typedef ioasid_t (*ioasid_alloc_fn_t)(ioasid_t min, ioasid_t max, void *data);
 typedef void (*ioasid_free_fn_t)(ioasid_t ioasid, void *data);
 
@@ -40,10 +41,6 @@ void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid,
 int ioasid_register_allocator(struct ioasid_allocator_ops *allocator);
 void ioasid_unregister_allocator(struct ioasid_allocator_ops *allocator);
 int ioasid_set_data(ioasid_t ioasid, void *data);
-static inline bool pasid_valid(ioasid_t ioasid)
-{
-	return ioasid != INVALID_IOASID;
-}
 
 #else /* !CONFIG_IOASID */
 static inline ioasid_t ioasid_alloc(struct ioasid_set *set, ioasid_t min,
include/linux/iommu-helper.h +1 -1
index 0aa922f6bfad..0147430d8a6c 100644
@@ -40,6 +40,6 @@ static inline unsigned long iommu_num_pages(unsigned long addr,
 
 	return DIV_ROUND_UP(size, io_page_size);
 }
-#define INVALID_IOASID	(-1U)
+#define IOMMU_PASID_INVALID	(-1U)
 
 #endif
include/linux/iommu.h +5 -2
index 46e1347bfa22..a694f6256d2d 100644
@@ -192,8 +192,11 @@ enum iommu_dev_features {
 	IOMMU_DEV_FEAT_IOPF,
 };
 
-#define IOMMU_PASID_INVALID	(-1U)
-
+typedef unsigned int ioasid_t;
+static inline bool pasid_valid(ioasid_t ioasid)
+{
+	return ioasid != IOMMU_PASID_INVALID;
+}
 #ifdef CONFIG_IOMMU_API
 
 /**
include/linux/sched/mm.h +1 -1
index ae5a3f16b321..0e0aa285a12f 100644
@@ -454,7 +454,7 @@ static inline void membarrier_update_current_mm(struct mm_struct *next_mm)
 #ifdef CONFIG_IOMMU_SVA
 static inline void mm_pasid_init(struct mm_struct *mm)
 {
-	mm->pasid = INVALID_IOASID;
+	mm->pasid = IOMMU_PASID_INVALID;
 }
 void mm_pasid_drop(struct mm_struct *mm);
 #else
mm/init-mm.c +2 -2
index c9327abb771c..a084039f55d8 100644
@@ -10,7 +10,7 @@
 
 #include <linux/atomic.h>
 #include <linux/user_namespace.h>
-#include <linux/ioasid.h>
+#include <linux/iommu.h>
 #include <asm/mmu.h>
 
 #ifndef INIT_MM_CONTEXT
@@ -40,7 +40,7 @@ struct mm_struct init_mm = {
 	.user_ns	= &init_user_ns,
 	.cpu_bitmap	= CPU_BITS_NONE,
 #ifdef CONFIG_IOMMU_SVA
-	.pasid		= INVALID_IOASID,
+	.pasid		= IOMMU_PASID_INVALID,
 #endif
 	INIT_MM_CONTEXT(init_mm)
 };
Signature · 1 line
2.25.1
Reviewed-by on v4 5/6
#29 lore raw

RE: [PATCH v4 5/6] iommu/ioasid: Rename INVALID_IOASID

In reply to Jacob Pan

drivers/dma/idxd/device.c static void idxd_device_set_perm_entry(struct · line 1200

Quoted 12 lines

From: Jacob Pan <[email protected]>
Sent: Thursday, March 2, 2023 7:57 AM

drivers/dma/idxd/device.c +1 -1
@@ -1194,7 +1194,7 @@ static void idxd_device_set_perm_entry(struct
 idxd_device *idxd,
 {
 	union msix_perm mperm;
 
-	if (ie->pasid == INVALID_IOASID)
+	if (ie->pasid == IOMMU_PASID_INVALID)
 		return;
 

if (!pasid_valid(ie->pasid))

same for other occurrences in this patch.

maintainer Reviewed-by on v4 5/6
#30 lore raw

Re: [PATCH v4 5/6] iommu/ioasid: Rename INVALID_IOASID

In reply to Jacob Pan
Quoted 7 lines · Jacob Pan

INVALID_IOASID and IOMMU_PASID_INVALID are duplicated. Rename INVALID_IOASID and consolidate since we are moving away from IOASID infrastructure.

Best regards,
baolu

author
#31 lore raw

PATCH v4 6/6 iommu: Remove ioasid infrastructure

In reply to Jacob Pan

From: Jason Gunthorpe <[email protected]>

This has no use anymore, delete it all.

---
v3: - put rename under a different patch
- delete makefile and Kconfig
v2:
- fix compile issue w/o CONFIG_IOMMU_SVA
- consolidate INVALID_IOASID w/ IOMMU_PASID_INVALID

---
 drivers/dma/idxd/idxd.h     |   1 -
 drivers/iommu/Kconfig       |   5 -
 drivers/iommu/Makefile      |   1 -
 drivers/iommu/intel/iommu.h |   1 -
 drivers/iommu/intel/svm.c   |   1 -
 drivers/iommu/ioasid.c      | 422 ------------------------------------
 drivers/iommu/iommu-sva.h   |   1 -
 include/linux/ioasid.h      |  80 -------
 include/linux/iommu.h       |   1 -
 9 files changed, 513 deletions(-)
 delete mode 100644 drivers/iommu/ioasid.c
 delete mode 100644 include/linux/ioasid.h
drivers/dma/idxd/idxd.h +0 -1
index 417e602a46b6..dd2a6ed8949b 100644
@@ -10,7 +10,6 @@
 #include <linux/cdev.h>
 #include <linux/idr.h>
 #include <linux/pci.h>
-#include <linux/ioasid.h>
 #include <linux/bitmap.h>
 #include <linux/perf_event.h>
 #include <linux/iommu.h>
drivers/iommu/Kconfig +0 -5
index 79707685d54a..06ddbbfec3bf 100644
@@ -3,10 +3,6 @@
 config IOMMU_IOVA
 	tristate
 
-# The IOASID library may also be used by non-IOMMU_API users
-config IOASID
-	tristate
-
 # IOMMU_API always gets selected by whoever wants it.
 config IOMMU_API
 	bool
@@ -158,7 +154,6 @@ config IOMMU_DMA
 # Shared Virtual Addressing
 config IOMMU_SVA
 	bool
-	select IOASID
 
 config FSL_PAMU
 	bool "Freescale IOMMU support"
drivers/iommu/Makefile +0 -1
index f461d0651385..769e43d780ce 100644
@@ -9,7 +9,6 @@ obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_DART) += io-pgtable-dart.o
-obj-$(CONFIG_IOASID) += ioasid.o
 obj-$(CONFIG_IOMMU_IOVA) += iova.o
 obj-$(CONFIG_OF_IOMMU)	+= of_iommu.o
 obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
drivers/iommu/intel/iommu.h +0 -1
index 6bdfbead82c4..80582e497782 100644
@@ -19,7 +19,6 @@
 #include <linux/iommu.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/dmar.h>
-#include <linux/ioasid.h>
 #include <linux/bitfield.h>
 #include <linux/xarray.h>
 
drivers/iommu/intel/svm.c +0 -1
index be98af2fce06..b4efc541f6b3 100644
@@ -17,7 +17,6 @@
 #include <linux/interrupt.h>
 #include <linux/mm_types.h>
 #include <linux/xarray.h>
-#include <linux/ioasid.h>
 #include <asm/page.h>
 #include <asm/fpu/api.h>
 
drivers/iommu/ioasid.c deleted +0 -422

422 lines not on this page · raw message

drivers/iommu/iommu-sva.h +0 -1
index c22d0174ad61..54946b5a7caf 100644
@@ -5,7 +5,6 @@
 #ifndef _IOMMU_SVA_H
 #define _IOMMU_SVA_H
 
-#include <linux/ioasid.h>
 #include <linux/mm_types.h>
 
 /* I/O Page fault */
include/linux/ioasid.h deleted +0 -80

80 lines not on this page · raw message

include/linux/iommu.h +0 -1
index a694f6256d2d..39a97bd8f04a 100644
@@ -13,7 +13,6 @@
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/of.h>
-#include <linux/ioasid.h>
 #include <uapi/linux/iommu.h>
 
 #define IOMMU_READ	(1 << 0)
Signature · 1 line
2.25.1