[PATCH v3 00/12] iommu/amd: SVA Support (Part 4) - SVA and IOPF

[PATCH v3 00/12] iommu/amd: SVA Support (Part 4) - SVA and IOPF

[Vasant Hegde]

This is part 4 of the 4-part series to introduce Share Virtual Address
(SVA) support for devices, which can support PCI ATS, PASID and PRI
capabilities. These devices are referred to as SVA-capable devices in
this series.

It contains the following enhancements:

* Patch 1 - 4:
  Rename, add support to enable/disable features, update DTE etc.

* Patch 5 - 6:
  Introduce SVA support

* Patch 7 - 12:
  Add IOPF support

This patch series is based on top of SVA Part 3.
  https://lore.kernel.org/linux-iommu/20230816174031.634453-1-vasant.hegde@amd.com/T/#t

Also depends on :
  1) Tina's SVA domain improvement
     https://lore.kernel.org/linux-iommu/20230827084401.819852-1-tina.zhang@intel.com/

  2) Baolu's IOPF improvement
     https://lore.kernel.org/linux-iommu/cbfbe969-1a92-52bf-f00c-3fb89feefd66@linux.intel.com/


This is also available at github :
  https://github.com/AMDESE/linux/tree/iommu_sva_part4_v3_v6.6_rc5


Thanks everyone who reviewed previous version and provided valuable feedbacks.

Changes from v2 -> v3:
  - Rename sva.c -> pasid.c
  - Changed amd_iommu_sva_supported() -> amd_iommu_pasid_supported()
  - Added patch to update/flush DTE
  - Rework part of SVA support (Patch 5 and 6)
  - Move IOPF enablement to PASID bind time

v2: https://lore.kernel.org/linux-iommu/20230911121046.1025732-1-vasant.hegde@amd.com/T/#u

Changes from v1 -> v2:
  - Added new patch to fix PASID override issue in GCR3 table
  - Complete rework of SVA code on top of Tina's SVA series.
  - Rework SVA enable code
  - Reworked IOPF handler code on top of Baolu's IOPF improvement series.

v1: https://lore.kernel.org/linux-iommu/20230823140415.729050-1-vasant.hegde@amd.com/T/#t

Thank you,
Vasant / Suravee


Suravee Suthikulpanit (5):
  iommu/amd: Add support to enable/disable PASID feature
  iommu/amd: Move PPR-related functions into ppr.c
  iommu/amd: Define per-IOMMU iopf_queue
  iommu/amd: Add support for page response
  iommu/amd: Introduce logic to enable/disable IOPF

Vasant Hegde (4):
  iommu/amd: Rename amd_iommu_v2_supported() as amd_iommu_pasid_supported()
  iommu/amd: Do not override PASID entry in GCR3 table
  iommu/amd: Introduce per device DTE update function
  iommu/amd: Initial SVA support for AMD IOMMU

Wei Huang (3):
  iommu/amd: Add support for enabling/disabling IOMMU features
  iommu/amd: Add support for add/remove device for IOPF
  iommu/amd: Add IO page fault notifier handler

 drivers/iommu/amd/Kconfig           |   2 +
 drivers/iommu/amd/Makefile          |   2 +-
 drivers/iommu/amd/amd_iommu.h       |  47 +++-
 drivers/iommu/amd/amd_iommu_types.h |  27 ++
 drivers/iommu/amd/init.c            |  72 ++----
 drivers/iommu/amd/iommu.c           | 189 ++++++++------
 drivers/iommu/amd/pasid.c           | 291 ++++++++++++++++++++++
 drivers/iommu/amd/ppr.c             | 366 ++++++++++++++++++++++++++++
 8 files changed, 858 insertions(+), 138 deletions(-)
 create mode 100644 drivers/iommu/amd/pasid.c
 create mode 100644 drivers/iommu/amd/ppr.c

-- 
2.31.1

[PATCH v3 01/12] iommu/amd: Rename amd_iommu_v2_supported() as amd_iommu_pasid_supported()

[Vasant Hegde]

To reflect its usage. No functional changes intended.

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu.h | 2 +-
 drivers/iommu/amd/init.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 1fceaf4a8229..0df0cb03329c 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -38,7 +38,7 @@ extern int amd_iommu_guest_ir;
 extern enum io_pgtable_fmt amd_iommu_pgtable;
 extern int amd_iommu_gpt_level;
 
-bool amd_iommu_v2_supported(void);
+bool amd_iommu_pasid_supported(void);
 struct amd_iommu *get_amd_iommu(unsigned int idx);
 u8 amd_iommu_pc_get_max_banks(unsigned int idx);
 bool amd_iommu_pc_supported(void);
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index c83bd0c2a1c9..c8b2c8038829 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3652,7 +3652,7 @@ __setup("ivrs_ioapic",		parse_ivrs_ioapic);
 __setup("ivrs_hpet",		parse_ivrs_hpet);
 __setup("ivrs_acpihid",		parse_ivrs_acpihid);
 
-bool amd_iommu_v2_supported(void)
+bool amd_iommu_pasid_supported(void)
 {
 	/* CPU page table size should match IOMMU guest page table size */
 	if (cpu_feature_enabled(X86_FEATURE_LA57) &&
-- 
2.31.1

[PATCH v3 02/12] iommu/amd: Do not override PASID entry in GCR3 table

[Vasant Hegde]

Return error if PASID is already exists.

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index d6e9f1ebd1ea..8e5d1dcc50dc 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1945,6 +1945,9 @@ static int __clear_gcr3(struct iommu_dev_data *dev_data, ioasid_t pasid)
 	if (pte == NULL)
 		return -EINVAL;
 
+	if (*pte & GCR3_VALID)
+		return -EBUSY;
+
 	*pte = 0;
 	amd_iommu_dev_flush_pasid_all(dev_data, pasid);
 
-- 
2.31.1

[PATCH v3 03/12] iommu/amd: Introduce per device DTE update function

[Vasant Hegde]

Consolidate per device update and flush logic into separate function.
Also make it as global function as it will be used in subsequent series
to update the DTE.

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu.h |  1 +
 drivers/iommu/amd/iommu.c     | 40 +++++++++++++++++++++--------------
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 0df0cb03329c..e3c7b99977c5 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -65,6 +65,7 @@ int amd_iommu_clear_gcr3(struct iommu_dev_data *dev_data, ioasid_t pasid);
 void amd_iommu_flush_all_caches(struct amd_iommu *iommu);
 void amd_iommu_update_and_flush_device_table(struct protection_domain *domain);
 void amd_iommu_domain_update(struct protection_domain *domain);
+void amd_iommu_dev_update_dte(struct iommu_dev_data *dev_data, bool set);
 void amd_iommu_domain_flush_complete(struct protection_domain *domain);
 void amd_iommu_domain_flush_pages(struct protection_domain *pdom,
 				  u64 address, size_t size);
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 8e5d1dcc50dc..39acf98e45dd 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -79,6 +79,7 @@ static void detach_device(struct device *dev);
 
 static void set_dte_entry(struct amd_iommu *iommu,
 			  struct iommu_dev_data *dev_data);
+static void clear_dte_entry(struct amd_iommu *iommu, u16 devid);
 
 /****************************************************************************
  *
@@ -1679,6 +1680,24 @@ static void domain_flush_np_cache(struct protection_domain *domain,
 }
 
 
+/* Update and flush DTE for the given device */
+void amd_iommu_dev_update_dte(struct iommu_dev_data *dev_data, bool set)
+{
+	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev_data->dev);
+
+	if (!iommu)
+		return;
+
+	if (set)
+		set_dte_entry(iommu, dev_data);
+	else
+		clear_dte_entry(iommu, dev_data->devid);
+
+	clone_aliases(iommu, dev_data->dev);
+
+	device_flush_dte(dev_data);
+}
+
 /*
  * This function flushes the DTEs for all devices in domain
  */
@@ -1804,7 +1823,6 @@ static void free_gcr3_tbl_level2(u64 *tbl)
 static void free_gcr3_table(struct iommu_dev_data *dev_data)
 {
 	struct gcr3_tbl_info *gcr3_info = &dev_data->gcr3_info;
-	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev_data->dev);
 
 	if (gcr3_info->glx == 2)
 		free_gcr3_tbl_level2(gcr3_info->gcr3_tbl);
@@ -1815,9 +1833,7 @@ static void free_gcr3_table(struct iommu_dev_data *dev_data)
 
 	gcr3_info->glx = 0;
 
-	set_dte_entry(iommu, dev_data);
-	clone_aliases(iommu, dev_data->dev);
-	device_flush_dte(dev_data);
+	amd_iommu_dev_update_dte(dev_data, true);
 
 	free_page((unsigned long)gcr3_info->gcr3_tbl);
 	gcr3_info->gcr3_tbl = NULL;
@@ -1842,7 +1858,6 @@ static int get_gcr3_levels(int pasids)
 static int setup_gcr3_table(struct iommu_dev_data *dev_data, int pasids)
 {
 	struct gcr3_tbl_info *gcr3_info = &dev_data->gcr3_info;
-	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev_data->dev);
 	int levels = get_gcr3_levels(pasids);
 
 	if (levels > amd_iommu_max_glx_val)
@@ -1858,9 +1873,7 @@ static int setup_gcr3_table(struct iommu_dev_data *dev_data, int pasids)
 
 	gcr3_info->glx = levels;
 
-	set_dte_entry(iommu, dev_data);
-	clone_aliases(iommu, dev_data->dev);
-	device_flush_dte(dev_data);
+	amd_iommu_dev_update_dte(dev_data, true);
 
 	return 0;
 }
@@ -2138,10 +2151,7 @@ static int do_attach(struct iommu_dev_data *dev_data,
 	}
 
 	/* Update device table */
-	set_dte_entry(iommu, dev_data);
-	clone_aliases(iommu, dev_data->dev);
-
-	device_flush_dte(dev_data);
+	amd_iommu_dev_update_dte(dev_data, true);
 
 	return ret;
 }
@@ -2164,11 +2174,9 @@ static void do_detach(struct iommu_dev_data *dev_data)
 	/* Update data structures */
 	dev_data->domain = NULL;
 	list_del(&dev_data->list);
-	clear_dte_entry(iommu, dev_data->devid);
-	clone_aliases(iommu, dev_data->dev);
 
-	/* Flush the DTE entry */
-	device_flush_dte(dev_data);
+	/* Clear DTE and flush the entry */
+	amd_iommu_dev_update_dte(dev_data, false);
 
 	/* Flush IOTLB and wait for the flushes to finish */
 	amd_iommu_domain_flush_all(domain);
-- 
2.31.1

[PATCH v3 04/12] iommu/amd: Add support for enabling/disabling IOMMU features

[Vasant Hegde]

From: Wei Huang <wei.huang2@amd.com>

Add support for struct iommu_ops.dev_{enable/disable}_feat. Please note
that the empty feature switches will be populated by subsequent patches.

Signed-off-by: Wei Huang <wei.huang2@amd.com>
Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/iommu.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 39acf98e45dd..22e0703cb500 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2774,6 +2774,32 @@ static bool amd_iommu_enforce_cache_coherency(struct iommu_domain *domain)
 	return true;
 }
 
