[PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node

Hi,

The series adds support to IORT RMR nodes specified in IORT
Revision E.b -ARM DEN 0049E[0]. RMR nodes are used to describe
memory ranges that are used by endpoints and require a unity
mapping in SMMU.

We have faced issues with 3408iMR RAID controller cards which
fail to boot when SMMU is enabled. This is because these
controllers make use of host memory for various caching related
purposes and when SMMU is enabled the iMR firmware fails to 
access these memory regions as there is no mapping for them.
IORT RMR provides a way for UEFI to describe and report these
memory regions so that the kernel can make a unity mapping for
these in SMMU.

Change History: 

v6 --> v7

The only change from v6 is the fix pointed out by Steve to
the SMMUv2 SMR bypass install in patch #8.

Thanks to the Tested-by tags by Laurentiu with SMMUv2 and
Hanjun/Huiqiang with SMMUv3 for v6. I haven't added the tags
yet as the series still needs more review[1].

Feedback and tests on this series is very much appreciated.

v5 --> v6
- Addressed comments from Robin & Lorenzo.
  : Moved iort_parse_rmr() to acpi_iort_init() from
    iort_init_platform_devices().
  : Removed use of struct iort_rmr_entry during the initial
    parse. Using struct iommu_resv_region instead.
  : Report RMR address alignment and overlap errors, but continue.
  : Reworked arm_smmu_init_bypass_stes() (patch # 6).
- Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8).
- Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based
  on Type of RMR region. Suggested by Jon N.

Thanks,
Shameer
[0] https://developer.arm.com/documentation/den0049/latest/
[1] https://lore.kernel.org/linux-acpi/20210716083442.1708-1-shameerali.kolothum.thodi@huawei.com/T/#m043c95b869973a834b2fd57f3e1ed0325c84f3b7
------
v4 --> v5
 -Added a fw_data union to struct iommu_resv_region and removed
  struct iommu_rmr (Based on comments from Joerg/Robin).
 -Added iommu_put_rmrs() to release mem.
 -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by
  yet because of the above changes.

v3 -->v4
-Included the SMMUv2 SMR bypass install changes suggested by
 Steve(patch #7)
-As per Robin's comments, RMR reserve implementation is now
 more generic  (patch #8) and dropped v3 patches 8 and 10.
-Rebase to 5.13-rc1

RFC v2 --> v3
 -Dropped RFC tag as the ACPICA header changes are now ready to be
  part of 5.13[0]. But this series still has a dependency on that patch.
 -Added IORT E.b related changes(node flags, _DSM function 5 checks for
  PCIe).
 -Changed RMR to stream id mapping from M:N to M:1 as per the spec and
  discussion here[1].
 -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!)
------

Jon Nettleton (1):
  iommu/arm-smmu: Get associated RMR info and install bypass SMR

Shameer Kolothum (8):
  iommu: Introduce a union to struct iommu_resv_region
  ACPI/IORT: Add support for RMR node parsing
  iommu/dma: Introduce generic helper to retrieve RMR info
  ACPI/IORT: Add a helper to retrieve RMR memory regions
  iommu/arm-smmu-v3: Introduce strtab init helper
  iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force
    bypass
  iommu/arm-smmu-v3: Get associated RMR info and install bypass STE
  iommu/dma: Reserve any RMR regions associated with a dev

 drivers/acpi/arm64/iort.c                   | 172 +++++++++++++++++++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  76 +++++++--
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  48 ++++++
 drivers/iommu/dma-iommu.c                   |  89 +++++++++-
 include/linux/acpi_iort.h                   |   7 +
 include/linux/dma-iommu.h                   |  13 ++
 include/linux/iommu.h                       |  11 ++
 7 files changed, 393 insertions(+), 23 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

A union is introduced to struct iommu_resv_region to hold
any firmware specific data. This is in preparation to add
support for IORT RMR reserve regions and the union now holds
the RMR specific information.

Signed-off-by: Shameer Kolothum <redacted>
---
 include/linux/iommu.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 32d448050bf7..bd0e4641c569 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h

@@ -114,6 +114,13 @@ enum iommu_resv_type {
 	IOMMU_RESV_SW_MSI,
 };
 
+struct iommu_iort_rmr_data {
+#define IOMMU_RMR_REMAP_PERMITTED	(1 << 0)
+	u32 flags;
+	u32 sid;	/* Stream Id associated with RMR entry */
+	void *smmu;	/* Associated IORT SMMU node pointer */
+};
+
 /**
  * struct iommu_resv_region - descriptor for a reserved memory region
  * @list: Linked list pointers

@@ -121,6 +128,7 @@ enum iommu_resv_type {
  * @length: Length of the region in bytes
  * @prot: IOMMU Protection flags (READ/WRITE/...)
  * @type: Type of the reserved region
+ * @rmr: ACPI IORT RMR specific data
  */
 struct iommu_resv_region {
 	struct list_head	list;

@@ -128,6 +136,9 @@ struct iommu_resv_region {
 	size_t			length;
 	int			prot;
 	enum iommu_resv_type	type;
+	union {
+		struct iommu_iort_rmr_data rmr;
+	} fw_data;
 };
 
 /**

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Add support for parsing RMR node information from ACPI.

Find the associated streamid and smmu node info from the
RMR node and populate a linked list with RMR memory
descriptors.

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/acpi/arm64/iort.c | 134 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 133 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 3b23fb775ac4..d76ba46ebe67 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c

@@ -40,6 +40,8 @@ struct iort_fwnode {
 static LIST_HEAD(iort_fwnode_list);
 static DEFINE_SPINLOCK(iort_fwnode_lock);
 
+static LIST_HEAD(iort_rmr_list);	/* list of RMR regions from ACPI */
+
 /**
  * iort_set_fwnode() - Create iort_fwnode and use it to register
  *		       iommu data in the iort_fwnode_list

@@ -393,7 +395,8 @@ static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
 		if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT ||
 		    node->type == ACPI_IORT_NODE_PCI_ROOT_COMPLEX ||
 		    node->type == ACPI_IORT_NODE_SMMU_V3 ||
-		    node->type == ACPI_IORT_NODE_PMCG) {
+		    node->type == ACPI_IORT_NODE_PMCG ||
+		    node->type == ACPI_IORT_NODE_RMR) {
 			*id_out = map->output_base;
 			return parent;
 		}

@@ -1566,6 +1569,134 @@ static void __init iort_enable_acs(struct acpi_iort_node *iort_node)
 #else
 static inline void iort_enable_acs(struct acpi_iort_node *iort_node) { }
 #endif
+static void iort_rmr_desc_check_overlap(struct acpi_iort_rmr_desc *desc, u32 count)
+{
+	int i, j;
+
+	for (i = 0; i < count; i++) {
+		u64 end, start = desc[i].base_address, length = desc[i].length;
+
+		end = start + length - 1;
+
+		/* Check for address overlap */
+		for (j = i + 1; j < count; j++) {
+			u64 e_start = desc[j].base_address;
+			u64 e_end = e_start + desc[j].length - 1;
+
+			if (start <= e_end && end >= e_start)
+				pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] overlaps, continue anyway\n",
+				       start, end);
+		}
+	}
+}
+
+static void __init iort_node_get_rmr_info(struct acpi_iort_node *iort_node)
+{
+	struct acpi_iort_node *smmu;
+	struct acpi_iort_rmr *rmr;
+	struct acpi_iort_rmr_desc *rmr_desc;
+	u32 map_count = iort_node->mapping_count;
+	u32 sid;
+	int i;
+
+	if (!iort_node->mapping_offset || map_count != 1) {
+		pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	/* Retrieve associated smmu and stream id */
+	smmu = iort_node_get_id(iort_node, &sid, 0);
+	if (!smmu) {
+		pr_err(FW_BUG "Invalid SMMU reference, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	/* Retrieve RMR data */
+	rmr = (struct acpi_iort_rmr *)iort_node->node_data;
+	if (!rmr->rmr_offset || !rmr->rmr_count) {
+		pr_err(FW_BUG "Invalid RMR descriptor array, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, iort_node,
+				rmr->rmr_offset);
+
+	iort_rmr_desc_check_overlap(rmr_desc, rmr->rmr_count);
+
+	for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) {
+		struct iommu_resv_region *region;
+		enum iommu_resv_type type;
+		int prot = IOMMU_READ | IOMMU_WRITE;
+		u64 addr = rmr_desc->base_address, size = rmr_desc->length;
+
+		if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) {
+			/* PAGE align base addr and size */
+			addr &= PAGE_MASK;
+			size = PAGE_ALIGN(size + offset_in_page(rmr_desc->base_address));
+
+			pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not aligned to 64K, continue with [0x%llx - 0x%llx]\n",
+			       rmr_desc->base_address,
+			       rmr_desc->base_address + rmr_desc->length - 1,
+			       addr, addr + size - 1);
+		}
+		if (rmr->flags & IOMMU_RMR_REMAP_PERMITTED) {
+			type = IOMMU_RESV_DIRECT_RELAXABLE;
+			/*
+			 * Set IOMMU_CACHE as IOMMU_RESV_DIRECT_RELAXABLE is
+			 * normally used for allocated system memory that is
+			 * then used for device specific reserved regions.
+			 */
+			prot |= IOMMU_CACHE;
+		} else {
+			type = IOMMU_RESV_DIRECT;
+			/*
+			 * Set IOMMU_MMIO as IOMMU_RESV_DIRECT is normally used
+			 * for device memory like MSI doorbell.
+			 */
+			prot |= IOMMU_MMIO;
+		}
+
+		region = iommu_alloc_resv_region(addr, size, prot, type);
+		if (region) {
+			region->fw_data.rmr.flags = rmr->flags;
+			region->fw_data.rmr.sid = sid;
+			region->fw_data.rmr.smmu = smmu;
+			list_add_tail(&region->list, &iort_rmr_list);
+		}
+	}
+}
+
+static void __init iort_parse_rmr(void)
+{
+	struct acpi_iort_node *iort_node, *iort_end;
+	struct acpi_table_iort *iort;
+	int i;
+
+	if (iort_table->revision < 3)
+		return;
+
+	iort = (struct acpi_table_iort *)iort_table;
+
+	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+				 iort->node_offset);
+	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+				iort_table->length);
+
+	for (i = 0; i < iort->node_count; i++) {
+		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
+			       "IORT node pointer overflows, bad table!\n"))
+			return;
+
+		if (iort_node->type == ACPI_IORT_NODE_RMR)
+			iort_node_get_rmr_info(iort_node);
+
+		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
+					 iort_node->length);
+	}
+}
 
 static void __init iort_init_platform_devices(void)
 {

@@ -1636,6 +1767,7 @@ void __init acpi_iort_init(void)
 	}
 
 	iort_init_platform_devices();
+	iort_parse_rmr();
 }
 
 #ifdef CONFIG_ZONE_DMA

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Reserved Memory Regions(RMR) associated with an IOMMU can be
described through ACPI IORT tables in systems with devices
that require a unity mapping or bypass for those
regions.

Introduce a generic interface so that IOMMU drivers can retrieve
and set up necessary mappings.

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/iommu/dma-iommu.c | 29 +++++++++++++++++++++++++++++
 include/linux/dma-iommu.h | 13 +++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..2fa2445e9070 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c

@@ -174,6 +174,35 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 }
 EXPORT_SYMBOL(iommu_put_dma_cookie);
 
+/**
+ *
+ * iommu_dma_get_rmrs - Retrieve Reserved Memory Regions(RMRs) associated
+ *                      with a given IOMMU
+ * @iommu_fwnode: fwnode associated with IOMMU
+ * @list: RMR list to be populated
+ *
+ */
+int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode,
+		       struct list_head *list)
+{
+	return -EINVAL;
+}
+EXPORT_SYMBOL(iommu_dma_get_rmrs);
+
+/**
+ *
+ * iommu_dma_put_rmrs - Release Reserved Memory Regions(RMRs) associated
+ *                      with a given IOMMU
+ * @iommu_fwnode: fwnode associated with IOMMU
+ * @list: RMR list
+ *
+ */
+void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode,
+			struct list_head *list)
+{
+}
+EXPORT_SYMBOL(iommu_dma_put_rmrs);
+
 /**
  * iommu_dma_get_resv_regions - Reserved region driver helper
  * @dev: Device from iommu_get_resv_regions()

diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index 758ca4694257..3b7b2d096c6e 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h

@@ -42,12 +42,16 @@ void iommu_dma_free_cpu_cached_iovas(unsigned int cpu,
 
 extern bool iommu_dma_forcedac;
 
+int iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct list_head *list);
+void iommu_dma_put_rmrs(struct fwnode_handle *iommu, struct list_head *list);
+
 #else /* CONFIG_IOMMU_DMA */
 
 struct iommu_domain;
 struct msi_desc;
 struct msi_msg;
 struct device;
+struct fwnode_handle;
 
 static inline void iommu_setup_dma_ops(struct device *dev, u64 dma_base,
 				       u64 dma_limit)

@@ -83,5 +87,14 @@ static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_he
 {
 }
 
+static int iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct list_head *list)
+{
+	return -ENODEV;
+}
+
+static void iommu_dma_put_rmrs(struct fwnode_handle *iommu, struct list_head *list)
+{
+}
+
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __DMA_IOMMU_H */

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Add a helper function (iort_iommu_get_rmrs()) that retrieves RMR
memory descriptors associated with a given IOMMU. This will be used
by IOMMU drivers to setup necessary mappings.

Invoke it from the generic helper iommu_dma_get_rmrs().

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/acpi/arm64/iort.c | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/iommu/dma-iommu.c |  4 ++++
 include/linux/acpi_iort.h |  7 +++++++
 3 files changed, 49 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index d76ba46ebe67..3c32d62e63b6 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c

@@ -809,6 +809,42 @@ static struct acpi_iort_node *iort_get_msi_resv_iommu(struct device *dev)
 	return NULL;
 }
 
+/**
+ * iort_iommu_get_rmrs() - Helper to retrieve RMR info associated with IOMMU
+ * @iommu_fwnode: fwnode for the IOMMU
+ * @head: RMR list head to be populated
+ *
+ * Returns: 0 on success, <0 failure. Please note, we will keep the already
+ *          allocated RMR reserve regions in case of a kmemdup()
+ *          failure.
+ */
+int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode,
+			struct list_head *head)
+{
+	struct iommu_resv_region *e;
+	struct acpi_iort_node *iommu;
+	int rmrs = 0;
+
+	iommu = iort_get_iort_node(iommu_fwnode);
+	if (!iommu || list_empty(&iort_rmr_list))
+		return -ENODEV;
+
+	list_for_each_entry(e, &iort_rmr_list, list) {
+		struct iommu_resv_region *region;
+
+		if (e->fw_data.rmr.smmu != iommu)
+			continue;
+
+		region = kmemdup(e, sizeof(*region), GFP_KERNEL);
+		if (region) {
+			list_add_tail(&region->list, head);
+			rmrs++;
+		}
+	}
+
+	return (rmrs == 0) ? -ENODEV : 0;
+}
+
 /**
  * iort_iommu_msi_get_resv_regions - Reserved region driver helper
  * @dev: Device from iommu_get_resv_regions()

@@ -1041,6 +1077,8 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
 { return 0; }
 int iort_iommu_configure_id(struct device *dev, const u32 *input_id)
 { return -ENODEV; }
+int iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct list_head *head)
+{ return -ENODEV; }
 #endif
 
 static int nc_dma_get_range(struct device *dev, u64 *size)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 2fa2445e9070..1b6e27475279 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c

@@ -185,6 +185,9 @@ EXPORT_SYMBOL(iommu_put_dma_cookie);
 int iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode,
 		       struct list_head *list)
 {
+	if (!is_of_node(iommu_fwnode))
+		return iort_iommu_get_rmrs(iommu_fwnode, list);
+
 	return -EINVAL;
 }
 EXPORT_SYMBOL(iommu_dma_get_rmrs);

@@ -200,6 +203,7 @@ EXPORT_SYMBOL(iommu_dma_get_rmrs);
 void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode,
 			struct list_head *list)
 {
+	generic_iommu_put_resv_regions(iommu_fwnode->dev, list);
 }
 EXPORT_SYMBOL(iommu_dma_put_rmrs);
 

diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index f1f0842a2cb2..d8c030c103f5 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h

@@ -38,6 +38,8 @@ int iort_dma_get_ranges(struct device *dev, u64 *size);
 int iort_iommu_configure_id(struct device *dev, const u32 *id_in);
 int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head);
 phys_addr_t acpi_iort_dma_get_max_cpu_address(void);
+int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode,
+			struct list_head *list);
 #else
 static inline void acpi_iort_init(void) { }
 static inline u32 iort_msi_map_id(struct device *dev, u32 id)

@@ -57,6 +59,11 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
 
 static inline phys_addr_t acpi_iort_dma_get_max_cpu_address(void)
 { return PHYS_ADDR_MAX; }
+
+static inline
+int iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode,
+			struct list_head *list)
+{ return -ENODEV; }
 #endif
 
 #endif /* __ACPI_IORT_H__ */

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Introduce a helper to check the sid range and to init the l2 strtab
entries(bypass). This will be useful when we have to initialize the
l2 strtab with bypass for RMR SIDs.

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 28 +++++++++++----------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 235f9bdaeaf2..31940e53c675 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -2518,6 +2518,19 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
 	return sid < limit;
 }
 
+static int arm_smmu_init_sid_strtab(struct arm_smmu_device *smmu, u32 sid)
+{
+	/* Check the SIDs are in range of the SMMU and our stream table */
+	if (!arm_smmu_sid_in_range(smmu, sid))
+		return -ERANGE;
+
+	/* Ensure l2 strtab is initialised */
+	if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB)
+		return arm_smmu_init_l2_strtab(smmu, sid);
+
+	return 0;
+}
+
 static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 				  struct arm_smmu_master *master)
 {

@@ -2541,20 +2554,9 @@ static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
 		new_stream->id = sid;
 		new_stream->master = master;
 
-		/*
-		 * Check the SIDs are in range of the SMMU and our stream table
-		 */
-		if (!arm_smmu_sid_in_range(smmu, sid)) {
-			ret = -ERANGE;
+		ret = arm_smmu_init_sid_strtab(smmu, sid);
+		if (ret)
 			break;
-		}
-
-		/* Ensure l2 strtab is initialised */
-		if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
-			ret = arm_smmu_init_l2_strtab(smmu, sid);
-			if (ret)
-				break;
-		}
 
 		/* Insert into SID tree */
 		new_node = &(smmu->streams.rb_node);

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

By default, disable_bypass flag is set and any dev without
an iommu domain installs STE with CFG_ABORT during
arm_smmu_init_bypass_stes(). Introduce a "force" flag and
move the STE update logic to arm_smmu_init_bypass_stes()
so that we can force it to install CFG_BYPASS STE for specific
SIDs.

This will be useful in follow-up patch to install bypass
for IORT RMR SIDs.

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 31940e53c675..85f6f1925a36 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -1357,12 +1357,21 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_master *master, u32 sid,
 		arm_smmu_cmdq_issue_cmd(smmu, &prefetch_cmd);
 }
 
-static void arm_smmu_init_bypass_stes(__le64 *strtab, unsigned int nent)
+static void arm_smmu_init_bypass_stes(__le64 *strtab, unsigned int nent, bool force)
 {
 	unsigned int i;
+	u64 val = STRTAB_STE_0_V;
+
+	if (disable_bypass && !force)
+		val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
+	else
+		val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);
 
 	for (i = 0; i < nent; ++i) {
-		arm_smmu_write_strtab_ent(NULL, -1, strtab);
+		strtab[0] = cpu_to_le64(val);
+		strtab[1] = cpu_to_le64(FIELD_PREP(STRTAB_STE_1_SHCFG,
+						   STRTAB_STE_1_SHCFG_INCOMING));
+		strtab[2] = 0;
 		strtab += STRTAB_STE_DWORDS;
 	}
 }

@@ -1390,7 +1399,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 		return -ENOMEM;
 	}
 
-	arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
+	arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT, false);
 	arm_smmu_write_strtab_l1_desc(strtab, desc);
 	return 0;
 }

@@ -3042,7 +3051,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
 	reg |= FIELD_PREP(STRTAB_BASE_CFG_LOG2SIZE, smmu->sid_bits);
 	cfg->strtab_base_cfg = reg;
 
-	arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents);
+	arm_smmu_init_bypass_stes(strtab, cfg->num_l1_ents, false);
 	return 0;
 }
 

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Check if there is any RMR info associated with the devices behind
the SMMUv3 and if any, install bypass STEs for them. This is to
keep any ongoing traffic associated with these devices alive
when we enable/reset SMMUv3 during probe().

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 31 +++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 85f6f1925a36..1165605d6f7a 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -3760,6 +3760,34 @@ static void __iomem *arm_smmu_ioremap(struct device *dev, resource_size_t start,
 	return devm_ioremap_resource(dev, &res);
 }
 
+static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu)
+{
+	struct list_head rmr_list;
+	struct iommu_resv_region *e;
+	int ret;
+
+	INIT_LIST_HEAD(&rmr_list);
+	if (iommu_dma_get_rmrs(dev_fwnode(smmu->dev), &rmr_list))
+		return;
+
+	list_for_each_entry(e, &rmr_list, list) {
+		__le64 *step;
+		u32 sid = e->fw_data.rmr.sid;
+
+		ret = arm_smmu_init_sid_strtab(smmu, sid);
+		if (ret) {
+			dev_err(smmu->dev, "RMR SID(0x%x) bypass failed\n",
+				sid);
+			continue;
+		}
+
+		step = arm_smmu_get_step_for_sid(smmu, sid);
+		arm_smmu_init_bypass_stes(step, 1, true);
+	}
+
+	iommu_dma_put_rmrs(dev_fwnode(smmu->dev), &rmr_list);
+}
+
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	int irq, ret;

@@ -3841,6 +3869,9 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	/* Record our private device structure */
 	platform_set_drvdata(pdev, smmu);
 
+	/* Check for RMRs and install bypass STEs if any */
+	arm_smmu_rmr_install_bypass_ste(smmu);
+
 	/* Reset the device */
 	ret = arm_smmu_device_reset(smmu, bypass);
 	if (ret)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

From: Jon Nettleton <redacted>

Check if there is any RMR info associated with the devices behind
the SMMU and if any, install bypass SMRs for them. This is to
keep any ongoing traffic associated with these devices alive
when we enable/reset SMMU during probe().

Signed-off-by: Jon Nettleton <redacted>
Signed-off-by: Steven Price <steven.price@arm.com>
Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/iommu/arm/arm-smmu/arm-smmu.c | 48 +++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index f22dbeb1e510..315feab9e85b 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c

@@ -2063,6 +2063,50 @@ err_reset_platform_ops: __maybe_unused;
 	return err;
 }
 
+static void arm_smmu_rmr_install_bypass_smr(struct arm_smmu_device *smmu)
+{
+	struct list_head rmr_list;
+	struct iommu_resv_region *e;
+	int i, cnt = 0;
+	u32 reg;
+
+	INIT_LIST_HEAD(&rmr_list);
+	if (iommu_dma_get_rmrs(dev_fwnode(smmu->dev), &rmr_list))
+		return;
+
+	/*
+	 * Rather than trying to look at existing mappings that
+	 * are setup by the firmware and then invalidate the ones
+	 * that do no have matching RMR entries, just disable the
+	 * SMMU until it gets enabled again in the reset routine.
+	 */
+	reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0);
+	reg |= ARM_SMMU_sCR0_CLIENTPD;
+	arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sCR0, reg);
+
+	list_for_each_entry(e, &rmr_list, list) {
+		u32 sid = e->fw_data.rmr.sid;
+
+		i = arm_smmu_find_sme(smmu, sid, ~0);
+		if (i < 0)
+			continue;
+		if (smmu->s2crs[i].count == 0) {
+			smmu->smrs[i].id = sid;
+			smmu->smrs[i].mask = 0;
+			smmu->smrs[i].valid = true;
+		}
+		smmu->s2crs[i].count++;
+		smmu->s2crs[i].type = S2CR_TYPE_BYPASS;
+		smmu->s2crs[i].privcfg = S2CR_PRIVCFG_DEFAULT;
+
+		cnt++;
+	}
+
+	dev_notice(smmu->dev, "\tpreserved %d boot mapping%s\n", cnt,
+		   cnt == 1 ? "" : "s");
+	iommu_dma_put_rmrs(dev_fwnode(smmu->dev), &rmr_list);
+}
+
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	struct resource *res;

@@ -2189,6 +2233,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, smmu);
+
+	/* Check for RMRs and install bypass SMRs if any */
+	arm_smmu_rmr_install_bypass_smr(smmu);
+
 	arm_smmu_device_reset(smmu);
 	arm_smmu_test_smr_masks(smmu);
 

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Get ACPI IORT RMR regions associated with a dev reserved
so that there is a unity mapping for them in SMMU.

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/iommu/dma-iommu.c | 56 +++++++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 1b6e27475279..c1ae0c3d4b33 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c

@@ -207,22 +207,68 @@ void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode,
 }
 EXPORT_SYMBOL(iommu_dma_put_rmrs);
 
