[PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Shameer Kolothum]

Hi,

Since we now have an updated verion[0] of IORT spec(E.d) which
addresses the memory attributes issues discussed here [1],
this series now make use of it.

The pull request for ACPICA E.d related changes are already
raised and can be found here,
https://github.com/acpica/acpica/pull/752

v7 --> v8
  - Patch #1 has temp definitions for RMR related changes till
    the ACPICA header changes are part of kernel.
  - No early parsing of RMR node info and is only parsed at the
    time of use.
  - Changes to the RMR get/put API format compared to the
    previous version.
  - Support for RMR descriptor shared by multiple stream IDs.

Please take a look and let me know your thoughts.

Thanks,
Shameer
[0] https://developer.arm.com/documentation/den0049/ed/
[1] https://lore.kernel.org/linux-acpi/20210805160319.GB23085@lpieralisi/

From old:
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
 -fix pointed out by Steve to the SMMUv2 SMR bypass install in patch #8.

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.

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 (10):
  ACPI/IORT: Add temporary RMR node flag definitions
  iommu: Introduce a union to struct iommu_resv_region
  ACPI/IORT: Add helper functions to parse RMR nodes
  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/arm-smmu-v3: Reserve any RMR regions associated with a dev
  iommu/arm-smmu: Reserve any RMR regions associated with a dev

 drivers/acpi/arm64/iort.c                   | 305 ++++++++++++++++++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  91 ++++--
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  65 ++++-
 drivers/iommu/dma-iommu.c                   |  25 ++
 include/linux/acpi_iort.h                   |  14 +
 include/linux/dma-iommu.h                   |  14 +
 include/linux/iommu.h                       |   9 +
 7 files changed, 504 insertions(+), 19 deletions(-)

-- 
2.25.1


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

[PATCH v8 01/11] ACPI/IORT: Add temporary RMR node flag definitions

[Shameer Kolothum]

IORT rev E.d introduces more details into the RMR node Flags
field. Add temporary definitions to describe and access these
Flags field until ACPICA header is updated to support E.d.

This patch can be reverted once the include/acpi/actbl2.h has
all the relevant definitions.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
Please find the ACPICA E.d related changes pull request here,
https://github.com/acpica/acpica/pull/752

---
 drivers/acpi/arm64/iort.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index f2f8f05662de..0730c4dbb700 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -25,6 +25,30 @@
 #define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
 				(1 << ACPI_IORT_NODE_SMMU_V3))
 
+/*
+ * The following RMR related definitions are temporary and
+ * can be removed once ACPICA headers support IORT rev E.d
+ */
+#ifndef ACPI_IORT_RMR_REMAP_PERMITTED
+#define ACPI_IORT_RMR_REMAP_PERMITTED	(1)
+#endif
+
+#ifndef ACPI_IORT_RMR_ACCESS_PRIVILEGE
+#define ACPI_IORT_RMR_ACCESS_PRIVILEGE	(1 << 1)
+#endif
+
+#ifndef ACPI_IORT_RMR_ACCESS_ATTRIBUTES
+#define ACPI_IORT_RMR_ACCESS_ATTRIBUTES(flags)	(((flags) >> 2) & 0xFF)
+#endif
+
+#ifndef ACPI_IORT_RMR_ATTR_DEVICE_GRE
+#define ACPI_IORT_RMR_ATTR_DEVICE_GRE	0x03
+#endif
+
+#ifndef ACPI_IORT_RMR_ATTR_NORMAL
+#define ACPI_IORT_RMR_ATTR_NORMAL	0x05
+#endif
+
 struct iort_its_msi_chip {
 	struct list_head	list;
 	struct fwnode_handle	*fw_node;
-- 
2.25.1


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

Re: [PATCH v8 01/11] ACPI/IORT: Add temporary RMR node flag definitions

[Robin Murphy]

On 2022-02-21 15:43, Shameer Kolothum via iommu wrote:
> IORT rev E.d introduces more details into the RMR node Flags
> field. Add temporary definitions to describe and access these
> Flags field until ACPICA header is updated to support E.d.
> 
> This patch can be reverted once the include/acpi/actbl2.h has
> all the relevant definitions.
> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
> Please find the ACPICA E.d related changes pull request here,
> https://github.com/acpica/acpica/pull/752
> 
> ---
>   drivers/acpi/arm64/iort.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index f2f8f05662de..0730c4dbb700 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -25,6 +25,30 @@
>   #define IORT_IOMMU_TYPE		((1 << ACPI_IORT_NODE_SMMU) |	\
>   				(1 << ACPI_IORT_NODE_SMMU_V3))
>   
> +/*
> + * The following RMR related definitions are temporary and
> + * can be removed once ACPICA headers support IORT rev E.d
> + */
> +#ifndef ACPI_IORT_RMR_REMAP_PERMITTED
> +#define ACPI_IORT_RMR_REMAP_PERMITTED	(1)
> +#endif
> +
> +#ifndef ACPI_IORT_RMR_ACCESS_PRIVILEGE
> +#define ACPI_IORT_RMR_ACCESS_PRIVILEGE	(1 << 1)
> +#endif
> +
> +#ifndef ACPI_IORT_RMR_ACCESS_ATTRIBUTES
> +#define ACPI_IORT_RMR_ACCESS_ATTRIBUTES(flags)	(((flags) >> 2) & 0xFF)
> +#endif
> +
> +#ifndef ACPI_IORT_RMR_ATTR_DEVICE_GRE
> +#define ACPI_IORT_RMR_ATTR_DEVICE_GRE	0x03
> +#endif
> +
> +#ifndef ACPI_IORT_RMR_ATTR_NORMAL
> +#define ACPI_IORT_RMR_ATTR_NORMAL	0x05

For the record, I've commented directly on the ACPICA pull request that 
I think this should be more clearly named to indicate that it means 
Normal Write-Back Cacheable, rather than being potentially ambiguous 
about cacheability.

Robin.

