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

[PATCH v6 00/15] 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 - 3:
  Rename, add support to enable/disable features, update DTE etc.

* Patch 4 - 12:
  Add IOPF support

* Patch 13 - 15:
  Introduce SVA support


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

Also depends on :
  1) Baolu's IOPF improvement v12
     https://lore.kernel.org/linux-iommu/c918825d-ad74-4b1d-abcc-31c3f3ea4620@linux.intel.com/T/#t


This is also available at github :
  https://github.com/AMDESE/linux/tree/iommu_sva_part4_v6_v6.8_rc2

Thanks everyone who reviewed previous version and provided valuable feedbacks.


Changes from v5 -> v6:
  - Rebased on top of SVA Part3 v7
  - Reorganized, added few patches
    - Patch to track per device max PASIDs
    - Patch to setup GCR3 table for SVA capable domain
    - Patch to enable PASID/PRI only when domain is capable of PRI support
  - Dropped below patches as they are not needed anymore
    - iommu/amd: Add GCR3 [un]initialization function
    - iommu/amd: Add support for add/remove device for IOPF
  - Rework sev[/remove_dev]_dev_pasid() functions
  - Dropped late gcr3 setup patch

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

Changes from v4 -> v5:
  - Rebased on top of v6.7-rc8 + SVA Part3 patches
  - Few minor improvements like renaming structure name for better, introducing macros, etc

V4: https://lore.kernel.org/linux-iommu/20231221111558.64652-1-vasant.hegde@amd.com/

Changes from v3 -> v4:
  - Moved amd_iommu_dev_update_dte() after set/clear_dte() so that we can avoid
    forward declaration
  - Dropped "iommu/amd: Do not override PASID entry in GCR3 table"
  - Added patch to fix PPR interrupt processing logic
  - Renamed enable_iommus_v2() -> enable_iommus_ppr()
  - Added ops->domain_alloc_sva()
  - Added domain_alloc_sva() support and reorganize SVA patches
  - In error path iommu_call_iopf_notifier() calls amd_iommu_complete_ppr()
    instead of amd_iommu_page_response()


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

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
  - 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


Jason Gunthorpe (1):
  iommu: Add ops->domain_alloc_sva()

Suravee Suthikulpanit (3):
  iommu/amd: Move PPR-related functions into ppr.c
  iommu/amd: Define per-IOMMU iopf_queue
  iommu/amd: Add support for page response

Vasant Hegde (9):
  iommu/amd: Rename amd_iommu_v2_supported() as
    amd_iommu_pasid_supported()
  iommu/amd: Introduce per device DTE update function
  iommu/amd: Fix PPR interrupt processing logic
  iommu/amd: Introduce iommu_dev_data.max_pasids
  iommu/amd: Setup GCR3 table in advance if domain is SVA capable
  iommu/amd: Enable PCI features based on attached domain capability
  iommu/amd: Add support for enable/disable IOPF
  iommu/amd: Initial SVA support for AMD IOMMU
  iommu/amd: Add SVA domain support

Wei Huang (2):
  iommu/amd: Add support for enabling/disabling IOMMU features
  iommu/amd: Add IO page fault notifier handler

 drivers/iommu/amd/Kconfig           |   2 +
 drivers/iommu/amd/Makefile          |   2 +-
 drivers/iommu/amd/amd_iommu.h       |  50 ++++-
 drivers/iommu/amd/amd_iommu_types.h |  33 ++++
 drivers/iommu/amd/init.c            |  85 +++------
 drivers/iommu/amd/iommu.c           | 284 ++++++++++++++++++----------
 drivers/iommu/amd/pasid.c           | 202 ++++++++++++++++++++
 drivers/iommu/amd/ppr.c             | 284 ++++++++++++++++++++++++++++
 drivers/iommu/iommu-sva.c           |  16 +-
 include/linux/iommu.h               |   3 +
 10 files changed, 788 insertions(+), 173 deletions(-)
 create mode 100644 drivers/iommu/amd/pasid.c
 create mode 100644 drivers/iommu/amd/ppr.c

-- 
2.31.1

[PATCH v6 01/15] 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>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.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 b61293f2aef6..334394d14753 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 959820ccfbcc..62f695c519b5 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 v6 02/15] 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>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/amd/amd_iommu.h |  1 +
 drivers/iommu/amd/iommu.c     | 26 ++++++++++++++++++--------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 334394d14753..24a04731ac8b 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -64,6 +64,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 *domain,
 				  u64 address, size_t size);
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 1769f3860ea7..ed8245e66098 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2003,6 +2003,21 @@ static void clear_dte_entry(struct amd_iommu *iommu, u16 devid)
 	amd_iommu_apply_erratum_63(iommu, devid);
 }
 
+/* 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 (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);
+	iommu_completion_wait(iommu);
+}
+
 static int do_attach(struct iommu_dev_data *dev_data,
 		     struct protection_domain *domain)
 {
@@ -2037,10 +2052,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;
 }
@@ -2059,11 +2071,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 v6 03/15] 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>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.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 ed8245e66098..75c2a48c60d7 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2793,6 +2793,32 @@ static const struct iommu_dirty_ops amd_dirty_ops = {
 	.read_and_clear_dirty = amd_iommu_read_and_clear_dirty,
 };
 
+static int amd_iommu_dev_enable_feature(struct device *dev,
+					enum iommu_dev_features feat)
+{
+	int ret;
+
+	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,
@@ -2805,6 +2831,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 v6 04/15] 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>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.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 f454fbb1569e..93b11b6d764f 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 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 24a04731ac8b..353d59111cb2 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);
@@ -57,6 +63,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);
+
 /*
  * This function flushes all internal caches of
  * the IOMMU used by this driver.
@@ -86,9 +100,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 62f695c519b5..d25a706adcfc 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)) {
@@ -2841,7 +2794,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);
 }
 
 static void enable_iommus_vapic(void)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 75c2a48c60d7..59fc0efce6fd 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -822,59 +822,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);
 
@@ -995,7 +942,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 v6 05/15] iommu/amd: Fix PPR interrupt processing logic

[Vasant Hegde]

* Do not re-read ppr head pointer as its just updated by the driver.

* Do not read PPR buffer tail pointer inside while loop. If IOMMU
  generates PPR events continuously then completing interrupt processing
  takes long time. In worst case it may cause infinite loop.

Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/amd/ppr.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/iommu/amd/ppr.c b/drivers/iommu/amd/ppr.c
index 673fcc30f9dc..d43a616c0c36 100644
--- a/drivers/iommu/amd/ppr.c
+++ b/drivers/iommu/amd/ppr.c
@@ -104,9 +104,5 @@ void amd_iommu_poll_ppr_log(struct amd_iommu *iommu)
 		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 v6 06/15] iommu/amd: Introduce iommu_dev_data.max_pasids

[Vasant Hegde]

This variable will track the number of PASIDs supported by the device.
If IOMMU or device doesn't support PASID then it will be zero.

This will be used while allocating GCR3 table to decide required number
of PASID table levels. Also in PASID bind path it will use this variable
to check whether device supports PASID or not.

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

diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index d5786c33aee6..0abf590f6bc9 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -821,6 +821,7 @@ struct iommu_dev_data {
 	struct device *dev;
 	u16 devid;			  /* PCI Device ID */
 
+	u32 max_pasids;			  /* Max supported PASIDs */
 	u32 flags;			  /* Holds AMD_IOMMU_DEVICE_FLAG_<*> */
 	int ats_qdep;
 	u8 ats_enabled  :1;		  /* ATS state */
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 59fc0efce6fd..fe9c00ed7e00 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2105,6 +2105,7 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 {
 	struct iommu_device *iommu_dev;
 	struct amd_iommu *iommu;
+	struct iommu_dev_data *dev_data;
 	int ret;
 
 	if (!check_device(dev))
@@ -2132,6 +2133,17 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 		iommu_dev = &iommu->iommu;
 	}
 
+	/*
+	 * If IOMMU and device supports PASID then it will contain max
+	 * supported PASIDs, else it will be zero.
+	 */
+	dev_data = dev_iommu_priv_get(dev);
+	if (amd_iommu_pasid_supported() && dev_is_pci(dev) &&
+	    pdev_pasid_supported(dev_data)) {
+		dev_data->max_pasids = min_t(u32, iommu->iommu.max_pasids,
+					     pci_max_pasids(to_pci_dev(dev)));
+	}
+
 	iommu_completion_wait(iommu);
 
 	return iommu_dev;
-- 
2.31.1

[PATCH v6 07/15] iommu/amd: Setup GCR3 table in advance if domain is SVA capable

[Vasant Hegde]

SVA can be supported if domain is in passthrough mode or paging domain
with v2 page table. Current code sets up GCR3 table for domain with v2
page table only. Setup GCR3 table for all SVA capable domains.

  - Move GCR3 init/destroy to separate function.

  - Change default GCR3 table to use MAX supported PASIDs. Ideally it
    should use 1 level PASID table as its using PASID zero only. But we
    don't have support to extend PASID table yet. We will fix this later.

  - When domain is configured with passthrough mode, allocate default GCR3
    table only if device is SVA capable.

Note that in attach_device() path it will not know whether device will use
SVA or not. If device is attached to passthrough domain and if it doesn't
use SVA then GCR3 table will never be used. We will endup wasting memory
allocated for GCR3 table. This is done to avoid DTE update when
attaching PASID to device.

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

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index fe9c00ed7e00..693b9ba9cef0 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -93,6 +93,21 @@ static inline bool pdom_is_v2_pgtbl_mode(struct protection_domain *pdom)
 	return (pdom && (pdom->pd_mode == PD_MODE_V2));
 }
 
+static inline bool pdom_is_in_pt_mode(struct protection_domain *pdom)
+{
+	return (pdom->domain.type == IOMMU_DOMAIN_IDENTITY);
+}
+
+/*
+ * 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
+ * or passthrough mode can be used for PASID.
+ */
+static inline bool pdom_is_sva_capable(struct protection_domain *pdom)
+{
+	return pdom_is_v2_pgtbl_mode(pdom) || pdom_is_in_pt_mode(pdom);
+}
+
 static inline int get_acpihid_device_id(struct device *dev,
 					struct acpihid_map_entry **entry)
 {
@@ -1965,6 +1980,58 @@ void amd_iommu_dev_update_dte(struct iommu_dev_data *dev_data, bool set)
 	iommu_completion_wait(iommu);
 }
 
