[PATCH v3 0/5] iommu/amd: SVA Support (part 2) - deprecate iommu_v2 module

[PATCH v3 0/5] iommu/amd: SVA Support (part 2) - deprecate iommu_v2 module

[Vasant Hegde]

This is part 2 of the 4-part series to introduce Share Virtual Address
(SVA) support. This series removes iommu_v2 module support.

AMD GPU driver which was the only in-kernel user of iommu_v2 module is
removed dependency on iommu_v2 module. These changes are already in
upstream (v6.6 merge window).

Also subsequent series will add SVA spuport in AMD IOMMU driver. Device
drivers are expected to use common SVA framework to enable device
PASID/PRI features.

Hence this series removes iommu_v2 module and the functions used to support
iommu_v2 module. Note that there are certain functions (ex: PPR interrupt
handling code) which will be used in subsequent series for SVA support.
Hence not removed those functions.

This patch series is based on top of SVA Part 1 [1] which in turn based on top of
upstrem v6.6-rc1.
  https://lore.kernel.org/linux-iommu/20230921092147.5930-1-vasant.hegde@amd.com/T/#t

This is also available at github :
  https://github.com/AMDESE/linux/tree/iommu_sva_part2_v3_iommu_v2_v6.6_rc1


Changes from v2 -> v3:
  - Updated description as AMD GPU related changes are merged in v6.6 merge window
  - Few other updated to description based on Jerry's suggestion
  - Added Reviewed-by tags

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

Changes from v1 -> v2:
  - Added patch to revert commit 2380f1e8195e

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


Thank you,
Vasant / Suravee

Vasant Hegde (5):
  iommu/amd: Remove iommu_v2 module
  iommu/amd: Remove PPR support
  iommu/amd: Remove amd_iommu_device_info()
  iommu/amd: Remove unused EXPORT_SYMBOLS
  Revert "iommu: Fix false ownership failure on AMD systems with PASID activated"

 drivers/iommu/amd/Kconfig           |   9 -
 drivers/iommu/amd/Makefile          |   1 -
 drivers/iommu/amd/amd_iommu.h       |   7 -
 drivers/iommu/amd/amd_iommu_types.h |  21 +-
 drivers/iommu/amd/init.c            |   1 -
 drivers/iommu/amd/iommu.c           | 133 +---
 drivers/iommu/amd/iommu_v2.c        | 996 ----------------------------
 drivers/iommu/iommu.c               |  20 +-
 include/linux/amd-iommu.h           | 120 ----
 9 files changed, 10 insertions(+), 1298 deletions(-)
 delete mode 100644 drivers/iommu/amd/iommu_v2.c

-- 
2.31.1

[PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Vasant Hegde]

AMD GPU driver which was the only in-kernel user of iommu_v2 module
removed dependency on iommu_v2 module.

Also we are working on adding SVA support in AMD IOMMU driver. Device
drivers are expected to use common SVA framework to enable device
PASID/PRI features.

Removing iommu_v2 module and then adding SVA simplifies the development.
Hence remove iommu_v2 module.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Felix Kuehling <Felix.Kuehling@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
 drivers/iommu/amd/Kconfig     |   9 -
 drivers/iommu/amd/Makefile    |   1 -
 drivers/iommu/amd/amd_iommu.h |   5 -
 drivers/iommu/amd/iommu.c     |  40 --
 drivers/iommu/amd/iommu_v2.c  | 996 ----------------------------------
 include/linux/amd-iommu.h     |  94 ----
 6 files changed, 1145 deletions(-)
 delete mode 100644 drivers/iommu/amd/iommu_v2.c

diff --git a/drivers/iommu/amd/Kconfig b/drivers/iommu/amd/Kconfig
index 9b5fc3356bf2..75132ae861a2 100644
--- a/drivers/iommu/amd/Kconfig
+++ b/drivers/iommu/amd/Kconfig
@@ -22,15 +22,6 @@ config AMD_IOMMU
 	  your BIOS for an option to enable it or if you have an IVRS ACPI
 	  table.
 
-config AMD_IOMMU_V2
-	tristate "AMD IOMMU Version 2 driver"
-	depends on AMD_IOMMU
-	select MMU_NOTIFIER
-	help
-	  This option enables support for the AMD IOMMUv2 features of the IOMMU
-	  hardware. Select this option if you want to use devices that support
-	  the PCI PRI and PASID interface.
-
 config AMD_IOMMU_DEBUGFS
 	bool "Enable AMD IOMMU internals in DebugFS"
 	depends on AMD_IOMMU && IOMMU_DEBUGFS
diff --git a/drivers/iommu/amd/Makefile b/drivers/iommu/amd/Makefile
index 773d8aa00283..f454fbb1569e 100644
--- a/drivers/iommu/amd/Makefile
+++ b/drivers/iommu/amd/Makefile
@@ -1,4 +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_DEBUGFS) += debugfs.o
-obj-$(CONFIG_AMD_IOMMU_V2) += iommu_v2.o
diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 9df53961d5ef..5b8a1e2dd3d0 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -38,9 +38,6 @@ extern int amd_iommu_guest_ir;
 extern enum io_pgtable_fmt amd_iommu_pgtable;
 extern int amd_iommu_gpt_level;
 
-/* IOMMUv2 specific functions */
-struct iommu_domain;
-
 bool amd_iommu_v2_supported(void);
 struct amd_iommu *get_amd_iommu(unsigned int idx);
 u8 amd_iommu_pc_get_max_banks(unsigned int idx);
@@ -57,8 +54,6 @@ void amd_iommu_pdev_disable_cap_pri(struct pci_dev *pdev);
 
 int amd_iommu_register_ppr_notifier(struct notifier_block *nb);
 int amd_iommu_unregister_ppr_notifier(struct notifier_block *nb);
-void amd_iommu_domain_direct_map(struct iommu_domain *dom);
-int amd_iommu_domain_enable_v2(struct iommu_domain *dom, int pasids);
 int amd_iommu_flush_page(struct iommu_domain *dom, u32 pasid, u64 address);
 void amd_iommu_update_and_flush_device_table(struct protection_domain *domain);
 void amd_iommu_domain_update(struct protection_domain *domain);
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index f3448a2b6c0e..fd0d7b2f30dc 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2568,46 +2568,6 @@ int amd_iommu_unregister_ppr_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL(amd_iommu_unregister_ppr_notifier);
 
-void amd_iommu_domain_direct_map(struct iommu_domain *dom)
-{
-	struct protection_domain *domain = to_pdomain(dom);
-	unsigned long flags;
-
-	spin_lock_irqsave(&domain->lock, flags);
-
-	if (domain->iop.pgtbl_cfg.tlb)
-		free_io_pgtable_ops(&domain->iop.iop.ops);
-
-	spin_unlock_irqrestore(&domain->lock, flags);
-}
-EXPORT_SYMBOL(amd_iommu_domain_direct_map);
-
-int amd_iommu_domain_enable_v2(struct iommu_domain *dom, int pasids)
-{
-	struct protection_domain *pdom = to_pdomain(dom);
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&pdom->lock, flags);
-
-	/*
-	 * Save us all sanity checks whether devices already in the
-	 * domain support IOMMUv2. Just force that the domain has no
-	 * devices attached when it is switched into IOMMUv2 mode.
-	 */
-	ret = -EBUSY;
-	if (pdom->dev_cnt > 0 || pdom->flags & PD_IOMMUV2_MASK)
-		goto out;
-
-	if (!pdom->gcr3_tbl)
-		ret = setup_gcr3_table(pdom, pasids);
-
-out:
-	spin_unlock_irqrestore(&pdom->lock, flags);
-	return ret;
-}
-EXPORT_SYMBOL(amd_iommu_domain_enable_v2);
-
 static int __flush_pasid(struct protection_domain *domain, u32 pasid,
 			 u64 address, bool size)
 {
diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c
deleted file mode 100644
index 57c2fb1146e2..000000000000
--- a/drivers/iommu/amd/iommu_v2.c
+++ /dev/null
@@ -1,996 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (C) 2010-2012 Advanced Micro Devices, Inc.
- * Author: Joerg Roedel <jroedel@suse.de>
- */
-
-#define pr_fmt(fmt)     "AMD-Vi: " fmt
-
-#include <linux/refcount.h>
-#include <linux/mmu_notifier.h>
-#include <linux/amd-iommu.h>
-#include <linux/mm_types.h>
-#include <linux/profile.h>
-#include <linux/module.h>
-#include <linux/sched.h>
-#include <linux/sched/mm.h>
-#include <linux/wait.h>
-#include <linux/pci.h>
-#include <linux/gfp.h>
-#include <linux/cc_platform.h>
-
-#include "amd_iommu.h"
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Joerg Roedel <jroedel@suse.de>");
-
-#define PRI_QUEUE_SIZE		512
-
-struct pri_queue {
-	atomic_t inflight;
-	bool finish;
-	int status;
-};
-
-struct pasid_state {
-	struct list_head list;			/* For global state-list */
-	refcount_t count;				/* Reference count */
-	unsigned mmu_notifier_count;		/* Counting nested mmu_notifier
-						   calls */
-	struct mm_struct *mm;			/* mm_struct for the faults */
-	struct mmu_notifier mn;                 /* mmu_notifier handle */
-	struct pri_queue pri[PRI_QUEUE_SIZE];	/* PRI tag states */
-	struct device_state *device_state;	/* Link to our device_state */
-	u32 pasid;				/* PASID index */
-	bool invalid;				/* Used during setup and
-						   teardown of the pasid */
-	spinlock_t lock;			/* Protect pri_queues and
-						   mmu_notifer_count */
-	wait_queue_head_t wq;			/* To wait for count == 0 */
-};
-
-struct device_state {
-	struct list_head list;
-	u32 sbdf;
-	atomic_t count;
-	struct pci_dev *pdev;
-	struct pasid_state **states;
-	struct iommu_domain *domain;
-	int pasid_levels;
-	int max_pasids;
-	amd_iommu_invalid_ppr_cb inv_ppr_cb;
-	amd_iommu_invalidate_ctx inv_ctx_cb;
-	spinlock_t lock;
-	wait_queue_head_t wq;
-};
-
-struct fault {
-	struct work_struct work;
-	struct device_state *dev_state;
-	struct pasid_state *state;
-	struct mm_struct *mm;
-	u64 address;
-	u32 pasid;
-	u16 tag;
-	u16 finish;
-	u16 flags;
-};
-
-static LIST_HEAD(state_list);
-static DEFINE_SPINLOCK(state_lock);
-
-static struct workqueue_struct *iommu_wq;
-
-static void free_pasid_states(struct device_state *dev_state);
-
-static struct device_state *__get_device_state(u32 sbdf)
-{
-	struct device_state *dev_state;
-
-	list_for_each_entry(dev_state, &state_list, list) {
-		if (dev_state->sbdf == sbdf)
-			return dev_state;
-	}
-
-	return NULL;
-}
-
-static struct device_state *get_device_state(u32 sbdf)
-{
-	struct device_state *dev_state;
-	unsigned long flags;
-
-	spin_lock_irqsave(&state_lock, flags);
-	dev_state = __get_device_state(sbdf);
-	if (dev_state != NULL)
-		atomic_inc(&dev_state->count);
-	spin_unlock_irqrestore(&state_lock, flags);
-
-	return dev_state;
-}
-
-static void free_device_state(struct device_state *dev_state)
-{
-	struct iommu_group *group;
-
-	/* Get rid of any remaining pasid states */
-	free_pasid_states(dev_state);
-
-	/*
-	 * Wait until the last reference is dropped before freeing
-	 * the device state.
-	 */
-	wait_event(dev_state->wq, !atomic_read(&dev_state->count));
-
-	/*
-	 * First detach device from domain - No more PRI requests will arrive
-	 * from that device after it is unbound from the IOMMUv2 domain.
-	 */
-	group = iommu_group_get(&dev_state->pdev->dev);
-	if (WARN_ON(!group))
-		return;
-
-	iommu_detach_group(dev_state->domain, group);
-
-	iommu_group_put(group);
-
-	/* Everything is down now, free the IOMMUv2 domain */
-	iommu_domain_free(dev_state->domain);
-
-	/* Finally get rid of the device-state */
-	kfree(dev_state);
-}
-
-static void put_device_state(struct device_state *dev_state)
-{
-	if (atomic_dec_and_test(&dev_state->count))
-		wake_up(&dev_state->wq);
-}
-
-/* Must be called under dev_state->lock */
-static struct pasid_state **__get_pasid_state_ptr(struct device_state *dev_state,
-						  u32 pasid, bool alloc)
-{
-	struct pasid_state **root, **ptr;
-	int level, index;
-
-	level = dev_state->pasid_levels;
-	root  = dev_state->states;
-
-	while (true) {
-
-		index = (pasid >> (9 * level)) & 0x1ff;
-		ptr   = &root[index];
-
-		if (level == 0)
-			break;
-
-		if (*ptr == NULL) {
-			if (!alloc)
-				return NULL;
-
-			*ptr = (void *)get_zeroed_page(GFP_ATOMIC);
-			if (*ptr == NULL)
-				return NULL;
-		}
-
-		root   = (struct pasid_state **)*ptr;
-		level -= 1;
-	}
-
-	return ptr;
-}
-
-static int set_pasid_state(struct device_state *dev_state,
-			   struct pasid_state *pasid_state,
-			   u32 pasid)
-{
-	struct pasid_state **ptr;
-	unsigned long flags;
-	int ret;
-
-	spin_lock_irqsave(&dev_state->lock, flags);
-	ptr = __get_pasid_state_ptr(dev_state, pasid, true);
-
-	ret = -ENOMEM;
-	if (ptr == NULL)
-		goto out_unlock;
-
-	ret = -ENOMEM;
-	if (*ptr != NULL)
-		goto out_unlock;
-
-	*ptr = pasid_state;
-
-	ret = 0;
-
-out_unlock:
-	spin_unlock_irqrestore(&dev_state->lock, flags);
-
-	return ret;
-}
-
-static void clear_pasid_state(struct device_state *dev_state, u32 pasid)
-{
-	struct pasid_state **ptr;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev_state->lock, flags);
-	ptr = __get_pasid_state_ptr(dev_state, pasid, true);
-
-	if (ptr == NULL)
-		goto out_unlock;
-
-	*ptr = NULL;
-
-out_unlock:
-	spin_unlock_irqrestore(&dev_state->lock, flags);
-}
-
-static struct pasid_state *get_pasid_state(struct device_state *dev_state,
-					   u32 pasid)
-{
-	struct pasid_state **ptr, *ret = NULL;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev_state->lock, flags);
-	ptr = __get_pasid_state_ptr(dev_state, pasid, false);
-
-	if (ptr == NULL)
-		goto out_unlock;
-
-	ret = *ptr;
-	if (ret)
-		refcount_inc(&ret->count);
-
-out_unlock:
-	spin_unlock_irqrestore(&dev_state->lock, flags);
-
-	return ret;
-}
-
-static void free_pasid_state(struct pasid_state *pasid_state)
-{
-	kfree(pasid_state);
-}
-
-static void put_pasid_state(struct pasid_state *pasid_state)
-{
-	if (refcount_dec_and_test(&pasid_state->count))
-		wake_up(&pasid_state->wq);
-}
-
-static void put_pasid_state_wait(struct pasid_state *pasid_state)
-{
-	if (!refcount_dec_and_test(&pasid_state->count))
-		wait_event(pasid_state->wq, !refcount_read(&pasid_state->count));
-	free_pasid_state(pasid_state);
-}
-
-static void unbind_pasid(struct pasid_state *pasid_state)
-{
-	struct iommu_domain *domain;
-
-	domain = pasid_state->device_state->domain;
-
-	/*
-	 * Mark pasid_state as invalid, no more faults will we added to the
-	 * work queue after this is visible everywhere.
-	 */
-	pasid_state->invalid = true;
-
-	/* Make sure this is visible */
-	smp_wmb();
-
-	/* After this the device/pasid can't access the mm anymore */
-	amd_iommu_domain_clear_gcr3(domain, pasid_state->pasid);
-
-	/* Make sure no more pending faults are in the queue */
-	flush_workqueue(iommu_wq);
-}
-
-static void free_pasid_states_level1(struct pasid_state **tbl)
-{
-	int i;
-
-	for (i = 0; i < 512; ++i) {
-		if (tbl[i] == NULL)
-			continue;
-
-		free_page((unsigned long)tbl[i]);
-	}
-}
-
-static void free_pasid_states_level2(struct pasid_state **tbl)
-{
-	struct pasid_state **ptr;
-	int i;
-
-	for (i = 0; i < 512; ++i) {
-		if (tbl[i] == NULL)
-			continue;
-
-		ptr = (struct pasid_state **)tbl[i];
-		free_pasid_states_level1(ptr);
-	}
-}
-
-static void free_pasid_states(struct device_state *dev_state)
-{
-	struct pasid_state *pasid_state;
-	int i;
-
-	for (i = 0; i < dev_state->max_pasids; ++i) {
-		pasid_state = get_pasid_state(dev_state, i);
-		if (pasid_state == NULL)
-			continue;
-
-		put_pasid_state(pasid_state);
-
-		/* Clear the pasid state so that the pasid can be re-used */
-		clear_pasid_state(dev_state, pasid_state->pasid);
-
-		/*
-		 * This will call the mn_release function and
-		 * unbind the PASID
-		 */
-		mmu_notifier_unregister(&pasid_state->mn, pasid_state->mm);
-
-		put_pasid_state_wait(pasid_state); /* Reference taken in
-						      amd_iommu_bind_pasid */
-
-		/* Drop reference taken in amd_iommu_bind_pasid */
-		put_device_state(dev_state);
-	}
-
-	if (dev_state->pasid_levels == 2)
-		free_pasid_states_level2(dev_state->states);
-	else if (dev_state->pasid_levels == 1)
-		free_pasid_states_level1(dev_state->states);
-	else
-		BUG_ON(dev_state->pasid_levels != 0);
-
-	free_page((unsigned long)dev_state->states);
-}
-
-static struct pasid_state *mn_to_state(struct mmu_notifier *mn)
-{
-	return container_of(mn, struct pasid_state, mn);
-}
-
-static void mn_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
-					struct mm_struct *mm,
-					unsigned long start, unsigned long end)
-{
-	struct pasid_state *pasid_state;
-	struct device_state *dev_state;
-
-	pasid_state = mn_to_state(mn);
-	dev_state   = pasid_state->device_state;
-
-	if ((start ^ (end - 1)) < PAGE_SIZE)
-		amd_iommu_flush_page(dev_state->domain, pasid_state->pasid,
-				     start);
-	else
-		amd_iommu_flush_tlb(dev_state->domain, pasid_state->pasid);
-}
-
-static void mn_release(struct mmu_notifier *mn, struct mm_struct *mm)
-{
-	struct pasid_state *pasid_state;
-	struct device_state *dev_state;
-	bool run_inv_ctx_cb;
-
-	might_sleep();
-
-	pasid_state    = mn_to_state(mn);
-	dev_state      = pasid_state->device_state;
-	run_inv_ctx_cb = !pasid_state->invalid;
-
-	if (run_inv_ctx_cb && dev_state->inv_ctx_cb)
-		dev_state->inv_ctx_cb(dev_state->pdev, pasid_state->pasid);
-
-	unbind_pasid(pasid_state);
-}
-
-static const struct mmu_notifier_ops iommu_mn = {
-	.release			= mn_release,
-	.arch_invalidate_secondary_tlbs	= mn_arch_invalidate_secondary_tlbs,
-};
-
-static void set_pri_tag_status(struct pasid_state *pasid_state,
-			       u16 tag, int status)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&pasid_state->lock, flags);
-	pasid_state->pri[tag].status = status;
-	spin_unlock_irqrestore(&pasid_state->lock, flags);
-}
-
-static void finish_pri_tag(struct device_state *dev_state,
-			   struct pasid_state *pasid_state,
-			   u16 tag)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&pasid_state->lock, flags);
-	if (atomic_dec_and_test(&pasid_state->pri[tag].inflight) &&
-	    pasid_state->pri[tag].finish) {
-		amd_iommu_complete_ppr(dev_state->pdev, pasid_state->pasid,
-				       pasid_state->pri[tag].status, tag);
-		pasid_state->pri[tag].finish = false;
-		pasid_state->pri[tag].status = PPR_SUCCESS;
-	}
-	spin_unlock_irqrestore(&pasid_state->lock, flags);
-}
-
-static void handle_fault_error(struct fault *fault)
-{
-	int status;
-
-	if (!fault->dev_state->inv_ppr_cb) {
-		set_pri_tag_status(fault->state, fault->tag, PPR_INVALID);
-		return;
-	}
-
-	status = fault->dev_state->inv_ppr_cb(fault->dev_state->pdev,
-					      fault->pasid,
-					      fault->address,
-					      fault->flags);
-	switch (status) {
-	case AMD_IOMMU_INV_PRI_RSP_SUCCESS:
-		set_pri_tag_status(fault->state, fault->tag, PPR_SUCCESS);
-		break;
-	case AMD_IOMMU_INV_PRI_RSP_INVALID:
-		set_pri_tag_status(fault->state, fault->tag, PPR_INVALID);
-		break;
-	case AMD_IOMMU_INV_PRI_RSP_FAIL:
-		set_pri_tag_status(fault->state, fault->tag, PPR_FAILURE);
-		break;
-	default:
-		BUG();
-	}
-}
-
-static bool access_error(struct vm_area_struct *vma, struct fault *fault)
-{
-	unsigned long requested = 0;
-
-	if (fault->flags & PPR_FAULT_EXEC)
-		requested |= VM_EXEC;
-
-	if (fault->flags & PPR_FAULT_READ)
-		requested |= VM_READ;
-
-	if (fault->flags & PPR_FAULT_WRITE)
-		requested |= VM_WRITE;
-
-	return (requested & ~vma->vm_flags) != 0;
-}
-
-static void do_fault(struct work_struct *work)
-{
-	struct fault *fault = container_of(work, struct fault, work);
-	struct vm_area_struct *vma;
-	vm_fault_t ret = VM_FAULT_ERROR;
-	unsigned int flags = 0;
-	struct mm_struct *mm;
-	u64 address;
-
-	mm = fault->state->mm;
-	address = fault->address;
-
-	if (fault->flags & PPR_FAULT_USER)
-		flags |= FAULT_FLAG_USER;
-	if (fault->flags & PPR_FAULT_WRITE)
-		flags |= FAULT_FLAG_WRITE;
-	flags |= FAULT_FLAG_REMOTE;
-
-	mmap_read_lock(mm);
-	vma = vma_lookup(mm, address);
-	if (!vma)
-		/* failed to get a vma in the right range */
-		goto out;
-
-	/* Check if we have the right permissions on the vma */
-	if (access_error(vma, fault))
-		goto out;
-
-	ret = handle_mm_fault(vma, address, flags, NULL);
-out:
-	mmap_read_unlock(mm);
-
-	if (ret & VM_FAULT_ERROR)
-		/* failed to service fault */
-		handle_fault_error(fault);
-
-	finish_pri_tag(fault->dev_state, fault->state, fault->tag);
-
-	put_pasid_state(fault->state);
-
-	kfree(fault);
-}
-
-static int ppr_notifier(struct notifier_block *nb, unsigned long e, void *data)
-{
-	struct amd_iommu_fault *iommu_fault;
-	struct pasid_state *pasid_state;
-	struct device_state *dev_state;
-	struct pci_dev *pdev = NULL;
-	unsigned long flags;
-	struct fault *fault;
-	bool finish;
-	u16 tag, devid, seg_id;
-	int ret;
-
-	iommu_fault = data;
-	tag         = iommu_fault->tag & 0x1ff;
-	finish      = (iommu_fault->tag >> 9) & 1;
-
-	seg_id = PCI_SBDF_TO_SEGID(iommu_fault->sbdf);
-	devid = PCI_SBDF_TO_DEVID(iommu_fault->sbdf);
-	pdev = pci_get_domain_bus_and_slot(seg_id, PCI_BUS_NUM(devid),
-					   devid & 0xff);
-	if (!pdev)
-		return -ENODEV;
-
-	ret = NOTIFY_DONE;
-
-	/* In kdump kernel pci dev is not initialized yet -> send INVALID */
-	if (amd_iommu_is_attach_deferred(&pdev->dev)) {
-		amd_iommu_complete_ppr(pdev, iommu_fault->pasid,
-				       PPR_INVALID, tag);
-		goto out;
-	}
-
-	dev_state = get_device_state(iommu_fault->sbdf);
-	if (dev_state == NULL)
-		goto out;
-
-	pasid_state = get_pasid_state(dev_state, iommu_fault->pasid);
-	if (pasid_state == NULL || pasid_state->invalid) {
-		/* We know the device but not the PASID -> send INVALID */
-		amd_iommu_complete_ppr(dev_state->pdev, iommu_fault->pasid,
-				       PPR_INVALID, tag);
-		goto out_drop_state;
-	}
-
-	spin_lock_irqsave(&pasid_state->lock, flags);
-	atomic_inc(&pasid_state->pri[tag].inflight);
-	if (finish)
-		pasid_state->pri[tag].finish = true;
-	spin_unlock_irqrestore(&pasid_state->lock, flags);
-
-	fault = kzalloc(sizeof(*fault), GFP_ATOMIC);
-	if (fault == NULL) {
-		/* We are OOM - send success and let the device re-fault */
-		finish_pri_tag(dev_state, pasid_state, tag);
-		goto out_drop_state;
-	}
-
-	fault->dev_state = dev_state;
-	fault->address   = iommu_fault->address;
-	fault->state     = pasid_state;
-	fault->tag       = tag;
-	fault->finish    = finish;
-	fault->pasid     = iommu_fault->pasid;
-	fault->flags     = iommu_fault->flags;
-	INIT_WORK(&fault->work, do_fault);
-
-	queue_work(iommu_wq, &fault->work);
-
-	ret = NOTIFY_OK;
-
-out_drop_state:
-
-	if (ret != NOTIFY_OK && pasid_state)
-		put_pasid_state(pasid_state);
-
-	put_device_state(dev_state);
-
-out:
-	pci_dev_put(pdev);
-	return ret;
-}
-
-static struct notifier_block ppr_nb = {
-	.notifier_call = ppr_notifier,
-};
-
-int amd_iommu_bind_pasid(struct pci_dev *pdev, u32 pasid,
-			 struct task_struct *task)
-{
-	struct pasid_state *pasid_state;
-	struct device_state *dev_state;
-	struct mm_struct *mm;
-	u32 sbdf;
-	int ret;
-
-	might_sleep();
-
-	if (!amd_iommu_v2_supported())
-		return -ENODEV;
-
-	sbdf      = get_pci_sbdf_id(pdev);
-	dev_state = get_device_state(sbdf);
-
-	if (dev_state == NULL)
-		return -EINVAL;
-
-	ret = -EINVAL;
-	if (pasid >= dev_state->max_pasids)
-		goto out;
-
-	ret = -ENOMEM;
-	pasid_state = kzalloc(sizeof(*pasid_state), GFP_KERNEL);
-	if (pasid_state == NULL)
-		goto out;
-
-
-	refcount_set(&pasid_state->count, 1);
-	init_waitqueue_head(&pasid_state->wq);
-	spin_lock_init(&pasid_state->lock);
-
-	mm                        = get_task_mm(task);
-	pasid_state->mm           = mm;
-	pasid_state->device_state = dev_state;
-	pasid_state->pasid        = pasid;
-	pasid_state->invalid      = true; /* Mark as valid only if we are
-					     done with setting up the pasid */
-	pasid_state->mn.ops       = &iommu_mn;
-
-	if (pasid_state->mm == NULL)
-		goto out_free;
-
-	ret = mmu_notifier_register(&pasid_state->mn, mm);
-	if (ret)
-		goto out_free;
-
-	ret = set_pasid_state(dev_state, pasid_state, pasid);
-	if (ret)
-		goto out_unregister;
-
-	ret = amd_iommu_domain_set_gcr3(dev_state->domain, pasid,
-					__pa(pasid_state->mm->pgd));
-	if (ret)
-		goto out_clear_state;
-
-	/* Now we are ready to handle faults */
-	pasid_state->invalid = false;
-
-	/*
-	 * Drop the reference to the mm_struct here. We rely on the
-	 * mmu_notifier release call-back to inform us when the mm
-	 * is going away.
-	 */
-	mmput(mm);
-
-	return 0;
-
-out_clear_state:
-	clear_pasid_state(dev_state, pasid);
-
-out_unregister:
-	mmu_notifier_unregister(&pasid_state->mn, mm);
-	mmput(mm);
-
-out_free:
-	free_pasid_state(pasid_state);
-
-out:
-	put_device_state(dev_state);
-
-	return ret;
-}
-EXPORT_SYMBOL(amd_iommu_bind_pasid);
-
-void amd_iommu_unbind_pasid(struct pci_dev *pdev, u32 pasid)
-{
-	struct pasid_state *pasid_state;
-	struct device_state *dev_state;
-	u32 sbdf;
-
-	might_sleep();
-
-	if (!amd_iommu_v2_supported())
-		return;
-
-	sbdf = get_pci_sbdf_id(pdev);
-	dev_state = get_device_state(sbdf);
-	if (dev_state == NULL)
-		return;
-
-	if (pasid >= dev_state->max_pasids)
-		goto out;
-
-	pasid_state = get_pasid_state(dev_state, pasid);
-	if (pasid_state == NULL)
-		goto out;
-	/*
-	 * Drop reference taken here. We are safe because we still hold
-	 * the reference taken in the amd_iommu_bind_pasid function.
-	 */
-	put_pasid_state(pasid_state);
-
-	/* Clear the pasid state so that the pasid can be re-used */
-	clear_pasid_state(dev_state, pasid_state->pasid);
-
-	/*
-	 * Call mmu_notifier_unregister to drop our reference
-	 * to pasid_state->mm
-	 */
-	mmu_notifier_unregister(&pasid_state->mn, pasid_state->mm);
-
-	put_pasid_state_wait(pasid_state); /* Reference taken in
-					      amd_iommu_bind_pasid */
-out:
-	/* Drop reference taken in this function */
-	put_device_state(dev_state);
-
-	/* Drop reference taken in amd_iommu_bind_pasid */
-	put_device_state(dev_state);
-}
-EXPORT_SYMBOL(amd_iommu_unbind_pasid);
-
-int amd_iommu_init_device(struct pci_dev *pdev, int pasids)
-{
-	struct device_state *dev_state;
-	struct iommu_group *group;
-	unsigned long flags;
-	int ret, tmp;
-	u32 sbdf;
-
-	might_sleep();
-
-	/*
-	 * When memory encryption is active the device is likely not in a
-	 * direct-mapped domain. Forbid using IOMMUv2 functionality for now.
-	 */
-	if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
-		return -ENODEV;
-
-	if (!amd_iommu_v2_supported())
-		return -ENODEV;
-
-	if (pasids <= 0 || pasids > (PASID_MASK + 1))
-		return -EINVAL;
-
-	sbdf = get_pci_sbdf_id(pdev);
-
-	dev_state = kzalloc(sizeof(*dev_state), GFP_KERNEL);
-	if (dev_state == NULL)
-		return -ENOMEM;
-
-	spin_lock_init(&dev_state->lock);
-	init_waitqueue_head(&dev_state->wq);
-	dev_state->pdev  = pdev;
-	dev_state->sbdf = sbdf;
-
-	tmp = pasids;
-	for (dev_state->pasid_levels = 0; (tmp - 1) & ~0x1ff; tmp >>= 9)
-		dev_state->pasid_levels += 1;
-
-	atomic_set(&dev_state->count, 1);
-	dev_state->max_pasids = pasids;
-
-	ret = -ENOMEM;
-	dev_state->states = (void *)get_zeroed_page(GFP_KERNEL);
-	if (dev_state->states == NULL)
-		goto out_free_dev_state;
-
-	dev_state->domain = iommu_domain_alloc(&pci_bus_type);
-	if (dev_state->domain == NULL)
-		goto out_free_states;
-
-	/* See iommu_is_default_domain() */
-	dev_state->domain->type = IOMMU_DOMAIN_IDENTITY;
-	amd_iommu_domain_direct_map(dev_state->domain);
-
-	ret = amd_iommu_domain_enable_v2(dev_state->domain, pasids);
-	if (ret)
-		goto out_free_domain;
-
-	group = iommu_group_get(&pdev->dev);
-	if (!group) {
-		ret = -EINVAL;
-		goto out_free_domain;
-	}
-
-	ret = iommu_attach_group(dev_state->domain, group);
-	if (ret != 0)
-		goto out_drop_group;
-
-	iommu_group_put(group);
-
-	spin_lock_irqsave(&state_lock, flags);
-
-	if (__get_device_state(sbdf) != NULL) {
-		spin_unlock_irqrestore(&state_lock, flags);
-		ret = -EBUSY;
-		goto out_free_domain;
-	}
-
-	list_add_tail(&dev_state->list, &state_list);
-
-	spin_unlock_irqrestore(&state_lock, flags);
-
-	return 0;
-
-out_drop_group:
-	iommu_group_put(group);
-
-out_free_domain:
-	iommu_domain_free(dev_state->domain);
-
-out_free_states:
-	free_page((unsigned long)dev_state->states);
-
-out_free_dev_state:
-	kfree(dev_state);
-
-	return ret;
-}
-EXPORT_SYMBOL(amd_iommu_init_device);
-
-void amd_iommu_free_device(struct pci_dev *pdev)
-{
-	struct device_state *dev_state;
-	unsigned long flags;
-	u32 sbdf;
-
-	if (!amd_iommu_v2_supported())
-		return;
-
-	sbdf = get_pci_sbdf_id(pdev);
-
-	spin_lock_irqsave(&state_lock, flags);
-
-	dev_state = __get_device_state(sbdf);
-	if (dev_state == NULL) {
-		spin_unlock_irqrestore(&state_lock, flags);
-		return;
-	}
-
-	list_del(&dev_state->list);
-
-	spin_unlock_irqrestore(&state_lock, flags);
-
-	put_device_state(dev_state);
-	free_device_state(dev_state);
-}
-EXPORT_SYMBOL(amd_iommu_free_device);
-
-int amd_iommu_set_invalid_ppr_cb(struct pci_dev *pdev,
-				 amd_iommu_invalid_ppr_cb cb)
-{
-	struct device_state *dev_state;
-	unsigned long flags;
-	u32 sbdf;
-	int ret;
-
-	if (!amd_iommu_v2_supported())
-		return -ENODEV;
-
-	sbdf = get_pci_sbdf_id(pdev);
-
-	spin_lock_irqsave(&state_lock, flags);
-
-	ret = -EINVAL;
-	dev_state = __get_device_state(sbdf);
-	if (dev_state == NULL)
-		goto out_unlock;
-
-	dev_state->inv_ppr_cb = cb;
-
-	ret = 0;
-
-out_unlock:
-	spin_unlock_irqrestore(&state_lock, flags);
-
-	return ret;
-}
-EXPORT_SYMBOL(amd_iommu_set_invalid_ppr_cb);
-
-int amd_iommu_set_invalidate_ctx_cb(struct pci_dev *pdev,
-				    amd_iommu_invalidate_ctx cb)
-{
-	struct device_state *dev_state;
-	unsigned long flags;
-	u32 sbdf;
-	int ret;
-
-	if (!amd_iommu_v2_supported())
-		return -ENODEV;
-
-	sbdf = get_pci_sbdf_id(pdev);
-
-	spin_lock_irqsave(&state_lock, flags);
-
-	ret = -EINVAL;
-	dev_state = __get_device_state(sbdf);
-	if (dev_state == NULL)
-		goto out_unlock;
-
-	dev_state->inv_ctx_cb = cb;
-
-	ret = 0;
-
-out_unlock:
-	spin_unlock_irqrestore(&state_lock, flags);
-
-	return ret;
-}
-EXPORT_SYMBOL(amd_iommu_set_invalidate_ctx_cb);
-
-static int __init amd_iommu_v2_init(void)
-{
-	int ret;
-
-	if (!amd_iommu_v2_supported()) {
-		pr_info("AMD IOMMUv2 functionality not available on this system - This is not a bug.\n");
-		/*
-		 * Load anyway to provide the symbols to other modules
-		 * which may use AMD IOMMUv2 optionally.
-		 */
-		return 0;
-	}
-
-	ret = -ENOMEM;
-	iommu_wq = alloc_workqueue("amd_iommu_v2", WQ_MEM_RECLAIM, 0);
-	if (iommu_wq == NULL)
-		goto out;
-
-	amd_iommu_register_ppr_notifier(&ppr_nb);
-
-	pr_info("AMD IOMMUv2 loaded and initialized\n");
-
-	return 0;
-
-out:
-	return ret;
-}
-
-static void __exit amd_iommu_v2_exit(void)
-{
-	struct device_state *dev_state, *next;
-	unsigned long flags;
-	LIST_HEAD(freelist);
-
-	if (!amd_iommu_v2_supported())
-		return;
-
-	amd_iommu_unregister_ppr_notifier(&ppr_nb);
-
-	flush_workqueue(iommu_wq);
-
-	/*
-	 * The loop below might call flush_workqueue(), so call
-	 * destroy_workqueue() after it
-	 */
-	spin_lock_irqsave(&state_lock, flags);
-
-	list_for_each_entry_safe(dev_state, next, &state_list, list) {
-		WARN_ON_ONCE(1);
-
-		put_device_state(dev_state);
-		list_del(&dev_state->list);
-		list_add_tail(&dev_state->list, &freelist);
-	}
-
-	spin_unlock_irqrestore(&state_lock, flags);
-
-	/*
-	 * Since free_device_state waits on the count to be zero,
-	 * we need to free dev_state outside the spinlock.
-	 */
-	list_for_each_entry_safe(dev_state, next, &freelist, list) {
-		list_del(&dev_state->list);
-		free_device_state(dev_state);
-	}
-
-	destroy_workqueue(iommu_wq);
-}
-
-module_init(amd_iommu_v2_init);
-module_exit(amd_iommu_v2_exit);
diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h
index 99a5201d9e62..35ca00b09384 100644
--- a/include/linux/amd-iommu.h
+++ b/include/linux/amd-iommu.h
@@ -33,84 +33,6 @@ struct pci_dev;
 
 extern int amd_iommu_detect(void);
 