+static bool iommu_dma_dev_has_rmr(struct iommu_fwspec *fwspec,
+				  struct iommu_resv_region *e)
+{
+	int i;
+
+	for (i = 0; i < fwspec->num_ids; i++) {
+		if (e->fw_data.rmr.sid == fwspec->ids[i])
+			return true;
+	}
+
+	return false;
+}
+
+static void iommu_dma_get_rmr_resv_regions(struct device *dev,
+					   struct list_head *list)
+{
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	struct list_head rmr_list;
+	struct iommu_resv_region *rmr, *tmp;
+
+	INIT_LIST_HEAD(&rmr_list);
+	if (iommu_dma_get_rmrs(fwspec->iommu_fwnode, &rmr_list))
+		return;
+
+	if (dev_is_pci(dev)) {
+		struct pci_dev *pdev = to_pci_dev(dev);
+		struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus);
+
+		if (!host->preserve_config)
+			return;
+	}
+
+	list_for_each_entry_safe(rmr, tmp, &rmr_list, list) {
+		if (!iommu_dma_dev_has_rmr(fwspec, rmr))
+			continue;
+
+		/* Remove from iommu RMR list and add to dev resv_regions */
+		list_del_init(&rmr->list);
+		list_add_tail(&rmr->list, list);
+	}
+
+	iommu_dma_put_rmrs(fwspec->iommu_fwnode, &rmr_list);
+}
+
 /**
  * iommu_dma_get_resv_regions - Reserved region driver helper
  * @dev: Device from iommu_get_resv_regions()
  * @list: Reserved region list from iommu_get_resv_regions()
  *
  * IOMMU drivers can use this to implement their .get_resv_regions callback
- * for general non-IOMMU-specific reservations. Currently, this covers GICv3
- * ITS region reservation on ACPI based ARM platforms that may require HW MSI
- * reservation.
+ * for general non-IOMMU-specific reservations. Currently this covers,
+ *  -GICv3 ITS region reservation on ACPI based ARM platforms that may
+ *   require HW MSI reservation.
+ *  -Any ACPI IORT RMR memory range reservations (IORT spec rev E.b)
  */
 void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 {
 
-	if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
+	if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode)) {
 		iort_iommu_msi_get_resv_regions(dev, list);
-
+		iommu_dma_get_rmr_resv_regions(dev, list);
+	}
 }
 EXPORT_SYMBOL(iommu_dma_get_resv_regions);
 

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, 5 Aug 2021 at 10:10, Shameer Kolothum
[off-list ref] wrote:

Hi,

The series adds support to IORT RMR nodes specified in IORT
Revision E.b -ARM DEN 0049E[0]. RMR nodes are used to describe
memory ranges that are used by endpoints and require a unity
mapping in SMMU.

We have faced issues with 3408iMR RAID controller cards which
fail to boot when SMMU is enabled. This is because these
controllers make use of host memory for various caching related
purposes and when SMMU is enabled the iMR firmware fails to
access these memory regions as there is no mapping for them.
IORT RMR provides a way for UEFI to describe and report these
memory regions so that the kernel can make a unity mapping for
these in SMMU.

Does this mean we are ignoring the RMR memory ranges, and exposing the
entire physical address space to devices using the stream IDs in
question?

Change History:

v6 --> v7

The only change from v6 is the fix pointed out by Steve to
the SMMUv2 SMR bypass install in patch #8.

Thanks to the Tested-by tags by Laurentiu with SMMUv2 and
Hanjun/Huiqiang with SMMUv3 for v6. I haven't added the tags
yet as the series still needs more review[1].

Feedback and tests on this series is very much appreciated.

v5 --> v6
- Addressed comments from Robin & Lorenzo.
  : Moved iort_parse_rmr() to acpi_iort_init() from
    iort_init_platform_devices().
  : Removed use of struct iort_rmr_entry during the initial
    parse. Using struct iommu_resv_region instead.
  : Report RMR address alignment and overlap errors, but continue.
  : Reworked arm_smmu_init_bypass_stes() (patch # 6).
- Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8).
- Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based
  on Type of RMR region. Suggested by Jon N.

Thanks,
Shameer
[0] https://developer.arm.com/documentation/den0049/latest/
[1] https://lore.kernel.org/linux-acpi/20210716083442.1708-1-shameerali.kolothum.thodi@huawei.com/T/#m043c95b869973a834b2fd57f3e1ed0325c84f3b7
------
v4 --> v5
 -Added a fw_data union to struct iommu_resv_region and removed
  struct iommu_rmr (Based on comments from Joerg/Robin).
 -Added iommu_put_rmrs() to release mem.
 -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by
  yet because of the above changes.