+/*
+ * If domain is SVA capable then initialize GCR3 table. Also if domain is
+ * in v2 page table mode then update GCR3[0].
+ */
+static int init_gcr3_table(struct iommu_dev_data *dev_data,
+			   struct protection_domain *pdom)
+{
+	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
+	int max_pasids = dev_data->max_pasids;
+	int ret = 0;
+
+	 /*
+	  * If domain is in pt mode then setup GCR3 table only if device
+	  * is PASID capable
+	  */
+	if (pdom_is_in_pt_mode(pdom) && !pdev_pasid_supported(dev_data))
+		return ret;
+
+	/*
+	 * By default, setup GCR3 table to support MAX PASIDs
+	 * supported by the device/IOMMU.
+	 */
+	ret = setup_gcr3_table(&dev_data->gcr3_info, iommu,
+			       max_pasids > 0 ?  max_pasids : 1);
+	if (ret)
+		return ret;
+
+	/* Setup GCR3[0] only if domain is setup with v2 page table mode */
+	if (!pdom_is_v2_pgtbl_mode(pdom))
+		return ret;
+
+	ret = update_gcr3(dev_data, 0, iommu_virt_to_phys(pdom->iop.pgd), true);
+	if (ret)
+		free_gcr3_table(&dev_data->gcr3_info);
+
+	return ret;
+}
+
+static void destroy_gcr3_table(struct iommu_dev_data *dev_data,
+			       struct protection_domain *pdom)
+{
+	struct gcr3_tbl_info *gcr3_info = &dev_data->gcr3_info;
+
+	if (pdom_is_v2_pgtbl_mode(pdom))
+		update_gcr3(dev_data, 0, 0, false);
+
+	if (gcr3_info->gcr3_tbl == NULL)
+		return;
+
+	free_gcr3_table(gcr3_info);
+}
+
 static int do_attach(struct iommu_dev_data *dev_data,
 		     struct protection_domain *domain)
 {
@@ -1983,19 +2050,10 @@ static int do_attach(struct iommu_dev_data *dev_data,
 	domain->dev_iommu[iommu->index] += 1;
 	domain->dev_cnt                 += 1;
 
-	/* Init GCR3 table and update device table */
-	if (domain->pd_mode == PD_MODE_V2) {
-		/* By default, setup GCR3 table to support single PASID */
-		ret = setup_gcr3_table(&dev_data->gcr3_info, iommu, 1);
+	if (pdom_is_sva_capable(domain)) {
+		ret = init_gcr3_table(dev_data, domain);
 		if (ret)
 			return ret;
-
-		ret = update_gcr3(dev_data, 0,
-				  iommu_virt_to_phys(domain->iop.pgd), true);
-		if (ret) {
-			free_gcr3_table(&dev_data->gcr3_info);
-			return ret;
-		}
 	}
 
 	/* Update device table */
@@ -2010,10 +2068,8 @@ static void do_detach(struct iommu_dev_data *dev_data)
 	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
 
 	/* Clear GCR3 table */
-	if (domain->pd_mode == PD_MODE_V2) {
-		update_gcr3(dev_data, 0, 0, false);
-		free_gcr3_table(&dev_data->gcr3_info);
-	}
+	if (pdom_is_sva_capable(domain))
+		destroy_gcr3_table(dev_data, domain);
 
 	/* Update data structures */
 	dev_data->domain = NULL;
-- 
2.31.1

[PATCH v6 08/15] iommu/amd: Enable PCI features based on attached domain capability

[Vasant Hegde]

Commit eda8c2860ab ("iommu/amd: Enable device ATS/PASID/PRI capabilities
independently") changed the way it enables device capability while
attaching devices. I missed to account the attached domain capability.
Meaning if domain is not capable of handling PASID/PRI (ex: paging
domain with v1 page table) then enabling device feature is not required.

This patch enables PASID/PRI only if domain is capable of handling SVA.
Also move pci feature enablement to do_attach() function so that we make
SVA capability in one place. Finally make PRI enable/disable functions as
static functions.

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

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 353d59111cb2..54ec9affb1f4 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -54,10 +54,6 @@ int amd_iommu_pc_get_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
 int amd_iommu_pc_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
 			 u8 fxn, u64 *value);
 
-/* Device capabilities */
-int amd_iommu_pdev_enable_cap_pri(struct pci_dev *pdev);
-void amd_iommu_pdev_disable_cap_pri(struct pci_dev *pdev);
-
 /* GCR3 setup */
 int amd_iommu_set_gcr3(struct iommu_dev_data *dev_data,
 		       ioasid_t pasid, unsigned long gcr3);
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 693b9ba9cef0..1c3a4914cd35 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -403,7 +403,7 @@ static inline void pdev_disable_cap_ats(struct pci_dev *pdev)
 	}
 }
 
-int amd_iommu_pdev_enable_cap_pri(struct pci_dev *pdev)
+static inline int pdev_enable_cap_pri(struct pci_dev *pdev)
 {
 	struct iommu_dev_data *dev_data = dev_iommu_priv_get(&pdev->dev);
 	int ret = -EINVAL;
@@ -411,6 +411,9 @@ int amd_iommu_pdev_enable_cap_pri(struct pci_dev *pdev)
 	if (dev_data->pri_enabled)
 		return 0;
 
+	if (!dev_data->ats_enabled)
+		return 0;
+
 	if (dev_data->flags & AMD_IOMMU_DEVICE_FLAG_PRI_SUP) {
 		/*
 		 * First reset the PRI state of the device.
@@ -427,7 +430,7 @@ int amd_iommu_pdev_enable_cap_pri(struct pci_dev *pdev)
 	return ret;
 }
 
-void amd_iommu_pdev_disable_cap_pri(struct pci_dev *pdev)
+static inline void pdev_disable_cap_pri(struct pci_dev *pdev)
 {
 	struct iommu_dev_data *dev_data = dev_iommu_priv_get(&pdev->dev);
 
@@ -469,15 +472,14 @@ static void pdev_enable_caps(struct pci_dev *pdev)
 {
 	pdev_enable_cap_ats(pdev);
 	pdev_enable_cap_pasid(pdev);
-	amd_iommu_pdev_enable_cap_pri(pdev);
-
+	pdev_enable_cap_pri(pdev);
 }
 
 static void pdev_disable_caps(struct pci_dev *pdev)
 {
 	pdev_disable_cap_ats(pdev);
 	pdev_disable_cap_pasid(pdev);
-	amd_iommu_pdev_disable_cap_pri(pdev);
+	pdev_disable_cap_pri(pdev);
 }
 
 /*
@@ -2036,6 +2038,7 @@ static int do_attach(struct iommu_dev_data *dev_data,
 		     struct protection_domain *domain)
 {
 	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
+	struct pci_dev *pdev;
 	int ret = 0;
 
 	/* Update data structures */
@@ -2050,10 +2053,16 @@ static int do_attach(struct iommu_dev_data *dev_data,
 	domain->dev_iommu[iommu->index] += 1;
 	domain->dev_cnt                 += 1;
 
+	pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
 	if (pdom_is_sva_capable(domain)) {
 		ret = init_gcr3_table(dev_data, domain);
 		if (ret)
 			return ret;
+
+		if (pdev)
+			pdev_enable_caps(pdev);
+	} else if (pdev) {
+		pdev_enable_cap_ats(pdev);
 	}
 
 	/* Update device table */
@@ -2108,9 +2117,6 @@ static int attach_device(struct device *dev,
 		goto out;
 	}
 
-	if (dev_is_pci(dev))
-		pdev_enable_caps(to_pci_dev(dev));
-
 	ret = do_attach(dev_data, domain);
 
 out:
-- 
2.31.1

[PATCH v6 09/15] 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.

Also rename enable_iommus_v2() -> enable_iommus_ppr() to reflect its
usage. And add amd_iommu_gt_ppr_supported() check before enabling 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/amd_iommu.h       |  5 +++++
 drivers/iommu/amd/amd_iommu_types.h |  4 ++++
 drivers/iommu/amd/init.c            | 18 +++++++++++++++--
 drivers/iommu/amd/ppr.c             | 31 +++++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 54ec9affb1f4..f2084f90f056 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -45,6 +45,11 @@ extern enum io_pgtable_fmt amd_iommu_pgtable;
 extern int amd_iommu_gpt_level;
 
 bool amd_iommu_pasid_supported(void);
+
+/* 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 0abf590f6bc9..36586ff873da 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -770,6 +770,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 d25a706adcfc..d4fcb8c2b8ea 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)
@@ -2104,6 +2105,16 @@ static int __init iommu_init_pci(struct amd_iommu *iommu)
 	if (ret)
 		return ret;
 
+	/*
+	 * Allocate per IOMMU IOPF queue here so that in attach device path,
+	 * PRI capable device can be added to IOPF queue
+	 */
+	if (amd_iommu_gt_ppr_supported()) {
+		ret = amd_iommu_iopf_init(iommu);
+		if (ret)
+			return ret;
+	}
+
 	iommu_device_register(&iommu->iommu, &amd_iommu_ops, NULL);
 
 	return pci_enable_device(iommu->dev);
@@ -2789,10 +2800,13 @@ static void early_enable_iommus(void)
 	}
 }
 
-static void enable_iommus_v2(void)
+static void enable_iommus_ppr(void)
 {
 	struct amd_iommu *iommu;
 
+	if (!amd_iommu_gt_ppr_supported())
+		return;
+
 	for_each_iommu(iommu)
 		amd_iommu_enable_ppr_log(iommu);
 }
@@ -3130,7 +3144,7 @@ static int amd_iommu_enable_interrupts(void)
 	 * PPR and GA log interrupt for all IOMMUs.
 	 */
 	enable_iommus_vapic();
-	enable_iommus_v2();
+	enable_iommus_ppr();
 
 out:
 	return ret;
diff --git a/drivers/iommu/amd/ppr.c b/drivers/iommu/amd/ppr.c
index d43a616c0c36..39ea979c7134 100644
--- a/drivers/iommu/amd/ppr.c
+++ b/drivers/iommu/amd/ppr.c
@@ -106,3 +106,34 @@ void amd_iommu_poll_ppr_log(struct amd_iommu *iommu)
 		/* TODO: PPR Handler will be added when we add IOPF support */
 	}
 }
+
+/**************************************************************
+ *
+ * IOPF handling stuff
+ */
+
+/* Setup per-IOMMU IOPF queue if not exist. */
+int amd_iommu_iopf_init(struct amd_iommu *iommu)
+{
+	int ret = 0;
+
+	if (iommu->iopf_queue)
+		return ret;
+
+	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;
+
+	return ret;
+}
+
+/* Destroy per-IOMMU IOPF queue if no longer needed. */
+void amd_iommu_iopf_uninit(struct amd_iommu *iommu)
+{
+	iopf_queue_free(iommu->iopf_queue);
+	iommu->iopf_queue = NULL;
+}
-- 
2.31.1

[PATCH v6 10/15] 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.

Also update amd_iommu_complete_ppr() to accept 'struct device' instead
of pdev as it just need device reference.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@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 | 5 +++--
 drivers/iommu/amd/iommu.c     | 8 ++++----
 drivers/iommu/amd/ppr.c       | 6 ++++++
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index f2084f90f056..030a04451599 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -49,6 +49,8 @@ bool amd_iommu_pasid_supported(void);
 /* IOPF */
 int amd_iommu_iopf_init(struct amd_iommu *iommu);
 void amd_iommu_iopf_uninit(struct amd_iommu *iommu);