-/**
- * amd_iommu_init_device() - Init device for use with IOMMUv2 driver
- * @pdev: The PCI device to initialize
- * @pasids: Number of PASIDs to support for this device
- *
- * This function does all setup for the device pdev so that it can be
- * used with IOMMUv2.
- * Returns 0 on success or negative value on error.
- */
-extern int amd_iommu_init_device(struct pci_dev *pdev, int pasids);
-
-/**
- * amd_iommu_free_device() - Free all IOMMUv2 related device resources
- *			     and disable IOMMUv2 usage for this device
- * @pdev: The PCI device to disable IOMMUv2 usage for'
- */
-extern void amd_iommu_free_device(struct pci_dev *pdev);
-
-/**
- * amd_iommu_bind_pasid() - Bind a given task to a PASID on a device
- * @pdev: The PCI device to bind the task to
- * @pasid: The PASID on the device the task should be bound to
- * @task: the task to bind
- *
- * The function returns 0 on success or a negative value on error.
- */
-extern int amd_iommu_bind_pasid(struct pci_dev *pdev, u32 pasid,
-				struct task_struct *task);
-
-/**
- * amd_iommu_unbind_pasid() - Unbind a PASID from its task on
- *			      a device
- * @pdev: The device of the PASID
- * @pasid: The PASID to unbind
- *
- * When this function returns the device is no longer using the PASID
- * and the PASID is no longer bound to its task.
- */
-extern void amd_iommu_unbind_pasid(struct pci_dev *pdev, u32 pasid);
-
-/**
- * amd_iommu_set_invalid_ppr_cb() - Register a call-back for failed
- *				    PRI requests
- * @pdev: The PCI device the call-back should be registered for
- * @cb: The call-back function
- *
- * The IOMMUv2 driver invokes this call-back when it is unable to
- * successfully handle a PRI request. The device driver can then decide
- * which PRI response the device should see. Possible return values for
- * the call-back are:
- *
- * - AMD_IOMMU_INV_PRI_RSP_SUCCESS - Send SUCCESS back to the device
- * - AMD_IOMMU_INV_PRI_RSP_INVALID - Send INVALID back to the device
- * - AMD_IOMMU_INV_PRI_RSP_FAIL    - Send Failure back to the device,
- *				     the device is required to disable
- *				     PRI when it receives this response
- *
- * The function returns 0 on success or negative value on error.
- */
-#define AMD_IOMMU_INV_PRI_RSP_SUCCESS	0
-#define AMD_IOMMU_INV_PRI_RSP_INVALID	1
-#define AMD_IOMMU_INV_PRI_RSP_FAIL	2
-
-typedef int (*amd_iommu_invalid_ppr_cb)(struct pci_dev *pdev,
-					u32 pasid,
-					unsigned long address,
-					u16);
-
-extern int amd_iommu_set_invalid_ppr_cb(struct pci_dev *pdev,
-					amd_iommu_invalid_ppr_cb cb);
-
-#define PPR_FAULT_EXEC	(1 << 1)
-#define PPR_FAULT_READ  (1 << 2)
-#define PPR_FAULT_WRITE (1 << 5)
-#define PPR_FAULT_USER  (1 << 6)
-#define PPR_FAULT_RSVD  (1 << 7)
-#define PPR_FAULT_GN    (1 << 8)
-
 /**
  * amd_iommu_device_info() - Get information about IOMMUv2 support of a
  *			     PCI device
@@ -137,22 +59,6 @@ struct amd_iommu_device_info {
 extern int amd_iommu_device_info(struct pci_dev *pdev,
 				 struct amd_iommu_device_info *info);
 
-/**
- * amd_iommu_set_invalidate_ctx_cb() - Register a call-back for invalidating
- *				       a pasid context. This call-back is
- *				       invoked when the IOMMUv2 driver needs to
- *				       invalidate a PASID context, for example
- *				       because the task that is bound to that
- *				       context is about to exit.
- *
- * @pdev: The PCI device the call-back should be registered for
- * @cb: The call-back function
- */
-
-typedef void (*amd_iommu_invalidate_ctx)(struct pci_dev *pdev, u32 pasid);
-
-extern int amd_iommu_set_invalidate_ctx_cb(struct pci_dev *pdev,
-					   amd_iommu_invalidate_ctx cb);
 #else /* CONFIG_AMD_IOMMU */
 
 static inline int amd_iommu_detect(void) { return -ENODEV; }
-- 
2.31.1

RE: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Deucher, Alexander]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Hegde, Vasant <Vasant.Hegde@amd.com>
> Sent: Thursday, September 21, 2023 5:32 AM
> To: iommu@lists.linux.dev; joro@8bytes.org
> Cc: Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Huang2, Wei
> <Wei.Huang2@amd.com>; jsnitsel@redhat.com; jgg@ziepe.ca; Hegde, Vasant
> <Vasant.Hegde@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>;
> Jason Gunthorpe <jgg@nvidia.com>
> Subject: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>
> AMD GPU driver which was the only in-kernel user of iommu_v2 module
> removed dependency on iommu_v2 module.
>
> Also we are working on adding SVA support in AMD IOMMU driver. Device
> drivers are expected to use common SVA framework to enable device
> PASID/PRI features.
>
> Removing iommu_v2 module and then adding SVA simplifies the
> development.
> Hence remove iommu_v2 module.

Does this patch or the following patches make any functional changes for devices?  E.g., devices which supported ATS would have been put into an identity mapping mode previously  Is that still retained?

Thanks,

Alex