v3 -->v4
-Included the SMMUv2 SMR bypass install changes suggested by
 Steve(patch #7)
-As per Robin's comments, RMR reserve implementation is now
 more generic  (patch #8) and dropped v3 patches 8 and 10.
-Rebase to 5.13-rc1

RFC v2 --> v3
 -Dropped RFC tag as the ACPICA header changes are now ready to be
  part of 5.13[0]. But this series still has a dependency on that patch.
 -Added IORT E.b related changes(node flags, _DSM function 5 checks for
  PCIe).
 -Changed RMR to stream id mapping from M:N to M:1 as per the spec and
  discussion here[1].
 -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!)
------

Jon Nettleton (1):
  iommu/arm-smmu: Get associated RMR info and install bypass SMR

Shameer Kolothum (8):
  iommu: Introduce a union to struct iommu_resv_region
  ACPI/IORT: Add support for RMR node parsing
  iommu/dma: Introduce generic helper to retrieve RMR info
  ACPI/IORT: Add a helper to retrieve RMR memory regions
  iommu/arm-smmu-v3: Introduce strtab init helper
  iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force
    bypass
  iommu/arm-smmu-v3: Get associated RMR info and install bypass STE
  iommu/dma: Reserve any RMR regions associated with a dev

 drivers/acpi/arm64/iort.c                   | 172 +++++++++++++++++++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  76 +++++++--
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  48 ++++++
 drivers/iommu/dma-iommu.c                   |  89 +++++++++-
 include/linux/acpi_iort.h                   |   7 +
 include/linux/dma-iommu.h                   |  13 ++
 include/linux/iommu.h                       |  11 ++
 7 files changed, 393 insertions(+), 23 deletions(-)

--
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-----Original Message-----
From: Ard Biesheuvel [mailto:ardb@kernel.org]
Sent: 05 August 2021 14:23
To: Shameerali Kolothum Thodi <redacted>
Cc: Linux ARM <redacted>; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Linuxarm [off-list ref];
Lorenzo Pieralisi [off-list ref]; Joerg Roedel
[off-list ref]; Robin Murphy [off-list ref]; Will Deacon
[off-list ref]; wanghuiqiang [off-list ref]; Guohanjun
(Hanjun Guo) [off-list ref]; Steven Price
[off-list ref]; Sami Mujawar [off-list ref]; Jon
Nettleton [off-list ref]; Eric Auger [off-list ref];
yangyicong [off-list ref]
Subject: Re: [PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node

On Thu, 5 Aug 2021 at 10:10, Shameer Kolothum
[off-list ref] wrote:

quoted

Hi,

The series adds support to IORT RMR nodes specified in IORT
Revision E.b -ARM DEN 0049E[0]. RMR nodes are used to describe
memory ranges that are used by endpoints and require a unity
mapping in SMMU.

We have faced issues with 3408iMR RAID controller cards which
fail to boot when SMMU is enabled. This is because these
controllers make use of host memory for various caching related
purposes and when SMMU is enabled the iMR firmware fails to
access these memory regions as there is no mapping for them.
IORT RMR provides a way for UEFI to describe and report these
memory regions so that the kernel can make a unity mapping for
these in SMMU.

Does this mean we are ignoring the RMR memory ranges, and exposing the
entire physical address space to devices using the stream IDs in
question?

Nope. RMR node is used to describe the memory ranges used by end points
behind SMMU. And this information is used to create 1 : 1 mappings for those
ranges in SMMU. Anything outside those ranges will result in translation
fault(if there are no other dynamic DMA mappings).

Thanks,
Shameer

quoted

Change History:

v6 --> v7

The only change from v6 is the fix pointed out by Steve to
the SMMUv2 SMR bypass install in patch #8.

Thanks to the Tested-by tags by Laurentiu with SMMUv2 and
Hanjun/Huiqiang with SMMUv3 for v6. I haven't added the tags
yet as the series still needs more review[1].

Feedback and tests on this series is very much appreciated.

v5 --> v6
- Addressed comments from Robin & Lorenzo.
  : Moved iort_parse_rmr() to acpi_iort_init() from
    iort_init_platform_devices().
  : Removed use of struct iort_rmr_entry during the initial
    parse. Using struct iommu_resv_region instead.
  : Report RMR address alignment and overlap errors, but continue.
  : Reworked arm_smmu_init_bypass_stes() (patch # 6).
- Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8).
- Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based
  on Type of RMR region. Suggested by Jon N.

Thanks,
Shameer
[0] https://developer.arm.com/documentation/den0049/latest/
[1]

https://lore.kernel.org/linux-acpi/20210716083442.1708-1-shameerali.koloth
um.thodi@huawei.com/T/#m043c95b869973a834b2fd57f3e1ed0325c84f3b7

quoted

------
v4 --> v5
 -Added a fw_data union to struct iommu_resv_region and removed
  struct iommu_rmr (Based on comments from Joerg/Robin).
 -Added iommu_put_rmrs() to release mem.
 -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by
  yet because of the above changes.

v3 -->v4
-Included the SMMUv2 SMR bypass install changes suggested by
 Steve(patch #7)
-As per Robin's comments, RMR reserve implementation is now
 more generic  (patch #8) and dropped v3 patches 8 and 10.
-Rebase to 5.13-rc1

RFC v2 --> v3
 -Dropped RFC tag as the ACPICA header changes are now ready to be
  part of 5.13[0]. But this series still has a dependency on that patch.
 -Added IORT E.b related changes(node flags, _DSM function 5 checks for
  PCIe).
 -Changed RMR to stream id mapping from M:N to M:1 as per the spec and
  discussion here[1].
 -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!)
------

Jon Nettleton (1):
  iommu/arm-smmu: Get associated RMR info and install bypass SMR

Shameer Kolothum (8):
  iommu: Introduce a union to struct iommu_resv_region
  ACPI/IORT: Add support for RMR node parsing
  iommu/dma: Introduce generic helper to retrieve RMR info
  ACPI/IORT: Add a helper to retrieve RMR memory regions
  iommu/arm-smmu-v3: Introduce strtab init helper
  iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force
    bypass
  iommu/arm-smmu-v3: Get associated RMR info and install bypass STE
  iommu/dma: Reserve any RMR regions associated with a dev

 drivers/acpi/arm64/iort.c                   | 172

+++++++++++++++++++-

quoted

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  76 +++++++--
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  48 ++++++
 drivers/iommu/dma-iommu.c                   |  89 +++++++++-
 include/linux/acpi_iort.h                   |   7 +
 include/linux/dma-iommu.h                   |  13 ++
 include/linux/iommu.h                       |  11 ++
 7 files changed, 393 insertions(+), 23 deletions(-)

--
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, 5 Aug 2021 at 15:35, Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

-----Original Message-----
From: Ard Biesheuvel [mailto:ardb@kernel.org]
Sent: 05 August 2021 14:23
To: Shameerali Kolothum Thodi <redacted>
Cc: Linux ARM <redacted>; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Linuxarm [off-list ref];
Lorenzo Pieralisi [off-list ref]; Joerg Roedel
[off-list ref]; Robin Murphy [off-list ref]; Will Deacon
[off-list ref]; wanghuiqiang [off-list ref]; Guohanjun
(Hanjun Guo) [off-list ref]; Steven Price
[off-list ref]; Sami Mujawar [off-list ref]; Jon
Nettleton [off-list ref]; Eric Auger [off-list ref];
yangyicong [off-list ref]
Subject: Re: [PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node

On Thu, 5 Aug 2021 at 10:10, Shameer Kolothum
[off-list ref] wrote:

quoted

Hi,

The series adds support to IORT RMR nodes specified in IORT
Revision E.b -ARM DEN 0049E[0]. RMR nodes are used to describe
memory ranges that are used by endpoints and require a unity
mapping in SMMU.

We have faced issues with 3408iMR RAID controller cards which
fail to boot when SMMU is enabled. This is because these
controllers make use of host memory for various caching related
purposes and when SMMU is enabled the iMR firmware fails to
access these memory regions as there is no mapping for them.
IORT RMR provides a way for UEFI to describe and report these
memory regions so that the kernel can make a unity mapping for
these in SMMU.

Does this mean we are ignoring the RMR memory ranges, and exposing the
entire physical address space to devices using the stream IDs in
question?

Nope. RMR node is used to describe the memory ranges used by end points
behind SMMU. And this information is used to create 1 : 1 mappings for those
ranges in SMMU. Anything outside those ranges will result in translation
fault(if there are no other dynamic DMA mappings).

Excellent! It was not obvious to me from looking at the patches, so I
had to ask.

Thanks,
Ard.

quoted

quoted

Change History:

v6 --> v7

The only change from v6 is the fix pointed out by Steve to
the SMMUv2 SMR bypass install in patch #8.

Thanks to the Tested-by tags by Laurentiu with SMMUv2 and
Hanjun/Huiqiang with SMMUv3 for v6. I haven't added the tags
yet as the series still needs more review[1].

Feedback and tests on this series is very much appreciated.

v5 --> v6
- Addressed comments from Robin & Lorenzo.
  : Moved iort_parse_rmr() to acpi_iort_init() from
    iort_init_platform_devices().
  : Removed use of struct iort_rmr_entry during the initial
    parse. Using struct iommu_resv_region instead.
  : Report RMR address alignment and overlap errors, but continue.
  : Reworked arm_smmu_init_bypass_stes() (patch # 6).
- Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8).
- Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based
  on Type of RMR region. Suggested by Jon N.

Thanks,
Shameer
[0] https://developer.arm.com/documentation/den0049/latest/
[1]

https://lore.kernel.org/linux-acpi/20210716083442.1708-1-shameerali.koloth
um.thodi@huawei.com/T/#m043c95b869973a834b2fd57f3e1ed0325c84f3b7

quoted

------
v4 --> v5
 -Added a fw_data union to struct iommu_resv_region and removed
  struct iommu_rmr (Based on comments from Joerg/Robin).
 -Added iommu_put_rmrs() to release mem.
 -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by
  yet because of the above changes.

v3 -->v4
-Included the SMMUv2 SMR bypass install changes suggested by
 Steve(patch #7)
-As per Robin's comments, RMR reserve implementation is now
 more generic  (patch #8) and dropped v3 patches 8 and 10.
-Rebase to 5.13-rc1

RFC v2 --> v3
 -Dropped RFC tag as the ACPICA header changes are now ready to be
  part of 5.13[0]. But this series still has a dependency on that patch.
 -Added IORT E.b related changes(node flags, _DSM function 5 checks for
  PCIe).
 -Changed RMR to stream id mapping from M:N to M:1 as per the spec and
  discussion here[1].
 -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!)
------

Jon Nettleton (1):
  iommu/arm-smmu: Get associated RMR info and install bypass SMR

Shameer Kolothum (8):
  iommu: Introduce a union to struct iommu_resv_region
  ACPI/IORT: Add support for RMR node parsing
  iommu/dma: Introduce generic helper to retrieve RMR info
  ACPI/IORT: Add a helper to retrieve RMR memory regions
  iommu/arm-smmu-v3: Introduce strtab init helper
  iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force
    bypass
  iommu/arm-smmu-v3: Get associated RMR info and install bypass STE
  iommu/dma: Reserve any RMR regions associated with a dev

 drivers/acpi/arm64/iort.c                   | 172

+++++++++++++++++++-

quoted

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  76 +++++++--
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  48 ++++++
 drivers/iommu/dma-iommu.c                   |  89 +++++++++-
 include/linux/acpi_iort.h                   |   7 +
 include/linux/dma-iommu.h                   |  13 ++
 include/linux/iommu.h                       |  11 ++
 7 files changed, 393 insertions(+), 23 deletions(-)

--
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Aug 05, 2021 at 09:07:19AM +0100, Shameer Kolothum wrote:

Add a helper function (iort_iommu_get_rmrs()) that retrieves RMR
memory descriptors associated with a given IOMMU. This will be used
by IOMMU drivers to setup necessary mappings.

Invoke it from the generic helper iommu_dma_get_rmrs().

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/acpi/arm64/iort.c | 38 ++++++++++++++++++++++++++++++++++++++
 drivers/iommu/dma-iommu.c |  4 ++++
 include/linux/acpi_iort.h |  7 +++++++
 3 files changed, 49 insertions(+)

Reviewed-by: Lorenzo Pieralisi <redacted>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Aug 05, 2021 at 09:07:17AM +0100, Shameer Kolothum wrote:

[...]

+static void __init iort_node_get_rmr_info(struct acpi_iort_node *iort_node)
+{
+	struct acpi_iort_node *smmu;
+	struct acpi_iort_rmr *rmr;
+	struct acpi_iort_rmr_desc *rmr_desc;
+	u32 map_count = iort_node->mapping_count;
+	u32 sid;
+	int i;
+
+	if (!iort_node->mapping_offset || map_count != 1) {
+		pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	/* Retrieve associated smmu and stream id */
+	smmu = iort_node_get_id(iort_node, &sid, 0);
+	if (!smmu) {
+		pr_err(FW_BUG "Invalid SMMU reference, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	/* Retrieve RMR data */
+	rmr = (struct acpi_iort_rmr *)iort_node->node_data;
+	if (!rmr->rmr_offset || !rmr->rmr_count) {
+		pr_err(FW_BUG "Invalid RMR descriptor array, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, iort_node,
+				rmr->rmr_offset);
+
+	iort_rmr_desc_check_overlap(rmr_desc, rmr->rmr_count);
+
+	for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) {
+		struct iommu_resv_region *region;
+		enum iommu_resv_type type;
+		int prot = IOMMU_READ | IOMMU_WRITE;
+		u64 addr = rmr_desc->base_address, size = rmr_desc->length;
+
+		if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) {
+			/* PAGE align base addr and size */
+			addr &= PAGE_MASK;
+			size = PAGE_ALIGN(size + offset_in_page(rmr_desc->base_address));
+
+			pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not aligned to 64K, continue with [0x%llx - 0x%llx]\n",
+			       rmr_desc->base_address,
+			       rmr_desc->base_address + rmr_desc->length - 1,
+			       addr, addr + size - 1);
+		}
+		if (rmr->flags & IOMMU_RMR_REMAP_PERMITTED) {
+			type = IOMMU_RESV_DIRECT_RELAXABLE;
+			/*
+			 * Set IOMMU_CACHE as IOMMU_RESV_DIRECT_RELAXABLE is
+			 * normally used for allocated system memory that is
+			 * then used for device specific reserved regions.
+			 */
+			prot |= IOMMU_CACHE;
+		} else {
+			type = IOMMU_RESV_DIRECT;
+			/*
+			 * Set IOMMU_MMIO as IOMMU_RESV_DIRECT is normally used
+			 * for device memory like MSI doorbell.
+			 */
+			prot |= IOMMU_MMIO;
+		}

On the prot value assignment based on the remapping flag, I'd like to
hear Robin/Joerg's opinion, I'd avoid being in a situation where
"normally" this would work but then we have to quirk it.

Is this a valid assumption _always_ ?

Thanks,
Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Aug 5, 2021 at 6:03 PM Lorenzo Pieralisi
[off-list ref] wrote:

On Thu, Aug 05, 2021 at 09:07:17AM +0100, Shameer Kolothum wrote:

[...]

quoted

+static void __init iort_node_get_rmr_info(struct acpi_iort_node *iort_node)
+{
+     struct acpi_iort_node *smmu;
+     struct acpi_iort_rmr *rmr;
+     struct acpi_iort_rmr_desc *rmr_desc;
+     u32 map_count = iort_node->mapping_count;
+     u32 sid;
+     int i;
+
+     if (!iort_node->mapping_offset || map_count != 1) {
+             pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
+                    iort_node);
+             return;
+     }
+
+     /* Retrieve associated smmu and stream id */
+     smmu = iort_node_get_id(iort_node, &sid, 0);
+     if (!smmu) {
+             pr_err(FW_BUG "Invalid SMMU reference, skipping RMR node %p\n",
+                    iort_node);
+             return;
+     }
+
+     /* Retrieve RMR data */
+     rmr = (struct acpi_iort_rmr *)iort_node->node_data;
+     if (!rmr->rmr_offset || !rmr->rmr_count) {
+             pr_err(FW_BUG "Invalid RMR descriptor array, skipping RMR node %p\n",
+                    iort_node);
+             return;
+     }
+
+     rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, iort_node,
+                             rmr->rmr_offset);
+
+     iort_rmr_desc_check_overlap(rmr_desc, rmr->rmr_count);
+
+     for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) {
+             struct iommu_resv_region *region;
+             enum iommu_resv_type type;
+             int prot = IOMMU_READ | IOMMU_WRITE;
+             u64 addr = rmr_desc->base_address, size = rmr_desc->length;
+
+             if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) {
+                     /* PAGE align base addr and size */
+                     addr &= PAGE_MASK;
+                     size = PAGE_ALIGN(size + offset_in_page(rmr_desc->base_address));
+
+                     pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not aligned to 64K, continue with [0x%llx - 0x%llx]\n",
+                            rmr_desc->base_address,
+                            rmr_desc->base_address + rmr_desc->length - 1,
+                            addr, addr + size - 1);
+             }
+             if (rmr->flags & IOMMU_RMR_REMAP_PERMITTED) {
+                     type = IOMMU_RESV_DIRECT_RELAXABLE;
+                     /*
+                      * Set IOMMU_CACHE as IOMMU_RESV_DIRECT_RELAXABLE is
+                      * normally used for allocated system memory that is
+                      * then used for device specific reserved regions.
+                      */
+                     prot |= IOMMU_CACHE;
+             } else {
+                     type = IOMMU_RESV_DIRECT;
+                     /*
+                      * Set IOMMU_MMIO as IOMMU_RESV_DIRECT is normally used
+                      * for device memory like MSI doorbell.
+                      */
+                     prot |= IOMMU_MMIO;
+             }

On the prot value assignment based on the remapping flag, I'd like to
hear Robin/Joerg's opinion, I'd avoid being in a situation where
"normally" this would work but then we have to quirk it.

Is this a valid assumption _always_ ?

These assumptions were made based on the historic use cases I could
find reading the history.  There aren't many known examples "in the wild"
because so far we haven't had a mechanism other than quirks based
around device-tree implementations.

Ultimately I believe the proper solution will need to be another flag
in the RMR table that specifies the type of memory an RMR Node
describes, not just the base and length.

-Jon

Thanks,
Lorenzo

quoted

+
+             region = iommu_alloc_resv_region(addr, size, prot, type);
+             if (region) {
+                     region->fw_data.rmr.flags = rmr->flags;
+                     region->fw_data.rmr.sid = sid;
+                     region->fw_data.rmr.smmu = smmu;
+                     list_add_tail(&region->list, &iort_rmr_list);
+             }
+     }
+}
+
+static void __init iort_parse_rmr(void)
+{
+     struct acpi_iort_node *iort_node, *iort_end;
+     struct acpi_table_iort *iort;
+     int i;
+
+     if (iort_table->revision < 3)
+             return;
+
+     iort = (struct acpi_table_iort *)iort_table;
+
+     iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+                              iort->node_offset);
+     iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+                             iort_table->length);
+
+     for (i = 0; i < iort->node_count; i++) {
+             if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
+                            "IORT node pointer overflows, bad table!\n"))
+                     return;
+
+             if (iort_node->type == ACPI_IORT_NODE_RMR)
+                     iort_node_get_rmr_info(iort_node);
+
+             iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
+                                      iort_node->length);
+     }
+}

 static void __init iort_init_platform_devices(void)
 {

@@ -1636,6 +1767,7 @@ void __init acpi_iort_init(void)
      }

      iort_init_platform_devices();
+     iort_parse_rmr();
 }

 #ifdef CONFIG_ZONE_DMA
--

2.17.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 8/5/2021 7:03 PM, Lorenzo Pieralisi wrote:

On Thu, Aug 05, 2021 at 09:07:17AM +0100, Shameer Kolothum wrote:

[...]

quoted

+static void __init iort_node_get_rmr_info(struct acpi_iort_node *iort_node)
+{
+	struct acpi_iort_node *smmu;
+	struct acpi_iort_rmr *rmr;
+	struct acpi_iort_rmr_desc *rmr_desc;
+	u32 map_count = iort_node->mapping_count;
+	u32 sid;
+	int i;
+
+	if (!iort_node->mapping_offset || map_count != 1) {
+		pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	/* Retrieve associated smmu and stream id */
+	smmu = iort_node_get_id(iort_node, &sid, 0);
+	if (!smmu) {
+		pr_err(FW_BUG "Invalid SMMU reference, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	/* Retrieve RMR data */
+	rmr = (struct acpi_iort_rmr *)iort_node->node_data;
+	if (!rmr->rmr_offset || !rmr->rmr_count) {
+		pr_err(FW_BUG "Invalid RMR descriptor array, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, iort_node,
+				rmr->rmr_offset);
+
+	iort_rmr_desc_check_overlap(rmr_desc, rmr->rmr_count);
+
+	for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) {
+		struct iommu_resv_region *region;
+		enum iommu_resv_type type;
+		int prot = IOMMU_READ | IOMMU_WRITE;
+		u64 addr = rmr_desc->base_address, size = rmr_desc->length;
+
+		if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) {
+			/* PAGE align base addr and size */
+			addr &= PAGE_MASK;
+			size = PAGE_ALIGN(size + offset_in_page(rmr_desc->base_address));
+
+			pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not aligned to 64K, continue with [0x%llx - 0x%llx]\n",
+			       rmr_desc->base_address,
+			       rmr_desc->base_address + rmr_desc->length - 1,
+			       addr, addr + size - 1);
+		}
+		if (rmr->flags & IOMMU_RMR_REMAP_PERMITTED) {
+			type = IOMMU_RESV_DIRECT_RELAXABLE;
+			/*
+			 * Set IOMMU_CACHE as IOMMU_RESV_DIRECT_RELAXABLE is
+			 * normally used for allocated system memory that is
+			 * then used for device specific reserved regions.
+			 */
+			prot |= IOMMU_CACHE;
+		} else {
+			type = IOMMU_RESV_DIRECT;
+			/*
+			 * Set IOMMU_MMIO as IOMMU_RESV_DIRECT is normally used
+			 * for device memory like MSI doorbell.
+			 */
+			prot |= IOMMU_MMIO;
+		}

On the prot value assignment based on the remapping flag, I'd like to
hear Robin/Joerg's opinion, I'd avoid being in a situation where
"normally" this would work but then we have to quirk it.

Is this a valid assumption _always_ ?

I think we enable quite a bit of platforms with this assumption, so IMHO
it's a fair compromise for now. As per Jon's comment and oob
discussions, in the long run the spec should probably be updated to
include a way of explicitly specifying memory attributes.

---
Thanks & Best Regards, Laurentiu

quoted

+
+		region = iommu_alloc_resv_region(addr, size, prot, type);
+		if (region) {
+			region->fw_data.rmr.flags = rmr->flags;
+			region->fw_data.rmr.sid = sid;
+			region->fw_data.rmr.smmu = smmu;
+			list_add_tail(&region->list, &iort_rmr_list);
+		}
+	}
+}
+
+static void __init iort_parse_rmr(void)
+{
+	struct acpi_iort_node *iort_node, *iort_end;
+	struct acpi_table_iort *iort;
+	int i;
+
+	if (iort_table->revision < 3)
+		return;
+
+	iort = (struct acpi_table_iort *)iort_table;
+
+	iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+				 iort->node_offset);
+	iort_end = ACPI_ADD_PTR(struct acpi_iort_node, iort,
+				iort_table->length);
+
+	for (i = 0; i < iort->node_count; i++) {
+		if (WARN_TAINT(iort_node >= iort_end, TAINT_FIRMWARE_WORKAROUND,
+			       "IORT node pointer overflows, bad table!\n"))
+			return;
+
+		if (iort_node->type == ACPI_IORT_NODE_RMR)
+			iort_node_get_rmr_info(iort_node);
+
+		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
+					 iort_node->length);
+	}
+}
 
 static void __init iort_init_platform_devices(void)
 {

@@ -1636,6 +1767,7 @@ void __init acpi_iort_init(void)
 	}
 
 	iort_init_platform_devices();