+static int amd_iommu_dev_enable_feature(struct device *dev,
+					enum iommu_dev_features feat)
+{
+	int ret;
+
+	switch (feat) {
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+static int amd_iommu_dev_disable_feature(struct device *dev,
+					 enum iommu_dev_features feat)
+{
+	int ret;
+
+	switch (feat) {
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
 const struct iommu_ops amd_iommu_ops = {
 	.capable = amd_iommu_capable,
 	.domain_alloc = amd_iommu_domain_alloc,
@@ -2785,6 +2811,8 @@ const struct iommu_ops amd_iommu_ops = {
 	.is_attach_deferred = amd_iommu_is_attach_deferred,
 	.pgsize_bitmap	= AMD_IOMMU_PGSIZES,
 	.def_domain_type = amd_iommu_def_domain_type,
+	.dev_enable_feat = amd_iommu_dev_enable_feature,
+	.dev_disable_feat = amd_iommu_dev_disable_feature,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= amd_iommu_attach_device,
 		.map_pages	= amd_iommu_map_pages,
-- 
2.31.1

[PATCH v3 05/12] iommu/amd: Initial SVA support for AMD IOMMU

[Vasant Hegde]

This includes :
  - Add data structure to track per protection domain dev/pasid binding details

  - Add mmu notifier to protection domain, so that we can retrieve SVA
    protection domain in invalidation path.

  - Move 'to_pdomain()' to header file

  - Add helper function to allocate/free SVA domain

  - Add iommu_ops.remove_dev_pasid support. It will unbind PASID from
    device. Also remove pasid data from protection domain.

  - mmu notifier for TLB invalidation

  - Add IOMMU_SVA as dependency to AMD_IOMMU driver

For a given PASID, iommu_set_dev_pasid() will bind all devices to same
SVA protection domain (1 PASID : 1 SVA protection domain : N devices).
This protection domain is different from device protection domain (one
that's mapped in attach_device() path). IOMMU uses domain ID for caching,
etc. In invalidation path we retrieve domain ID from iommu_dev_data
structure and use that for invalidation.

Co-developed-by: Wei Huang <wei.huang2@amd.com>
Signed-off-by: Wei Huang <wei.huang2@amd.com>
Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/Kconfig           |   1 +
 drivers/iommu/amd/Makefile          |   2 +-
 drivers/iommu/amd/amd_iommu.h       |  13 ++
 drivers/iommu/amd/amd_iommu_types.h |  15 +++
 drivers/iommu/amd/iommu.c           |  11 +-
 drivers/iommu/amd/pasid.c           | 200 ++++++++++++++++++++++++++++
 6 files changed, 235 insertions(+), 7 deletions(-)
 create mode 100644 drivers/iommu/amd/pasid.c

diff --git a/drivers/iommu/amd/Kconfig b/drivers/iommu/amd/Kconfig
index 75132ae861a2..cabf4ccde1ed 100644
--- a/drivers/iommu/amd/Kconfig
+++ b/drivers/iommu/amd/Kconfig
@@ -10,6 +10,7 @@ config AMD_IOMMU
 	select IOMMU_API
 	select IOMMU_IOVA
 	select IOMMU_IO_PGTABLE
+	select IOMMU_SVA
 	depends on X86_64 && PCI && ACPI && HAVE_CMPXCHG_DOUBLE
 	help
 	  With this option you can enable support for AMD IOMMU hardware in
diff --git a/drivers/iommu/amd/Makefile b/drivers/iommu/amd/Makefile
index f454fbb1569e..a83e31208cdb 100644
--- a/drivers/iommu/amd/Makefile
+++ b/drivers/iommu/amd/Makefile
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_AMD_IOMMU) += iommu.o init.o quirks.o io_pgtable.o io_pgtable_v2.o
+obj-$(CONFIG_AMD_IOMMU) += iommu.o init.o quirks.o io_pgtable.o io_pgtable_v2.o pasid.o
 obj-$(CONFIG_AMD_IOMMU_DEBUGFS) += debugfs.o
diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index e3c7b99977c5..efcbec84d096 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -38,7 +38,15 @@ extern int amd_iommu_guest_ir;
 extern enum io_pgtable_fmt amd_iommu_pgtable;
 extern int amd_iommu_gpt_level;
 
+/* Protection domain ops */
+struct protection_domain *amd_iommu_sva_domain_alloc(
+				     struct protection_domain *pdom);
+void amd_iommu_domain_free(struct iommu_domain *dom);
+void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid);
+
+/* SVA/PASID */
 bool amd_iommu_pasid_supported(void);
+
 struct amd_iommu *get_amd_iommu(unsigned int idx);
 u8 amd_iommu_pc_get_max_banks(unsigned int idx);
 bool amd_iommu_pc_supported(void);
@@ -170,6 +178,11 @@ static inline struct amd_iommu *get_amd_iommu_from_dev(struct device *dev)
 	return container_of(iommu, struct amd_iommu, iommu);
 }
 
+static inline struct protection_domain *to_pdomain(struct iommu_domain *dom)
+{
+	return container_of(dom, struct protection_domain, domain);
+}
+
 bool translation_pre_enabled(struct amd_iommu *iommu);
 bool amd_iommu_is_attach_deferred(struct device *dev);
 int __init add_special_device(u8 type, u8 id, u32 *devid, bool cmd_line);
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index da94dca1eb92..20961353460d 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -8,7 +8,9 @@
 #ifndef _ASM_X86_AMD_IOMMU_TYPES_H
 #define _ASM_X86_AMD_IOMMU_TYPES_H
 
+#include <linux/iommu.h>
 #include <linux/types.h>
+#include <linux/mmu_notifier.h>
 #include <linux/mutex.h>
 #include <linux/msi.h>
 #include <linux/list.h>
@@ -541,6 +543,16 @@ enum protection_domain_mode {
 	PD_MODE_V2,
 };
 
+/* Track PASID list for the protection domain */
+struct pdom_pasid_data {
+	/* PASID attached to the protection domain */
+	ioasid_t pasid;
+	/* Points to attached device data */
+	struct iommu_dev_data *dev_data;
+	/* Link to protection domain */
+	struct list_head pdom_link;
+};
+
 /*
  * This structure contains generic data for  IOMMU protection domains
  * independent of their use.
@@ -556,6 +568,9 @@ struct protection_domain {
 	enum protection_domain_mode pd_mode; /* Track page table type */
 	unsigned dev_cnt;	/* devices assigned to this domain */
 	unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
+
+	struct mmu_notifier mn;	/* mmu notifier for the SVA domain */
+	struct list_head pasid_list; /* List of pdom_pasid_data */
 };
 
 /*
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 22e0703cb500..0974c88e39ce 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -201,11 +201,6 @@ static struct amd_iommu *rlookup_amd_iommu(struct device *dev)
 	return __rlookup_amd_iommu(seg, PCI_SBDF_TO_DEVID(devid));
 }
 
-static struct protection_domain *to_pdomain(struct iommu_domain *dom)
-{
-	return container_of(dom, struct protection_domain, domain);
-}
-
 static struct iommu_dev_data *alloc_dev_data(struct amd_iommu *iommu, u16 devid)
 {
 	struct iommu_dev_data *dev_data;
@@ -2415,6 +2410,7 @@ static struct protection_domain *protection_domain_alloc(unsigned int type)
 
 	spin_lock_init(&domain->lock);
 	INIT_LIST_HEAD(&domain->dev_list);
+	INIT_LIST_HEAD(&domain->pasid_list);
 	domain->nid = NUMA_NO_NODE;
 
 	switch (type) {
@@ -2432,6 +2428,8 @@ static struct protection_domain *protection_domain_alloc(unsigned int type)
 	case IOMMU_DOMAIN_UNMANAGED:
 		pgtable = AMD_IOMMU_V1;
 		break;
+	case IOMMU_DOMAIN_SVA:
+		return amd_iommu_sva_domain_alloc(domain);
 	default:
 		goto out_err;
 	}
@@ -2492,7 +2490,7 @@ static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
 	return &domain->domain;
 }
 
-static void amd_iommu_domain_free(struct iommu_domain *dom)
+void amd_iommu_domain_free(struct iommu_domain *dom)
 {
 	struct protection_domain *domain;
 	unsigned long flags;
@@ -2813,6 +2811,7 @@ const struct iommu_ops amd_iommu_ops = {
 	.def_domain_type = amd_iommu_def_domain_type,
 	.dev_enable_feat = amd_iommu_dev_enable_feature,
 	.dev_disable_feat = amd_iommu_dev_disable_feature,
+	.remove_dev_pasid = amd_iommu_remove_dev_pasid,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= amd_iommu_attach_device,
 		.map_pages	= amd_iommu_map_pages,
diff --git a/drivers/iommu/amd/pasid.c b/drivers/iommu/amd/pasid.c
new file mode 100644
index 000000000000..c251b274eda0
--- /dev/null
+++ b/drivers/iommu/amd/pasid.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 Advanced Micro Devices, Inc.
+ */
+
+#define pr_fmt(fmt)     "AMD-Vi: " fmt
+#define dev_fmt(fmt)    pr_fmt(fmt)
+
+#include <linux/iommu.h>
+#include <linux/mm_types.h>
+
+#include "amd_iommu.h"
+
+static void dev_pasid_remove(struct pdom_pasid_data *pasid_data)
+{
+	/* make it visible */
+	smp_wmb();
+
+	/* Update GCR3 table and flush IOTLB */
+	amd_iommu_clear_gcr3(pasid_data->dev_data, pasid_data->pasid);
+
+	list_del(&pasid_data->pdom_link);
+	kfree(pasid_data);
+}
+
+static struct pdom_pasid_data *get_pdom_pasid_data(struct protection_domain *pdom,
+					   struct device *dev, ioasid_t pasid)
+{
+	struct pdom_pasid_data *pasid_data;
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
+
+	list_for_each_entry(pasid_data, &pdom->pasid_list, pdom_link) {
+		if (pasid_data->pasid == pasid &&
+		    pasid_data->dev_data == dev_data)
+			return pasid_data;
+	}
+
+	return NULL;
+}
+
+static void sva_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
+				    struct mm_struct *mm,
+				    unsigned long start, unsigned long end)
+{
+	struct protection_domain *sva_pdom;
+	struct pdom_pasid_data *pasid_data;
+	struct iommu_dev_data *dev_data;
+	unsigned long flags;
+
+	sva_pdom = container_of(mn, struct protection_domain, mn);
+
+	spin_lock_irqsave(&sva_pdom->lock, flags);
+
+	list_for_each_entry(pasid_data, &sva_pdom->pasid_list, pdom_link) {
+		dev_data = pasid_data->dev_data;
+
+		spin_lock(&dev_data->lock);
+		amd_iommu_dev_flush_pasid_pages(dev_data, pasid_data->pasid,
+						start, end - start);
+		spin_unlock(&dev_data->lock);
+	}
+
+	spin_unlock_irqrestore(&sva_pdom->lock, flags);
+}
+
+static void sva_mn_release(struct mmu_notifier *mn, struct mm_struct *mm)
+{
+	struct pdom_pasid_data *pasid_data, *next;
+	struct protection_domain *sva_pdom;
+	unsigned long flags;
+
+	sva_pdom = container_of(mn, struct protection_domain, mn);
+
+	spin_lock_irqsave(&sva_pdom->lock, flags);
+
+	/* Assume pasid_list contains same PASID with different devices */
+	list_for_each_entry_safe(pasid_data, next,
+				 &sva_pdom->pasid_list, pdom_link) {
+		dev_pasid_remove(pasid_data);
+	}
+
+	spin_unlock_irqrestore(&sva_pdom->lock, flags);
+}
+
+static const struct mmu_notifier_ops sva_mn = {
+	.arch_invalidate_secondary_tlbs = sva_arch_invalidate_secondary_tlbs,
+	.release = sva_mn_release,
+};
+
+static int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
+				   struct device *dev, ioasid_t pasid)
+{
+	struct pdom_pasid_data *pasid_data;
+	struct protection_domain *sva_pdom = to_pdomain(domain);
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
+	unsigned long flags;
+	int ret = -EINVAL;
+
+	/* PASID zero is used for requests from the I/O device without PASID */
+	if (pasid == 0 || pasid >= dev->iommu->max_pasids)
+		return ret;
+
+	/* Use SVA protection domain lock */
+	spin_lock_irqsave(&sva_pdom->lock, flags);
+
+	/* Add PASID to protection domain pasid list */
+	pasid_data = kzalloc(sizeof(*pasid_data), GFP_KERNEL);
+	if (pasid_data == NULL) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	pasid_data->pasid = pasid;
+	pasid_data->dev_data = dev_data;
+
+	if (list_empty(&sva_pdom->pasid_list)) {
+		sva_pdom->mn.ops = &sva_mn;
+
+		ret = mmu_notifier_register(&sva_pdom->mn, domain->mm);
+		if (ret) {
+			sva_pdom->mn.ops = NULL;
+			goto out_free_pasid_data;
+		}
+	}
+
+	/* Setup GCR3 table */
+	ret = amd_iommu_set_gcr3(dev_data, pasid,
+				 iommu_virt_to_phys(domain->mm->pgd));
+	if (ret)
+		goto out_unreg_notifier;
+
+	list_add(&pasid_data->pdom_link, &sva_pdom->pasid_list);
+	spin_unlock_irqrestore(&sva_pdom->lock, flags);
+	return ret;
+
+out_unreg_notifier:
+	mmu_notifier_unregister(&sva_pdom->mn, domain->mm);
+	sva_pdom->mn.ops = NULL;
+
+out_free_pasid_data:
+	kfree(pasid_data);
+
+out:
+	spin_unlock_irqrestore(&sva_pdom->lock, flags);
+	return ret;
+}
+
+void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
+{
+	struct pdom_pasid_data *pasid_data;
+	struct protection_domain *sva_pdom;
+	struct iommu_domain *domain;
+	unsigned long flags;
+
+	if (pasid == 0 || pasid >= dev->iommu->max_pasids)
+		return;
+
+	/* Get protection domain */
+	domain = iommu_get_domain_for_dev_pasid(dev, pasid, IOMMU_DOMAIN_SVA);
+	if (!domain)
+		return;
+	sva_pdom = to_pdomain(domain);
+
+	/* Ensure that all queued faults have been processed */
+	iopf_queue_flush_dev(dev, pasid);
+
+	spin_lock_irqsave(&sva_pdom->lock, flags);
+
+	pasid_data = get_pdom_pasid_data(sva_pdom, dev, pasid);
+	if (!pasid_data) {
+		spin_unlock_irqrestore(&sva_pdom->lock, flags);
+		return;
+	}
+
+	dev_pasid_remove(pasid_data);
+
+	spin_unlock_irqrestore(&sva_pdom->lock, flags);
+}
+
+static void iommu_sva_domain_free(struct iommu_domain *domain)
+{
+	struct protection_domain *sva_pdom = to_pdomain(domain);
+
+	if (sva_pdom->mn.ops)
+		mmu_notifier_unregister(&sva_pdom->mn, domain->mm);
+
+	amd_iommu_domain_free(domain);
+}
+
+static const struct iommu_domain_ops amd_sva_domain_ops = {
+	.set_dev_pasid = iommu_sva_set_dev_pasid,
+	.free	       = iommu_sva_domain_free
+};
+
+struct protection_domain *amd_iommu_sva_domain_alloc(struct protection_domain *pdom)
+{
+	pdom->domain.ops = &amd_sva_domain_ops;
+
+	return pdom;
+}
-- 
2.31.1

[PATCH v3 06/12] iommu/amd: Add support to enable/disable PASID feature

[Vasant Hegde]

From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

It seems iommu_dev_enable_feature(SVA) will be deprecated soon. Hence in
this path we just return success.

Instead we add necessary check during pasid bind to device. In this path
it checks whether device GCR3 table is setup or not. If not it will
setup GCR3 table.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Co-developed-by: Wei Huang <wei.huang2@amd.com>
Signed-off-by: Wei Huang <wei.huang2@amd.com>
Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu.h |  2 +
 drivers/iommu/amd/iommu.c     | 44 +++++++++++++++++++++
 drivers/iommu/amd/pasid.c     | 72 +++++++++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index efcbec84d096..216792006891 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -46,6 +46,8 @@ void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid);
 
 /* SVA/PASID */
 bool amd_iommu_pasid_supported(void);