>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
> ---
>  drivers/iommu/amd/Kconfig     |   9 -
>  drivers/iommu/amd/Makefile    |   1 -
>  drivers/iommu/amd/amd_iommu.h |   5 -
>  drivers/iommu/amd/iommu.c     |  40 --
>  drivers/iommu/amd/iommu_v2.c  | 996 ----------------------------------
>  include/linux/amd-iommu.h     |  94 ----
>  6 files changed, 1145 deletions(-)
>  delete mode 100644 drivers/iommu/amd/iommu_v2.c
>
> diff --git a/drivers/iommu/amd/Kconfig b/drivers/iommu/amd/Kconfig index
> 9b5fc3356bf2..75132ae861a2 100644
> --- a/drivers/iommu/amd/Kconfig
> +++ b/drivers/iommu/amd/Kconfig
> @@ -22,15 +22,6 @@ config AMD_IOMMU
>         your BIOS for an option to enable it or if you have an IVRS ACPI
>         table.
>
> -config AMD_IOMMU_V2
> -     tristate "AMD IOMMU Version 2 driver"
> -     depends on AMD_IOMMU
> -     select MMU_NOTIFIER
> -     help
> -       This option enables support for the AMD IOMMUv2 features of the
> IOMMU
> -       hardware. Select this option if you want to use devices that support
> -       the PCI PRI and PASID interface.
> -
>  config AMD_IOMMU_DEBUGFS
>       bool "Enable AMD IOMMU internals in DebugFS"
>       depends on AMD_IOMMU && IOMMU_DEBUGFS
> diff --git a/drivers/iommu/amd/Makefile b/drivers/iommu/amd/Makefile
> index 773d8aa00283..f454fbb1569e 100644
> --- a/drivers/iommu/amd/Makefile
> +++ b/drivers/iommu/amd/Makefile
> @@ -1,4 +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_DEBUGFS) += debugfs.o
> -obj-$(CONFIG_AMD_IOMMU_V2) += iommu_v2.o diff --git
> a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index 9df53961d5ef..5b8a1e2dd3d0 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -38,9 +38,6 @@ extern int amd_iommu_guest_ir;  extern enum
> io_pgtable_fmt amd_iommu_pgtable;  extern int amd_iommu_gpt_level;
>
> -/* IOMMUv2 specific functions */
> -struct iommu_domain;
> -
>  bool amd_iommu_v2_supported(void);
>  struct amd_iommu *get_amd_iommu(unsigned int idx);
>  u8 amd_iommu_pc_get_max_banks(unsigned int idx); @@ -57,8 +54,6 @@
> void amd_iommu_pdev_disable_cap_pri(struct pci_dev *pdev);
>
>  int amd_iommu_register_ppr_notifier(struct notifier_block *nb);  int
> amd_iommu_unregister_ppr_notifier(struct notifier_block *nb); -void
> amd_iommu_domain_direct_map(struct iommu_domain *dom); -int
> amd_iommu_domain_enable_v2(struct iommu_domain *dom, int pasids);
> int amd_iommu_flush_page(struct iommu_domain *dom, u32 pasid, u64
> address);  void amd_iommu_update_and_flush_device_table(struct
> protection_domain *domain);  void amd_iommu_domain_update(struct
> protection_domain *domain); diff --git a/drivers/iommu/amd/iommu.c
> b/drivers/iommu/amd/iommu.c index f3448a2b6c0e..fd0d7b2f30dc 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2568,46 +2568,6 @@ int amd_iommu_unregister_ppr_notifier(struct
> notifier_block *nb)  }
> EXPORT_SYMBOL(amd_iommu_unregister_ppr_notifier);
>
> -void amd_iommu_domain_direct_map(struct iommu_domain *dom) -{
> -     struct protection_domain *domain = to_pdomain(dom);
> -     unsigned long flags;
> -
> -     spin_lock_irqsave(&domain->lock, flags);
> -
> -     if (domain->iop.pgtbl_cfg.tlb)
> -             free_io_pgtable_ops(&domain->iop.iop.ops);
> -
> -     spin_unlock_irqrestore(&domain->lock, flags);
> -}
> -EXPORT_SYMBOL(amd_iommu_domain_direct_map);
> -
> -int amd_iommu_domain_enable_v2(struct iommu_domain *dom, int pasids)
> -{
> -     struct protection_domain *pdom = to_pdomain(dom);
> -     unsigned long flags;
> -     int ret;
> -
> -     spin_lock_irqsave(&pdom->lock, flags);
> -
> -     /*
> -      * Save us all sanity checks whether devices already in the
> -      * domain support IOMMUv2. Just force that the domain has no
> -      * devices attached when it is switched into IOMMUv2 mode.
> -      */
> -     ret = -EBUSY;
> -     if (pdom->dev_cnt > 0 || pdom->flags & PD_IOMMUV2_MASK)
> -             goto out;
> -
> -     if (!pdom->gcr3_tbl)
> -             ret = setup_gcr3_table(pdom, pasids);
> -
> -out:
> -     spin_unlock_irqrestore(&pdom->lock, flags);
> -     return ret;
> -}
> -EXPORT_SYMBOL(amd_iommu_domain_enable_v2);
> -
>  static int __flush_pasid(struct protection_domain *domain, u32 pasid,
>                        u64 address, bool size)
>  {
> diff --git a/drivers/iommu/amd/iommu_v2.c
> b/drivers/iommu/amd/iommu_v2.c deleted file mode 100644 index
> 57c2fb1146e2..000000000000
> --- a/drivers/iommu/amd/iommu_v2.c
> +++ /dev/null
> @@ -1,996 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Copyright (C) 2010-2012 Advanced Micro Devices, Inc.
> - * Author: Joerg Roedel <jroedel@suse.de>
> - */
> -
> -#define pr_fmt(fmt)     "AMD-Vi: " fmt
> -
> -#include <linux/refcount.h>
> -#include <linux/mmu_notifier.h>
> -#include <linux/amd-iommu.h>
> -#include <linux/mm_types.h>
> -#include <linux/profile.h>
> -#include <linux/module.h>
> -#include <linux/sched.h>
> -#include <linux/sched/mm.h>
> -#include <linux/wait.h>
> -#include <linux/pci.h>
> -#include <linux/gfp.h>
> -#include <linux/cc_platform.h>
> -
> -#include "amd_iommu.h"
> -
> -MODULE_LICENSE("GPL v2");
> -MODULE_AUTHOR("Joerg Roedel <jroedel@suse.de>");
> -
> -#define PRI_QUEUE_SIZE               512
> -
> -struct pri_queue {
> -     atomic_t inflight;
> -     bool finish;
> -     int status;
> -};
> -
> -struct pasid_state {
> -     struct list_head list;                  /* For global state-list */
> -     refcount_t count;                               /* Reference count */
> -     unsigned mmu_notifier_count;            /* Counting nested
> mmu_notifier
> -                                                calls */
> -     struct mm_struct *mm;                   /* mm_struct for the faults */
> -     struct mmu_notifier mn;                 /* mmu_notifier handle */
> -     struct pri_queue pri[PRI_QUEUE_SIZE];   /* PRI tag states */
> -     struct device_state *device_state;      /* Link to our device_state */
> -     u32 pasid;                              /* PASID index */
> -     bool invalid;                           /* Used during setup and
> -                                                teardown of the pasid */
> -     spinlock_t lock;                        /* Protect pri_queues and
> -                                                mmu_notifer_count */
> -     wait_queue_head_t wq;                   /* To wait for count ==
> 0 */
> -};
> -
> -struct device_state {
> -     struct list_head list;
> -     u32 sbdf;
> -     atomic_t count;
> -     struct pci_dev *pdev;
> -     struct pasid_state **states;
> -     struct iommu_domain *domain;
> -     int pasid_levels;
> -     int max_pasids;
> -     amd_iommu_invalid_ppr_cb inv_ppr_cb;
> -     amd_iommu_invalidate_ctx inv_ctx_cb;
> -     spinlock_t lock;
> -     wait_queue_head_t wq;
> -};
> -
> -struct fault {
> -     struct work_struct work;
> -     struct device_state *dev_state;
> -     struct pasid_state *state;
> -     struct mm_struct *mm;
> -     u64 address;
> -     u32 pasid;
> -     u16 tag;
> -     u16 finish;
> -     u16 flags;
> -};
> -
> -static LIST_HEAD(state_list);
> -static DEFINE_SPINLOCK(state_lock);
> -
> -static struct workqueue_struct *iommu_wq;
> -
> -static void free_pasid_states(struct device_state *dev_state);
> -
> -static struct device_state *__get_device_state(u32 sbdf) -{
> -     struct device_state *dev_state;
> -
> -     list_for_each_entry(dev_state, &state_list, list) {
> -             if (dev_state->sbdf == sbdf)
> -                     return dev_state;
> -     }
> -
> -     return NULL;
> -}
> -
> -static struct device_state *get_device_state(u32 sbdf) -{
> -     struct device_state *dev_state;
> -     unsigned long flags;
> -
> -     spin_lock_irqsave(&state_lock, flags);
> -     dev_state = __get_device_state(sbdf);
> -     if (dev_state != NULL)
> -             atomic_inc(&dev_state->count);
> -     spin_unlock_irqrestore(&state_lock, flags);
> -
> -     return dev_state;
> -}
> -
> -static void free_device_state(struct device_state *dev_state) -{
> -     struct iommu_group *group;
> -
> -     /* Get rid of any remaining pasid states */
> -     free_pasid_states(dev_state);
> -
> -     /*
> -      * Wait until the last reference is dropped before freeing
> -      * the device state.
> -      */
> -     wait_event(dev_state->wq, !atomic_read(&dev_state->count));
> -
> -     /*
> -      * First detach device from domain - No more PRI requests will arrive
> -      * from that device after it is unbound from the IOMMUv2 domain.
> -      */
> -     group = iommu_group_get(&dev_state->pdev->dev);
> -     if (WARN_ON(!group))
> -             return;
> -
> -     iommu_detach_group(dev_state->domain, group);
> -
> -     iommu_group_put(group);
> -
> -     /* Everything is down now, free the IOMMUv2 domain */
> -     iommu_domain_free(dev_state->domain);
> -
> -     /* Finally get rid of the device-state */
> -     kfree(dev_state);
> -}
> -
> -static void put_device_state(struct device_state *dev_state) -{
> -     if (atomic_dec_and_test(&dev_state->count))
> -             wake_up(&dev_state->wq);
> -}
> -
> -/* Must be called under dev_state->lock */ -static struct pasid_state
> **__get_pasid_state_ptr(struct device_state *dev_state,
> -                                               u32 pasid, bool alloc)
> -{
> -     struct pasid_state **root, **ptr;
> -     int level, index;
> -
> -     level = dev_state->pasid_levels;
> -     root  = dev_state->states;
> -
> -     while (true) {
> -
> -             index = (pasid >> (9 * level)) & 0x1ff;
> -             ptr   = &root[index];
> -
> -             if (level == 0)
> -                     break;
> -
> -             if (*ptr == NULL) {
> -                     if (!alloc)
> -                             return NULL;
> -
> -                     *ptr = (void *)get_zeroed_page(GFP_ATOMIC);
> -                     if (*ptr == NULL)
> -                             return NULL;
> -             }
> -
> -             root   = (struct pasid_state **)*ptr;
> -             level -= 1;
> -     }
> -
> -     return ptr;
> -}
> -
> -static int set_pasid_state(struct device_state *dev_state,
> -                        struct pasid_state *pasid_state,
> -                        u32 pasid)
> -{
> -     struct pasid_state **ptr;
> -     unsigned long flags;
> -     int ret;
> -
> -     spin_lock_irqsave(&dev_state->lock, flags);
> -     ptr = __get_pasid_state_ptr(dev_state, pasid, true);
> -
> -     ret = -ENOMEM;
> -     if (ptr == NULL)
> -             goto out_unlock;
> -
> -     ret = -ENOMEM;
> -     if (*ptr != NULL)
> -             goto out_unlock;
> -
> -     *ptr = pasid_state;
> -
> -     ret = 0;
> -
> -out_unlock:
> -     spin_unlock_irqrestore(&dev_state->lock, flags);
> -
> -     return ret;
> -}
> -
> -static void clear_pasid_state(struct device_state *dev_state, u32 pasid) -{
> -     struct pasid_state **ptr;
> -     unsigned long flags;
> -
> -     spin_lock_irqsave(&dev_state->lock, flags);
> -     ptr = __get_pasid_state_ptr(dev_state, pasid, true);
> -
> -     if (ptr == NULL)
> -             goto out_unlock;
> -
> -     *ptr = NULL;
> -
> -out_unlock:
> -     spin_unlock_irqrestore(&dev_state->lock, flags);
> -}
> -
> -static struct pasid_state *get_pasid_state(struct device_state *dev_state,
> -                                        u32 pasid)
> -{
> -     struct pasid_state **ptr, *ret = NULL;
> -     unsigned long flags;
> -
> -     spin_lock_irqsave(&dev_state->lock, flags);
> -     ptr = __get_pasid_state_ptr(dev_state, pasid, false);
> -
> -     if (ptr == NULL)
> -             goto out_unlock;
> -
> -     ret = *ptr;
> -     if (ret)
> -             refcount_inc(&ret->count);
> -
> -out_unlock:
> -     spin_unlock_irqrestore(&dev_state->lock, flags);
> -
> -     return ret;
> -}
> -
> -static void free_pasid_state(struct pasid_state *pasid_state) -{
> -     kfree(pasid_state);
> -}
> -
> -static void put_pasid_state(struct pasid_state *pasid_state) -{
> -     if (refcount_dec_and_test(&pasid_state->count))
> -             wake_up(&pasid_state->wq);
> -}
> -
> -static void put_pasid_state_wait(struct pasid_state *pasid_state) -{
> -     if (!refcount_dec_and_test(&pasid_state->count))
> -             wait_event(pasid_state->wq, !refcount_read(&pasid_state-
> >count));
> -     free_pasid_state(pasid_state);
> -}
> -
> -static void unbind_pasid(struct pasid_state *pasid_state) -{
> -     struct iommu_domain *domain;
> -
> -     domain = pasid_state->device_state->domain;
> -
> -     /*
> -      * Mark pasid_state as invalid, no more faults will we added to the
> -      * work queue after this is visible everywhere.
> -      */
> -     pasid_state->invalid = true;
> -
> -     /* Make sure this is visible */
> -     smp_wmb();
> -
> -     /* After this the device/pasid can't access the mm anymore */
> -     amd_iommu_domain_clear_gcr3(domain, pasid_state->pasid);
> -
> -     /* Make sure no more pending faults are in the queue */
> -     flush_workqueue(iommu_wq);
> -}
> -
> -static void free_pasid_states_level1(struct pasid_state **tbl) -{
> -     int i;
> -
> -     for (i = 0; i < 512; ++i) {
> -             if (tbl[i] == NULL)
> -                     continue;
> -
> -             free_page((unsigned long)tbl[i]);
> -     }
> -}
> -
> -static void free_pasid_states_level2(struct pasid_state **tbl) -{
> -     struct pasid_state **ptr;
> -     int i;
> -
> -     for (i = 0; i < 512; ++i) {
> -             if (tbl[i] == NULL)
> -                     continue;
> -
> -             ptr = (struct pasid_state **)tbl[i];
> -             free_pasid_states_level1(ptr);
> -     }
> -}
> -
> -static void free_pasid_states(struct device_state *dev_state) -{
> -     struct pasid_state *pasid_state;
> -     int i;
> -
> -     for (i = 0; i < dev_state->max_pasids; ++i) {
> -             pasid_state = get_pasid_state(dev_state, i);
> -             if (pasid_state == NULL)
> -                     continue;
> -
> -             put_pasid_state(pasid_state);
> -
> -             /* Clear the pasid state so that the pasid can be re-used */
> -             clear_pasid_state(dev_state, pasid_state->pasid);
> -
> -             /*
> -              * This will call the mn_release function and
> -              * unbind the PASID
> -              */
> -             mmu_notifier_unregister(&pasid_state->mn, pasid_state-
> >mm);
> -
> -             put_pasid_state_wait(pasid_state); /* Reference taken in
> -                                                   amd_iommu_bind_pasid
> */
> -
> -             /* Drop reference taken in amd_iommu_bind_pasid */
> -             put_device_state(dev_state);
> -     }
> -
> -     if (dev_state->pasid_levels == 2)
> -             free_pasid_states_level2(dev_state->states);
> -     else if (dev_state->pasid_levels == 1)
> -             free_pasid_states_level1(dev_state->states);
> -     else
> -             BUG_ON(dev_state->pasid_levels != 0);
> -
> -     free_page((unsigned long)dev_state->states);
> -}
> -
> -static struct pasid_state *mn_to_state(struct mmu_notifier *mn) -{
> -     return container_of(mn, struct pasid_state, mn);
> -}
> -
> -static void mn_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
> -                                     struct mm_struct *mm,
> -                                     unsigned long start, unsigned long
> end)
> -{
> -     struct pasid_state *pasid_state;
> -     struct device_state *dev_state;
> -
> -     pasid_state = mn_to_state(mn);
> -     dev_state   = pasid_state->device_state;
> -
> -     if ((start ^ (end - 1)) < PAGE_SIZE)
> -             amd_iommu_flush_page(dev_state->domain, pasid_state-
> >pasid,
> -                                  start);
> -     else
> -             amd_iommu_flush_tlb(dev_state->domain, pasid_state-
> >pasid);
> -}
> -
> -static void mn_release(struct mmu_notifier *mn, struct mm_struct *mm) -{
> -     struct pasid_state *pasid_state;
> -     struct device_state *dev_state;
> -     bool run_inv_ctx_cb;
> -
> -     might_sleep();
> -
> -     pasid_state    = mn_to_state(mn);
> -     dev_state      = pasid_state->device_state;
> -     run_inv_ctx_cb = !pasid_state->invalid;
> -
> -     if (run_inv_ctx_cb && dev_state->inv_ctx_cb)
> -             dev_state->inv_ctx_cb(dev_state->pdev, pasid_state->pasid);
> -
> -     unbind_pasid(pasid_state);
> -}
> -
> -static const struct mmu_notifier_ops iommu_mn = {
> -     .release                        = mn_release,
> -     .arch_invalidate_secondary_tlbs =
> mn_arch_invalidate_secondary_tlbs,
> -};
> -
> -static void set_pri_tag_status(struct pasid_state *pasid_state,
> -                            u16 tag, int status)
> -{
> -     unsigned long flags;
> -
> -     spin_lock_irqsave(&pasid_state->lock, flags);
> -     pasid_state->pri[tag].status = status;
> -     spin_unlock_irqrestore(&pasid_state->lock, flags);
> -}
> -
> -static void finish_pri_tag(struct device_state *dev_state,
> -                        struct pasid_state *pasid_state,
> -                        u16 tag)
> -{
> -     unsigned long flags;
> -
> -     spin_lock_irqsave(&pasid_state->lock, flags);
> -     if (atomic_dec_and_test(&pasid_state->pri[tag].inflight) &&
> -         pasid_state->pri[tag].finish) {
> -             amd_iommu_complete_ppr(dev_state->pdev, pasid_state-
> >pasid,
> -                                    pasid_state->pri[tag].status, tag);
> -             pasid_state->pri[tag].finish = false;
> -             pasid_state->pri[tag].status = PPR_SUCCESS;
> -     }
> -     spin_unlock_irqrestore(&pasid_state->lock, flags);
> -}
> -
> -static void handle_fault_error(struct fault *fault) -{
> -     int status;
> -
> -     if (!fault->dev_state->inv_ppr_cb) {
> -             set_pri_tag_status(fault->state, fault->tag, PPR_INVALID);
> -             return;
> -     }
> -
> -     status = fault->dev_state->inv_ppr_cb(fault->dev_state->pdev,
> -                                           fault->pasid,
> -                                           fault->address,
> -                                           fault->flags);
> -     switch (status) {
> -     case AMD_IOMMU_INV_PRI_RSP_SUCCESS:
> -             set_pri_tag_status(fault->state, fault->tag, PPR_SUCCESS);
> -             break;
> -     case AMD_IOMMU_INV_PRI_RSP_INVALID:
> -             set_pri_tag_status(fault->state, fault->tag, PPR_INVALID);
> -             break;
> -     case AMD_IOMMU_INV_PRI_RSP_FAIL:
> -             set_pri_tag_status(fault->state, fault->tag, PPR_FAILURE);
> -             break;
> -     default:
> -             BUG();
> -     }
> -}
> -
> -static bool access_error(struct vm_area_struct *vma, struct fault *fault) -{
> -     unsigned long requested = 0;
> -
> -     if (fault->flags & PPR_FAULT_EXEC)
> -             requested |= VM_EXEC;
> -
> -     if (fault->flags & PPR_FAULT_READ)
> -             requested |= VM_READ;
> -
> -     if (fault->flags & PPR_FAULT_WRITE)
> -             requested |= VM_WRITE;
> -
> -     return (requested & ~vma->vm_flags) != 0;
> -}
> -
> -static void do_fault(struct work_struct *work) -{
> -     struct fault *fault = container_of(work, struct fault, work);
> -     struct vm_area_struct *vma;
> -     vm_fault_t ret = VM_FAULT_ERROR;
> -     unsigned int flags = 0;
> -     struct mm_struct *mm;
> -     u64 address;
> -
> -     mm = fault->state->mm;
> -     address = fault->address;
> -
> -     if (fault->flags & PPR_FAULT_USER)
> -             flags |= FAULT_FLAG_USER;
> -     if (fault->flags & PPR_FAULT_WRITE)
> -             flags |= FAULT_FLAG_WRITE;
> -     flags |= FAULT_FLAG_REMOTE;
> -
> -     mmap_read_lock(mm);
> -     vma = vma_lookup(mm, address);
> -     if (!vma)
> -             /* failed to get a vma in the right range */
> -             goto out;
> -
> -     /* Check if we have the right permissions on the vma */
> -     if (access_error(vma, fault))
> -             goto out;
> -
> -     ret = handle_mm_fault(vma, address, flags, NULL);
> -out:
> -     mmap_read_unlock(mm);
> -
> -     if (ret & VM_FAULT_ERROR)
> -             /* failed to service fault */
> -             handle_fault_error(fault);
> -
> -     finish_pri_tag(fault->dev_state, fault->state, fault->tag);
> -
> -     put_pasid_state(fault->state);
> -
> -     kfree(fault);
> -}
> -
> -static int ppr_notifier(struct notifier_block *nb, unsigned long e, void *data) -
> {
> -     struct amd_iommu_fault *iommu_fault;
> -     struct pasid_state *pasid_state;
> -     struct device_state *dev_state;
> -     struct pci_dev *pdev = NULL;
> -     unsigned long flags;
> -     struct fault *fault;
> -     bool finish;
> -     u16 tag, devid, seg_id;
> -     int ret;
> -
> -     iommu_fault = data;
> -     tag         = iommu_fault->tag & 0x1ff;
> -     finish      = (iommu_fault->tag >> 9) & 1;
> -
> -     seg_id = PCI_SBDF_TO_SEGID(iommu_fault->sbdf);
> -     devid = PCI_SBDF_TO_DEVID(iommu_fault->sbdf);
> -     pdev = pci_get_domain_bus_and_slot(seg_id, PCI_BUS_NUM(devid),
> -                                        devid & 0xff);
> -     if (!pdev)
> -             return -ENODEV;
> -
> -     ret = NOTIFY_DONE;
> -
> -     /* In kdump kernel pci dev is not initialized yet -> send INVALID */
> -     if (amd_iommu_is_attach_deferred(&pdev->dev)) {
> -             amd_iommu_complete_ppr(pdev, iommu_fault->pasid,
> -                                    PPR_INVALID, tag);
> -             goto out;
> -     }
> -
> -     dev_state = get_device_state(iommu_fault->sbdf);
> -     if (dev_state == NULL)
> -             goto out;
> -
> -     pasid_state = get_pasid_state(dev_state, iommu_fault->pasid);
> -     if (pasid_state == NULL || pasid_state->invalid) {
> -             /* We know the device but not the PASID -> send INVALID */
> -             amd_iommu_complete_ppr(dev_state->pdev, iommu_fault-
> >pasid,
> -                                    PPR_INVALID, tag);
> -             goto out_drop_state;
> -     }
> -
> -     spin_lock_irqsave(&pasid_state->lock, flags);
> -     atomic_inc(&pasid_state->pri[tag].inflight);
> -     if (finish)
> -             pasid_state->pri[tag].finish = true;
> -     spin_unlock_irqrestore(&pasid_state->lock, flags);
> -
> -     fault = kzalloc(sizeof(*fault), GFP_ATOMIC);
> -     if (fault == NULL) {
> -             /* We are OOM - send success and let the device re-fault */
> -             finish_pri_tag(dev_state, pasid_state, tag);
> -             goto out_drop_state;
> -     }
> -
> -     fault->dev_state = dev_state;
> -     fault->address   = iommu_fault->address;
> -     fault->state     = pasid_state;
> -     fault->tag       = tag;
> -     fault->finish    = finish;
> -     fault->pasid     = iommu_fault->pasid;
> -     fault->flags     = iommu_fault->flags;
> -     INIT_WORK(&fault->work, do_fault);
> -
> -     queue_work(iommu_wq, &fault->work);
> -
> -     ret = NOTIFY_OK;
> -
> -out_drop_state:
> -
> -     if (ret != NOTIFY_OK && pasid_state)
> -             put_pasid_state(pasid_state);
> -
> -     put_device_state(dev_state);
> -
> -out:
> -     pci_dev_put(pdev);
> -     return ret;
> -}
> -
> -static struct notifier_block ppr_nb = {
> -     .notifier_call = ppr_notifier,
> -};
> -
> -int amd_iommu_bind_pasid(struct pci_dev *pdev, u32 pasid,
> -                      struct task_struct *task)
> -{
> -     struct pasid_state *pasid_state;
> -     struct device_state *dev_state;
> -     struct mm_struct *mm;
> -     u32 sbdf;
> -     int ret;
> -
> -     might_sleep();
> -
> -     if (!amd_iommu_v2_supported())
> -             return -ENODEV;
> -
> -     sbdf      = get_pci_sbdf_id(pdev);
> -     dev_state = get_device_state(sbdf);
> -
> -     if (dev_state == NULL)
> -             return -EINVAL;
> -
> -     ret = -EINVAL;
> -     if (pasid >= dev_state->max_pasids)
> -             goto out;
> -
> -     ret = -ENOMEM;
> -     pasid_state = kzalloc(sizeof(*pasid_state), GFP_KERNEL);
> -     if (pasid_state == NULL)
> -             goto out;
> -
> -
> -     refcount_set(&pasid_state->count, 1);
> -     init_waitqueue_head(&pasid_state->wq);
> -     spin_lock_init(&pasid_state->lock);
> -
> -     mm                        = get_task_mm(task);
> -     pasid_state->mm           = mm;
> -     pasid_state->device_state = dev_state;
> -     pasid_state->pasid        = pasid;
> -     pasid_state->invalid      = true; /* Mark as valid only if we are
> -                                          done with setting up the pasid */
> -     pasid_state->mn.ops       = &iommu_mn;
> -
> -     if (pasid_state->mm == NULL)
> -             goto out_free;
> -
> -     ret = mmu_notifier_register(&pasid_state->mn, mm);
> -     if (ret)
> -             goto out_free;
> -
> -     ret = set_pasid_state(dev_state, pasid_state, pasid);
> -     if (ret)
> -             goto out_unregister;
> -
> -     ret = amd_iommu_domain_set_gcr3(dev_state->domain, pasid,
> -                                     __pa(pasid_state->mm->pgd));
> -     if (ret)
> -             goto out_clear_state;
> -
> -     /* Now we are ready to handle faults */
> -     pasid_state->invalid = false;
> -
> -     /*
> -      * Drop the reference to the mm_struct here. We rely on the
> -      * mmu_notifier release call-back to inform us when the mm
> -      * is going away.
> -      */
> -     mmput(mm);
> -
> -     return 0;
> -
> -out_clear_state:
> -     clear_pasid_state(dev_state, pasid);
> -
> -out_unregister:
> -     mmu_notifier_unregister(&pasid_state->mn, mm);
> -     mmput(mm);
> -
> -out_free:
> -     free_pasid_state(pasid_state);
> -
> -out:
> -     put_device_state(dev_state);
> -
> -     return ret;
> -}
> -EXPORT_SYMBOL(amd_iommu_bind_pasid);
> -
> -void amd_iommu_unbind_pasid(struct pci_dev *pdev, u32 pasid) -{
> -     struct pasid_state *pasid_state;
> -     struct device_state *dev_state;
> -     u32 sbdf;
> -
> -     might_sleep();
> -
> -     if (!amd_iommu_v2_supported())
> -             return;
> -
> -     sbdf = get_pci_sbdf_id(pdev);
> -     dev_state = get_device_state(sbdf);
> -     if (dev_state == NULL)
> -             return;
> -
> -     if (pasid >= dev_state->max_pasids)
> -             goto out;
> -
> -     pasid_state = get_pasid_state(dev_state, pasid);
> -     if (pasid_state == NULL)
> -             goto out;
> -     /*
> -      * Drop reference taken here. We are safe because we still hold
> -      * the reference taken in the amd_iommu_bind_pasid function.
> -      */
> -     put_pasid_state(pasid_state);
> -
> -     /* Clear the pasid state so that the pasid can be re-used */
> -     clear_pasid_state(dev_state, pasid_state->pasid);
> -
> -     /*
> -      * Call mmu_notifier_unregister to drop our reference
> -      * to pasid_state->mm
> -      */
> -     mmu_notifier_unregister(&pasid_state->mn, pasid_state->mm);
> -
> -     put_pasid_state_wait(pasid_state); /* Reference taken in
> -                                           amd_iommu_bind_pasid */
> -out:
> -     /* Drop reference taken in this function */
> -     put_device_state(dev_state);
> -
> -     /* Drop reference taken in amd_iommu_bind_pasid */
> -     put_device_state(dev_state);
> -}
> -EXPORT_SYMBOL(amd_iommu_unbind_pasid);
> -
> -int amd_iommu_init_device(struct pci_dev *pdev, int pasids) -{
> -     struct device_state *dev_state;
> -     struct iommu_group *group;
> -     unsigned long flags;
> -     int ret, tmp;
> -     u32 sbdf;
> -
> -     might_sleep();
> -
> -     /*
> -      * When memory encryption is active the device is likely not in a
> -      * direct-mapped domain. Forbid using IOMMUv2 functionality for
> now.
> -      */
> -     if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> -             return -ENODEV;
> -
> -     if (!amd_iommu_v2_supported())
> -             return -ENODEV;
> -
> -     if (pasids <= 0 || pasids > (PASID_MASK + 1))
> -             return -EINVAL;
> -
> -     sbdf = get_pci_sbdf_id(pdev);
> -
> -     dev_state = kzalloc(sizeof(*dev_state), GFP_KERNEL);
> -     if (dev_state == NULL)
> -             return -ENOMEM;
> -
> -     spin_lock_init(&dev_state->lock);
> -     init_waitqueue_head(&dev_state->wq);
> -     dev_state->pdev  = pdev;
> -     dev_state->sbdf = sbdf;
> -
> -     tmp = pasids;
> -     for (dev_state->pasid_levels = 0; (tmp - 1) & ~0x1ff; tmp >>= 9)
> -             dev_state->pasid_levels += 1;
> -
> -     atomic_set(&dev_state->count, 1);
> -     dev_state->max_pasids = pasids;
> -
> -     ret = -ENOMEM;
> -     dev_state->states = (void *)get_zeroed_page(GFP_KERNEL);
> -     if (dev_state->states == NULL)
> -             goto out_free_dev_state;
> -
> -     dev_state->domain = iommu_domain_alloc(&pci_bus_type);
> -     if (dev_state->domain == NULL)
> -             goto out_free_states;
> -
> -     /* See iommu_is_default_domain() */
> -     dev_state->domain->type = IOMMU_DOMAIN_IDENTITY;
> -     amd_iommu_domain_direct_map(dev_state->domain);
> -
> -     ret = amd_iommu_domain_enable_v2(dev_state->domain, pasids);
> -     if (ret)
> -             goto out_free_domain;
> -
> -     group = iommu_group_get(&pdev->dev);
> -     if (!group) {
> -             ret = -EINVAL;
> -             goto out_free_domain;
> -     }
> -
> -     ret = iommu_attach_group(dev_state->domain, group);
> -     if (ret != 0)
> -             goto out_drop_group;
> -
> -     iommu_group_put(group);
> -
> -     spin_lock_irqsave(&state_lock, flags);
> -
> -     if (__get_device_state(sbdf) != NULL) {
> -             spin_unlock_irqrestore(&state_lock, flags);
> -             ret = -EBUSY;
> -             goto out_free_domain;
> -     }
> -
> -     list_add_tail(&dev_state->list, &state_list);
> -
> -     spin_unlock_irqrestore(&state_lock, flags);
> -
> -     return 0;
> -
> -out_drop_group:
> -     iommu_group_put(group);
> -
> -out_free_domain:
> -     iommu_domain_free(dev_state->domain);
> -
> -out_free_states:
> -     free_page((unsigned long)dev_state->states);
> -
> -out_free_dev_state:
> -     kfree(dev_state);
> -
> -     return ret;
> -}
> -EXPORT_SYMBOL(amd_iommu_init_device);
> -
> -void amd_iommu_free_device(struct pci_dev *pdev) -{
> -     struct device_state *dev_state;
> -     unsigned long flags;
> -     u32 sbdf;
> -
> -     if (!amd_iommu_v2_supported())
> -             return;
> -
> -     sbdf = get_pci_sbdf_id(pdev);
> -
> -     spin_lock_irqsave(&state_lock, flags);
> -
> -     dev_state = __get_device_state(sbdf);
> -     if (dev_state == NULL) {
> -             spin_unlock_irqrestore(&state_lock, flags);
> -             return;
> -     }
> -
> -     list_del(&dev_state->list);
> -
> -     spin_unlock_irqrestore(&state_lock, flags);
> -
> -     put_device_state(dev_state);
> -     free_device_state(dev_state);
> -}
> -EXPORT_SYMBOL(amd_iommu_free_device);
> -
> -int amd_iommu_set_invalid_ppr_cb(struct pci_dev *pdev,
> -                              amd_iommu_invalid_ppr_cb cb)
> -{
> -     struct device_state *dev_state;
> -     unsigned long flags;
> -     u32 sbdf;
> -     int ret;
> -
> -     if (!amd_iommu_v2_supported())
> -             return -ENODEV;
> -
> -     sbdf = get_pci_sbdf_id(pdev);
> -
> -     spin_lock_irqsave(&state_lock, flags);
> -
> -     ret = -EINVAL;
> -     dev_state = __get_device_state(sbdf);
> -     if (dev_state == NULL)
> -             goto out_unlock;
> -
> -     dev_state->inv_ppr_cb = cb;
> -
> -     ret = 0;
> -
> -out_unlock:
> -     spin_unlock_irqrestore(&state_lock, flags);
> -
> -     return ret;
> -}
> -EXPORT_SYMBOL(amd_iommu_set_invalid_ppr_cb);
> -
> -int amd_iommu_set_invalidate_ctx_cb(struct pci_dev *pdev,
> -                                 amd_iommu_invalidate_ctx cb)
> -{
> -     struct device_state *dev_state;
> -     unsigned long flags;
> -     u32 sbdf;
> -     int ret;
> -
> -     if (!amd_iommu_v2_supported())
> -             return -ENODEV;
> -
> -     sbdf = get_pci_sbdf_id(pdev);
> -
> -     spin_lock_irqsave(&state_lock, flags);
> -
> -     ret = -EINVAL;
> -     dev_state = __get_device_state(sbdf);
> -     if (dev_state == NULL)
> -             goto out_unlock;
> -
> -     dev_state->inv_ctx_cb = cb;
> -
> -     ret = 0;
> -
> -out_unlock:
> -     spin_unlock_irqrestore(&state_lock, flags);
> -
> -     return ret;
> -}
> -EXPORT_SYMBOL(amd_iommu_set_invalidate_ctx_cb);
> -
> -static int __init amd_iommu_v2_init(void) -{
> -     int ret;
> -
> -     if (!amd_iommu_v2_supported()) {
> -             pr_info("AMD IOMMUv2 functionality not available on this
> system - This is not a bug.\n");
> -             /*
> -              * Load anyway to provide the symbols to other modules
> -              * which may use AMD IOMMUv2 optionally.
> -              */
> -             return 0;
> -     }
> -
> -     ret = -ENOMEM;
> -     iommu_wq = alloc_workqueue("amd_iommu_v2",
> WQ_MEM_RECLAIM, 0);
> -     if (iommu_wq == NULL)
> -             goto out;
> -
> -     amd_iommu_register_ppr_notifier(&ppr_nb);
> -
> -     pr_info("AMD IOMMUv2 loaded and initialized\n");
> -
> -     return 0;
> -
> -out:
> -     return ret;
> -}
> -
> -static void __exit amd_iommu_v2_exit(void) -{
> -     struct device_state *dev_state, *next;
> -     unsigned long flags;
> -     LIST_HEAD(freelist);
> -
> -     if (!amd_iommu_v2_supported())
> -             return;
> -
> -     amd_iommu_unregister_ppr_notifier(&ppr_nb);
> -
> -     flush_workqueue(iommu_wq);
> -
> -     /*
> -      * The loop below might call flush_workqueue(), so call
> -      * destroy_workqueue() after it
> -      */
> -     spin_lock_irqsave(&state_lock, flags);
> -
> -     list_for_each_entry_safe(dev_state, next, &state_list, list) {
> -             WARN_ON_ONCE(1);
> -
> -             put_device_state(dev_state);
> -             list_del(&dev_state->list);
> -             list_add_tail(&dev_state->list, &freelist);
> -     }
> -
> -     spin_unlock_irqrestore(&state_lock, flags);
> -
> -     /*
> -      * Since free_device_state waits on the count to be zero,
> -      * we need to free dev_state outside the spinlock.
> -      */
> -     list_for_each_entry_safe(dev_state, next, &freelist, list) {
> -             list_del(&dev_state->list);
> -             free_device_state(dev_state);
> -     }
> -
> -     destroy_workqueue(iommu_wq);
> -}
> -
> -module_init(amd_iommu_v2_init);
> -module_exit(amd_iommu_v2_exit);
> diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h index
> 99a5201d9e62..35ca00b09384 100644
> --- a/include/linux/amd-iommu.h
> +++ b/include/linux/amd-iommu.h
> @@ -33,84 +33,6 @@ struct pci_dev;
>
>  extern int amd_iommu_detect(void);
>
> -/**
> - * amd_iommu_init_device() - Init device for use with IOMMUv2 driver
> - * @pdev: The PCI device to initialize
> - * @pasids: Number of PASIDs to support for this device
> - *
> - * This function does all setup for the device pdev so that it can be
> - * used with IOMMUv2.
> - * Returns 0 on success or negative value on error.
> - */
> -extern int amd_iommu_init_device(struct pci_dev *pdev, int pasids);
> -
> -/**
> - * amd_iommu_free_device() - Free all IOMMUv2 related device resources
> - *                        and disable IOMMUv2 usage for this device
> - * @pdev: The PCI device to disable IOMMUv2 usage for'
> - */
> -extern void amd_iommu_free_device(struct pci_dev *pdev);
> -
> -/**
> - * amd_iommu_bind_pasid() - Bind a given task to a PASID on a device
> - * @pdev: The PCI device to bind the task to
> - * @pasid: The PASID on the device the task should be bound to
> - * @task: the task to bind
> - *
> - * The function returns 0 on success or a negative value on error.
> - */
> -extern int amd_iommu_bind_pasid(struct pci_dev *pdev, u32 pasid,
> -                             struct task_struct *task);
> -
> -/**
> - * amd_iommu_unbind_pasid() - Unbind a PASID from its task on
> - *                         a device
> - * @pdev: The device of the PASID
> - * @pasid: The PASID to unbind
> - *
> - * When this function returns the device is no longer using the PASID
> - * and the PASID is no longer bound to its task.
> - */
> -extern void amd_iommu_unbind_pasid(struct pci_dev *pdev, u32 pasid);
> -
> -/**
> - * amd_iommu_set_invalid_ppr_cb() - Register a call-back for failed
> - *                               PRI requests
> - * @pdev: The PCI device the call-back should be registered for
> - * @cb: The call-back function
> - *
> - * The IOMMUv2 driver invokes this call-back when it is unable to
> - * successfully handle a PRI request. The device driver can then decide
> - * which PRI response the device should see. Possible return values for
> - * the call-back are:
> - *
> - * - AMD_IOMMU_INV_PRI_RSP_SUCCESS - Send SUCCESS back to the
> device
> - * - AMD_IOMMU_INV_PRI_RSP_INVALID - Send INVALID back to the device
> - * - AMD_IOMMU_INV_PRI_RSP_FAIL    - Send Failure back to the device,
> - *                                the device is required to disable
> - *                                PRI when it receives this response
> - *
> - * The function returns 0 on success or negative value on error.
> - */
> -#define AMD_IOMMU_INV_PRI_RSP_SUCCESS        0
> -#define AMD_IOMMU_INV_PRI_RSP_INVALID        1
> -#define AMD_IOMMU_INV_PRI_RSP_FAIL   2
> -
> -typedef int (*amd_iommu_invalid_ppr_cb)(struct pci_dev *pdev,
> -                                     u32 pasid,
> -                                     unsigned long address,
> -                                     u16);
> -
> -extern int amd_iommu_set_invalid_ppr_cb(struct pci_dev *pdev,
> -                                     amd_iommu_invalid_ppr_cb cb);
> -
> -#define PPR_FAULT_EXEC       (1 << 1)
> -#define PPR_FAULT_READ  (1 << 2)
> -#define PPR_FAULT_WRITE (1 << 5)
> -#define PPR_FAULT_USER  (1 << 6)
> -#define PPR_FAULT_RSVD  (1 << 7)
> -#define PPR_FAULT_GN    (1 << 8)
> -
>  /**
>   * amd_iommu_device_info() - Get information about IOMMUv2 support of a
>   *                        PCI device
> @@ -137,22 +59,6 @@ struct amd_iommu_device_info {  extern int
> amd_iommu_device_info(struct pci_dev *pdev,
>                                struct amd_iommu_device_info *info);
>
> -/**
> - * amd_iommu_set_invalidate_ctx_cb() - Register a call-back for invalidating
> - *                                  a pasid context. This call-back is
> - *                                  invoked when the IOMMUv2 driver needs
> to
> - *                                  invalidate a PASID context, for example
> - *                                  because the task that is bound to that
> - *                                  context is about to exit.
> - *
> - * @pdev: The PCI device the call-back should be registered for
> - * @cb: The call-back function
> - */
> -
> -typedef void (*amd_iommu_invalidate_ctx)(struct pci_dev *pdev, u32 pasid);
> -
> -extern int amd_iommu_set_invalidate_ctx_cb(struct pci_dev *pdev,
> -                                        amd_iommu_invalidate_ctx cb);
>  #else /* CONFIG_AMD_IOMMU */
>
>  static inline int amd_iommu_detect(void) { return -ENODEV; }
> --
> 2.31.1

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Jason Gunthorpe]