+	iort_parse_rmr();
 }
 
 #ifdef CONFIG_ZONE_DMA

-- 
2.17.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.linuxfoundation.org%2Fmailman%2Flistinfo%2Fiommu&data=04%7C01%7Claurentiu.tudor%40nxp.com%7Cb020e5093dee4374ee0b08d9582a9238%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637637762131278563%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=8Q%2Fu5qawL94YhbKLujOAlJjTVEWZircjviccWnnqPxs%3D&reserved=0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 05/08/2021 09:07, Shameer Kolothum wrote:

NIT: This will provoke a kernel-doc warning as the field name in the
structure is 'fw_data' not 'rmr' ('rmr being a field of the anonymous
union).

I've also retested this series on my Juno setup, so feel free to add:

Tested-by: Steven Price <steven.price@arm.com>

Thanks,

Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Aug 5, 2021 at 4:09 PM Ard Biesheuvel [off-list ref] wrote:

On Thu, 5 Aug 2021 at 15:35, Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Ard Biesheuvel [mailto:ardb@kernel.org]
Sent: 05 August 2021 14:23
To: Shameerali Kolothum Thodi <redacted>
Cc: Linux ARM <redacted>; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Linuxarm [off-list ref];
Lorenzo Pieralisi [off-list ref]; Joerg Roedel
[off-list ref]; Robin Murphy [off-list ref]; Will Deacon
[off-list ref]; wanghuiqiang [off-list ref]; Guohanjun
(Hanjun Guo) [off-list ref]; Steven Price
[off-list ref]; Sami Mujawar [off-list ref]; Jon
Nettleton [off-list ref]; Eric Auger [off-list ref];
yangyicong [off-list ref]
Subject: Re: [PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node

On Thu, 5 Aug 2021 at 10:10, Shameer Kolothum
[off-list ref] wrote:

quoted

Hi,

The series adds support to IORT RMR nodes specified in IORT
Revision E.b -ARM DEN 0049E[0]. RMR nodes are used to describe
memory ranges that are used by endpoints and require a unity
mapping in SMMU.

We have faced issues with 3408iMR RAID controller cards which
fail to boot when SMMU is enabled. This is because these
controllers make use of host memory for various caching related
purposes and when SMMU is enabled the iMR firmware fails to
access these memory regions as there is no mapping for them.
IORT RMR provides a way for UEFI to describe and report these
memory regions so that the kernel can make a unity mapping for
these in SMMU.

Does this mean we are ignoring the RMR memory ranges, and exposing the
entire physical address space to devices using the stream IDs in
question?

Nope. RMR node is used to describe the memory ranges used by end points
behind SMMU. And this information is used to create 1 : 1 mappings for those
ranges in SMMU. Anything outside those ranges will result in translation
fault(if there are no other dynamic DMA mappings).

Excellent! It was not obvious to me from looking at the patches, so I
had to ask.

Thanks,
Ard.

quoted

quoted

quoted

Change History:

v6 --> v7

The only change from v6 is the fix pointed out by Steve to
the SMMUv2 SMR bypass install in patch #8.

Thanks to the Tested-by tags by Laurentiu with SMMUv2 and
Hanjun/Huiqiang with SMMUv3 for v6. I haven't added the tags
yet as the series still needs more review[1].

Feedback and tests on this series is very much appreciated.

v5 --> v6
- Addressed comments from Robin & Lorenzo.
  : Moved iort_parse_rmr() to acpi_iort_init() from
    iort_init_platform_devices().
  : Removed use of struct iort_rmr_entry during the initial
    parse. Using struct iommu_resv_region instead.
  : Report RMR address alignment and overlap errors, but continue.
  : Reworked arm_smmu_init_bypass_stes() (patch # 6).
- Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8).
- Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based
  on Type of RMR region. Suggested by Jon N.

Thanks,
Shameer
[0] https://developer.arm.com/documentation/den0049/latest/
[1]

https://lore.kernel.org/linux-acpi/20210716083442.1708-1-shameerali.koloth
um.thodi@huawei.com/T/#m043c95b869973a834b2fd57f3e1ed0325c84f3b7

quoted

------
v4 --> v5
 -Added a fw_data union to struct iommu_resv_region and removed
  struct iommu_rmr (Based on comments from Joerg/Robin).
 -Added iommu_put_rmrs() to release mem.
 -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by
  yet because of the above changes.

v3 -->v4
-Included the SMMUv2 SMR bypass install changes suggested by
 Steve(patch #7)
-As per Robin's comments, RMR reserve implementation is now
 more generic  (patch #8) and dropped v3 patches 8 and 10.
-Rebase to 5.13-rc1

RFC v2 --> v3
 -Dropped RFC tag as the ACPICA header changes are now ready to be
  part of 5.13[0]. But this series still has a dependency on that patch.
 -Added IORT E.b related changes(node flags, _DSM function 5 checks for
  PCIe).
 -Changed RMR to stream id mapping from M:N to M:1 as per the spec and
  discussion here[1].
 -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!)
------

Jon Nettleton (1):
  iommu/arm-smmu: Get associated RMR info and install bypass SMR

Shameer Kolothum (8):
  iommu: Introduce a union to struct iommu_resv_region
  ACPI/IORT: Add support for RMR node parsing
  iommu/dma: Introduce generic helper to retrieve RMR info
  ACPI/IORT: Add a helper to retrieve RMR memory regions
  iommu/arm-smmu-v3: Introduce strtab init helper
  iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force
    bypass
  iommu/arm-smmu-v3: Get associated RMR info and install bypass STE
  iommu/dma: Reserve any RMR regions associated with a dev

 drivers/acpi/arm64/iort.c                   | 172

+++++++++++++++++++-

quoted

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  76 +++++++--
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  48 ++++++
 drivers/iommu/dma-iommu.c                   |  89 +++++++++-
 include/linux/acpi_iort.h                   |   7 +
 include/linux/dma-iommu.h                   |  13 ++
 include/linux/iommu.h                       |  11 ++
 7 files changed, 393 insertions(+), 23 deletions(-)

--
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Ping,  Can we get some movement on this patchset?  The standard was
was ratified over a year ago, and there is real world hardware that is
using or needs this functionality.

Thanks,
-Jon

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 2021-08-05 17:03, Lorenzo Pieralisi wrote:

On Thu, Aug 05, 2021 at 09:07:17AM +0100, Shameer Kolothum wrote:

[...]

quoted

+static void __init iort_node_get_rmr_info(struct acpi_iort_node *iort_node)
+{
+	struct acpi_iort_node *smmu;
+	struct acpi_iort_rmr *rmr;
+	struct acpi_iort_rmr_desc *rmr_desc;
+	u32 map_count = iort_node->mapping_count;
+	u32 sid;
+	int i;
+
+	if (!iort_node->mapping_offset || map_count != 1) {
+		pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	/* Retrieve associated smmu and stream id */
+	smmu = iort_node_get_id(iort_node, &sid, 0);
+	if (!smmu) {
+		pr_err(FW_BUG "Invalid SMMU reference, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	/* Retrieve RMR data */
+	rmr = (struct acpi_iort_rmr *)iort_node->node_data;
+	if (!rmr->rmr_offset || !rmr->rmr_count) {
+		pr_err(FW_BUG "Invalid RMR descriptor array, skipping RMR node %p\n",
+		       iort_node);
+		return;
+	}
+
+	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, iort_node,
+				rmr->rmr_offset);
+
+	iort_rmr_desc_check_overlap(rmr_desc, rmr->rmr_count);
+
+	for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) {
+		struct iommu_resv_region *region;
+		enum iommu_resv_type type;
+		int prot = IOMMU_READ | IOMMU_WRITE;
+		u64 addr = rmr_desc->base_address, size = rmr_desc->length;
+
+		if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) {
+			/* PAGE align base addr and size */
+			addr &= PAGE_MASK;
+			size = PAGE_ALIGN(size + offset_in_page(rmr_desc->base_address));
+
+			pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not aligned to 64K, continue with [0x%llx - 0x%llx]\n",
+			       rmr_desc->base_address,
+			       rmr_desc->base_address + rmr_desc->length - 1,
+			       addr, addr + size - 1);
+		}
+		if (rmr->flags & IOMMU_RMR_REMAP_PERMITTED) {
+			type = IOMMU_RESV_DIRECT_RELAXABLE;
+			/*
+			 * Set IOMMU_CACHE as IOMMU_RESV_DIRECT_RELAXABLE is
+			 * normally used for allocated system memory that is
+			 * then used for device specific reserved regions.
+			 */
+			prot |= IOMMU_CACHE;
+		} else {
+			type = IOMMU_RESV_DIRECT;
+			/*
+			 * Set IOMMU_MMIO as IOMMU_RESV_DIRECT is normally used
+			 * for device memory like MSI doorbell.
+			 */
+			prot |= IOMMU_MMIO;
+		}

On the prot value assignment based on the remapping flag, I'd like to
hear Robin/Joerg's opinion, I'd avoid being in a situation where
"normally" this would work but then we have to quirk it.

Is this a valid assumption _always_ ?

No. Certainly applying IOMMU_CACHE without reference to the device's 
_CCA attribute or how CPUs may be accessing a shared buffer could lead 
to a loss of coherency. At worst, applying IOMMU_MMIO to a 
device-private buffer *could* cause the device to lose coherency with 
itself if the memory underlying the RMR may have allocated into system 
caches. Note that the expected use for non-remappable RMRs is the device 
holding some sort of long-lived private data in system RAM - the MSI 
doorbell trick is far more of a niche hack really.

At the very least I think we need to refer to the device's memory access 
properties here.

Jon, Laurentiu - how do RMRs correspond to the EFI memory map on your 
firmware? I'm starting to think that as long as the underlying memory is 
described appropriately there then we should be able to infer correct 
attributes from the EFI memory type and flags.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Mon, Sep 6, 2021 at 7:44 PM Robin Murphy [off-list ref] wrote:

On 2021-08-05 17:03, Lorenzo Pieralisi wrote:

quoted

On Thu, Aug 05, 2021 at 09:07:17AM +0100, Shameer Kolothum wrote:

[...]

quoted

+static void __init iort_node_get_rmr_info(struct acpi_iort_node *iort_node)
+{
+    struct acpi_iort_node *smmu;
+    struct acpi_iort_rmr *rmr;
+    struct acpi_iort_rmr_desc *rmr_desc;
+    u32 map_count = iort_node->mapping_count;
+    u32 sid;
+    int i;
+
+    if (!iort_node->mapping_offset || map_count != 1) {
+            pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
+                   iort_node);
+            return;
+    }
+
+    /* Retrieve associated smmu and stream id */
+    smmu = iort_node_get_id(iort_node, &sid, 0);
+    if (!smmu) {
+            pr_err(FW_BUG "Invalid SMMU reference, skipping RMR node %p\n",
+                   iort_node);
+            return;
+    }
+
+    /* Retrieve RMR data */
+    rmr = (struct acpi_iort_rmr *)iort_node->node_data;
+    if (!rmr->rmr_offset || !rmr->rmr_count) {
+            pr_err(FW_BUG "Invalid RMR descriptor array, skipping RMR node %p\n",
+                   iort_node);
+            return;
+    }
+
+    rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, iort_node,
+                            rmr->rmr_offset);
+
+    iort_rmr_desc_check_overlap(rmr_desc, rmr->rmr_count);
+
+    for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) {
+            struct iommu_resv_region *region;
+            enum iommu_resv_type type;
+            int prot = IOMMU_READ | IOMMU_WRITE;
+            u64 addr = rmr_desc->base_address, size = rmr_desc->length;
+
+            if (!IS_ALIGNED(addr, SZ_64K) || !IS_ALIGNED(size, SZ_64K)) {
+                    /* PAGE align base addr and size */
+                    addr &= PAGE_MASK;
+                    size = PAGE_ALIGN(size + offset_in_page(rmr_desc->base_address));
+
+                    pr_err(FW_BUG "RMR descriptor[0x%llx - 0x%llx] not aligned to 64K, continue with [0x%llx - 0x%llx]\n",
+                           rmr_desc->base_address,
+                           rmr_desc->base_address + rmr_desc->length - 1,
+                           addr, addr + size - 1);
+            }
+            if (rmr->flags & IOMMU_RMR_REMAP_PERMITTED) {
+                    type = IOMMU_RESV_DIRECT_RELAXABLE;
+                    /*
+                     * Set IOMMU_CACHE as IOMMU_RESV_DIRECT_RELAXABLE is
+                     * normally used for allocated system memory that is
+                     * then used for device specific reserved regions.
+                     */
+                    prot |= IOMMU_CACHE;
+            } else {
+                    type = IOMMU_RESV_DIRECT;
+                    /*
+                     * Set IOMMU_MMIO as IOMMU_RESV_DIRECT is normally used
+                     * for device memory like MSI doorbell.
+                     */
+                    prot |= IOMMU_MMIO;
+            }

On the prot value assignment based on the remapping flag, I'd like to
hear Robin/Joerg's opinion, I'd avoid being in a situation where
"normally" this would work but then we have to quirk it.

Is this a valid assumption _always_ ?

No. Certainly applying IOMMU_CACHE without reference to the device's
_CCA attribute or how CPUs may be accessing a shared buffer could lead
to a loss of coherency. At worst, applying IOMMU_MMIO to a
device-private buffer *could* cause the device to lose coherency with
itself if the memory underlying the RMR may have allocated into system
caches. Note that the expected use for non-remappable RMRs is the device
holding some sort of long-lived private data in system RAM - the MSI
doorbell trick is far more of a niche hack really.

At the very least I think we need to refer to the device's memory access
properties here.

Jon, Laurentiu - how do RMRs correspond to the EFI memory map on your
firmware? I'm starting to think that as long as the underlying memory is
described appropriately there then we should be able to infer correct
attributes from the EFI memory type and flags.

The devices are all cache coherent and marked as _CCA, 1.  The Memory
regions are in the virt table as ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.

The current chicken and egg problem we have is that during the fsl-mc-bus
initialization we call

error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
                                              &mc_stream_id);

which gets deferred because the SMMU has not been initialized yet. Then we
initialize the RMR tables but there is no device reference there to be able to
query device properties, only the stream id.  After the IORT tables are parsed
and the SMMU is setup, on the second device probe we associate everything
based on the stream id and the fsl-mc-bus device is able to claim its 1-1 DMA
mappings.

cat /sys/kernel/iommu_groups/0/reserved_regions
0x0000000001000000 0x0000000010ffffff direct-relaxable
0x0000000008000000 0x00000000080fffff msi
0x000000080c000000 0x000000081bffffff direct-relaxable
0x0000001c00000000 0x0000001c001fffff direct-relaxable
0x0000002080000000 0x000000209fffffff direct-relaxable

-Jon

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 06 September 2021 20:51
To: Robin Murphy <robin.murphy@arm.com>
Cc: Lorenzo Pieralisi <redacted>; Shameerali Kolothum Thodi
[off-list ref]; Laurentiu Tudor
[off-list ref]; linux-arm-kernel
[off-list ref]; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Linuxarm [off-list ref];
Joerg Roedel [off-list ref]; Will Deacon [off-list ref];
wanghuiqiang [off-list ref]; Guohanjun (Hanjun Guo)
[off-list ref]; Steven Price [off-list ref]; Sami
Mujawar [off-list ref]; Eric Auger [off-list ref];
yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing

[...]

quoted

quoted

On the prot value assignment based on the remapping flag, I'd like
to hear Robin/Joerg's opinion, I'd avoid being in a situation where
"normally" this would work but then we have to quirk it.

Is this a valid assumption _always_ ?

No. Certainly applying IOMMU_CACHE without reference to the device's
_CCA attribute or how CPUs may be accessing a shared buffer could lead
to a loss of coherency. At worst, applying IOMMU_MMIO to a
device-private buffer *could* cause the device to lose coherency with
itself if the memory underlying the RMR may have allocated into system
caches. Note that the expected use for non-remappable RMRs is the
device holding some sort of long-lived private data in system RAM -
the MSI doorbell trick is far more of a niche hack really.

At the very least I think we need to refer to the device's memory
access properties here.

Jon, Laurentiu - how do RMRs correspond to the EFI memory map on your
firmware? I'm starting to think that as long as the underlying memory
is described appropriately there then we should be able to infer
correct attributes from the EFI memory type and flags.

The devices are all cache coherent and marked as _CCA, 1.  The Memory
regions are in the virt table as ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.

The current chicken and egg problem we have is that during the fsl-mc-bus
initialization we call

error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
                                              &mc_stream_id);