> +#endif
> +
>   struct iort_its_msi_chip {
>   	struct list_head	list;
>   	struct fwnode_handle	*fw_node;

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

[PATCH v8 02/11] iommu: Introduce a union to struct iommu_resv_region

[Shameer Kolothum]

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 <shameerali.kolothum.thodi@huawei.com>
---
 include/linux/iommu.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index de0c57a567c8..b06952a75f95 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -126,6 +126,11 @@ enum iommu_resv_type {
 	IOMMU_RESV_SW_MSI,
 };
 
+struct iommu_iort_rmr_data {
+	u32 *sids;	/* Stream Ids associated with IORT RMR entry */
+	u32 num_sids;
+};
+
 /**
  * struct iommu_resv_region - descriptor for a reserved memory region
  * @list: Linked list pointers
@@ -133,6 +138,7 @@ enum iommu_resv_type {
  * @length: Length of the region in bytes
  * @prot: IOMMU Protection flags (READ/WRITE/...)
  * @type: Type of the reserved region
+ * @fw_data: FW specific reserved region data
  */
 struct iommu_resv_region {
 	struct list_head	list;
@@ -140,6 +146,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.25.1


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

Re: [PATCH v8 02/11] iommu: Introduce a union to struct iommu_resv_region

[Robin Murphy]

On 2022-02-21 15:43, Shameer Kolothum wrote:
> 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 <shameerali.kolothum.thodi@huawei.com>
> ---
>   include/linux/iommu.h | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index de0c57a567c8..b06952a75f95 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -126,6 +126,11 @@ enum iommu_resv_type {
>   	IOMMU_RESV_SW_MSI,
>   };
>   
> +struct iommu_iort_rmr_data {
> +	u32 *sids;	/* Stream Ids associated with IORT RMR entry */

Please make this const.

Further nit: capitalisation of "IDs" in the comment, otherwise I might 
worry about the possibility of Stream Egos too :P

> +	u32 num_sids;
> +};
> +
>   /**
>    * struct iommu_resv_region - descriptor for a reserved memory region
>    * @list: Linked list pointers
> @@ -133,6 +138,7 @@ enum iommu_resv_type {
>    * @length: Length of the region in bytes
>    * @prot: IOMMU Protection flags (READ/WRITE/...)
>    * @type: Type of the reserved region
> + * @fw_data: FW specific reserved region data

Nit: we've got plenty of room to spell out "Firmware-specific", and it 
never hurts to make documentation as easy to read as possible.

Thanks,
Robin.

>    */
>   struct iommu_resv_region {
>   	struct list_head	list;
> @@ -140,6 +146,9 @@ struct iommu_resv_region {
>   	size_t			length;
>   	int			prot;
>   	enum iommu_resv_type	type;
> +	union {
> +		struct iommu_iort_rmr_data rmr;
> +	} fw_data;
>   };
>   
>   /**

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

RE: [PATCH v8 02/11] iommu: Introduce a union to struct iommu_resv_region

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: 22 March 2022 18:27
> 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
> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
> joro@8bytes.org; will@kernel.org; wanghuiqiang
> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; steven.price@arm.com; Sami.Mujawar@arm.com;
> jon@solid-run.com; eric.auger@redhat.com; yangyicong
> <yangyicong@huawei.com>
> Subject: Re: [PATCH v8 02/11] iommu: Introduce a union to struct
> iommu_resv_region
> 
> On 2022-02-21 15:43, Shameer Kolothum wrote:
> > 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
> <shameerali.kolothum.thodi@huawei.com>
> > ---
> >   include/linux/iommu.h | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index de0c57a567c8..b06952a75f95 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -126,6 +126,11 @@ enum iommu_resv_type {
> >   	IOMMU_RESV_SW_MSI,
> >   };
> >
> > +struct iommu_iort_rmr_data {
> > +	u32 *sids;	/* Stream Ids associated with IORT RMR entry */
> 
> Please make this const.
> 
> Further nit: capitalisation of "IDs" in the comment, otherwise I might
> worry about the possibility of Stream Egos too :P

True :). Will do that.

Thanks,
Shameer 

> 
> > +	u32 num_sids;
> > +};
> > +
> >   /**
> >    * struct iommu_resv_region - descriptor for a reserved memory region
> >    * @list: Linked list pointers
> > @@ -133,6 +138,7 @@ enum iommu_resv_type {
> >    * @length: Length of the region in bytes
> >    * @prot: IOMMU Protection flags (READ/WRITE/...)
> >    * @type: Type of the reserved region
> > + * @fw_data: FW specific reserved region data
> 
> Nit: we've got plenty of room to spell out "Firmware-specific", and it
> never hurts to make documentation as easy to read as possible.
> 
> Thanks,
> Robin.
> 
> >    */
> >   struct iommu_resv_region {
> >   	struct list_head	list;
> > @@ -140,6 +146,9 @@ struct iommu_resv_region {
> >   	size_t			length;
> >   	int			prot;
> >   	enum iommu_resv_type	type;
> > +	union {
> > +		struct iommu_iort_rmr_data rmr;
> > +	} fw_data;
> >   };
> >
> >   /**
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR nodes

[Shameer Kolothum]

The helper functions here parse through the IORT RMR nodes and
populate a reserved region list  corresponding to a given iommu
and device(optional). These also go through the ID mappings of
the RMR node and retrieves all the SIDs associated with a RMR
descriptor.

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

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 0730c4dbb700..05da9ebff50a 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -830,6 +830,231 @@ static struct acpi_iort_node *iort_get_msi_resv_iommu(struct device *dev)
 	return NULL;
 }
 
+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);
+		}
+	}
+}
+
+/*
+ * Please note, we will keep the already allocated RMR reserve
+ * regions in case of a memory allocation failure.
+ */
+static void iort_rmr_get_resv_regions(struct acpi_iort_node *node,
+				      struct acpi_iort_node *smmu,
+				      u32 *sids, u32 num_sids,
+				      struct list_head *head)
+{
+	struct acpi_iort_rmr *rmr = (struct acpi_iort_rmr *)node->node_data;
+	struct acpi_iort_rmr_desc *rmr_desc;
+	int i;
+
+	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, 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;
+		u32  *sids_copy;
+		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 & ACPI_IORT_RMR_REMAP_PERMITTED)
+			type = IOMMU_RESV_DIRECT_RELAXABLE;
+		else
+			type = IOMMU_RESV_DIRECT;
+
+		if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE)
+			prot |= IOMMU_PRIV;
+
+		/* Attributes 0x00 - 0x03 represents device memory */
+		if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) <=
+				ACPI_IORT_RMR_ATTR_DEVICE_GRE)
+			prot |= IOMMU_MMIO;
+		else if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) ==
+				ACPI_IORT_RMR_ATTR_NORMAL)
+			prot |= IOMMU_CACHE;
+
+		/* Create a copy of sids array to associate with this resv region */
+		sids_copy = kmemdup(sids, num_sids * sizeof(*sids), GFP_KERNEL);
+		if (!sids_copy)
+			return;
+
+		region = iommu_alloc_resv_region(addr, size, prot, type);
+		if (!region) {
+			kfree(sids_copy);
+			return;
+		}
+
+		region->fw_data.rmr.sids = sids_copy;
+		region->fw_data.rmr.num_sids = num_sids;
+		list_add_tail(&region->list, head);
+	}
+}
+
+static u32 *iort_rmr_alloc_sids(u32 *sids, u32 count, u32 id_start,
+				u32 new_count)
+{
+	u32 *new_sids;
+	u32 total_count = count + new_count;
+	int i;
+
+	new_sids = krealloc_array(sids, count + new_count,
+				  sizeof(*new_sids), GFP_KERNEL);
+	if (!new_sids)
+		return NULL;
+
+	/*Update new ones */
+	for (i = count; i < total_count; i++)
+		new_sids[i] = id_start++;
+
+	return new_sids;
+}
+
+static bool iort_rmr_has_dev(struct device *dev, u32 id_start,
+			     u32 id_count)
+{
+	int i;
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+
+	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 false;
+	}
+
+	for (i = 0; i < fwspec->num_ids; i++) {
+		if (fwspec->ids[i] >= id_start &&
+		    fwspec->ids[i] <= id_start + id_count)
+			return true;
+	}
+
+	return false;
+}
+
+static void iort_node_get_rmr_info(struct acpi_iort_node *node,
+				   struct acpi_iort_node *iommu,
+				   struct device *dev, struct list_head *head)
+{
+	struct acpi_iort_node *smmu = NULL;
+	struct acpi_iort_rmr *rmr;
+	struct acpi_iort_id_mapping *map;
+	u32 *sids = NULL;
+	u32 num_sids = 0;
+	int i;
+
+	if (!node->mapping_offset || !node->mapping_count) {
+		pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
+		       node);
+		return;
+	}
+
+	rmr = (struct acpi_iort_rmr *)node->node_data;
+	if (!rmr->rmr_offset || !rmr->rmr_count)
+		return;
+
+	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
+			   node->mapping_offset);
+
+	/*
+	 * Go through the ID mappings and see if we have a match
+	 * for smmu and dev(if !NULL). If found, get the sids
+	 * for the Node.
+	 * Please note, id_count is equal to the number of IDs
+	 * in the range minus one.
+	 */
+	for (i = 0; i < node->mapping_count; i++, map++) {
+		struct acpi_iort_node *parent;
+
+		if (!map->id_count)
+			continue;
+
+		parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
+				      map->output_reference);
+		if (parent != iommu)
+			continue;
+
+		/* If dev is valid, check RMR node corresponds to the dev sid */
+		if (dev && !iort_rmr_has_dev(dev, map->output_base,
+					     map->id_count))
+			continue;
+
+		/* Retrieve sids associated with the Node. */
+		sids = iort_rmr_alloc_sids(sids, num_sids, map->output_base,
+					   map->id_count + 1);
+		if (!sids)
+			return;
+
+		num_sids += map->id_count + 1;
+	}
+
+	if (!sids)
+		return;
+
+	iort_rmr_get_resv_regions(node, smmu, sids, num_sids, head);
+	kfree(sids);
+}
+
+static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,
+			   struct list_head *head)
+{
+	struct acpi_table_iort *iort;
+	struct acpi_iort_node *iort_node, *iort_end;
+	int i;
+
+	if (iort_table->revision < 5)
+		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, iommu, dev, head);
+
+		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
+					 iort_node->length);
+	}
+}
+
 /**
  * iort_iommu_msi_get_resv_regions - Reserved region driver helper
  * @dev: Device from iommu_get_resv_regions()
-- 
2.25.1


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

Re: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR nodes

[Lorenzo Pieralisi]

On Mon, Feb 21, 2022 at 03:43:36PM +0000, Shameer Kolothum wrote:
> The helper functions here parse through the IORT RMR nodes and
> populate a reserved region list  corresponding to a given iommu
> and device(optional). These also go through the ID mappings of
> the RMR node and retrieves all the SIDs associated with a RMR
> descriptor.
> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  drivers/acpi/arm64/iort.c | 225 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 225 insertions(+)

I have very minor comments - I would ask Robin to ack the updated
flags management.

Functions should be introduced where they are used, this patch
is not bisectable:

drivers/acpi/arm64/iort.c:1028:13: warning: ‘iort_find_rmrs’ defined but not used [-Wunused-function]
 1028 | static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,

> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 0730c4dbb700..05da9ebff50a 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -830,6 +830,231 @@ static struct acpi_iort_node *iort_get_msi_resv_iommu(struct device *dev)
>  	return NULL;
>  }
>  
> +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;

We could probably check for length != 0, the kernel is not there to
validate firmware but this would not hurt either.

> +
> +		/* 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);
> +		}
> +	}
> +}
> +
> +/*
> + * Please note, we will keep the already allocated RMR reserve
> + * regions in case of a memory allocation failure.
> + */
> +static void iort_rmr_get_resv_regions(struct acpi_iort_node *node,
> +				      struct acpi_iort_node *smmu,
> +				      u32 *sids, u32 num_sids,
> +				      struct list_head *head)
> +{
> +	struct acpi_iort_rmr *rmr = (struct acpi_iort_rmr *)node->node_data;
> +	struct acpi_iort_rmr_desc *rmr_desc;
> +	int i;
> +
> +	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, 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;
> +		u32  *sids_copy;
> +		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 & ACPI_IORT_RMR_REMAP_PERMITTED)
> +			type = IOMMU_RESV_DIRECT_RELAXABLE;
> +		else
> +			type = IOMMU_RESV_DIRECT;
> +
> +		if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE)
> +			prot |= IOMMU_PRIV;
> +
> +		/* Attributes 0x00 - 0x03 represents device memory */
> +		if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) <=
> +				ACPI_IORT_RMR_ATTR_DEVICE_GRE)
> +			prot |= IOMMU_MMIO;
> +		else if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) ==
> +				ACPI_IORT_RMR_ATTR_NORMAL)
> +			prot |= IOMMU_CACHE;
> +
> +		/* Create a copy of sids array to associate with this resv region */
> +		sids_copy = kmemdup(sids, num_sids * sizeof(*sids), GFP_KERNEL);
> +		if (!sids_copy)
> +			return;
> +
> +		region = iommu_alloc_resv_region(addr, size, prot, type);
> +		if (!region) {
> +			kfree(sids_copy);
> +			return;
> +		}
> +
> +		region->fw_data.rmr.sids = sids_copy;
> +		region->fw_data.rmr.num_sids = num_sids;
> +		list_add_tail(&region->list, head);
> +	}
> +}
> +
> +static u32 *iort_rmr_alloc_sids(u32 *sids, u32 count, u32 id_start,
> +				u32 new_count)
> +{
> +	u32 *new_sids;
> +	u32 total_count = count + new_count;
> +	int i;
> +
> +	new_sids = krealloc_array(sids, count + new_count,
> +				  sizeof(*new_sids), GFP_KERNEL);
> +	if (!new_sids)
> +		return NULL;
> +
> +	/*Update new ones */

/* Update new ones */

I'd remove the comment, or improve it, as it stands it does not
help much - we can read the code.

> +	for (i = count; i < total_count; i++)
> +		new_sids[i] = id_start++;
> +
> +	return new_sids;
> +}
> +
> +static bool iort_rmr_has_dev(struct device *dev, u32 id_start,
> +			     u32 id_count)
> +{
> +	int i;
> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> +	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)

Please add a comment here - it helps explain the logic behind it.

Thanks,
Lorenzo

> +			return false;
> +	}
> +
> +	for (i = 0; i < fwspec->num_ids; i++) {
> +		if (fwspec->ids[i] >= id_start &&
> +		    fwspec->ids[i] <= id_start + id_count)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void iort_node_get_rmr_info(struct acpi_iort_node *node,
> +				   struct acpi_iort_node *iommu,
> +				   struct device *dev, struct list_head *head)
> +{
> +	struct acpi_iort_node *smmu = NULL;
> +	struct acpi_iort_rmr *rmr;
> +	struct acpi_iort_id_mapping *map;
> +	u32 *sids = NULL;
> +	u32 num_sids = 0;
> +	int i;
> +
> +	if (!node->mapping_offset || !node->mapping_count) {
> +		pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
> +		       node);
> +		return;
> +	}
> +
> +	rmr = (struct acpi_iort_rmr *)node->node_data;
> +	if (!rmr->rmr_offset || !rmr->rmr_count)
> +		return;
> +
> +	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
> +			   node->mapping_offset);
> +
> +	/*
> +	 * Go through the ID mappings and see if we have a match
> +	 * for smmu and dev(if !NULL). If found, get the sids
> +	 * for the Node.
> +	 * Please note, id_count is equal to the number of IDs
> +	 * in the range minus one.
> +	 */
> +	for (i = 0; i < node->mapping_count; i++, map++) {
> +		struct acpi_iort_node *parent;
> +
> +		if (!map->id_count)
> +			continue;
> +
> +		parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				      map->output_reference);
> +		if (parent != iommu)
> +			continue;
> +
> +		/* If dev is valid, check RMR node corresponds to the dev sid */
> +		if (dev && !iort_rmr_has_dev(dev, map->output_base,
> +					     map->id_count))
> +			continue;
> +
> +		/* Retrieve sids associated with the Node. */
> +		sids = iort_rmr_alloc_sids(sids, num_sids, map->output_base,
> +					   map->id_count + 1);
> +		if (!sids)
> +			return;
> +
> +		num_sids += map->id_count + 1;
> +	}
> +
> +	if (!sids)
> +		return;
> +
> +	iort_rmr_get_resv_regions(node, smmu, sids, num_sids, head);
> +	kfree(sids);
> +}
> +
> +static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,
> +			   struct list_head *head)
> +{
> +	struct acpi_table_iort *iort;
> +	struct acpi_iort_node *iort_node, *iort_end;
> +	int i;
> +
> +	if (iort_table->revision < 5)
> +		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, iommu, dev, head);
> +
> +		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
> +					 iort_node->length);
> +	}
> +}
> +
>  /**
>   * iort_iommu_msi_get_resv_regions - Reserved region driver helper
>   * @dev: Device from iommu_get_resv_regions()
> -- 
> 2.25.1
> 

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

RE: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR nodes

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@arm.com]
> Sent: 24 February 2022 10:14
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org;
> iommu@lists.linux-foundation.org; Linuxarm <linuxarm@huawei.com>;
> joro@8bytes.org; robin.murphy@arm.com; will@kernel.org; wanghuiqiang
> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; steven.price@arm.com; Sami.Mujawar@arm.com;
> jon@solid-run.com; eric.auger@redhat.com; yangyicong
> <yangyicong@huawei.com>
> Subject: Re: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR
> nodes
> 
> On Mon, Feb 21, 2022 at 03:43:36PM +0000, Shameer Kolothum wrote:
> > The helper functions here parse through the IORT RMR nodes and
> > populate a reserved region list  corresponding to a given iommu
> > and device(optional). These also go through the ID mappings of
> > the RMR node and retrieves all the SIDs associated with a RMR
> > descriptor.
> >
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  drivers/acpi/arm64/iort.c | 225
> ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 225 insertions(+)
> 
> I have very minor comments - I would ask Robin to ack the updated
> flags management.
> 
> Functions should be introduced where they are used, this patch
> is not bisectable:

Right. I missed the below warning. Will do.

> drivers/acpi/arm64/iort.c:1028:13: warning: ‘iort_find_rmrs’ defined but not
> used [-Wunused-function]
>  1028 | static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device
> *dev,
> 
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 0730c4dbb700..05da9ebff50a 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -830,6 +830,231 @@ static struct acpi_iort_node
> *iort_get_msi_resv_iommu(struct device *dev)
> >  	return NULL;
> >  }
> >
> > +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;
> 
> We could probably check for length != 0, the kernel is not there to
> validate firmware but this would not hurt either.

Ok.

> > +
> > +		/* 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);
> > +		}
> > +	}
> > +}
> > +
> > +/*
> > + * Please note, we will keep the already allocated RMR reserve
> > + * regions in case of a memory allocation failure.
> > + */
> > +static void iort_rmr_get_resv_regions(struct acpi_iort_node *node,
> > +				      struct acpi_iort_node *smmu,
> > +				      u32 *sids, u32 num_sids,
> > +				      struct list_head *head)
> > +{
> > +	struct acpi_iort_rmr *rmr = (struct acpi_iort_rmr *)node->node_data;
> > +	struct acpi_iort_rmr_desc *rmr_desc;
> > +	int i;
> > +
> > +	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, 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;
> > +		u32  *sids_copy;
> > +		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 & ACPI_IORT_RMR_REMAP_PERMITTED)
> > +			type = IOMMU_RESV_DIRECT_RELAXABLE;
> > +		else
> > +			type = IOMMU_RESV_DIRECT;
> > +
> > +		if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE)
> > +			prot |= IOMMU_PRIV;
> > +
> > +		/* Attributes 0x00 - 0x03 represents device memory */
> > +		if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) <=
> > +				ACPI_IORT_RMR_ATTR_DEVICE_GRE)
> > +			prot |= IOMMU_MMIO;
> > +		else if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) ==
> > +				ACPI_IORT_RMR_ATTR_NORMAL)
> > +			prot |= IOMMU_CACHE;
> > +
> > +		/* Create a copy of sids array to associate with this resv region */
> > +		sids_copy = kmemdup(sids, num_sids * sizeof(*sids), GFP_KERNEL);
> > +		if (!sids_copy)
> > +			return;
> > +
> > +		region = iommu_alloc_resv_region(addr, size, prot, type);
> > +		if (!region) {
> > +			kfree(sids_copy);
> > +			return;
> > +		}
> > +
> > +		region->fw_data.rmr.sids = sids_copy;
> > +		region->fw_data.rmr.num_sids = num_sids;
> > +		list_add_tail(&region->list, head);
> > +	}
> > +}
> > +
> > +static u32 *iort_rmr_alloc_sids(u32 *sids, u32 count, u32 id_start,
> > +				u32 new_count)
> > +{
> > +	u32 *new_sids;
> > +	u32 total_count = count + new_count;
> > +	int i;
> > +
> > +	new_sids = krealloc_array(sids, count + new_count,
> > +				  sizeof(*new_sids), GFP_KERNEL);
> > +	if (!new_sids)
> > +		return NULL;
> > +
> > +	/*Update new ones */
> 
> /* Update new ones */
> 
> I'd remove the comment, or improve it, as it stands it does not
> help much - we can read the code.

Yeah. Will remove that.

> > +	for (i = count; i < total_count; i++)
> > +		new_sids[i] = id_start++;
> > +
> > +	return new_sids;
> > +}
> > +
> > +static bool iort_rmr_has_dev(struct device *dev, u32 id_start,
> > +			     u32 id_count)
> > +{
> > +	int i;
> > +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > +
> > +	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)
> 
> Please add a comment here - it helps explain the logic behind it.

Ok. I will add a comment here and a reference to spec.

Thanks,
Shameer

> 
> Thanks,
> Lorenzo
> 
> > +			return false;
> > +	}
> > +
> > +	for (i = 0; i < fwspec->num_ids; i++) {
> > +		if (fwspec->ids[i] >= id_start &&
> > +		    fwspec->ids[i] <= id_start + id_count)
> > +			return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> > +static void iort_node_get_rmr_info(struct acpi_iort_node *node,
> > +				   struct acpi_iort_node *iommu,
> > +				   struct device *dev, struct list_head *head)
> > +{
> > +	struct acpi_iort_node *smmu = NULL;
> > +	struct acpi_iort_rmr *rmr;
> > +	struct acpi_iort_id_mapping *map;
> > +	u32 *sids = NULL;
> > +	u32 num_sids = 0;
> > +	int i;
> > +
> > +	if (!node->mapping_offset || !node->mapping_count) {
> > +		pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
> > +		       node);
> > +		return;
> > +	}
> > +
> > +	rmr = (struct acpi_iort_rmr *)node->node_data;
> > +	if (!rmr->rmr_offset || !rmr->rmr_count)
> > +		return;
> > +
> > +	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
> > +			   node->mapping_offset);
> > +
> > +	/*
> > +	 * Go through the ID mappings and see if we have a match
> > +	 * for smmu and dev(if !NULL). If found, get the sids
> > +	 * for the Node.
> > +	 * Please note, id_count is equal to the number of IDs
> > +	 * in the range minus one.
> > +	 */
> > +	for (i = 0; i < node->mapping_count; i++, map++) {
> > +		struct acpi_iort_node *parent;
> > +
> > +		if (!map->id_count)
> > +			continue;
> > +
> > +		parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> > +				      map->output_reference);
> > +		if (parent != iommu)
> > +			continue;
> > +
> > +		/* If dev is valid, check RMR node corresponds to the dev sid */
> > +		if (dev && !iort_rmr_has_dev(dev, map->output_base,
> > +					     map->id_count))
> > +			continue;
> > +
> > +		/* Retrieve sids associated with the Node. */
> > +		sids = iort_rmr_alloc_sids(sids, num_sids, map->output_base,
> > +					   map->id_count + 1);
> > +		if (!sids)
> > +			return;
> > +
> > +		num_sids += map->id_count + 1;
> > +	}
> > +
> > +	if (!sids)
> > +		return;
> > +
> > +	iort_rmr_get_resv_regions(node, smmu, sids, num_sids, head);
> > +	kfree(sids);
> > +}
> > +
> > +static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,
> > +			   struct list_head *head)
> > +{
> > +	struct acpi_table_iort *iort;
> > +	struct acpi_iort_node *iort_node, *iort_end;
> > +	int i;
> > +
> > +	if (iort_table->revision < 5)
> > +		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, iommu, dev, head);
> > +
> > +		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
> > +					 iort_node->length);
> > +	}
> > +}
> > +
> >  /**
> >   * iort_iommu_msi_get_resv_regions - Reserved region driver helper
> >   * @dev: Device from iommu_get_resv_regions()
> > --
> > 2.25.1
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR nodes

[Eric Auger]

Hi Shameer,

On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> The helper functions here parse through the IORT RMR nodes and
> populate a reserved region list  corresponding to a given iommu
> and device(optional). These also go through the ID mappings of
> the RMR node and retrieves all the SIDs associated with a RMR
> descriptor.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  drivers/acpi/arm64/iort.c | 225 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 225 insertions(+)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 0730c4dbb700..05da9ebff50a 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -830,6 +830,231 @@ static struct acpi_iort_node *iort_get_msi_resv_iommu(struct device *dev)
>  	return NULL;
>  }
>  
> +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);
> +		}
> +	}
> +}
> +
> +/*
> + * Please note, we will keep the already allocated RMR reserve
> + * regions in case of a memory allocation failure.
> + */
> +static void iort_rmr_get_resv_regions(struct acpi_iort_node *node,
> +				      struct acpi_iort_node *smmu,
> +				      u32 *sids, u32 num_sids,
> +				      struct list_head *head)
> +{
> +	struct acpi_iort_rmr *rmr = (struct acpi_iort_rmr *)node->node_data;
> +	struct acpi_iort_rmr_desc *rmr_desc;
> +	int i;
> +
> +	rmr_desc = ACPI_ADD_PTR(struct acpi_iort_rmr_desc, 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;
> +		u32  *sids_copy;
> +		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 & ACPI_IORT_RMR_REMAP_PERMITTED)
> +			type = IOMMU_RESV_DIRECT_RELAXABLE;
> +		else
> +			type = IOMMU_RESV_DIRECT;
> +
> +		if (rmr->flags & ACPI_IORT_RMR_ACCESS_PRIVILEGE)
> +			prot |= IOMMU_PRIV;
> +
> +		/* Attributes 0x00 - 0x03 represents device memory */
> +		if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) <=
> +				ACPI_IORT_RMR_ATTR_DEVICE_GRE)
> +			prot |= IOMMU_MMIO;
> +		else if (ACPI_IORT_RMR_ACCESS_ATTRIBUTES(rmr->flags) ==
> +				ACPI_IORT_RMR_ATTR_NORMAL)
> +			prot |= IOMMU_CACHE;
> +
> +		/* Create a copy of sids array to associate with this resv region */
> +		sids_copy = kmemdup(sids, num_sids * sizeof(*sids), GFP_KERNEL);
> +		if (!sids_copy)
> +			return;
> +
> +		region = iommu_alloc_resv_region(addr, size, prot, type);
> +		if (!region) {
> +			kfree(sids_copy);
> +			return;
> +		}
> +
> +		region->fw_data.rmr.sids = sids_copy;
> +		region->fw_data.rmr.num_sids = num_sids;
> +		list_add_tail(&region->list, head);
> +	}
> +}
> +
> +static u32 *iort_rmr_alloc_sids(u32 *sids, u32 count, u32 id_start,
> +				u32 new_count)
> +{
> +	u32 *new_sids;
> +	u32 total_count = count + new_count;
> +	int i;
> +
> +	new_sids = krealloc_array(sids, count + new_count,
> +				  sizeof(*new_sids), GFP_KERNEL);
> +	if (!new_sids)
> +		return NULL;
> +
> +	/*Update new ones */
> +	for (i = count; i < total_count; i++)
> +		new_sids[i] = id_start++;
> +
> +	return new_sids;
> +}
> +
> +static bool iort_rmr_has_dev(struct device *dev, u32 id_start,
> +			     u32 id_count)
> +{
> +	int i;
> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> +	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 false;
> +	}
> +
> +	for (i = 0; i < fwspec->num_ids; i++) {
> +		if (fwspec->ids[i] >= id_start &&
> +		    fwspec->ids[i] <= id_start + id_count)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void iort_node_get_rmr_info(struct acpi_iort_node *node,
> +				   struct acpi_iort_node *iommu,
> +				   struct device *dev, struct list_head *head)
> +{
> +	struct acpi_iort_node *smmu = NULL;
> +	struct acpi_iort_rmr *rmr;
> +	struct acpi_iort_id_mapping *map;
> +	u32 *sids = NULL;
> +	u32 num_sids = 0;
> +	int i;
> +
> +	if (!node->mapping_offset || !node->mapping_count) {
> +		pr_err(FW_BUG "Invalid ID mapping, skipping RMR node %p\n",
> +		       node);
> +		return;
> +	}
> +
> +	rmr = (struct acpi_iort_rmr *)node->node_data;
> +	if (!rmr->rmr_offset || !rmr->rmr_count)
> +		return;
> +
> +	map = ACPI_ADD_PTR(struct acpi_iort_id_mapping, node,
> +			   node->mapping_offset);
> +
> +	/*
> +	 * Go through the ID mappings and see if we have a match
> +	 * for smmu and dev(if !NULL). If found, get the sids
> +	 * for the Node.
> +	 * Please note, id_count is equal to the number of IDs
> +	 * in the range minus one.
> +	 */
> +	for (i = 0; i < node->mapping_count; i++, map++) {
> +		struct acpi_iort_node *parent;
> +
> +		if (!map->id_count)
> +			continue;
> +
> +		parent = ACPI_ADD_PTR(struct acpi_iort_node, iort_table,
> +				      map->output_reference);
> +		if (parent != iommu)
> +			continue;
> +
> +		/* If dev is valid, check RMR node corresponds to the dev sid */
> +		if (dev && !iort_rmr_has_dev(dev, map->output_base,
> +					     map->id_count))
> +			continue;
> +
> +		/* Retrieve sids associated with the Node. */
> +		sids = iort_rmr_alloc_sids(sids, num_sids, map->output_base,
> +					   map->id_count + 1);
> +		if (!sids)
> +			return;
> +
> +		num_sids += map->id_count + 1;
> +	}
> +
> +	if (!sids)
> +		return;
> +
> +	iort_rmr_get_resv_regions(node, smmu, sids, num_sids, head);
> +	kfree(sids);
> +}
> +
> +static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,
> +			   struct list_head *head)
> +{
> +	struct acpi_table_iort *iort;
> +	struct acpi_iort_node *iort_node, *iort_end;
> +	int i;
> +
> +	if (iort_table->revision < 5)
This means E.b and E.c revs are not supported. Is it what we want?

Thanks

Eric
> +		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, iommu, dev, head);
> +
> +		iort_node = ACPI_ADD_PTR(struct acpi_iort_node, iort_node,
> +					 iort_node->length);
> +	}
> +}
> +
>  /**
>   * iort_iommu_msi_get_resv_regions - Reserved region driver helper
>   * @dev: Device from iommu_get_resv_regions()


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

RE: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR nodes

[Shameerali Kolothum Thodi]

Hi Eric,

> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@redhat.com]
> Sent: 10 March 2022 10:32
> 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
> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
> joro@8bytes.org; robin.murphy@arm.com; will@kernel.org; wanghuiqiang
> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; steven.price@arm.com; Sami.Mujawar@arm.com;
> jon@solid-run.com; yangyicong <yangyicong@huawei.com>
> Subject: Re: [PATCH v8 03/11] ACPI/IORT: Add helper functions to parse RMR
> nodes
> 
> Hi Shameer,
> 
> On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> > The helper functions here parse through the IORT RMR nodes and
> > populate a reserved region list  corresponding to a given iommu
> > and device(optional). These also go through the ID mappings of
> > the RMR node and retrieves all the SIDs associated with a RMR
> > descriptor.
> >
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  drivers/acpi/arm64/iort.c | 225
> ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 225 insertions(+)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 0730c4dbb700..05da9ebff50a 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -830,6 +830,231 @@ static struct acpi_iort_node
> *iort_get_msi_resv_iommu(struct device *dev)
> >  	return NULL;
> >  }
> >
[...]

> > +static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,
> > +			   struct list_head *head)
> > +{
> > +	struct acpi_table_iort *iort;
> > +	struct acpi_iort_node *iort_node, *iort_end;
> > +	int i;
> > +
> > +	if (iort_table->revision < 5)
> This means E.b and E.c revs are not supported. Is it what we want?

Yes. E.b lacks memory attributes info associated with RMR node. Though E.c 
added those, it broke backward compatibility with ACPICA E.b support and
is now deprecated.

Thanks,
Shameer


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

[PATCH v8 04/11] iommu/dma: Introduce generic helper to retrieve RMR info

[Shameer Kolothum]

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 <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/dma-iommu.c | 21 +++++++++++++++++++++
 include/linux/dma-iommu.h | 14 ++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d85d54f2b549..65ab01d5128b 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -370,6 +370,27 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 	domain->iova_cookie = NULL;
 }
 
+/**
+ *
+ * iommu_dma_get_rmrs - Retrieve Reserved Memory Regions(RMRs) associated
+ *                      with a given IOMMU and device(optional)
+ * @iommu_fwnode: fwnode associated with IOMMU
+ * @dev: Optional associated device
+ * @list: Reserved region list to be populated
+ *
+ */
+void iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
+			struct list_head *list)
+{
+}
+EXPORT_SYMBOL(iommu_dma_get_rmrs);
+
+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 24607dc3c2ac..897b457cad2c 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -43,12 +43,17 @@ void iommu_dma_free_cpu_cached_iovas(unsigned int cpu,
 
 extern bool iommu_dma_forcedac;
 
+void iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct device *dev,
+			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)
@@ -89,5 +94,14 @@ static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_he
 {
 }
 