+void 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);
@@ -69,8 +71,7 @@ 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);
+int amd_iommu_complete_ppr(struct device *dev, u32 pasid, int status, int tag);
 
 /*
  * This function flushes all internal caches of
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 1c3a4914cd35..0fa172e0583d 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1632,15 +1632,14 @@ void amd_iommu_domain_update(struct protection_domain *domain)
 	amd_iommu_domain_flush_all(domain);
 }
 
-int amd_iommu_complete_ppr(struct pci_dev *pdev, u32 pasid,
-			   int status, int tag)
+int amd_iommu_complete_ppr(struct device *dev, u32 pasid, int status, int tag)
 {
 	struct iommu_dev_data *dev_data;
 	struct amd_iommu *iommu;
 	struct iommu_cmd cmd;
 
-	dev_data = dev_iommu_priv_get(&pdev->dev);
-	iommu    = get_amd_iommu_from_dev(&pdev->dev);
+	dev_data = dev_iommu_priv_get(dev);
+	iommu    = get_amd_iommu_from_dev(dev);
 
 	build_complete_ppr(&cmd, dev_data->devid, pasid, status,
 			   tag, dev_data->pri_tlp);
@@ -2854,6 +2853,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,
+	.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 39ea979c7134..a6724809753c 100644
--- a/drivers/iommu/amd/ppr.c
+++ b/drivers/iommu/amd/ppr.c
@@ -137,3 +137,9 @@ void amd_iommu_iopf_uninit(struct amd_iommu *iommu)
 	iopf_queue_free(iommu->iopf_queue);
 	iommu->iopf_queue = NULL;
 }
+
+void amd_iommu_page_response(struct device *dev, struct iopf_fault *evt,
+			     struct iommu_page_response *resp)
+{
+	amd_iommu_complete_ppr(dev, resp->pasid, resp->code, resp->grpid);
+}
-- 
2.31.1

[PATCH v6 11/15] 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 36586ff873da..de637006dc82 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -251,6 +251,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 a6724809753c..46d0ad13ac08 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_dbg(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_dbg(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 iommu_dev_data *dev_data;
+	struct iopf_fault event;
+	struct pci_dev *pdev;
+	u16 devid = PPR_DEVID(raw[0]);
+
+	if (PPR_REQ_TYPE(raw[0]) != PPR_REQ_FAULT) {
+		pr_info_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
+	 */
+	dev_data = dev_iommu_priv_get(&pdev->dev);
+	if (event.fault.prm.pasid == 0 ||
+	    event.fault.prm.pasid >= dev_data->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 */
+	iommu_report_device_fault(&pdev->dev, &event);
+
+	return;
+
+out:
+	/* Nobody cared, abort */
+	amd_iommu_complete_ppr(&pdev->dev, 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);
 	}
 }
 
-- 
2.31.1

[PATCH v6 12/15] iommu/amd: Add support for enable/disable IOPF

[Vasant Hegde]

Return success from enable_feature(IOPF) path as this interface is going
away. Instead we will enable/disable IOPF support in attach/detach device
path.

In attach device path, if device is capable of PRI, then we will add it to
per IOMMU IOPF queue and enable PPR support in IOMMU. Also it will
attach device to domain even if it fails to enable PRI or add device to
IOPF queue as device can continue to work without PRI support.

In detach device patch it follows following sequence:
  - Flush the queue for the given device
  - Disable PPR support in DTE[devid]
  - Remove device from IOPF queue
  - Disable device PRI

Also add IOMMU_IOPF as dependency to AMD_IOMMU driver.

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/amd_iommu.h |  4 ++++
 drivers/iommu/amd/iommu.c     | 40 +++++++++++++++++++++++++++-------
 drivers/iommu/amd/ppr.c       | 41 +++++++++++++++++++++++++++++++++++
 4 files changed, 78 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/amd/Kconfig b/drivers/iommu/amd/Kconfig
index 443b2c13c37b..d563f6d496ca 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_IOPF
 	select IOMMUFD_DRIVER if IOMMUFD
 	depends on X86_64 && PCI && ACPI && HAVE_CMPXCHG_DOUBLE
 	help
diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 030a04451599..384298206a2d 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -51,6 +51,10 @@ int amd_iommu_iopf_init(struct amd_iommu *iommu);
 void amd_iommu_iopf_uninit(struct amd_iommu *iommu);
 void amd_iommu_page_response(struct device *dev, struct iopf_fault *evt,
 			     struct iommu_page_response *resp);
+int amd_iommu_iopf_add_device(struct amd_iommu *iommu,
+			      struct iommu_dev_data *dev_data);
+void amd_iommu_iopf_remove_device(struct amd_iommu *iommu,
+				  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 0fa172e0583d..21cc6654410f 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2058,12 +2058,22 @@ static int do_attach(struct iommu_dev_data *dev_data,
 		if (ret)
 			return ret;
 
-		if (pdev)
+		if (pdev) {
 			pdev_enable_caps(pdev);
+
+			/*
+			 * Device can continue to function even if IOPF
+			 * enablement failed. Hence in error path just
+			 * disable device PRI support.
+			 */
+			if (amd_iommu_iopf_add_device(iommu, dev_data))
+				pdev_disable_cap_pri(pdev);
+		}
 	} else if (pdev) {
 		pdev_enable_cap_ats(pdev);
 	}
 
+
 	/* Update device table */
 	amd_iommu_dev_update_dte(dev_data, true);
 
@@ -2131,12 +2141,11 @@ static int attach_device(struct device *dev,
  */
 static void detach_device(struct device *dev)
 {
-	struct protection_domain *domain;
-	struct iommu_dev_data *dev_data;
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
+	struct protection_domain *domain = dev_data->domain;
+	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
 	unsigned long flags;
-
-	dev_data = dev_iommu_priv_get(dev);
-	domain   = dev_data->domain;
+	bool ppr = dev_data->ppr;
 
 	spin_lock_irqsave(&domain->lock, flags);
 
@@ -2151,8 +2160,19 @@ static void detach_device(struct device *dev)
 	if (WARN_ON(!dev_data->domain))
 		goto out;
 
+	if (ppr) {
+		iopf_queue_flush_dev(dev);
+
+		/* Updated here so that it gets reflected in DTE */
+		dev_data->ppr = false;
+	}
+
 	do_detach(dev_data);
 
+	/* Remove IOPF handler */
+	if (ppr)
+		amd_iommu_iopf_remove_device(iommu, dev_data);
+
 	if (dev_is_pci(dev))
 		pdev_disable_caps(to_pci_dev(dev));
 
@@ -2816,9 +2836,11 @@ static const struct iommu_dirty_ops amd_dirty_ops = {
 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_IOPF:
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -2829,9 +2851,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_IOPF:
+		break;
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/drivers/iommu/amd/ppr.c b/drivers/iommu/amd/ppr.c
index 46d0ad13ac08..dca4bf7f704e 100644
--- a/drivers/iommu/amd/ppr.c
+++ b/drivers/iommu/amd/ppr.c
@@ -241,3 +241,44 @@ void amd_iommu_page_response(struct device *dev, struct iopf_fault *evt,
 {
 	amd_iommu_complete_ppr(dev, resp->pasid, resp->code, resp->grpid);
 }
+
+int amd_iommu_iopf_add_device(struct amd_iommu *iommu,
+			      struct iommu_dev_data *dev_data)
+{
+	unsigned long flags;
+	int ret = 0;
+
+	if (!dev_data->pri_enabled)
+		return ret;
+
+	raw_spin_lock_irqsave(&iommu->lock, flags);
+
+	if (!iommu->iopf_queue) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	ret = iopf_queue_add_device(iommu->iopf_queue, dev_data->dev);
+	if (ret)
+		goto out_unlock;
+
+	dev_data->ppr = true;
+
+out_unlock:
+	raw_spin_unlock_irqrestore(&iommu->lock, flags);
+	return ret;
+}
+
+/* Its assumed that caller has verified that device was added to iopf queue */
+void amd_iommu_iopf_remove_device(struct amd_iommu *iommu,
+				  struct iommu_dev_data *dev_data)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&iommu->lock, flags);
+
+	iopf_queue_remove_device(iommu->iopf_queue, dev_data->dev);
+	dev_data->ppr = false;
+
+	raw_spin_unlock_irqrestore(&iommu->lock, flags);
+}
-- 
2.31.1

[PATCH v6 13/15] iommu/amd: Initial SVA support for AMD IOMMU

[Vasant Hegde]

This includes :
  - Add data structure to track per protection domain dev/pasid binding details
    protection_domain->dev_data_list will track attached list of
    dev_data/PASIDs.

  - Move 'to_pdomain()' to header file

  - Add iommu_sva_set_dev_pasid(). It will check whether PASID is supported
    or not. Also adds PASID to SVA protection domain list as well as to
    device GCR3 table.

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

  - 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,
invalidation, etc. In SVA mode it will use per-device-domain-ID. Hence in
invalidation path we retrieve domain ID from gcr3_info_table 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       |  11 +++
 drivers/iommu/amd/amd_iommu_types.h |  19 +++++
 drivers/iommu/amd/iommu.c           |   7 +-
 drivers/iommu/amd/pasid.c           | 122 ++++++++++++++++++++++++++++
 6 files changed, 156 insertions(+), 6 deletions(-)
 create mode 100644 drivers/iommu/amd/pasid.c

diff --git a/drivers/iommu/amd/Kconfig b/drivers/iommu/amd/Kconfig
index d563f6d496ca..68d8fc107cb9 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
 	select IOMMU_IOPF
 	select IOMMUFD_DRIVER if IOMMUFD
 	depends on X86_64 && PCI && ACPI && HAVE_CMPXCHG_DOUBLE
diff --git a/drivers/iommu/amd/Makefile b/drivers/iommu/amd/Makefile
index 93b11b6d764f..9de33b2d42f5 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 ppr.o
+obj-$(CONFIG_AMD_IOMMU) += iommu.o init.o quirks.o io_pgtable.o io_pgtable_v2.o ppr.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 384298206a2d..ceecc18279ab 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -44,6 +44,12 @@ extern int amd_iommu_guest_ir;
 extern enum io_pgtable_fmt amd_iommu_pgtable;
 extern int amd_iommu_gpt_level;
 
+/* Protection domain ops */
+int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
+			    struct device *dev, ioasid_t pasid);
+void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid);
+
+/* SVA/PASID */
 bool amd_iommu_pasid_supported(void);
 
 /* IOPF */
@@ -187,6 +193,11 @@ static inline struct amd_iommu *get_amd_iommu_from_dev_data(struct iommu_dev_dat
 	return iommu_get_iommu_dev(dev_data->dev, 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 de637006dc82..1ae2a9b3e383 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>
@@ -511,6 +513,11 @@ extern struct kmem_cache *amd_iommu_irq_cache;
 	list_for_each_entry((iommu), &amd_iommu_list, list)
 #define for_each_iommu_safe(iommu, next) \
 	list_for_each_entry_safe((iommu), (next), &amd_iommu_list, list)
+/* Making iterating over protection_domain->dev_data_list easier */
+#define for_each_pdom_dev_data(pdom_dev_data, pdom) \
+	list_for_each_entry(pdom_dev_data, &pdom->dev_data_list, list)
+#define for_each_pdom_dev_data_safe(pdom_dev_data, next, pdom) \
+	list_for_each_entry_safe((pdom_dev_data), (next), &pdom->dev_data_list, list)
 
 #define APERTURE_RANGE_SHIFT	27	/* 128 MB */
 #define APERTURE_RANGE_SIZE	(1ULL << APERTURE_RANGE_SHIFT)
@@ -560,6 +567,16 @@ enum protection_domain_mode {
 	PD_MODE_V2,
 };
 
+/* Track dev_data/PASID list for the protection domain */
+struct pdom_dev_data {
+	/* Points to attached device data */
+	struct iommu_dev_data *dev_data;
+	/* PASID attached to the protection domain */
+	ioasid_t pasid;
+	/* For protection_domain->dev_data_list */
+	struct list_head list;
+};
+
 /*
  * This structure contains generic data for  IOMMU protection domains
  * independent of their use.
@@ -576,6 +593,8 @@ struct protection_domain {
 	bool dirty_tracking;	/* dirty tracking is enabled in the domain */
 	unsigned dev_cnt;	/* devices assigned to this domain */
 	unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
+
+	struct list_head dev_data_list; /* List of pdom_dev_data */
 };
 
 /*
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 21cc6654410f..36ae01d53bfa 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -198,11 +198,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;
@@ -2348,6 +2343,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->dev_data_list);
 	domain->nid = NUMA_NO_NODE;
 
 	switch (type) {
@@ -2877,6 +2873,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,
 	.page_response = amd_iommu_page_response,
 	.default_domain_ops = &(const struct iommu_domain_ops) {
 		.attach_dev	= amd_iommu_attach_device,
diff --git a/drivers/iommu/amd/pasid.c b/drivers/iommu/amd/pasid.c
new file mode 100644
index 000000000000..c96384ae3442
--- /dev/null
+++ b/drivers/iommu/amd/pasid.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 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 inline bool is_pasid_enabled(struct iommu_dev_data *dev_data)
+{
+	if (dev_data->pasid_enabled && dev_data->max_pasids &&
+	    dev_data->gcr3_info.gcr3_tbl != NULL)
+		return true;
+
+	return false;
+}
+
+static inline bool is_pasid_valid(struct iommu_dev_data *dev_data,
+				  ioasid_t pasid)
+{
+	if (pasid > 0 && pasid < dev_data->max_pasids)
+		return true;
+
+	return false;
+}
+
+static void remove_dev_pasid(struct pdom_dev_data *pdom_dev_data)
+{
+	/* Update GCR3 table and flush IOTLB */
+	amd_iommu_clear_gcr3(pdom_dev_data->dev_data, pdom_dev_data->pasid);
+
+	list_del(&pdom_dev_data->list);
+	kfree(pdom_dev_data);
+}
+
+/* Clear PASID from device GCR3 table and remove pdom_dev_data from list */
+static void remove_pdom_dev_pasid(struct protection_domain *pdom,
+				  struct device *dev, ioasid_t pasid)
+{
+	struct pdom_dev_data *pdom_dev_data;
+	struct iommu_dev_data *dev_data = dev_iommu_priv_get(dev);
+
+	lockdep_assert_held(&pdom->lock);
+
+	for_each_pdom_dev_data(pdom_dev_data, pdom) {
+		if (pdom_dev_data->dev_data == dev_data &&
+		    pdom_dev_data->pasid == pasid) {
+			remove_dev_pasid(pdom_dev_data);
+			break;
+		}
+	}
+}
+
+int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
+			    struct device *dev, ioasid_t pasid)
+{
+	struct pdom_dev_data *pdom_dev_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 (!is_pasid_valid(dev_data, pasid))
+		return ret;
+
+	/* Make sure PASID is enabled */
+	if (!is_pasid_enabled(dev_data))
+		return ret;
+
+	/* Add PASID to protection domain pasid list */
+	pdom_dev_data = kzalloc(sizeof(*pdom_dev_data), GFP_KERNEL);
+	if (pdom_dev_data == NULL)
+		return ret;
+
+	pdom_dev_data->pasid = pasid;
+	pdom_dev_data->dev_data = dev_data;
+
+	spin_lock_irqsave(&sva_pdom->lock, flags);
+
+	/* Setup GCR3 table */
+	ret = amd_iommu_set_gcr3(dev_data, pasid,
+				 iommu_virt_to_phys(domain->mm->pgd));
+	if (ret) {
+		kfree(pdom_dev_data);
+		goto out_unlock;
+	}
+
+	list_add(&pdom_dev_data->list, &sva_pdom->dev_data_list);
+
+out_unlock:
+	spin_unlock_irqrestore(&sva_pdom->lock, flags);
+	return ret;
+}
+
+void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
+{
+	struct protection_domain *sva_pdom;
+	struct iommu_domain *domain;
+	unsigned long flags;
+
+	if (!is_pasid_valid(dev_iommu_priv_get(dev), pasid))
+		return;
+
+	/* Get protection domain */
+	domain = iommu_get_domain_for_dev_pasid(dev, pasid, IOMMU_DOMAIN_SVA);
+	if (!domain)
+		return;
+	sva_pdom = to_pdomain(domain);
+
+	spin_lock_irqsave(&sva_pdom->lock, flags);
+
+	/* Remove PASID from dev_data_list */
+	remove_pdom_dev_pasid(sva_pdom, dev, pasid);
+
+	spin_unlock_irqrestore(&sva_pdom->lock, flags);
+}
-- 
2.31.1