which gets deferred because the SMMU has not been initialized yet. Then we
initialize the RMR tables but there is no device reference there to be able to
query device properties, only the stream id.  After the IORT tables are parsed
and the SMMU is setup, on the second device probe we associate everything
based on the stream id and the fsl-mc-bus device is able to claim its 1-1 DMA
mappings.

Can we solve this order problem by delaying the iommu_alloc_resv_region()
to the iommu_dma_get_rmr_resv_regions(dev, list) ? We could invoke
device_get_dma_attr() from there which I believe will return the _CCA attribute.

Or is that still early to invoke that?

Thanks,
Shameer

cat /sys/kernel/iommu_groups/0/reserved_regions
0x0000000001000000 0x0000000010ffffff direct-relaxable
0x0000000008000000 0x00000000080fffff msi
0x000000080c000000 0x000000081bffffff direct-relaxable
0x0000001c00000000 0x0000001c001fffff direct-relaxable
0x0000002080000000 0x000000209fffffff direct-relaxable

-Jon

quoted

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Sep 16, 2021 at 9:26 AM Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 06 September 2021 20:51
To: Robin Murphy <robin.murphy@arm.com>
Cc: Lorenzo Pieralisi <redacted>; Shameerali Kolothum Thodi
[off-list ref]; Laurentiu Tudor
[off-list ref]; linux-arm-kernel
[off-list ref]; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Linuxarm [off-list ref];
Joerg Roedel [off-list ref]; Will Deacon [off-list ref];
wanghuiqiang [off-list ref]; Guohanjun (Hanjun Guo)
[off-list ref]; Steven Price [off-list ref]; Sami
Mujawar [off-list ref]; Eric Auger [off-list ref];
yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing

[...]

quoted

quoted

quoted

On the prot value assignment based on the remapping flag, I'd like
to hear Robin/Joerg's opinion, I'd avoid being in a situation where
"normally" this would work but then we have to quirk it.

Is this a valid assumption _always_ ?

No. Certainly applying IOMMU_CACHE without reference to the device's
_CCA attribute or how CPUs may be accessing a shared buffer could lead
to a loss of coherency. At worst, applying IOMMU_MMIO to a
device-private buffer *could* cause the device to lose coherency with
itself if the memory underlying the RMR may have allocated into system
caches. Note that the expected use for non-remappable RMRs is the
device holding some sort of long-lived private data in system RAM -
the MSI doorbell trick is far more of a niche hack really.

At the very least I think we need to refer to the device's memory
access properties here.

Jon, Laurentiu - how do RMRs correspond to the EFI memory map on your
firmware? I'm starting to think that as long as the underlying memory
is described appropriately there then we should be able to infer
correct attributes from the EFI memory type and flags.

The devices are all cache coherent and marked as _CCA, 1.  The Memory
regions are in the virt table as ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.

The current chicken and egg problem we have is that during the fsl-mc-bus
initialization we call

error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
                                              &mc_stream_id);

which gets deferred because the SMMU has not been initialized yet. Then we
initialize the RMR tables but there is no device reference there to be able to
query device properties, only the stream id.  After the IORT tables are parsed
and the SMMU is setup, on the second device probe we associate everything
based on the stream id and the fsl-mc-bus device is able to claim its 1-1 DMA
mappings.

Can we solve this order problem by delaying the iommu_alloc_resv_region()
to the iommu_dma_get_rmr_resv_regions(dev, list) ? We could invoke
device_get_dma_attr() from there which I believe will return the _CCA attribute.

Or is that still early to invoke that?

That looks like it should work. Do we then also need to parse through the
VirtualMemoryTable matching the start and end addresses to determine the
other memory attributes like MMIO?

-Jon

Thanks,
Shameer

quoted

cat /sys/kernel/iommu_groups/0/reserved_regions
0x0000000001000000 0x0000000010ffffff direct-relaxable
0x0000000008000000 0x00000000080fffff msi
0x000000080c000000 0x000000081bffffff direct-relaxable
0x0000001c00000000 0x0000001c001fffff direct-relaxable
0x0000002080000000 0x000000209fffffff direct-relaxable

-Jon

quoted

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 16 September 2021 08:52
To: Shameerali Kolothum Thodi <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>; Lorenzo Pieralisi
[off-list ref]; Laurentiu Tudor [off-list ref];
linux-arm-kernel [off-list ref]; ACPI Devel Maling
List [off-list ref]; Linux IOMMU
[off-list ref]; Joerg Roedel [off-list ref]; Will
Deacon [off-list ref]; wanghuiqiang [off-list ref];
Guohanjun (Hanjun Guo) [off-list ref]; Steven Price
[off-list ref]; Sami Mujawar [off-list ref]; Eric
Auger [off-list ref]; yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing

On Thu, Sep 16, 2021 at 9:26 AM Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 06 September 2021 20:51
To: Robin Murphy <robin.murphy@arm.com>
Cc: Lorenzo Pieralisi <redacted>; Shameerali
Kolothum Thodi [off-list ref]; Laurentiu
Tudor [off-list ref]; linux-arm-kernel
[off-list ref]; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Linuxarm [off-list ref];
Joerg Roedel [off-list ref]; Will Deacon [off-list ref];
wanghuiqiang [off-list ref]; Guohanjun (Hanjun Guo)
[off-list ref]; Steven Price [off-list ref]; Sami
Mujawar [off-list ref]; Eric Auger

[off-list ref];

quoted

quoted

yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node
parsing

[...]

quoted

quoted

quoted

On the prot value assignment based on the remapping flag, I'd
like to hear Robin/Joerg's opinion, I'd avoid being in a
situation where "normally" this would work but then we have to quirk

it.

quoted

quoted

quoted

quoted

Is this a valid assumption _always_ ?

No. Certainly applying IOMMU_CACHE without reference to the
device's _CCA attribute or how CPUs may be accessing a shared
buffer could lead to a loss of coherency. At worst, applying
IOMMU_MMIO to a device-private buffer *could* cause the device to
lose coherency with itself if the memory underlying the RMR may
have allocated into system caches. Note that the expected use for
non-remappable RMRs is the device holding some sort of long-lived
private data in system RAM - the MSI doorbell trick is far more of a niche

hack really.

quoted

quoted

quoted

At the very least I think we need to refer to the device's memory
access properties here.

Jon, Laurentiu - how do RMRs correspond to the EFI memory map on
your firmware? I'm starting to think that as long as the
underlying memory is described appropriately there then we should
be able to infer correct attributes from the EFI memory type and flags.

The devices are all cache coherent and marked as _CCA, 1.  The
Memory regions are in the virt table as

ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.

quoted

quoted

The current chicken and egg problem we have is that during the
fsl-mc-bus initialization we call

error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
                                              &mc_stream_id);

which gets deferred because the SMMU has not been initialized yet.
Then we initialize the RMR tables but there is no device reference
there to be able to query device properties, only the stream id.
After the IORT tables are parsed and the SMMU is setup, on the
second device probe we associate everything based on the stream id
and the fsl-mc-bus device is able to claim its 1-1 DMA mappings.

Can we solve this order problem by delaying the
iommu_alloc_resv_region() to the iommu_dma_get_rmr_resv_regions(dev,
list) ? We could invoke
device_get_dma_attr() from there which I believe will return the _CCA

attribute.

quoted

Or is that still early to invoke that?

That looks like it should work. Do we then also need to parse through the
VirtualMemoryTable matching the start and end addresses to determine the
other memory attributes like MMIO?

Yes. But that looks tricky as I can't find that readily available on Arm, like the
efi_mem_attributes(). I will take a look.

Please let me know if there is one or any other easy way to retrieve it.

Thanks,
Shameer

-Jon

quoted

Thanks,
Shameer

quoted

cat /sys/kernel/iommu_groups/0/reserved_regions
0x0000000001000000 0x0000000010ffffff direct-relaxable
0x0000000008000000 0x00000000080fffff msi
0x000000080c000000 0x000000081bffffff direct-relaxable
0x0000001c00000000 0x0000001c001fffff direct-relaxable
0x0000002080000000 0x000000209fffffff direct-relaxable