On Thu, Sep 21, 2023 at 02:05:37PM +0000, Deucher, Alexander wrote:
> [AMD Official Use Only - General]
> 
> > -----Original Message-----
> > From: Hegde, Vasant <Vasant.Hegde@amd.com>
> > Sent: Thursday, September 21, 2023 5:32 AM
> > To: iommu@lists.linux.dev; joro@8bytes.org
> > Cc: Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Huang2, Wei
> > <Wei.Huang2@amd.com>; jsnitsel@redhat.com; jgg@ziepe.ca; Hegde, Vasant
> > <Vasant.Hegde@amd.com>; Deucher, Alexander
> > <Alexander.Deucher@amd.com>; Kuehling, Felix <Felix.Kuehling@amd.com>;
> > Jason Gunthorpe <jgg@nvidia.com>
> > Subject: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
> >
> > AMD GPU driver which was the only in-kernel user of iommu_v2 module
> > removed dependency on iommu_v2 module.
> >
> > Also we are working on adding SVA support in AMD IOMMU driver. Device
> > drivers are expected to use common SVA framework to enable device
> > PASID/PRI features.
> >
> > Removing iommu_v2 module and then adding SVA simplifies the
> > development.
> > Hence remove iommu_v2 module.
> 
> Does this patch or the following patches make any functional changes
> for devices?  E.g., devices which supported ATS would have been put
> into an identity mapping mode previously Is that still retained?

Huh? Why would we ever want to do that?

Policy for the default domain belongs in the iommu subsystem except in
extreme cases, the AMD driver should not be doing random things like
forcing identity for ATS capable PCI devices.

Jason

RE: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Deucher, Alexander]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, September 21, 2023 10:14 AM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>
> Cc: Hegde, Vasant <Vasant.Hegde@amd.com>; iommu@lists.linux.dev;
> joro@8bytes.org; Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>;
> Huang2, Wei <Wei.Huang2@amd.com>; jsnitsel@redhat.com; Kuehling, Felix
> <Felix.Kuehling@amd.com>
> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>
> On Thu, Sep 21, 2023 at 02:05:37PM +0000, Deucher, Alexander wrote:
> > [AMD Official Use Only - General]
> >
> > > -----Original Message-----
> > > From: Hegde, Vasant <Vasant.Hegde@amd.com>
> > > Sent: Thursday, September 21, 2023 5:32 AM
> > > To: iommu@lists.linux.dev; joro@8bytes.org
> > > Cc: Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Huang2,
> > > Wei <Wei.Huang2@amd.com>; jsnitsel@redhat.com; jgg@ziepe.ca;
> Hegde,
> > > Vasant <Vasant.Hegde@amd.com>; Deucher, Alexander
> > > <Alexander.Deucher@amd.com>; Kuehling, Felix
> > > <Felix.Kuehling@amd.com>; Jason Gunthorpe <jgg@nvidia.com>
> > > Subject: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
> > >
> > > AMD GPU driver which was the only in-kernel user of iommu_v2 module
> > > removed dependency on iommu_v2 module.
> > >
> > > Also we are working on adding SVA support in AMD IOMMU driver.
> > > Device drivers are expected to use common SVA framework to enable
> > > device PASID/PRI features.
> > >
> > > Removing iommu_v2 module and then adding SVA simplifies the
> > > development.
> > > Hence remove iommu_v2 module.
> >
> > Does this patch or the following patches make any functional changes
> > for devices?  E.g., devices which supported ATS would have been put
> > into an identity mapping mode previously Is that still retained?
>
> Huh? Why would we ever want to do that?
>
> Policy for the default domain belongs in the iommu subsystem except in
> extreme cases, the AMD driver should not be doing random things like forcing
> identity for ATS capable PCI devices.

I'm just concerned about regressions in random devices due to a change in policy in the IOMMU driver.  Previously the IOMMU driver would put ATS compatible devices into 1:1 mode.  Also some of the earlier integrated GPUs require 1:1 mapping for display from system memory due to hardware limitations.  There were also a lot of sbios bugs in the carrizo/raven timeframe because windows didn't enable the IOMMU so lots of OEMs had bogus IOMMU ACPI tables which didn't cause problems when 1:1 mode was used.

Alex

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Jason Gunthorpe]

On Thu, Sep 21, 2023 at 03:15:42PM +0000, Deucher, Alexander wrote:
> > > Does this patch or the following patches make any functional changes
> > > for devices?  E.g., devices which supported ATS would have been put
> > > into an identity mapping mode previously Is that still retained?
> >
> > Huh? Why would we ever want to do that?
> >
> > Policy for the default domain belongs in the iommu subsystem except in
> > extreme cases, the AMD driver should not be doing random things like forcing
> > identity for ATS capable PCI devices.
> 
> I'm just concerned about regressions in random devices due to a
> change in policy in the IOMMU driver.  Previously the IOMMU driver
> would put ATS compatible devices into 1:1 mode.  Also some of the
> earlier integrated GPUs require 1:1 mapping for display from system
> memory due to hardware limitations.  There were also a lot of sbios
> bugs in the carrizo/raven timeframe because windows didn't enable
> the IOMMU so lots of OEMs had bogus IOMMU ACPI tables which didn't
> cause problems when 1:1 mode was used.