+static inline void iommu_dma_get_rmrs(struct fwnode_handle *iommu, struct device *dev,
+				      struct list_head *list)
+{
+}
+
+static inline void iommu_dma_put_rmrs(struct fwnode_handle *iommu, struct list_head *list)
+{
+}
+
 #endif	/* CONFIG_IOMMU_DMA */
 #endif	/* __DMA_IOMMU_H */
-- 
2.25.1


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

[PATCH v8 05/11] ACPI/IORT: Add a helper to retrieve RMR memory regions

[Shameer Kolothum]

Add helper functions (iort_iommu_get/put_rmrs()) that
retrieves/releases RMR memory descriptors associated
with a given IOMMU. This will be used by IOMMU drivers
to set up necessary mappings.

Invoke it from the generic iommu helper functions.

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

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 05da9ebff50a..b2c959c72fb2 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1055,6 +1055,57 @@ static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,
 	}
 }
 
+/**
+ * iort_iommu_dma_put_rmrs - Free any memory associated with RMRs.
+ * @iommu_fwnode: fwnode associated with IOMMU
+ * @head: Resereved region list
+ *
+ * This function go through the provided reserved region list and
+ * free up memory associated with RMR entries and delete them from
+ * the list.
+ */
+void iort_iommu_put_rmrs(struct fwnode_handle *iommu_fwnode,
+			 struct list_head *head)
+{
+	struct iommu_resv_region *e, *tmp;
+
+	/*
+	 * RMR entries will have mem allocated for fw_data.rmr.sids.
+	 * Free the mem and delete the node.
+	 */
+	list_for_each_entry_safe(e, tmp, head, list) {
+		if (e->fw_data.rmr.sids) {
+			kfree(e->fw_data.rmr.sids);
+			list_del(&e->list);
+			kfree(e);
+		}
+	}
+}
+
+/**
+ *
+ * iort_iommu_dma_get_rmrs - Retrieve Reserved Memory Regions(RMRs) associated
+ *                      with a given IOMMU and dev.
+ * @iommu_fwnode: fwnode associated with IOMMU
+ * @dev: Device associated with RMR(Optional)
+ * @list: RMR list to be populated
+ *
+ * This function populates the RMR list associated with a given IOMMU and
+ * dev(if provided). If dev is NULL, the function populates all the RMRs
+ * associated with the given IOMMU.
+ */
+void iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
+			 struct list_head *head)
+{
+	struct acpi_iort_node *iommu;
+
+	iommu = iort_get_iort_node(iommu_fwnode);
+	if (!iommu)
+		return;
+
+	iort_find_rmrs(iommu, dev, head);
+}
+
 /**
  * iort_iommu_msi_get_resv_regions - Reserved region driver helper
  * @dev: Device from iommu_get_resv_regions()
@@ -1287,6 +1338,11 @@ 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; }
+void iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct device *dev,
+			 struct list_head *head)
+{ }
+void iort_iommu_put_rmrs(struct fwnode_handle *fwnode, struct list_head *head)
+{ }
 #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 65ab01d5128b..b33e4df85de1 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -382,12 +382,16 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 void iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
 			struct list_head *list)
 {
+	if (!is_of_node(iommu_fwnode))
+		iort_iommu_get_rmrs(iommu_fwnode, dev, list);
 }
 EXPORT_SYMBOL(iommu_dma_get_rmrs);
 
 void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode,
 			struct list_head *list)
 {
+	if (!is_of_node(iommu_fwnode))
+		iort_iommu_put_rmrs(iommu_fwnode, list);
 }
 EXPORT_SYMBOL(iommu_dma_put_rmrs);
 
diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
index f1f0842a2cb2..212f7f178ec3 100644
--- a/include/linux/acpi_iort.h
+++ b/include/linux/acpi_iort.h
@@ -38,6 +38,10 @@ 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);
+void iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
+			 struct list_head *list);
+void iort_iommu_put_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 +61,16 @@ 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
+void iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
+			 struct list_head *list)
+{  }
+void iort_iommu_put_rmrs(struct fwnode_handle *iommu_fwnode,
+			 struct list_head *list)
+{ }
+
+{ }
 #endif
 
 #endif /* __ACPI_IORT_H__ */
-- 
2.25.1


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

Re: [PATCH v8 05/11] ACPI/IORT: Add a helper to retrieve RMR memory regions

[Lorenzo Pieralisi]

On Mon, Feb 21, 2022 at 03:43:38PM +0000, Shameer Kolothum wrote:
> Add helper functions (iort_iommu_get/put_rmrs()) that
> retrieves/releases RMR memory descriptors associated
> with a given IOMMU. This will be used by IOMMU drivers
> to set up necessary mappings.
> 
> Invoke it from the generic iommu helper functions.
> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  drivers/acpi/arm64/iort.c | 56 +++++++++++++++++++++++++++++++++++++++
>  drivers/iommu/dma-iommu.c |  4 +++
>  include/linux/acpi_iort.h | 14 ++++++++++
>  3 files changed, 74 insertions(+)

I would squash this patch with the previous one - at least the
iommu_dma_get/put_rmrs() are actually used in the patch that is
adding them.

Lorenzo

> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 05da9ebff50a..b2c959c72fb2 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1055,6 +1055,57 @@ static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,
>  	}
>  }
>  
> +/**
> + * iort_iommu_dma_put_rmrs - Free any memory associated with RMRs.
> + * @iommu_fwnode: fwnode associated with IOMMU
> + * @head: Resereved region list
> + *
> + * This function go through the provided reserved region list and
> + * free up memory associated with RMR entries and delete them from
> + * the list.
> + */
> +void iort_iommu_put_rmrs(struct fwnode_handle *iommu_fwnode,
> +			 struct list_head *head)
> +{
> +	struct iommu_resv_region *e, *tmp;
> +
> +	/*
> +	 * RMR entries will have mem allocated for fw_data.rmr.sids.
> +	 * Free the mem and delete the node.
> +	 */
> +	list_for_each_entry_safe(e, tmp, head, list) {
> +		if (e->fw_data.rmr.sids) {
> +			kfree(e->fw_data.rmr.sids);
> +			list_del(&e->list);
> +			kfree(e);
> +		}
> +	}
> +}
> +
> +/**
> + *
> + * iort_iommu_dma_get_rmrs - Retrieve Reserved Memory Regions(RMRs) associated
> + *                      with a given IOMMU and dev.
> + * @iommu_fwnode: fwnode associated with IOMMU
> + * @dev: Device associated with RMR(Optional)
> + * @list: RMR list to be populated
> + *
> + * This function populates the RMR list associated with a given IOMMU and
> + * dev(if provided). If dev is NULL, the function populates all the RMRs
> + * associated with the given IOMMU.
> + */
> +void iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
> +			 struct list_head *head)
> +{
> +	struct acpi_iort_node *iommu;
> +
> +	iommu = iort_get_iort_node(iommu_fwnode);
> +	if (!iommu)
> +		return;
> +
> +	iort_find_rmrs(iommu, dev, head);
> +}
> +
>  /**
>   * iort_iommu_msi_get_resv_regions - Reserved region driver helper
>   * @dev: Device from iommu_get_resv_regions()
> @@ -1287,6 +1338,11 @@ 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; }
> +void iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct device *dev,
> +			 struct list_head *head)
> +{ }
> +void iort_iommu_put_rmrs(struct fwnode_handle *fwnode, struct list_head *head)
> +{ }
>  #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 65ab01d5128b..b33e4df85de1 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -382,12 +382,16 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
>  void iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
>  			struct list_head *list)
>  {
> +	if (!is_of_node(iommu_fwnode))
> +		iort_iommu_get_rmrs(iommu_fwnode, dev, list);
>  }
>  EXPORT_SYMBOL(iommu_dma_get_rmrs);
>  
>  void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode,
>  			struct list_head *list)
>  {
> +	if (!is_of_node(iommu_fwnode))
> +		iort_iommu_put_rmrs(iommu_fwnode, list);
>  }
>  EXPORT_SYMBOL(iommu_dma_put_rmrs);
>  
> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
> index f1f0842a2cb2..212f7f178ec3 100644
> --- a/include/linux/acpi_iort.h
> +++ b/include/linux/acpi_iort.h
> @@ -38,6 +38,10 @@ 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);
> +void iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
> +			 struct list_head *list);
> +void iort_iommu_put_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 +61,16 @@ 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
> +void iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
> +			 struct list_head *list)
> +{  }
> +void iort_iommu_put_rmrs(struct fwnode_handle *iommu_fwnode,
> +			 struct list_head *list)
> +{ }
> +
> +{ }
>  #endif
>  
>  #endif /* __ACPI_IORT_H__ */
> -- 
> 2.25.1
> 

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

Re: [PATCH v8 05/11] ACPI/IORT: Add a helper to retrieve RMR memory regions

[Robin Murphy]

On 2022-02-21 15:43, Shameer Kolothum via iommu wrote:
> Add helper functions (iort_iommu_get/put_rmrs()) that
> retrieves/releases RMR memory descriptors associated
> with a given IOMMU. This will be used by IOMMU drivers
> to set up necessary mappings.
> 
> Invoke it from the generic iommu helper functions.

