Re: [Patch v1] iommu: arm-smmu: disable large page mappings for Nvidia arm-smmu

2022-04-19 Thread Ashish Mhetre via iommu



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

2022-04-19 Thread cgel . zte
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()

2022-04-19 Thread Nicolin Chen via iommu
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()

2022-04-19 Thread Jason Gunthorpe
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()

2022-04-19 Thread Robin Murphy

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()

2022-04-19 Thread Nicolin Chen via iommu
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

2022-04-19 Thread Robin Murphy

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

2022-04-19 Thread Robin Murphy

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

2022-04-19 Thread Nicolin Chen via iommu
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

2022-04-19 Thread Jason Gunthorpe
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

2022-04-19 Thread Laurentiu Tudor

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()

2022-04-19 Thread Will Deacon
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()

2022-04-19 Thread Will Deacon
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

2022-04-19 Thread Jon Hunter via iommu



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

2022-04-19 Thread Lu Baolu

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

2022-04-19 Thread Christoph Hellwig
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

2022-04-19 Thread AngeloGioacchino Del Regno

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

2022-04-19 Thread Shameerali Kolothum Thodi via iommu


> -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

2022-04-19 Thread Robin Murphy

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