That is a fair concern.

But we need to root those out and fix them as narrow quirks. We don't
want a driver to have blanket ATS == identity configuration, it will
harm other legitimate applications.

eg if you want to narrowly quirk integrated GPUs and carrizo/raven
that would be great.

Jason

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Baolu Lu]

On 9/22/23 12:31 AM, Jason Gunthorpe wrote:
> On Thu, Sep 21, 2023 at 03:15:42PM +0000, Deucher, Alexander wrote:
>>>> Does this patch or the following patches make any functional changes
>>>> for devices?  E.g., devices which supported ATS would have been put
>>>> into an identity mapping mode previously Is that still retained?
>>>
>>> Huh? Why would we ever want to do that?
>>>
>>> Policy for the default domain belongs in the iommu subsystem except in
>>> extreme cases, the AMD driver should not be doing random things like forcing
>>> identity for ATS capable PCI devices.
>>
>> I'm just concerned about regressions in random devices due to a
>> change in policy in the IOMMU driver.  Previously the IOMMU driver
>> would put ATS compatible devices into 1:1 mode.  Also some of the
>> earlier integrated GPUs require 1:1 mapping for display from system
>> memory due to hardware limitations.  There were also a lot of sbios
>> bugs in the carrizo/raven timeframe because windows didn't enable
>> the IOMMU so lots of OEMs had bogus IOMMU ACPI tables which didn't
>> cause problems when 1:1 mode was used.
> 
> That is a fair concern.
> 
> But we need to root those out and fix them as narrow quirks. We don't
> want a driver to have blanket ATS == identity configuration, it will
> harm other legitimate applications.

I am interested in the policy that an IOMMU driver would apply to ATS
when the default domain is configured in PASSTHROUGH mode.

I have considered the following cases:

1) PCI device supports ATS but *no* PASID
    In this case, ATS only means a TLB cache in the device, and it does
    not make much sense to cache the 1:1 mappings in the device. Even
    worse, the ATS translation requests could probably cause I/O
    congestion in corner cases. Therefore, the best choice is probably to
    disable ATS on the device.

2) PCI device supports ATS and PASID
    In this case, ATS is required for PRI, so we should enable it even if
    the default domain has been configured to PASSTHROUGH mode.

Is this the right way to think about it?

Best regards,
baolu

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Vasant Hegde]

Hi Baolu,

On 9/22/2023 7:53 AM, Baolu Lu wrote:
> On 9/22/23 12:31 AM, Jason Gunthorpe wrote:
>> On Thu, Sep 21, 2023 at 03:15:42PM +0000, Deucher, Alexander wrote:
>>>>> Does this patch or the following patches make any functional changes
>>>>> for devices?  E.g., devices which supported ATS would have been put
>>>>> into an identity mapping mode previously Is that still retained?
>>>>
>>>> Huh? Why would we ever want to do that?
>>>>
>>>> Policy for the default domain belongs in the iommu subsystem except in
>>>> extreme cases, the AMD driver should not be doing random things like forcing
>>>> identity for ATS capable PCI devices.
>>>
>>> I'm just concerned about regressions in random devices due to a
>>> change in policy in the IOMMU driver.  Previously the IOMMU driver
>>> would put ATS compatible devices into 1:1 mode.  Also some of the
>>> earlier integrated GPUs require 1:1 mapping for display from system
>>> memory due to hardware limitations.  There were also a lot of sbios
>>> bugs in the carrizo/raven timeframe because windows didn't enable
>>> the IOMMU so lots of OEMs had bogus IOMMU ACPI tables which didn't
>>> cause problems when 1:1 mode was used.
>>
>> That is a fair concern.
>>
>> But we need to root those out and fix them as narrow quirks. We don't
>> want a driver to have blanket ATS == identity configuration, it will
>> harm other legitimate applications.
> 
> I am interested in the policy that an IOMMU driver would apply to ATS
> when the default domain is configured in PASSTHROUGH mode.
> 
> I have considered the following cases:
> 
> 1) PCI device supports ATS but *no* PASID
>    In this case, ATS only means a TLB cache in the device, and it does
>    not make much sense to cache the 1:1 mappings in the device. Even
>    worse, the ATS translation requests could probably cause I/O
>    congestion in corner cases. Therefore, the best choice is probably to
>    disable ATS on the device.

IMO we shouldn't enable ATS in passthrough mode.

Today AMD IOMMU driver by default enables ATS (even if device is in passthrough
mode). This came up in one of our internal discussion. I intend to look into it
ATS related stuff in detail after SVA series.


> 
> 2) PCI device supports ATS and PASID
>    In this case, ATS is required for PRI, so we should enable it even if
>    the default domain has been configured to PASSTHROUGH mode.

IMO we should enable ATS along with enable_feature(PRI).


-Vasant

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Jason Gunthorpe]

On Fri, Sep 22, 2023 at 02:22:11PM +0530, Vasant Hegde wrote:
> > 2) PCI device supports ATS and PASID
> >    In this case, ATS is required for PRI, so we should enable it even if
> >    the default domain has been configured to PASSTHROUGH mode.
> 
> IMO we should enable ATS along with enable_feature(PRI).

Definately not. ATS is valuable and importnat without PRI.

Jason

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Vasant Hegde]

Jason,

On 9/22/2023 5:30 PM, Jason Gunthorpe wrote:
> On Fri, Sep 22, 2023 at 02:22:11PM +0530, Vasant Hegde wrote:
>>> 2) PCI device supports ATS and PASID
>>>    In this case, ATS is required for PRI, so we should enable it even if
>>>    the default domain has been configured to PASSTHROUGH mode.
>>
>> IMO we should enable ATS along with enable_feature(PRI).
> 
> Definately not. ATS is valuable and importnat without PRI.

I get that. But I am not sure whether I understood the usecase when domain is in
passthrough mode. Do you have any usecase to enable ATS when device is in pass
through mode?

-Vasant

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Jason Gunthorpe]

On Mon, Sep 25, 2023 at 08:41:41PM +0530, Vasant Hegde wrote:
> Jason,
> 
> On 9/22/2023 5:30 PM, Jason Gunthorpe wrote:
> > On Fri, Sep 22, 2023 at 02:22:11PM +0530, Vasant Hegde wrote:
> >>> 2) PCI device supports ATS and PASID
> >>>    In this case, ATS is required for PRI, so we should enable it even if
> >>>    the default domain has been configured to PASSTHROUGH mode.
> >>
> >> IMO we should enable ATS along with enable_feature(PRI).
> > 
> > Definately not. ATS is valuable and importnat without PRI.
> 
> I get that. But I am not sure whether I understood the usecase when domain is in
> passthrough mode. Do you have any usecase to enable ATS when device is in pass
> through mode?

Yes, we can have the main device functionality running in passthrough
and still support SVA on a PASID. This requires ATS to be globally on
for the function.

Jason

RE: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: Hegde, Vasant <Vasant.Hegde@amd.com>
> Sent: Monday, September 25, 2023 11:12 AM
> To: Jason Gunthorpe <jgg@nvidia.com>
> Cc: Baolu Lu <baolu.lu@linux.intel.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; iommu@lists.linux.dev; joro@8bytes.org;
> Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Huang2, Wei
> <Wei.Huang2@amd.com>; jsnitsel@redhat.com; Kuehling, Felix
> <Felix.Kuehling@amd.com>
> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>
> Jason,
>
> On 9/22/2023 5:30 PM, Jason Gunthorpe wrote:
> > On Fri, Sep 22, 2023 at 02:22:11PM +0530, Vasant Hegde wrote:
> >>> 2) PCI device supports ATS and PASID
> >>>    In this case, ATS is required for PRI, so we should enable it
> >>> even if
> >>>    the default domain has been configured to PASSTHROUGH mode.
> >>
> >> IMO we should enable ATS along with enable_feature(PRI).
> >
> > Definately not. ATS is valuable and importnat without PRI.
>
> I get that. But I am not sure whether I understood the usecase when domain is
> in passthrough mode. Do you have any usecase to enable ATS when device is
> in pass through mode?

It should still reduce latency.  Even in 1:1 mode, there is still an address lookup that can be cached.

Alex

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Jason Gunthorpe]

On Mon, Sep 25, 2023 at 04:31:50PM +0000, Deucher, Alexander wrote:

> > On 9/22/2023 5:30 PM, Jason Gunthorpe wrote:
> > > On Fri, Sep 22, 2023 at 02:22:11PM +0530, Vasant Hegde wrote:
> > >>> 2) PCI device supports ATS and PASID
> > >>>    In this case, ATS is required for PRI, so we should enable it
> > >>> even if
> > >>>    the default domain has been configured to PASSTHROUGH mode.
> > >>
> > >> IMO we should enable ATS along with enable_feature(PRI).
> > >
> > > Definately not. ATS is valuable and importnat without PRI.
> >
> > I get that. But I am not sure whether I understood the usecase when domain is
> > in passthrough mode. Do you have any usecase to enable ATS when device is
> > in pass through mode?
> 
> It should still reduce latency.  Even in 1:1 mode, there is still an
> address lookup that can be cached.

Oh, on a lot of devices the ATS bits just enable device's PCI to issue
ATS, it doesn't mean that *every* operation has to be ATS.

So the non-SVA transactions can freely run in non-ATS mode with 1:1
while only SVA operations use ATS.

IIRC most IOMMU still have a lookup for a translated request to
validate that the RID is authorized to use them. So there is no
savings, just overhead.

Jason

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Jason Gunthorpe]

On Fri, Sep 22, 2023 at 10:23:26AM +0800, Baolu Lu wrote:

> 1) PCI device supports ATS but *no* PASID
>    In this case, ATS only means a TLB cache in the device, and it does
>    not make much sense to cache the 1:1 mappings in the device. Even
>    worse, the ATS translation requests could probably cause I/O
>    congestion in corner cases. Therefore, the best choice is probably to
>    disable ATS on the device.

Nope. We have important use cases where ATS must always be on. :(

You might make this argument if ACS is also non-isolating though..

Regardless I think we need to get into a position where the iommu core
is deciding if PRI or ATS is enabled for a device, not the iommu
driver.

Jason

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Baolu Lu]

On 2023/9/22 19:59, Jason Gunthorpe wrote:
> On Fri, Sep 22, 2023 at 10:23:26AM +0800, Baolu Lu wrote:
> 
>> 1) PCI device supports ATS but*no*  PASID
>>     In this case, ATS only means a TLB cache in the device, and it does
>>     not make much sense to cache the 1:1 mappings in the device. Even
>>     worse, the ATS translation requests could probably cause I/O
>>     congestion in corner cases. Therefore, the best choice is probably to
>>     disable ATS on the device.
> Nope. We have important use cases where ATS must always be on. 🙁

Oh, I didn't know that. Thank you for letting me know.

> 
> You might make this argument if ACS is also non-isolating though..
> 
> Regardless I think we need to get into a position where the iommu core
> is deciding if PRI or ATS is enabled for a device, not the iommu
> driver.

Agreed. I ever had a series to achieve this and it may be time to
revisit them.

One additional concern is how the core knows whether ATS should be
enabled. In my previous design, the IOMMU core turns on ATS by default
if the device is capable of it, but the driver could enable/disable it
from its driver probe() callback.

Does it make sense?

Best regards,
baolu

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Jason Gunthorpe]

On Fri, Sep 22, 2023 at 08:13:02PM +0800, Baolu Lu wrote:

> > You might make this argument if ACS is also non-isolating though..
> > 
> > Regardless I think we need to get into a position where the iommu core
> > is deciding if PRI or ATS is enabled for a device, not the iommu
> > driver.
> 
> Agreed. I ever had a series to achieve this and it may be time to
> revisit them.
> 
> One additional concern is how the core knows whether ATS should be
> enabled. In my previous design, the IOMMU core turns on ATS by default
> if the device is capable of it, but the driver could enable/disable it
> from its driver probe() callback.

I think that is a good place to start, maybe we have some command line
and/or sysfs like we have for the default domain policy.

IMHO iommu drivers should not override this.

I think it was a mistake that iommu drivers could override to identity
domains. If we need quirks then the core code should have the quirk
list, similar to how PCI works.

Jason

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Baolu Lu]

On 2023/9/22 20:18, Jason Gunthorpe wrote:
> On Fri, Sep 22, 2023 at 08:13:02PM +0800, Baolu Lu wrote:
> 
>>> You might make this argument if ACS is also non-isolating though..
>>>
>>> Regardless I think we need to get into a position where the iommu core
>>> is deciding if PRI or ATS is enabled for a device, not the iommu
>>> driver.
>>
>> Agreed. I ever had a series to achieve this and it may be time to
>> revisit them.
>>
>> One additional concern is how the core knows whether ATS should be
>> enabled. In my previous design, the IOMMU core turns on ATS by default
>> if the device is capable of it, but the driver could enable/disable it
>> from its driver probe() callback.
> 
> I think that is a good place to start, maybe we have some command line
> and/or sysfs like we have for the default domain policy.

Yes. It sounds reasonable.

> 
> IMHO iommu drivers should not override this.
> 
> I think it was a mistake that iommu drivers could override to identity
> domains. If we need quirks then the core code should have the quirk
> list, similar to how PCI works.

If I remember it correctly, you have posted a series to address this. Or
I might misunderstood it.

Best regards,
baolu

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Jason Gunthorpe]

On Fri, Sep 22, 2023 at 08:42:04PM +0800, Baolu Lu wrote:
> > IMHO iommu drivers should not override this.
> > 
> > I think it was a mistake that iommu drivers could override to identity
> > domains. If we need quirks then the core code should have the quirk
> > list, similar to how PCI works.
> 
> If I remember it correctly, you have posted a series to address this. Or
> I might misunderstood it.

Ye,s I have one someplace, waiting for some of the precursor stuff to
get merged before I revisit it

Jason

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[joro]

On Fri, Sep 22, 2023 at 09:18:12AM -0300, Jason Gunthorpe wrote:
> IMHO iommu drivers should not override this.
> 
> I think it was a mistake that iommu drivers could override to identity
> domains. If we need quirks then the core code should have the quirk
> list, similar to how PCI works.

Sounds reasonable, but if we transition over to an iommu core facility
for quirks, we must do it in a way that does not cause temporary
regressions for existing hardware.

That being said, Alexander, can you please test this series for
regressions on the older hardware to make sure nothing regresses there?

Regards,

	Joerg

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Vasant Hegde]

Joerg,

On 9/25/2023 2:34 PM, joro@8bytes.org wrote:
> On Fri, Sep 22, 2023 at 09:18:12AM -0300, Jason Gunthorpe wrote:
>> IMHO iommu drivers should not override this.
>>
>> I think it was a mistake that iommu drivers could override to identity
>> domains. If we need quirks then the core code should have the quirk
>> list, similar to how PCI works.
> 
> Sounds reasonable, but if we transition over to an iommu core facility
> for quirks, we must do it in a way that does not cause temporary
> regressions for existing hardware.
> 
> That being said, Alexander, can you please test this series for
> regressions on the older hardware to make sure nothing regresses there?

This series just removes iommu_v2 module related code as we don't have any
in-kernel user of this module.

It doesn't modify how we allocate default domain for devices (GPUs with
PASID/PRI will be still in 1 x 1 mapping irrespective of boot iommu mode).
Hence IMO this series should not cause any regression.

I understand Alex's concern. We will work with Alex and make sure there is no
regression whenever we modify default domain allocation (1 x 1 mapping).

-Vasant

RE: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Deucher, Alexander]

[AMD Official Use Only - General]

> -----Original Message-----
> From: joro@8bytes.org <joro@8bytes.org>
> Sent: Monday, September 25, 2023 5:05 AM
> To: Jason Gunthorpe <jgg@nvidia.com>
> Cc: Baolu Lu <baolu.lu@linux.intel.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Hegde, Vasant <Vasant.Hegde@amd.com>;
> iommu@lists.linux.dev; Suthikulpanit, Suravee
> <Suravee.Suthikulpanit@amd.com>; Huang2, Wei <Wei.Huang2@amd.com>;
> jsnitsel@redhat.com; Kuehling, Felix <Felix.Kuehling@amd.com>
> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>
> On Fri, Sep 22, 2023 at 09:18:12AM -0300, Jason Gunthorpe wrote:
> > IMHO iommu drivers should not override this.
> >
> > I think it was a mistake that iommu drivers could override to identity
> > domains. If we need quirks then the core code should have the quirk
> > list, similar to how PCI works.
>
> Sounds reasonable, but if we transition over to an iommu core facility for
> quirks, we must do it in a way that does not cause temporary regressions for
> existing hardware.
>
> That being said, Alexander, can you please test this series for regressions on
> the older hardware to make sure nothing regresses there?
>

We've tested this and can confirm no regressions.

Alex

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Vasant Hegde]

Hi Alex,


On 9/28/2023 8:22 PM, Deucher, Alexander wrote:
> [AMD Official Use Only - General]
> 
>> -----Original Message-----
>> From: joro@8bytes.org <joro@8bytes.org>
>> Sent: Monday, September 25, 2023 5:05 AM
>> To: Jason Gunthorpe <jgg@nvidia.com>
>> Cc: Baolu Lu <baolu.lu@linux.intel.com>; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Hegde, Vasant <Vasant.Hegde@amd.com>;
>> iommu@lists.linux.dev; Suthikulpanit, Suravee
>> <Suravee.Suthikulpanit@amd.com>; Huang2, Wei <Wei.Huang2@amd.com>;
>> jsnitsel@redhat.com; Kuehling, Felix <Felix.Kuehling@amd.com>
>> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>>
>> On Fri, Sep 22, 2023 at 09:18:12AM -0300, Jason Gunthorpe wrote:
>>> IMHO iommu drivers should not override this.
>>>
>>> I think it was a mistake that iommu drivers could override to identity
>>> domains. If we need quirks then the core code should have the quirk
>>> list, similar to how PCI works.
>>
>> Sounds reasonable, but if we transition over to an iommu core facility for
>> quirks, we must do it in a way that does not cause temporary regressions for
>> existing hardware.
>>
>> That being said, Alexander, can you please test this series for regressions on
>> the older hardware to make sure nothing regresses there?
>>
> 
> We've tested this and can confirm no regressions.