[PATCH v6 14/15] iommu: Add ops->domain_alloc_sva()

[Vasant Hegde]

From: Jason Gunthorpe <jgg@nvidia.com>

Make a new op that receives the device and the mm_struct that the SVA
domain should be created for. Unlike domain_alloc_paging() the dev
argument is never NULL here.

This allows drivers to fully initialize the SVA domain and allocate the
mmu_notifier during allocation. It allows the notifier lifetime to follow
the lifetime of the iommu_domain.

Since we have only one call site, upgrade the new op to return ERR_PTR
instead of NULL.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
[Removed smmu3 related changes - Vasant]
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Tina Zhang <tina.zhang@intel.com>
---
 drivers/iommu/iommu-sva.c | 16 +++++++++++-----
 include/linux/iommu.h     |  3 +++
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index b51995b4fe90..c2e3a083ea43 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -99,8 +99,8 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
 
 	/* Allocate a new domain and set it on device pasid. */
 	domain = iommu_sva_domain_alloc(dev, mm);
-	if (!domain) {
-		ret = -ENOMEM;
+	if (IS_ERR(domain)) {
+		ret = PTR_ERR(domain);
 		goto out_free_handle;
 	}
 
@@ -266,9 +266,15 @@ struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
 	const struct iommu_ops *ops = dev_iommu_ops(dev);
 	struct iommu_domain *domain;
 
-	domain = ops->domain_alloc(IOMMU_DOMAIN_SVA);
-	if (!domain)
-		return NULL;
+	if (ops->domain_alloc_sva) {
+		domain = ops->domain_alloc_sva(dev, mm);
+		if (IS_ERR(domain))
+			return domain;
+	} else {
+		domain = ops->domain_alloc(IOMMU_DOMAIN_SVA);
+		if (!domain)
+			return ERR_PTR(-ENOMEM);
+	}
 
 	domain->type = IOMMU_DOMAIN_SVA;
 	mmgrab(mm);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index de839fd01bb8..d76ffd4701c9 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -518,6 +518,7 @@ static inline int __iommu_copy_struct_from_user_array(
  *                     Upon failure, ERR_PTR must be returned.
  * @domain_alloc_paging: Allocate an iommu_domain that can be used for
  *                       UNMANAGED, DMA, and DMA_FQ domain types.
+ * @domain_alloc_sva: Allocate an iommu_domain for Shared Virtual Addressing.
  * @probe_device: Add device to iommu driver handling
  * @release_device: Remove device from iommu driver handling
  * @probe_finalize: Do final setup work after the device is added to an IOMMU
@@ -558,6 +559,8 @@ struct iommu_ops {
 		struct device *dev, u32 flags, struct iommu_domain *parent,
 		const struct iommu_user_data *user_data);
 	struct iommu_domain *(*domain_alloc_paging)(struct device *dev);
+	struct iommu_domain *(*domain_alloc_sva)(struct device *dev,
+						 struct mm_struct *mm);
 
 	struct iommu_device *(*probe_device)(struct device *dev);
 	void (*release_device)(struct device *dev);
-- 
2.31.1

[PATCH v6 15/15] iommu/amd: Add SVA domain support

[Vasant Hegde]

- Allocate SVA domain and setup mmu notifier. In free path unregister
  mmu notifier and free protection domain.

- Add mmu notifier callback function. It will retrieve SVA protection
  domain and invalidates IO/TLB.

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/amd/amd_iommu.h       |  5 ++
 drivers/iommu/amd/amd_iommu_types.h |  1 +
 drivers/iommu/amd/iommu.c           | 10 ++--
 drivers/iommu/amd/pasid.c           | 80 +++++++++++++++++++++++++++++
 4 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index ceecc18279ab..5f90082316b5 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -45,6 +45,11 @@ extern enum io_pgtable_fmt amd_iommu_pgtable;
 extern int amd_iommu_gpt_level;
 
 /* Protection domain ops */
+struct protection_domain *protection_domain_alloc(unsigned int type);
+void protection_domain_free(struct protection_domain *domain);
+struct iommu_domain *amd_iommu_domain_alloc_sva(struct device *dev,
+						struct mm_struct *mm);
+void amd_iommu_domain_free(struct iommu_domain *dom);
 int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
 			    struct device *dev, ioasid_t pasid);
 void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid);
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 1ae2a9b3e383..1c5ee8ebab49 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -594,6 +594,7 @@ struct protection_domain {
 	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 dev_data_list; /* List of pdom_dev_data */
 };
 
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 36ae01d53bfa..f46608ddd13d 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2283,7 +2283,7 @@ static void cleanup_domain(struct protection_domain *domain)
 	WARN_ON(domain->dev_cnt != 0);
 }
 
-static void protection_domain_free(struct protection_domain *domain)
+void protection_domain_free(struct protection_domain *domain)
 {
 	if (!domain)
 		return;
@@ -2326,7 +2326,7 @@ static int protection_domain_init_v2(struct protection_domain *pdom)
 	return 0;
 }
 
