mbox series

[v9,00/11] ACPI/IORT: Support for IORT RMR node

Message ID 20220404124209.1086-1-shameerali.kolothum.thodi@huawei.com
Headers show
Series ACPI/IORT: Support for IORT RMR node | expand

Message

Shameerali Kolothum Thodi April 4, 2022, 12:41 p.m. UTC
Hi

v8 --> v9
 - Adressed comments from Robin on interfaces as discussed here[0].
 - Addressed comments from Lorenzo.
 
Though functionally there aren't any major changes, the interfaces have
changed from v8 and for that reason not included the T-by tags from
Steve and Eric yet(Many thanks for that). Appreciate it if you could
give this a spin and let me know.

(The revised ACPICA pull request for IORT E.d related changes is
here[1] and this is now merged to acpica:master.)

Please take a look and let me know your thoughts.

Thanks,
Shameer
[0] https://lore.kernel.org/linux-arm-kernel/c982f1d7-c565-769a-abae-79c962969d88@arm.com/
[1] https://github.com/acpica/acpica/pull/765

Comments

Lorenzo Pieralisi April 6, 2022, 3:53 p.m. UTC | #1
On Mon, Apr 04, 2022 at 01:41:59PM +0100, Shameer Kolothum 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/765
> 
> This is now merged to acpica:master.

I assume we can drop this patch when ACPICA Linuxized patches hit
Rafael's tree.

> ---
>  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..fd06cf43ba31 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_IWB_OWB
> +#define ACPI_IORT_RMR_ATTR_NORMAL_IWB_OWB	0x05
> +#endif
> +
>  struct iort_its_msi_chip {
>  	struct list_head	list;
>  	struct fwnode_handle	*fw_node;
> -- 
> 2.25.1
>
Christoph Hellwig April 7, 2022, 1:26 p.m. UTC | #2
>  
> +static inline void iommu_dma_put_resv_regions(struct device *dev, struct list_head *list)
> +{
> +}
> +
>  #endif	/* CONFIG_IOMMU_DMA */

This changes behavior when CONFIG_IOMMU_DMA is not set.  So e.g. on
ARM all the drivers that are using the new helper now fail to release
reserved regions.
Steven Price April 7, 2022, 3:43 p.m. UTC | #3
Hi Shameer,

I've given it a spin on my Juno board and the EFIFB works fine with it.
However I am getting a warning:

ACPI: IORT: [Firmware Bug]: RMR descriptor[0xf80d0000] with zero length,
continue anyway

Which on examination of the IORT table is correct - the firmware does
indeed seem to have a bug and the length in the IORT table is 0,
hopefully I can get that fixed. However since it "all works" that points
out that the reserved memory region isn't actually used. Instead the
existing entries from the SMMU are reused (even though they don't match
the address/size region in the RMR).

I'm not sure if that matters but I thought it worth pointing out as it's
not currently spelt out that the RMR descriptor's content is currently
actually ignored.

Anyway, FWIW:

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

Steve

On 04/04/2022 13:41, Shameer Kolothum wrote:
> Hi
> 
> v8 --> v9
>  - Adressed comments from Robin on interfaces as discussed here[0].
>  - Addressed comments from Lorenzo.
>  
> Though functionally there aren't any major changes, the interfaces have
> changed from v8 and for that reason not included the T-by tags from
> Steve and Eric yet(Many thanks for that). Appreciate it if you could
> give this a spin and let me know.
> 
> (The revised ACPICA pull request for IORT E.d related changes is
> here[1] and this is now merged to acpica:master.)
> 
> Please take a look and let me know your thoughts.
> 
> Thanks,
> Shameer
> [0] https://lore.kernel.org/linux-arm-kernel/c982f1d7-c565-769a-abae-79c962969d88@arm.com/
> [1] https://github.com/acpica/acpica/pull/765
> 
> 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:
> 
> 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.
> 
> 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: Make iort_iommu_msi_get_resv_regions() return void
>   ACPI/IORT: Provide a generic helper to retrieve reserve regions
>   iommu/dma: Introduce a helper to remove reserved regions
>   ACPI/IORT: Add support to retrieve IORT RMR reserved regions
>   ACPI/IORT: Add a helper to retrieve RMR info directly
>   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
> 
>  drivers/acpi/arm64/iort.c                   | 369 ++++++++++++++++++--
>  drivers/iommu/apple-dart.c                  |   2 +-
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  80 ++++-
>  drivers/iommu/arm/arm-smmu/arm-smmu.c       |  54 ++-
>  drivers/iommu/dma-iommu.c                   |  11 +-
>  drivers/iommu/virtio-iommu.c                |   2 +-
>  include/linux/acpi_iort.h                   |  18 +-
>  include/linux/dma-iommu.h                   |   5 +
>  include/linux/iommu.h                       |   9 +
>  9 files changed, 505 insertions(+), 45 deletions(-)
>
Laurentiu Tudor April 19, 2022, 2:50 p.m. UTC | #4
Hi Shameer,

On 4/4/2022 3:41 PM, Shameer Kolothum wrote:
> Hi
> 
> v8 --> v9
>   - Adressed comments from Robin on interfaces as discussed here[0].
>   - Addressed comments from Lorenzo.
>   
> Though functionally there aren't any major changes, the interfaces have
> changed from v8 and for that reason not included the T-by tags from
> Steve and Eric yet(Many thanks for that). Appreciate it if you could
> give this a spin and let me know.
> 
> (The revised ACPICA pull request for IORT E.d related changes is
> here[1] and this is now merged to acpica:master.)
> 
> Please take a look and let me know your thoughts.
> 
> Thanks,
> Shameer
> [0] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-arm-kernel%2Fc982f1d7-c565-769a-abae-79c962969d88%40arm.com%2F&amp;data=04%7C01%7Claurentiu.tudor%40nxp.com%7C2c1805513e0c4509194608da1638a306%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637846729754056888%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=R2Rw4JK1ugTN1QA4umzMuLVem2oS9BZucbvNoZqSJ3I%3D&amp;reserved=0
> [1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Facpica%2Facpica%2Fpull%2F765&amp;data=04%7C01%7Claurentiu.tudor%40nxp.com%7C2c1805513e0c4509194608da1638a306%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637846729754056888%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=0aC%2BwZXPL4b0ZWIXw3TGwFE3NqPUMVJ3IbpsxeQF8zw%3D&amp;reserved=0
> 
>  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:
> 
> 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.
> 
> 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: Make iort_iommu_msi_get_resv_regions() return void
>    ACPI/IORT: Provide a generic helper to retrieve reserve regions
>    iommu/dma: Introduce a helper to remove reserved regions
>    ACPI/IORT: Add support to retrieve IORT RMR reserved regions
>    ACPI/IORT: Add a helper to retrieve RMR info directly
>    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
> 
>   drivers/acpi/arm64/iort.c                   | 369 ++++++++++++++++++--
>   drivers/iommu/apple-dart.c                  |   2 +-
>   drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  80 ++++-
>   drivers/iommu/arm/arm-smmu/arm-smmu.c       |  54 ++-
>   drivers/iommu/dma-iommu.c                   |  11 +-
>   drivers/iommu/virtio-iommu.c                |   2 +-
>   include/linux/acpi_iort.h                   |  18 +-
>   include/linux/dma-iommu.h                   |   5 +
>   include/linux/iommu.h                       |   9 +
>   9 files changed, 505 insertions(+), 45 deletions(-)
> 

I've tested the patches on a NXP LX2160A with SMMUv2 and things look 
fine. Thanks!

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

---
Best Regards, Laurentiu