Thanks! I will add your 'Tested-by'. Hope you are fine with that.

-Vasant

RE: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: Hegde, Vasant <Vasant.Hegde@amd.com>
> Sent: Thursday, October 5, 2023 7:29 AM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>; joro@8bytes.org;
> Jason Gunthorpe <jgg@nvidia.com>
> Cc: Baolu Lu <baolu.lu@linux.intel.com>; iommu@lists.linux.dev;
> Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Huang2, Wei
> <Wei.Huang2@amd.com>; jsnitsel@redhat.com; Kuehling, Felix
> <Felix.Kuehling@amd.com>
> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>
> Hi Alex,
>
>
> On 9/28/2023 8:22 PM, Deucher, Alexander wrote:
> > [AMD Official Use Only - General]
> >
> >> -----Original Message-----
> >> From: joro@8bytes.org <joro@8bytes.org>
> >> Sent: Monday, September 25, 2023 5:05 AM
> >> To: Jason Gunthorpe <jgg@nvidia.com>
> >> Cc: Baolu Lu <baolu.lu@linux.intel.com>; Deucher, Alexander
> >> <Alexander.Deucher@amd.com>; Hegde, Vasant
> <Vasant.Hegde@amd.com>;
> >> iommu@lists.linux.dev; Suthikulpanit, Suravee
> >> <Suravee.Suthikulpanit@amd.com>; Huang2, Wei
> <Wei.Huang2@amd.com>;
> >> jsnitsel@redhat.com; Kuehling, Felix <Felix.Kuehling@amd.com>
> >> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
> >>
> >> On Fri, Sep 22, 2023 at 09:18:12AM -0300, Jason Gunthorpe wrote:
> >>> IMHO iommu drivers should not override this.
> >>>
> >>> I think it was a mistake that iommu drivers could override to
> >>> identity domains. If we need quirks then the core code should have
> >>> the quirk list, similar to how PCI works.
> >>
> >> Sounds reasonable, but if we transition over to an iommu core
> >> facility for quirks, we must do it in a way that does not cause
> >> temporary regressions for existing hardware.
> >>
> >> That being said, Alexander, can you please test this series for
> >> regressions on the older hardware to make sure nothing regresses there?
> >>
> >
> > We've tested this and can confirm no regressions.
>
> Thanks! I will add your 'Tested-by'. Hope you are fine with that.

Sure.  Thanks!

Alex

RE: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Friday, September 22, 2023 8:13 AM
> To: Jason Gunthorpe <jgg@nvidia.com>
> Cc: baolu.lu@linux.intel.com; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Hegde, Vasant <Vasant.Hegde@amd.com>;
> iommu@lists.linux.dev; joro@8bytes.org; Suthikulpanit, Suravee
> <Suravee.Suthikulpanit@amd.com>; Huang2, Wei <Wei.Huang2@amd.com>;
> jsnitsel@redhat.com; Kuehling, Felix <Felix.Kuehling@amd.com>
> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>
> On 2023/9/22 19:59, Jason Gunthorpe wrote:
> > On Fri, Sep 22, 2023 at 10:23:26AM +0800, Baolu Lu wrote:
> >
> >> 1) PCI device supports ATS but*no*  PASID
> >>     In this case, ATS only means a TLB cache in the device, and it does
> >>     not make much sense to cache the 1:1 mappings in the device. Even
> >>     worse, the ATS translation requests could probably cause I/O
> >>     congestion in corner cases. Therefore, the best choice is probably to
> >>     disable ATS on the device.
> > Nope. We have important use cases where ATS must always be on. 🙁
>
> Oh, I didn't know that. Thank you for letting me know.
>
> >
> > You might make this argument if ACS is also non-isolating though..
> >
> > Regardless I think we need to get into a position where the iommu core
> > is deciding if PRI or ATS is enabled for a device, not the iommu
> > driver.
>
> Agreed. I ever had a series to achieve this and it may be time to revisit them.
>
> One additional concern is how the core knows whether ATS should be
> enabled. In my previous design, the IOMMU core turns on ATS by default if
> the device is capable of it, but the driver could enable/disable it from its driver
> probe() callback.
>
> Does it make sense?

Yes please!  We have a number of discrete PCI devices which support ATS where we have to explicitly disable it via PCI quirks because ATS TLB invalidations will hang the card without some workarounds in the driver being done first.  If the driver could select when to enable ATS, we could remove those quirks and re-enable ATS on those devices.

Alex

RE: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Deucher, Alexander]

[Public]

> -----Original Message-----
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, September 21, 2023 12:32 PM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>
> Cc: Hegde, Vasant <Vasant.Hegde@amd.com>; iommu@lists.linux.dev;
> joro@8bytes.org; Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>;
> Huang2, Wei <Wei.Huang2@amd.com>; jsnitsel@redhat.com; Kuehling, Felix
> <Felix.Kuehling@amd.com>
> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>
> On Thu, Sep 21, 2023 at 03:15:42PM +0000, Deucher, Alexander wrote:
> > > > Does this patch or the following patches make any functional
> > > > changes for devices?  E.g., devices which supported ATS would have
> > > > been put into an identity mapping mode previously Is that still retained?
> > >
> > > Huh? Why would we ever want to do that?
> > >
> > > Policy for the default domain belongs in the iommu subsystem except
> > > in extreme cases, the AMD driver should not be doing random things
> > > like forcing identity for ATS capable PCI devices.
> >
> > I'm just concerned about regressions in random devices due to a change
> > in policy in the IOMMU driver.  Previously the IOMMU driver would put
> > ATS compatible devices into 1:1 mode.  Also some of the earlier
> > integrated GPUs require 1:1 mapping for display from system memory due
> > to hardware limitations.  There were also a lot of sbios bugs in the
> > carrizo/raven timeframe because windows didn't enable the IOMMU so
> > lots of OEMs had bogus IOMMU ACPI tables which didn't cause problems
> > when 1:1 mode was used.
>
> That is a fair concern.
>
> But we need to root those out and fix them as narrow quirks. We don't want a
> driver to have blanket ATS == identity configuration, it will harm other
> legitimate applications.
>
> eg if you want to narrowly quirk integrated GPUs and carrizo/raven that
> would be great.

I'm not opposed to quirks tables for those old systems.

Alex

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Vasant Hegde]

Hi Alex,


On 9/21/2023 8:45 PM, Deucher, Alexander wrote:
> [AMD Official Use Only - General]
> 
>> -----Original Message-----
>> From: Jason Gunthorpe <jgg@nvidia.com>
>> Sent: Thursday, September 21, 2023 10:14 AM
>> To: Deucher, Alexander <Alexander.Deucher@amd.com>
>> Cc: Hegde, Vasant <Vasant.Hegde@amd.com>; iommu@lists.linux.dev;
>> joro@8bytes.org; Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>;
>> Huang2, Wei <Wei.Huang2@amd.com>; jsnitsel@redhat.com; Kuehling, Felix
>> <Felix.Kuehling@amd.com>
>> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>>
>> On Thu, Sep 21, 2023 at 02:05:37PM +0000, Deucher, Alexander wrote:
>>> [AMD Official Use Only - General]
>>>
>>>> -----Original Message-----
>>>> From: Hegde, Vasant <Vasant.Hegde@amd.com>
>>>> Sent: Thursday, September 21, 2023 5:32 AM
>>>> To: iommu@lists.linux.dev; joro@8bytes.org
>>>> Cc: Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Huang2,
>>>> Wei <Wei.Huang2@amd.com>; jsnitsel@redhat.com; jgg@ziepe.ca;
>> Hegde,
>>>> Vasant <Vasant.Hegde@amd.com>; Deucher, Alexander
>>>> <Alexander.Deucher@amd.com>; Kuehling, Felix
>>>> <Felix.Kuehling@amd.com>; Jason Gunthorpe <jgg@nvidia.com>
>>>> Subject: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>>>>
>>>> AMD GPU driver which was the only in-kernel user of iommu_v2 module
>>>> removed dependency on iommu_v2 module.
>>>>
>>>> Also we are working on adding SVA support in AMD IOMMU driver.
>>>> Device drivers are expected to use common SVA framework to enable
>>>> device PASID/PRI features.
>>>>
>>>> Removing iommu_v2 module and then adding SVA simplifies the
>>>> development.
>>>> Hence remove iommu_v2 module.
>>>
>>> Does this patch or the following patches make any functional changes
>>> for devices?  E.g., devices which supported ATS would have been put
>>> into an identity mapping mode previously Is that still retained?
>>
>> Huh? Why would we ever want to do that?
>>
>> Policy for the default domain belongs in the iommu subsystem except in
>> extreme cases, the AMD driver should not be doing random things like forcing
>> identity for ATS capable PCI devices.
> 
> I'm just concerned about regressions in random devices due to a change in policy in the IOMMU driver.  Previously the IOMMU driver would put ATS compatible devices into 1:1 mode.  Also some of the earlier integrated GPUs require 1:1 mapping for display from system memory due to hardware limitations.  There were also a lot of sbios bugs in the carrizo/raven timeframe because windows didn't enable the IOMMU so lots of OEMs had bogus IOMMU ACPI tables which didn't cause problems when 1:1 mode was used.

This patchset is not making functional change to default domain (it still puts
PASID capable devices in passthrough mode. 1 x 1 mapping). But I do plan to make
changes after SVA and invalidation improvement series gets merged.

Couple of things :
  - If device supports only ATS (no PASID, PRI), then we don't force 1 x 1 mapping.

  - If system is booted with SME (mem_encrypt=on), we force all devices to use
DMA translation mode and iommu_v2 module is not supported (no PASID support).

  - If device is capable of PASID, then we force 1 x 1 mapping. My understanding
is device wanted to use iommu_v2 module (enable PASID/PRI). There is no easy way
to switch from IOMMU DMA translation mode (V1 page table mode) to PASID support
mode (V2 page table). Hence 1 x 1 mapping was enforced.

Do you see any issue if we boot these devices with V2 page table mode (it will
go through IOMMU DMA translation mode) and don't switch page table mode during
runtime? -OR- older GPU's always expects 1 x 1 mapping?


-Vasant

RE: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Deucher, Alexander]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Hegde, Vasant <Vasant.Hegde@amd.com>
> Sent: Friday, September 22, 2023 2:45 AM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>; Jason Gunthorpe
> <jgg@nvidia.com>
> Cc: iommu@lists.linux.dev; joro@8bytes.org; Suthikulpanit, Suravee
> <Suravee.Suthikulpanit@amd.com>; Huang2, Wei <Wei.Huang2@amd.com>;
> jsnitsel@redhat.com; Kuehling, Felix <Felix.Kuehling@amd.com>
> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>
> Hi Alex,
>
>
> On 9/21/2023 8:45 PM, Deucher, Alexander wrote:
> > [AMD Official Use Only - General]
> >
> >> -----Original Message-----
> >> From: Jason Gunthorpe <jgg@nvidia.com>
> >> Sent: Thursday, September 21, 2023 10:14 AM
> >> To: Deucher, Alexander <Alexander.Deucher@amd.com>
> >> Cc: Hegde, Vasant <Vasant.Hegde@amd.com>; iommu@lists.linux.dev;
> >> joro@8bytes.org; Suthikulpanit, Suravee
> >> <Suravee.Suthikulpanit@amd.com>; Huang2, Wei
> <Wei.Huang2@amd.com>;
> >> jsnitsel@redhat.com; Kuehling, Felix <Felix.Kuehling@amd.com>
> >> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
> >>
> >> On Thu, Sep 21, 2023 at 02:05:37PM +0000, Deucher, Alexander wrote:
> >>> [AMD Official Use Only - General]
> >>>
> >>>> -----Original Message-----
> >>>> From: Hegde, Vasant <Vasant.Hegde@amd.com>
> >>>> Sent: Thursday, September 21, 2023 5:32 AM
> >>>> To: iommu@lists.linux.dev; joro@8bytes.org
> >>>> Cc: Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Huang2,
> >>>> Wei <Wei.Huang2@amd.com>; jsnitsel@redhat.com; jgg@ziepe.ca;
> >> Hegde,
> >>>> Vasant <Vasant.Hegde@amd.com>; Deucher, Alexander
> >>>> <Alexander.Deucher@amd.com>; Kuehling, Felix
> >>>> <Felix.Kuehling@amd.com>; Jason Gunthorpe <jgg@nvidia.com>
> >>>> Subject: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
> >>>>
> >>>> AMD GPU driver which was the only in-kernel user of iommu_v2 module
> >>>> removed dependency on iommu_v2 module.
> >>>>
> >>>> Also we are working on adding SVA support in AMD IOMMU driver.
> >>>> Device drivers are expected to use common SVA framework to enable
> >>>> device PASID/PRI features.
> >>>>
> >>>> Removing iommu_v2 module and then adding SVA simplifies the
> >>>> development.
> >>>> Hence remove iommu_v2 module.
> >>>
> >>> Does this patch or the following patches make any functional changes
> >>> for devices?  E.g., devices which supported ATS would have been put
> >>> into an identity mapping mode previously Is that still retained?
> >>
> >> Huh? Why would we ever want to do that?
> >>
> >> Policy for the default domain belongs in the iommu subsystem except
> >> in extreme cases, the AMD driver should not be doing random things
> >> like forcing identity for ATS capable PCI devices.
> >
> > I'm just concerned about regressions in random devices due to a change in
> policy in the IOMMU driver.  Previously the IOMMU driver would put ATS
> compatible devices into 1:1 mode.  Also some of the earlier integrated GPUs
> require 1:1 mapping for display from system memory due to hardware
> limitations.  There were also a lot of sbios bugs in the carrizo/raven timeframe
> because windows didn't enable the IOMMU so lots of OEMs had bogus
> IOMMU ACPI tables which didn't cause problems when 1:1 mode was used.
>
> This patchset is not making functional change to default domain (it still puts
> PASID capable devices in passthrough mode. 1 x 1 mapping). But I do plan to
> make changes after SVA and invalidation improvement series gets merged.
>
> Couple of things :
>   - If device supports only ATS (no PASID, PRI), then we don't force 1 x 1
> mapping.
>
>   - If system is booted with SME (mem_encrypt=on), we force all devices to use
> DMA translation mode and iommu_v2 module is not supported (no PASID
> support).

We already have some quirks for some APUs where you have to pick between SME and display because the extra IOMMU latency causes problems with high res modes (even in 1:1 mode).

>
>   - If device is capable of PASID, then we force 1 x 1 mapping. My
> understanding is device wanted to use iommu_v2 module (enable PASID/PRI).
> There is no easy way to switch from IOMMU DMA translation mode (V1 page
> table mode) to PASID support mode (V2 page table). Hence 1 x 1 mapping was
> enforced.
>
> Do you see any issue if we boot these devices with V2 page table mode (it will
> go through IOMMU DMA translation mode) and don't switch page table
> mode during runtime? -OR- older GPU's always expects 1 x 1 mapping?
>

All of the integrated devices I'm concerned about support ATS/PRI/PASID.  The issue is the display hardware on some of them requires 1x1 mode.  I'm not opposed to a 1:1 quirk list if that is the way we want to go.

Alex

Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module

[Vasant Hegde]

Alex,


On 9/22/2023 11:43 PM, Deucher, Alexander wrote:
> [AMD Official Use Only - General]
> 
>> -----Original Message-----
>> From: Hegde, Vasant <Vasant.Hegde@amd.com>
>> Sent: Friday, September 22, 2023 2:45 AM
>> To: Deucher, Alexander <Alexander.Deucher@amd.com>; Jason Gunthorpe
>> <jgg@nvidia.com>
>> Cc: iommu@lists.linux.dev; joro@8bytes.org; Suthikulpanit, Suravee
>> <Suravee.Suthikulpanit@amd.com>; Huang2, Wei <Wei.Huang2@amd.com>;
>> jsnitsel@redhat.com; Kuehling, Felix <Felix.Kuehling@amd.com>
>> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>>
>> Hi Alex,
>>
>>
>> On 9/21/2023 8:45 PM, Deucher, Alexander wrote:
>>> [AMD Official Use Only - General]
>>>
>>>> -----Original Message-----
>>>> From: Jason Gunthorpe <jgg@nvidia.com>
>>>> Sent: Thursday, September 21, 2023 10:14 AM
>>>> To: Deucher, Alexander <Alexander.Deucher@amd.com>
>>>> Cc: Hegde, Vasant <Vasant.Hegde@amd.com>; iommu@lists.linux.dev;
>>>> joro@8bytes.org; Suthikulpanit, Suravee
>>>> <Suravee.Suthikulpanit@amd.com>; Huang2, Wei
>> <Wei.Huang2@amd.com>;
>>>> jsnitsel@redhat.com; Kuehling, Felix <Felix.Kuehling@amd.com>
>>>> Subject: Re: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>>>>
>>>> On Thu, Sep 21, 2023 at 02:05:37PM +0000, Deucher, Alexander wrote:
>>>>> [AMD Official Use Only - General]
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Hegde, Vasant <Vasant.Hegde@amd.com>
>>>>>> Sent: Thursday, September 21, 2023 5:32 AM
>>>>>> To: iommu@lists.linux.dev; joro@8bytes.org
>>>>>> Cc: Suthikulpanit, Suravee <Suravee.Suthikulpanit@amd.com>; Huang2,
>>>>>> Wei <Wei.Huang2@amd.com>; jsnitsel@redhat.com; jgg@ziepe.ca;
>>>> Hegde,
>>>>>> Vasant <Vasant.Hegde@amd.com>; Deucher, Alexander
>>>>>> <Alexander.Deucher@amd.com>; Kuehling, Felix
>>>>>> <Felix.Kuehling@amd.com>; Jason Gunthorpe <jgg@nvidia.com>
>>>>>> Subject: [PATCH v3 1/5] iommu/amd: Remove iommu_v2 module
>>>>>>
>>>>>> AMD GPU driver which was the only in-kernel user of iommu_v2 module
>>>>>> removed dependency on iommu_v2 module.
>>>>>>
>>>>>> Also we are working on adding SVA support in AMD IOMMU driver.
>>>>>> Device drivers are expected to use common SVA framework to enable
>>>>>> device PASID/PRI features.
>>>>>>
>>>>>> Removing iommu_v2 module and then adding SVA simplifies the
>>>>>> development.
>>>>>> Hence remove iommu_v2 module.
>>>>>
>>>>> Does this patch or the following patches make any functional changes
>>>>> for devices?  E.g., devices which supported ATS would have been put
>>>>> into an identity mapping mode previously Is that still retained?
>>>>
>>>> Huh? Why would we ever want to do that?
>>>>
>>>> Policy for the default domain belongs in the iommu subsystem except
>>>> in extreme cases, the AMD driver should not be doing random things
>>>> like forcing identity for ATS capable PCI devices.
>>>
>>> I'm just concerned about regressions in random devices due to a change in
>> policy in the IOMMU driver.  Previously the IOMMU driver would put ATS
>> compatible devices into 1:1 mode.  Also some of the earlier integrated GPUs
>> require 1:1 mapping for display from system memory due to hardware
>> limitations.  There were also a lot of sbios bugs in the carrizo/raven timeframe
>> because windows didn't enable the IOMMU so lots of OEMs had bogus
>> IOMMU ACPI tables which didn't cause problems when 1:1 mode was used.
>>
>> This patchset is not making functional change to default domain (it still puts
>> PASID capable devices in passthrough mode. 1 x 1 mapping). But I do plan to
>> make changes after SVA and invalidation improvement series gets merged.
>>
>> Couple of things :
>>   - If device supports only ATS (no PASID, PRI), then we don't force 1 x 1
>> mapping.
>>
>>   - If system is booted with SME (mem_encrypt=on), we force all devices to use
>> DMA translation mode and iommu_v2 module is not supported (no PASID
>> support).
> 
> We already have some quirks for some APUs where you have to pick between SME and display because the extra IOMMU latency causes problems with high res modes (even in 1:1 mode).

