Re: [Patch v1] iommu: arm-smmu: disable large page mappings for Nvidia arm-smmu
On 4/20/2022 1:57 AM, Robin Murphy wrote: External email: Use caution opening links or attachments On 2022-04-17 10:04, Ashish Mhetre wrote: Tegra194 and Tegra234 SoCs have the erratum that causes walk cache entries to not be invalidated correctly. The problem is that the walk cache index generated for IOVA is not same across translation and invalidation requests. This is leading to page faults when PMD entry is released during unmap and populated with new PTE table during subsequent map request. Disabling large page mappings avoids the release of PMD entry and avoid translations seeing stale PMD entry in walk cache. Fix this by limiting the page mappings to PAGE_SIZE for Tegra194 and Tegra234 devices. This is recommended fix from Tegra hardware design team. Is this related to any of the several known MMU-500 invalidation errata, or is it definitely specific to something NVIDIA have done with their integration? It's not a known MMU-500 errata. It is specific to NVIDIA. Co-developed-by: Pritesh Raithatha Signed-off-by: Pritesh Raithatha Signed-off-by: Ashish Mhetre --- drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 23 drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 +++ drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 + 3 files changed, 27 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c index 01e9b50b10a1..b7a3d06da2f4 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c @@ -258,6 +258,27 @@ static void nvidia_smmu_probe_finalize(struct arm_smmu_device *smmu, struct devi dev_name(dev), err); } +static void nvidia_smmu_cfg_pgsize_bitmap(struct arm_smmu_device *smmu) +{ + const struct device_node *np = smmu->dev->of_node; + + /* + * Tegra194 and Tegra234 SoCs have the erratum that causes walk cache + * entries to not be invalidated correctly. The problem is that the walk + * cache index generated for IOVA is not same across translation and + * invalidation requests. This is leading to page faults when PMD entry + * is released during unmap and populated with new PTE table during + * subsequent map request. Disabling large page mappings avoids the + * release of PMD entry and avoid translations seeing stale PMD entry in + * walk cache. + * Fix this by limiting the page mappings to PAGE_SIZE on Tegra194 and + * Tegra234. + */ + if (of_device_is_compatible(np, "nvidia,tegra234-smmu") || + of_device_is_compatible(np, "nvidia,tegra194-smmu")) + smmu->pgsize_bitmap = PAGE_SIZE; +} + static const struct arm_smmu_impl nvidia_smmu_impl = { .read_reg = nvidia_smmu_read_reg, .write_reg = nvidia_smmu_write_reg, @@ -268,10 +289,12 @@ static const struct arm_smmu_impl nvidia_smmu_impl = { .global_fault = nvidia_smmu_global_fault, .context_fault = nvidia_smmu_context_fault, .probe_finalize = nvidia_smmu_probe_finalize, + .cfg_pgsize_bitmap = nvidia_smmu_cfg_pgsize_bitmap, }; static const struct arm_smmu_impl nvidia_smmu_single_impl = { .probe_finalize = nvidia_smmu_probe_finalize, + .cfg_pgsize_bitmap = nvidia_smmu_cfg_pgsize_bitmap, }; struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 568cce590ccc..3692a19a588a 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1872,6 +1872,9 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH64_64K) smmu->pgsize_bitmap |= SZ_64K | SZ_512M; + if (smmu->impl && smmu->impl->cfg_pgsize_bitmap) + smmu->impl->cfg_pgsize_bitmap(smmu); I'm not the biggest fan of adding a super-specific hook for this, when it seems like it could just as easily be handled in the init_context hook, which is where it is precisely for the purpose of mangling the pgtable_cfg to influence io-pgtable's behaviour. Yes, we can use init_context() to override pgsize_bitmap. I'll update that in next version. Thanks, Robin. + if (arm_smmu_ops.pgsize_bitmap == -1UL) arm_smmu_ops.pgsize_bitmap = smmu->pgsize_bitmap; else diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h index 2b9b42fb6f30..5d9b03024969 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h @@ -442,6 +442,7 @@ struct arm_smmu_impl { void (*write_s2cr)(struct arm_smmu_device *smmu, int idx); void (*write_sctlr)(struct arm_smmu_device *smmu, int idx, u32 reg); void (*probe_finalize)(struct arm_smmu_device *smmu, struct device *dev); + void (*cfg_pgsize_bitmap)(struct arm_smmu_device *smmu); }; #defi
[PATCH] iommu/omap: using pm_runtime_resume_and_get to simplify the code
From: Minghao Chi Using pm_runtime_resume_and_get() to replace pm_runtime_get_sync and pm_runtime_put_noidle. This change is just to simplify the code, no actual functional changes. Reported-by: Zeal Robot Signed-off-by: Minghao Chi --- drivers/iommu/omap-iommu.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c index 4aab631ef517..7cfa80ccd9a8 100644 --- a/drivers/iommu/omap-iommu.c +++ b/drivers/iommu/omap-iommu.c @@ -193,9 +193,7 @@ static int iommu_enable(struct omap_iommu *obj) { int ret; - ret = pm_runtime_get_sync(obj->dev); - if (ret < 0) - pm_runtime_put_noidle(obj->dev); + ret = pm_runtime_resume_and_get(obj->dev); return ret < 0 ? ret : 0; } -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu-v3: Fix size calculation in arm_smmu_mm_invalidate_range()
On Tue, Apr 19, 2022 at 08:10:34PM -0300, Jason Gunthorpe wrote: > > - size_t size = end - start + 1; > > + size_t size; > > + > > + /* > > + * The mm_types defines vm_end as the first byte after the end > > address, > > + * different from IOMMU subsystem using the last address of an address > > + * range. So do a simple translation here by calculating size > > correctly. > > + */ > > + size = end - start; > > I would skip the comment though It's a bit of highlight here to help us remember in the future, per Robin's comments at my previous patch. Thanks! Nic ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu-v3: Fix size calculation in arm_smmu_mm_invalidate_range()
On Tue, Apr 19, 2022 at 02:01:58PM -0700, Nicolin Chen wrote: > The arm_smmu_mm_invalidate_range function is designed to be called > by mm core for Shared Virtual Addressing purpose between IOMMU and > CPU MMU. However, the ways of two subsystems defining their "end" > addresses are slightly different. IOMMU defines its "end" address > using the last address of an address range, while mm core defines > that using the following address of an address range: > > include/linux/mm_types.h: > unsigned long vm_end; > /* The first byte after our end address ... > > This mismatch resulted in an incorrect calculation for size so it > failed to be page-size aligned. Further, it caused a dead loop at > "while (iova < end)" check in __arm_smmu_tlb_inv_range function. > > This patch fixes the issue by doing the calculation correctly. > > Fixes: 2f7e8c553e98d ("iommu/arm-smmu-v3: Hook up ATC invalidation to mm ops") > Cc: sta...@vger.kernel.org > Signed-off-by: Nicolin Chen > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) Reviewed-by: Jason Gunthorpe > - size_t size = end - start + 1; > + size_t size; > + > + /* > + * The mm_types defines vm_end as the first byte after the end address, > + * different from IOMMU subsystem using the last address of an address > + * range. So do a simple translation here by calculating size correctly. > + */ > + size = end - start; I would skip the comment though Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu-v3: Fix size calculation in arm_smmu_mm_invalidate_range()
On 2022-04-19 22:01, Nicolin Chen wrote: The arm_smmu_mm_invalidate_range function is designed to be called by mm core for Shared Virtual Addressing purpose between IOMMU and CPU MMU. However, the ways of two subsystems defining their "end" addresses are slightly different. IOMMU defines its "end" address using the last address of an address range, while mm core defines that using the following address of an address range: include/linux/mm_types.h: unsigned long vm_end; /* The first byte after our end address ... This mismatch resulted in an incorrect calculation for size so it failed to be page-size aligned. Further, it caused a dead loop at "while (iova < end)" check in __arm_smmu_tlb_inv_range function. This patch fixes the issue by doing the calculation correctly. Reviewed-by: Robin Murphy Fixes: 2f7e8c553e98d ("iommu/arm-smmu-v3: Hook up ATC invalidation to mm ops") Cc: sta...@vger.kernel.org Signed-off-by: Nicolin Chen --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index 22ddd05bbdcd..c623dae1e115 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -183,7 +183,14 @@ static void arm_smmu_mm_invalidate_range(struct mmu_notifier *mn, { struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn); struct arm_smmu_domain *smmu_domain = smmu_mn->domain; - size_t size = end - start + 1; + size_t size; + + /* +* The mm_types defines vm_end as the first byte after the end address, +* different from IOMMU subsystem using the last address of an address +* range. So do a simple translation here by calculating size correctly. +*/ + size = end - start; if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_BTM)) arm_smmu_tlb_inv_range_asid(start, size, smmu_mn->cd->asid, ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/arm-smmu-v3: Fix size calculation in arm_smmu_mm_invalidate_range()
The arm_smmu_mm_invalidate_range function is designed to be called by mm core for Shared Virtual Addressing purpose between IOMMU and CPU MMU. However, the ways of two subsystems defining their "end" addresses are slightly different. IOMMU defines its "end" address using the last address of an address range, while mm core defines that using the following address of an address range: include/linux/mm_types.h: unsigned long vm_end; /* The first byte after our end address ... This mismatch resulted in an incorrect calculation for size so it failed to be page-size aligned. Further, it caused a dead loop at "while (iova < end)" check in __arm_smmu_tlb_inv_range function. This patch fixes the issue by doing the calculation correctly. Fixes: 2f7e8c553e98d ("iommu/arm-smmu-v3: Hook up ATC invalidation to mm ops") Cc: sta...@vger.kernel.org Signed-off-by: Nicolin Chen --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index 22ddd05bbdcd..c623dae1e115 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -183,7 +183,14 @@ static void arm_smmu_mm_invalidate_range(struct mmu_notifier *mn, { struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn); struct arm_smmu_domain *smmu_domain = smmu_mn->domain; - size_t size = end - start + 1; + size_t size; + + /* +* The mm_types defines vm_end as the first byte after the end address, +* different from IOMMU subsystem using the last address of an address +* range. So do a simple translation here by calculating size correctly. +*/ + size = end - start; if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_BTM)) arm_smmu_tlb_inv_range_asid(start, size, smmu_mn->cd->asid, -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Patch v1] iommu: arm-smmu: disable large page mappings for Nvidia arm-smmu
On 2022-04-17 10:04, Ashish Mhetre wrote: Tegra194 and Tegra234 SoCs have the erratum that causes walk cache entries to not be invalidated correctly. The problem is that the walk cache index generated for IOVA is not same across translation and invalidation requests. This is leading to page faults when PMD entry is released during unmap and populated with new PTE table during subsequent map request. Disabling large page mappings avoids the release of PMD entry and avoid translations seeing stale PMD entry in walk cache. Fix this by limiting the page mappings to PAGE_SIZE for Tegra194 and Tegra234 devices. This is recommended fix from Tegra hardware design team. Is this related to any of the several known MMU-500 invalidation errata, or is it definitely specific to something NVIDIA have done with their integration? Co-developed-by: Pritesh Raithatha Signed-off-by: Pritesh Raithatha Signed-off-by: Ashish Mhetre --- drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 23 drivers/iommu/arm/arm-smmu/arm-smmu.c| 3 +++ drivers/iommu/arm/arm-smmu/arm-smmu.h| 1 + 3 files changed, 27 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c index 01e9b50b10a1..b7a3d06da2f4 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c @@ -258,6 +258,27 @@ static void nvidia_smmu_probe_finalize(struct arm_smmu_device *smmu, struct devi dev_name(dev), err); } +static void nvidia_smmu_cfg_pgsize_bitmap(struct arm_smmu_device *smmu) +{ + const struct device_node *np = smmu->dev->of_node; + + /* +* Tegra194 and Tegra234 SoCs have the erratum that causes walk cache +* entries to not be invalidated correctly. The problem is that the walk +* cache index generated for IOVA is not same across translation and +* invalidation requests. This is leading to page faults when PMD entry +* is released during unmap and populated with new PTE table during +* subsequent map request. Disabling large page mappings avoids the +* release of PMD entry and avoid translations seeing stale PMD entry in +* walk cache. +* Fix this by limiting the page mappings to PAGE_SIZE on Tegra194 and +* Tegra234. +*/ + if (of_device_is_compatible(np, "nvidia,tegra234-smmu") || + of_device_is_compatible(np, "nvidia,tegra194-smmu")) + smmu->pgsize_bitmap = PAGE_SIZE; +} + static const struct arm_smmu_impl nvidia_smmu_impl = { .read_reg = nvidia_smmu_read_reg, .write_reg = nvidia_smmu_write_reg, @@ -268,10 +289,12 @@ static const struct arm_smmu_impl nvidia_smmu_impl = { .global_fault = nvidia_smmu_global_fault, .context_fault = nvidia_smmu_context_fault, .probe_finalize = nvidia_smmu_probe_finalize, + .cfg_pgsize_bitmap = nvidia_smmu_cfg_pgsize_bitmap, }; static const struct arm_smmu_impl nvidia_smmu_single_impl = { .probe_finalize = nvidia_smmu_probe_finalize, + .cfg_pgsize_bitmap = nvidia_smmu_cfg_pgsize_bitmap, }; struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 568cce590ccc..3692a19a588a 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1872,6 +1872,9 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH64_64K) smmu->pgsize_bitmap |= SZ_64K | SZ_512M; + if (smmu->impl && smmu->impl->cfg_pgsize_bitmap) + smmu->impl->cfg_pgsize_bitmap(smmu); I'm not the biggest fan of adding a super-specific hook for this, when it seems like it could just as easily be handled in the init_context hook, which is where it is precisely for the purpose of mangling the pgtable_cfg to influence io-pgtable's behaviour. Thanks, Robin. + if (arm_smmu_ops.pgsize_bitmap == -1UL) arm_smmu_ops.pgsize_bitmap = smmu->pgsize_bitmap; else diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h index 2b9b42fb6f30..5d9b03024969 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h @@ -442,6 +442,7 @@ struct arm_smmu_impl { void (*write_s2cr)(struct arm_smmu_device *smmu, int idx); void (*write_sctlr)(struct arm_smmu_device *smmu, int idx, u32 reg); void (*probe_finalize)(struct arm_smmu_device *smmu, struct device *dev); + void (*cfg_pgsize_bitmap)(struct arm_smmu_device *smmu); }; #define INVALID_SMENDX -1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu-v3: Align size in __arm_smmu_tlb_inv_range
On 2022-04-19 21:05, Nicolin Chen wrote: On Tue, Apr 19, 2022 at 05:02:33PM -0300, Jason Gunthorpe wrote: diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index d816759a6bcf..e280568bb513 100644 +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -183,7 +183,7 @@ static void arm_smmu_mm_invalidate_range(struct mmu_notifier *mn, { struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn); struct arm_smmu_domain *smmu_domain = smmu_mn->domain; - size_t size = end - start + 1; + size_t size = end - start; +1 to this bug fix. You should send a formal patch for stable with a fixes/etc mmu notifiers uses 'end' not 'last' in alignment with how VMA's work: include/linux/mm_types.h: unsigned long vm_end; /* The first byte after our end address Thanks for the review! Yea, I will send a new patch. Yup, +1 from me too - this is exactly the kind of thing I suspected - and I reckon it might even be worth a comment in the code here that mm's "end" is an exclusive limit, to help us remember in future. If there doesn't look to be any way for completely arbitrarily-aligned addresses to slip through then I'd be tempted to leave it at that (i.e. reason that if the infinite loop can only happen due to catastrophic failure then it's beyond the scope of things that are worth trying to mitigate), but I'll let Jean and Will have the final say there. Cheers, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu-v3: Align size in __arm_smmu_tlb_inv_range
On Tue, Apr 19, 2022 at 05:02:33PM -0300, Jason Gunthorpe wrote: > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > > index d816759a6bcf..e280568bb513 100644 > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > > @@ -183,7 +183,7 @@ static void arm_smmu_mm_invalidate_range(struct > > mmu_notifier *mn, > > { > > struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn); > > struct arm_smmu_domain *smmu_domain = smmu_mn->domain; > > - size_t size = end - start + 1; > > + size_t size = end - start; > > +1 to this bug fix. You should send a formal patch for stable with a fixes/etc > > mmu notifiers uses 'end' not 'last' in alignment with how VMA's work: > > include/linux/mm_types.h: unsigned long vm_end; /* The first > byte after our end address Thanks for the review! Yea, I will send a new patch. Nic ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu-v3: Align size in __arm_smmu_tlb_inv_range
On Fri, Apr 15, 2022 at 07:03:20PM -0700, Nicolin Chen wrote: > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > index d816759a6bcf..e280568bb513 100644 > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > @@ -183,7 +183,7 @@ static void arm_smmu_mm_invalidate_range(struct > mmu_notifier *mn, > { > struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn); > struct arm_smmu_domain *smmu_domain = smmu_mn->domain; > - size_t size = end - start + 1; > + size_t size = end - start; +1 to this bug fix. You should send a formal patch for stable with a fixes/etc mmu notifiers uses 'end' not 'last' in alignment with how VMA's work: include/linux/mm_types.h: unsigned long vm_end; /* The first byte after our end address Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 00/11] ACPI/IORT: Support for IORT RMR node
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&data=04%7C01%7Claurentiu.tudor%40nxp.com%7C2c1805513e0c4509194608da1638a306%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637846729754056888%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=R2Rw4JK1ugTN1QA4umzMuLVem2oS9BZucbvNoZqSJ3I%3D&reserved=0 [1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Facpica%2Facpica%2Fpull%2F765&data=04%7C01%7Claurentiu.tudor%40nxp.com%7C2c1805513e0c4509194608da1638a306%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637846729754056888%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=0aC%2BwZXPL4b0ZWIXw3TGwFE3NqPUMVJ3IbpsxeQF8zw%3D&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 |
Re: [PATCH 05/13] iommu/arm-smmu-v3: Clean up bus_set_iommu()
On Thu, Apr 14, 2022 at 01:42:34PM +0100, Robin Murphy wrote: > Stop calling bus_set_iommu() since it's now unnecessary, and simplify > the probe failure path accordingly. > > Signed-off-by: Robin Murphy > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 53 + > 1 file changed, 2 insertions(+), 51 deletions(-) Acked-by: Will Deacon Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 04/13] iommu/arm-smmu: Clean up bus_set_iommu()
On Thu, Apr 14, 2022 at 01:42:33PM +0100, Robin Murphy wrote: > Stop calling bus_set_iommu() since it's now unnecessary. With device > probes now replayed for every IOMMU instance registration, the whole > sorry ordering workaround for legacy DT bindings goes too, hooray! Ha, I hope you tested this! > Signed-off-by: Robin Murphy > --- > drivers/iommu/arm/arm-smmu/arm-smmu.c | 84 +-- > 1 file changed, 2 insertions(+), 82 deletions(-) Assuming it works, Acked-by: Will Deacon Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Patch v1] iommu: arm-smmu: disable large page mappings for Nvidia arm-smmu
On 17/04/2022 10:04, Ashish Mhetre wrote: Tegra194 and Tegra234 SoCs have the erratum that causes walk cache entries to not be invalidated correctly. The problem is that the walk cache index generated for IOVA is not same across translation and invalidation requests. This is leading to page faults when PMD entry is released during unmap and populated with new PTE table during subsequent map request. Disabling large page mappings avoids the release of PMD entry and avoid translations seeing stale PMD entry in walk cache. Fix this by limiting the page mappings to PAGE_SIZE for Tegra194 and Tegra234 devices. This is recommended fix from Tegra hardware design team. Co-developed-by: Pritesh Raithatha Signed-off-by: Pritesh Raithatha Signed-off-by: Ashish Mhetre --- drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c | 23 drivers/iommu/arm/arm-smmu/arm-smmu.c| 3 +++ drivers/iommu/arm/arm-smmu/arm-smmu.h| 1 + 3 files changed, 27 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c index 01e9b50b10a1..b7a3d06da2f4 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c @@ -258,6 +258,27 @@ static void nvidia_smmu_probe_finalize(struct arm_smmu_device *smmu, struct devi dev_name(dev), err); } +static void nvidia_smmu_cfg_pgsize_bitmap(struct arm_smmu_device *smmu) +{ + const struct device_node *np = smmu->dev->of_node; + + /* +* Tegra194 and Tegra234 SoCs have the erratum that causes walk cache +* entries to not be invalidated correctly. The problem is that the walk +* cache index generated for IOVA is not same across translation and +* invalidation requests. This is leading to page faults when PMD entry +* is released during unmap and populated with new PTE table during +* subsequent map request. Disabling large page mappings avoids the +* release of PMD entry and avoid translations seeing stale PMD entry in +* walk cache. +* Fix this by limiting the page mappings to PAGE_SIZE on Tegra194 and +* Tegra234. +*/ + if (of_device_is_compatible(np, "nvidia,tegra234-smmu") || + of_device_is_compatible(np, "nvidia,tegra194-smmu")) + smmu->pgsize_bitmap = PAGE_SIZE; +} + static const struct arm_smmu_impl nvidia_smmu_impl = { .read_reg = nvidia_smmu_read_reg, .write_reg = nvidia_smmu_write_reg, @@ -268,10 +289,12 @@ static const struct arm_smmu_impl nvidia_smmu_impl = { .global_fault = nvidia_smmu_global_fault, .context_fault = nvidia_smmu_context_fault, .probe_finalize = nvidia_smmu_probe_finalize, + .cfg_pgsize_bitmap = nvidia_smmu_cfg_pgsize_bitmap, }; static const struct arm_smmu_impl nvidia_smmu_single_impl = { .probe_finalize = nvidia_smmu_probe_finalize, + .cfg_pgsize_bitmap = nvidia_smmu_cfg_pgsize_bitmap, }; struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index 568cce590ccc..3692a19a588a 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1872,6 +1872,9 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH64_64K) smmu->pgsize_bitmap |= SZ_64K | SZ_512M; + if (smmu->impl && smmu->impl->cfg_pgsize_bitmap) + smmu->impl->cfg_pgsize_bitmap(smmu); + if (arm_smmu_ops.pgsize_bitmap == -1UL) arm_smmu_ops.pgsize_bitmap = smmu->pgsize_bitmap; else diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h index 2b9b42fb6f30..5d9b03024969 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h @@ -442,6 +442,7 @@ struct arm_smmu_impl { void (*write_s2cr)(struct arm_smmu_device *smmu, int idx); void (*write_sctlr)(struct arm_smmu_device *smmu, int idx, u32 reg); void (*probe_finalize)(struct arm_smmu_device *smmu, struct device *dev); + void (*cfg_pgsize_bitmap)(struct arm_smmu_device *smmu); }; #define INVALID_SMENDX -1 Reviewed-by: Jon Hunter Tested-by: Jon Hunter Thanks! Jon -- nvpublic ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/13] iommu: Move bus setup to IOMMU device registration
On 2022/4/19 15:20, Robin Murphy wrote: On 2022-04-19 00:37, Lu Baolu wrote: On 2022/4/19 6:09, Robin Murphy wrote: On 2022-04-16 01:04, Lu Baolu wrote: On 2022/4/14 20:42, Robin Murphy wrote: @@ -1883,27 +1900,12 @@ static int iommu_bus_init(struct bus_type *bus) */ int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops) { - int err; - - if (ops == NULL) { - bus->iommu_ops = NULL; - return 0; - } - - if (bus->iommu_ops != NULL) + if (bus->iommu_ops && ops && bus->iommu_ops != ops) return -EBUSY; bus->iommu_ops = ops; Do we still need to keep above lines in bus_set_iommu()? It preserves the existing behaviour until each callsite and its associated error handling are removed later on, which seems like as good a thing to do as any. Since I'm already relaxing iommu_device_register() to a warn-but-continue behaviour while it keeps the bus ops on life-support internally, I figured not changing too much at once would make it easier to bisect any potential issues arising from this first step. Fair enough. Thank you for the explanation. Do you have a public tree that I could pull these patches and try them on an Intel hardware? Or perhaps you have done this? I like the whole idea of this series, but it's better to try it with a real hardware. I haven't bothered with separate branches since there's so many different pieces in-flight, but my complete (unstable) development branch can be found here: https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/bus For now I'd recommend winding the head back to "iommu: Clean up bus_set_iommu()" for testing - some of the patches above that have already been posted and picked up by their respective subsystems, but others are incomplete and barely compile-tested. I'll probably rearrange it later this week to better reflect what's happened so far. Okay, thanks for sharing. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 06/11] ACPI/IORT: Add support to retrieve IORT RMR reserved regions
On Tue, Apr 19, 2022 at 08:31:55AM +, Shameerali Kolothum Thodi via iommu wrote: > Sorry for the delayed response(was on holidays). So if we move the > iommu_dma_put_resv_region() call to generic_iommu_put_resv_regions() , > will that address the concerns here? > > I think it will resolve the issue in 05/11 as well pointed out by Christoph > where we end up not releasing reserved regions when > CONFIG_IOMMU_DMA is not set. As Robin pointed out we might not really deduct that ACPI means RMR. I suspect the best would be to just attach a free callback to the regions. Either by adding it directly to struct iommu_resv_region if we thing there are few enough regions to not be worried about the memory use, or other by adding a container struct for the list_head that contains the free callback. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 00/34] MT8195 IOMMU SUPPORT
Il 07/04/22 09:56, Yong Wu ha scritto: This patchset adds MT8195 iommu support. Hello maintainers, This is a friendly ping to avoid forgetting about this fully reviewed and fully tested review. Cheers, Angelo MT8195 have 3 IOMMU HWs. 2 IOMMU HW is for multimedia, and 1 IOMMU HW is for infra-master, like PCIe/USB. About the 2 MM IOMMU HW, something like this: IOMMU(VDO) IOMMU(VPP) | | SMI_COMMON(VDO) SMI_COMMON(VPP) --- | | ... | | ... larb0 larb2 ...larb1 larb3... these two MM IOMMU HW share a pgtable. About the INFRA IOMMU, it don't have larbs, the master connects the iommu directly. It use a independent pgtable. Also, mt8195 IOMMU bank supports. Normally the IOMMU register size only is 0x1000. In this IOMMU HW, the register size is 5 * 0x1000. each 0x1000 is a bank. the banks' register look like this: |bank0 | bank1 | bank2 | bank3 | bank4| |global | |control| null |regs | - |bank |bank |bank |bank |bank | |regs |regs |regs |regs |regs | | | | | | | - All the banks share some global control registers, and each bank have its special bank registers, like pgtable base register, tlb operation registers, the fault status registers. In mt8195, we enable this bank feature for infra iommu, We put PCIe in bank0 and USB in bank4. they have independent pgtable. Change note: v6: Rebase on v5.18-rc1. v5: https://lore.kernel.org/linux-iommu/20220217113453.13658-1-yong...@mediatek.com 1) Base on next-20220216 2) Remove a patch for kmalloc for protect buffer. keep the kzalloc for it. 3) minor fix from AngeloGioacchino, like rename the error label name (data_unlock to err_unlock). Note, keep the TODO for component compare_of[26/34]. v4: https://lore.kernel.org/linux-iommu/20220125085634.17972-1-yong...@mediatek.com/ 1) Base on v5.16-rc1 2) Base on tlb logic 2 patchset, some patches in v3 has already gone through that patchset. 3) Due to the unreadable union for v1/v2(comment in 26/33 of v3), I separate mtk_iommu_data for v1 and v2 totally, then remove mtk_iommu.h. please see patch[26/35][27/35]. 4) add two mutex for the internal data. patch[6/35][7/35]. 5) add a new flag PM_CLK_AO. v3: https://lore.kernel.org/linux-mediatek/20210923115840.17813-1-yong...@mediatek.com/ 1) base on v5.15-rc1 2) Adjust devlink with smi-common, not use the property(sub-sommon). 3) Adjust tlb_flush_all flow, a) Fix tlb_flush_all only is supported in bank0. b) add tlb-flush-all in the resume callback. c) remove the pm status checking in tlb-flush-all. The reason are showed in the commit message. 4) Allow IOMMU_DOMAIN_UNMANAGED since PCIe VFIO use that. 5) Fix a clk warning and a null abort when unbind the iommu driver. v2: https://lore.kernel.org/linux-mediatek/20210813065324.29220-1-yong...@mediatek.com/ 1) Base on v5.14-rc1. 2) Fix build fail for arm32. 3) Fix dt-binding issue from Rob. 4) Fix the bank issue when tlb flush. v1 always use bank->base. 5) adjust devlink with smi-common since the node may be smi-sub-common. 6) other changes: like reword some commit message(removing many "This patch..."); seperate serveral patches. v1: https://lore.kernel.org/linux-mediatek/20210630023504.18177-1-yong...@mediatek.com/ Base on v5.13-rc1 Yong Wu (34): dt-bindings: mediatek: mt8195: Add binding for MM IOMMU dt-bindings: mediatek: mt8195: Add binding for infra IOMMU iommu/mediatek: Fix 2 HW sharing pgtable issue iommu/mediatek: Add list_del in mtk_iommu_remove iommu/mediatek: Remove clk_disable in mtk_iommu_remove iommu/mediatek: Add mutex for m4u_group and m4u_dom in data iommu/mediatek: Add mutex for data in the mtk_iommu_domain iommu/mediatek: Adapt sharing and non-sharing pgtable case iommu/mediatek: Add 12G~16G support for multi domains iommu/mediatek: Add a flag DCM_DISABLE iommu/mediatek: Add a flag NON_STD_AXI iommu/mediatek: Remove the granule in the tlb flush iommu/mediatek: Always enable output PA over 32bits in isr iommu/mediatek: Add SUB_COMMON_3BITS flag iommu/mediatek: Add IOMMU_TYPE flag iommu/mediatek: Contain MM IOMMU flow with the MM TYPE iommu/mediatek: Adjust device link when it is sub-common iommu/mediatek: Allow IOMMU_DOMAIN_UNMANAGED for PCIe VFIO iommu/mediatek: Add a PM_CLK_AO flag for infra iommu iommu/mediatek: Add infra iommu support iommu/mediatek: Add PCIe support iommu/mediatek: Add mt8195 support iommu/mediatek: Only adjust code about register base
RE: [PATCH v9 06/11] ACPI/IORT: Add support to retrieve IORT RMR reserved regions
> -Original Message- > From: Christoph Hellwig [mailto:h...@infradead.org] > Sent: 07 April 2022 15:00 > To: Robin Murphy > Cc: Christoph Hellwig ; Shameerali Kolothum Thodi > ; j...@solid-run.com; Linuxarm > ; steven.pr...@arm.com; linux-a...@vger.kernel.org; > iommu@lists.linux-foundation.org; wanghuiqiang > ; Guohanjun (Hanjun Guo) > ; yangyicong ; > sami.muja...@arm.com; w...@kernel.org; > linux-arm-ker...@lists.infradead.org > Subject: Re: [PATCH v9 06/11] ACPI/IORT: Add support to retrieve IORT RMR > reserved regions > > On Thu, Apr 07, 2022 at 02:53:38PM +0100, Robin Murphy wrote: > > > Why can't this just go into generic_iommu_put_resv_regions? The idea > > > that the iommu low-level drivers need to call into dma-iommu which is > > > a consumer of the IOMMU API is odd. Especially if that just calls out > > > to ACPI code and generic IOMMU code only anyway. > > > > Because assuming ACPI means IORT is not generic. Part of the aim in adding > > the union to iommu_resv_region is that stuff like AMD's unity_map_entry > and > > Intel's dmar_rmrr_unit can be folded into it as well, and their reserved > > region handling correspondingly simplified too. > > > > The iommu_dma_{get,put}_resv_region() helpers are kind of intended to be > > specific to the fwnode mechanism which deals with IORT and devicetree > (once > > the reserved region bindings are fully worked out). > > But IORT is not driver₋specific code. So we'll need a USE_IORT flag > somewhere in core IOMMU code instead of trying to stuff this into > driver operations. and dma-iommu mostly certainly implies IORT even > less than ACPI. Sorry for the delayed response(was on holidays). So if we move the iommu_dma_put_resv_region() call to generic_iommu_put_resv_regions() , will that address the concerns here? I think it will resolve the issue in 05/11 as well pointed out by Christoph where we end up not releasing reserved regions when CONFIG_IOMMU_DMA is not set. Something like below, diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index f2c45b85b9fc..d08204a25ba8 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2605,6 +2605,8 @@ void generic_iommu_put_resv_regions(struct device *dev, struct list_head *list) { struct iommu_resv_region *entry, *next; + iommu_dma_put_resv_regions(dev, list); + list_for_each_entry_safe(entry, next, list, list) kfree(entry); } diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 5811233dc9fb..8cb1e419db49 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -393,8 +393,6 @@ 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); } Please let me know if this is not good enough. Thanks, Shameer ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/13] iommu: Move bus setup to IOMMU device registration
On 2022-04-19 00:37, Lu Baolu wrote: On 2022/4/19 6:09, Robin Murphy wrote: On 2022-04-16 01:04, Lu Baolu wrote: On 2022/4/14 20:42, Robin Murphy wrote: @@ -1883,27 +1900,12 @@ static int iommu_bus_init(struct bus_type *bus) */ int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops) { - int err; - - if (ops == NULL) { - bus->iommu_ops = NULL; - return 0; - } - - if (bus->iommu_ops != NULL) + if (bus->iommu_ops && ops && bus->iommu_ops != ops) return -EBUSY; bus->iommu_ops = ops; Do we still need to keep above lines in bus_set_iommu()? It preserves the existing behaviour until each callsite and its associated error handling are removed later on, which seems like as good a thing to do as any. Since I'm already relaxing iommu_device_register() to a warn-but-continue behaviour while it keeps the bus ops on life-support internally, I figured not changing too much at once would make it easier to bisect any potential issues arising from this first step. Fair enough. Thank you for the explanation. Do you have a public tree that I could pull these patches and try them on an Intel hardware? Or perhaps you have done this? I like the whole idea of this series, but it's better to try it with a real hardware. I haven't bothered with separate branches since there's so many different pieces in-flight, but my complete (unstable) development branch can be found here: https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/bus For now I'd recommend winding the head back to "iommu: Clean up bus_set_iommu()" for testing - some of the patches above that have already been posted and picked up by their respective subsystems, but others are incomplete and barely compile-tested. I'll probably rearrange it later this week to better reflect what's happened so far. Cheers, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu