| Message ID | 20220221154344.2126-1-shameerali.kolothum.thodi@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | ACPI/IORT: Support for IORT RMR node | expand |
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(-) >
> -----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
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(-) >
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(-) >
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(-) >> >
> -----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
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(-) >>> >> >
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.
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
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(-) >
> -----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
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(-)