Thanks for clarifying.

> 
>>
>>   - If device is capable of PASID, then we force 1 x 1 mapping. My
>> understanding is device wanted to use iommu_v2 module (enable PASID/PRI).
>> There is no easy way to switch from IOMMU DMA translation mode (V1 page
>> table mode) to PASID support mode (V2 page table). Hence 1 x 1 mapping was
>> enforced.
>>
>> Do you see any issue if we boot these devices with V2 page table mode (it will
>> go through IOMMU DMA translation mode) and don't switch page table
>> mode during runtime? -OR- older GPU's always expects 1 x 1 mapping?
>>
> 
> All of the integrated devices I'm concerned about support ATS/PRI/PASID.  The issue is the display hardware on some of them requires 1x1 mode.  I'm not opposed to a 1:1 quirk list if that is the way we want to go.

Got it. Yeah. We will add some quirks before making changes to default domain.

-Vasant

[PATCH v3 2/5] iommu/amd: Remove PPR support

[Vasant Hegde]

Remove PPR handler and notifier related functions as its not used
anymore. Note that we are retaining PPR interrupt handler support
as it will be re-used when we introduce IOPF support.

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
 drivers/iommu/amd/amd_iommu.h       |  2 --
 drivers/iommu/amd/amd_iommu_types.h | 13 ---------
 drivers/iommu/amd/iommu.c           | 45 +----------------------------
 3 files changed, 1 insertion(+), 59 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 5b8a1e2dd3d0..86be1edd50ee 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -52,8 +52,6 @@ int amd_iommu_pc_set_reg(struct amd_iommu *iommu, u8 bank, u8 cntr,
 int amd_iommu_pdev_enable_cap_pri(struct pci_dev *pdev);
 void amd_iommu_pdev_disable_cap_pri(struct pci_dev *pdev);
 
-int amd_iommu_register_ppr_notifier(struct notifier_block *nb);
-int amd_iommu_unregister_ppr_notifier(struct notifier_block *nb);
 int amd_iommu_flush_page(struct iommu_domain *dom, u32 pasid, u64 address);
 void amd_iommu_update_and_flush_device_table(struct protection_domain *domain);
 void amd_iommu_domain_update(struct protection_domain *domain);
diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 5be563d55ad6..25b731f3d984 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -509,19 +509,6 @@ extern struct kmem_cache *amd_iommu_irq_cache;
 #define APERTURE_RANGE_INDEX(a)	((a) >> APERTURE_RANGE_SHIFT)
 #define APERTURE_PAGE_INDEX(a)	(((a) >> 21) & 0x3fULL)
 
-/*
- * This struct is used to pass information about
- * incoming PPR faults around.
- */
-struct amd_iommu_fault {
-	u64 address;    /* IO virtual address of the fault*/
-	u32 pasid;      /* Address space identifier */
-	u32 sbdf;	/* Originating PCI device id */
-	u16 tag;        /* PPR tag */
-	u16 flags;      /* Fault flags */
-
-};
-
 
 struct amd_iommu;
 struct iommu_domain;
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index fd0d7b2f30dc..5abf181f5ecd 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -64,7 +64,6 @@ LIST_HEAD(acpihid_map);
 
 const struct iommu_ops amd_iommu_ops;
 
-static ATOMIC_NOTIFIER_HEAD(ppr_notifier);
 int amd_iommu_max_glx_val = -1;
 
 /*
@@ -815,24 +814,6 @@ static void iommu_poll_events(struct amd_iommu *iommu)
 	writel(head, iommu->mmio_base + MMIO_EVT_HEAD_OFFSET);
 }
 
-static void iommu_handle_ppr_entry(struct amd_iommu *iommu, u64 *raw)
-{
-	struct amd_iommu_fault fault;
-
-	if (PPR_REQ_TYPE(raw[0]) != PPR_REQ_FAULT) {
-		pr_err_ratelimited("Unknown PPR request received\n");
-		return;
-	}
-
-	fault.address   = raw[1];
-	fault.pasid     = PPR_PASID(raw[0]);
-	fault.sbdf      = PCI_SEG_DEVID_TO_SBDF(iommu->pci_seg->id, PPR_DEVID(raw[0]));
-	fault.tag       = PPR_TAG(raw[0]);
-	fault.flags     = PPR_FLAGS(raw[0]);
-
-	atomic_notifier_call_chain(&ppr_notifier, 0, &fault);
-}
-
 static void iommu_poll_ppr_log(struct amd_iommu *iommu)
 {
 	u32 head, tail;
@@ -878,8 +859,7 @@ static void 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);
 
-		/* Handle PPR entry */
-		iommu_handle_ppr_entry(iommu, entry);
+		/* TODO: PPR Handler will be added when we add IOPF support */
 
 		/* Refresh ring-buffer information */
 		head = readl(iommu->mmio_base + MMIO_PPR_HEAD_OFFSET);
@@ -2545,29 +2525,6 @@ const struct iommu_ops amd_iommu_ops = {
 	}
 };
 
-/*****************************************************************************
- *
- * The next functions do a basic initialization of IOMMU for pass through
- * mode
- *
- * In passthrough mode the IOMMU is initialized and enabled but not used for
- * DMA-API translation.
- *
- *****************************************************************************/
-
-/* IOMMUv2 specific functions */
-int amd_iommu_register_ppr_notifier(struct notifier_block *nb)
-{
-	return atomic_notifier_chain_register(&ppr_notifier, nb);
-}
-EXPORT_SYMBOL(amd_iommu_register_ppr_notifier);
-
-int amd_iommu_unregister_ppr_notifier(struct notifier_block *nb)
-{
-	return atomic_notifier_chain_unregister(&ppr_notifier, nb);
-}
-EXPORT_SYMBOL(amd_iommu_unregister_ppr_notifier);
-
 static int __flush_pasid(struct protection_domain *domain, u32 pasid,
 			 u64 address, bool size)
 {
-- 
2.31.1

[PATCH v3 3/5] iommu/amd: Remove amd_iommu_device_info()

[Vasant Hegde]

No one is using this function. Hence remove it. Also move PCI device
feature detection flags to amd_iommu_types.h as its only used inside
AMD IOMMU driver.

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
 drivers/iommu/amd/amd_iommu_types.h |  8 ++++++
 drivers/iommu/amd/iommu.c           | 42 -----------------------------
 include/linux/amd-iommu.h           | 26 ------------------
 3 files changed, 8 insertions(+), 68 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 25b731f3d984..e742006f2885 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -786,6 +786,14 @@ struct devid_map {
 	bool cmd_line;
 };
 
+#define AMD_IOMMU_DEVICE_FLAG_ATS_SUP     0x1    /* ATS feature supported */
+#define AMD_IOMMU_DEVICE_FLAG_PRI_SUP     0x2    /* PRI feature supported */
+#define AMD_IOMMU_DEVICE_FLAG_PASID_SUP   0x4    /* PASID context supported */
+/* Device may request execution on memory pages */
+#define AMD_IOMMU_DEVICE_FLAG_EXEC_SUP    0x8
+/* Device may request super-user privileges */
+#define AMD_IOMMU_DEVICE_FLAG_PRIV_SUP   0x10
+
 /*
  * This struct contains device specific data for the IOMMU
  */
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 5abf181f5ecd..8f43d89a98db 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2741,48 +2741,6 @@ int amd_iommu_complete_ppr(struct pci_dev *pdev, u32 pasid,
 }
 EXPORT_SYMBOL(amd_iommu_complete_ppr);
 
-int amd_iommu_device_info(struct pci_dev *pdev,
-                          struct amd_iommu_device_info *info)
-{
-	int max_pasids;
-	int pos;
-
-	if (pdev == NULL || info == NULL)
-		return -EINVAL;
-
-	if (!amd_iommu_v2_supported())
-		return -EINVAL;
-
-	memset(info, 0, sizeof(*info));
-
-	if (pci_ats_supported(pdev))
-		info->flags |= AMD_IOMMU_DEVICE_FLAG_ATS_SUP;
-
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI);
-	if (pos)
-		info->flags |= AMD_IOMMU_DEVICE_FLAG_PRI_SUP;
-
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PASID);
-	if (pos) {
-		int features;
-
-		max_pasids = 1 << (9 * (amd_iommu_max_glx_val + 1));
-		max_pasids = min(max_pasids, (1 << 20));
-
-		info->flags |= AMD_IOMMU_DEVICE_FLAG_PASID_SUP;
-		info->max_pasids = min(pci_max_pasids(pdev), max_pasids);
-
-		features = pci_pasid_features(pdev);
-		if (features & PCI_PASID_CAP_EXEC)
-			info->flags |= AMD_IOMMU_DEVICE_FLAG_EXEC_SUP;
-		if (features & PCI_PASID_CAP_PRIV)
-			info->flags |= AMD_IOMMU_DEVICE_FLAG_PRIV_SUP;
-	}
-
-	return 0;
-}
-EXPORT_SYMBOL(amd_iommu_device_info);
-
 #ifdef CONFIG_IRQ_REMAP
 
 /*****************************************************************************
diff --git a/include/linux/amd-iommu.h b/include/linux/amd-iommu.h
index 35ca00b09384..dc7ed2f46886 100644
--- a/include/linux/amd-iommu.h
+++ b/include/linux/amd-iommu.h
@@ -33,32 +33,6 @@ struct pci_dev;
 
 extern int amd_iommu_detect(void);
 
-/**
- * amd_iommu_device_info() - Get information about IOMMUv2 support of a
- *			     PCI device
- * @pdev: PCI device to query information from
- * @info: A pointer to an amd_iommu_device_info structure which will contain
- *	  the information about the PCI device
- *
- * Returns 0 on success, negative value on error
- */
-
-#define AMD_IOMMU_DEVICE_FLAG_ATS_SUP     0x1    /* ATS feature supported */
-#define AMD_IOMMU_DEVICE_FLAG_PRI_SUP     0x2    /* PRI feature supported */
-#define AMD_IOMMU_DEVICE_FLAG_PASID_SUP   0x4    /* PASID context supported */
-#define AMD_IOMMU_DEVICE_FLAG_EXEC_SUP    0x8    /* Device may request execution
-						    on memory pages */
-#define AMD_IOMMU_DEVICE_FLAG_PRIV_SUP   0x10    /* Device may request
-						    super-user privileges */
-
-struct amd_iommu_device_info {
-	int max_pasids;
-	u32 flags;
-};
-
-extern int amd_iommu_device_info(struct pci_dev *pdev,
-				 struct amd_iommu_device_info *info);
-
 #else /* CONFIG_AMD_IOMMU */
 
 static inline int amd_iommu_detect(void) { return -ENODEV; }
-- 
2.31.1

[PATCH v3 4/5] iommu/amd: Remove unused EXPORT_SYMBOLS

[Vasant Hegde]

Drop EXPORT_SYMBOLS for the functions that are not used by any modules.

Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
 drivers/iommu/amd/init.c  | 1 -
 drivers/iommu/amd/iommu.c | 6 ------
 2 files changed, 7 deletions(-)

diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 463e68a88b17..64bcf3df37ee 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -3666,7 +3666,6 @@ bool amd_iommu_v2_supported(void)
 	 */
 	return amd_iommu_gt_ppr_supported() && !amd_iommu_snp_en;
 }
-EXPORT_SYMBOL(amd_iommu_v2_supported);
 
 struct amd_iommu *get_amd_iommu(unsigned int idx)
 {
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 8f43d89a98db..d996a6f22a5f 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2446,7 +2446,6 @@ bool amd_iommu_is_attach_deferred(struct device *dev)
 
 	return dev_data->defer_attach;
 }
-EXPORT_SYMBOL_GPL(amd_iommu_is_attach_deferred);
 
 static void amd_iommu_flush_iotlb_all(struct iommu_domain *domain)
 {
@@ -2606,7 +2605,6 @@ int amd_iommu_flush_page(struct iommu_domain *dom, u32 pasid,
 
 	return ret;
 }
-EXPORT_SYMBOL(amd_iommu_flush_page);
 
 static int __amd_iommu_flush_tlb(struct protection_domain *domain, u32 pasid)
 {
@@ -2626,7 +2624,6 @@ int amd_iommu_flush_tlb(struct iommu_domain *dom, u32 pasid)
 
 	return ret;
 }
-EXPORT_SYMBOL(amd_iommu_flush_tlb);
 
 static u64 *__get_gcr3_pte(u64 *root, int level, u32 pasid, bool alloc)
 {
@@ -2706,7 +2703,6 @@ int amd_iommu_domain_set_gcr3(struct iommu_domain *dom, u32 pasid,
 
 	return ret;
 }
-EXPORT_SYMBOL(amd_iommu_domain_set_gcr3);
 
 int amd_iommu_domain_clear_gcr3(struct iommu_domain *dom, u32 pasid)
 {
@@ -2720,7 +2716,6 @@ int amd_iommu_domain_clear_gcr3(struct iommu_domain *dom, u32 pasid)
 
 	return ret;
 }
-EXPORT_SYMBOL(amd_iommu_domain_clear_gcr3);
 
 int amd_iommu_complete_ppr(struct pci_dev *pdev, u32 pasid,
 			   int status, int tag)
@@ -2739,7 +2734,6 @@ int amd_iommu_complete_ppr(struct pci_dev *pdev, u32 pasid,
 
 	return iommu_queue_command(iommu, &cmd);
 }
-EXPORT_SYMBOL(amd_iommu_complete_ppr);
 
 #ifdef CONFIG_IRQ_REMAP
 
-- 
2.31.1

[PATCH v3 5/5] Revert "iommu: Fix false ownership failure on AMD systems with PASID activated"

[Vasant Hegde]

This reverts commit 2380f1e8195ef612deea1dc7a3d611c5d2b9b56a.

Previous patch removed AMD iommu_v2 module. Hence its safe to revert this
workaround.

Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Jason Gunthorpe <jgg@nvidia.com>
Cc: Joerg Roedel <jroedel@suse.de>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
 drivers/iommu/iommu.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index f3231042954b..b74befc08036 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2899,24 +2899,6 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
 	return ret ?: count;
 }
 
-static bool iommu_is_default_domain(struct iommu_group *group)
-{
-	if (group->domain == group->default_domain)
-		return true;
-
-	/*
-	 * If the default domain was set to identity and it is still an identity
-	 * domain then we consider this a pass. This happens because of
-	 * amd_iommu_init_device() replacing the default idenytity domain with an
-	 * identity domain that has a different configuration for AMDGPU.
-	 */
-	if (group->default_domain &&
-	    group->default_domain->type == IOMMU_DOMAIN_IDENTITY &&
-	    group->domain && group->domain->type == IOMMU_DOMAIN_IDENTITY)
-		return true;
-	return false;
-}
-
 /**
  * iommu_device_use_default_domain() - Device driver wants to handle device
  *                                     DMA through the kernel DMA API.
@@ -2935,7 +2917,7 @@ int iommu_device_use_default_domain(struct device *dev)
 
 	mutex_lock(&group->mutex);
 	if (group->owner_cnt) {
-		if (group->owner || !iommu_is_default_domain(group) ||
+		if (group->domain != group->default_domain || group->owner ||
 		    !xa_empty(&group->pasid_array)) {
 			ret = -EBUSY;
 			goto unlock_out;
-- 
2.31.1

Re: [PATCH v3 0/5] iommu/amd: SVA Support (part 2) - deprecate iommu_v2 module

[Joerg Roedel]

Hi Vasant,

On Thu, Sep 21, 2023 at 09:31:35AM +0000, Vasant Hegde wrote:
> Vasant Hegde (5):
>   iommu/amd: Remove iommu_v2 module
>   iommu/amd: Remove PPR support
>   iommu/amd: Remove amd_iommu_device_info()
>   iommu/amd: Remove unused EXPORT_SYMBOLS
>   Revert "iommu: Fix false ownership failure on AMD systems with PASID activated"

Alright, since this causes no regressions for Radeon, can you please
re-send with all Acks/Review tags? I will put it into the IOMMU tree
then.

Regards,

	Joerg

Re: [PATCH v3 0/5] iommu/amd: SVA Support (part 2) - deprecate iommu_v2 module

[Vasant Hegde]

Hi Joerg,


On 10/2/2023 12:02 PM, Joerg Roedel wrote:
> Hi Vasant,
> 
> On Thu, Sep 21, 2023 at 09:31:35AM +0000, Vasant Hegde wrote:
>> Vasant Hegde (5):
>>   iommu/amd: Remove iommu_v2 module
>>   iommu/amd: Remove PPR support
>>   iommu/amd: Remove amd_iommu_device_info()
>>   iommu/amd: Remove unused EXPORT_SYMBOLS
>>   Revert "iommu: Fix false ownership failure on AMD systems with PASID activated"
> 
> Alright, since this causes no regressions for Radeon, can you please
> re-send with all Acks/Review tags? I will put it into the IOMMU tree
> then.

Sure.

Thanks
Vasant