-static struct protection_domain *protection_domain_alloc(unsigned int type)
+struct protection_domain *protection_domain_alloc(unsigned int type)
 {
 	struct io_pgtable_ops *pgtbl_ops;
 	struct protection_domain *domain;
@@ -2349,6 +2349,7 @@ static struct protection_domain *protection_domain_alloc(unsigned int type)
 	switch (type) {
 	/* No need to allocate io pgtable ops in passthrough mode */
 	case IOMMU_DOMAIN_IDENTITY:
+	case IOMMU_DOMAIN_SVA:
 		return domain;
 	case IOMMU_DOMAIN_DMA:
 		pgtable = amd_iommu_pgtable;
@@ -2468,7 +2469,7 @@ amd_iommu_domain_alloc_user(struct device *dev, u32 flags,
 	return do_iommu_domain_alloc(type, dev, flags);
 }
 
-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;
@@ -2836,6 +2837,7 @@ static int amd_iommu_dev_enable_feature(struct device *dev,
 
 	switch (feat) {
 	case IOMMU_DEV_FEAT_IOPF:
+	case IOMMU_DEV_FEAT_SVA:
 		break;
 	default:
 		ret = -EINVAL;
@@ -2851,6 +2853,7 @@ static int amd_iommu_dev_disable_feature(struct device *dev,
 
 	switch (feat) {
 	case IOMMU_DEV_FEAT_IOPF:
+	case IOMMU_DEV_FEAT_SVA:
 		break;
 	default:
 		ret = -EINVAL;
@@ -2863,6 +2866,7 @@ const struct iommu_ops amd_iommu_ops = {
 	.capable = amd_iommu_capable,
 	.domain_alloc = amd_iommu_domain_alloc,
 	.domain_alloc_user = amd_iommu_domain_alloc_user,
+	.domain_alloc_sva = amd_iommu_domain_alloc_sva,
 	.probe_device = amd_iommu_probe_device,
 	.release_device = amd_iommu_release_device,
 	.probe_finalize = amd_iommu_probe_finalize,
diff --git a/drivers/iommu/amd/pasid.c b/drivers/iommu/amd/pasid.c
index c96384ae3442..04beb2217d93 100644
--- a/drivers/iommu/amd/pasid.c
+++ b/drivers/iommu/amd/pasid.c
@@ -56,6 +56,49 @@ static void remove_pdom_dev_pasid(struct protection_domain *pdom,
 	}
 }
 
+static void sva_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
+				    struct mm_struct *mm,
+				    unsigned long start, unsigned long end)
+{
+	struct pdom_dev_data *pdom_dev_data;
+	struct protection_domain *sva_pdom;
+	unsigned long flags;
+
+	sva_pdom = container_of(mn, struct protection_domain, mn);
+
+	spin_lock_irqsave(&sva_pdom->lock, flags);
+
+	for_each_pdom_dev_data(pdom_dev_data, sva_pdom) {
+		amd_iommu_dev_flush_pasid_pages(pdom_dev_data->dev_data,
+						pdom_dev_data->pasid,
+						start, end - start);
+	}
+
+	spin_unlock_irqrestore(&sva_pdom->lock, flags);
+}
+
+static void sva_mn_release(struct mmu_notifier *mn, struct mm_struct *mm)
+{
+	struct pdom_dev_data *pdom_dev_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 dev_data_list contains same PASID with different devices */
+	for_each_pdom_dev_data_safe(pdom_dev_data, next, sva_pdom)
+		remove_dev_pasid(pdom_dev_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,
+};
+
 int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
 			    struct device *dev, ioasid_t pasid)
 {
@@ -120,3 +163,40 @@ void amd_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 
 	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 iommu_domain *amd_iommu_domain_alloc_sva(struct device *dev,
+						struct mm_struct *mm)
+{
+	struct protection_domain *pdom;
+	int ret;
+
+	pdom = protection_domain_alloc(IOMMU_DOMAIN_SVA);
+	if (!pdom)
+		return ERR_PTR(-ENOMEM);
+
+	pdom->domain.ops = &amd_sva_domain_ops;
+	pdom->mn.ops = &sva_mn;
+
+	ret = mmu_notifier_register(&pdom->mn, mm);
+	if (ret) {
+		protection_domain_free(pdom);
+		return ERR_PTR(ret);
+	}
+
+	return &pdom->domain;
+}
-- 
2.31.1

Re: [PATCH v6 06/15] iommu/amd: Introduce iommu_dev_data.max_pasids

[Jason Gunthorpe]

On Fri, Feb 09, 2024 at 11:29:21AM +0000, Vasant Hegde wrote:
> This variable will track the number of PASIDs supported by the device.
> If IOMMU or device doesn't support PASID then it will be zero.
> 
> This will be used while allocating GCR3 table to decide required number
> of PASID table levels. Also in PASID bind path it will use this variable
> to check whether device supports PASID or not.
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/amd/amd_iommu_types.h |  1 +
>  drivers/iommu/amd/iommu.c           | 12 ++++++++++++
>  2 files changed, 13 insertions(+)

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

Jason

Re: [PATCH v6 07/15] iommu/amd: Setup GCR3 table in advance if domain is SVA capable

[Jason Gunthorpe]

On Fri, Feb 09, 2024 at 11:29:22AM +0000, Vasant Hegde wrote:
> SVA can be supported if domain is in passthrough mode or paging domain
> with v2 page table. Current code sets up GCR3 table for domain with v2
> page table only. Setup GCR3 table for all SVA capable domains.
> 
>   - Move GCR3 init/destroy to separate function.
> 
>   - Change default GCR3 table to use MAX supported PASIDs. Ideally it
>     should use 1 level PASID table as its using PASID zero only. But we
>     don't have support to extend PASID table yet. We will fix this later.
> 
>   - When domain is configured with passthrough mode, allocate default GCR3
>     table only if device is SVA capable.
> 
> Note that in attach_device() path it will not know whether device will use
> SVA or not. If device is attached to passthrough domain and if it doesn't
> use SVA then GCR3 table will never be used. We will endup wasting memory
> allocated for GCR3 table. This is done to avoid DTE update when
> attaching PASID to device.

It is certainly not elegant, but I can appreciate why you don't want
to tackle the DTE update right now.

> +/*
> + * If domain is SVA capable then initialize GCR3 table. Also if domain is
> + * in v2 page table mode then update GCR3[0].
> + */
> +static int init_gcr3_table(struct iommu_dev_data *dev_data,
> +			   struct protection_domain *pdom)
> +{
> +	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
> +	int max_pasids = dev_data->max_pasids;
> +	int ret = 0;
> +
> +	 /*
> +	  * If domain is in pt mode then setup GCR3 table only if device
> +	  * is PASID capable
> +	  */
> +	if (pdom_is_in_pt_mode(pdom) && !pdev_pasid_supported(dev_data))
> +		return ret;

This logic seems like it would be clearer as:

if (WARN_ON(gcr3_info->gcr3_tbl))
   return -EINVAL;
if (pdom->domain.type == IOMMU_DOMAIN_IDENTITIY)
    max_pasids = dev_data->max_pasids;
else if (pdom_is_v2_pgtbl_mode(pdom))
    max_pasids = min(1, dev_data->pasids);
else
    max_pasids = 0;

if (!max_pasids)
    return 0;

ret = setup_gcr3_table(&dev_data->gcr3_info, iommu, max_pasids)
if (ret)
   return ret;
return 0;

> +static void destroy_gcr3_table(struct iommu_dev_data *dev_data,
> +			       struct protection_domain *pdom)
> +{
> +	struct gcr3_tbl_info *gcr3_info = &dev_data->gcr3_info;
> +
> +	if (pdom_is_v2_pgtbl_mode(pdom))
> +		update_gcr3(dev_data, 0, 0, false);

This doesn't make alot of sense, we are about to free this table
memory, why zero it and flush? If the DTE still points here we are in
trouble!

Similar comment for storing the gcr3 in init_gcr3_table(), that should
just stay in the caller..

> @@ -2010,10 +2068,8 @@ static void do_detach(struct iommu_dev_data *dev_data)
>  	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
>  
>  	/* Clear GCR3 table */
> -	if (domain->pd_mode == PD_MODE_V2) {
> -		update_gcr3(dev_data, 0, 0, false);
> -		free_gcr3_table(&dev_data->gcr3_info);
> -	}
> +	if (pdom_is_sva_capable(domain))
> +		destroy_gcr3_table(dev_data, domain);

if (dev_data->gcr3_info)
	destroy_gcr3_table(dev_data, domain);

?

Don't really care how we got here..

Anyhow, this looks like the right thing in the big picture, nit picks
aside, so:

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

Jason

Re: [PATCH v6 08/15] iommu/amd: Enable PCI features based on attached domain capability

[Jason Gunthorpe]

On Fri, Feb 09, 2024 at 11:29:23AM +0000, Vasant Hegde wrote:
> Commit eda8c2860ab ("iommu/amd: Enable device ATS/PASID/PRI capabilities
> independently") changed the way it enables device capability while
> attaching devices. I missed to account the attached domain capability.
> Meaning if domain is not capable of handling PASID/PRI (ex: paging
> domain with v1 page table) then enabling device feature is not required.
> 
> This patch enables PASID/PRI only if domain is capable of handling SVA.
> Also move pci feature enablement to do_attach() function so that we make
> SVA capability in one place. Finally make PRI enable/disable functions as
> static functions.

> @@ -2036,6 +2038,7 @@ static int do_attach(struct iommu_dev_data *dev_data,
>  		     struct protection_domain *domain)
>  {
>  	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
> +	struct pci_dev *pdev;
>  	int ret = 0;
>  
>  	/* Update data structures */
> @@ -2050,10 +2053,16 @@ static int do_attach(struct iommu_dev_data *dev_data,
>  	domain->dev_iommu[iommu->index] += 1;
>  	domain->dev_cnt                 += 1;
>  
> +	pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
>  	if (pdom_is_sva_capable(domain)) {
>  		ret = init_gcr3_table(dev_data, domain);
>  		if (ret)
>  			return ret;
> +
> +		if (pdev)
> +			pdev_enable_caps(pdev);

But here we are turning PRI on for an IDENTITY domain and for a v2
domain that doesn't have a PRI handler. This is not technically what
we want to see, the PRI capability at the device should only be turned
on when a PRI handler is available in the SW side. Otherwise PRI
should be off and non-present ATS responses should fail in the device.

ATS is not like that, ATS should be enabled whenever the system iommu
can support it. For AMD I guess that is always?

Also, PASID enablement looks like the wrong spot too, it should be
done in the probe_device() callback and stay enabled. The bit is set
if the IOMMU HW can decode the PASID TLP. Ie don't enable it on old
IOMMU HW that can't understand PASID.

However all of this is basically pre-existing problems at this point
and this patch is improving things as they are so

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

Jason

Re: [PATCH v6 09/15] iommu/amd: Define per-IOMMU iopf_queue

[Jason Gunthorpe]

On Fri, Feb 09, 2024 at 11:29:24AM +0000, Vasant Hegde wrote:
> 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.
> 
> Also rename enable_iommus_v2() -> enable_iommus_ppr() to reflect its
> usage. And add amd_iommu_gt_ppr_supported() check before enabling 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/amd_iommu.h       |  5 +++++
>  drivers/iommu/amd/amd_iommu_types.h |  4 ++++
>  drivers/iommu/amd/init.c            | 18 +++++++++++++++--
>  drivers/iommu/amd/ppr.c             | 31 +++++++++++++++++++++++++++++
>  4 files changed, 56 insertions(+), 2 deletions(-)

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

Jason

Re: [PATCH v6 10/15] iommu/amd: Add support for page response

[Jason Gunthorpe]

On Fri, Feb 09, 2024 at 11:29:25AM +0000, Vasant Hegde wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> This generates AMD IOMMU COMPLETE_PPR_REQUEST for the specified device
> with the specified PRI Response Code.
> 
> Also update amd_iommu_complete_ppr() to accept 'struct device' instead
> of pdev as it just need device reference.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@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 | 5 +++--
>  drivers/iommu/amd/iommu.c     | 8 ++++----
>  drivers/iommu/amd/ppr.c       | 6 ++++++
>  3 files changed, 13 insertions(+), 6 deletions(-)

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

Jason

Re: [PATCH v6 11/15] iommu/amd: Add IO page fault notifier handler

[Jason Gunthorpe]

On Fri, Feb 09, 2024 at 11:29:26AM +0000, Vasant Hegde wrote:

> +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_dbg(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;

Someday this will not be an error..

> +static void iommu_call_iopf_notifier(struct amd_iommu *iommu, u64 *raw)
> +{
> +	struct iommu_dev_data *dev_data;
> +	struct iopf_fault event;
> +	struct pci_dev *pdev;
> +	u16 devid = PPR_DEVID(raw[0]);
> +
> +	if (PPR_REQ_TYPE(raw[0]) != PPR_REQ_FAULT) {
> +		pr_info_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
> +	 */
> +	dev_data = dev_iommu_priv_get(&pdev->dev);
> +	if (event.fault.prm.pasid == 0 ||
> +	    event.fault.prm.pasid >= dev_data->max_pasids) {
> +		pr_info_ratelimited("Invalid PASID : 0x%x, device : 0x%x\n",
> +				    event.fault.prm.pasid, pdev->dev.id);
> +		goto out;
> +	}

Why even do this check? The core code is perfectly fine to take in a
big pasid value, it will not match anything in the xarray and just be
completed with error anyhow.

Looks fine otherwise

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

Jason

Re: [PATCH v6 12/15] iommu/amd: Add support for enable/disable IOPF

[Jason Gunthorpe]

On Fri, Feb 09, 2024 at 11:29:27AM +0000, Vasant Hegde wrote:
> diff --git a/drivers/iommu/amd/Kconfig b/drivers/iommu/amd/Kconfig
> index 443b2c13c37b..d563f6d496ca 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_IOPF

Makes me wonder if the previous patches actually compile fully?

> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 0fa172e0583d..21cc6654410f 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2058,12 +2058,22 @@ static int do_attach(struct iommu_dev_data *dev_data,
>  		if (ret)
>  			return ret;
>  
> -		if (pdev)
> +		if (pdev) {
>  			pdev_enable_caps(pdev);
> +
> +			/*
> +			 * Device can continue to function even if IOPF
> +			 * enablement failed. Hence in error path just
> +			 * disable device PRI support.
> +			 */
> +			if (amd_iommu_iopf_add_device(iommu, dev_data))
> +				pdev_disable_cap_pri(pdev);
> +		}
>  	} else if (pdev) {
>  		pdev_enable_cap_ats(pdev);
>  	}
>  
> +
>  	/* Update device table */

Extra whitespace

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

I already remarked about enabling PRI in the wrong spot..

Jason

Re: [PATCH v6 13/15] iommu/amd: Initial SVA support for AMD IOMMU

[Jason Gunthorpe]

On Fri, Feb 09, 2024 at 11:29:28AM +0000, Vasant Hegde wrote:
> @@ -560,6 +567,16 @@ enum protection_domain_mode {
>  	PD_MODE_V2,
>  };
>  
> +/* Track dev_data/PASID list for the protection domain */
> +struct pdom_dev_data {
> +	/* Points to attached device data */
> +	struct iommu_dev_data *dev_data;
> +	/* PASID attached to the protection domain */
> +	ioasid_t pasid;
> +	/* For protection_domain->dev_data_list */
> +	struct list_head list;

'item' is a much better name than 'list'

> +int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
> +			    struct device *dev, ioasid_t pasid)
> +{
> +	struct pdom_dev_data *pdom_dev_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 (!is_pasid_valid(dev_data, pasid))
> +		return ret;
> +
> +	/* Make sure PASID is enabled */
> +	if (!is_pasid_enabled(dev_data))
> +		return ret;
> +
> +	/* Add PASID to protection domain pasid list */
> +	pdom_dev_data = kzalloc(sizeof(*pdom_dev_data), GFP_KERNEL);
> +	if (pdom_dev_data == NULL)
> +		return ret;
> +
> +	pdom_dev_data->pasid = pasid;
> +	pdom_dev_data->dev_data = dev_data;
> +
> +	spin_lock_irqsave(&sva_pdom->lock, flags);
> +
> +	/* Setup GCR3 table */
> +	ret = amd_iommu_set_gcr3(dev_data, pasid,
> +				 iommu_virt_to_phys(domain->mm->pgd));
> +	if (ret) {
> +		kfree(pdom_dev_data);
> +		goto out_unlock;

BTW, I'm not confident in any of the error unwinds around command
execution failure. The sync failed, it doesn't mean the HW didn't
already load the new CD table entry. The driver is basically totally
wrecked at this point as it can't assume the new entry hasn't been
read and it can't assume the old entry is flushed out. :|

Otherwise looks good:

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

Jason

Re: [PATCH v6 00/15] iommu/amd: SVA Support (Part 4) - SVA and IOPF

[Jason Gunthorpe]

On Fri, Feb 09, 2024 at 11:29:15AM +0000, Vasant Hegde wrote:
> Jason Gunthorpe (1):
>   iommu: Add ops->domain_alloc_sva()
> 
> Suravee Suthikulpanit (3):
>   iommu/amd: Move PPR-related functions into ppr.c
>   iommu/amd: Define per-IOMMU iopf_queue
>   iommu/amd: Add support for page response
> 
> Vasant Hegde (9):
>   iommu/amd: Rename amd_iommu_v2_supported() as
>     amd_iommu_pasid_supported()
>   iommu/amd: Introduce per device DTE update function
>   iommu/amd: Fix PPR interrupt processing logic
>   iommu/amd: Introduce iommu_dev_data.max_pasids
>   iommu/amd: Setup GCR3 table in advance if domain is SVA capable
>   iommu/amd: Enable PCI features based on attached domain capability
>   iommu/amd: Add support for enable/disable IOPF
>   iommu/amd: Initial SVA support for AMD IOMMU
>   iommu/amd: Add SVA domain support
> 
> Wei Huang (2):
>   iommu/amd: Add support for enabling/disabling IOMMU features
>   iommu/amd: Add IO page fault notifier handler

There is still more to be done in the drievr, but this looks
structured well now that it should go ahead. I think you need to
rebase this on the Joerg's latest tree?

Either way, fingers crossed for this cycle, sorry to have been so late
with this.

Jason

Re: [PATCH v6 00/15] iommu/amd: SVA Support (Part 4) - SVA and IOPF

[Vasant Hegde]

Jason,


On 3/5/2024 6:22 AM, Jason Gunthorpe wrote:
> On Fri, Feb 09, 2024 at 11:29:15AM +0000, Vasant Hegde wrote:
>> Jason Gunthorpe (1):
>>   iommu: Add ops->domain_alloc_sva()
>>
>> Suravee Suthikulpanit (3):
>>   iommu/amd: Move PPR-related functions into ppr.c
>>   iommu/amd: Define per-IOMMU iopf_queue
>>   iommu/amd: Add support for page response
>>
>> Vasant Hegde (9):
>>   iommu/amd: Rename amd_iommu_v2_supported() as
>>     amd_iommu_pasid_supported()
>>   iommu/amd: Introduce per device DTE update function
>>   iommu/amd: Fix PPR interrupt processing logic
>>   iommu/amd: Introduce iommu_dev_data.max_pasids
>>   iommu/amd: Setup GCR3 table in advance if domain is SVA capable
>>   iommu/amd: Enable PCI features based on attached domain capability
>>   iommu/amd: Add support for enable/disable IOPF
>>   iommu/amd: Initial SVA support for AMD IOMMU
>>   iommu/amd: Add SVA domain support
>>
>> Wei Huang (2):
>>   iommu/amd: Add support for enabling/disabling IOMMU features
>>   iommu/amd: Add IO page fault notifier handler
> 
> There is still more to be done in the drievr, but this looks

Yep. I am hoping to post some of the cleanups and other stuff like BLOCKED
domain soon.


> structured well now that it should go ahead. I think you need to
> rebase this on the Joerg's latest tree?


> 
> Either way, fingers crossed for this cycle, sorry to have been so late
> with this.

Thanks for the review. I will rebase it and post it soon.



-Vasant

Re: [PATCH v6 08/15] iommu/amd: Enable PCI features based on attached domain capability

[Vasant Hegde]

Jason,


On 3/5/2024 6:02 AM, Jason Gunthorpe wrote:
> On Fri, Feb 09, 2024 at 11:29:23AM +0000, Vasant Hegde wrote:
>> Commit eda8c2860ab ("iommu/amd: Enable device ATS/PASID/PRI capabilities
>> independently") changed the way it enables device capability while
>> attaching devices. I missed to account the attached domain capability.
>> Meaning if domain is not capable of handling PASID/PRI (ex: paging
>> domain with v1 page table) then enabling device feature is not required.
>>
>> This patch enables PASID/PRI only if domain is capable of handling SVA.
>> Also move pci feature enablement to do_attach() function so that we make
>> SVA capability in one place. Finally make PRI enable/disable functions as
>> static functions.
> 
>> @@ -2036,6 +2038,7 @@ static int do_attach(struct iommu_dev_data *dev_data,
>>  		     struct protection_domain *domain)
>>  {
>>  	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
>> +	struct pci_dev *pdev;
>>  	int ret = 0;
>>  
>>  	/* Update data structures */
>> @@ -2050,10 +2053,16 @@ static int do_attach(struct iommu_dev_data *dev_data,
>>  	domain->dev_iommu[iommu->index] += 1;
>>  	domain->dev_cnt                 += 1;
>>  
>> +	pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
>>  	if (pdom_is_sva_capable(domain)) {
>>  		ret = init_gcr3_table(dev_data, domain);
>>  		if (ret)
>>  			return ret;
>> +
>> +		if (pdev)
>> +			pdev_enable_caps(pdev);
> 
> But here we are turning PRI on for an IDENTITY domain and for a v2
> domain that doesn't have a PRI handler. This is not technically what
> we want to see, the PRI capability at the device should only be turned
> on when a PRI handler is available in the SW side. Otherwise PRI
> should be off and non-present ATS responses should fail in the device.

We are setting up handler right after enabling capability for both IDENTIFY and
v2 domain as both can support SVA mode.

> 
> ATS is not like that, ATS should be enabled whenever the system iommu
> can support it. For AMD I guess that is always?

Yes. If device supports ATS then its enabled always.


> 
> Also, PASID enablement looks like the wrong spot too, it should be
> done in the probe_device() callback and stay enabled. The bit is set
> if the IOMMU HW can decode the PASID TLP. Ie don't enable it on old
> IOMMU HW that can't understand PASID.

Our V1 page table is not compatible with PASID. Hence currently its done in
attach_device path.

> 
> However all of this is basically pre-existing problems at this point
> and this patch is improving things as they are so

Correct. I will revisit some of these code paths later.

-Vasant

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

Re: [PATCH v6 12/15] iommu/amd: Add support for enable/disable IOPF

[Vasant Hegde]

Jason,


On 3/5/2024 6:12 AM, Jason Gunthorpe wrote:
> On Fri, Feb 09, 2024 at 11:29:27AM +0000, Vasant Hegde wrote:
>> diff --git a/drivers/iommu/amd/Kconfig b/drivers/iommu/amd/Kconfig
>> index 443b2c13c37b..d563f6d496ca 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_IOPF
> 
> Makes me wonder if the previous patches actually compile fully?

It compiles as IOPF related functions are defined in iommu.h. But as expected
IOPF features does not work fine.

> 
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index 0fa172e0583d..21cc6654410f 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -2058,12 +2058,22 @@ static int do_attach(struct iommu_dev_data *dev_data,
>>  		if (ret)
>>  			return ret;
>>  
>> -		if (pdev)
>> +		if (pdev) {
>>  			pdev_enable_caps(pdev);
>> +
>> +			/*
>> +			 * Device can continue to function even if IOPF
>> +			 * enablement failed. Hence in error path just
>> +			 * disable device PRI support.
>> +			 */
>> +			if (amd_iommu_iopf_add_device(iommu, dev_data))
>> +				pdev_disable_cap_pri(pdev);
>> +		}
>>  	} else if (pdev) {
>>  		pdev_enable_cap_ats(pdev);
>>  	}
>>  
>> +
>>  	/* Update device table */
> 
> Extra whitespace

Fixed.

-Vasant

> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> 
> I already remarked about enabling PRI in the wrong spot..
> 
> Jason

Re: [PATCH v6 08/15] iommu/amd: Enable PCI features based on attached domain capability

[Jason Gunthorpe]

On Tue, Mar 05, 2024 at 08:40:08PM +0530, Vasant Hegde wrote:
> Jason,
> 
> 
> On 3/5/2024 6:02 AM, Jason Gunthorpe wrote:
> > On Fri, Feb 09, 2024 at 11:29:23AM +0000, Vasant Hegde wrote:
> >> Commit eda8c2860ab ("iommu/amd: Enable device ATS/PASID/PRI capabilities
> >> independently") changed the way it enables device capability while
> >> attaching devices. I missed to account the attached domain capability.
> >> Meaning if domain is not capable of handling PASID/PRI (ex: paging
> >> domain with v1 page table) then enabling device feature is not required.
> >>
> >> This patch enables PASID/PRI only if domain is capable of handling SVA.
> >> Also move pci feature enablement to do_attach() function so that we make
> >> SVA capability in one place. Finally make PRI enable/disable functions as
> >> static functions.
> > 
> >> @@ -2036,6 +2038,7 @@ static int do_attach(struct iommu_dev_data *dev_data,
> >>  		     struct protection_domain *domain)
> >>  {
> >>  	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
> >> +	struct pci_dev *pdev;
> >>  	int ret = 0;
> >>  
> >>  	/* Update data structures */
> >> @@ -2050,10 +2053,16 @@ static int do_attach(struct iommu_dev_data *dev_data,
> >>  	domain->dev_iommu[iommu->index] += 1;
> >>  	domain->dev_cnt                 += 1;
> >>  
> >> +	pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
> >>  	if (pdom_is_sva_capable(domain)) {
> >>  		ret = init_gcr3_table(dev_data, domain);
> >>  		if (ret)
> >>  			return ret;
> >> +
> >> +		if (pdev)
> >> +			pdev_enable_caps(pdev);
> > 
> > But here we are turning PRI on for an IDENTITY domain and for a v2
> > domain that doesn't have a PRI handler. This is not technically what
> > we want to see, the PRI capability at the device should only be turned
> > on when a PRI handler is available in the SW side. Otherwise PRI
> > should be off and non-present ATS responses should fail in the device.
> 
> We are setting up handler right after enabling capability for both IDENTIFY and
> v2 domain as both can support SVA mode.

It is enabling PRI when the device can enable PRI but before any
driver has actually asked for any PRI features. It is technically too
early.

It should be enabled when a SVA domain is first attached.

Like we don't want the HW to generate PRI faults from a VM
deliberately banging on non-translated page table memory. This is a
pure DOS attack on the hypervisor kernel and should be prevented by
having PRI turned off.

> > Also, PASID enablement looks like the wrong spot too, it should be
> > done in the probe_device() callback and stay enabled. The bit is set
> > if the IOMMU HW can decode the PASID TLP. Ie don't enable it on old
> > IOMMU HW that can't understand PASID.
> 
> Our V1 page table is not compatible with PASID. Hence currently its done in
> attach_device path.

What does "compatible" mean exactly?

The PASID cap on the device is only about the IOMMU HW ability to
properly *parse* PASID, it can fail every PASID tagged TLP and it
would still be fine to turn it on at the device always.

v1 is clearly not able to route PASID to different translations, but
as long as a v1 DTE fails every single PASID tagged TLP you can always
turn PASID on at the source just fine.

It would be a very weird design if v2 HW makes the DTE in v1 mode just
discards the PASID and translates everything with the v1 table. I'd
also wonder if the identity/blocked cases are similarly weird?

IOW, I would expect to see probe_device check if the HW supports V2,
if it does it knows the HW can parse the PASID in the TLP and will
abort PASID's without translations (ie v1 DTEs). Then you turn on
PASID support always. If the HW supports only v1 then assume it
doesn't know how to parse the PASID and will ignore it and fold all
PASID requests into the RID. Leave PASID always off.

Jason

Re: [PATCH v6 08/15] iommu/amd: Enable PCI features based on attached domain capability

[Vasant Hegde]

Jason,


On 3/5/2024 9:31 PM, Jason Gunthorpe wrote:
> On Tue, Mar 05, 2024 at 08:40:08PM +0530, Vasant Hegde wrote:
>> Jason,
>>
>>
>> On 3/5/2024 6:02 AM, Jason Gunthorpe wrote:
>>> On Fri, Feb 09, 2024 at 11:29:23AM +0000, Vasant Hegde wrote:
>>>> Commit eda8c2860ab ("iommu/amd: Enable device ATS/PASID/PRI capabilities
>>>> independently") changed the way it enables device capability while
>>>> attaching devices. I missed to account the attached domain capability.
>>>> Meaning if domain is not capable of handling PASID/PRI (ex: paging
>>>> domain with v1 page table) then enabling device feature is not required.
>>>>
>>>> This patch enables PASID/PRI only if domain is capable of handling SVA.
>>>> Also move pci feature enablement to do_attach() function so that we make
>>>> SVA capability in one place. Finally make PRI enable/disable functions as
>>>> static functions.
>>>
>>>> @@ -2036,6 +2038,7 @@ static int do_attach(struct iommu_dev_data *dev_data,
>>>>  		     struct protection_domain *domain)
>>>>  {
>>>>  	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
>>>> +	struct pci_dev *pdev;
>>>>  	int ret = 0;
>>>>  
>>>>  	/* Update data structures */
>>>> @@ -2050,10 +2053,16 @@ static int do_attach(struct iommu_dev_data *dev_data,
>>>>  	domain->dev_iommu[iommu->index] += 1;
>>>>  	domain->dev_cnt                 += 1;
>>>>  
>>>> +	pdev = dev_is_pci(dev_data->dev) ? to_pci_dev(dev_data->dev) : NULL;
>>>>  	if (pdom_is_sva_capable(domain)) {
>>>>  		ret = init_gcr3_table(dev_data, domain);
>>>>  		if (ret)
>>>>  			return ret;
>>>> +
>>>> +		if (pdev)
>>>> +			pdev_enable_caps(pdev);
>>>
>>> But here we are turning PRI on for an IDENTITY domain and for a v2
>>> domain that doesn't have a PRI handler. This is not technically what
>>> we want to see, the PRI capability at the device should only be turned
>>> on when a PRI handler is available in the SW side. Otherwise PRI
>>> should be off and non-present ATS responses should fail in the device.
>>
>> We are setting up handler right after enabling capability for both IDENTIFY and
>> v2 domain as both can support SVA mode.
> 
> It is enabling PRI when the device can enable PRI but before any
> driver has actually asked for any PRI features. It is technically too
> early.
> 
> It should be enabled when a SVA domain is first attached.
> 

That's true.

That's what I did in one of the iteration (Initially it was in
feature_enable(IOPF), then moved to set_dev_pasid() path and eventually I did it
in attach device path).


> Like we don't want the HW to generate PRI faults from a VM
> deliberately banging on non-translated page table memory. This is a
> pure DOS attack on the hypervisor kernel and should be prevented by
> having PRI turned off.

Currently for passthrough devices we use v1 page table. So its fine for now.
Domain allocation code needs some rework. We will consider this scenario when we
touch that code.


> 
>>> Also, PASID enablement looks like the wrong spot too, it should be
>>> done in the probe_device() callback and stay enabled. The bit is set
>>> if the IOMMU HW can decode the PASID TLP. Ie don't enable it on old
>>> IOMMU HW that can't understand PASID.
>>
>> Our V1 page table is not compatible with PASID. Hence currently its done in
>> attach_device path.
> 
> What does "compatible" mean exactly?
> 
> The PASID cap on the device is only about the IOMMU HW ability to
> properly *parse* PASID, it can fail every PASID tagged TLP and it
> would still be fine to turn it on at the device always.
> 
> v1 is clearly not able to route PASID to different translations, but
> as long as a v1 DTE fails every single PASID tagged TLP you can always
> turn PASID on at the source just fine.
> 
> It would be a very weird design if v2 HW makes the DTE in v1 mode just
> discards the PASID and translates everything with the v1 table. I'd
> also wonder if the identity/blocked cases are similarly weird?

Passthrough is fine. We will set GCR3 and GV bits.

I am still not sure about BLOCKED domain (needs to investigate) as it needs TV
bit to be set to zero.


> 
> IOW, I would expect to see probe_device check if the HW supports V2,
> if it does it knows the HW can parse the PASID in the TLP and will
> abort PASID's without translations (ie v1 DTEs). Then you turn on
> PASID support always. If the HW supports only v1 then assume it
> doesn't know how to parse the PASID and will ignore it and fold all
> PASID requests into the RID. Leave PASID always off.

Ok. I will revisit this later.

-Vasant

Re: [PATCH v6 11/15] iommu/amd: Add IO page fault notifier handler

[Vasant Hegde]

Jason,


On 3/5/2024 6:10 AM, Jason Gunthorpe wrote:
> On Fri, Feb 09, 2024 at 11:29:26AM +0000, Vasant Hegde wrote:
> 
>> +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_dbg(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;
> 
> Someday this will not be an error..

Ok.

> 
>> +static void iommu_call_iopf_notifier(struct amd_iommu *iommu, u64 *raw)
>> +{
>> +	struct iommu_dev_data *dev_data;
>> +	struct iopf_fault event;
>> +	struct pci_dev *pdev;
>> +	u16 devid = PPR_DEVID(raw[0]);
>> +
>> +	if (PPR_REQ_TYPE(raw[0]) != PPR_REQ_FAULT) {
>> +		pr_info_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
>> +	 */
>> +	dev_data = dev_iommu_priv_get(&pdev->dev);
>> +	if (event.fault.prm.pasid == 0 ||
>> +	    event.fault.prm.pasid >= dev_data->max_pasids) {
>> +		pr_info_ratelimited("Invalid PASID : 0x%x, device : 0x%x\n",
>> +				    event.fault.prm.pasid, pdev->dev.id);
>> +		goto out;
>> +	}
> 
> Why even do this check? The core code is perfectly fine to take in a
> big pasid value, it will not match anything in the xarray and just be
> completed with error anyhow.

We just wanted to make sure device/IOMMU can handle PASID or not before passing
it to iommu_report_device_fault().



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

Thanks
-Vasant

Re: [PATCH v6 13/15] iommu/amd: Initial SVA support for AMD IOMMU

[Vasant Hegde]

On 3/5/2024 6:20 AM, Jason Gunthorpe wrote:
> On Fri, Feb 09, 2024 at 11:29:28AM +0000, Vasant Hegde wrote:
>> @@ -560,6 +567,16 @@ enum protection_domain_mode {
>>  	PD_MODE_V2,
>>  };
>>  
>> +/* Track dev_data/PASID list for the protection domain */
>> +struct pdom_dev_data {
>> +	/* Points to attached device data */
>> +	struct iommu_dev_data *dev_data;
>> +	/* PASID attached to the protection domain */
>> +	ioasid_t pasid;
>> +	/* For protection_domain->dev_data_list */
>> +	struct list_head list;
> 
> 'item' is a much better name than 'list'

We have been using 'list' in all structures. So I just kept it same.

> 
>> +int iommu_sva_set_dev_pasid(struct iommu_domain *domain,
>> +			    struct device *dev, ioasid_t pasid)
>> +{
>> +	struct pdom_dev_data *pdom_dev_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 (!is_pasid_valid(dev_data, pasid))
>> +		return ret;
>> +
>> +	/* Make sure PASID is enabled */
>> +	if (!is_pasid_enabled(dev_data))
>> +		return ret;
>> +
>> +	/* Add PASID to protection domain pasid list */
>> +	pdom_dev_data = kzalloc(sizeof(*pdom_dev_data), GFP_KERNEL);
>> +	if (pdom_dev_data == NULL)
>> +		return ret;
>> +
>> +	pdom_dev_data->pasid = pasid;
>> +	pdom_dev_data->dev_data = dev_data;
>> +
>> +	spin_lock_irqsave(&sva_pdom->lock, flags);
>> +
>> +	/* Setup GCR3 table */
>> +	ret = amd_iommu_set_gcr3(dev_data, pasid,
>> +				 iommu_virt_to_phys(domain->mm->pgd));
>> +	if (ret) {
>> +		kfree(pdom_dev_data);
>> +		goto out_unlock;
> 
> BTW, I'm not confident in any of the error unwinds around command
> execution failure. The sync failed, it doesn't mean the HW didn't
> already load the new CD table entry. The driver is basically totally
> wrecked at this point as it can't assume the new entry hasn't been
> read and it can't assume the old entry is flushed out. :|

Yep. This is tricky path.

But for now this particular one just adding PASID to GCR3 table. So if we enter
error path means we didn't add PASID to gcr3 table. So it should work fine.

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

Thanks.
-Vasant

> 
> Jason

Re: [PATCH v6 07/15] iommu/amd: Setup GCR3 table in advance if domain is SVA capable

[Vasant Hegde]

On 3/5/2024 5:41 AM, Jason Gunthorpe wrote:
> On Fri, Feb 09, 2024 at 11:29:22AM +0000, Vasant Hegde wrote:
>> SVA can be supported if domain is in passthrough mode or paging domain
>> with v2 page table. Current code sets up GCR3 table for domain with v2
>> page table only. Setup GCR3 table for all SVA capable domains.
>>
>>   - Move GCR3 init/destroy to separate function.
>>
>>   - Change default GCR3 table to use MAX supported PASIDs. Ideally it
>>     should use 1 level PASID table as its using PASID zero only. But we
>>     don't have support to extend PASID table yet. We will fix this later.
>>
>>   - When domain is configured with passthrough mode, allocate default GCR3
>>     table only if device is SVA capable.
>>
>> Note that in attach_device() path it will not know whether device will use
>> SVA or not. If device is attached to passthrough domain and if it doesn't
>> use SVA then GCR3 table will never be used. We will endup wasting memory
>> allocated for GCR3 table. This is done to avoid DTE update when
>> attaching PASID to device.
> 
> It is certainly not elegant, but I can appreciate why you don't want
> to tackle the DTE update right now.
> 
>> +/*
>> + * If domain is SVA capable then initialize GCR3 table. Also if domain is
>> + * in v2 page table mode then update GCR3[0].
>> + */
>> +static int init_gcr3_table(struct iommu_dev_data *dev_data,
>> +			   struct protection_domain *pdom)
>> +{
>> +	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
>> +	int max_pasids = dev_data->max_pasids;
>> +	int ret = 0;
>> +
>> +	 /*
>> +	  * If domain is in pt mode then setup GCR3 table only if device
>> +	  * is PASID capable
>> +	  */
>> +	if (pdom_is_in_pt_mode(pdom) && !pdev_pasid_supported(dev_data))
>> +		return ret;
> 
> This logic seems like it would be clearer as:
> 
> if (WARN_ON(gcr3_info->gcr3_tbl))
>    return -EINVAL;
> if (pdom->domain.type == IOMMU_DOMAIN_IDENTITIY)
>     max_pasids = dev_data->max_pasids;
> else if (pdom_is_v2_pgtbl_mode(pdom))
>     max_pasids = min(1, dev_data->pasids);
> else
>     max_pasids = 0;
> 
> if (!max_pasids)
>     return 0;
> 
> ret = setup_gcr3_table(&dev_data->gcr3_info, iommu, max_pasids)
> if (ret)
>    return ret;
> return 0;

Will look into it later.

> 
>> +static void destroy_gcr3_table(struct iommu_dev_data *dev_data,
>> +			       struct protection_domain *pdom)
>> +{
>> +	struct gcr3_tbl_info *gcr3_info = &dev_data->gcr3_info;
>> +
>> +	if (pdom_is_v2_pgtbl_mode(pdom))
>> +		update_gcr3(dev_data, 0, 0, false);
> 
> This doesn't make alot of sense, we are about to free this table
> memory, why zero it and flush? If the DTE still points here we are in
> trouble!

This is for GCR3[0].. But yeah. detach path we can just ignore this and free
gcr3 table. Will fix it later.

> 
> Similar comment for storing the gcr3 in init_gcr3_table(), that should
> just stay in the caller..
> 
>> @@ -2010,10 +2068,8 @@ static void do_detach(struct iommu_dev_data *dev_data)
>>  	struct amd_iommu *iommu = get_amd_iommu_from_dev_data(dev_data);
>>  
>>  	/* Clear GCR3 table */
>> -	if (domain->pd_mode == PD_MODE_V2) {
>> -		update_gcr3(dev_data, 0, 0, false);
>> -		free_gcr3_table(&dev_data->gcr3_info);
>> -	}
>> +	if (pdom_is_sva_capable(domain))
>> +		destroy_gcr3_table(dev_data, domain);
> 
> if (dev_data->gcr3_info)
> 	destroy_gcr3_table(dev_data, domain);

Makes sense. I will post separate patch later.

> 
> ?
> 
> Don't really care how we got here..
> 
> Anyhow, this looks like the right thing in the big picture, nit picks
> aside, so:
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Thanks
-Vasant

Re: [PATCH v6 11/15] iommu/amd: Add IO page fault notifier handler

[Jason Gunthorpe]

On Mon, Mar 11, 2024 at 04:30:14PM +0530, Vasant Hegde wrote:
> >> +	/*
> >> +	 * PASID zero is used for requests from the I/O device without
> >> +	 * a PASID
> >> +	 */
> >> +	dev_data = dev_iommu_priv_get(&pdev->dev);
> >> +	if (event.fault.prm.pasid == 0 ||
> >> +	    event.fault.prm.pasid >= dev_data->max_pasids) {
> >> +		pr_info_ratelimited("Invalid PASID : 0x%x, device : 0x%x\n",
> >> +				    event.fault.prm.pasid, pdev->dev.id);
> >> +		goto out;
> >> +	}
> > 
> > Why even do this check? The core code is perfectly fine to take in a
> > big pasid value, it will not match anything in the xarray and just be
> > completed with error anyhow.
> 
> We just wanted to make sure device/IOMMU can handle PASID or not before passing
> it to iommu_report_device_fault().

If it can't handle PASID then the pasid xarray will be empty and it
will be fine.

Jason

Re: [PATCH v6 13/15] iommu/amd: Initial SVA support for AMD IOMMU

[Jason Gunthorpe]

On Mon, Mar 11, 2024 at 04:41:51PM +0530, Vasant Hegde wrote:

> >> +	/* Setup GCR3 table */
> >> +	ret = amd_iommu_set_gcr3(dev_data, pasid,
> >> +				 iommu_virt_to_phys(domain->mm->pgd));
> >> +	if (ret) {
> >> +		kfree(pdom_dev_data);
> >> +		goto out_unlock;
> > 
> > BTW, I'm not confident in any of the error unwinds around command
> > execution failure. The sync failed, it doesn't mean the HW didn't
> > already load the new CD table entry. The driver is basically totally
> > wrecked at this point as it can't assume the new entry hasn't been
> > read and it can't assume the old entry is flushed out. :|
> 
> Yep. This is tricky path.
> 
> But for now this particular one just adding PASID to GCR3 table. So if we enter
> error path means we didn't add PASID to gcr3 table. So it should work fine.

Not quite. The HW could have cached the new GCR3 table entry even
though the invalidation failed.

So on the failure path you have to assume the HW is still using the
GCR3 which the failure path is busy freeing..

There is no way to make this work right. Either kill the kernel on
invalidate failures (ie try forever) or assume that the HW will
eventually become consistent and continue forward..

Jason

Re: [PATCH v6 13/15] iommu/amd: Initial SVA support for AMD IOMMU

[Vasant Hegde]

Jason,


On 3/19/2024 11:26 PM, Jason Gunthorpe wrote:
> On Mon, Mar 11, 2024 at 04:41:51PM +0530, Vasant Hegde wrote:
> 
>>>> +	/* Setup GCR3 table */
>>>> +	ret = amd_iommu_set_gcr3(dev_data, pasid,
>>>> +				 iommu_virt_to_phys(domain->mm->pgd));
>>>> +	if (ret) {
>>>> +		kfree(pdom_dev_data);
>>>> +		goto out_unlock;
>>>
>>> BTW, I'm not confident in any of the error unwinds around command
>>> execution failure. The sync failed, it doesn't mean the HW didn't
>>> already load the new CD table entry. The driver is basically totally
>>> wrecked at this point as it can't assume the new entry hasn't been
>>> read and it can't assume the old entry is flushed out. :|
>>
>> Yep. This is tricky path.
>>
>> But for now this particular one just adding PASID to GCR3 table. So if we enter
>> error path means we didn't add PASID to gcr3 table. So it should work fine.
> 
> Not quite. The HW could have cached the new GCR3 table entry even
> though the invalidation failed.
> 
> So on the failure path you have to assume the HW is still using the
> GCR3 which the failure path is busy freeing..
> 
> There is no way to make this work right. Either kill the kernel on
> invalidate failures (ie try forever) or assume that the HW will
> eventually become consistent and continue forward..

Currently we ignore invalidation return value. So we go to failure path only if
we fail to add PASID to GCR3 table.

-Vasant