+int amd_iommu_gcr3_init(struct iommu_dev_data *dev_data, ioasid_t pasids);
+void amd_iommu_gcr3_uninit(struct iommu_dev_data *dev_data);
 
 struct amd_iommu *get_amd_iommu(unsigned int idx);
 u8 amd_iommu_pc_get_max_banks(unsigned int idx);
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 0974c88e39ce..a34998bbb779 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -87,6 +87,11 @@ static void clear_dte_entry(struct amd_iommu *iommu, u16 devid);
  *
  ****************************************************************************/
 
+static inline bool pdom_is_pt_mode(struct protection_domain *pdom)
+{
+	return (pdom->domain.type == IOMMU_DOMAIN_IDENTITY);
+}
+
 /*
  * For invalidation request without PASID, get the pasid based on
  * domain page table mode.
@@ -1977,6 +1982,39 @@ int amd_iommu_clear_gcr3(struct iommu_dev_data *dev_data, ioasid_t pasid)
 	return ret;
 }
 
+int amd_iommu_gcr3_init(struct iommu_dev_data *dev_data, ioasid_t pasids)
+{
+	struct protection_domain *pdom = dev_data->domain;
+	int ret = 0;
+
+	lockdep_assert_held(&dev_data->lock);
+
+	/*
+	 * We cannot support PASID w/ existing v1 page table in the same domain
+	 * since it will be nested. However, existing domain w/ v2 page table
+	 * can be used for PASID.
+	 */
+	if (pdom->pd_mode == PD_MODE_V1)
+		return -EOPNOTSUPP;
+
+	/* Allocate GCR3 table */
+	if (pdom_is_pt_mode(dev_data->domain) &&
+	    dev_data->gcr3_info.gcr3_tbl == NULL) {
+		ret = setup_gcr3_table(dev_data, pasids);
+	}
+
+	return ret;
+}
+
+void amd_iommu_gcr3_uninit(struct iommu_dev_data *dev_data)
+{
+	lockdep_assert_held(&dev_data->lock);
+
+	/* Free GCR3 table */
+	if (pdom_is_pt_mode(dev_data->domain))
+		free_gcr3_table(dev_data);
+}
+
 static void set_dte_entry(struct amd_iommu *iommu,
 			  struct iommu_dev_data *dev_data)
 {
@@ -2778,6 +2816,9 @@ static int amd_iommu_dev_enable_feature(struct device *dev,
 	int ret;
 
 	switch (feat) {
+	case IOMMU_DEV_FEAT_SVA:
+		ret = 0;
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -2791,6 +2832,9 @@ static int amd_iommu_dev_disable_feature(struct device *dev,
 	int ret;
 
 	switch (feat) {
+	case IOMMU_DEV_FEAT_SVA:
+		ret = 0;
+		break;
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/drivers/iommu/amd/pasid.c b/drivers/iommu/amd/pasid.c
index c251b274eda0..f064d0ed1138 100644
--- a/drivers/iommu/amd/pasid.c
+++ b/drivers/iommu/amd/pasid.c
@@ -11,6 +11,66 @@
 
 #include "amd_iommu.h"
 
+
+static inline bool is_gcr3_table_empty(struct iommu_dev_data *dev_data)
+{
+	return (dev_data->gcr3_info.pasid_cnt == 0);
+}
+
+static inline bool is_pasid_enabled(struct iommu_dev_data *dev_data)
+{
+	if (dev_data->gcr3_info.gcr3_tbl != NULL &&
+	    !is_gcr3_table_empty(dev_data)) {
+		return true;
+	}
+
+	return false;
+}
+
+static int iommu_pasid_enable(struct iommu_dev_data *dev_data)
+{
+	struct device *dev = dev_data->dev;
+	int ret = 0;
+
+	spin_lock(&dev_data->lock);
+
+	if (is_pasid_enabled(dev_data))
+		goto out;
+
+	if (!amd_iommu_pasid_supported()) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	/* attach_device path enables device PASID feature */
+	if (!dev_data->pasid_enabled) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = amd_iommu_gcr3_init(dev_data, dev->iommu->max_pasids);
+
+out:
+	spin_unlock(&dev_data->lock);
+	return ret;
+}
+
+static void iommu_pasid_disable(struct iommu_dev_data *dev_data)
+{
+	spin_lock(&dev_data->lock);
+
+	if (!is_gcr3_table_empty(dev_data))
+		goto out;
+
+	if (dev_data->gcr3_info.gcr3_tbl == NULL)
+		goto out;
+
+	amd_iommu_gcr3_uninit(dev_data);
+
+out:
+	spin_unlock(&dev_data->lock);
+}
+
 static void dev_pasid_remove(struct pdom_pasid_data *pasid_data)
 {
 	/* make it visible */
@@ -103,6 +163,13 @@ static int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
 	/* Use SVA protection domain lock */
 	spin_lock_irqsave(&sva_pdom->lock, flags);
 
+	/* Make sure PASID is enabled */
+	if (!is_pasid_enabled(dev_data)) {
+		ret = iommu_pasid_enable(dev_data);
+		if (ret)
+			goto out;
+	}
+
 	/* Add PASID to protection domain pasid list */
 	pasid_data = kzalloc(sizeof(*pasid_data), GFP_KERNEL);
 	if (pasid_data == NULL) {
@@ -150,6 +217,7 @@ void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 	struct pdom_pasid_data *pasid_data;
 	struct protection_domain *sva_pdom;
 	struct iommu_domain *domain;
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
 	unsigned long flags;
 
 	if (pasid == 0 || pasid >= dev->iommu->max_pasids)
@@ -174,6 +242,10 @@ void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 
 	dev_pasid_remove(pasid_data);
 
+	/* Remove GCR3 table */
+	if (is_gcr3_table_empty(dev_data))
+		iommu_pasid_disable(dev_data);
+
 	spin_unlock_irqrestore(&sva_pdom->lock, flags);
 }
 
-- 
2.31.1

[PATCH v3 07/12] iommu/amd: Move PPR-related functions into ppr.c

[Vasant Hegde]

From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

In preparation to subsequent PPR-related patches, and also remove static
declaration for certain helper functions so that it can be reused in other
files.

Also rename below functions:
  alloc_ppr_log        -> amd_iommu_alloc_ppr_log
  iommu_enable_ppr_log -> amd_iommu_enable_ppr_log
  free_ppr_log         -> amd_iommu_free_ppr_log
  iommu_poll_ppr_log   -> amd_iommu_poll_ppr_log

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/Makefile    |   2 +-
 drivers/iommu/amd/amd_iommu.h |  17 +++++-
 drivers/iommu/amd/init.c      |  65 +++-----------------
 drivers/iommu/amd/iommu.c     |  55 +----------------
 drivers/iommu/amd/ppr.c       | 112 ++++++++++++++++++++++++++++++++++
 5 files changed, 137 insertions(+), 114 deletions(-)
 create mode 100644 drivers/iommu/amd/ppr.c

diff --git a/drivers/iommu/amd/Makefile b/drivers/iommu/amd/Makefile
index a83e31208cdb..0272f00830df 100644
--- a/drivers/iommu/amd/Makefile
+++ b/drivers/iommu/amd/Makefile
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_AMD_IOMMU) += iommu.o init.o quirks.o io_pgtable.o io_pgtable_v2.o pasid.o
+obj-$(CONFIG_AMD_IOMMU) += iommu.o init.o quirks.o io_pgtable.o io_pgtable_v2.o pasid.o ppr.o
 obj-$(CONFIG_AMD_IOMMU_DEBUGFS) += debugfs.o
diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 216792006891..88a6a2321dd5 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -17,10 +17,16 @@ irqreturn_t amd_iommu_int_thread_pprlog(int irq, void *data);
 irqreturn_t amd_iommu_int_thread_galog(int irq, void *data);
 irqreturn_t amd_iommu_int_handler(int irq, void *data);
 void amd_iommu_apply_erratum_63(struct amd_iommu *iommu, u16 devid);
+void amd_iommu_restart_log(struct amd_iommu *iommu, const char *evt_type,
+			   u8 cntrl_intr, u8 cntrl_log,
+			   u32 status_run_mask, u32 status_overflow_mask);
 void amd_iommu_restart_event_logging(struct amd_iommu *iommu);
 void amd_iommu_restart_ga_log(struct amd_iommu *iommu);
 void amd_iommu_restart_ppr_log(struct amd_iommu *iommu);
 void amd_iommu_set_rlookup_table(struct amd_iommu *iommu, u16 devid);
+void iommu_feature_enable(struct amd_iommu *iommu, u8 bit);
+void *__init iommu_alloc_4k_pages(struct amd_iommu *iommu,
+				  gfp_t gfp, size_t size);
 
 #ifdef CONFIG_AMD_IOMMU_DEBUGFS
 void amd_iommu_debugfs_setup(struct amd_iommu *iommu);
@@ -67,6 +73,14 @@ int amd_iommu_set_gcr3(struct iommu_dev_data *dev_data,
 		       ioasid_t pasid, unsigned long gcr3);
 int amd_iommu_clear_gcr3(struct iommu_dev_data *dev_data, ioasid_t pasid);
 
+/* PPR */
+int __init amd_iommu_alloc_ppr_log(struct amd_iommu *iommu);
+void __init amd_iommu_free_ppr_log(struct amd_iommu *iommu);
+void amd_iommu_enable_ppr_log(struct amd_iommu *iommu);
+void amd_iommu_poll_ppr_log(struct amd_iommu *iommu);
+int amd_iommu_complete_ppr(struct pci_dev *pdev, u32 pasid,
+			   int status, int tag);
+
 /* TLB flush */
 /*
  * This function flushes all internal caches of
@@ -97,9 +111,6 @@ static inline int amd_iommu_create_irq_domain(struct amd_iommu *iommu)
 #define PPR_INVALID			0x1
 #define PPR_FAILURE			0xf
 
-int amd_iommu_complete_ppr(struct pci_dev *pdev, u32 pasid,
-			   int status, int tag);
-
 static inline bool is_rd890_iommu(struct pci_dev *pdev)
 {
 	return (pdev->vendor == PCI_VENDOR_ID_ATI) &&
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index c8b2c8038829..05efab64c86a 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -418,7 +418,7 @@ static void iommu_set_device_table(struct amd_iommu *iommu)
 }
 
 /* Generic functions to enable/disable certain features of the IOMMU. */
-static void iommu_feature_enable(struct amd_iommu *iommu, u8 bit)
+void iommu_feature_enable(struct amd_iommu *iommu, u8 bit)
 {
 	u64 ctrl;
 
@@ -745,9 +745,9 @@ static int __init alloc_command_buffer(struct amd_iommu *iommu)
  * Interrupt handler has processed all pending events and adjusted head
  * and tail pointer. Reset overflow mask and restart logging again.
  */
-static void amd_iommu_restart_log(struct amd_iommu *iommu, const char *evt_type,
-				  u8 cntrl_intr, u8 cntrl_log,
-				  u32 status_run_mask, u32 status_overflow_mask)
+void amd_iommu_restart_log(struct amd_iommu *iommu, const char *evt_type,
+			   u8 cntrl_intr, u8 cntrl_log,
+			   u32 status_run_mask, u32 status_overflow_mask)
 {
 	u32 status;
 
@@ -788,17 +788,6 @@ void amd_iommu_restart_ga_log(struct amd_iommu *iommu)
 			      MMIO_STATUS_GALOG_OVERFLOW_MASK);
 }
 
-/*
- * This function restarts ppr logging in case the IOMMU experienced
- * PPR log overflow.
- */
-void amd_iommu_restart_ppr_log(struct amd_iommu *iommu)
-{
-	amd_iommu_restart_log(iommu, "PPR", CONTROL_PPRINT_EN,
-			      CONTROL_PPRLOG_EN, MMIO_STATUS_PPR_RUN_MASK,
-			      MMIO_STATUS_PPR_OVERFLOW_MASK);
-}
-
 /*
  * This function resets the command buffer if the IOMMU stopped fetching
  * commands from it.
@@ -847,8 +836,8 @@ static void __init free_command_buffer(struct amd_iommu *iommu)
 	free_pages((unsigned long)iommu->cmd_buf, get_order(CMD_BUFFER_SIZE));
 }
 
-static void *__init iommu_alloc_4k_pages(struct amd_iommu *iommu,
-					 gfp_t gfp, size_t size)
+void *__init iommu_alloc_4k_pages(struct amd_iommu *iommu, gfp_t gfp,
+				  size_t size)
 {
 	int order = get_order(size);
 	void *buf = (void *)__get_free_pages(gfp, order);
@@ -903,42 +892,6 @@ static void __init free_event_buffer(struct amd_iommu *iommu)
 	free_pages((unsigned long)iommu->evt_buf, get_order(EVT_BUFFER_SIZE));
 }
 
-/* allocates the memory where the IOMMU will log its events to */
-static int __init alloc_ppr_log(struct amd_iommu *iommu)
-{
-	iommu->ppr_log = iommu_alloc_4k_pages(iommu, GFP_KERNEL | __GFP_ZERO,
-					      PPR_LOG_SIZE);
-
-	return iommu->ppr_log ? 0 : -ENOMEM;
-}
-
-static void iommu_enable_ppr_log(struct amd_iommu *iommu)
-{
-	u64 entry;
-
-	if (iommu->ppr_log == NULL)
-		return;
-
-	iommu_feature_enable(iommu, CONTROL_PPR_EN);
-
-	entry = iommu_virt_to_phys(iommu->ppr_log) | PPR_LOG_SIZE_512;
-
-	memcpy_toio(iommu->mmio_base + MMIO_PPR_LOG_OFFSET,
-		    &entry, sizeof(entry));
-
-	/* set head and tail to zero manually */
-	writel(0x00, iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
-	writel(0x00, iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
-
-	iommu_feature_enable(iommu, CONTROL_PPRLOG_EN);
-	iommu_feature_enable(iommu, CONTROL_PPRINT_EN);
-}
-
-static void __init free_ppr_log(struct amd_iommu *iommu)
-{
-	free_pages((unsigned long)iommu->ppr_log, get_order(PPR_LOG_SIZE));
-}
-
 static void free_ga_log(struct amd_iommu *iommu)
 {
 #ifdef CONFIG_IRQ_REMAP
@@ -1682,7 +1635,7 @@ static void __init free_iommu_one(struct amd_iommu *iommu)
 	free_cwwb_sem(iommu);
 	free_command_buffer(iommu);
 	free_event_buffer(iommu);
-	free_ppr_log(iommu);
+	amd_iommu_free_ppr_log(iommu);
 	free_ga_log(iommu);
 	iommu_unmap_mmio_space(iommu);
 }
@@ -2095,7 +2048,7 @@ static int __init iommu_init_pci(struct amd_iommu *iommu)
 			amd_iommu_max_glx_val = min(amd_iommu_max_glx_val, glxval);
 	}
 
-	if (check_feature(FEATURE_PPR) && alloc_ppr_log(iommu))
+	if (check_feature(FEATURE_PPR) && amd_iommu_alloc_ppr_log(iommu))
 		return -ENOMEM;
 
 	if (iommu->cap & (1UL << IOMMU_CAP_NPCACHE)) {
@@ -2839,7 +2792,7 @@ static void enable_iommus_v2(void)
 	struct amd_iommu *iommu;
 
 	for_each_iommu(iommu) {
-		iommu_enable_ppr_log(iommu);
+		amd_iommu_enable_ppr_log(iommu);
 		iommu_enable_gt(iommu);
 	}
 }
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a34998bbb779..7512553f2269 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -842,59 +842,6 @@ static void iommu_poll_events(struct amd_iommu *iommu)
 	writel(head, iommu->mmio_base + MMIO_EVT_HEAD_OFFSET);
 }
 
-static void iommu_poll_ppr_log(struct amd_iommu *iommu)
-{
-	u32 head, tail;
-
-	if (iommu->ppr_log == NULL)
-		return;
-
-	head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
-	tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
-
-	while (head != tail) {
-		volatile u64 *raw;
-		u64 entry[2];
-		int i;
-
-		raw = (u64 *)(iommu->ppr_log + head);
-
-		/*
-		 * Hardware bug: Interrupt may arrive before the entry is
-		 * written to memory. If this happens we need to wait for the
-		 * entry to arrive.
-		 */
-		for (i = 0; i < LOOP_TIMEOUT; ++i) {
-			if (PPR_REQ_TYPE(raw[0]) != 0)
-				break;
-			udelay(1);
-		}
-
-		/* Avoid memcpy function-call overhead */
-		entry[0] = raw[0];
-		entry[1] = raw[1];
-
-		/*
-		 * To detect the hardware errata 733 we need to clear the
-		 * entry back to zero. This issue does not exist on SNP
-		 * enabled system. Also this buffer is not writeable on
-		 * SNP enabled system.
-		 */
-		if (!amd_iommu_snp_en)
-			raw[0] = raw[1] = 0UL;
-
-		/* Update head pointer of hardware ring-buffer */
-		head = (head + PPR_ENTRY_SIZE) % PPR_LOG_SIZE;
-		writel(head, iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
-
-		/* TODO: PPR Handler will be added when we add IOPF support */
-
-		/* Refresh ring-buffer information */
-		head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
-		tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
-	}
-}
-
 #ifdef CONFIG_IRQ_REMAP
 static int (*iommu_ga_log_notifier)(u32);
 
@@ -1015,7 +962,7 @@ irqreturn_t amd_iommu_int_thread_pprlog(int irq, void *data)
 {
 	amd_iommu_handle_irq(data, "PPR", MMIO_STATUS_PPR_INT_MASK,
 			     MMIO_STATUS_PPR_OVERFLOW_MASK,
-			     iommu_poll_ppr_log, amd_iommu_restart_ppr_log);
+			     amd_iommu_poll_ppr_log, amd_iommu_restart_ppr_log);
 
 	return IRQ_HANDLED;
 }
diff --git a/drivers/iommu/amd/ppr.c b/drivers/iommu/amd/ppr.c
new file mode 100644
index 000000000000..673fcc30f9dc
--- /dev/null
+++ b/drivers/iommu/amd/ppr.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 Advanced Micro Devices, Inc.
+ */
+
+#define pr_fmt(fmt)     "AMD-Vi: " fmt
+#define dev_fmt(fmt)    pr_fmt(fmt)
+
+#include <linux/amd-iommu.h>
+#include <linux/delay.h>
+#include <linux/mmu_notifier.h>
+
+#include "amd_iommu.h"
+#include "amd_iommu_types.h"
+
+int __init amd_iommu_alloc_ppr_log(struct amd_iommu *iommu)
+{
+	iommu->ppr_log = iommu_alloc_4k_pages(iommu, GFP_KERNEL | __GFP_ZERO,
+					      PPR_LOG_SIZE);
+	return iommu->ppr_log ? 0 : -ENOMEM;
+}
+
+void amd_iommu_enable_ppr_log(struct amd_iommu *iommu)
+{
+	u64 entry;
+
+	if (iommu->ppr_log == NULL)
+		return;
+
+	iommu_feature_enable(iommu, CONTROL_PPR_EN);
+
+	entry = iommu_virt_to_phys(iommu->ppr_log) | PPR_LOG_SIZE_512;
+
+	memcpy_toio(iommu->mmio_base + MMIO_PPR_LOG_OFFSET,
+		    &entry, sizeof(entry));
+
+	/* set head and tail to zero manually */
+	writel(0x00, iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
+	writel(0x00, iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
+
+	iommu_feature_enable(iommu, CONTROL_PPRINT_EN);
+	iommu_feature_enable(iommu, CONTROL_PPRLOG_EN);
+}
+
+void __init amd_iommu_free_ppr_log(struct amd_iommu *iommu)
+{
+	free_pages((unsigned long)iommu->ppr_log, get_order(PPR_LOG_SIZE));
+}
+
+/*
+ * This function restarts ppr logging in case the IOMMU experienced
+ * PPR log overflow.
+ */
+void amd_iommu_restart_ppr_log(struct amd_iommu *iommu)
+{
+	amd_iommu_restart_log(iommu, "PPR", CONTROL_PPRINT_EN,
+			      CONTROL_PPRLOG_EN, MMIO_STATUS_PPR_RUN_MASK,
+			      MMIO_STATUS_PPR_OVERFLOW_MASK);
+}
+
+void amd_iommu_poll_ppr_log(struct amd_iommu *iommu)
+{
+	u32 head, tail;
+
+	if (iommu->ppr_log == NULL)
+		return;
+
+	head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
+	tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
+
+	while (head != tail) {
+		volatile u64 *raw;
+		u64 entry[2];
+		int i;
+
+		raw = (u64 *)(iommu->ppr_log + head);
+
+		/*
+		 * Hardware bug: Interrupt may arrive before the entry is
+		 * written to memory. If this happens we need to wait for the
+		 * entry to arrive.
+		 */
+		for (i = 0; i < LOOP_TIMEOUT; ++i) {
+			if (PPR_REQ_TYPE(raw[0]) != 0)
+				break;
+			udelay(1);
+		}
+
+		/* Avoid memcpy function-call overhead */
+		entry[0] = raw[0];
+		entry[1] = raw[1];
+
+		/*
+		 * To detect the hardware errata 733 we need to clear the
+		 * entry back to zero. This issue does not exist on SNP
+		 * enabled system. Also this buffer is not writeable on
+		 * SNP enabled system.
+		 */
+		if (!amd_iommu_snp_en)
+			raw[0] = raw[1] = 0UL;
+
+		/* Update head pointer of hardware ring-buffer */
+		head = (head + PPR_ENTRY_SIZE) % PPR_LOG_SIZE;
+		writel(head, iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
+
+		/* TODO: PPR Handler will be added when we add IOPF support */
+
+		/* Refresh ring-buffer information */
+		head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
+		tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
+	}
+}
-- 
2.31.1

[PATCH v3 08/12] iommu/amd: Define per-IOMMU iopf_queue

[Vasant Hegde]

From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

AMD IOMMU hardware supports PCI Peripheral Paging Request (PPR) using
a PPR log, which is a circular buffer containing requests from downstream
end-point devices.

There is one PPR log per IOMMU instance. Therefore, allocate an iopf_queue
per IOMMU instance during driver initialization, and free the queue during
driver deinitialization.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu.h       |  4 +++
 drivers/iommu/amd/amd_iommu_types.h |  4 +++
 drivers/iommu/amd/init.c            |  5 ++++
 drivers/iommu/amd/ppr.c             | 42 +++++++++++++++++++++++++++++
 4 files changed, 55 insertions(+)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 88a6a2321dd5..63ddfbde8d82 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -55,6 +55,10 @@ bool amd_iommu_pasid_supported(void);
 int amd_iommu_gcr3_init(struct iommu_dev_data *dev_data, ioasid_t pasids);
 void amd_iommu_gcr3_uninit(struct iommu_dev_data *dev_data);
 
+/* IOPF */
+int amd_iommu_iopf_init(struct amd_iommu *iommu);
+void amd_iommu_iopf_uninit(struct amd_iommu *iommu);
+
 struct amd_iommu *get_amd_iommu(unsigned int idx);
 u8 amd_iommu_pc_get_max_banks(unsigned int idx);
 bool amd_iommu_pc_supported(void);
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 20961353460d..d55a564fbd50 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -773,6 +773,10 @@ struct amd_iommu {
 	/* DebugFS Info */
 	struct dentry *debugfs;
 #endif
+
+	/* IOPF support */
+	struct iopf_queue *iopf_queue;
+	unsigned char iopfq_name[32];
 };
 
 static inline struct amd_iommu *dev_to_amd_iommu(struct device *dev)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 05efab64c86a..80b4d3104b70 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1638,6 +1638,7 @@ static void __init free_iommu_one(struct amd_iommu *iommu)
 	amd_iommu_free_ppr_log(iommu);
 	free_ga_log(iommu);
 	iommu_unmap_mmio_space(iommu);
+	amd_iommu_iopf_uninit(iommu);
 }
 
 static void __init free_iommu_all(void)
@@ -2791,9 +2792,13 @@ static void enable_iommus_v2(void)
 {
 	struct amd_iommu *iommu;
 
+	if (!amd_iommu_gt_ppr_supported())
+		return;
+
 	for_each_iommu(iommu) {
 		amd_iommu_enable_ppr_log(iommu);
 		iommu_enable_gt(iommu);
+		amd_iommu_iopf_init(iommu);
 	}
 }
 
diff --git a/drivers/iommu/amd/ppr.c b/drivers/iommu/amd/ppr.c
index 673fcc30f9dc..9816d7dbcbf0 100644
--- a/drivers/iommu/amd/ppr.c
+++ b/drivers/iommu/amd/ppr.c
@@ -110,3 +110,45 @@ void amd_iommu_poll_ppr_log(struct amd_iommu *iommu)
 		tail = readl(iommu->mmio_base + MMIO_PPR_TAIL_OFFSET);
 	}
 }
+
+/**************************************************************
+ *
+ * IOPF handling stuff
+ */
+
+/* Setup per-IOMMU IOPF queue if not exist. */
+int amd_iommu_iopf_init(struct amd_iommu *iommu)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	raw_spin_lock_irqsave(&iommu->lock, flags);
+
+	if (iommu->iopf_queue)
+		goto out;
+
+	snprintf(iommu->iopfq_name, sizeof(iommu->iopfq_name),
+		 "amdiommu-%#x-iopfq",
+		 PCI_SEG_DEVID_TO_SBDF(iommu->pci_seg->id, iommu->devid));
+
+	iommu->iopf_queue = iopf_queue_alloc(iommu->iopfq_name);
+	if (!iommu->iopf_queue)
+		ret = -ENOMEM;
+
+out:
+	raw_spin_unlock_irqrestore(&iommu->lock, flags);
+	return ret;
+}
+
+/* Destroy per-IOMMU IOPF queue if no longer needed. */
+void amd_iommu_iopf_uninit(struct amd_iommu *iommu)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&iommu->lock, flags);
+
+	iopf_queue_free(iommu->iopf_queue);
+	iommu->iopf_queue = NULL;
+
+	raw_spin_unlock_irqrestore(&iommu->lock, flags);
+}
-- 
2.31.1

[PATCH v3 09/12] iommu/amd: Add support for page response

[Vasant Hegde]

From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

This generates AMD IOMMU COMPLETE_PPR_REQUEST for the specified device
with the specified PRI Response Code.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Wei Huang <wei.huang2@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu.h |  3 +++
 drivers/iommu/amd/iommu.c     |  1 +
 drivers/iommu/amd/ppr.c       | 15 +++++++++++++++
 3 files changed, 19 insertions(+)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 63ddfbde8d82..af2d9023e557 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -58,6 +58,9 @@ void amd_iommu_gcr3_uninit(struct iommu_dev_data *dev_data);
 /* IOPF */
 int amd_iommu_iopf_init(struct amd_iommu *iommu);
 void amd_iommu_iopf_uninit(struct amd_iommu *iommu);
+int amd_iommu_page_response(struct device *dev,
+			    struct iopf_fault *evt,
+			    struct iommu_page_response *resp);
 
 struct amd_iommu *get_amd_iommu(unsigned int idx);
 u8 amd_iommu_pc_get_max_banks(unsigned int idx);
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 7512553f2269..4d0dbb37a548 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2803,6 +2803,7 @@ const struct iommu_ops amd_iommu_ops = {
 	.dev_enable_feat = amd_iommu_dev_enable_feature,
 	.dev_disable_feat = amd_iommu_dev_disable_feature,
 	.remove_dev_pasid = amd_iommu_remove_dev_pasid,
+	.page_response = amd_iommu_page_response,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= amd_iommu_attach_device,
 		.map_pages	= amd_iommu_map_pages,
diff --git a/drivers/iommu/amd/ppr.c b/drivers/iommu/amd/ppr.c
index 9816d7dbcbf0..2a0414e3d689 100644
--- a/drivers/iommu/amd/ppr.c
+++ b/drivers/iommu/amd/ppr.c
@@ -152,3 +152,18 @@ void amd_iommu_iopf_uninit(struct amd_iommu *iommu)
 
 	raw_spin_unlock_irqrestore(&iommu->lock, flags);
 }
+
+int amd_iommu_page_response(struct device *dev,
+			    struct iopf_fault *evt,
+			    struct iommu_page_response *resp)
+{
+	struct pci_dev *pdev;
+
+	if (!dev || !dev_is_pci(dev))
+		return -ENODEV;
+
+	pdev = to_pci_dev(dev);
+
+	return amd_iommu_complete_ppr(pdev, resp->pasid, resp->code,
+				      resp->grpid);
+}
-- 
2.31.1

[PATCH v3 10/12] iommu/amd: Add support for add/remove device for IOPF

[Vasant Hegde]

From: Wei Huang <wei.huang2@amd.com>

Which adds/removes the device to the corresponding per-IOMMU iopf_queue.
It also registers device fault handler. These interfaces are called
from IOPF feature enable/disable path.

Signed-off-by: Wei Huang <wei.huang2@amd.com>
Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu.h |  2 ++
 drivers/iommu/amd/ppr.c       | 38 +++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index af2d9023e557..d14cfd16f125 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -58,6 +58,8 @@ void amd_iommu_gcr3_uninit(struct iommu_dev_data *dev_data);
 /* IOPF */
 int amd_iommu_iopf_init(struct amd_iommu *iommu);
 void amd_iommu_iopf_uninit(struct amd_iommu *iommu);
+int amd_iommu_iopf_add_device(struct amd_iommu *iommu, struct device *dev);
+int amd_iommu_iopf_remove_device(struct amd_iommu *iommu, struct device *dev);
 int amd_iommu_page_response(struct device *dev,
 			    struct iopf_fault *evt,
 			    struct iommu_page_response *resp);
diff --git a/drivers/iommu/amd/ppr.c b/drivers/iommu/amd/ppr.c
index 2a0414e3d689..0371df24a2b4 100644
--- a/drivers/iommu/amd/ppr.c
+++ b/drivers/iommu/amd/ppr.c
@@ -167,3 +167,41 @@ int amd_iommu_page_response(struct device *dev,
 	return amd_iommu_complete_ppr(pdev, resp->pasid, resp->code,
 				      resp->grpid);
 }
+
+int amd_iommu_iopf_add_device(struct amd_iommu *iommu, struct device *dev)
+{
+	unsigned long flags;
+	int ret = -EINVAL;
+
+	raw_spin_lock_irqsave(&iommu->lock, flags);
+
+	if (!iommu->iopf_queue)
+		goto out;
+
+	ret = iopf_queue_add_device(iommu->iopf_queue, dev);
+
+out:
+	raw_spin_unlock_irqrestore(&iommu->lock, flags);
+	return ret;
+}
+
+int amd_iommu_iopf_remove_device(struct amd_iommu *iommu, struct device *dev)
+{
+	unsigned long flags;
+	int ret = -EINVAL;
+
+	raw_spin_lock_irqsave(&iommu->lock, flags);
+
+	if (!iommu->iopf_queue)
+		goto out;
+
+	ret = iopf_queue_remove_device(iommu->iopf_queue, dev);
+	if (ret) {
+		pr_warn("Failed to remove device (0x%x) from iopf queue\n",
+			dev->id);
+	}
+
+out:
+	raw_spin_unlock_irqrestore(&iommu->lock, flags);
+	return ret;
+}
-- 
2.31.1

[PATCH v3 11/12] iommu/amd: Add IO page fault notifier handler

[Vasant Hegde]

From: Wei Huang <wei.huang2@amd.com>

Whenever there is a page fault IOMMU logs entry to ppr log and sends
interrupt to host. We have to handle the page fault and respond to IOMMU.

Add support to validate page fault request and hook it to core iommu
page fault handler.

Signed-off-by: Wei Huang <wei.huang2@amd.com>
Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/amd_iommu_types.h |   8 +++
 drivers/iommu/amd/ppr.c             | 100 +++++++++++++++++++++++++++-
 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index d55a564fbd50..c3b41db34b81 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -250,6 +250,14 @@
 #define PPR_ENTRY_SIZE		16
 #define PPR_LOG_SIZE		(PPR_ENTRY_SIZE * PPR_LOG_ENTRIES)
 
+/* PAGE_SERVICE_REQUEST PPR Log Buffer Entry flags */
+#define PPR_FLAG_EXEC		0x002	/* Execute permission requested */
+#define PPR_FLAG_READ		0x004	/* Read permission requested */
+#define PPR_FLAG_WRITE		0x020	/* Write permission requested */
+#define PPR_FLAG_US		0x040	/* 1: User, 0: Supervisor */
+#define PPR_FLAG_RVSD		0x080	/* Reserved bit not zero */
+#define PPR_FLAG_GN		0x100	/* GVA and PASID is valid */
+
 #define PPR_REQ_TYPE(x)		(((x) >> 60) & 0xfULL)
 #define PPR_FLAGS(x)		(((x) >> 48) & 0xfffULL)
 #define PPR_DEVID(x)		((x) & 0xffffULL)
diff --git a/drivers/iommu/amd/ppr.c b/drivers/iommu/amd/ppr.c
index 0371df24a2b4..63dcf46c2533 100644
--- a/drivers/iommu/amd/ppr.c
+++ b/drivers/iommu/amd/ppr.c
@@ -58,6 +58,103 @@ void amd_iommu_restart_ppr_log(struct amd_iommu *iommu)
 			      MMIO_STATUS_PPR_OVERFLOW_MASK);
 }
 
+static inline u32 ppr_flag_to_fault_perm(u16 flag)
+{
+	int perm = 0;
+
+	if (flag & PPR_FLAG_READ)
+		perm |= IOMMU_FAULT_PERM_READ;
+	if (flag & PPR_FLAG_WRITE)
+		perm |= IOMMU_FAULT_PERM_WRITE;
+	if (flag & PPR_FLAG_EXEC)
+		perm |= IOMMU_FAULT_PERM_EXEC;
+	if (!(flag & PPR_FLAG_US))
+		perm |= IOMMU_FAULT_PERM_PRIV;
+
+	return perm;
+}
+
+static bool ppr_is_valid(struct amd_iommu *iommu, u64 *raw)
+{
+	struct device *dev = iommu->iommu.dev;
+	u16 devid = PPR_DEVID(raw[0]);
+
+	if (!(PPR_FLAGS(raw[0]) & PPR_FLAG_GN)) {
+		dev_info(dev, "PPR logged [Request ignored due to GN=0 (device=%04x:%02x:%02x.%x "
+			 "pasid=0x%05llx address=0x%llx flags=0x%04llx tag=0x%03llx]\n",
+			 iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
+			 PPR_PASID(raw[0]), raw[1], PPR_FLAGS(raw[0]), PPR_TAG(raw[0]));
+		return false;
+	}
+
+	if (PPR_FLAGS(raw[0]) & PPR_FLAG_RVSD) {
+		dev_info(dev, "PPR logged [Invalid request format (device=%04x:%02x:%02x.%x "
+			 "pasid=0x%05llx address=0x%llx flags=0x%04llx tag=0x%03llx]\n",
+			 iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
+			 PPR_PASID(raw[0]), raw[1], PPR_FLAGS(raw[0]), PPR_TAG(raw[0]));
+		return false;
+	}
+
+	return true;
+}
+
+static void iommu_call_iopf_notifier(struct amd_iommu *iommu, u64 *raw)
+{
+	struct iopf_fault event;
+	struct pci_dev *pdev;
+	int ret = -EINVAL;
+	u16 devid = PPR_DEVID(raw[0]);
+
+	if (PPR_REQ_TYPE(raw[0]) != PPR_REQ_FAULT) {
+		pr_err_ratelimited("Unknown PPR request received\n");
+		return;
+	}
+
+	pdev = pci_get_domain_bus_and_slot(iommu->pci_seg->id,
+					   PCI_BUS_NUM(devid), devid & 0xff);
+	if (!pdev)
+		return;
+
+	if (!ppr_is_valid(iommu, raw))
+		goto out;
+
+	memset(&event, 0, sizeof(struct iopf_fault));
+
+	event.fault.type = IOMMU_FAULT_PAGE_REQ;
+	event.fault.prm.perm = ppr_flag_to_fault_perm(PPR_FLAGS(raw[0]));
+	event.fault.prm.addr = (u64)(raw[1] & PAGE_MASK);
+	event.fault.prm.pasid = PPR_PASID(raw[0]);
+	event.fault.prm.grpid = PPR_TAG(raw[0]) & 0x1FF;
+
+	/*
+	 * PASID zero is used for requests from the I/O device without
+	 * a PASID
+	 */
+	if (event.fault.prm.pasid == 0 ||
+	    event.fault.prm.pasid >= pdev->dev.iommu->max_pasids) {
+		pr_info_ratelimited("Invalid PASID : 0x%x, device : 0x%x\n",
+				    event.fault.prm.pasid, pdev->dev.id);
+		goto out;
+	}
+
+
+	event.fault.prm.flags |= IOMMU_FAULT_PAGE_RESPONSE_NEEDS_PASID;
+	event.fault.prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
+	if (PPR_TAG(raw[0]) & 0x200)
+		event.fault.prm.flags |= IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE;
+
+	/* Submit event */
+	ret = iommu_report_device_fault(&pdev->dev, &event);
+
+out:
+	if (ret) {
+		/* Nobody cared, abort */
+		amd_iommu_complete_ppr(pdev, PPR_PASID(raw[0]),
+				       IOMMU_PAGE_RESP_FAILURE,
+				       PPR_TAG(raw[0]) & 0x1FF);
+	}
+}
+
 void amd_iommu_poll_ppr_log(struct amd_iommu *iommu)
 {
 	u32 head, tail;
@@ -103,7 +200,8 @@ void amd_iommu_poll_ppr_log(struct amd_iommu *iommu)
 		head = (head + PPR_ENTRY_SIZE) % PPR_LOG_SIZE;
 		writel(head, iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
 
-		/* TODO: PPR Handler will be added when we add IOPF support */
+		/* Handle PPR entry */
+		iommu_call_iopf_notifier(iommu, entry);
 
 		/* Refresh ring-buffer information */
 		head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
-- 
2.31.1

[PATCH v3 12/12] iommu/amd: Introduce logic to enable/disable IOPF

[Vasant Hegde]

From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

Add support to enable/disable IOPF feature. So that PRI capable device
can make use of page fault handler in SVA mode.

Note that this patch enables PRI while binding PASID to device as I was
told enable_feature(PRI) interface is going away.

Also add IOMMU_IOPF as dependency to AMD_IOMMU driver.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
 drivers/iommu/amd/Kconfig     |  1 +
 drivers/iommu/amd/amd_iommu.h |  3 ++
 drivers/iommu/amd/iommu.c     | 15 ++++++---
 drivers/iommu/amd/pasid.c     | 31 ++++++++++++++----
 drivers/iommu/amd/ppr.c       | 61 +++++++++++++++++++++++++++++++++++
 5 files changed, 101 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/amd/Kconfig b/drivers/iommu/amd/Kconfig
index cabf4ccde1ed..22e33179c593 100644
--- a/drivers/iommu/amd/Kconfig
+++ b/drivers/iommu/amd/Kconfig
@@ -11,6 +11,7 @@ config AMD_IOMMU
 	select IOMMU_IOVA
 	select IOMMU_IO_PGTABLE
 	select IOMMU_SVA
+	select IOMMU_IOPF
 	depends on X86_64 && PCI && ACPI && HAVE_CMPXCHG_DOUBLE
 	help
 	  With this option you can enable support for AMD IOMMU hardware in
diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index d14cfd16f125..541121718ccb 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -63,6 +63,8 @@ int amd_iommu_iopf_remove_device(struct amd_iommu *iommu, struct device *dev);
 int amd_iommu_page_response(struct device *dev,
 			    struct iopf_fault *evt,
 			    struct iommu_page_response *resp);
+int amd_iommu_iopf_enable_device(struct device *dev);
+int amd_iommu_iopf_disable_device(struct device *dev);
 
 struct amd_iommu *get_amd_iommu(unsigned int idx);
 u8 amd_iommu_pc_get_max_banks(unsigned int idx);
@@ -76,6 +78,7 @@ int amd_iommu_pc_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
 /* Device capabilities */
 int amd_iommu_pdev_enable_cap_pri(struct pci_dev *pdev);
 void amd_iommu_pdev_disable_cap_pri(struct pci_dev *pdev);
+bool amd_iommu_pdev_pri_supported(struct pci_dev *pdev);
 
 /* GCR3 setup */
 int amd_iommu_set_gcr3(struct iommu_dev_data *dev_data,
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 4d0dbb37a548..c434abab1fda 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -351,6 +351,13 @@ static inline bool pdev_pasid_supported(struct iommu_dev_data *dev_data)
 	return (dev_data->flags & AMD_IOMMU_DEVICE_FLAG_PASID_SUP);
 }
 
+inline bool amd_iommu_pdev_pri_supported(struct pci_dev *pdev)
+{
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(&pdev->dev);
+
+	return (dev_data->flags & AMD_IOMMU_DEVICE_FLAG_PRI_SUP);
+}
+
 static u32 pdev_get_caps(struct pci_dev *pdev)
 {
 	int features;
@@ -2760,11 +2767,11 @@ static bool amd_iommu_enforce_cache_coherency(struct iommu_domain *domain)
 static int amd_iommu_dev_enable_feature(struct device *dev,
 					enum iommu_dev_features feat)
 {
-	int ret;
+	int ret = 0;
 
 	switch (feat) {
 	case IOMMU_DEV_FEAT_SVA:
-		ret = 0;
+	case IOMMU_DEV_FEAT_IOPF:
 		break;
 	default:
 		ret = -EINVAL;
@@ -2776,11 +2783,11 @@ static int amd_iommu_dev_enable_feature(struct device *dev,
 static int amd_iommu_dev_disable_feature(struct device *dev,
 					 enum iommu_dev_features feat)
 {
-	int ret;
+	int ret = 0;
 
 	switch (feat) {
 	case IOMMU_DEV_FEAT_SVA:
-		ret = 0;
+	case IOMMU_DEV_FEAT_IOPF:
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/drivers/iommu/amd/pasid.c b/drivers/iommu/amd/pasid.c
index f064d0ed1138..c6764421d6c1 100644
--- a/drivers/iommu/amd/pasid.c
+++ b/drivers/iommu/amd/pasid.c
@@ -71,6 +71,27 @@ static void iommu_pasid_disable(struct iommu_dev_data *dev_data)
 	spin_unlock(&dev_data->lock);
 }
 
+static int iommu_setup_pasid_pri(struct iommu_dev_data *dev_data)
+{
+	struct pci_dev *pdev;
+	int ret;
+
+	if (is_pasid_enabled(dev_data))
+		return 0;
+
+	ret = iommu_pasid_enable(dev_data);
+	if (ret)
+		return ret;
+
+	pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
+	if (!pdev || !amd_iommu_pdev_pri_supported(pdev))
+		return ret;
+
+	ret = amd_iommu_iopf_enable_device(dev_data->dev);
+
+	return ret;
+}
+
 static void dev_pasid_remove(struct pdom_pasid_data *pasid_data)
 {
 	/* make it visible */
@@ -163,12 +184,10 @@ static int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
 	/* Use SVA protection domain lock */
 	spin_lock_irqsave(&sva_pdom->lock, flags);
 
-	/* Make sure PASID is enabled */
-	if (!is_pasid_enabled(dev_data)) {
-		ret = iommu_pasid_enable(dev_data);
-		if (ret)
-			goto out;
-	}
+	/* Make sure PASID/PRI is enabled */
+	ret = iommu_setup_pasid_pri(dev_data);
+	if (ret)
+		goto out;
 
 	/* Add PASID to protection domain pasid list */
 	pasid_data = kzalloc(sizeof(*pasid_data), GFP_KERNEL);
diff --git a/drivers/iommu/amd/ppr.c b/drivers/iommu/amd/ppr.c
index 63dcf46c2533..7c4b7766c602 100644
--- a/drivers/iommu/amd/ppr.c
+++ b/drivers/iommu/amd/ppr.c
@@ -303,3 +303,64 @@ int amd_iommu_iopf_remove_device(struct amd_iommu *iommu, struct device *dev)
 	raw_spin_unlock_irqrestore(&iommu->lock, flags);
 	return ret;
 }
+
+static int iopf_update_device(struct device *dev, bool enable)
+{
+	struct protection_domain *pdom;
+	struct pci_dev *pdev = dev_is_pci(dev) ? to_pci_dev(dev) : NULL;
+	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev);
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
+	int ret = 0;
+
+	if (!pdev || !iommu || !dev_data)
+		return -EINVAL;
+
+	pdom = dev_data->domain;
+	if (!dev_data->domain)
+		return -EINVAL;
+
+	spin_lock(&dev_data->lock);
+
+	if (enable) {
+		if (dev_data->ppr)
+			goto out;
+
+		ret = amd_iommu_iopf_add_device(iommu, dev);
+		if (ret)
+			goto out;
+
+		dev_data->ppr = true;
+	} else {
+		ret = amd_iommu_iopf_remove_device(iommu, dev);
+		dev_data->ppr = false;
+	}
+
+	amd_iommu_dev_update_dte(dev_data, true);
+
+out:
+	spin_unlock(&dev_data->lock);
+	return ret;
+}
+
+int amd_iommu_iopf_enable_device(struct device *dev)
+{
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
+
+	if (!dev_data)
+		return -EINVAL;
+
+	if (!dev_data->ats_enabled || !dev_data->pri_enabled)
+		return -EINVAL;
+
+	return iopf_update_device(dev, true);
+}
+
+int amd_iommu_iopf_disable_device(struct device *dev)
+{
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
+
+	if (!dev_data || !dev_data->pri_enabled)
+		return -EINVAL;
+
+	return iopf_update_device(dev, false);
+}
-- 
2.31.1

Re: [PATCH v3 01/12] iommu/amd: Rename amd_iommu_v2_supported() as amd_iommu_pasid_supported()

[Jason Gunthorpe]

On Mon, Oct 16, 2023 at 10:43:40AM +0000, Vasant Hegde wrote:
> To reflect its usage. No functional changes intended.
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu.h | 2 +-
>  drivers/iommu/amd/init.c      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

Re: [PATCH v3 02/12] iommu/amd: Do not override PASID entry in GCR3 table

[Jason Gunthorpe]

On Mon, Oct 16, 2023 at 10:43:41AM +0000, Vasant Hegde wrote:
> Return error if PASID is already exists.
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/amd/iommu.c | 3 +++
>  1 file changed, 3 insertions(+)

Why is this an error?

ops->set_dev_pasid() should replace any current translation at any
PASID with the requested new translation. It is not an error to call
it while the PASID already has a translation.

Jason

Re: [PATCH v3 03/12] iommu/amd: Introduce per device DTE update function

[Jason Gunthorpe]

On Mon, Oct 16, 2023 at 10:43:42AM +0000, Vasant Hegde wrote:

> @@ -1679,6 +1680,24 @@ static void domain_flush_np_cache(struct protection_domain *domain,
>  }
>  
>  
> +/* Update and flush DTE for the given device */
> +void amd_iommu_dev_update_dte(struct iommu_dev_data *dev_data, bool set)
> +{
> +	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev_data->dev);
> +
> +	if (!iommu)
> +		return;

Can't be null

> +	if (set)
> +		set_dte_entry(iommu, dev_data);
> +	else
> +		clear_dte_entry(iommu, dev_data->devid);
> +
> +	clone_aliases(iommu, dev_data->dev);
> +
> +	device_flush_dte(dev_data);

This should take in iommu too..

> +}

Makes sense though

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

My other note was to basically do this:

void amd_iommu_dev_update_dte(struct iommu_dev_data *dev_data, const struct amd_dte *dte)

Where the caller functions would generate the DTE content instead of
having ste_dte_entry try to reverse engineer which caller is calling
it.

Jason

Re: [PATCH v3 04/12] iommu/amd: Add support for enabling/disabling IOMMU features

[Jason Gunthorpe]

On Mon, Oct 16, 2023 at 10:43:43AM +0000, Vasant Hegde wrote:
> From: Wei Huang <wei.huang2@amd.com>
> 
> Add support for struct iommu_ops.dev_{enable/disable}_feat. Please note
> that the empty feature switches will be populated by subsequent patches.
> 
> Signed-off-by: Wei Huang <wei.huang2@amd.com>
> Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/amd/iommu.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

Re: [PATCH v3 05/12] iommu/amd: Initial SVA support for AMD IOMMU

[Jason Gunthorpe]

On Mon, Oct 16, 2023 at 10:43:44AM +0000, Vasant Hegde wrote:
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index da94dca1eb92..20961353460d 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -8,7 +8,9 @@
>  #ifndef _ASM_X86_AMD_IOMMU_TYPES_H
>  #define _ASM_X86_AMD_IOMMU_TYPES_H
>  
> +#include <linux/iommu.h>
>  #include <linux/types.h>
> +#include <linux/mmu_notifier.h>
>  #include <linux/mutex.h>
>  #include <linux/msi.h>
>  #include <linux/list.h>
> @@ -541,6 +543,16 @@ enum protection_domain_mode {
>  	PD_MODE_V2,
>  };
>  
> +/* Track PASID list for the protection domain */
> +struct pdom_pasid_data {
> +	/* PASID attached to the protection domain */
> +	ioasid_t pasid;
> +	/* Points to attached device data */
> +	struct iommu_dev_data *dev_data;
> +	/* Link to protection domain */
> +	struct list_head pdom_link;
> +};
> +
>  /*
>   * This structure contains generic data for  IOMMU protection domains
>   * independent of their use.
> @@ -556,6 +568,9 @@ struct protection_domain {
>  	enum protection_domain_mode pd_mode; /* Track page table type */
>  	unsigned dev_cnt;	/* devices assigned to this domain */
>  	unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
> +
> +	struct mmu_notifier mn;	/* mmu notifier for the SVA domain */
> +	struct list_head pasid_list; /* List of pdom_pasid_data */
>  };

Oh, see here is the data structure I mentioned earlier, it just needs
to be done as part of the invalidation cleanup and replace the other
things I mentioned. Has nothing to do with SVA.

> @@ -2432,6 +2428,8 @@ static struct protection_domain *protection_domain_alloc(unsigned int type)
>  	case IOMMU_DOMAIN_UNMANAGED:
>  		pgtable = AMD_IOMMU_V1;
>  		break;
> +	case IOMMU_DOMAIN_SVA:
> +		return amd_iommu_sva_domain_alloc(domain);

Same remark as I gave intel, you should take my patch (strip the ARM
parts) to give this it's own op which significantly cleans up this
flow.

https://lore.kernel.org/linux-iommu/20-v2-16665a652079+5947-smmuv3_newapi_p2_jgg@nvidia.com/

> +static void dev_pasid_remove(struct pdom_pasid_data *pasid_data)
> +{
> +	/* make it visible */
> +	smp_wmb();

What is "it"? Don't put barriers like this, the barrier should
immediately follow whatever store it is protecting.

> +static struct pdom_pasid_data *get_pdom_pasid_data(struct protection_domain *pdom,
> +					   struct device *dev, ioasid_t pasid)
> +{
> +	struct pdom_pasid_data *pasid_data;
> +	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
> +

Add a lockdep assertion, but it seems strange a function like this
would even exist.. Better to call it "remove_pdom_pasid_data" and
include the list_del(). And then pull the locking into here too.

> +static void sva_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
> +				    struct mm_struct *mm,
> +				    unsigned long start, unsigned long end)
> +{
> +	struct protection_domain *sva_pdom;
> +	struct pdom_pasid_data *pasid_data;
> +	struct iommu_dev_data *dev_data;
> +	unsigned long flags;
> +
> +	sva_pdom = container_of(mn, struct protection_domain, mn);
> +
> +	spin_lock_irqsave(&sva_pdom->lock, flags);
> +
> +	list_for_each_entry(pasid_data, &sva_pdom->pasid_list, pdom_link) {
> +		dev_data = pasid_data->dev_data;
> +
> +		spin_lock(&dev_data->lock);
> +		amd_iommu_dev_flush_pasid_pages(dev_data, pasid_data->pasid,
> +						start, end - start);

Nope, can't unlock while still holding the pointer. Use a mutex or a
rcu scheme. Again this should all be general non-SVA code to fix the
entire invalidation logic.

> +static void sva_mn_release(struct mmu_notifier *mn, struct mm_struct *mm)
> +{
> +	struct pdom_pasid_data *pasid_data, *next;
> +	struct protection_domain *sva_pdom;
> +	unsigned long flags;
> +
> +	sva_pdom = container_of(mn, struct protection_domain, mn);
> +
> +	spin_lock_irqsave(&sva_pdom->lock, flags);
> +
> +	/* Assume pasid_list contains same PASID with different devices */
> +	list_for_each_entry_safe(pasid_data, next,
> +				 &sva_pdom->pasid_list, pdom_link) {
> +		dev_pasid_remove(pasid_data);
> +	}

Be mindful of any logging in other parts of the driver.. Ideally a
non-present PASID translation will generate a debugging log but a
released SVA will not.

> +static int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
> +				   struct device *dev, ioasid_t pasid)
> +{
> +	struct pdom_pasid_data *pasid_data;
> +	struct protection_domain *sva_pdom = to_pdomain(domain);
> +	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
> +	unsigned long flags;
> +	int ret = -EINVAL;
> +
> +	/* PASID zero is used for requests from the I/O device without PASID */
> +	if (pasid == 0 || pasid >= dev->iommu->max_pasids)
> +		return ret;
> +
> +	/* Use SVA protection domain lock */
> +	spin_lock_irqsave(&sva_pdom->lock, flags);
> +
> +	/* Add PASID to protection domain pasid list */
> +	pasid_data = kzalloc(sizeof(*pasid_data), GFP_KERNEL);

GFP_KERNEL under a spinlock - did you test this with full debugging?

> +	if (pasid_data == NULL) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	pasid_data->pasid = pasid;
> +	pasid_data->dev_data = dev_data;
> +
> +	if (list_empty(&sva_pdom->pasid_list)) {
> +		sva_pdom->mn.ops = &sva_mn;
> +
> +		ret = mmu_notifier_register(&sva_pdom->mn, domain->mm);
> +		if (ret) {
> +			sva_pdom->mn.ops = NULL;
> +			goto out_free_pasid_data;
> +		}
> +	}
> +
> +	/* Setup GCR3 table */
> +	ret = amd_iommu_set_gcr3(dev_data, pasid,
> +				 iommu_virt_to_phys(domain->mm->pgd));
> +	if (ret)
> +		goto out_unreg_notifier;

Something looks missing, what if the RID domain hasn't installed a
GCR3? Eg it is a v1 domain or something?

> +	list_add(&pasid_data->pdom_link, &sva_pdom->pasid_list);
> +	spin_unlock_irqrestore(&sva_pdom->lock, flags);
> +	return ret;
> +
> +out_unreg_notifier:
> +	mmu_notifier_unregister(&sva_pdom->mn, domain->mm);
> +	sva_pdom->mn.ops = NULL;

I'm pretty sure you can't actually unregister while holding the
spinlock without creating a deadlock.

Take my SVA patch and move notifier registration to alloc/free and out
of attach to fix this.

Jason

Re: [PATCH v3 11/12] iommu/amd: Add IO page fault notifier handler

[Jason Gunthorpe]

On Mon, Oct 16, 2023 at 10:43:50AM +0000, Vasant Hegde wrote:
> From: Wei Huang <wei.huang2@amd.com>
> 
> Whenever there is a page fault IOMMU logs entry to ppr log and sends
> interrupt to host. We have to handle the page fault and respond to IOMMU.
> 
> Add support to validate page fault request and hook it to core iommu
> page fault handler.
> 
> Signed-off-by: Wei Huang <wei.huang2@amd.com>
> Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu_types.h |   8 +++
>  drivers/iommu/amd/ppr.c             | 100 +++++++++++++++++++++++++++-
>  2 files changed, 107 insertions(+), 1 deletion(-)

These patches don't seem ordered quite right, the patch creating the
SVA domain should be last, it should be organized so the SVA domain
doesn't work then following patches fix it.

Jason

Re: [PATCH v3 02/12] iommu/amd: Do not override PASID entry in GCR3 table

[Vasant Hegde]

On 11/6/2023 11:21 PM, Jason Gunthorpe wrote:
> On Mon, Oct 16, 2023 at 10:43:41AM +0000, Vasant Hegde wrote:
>> Return error if PASID is already exists.
>>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>>  drivers/iommu/amd/iommu.c | 3 +++
>>  1 file changed, 3 insertions(+)
> 
> Why is this an error?
> 
> ops->set_dev_pasid() should replace any current translation at any
> PASID with the requested new translation. It is not an error to call
> it while the PASID already has a translation.

My bad. I did mess up this patch when I rebased. Check should be in __set_gcr3()
 (like below):

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 691c040216e3..558e0c196172 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1879,6 +1879,9 @@ static int __set_gcr3(struct iommu_dev_data *dev_data,
 	if (pte == NULL)
 		return -ENOMEM;

+	if (*pte & GCR3_VALID)
+		return -EBUSY;
+
 	*pte = (gcr3 & PAGE_MASK) | GCR3_VALID;
 	amd_iommu_dev_flush_pasid_all(dev_data, pasid);



I believe as a sane design it should first remove existing mapping (that's why
we have remove_dev_pasid() interface) before adding new mapping. That's why I
introduced this patch.

-Vasant

Re: [PATCH v3 11/12] iommu/amd: Add IO page fault notifier handler

[Vasant Hegde]

On 11/7/2023 4:50 AM, Jason Gunthorpe wrote:
> On Mon, Oct 16, 2023 at 10:43:50AM +0000, Vasant Hegde wrote:
>> From: Wei Huang <wei.huang2@amd.com>
>>
>> Whenever there is a page fault IOMMU logs entry to ppr log and sends
>> interrupt to host. We have to handle the page fault and respond to IOMMU.
>>
>> Add support to validate page fault request and hook it to core iommu
>> page fault handler.
>>
>> Signed-off-by: Wei Huang <wei.huang2@amd.com>
>> Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>>  drivers/iommu/amd/amd_iommu_types.h |   8 +++
>>  drivers/iommu/amd/ppr.c             | 100 +++++++++++++++++++++++++++-
>>  2 files changed, 107 insertions(+), 1 deletion(-)
> 
> These patches don't seem ordered quite right, the patch creating the
> SVA domain should be last, it should be organized so the SVA domain
> doesn't work then following patches fix it.

We support PASID without PRI support. Hence we have SVA related patches first
and then PRI enablement series.

-Vasant

Re: [PATCH v3 03/12] iommu/amd: Introduce per device DTE update function

[Vasant Hegde]

On 11/6/2023 11:24 PM, Jason Gunthorpe wrote:
> On Mon, Oct 16, 2023 at 10:43:42AM +0000, Vasant Hegde wrote:
> 
>> @@ -1679,6 +1680,24 @@ static void domain_flush_np_cache(struct protection_domain *domain,
>>  }
>>  
>>  
>> +/* Update and flush DTE for the given device */
>> +void amd_iommu_dev_update_dte(struct iommu_dev_data *dev_data, bool set)
>> +{
>> +	struct amd_iommu *iommu = get_amd_iommu_from_dev(dev_data->dev);
>> +
>> +	if (!iommu)
>> +		return;
> 
> Can't be null

Fixed.

> 
>> +	if (set)
>> +		set_dte_entry(iommu, dev_data);
>> +	else
>> +		clear_dte_entry(iommu, dev_data->devid);
>> +
>> +	clone_aliases(iommu, dev_data->dev);
>> +
>> +	device_flush_dte(dev_data);
> 
> This should take in iommu too..

Not required. Device is behind IOMMU and we can get iommu pointer from dev_data.


> 
>> +}
> 
> Makes sense though
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> 
> My other note was to basically do this:
> 
> void amd_iommu_dev_update_dte(struct iommu_dev_data *dev_data, const struct amd_dte *dte)
> 
> Where the caller functions would generate the DTE content instead of
> having ste_dte_entry try to reverse engineer which caller is calling
> it.

dev_data has all required data to configure the DTE. So I dont think introducing
another structure is necessary.

-Vasant

Re: [PATCH v3 02/12] iommu/amd: Do not override PASID entry in GCR3 table

[Jason Gunthorpe]

On Tue, Nov 07, 2023 at 11:56:26AM +0530, Vasant Hegde wrote:

> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 691c040216e3..558e0c196172 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -1879,6 +1879,9 @@ static int __set_gcr3(struct iommu_dev_data *dev_data,
>  	if (pte == NULL)
>  		return -ENOMEM;
> 
> +	if (*pte & GCR3_VALID)
> +		return -EBUSY;
> +
>  	*pte = (gcr3 & PAGE_MASK) | GCR3_VALID;
>  	amd_iommu_dev_flush_pasid_all(dev_data, pasid);
> 
> 
> 
> I believe as a sane design it should first remove existing mapping (that's why
> we have remove_dev_pasid() interface) before adding new mapping. That's why I
> introduced this patch.

Again, no, all these ops are replace. The ideal driver should simply
write the new GCR3 entry with the new value in a single step. No
detach/attach cycle.

Jason

Re: [PATCH v3 11/12] iommu/amd: Add IO page fault notifier handler

[Jason Gunthorpe]

On Tue, Nov 07, 2023 at 12:12:31PM +0530, Vasant Hegde wrote:
> 
> 
> On 11/7/2023 4:50 AM, Jason Gunthorpe wrote:
> > On Mon, Oct 16, 2023 at 10:43:50AM +0000, Vasant Hegde wrote:
> >> From: Wei Huang <wei.huang2@amd.com>
> >>
> >> Whenever there is a page fault IOMMU logs entry to ppr log and sends
> >> interrupt to host. We have to handle the page fault and respond to IOMMU.
> >>
> >> Add support to validate page fault request and hook it to core iommu
> >> page fault handler.
> >>
> >> Signed-off-by: Wei Huang <wei.huang2@amd.com>
> >> Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >> Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
> >> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> >> ---
> >>  drivers/iommu/amd/amd_iommu_types.h |   8 +++
> >>  drivers/iommu/amd/ppr.c             | 100 +++++++++++++++++++++++++++-
> >>  2 files changed, 107 insertions(+), 1 deletion(-)
> > 
> > These patches don't seem ordered quite right, the patch creating the
> > SVA domain should be last, it should be organized so the SVA domain
> > doesn't work then following patches fix it.
> 
> We support PASID without PRI support. Hence we have SVA related patches first
> and then PRI enablement series.

How can you support SVA without PRI?

As i've said before the right order is to enable PASID v2 unmanaged
domains without PRI, then add PRI support, then do SVA's mmu notifier
as the last step.

Jason

Re: [PATCH v3 03/12] iommu/amd: Introduce per device DTE update function

[Jason Gunthorpe]

On Tue, Nov 07, 2023 at 12:17:54PM +0530, Vasant Hegde wrote:

> > Where the caller functions would generate the DTE content instead of
> > having ste_dte_entry try to reverse engineer which caller is calling
> > it.
> 
> dev_data has all required data to configure the DTE. So I dont think introducing
> another structure is necessary.

Again look at SMMUv3 and understand why I changed it from trying to
deduce the DTE from the dev_data into having a arm_smmu_ste struct
instead. It made eveything much cleaner.

Jason

Re: [PATCH v3 05/12] iommu/amd: Initial SVA support for AMD IOMMU

[Vasant Hegde]

On 11/7/2023 4:48 AM, Jason Gunthorpe wrote:
> On Mon, Oct 16, 2023 at 10:43:44AM +0000, Vasant Hegde wrote:
>> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
>> index da94dca1eb92..20961353460d 100644
>> --- a/drivers/iommu/amd/amd_iommu_types.h
>> +++ b/drivers/iommu/amd/amd_iommu_types.h
>> @@ -8,7 +8,9 @@
>>  #ifndef _ASM_X86_AMD_IOMMU_TYPES_H
>>  #define _ASM_X86_AMD_IOMMU_TYPES_H
>>  
>> +#include <linux/iommu.h>
>>  #include <linux/types.h>
>> +#include <linux/mmu_notifier.h>
>>  #include <linux/mutex.h>
>>  #include <linux/msi.h>
>>  #include <linux/list.h>
>> @@ -541,6 +543,16 @@ enum protection_domain_mode {
>>  	PD_MODE_V2,
>>  };
>>  
>> +/* Track PASID list for the protection domain */
>> +struct pdom_pasid_data {
>> +	/* PASID attached to the protection domain */
>> +	ioasid_t pasid;
>> +	/* Points to attached device data */
>> +	struct iommu_dev_data *dev_data;
>> +	/* Link to protection domain */
>> +	struct list_head pdom_link;
>> +};
>> +
>>  /*
>>   * This structure contains generic data for  IOMMU protection domains
>>   * independent of their use.
>> @@ -556,6 +568,9 @@ struct protection_domain {
>>  	enum protection_domain_mode pd_mode; /* Track page table type */
>>  	unsigned dev_cnt;	/* devices assigned to this domain */
>>  	unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
>> +
>> +	struct mmu_notifier mn;	/* mmu notifier for the SVA domain */
>> +	struct list_head pasid_list; /* List of pdom_pasid_data */
>>  };
> 
> Oh, see here is the data structure I mentioned earlier, it just needs
> to be done as part of the invalidation cleanup and replace the other
> things I mentioned. Has nothing to do with SVA.
> 
>> @@ -2432,6 +2428,8 @@ static struct protection_domain *protection_domain_alloc(unsigned int type)
>>  	case IOMMU_DOMAIN_UNMANAGED:
>>  		pgtable = AMD_IOMMU_V1;
>>  		break;
>> +	case IOMMU_DOMAIN_SVA:
>> +		return amd_iommu_sva_domain_alloc(domain);
> 
> Same remark as I gave intel, you should take my patch (strip the ARM
> parts) to give this it's own op which significantly cleans up this
> flow.
> 
> https://lore.kernel.org/linux-iommu/20-v2-16665a652079+5947-smmuv3_newapi_p2_jgg@nvidia.com/

Ok. I have grabbed above patch (minus arm stuff) and added as separate patch in
my series.

> 
>> +static void dev_pasid_remove(struct pdom_pasid_data *pasid_data)
>> +{
>> +	/* make it visible */
>> +	smp_wmb();
> 
> What is "it"? Don't put barriers like this, the barrier should
> immediately follow whatever store it is protecting.

Fixed.

> 
>> +static struct pdom_pasid_data *get_pdom_pasid_data(struct protection_domain *pdom,
>> +					   struct device *dev, ioasid_t pasid)
>> +{
>> +	struct pdom_pasid_data *pasid_data;
>> +	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
>> +
> 
> Add a lockdep assertion, but it seems strange a function like this
> would even exist.. Better to call it "remove_pdom_pasid_data" and
> include the list_del(). And then pull the locking into here too.


Ok. I have renamed these function names.

> 
>> +static void sva_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
>> +				    struct mm_struct *mm,
>> +				    unsigned long start, unsigned long end)
>> +{
>> +	struct protection_domain *sva_pdom;
>> +	struct pdom_pasid_data *pasid_data;
>> +	struct iommu_dev_data *dev_data;
>> +	unsigned long flags;
>> +
>> +	sva_pdom = container_of(mn, struct protection_domain, mn);
>> +
>> +	spin_lock_irqsave(&sva_pdom->lock, flags);
>> +
>> +	list_for_each_entry(pasid_data, &sva_pdom->pasid_list, pdom_link) {
>> +		dev_data = pasid_data->dev_data;
>> +
>> +		spin_lock(&dev_data->lock);
>> +		amd_iommu_dev_flush_pasid_pages(dev_data, pasid_data->pasid,
>> +						start, end - start);
> 
> Nope, can't unlock while still holding the pointer. Use a mutex or a
> rcu scheme. Again this should all be general non-SVA code to fix the
> entire invalidation logic.
> 
>> +static void sva_mn_release(struct mmu_notifier *mn, struct mm_struct *mm)
>> +{
>> +	struct pdom_pasid_data *pasid_data, *next;
>> +	struct protection_domain *sva_pdom;
>> +	unsigned long flags;
>> +
>> +	sva_pdom = container_of(mn, struct protection_domain, mn);
>> +
>> +	spin_lock_irqsave(&sva_pdom->lock, flags);
>> +
>> +	/* Assume pasid_list contains same PASID with different devices */
>> +	list_for_each_entry_safe(pasid_data, next,
>> +				 &sva_pdom->pasid_list, pdom_link) {
>> +		dev_pasid_remove(pasid_data);
>> +	}
> 
> Be mindful of any logging in other parts of the driver.. Ideally a
> non-present PASID translation will generate a debugging log but a
> released SVA will not.

Ok.

> 
>> +static int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
>> +				   struct device *dev, ioasid_t pasid)
>> +{
>> +	struct pdom_pasid_data *pasid_data;
>> +	struct protection_domain *sva_pdom = to_pdomain(domain);
>> +	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
>> +	unsigned long flags;
>> +	int ret = -EINVAL;
>> +
>> +	/* PASID zero is used for requests from the I/O device without PASID */
>> +	if (pasid == 0 || pasid >= dev->iommu->max_pasids)
>> +		return ret;
>> +
>> +	/* Use SVA protection domain lock */
>> +	spin_lock_irqsave(&sva_pdom->lock, flags);
>> +
>> +	/* Add PASID to protection domain pasid list */
>> +	pasid_data = kzalloc(sizeof(*pasid_data), GFP_KERNEL);
> 
> GFP_KERNEL under a spinlock - did you test this with full debugging?

Fixed.

> 
>> +	if (pasid_data == NULL) {
>> +		ret = -ENOMEM;
>> +		goto out;
>> +	}
>> +
>> +	pasid_data->pasid = pasid;
>> +	pasid_data->dev_data = dev_data;
>> +
>> +	if (list_empty(&sva_pdom->pasid_list)) {
>> +		sva_pdom->mn.ops = &sva_mn;
>> +
>> +		ret = mmu_notifier_register(&sva_pdom->mn, domain->mm);
>> +		if (ret) {
>> +			sva_pdom->mn.ops = NULL;
>> +			goto out_free_pasid_data;
>> +		}
>> +	}
>> +
>> +	/* Setup GCR3 table */
>> +	ret = amd_iommu_set_gcr3(dev_data, pasid,
>> +				 iommu_virt_to_phys(domain->mm->pgd));
>> +	if (ret)
>> +		goto out_unreg_notifier;
> 
> Something looks missing, what if the RID domain hasn't installed a
> GCR3? Eg it is a v1 domain or something?

I do have check in next patch. Anyway I have re-arranged patches and its all
part of single patch now.

> 
>> +	list_add(&pasid_data->pdom_link, &sva_pdom->pasid_list);
>> +	spin_unlock_irqrestore(&sva_pdom->lock, flags);
>> +	return ret;
>> +
>> +out_unreg_notifier:
>> +	mmu_notifier_unregister(&sva_pdom->mn, domain->mm);
>> +	sva_pdom->mn.ops = NULL;
> 
> I'm pretty sure you can't actually unregister while holding the
> spinlock without creating a deadlock.
> 
> Take my SVA patch and move notifier registration to alloc/free and out
> of attach to fix this.

mmu_notifier is moved to domain alloc/free.

-Vasant