-Jon

quoted

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On Thu, Sep 16, 2021 at 10:26 AM Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 16 September 2021 08:52
To: Shameerali Kolothum Thodi <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>; Lorenzo Pieralisi
[off-list ref]; Laurentiu Tudor [off-list ref];
linux-arm-kernel [off-list ref]; ACPI Devel Maling
List [off-list ref]; Linux IOMMU
[off-list ref]; Joerg Roedel [off-list ref]; Will
Deacon [off-list ref]; wanghuiqiang [off-list ref];
Guohanjun (Hanjun Guo) [off-list ref]; Steven Price
[off-list ref]; Sami Mujawar [off-list ref]; Eric
Auger [off-list ref]; yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing

On Thu, Sep 16, 2021 at 9:26 AM Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 06 September 2021 20:51
To: Robin Murphy <robin.murphy@arm.com>
Cc: Lorenzo Pieralisi <redacted>; Shameerali
Kolothum Thodi [off-list ref]; Laurentiu
Tudor [off-list ref]; linux-arm-kernel
[off-list ref]; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Linuxarm [off-list ref];
Joerg Roedel [off-list ref]; Will Deacon [off-list ref];
wanghuiqiang [off-list ref]; Guohanjun (Hanjun Guo)
[off-list ref]; Steven Price [off-list ref]; Sami
Mujawar [off-list ref]; Eric Auger

[off-list ref];

quoted

quoted

yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node
parsing

[...]

quoted

quoted

quoted

On the prot value assignment based on the remapping flag, I'd
like to hear Robin/Joerg's opinion, I'd avoid being in a
situation where "normally" this would work but then we have to quirk

it.

quoted

quoted

quoted

quoted

Is this a valid assumption _always_ ?

No. Certainly applying IOMMU_CACHE without reference to the
device's _CCA attribute or how CPUs may be accessing a shared
buffer could lead to a loss of coherency. At worst, applying
IOMMU_MMIO to a device-private buffer *could* cause the device to
lose coherency with itself if the memory underlying the RMR may
have allocated into system caches. Note that the expected use for
non-remappable RMRs is the device holding some sort of long-lived
private data in system RAM - the MSI doorbell trick is far more of a niche

hack really.

quoted

quoted

quoted

At the very least I think we need to refer to the device's memory
access properties here.

Jon, Laurentiu - how do RMRs correspond to the EFI memory map on
your firmware? I'm starting to think that as long as the
underlying memory is described appropriately there then we should
be able to infer correct attributes from the EFI memory type and flags.

The devices are all cache coherent and marked as _CCA, 1.  The
Memory regions are in the virt table as

ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.

quoted

quoted

The current chicken and egg problem we have is that during the
fsl-mc-bus initialization we call

error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
                                              &mc_stream_id);

which gets deferred because the SMMU has not been initialized yet.
Then we initialize the RMR tables but there is no device reference
there to be able to query device properties, only the stream id.
After the IORT tables are parsed and the SMMU is setup, on the
second device probe we associate everything based on the stream id
and the fsl-mc-bus device is able to claim its 1-1 DMA mappings.

Can we solve this order problem by delaying the
iommu_alloc_resv_region() to the iommu_dma_get_rmr_resv_regions(dev,
list) ? We could invoke
device_get_dma_attr() from there which I believe will return the _CCA

attribute.

quoted

Or is that still early to invoke that?

That looks like it should work. Do we then also need to parse through the
VirtualMemoryTable matching the start and end addresses to determine the
other memory attributes like MMIO?

Yes. But that looks tricky as I can't find that readily available on Arm, like the
efi_mem_attributes(). I will take a look.

Please let me know if there is one or any other easy way to retrieve it.

maybe we don't need to.  Maybe it is enough to just move
iommu_alloc_resv_regions and then set the IOMMU_CACHE flag
if type = IOMMU_RESV_DIRECT_RELAXABLE and _CCN=1?

-Jon

Thanks,
Shameer

quoted

-Jon

quoted

Thanks,
Shameer

quoted

cat /sys/kernel/iommu_groups/0/reserved_regions
0x0000000001000000 0x0000000010ffffff direct-relaxable
0x0000000008000000 0x00000000080fffff msi
0x000000080c000000 0x000000081bffffff direct-relaxable
0x0000001c00000000 0x0000001c001fffff direct-relaxable
0x0000002080000000 0x000000209fffffff direct-relaxable

-Jon

quoted

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 16 September 2021 12:17
To: Shameerali Kolothum Thodi <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>; Lorenzo Pieralisi
[off-list ref]; Laurentiu Tudor [off-list ref];
linux-arm-kernel [off-list ref]; ACPI Devel Maling
List [off-list ref]; Linux IOMMU
[off-list ref]; Joerg Roedel [off-list ref]; Will
Deacon [off-list ref]; wanghuiqiang [off-list ref];
Guohanjun (Hanjun Guo) [off-list ref]; Steven Price
[off-list ref]; Sami Mujawar [off-list ref]; Eric
Auger [off-list ref]; yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing

On Thu, Sep 16, 2021 at 10:26 AM Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 16 September 2021 08:52
To: Shameerali Kolothum Thodi

[off-list ref]

quoted

quoted

Cc: Robin Murphy <robin.murphy@arm.com>; Lorenzo Pieralisi
[off-list ref]; Laurentiu Tudor
[off-list ref]; linux-arm-kernel
[off-list ref]; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Joerg Roedel [off-list ref];
Will Deacon [off-list ref]; wanghuiqiang
[off-list ref]; Guohanjun (Hanjun Guo)
[off-list ref]; Steven Price [off-list ref]; Sami
Mujawar [off-list ref]; Eric Auger

[off-list ref];

quoted

quoted

yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node
parsing

On Thu, Sep 16, 2021 at 9:26 AM Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 06 September 2021 20:51
To: Robin Murphy <robin.murphy@arm.com>
Cc: Lorenzo Pieralisi <redacted>; Shameerali
Kolothum Thodi [off-list ref]; Laurentiu
Tudor [off-list ref]; linux-arm-kernel
[off-list ref]; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Linuxarm
[off-list ref]; Joerg Roedel [off-list ref]; Will
Deacon [off-list ref]; wanghuiqiang
[off-list ref]; Guohanjun (Hanjun Guo)
[off-list ref]; Steven Price [off-list ref];
Sami Mujawar [off-list ref]; Eric Auger

[off-list ref];

quoted

quoted

yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node
parsing

[...]

quoted

quoted

quoted

On the prot value assignment based on the remapping flag,
I'd like to hear Robin/Joerg's opinion, I'd avoid being in a
situation where "normally" this would work but then we have
to quirk

it.

quoted

quoted

quoted

quoted

Is this a valid assumption _always_ ?

No. Certainly applying IOMMU_CACHE without reference to the
device's _CCA attribute or how CPUs may be accessing a shared
buffer could lead to a loss of coherency. At worst, applying
IOMMU_MMIO to a device-private buffer *could* cause the device
to lose coherency with itself if the memory underlying the RMR
may have allocated into system caches. Note that the expected
use for non-remappable RMRs is the device holding some sort of
long-lived private data in system RAM - the MSI doorbell trick
is far more of a niche

hack really.

quoted

quoted

quoted

At the very least I think we need to refer to the device's
memory access properties here.

Jon, Laurentiu - how do RMRs correspond to the EFI memory map
on your firmware? I'm starting to think that as long as the
underlying memory is described appropriately there then we
should be able to infer correct attributes from the EFI memory type

and flags.

quoted

quoted

quoted

quoted

The devices are all cache coherent and marked as _CCA, 1.  The
Memory regions are in the virt table as

ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.

quoted

quoted

The current chicken and egg problem we have is that during the
fsl-mc-bus initialization we call

error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
                                              &mc_stream_id);

which gets deferred because the SMMU has not been initialized yet.
Then we initialize the RMR tables but there is no device
reference there to be able to query device properties, only the stream

id.

quoted

quoted

quoted

quoted

After the IORT tables are parsed and the SMMU is setup, on the
second device probe we associate everything based on the stream
id and the fsl-mc-bus device is able to claim its 1-1 DMA mappings.

Can we solve this order problem by delaying the
iommu_alloc_resv_region() to the
iommu_dma_get_rmr_resv_regions(dev,
list) ? We could invoke
device_get_dma_attr() from there which I believe will return the
_CCA

attribute.

quoted

Or is that still early to invoke that?

That looks like it should work. Do we then also need to parse
through the VirtualMemoryTable matching the start and end addresses
to determine the other memory attributes like MMIO?

Yes. But that looks tricky as I can't find that readily available on
Arm, like the efi_mem_attributes(). I will take a look.

Please let me know if there is one or any other easy way to retrieve it.

maybe we don't need to.  Maybe it is enough to just move
iommu_alloc_resv_regions and then set the IOMMU_CACHE flag if type =
IOMMU_RESV_DIRECT_RELAXABLE and _CCN=1?

It looks like we could simply call efi_mem_type() and check for
EFI_MEMORY_MAPPED_IO. I have updated the code to set the
RMR prot value based on _CCA and EFI md type. Please see the 
last commit on this branch here(not tested),

https://github.com/hisilicon/kernel-dev/commits/private-v5.14-rc4-rmr-v7-ext

Please take a look and let me know if this is good enough to solve this problem.

Thanks,
Shameer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Shameer,

On 8/5/21 10:07 AM, Shameer Kolothum wrote:

Hi,

The series adds support to IORT RMR nodes specified in IORT
Revision E.b -ARM DEN 0049E[0]. RMR nodes are used to describe
memory ranges that are used by endpoints and require a unity
mapping in SMMU.

I used your series and RMRs to force a guest iommu (vSMMUv3 nested stage
use case) to have a flat mapping for IOVAs within [0x8000000, 0x8100000]
(matching MSI_IOVA_BASE and MSI_IOVA_LENGTH) used by the host to map MSI
physical doorbells.

That way when an assigned device protected by a vSMMUv3 implemented upon
nested stage issues an MSI transaction, let's say using IOVA=0x8000000,
we would get:
                    S1 (guest)           S2 (host)
0x8000000            0x8000000            Physical DB

This method was suggested by Jean-Philippe (added in CC) and it
simplifies the nested stage integration because we don't have to care
about nested stage MSI bindings.

However if I understand correctly we cannot define a range of SIDs using
the same RMR (due to the single mapping bit which must be set, Table 5
flags format). This is a spec restriction and not an issue with your series.

As VFIO devices can be hot-plugged we thus need to create as many RMR
nodes as potential BDFs, leading to 256 * 6 = 1536 RMR nodes if you have
5 pcie root ports as it is usual in VMs. Then this causes some trouble
at qemu level for instance, wrt migration. See [RFC]
hw/arm/virt-acpi-build: Add IORT RMR regions to handle MSI nested binding.

Do you know if there is a plan to remove the single mapping limitation
in the spec?

Thanks

Eric

We have faced issues with 3408iMR RAID controller cards which
fail to boot when SMMU is enabled. This is because these
controllers make use of host memory for various caching related
purposes and when SMMU is enabled the iMR firmware fails to 
access these memory regions as there is no mapping for them.
IORT RMR provides a way for UEFI to describe and report these
memory regions so that the kernel can make a unity mapping for
these in SMMU.

Change History: 

v6 --> v7

The only change from v6 is the fix pointed out by Steve to
the SMMUv2 SMR bypass install in patch #8.

Thanks to the Tested-by tags by Laurentiu with SMMUv2 and
Hanjun/Huiqiang with SMMUv3 for v6. I haven't added the tags
yet as the series still needs more review[1].

Feedback and tests on this series is very much appreciated.

v5 --> v6
- Addressed comments from Robin & Lorenzo.
  : Moved iort_parse_rmr() to acpi_iort_init() from
    iort_init_platform_devices().
  : Removed use of struct iort_rmr_entry during the initial
    parse. Using struct iommu_resv_region instead.
  : Report RMR address alignment and overlap errors, but continue.
  : Reworked arm_smmu_init_bypass_stes() (patch # 6).
- Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8).
- Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based
  on Type of RMR region. Suggested by Jon N.

Thanks,
Shameer
[0] https://developer.arm.com/documentation/den0049/latest/
[1] https://lore.kernel.org/linux-acpi/20210716083442.1708-1-shameerali.kolothum.thodi@huawei.com/T/#m043c95b869973a834b2fd57f3e1ed0325c84f3b7
------
v4 --> v5
 -Added a fw_data union to struct iommu_resv_region and removed
  struct iommu_rmr (Based on comments from Joerg/Robin).
 -Added iommu_put_rmrs() to release mem.
 -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by
  yet because of the above changes.