iommu_dma_get_resv_regions() already exists - please extend that rather 
than adding a parallel implementation of the same thing but different. 
IORT should export a single get_resv_regions helper which combines the 
new RMRs with the existing MSI workaround, and a separate "do I need to 
bypass this StreamID" helper for the SMMU drivers to call directly at 
reset time, since the latter isn't really an iommu-dma responsibility.

I'm happy to do that just by shuffling wrappers around for now - we can 
come back and streamline the code properly afterwards - but the sheer 
amount of indirection currently at play here is so hard to follow that 
it's not even all that easy to see how it's crossing abstraction levels 
improperly.

Thanks,
Robin.

> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>   drivers/acpi/arm64/iort.c | 56 +++++++++++++++++++++++++++++++++++++++
>   drivers/iommu/dma-iommu.c |  4 +++
>   include/linux/acpi_iort.h | 14 ++++++++++
>   3 files changed, 74 insertions(+)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 05da9ebff50a..b2c959c72fb2 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1055,6 +1055,57 @@ static void iort_find_rmrs(struct acpi_iort_node *iommu, struct device *dev,
>   	}
>   }
>   
> +/**
> + * iort_iommu_dma_put_rmrs - Free any memory associated with RMRs.
> + * @iommu_fwnode: fwnode associated with IOMMU
> + * @head: Resereved region list
> + *
> + * This function go through the provided reserved region list and
> + * free up memory associated with RMR entries and delete them from
> + * the list.
> + */
> +void iort_iommu_put_rmrs(struct fwnode_handle *iommu_fwnode,
> +			 struct list_head *head)
> +{
> +	struct iommu_resv_region *e, *tmp;
> +
> +	/*
> +	 * RMR entries will have mem allocated for fw_data.rmr.sids.
> +	 * Free the mem and delete the node.
> +	 */
> +	list_for_each_entry_safe(e, tmp, head, list) {
> +		if (e->fw_data.rmr.sids) {
> +			kfree(e->fw_data.rmr.sids);
> +			list_del(&e->list);
> +			kfree(e);
> +		}
> +	}
> +}
> +
> +/**
> + *
> + * iort_iommu_dma_get_rmrs - Retrieve Reserved Memory Regions(RMRs) associated
> + *                      with a given IOMMU and dev.
> + * @iommu_fwnode: fwnode associated with IOMMU
> + * @dev: Device associated with RMR(Optional)
> + * @list: RMR list to be populated
> + *
> + * This function populates the RMR list associated with a given IOMMU and
> + * dev(if provided). If dev is NULL, the function populates all the RMRs
> + * associated with the given IOMMU.
> + */
> +void iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
> +			 struct list_head *head)
> +{
> +	struct acpi_iort_node *iommu;
> +
> +	iommu = iort_get_iort_node(iommu_fwnode);
> +	if (!iommu)
> +		return;
> +
> +	iort_find_rmrs(iommu, dev, head);
> +}
> +
>   /**
>    * iort_iommu_msi_get_resv_regions - Reserved region driver helper
>    * @dev: Device from iommu_get_resv_regions()
> @@ -1287,6 +1338,11 @@ 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; }
> +void iort_iommu_get_rmrs(struct fwnode_handle *fwnode, struct device *dev,
> +			 struct list_head *head)
> +{ }
> +void iort_iommu_put_rmrs(struct fwnode_handle *fwnode, struct list_head *head)
> +{ }
>   #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 65ab01d5128b..b33e4df85de1 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -382,12 +382,16 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
>   void iommu_dma_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
>   			struct list_head *list)
>   {
> +	if (!is_of_node(iommu_fwnode))
> +		iort_iommu_get_rmrs(iommu_fwnode, dev, list);
>   }
>   EXPORT_SYMBOL(iommu_dma_get_rmrs);
>   
>   void iommu_dma_put_rmrs(struct fwnode_handle *iommu_fwnode,
>   			struct list_head *list)
>   {
> +	if (!is_of_node(iommu_fwnode))
> +		iort_iommu_put_rmrs(iommu_fwnode, list);
>   }
>   EXPORT_SYMBOL(iommu_dma_put_rmrs);
>   
> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
> index f1f0842a2cb2..212f7f178ec3 100644
> --- a/include/linux/acpi_iort.h
> +++ b/include/linux/acpi_iort.h
> @@ -38,6 +38,10 @@ 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);
> +void iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
> +			 struct list_head *list);
> +void iort_iommu_put_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 +61,16 @@ 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
> +void iort_iommu_get_rmrs(struct fwnode_handle *iommu_fwnode, struct device *dev,
> +			 struct list_head *list)
> +{  }
> +void iort_iommu_put_rmrs(struct fwnode_handle *iommu_fwnode,
> +			 struct list_head *list)
> +{ }
> +
> +{ }
>   #endif
>   
>   #endif /* __ACPI_IORT_H__ */

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

RE: [PATCH v8 05/11] ACPI/IORT: Add a helper to retrieve RMR memory regions

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy@arm.com]
> Sent: 22 March 2022 19:09
> 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
> Cc: jon@solid-run.com; Linuxarm <linuxarm@huawei.com>;
> steven.price@arm.com; Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
> yangyicong <yangyicong@huawei.com>; Sami.Mujawar@arm.com;
> will@kernel.org; wanghuiqiang <wanghuiqiang@huawei.com>
> Subject: Re: [PATCH v8 05/11] ACPI/IORT: Add a helper to retrieve RMR
> memory regions
> 
> On 2022-02-21 15:43, Shameer Kolothum via iommu wrote:
> > Add helper functions (iort_iommu_get/put_rmrs()) that
> > retrieves/releases RMR memory descriptors associated
> > with a given IOMMU. This will be used by IOMMU drivers
> > to set up necessary mappings.
> >
> > Invoke it from the generic iommu helper functions.
> 
> iommu_dma_get_resv_regions() already exists - please extend that rather
> than adding a parallel implementation of the same thing but different.
> IORT should export a single get_resv_regions helper which combines the
> new RMRs with the existing MSI workaround, and a separate "do I need to
> bypass this StreamID" helper for the SMMU drivers to call directly at
> reset time, since the latter isn't really an iommu-dma responsibility.

Right. I actually had couple of back and forth on the interfaces and settled
on this mainly because it just requires a single interface from IORT for both
get_resv_regions() and SMMU driver reset bypass path.
ie, iort_iommu_get/put_rmrs()).

But agree the above comment is also valid. 

> I'm happy to do that just by shuffling wrappers around for now - we can
> come back and streamline the code properly afterwards - but the sheer
> amount of indirection currently at play here is so hard to follow that
> it's not even all that easy to see how it's crossing abstraction levels
> improperly.

Please find below the revised ones. Please take a look and let me know.

Thanks,
Shameer

iommu-dma:

void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) {

        if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
                iort_iommu_get_resv_regions(dev, list);
}

void iommu_dma_put_resv_regions(struct device *dev, struct list_head *list) {

        if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
                iort_iommu_put_resv_regions(dev, list);
        generic_iommu_put_resv_regions(dev, list);
}

iort:

void iort_iommu_get_resv_regions(struct device *dev, struct list_head *head) {
        struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);

        iort_iommu_msi_get_resv_regions(dev, head);
        iort_iommu_get_rmrs(fwspec->iommu_fwnode, dev, head);
}

void iort_iommu_put_resv_regions(struct device *dev, struct list_head *head) {
   ./*Free both RMRs and HW MSI ones */
}

/* The below ones will be directly called from SMMU drivers during reset */
void iort_get_rmr_sids(struct fwnode_handle *iommu_fwnode, struct list_head *head) {
        iort_iommu_get_rmrs(iommu_fwnode, NULL, head); }
}

void iort_put_rmr_sids(struct fwnode_handle *iommu_fwnode, struct list_head *head) {
        iort_iommu_put_resv_regions(NULL, head);
}
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v8 05/11] ACPI/IORT: Add a helper to retrieve RMR memory regions

[Robin Murphy]

On 2022-03-23 16:06, Shameerali Kolothum Thodi wrote:
> 
> 
>> -----Original Message-----
>> From: Robin Murphy [mailto:robin.murphy@arm.com]
>> Sent: 22 March 2022 19:09
>> 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
>> Cc: jon@solid-run.com; Linuxarm <linuxarm@huawei.com>;
>> steven.price@arm.com; Guohanjun (Hanjun Guo) <guohanjun@huawei.com>;
>> yangyicong <yangyicong@huawei.com>; Sami.Mujawar@arm.com;
>> will@kernel.org; wanghuiqiang <wanghuiqiang@huawei.com>
>> Subject: Re: [PATCH v8 05/11] ACPI/IORT: Add a helper to retrieve RMR
>> memory regions
>>
>> On 2022-02-21 15:43, Shameer Kolothum via iommu wrote:
>>> Add helper functions (iort_iommu_get/put_rmrs()) that
>>> retrieves/releases RMR memory descriptors associated
>>> with a given IOMMU. This will be used by IOMMU drivers
>>> to set up necessary mappings.
>>>
>>> Invoke it from the generic iommu helper functions.
>>
>> iommu_dma_get_resv_regions() already exists - please extend that rather
>> than adding a parallel implementation of the same thing but different.
>> IORT should export a single get_resv_regions helper which combines the
>> new RMRs with the existing MSI workaround, and a separate "do I need to
>> bypass this StreamID" helper for the SMMU drivers to call directly at
>> reset time, since the latter isn't really an iommu-dma responsibility.
> 
> Right. I actually had couple of back and forth on the interfaces and settled
> on this mainly because it just requires a single interface from IORT for both
> get_resv_regions() and SMMU driver reset bypass path.
> ie, iort_iommu_get/put_rmrs()).
> 
> But agree the above comment is also valid.
> 
>> I'm happy to do that just by shuffling wrappers around for now - we can
>> come back and streamline the code properly afterwards - but the sheer
>> amount of indirection currently at play here is so hard to follow that
>> it's not even all that easy to see how it's crossing abstraction levels
>> improperly.
> 
> Please find below the revised ones. Please take a look and let me know.

Yeah, that looks like it should work!

Cheers,
Robin.

> 
> Thanks,
> Shameer
> 
> iommu-dma:
> 
> void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list) {
> 
>          if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
>                  iort_iommu_get_resv_regions(dev, list);
> }
> 
> void iommu_dma_put_resv_regions(struct device *dev, struct list_head *list) {
> 
>          if (!is_of_node(dev_iommu_fwspec_get(dev)->iommu_fwnode))
>                  iort_iommu_put_resv_regions(dev, list);
>          generic_iommu_put_resv_regions(dev, list);
> }
> 
> iort:
> 
> void iort_iommu_get_resv_regions(struct device *dev, struct list_head *head) {
>          struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> 
>          iort_iommu_msi_get_resv_regions(dev, head);
>          iort_iommu_get_rmrs(fwspec->iommu_fwnode, dev, head);
> }
> 
> void iort_iommu_put_resv_regions(struct device *dev, struct list_head *head) {
>     ./*Free both RMRs and HW MSI ones */
> }
> 
> /* The below ones will be directly called from SMMU drivers during reset */
> void iort_get_rmr_sids(struct fwnode_handle *iommu_fwnode, struct list_head *head) {
>          iort_iommu_get_rmrs(iommu_fwnode, NULL, head); }
> }
> 
> void iort_put_rmr_sids(struct fwnode_handle *iommu_fwnode, struct list_head *head) {
>          iort_iommu_put_resv_regions(NULL, head);
> }

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

[PATCH v8 06/11] iommu/arm-smmu-v3: Introduce strtab init helper

[Shameer Kolothum]

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 <shameerali.kolothum.thodi@huawei.com>
---
 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 6dc6d8b6b368..ffabb6926da2 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2536,6 +2536,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)
 {
@@ -2559,20 +2572,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.25.1


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

[PATCH v8 07/11] iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force bypass

[Shameer Kolothum]

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 <shameerali.kolothum.thodi@huawei.com>
---
 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 ffabb6926da2..04033d4dd1ac 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1380,12 +1380,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;
 	}
 }
@@ -1413,7 +1422,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;
 }
@@ -3060,7 +3069,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.25.1


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

[PATCH v8 08/11] iommu/arm-smmu-v3: Get associated RMR info and install bypass STE

[Shameer Kolothum]

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 <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 33 +++++++++++++++++++++
 1 file changed, 33 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 04033d4dd1ac..dee3197474b7 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3763,6 +3763,36 @@ 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;
+
+	INIT_LIST_HEAD(&rmr_list);
+	iommu_dma_get_rmrs(dev_fwnode(smmu->dev), NULL, &rmr_list);
+
+	list_for_each_entry(e, &rmr_list, list) {
+		__le64 *step;
+		u32 *sids = e->fw_data.rmr.sids;
+		u32 num_sids = e->fw_data.rmr.num_sids;
+		int ret, i;
+
+		for (i = 0; i < num_sids; i++) {
+			ret = arm_smmu_init_sid_strtab(smmu, sids[i]);
+			if (ret) {
+				dev_err(smmu->dev, "RMR SID(0x%x) bypass failed\n",
+					sids[i]);
+				continue;
+			}
+
+			step = arm_smmu_get_step_for_sid(smmu, sids[i]);
+			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;
@@ -3844,6 +3874,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.25.1


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

[PATCH v8 09/11] iommu/arm-smmu: Get associated RMR info and install bypass SMR

[Shameer Kolothum]

From: Jon Nettleton <jon@solid-run.com>

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 <jon@solid-run.com>
Signed-off-by: Steven Price <steven.price@arm.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm/arm-smmu/arm-smmu.c | 52 +++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 4bc75c4ce402..19dcae41d1aa 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -2066,6 +2066,54 @@ 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 idx, cnt = 0;
+	u32 reg;
+
+	INIT_LIST_HEAD(&rmr_list);
+	iommu_dma_get_rmrs(dev_fwnode(smmu->dev), NULL, &rmr_list);
+
+	/*
+	 * 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 *sids = e->fw_data.rmr.sids;
+		u32 num_sids = e->fw_data.rmr.num_sids;
+		int i;
+
+		for (i = 0; i < num_sids; i++) {
+			idx = arm_smmu_find_sme(smmu, sids[i], ~0);
+			if (idx < 0)
+				continue;
+
+			if (smmu->s2crs[idx].count == 0) {
+				smmu->smrs[idx].id = sids[i];
+				smmu->smrs[idx].mask = 0;
+				smmu->smrs[idx].valid = true;
+			}
+			smmu->s2crs[idx].count++;
+			smmu->s2crs[idx].type = S2CR_TYPE_BYPASS;
+			smmu->s2crs[idx].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;
@@ -2192,6 +2240,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.25.1


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

[PATCH v8 10/11] iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev

[Shameer Kolothum]

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 <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

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 dee3197474b7..ef2972483fd7 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2759,6 +2759,7 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 {
 	struct iommu_resv_region *region;
 	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 
 	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
 					 prot, IOMMU_RESV_SW_MSI);
@@ -2768,6 +2769,16 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 	list_add_tail(&region->list, head);
 
 	iommu_dma_get_resv_regions(dev, head);
+	iommu_dma_get_rmrs(fwspec->iommu_fwnode, dev, head);
+}
+
+static void arm_smmu_put_resv_regions(struct device *dev,
+				      struct list_head *head)
+{
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+
+	iommu_dma_put_rmrs(fwspec->iommu_fwnode, head);
+	generic_iommu_put_resv_regions(dev, head);
 }
 
 static bool arm_smmu_dev_has_feature(struct device *dev,
@@ -2865,7 +2876,7 @@ static struct iommu_ops arm_smmu_ops = {
 	.enable_nesting		= arm_smmu_enable_nesting,
 	.of_xlate		= arm_smmu_of_xlate,
 	.get_resv_regions	= arm_smmu_get_resv_regions,
-	.put_resv_regions	= generic_iommu_put_resv_regions,
+	.put_resv_regions	= arm_smmu_put_resv_regions,
 	.dev_has_feat		= arm_smmu_dev_has_feature,
 	.dev_feat_enabled	= arm_smmu_dev_feature_enabled,
 	.dev_enable_feat	= arm_smmu_dev_enable_feature,
-- 
2.25.1


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

Re: [PATCH v8 10/11] iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev

[Robin Murphy]

On 2022-02-21 15:43, Shameer Kolothum wrote:
> 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 <shameerali.kolothum.thodi@huawei.com>
> ---
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> 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 dee3197474b7..ef2972483fd7 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2759,6 +2759,7 @@ static void arm_smmu_get_resv_regions(struct device *dev,
>   {
>   	struct iommu_resv_region *region;
>   	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>   
>   	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
>   					 prot, IOMMU_RESV_SW_MSI);
> @@ -2768,6 +2769,16 @@ static void arm_smmu_get_resv_regions(struct device *dev,
>   	list_add_tail(&region->list, head);
>   
>   	iommu_dma_get_resv_regions(dev, head);
> +	iommu_dma_get_rmrs(fwspec->iommu_fwnode, dev, head);
> +}
> +
> +static void arm_smmu_put_resv_regions(struct device *dev,
> +				      struct list_head *head)
> +{
> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> +
> +	iommu_dma_put_rmrs(fwspec->iommu_fwnode, head);
> +	generic_iommu_put_resv_regions(dev, head);
>   }

Tying in with my comment on patch #5, this should be a common 
iommu_dma_put_resv_regions() helper.

Thanks,
Robin.

>   static bool arm_smmu_dev_has_feature(struct device *dev,
> @@ -2865,7 +2876,7 @@ static struct iommu_ops arm_smmu_ops = {
>   	.enable_nesting		= arm_smmu_enable_nesting,
>   	.of_xlate		= arm_smmu_of_xlate,
>   	.get_resv_regions	= arm_smmu_get_resv_regions,
> -	.put_resv_regions	= generic_iommu_put_resv_regions,
> +	.put_resv_regions	= arm_smmu_put_resv_regions,
>   	.dev_has_feat		= arm_smmu_dev_has_feature,
>   	.dev_feat_enabled	= arm_smmu_dev_feature_enabled,
>   	.dev_enable_feat	= arm_smmu_dev_enable_feature,

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

[PATCH v8 11/11] iommu/arm-smmu: Reserve any RMR regions associated with a dev

[Shameer Kolothum]

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 <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm/arm-smmu/arm-smmu.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 19dcae41d1aa..55b9d8e525d3 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1558,6 +1558,7 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 {
 	struct iommu_resv_region *region;
 	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 
 	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
 					 prot, IOMMU_RESV_SW_MSI);
@@ -1567,6 +1568,16 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 	list_add_tail(&region->list, head);
 
 	iommu_dma_get_resv_regions(dev, head);
+	iommu_dma_get_rmrs(fwspec->iommu_fwnode, dev, head);
+}
+
+static void arm_smmu_put_resv_regions(struct device *dev,
+				      struct list_head *head)
+{
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+
+	iommu_dma_put_rmrs(fwspec->iommu_fwnode, head);
+	generic_iommu_put_resv_regions(dev, head);
 }
 
 static int arm_smmu_def_domain_type(struct device *dev)
@@ -1598,7 +1609,7 @@ static struct iommu_ops arm_smmu_ops = {
 	.set_pgtable_quirks	= arm_smmu_set_pgtable_quirks,
 	.of_xlate		= arm_smmu_of_xlate,
 	.get_resv_regions	= arm_smmu_get_resv_regions,
-	.put_resv_regions	= generic_iommu_put_resv_regions,
+	.put_resv_regions	= arm_smmu_put_resv_regions,
 	.def_domain_type	= arm_smmu_def_domain_type,
 	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
 	.owner			= THIS_MODULE,
-- 
2.25.1


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

Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Steven Price]

On 21/02/2022 15:43, Shameer Kolothum wrote:
> Hi,
> 
> Since we now have an updated verion[0] of IORT spec(E.d) which
> addresses the memory attributes issues discussed here [1],
> this series now make use of it.
> 
> The pull request for ACPICA E.d related changes are already
> raised and can be found here,
> https://github.com/acpica/acpica/pull/752
> 
> v7 --> v8
>   - Patch #1 has temp definitions for RMR related changes till
>     the ACPICA header changes are part of kernel.
>   - No early parsing of RMR node info and is only parsed at the
>     time of use.
>   - Changes to the RMR get/put API format compared to the
>     previous version.
>   - Support for RMR descriptor shared by multiple stream IDs.
> 
> Please take a look and let me know your thoughts.

Hi Shameer,

I've now been able to test this on the Juno platform with a modified
firmware supporting the newer spec (thanks Sami!). Everything works, so
feel free to add my:

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

(Note that I haven't tested the smmu-v3 support)

Thanks,

Steve

> Thanks,
> Shameer
> [0] https://developer.arm.com/documentation/den0049/ed/
> [1] https://lore.kernel.org/linux-acpi/20210805160319.GB23085@lpieralisi/
> 
> From old:
> 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
>  -fix pointed out by Steve to the SMMUv2 SMR bypass install in patch #8.
> 
> 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.
> 
> 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 (10):
>   ACPI/IORT: Add temporary RMR node flag definitions
>   iommu: Introduce a union to struct iommu_resv_region
>   ACPI/IORT: Add helper functions to parse RMR nodes
>   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/arm-smmu-v3: Reserve any RMR regions associated with a dev
>   iommu/arm-smmu: Reserve any RMR regions associated with a dev
> 
>  drivers/acpi/arm64/iort.c                   | 305 ++++++++++++++++++++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  91 ++++--
>  drivers/iommu/arm/arm-smmu/arm-smmu.c       |  65 ++++-
>  drivers/iommu/dma-iommu.c                   |  25 ++
>  include/linux/acpi_iort.h                   |  14 +
>  include/linux/dma-iommu.h                   |  14 +
>  include/linux/iommu.h                       |   9 +
>  7 files changed, 504 insertions(+), 19 deletions(-)
> 


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

RE: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Steven Price [mailto:steven.price@arm.com]
> Sent: 03 March 2022 10:38
> 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
> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
> joro@8bytes.org; robin.murphy@arm.com; will@kernel.org; wanghuiqiang
> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; Sami.Mujawar@arm.com; jon@solid-run.com;
> eric.auger@redhat.com; yangyicong <yangyicong@huawei.com>
> Subject: Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node
> 
> On 21/02/2022 15:43, Shameer Kolothum wrote:
> > Hi,
> >
> > Since we now have an updated verion[0] of IORT spec(E.d) which
> > addresses the memory attributes issues discussed here [1],
> > this series now make use of it.
> >
> > The pull request for ACPICA E.d related changes are already
> > raised and can be found here,
> > https://github.com/acpica/acpica/pull/752
> >
> > v7 --> v8
> >   - Patch #1 has temp definitions for RMR related changes till
> >     the ACPICA header changes are part of kernel.
> >   - No early parsing of RMR node info and is only parsed at the
> >     time of use.
> >   - Changes to the RMR get/put API format compared to the
> >     previous version.
> >   - Support for RMR descriptor shared by multiple stream IDs.
> >
> > Please take a look and let me know your thoughts.
> 
> Hi Shameer,
> 
> I've now been able to test this on the Juno platform with a modified
> firmware supporting the newer spec (thanks Sami!). Everything works, so
> feel free to add my:
> 
> Tested-by: Steven Price <steven.price@arm.com>
> 
> (Note that I haven't tested the smmu-v3 support)

Thanks Steve, for giving it a spin and verifying.

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

Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Eric Auger]

Hi Shameer,

On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> Hi,
>
> Since we now have an updated verion[0] of IORT spec(E.d) which
> addresses the memory attributes issues discussed here [1],
> this series now make use of it.
>
> The pull request for ACPICA E.d related changes are already
> raised and can be found here,
> https://github.com/acpica/acpica/pull/752
>
> v7 --> v8
>   - Patch #1 has temp definitions for RMR related changes till
>     the ACPICA header changes are part of kernel.
>   - No early parsing of RMR node info and is only parsed at the
>     time of use.
>   - Changes to the RMR get/put API format compared to the
>     previous version.
>   - Support for RMR descriptor shared by multiple stream IDs.

I tested it on guest side for host MSI SW RESV region flat mapping
(using both the old single mapping layout and the now allowed multiple
RID ID mapping format) and this worked for me. Feel free to add my

Tested-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric


>
> Please take a look and let me know your thoughts.
>
> Thanks,
> Shameer
> [0] https://developer.arm.com/documentation/den0049/ed/
> [1] https://lore.kernel.org/linux-acpi/20210805160319.GB23085@lpieralisi/
>
> From old:
> 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
>  -fix pointed out by Steve to the SMMUv2 SMR bypass install in patch #8.
>
> 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.
>
> 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 (10):
>   ACPI/IORT: Add temporary RMR node flag definitions
>   iommu: Introduce a union to struct iommu_resv_region
>   ACPI/IORT: Add helper functions to parse RMR nodes
>   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/arm-smmu-v3: Reserve any RMR regions associated with a dev
>   iommu/arm-smmu: Reserve any RMR regions associated with a dev
>
>  drivers/acpi/arm64/iort.c                   | 305 ++++++++++++++++++++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  91 ++++--
>  drivers/iommu/arm/arm-smmu/arm-smmu.c       |  65 ++++-
>  drivers/iommu/dma-iommu.c                   |  25 ++
>  include/linux/acpi_iort.h                   |  14 +
>  include/linux/dma-iommu.h                   |  14 +
>  include/linux/iommu.h                       |   9 +
>  7 files changed, 504 insertions(+), 19 deletions(-)
>


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

RE: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@redhat.com]
> Sent: 11 March 2022 08:07
> 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
> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
> joro@8bytes.org; robin.murphy@arm.com; will@kernel.org; wanghuiqiang
> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; steven.price@arm.com; Sami.Mujawar@arm.com;
> jon@solid-run.com; yangyicong <yangyicong@huawei.com>
> Subject: Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node
> 
> Hi Shameer,
> 
> On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> > Hi,
> >
> > Since we now have an updated verion[0] of IORT spec(E.d) which
> > addresses the memory attributes issues discussed here [1],
> > this series now make use of it.
> >
> > The pull request for ACPICA E.d related changes are already
> > raised and can be found here,
> > https://github.com/acpica/acpica/pull/752
> >
> > v7 --> v8
> >   - Patch #1 has temp definitions for RMR related changes till
> >     the ACPICA header changes are part of kernel.
> >   - No early parsing of RMR node info and is only parsed at the
> >     time of use.
> >   - Changes to the RMR get/put API format compared to the
> >     previous version.
> >   - Support for RMR descriptor shared by multiple stream IDs.
> 
> I tested it on guest side for host MSI SW RESV region flat mapping
> (using both the old single mapping layout and the now allowed multiple
> RID ID mapping format) and this worked for me. Feel free to add my
> 
> Tested-by: Eric Auger <eric.auger@redhat.com>

Thanks Eric for verifying this, especially the multiple RID mapping case.

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

Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Eric Auger]

Hi guys,

On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> Hi,
>
> Since we now have an updated verion[0] of IORT spec(E.d) which
> addresses the memory attributes issues discussed here [1],
> this series now make use of it.
>
> The pull request for ACPICA E.d related changes are already
> raised and can be found here,
> https://github.com/acpica/acpica/pull/752
>
> v7 --> v8
>   - Patch #1 has temp definitions for RMR related changes till
>     the ACPICA header changes are part of kernel.
>   - No early parsing of RMR node info and is only parsed at the
>     time of use.
>   - Changes to the RMR get/put API format compared to the
>     previous version.
>   - Support for RMR descriptor shared by multiple stream IDs.
>
> Please take a look and let me know your thoughts.
>
> Thanks,
> Shameer
> [0] https://developer.arm.com/documentation/den0049/ed/
I still have a question on the IORT E.d spec (unrelated to this series).

The spec mandates that if RMR nodes are presented in the IORT,
_DSM function #5 for the PCIe host bridge ACPI device object must return
0, indicating the OS must honour the PCI config that the FW computed at
boot time.

However implementing this _DSM #5 as above is known to prevent PCI
devices with IO ports from working, on aarch64 linux.

"
The reason is that EFI creates I/O port mappings below
    0x1000 (in fact, at 0). However Linux, for legacy reasons, does not
    support I/O ports <= 0x1000 on PCI, so the I/O assignment created by EFI
    is rejected.
   
    EFI creates the mappings primarily for itself, and up until DSM #5
    started to be enforced, all PCI resource allocations that existed at
    boot were ignored by Linux and recreated from scratch.
"

This is an excerpt of a qemu commit message that reverted the _DMS #5
change (Revert "acpi/gpex: Inform os to keep firmware resource map").
Has the situation changed since July 2021 (ie. has UEFI been reworked?).
[+ Ard]

Thank you in advance

Regards

Eric




> [1] https://lore.kernel.org/linux-acpi/20210805160319.GB23085@lpieralisi/
>
> From old:
> 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
>  -fix pointed out by Steve to the SMMUv2 SMR bypass install in patch #8.
>
> 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.
>
> 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 (10):
>   ACPI/IORT: Add temporary RMR node flag definitions
>   iommu: Introduce a union to struct iommu_resv_region
>   ACPI/IORT: Add helper functions to parse RMR nodes
>   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/arm-smmu-v3: Reserve any RMR regions associated with a dev
>   iommu/arm-smmu: Reserve any RMR regions associated with a dev
>
>  drivers/acpi/arm64/iort.c                   | 305 ++++++++++++++++++++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  91 ++++--
>  drivers/iommu/arm/arm-smmu/arm-smmu.c       |  65 ++++-
>  drivers/iommu/dma-iommu.c                   |  25 ++
>  include/linux/acpi_iort.h                   |  14 +
>  include/linux/dma-iommu.h                   |  14 +
>  include/linux/iommu.h                       |   9 +
>  7 files changed, 504 insertions(+), 19 deletions(-)
>


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

Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Robin Murphy]

On 2022-03-11 08:19, Eric Auger wrote:
> Hi guys,
> 
> On 2/21/22 4:43 PM, Shameer Kolothum wrote:
>> Hi,
>>
>> Since we now have an updated verion[0] of IORT spec(E.d) which
>> addresses the memory attributes issues discussed here [1],
>> this series now make use of it.
>>
>> The pull request for ACPICA E.d related changes are already
>> raised and can be found here,
>> https://github.com/acpica/acpica/pull/752
>>
>> v7 --> v8
>>    - Patch #1 has temp definitions for RMR related changes till
>>      the ACPICA header changes are part of kernel.
>>    - No early parsing of RMR node info and is only parsed at the
>>      time of use.
>>    - Changes to the RMR get/put API format compared to the
>>      previous version.
>>    - Support for RMR descriptor shared by multiple stream IDs.
>>
>> Please take a look and let me know your thoughts.
>>
>> Thanks,
>> Shameer
>> [0] https://developer.arm.com/documentation/den0049/ed/
> I still have a question on the IORT E.d spec (unrelated to this series).
> 
> The spec mandates that if RMR nodes are presented in the IORT,
> _DSM function #5 for the PCIe host bridge ACPI device object must return
> 0, indicating the OS must honour the PCI config that the FW computed at
> boot time.
> 
> However implementing this _DSM #5 as above is known to prevent PCI
> devices with IO ports from working, on aarch64 linux.
> 
> "
> The reason is that EFI creates I/O port mappings below
>      0x1000 (in fact, at 0). However Linux, for legacy reasons, does not
>      support I/O ports <= 0x1000 on PCI, so the I/O assignment created by EFI
>      is rejected.
>     
>      EFI creates the mappings primarily for itself, and up until DSM #5
>      started to be enforced, all PCI resource allocations that existed at
>      boot were ignored by Linux and recreated from scratch.
> "
> 
> This is an excerpt of a qemu commit message that reverted the _DMS #5
> change (Revert "acpi/gpex: Inform os to keep firmware resource map").
> Has the situation changed since July 2021 (ie. has UEFI been reworked?).
> [+ Ard]

FWIW I wasn't aware of that, but if it's an issue then it will need to 
be fixed in Linux or UEFI's PCI resource code (arguably if UEFI has 
already allocated from the bottom of I/O space then Linux should be safe 
to assume that there are no legacy PC I/O resources to worry about). The 
DSM is required to prevent bus numbers being reassigned, because if that 
happens then any PCI StreamIDs referenced in IORT may suddenly become 
meaningless and the association of root complex nodes and RMRs to 
physical hardware lost.

Robin.

> Thank you in advance
> 
> Regards
> 
> Eric
> 
> 
> 
> 
>> [1] https://lore.kernel.org/linux-acpi/20210805160319.GB23085@lpieralisi/
>>
>>  From old:
>> 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
>>   -fix pointed out by Steve to the SMMUv2 SMR bypass install in patch #8.
>>
>> 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.
>>
>> 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 (10):
>>    ACPI/IORT: Add temporary RMR node flag definitions
>>    iommu: Introduce a union to struct iommu_resv_region
>>    ACPI/IORT: Add helper functions to parse RMR nodes
>>    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/arm-smmu-v3: Reserve any RMR regions associated with a dev
>>    iommu/arm-smmu: Reserve any RMR regions associated with a dev
>>
>>   drivers/acpi/arm64/iort.c                   | 305 ++++++++++++++++++++
>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  91 ++++--
>>   drivers/iommu/arm/arm-smmu/arm-smmu.c       |  65 ++++-
>>   drivers/iommu/dma-iommu.c                   |  25 ++
>>   include/linux/acpi_iort.h                   |  14 +
>>   include/linux/dma-iommu.h                   |  14 +
>>   include/linux/iommu.h                       |   9 +
>>   7 files changed, 504 insertions(+), 19 deletions(-)
>>
> 

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

Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Eric Auger]

Hi Robin

On 3/11/22 11:34 AM, Robin Murphy wrote:
> On 2022-03-11 08:19, Eric Auger wrote:
>> Hi guys,
>>
>> On 2/21/22 4:43 PM, Shameer Kolothum wrote:
>>> Hi,
>>>
>>> Since we now have an updated verion[0] of IORT spec(E.d) which
>>> addresses the memory attributes issues discussed here [1],
>>> this series now make use of it.
>>>
>>> The pull request for ACPICA E.d related changes are already
>>> raised and can be found here,
>>> https://github.com/acpica/acpica/pull/752
>>>
>>> v7 --> v8
>>>    - Patch #1 has temp definitions for RMR related changes till
>>>      the ACPICA header changes are part of kernel.
>>>    - No early parsing of RMR node info and is only parsed at the
>>>      time of use.
>>>    - Changes to the RMR get/put API format compared to the
>>>      previous version.
>>>    - Support for RMR descriptor shared by multiple stream IDs.
>>>
>>> Please take a look and let me know your thoughts.
>>>
>>> Thanks,
>>> Shameer
>>> [0] https://developer.arm.com/documentation/den0049/ed/
>> I still have a question on the IORT E.d spec (unrelated to this series).
>>
>> The spec mandates that if RMR nodes are presented in the IORT,
>> _DSM function #5 for the PCIe host bridge ACPI device object must return
>> 0, indicating the OS must honour the PCI config that the FW computed at
>> boot time.
>>
>> However implementing this _DSM #5 as above is known to prevent PCI
>> devices with IO ports from working, on aarch64 linux.
>>
>> "
>> The reason is that EFI creates I/O port mappings below
>>      0x1000 (in fact, at 0). However Linux, for legacy reasons, does not
>>      support I/O ports <= 0x1000 on PCI, so the I/O assignment
>> created by EFI
>>      is rejected.
>>          EFI creates the mappings primarily for itself, and up until
>> DSM #5
>>      started to be enforced, all PCI resource allocations that
>> existed at
>>      boot were ignored by Linux and recreated from scratch.
>> "
>>
>> This is an excerpt of a qemu commit message that reverted the _DMS #5
>> change (Revert "acpi/gpex: Inform os to keep firmware resource map").
>> Has the situation changed since July 2021 (ie. has UEFI been reworked?).
>> [+ Ard]
>
> FWIW I wasn't aware of that, but if it's an issue then it will need to
> be fixed in Linux or UEFI's PCI resource code (arguably if UEFI has
> already allocated from the bottom of I/O space then Linux should be
> safe to assume that there are no legacy PC I/O resources to worry
> about). The DSM is required to prevent bus numbers being reassigned,
> because if that happens then any PCI StreamIDs referenced in IORT may
> suddenly become meaningless and the association of root complex nodes
> and RMRs to physical hardware lost.

Thank you for confirming and explaining the need for DSM #5. Ard, please
could you confirm that the incompatibility with PCI devices with IO
ports is still there?

Eric

>
> Robin.
>
>> Thank you in advance
>>
>> Regards
>>
>> Eric
>>
>>
>>
>>
>>> [1]
>>> https://lore.kernel.org/linux-acpi/20210805160319.GB23085@lpieralisi/
>>>
>>>  From old:
>>> 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
>>>   -fix pointed out by Steve to the SMMUv2 SMR bypass install in
>>> patch #8.
>>>
>>> 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.
>>>
>>> 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 (10):
>>>    ACPI/IORT: Add temporary RMR node flag definitions
>>>    iommu: Introduce a union to struct iommu_resv_region
>>>    ACPI/IORT: Add helper functions to parse RMR nodes
>>>    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/arm-smmu-v3: Reserve any RMR regions associated with a dev
>>>    iommu/arm-smmu: Reserve any RMR regions associated with a dev
>>>
>>>   drivers/acpi/arm64/iort.c                   | 305
>>> ++++++++++++++++++++
>>>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  91 ++++--
>>>   drivers/iommu/arm/arm-smmu/arm-smmu.c       |  65 ++++-
>>>   drivers/iommu/dma-iommu.c                   |  25 ++
>>>   include/linux/acpi_iort.h                   |  14 +
>>>   include/linux/dma-iommu.h                   |  14 +
>>>   include/linux/iommu.h                       |   9 +
>>>   7 files changed, 504 insertions(+), 19 deletions(-)
>>>
>>
>


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

Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Ard Biesheuvel]

On Mon, 14 Mar 2022 at 11:37, Eric Auger <eric.auger@redhat.com> wrote:
>
> Hi Robin
>
> On 3/11/22 11:34 AM, Robin Murphy wrote:
> > On 2022-03-11 08:19, Eric Auger wrote:
> >> Hi guys,
> >>
> >> On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> >>> Hi,
> >>>
> >>> Since we now have an updated verion[0] of IORT spec(E.d) which
> >>> addresses the memory attributes issues discussed here [1],
> >>> this series now make use of it.
> >>>
> >>> The pull request for ACPICA E.d related changes are already
> >>> raised and can be found here,
> >>> https://github.com/acpica/acpica/pull/752
> >>>
> >>> v7 --> v8
> >>>    - Patch #1 has temp definitions for RMR related changes till
> >>>      the ACPICA header changes are part of kernel.
> >>>    - No early parsing of RMR node info and is only parsed at the
> >>>      time of use.
> >>>    - Changes to the RMR get/put API format compared to the
> >>>      previous version.
> >>>    - Support for RMR descriptor shared by multiple stream IDs.
> >>>
> >>> Please take a look and let me know your thoughts.
> >>>
> >>> Thanks,
> >>> Shameer
> >>> [0] https://developer.arm.com/documentation/den0049/ed/
> >> I still have a question on the IORT E.d spec (unrelated to this series).
> >>
> >> The spec mandates that if RMR nodes are presented in the IORT,
> >> _DSM function #5 for the PCIe host bridge ACPI device object must return
> >> 0, indicating the OS must honour the PCI config that the FW computed at
> >> boot time.
> >>
> >> However implementing this _DSM #5 as above is known to prevent PCI
> >> devices with IO ports from working, on aarch64 linux.
> >>
> >> "
> >> The reason is that EFI creates I/O port mappings below
> >>      0x1000 (in fact, at 0). However Linux, for legacy reasons, does not
> >>      support I/O ports <= 0x1000 on PCI, so the I/O assignment
> >> created by EFI
> >>      is rejected.
> >>          EFI creates the mappings primarily for itself, and up until
> >> DSM #5
> >>      started to be enforced, all PCI resource allocations that
> >> existed at
> >>      boot were ignored by Linux and recreated from scratch.
> >> "
> >>
> >> This is an excerpt of a qemu commit message that reverted the _DMS #5
> >> change (Revert "acpi/gpex: Inform os to keep firmware resource map").
> >> Has the situation changed since July 2021 (ie. has UEFI been reworked?).
> >> [+ Ard]
> >
> > FWIW I wasn't aware of that, but if it's an issue then it will need to
> > be fixed in Linux or UEFI's PCI resource code (arguably if UEFI has
> > already allocated from the bottom of I/O space then Linux should be
> > safe to assume that there are no legacy PC I/O resources to worry
> > about). The DSM is required to prevent bus numbers being reassigned,
> > because if that happens then any PCI StreamIDs referenced in IORT may
> > suddenly become meaningless and the association of root complex nodes
> > and RMRs to physical hardware lost.
>
> Thank you for confirming and explaining the need for DSM #5. Ard, please
> could you confirm that the incompatibility with PCI devices with IO
> ports is still there?
>

Yes, and this needs to be fixed in Linux. The firmware complies with
the pertinent specifications, and it is Linux that deviates from this
for legacy reasons.

IIRC, this came up on the mailing list at some point, and one of the
issues is that I/O port 0x0 is mistaken for 'no resource' or some
other exceptional case like that, so even if we fix the arbitrary
limit of 0x1000, we may still run into trouble when devices uses I/O
port 0x0.

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

Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Lorenzo Pieralisi]

On Mon, Mar 14, 2022 at 11:43:51AM +0100, Ard Biesheuvel wrote:
> On Mon, 14 Mar 2022 at 11:37, Eric Auger <eric.auger@redhat.com> wrote:
> >
> > Hi Robin
> >
> > On 3/11/22 11:34 AM, Robin Murphy wrote:
> > > On 2022-03-11 08:19, Eric Auger wrote:
> > >> Hi guys,
> > >>
> > >> On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> > >>> Hi,
> > >>>
> > >>> Since we now have an updated verion[0] of IORT spec(E.d) which
> > >>> addresses the memory attributes issues discussed here [1],
> > >>> this series now make use of it.
> > >>>
> > >>> The pull request for ACPICA E.d related changes are already
> > >>> raised and can be found here,
> > >>> https://github.com/acpica/acpica/pull/752
> > >>>
> > >>> v7 --> v8
> > >>>    - Patch #1 has temp definitions for RMR related changes till
> > >>>      the ACPICA header changes are part of kernel.
> > >>>    - No early parsing of RMR node info and is only parsed at the
> > >>>      time of use.
> > >>>    - Changes to the RMR get/put API format compared to the
> > >>>      previous version.
> > >>>    - Support for RMR descriptor shared by multiple stream IDs.
> > >>>
> > >>> Please take a look and let me know your thoughts.
> > >>>
> > >>> Thanks,
> > >>> Shameer
> > >>> [0] https://developer.arm.com/documentation/den0049/ed/
> > >> I still have a question on the IORT E.d spec (unrelated to this series).
> > >>
> > >> The spec mandates that if RMR nodes are presented in the IORT,
> > >> _DSM function #5 for the PCIe host bridge ACPI device object must return
> > >> 0, indicating the OS must honour the PCI config that the FW computed at
> > >> boot time.
> > >>
> > >> However implementing this _DSM #5 as above is known to prevent PCI
> > >> devices with IO ports from working, on aarch64 linux.
> > >>
> > >> "
> > >> The reason is that EFI creates I/O port mappings below
> > >>      0x1000 (in fact, at 0). However Linux, for legacy reasons, does not
> > >>      support I/O ports <= 0x1000 on PCI, so the I/O assignment
> > >> created by EFI
> > >>      is rejected.
> > >>          EFI creates the mappings primarily for itself, and up until
> > >> DSM #5
> > >>      started to be enforced, all PCI resource allocations that
> > >> existed at
> > >>      boot were ignored by Linux and recreated from scratch.
> > >> "
> > >>
> > >> This is an excerpt of a qemu commit message that reverted the _DMS #5
> > >> change (Revert "acpi/gpex: Inform os to keep firmware resource map").
> > >> Has the situation changed since July 2021 (ie. has UEFI been reworked?).
> > >> [+ Ard]
> > >
> > > FWIW I wasn't aware of that, but if it's an issue then it will need to
> > > be fixed in Linux or UEFI's PCI resource code (arguably if UEFI has
> > > already allocated from the bottom of I/O space then Linux should be
> > > safe to assume that there are no legacy PC I/O resources to worry
> > > about). The DSM is required to prevent bus numbers being reassigned,
> > > because if that happens then any PCI StreamIDs referenced in IORT may
> > > suddenly become meaningless and the association of root complex nodes
> > > and RMRs to physical hardware lost.
> >
> > Thank you for confirming and explaining the need for DSM #5. Ard, please
> > could you confirm that the incompatibility with PCI devices with IO
> > ports is still there?
> >
> 
> Yes, and this needs to be fixed in Linux. The firmware complies with
> the pertinent specifications, and it is Linux that deviates from this
> for legacy reasons.
> 
> IIRC, this came up on the mailing list at some point, and one of the
> issues is that I/O port 0x0 is mistaken for 'no resource' or some
> other exceptional case like that, so even if we fix the arbitrary
> limit of 0x1000, we may still run into trouble when devices uses I/O
> port 0x0.

Yes, I need to go back to that thread to sort this out.

Thanks,
Lorenzo

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

Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Eric Auger]

Hi Shameer,
On 2/21/22 4:43 PM, Shameer Kolothum wrote:
> Hi,
>
> Since we now have an updated verion[0] of IORT spec(E.d) which
> addresses the memory attributes issues discussed here [1],
> this series now make use of it.
>
> The pull request for ACPICA E.d related changes are already
> raised and can be found here,
> https://github.com/acpica/acpica/pull/752
>
> v7 --> v8
>   - Patch #1 has temp definitions for RMR related changes till
>     the ACPICA header changes are part of kernel.
>   - No early parsing of RMR node info and is only parsed at the
>     time of use.
>   - Changes to the RMR get/put API format compared to the
>     previous version.
>   - Support for RMR descriptor shared by multiple stream IDs.
>
> Please take a look and let me know your thoughts.
>
> Thanks,
> Shameer
> [0] https://developer.arm.com/documentation/den0049/ed/
> [1] https://lore.kernel.org/linux-acpi/20210805160319.GB23085@lpieralisi/
>
> From old:
> 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
>  -fix pointed out by Steve to the SMMUv2 SMR bypass install in patch #8.
>
> 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.
>
> 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 (10):
>   ACPI/IORT: Add temporary RMR node flag definitions
>   iommu: Introduce a union to struct iommu_resv_region
>   ACPI/IORT: Add helper functions to parse RMR nodes
>   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/arm-smmu-v3: Reserve any RMR regions associated with a dev
>   iommu/arm-smmu: Reserve any RMR regions associated with a dev
fyi, the last 2 patches have conflicts with
[PATCH v4 9/9] iommu: Split struct iommu_ops
which was applied on core branch.

Thanks

Eric
>
>  drivers/acpi/arm64/iort.c                   | 305 ++++++++++++++++++++
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  91 ++++--
>  drivers/iommu/arm/arm-smmu/arm-smmu.c       |  65 ++++-
>  drivers/iommu/dma-iommu.c                   |  25 ++
>  include/linux/acpi_iort.h                   |  14 +
>  include/linux/dma-iommu.h                   |  14 +
>  include/linux/iommu.h                       |   9 +
>  7 files changed, 504 insertions(+), 19 deletions(-)
>


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

RE: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Eric Auger [mailto:eric.auger@redhat.com]
> Sent: 15 March 2022 17:53
> 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
> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com;
> joro@8bytes.org; robin.murphy@arm.com; will@kernel.org; wanghuiqiang
> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo)
> <guohanjun@huawei.com>; steven.price@arm.com; Sami.Mujawar@arm.com;
> jon@solid-run.com; yangyicong <yangyicong@huawei.com>
> Subject: Re: [PATCH v8 00/11] ACPI/IORT: Support for IORT RMR node
> 
> Hi Shameer,

[...]

> >
> > Jon Nettleton (1):
> >   iommu/arm-smmu: Get associated RMR info and install bypass SMR
> >
> > Shameer Kolothum (10):
> >   ACPI/IORT: Add temporary RMR node flag definitions
> >   iommu: Introduce a union to struct iommu_resv_region
> >   ACPI/IORT: Add helper functions to parse RMR nodes
> >   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/arm-smmu-v3: Reserve any RMR regions associated with a dev
> >   iommu/arm-smmu: Reserve any RMR regions associated with a dev
> fyi, the last 2 patches have conflicts with
> [PATCH v4 9/9] iommu: Split struct iommu_ops
> which was applied on core branch.

Hi Eric,

Thanks for the heads up. I am going to respin this series soon.

All,

Please let me know if there are any further comments on this series.

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