[RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node

[RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node

[Shameer Kolothum]

The series adds support to IORT RMR nodes specified in IORT
Revision E -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.

RFC because, Patch #1 is to update the actbl2.h and should be done
through acpica update. I have send out a pull request[1] for that.

Tests:

With a UEFI, that reports the RMR for the dev,
....
[16F0h 5872   1]                         Type : 06
[16F1h 5873   2]                       Length : 007C
[16F3h 5875   1]                     Revision : 00
[1038h 0056   2]                     Reserved : 00000000
[1038h 0056   2]                   Identifier : 00000000
[16F8h 5880   4]                Mapping Count : 00000001
[16FCh 5884   4]               Mapping Offset : 00000040

[1700h 5888   4]    Number of RMR Descriptors : 00000002
[1704h 5892   4]        RMR Descriptor Offset : 00000018

[1708h 5896   8]          Base Address of RMR : 0000E6400000
[1710h 5904   8]                Length of RMR : 000000100000
[1718h 5912   4]                     Reserved : 00000000

[171Ch 5916   8]          Base Address of RMR : 0000000027B00000
[1724h 5924   8]                Length of RMR : 0000000000C00000
[172Ch 5932   4]                     Reserved : 00000000

[1730h 5936   4]                   Input base : 00000000
[1734h 5940   4]                     ID Count : 00000001
[1738h 5944   4]                  Output Base : 00000003
[173Ch 5948   4]             Output Reference : 00000064
[1740h 5952   4]        Flags (decoded below) : 00000001
                               Single Mapping : 1
...

Without the series the RAID controller initilization fails as
below,

...
[   12.631117] megaraid_sas 0000:03:00.0: FW supports sync cache        : Yes   
[   12.637360] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009                                                   
[   18.776377] megaraid_sas 0000:03:00.0: Init cmd return status FAILED for SCSI host 0                                                                         
[   23.019383] megaraid_sas 0000:03:00.0: Waiting for FW to come to ready state 
[  106.684281] megaraid_sas 0000:03:00.0: FW in FAULT state, Fault code:0x10000 subcode:0x0 func:megasas_transition_to_ready                                    
[  106.695186] megaraid_sas 0000:03:00.0: System Register set:                  
[  106.889787] megaraid_sas 0000:03:00.0: Failed to transition controller to ready for scsi0.                                                                   
[  106.910475] megaraid_sas 0000:03:00.0: Failed from megasas_init_fw 6407      
estuary:/$

With the series, now the kernel has direct mapping for the dev as
below,

estuary:/$ cat /sys/kernel/iommu_groups/0/reserved_regions                      
0x0000000008000000 0x00000000080fffff msi                                       
0x0000000027b00000 0x00000000286fffff direct                                    
0x00000000e6400000 0x00000000e64fffff direct                                    
estuary:/$

....
[   12.254318] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009                                                   
[   12.739089] megaraid_sas 0000:03:00.0: FW provided supportMaxExtLDs: 0      max_lds: 32                                                                      
[   12.746628] megaraid_sas 0000:03:00.0: controller type       : iMR(0MB)      
[   12.752694] megaraid_sas 0000:03:00.0: Online Controller Reset(OCR)  : Enabled                                                                               
[   12.759798] megaraid_sas 0000:03:00.0: Secure JBOD support   : Yes           
[   12.765778] megaraid_sas 0000:03:00.0: NVMe passthru support : Yes           
[   12.771931] megaraid_sas 0000:03:00.0: FW provided TM TaskAbort/Reset timeou: 6 secs/60 secs                                                                 
[   12.780503] megaraid_sas 0000:03:00.0: JBOD sequence map support     : Yes   
[   12.787000] megaraid_sas 0000:03:00.0: PCI Lane Margining support    : No    
[   12.819179] megaraid_sas 0000:03:00.0: NVME page size        : (4096)        
[   12.825672] megaraid_sas 0000:03:00.0: megasas_enable_intr_fusion is called outbound_intr_mask:0x40000000                                                    
[   12.835199] megaraid_sas 0000:03:00.0: INIT adapter done                     
[   12.873932] megaraid_sas 0000:03:00.0: pci id                : (0x1000)/(0x0017)/(0x19e5)/(0xd213)                                                           
[   12.881644] megaraid_sas 0000:03:00.0: unevenspan support    : no            
[   12.887451] megaraid_sas 0000:03:00.0: firmware crash dump   : no            
[   12.893344] megaraid_sas 0000:03:00.0: JBOD sequence map     : enabled       

RAID controller init is now success and can detect the drives
attached as well.

Thanks,
Shameer

[0]. https://developer.arm.com/documentation/den0049/latest/
[1]. https://github.com/acpica/acpica/pull/638

Shameer Kolothum (4):
  ACPICA: IORT: Update for revision E
  ACPI/IORT: Add support for RMR node parsing
  ACPI/IORT: Add RMR memory regions reservation helper
  iommu/dma: Reserve any RMR regions associated with a dev

 drivers/acpi/arm64/iort.c | 175 +++++++++++++++++++++++++++++++++++++-
 drivers/iommu/dma-iommu.c |  12 ++-
 include/acpi/actbl2.h     |  25 ++++--
 include/linux/acpi_iort.h |   4 +
 4 files changed, 205 insertions(+), 11 deletions(-)

-- 
2.17.1

[RFC PATCH 1/4] ACPICA: IORT: Update for revision E

[Shameer Kolothum]

IORT revision E contains a few additions like,
    -Added an identifier field in the node descriptors to aid table
     cross-referencing.
    -Introduced the Reserved Memory Range(RMR) node. This is used
     to describe memory ranges that are used by endpoints and requires
     a unity mapping in SMMU.
    -Introduced a flag in the RC node to express support for PRI.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 -This should be updated through acpica git. I have sent out a pull
  request for the same here,
  https://github.com/acpica/acpica/pull/638

Please help to review.
---
 include/acpi/actbl2.h | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index ec66779cb193..274fce7b5c01 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -68,7 +68,7 @@
  * IORT - IO Remapping Table
  *
  * Conforms to "IO Remapping Table System Software on ARM Platforms",
- * Document number: ARM DEN 0049D, March 2018
+ * Document number: ARM DEN 0049E, June 2020
  *
  ******************************************************************************/
 
@@ -86,7 +86,8 @@ struct acpi_iort_node {
 	u8 type;
 	u16 length;
 	u8 revision;
-	u32 reserved;
+	u16 reserved;
+	u16 identifier;
 	u32 mapping_count;
 	u32 mapping_offset;
 	char node_data[1];
@@ -100,7 +101,8 @@ enum acpi_iort_node_type {
 	ACPI_IORT_NODE_PCI_ROOT_COMPLEX = 0x02,
 	ACPI_IORT_NODE_SMMU = 0x03,
 	ACPI_IORT_NODE_SMMU_V3 = 0x04,
-	ACPI_IORT_NODE_PMCG = 0x05
+	ACPI_IORT_NODE_PMCG = 0x05,
+	ACPI_IORT_NODE_RMR = 0x06,
 };
 
 struct acpi_iort_id_mapping {
@@ -167,10 +169,10 @@ struct acpi_iort_root_complex {
 	u8 reserved[3];		/* Reserved, must be zero */
 };
 
-/* Values for ats_attribute field above */
+/* Masks for ats_attribute field above */
 
-#define ACPI_IORT_ATS_SUPPORTED         0x00000001	/* The root complex supports ATS */
-#define ACPI_IORT_ATS_UNSUPPORTED       0x00000000	/* The root complex doesn't support ATS */
+#define ACPI_IORT_ATS_SUPPORTED         (1)	/* The root complex supports ATS */
+#define ACPI_IORT_PRI_SUPPORTED         (1<<1)	/* The root complex supports PRI */
 
 struct acpi_iort_smmu {
 	u64 base_address;	/* SMMU base address */
@@ -241,6 +243,17 @@ struct acpi_iort_pmcg {
 	u64 page1_base_address;
 };
 
+struct acpi_iort_rmr {
+	u32 rmr_count;
+	u32 rmr_offset;
+};
+
+struct acpi_iort_rmr_desc {
+	u64 base_address;
+	u64 length;
+	u32 reserved;
+};
+
 /*******************************************************************************
  *
  * IVRS - I/O Virtualization Reporting Structure
-- 
2.17.1

[RFC PATCH 2/4] ACPI/IORT: Add support for RMR node parsing

[Shameer Kolothum]

Add support for parsing RMR node information from ACPI.
Find associated stream ids and smmu node info from the
RMR node and populate a linked list with RMR memory
descriptors.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 drivers/acpi/arm64/iort.c | 119 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 118 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 9929ff50c0c0..b32cd53cca08 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -40,6 +40,25 @@ struct iort_fwnode {
 static LIST_HEAD(iort_fwnode_list);
 static DEFINE_SPINLOCK(iort_fwnode_lock);
 
+struct iort_rmr_id {
+	u32  sid;
+	struct acpi_iort_node *smmu;
+};
+
+/*
+ * One entry for IORT RMR.
+ */
+struct iort_rmr_entry {
+	struct list_head list;
+
+	unsigned int rmr_ids_num;
+	struct iort_rmr_id *rmr_ids;
+
+	struct acpi_iort_rmr_desc *rmr_desc;
+};
+
+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 +412,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;
 		}
@@ -1647,6 +1667,100 @@ 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 int iort_rmr_desc_valid(struct acpi_iort_rmr_desc *desc)
+{
+	struct iort_rmr_entry *e;
+	u64 end, start = desc->base_address, length = desc->length;
+
+	if ((!IS_ALIGNED(start, SZ_64K)) || (length % SZ_64K != 0))
+		return -EINVAL;
+
+	end = start + length - 1;
+
+	/* Check for address overlap */
+	list_for_each_entry(e, &iort_rmr_list, list) {
+		u64 e_start = e->rmr_desc->base_address;
+		u64 e_end = e_start + e->rmr_desc->length - 1;
+
+		if (start <= e_end && end >= e_start)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int __init iort_parse_rmr(struct acpi_iort_node *iort_node)
+{
+	struct iort_rmr_id *rmr_ids, *ids;
+	struct iort_rmr_entry *e;
+	struct acpi_iort_rmr *rmr;
+	struct acpi_iort_rmr_desc *rmr_desc;
+	u32 map_count = iort_node->mapping_count;
+	int i, ret = 0, desc_count = 0;
+
+	if (iort_node->type != ACPI_IORT_NODE_RMR)
+		return 0;
+
+	if (!iort_node->mapping_offset || !map_count) {
+		pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
+		       iort_node);
+		return -EINVAL;
+	}
+
+	rmr_ids = kmalloc(sizeof(*rmr_ids) * map_count, GFP_KERNEL);
+	if (!rmr_ids)
+		return -ENOMEM;
+
+	/* Retrieve associated smmu and stream id */
+	ids = rmr_ids;
+	for (i = 0; i < map_count; i++, ids++) {
+		ids->smmu = iort_node_get_id(iort_node, &ids->sid, i);
+		if (!ids->smmu) {
+			pr_err(FW_BUG "Invalid SMMU reference, skipping RMR node %p\n",
+			       iort_node);
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
+	/* 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);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, iort_node,
+				rmr->rmr_offset);
+
+	for (i = 0; i < rmr->rmr_count; i++, rmr_desc++) {
+		ret = iort_rmr_desc_valid(rmr_desc);
+		if (ret) {
+			pr_err(FW_BUG "Invalid RMR descriptor[%d] for node %p, skipping...\n",
+			       i, iort_node);
+			goto out;
+		}
+
+		e = kmalloc(sizeof(*e), GFP_KERNEL);
+		if (!e)
+			goto out;
+		e->rmr_ids_num = map_count;
+		e->rmr_ids = rmr_ids;
+		e->rmr_desc = rmr_desc;
+
+		list_add_tail(&e->list, &iort_rmr_list);
+		desc_count++;
+	}
+
+	return 0;
+
+out:
+	if (!desc_count)
+		kfree(rmr_ids);
+	return ret;
+}
 
 static void __init iort_init_platform_devices(void)
 {
@@ -1676,6 +1790,9 @@ static void __init iort_init_platform_devices(void)
 
 		iort_enable_acs(iort_node);
 
+		if (iort_table->revision == 1)
+			iort_parse_rmr(iort_node);
+
 		ops = iort_get_dev_cfg(iort_node);
 		if (ops) {
 			fwnode = acpi_alloc_fwnode_static();
-- 
2.17.1

[RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions reservation helper

[Shameer Kolothum]

Add a helper function that retrieves RMR memory descriptors
associated with a given endpoint dev. These memory regions
should have a unity mapping in the SMMU. So reserve them as
IOMMU_RESV_DIRECT.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 drivers/acpi/arm64/iort.c | 56 +++++++++++++++++++++++++++++++++++++++
 include/linux/acpi_iort.h |  4 +++
 2 files changed, 60 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index b32cd53cca08..c0700149e60b 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -842,6 +842,60 @@ static inline int iort_add_device_replay(struct device *dev)
 	return err;
 }
 
+static bool iort_dev_has_rmr(struct device *dev, struct iort_rmr_entry *e)
+{
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	struct acpi_iort_node *iommu;
+	struct iort_rmr_id *rmr_ids = e->rmr_ids;
+	int i, j;
+
+	iommu = iort_get_iort_node(fwspec->iommu_fwnode);
+
+	for (i = 0; i < e->rmr_ids_num; i++, rmr_ids++) {
+		for (j = 0; j < fwspec->num_ids; j++) {
+			if (rmr_ids->sid == fwspec->ids[j] &&
+			    rmr_ids->smmu == iommu)
+				return true;
+		}
+	}
+
+	return false;
+}
+
+/**
+ * iort_dev_rmr_get_resv_regions - RMR Reserved region driver helper
+ * @dev: Device from iommu_get_resv_regions()
+ * @head: Reserved region list from iommu_get_resv_regions()
+ *
+ * Returns: 0 on success, <0 failure
+ */
+int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head *head)
+{
+	struct iort_rmr_entry *e;
+
+	list_for_each_entry(e, &iort_rmr_list, list) {
+		struct iommu_resv_region *region;
+		struct acpi_iort_rmr_desc *rmr;
+		int prot = IOMMU_READ | IOMMU_WRITE |
+			   IOMMU_NOEXEC | IOMMU_MMIO;
+
+		if (!iort_dev_has_rmr(dev, e))
+			continue;
+
+		rmr = e->rmr_desc;
+		region = iommu_alloc_resv_region(rmr->base_address,
+						 rmr->length, prot,
+						 IOMMU_RESV_DIRECT);
+		if (!region) {
+			dev_err(dev, "Out of memory allocating RMR regions\n");
+			return -ENOMEM;
+		}
+		list_add_tail(&region->list, head);
+	}
+
+	return 0;
+}
+
 /**
  * iort_iommu_msi_get_resv_regions - Reserved region driver helper
  * @dev: Device from iommu_get_resv_regions()
@@ -1112,6 +1166,8 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
 const struct iommu_ops *iort_iommu_configure_id(struct device *dev,
 						const u32 *input_id)
 { return NULL; }
+int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head *head)
+{ return 0; }
 #endif
 
 static int nc_dma_get_range(struct device *dev, u64 *size)
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index 20a32120bb88..6dd89faf340c 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -38,6 +38,7 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *size);
 const struct iommu_ops *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);
+int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head *head);
 #else
 static inline void acpi_iort_init(void) { }
 static inline u32 iort_msi_map_id(struct device *dev, u32 id)
@@ -55,6 +56,9 @@ static inline const struct iommu_ops *iort_iommu_configure_id(
 static inline
 int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
 { return 0; }
+static inline
+int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head *head)
+{ return 0; }
 #endif
 
 #endif /* __ACPI_IORT_H__ */
-- 
2.17.1

[RFC PATCH 4/4] iommu/dma: Reserve any RMR regions associated with a dev

[Shameer Kolothum]

Added support to get RMR regions associated with a dev
reserved so that there is a unity mapping for them in
SMMU.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/dma-iommu.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 0cbcd3fc3e7e..31eec16f2af8 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -153,15 +153,19 @@ EXPORT_SYMBOL(iommu_put_dma_cookie);
  * @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.
+ *  - ACPI IORT RMR memory range reservations that require a unity mapping
+ *    in the SMMU for a given endpoint device.
  */
 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);
+		iort_dev_rmr_get_resv_regions(dev, list);
+	}
 
 }
 EXPORT_SYMBOL(iommu_dma_get_resv_regions);
-- 
2.17.1

Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node

[Steven Price]

On 27/10/2020 11:26, Shameer Kolothum wrote:
> The series adds support to IORT RMR nodes specified in IORT
> Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory
> ranges that are used by endpoints and require a unity mapping
> in SMMU.

Hi Shameer,

I've also been taking a look at RMR, and Sami is helping me get set up 
so that I can do some testing. We're hoping to be able to test an EFI 
framebuffer or splash screen - which has the added complication of the 
unity mapping becoming redundant if a native display driver takes over 
the display controller.

I've looked through your series and the code looks correct to me. 
Hopefully I'll be able to give it some testing soon.

Thanks,

Steve

> 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.
> 
> RFC because, Patch #1 is to update the actbl2.h and should be done
> through acpica update. I have send out a pull request[1] for that.
> 
> Tests:
> 
> With a UEFI, that reports the RMR for the dev,
> ....
> [16F0h 5872   1]                         Type : 06
> [16F1h 5873   2]                       Length : 007C
> [16F3h 5875   1]                     Revision : 00
> [1038h 0056   2]                     Reserved : 00000000
> [1038h 0056   2]                   Identifier : 00000000
> [16F8h 5880   4]                Mapping Count : 00000001
> [16FCh 5884   4]               Mapping Offset : 00000040
> 
> [1700h 5888   4]    Number of RMR Descriptors : 00000002
> [1704h 5892   4]        RMR Descriptor Offset : 00000018
> 
> [1708h 5896   8]          Base Address of RMR : 0000E6400000
> [1710h 5904   8]                Length of RMR : 000000100000
> [1718h 5912   4]                     Reserved : 00000000
> 
> [171Ch 5916   8]          Base Address of RMR : 0000000027B00000
> [1724h 5924   8]                Length of RMR : 0000000000C00000
> [172Ch 5932   4]                     Reserved : 00000000
> 
> [1730h 5936   4]                   Input base : 00000000
> [1734h 5940   4]                     ID Count : 00000001
> [1738h 5944   4]                  Output Base : 00000003
> [173Ch 5948   4]             Output Reference : 00000064
> [1740h 5952   4]        Flags (decoded below) : 00000001
>                                 Single Mapping : 1
> ...
> 
> Without the series the RAID controller initilization fails as
> below,
> 
> ...
> [   12.631117] megaraid_sas 0000:03:00.0: FW supports sync cache        : Yes
> [   12.637360] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009
> [   18.776377] megaraid_sas 0000:03:00.0: Init cmd return status FAILED for SCSI host 0
> [   23.019383] megaraid_sas 0000:03:00.0: Waiting for FW to come to ready state
> [  106.684281] megaraid_sas 0000:03:00.0: FW in FAULT state, Fault code:0x10000 subcode:0x0 func:megasas_transition_to_ready
> [  106.695186] megaraid_sas 0000:03:00.0: System Register set:
> [  106.889787] megaraid_sas 0000:03:00.0: Failed to transition controller to ready for scsi0.
> [  106.910475] megaraid_sas 0000:03:00.0: Failed from megasas_init_fw 6407
> estuary:/$
> 
> With the series, now the kernel has direct mapping for the dev as
> below,
> 
> estuary:/$ cat /sys/kernel/iommu_groups/0/reserved_regions
> 0x0000000008000000 0x00000000080fffff msi
> 0x0000000027b00000 0x00000000286fffff direct
> 0x00000000e6400000 0x00000000e64fffff direct
> estuary:/$
> 
> ....
> [   12.254318] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009
> [   12.739089] megaraid_sas 0000:03:00.0: FW provided supportMaxExtLDs: 0      max_lds: 32
> [   12.746628] megaraid_sas 0000:03:00.0: controller type       : iMR(0MB)
> [   12.752694] megaraid_sas 0000:03:00.0: Online Controller Reset(OCR)  : Enabled
> [   12.759798] megaraid_sas 0000:03:00.0: Secure JBOD support   : Yes
> [   12.765778] megaraid_sas 0000:03:00.0: NVMe passthru support : Yes
> [   12.771931] megaraid_sas 0000:03:00.0: FW provided TM TaskAbort/Reset timeou: 6 secs/60 secs
> [   12.780503] megaraid_sas 0000:03:00.0: JBOD sequence map support     : Yes
> [   12.787000] megaraid_sas 0000:03:00.0: PCI Lane Margining support    : No
> [   12.819179] megaraid_sas 0000:03:00.0: NVME page size        : (4096)
> [   12.825672] megaraid_sas 0000:03:00.0: megasas_enable_intr_fusion is called outbound_intr_mask:0x40000000
> [   12.835199] megaraid_sas 0000:03:00.0: INIT adapter done
> [   12.873932] megaraid_sas 0000:03:00.0: pci id                : (0x1000)/(0x0017)/(0x19e5)/(0xd213)
> [   12.881644] megaraid_sas 0000:03:00.0: unevenspan support    : no
> [   12.887451] megaraid_sas 0000:03:00.0: firmware crash dump   : no
> [   12.893344] megaraid_sas 0000:03:00.0: JBOD sequence map     : enabled
> 
> RAID controller init is now success and can detect the drives
> attached as well.
> 
> Thanks,
> Shameer
> 
> [0]. https://developer.arm.com/documentation/den0049/latest/
> [1]. https://github.com/acpica/acpica/pull/638
> 
> Shameer Kolothum (4):
>    ACPICA: IORT: Update for revision E
>    ACPI/IORT: Add support for RMR node parsing
>    ACPI/IORT: Add RMR memory regions reservation helper
>    iommu/dma: Reserve any RMR regions associated with a dev
> 
>   drivers/acpi/arm64/iort.c | 175 +++++++++++++++++++++++++++++++++++++-
>   drivers/iommu/dma-iommu.c |  12 ++-
>   include/acpi/actbl2.h     |  25 ++++--
>   include/linux/acpi_iort.h |   4 +
>   4 files changed, 205 insertions(+), 11 deletions(-)
> 

RE: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node

[Shameerali Kolothum Thodi]

Hi Steve,

> -----Original Message-----
> From: Steven Price [mailto:steven.price@arm.com]
> Sent: 28 October 2020 16:44
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> iommu@lists.linux-foundation.org; devel@acpica.org
> Cc: lorenzo.pieralisi@arm.com; joro@8bytes.org; Jonathan Cameron
> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>;
> Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; robin.murphy@arm.com;
> wanghuiqiang <wanghuiqiang@huawei.com>; Sami Mujawar
> <Sami.Mujawar@arm.com>
> Subject: Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node
> 
> On 27/10/2020 11:26, Shameer Kolothum wrote:
> > The series adds support to IORT RMR nodes specified in IORT
> > Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory
> > ranges that are used by endpoints and require a unity mapping
> > in SMMU.
> 
> Hi Shameer,
> 
> I've also been taking a look at RMR, and Sami is helping me get set up
> so that I can do some testing. We're hoping to be able to test an EFI
> framebuffer or splash screen - which has the added complication of the
> unity mapping becoming redundant if a native display driver takes over
> the display controller.
> 
> I've looked through your series and the code looks correct to me.

Thanks for taking a look and the details.

> Hopefully I'll be able to give it some testing soon.

Cool. Please update once you get a chance run the tests.

Thanks,
Shameer
 
> Thanks,
> 
> Steve
> 
> > 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.
> >
> > RFC because, Patch #1 is to update the actbl2.h and should be done
> > through acpica update. I have send out a pull request[1] for that.
> >
> > Tests:
> >
> > With a UEFI, that reports the RMR for the dev,
> > ....
> > [16F0h 5872   1]                         Type : 06
> > [16F1h 5873   2]                       Length : 007C
> > [16F3h 5875   1]                     Revision : 00
> > [1038h 0056   2]                     Reserved : 00000000
> > [1038h 0056   2]                   Identifier : 00000000
> > [16F8h 5880   4]                Mapping Count : 00000001
> > [16FCh 5884   4]               Mapping Offset : 00000040
> >
> > [1700h 5888   4]    Number of RMR Descriptors : 00000002
> > [1704h 5892   4]        RMR Descriptor Offset : 00000018
> >
> > [1708h 5896   8]          Base Address of RMR : 0000E6400000
> > [1710h 5904   8]                Length of RMR : 000000100000
> > [1718h 5912   4]                     Reserved : 00000000
> >
> > [171Ch 5916   8]          Base Address of RMR : 0000000027B00000
> > [1724h 5924   8]                Length of RMR : 0000000000C00000
> > [172Ch 5932   4]                     Reserved : 00000000
> >
> > [1730h 5936   4]                   Input base : 00000000
> > [1734h 5940   4]                     ID Count : 00000001
> > [1738h 5944   4]                  Output Base : 00000003
> > [173Ch 5948   4]             Output Reference : 00000064
> > [1740h 5952   4]        Flags (decoded below) : 00000001
> >                                 Single Mapping : 1
> > ...
> >
> > Without the series the RAID controller initilization fails as
> > below,
> >
> > ...
> > [   12.631117] megaraid_sas 0000:03:00.0: FW supports sync
> cache        : Yes
> > [   12.637360] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is
> called outbound_intr_mask:0x40000009
> > [   18.776377] megaraid_sas 0000:03:00.0: Init cmd return status FAILED
> for SCSI host 0
> > [   23.019383] megaraid_sas 0000:03:00.0: Waiting for FW to come to
> ready state
> > [  106.684281] megaraid_sas 0000:03:00.0: FW in FAULT state, Fault
> code:0x10000 subcode:0x0 func:megasas_transition_to_ready
> > [  106.695186] megaraid_sas 0000:03:00.0: System Register set:
> > [  106.889787] megaraid_sas 0000:03:00.0: Failed to transition controller to
> ready for scsi0.
> > [  106.910475] megaraid_sas 0000:03:00.0: Failed from megasas_init_fw
> 6407
> > estuary:/$
> >
> > With the series, now the kernel has direct mapping for the dev as
> > below,
> >
> > estuary:/$ cat /sys/kernel/iommu_groups/0/reserved_regions
> > 0x0000000008000000 0x00000000080fffff msi
> > 0x0000000027b00000 0x00000000286fffff direct
> > 0x00000000e6400000 0x00000000e64fffff direct
> > estuary:/$
> >
> > ....
> > [   12.254318] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is
> called outbound_intr_mask:0x40000009
> > [   12.739089] megaraid_sas 0000:03:00.0: FW provided supportMaxExtLDs:
> 0      max_lds: 32
> > [   12.746628] megaraid_sas 0000:03:00.0: controller type       :
> iMR(0MB)
> > [   12.752694] megaraid_sas 0000:03:00.0: Online Controller Reset(OCR)  :
> Enabled
> > [   12.759798] megaraid_sas 0000:03:00.0: Secure JBOD support   : Yes
> > [   12.765778] megaraid_sas 0000:03:00.0: NVMe passthru support : Yes
> > [   12.771931] megaraid_sas 0000:03:00.0: FW provided TM
> TaskAbort/Reset timeou: 6 secs/60 secs
> > [   12.780503] megaraid_sas 0000:03:00.0: JBOD sequence map
> support     : Yes
> > [   12.787000] megaraid_sas 0000:03:00.0: PCI Lane Margining
> support    : No
> > [   12.819179] megaraid_sas 0000:03:00.0: NVME page size        :
> (4096)
> > [   12.825672] megaraid_sas 0000:03:00.0: megasas_enable_intr_fusion is
> called outbound_intr_mask:0x40000000
> > [   12.835199] megaraid_sas 0000:03:00.0: INIT adapter done
> > [   12.873932] megaraid_sas 0000:03:00.0: pci id                :
> (0x1000)/(0x0017)/(0x19e5)/(0xd213)
> > [   12.881644] megaraid_sas 0000:03:00.0: unevenspan support    : no
> > [   12.887451] megaraid_sas 0000:03:00.0: firmware crash dump   : no
> > [   12.893344] megaraid_sas 0000:03:00.0: JBOD sequence map     :
> enabled
> >
> > RAID controller init is now success and can detect the drives
> > attached as well.
> >
> > Thanks,
> > Shameer
> >
> > [0]. https://developer.arm.com/documentation/den0049/latest/
> > [1]. https://github.com/acpica/acpica/pull/638
> >
> > Shameer Kolothum (4):
> >    ACPICA: IORT: Update for revision E
> >    ACPI/IORT: Add support for RMR node parsing
> >    ACPI/IORT: Add RMR memory regions reservation helper
> >    iommu/dma: Reserve any RMR regions associated with a dev
> >
> >   drivers/acpi/arm64/iort.c | 175
> +++++++++++++++++++++++++++++++++++++-
> >   drivers/iommu/dma-iommu.c |  12 ++-
> >   include/acpi/actbl2.h     |  25 ++++--
> >   include/linux/acpi_iort.h |   4 +
> >   4 files changed, 205 insertions(+), 11 deletions(-)
> >

RE: [Devel] [RFC PATCH 1/4] ACPICA: IORT: Update for revision E

[Shameerali Kolothum Thodi]

Hi Erik,

[Adding back the original MLs + CC list]

> -----Original Message-----
> From: Kaneda, Erik [mailto:erik.kaneda@intel.com]
> Sent: 28 October 2020 18:20
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> Moore, Robert <robert.moore@intel.com>; Wysocki, Rafael J
> <rafael.j.wysocki@intel.com>
> Subject: RE: [Devel] [RFC PATCH 1/4] ACPICA: IORT: Update for revision E
> 
> 
> 
> > -----Original Message-----
> > From: Shameerali Kolothum Thodi
> > <shameerali.kolothum.thodi@huawei.com>
> > Sent: Wednesday, October 28, 2020 1:15 AM
> > To: Moore, Robert <robert.moore@intel.com>
> > Cc: Kaneda, Erik <erik.kaneda@intel.com>
> > Subject: RE: [Devel] [RFC PATCH 1/4] ACPICA: IORT: Update for revision
> > E
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Moore, Robert [mailto:robert.moore@intel.com]
> > > Sent: 27 October 2020 21:46
> > > To: Shameerali Kolothum Thodi
> > <shameerali.kolothum.thodi@huawei.com>
> > > Cc: Kaneda, Erik <erik.kaneda@intel.com>
> > > Subject: RE: [Devel] [RFC PATCH 1/4] ACPICA: IORT: Update for
> > > revision E
> > >
> > > This patch will need to be ported to the native ACPICA format.
> >
> > Thanks for your feedback. Sorry that, I am not familiar with the process here.
> > As mentioned
> > below, I thought a pull request via acpica git is enough, which you
> > can find here,
> >
> > https://github.com/acpica/acpica/pull/638
> >
> > The above pull includes this header file change + other changes
> > related to RMR for tools.
> >
> > Please let me know what else needs to be done when you say "native
> > format".
> 
> Oh ok, thanks for the pull request. You're submitting this as an RFC for linux. Do
> you expect the IORT definition to change throughout linux's RFC period? If you
> don't plan on changing the IORT definition, we can look at the pull request.

The IORT revision E document is published here,
https://developer.arm.com/documentation/den0049/latest/
and I don’t think it will change now (But ARM folks can confirm this).

I submitted this as an RFC, because of the very dependency on this actbl2.h
header updates and this being not available in kernel. 

> Once we merge it, I'll submit the ACPICA patches to Rafael after we do a
> release.

Ok. Thanks for the details. Looking forward to getting this merged to kernel soon :)

Thanks,
Shameer
 
> Erik
> >
> > Thanks,
> > Shameer
> >
> >
> > >
> > >
> > > -----Original Message-----
> > > From: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > > Sent: Tuesday, October 27, 2020 4:27 AM
> > > To: linux-arm-kernel@lists.infradead.org;
> > > linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org;
> > > devel@acpica.org
> > > Cc: linuxarm@huawei.com; lorenzo.pieralisi@arm.com; joro@8bytes.org;
> > > robin.murphy@arm.com; wanghuiqiang@huawei.com;
> > > jonathan.cameron@huawei.com
> > > Subject: [Devel] [RFC PATCH 1/4] ACPICA: IORT: Update for revision E
> > >
> > > IORT revision E contains a few additions like,
> > >     -Added an identifier field in the node descriptors to aid table
> > >      cross-referencing.
> > >     -Introduced the Reserved Memory Range(RMR) node. This is used
> > >      to describe memory ranges that are used by endpoints and
> > > requires
> > >      a unity mapping in SMMU.
> > >     -Introduced a flag in the RC node to express support for PRI.
> > >
> > > Signed-off-by: Shameer Kolothum
> > <shameerali.kolothum.thodi@huawei.com>
> > > ---
> > >  -This should be updated through acpica git. I have sent out a pull
> > >   request for the same here,
> > >   https://github.com/acpica/acpica/pull/638
> > >
> > > Please help to review.
> > > ---
> > >  include/acpi/actbl2.h | 25 +++++++++++++++++++------
> > >  1 file changed, 19 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h index
> > > ec66779cb193..274fce7b5c01 100644
> > > --- a/include/acpi/actbl2.h
> > > +++ b/include/acpi/actbl2.h
> > > @@ -68,7 +68,7 @@
> > >   * IORT - IO Remapping Table
> > >   *
> > >   * Conforms to "IO Remapping Table System Software on ARM
> > > Platforms",
> > > - * Document number: ARM DEN 0049D, March 2018
> > > + * Document number: ARM DEN 0049E, June 2020
> > >   *
> > >
> > >
> > **********************************************************
> > ******
> > > **************/
> > >
> > > @@ -86,7 +86,8 @@ struct acpi_iort_node {
> > >  	u8 type;
> > >  	u16 length;
> > >  	u8 revision;
> > > -	u32 reserved;
> > > +	u16 reserved;
> > > +	u16 identifier;
> > >  	u32 mapping_count;
> > >  	u32 mapping_offset;
> > >  	char node_data[1];
> > > @@ -100,7 +101,8 @@ enum acpi_iort_node_type {
> > >  	ACPI_IORT_NODE_PCI_ROOT_COMPLEX = 0x02,
> > >  	ACPI_IORT_NODE_SMMU = 0x03,
> > >  	ACPI_IORT_NODE_SMMU_V3 = 0x04,
> > > -	ACPI_IORT_NODE_PMCG = 0x05
> > > +	ACPI_IORT_NODE_PMCG = 0x05,
> > > +	ACPI_IORT_NODE_RMR = 0x06,
> > >  };
> > >
> > >  struct acpi_iort_id_mapping {
> > > @@ -167,10 +169,10 @@ struct acpi_iort_root_complex {
> > >  	u8 reserved[3];		/* Reserved, must be zero */
> > >  };
> > >
> > > -/* Values for ats_attribute field above */
> > > +/* Masks for ats_attribute field above */
> > >
> > > -#define ACPI_IORT_ATS_SUPPORTED         0x00000001	/* The
> root
> > > complex supports ATS */
> > > -#define ACPI_IORT_ATS_UNSUPPORTED       0x00000000	/* The
> root
> > > complex doesn't support ATS */
> > > +#define ACPI_IORT_ATS_SUPPORTED         (1)	/* The root complex
> > > supports ATS */
> > > +#define ACPI_IORT_PRI_SUPPORTED         (1<<1)	/* The root
> complex
> > > supports PRI */
> > >
> > >  struct acpi_iort_smmu {
> > >  	u64 base_address;	/* SMMU base address */
> > > @@ -241,6 +243,17 @@ struct acpi_iort_pmcg {
> > >  	u64 page1_base_address;
> > >  };
> > >
> > > +struct acpi_iort_rmr {
> > > +	u32 rmr_count;
> > > +	u32 rmr_offset;
> > > +};
> > > +
> > > +struct acpi_iort_rmr_desc {
> > > +	u64 base_address;
> > > +	u64 length;
> > > +	u32 reserved;
> > > +};
> > > +
> > >
> > >
> > /**********************************************************
> > *****
> > > ****************
> > >   *
> > >   * IVRS - I/O Virtualization Reporting Structure
> > > --
> > > 2.17.1
> > > _______________________________________________
> > > Devel mailing list -- devel@acpica.org To unsubscribe send an email
> > > to
> > > devel-
> > leave@acpica.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name
> > > )s

Re: [Devel] [RFC PATCH 2/4] ACPI/IORT: Add support for RMR node parsing

[David E. Box]

Hi,

On Tue, 2020-10-27 at 11:26 +0000, Shameer Kolothum wrote:

...

> @@ -1647,6 +1667,100 @@ 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 int iort_rmr_desc_valid(struct acpi_iort_rmr_desc *desc)
> +{
> +	struct iort_rmr_entry *e;
> +	u64 end, start = desc->base_address, length = desc->length;
> +
> +	if ((!IS_ALIGNED(start, SZ_64K)) || (length % SZ_64K != 0))

You could just do:

if ((!IS_ALIGNED(start, SZ_64K)) || (length % SZ_64K))

David

Re: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions reservation helper

[Robin Murphy]

On 2020-10-27 11:26, Shameer Kolothum wrote:
> Add a helper function that retrieves RMR memory descriptors
> associated with a given endpoint dev. These memory regions
> should have a unity mapping in the SMMU. So reserve them as
> IOMMU_RESV_DIRECT.

As a general observation, we also need a way into this that isn't from 
the perspective of endpoint devices. With SMMUv3 we need to know all the 
active stream IDs relevant to a given SMMU instance at probe time, so 
that we can set up some kind of valid stream table entries *before* 
enabling the SMMU in the reset routine. Otherwise we're just going to 
kill ongoing traffic (e.g. EFI GOP) with C_BAD_STE long before we ever 
start adding devices and worrying about reserved regions for them. 
Similarly for the initial SMR/S2CR state on SMMUv2 with disable_bypass.

Robin.

> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>   drivers/acpi/arm64/iort.c | 56 +++++++++++++++++++++++++++++++++++++++
>   include/linux/acpi_iort.h |  4 +++
>   2 files changed, 60 insertions(+)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index b32cd53cca08..c0700149e60b 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -842,6 +842,60 @@ static inline int iort_add_device_replay(struct device *dev)
>   	return err;
>   }
>   
> +static bool iort_dev_has_rmr(struct device *dev, struct iort_rmr_entry *e)
> +{
> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +	struct acpi_iort_node *iommu;
> +	struct iort_rmr_id *rmr_ids = e->rmr_ids;
> +	int i, j;
> +
> +	iommu = iort_get_iort_node(fwspec->iommu_fwnode);
> +
> +	for (i = 0; i < e->rmr_ids_num; i++, rmr_ids++) {
> +		for (j = 0; j < fwspec->num_ids; j++) {
> +			if (rmr_ids->sid == fwspec->ids[j] &&
> +			    rmr_ids->smmu == iommu)
> +				return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
> +/**
> + * iort_dev_rmr_get_resv_regions - RMR Reserved region driver helper
> + * @dev: Device from iommu_get_resv_regions()
> + * @head: Reserved region list from iommu_get_resv_regions()
> + *
> + * Returns: 0 on success, <0 failure
> + */
> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head *head)
> +{
> +	struct iort_rmr_entry *e;
> +
> +	list_for_each_entry(e, &iort_rmr_list, list) {
> +		struct iommu_resv_region *region;
> +		struct acpi_iort_rmr_desc *rmr;
> +		int prot = IOMMU_READ | IOMMU_WRITE |
> +			   IOMMU_NOEXEC | IOMMU_MMIO;
> +
> +		if (!iort_dev_has_rmr(dev, e))
> +			continue;
> +
> +		rmr = e->rmr_desc;
> +		region = iommu_alloc_resv_region(rmr->base_address,
> +						 rmr->length, prot,
> +						 IOMMU_RESV_DIRECT);
> +		if (!region) {
> +			dev_err(dev, "Out of memory allocating RMR regions\n");
> +			return -ENOMEM;
> +		}
> +		list_add_tail(&region->list, head);
> +	}
> +
> +	return 0;
> +}
> +
>   /**
>    * iort_iommu_msi_get_resv_regions - Reserved region driver helper
>    * @dev: Device from iommu_get_resv_regions()
> @@ -1112,6 +1166,8 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
>   const struct iommu_ops *iort_iommu_configure_id(struct device *dev,
>   						const u32 *input_id)
>   { return NULL; }
> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head *head)
> +{ return 0; }
>   #endif
>   
>   static int nc_dma_get_range(struct device *dev, u64 *size)
> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
> index 20a32120bb88..6dd89faf340c 100644
> --- a/include/linux/acpi_iort.h
> +++ b/include/linux/acpi_iort.h
> @@ -38,6 +38,7 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *size);
>   const struct iommu_ops *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);
> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head *head);
>   #else
>   static inline void acpi_iort_init(void) { }
>   static inline u32 iort_msi_map_id(struct device *dev, u32 id)
> @@ -55,6 +56,9 @@ static inline const struct iommu_ops *iort_iommu_configure_id(
>   static inline
>   int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head)
>   { return 0; }
> +static inline
> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head *head)
> +{ return 0; }
>   #endif
>   
>   #endif /* __ACPI_IORT_H__ */
> 

RE: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions reservation helper

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: 05 November 2020 18:04
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> iommu@lists.linux-foundation.org; devel@acpica.org
> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
> joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun
> (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: Re: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions reservation
> helper
> 
> On 2020-10-27 11:26, Shameer Kolothum wrote:
> > Add a helper function that retrieves RMR memory descriptors
> > associated with a given endpoint dev. These memory regions
> > should have a unity mapping in the SMMU. So reserve them as
> > IOMMU_RESV_DIRECT.
> 
> As a general observation, we also need a way into this that isn't from
> the perspective of endpoint devices. With SMMUv3 we need to know all the
> active stream IDs relevant to a given SMMU instance at probe time, so
> that we can set up some kind of valid stream table entries *before*
> enabling the SMMU in the reset routine.

So I guess, the idea is to provide an interface here to retrieve those active
stream Ids? The problem is, at present(AFAICS), RMR doesn’t have any
means to specify such devices. 

 Otherwise we're just going to
> kill ongoing traffic (e.g. EFI GOP) with C_BAD_STE long before we ever
> start adding devices and worrying about reserved regions for them.
> Similarly for the initial SMR/S2CR state on SMMUv2 with disable_bypass.

Ok. I see the discussion here,
https://lore.kernel.org/linux-iommu/484b9e90-7395-6161-577c-4d3f3716997e@arm.com/

From what I gather, the plan is to setup a default IDENTITY_DOMAIN for
devices that have live stream going on during boot/SMMU probe time. 

Thanks,
Shameer

> Robin.
> 
> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > ---
> >   drivers/acpi/arm64/iort.c | 56
> +++++++++++++++++++++++++++++++++++++++
> >   include/linux/acpi_iort.h |  4 +++
> >   2 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index b32cd53cca08..c0700149e60b 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -842,6 +842,60 @@ static inline int iort_add_device_replay(struct
> device *dev)
> >   	return err;
> >   }
> >
> > +static bool iort_dev_has_rmr(struct device *dev, struct iort_rmr_entry *e)
> > +{
> > +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > +	struct acpi_iort_node *iommu;
> > +	struct iort_rmr_id *rmr_ids = e->rmr_ids;
> > +	int i, j;
> > +
> > +	iommu = iort_get_iort_node(fwspec->iommu_fwnode);
> > +
> > +	for (i = 0; i < e->rmr_ids_num; i++, rmr_ids++) {
> > +		for (j = 0; j < fwspec->num_ids; j++) {
> > +			if (rmr_ids->sid == fwspec->ids[j] &&
> > +			    rmr_ids->smmu == iommu)
> > +				return true;
> > +		}
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +/**
> > + * iort_dev_rmr_get_resv_regions - RMR Reserved region driver helper
> > + * @dev: Device from iommu_get_resv_regions()
> > + * @head: Reserved region list from iommu_get_resv_regions()
> > + *
> > + * Returns: 0 on success, <0 failure
> > + */
> > +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
> *head)
> > +{
> > +	struct iort_rmr_entry *e;
> > +
> > +	list_for_each_entry(e, &iort_rmr_list, list) {
> > +		struct iommu_resv_region *region;
> > +		struct acpi_iort_rmr_desc *rmr;
> > +		int prot = IOMMU_READ | IOMMU_WRITE |
> > +			   IOMMU_NOEXEC | IOMMU_MMIO;
> > +
> > +		if (!iort_dev_has_rmr(dev, e))
> > +			continue;
> > +
> > +		rmr = e->rmr_desc;
> > +		region = iommu_alloc_resv_region(rmr->base_address,
> > +						 rmr->length, prot,
> > +						 IOMMU_RESV_DIRECT);
> > +		if (!region) {
> > +			dev_err(dev, "Out of memory allocating RMR regions\n");
> > +			return -ENOMEM;
> > +		}
> > +		list_add_tail(&region->list, head);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >   /**
> >    * iort_iommu_msi_get_resv_regions - Reserved region driver helper
> >    * @dev: Device from iommu_get_resv_regions()
> > @@ -1112,6 +1166,8 @@ int iort_iommu_msi_get_resv_regions(struct
> device *dev, struct list_head *head)
> >   const struct iommu_ops *iort_iommu_configure_id(struct device *dev,
> >   						const u32 *input_id)
> >   { return NULL; }
> > +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
> *head)
> > +{ return 0; }
> >   #endif
> >
> >   static int nc_dma_get_range(struct device *dev, u64 *size)
> > diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
> > index 20a32120bb88..6dd89faf340c 100644
> > --- a/include/linux/acpi_iort.h
> > +++ b/include/linux/acpi_iort.h
> > @@ -38,6 +38,7 @@ void iort_dma_setup(struct device *dev, u64
> *dma_addr, u64 *size);
> >   const struct iommu_ops *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);
> > +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
> *head);
> >   #else
> >   static inline void acpi_iort_init(void) { }
> >   static inline u32 iort_msi_map_id(struct device *dev, u32 id)
> > @@ -55,6 +56,9 @@ static inline const struct iommu_ops
> *iort_iommu_configure_id(
> >   static inline
> >   int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head
> *head)
> >   { return 0; }
> > +static inline
> > +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
> *head)
> > +{ return 0; }
> >   #endif
> >
> >   #endif /* __ACPI_IORT_H__ */
> >

Re: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions reservation helper

[Robin Murphy]

On 2020-11-06 08:30, Shameerali Kolothum Thodi wrote:
> 
> 
>> -----Original Message-----
>> From: Robin Murphy [mailto:robin.murphy@arm.com]
>> Sent: 05 November 2020 18:04
>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
>> iommu@lists.linux-foundation.org; devel@acpica.org
>> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
>> joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun
>> (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron
>> <jonathan.cameron@huawei.com>
>> Subject: Re: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions reservation
>> helper
>>
>> On 2020-10-27 11:26, Shameer Kolothum wrote:
>>> Add a helper function that retrieves RMR memory descriptors
>>> associated with a given endpoint dev. These memory regions
>>> should have a unity mapping in the SMMU. So reserve them as
>>> IOMMU_RESV_DIRECT.
>>
>> As a general observation, we also need a way into this that isn't from
>> the perspective of endpoint devices. With SMMUv3 we need to know all the
>> active stream IDs relevant to a given SMMU instance at probe time, so
>> that we can set up some kind of valid stream table entries *before*
>> enabling the SMMU in the reset routine.
> 
> So I guess, the idea is to provide an interface here to retrieve those active
> stream Ids? The problem is, at present(AFAICS), RMR doesn’t have any
> means to specify such devices.

I'm thinking we need to check if any RMRs exist in the IORT, if so find 
the devices that map to them, then resolve those devices' ID mappings. 
In terms of the interface, maybe the better option is to do something 
closer to what intel-iommu does and handle *all* the processing up-front 
- let the IOMMU driver just call into the firmware code once to retrieve 
a complete list of all the relevant abstracted RMR (or DT equivalent) 
information, so it can then resolve both its own early initialisation 
and the later per-device setup without calling back into firmware code 
again.

>   Otherwise we're just going to
>> kill ongoing traffic (e.g. EFI GOP) with C_BAD_STE long before we ever
>> start adding devices and worrying about reserved regions for them.
>> Similarly for the initial SMR/S2CR state on SMMUv2 with disable_bypass.
> 
> Ok. I see the discussion here,
> https://lore.kernel.org/linux-iommu/484b9e90-7395-6161-577c-4d3f3716997e@arm.com/
> 
>  From what I gather, the plan is to setup a default IDENTITY_DOMAIN for
> devices that have live stream going on during boot/SMMU probe time.

Well, whether we use bypass or a temporary translation context for the 
period before we even get to default domains... that's a discussion that 
can wait until we have the ability to set up the STEs at all ;)

Robin.

>>> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
>>> ---
>>>    drivers/acpi/arm64/iort.c | 56
>> +++++++++++++++++++++++++++++++++++++++
>>>    include/linux/acpi_iort.h |  4 +++
>>>    2 files changed, 60 insertions(+)
>>>
>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
>>> index b32cd53cca08..c0700149e60b 100644
>>> --- a/drivers/acpi/arm64/iort.c
>>> +++ b/drivers/acpi/arm64/iort.c
>>> @@ -842,6 +842,60 @@ static inline int iort_add_device_replay(struct
>> device *dev)
>>>    	return err;
>>>    }
>>>
>>> +static bool iort_dev_has_rmr(struct device *dev, struct iort_rmr_entry *e)
>>> +{
>>> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>>> +	struct acpi_iort_node *iommu;
>>> +	struct iort_rmr_id *rmr_ids = e->rmr_ids;
>>> +	int i, j;
>>> +
>>> +	iommu = iort_get_iort_node(fwspec->iommu_fwnode);
>>> +
>>> +	for (i = 0; i < e->rmr_ids_num; i++, rmr_ids++) {
>>> +		for (j = 0; j < fwspec->num_ids; j++) {
>>> +			if (rmr_ids->sid == fwspec->ids[j] &&
>>> +			    rmr_ids->smmu == iommu)
>>> +				return true;
>>> +		}
>>> +	}
>>> +
>>> +	return false;
>>> +}
>>> +
>>> +/**
>>> + * iort_dev_rmr_get_resv_regions - RMR Reserved region driver helper
>>> + * @dev: Device from iommu_get_resv_regions()
>>> + * @head: Reserved region list from iommu_get_resv_regions()
>>> + *
>>> + * Returns: 0 on success, <0 failure
>>> + */
>>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
>> *head)
>>> +{
>>> +	struct iort_rmr_entry *e;
>>> +
>>> +	list_for_each_entry(e, &iort_rmr_list, list) {
>>> +		struct iommu_resv_region *region;
>>> +		struct acpi_iort_rmr_desc *rmr;
>>> +		int prot = IOMMU_READ | IOMMU_WRITE |
>>> +			   IOMMU_NOEXEC | IOMMU_MMIO;
>>> +
>>> +		if (!iort_dev_has_rmr(dev, e))
>>> +			continue;
>>> +
>>> +		rmr = e->rmr_desc;
>>> +		region = iommu_alloc_resv_region(rmr->base_address,
>>> +						 rmr->length, prot,
>>> +						 IOMMU_RESV_DIRECT);
>>> +		if (!region) {
>>> +			dev_err(dev, "Out of memory allocating RMR regions\n");
>>> +			return -ENOMEM;
>>> +		}
>>> +		list_add_tail(&region->list, head);
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    /**
>>>     * iort_iommu_msi_get_resv_regions - Reserved region driver helper
>>>     * @dev: Device from iommu_get_resv_regions()
>>> @@ -1112,6 +1166,8 @@ int iort_iommu_msi_get_resv_regions(struct
>> device *dev, struct list_head *head)
>>>    const struct iommu_ops *iort_iommu_configure_id(struct device *dev,
>>>    						const u32 *input_id)
>>>    { return NULL; }
>>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
>> *head)
>>> +{ return 0; }
>>>    #endif
>>>
>>>    static int nc_dma_get_range(struct device *dev, u64 *size)
>>> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
>>> index 20a32120bb88..6dd89faf340c 100644
>>> --- a/include/linux/acpi_iort.h
>>> +++ b/include/linux/acpi_iort.h
>>> @@ -38,6 +38,7 @@ void iort_dma_setup(struct device *dev, u64
>> *dma_addr, u64 *size);
>>>    const struct iommu_ops *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);
>>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
>> *head);
>>>    #else
>>>    static inline void acpi_iort_init(void) { }
>>>    static inline u32 iort_msi_map_id(struct device *dev, u32 id)
>>> @@ -55,6 +56,9 @@ static inline const struct iommu_ops
>> *iort_iommu_configure_id(
>>>    static inline
>>>    int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head
>> *head)
>>>    { return 0; }
>>> +static inline
>>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
>> *head)
>>> +{ return 0; }
>>>    #endif
>>>
>>>    #endif /* __ACPI_IORT_H__ */
>>>

Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node

[Steven Price]

On 28/10/2020 18:24, Shameerali Kolothum Thodi wrote:
> Hi Steve,
> 
>> -----Original Message-----
>> From: Steven Price [mailto:steven.price@arm.com]
>> Sent: 28 October 2020 16:44
>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
>> iommu@lists.linux-foundation.org; devel@acpica.org
>> Cc: lorenzo.pieralisi@arm.com; joro@8bytes.org; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>;
>> Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; robin.murphy@arm.com;
>> wanghuiqiang <wanghuiqiang@huawei.com>; Sami Mujawar
>> <Sami.Mujawar@arm.com>
>> Subject: Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node
>>
>> On 27/10/2020 11:26, Shameer Kolothum wrote:
>>> The series adds support to IORT RMR nodes specified in IORT
>>> Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory
>>> ranges that are used by endpoints and require a unity mapping
>>> in SMMU.
>>
>> Hi Shameer,
>>
>> I've also been taking a look at RMR, and Sami is helping me get set up
>> so that I can do some testing. We're hoping to be able to test an EFI
>> framebuffer or splash screen - which has the added complication of the
>> unity mapping becoming redundant if a native display driver takes over
>> the display controller.
>>
>> I've looked through your series and the code looks correct to me.
> 
> Thanks for taking a look and the details.
> 
>> Hopefully I'll be able to give it some testing soon.
> 
> Cool. Please update once you get a chance run the tests.

Hi Shameer,

Just to update on this, for the EFI framebuffer use case I hit exactly 
the issue that Robin has mentioned in another thread - the RMR is 
effectively ignored because the display controller isn't being handled 
by Linux (so there's no device to link it to). The splash screen might 
similarly flicker as the SMMU reset will initially block the traffic 
before the RMR region is enabled.

Steve

RE: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions reservation helper

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: 06 November 2020 13:06
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> iommu@lists.linux-foundation.org; devel@acpica.org
> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
> joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun
> (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>
> Subject: Re: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions reservation
> helper
> 
> On 2020-11-06 08:30, Shameerali Kolothum Thodi wrote:
> >
> >
> >> -----Original Message-----
> >> From: Robin Murphy [mailto:robin.murphy@arm.com]
> >> Sent: 05 November 2020 18:04
> >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> >> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> >> iommu@lists.linux-foundation.org; devel@acpica.org
> >> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
> >> joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun
> >> (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron
> >> <jonathan.cameron@huawei.com>
> >> Subject: Re: [RFC PATCH 3/4] ACPI/IORT: Add RMR memory regions
> reservation
> >> helper
> >>
> >> On 2020-10-27 11:26, Shameer Kolothum wrote:
> >>> Add a helper function that retrieves RMR memory descriptors
> >>> associated with a given endpoint dev. These memory regions
> >>> should have a unity mapping in the SMMU. So reserve them as
> >>> IOMMU_RESV_DIRECT.
> >>
> >> As a general observation, we also need a way into this that isn't from
> >> the perspective of endpoint devices. With SMMUv3 we need to know all the
> >> active stream IDs relevant to a given SMMU instance at probe time, so
> >> that we can set up some kind of valid stream table entries *before*
> >> enabling the SMMU in the reset routine.
> >
> > So I guess, the idea is to provide an interface here to retrieve those active
> > stream Ids? The problem is, at present(AFAICS), RMR doesn’t have any
> > means to specify such devices.
> 
> I'm thinking we need to check if any RMRs exist in the IORT, if so find
> the devices that map to them, then resolve those devices' ID mappings.
> In terms of the interface, maybe the better option is to do something
> closer to what intel-iommu does and handle *all* the processing up-front
> - let the IOMMU driver just call into the firmware code once to retrieve
> a complete list of all the relevant abstracted RMR (or DT equivalent)
> information, so it can then resolve both its own early initialisation
> and the later per-device setup without calling back into firmware code
> again.

Ok. I will try to change the interface then.

> >   Otherwise we're just going to
> >> kill ongoing traffic (e.g. EFI GOP) with C_BAD_STE long before we ever
> >> start adding devices and worrying about reserved regions for them.
> >> Similarly for the initial SMR/S2CR state on SMMUv2 with disable_bypass.
> >
> > Ok. I see the discussion here,
> >
> https://lore.kernel.org/linux-iommu/484b9e90-7395-6161-577c-4d3f3716997e
> @arm.com/
> >
> >  From what I gather, the plan is to setup a default IDENTITY_DOMAIN for
> > devices that have live stream going on during boot/SMMU probe time.
> 
> Well, whether we use bypass or a temporary translation context for the
> period before we even get to default domains... that's a discussion that
> can wait until we have the ability to set up the STEs at all ;)

Sure.

Thanks,
Shameer
 
> Robin.
> 
> >>> Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> >>> ---
> >>>    drivers/acpi/arm64/iort.c | 56
> >> +++++++++++++++++++++++++++++++++++++++
> >>>    include/linux/acpi_iort.h |  4 +++
> >>>    2 files changed, 60 insertions(+)
> >>>
> >>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> >>> index b32cd53cca08..c0700149e60b 100644
> >>> --- a/drivers/acpi/arm64/iort.c
> >>> +++ b/drivers/acpi/arm64/iort.c
> >>> @@ -842,6 +842,60 @@ static inline int iort_add_device_replay(struct
> >> device *dev)
> >>>    	return err;
> >>>    }
> >>>
> >>> +static bool iort_dev_has_rmr(struct device *dev, struct iort_rmr_entry
> *e)
> >>> +{
> >>> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> >>> +	struct acpi_iort_node *iommu;
> >>> +	struct iort_rmr_id *rmr_ids = e->rmr_ids;
> >>> +	int i, j;
> >>> +
> >>> +	iommu = iort_get_iort_node(fwspec->iommu_fwnode);
> >>> +
> >>> +	for (i = 0; i < e->rmr_ids_num; i++, rmr_ids++) {
> >>> +		for (j = 0; j < fwspec->num_ids; j++) {
> >>> +			if (rmr_ids->sid == fwspec->ids[j] &&
> >>> +			    rmr_ids->smmu == iommu)
> >>> +				return true;
> >>> +		}
> >>> +	}
> >>> +
> >>> +	return false;
> >>> +}
> >>> +
> >>> +/**
> >>> + * iort_dev_rmr_get_resv_regions - RMR Reserved region driver helper
> >>> + * @dev: Device from iommu_get_resv_regions()
> >>> + * @head: Reserved region list from iommu_get_resv_regions()
> >>> + *
> >>> + * Returns: 0 on success, <0 failure
> >>> + */
> >>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
> >> *head)
> >>> +{
> >>> +	struct iort_rmr_entry *e;
> >>> +
> >>> +	list_for_each_entry(e, &iort_rmr_list, list) {
> >>> +		struct iommu_resv_region *region;
> >>> +		struct acpi_iort_rmr_desc *rmr;
> >>> +		int prot = IOMMU_READ | IOMMU_WRITE |
> >>> +			   IOMMU_NOEXEC | IOMMU_MMIO;
> >>> +
> >>> +		if (!iort_dev_has_rmr(dev, e))
> >>> +			continue;
> >>> +
> >>> +		rmr = e->rmr_desc;
> >>> +		region = iommu_alloc_resv_region(rmr->base_address,
> >>> +						 rmr->length, prot,
> >>> +						 IOMMU_RESV_DIRECT);
> >>> +		if (!region) {
> >>> +			dev_err(dev, "Out of memory allocating RMR regions\n");
> >>> +			return -ENOMEM;
> >>> +		}
> >>> +		list_add_tail(&region->list, head);
> >>> +	}
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>>    /**
> >>>     * iort_iommu_msi_get_resv_regions - Reserved region driver helper
> >>>     * @dev: Device from iommu_get_resv_regions()
> >>> @@ -1112,6 +1166,8 @@ int iort_iommu_msi_get_resv_regions(struct
> >> device *dev, struct list_head *head)
> >>>    const struct iommu_ops *iort_iommu_configure_id(struct device *dev,
> >>>    						const u32 *input_id)
> >>>    { return NULL; }
> >>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
> >> *head)
> >>> +{ return 0; }
> >>>    #endif
> >>>
> >>>    static int nc_dma_get_range(struct device *dev, u64 *size)
> >>> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
> >>> index 20a32120bb88..6dd89faf340c 100644
> >>> --- a/include/linux/acpi_iort.h
> >>> +++ b/include/linux/acpi_iort.h
> >>> @@ -38,6 +38,7 @@ void iort_dma_setup(struct device *dev, u64
> >> *dma_addr, u64 *size);
> >>>    const struct iommu_ops *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);
> >>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
> >> *head);
> >>>    #else
> >>>    static inline void acpi_iort_init(void) { }
> >>>    static inline u32 iort_msi_map_id(struct device *dev, u32 id)
> >>> @@ -55,6 +56,9 @@ static inline const struct iommu_ops
> >> *iort_iommu_configure_id(
> >>>    static inline
> >>>    int iort_iommu_msi_get_resv_regions(struct device *dev, struct
> list_head
> >> *head)
> >>>    { return 0; }
> >>> +static inline
> >>> +int iort_dev_rmr_get_resv_regions(struct device *dev, struct list_head
> >> *head)
> >>> +{ return 0; }
> >>>    #endif
> >>>
> >>>    #endif /* __ACPI_IORT_H__ */
> >>>

RE: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Steven Price [mailto:steven.price@arm.com]
> Sent: 06 November 2020 15:22
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> iommu@lists.linux-foundation.org; devel@acpica.org
> Cc: lorenzo.pieralisi@arm.com; joro@8bytes.org; Jonathan Cameron
> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>;
> Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; Sami Mujawar
> <Sami.Mujawar@arm.com>; robin.murphy@arm.com; wanghuiqiang
> <wanghuiqiang@huawei.com>
> Subject: Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node
> 
> On 28/10/2020 18:24, Shameerali Kolothum Thodi wrote:
> > Hi Steve,
> >
> >> -----Original Message-----
> >> From: Steven Price [mailto:steven.price@arm.com]
> >> Sent: 28 October 2020 16:44
> >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> >> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> >> iommu@lists.linux-foundation.org; devel@acpica.org
> >> Cc: lorenzo.pieralisi@arm.com; joro@8bytes.org; Jonathan Cameron
> >> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>;
> >> Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
> robin.murphy@arm.com;
> >> wanghuiqiang <wanghuiqiang@huawei.com>; Sami Mujawar
> >> <Sami.Mujawar@arm.com>
> >> Subject: Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node
> >>
> >> On 27/10/2020 11:26, Shameer Kolothum wrote:
> >>> The series adds support to IORT RMR nodes specified in IORT
> >>> Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory
> >>> ranges that are used by endpoints and require a unity mapping
> >>> in SMMU.
> >>
> >> Hi Shameer,
> >>
> >> I've also been taking a look at RMR, and Sami is helping me get set up
> >> so that I can do some testing. We're hoping to be able to test an EFI
> >> framebuffer or splash screen - which has the added complication of the
> >> unity mapping becoming redundant if a native display driver takes over
> >> the display controller.
> >>
> >> I've looked through your series and the code looks correct to me.
> >
> > Thanks for taking a look and the details.
> >
> >> Hopefully I'll be able to give it some testing soon.
> >
> > Cool. Please update once you get a chance run the tests.
> 
> Hi Shameer,

Hi Steve,

> Just to update on this, for the EFI framebuffer use case I hit exactly
> the issue that Robin has mentioned in another thread - the RMR is
> effectively ignored because the display controller isn't being handled
> by Linux (so there's no device to link it to).

Thanks for the update. Here, by "ignored "you meant get_resv_regions()
is not called or not?

 The splash screen might
> similarly flicker as the SMMU reset will initially block the traffic
> before the RMR region is enabled.

Does that mean you somehow managed to make the unity
mapping but there was flicker during the SMMU reset to
unity mapping setup period. Sorry I am trying to understand
the exact behavior observed in this case.

Thanks,
Shameer

Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node

[Steven Price]

On 06/11/2020 16:17, Shameerali Kolothum Thodi wrote:
> 
> 
>> -----Original Message-----
>> From: Steven Price [mailto:steven.price@arm.com]
>> Sent: 06 November 2020 15:22
>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
>> iommu@lists.linux-foundation.org; devel@acpica.org
>> Cc: lorenzo.pieralisi@arm.com; joro@8bytes.org; Jonathan Cameron
>> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>;
>> Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; Sami Mujawar
>> <Sami.Mujawar@arm.com>; robin.murphy@arm.com; wanghuiqiang
>> <wanghuiqiang@huawei.com>
>> Subject: Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node
>>
>> On 28/10/2020 18:24, Shameerali Kolothum Thodi wrote:
>>> Hi Steve,
>>>
>>>> -----Original Message-----
>>>> From: Steven Price [mailto:steven.price@arm.com]
>>>> Sent: 28 October 2020 16:44
>>>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
>>>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
>>>> iommu@lists.linux-foundation.org; devel@acpica.org
>>>> Cc: lorenzo.pieralisi@arm.com; joro@8bytes.org; Jonathan Cameron
>>>> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>;
>>>> Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
>> robin.murphy@arm.com;
>>>> wanghuiqiang <wanghuiqiang@huawei.com>; Sami Mujawar
>>>> <Sami.Mujawar@arm.com>
>>>> Subject: Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node
>>>>
>>>> On 27/10/2020 11:26, Shameer Kolothum wrote:
>>>>> The series adds support to IORT RMR nodes specified in IORT
>>>>> Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory
>>>>> ranges that are used by endpoints and require a unity mapping
>>>>> in SMMU.
>>>>
>>>> Hi Shameer,
>>>>
>>>> I've also been taking a look at RMR, and Sami is helping me get set up
>>>> so that I can do some testing. We're hoping to be able to test an EFI
>>>> framebuffer or splash screen - which has the added complication of the
>>>> unity mapping becoming redundant if a native display driver takes over
>>>> the display controller.
>>>>
>>>> I've looked through your series and the code looks correct to me.
>>>
>>> Thanks for taking a look and the details.
>>>
>>>> Hopefully I'll be able to give it some testing soon.
>>>
>>> Cool. Please update once you get a chance run the tests.
>>
>> Hi Shameer,
> 
> Hi Steve,
> 
>> Just to update on this, for the EFI framebuffer use case I hit exactly
>> the issue that Robin has mentioned in another thread - the RMR is
>> effectively ignored because the display controller isn't being handled
>> by Linux (so there's no device to link it to).
> 
> Thanks for the update. Here, by "ignored "you meant get_resv_regions()
> is not called or not?

get_resv_regions() isn't called.

>   The splash screen might
>> similarly flicker as the SMMU reset will initially block the traffic
>> before the RMR region is enabled.
> 
> Does that mean you somehow managed to make the unity
> mapping but there was flicker during the SMMU reset to
> unity mapping setup period. Sorry I am trying to understand
> the exact behavior observed in this case.

I haven't yet got this completely working (on the board which I'm 
testing the display controller doesn't have any existing ACPI bindings). 
However from what I understand the SMMU reset would block all memory 
access for the display controller before the RMR region would be setup. 
I'm going to try to get the display controller working with ACPI so I 
can test this properly.

Steve

Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node

[Robin Murphy]

On 2020-11-06 16:26, Steven Price wrote:
> On 06/11/2020 16:17, Shameerali Kolothum Thodi wrote:
>>
>>
>>> -----Original Message-----
>>> From: Steven Price [mailto:steven.price@arm.com]
>>> Sent: 06 November 2020 15:22
>>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
>>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
>>> iommu@lists.linux-foundation.org; devel@acpica.org
>>> Cc: lorenzo.pieralisi@arm.com; joro@8bytes.org; Jonathan Cameron
>>> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>;
>>> Guohanjun (Hanjun Guo) <guohanjun@huawei.com>; Sami Mujawar
>>> <Sami.Mujawar@arm.com>; robin.murphy@arm.com; wanghuiqiang
>>> <wanghuiqiang@huawei.com>
>>> Subject: Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node
>>>
>>> On 28/10/2020 18:24, Shameerali Kolothum Thodi wrote:
>>>> Hi Steve,
>>>>
>>>>> -----Original Message-----
>>>>> From: Steven Price [mailto:steven.price@arm.com]
>>>>> Sent: 28 October 2020 16:44
>>>>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
>>>>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
>>>>> iommu@lists.linux-foundation.org; devel@acpica.org
>>>>> Cc: lorenzo.pieralisi@arm.com; joro@8bytes.org; Jonathan Cameron
>>>>> <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>;
>>>>> Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
>>> robin.murphy@arm.com;
>>>>> wanghuiqiang <wanghuiqiang@huawei.com>; Sami Mujawar
>>>>> <Sami.Mujawar@arm.com>
>>>>> Subject: Re: [RFC PATCH 0/4] ACPI/IORT: Support for IORT RMR node
>>>>>
>>>>> On 27/10/2020 11:26, Shameer Kolothum wrote:
>>>>>> The series adds support to IORT RMR nodes specified in IORT
>>>>>> Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory
>>>>>> ranges that are used by endpoints and require a unity mapping
>>>>>> in SMMU.
>>>>>
>>>>> Hi Shameer,
>>>>>
>>>>> I've also been taking a look at RMR, and Sami is helping me get set up
>>>>> so that I can do some testing. We're hoping to be able to test an EFI
>>>>> framebuffer or splash screen - which has the added complication of the
>>>>> unity mapping becoming redundant if a native display driver takes over
>>>>> the display controller.
>>>>>
>>>>> I've looked through your series and the code looks correct to me.
>>>>
>>>> Thanks for taking a look and the details.
>>>>
>>>>> Hopefully I'll be able to give it some testing soon.
>>>>
>>>> Cool. Please update once you get a chance run the tests.
>>>
>>> Hi Shameer,
>>
>> Hi Steve,
>>
>>> Just to update on this, for the EFI framebuffer use case I hit exactly
>>> the issue that Robin has mentioned in another thread - the RMR is
>>> effectively ignored because the display controller isn't being handled
>>> by Linux (so there's no device to link it to).
>>
>> Thanks for the update. Here, by "ignored "you meant get_resv_regions()
>> is not called or not?
> 
> get_resv_regions() isn't called.

Right, AIUI the EFI framebuffer "device" pretty much just represents a 
magic region of RAM, whose existence is based on EFI services - see 
register_gop_device() - regardless of whether the underlying physical 
hardware is described in DSDT and IORT such that a tangential 
iommu_probe_device() call might happen at all.

Robin.

>>   The splash screen might
>>> similarly flicker as the SMMU reset will initially block the traffic
>>> before the RMR region is enabled.
>>
>> Does that mean you somehow managed to make the unity
>> mapping but there was flicker during the SMMU reset to
>> unity mapping setup period. Sorry I am trying to understand
>> the exact behavior observed in this case.
> 
> I haven't yet got this completely working (on the board which I'm 
> testing the display controller doesn't have any existing ACPI bindings). 
> However from what I understand the SMMU reset would block all memory 
> access for the display controller before the RMR region would be setup. 
> I'm going to try to get the display controller working with ACPI so I 
> can test this properly.
> 
> Steve

RE: [Devel] Re: [RFC PATCH 2/4] ACPI/IORT: Add support for RMR node parsing

[Sami Mujawar]

Hi,

-----Original Message-----
From: David E. Box <david.e.box@linux.intel.com> 
Sent: 28 October 2020 06:44 PM
To: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>; linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org; devel@acpica.org
Cc: linuxarm@huawei.com; Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>; joro@8bytes.org; Robin Murphy <Robin.Murphy@arm.com>; wanghuiqiang@huawei.com; jonathan.cameron@huawei.com
Subject: [Devel] Re: [RFC PATCH 2/4] ACPI/IORT: Add support for RMR node parsing

Hi,

On Tue, 2020-10-27 at 11:26 +0000, Shameer Kolothum wrote:

...

> @@ -1647,6 +1667,100 @@ 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 int iort_rmr_desc_valid(struct acpi_iort_rmr_desc *desc)
> +{
> +	struct iort_rmr_entry *e;
> +	u64 end, start = desc->base_address, length = desc->length;
> +
> +	if ((!IS_ALIGNED(start, SZ_64K)) || (length % SZ_64K != 0))

You could just do:

if ((!IS_ALIGNED(start, SZ_64K)) || (length % SZ_64K))

[SAMI] In my opinion, the following may be better:
	if (!IS_ALIGNED(start, SZ_64K) || !IS_ALIGNED(length, SZ_64K)) 
[/SAMI]

Regards,

Sami Mujawar

David
_______________________________________________
Devel mailing list -- devel@acpica.org
To unsubscribe send an email to devel-leave@acpica.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

RE: [Devel] Re: [RFC PATCH 2/4] ACPI/IORT: Add support for RMR node parsing

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Sami Mujawar [mailto:Sami.Mujawar@arm.com]
> Sent: 09 November 2020 12:30
> To: david.e.box@linux.intel.com; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>;
> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> iommu@lists.linux-foundation.org; devel@acpica.org
> Cc: Linuxarm <linuxarm@huawei.com>; Lorenzo Pieralisi
> <Lorenzo.Pieralisi@arm.com>; joro@8bytes.org; Robin Murphy
> <Robin.Murphy@arm.com>; wanghuiqiang <wanghuiqiang@huawei.com>;
> Jonathan Cameron <jonathan.cameron@huawei.com>; nd <nd@arm.com>
> Subject: RE: [Devel] Re: [RFC PATCH 2/4] ACPI/IORT: Add support for RMR node
> parsing
> 
> Hi,
> 
> -----Original Message-----
> From: David E. Box <david.e.box@linux.intel.com>
> Sent: 28 October 2020 06:44 PM
> To: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>;
> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> iommu@lists.linux-foundation.org; devel@acpica.org
> Cc: linuxarm@huawei.com; Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>;
> joro@8bytes.org; Robin Murphy <Robin.Murphy@arm.com>;
> wanghuiqiang@huawei.com; jonathan.cameron@huawei.com
> Subject: [Devel] Re: [RFC PATCH 2/4] ACPI/IORT: Add support for RMR node
> parsing
> 
> Hi,
> 
> On Tue, 2020-10-27 at 11:26 +0000, Shameer Kolothum wrote:
> 
> ...
> 
> > @@ -1647,6 +1667,100 @@ 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 int iort_rmr_desc_valid(struct acpi_iort_rmr_desc *desc)
> > +{
> > +	struct iort_rmr_entry *e;
> > +	u64 end, start = desc->base_address, length = desc->length;
> > +
> > +	if ((!IS_ALIGNED(start, SZ_64K)) || (length % SZ_64K != 0))
> 
> You could just do:
> 
> if ((!IS_ALIGNED(start, SZ_64K)) || (length % SZ_64K))
> 
> [SAMI] In my opinion, the following may be better:
> 	if (!IS_ALIGNED(start, SZ_64K) || !IS_ALIGNED(length, SZ_64K))
> [/SAMI]

Thanks for your suggestions. I don't have a strong opinion on either
of those, but will change it with the latter one for now.

Thanks,
Shameer

> Regards,
> 
> Sami Mujawar
> 
> David
> _______________________________________________
> Devel mailing list -- devel@acpica.org
> To unsubscribe send an email to devel-leave@acpica.org
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s