v3 -->v4
-Included the SMMUv2 SMR bypass install changes suggested by
 Steve(patch #7)
-As per Robin's comments, RMR reserve implementation is now
 more generic  (patch #8) and dropped v3 patches 8 and 10.
-Rebase to 5.13-rc1

RFC v2 --> v3
 -Dropped RFC tag as the ACPICA header changes are now ready to be
  part of 5.13[0]. But this series still has a dependency on that patch.
 -Added IORT E.b related changes(node flags, _DSM function 5 checks for
  PCIe).
 -Changed RMR to stream id mapping from M:N to M:1 as per the spec and
  discussion here[1].
 -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!)
------

Jon Nettleton (1):
  iommu/arm-smmu: Get associated RMR info and install bypass SMR

Shameer Kolothum (8):
  iommu: Introduce a union to struct iommu_resv_region
  ACPI/IORT: Add support for RMR node parsing
  iommu/dma: Introduce generic helper to retrieve RMR info
  ACPI/IORT: Add a helper to retrieve RMR memory regions
  iommu/arm-smmu-v3: Introduce strtab init helper
  iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force
    bypass
  iommu/arm-smmu-v3: Get associated RMR info and install bypass STE
  iommu/dma: Reserve any RMR regions associated with a dev

 drivers/acpi/arm64/iort.c                   | 172 +++++++++++++++++++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  76 +++++++--
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  48 ++++++
 drivers/iommu/dma-iommu.c                   |  89 +++++++++-
 include/linux/acpi_iort.h                   |   7 +
 include/linux/dma-iommu.h                   |  13 ++
 include/linux/iommu.h                       |  11 ++
 7 files changed, 393 insertions(+), 23 deletions(-)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Eric,

-----Original Message-----
From: Eric Auger [mailto:eric.auger@redhat.com]
Sent: 30 September 2021 10:48
To: Shameerali Kolothum Thodi <redacted>;
linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
iommu@lists.linux-foundation.org; Jean-Philippe Brucker
[off-list ref]
Cc: Linuxarm <redacted>; lorenzo.pieralisi@arm.com;
joro@8bytes.org; robin.murphy@arm.com; will@kernel.org; wanghuiqiang
[off-list ref]; Guohanjun (Hanjun Guo)
[off-list ref]; steven.price@arm.com; Sami.Mujawar@arm.com;
jon@solid-run.com; yangyicong [off-list ref]
Subject: Re: [PATCH v7 0/9] ACPI/IORT: Support for IORT RMR node

Hi Shameer,

On 8/5/21 10:07 AM, Shameer Kolothum wrote:

quoted

Hi,

The series adds support to IORT RMR nodes specified in IORT
Revision E.b -ARM DEN 0049E[0]. RMR nodes are used to describe
memory ranges that are used by endpoints and require a unity
mapping in SMMU.

I used your series and RMRs to force a guest iommu (vSMMUv3 nested stage
use case) to have a flat mapping for IOVAs within [0x8000000, 0x8100000]
(matching MSI_IOVA_BASE and MSI_IOVA_LENGTH) used by the host to map
MSI
physical doorbells.

That way when an assigned device protected by a vSMMUv3 implemented
upon
nested stage issues an MSI transaction, let's say using IOVA=0x8000000,
we would get:
                    S1 (guest)           S2 (host)
0x8000000            0x8000000            Physical DB

This method was suggested by Jean-Philippe (added in CC) and it
simplifies the nested stage integration because we don't have to care
about nested stage MSI bindings.

However if I understand correctly we cannot define a range of SIDs using
the same RMR (due to the single mapping bit which must be set, Table 5
flags format). This is a spec restriction and not an issue with your series.

Yes. The spec currently mandates single mapping bit to be set.

As VFIO devices can be hot-plugged we thus need to create as many RMR
nodes as potential BDFs, leading to 256 * 6 = 1536 RMR nodes if you have
5 pcie root ports as it is usual in VMs. Then this causes some trouble
at qemu level for instance, wrt migration. See [RFC]
hw/arm/virt-acpi-build: Add IORT RMR regions to handle MSI nested binding.

Do you know if there is a plan to remove the single mapping limitation
in the spec?

I would imagine so. In an earlier comment[1], Robin did mention about possible
relaxing of this in future spec revision.

Thanks,
Shameer
1. https://lore.kernel.org/linux-arm-kernel/15c7fac0-11a8-0cdb-aac3-b5d8feb8f066@arm.com/

Thanks

Eric

quoted

We have faced issues with 3408iMR RAID controller cards which
fail to boot when SMMU is enabled. This is because these
controllers make use of host memory for various caching related
purposes and when SMMU is enabled the iMR firmware fails to
access these memory regions as there is no mapping for them.
IORT RMR provides a way for UEFI to describe and report these
memory regions so that the kernel can make a unity mapping for
these in SMMU.

Change History:

v6 --> v7

The only change from v6 is the fix pointed out by Steve to
the SMMUv2 SMR bypass install in patch #8.

Thanks to the Tested-by tags by Laurentiu with SMMUv2 and
Hanjun/Huiqiang with SMMUv3 for v6. I haven't added the tags
yet as the series still needs more review[1].

Feedback and tests on this series is very much appreciated.

v5 --> v6
- Addressed comments from Robin & Lorenzo.
  : Moved iort_parse_rmr() to acpi_iort_init() from
    iort_init_platform_devices().
  : Removed use of struct iort_rmr_entry during the initial
    parse. Using struct iommu_resv_region instead.
  : Report RMR address alignment and overlap errors, but continue.
  : Reworked arm_smmu_init_bypass_stes() (patch # 6).
- Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8).
- Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based
  on Type of RMR region. Suggested by Jon N.

Thanks,
Shameer
[0] https://developer.arm.com/documentation/den0049/latest/
[1]

https://lore.kernel.org/linux-acpi/20210716083442.1708-1-shameerali.koloth
um.thodi@huawei.com/T/#m043c95b869973a834b2fd57f3e1ed0325c84f3b7

quoted

------
v4 --> v5
 -Added a fw_data union to struct iommu_resv_region and removed
  struct iommu_rmr (Based on comments from Joerg/Robin).
 -Added iommu_put_rmrs() to release mem.
 -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by
  yet because of the above changes.

v3 -->v4
-Included the SMMUv2 SMR bypass install changes suggested by
 Steve(patch #7)
-As per Robin's comments, RMR reserve implementation is now
 more generic  (patch #8) and dropped v3 patches 8 and 10.
-Rebase to 5.13-rc1

RFC v2 --> v3
 -Dropped RFC tag as the ACPICA header changes are now ready to be
  part of 5.13[0]. But this series still has a dependency on that patch.
 -Added IORT E.b related changes(node flags, _DSM function 5 checks for
  PCIe).
 -Changed RMR to stream id mapping from M:N to M:1 as per the spec and
  discussion here[1].
 -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!)
------

Jon Nettleton (1):
  iommu/arm-smmu: Get associated RMR info and install bypass SMR

Shameer Kolothum (8):
  iommu: Introduce a union to struct iommu_resv_region
  ACPI/IORT: Add support for RMR node parsing
  iommu/dma: Introduce generic helper to retrieve RMR info
  ACPI/IORT: Add a helper to retrieve RMR memory regions
  iommu/arm-smmu-v3: Introduce strtab init helper
  iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force
    bypass
  iommu/arm-smmu-v3: Get associated RMR info and install bypass STE
  iommu/dma: Reserve any RMR regions associated with a dev

 drivers/acpi/arm64/iort.c                   | 172

+++++++++++++++++++-

quoted

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  76 +++++++--
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  48 ++++++
 drivers/iommu/dma-iommu.c                   |  89 +++++++++-
 include/linux/acpi_iort.h                   |   7 +
 include/linux/dma-iommu.h                   |  13 ++
 include/linux/iommu.h                       |  11 ++
 7 files changed, 393 insertions(+), 23 deletions(-)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 9/17/2021 2:26 PM, Shameerali Kolothum Thodi wrote:

quoted

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 16 September 2021 12:17
To: Shameerali Kolothum Thodi <redacted>
Cc: Robin Murphy <robin.murphy@arm.com>; Lorenzo Pieralisi
[off-list ref]; Laurentiu Tudor [off-list ref];
linux-arm-kernel [off-list ref]; ACPI Devel Maling
List [off-list ref]; Linux IOMMU
[off-list ref]; Joerg Roedel [off-list ref]; Will
Deacon [off-list ref]; wanghuiqiang [off-list ref];
Guohanjun (Hanjun Guo) [off-list ref]; Steven Price
[off-list ref]; Sami Mujawar [off-list ref]; Eric
Auger [off-list ref]; yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing

On Thu, Sep 16, 2021 at 10:26 AM Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 16 September 2021 08:52
To: Shameerali Kolothum Thodi

[off-list ref]

quoted

quoted

Cc: Robin Murphy <robin.murphy@arm.com>; Lorenzo Pieralisi
[off-list ref]; Laurentiu Tudor
[off-list ref]; linux-arm-kernel
[off-list ref]; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Joerg Roedel [off-list ref];
Will Deacon [off-list ref]; wanghuiqiang
[off-list ref]; Guohanjun (Hanjun Guo)
[off-list ref]; Steven Price [off-list ref]; Sami
Mujawar [off-list ref]; Eric Auger

[off-list ref];

quoted

quoted

yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node
parsing

On Thu, Sep 16, 2021 at 9:26 AM Shameerali Kolothum Thodi
[off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Jon Nettleton [mailto:jon@solid-run.com]
Sent: 06 September 2021 20:51
To: Robin Murphy <robin.murphy@arm.com>
Cc: Lorenzo Pieralisi <redacted>; Shameerali
Kolothum Thodi [off-list ref]; Laurentiu
Tudor [off-list ref]; linux-arm-kernel
[off-list ref]; ACPI Devel Maling List
[off-list ref]; Linux IOMMU
[off-list ref]; Linuxarm
[off-list ref]; Joerg Roedel [off-list ref]; Will
Deacon [off-list ref]; wanghuiqiang
[off-list ref]; Guohanjun (Hanjun Guo)
[off-list ref]; Steven Price [off-list ref];
Sami Mujawar [off-list ref]; Eric Auger

[off-list ref];

quoted

quoted

yangyicong [off-list ref]
Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node
parsing

[...]

quoted

quoted

quoted

On the prot value assignment based on the remapping flag,
I'd like to hear Robin/Joerg's opinion, I'd avoid being in a
situation where "normally" this would work but then we have
to quirk

it.

quoted

quoted

quoted

quoted

Is this a valid assumption _always_ ?

No. Certainly applying IOMMU_CACHE without reference to the
device's _CCA attribute or how CPUs may be accessing a shared
buffer could lead to a loss of coherency. At worst, applying
IOMMU_MMIO to a device-private buffer *could* cause the device
to lose coherency with itself if the memory underlying the RMR
may have allocated into system caches. Note that the expected
use for non-remappable RMRs is the device holding some sort of
long-lived private data in system RAM - the MSI doorbell trick
is far more of a niche

hack really.

quoted

quoted

quoted

At the very least I think we need to refer to the device's
memory access properties here.

Jon, Laurentiu - how do RMRs correspond to the EFI memory map
on your firmware? I'm starting to think that as long as the
underlying memory is described appropriately there then we
should be able to infer correct attributes from the EFI memory type

and flags.

quoted

quoted

quoted

quoted

The devices are all cache coherent and marked as _CCA, 1.  The
Memory regions are in the virt table as

ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.

quoted

quoted

The current chicken and egg problem we have is that during the
fsl-mc-bus initialization we call

error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
                                              &mc_stream_id);

which gets deferred because the SMMU has not been initialized yet.
Then we initialize the RMR tables but there is no device
reference there to be able to query device properties, only the stream

id.

quoted

quoted

quoted

quoted

After the IORT tables are parsed and the SMMU is setup, on the
second device probe we associate everything based on the stream
id and the fsl-mc-bus device is able to claim its 1-1 DMA mappings.

Can we solve this order problem by delaying the
iommu_alloc_resv_region() to the
iommu_dma_get_rmr_resv_regions(dev,
list) ? We could invoke
device_get_dma_attr() from there which I believe will return the
_CCA

attribute.

quoted

Or is that still early to invoke that?

That looks like it should work. Do we then also need to parse
through the VirtualMemoryTable matching the start and end addresses
to determine the other memory attributes like MMIO?

Yes. But that looks tricky as I can't find that readily available on
Arm, like the efi_mem_attributes(). I will take a look.

Please let me know if there is one or any other easy way to retrieve it.

maybe we don't need to.  Maybe it is enough to just move
iommu_alloc_resv_regions and then set the IOMMU_CACHE flag if type =
IOMMU_RESV_DIRECT_RELAXABLE and _CCN=1?

It looks like we could simply call efi_mem_type() and check for
EFI_MEMORY_MAPPED_IO. I have updated the code to set the
RMR prot value based on _CCA and EFI md type. Please see the 
last commit on this branch here(not tested),

https://github.com/hisilicon/kernel-dev/commits/private-v5.14-rc4-rmr-v7-ext

Please take a look and let me know if this is good enough to solve this problem.

Sorry for the delay, I managed to test on a NXP LX2160A and things look
fine, so:

Tested-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>

---
Best Regards, Laurentiu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 2021-08-05 09:07, Shameer Kolothum wrote:

Do we really need to duplicate all this data? AFAICS we could just save 
the acpi_iort_rmr pointer in the iommu_resv_region (with a forward 
declaration here if necessary) and defer parsing its actual mappings 
until the point where we can directly consume the results.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On 2021-08-05 09:07, Shameer Kolothum wrote: