Re: [PATCH v5 2/2] iommu/mediatek: Allow page table PA up to 35bit

[Yong Wu via iommu]

On Mon, 2022-05-16 at 22:16 +0800, yf.w...@mediatek.com wrote:
> From: Yunfei Wang 
> 
> Add the quirk IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT support, so that
> allows
> page table PA up to 35bit, not only in ZONE_DMA32.

Comment why this is needed.

e.g. For single normal zone.

> 
> Signed-off-by: Ning Li 
> Signed-off-by: Yunfei Wang 
> ---
>  drivers/iommu/mtk_iommu.c | 29 +
>  1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 6fd75a60abd6..1b9a876ef271 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -33,6 +33,7 @@
>  
>  #define REG_MMU_PT_BASE_ADDR 0x000
>  #define MMU_PT_ADDR_MASK GENMASK(31, 7)
> +#define MMU_PT_ADDR_2_0_MASK GENMASK(2, 0)
>  
>  #define REG_MMU_INVALIDATE   0x020
>  #define F_ALL_INVLD  0x2
> @@ -118,6 +119,7 @@
>  #define WR_THROT_EN  BIT(6)
>  #define HAS_LEGACY_IVRP_PADDRBIT(7)
>  #define IOVA_34_EN   BIT(8)
> +#define PGTABLE_PA_35_EN BIT(9)
>  
>  #define MTK_IOMMU_HAS_FLAG(pdata, _x) \
>   pdata)->flags) & (_x)) == (_x))
> @@ -401,6 +403,9 @@ static int mtk_iommu_domain_finalise(struct
> mtk_iommu_domain *dom,
>   .iommu_dev = data->dev,
>   };
>  
> + if (MTK_IOMMU_HAS_FLAG(data->plat_data, PGTABLE_PA_35_EN))
> + dom->cfg.quirks |= IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT;
> +
>   if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE))
>   dom->cfg.oas = data->enable_4GB ? 33 : 32;
>   else
> @@ -450,6 +455,7 @@ static int mtk_iommu_attach_device(struct
> iommu_domain *domain,
>   struct mtk_iommu_domain *dom = to_mtk_domain(domain);
>   struct device *m4udev = data->dev;
>   int ret, domid;
> + u32 regval;
>  
>   domid = mtk_iommu_get_domain_id(dev, data->plat_data);
>   if (domid < 0)
> @@ -472,8 +478,14 @@ static int mtk_iommu_attach_device(struct
> iommu_domain *domain,
>   return ret;
>   }
>   data->m4u_dom = dom;
> - writel(dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
> -data->base + REG_MMU_PT_BASE_ADDR);
> +
> + /* Bits[6:3] are invalid for mediatek platform */
> + if (MTK_IOMMU_HAS_FLAG(data->plat_data,
> PGTABLE_PA_35_EN))
> + regval = (dom->cfg.arm_v7s_cfg.ttbr &
> MMU_PT_ADDR_MASK) |
> +  (dom->cfg.arm_v7s_cfg.ttbr &
> MMU_PT_ADDR_2_0_MASK);

The bits[2:0] has already handled in ARM_V7S_TTBR_35BIT_PA of
patch[1/2], we need calculate it again here?

1) Extend arm_v7s_cfg.ttbr to u64, then we could put the special ttbr
logical into our file.

2) if 1) is not allowed, We have to put this in v7s.
if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT) {
   cfg->arm_v7s_cfg.ttbr = (paddr & GENMASK(31, 7)) |
upper_32_bits(paddr);
   return &data->iop;
}

This need Robin/Will confirm.

> + else
> + regval = dom->cfg.arm_v7s_cfg.ttbr &
> MMU_PT_ADDR_MASK;

Save this value to our data. then we don't need calculate it again in
the resume cb.   

> + writel(regval, data->base + REG_MMU_PT_BASE_ADDR);
>  
>   pm_runtime_put(m4udev);
>   }
> @@ -987,6 +999,7 @@ static int __maybe_unused
> mtk_iommu_runtime_resume(struct device *dev)
>   struct mtk_iommu_suspend_reg *reg = &data->reg;
>   struct mtk_iommu_domain *m4u_dom = data->m4u_dom;
>   void __iomem *base = data->base;
> + u32 regval;
>   int ret;
>  
>   ret = clk_prepare_enable(data->bclk);
> @@ -1010,7 +1023,14 @@ static int __maybe_unused
> mtk_iommu_runtime_resume(struct device *dev)
>   writel_relaxed(reg->int_main_control, base +
> REG_MMU_INT_MAIN_CONTROL);
>   writel_relaxed(reg->ivrp_paddr, base + REG_MMU_IVRP_PADDR);
>   writel_relaxed(reg->vld_pa_rng, base + REG_MMU_VLD_PA_RNG);
> - writel(m4u_dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK, base +
> REG_MMU_PT_BASE_ADDR);
> +
> + /* Bits[6:3] are invalid for mediatek platform */
> + if (MTK_IOMMU_HAS_FLAG(data->plat_data, PGTABLE_PA_35_EN))
> + regval = (m4u_dom->cfg.arm_v7s_cfg.ttbr &
> MMU_PT_ADDR_MASK) |
> +  (m4u_dom->cfg.arm_v7s_cfg.ttbr &
> MMU_PT_ADDR_2_0_MASK);
> + else
> + regval = m4u_dom->cfg.arm_v7s_cfg.ttbr &
> MMU_PT_ADDR_MASK;
> + writel(regval, base + REG_MMU_PT_BASE_ADDR);
>  
>   /*
>* Users may allocate dma buffer before they call
> pm_runtime_get,
> @@ -1038,7 +1058,8 @@ static const struct mtk_iommu_plat_data
> mt2712_data = {
>  
>  static const struct mtk_iommu_plat_data mt6779_data = {
>   .m4u_plat  = M4U_MT6779,
> - .flags = HAS_SUB_COMM | OUT_ORDER_WR_EN | WR_THROT_EN,
> + .flags = HAS_SUB_COMM | OUT_ORDER_WR_EN | WR_T

Re: [PATCH v4 1/6] iommu: Add a per domain PASID for DMA API

[Baolu Lu]

On 2022/5/19 02:21, Jacob Pan wrote:

DMA requests tagged with PASID can target individual IOMMU domains.
Introduce a domain-wide PASID for DMA API, it will be used on the same
mapping as legacy DMA without PASID. Let it be IOVA or PA in case of
identity domain.

Signed-off-by: Jacob Pan 
---
  include/linux/iommu.h | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 9405034e3013..36ad007084cc 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -106,6 +106,8 @@ struct iommu_domain {
enum iommu_page_response_code (*iopf_handler)(struct iommu_fault *fault,
  void *data);
void *fault_data;
+   ioasid_t dma_pasid; /* Used for DMA requests with PASID */


This looks more suitable for iommu_dma_cookie?


+   atomic_t dma_pasid_users;
  };
  
  static inline bool iommu_is_dma_domain(struct iommu_domain *domain)


Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 2/6] iommu: Add a helper to do PASID lookup from domain

[Christoph Hellwig]

On Wed, May 18, 2022 at 11:21:16AM -0700, Jacob Pan wrote:
> +ioasid_t iommu_get_pasid_from_domain(struct device *dev, struct iommu_domain 
> *domain)

Overly long line here.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 2/6] iommu: Add a helper to do PASID lookup from domain

[Baolu Lu]

On 2022/5/19 02:21, Jacob Pan wrote:

IOMMU group maintains a PASID array which stores the associated IOMMU
domains. This patch introduces a helper function to do domain to PASID
look up. It will be used by TLB flush and device-PASID attach verification.


Do you really need this?

The IOMMU driver has maintained a device tracking list for each domain.
It has been used for cache invalidation when unmap() is called against
dma domain.

Best regards,
baolu



Signed-off-by: Jacob Pan 
---
  drivers/iommu/iommu.c | 22 ++
  include/linux/iommu.h |  6 +-
  2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 00d0262a1fe9..22f44833db64 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3199,3 +3199,25 @@ struct iommu_domain *iommu_get_domain_for_iopf(struct 
device *dev,
  
  	return domain;

  }
+
+ioasid_t iommu_get_pasid_from_domain(struct device *dev, struct iommu_domain 
*domain)
+{
+   struct iommu_domain *tdomain;
+   struct iommu_group *group;
+   unsigned long index;
+   ioasid_t pasid = INVALID_IOASID;
+
+   group = iommu_group_get(dev);
+   if (!group)
+   return pasid;
+
+   xa_for_each(&group->pasid_array, index, tdomain) {
+   if (domain == tdomain) {
+   pasid = index;
+   break;
+   }
+   }
+   iommu_group_put(group);
+
+   return pasid;
+}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 36ad007084cc..c0440a4be699 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -694,7 +694,7 @@ void iommu_detach_device_pasid(struct iommu_domain *domain,
   struct device *dev, ioasid_t pasid);
  struct iommu_domain *
  iommu_get_domain_for_iopf(struct device *dev, ioasid_t pasid);
-
+ioasid_t iommu_get_pasid_from_domain(struct device *dev, struct iommu_domain 
*domain);
  #else /* CONFIG_IOMMU_API */
  
  struct iommu_ops {};

@@ -1070,6 +1070,10 @@ iommu_get_domain_for_iopf(struct device *dev, ioasid_t 
pasid)
  {
return NULL;
  }
+static ioasid_t iommu_get_pasid_from_domain(struct device *dev, struct 
iommu_domain *domain)
+{
+   return INVALID_IOASID;
+}
  #endif /* CONFIG_IOMMU_API */
  
  #ifdef CONFIG_IOMMU_SVA


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v5 1/2] iommu/io-pgtable-arm-v7s: Add a quirk to allow pgtable PA up to 35bit

[Miles Chen via iommu]

Hi Yunfei,

> The calling to kmem_cache_alloc for level 2 pgtable allocation may run
> in atomic context, and it fails sometimes when DMA32 zone runs out of
> memory.
> 
> Since Mediatek IOMMU hardware support at most 35bit PA in pgtable,
> so add a quirk to allow the PA of pgtables support up to bit35.
> 
> Signed-off-by: Ning Li 
> Signed-off-by: Yunfei Wang 
> ---
>  drivers/iommu/io-pgtable-arm-v7s.c | 56 ++
>  include/linux/io-pgtable.h | 15 +---
>  2 files changed, 52 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c 
> b/drivers/iommu/io-pgtable-arm-v7s.c

...snip...

> + gfp_t gfp_l1 = __GFP_ZERO | ARM_V7S_TABLE_GFP_DMA;
>   struct io_pgtable_cfg *cfg = &data->iop.cfg;
>   struct device *dev = cfg->iommu_dev;
>   phys_addr_t phys;
> @@ -241,9 +251,11 @@ static void *__arm_v7s_alloc_table(int lvl, gfp_t gfp,
>   size_t size = ARM_V7S_TABLE_SIZE(lvl, cfg);
>   void *table = NULL;
>  
> + if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_TTBR_EXT)
> + gfp_l1 = __GFP_ZERO;

__GFP_ZERO is an action modifier, if we do not want
ARM_V7S_TABLE_GFP_DMA (GFP_DMA/GFP_DMA32), use gfp_l1 = (GFP_KERNEL | 
__GFP_ZERO)

thanks,
Miles
> +
>   if (lvl == 1)
> - table = (void *)__get_free_pages(
> - __GFP_ZERO | ARM_V7S_TABLE_GFP_DMA, get_order(size));
> + table = (void *)__get_free_pages(gfp_l1, get_order(size));
>   else if (lvl == 2)
>   table = kmem_cache_zalloc(data->l2_tables, gfp);
>  
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2] iommu/vt-d: Make DMAR_UNITS_SUPPORTED a config setting

[Steve Wahl]

On Fri, May 13, 2022 at 10:09:46AM +0800, Baolu Lu wrote:
> On 2022/5/13 07:12, Steve Wahl wrote:
> > On Thu, May 12, 2022 at 10:13:09AM -0500, Steve Wahl wrote:
> > > To support up to 64 sockets with 10 DMAR units each (640), make the
> > > value of DMAR_UNITS_SUPPORTED adjustable by a config variable,
> > > CONFIG_DMAR_UNITS_SUPPORTED, and make it's default 1024 when MAXSMP is
> > > set.
> > > 
> > > If the available hardware exceeds DMAR_UNITS_SUPPORTED (previously set
> > > to MAX_IO_APICS, or 128), it causes these messages: "DMAR: Failed to
> > > allocate seq_id", "DMAR: Parse DMAR table failure.", and "x2apic: IRQ
> > > remapping doesn't support X2APIC mode x2apic disabled"; and the system
> > > fails to boot properly.
> > > 
> > > Signed-off-by: Steve Wahl 
> > 
> > I've received a report from the kernel test robot ,
> > that this patch causes an error (shown below) when
> > CONFIG_IOMMU_SUPPORT is not set.
> > 
> > In my opinion, this is because include/linux/dmar.h and
> > include/linux/intel-iommu are being #included when they are not really
> > being used.
> > 
> > I've tried placing the contents of intel-iommu.h within an #ifdef
> > CONFIG_INTEL_IOMMU, and that fixes the problem.
> > 
> > Two questions:
> > 
> > A) Is this the desired approach to to the fix?
> 
> Most part of include/linux/intel-iommu.h is private to Intel IOMMU
> driver. They should be put in a header like drivers/iommu/intel
> /iommu.h. Eventually, we should remove include/linux/intel-iommu.h
> and device drivers interact with iommu subsystem through the IOMMU
> kAPIs.
> 
> Best regards,
> baolu

Baolu's recent patch to move intel-iommu.h private still allows my
[PATCH v2] to apply with no changes, and gets rid of the compile
errors when CONFIG_IOMMU_SUPPORT is not set, so the kernel test robot
should be happy now.

Is there another step I should do to bring this patch back into focus?

Thanks.

--> Steve

-- 
Steve Wahl, Hewlett Packard Enterprise
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3] iommu: iommu_group_claim_dma_owner() must always assign a domain

[Eric Farman]

On Wed, 2022-05-18 at 16:14 -0300, Jason Gunthorpe wrote:
> On Wed, May 18, 2022 at 02:50:36PM -0400, Eric Farman wrote:
> 
> > I got a heads up from Matt about the s390 KVM vfio- variants
> > failing on
> > linux-next.
> > 
> > For vfio-ap and vfio-ccw, they fail on the above error. Both calls
> > to
> > __iommu_domain_alloc fail because while dev->dev->bus is non-NULL
> > (it
> > points to the mdev bus_type registered in mdev_init()), the bus-
> > > iommu_ops pointer is NULL. Which makes sense; the iommu_group is
> > > vfio-
> > noiommu, via vfio_register_emulated_iommu_dev(), and mdev didn't
> > establish an iommu_ops for its bus.
> 
> Oh, I think this is a VFIO problem, the iommu layer should not have
> to
> deal with these fake non-iommu groups.
> 
> From 9884850a5ceac957e6715beab0888294d4088877 Mon Sep 17 00:00:00
> 2001
> From: Jason Gunthorpe 
> Date: Wed, 18 May 2022 16:03:34 -0300
> Subject: [PATCH] vfio: Do not manipulate iommu dma_owner for fake
> iommu groups
> 
> Since asserting dma ownership now causes the group to have its DMA
> blocked
> the iommu layer requires a working iommu. This means the dma_owner
> APIs
> cannot be used on the fake groups that VFIO creates. Test for this
> and
> avoid calling them.
> 
> Otherwise asserting dma ownership will fail for VFIO mdev devices as
> a
> BLOCKING iommu_domain cannot be allocated due to the NULL iommu ops.
> 
> Fixes: 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must
> always assign a domain")
> Reported-by: Eric Farman 
> Signed-off-by: Jason Gunthorpe 

Ah, nice. That takes care of it for me, thank you!

Tested-by: Eric Farman 

> ---
>  drivers/vfio/vfio.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> I think this will have to go through Alex's tree due to all the other
> rework
> in this area.
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index cfcff7764403fc..f5ed03897210c3 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -927,7 +927,8 @@ static void __vfio_group_unset_container(struct
> vfio_group *group)
>   driver->ops->detach_group(container->iommu_data,
> group->iommu_group);
>  
> - iommu_group_release_dma_owner(group->iommu_group);
> + if (group->type == VFIO_IOMMU)
> + iommu_group_release_dma_owner(group->iommu_group);
>  
>   group->container = NULL;
>   group->container_users = 0;
> @@ -1001,9 +1002,11 @@ static int vfio_group_set_container(struct
> vfio_group *group, int container_fd)
>   goto unlock_out;
>   }
>  
> - ret = iommu_group_claim_dma_owner(group->iommu_group, f.file);
> - if (ret)
> - goto unlock_out;
> + if (group->type == VFIO_IOMMU) {
> + ret = iommu_group_claim_dma_owner(group->iommu_group,
> f.file);
> + if (ret)
> + goto unlock_out;
> + }
>  
>   driver = container->iommu_driver;
>   if (driver) {
> @@ -1011,7 +1014,9 @@ static int vfio_group_set_container(struct
> vfio_group *group, int container_fd)
>   group->iommu_group,
>   group->type);
>   if (ret) {
> - iommu_group_release_dma_owner(group-
> >iommu_group);
> + if (group->type == VFIO_IOMMU)
> + iommu_group_release_dma_owner(
> + group->iommu_group);
>   goto unlock_out;
>   }
>   }

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3] iommu: iommu_group_claim_dma_owner() must always assign a domain

[Jason Gunthorpe via iommu]

On Wed, May 18, 2022 at 02:50:36PM -0400, Eric Farman wrote:

> I got a heads up from Matt about the s390 KVM vfio- variants failing on
> linux-next.
> 
> For vfio-ap and vfio-ccw, they fail on the above error. Both calls to
> __iommu_domain_alloc fail because while dev->dev->bus is non-NULL (it
> points to the mdev bus_type registered in mdev_init()), the bus-
> >iommu_ops pointer is NULL. Which makes sense; the iommu_group is vfio-
> noiommu, via vfio_register_emulated_iommu_dev(), and mdev didn't
> establish an iommu_ops for its bus.

Oh, I think this is a VFIO problem, the iommu layer should not have to
deal with these fake non-iommu groups.

>From 9884850a5ceac957e6715beab0888294d4088877 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe 
Date: Wed, 18 May 2022 16:03:34 -0300
Subject: [PATCH] vfio: Do not manipulate iommu dma_owner for fake iommu groups

Since asserting dma ownership now causes the group to have its DMA blocked
the iommu layer requires a working iommu. This means the dma_owner APIs
cannot be used on the fake groups that VFIO creates. Test for this and
avoid calling them.

Otherwise asserting dma ownership will fail for VFIO mdev devices as a
BLOCKING iommu_domain cannot be allocated due to the NULL iommu ops.

Fixes: 0286300e6045 ("iommu: iommu_group_claim_dma_owner() must always assign a 
domain")
Reported-by: Eric Farman 
Signed-off-by: Jason Gunthorpe 
---
 drivers/vfio/vfio.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

I think this will have to go through Alex's tree due to all the other rework
in this area.

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index cfcff7764403fc..f5ed03897210c3 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -927,7 +927,8 @@ static void __vfio_group_unset_container(struct vfio_group 
*group)
driver->ops->detach_group(container->iommu_data,
  group->iommu_group);
 
-   iommu_group_release_dma_owner(group->iommu_group);
+   if (group->type == VFIO_IOMMU)
+   iommu_group_release_dma_owner(group->iommu_group);
 
group->container = NULL;
group->container_users = 0;
@@ -1001,9 +1002,11 @@ static int vfio_group_set_container(struct vfio_group 
*group, int container_fd)
goto unlock_out;
}
 
-   ret = iommu_group_claim_dma_owner(group->iommu_group, f.file);
-   if (ret)
-   goto unlock_out;
+   if (group->type == VFIO_IOMMU) {
+   ret = iommu_group_claim_dma_owner(group->iommu_group, f.file);
+   if (ret)
+   goto unlock_out;
+   }
 
driver = container->iommu_driver;
if (driver) {
@@ -1011,7 +1014,9 @@ static int vfio_group_set_container(struct vfio_group 
*group, int container_fd)
group->iommu_group,
group->type);
if (ret) {
-   iommu_group_release_dma_owner(group->iommu_group);
+   if (group->type == VFIO_IOMMU)
+   iommu_group_release_dma_owner(
+   group->iommu_group);
goto unlock_out;
}
}
-- 
2.36.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3] iommu: iommu_group_claim_dma_owner() must always assign a domain

[Eric Farman]

On Mon, 2022-05-09 at 13:19 -0300, Jason Gunthorpe via iommu wrote:
> Once the group enters 'owned' mode it can never be assigned back to
> the
> default_domain or to a NULL domain. It must always be actively
> assigned to
> a current domain. If the caller hasn't provided a domain then the
> core
> must provide an explicit DMA blocking domain that has no DMA map.
> 
> Lazily create a group-global blocking DMA domain when
> iommu_group_claim_dma_owner is first called and immediately assign
> the
> group to it. This ensures that DMA is immediately fully isolated on
> all
> IOMMU drivers.
> 
> If the user attaches/detaches while owned then detach will set the
> group
> back to the blocking domain.
> 
> Slightly reorganize the call chains so that
> __iommu_group_set_core_domain() is the function that removes any
> caller
> configured domain and sets the domains back a core owned domain with
> an
> appropriate lifetime.
> 
> __iommu_group_set_domain() is the worker function that can change the
> domain assigned to a group to any target domain, including NULL.
> 
> Add comments clarifying how the NULL vs detach_dev vs default_domain
> works
> based on Robin's remarks.
> 
> This fixes an oops with VFIO and SMMUv3 because VFIO will call
> iommu_detach_group() and then immediately iommu_domain_free(), but
> SMMUv3 has no way to know that the domain it is holding a pointer to
> has been freed. Now the iommu_detach_group() will assign the blocking
> domain and SMMUv3 will no longer hold a stale domain reference.
> 
> Fixes: 1ea2a07a532b ("iommu: Add DMA ownership management
> interfaces")
> Reported-by: Qian Cai 
> Tested-by: Baolu Lu 
> Tested-by: Nicolin Chen 
> Co-developed-by: Robin Murphy 
> Signed-off-by: Robin Murphy 
> Signed-off-by: Jason Gunthorpe 
> --
> 
> Just minor polishing as discussed
> 
> v3:
>  - Change names to __iommu_group_set_domain() /
>__iommu_group_set_core_domain()
>  - Clarify comments
>  - Call __iommu_group_set_domain() directly in
>iommu_group_release_dma_owner() since we know it is always
> selecting
>the default_domain
> v2: 
> https://lore.kernel.org/r/0-v2-f62259511ac0+6-iommu_dma_block_...@nvidia.com
>  - Remove redundant detach_dev ops check in __iommu_detach_device and
>make the added WARN_ON fail instead
>  - Check for blocking_domain in __iommu_attach_group() so VFIO can
>actually attach a new group
>  - Update comments and spelling
>  - Fix missed change to new_domain in iommu_group_do_detach_device()
> v1: 
> https://lore.kernel.org/r/0-v1-6e9d2d0a759d+11b-iommu_dma_block_...@nvidia.com
> 
> ---
>  drivers/iommu/iommu.c | 127 ++
> 
>  1 file changed, 91 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 0c42ece2585406..0b22e51e90f416 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -44,6 +44,7 @@ struct iommu_group {
>   char *name;
>   int id;
>   struct iommu_domain *default_domain;
> + struct iommu_domain *blocking_domain;
>   struct iommu_domain *domain;
>   struct list_head entry;
>   unsigned int owner_cnt;
> @@ -82,8 +83,8 @@ static int __iommu_attach_device(struct
> iommu_domain *domain,
>struct device *dev);
>  static int __iommu_attach_group(struct iommu_domain *domain,
>   struct iommu_group *group);
> -static void __iommu_detach_group(struct iommu_domain *domain,
> -  struct iommu_group *group);
> +static int __iommu_group_set_domain(struct iommu_group *group,
> + struct iommu_domain *new_domain);
>  static int iommu_create_device_direct_mappings(struct iommu_group
> *group,
>  struct device *dev);
>  static struct iommu_group *iommu_group_get_for_dev(struct device
> *dev);
> @@ -596,6 +597,8 @@ static void iommu_group_release(struct kobject
> *kobj)
>  
>   if (group->default_domain)
>   iommu_domain_free(group->default_domain);
> + if (group->blocking_domain)
> + iommu_domain_free(group->blocking_domain);
>  
>   kfree(group->name);
>   kfree(group);
> @@ -1907,6 +1910,24 @@ void iommu_domain_free(struct iommu_domain
> *domain)
>  }
>  EXPORT_SYMBOL_GPL(iommu_domain_free);
>  
> +/*
> + * Put the group's domain back to the appropriate core-owned domain
> - either the
> + * standard kernel-mode DMA configuration or an all-DMA-blocked
> domain.
> + */
> +static void __iommu_group_set_core_domain(struct iommu_group *group)
> +{
> + struct iommu_domain *new_domain;
> + int ret;
> +
> + if (group->owner)
> + new_domain = group->blocking_domain;
> + else
> + new_domain = group->default_domain;
> +
> + ret = __iommu_group_set_domain(group, new_domain);
> + WARN(ret, "iommu driver failed to attach the default/blocking
> domain");
> +}
> +
>  static int __iommu_at

Re: [PATCH v3 1/4] iommu/vt-d: Implement domain ops for attach_dev_pasid

[Jason Gunthorpe via iommu]

On Wed, May 18, 2022 at 11:42:04AM -0700, Jacob Pan wrote:

> > Yes.. It seems inefficient to iterate over that xarray multiple times
> > on the flush hot path, but maybe there is little choice. Try to use
> > use the xas iterators under the xa_lock spinlock..
> > 
> xas_for_each takes a max range, here we don't really have one. So I posted
> v4 w/o using the xas advanced API. Please let me know if you have
> suggestions.

You are supposed to use ULONG_MAX in cases like that.

> xa_for_each takes RCU read lock, it should be fast for tlb flush, right? The
> worst case maybe over flush when we have stale data but should be very rare.

Not really, xa_for_each walks the tree for every iteration, it is
slower than a linked list walk in any cases where the xarray is
multi-node. xas_for_each is able to retain a pointer where it is in
the tree so each iteration is usually just a pointer increment.

The downside is you cannot sleep while doing xas_for_each

> > The challenge will be accessing the group xa in the first place, but
> > maybe the core code can gain a function call to return a pointer to
> > that XA or something..
 
> I added a helper function to find the matching DMA API PASID in v4.

Again, why are we focused on DMA API? Nothing you build here should be
DMA API beyond the fact that the iommu_domain being attached is the
default domain.

Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg

[Rob Herring]

On Wed, May 18, 2022 at 12:07:58PM +0100, Robin Murphy wrote:
> On 2022-05-18 09:29, AngeloGioacchino Del Regno wrote:
> > Il 17/05/22 16:12, Robin Murphy ha scritto:
> > > On 2022-05-17 14:21, AngeloGioacchino Del Regno wrote:
> > > > This driver will get support for more SoCs and the list of infracfg
> > > > compatibles is expected to grow: in order to prevent getting this
> > > > situation out of control and see a long list of compatible strings,
> > > > add support to retrieve a handle to infracfg's regmap through a
> > > > new "mediatek,infracfg" phandle.
> > > > 
> > > > In order to keep retrocompatibility with older devicetrees, the old
> > > > way is kept in place, but also a dev_warn() was added to advertise
> > > > this change in hope that the user will see it and eventually update
> > > > the devicetree if this is possible.
> > > > 
> > > > Signed-off-by: AngeloGioacchino Del Regno
> > > > 
> > > > ---
> > > >   drivers/iommu/mtk_iommu.c | 40 +--
> > > >   1 file changed, 26 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > > > index 71b2ace74cd6..cfaaa98d2b50 100644
> > > > --- a/drivers/iommu/mtk_iommu.c
> > > > +++ b/drivers/iommu/mtk_iommu.c
> > > > @@ -1134,22 +1134,34 @@ static int mtk_iommu_probe(struct
> > > > platform_device *pdev)
> > > >   data->protect_base = ALIGN(virt_to_phys(protect),
> > > > MTK_PROTECT_PA_ALIGN);
> > > >   if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
> > > > -    switch (data->plat_data->m4u_plat) {
> > > > -    case M4U_MT2712:
> > > > -    p = "mediatek,mt2712-infracfg";
> > > > -    break;
> > > > -    case M4U_MT8173:
> > > > -    p = "mediatek,mt8173-infracfg";
> > > > -    break;
> > > > -    default:
> > > > -    p = NULL;
> > > > +    infracfg =
> > > > syscon_regmap_lookup_by_phandle(dev->of_node,
> > > > "mediatek,infracfg");
> > > > +    if (IS_ERR(infracfg)) {
> > > > +    dev_warn(dev, "Cannot find phandle to mediatek,infracfg:"
> > > > +  " Please update your devicetree.\n");
> > > 
> > > Is this really a dev_warn-level problem? There's no functional
> > > impact, given that we can't stop supporting the original binding any
> > > time soon, if ever, so I suspect this is more likely to just annoy
> > > users and CI systems than effect any significant change.
> > > 
> > 
> > The upstream devicetrees were updated to use the new handle and this is
> > a way to
> > warn about having outdated DTs... besides, I believe that CIs will
> > always get the
> > devicetree from the same tree that the kernel was compiled from (hence
> > no message
> > will be thrown).
> > 
> > In any case, if you think that a dev_info would be more appropriate, I
> > can change
> > that no problem.
> 
> If there's some functional impact from using the old binding vs. the new one
> then it's reasonable to inform the user of that (as we do in arm-smmu, for
> example).
> 
> However if you change an established binding for non-functional reasons,
> then you get to support both bindings, and it's not the end user's problem
> at all. There seems to be zero reason to update an existing DTB for this
> difference alone, so TBH I don't think it deserves a message at all.

It's also not the kernel's job to validate the DT. It's horrible at it 
and we have something else now.

Rob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 7/8] dt-bindings: iommu: mediatek: Require mediatek,infracfg for mt2712/8173

[Rob Herring]

On Wed, May 18, 2022 at 10:14:43AM +0200, AngeloGioacchino Del Regno wrote:
> Il 18/05/22 03:41, Rob Herring ha scritto:
> > On Tue, May 17, 2022 at 03:21:06PM +0200, AngeloGioacchino Del Regno wrote:
> > > Both MT2712 and MT8173 got a mediatek,infracfg phandle: add that to
> > > the required properties for these SoCs to deprecate the old way of
> > > looking for SoC-specific infracfg compatible in the entire devicetree.
> > 
> > Wait, what? If there's only one possible node that can match, I prefer
> > the 'old way'. Until we implemented a phandle cache, searching the
> > entire tree was how phandle lookups worked too, so not any better.
> > 
> > But if this makes things more consistent,
> > 
> > Acked-by: Rob Herring 
> 
> 
> Hello Rob,
> 
> This makes things definitely more consistent, as it's done like that on
> mtk-pm-domains and other mtk drivers as well.
> 
> The main reason why this phandle is useful, here and in other drivers, is
> that we're seeing a list of compatibles that is growing more and more, so
> you see stuff like (mockup names warning):
> 
> switch (some_model)
> case MT1000:
>   p = "mediatek,mt1000-infracfg";
>   break;
> case MT1001:
>   p = "mediatek,mt1001-infracfg";
>   break;
> case MT1002:
>   p = "mediatek,mt1002-infracfg";
>   break;
> .add another 20 SoCs, replicate this switch for 4/5 drivers

This type of property is used for poking random bits in another block 
(that's usually a collection of random bits). These interfaces don't 
tend to be that stable across many SoC generations. As there's no 
abstraction beyond perhaps what the offset is, the client side ends up 
needing to know the specifics of that block anyways. If the block is 
that stable, then perhaps it needs a common fallback compatible.

Sometimes these instances are also just places we haven't created a 
common subsystem for.

> and this is why I want the mtk_iommu driver to also get that phandle like
> some other drivers are already doing.

As I said, fine.

Rob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 0/7] iommu/vt-d: Make intel-iommu.h private

[Steve Wahl]

On Sat, May 14, 2022 at 09:43:15AM +0800, Lu Baolu wrote:
> Hi folks,
> 
> The include/linux/intel-iommu.h should be private to the Intel IOMMU
> driver. Other drivers or components should interact with the IOMMU
> drivers through the kAPIs provided by the iommu core.
> 
> This series cleanups all includes of intel-iommu.h outside of the Intel
> IOMMU driver and move this header from include/linux to
> drivers/iommu/intel/.
> 
> No functional changes intended. Please help to review and suggest.

I went through and examined the changes as well.  These changes make the
robots complaint against my patch go away, which is great by me!

Reviewed-by: Steve Wahl 

-- 
Steve Wahl, Hewlett Packard Enterprise
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v3 1/4] iommu/vt-d: Implement domain ops for attach_dev_pasid

[Jacob Pan]

Hi Jason,

On Wed, 11 May 2022 15:29:08 -0300, Jason Gunthorpe  wrote:

> On Wed, May 11, 2022 at 10:25:21AM -0700, Jacob Pan wrote:
> > Hi Jason,
> > 
> > On Wed, 11 May 2022 14:00:25 -0300, Jason Gunthorpe 
> > wrote: 
> > > On Wed, May 11, 2022 at 10:02:16AM -0700, Jacob Pan wrote:  
> > > > > > If not global, perhaps we could have a list of pasids (e.g.
> > > > > > xarray) attached to the device_domain_info. The TLB flush logic
> > > > > > would just go through the list w/o caring what the PASIDs are
> > > > > > for. Does it make sense to you?  
> > > > > 
> > > > > Sort of, but we shouldn't duplicate xarrays - the group already
> > > > > has this xarray - need to find some way to allow access to it
> > > > > from the driver.
> > > > > 
> > > > I am not following,  here are the PASIDs for devTLB flush which is
> > > > per device. Why group?
> > > 
> > > Because group is where the core code stores it.  
> > I see, with singleton group. I guess I can let dma-iommu code call
> > 
> > iommu_attach_dma_pasid {
> > iommu_attach_device_pasid();
> > Then the PASID will be stored in the group xa.  
> 
> Yes, again, the dma-iommu should not be any different from the normal
> unmanaged path. At this point there is no longer any difference, we
> should not invent new ones.
> 
> > The flush code can retrieve PASIDs from device_domain_info.device ->
> > group -> pasid_array.  Thanks for pointing it out, I missed the new
> > pasid_array.  
> 
> Yes.. It seems inefficient to iterate over that xarray multiple times
> on the flush hot path, but maybe there is little choice. Try to use
> use the xas iterators under the xa_lock spinlock..
> 
xas_for_each takes a max range, here we don't really have one. So I posted
v4 w/o using the xas advanced API. Please let me know if you have
suggestions.
xa_for_each takes RCU read lock, it should be fast for tlb flush, right? The
worst case maybe over flush when we have stale data but should be very rare.

> The challenge will be accessing the group xa in the first place, but
> maybe the core code can gain a function call to return a pointer to
> that XA or something..
> 
I added a helper function to find the matching DMA API PASID in v4.


Thanks,

Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v4 6/6] iommu/vt-d: Delete unused SVM flag

[Jacob Pan]

Supervisor PASID for SVA/SVM is no longer supported, delete the unused
flag.

Signed-off-by: Jacob Pan 
---
 drivers/iommu/intel/svm.c |  2 +-
 include/linux/intel-svm.h | 13 -
 2 files changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 44331db060e4..5b220d464218 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -750,7 +750,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
 * to unbind the mm while any page faults are 
outstanding.
 */
svm = pasid_private_find(req->pasid);
-   if (IS_ERR_OR_NULL(svm) || (svm->flags & 
SVM_FLAG_SUPERVISOR_MODE))
+   if (IS_ERR_OR_NULL(svm))
goto bad_req;
}
 
diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
index b3b125b332aa..6835a665c195 100644
--- a/include/linux/intel-svm.h
+++ b/include/linux/intel-svm.h
@@ -13,17 +13,4 @@
 #define PRQ_RING_MASK  ((0x1000 << PRQ_ORDER) - 0x20)
 #define PRQ_DEPTH  ((0x1000 << PRQ_ORDER) >> 5)
 
-/*
- * The SVM_FLAG_SUPERVISOR_MODE flag requests a PASID which can be used only
- * for access to kernel addresses. No IOTLB flushes are automatically done
- * for kernel mappings; it is valid only for access to the kernel's static
- * 1:1 mapping of physical memory — not to vmalloc or even module mappings.
- * A future API addition may permit the use of such ranges, by means of an
- * explicit IOTLB flush call (akin to the DMA API's unmap method).
- *
- * It is unlikely that we will ever hook into flush_tlb_kernel_range() to
- * do such IOTLB flushes automatically.
- */
-#define SVM_FLAG_SUPERVISOR_MODE   BIT(0)
-
 #endif /* __INTEL_SVM_H__ */
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v4 2/6] iommu: Add a helper to do PASID lookup from domain

[Jacob Pan]

IOMMU group maintains a PASID array which stores the associated IOMMU
domains. This patch introduces a helper function to do domain to PASID
look up. It will be used by TLB flush and device-PASID attach verification.

Signed-off-by: Jacob Pan 
---
 drivers/iommu/iommu.c | 22 ++
 include/linux/iommu.h |  6 +-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 00d0262a1fe9..22f44833db64 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3199,3 +3199,25 @@ struct iommu_domain *iommu_get_domain_for_iopf(struct 
device *dev,
 
return domain;
 }
+
+ioasid_t iommu_get_pasid_from_domain(struct device *dev, struct iommu_domain 
*domain)
+{
+   struct iommu_domain *tdomain;
+   struct iommu_group *group;
+   unsigned long index;
+   ioasid_t pasid = INVALID_IOASID;
+
+   group = iommu_group_get(dev);
+   if (!group)
+   return pasid;
+
+   xa_for_each(&group->pasid_array, index, tdomain) {
+   if (domain == tdomain) {
+   pasid = index;
+   break;
+   }
+   }
+   iommu_group_put(group);
+
+   return pasid;
+}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 36ad007084cc..c0440a4be699 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -694,7 +694,7 @@ void iommu_detach_device_pasid(struct iommu_domain *domain,
   struct device *dev, ioasid_t pasid);
 struct iommu_domain *
 iommu_get_domain_for_iopf(struct device *dev, ioasid_t pasid);
-
+ioasid_t iommu_get_pasid_from_domain(struct device *dev, struct iommu_domain 
*domain);
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};
@@ -1070,6 +1070,10 @@ iommu_get_domain_for_iopf(struct device *dev, ioasid_t 
pasid)
 {
return NULL;
 }
+static ioasid_t iommu_get_pasid_from_domain(struct device *dev, struct 
iommu_domain *domain)
+{
+   return INVALID_IOASID;
+}
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_SVA
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v4 5/6] dmaengine: idxd: Use DMA API for in-kernel DMA with PASID

[Jacob Pan]

The current in-kernel supervisor PASID support is based on the SVM/SVA
machinery in SVA lib. The binding between a kernel PASID and kernel
mapping has many flaws. See discussions in the link below.

This patch enables in-kernel DMA by switching from SVA lib to the
standard DMA mapping APIs. Since both DMA requests with and without
PASIDs are mapped identically, there is no change to how DMA APIs are
used after the kernel PASID is enabled.

Link: https://lore.kernel.org/linux-iommu/20210511194726.gp1002...@nvidia.com/
Signed-off-by: Jacob Pan 
---
 drivers/dma/idxd/idxd.h  |  1 -
 drivers/dma/idxd/init.c  | 34 +-
 drivers/dma/idxd/sysfs.c |  7 ---
 3 files changed, 9 insertions(+), 33 deletions(-)

diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index ccbefd0be617..190b08bd7c08 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -277,7 +277,6 @@ struct idxd_device {
struct idxd_wq **wqs;
struct idxd_engine **engines;
 
-   struct iommu_sva *sva;
unsigned int pasid;
 
int num_groups;
diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index e1b5d1e4a949..e2e1c0eae6d6 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include "../dmaengine.h"
@@ -466,36 +467,22 @@ static struct idxd_device *idxd_alloc(struct pci_dev 
*pdev, struct idxd_driver_d
 
 static int idxd_enable_system_pasid(struct idxd_device *idxd)
 {
-   int flags;
-   unsigned int pasid;
-   struct iommu_sva *sva;
+   u32 pasid;
+   int ret;
 
-   flags = SVM_FLAG_SUPERVISOR_MODE;
-
-   sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, &flags);
-   if (IS_ERR(sva)) {
-   dev_warn(&idxd->pdev->dev,
-"iommu sva bind failed: %ld\n", PTR_ERR(sva));
-   return PTR_ERR(sva);
-   }
-
-   pasid = iommu_sva_get_pasid(sva);
-   if (pasid == IOMMU_PASID_INVALID) {
-   iommu_sva_unbind_device(sva);
-   return -ENODEV;
+   ret = iommu_attach_dma_pasid(&idxd->pdev->dev, &pasid);
+   if (ret) {
+   dev_err(&idxd->pdev->dev, "No DMA PASID %d\n", ret);
+   return ret;
}
-
-   idxd->sva = sva;
idxd->pasid = pasid;
-   dev_dbg(&idxd->pdev->dev, "system pasid: %u\n", pasid);
+
return 0;
 }
 
 static void idxd_disable_system_pasid(struct idxd_device *idxd)
 {
-
-   iommu_sva_unbind_device(idxd->sva);
-   idxd->sva = NULL;
+   iommu_detach_dma_pasid(&idxd->pdev->dev);
 }
 
 static int idxd_probe(struct idxd_device *idxd)
@@ -527,10 +514,7 @@ static int idxd_probe(struct idxd_device *idxd)
else
set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
}
-   } else if (!sva) {
-   dev_warn(dev, "User forced SVA off via module param.\n");
}
-
idxd_read_caps(idxd);
idxd_read_table_offsets(idxd);
 
diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index dfd549685c46..a48928973bd4 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -839,13 +839,6 @@ static ssize_t wq_name_store(struct device *dev,
if (strlen(buf) > WQ_NAME_SIZE || strlen(buf) == 0)
return -EINVAL;
 
-   /*
-* This is temporarily placed here until we have SVM support for
-* dmaengine.
-*/
-   if (wq->type == IDXD_WQT_KERNEL && device_pasid_enabled(wq->idxd))
-   return -EOPNOTSUPP;
-
memset(wq->name, 0, WQ_NAME_SIZE + 1);
strncpy(wq->name, buf, WQ_NAME_SIZE);
strreplace(wq->name, '\n', '\0');
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v4 4/6] iommu: Add PASID support for DMA mapping API users

[Jacob Pan]

DMA mapping API is the de facto standard for in-kernel DMA. It operates
on a per device/RID basis which is not PASID-aware.

Some modern devices such as Intel Data Streaming Accelerator, PASID is
required for certain work submissions. To allow such devices use DMA
mapping API, we need the following functionalities:
1. Provide device a way to retrieve a PASID for work submission within
the kernel
2. Enable the kernel PASID on the IOMMU for the device
3. Attach the kernel PASID to the device's default DMA domain, let it
be IOVA or physical address in case of pass-through.

This patch introduces a driver facing API that enables DMA API
PASID usage. Once enabled, device drivers can continue to use DMA APIs as
is. There is no difference in dma_handle between without PASID and with
PASID.

Signed-off-by: Jacob Pan 
---
 drivers/iommu/dma-iommu.c | 114 ++
 include/linux/dma-iommu.h |   3 +
 2 files changed, 117 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 1ca85d37eeab..6ad7ba619ef0 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -34,6 +34,8 @@ struct iommu_dma_msi_page {
phys_addr_t phys;
 };
 
+static DECLARE_IOASID_SET(iommu_dma_pasid);
+
 enum iommu_dma_cookie_type {
IOMMU_DMA_IOVA_COOKIE,
IOMMU_DMA_MSI_COOKIE,
@@ -370,6 +372,118 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
domain->iova_cookie = NULL;
 }
 
+/* Protect iommu_domain DMA PASID data */
+static DEFINE_MUTEX(dma_pasid_lock);
+/**
+ * iommu_attach_dma_pasid --Attach a PASID for in-kernel DMA. Use the device's
+ * DMA domain.
+ * @dev: Device to be enabled
+ * @pasid: The returned kernel PASID to be used for DMA
+ *
+ * DMA request with PASID will be mapped the same way as the legacy DMA.
+ * If the device is in pass-through, PASID will also pass-through. If the
+ * device is in IOVA, the PASID will point to the same IOVA page table.
+ *
+ * @return err code or 0 on success
+ */
+int iommu_attach_dma_pasid(struct device *dev, ioasid_t *pasid)
+{
+   struct iommu_domain *dom;
+   ioasid_t id, max;
+   int ret = 0;
+
+   dom = iommu_get_domain_for_dev(dev);
+   if (!dom || !dom->ops || !dom->ops->attach_dev_pasid)
+   return -ENODEV;
+
+   /* Only support domain types that DMA API can be used */
+   if (dom->type == IOMMU_DOMAIN_UNMANAGED ||
+   dom->type == IOMMU_DOMAIN_BLOCKED) {
+   dev_warn(dev, "Invalid domain type %d", dom->type);
+   return -EPERM;
+   }
+
+   mutex_lock(&dma_pasid_lock);
+   id = dom->dma_pasid;
+   if (!id) {
+   /*
+* First device to use PASID in its DMA domain, allocate
+* a single PASID per DMA domain is all we need, it is also
+* good for performance when it comes down to IOTLB flush.
+*/
+   max = 1U << dev->iommu->pasid_bits;
+   if (!max) {
+   ret = -EINVAL;
+   goto done_unlock;
+   }
+
+   id = ioasid_alloc(&iommu_dma_pasid, 1, max, dev);
+   if (id == INVALID_IOASID) {
+   ret = -ENOMEM;
+   goto done_unlock;
+   }
+
+   dom->dma_pasid = id;
+   atomic_set(&dom->dma_pasid_users, 1);
+   }
+
+   ret = iommu_attach_device_pasid(dom, dev, id);
+   if (!ret) {
+   *pasid = id;
+   atomic_inc(&dom->dma_pasid_users);
+   goto done_unlock;
+   }
+
+   if (atomic_dec_and_test(&dom->dma_pasid_users)) {
+   ioasid_free(id);
+   dom->dma_pasid = 0;
+   }
+done_unlock:
+   mutex_unlock(&dma_pasid_lock);
+   return ret;
+}
+EXPORT_SYMBOL(iommu_attach_dma_pasid);
+
+/**
+ * iommu_detach_dma_pasid --Disable in-kernel DMA request with PASID
+ * @dev:   Device's PASID DMA to be disabled
+ *
+ * It is the device driver's responsibility to ensure no more incoming DMA
+ * requests with the kernel PASID before calling this function. IOMMU driver
+ * ensures PASID cache, IOTLBs related to the kernel PASID are cleared and
+ * drained.
+ *
+ */
+void iommu_detach_dma_pasid(struct device *dev)
+{
+   struct iommu_domain *dom;
+   ioasid_t pasid;
+
+   dom = iommu_get_domain_for_dev(dev);
+   if (WARN_ON(!dom || !dom->ops || !dom->ops->detach_dev_pasid))
+   return;
+
+   /* Only support DMA API managed domain type */
+   if (WARN_ON(dom->type == IOMMU_DOMAIN_UNMANAGED ||
+   dom->type == IOMMU_DOMAIN_BLOCKED))
+   return;
+
+   mutex_lock(&dma_pasid_lock);
+   pasid = iommu_get_pasid_from_domain(dev, dom);
+   if (!pasid || pasid == INVALID_IOASID) {
+   dev_err(dev, "No valid DMA PASID attached\n");
+   mutex_unlock(&dma_pasid_lock);
+   return

[PATCH v4 0/6] Enable PASID for DMA API users

[Jacob Pan]

Some modern accelerators such as Intel's Data Streaming Accelerator (DSA)
require PASID in DMA requests to be operational. Specifically, the work
submissions with ENQCMD on shared work queues require PASIDs. The use cases
include both user DMA with shared virtual addressing (SVA) and in-kernel
DMA similar to legacy DMA w/o PASID. Here we address the latter.

DMA mapping API is the de facto standard for in-kernel DMA. However, it
operates on a per device or Requester ID(RID) basis which is not
PASID-aware. To leverage DMA API for devices relies on PASIDs, this
patchset introduces the following APIs

1. A driver facing API that enables DMA API PASID usage:
iommu_attach_dma_pasid(struct device *dev, ioasid_t &pasid);
2. VT-d driver default domain op that allows attaching device-domain-PASID

Once PASID DMA is enabled and attached to the appropriate IOMMU domain,
device drivers can continue to use DMA APIs as-is. There is no difference
in terms of mapping in dma_handle between without PASID and with PASID.
The DMA mapping performed by IOMMU will be identical for both requests, let
it be IOVA or PA in case of pass-through.

In addition, this set converts DSA driver in-kernel DMA with PASID from SVA
lib to DMA API. There have been security and functional issues with the
kernel SVA approach:
(https://lore.kernel.org/linux-iommu/20210511194726.gp1002...@nvidia.com/)
The highlights are as the following:
 - The lack of IOTLB synchronization upon kernel page table updates.
   (vmalloc, module/BPF loading, CONFIG_DEBUG_PAGEALLOC etc.)
 - Other than slight more protection, using kernel virtual address (KVA)
has little advantage over physical address. There are also no use cases yet
where DMA engines need kernel virtual addresses for in-kernel DMA.

Subsequently, cleanup is done around the usage of sva_bind_device() for
in-kernel DMA. Removing special casing code in VT-d driver and tightening
SVA lib API.

This work and idea behind it is a collaboration with many people, many
thanks to Baolu Lu, Jason Gunthorpe, Dave Jiang, and others.


ChangeLog:
v4
- Rebased on "Baolu's SVA and IOPF refactoring" series v6.

(https://github.com/LuBaolu/intel-iommu/commits/iommu-sva-refactoring-v6)
- Fixed locking for protecting iommu domain PASID data
- Use iommu_attach_device_pasid() API instead of calling domain
  ops directly. This will leverage the common pasid_array that
  replaces driver specific storage in device_domain_info.
- Added a helper function to do look up in pasid_array from
  domain

v3
- Rebased on "Baolu's SVA and IOPF refactoring" series v5.

(https://github.com/LuBaolu/intel-iommu/commits/iommu-sva-refactoring-v5)
This version is significantly simplified by leveraging IOMMU domain
ops, attach_dev_pasid() op is implemented differently on a DMA domain
than on a SVA domain.
We currently have no need to support multiple PASIDs per DMA domain.
(https://lore.kernel.org/lkml/20220315142216.gv11...@nvidia.com/).
Removed PASID-device list from V2, a PASID field is introduced to
struct iommu_domain instead. It is intended for DMA requests with
PASID by all devices attached to the domain.

v2
- Do not reserve a special PASID for DMA API usage. Use IOASID
  allocation instead.
- Introduced a generic device-pasid-domain attachment IOMMU op.
  Replaced the DMA API only IOMMU op.
- Removed supervisor SVA support in VT-d
- Removed unused sva_bind_device parameters
- Use IOMMU specific data instead of struct device to store PASID
  info




*** SUBJECT HERE ***

*** BLURB HERE ***

Jacob Pan (6):
  iommu: Add a per domain PASID for DMA API
  iommu: Add a helper to do PASID lookup from domain
  iommu/vt-d: Implement domain ops for attach_dev_pasid
  iommu: Add PASID support for DMA mapping API users
  dmaengine: idxd: Use DMA API for in-kernel DMA with PASID
  iommu/vt-d: Delete unused SVM flag

 drivers/dma/idxd/idxd.h |   1 -
 drivers/dma/idxd/init.c |  34 +++
 drivers/dma/idxd/sysfs.c|   7 ---
 drivers/iommu/dma-iommu.c   | 114 
 drivers/iommu/intel/iommu.c |  72 ++-
 drivers/iommu/intel/svm.c   |   2 +-
 drivers/iommu/iommu.c   |  22 +++
 include/linux/dma-iommu.h   |   3 +
 include/linux/intel-svm.h   |  13 
 include/linux/iommu.h   |   8 ++-
 10 files changed, 226 insertions(+), 50 deletions(-)

-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v4 1/6] iommu: Add a per domain PASID for DMA API

[Jacob Pan]

DMA requests tagged with PASID can target individual IOMMU domains.
Introduce a domain-wide PASID for DMA API, it will be used on the same
mapping as legacy DMA without PASID. Let it be IOVA or PA in case of
identity domain.

Signed-off-by: Jacob Pan 
---
 include/linux/iommu.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 9405034e3013..36ad007084cc 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -106,6 +106,8 @@ struct iommu_domain {
enum iommu_page_response_code (*iopf_handler)(struct iommu_fault *fault,
  void *data);
void *fault_data;
+   ioasid_t dma_pasid; /* Used for DMA requests with PASID */
+   atomic_t dma_pasid_users;
 };
 
 static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v4 3/6] iommu/vt-d: Implement domain ops for attach_dev_pasid

[Jacob Pan]

On VT-d platforms with scalable mode enabled, devices issue DMA requests
with PASID need to attach PASIDs to given IOMMU domains. The attach
operation involves the following:
- Programming the PASID into the device's PASID table
- Tracking device domain and the PASID relationship
- Managing IOTLB and device TLB invalidations

This patch add attach_dev_pasid functions to the default domain ops which
is used by DMA and identity domain types. It could be extended to support
other domain types whenever necessary.

Signed-off-by: Lu Baolu 
Signed-off-by: Jacob Pan 
---
 drivers/iommu/intel/iommu.c | 72 +++--
 1 file changed, 70 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 1c2c92b657c7..75615c105fdf 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1556,12 +1556,18 @@ static void __iommu_flush_dev_iotlb(struct 
device_domain_info *info,
u64 addr, unsigned int mask)
 {
u16 sid, qdep;
+   ioasid_t pasid;
 
if (!info || !info->ats_enabled)
return;
 
sid = info->bus << 8 | info->devfn;
qdep = info->ats_qdep;
+   pasid = iommu_get_pasid_from_domain(info->dev, &info->domain->domain);
+   if (pasid != INVALID_IOASID) {
+   qi_flush_dev_iotlb_pasid(info->iommu, sid, info->pfsid,
+pasid, qdep, addr, mask);
+   }
qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
   qdep, addr, mask);
 }
@@ -1591,6 +1597,7 @@ static void iommu_flush_iotlb_psi(struct intel_iommu 
*iommu,
unsigned int mask = ilog2(aligned_pages);
uint64_t addr = (uint64_t)pfn << VTD_PAGE_SHIFT;
u16 did = domain->iommu_did[iommu->seq_id];
+   struct iommu_domain *iommu_domain = &domain->domain;
 
BUG_ON(pages == 0);
 
@@ -1599,6 +1606,9 @@ static void iommu_flush_iotlb_psi(struct intel_iommu 
*iommu,
 
if (domain_use_first_level(domain)) {
qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr, pages, ih);
+   /* flush additional kernel DMA PASIDs attached */
+   if (iommu_domain->dma_pasid)
+   qi_flush_piotlb(iommu, did, iommu_domain->dma_pasid, 
addr, pages, ih);
} else {
unsigned long bitmask = aligned_pages - 1;
 
@@ -4255,6 +4265,7 @@ static void __dmar_remove_one_dev_info(struct 
device_domain_info *info)
struct dmar_domain *domain;
struct intel_iommu *iommu;
unsigned long flags;
+   ioasid_t pasid;
 
assert_spin_locked(&device_domain_lock);
 
@@ -4265,10 +4276,15 @@ static void __dmar_remove_one_dev_info(struct 
device_domain_info *info)
domain = info->domain;
 
if (info->dev && !dev_is_real_dma_subdevice(info->dev)) {
-   if (dev_is_pci(info->dev) && sm_supported(iommu))
+   if (dev_is_pci(info->dev) && sm_supported(iommu)) {
intel_pasid_tear_down_entry(iommu, info->dev,
PASID_RID2PASID, false);
-
+   pasid = iommu_get_pasid_from_domain(info->dev,
+   
&info->domain->domain);
+   if (pasid != INVALID_IOASID)
+   intel_pasid_tear_down_entry(iommu, info->dev,
+   pasid, false);
+   }
iommu_disable_dev_iotlb(info);
domain_context_clear(info);
intel_pasid_free_table(info->dev);
@@ -4904,6 +4920,56 @@ static void intel_iommu_iotlb_sync_map(struct 
iommu_domain *domain,
}
 }
 
+static int intel_iommu_attach_dev_pasid(struct iommu_domain *domain,
+   struct device *dev,
+   ioasid_t pasid)
+{
+   struct device_domain_info *info = dev_iommu_priv_get(dev);
+   struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+   struct intel_iommu *iommu = info->iommu;
+   unsigned long flags;
+   int ret = 0;
+
+   if (!sm_supported(iommu) || !info)
+   return -ENODEV;
+
+   if (WARN_ON(pasid == PASID_RID2PASID))
+   return -EINVAL;
+
+   spin_lock_irqsave(&device_domain_lock, flags);
+   spin_lock(&iommu->lock);
+   if (hw_pass_through && domain_type_is_si(dmar_domain))
+   ret = intel_pasid_setup_pass_through(iommu, dmar_domain,
+dev, pasid);
+   else if (domain_use_first_level(dmar_domain))
+   ret = domain_setup_first_level(iommu, dmar_domain,
+  dev, pasid);
+   else
+   ret = intel_pasid_setup_second_level(iommu, dmar_domain,
+dev, pas

[PATCH] iommu/dma: Add config for PCI SAC address trick

[Robin Murphy]

For devices stuck behind a conventional PCI bus, saving extra cycles at
33MHz is probably fairly significant. However since native PCI Express
is now the norm for high-performance devices, the optimisation to always
prefer 32-bit addresses for the sake of avoiding DAC is starting to look
rather anachronistic. Technically 32-bit addresses do have shorter TLPs
on PCIe, but unless the device is saturating its link bandwidth with
small transfers it seems unlikely that the difference is appreciable.

What definitely is appreciable, however, is that the IOVA allocator
doesn't behave all that well once the 32-bit space starts getting full.
As DMA working sets get bigger, this optimisation increasingly backfires
and adds considerable overhead to the dma_map path for use-cases like
high-bandwidth networking. We've increasingly bandaged the allocator
in attempts to mitigate this, but it remains fundamentally at odds with
other valid requirements to try as hard as possible to satisfy a request
within the given limit; what we really need is to just avoid this odd
notion of a speculative allocation when it isn't beneficial anyway.

Unfortunately that's where things get awkward... Having been present on
x86 for 15 years or so now, it turns out there are systems which fail to
properly define the upper limit of usable IOVA space for certain devices
and this trick was the only thing letting them work OK. I had a similar
ulterior motive for a couple of early arm64 systems when originally
adding it to iommu-dma, but those really should now be fixed with proper
firmware bindings, and other arm64 users really need it out of the way,
so let's just leave it default-on for x86.

Signed-off-by: Robin Murphy 
---
 drivers/iommu/Kconfig | 24 
 drivers/iommu/dma-iommu.c |  2 +-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index c79a0df090c0..bf9b295f1c89 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -144,6 +144,30 @@ config IOMMU_DMA
select IRQ_MSI_IOMMU
select NEED_SG_DMA_LENGTH
 
+config IOMMU_DMA_PCI_SAC_OPT
+   bool "Enable 64-bit legacy PCI optimisation by default"
+   depends on IOMMU_DMA
+   default X86
+   help
+ Enable by default an IOMMU optimisation for 64-bit legacy PCI devices,
+ wherein the DMA API layer will always first try to allocate a 32-bit
+ DMA address suitable for a single address cycle, before falling back
+ to allocating from the full usable address range. If your system has
+ 64-bit legacy PCI devices in 32-bit slots where using dual address
+ cycles reduces DMA throughput significantly, this optimisation may be
+ beneficial to overall performance.
+
+ If you have a modern PCI Express based system, it should usually be
+ safe to say "n" here and avoid the potential extra allocation 
overhead.
+ However, beware that this optimisation has also historically papered
+ over bugs where the IOMMU address range above 32 bits is not fully
+ usable. If device DMA problems and/or IOMMU faults start occurring
+ with IOMMU translation enabled after disabling this option, it is
+ likely a sign of a latent driver or firmware/BIOS bug.
+
+ If this option is not set, the optimisation can be enabled at
+ boot time with the "iommu.forcedac=0" command-line argument.
+
 # Shared Virtual Addressing
 config IOMMU_SVA
bool
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 09f6e1c0f9c0..c8d409d3f861 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -66,7 +66,7 @@ struct iommu_dma_cookie {
 };
 
 static DEFINE_STATIC_KEY_FALSE(iommu_deferred_attach_enabled);
-bool iommu_dma_forcedac __read_mostly;
+bool iommu_dma_forcedac __read_mostly = 
!IS_ENABLED(CONFIG_IOMMU_DMA_PCI_SAC_OPT);
 
 static int __init iommu_dma_forcedac_setup(char *str)
 {
-- 
2.35.3.dirty

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v5 0/5] iommu: Support mappings/reservations in reserved-memory regions

[Thierry Reding]

On Sun, May 15, 2022 at 12:35:44PM +0200, Janne Grunau wrote:
> Hej,
> 
> I'm working on the display controller for Apple silicon SoCs and will 
> add some comments with support for it in mind.
> 
> added as...@lists.linux.dev to CC for the Apple silicon related aspects
> 
> On 2022-05-12 21:00:47 +0200, Thierry Reding wrote:
> > 
> > this is another attempt at solving the problem of passing IOMMU
> > configuration via device tree. It has significantly evolved since the
> > last attempt, based on the discussion that followed. The discussion can
> > be found here:
> > 
> >   
> > https://lore.kernel.org/all/20210423163234.3651547-1-thierry.red...@gmail.com/
> > 
> > Rather than using a memory-region specifier, this new version introduces
> > a new "iommu-addresses" property for the reserved-memory regions
> > themselves.
> 
> If experimented with both proposed bindings for dcp and I think this 
> binding is easer to understand and to work with.
> 
> > These are used to describe either a static mapping or
> > reservation that should be created for a given device. If both "reg" and
> > "iommu-addresses" properties are given, a mapping will be created
> > (typically this would be an identity mapping)
> 
> dcp on Apple silicon will not use identity mappings. The IOMMU supports 
> identity mapping but the pre-configured mappings setup by Apple's system 
> firmware will not work with identity mapping. It maps multiple regions 
> which are incompatible with a linear identity mapping. In addition the 
> embbeded aarch64 micro controllers used in the display subsystem appears 
> to use a heap mapped at low IOVA space starting at 0.
> 
> > whereas if only an "iommu-addresses" property is specified, a 
> > reservation for the specified range will be installed.
> > 
> > An example is included in the DT bindings, but here is an extract of
> > what I've used to test this:
> > 
> > reserved-memory {
> > #address-cells = <2>;
> > #size-cells = <2>;
> > ranges;
> > 
> > /*
> >  * Creates an identity mapping for the framebuffer that
> >  * the firmware has setup to scan out a bootsplash from.
> >  */
> > fb: framebuffer@92cb2000 {
> > reg = <0x0 0x92cb2000 0x0 0x0080>;
> > iommu-addresses = <&dc0 0x0 0x92cb2000 0x0 0x0080>;
> > };
> 
> The binding supports mapping the same region to multiple devices. The 
> code supports that and it will be used on Apple silicon. Not necessary 
> to extend and complicate the example for I wanted to mention it 
> explicitly.
> 
> > 
> > /*
> >  * Creates a reservation in the IOVA space to prevent
> >  * any buffers from being mapped to that region. Note
> >  * that on Tegra the range is actually quite different
> >  * from this, but it would conflict with the display
> >  * driver that I tested this against, so this is just
> >  * a dummy region for testing.
> >  */
> > adsp: reservation-adsp {
> > iommu-addresses = <&dc0 0x0 0x9000 0x0 0x0001>;
> > };
> > };
> > 
> > host1x@5000 {
> > dc@5420 {
> > memory-region = <&fb>, <&adsp>;
> > };
> > };
> > 
> > This is abbreviated a little to focus on the essentials. Note also that
> > the ADSP reservation is not actually used on this device and the driver
> > for this doesn't exist yet, but I wanted to include this variant for
> > testing, because we'll want to use these bindings for the reservation
> > use-case as well at some point.
> > 
> > Adding Alyssa and Janne who have in the past tried to make these
> > bindings work on Apple M1. Also adding Sameer from the Tegra audio team
> > to look at the ADSP reservation and double-check that this is suitable
> > for our needs.
> 
> The binding itself is sufficient for the needs of the display subsystem 
> on Apple silicon. The device tree parsing code for reserved regions is 
> of limited use in it's current form. We will have either to extend or 
> duplicate it to retrieve the non-identity mappings. That's our problem 
> to solve.

I had looked at it a bit to see if I could easily implement that, but
the direct mapping support in the IOMMU subsystem currently only
supports either reservations or identity mappings, so arbitrary mappings
would either have to be added to that code, or it would have to take a
different code path that basically goes through the same steps, except
that it uses different physical and I/O virtual addresses.

The easiest, I think, would be for struct iommu_resv_region to be
extended with a pair of start/length fields for the I/O virtual address
and then the rest of the code should mostly work. This shouldn't even be
very invasive, maybe just adding a version of iommu_alloc_resv_region()
that takes the I/O virtua

Re: [RFC PATCH] dma-iommu: Add iommu_dma_max_mapping_size()

[Robin Murphy]

On 2022-05-18 14:12, Christoph Hellwig wrote:

On Tue, May 17, 2022 at 11:40:52AM +0100, Robin Murphy wrote:

Indeed, sorry but NAK for this being nonsense. As I've said at least once
before, if the unnecessary SAC address allocation attempt slows down your
workload, make it not do that in the first place. If you don't like the
existing command-line parameter then fine, there are plenty of other
options, it just needs to be done in a way that doesn't break x86 systems
with dodgy firmware, as my first attempt turned out to.


What broke x86?


See the thread at [1] (and in case of curiosity the other IVRS patches I 
refer to therein were at [2]). Basically, undescribed limitations lead 
to DMA address truncation once iommu-dma starts allocating from what it 
thinks is the full usable IOVA range. Your typical desktop PC is 
unlikely to have enough concurrent DMA-mapped memory to overflow the 
32-bit IOVA space naturally, so this has probably been hiding an untold 
multitude of sins over the years.


Robin.

[1] 
https://lore.kernel.org/linux-iommu/e583fc6dd1fb4ffc90310ff4372ee776f9cc7a3c.1594207679.git.robin.mur...@arm.com/
[2] 
https://lore.kernel.org/linux-iommu/20200605145655.13639-1-seb...@amazon.de/

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH] dma-iommu: Add iommu_dma_max_mapping_size()

[Christoph Hellwig]

On Tue, May 17, 2022 at 01:02:00PM +0100, Robin Murphy wrote:
>> So how to inform the SCSI driver of this caching limit then so that it may 
>> limit the SGL length?
>
> Driver-specific mechanism; block-layer-specific mechanism; redefine this 
> whole API to something like dma_opt_mapping_size(), as a limit above which 
> mappings might become less efficient or start to fail (callback to my 
> thoughts on [1] as well, I suppose); many options. Just not imposing a 
> ridiculously low *maximum* on everyone wherein mapping calls "should not be 
> larger than the returned value" when that's clearly bollocks.

Well, for swiotlb it is a hard limit.  So if we want to go down that
route we need two APIs, one for the optimal size and one for the
hard limit.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH] dma-iommu: Add iommu_dma_max_mapping_size()

[Christoph Hellwig]

On Tue, May 17, 2022 at 11:40:52AM +0100, Robin Murphy wrote:
> Indeed, sorry but NAK for this being nonsense. As I've said at least once 
> before, if the unnecessary SAC address allocation attempt slows down your 
> workload, make it not do that in the first place. If you don't like the 
> existing command-line parameter then fine, there are plenty of other 
> options, it just needs to be done in a way that doesn't break x86 systems 
> with dodgy firmware, as my first attempt turned out to.

What broke x86?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v5 1/5] dt-bindings: reserved-memory: Document iommu-addresses

[Thierry Reding]

On Sun, May 15, 2022 at 12:45:54PM +0200, Janne Grunau wrote:
> On 2022-05-12 21:00:48 +0200, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > This adds the "iommu-addresses" property to reserved-memory nodes, which
> > allow describing the interaction of memory regions with IOMMUs. Two use-
> > cases are supported:
> > 
> >   1. Static mappings can be described by pairing the "iommu-addresses"
> >  property with a "reg" property. This is mostly useful for adopting
> >  firmware-allocated buffers via identity mappings. One common use-
> >  case where this is required is if early firmware or bootloaders
> >  have set up a bootsplash framebuffer that a display controller is
> >  actively scanning out from during the operating system boot
> >  process.
> > 
> >   2. If an "iommu-addresses" property exists without a "reg" property,
> >  the reserved-memory node describes an IOVA reservation. Such memory
> >  regions are excluded from the IOVA space available to operating
> >  system drivers and can be used for regions that must not be used to
> >  map arbitrary buffers.
> > 
> > Each mapping or reservation is tied to a specific device via a phandle
> > to the device's device tree node. This allows a reserved-memory region
> > to be reused across multiple devices.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> >  .../reserved-memory/reserved-memory.txt   |  1 -
> >  .../reserved-memory/reserved-memory.yaml  | 62 +++
> >  include/dt-bindings/reserved-memory.h |  8 +++
> >  3 files changed, 70 insertions(+), 1 deletion(-)
> >  delete mode 100644 
> > Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> >  create mode 100644 include/dt-bindings/reserved-memory.h
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
> > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > deleted file mode 100644
> > index 1810701a8509..
> > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > +++ /dev/null
> > @@ -1 +0,0 @@
> > -This file has been moved to reserved-memory.yaml.
> > diff --git 
> > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml 
> > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> > index 7a0744052ff6..3a769aa66e1c 100644
> > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> > @@ -52,6 +52,30 @@ properties:
> >Address and Length pairs. Specifies regions of memory that are
> >acceptable to allocate from.
> >  
> > +  iommu-addresses:
> > +$ref: /schemas/types.yaml#/definitions/phandle-array
> > +description: >
> > +  A list of phandle and specifier pairs that describe static IO virtual
> > +  address space mappings and carveouts associated with a given reserved
> > +  memory region. The phandle in the first cell refers to the device for
> > +  which the mapping or carveout is to be created.
> > +
> > +  The specifier consists of an address/size pair and denotes the IO
> > +  virtual address range of the region for the given device. The exact
> > +  format depends on the values of the "#address-cells" and 
> > "#size-cells"
> > +  properties of the device referenced via the phandle.
> > +
> > +  When used in combination with a "reg" property, an IOVA mapping is to
> > +  be established for this memory region. One example where this can be
> > +  useful is to create an identity mapping for physical memory that the
> > +  firmware has configured some hardware to access (such as a bootsplash
> > +  framebuffer).
> > +
> > +  If no "reg" property is specified, the "iommu-addresses" property
> > +  defines carveout regions in the IOVA space for the given device. This
> > +  can be useful if a certain memory region should not be mapped through
> > +  the IOMMU.
> > +
> >no-map:
> >  type: boolean
> >  description: >
> > @@ -97,4 +121,42 @@ oneOf:
> >  
> >  additionalProperties: true
> >  
> > +examples:
> > +  - |
> > +reserved-memory {
> > +  #address-cells = <2>;
> > +  #size-cells = <2>;
> > +  ranges;
> > +
> > +  adsp: reservation-adsp {
> > +/*
> > + * Restrict IOVA mappings for ADSP buffers to the 512 MiB region
> > + * from 0x4000 - 0x5fff. Anything outside is reserved by
> > + * the ADSP for I/O memory and private memory allocations.
> > + */
> > +iommu-addresses = <0x0 0x 0x00 0x4000>,
> > +  <0x0 0x6000 0xff 0xa000>;
> 
> This misses the device's phandle. One could argue it's not necessary for 
> reservations but it will complicate the parsing code and the current 
> parsing code is not prepared for it.

Ugh... I evidently messe

Re: [PATCH v5 2/5] iommu: Implement of_iommu_get_resv_regions()

[Thierry Reding]

On Sun, May 15, 2022 at 01:10:38PM +0200, Janne Grunau wrote:
> On 2022-05-12 21:00:49 +0200, Thierry Reding wrote:
> > From: Thierry Reding 
> > 
> > This is an implementation that IOMMU drivers can use to obtain reserved
> > memory regions from a device tree node. It uses the reserved-memory DT
> > bindings to find the regions associated with a given device. If these
> > regions are marked accordingly, identity mappings will be created for
> > them in the IOMMU domain that the devices will be attached to.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> > Changes in v5:
> > - update for new "iommu-addresses" device tree bindings
> > 
> > Changes in v4:
> > - fix build failure on !CONFIG_OF_ADDRESS
> > 
> > Changes in v3:
> > - change "active" property to identity mapping flag that is part of the
> >   memory region specifier (as defined by #memory-region-cells) to allow
> >   per-reference flags to be used
> > 
> > Changes in v2:
> > - use "active" property to determine whether direct mappings are needed
> > 
> >  drivers/iommu/of_iommu.c | 90 
> >  include/linux/of_iommu.h |  8 
> >  2 files changed, 98 insertions(+)
> > 
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 5696314ae69e..9e341b5e307f 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -11,12 +11,15 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> >  #include 
> >  
> > +#include 
> > +
> >  #define NO_IOMMU   1
> >  
> >  static int of_iommu_xlate(struct device *dev,
> > @@ -172,3 +175,90 @@ const struct iommu_ops *of_iommu_configure(struct 
> > device *dev,
> >  
> > return ops;
> >  }
> > +
> > +/**
> > + * of_iommu_get_resv_regions - reserved region driver helper for device 
> > tree
> > + * @dev: device for which to get reserved regions
> > + * @list: reserved region list
> > + *
> > + * IOMMU drivers can use this to implement their .get_resv_regions() 
> > callback
> > + * for memory regions attached to a device tree node. See the 
> > reserved-memory
> > + * device tree bindings on how to use these:
> > + *
> > + *   Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > + */
> > +void of_iommu_get_resv_regions(struct device *dev, struct list_head *list)
> > +{
> > +#if IS_ENABLED(CONFIG_OF_ADDRESS)
> > +   struct of_phandle_iterator it;
> > +   int err;
> > +
> > +   of_for_each_phandle(&it, err, dev->of_node, "memory-region", NULL, 0) {
> > +   struct iommu_resv_region *region;
> > +   struct resource res;
> > +   const __be32 *maps;
> > +   int size;
> 
> Adding 'if (!of_device_is_available(it.node)) continue;' here would help 
> backwards compatibility. My plan was to add the reserved regions with 
> "iommu-addresses" with all zero adresses and sizes with status = 
> "disabled" to the devicetree. A bootloader update is required to fill 
> those.

Yes, good point. My plan was originally to have the bootloader/firmware
generate these nodes in their entirety, but yeah, prepopulating them and
having firmware just fill in updated values and setting status = "okay"
seems reasonable to me.

> > +
> > +   memset(&res, 0, sizeof(res));
> > +
> > +   /*
> > +* The "reg" property is optional and can be omitted by 
> > reserved-memory regions
> > +* that represent reservations in the IOVA space, which are 
> > regions that should
> > +* not be mapped.
> > +*/
> > +   if (of_find_property(it.node, "reg", NULL)) {
> > +   err = of_address_to_resource(it.node, 0, &res);
> > +   if (err < 0) {
> > +   dev_err(dev, "failed to parse memory region 
> > %pOF: %d\n",
> > +   it.node, err);
> > +   continue;
> > +   }
> > +   }
> > +
> > +   maps = of_get_property(it.node, "iommu-addresses", &size);
> > +   if (maps) {
> > +   const __be32 *end = maps + size / sizeof(__be32);
> > +   struct device_node *np;
> > +   unsigned int index = 0;
> > +   u32 phandle;
> > +   int na, ns;
> > +
> > +   while (maps < end) {
> > +   phys_addr_t start, end;
> > +   size_t length;
> > +
> > +   phandle = be32_to_cpup(maps++);
> > +   np = of_find_node_by_phandle(phandle);
> > +   na = of_n_addr_cells(np);
> > +   ns = of_n_size_cells(np);
> > +
> > +   start = of_translate_dma_address(np, maps);
> > +   length = of_read_number(maps + na, ns);
> 
> alternatively we could handle mappings/reservations with length 0 as 
> error and sk

Re: [PATCH v2 1/7] dt-bindings: iommu: mediatek: Add phandles for mediatek infra/pericfg

[AngeloGioacchino Del Regno]

Il 18/05/22 13:29, Matthias Brugger ha scritto:



On 18/05/2022 12:04, AngeloGioacchino Del Regno wrote:

Add properties "mediatek,infracfg" and "mediatek,pericfg" to let the
mtk_iommu driver retrieve phandles to the infracfg and pericfg syscon(s)
instead of performing a per-soc compatible lookup.

Signed-off-by: AngeloGioacchino Del Regno 

---
  .../devicetree/bindings/iommu/mediatek,iommu.yaml | 8 
  1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml 
b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml

index 2ae3bbad7f1a..c4af41947593 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -101,6 +101,10 @@ properties:
  items:
    - const: bclk
+  mediatek,infracfg:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: The phandle to the mediatek infracfg syscon
+
    mediatek,larbs:
  $ref: /schemas/types.yaml#/definitions/phandle-array
  minItems: 1
@@ -112,6 +116,10 @@ properties:
    Refer to bindings/memory-controllers/mediatek,smi-larb.yaml. It must 
sort
    according to the local arbiter index, like larb0, larb1, larb2...
+  mediatek,pericfg:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: The phandle to the mediatek pericfg syscon
+


I didn't explain myself. What I was suguesting was to squash the patch that add 
requiered mediatek,infracfg with the patch that adds mediatk,infracfg to the 
binding description. And then squash the both patches adding pericfg as well.


Sorry Matthias, I'm not sure ... I think I'm misunderstanding you again...
...but if I'm not, I don't think that squashing actual code and bindings 
together
is something acceptable?

I've made that kind of mistake in the past and I was told multiple times that
dt-bindings changes shall be sent separately from the actual driver changes.

Cheers,
Angelo



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 1/7] dt-bindings: iommu: mediatek: Add phandles for mediatek infra/pericfg

[Matthias Brugger]

On 18/05/2022 12:04, AngeloGioacchino Del Regno wrote:

Add properties "mediatek,infracfg" and "mediatek,pericfg" to let the
mtk_iommu driver retrieve phandles to the infracfg and pericfg syscon(s)
instead of performing a per-soc compatible lookup.

Signed-off-by: AngeloGioacchino Del Regno 

---
  .../devicetree/bindings/iommu/mediatek,iommu.yaml | 8 
  1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml 
b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index 2ae3bbad7f1a..c4af41947593 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -101,6 +101,10 @@ properties:
  items:
- const: bclk
  
+  mediatek,infracfg:

+$ref: /schemas/types.yaml#/definitions/phandle
+description: The phandle to the mediatek infracfg syscon
+
mediatek,larbs:
  $ref: /schemas/types.yaml#/definitions/phandle-array
  minItems: 1
@@ -112,6 +116,10 @@ properties:
Refer to bindings/memory-controllers/mediatek,smi-larb.yaml. It must 
sort
according to the local arbiter index, like larb0, larb1, larb2...
  
+  mediatek,pericfg:

+$ref: /schemas/types.yaml#/definitions/phandle
+description: The phandle to the mediatek pericfg syscon
+


I didn't explain myself. What I was suguesting was to squash the patch that add 
requiered mediatek,infracfg with the patch that adds mediatk,infracfg to the 
binding description. And then squash the both patches adding pericfg as well.


Regards,
Matthias



'#iommu-cells':
  const: 1
  description: |

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg

[Robin Murphy]

On 2022-05-18 09:29, AngeloGioacchino Del Regno wrote:

Il 17/05/22 16:12, Robin Murphy ha scritto:

On 2022-05-17 14:21, AngeloGioacchino Del Regno wrote:

This driver will get support for more SoCs and the list of infracfg
compatibles is expected to grow: in order to prevent getting this
situation out of control and see a long list of compatible strings,
add support to retrieve a handle to infracfg's regmap through a
new "mediatek,infracfg" phandle.

In order to keep retrocompatibility with older devicetrees, the old
way is kept in place, but also a dev_warn() was added to advertise
this change in hope that the user will see it and eventually update
the devicetree if this is possible.

Signed-off-by: AngeloGioacchino Del Regno 


---
  drivers/iommu/mtk_iommu.c | 40 +--
  1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 71b2ace74cd6..cfaaa98d2b50 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1134,22 +1134,34 @@ static int mtk_iommu_probe(struct 
platform_device *pdev)
  data->protect_base = ALIGN(virt_to_phys(protect), 
MTK_PROTECT_PA_ALIGN);

  if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
-    switch (data->plat_data->m4u_plat) {
-    case M4U_MT2712:
-    p = "mediatek,mt2712-infracfg";
-    break;
-    case M4U_MT8173:
-    p = "mediatek,mt8173-infracfg";
-    break;
-    default:
-    p = NULL;
+    infracfg = syscon_regmap_lookup_by_phandle(dev->of_node, 
"mediatek,infracfg");

+    if (IS_ERR(infracfg)) {
+    dev_warn(dev, "Cannot find phandle to mediatek,infracfg:"
+  " Please update your devicetree.\n");


Is this really a dev_warn-level problem? There's no functional impact, 
given that we can't stop supporting the original binding any time 
soon, if ever, so I suspect this is more likely to just annoy users 
and CI systems than effect any significant change.




The upstream devicetrees were updated to use the new handle and this is 
a way to
warn about having outdated DTs... besides, I believe that CIs will 
always get the
devicetree from the same tree that the kernel was compiled from (hence 
no message

will be thrown).

In any case, if you think that a dev_info would be more appropriate, I 
can change

that no problem.


If there's some functional impact from using the old binding vs. the new 
one then it's reasonable to inform the user of that (as we do in 
arm-smmu, for example).


However if you change an established binding for non-functional reasons, 
then you get to support both bindings, and it's not the end user's 
problem at all. There seems to be zero reason to update an existing DTB 
for this difference alone, so TBH I don't think it deserves a message at 
all.



+    /*
+ * Legacy devicetrees will not specify a phandle to
+ * mediatek,infracfg: in that case, we use the older
+ * way to retrieve a syscon to infra.
+ *
+ * This is for retrocompatibility purposes only, hence
+ * no more compatibles shall be added to this.
+ */
+    switch (data->plat_data->m4u_plat) {
+    case M4U_MT2712:
+    p = "mediatek,mt2712-infracfg";
+    break;
+    case M4U_MT8173:
+    p = "mediatek,mt8173-infracfg";
+    break;
+    default:
+    p = NULL;
+    }
+
+    infracfg = syscon_regmap_lookup_by_compatible(p);


Would it not make sense to punt this over to the same mechanism as for 
pericfg, such that it simplifies down to something like:


 if (IS_ERR(infracfg) && plat_data->infracfg) {
 infracfg = 
syscon_regmap_lookup_by_compatible(plat_data->infracfg);

 ...
 }

?

TBH if we're still going to have a load of per-SoC data in the driver 
anyway then I don't see that we really gain much by delegating one 
aspect of it to DT, but meh. I would note that with the phandle 
approach, you still need some *other* flag in the driver to know 
whether a phandle is expected to be present or not, whereas a NULL vs. 
non-NULL string is at least neatly self-describing.




That would be possible but, as Yong also pointed out, we should try to 
reduce the
per-SoC data in the driver by commonizing as much as possible, because 
this driver
supports a very long list of SoCs (even though they're not all 
upstreamed yet),
and the list is going to grow even more with time: this is also why I 
have changed

the MT8195 pericfg regmap lookup with a phandle like I've done for infra.


That's fair enough, but it's not what the commit message says. The "long 
list of compatible strings" complaint could be addressed at face value 
by refactoring without changing the DT binding at all.


However, I didn't think I'd have to point out why that argument d

[PATCH v2 1/2] dt-bindings: mediatek: Add bindings for MT6795 M4U

[AngeloGioacchino Del Regno]

Add bindings for the MediaTek Helio X10 (MT6795) IOMMU/M4U.

Signed-off-by: AngeloGioacchino Del Regno 

---
 .../bindings/iommu/mediatek,iommu.yaml|  4 +
 include/dt-bindings/memory/mt6795-larb-port.h | 96 +++
 2 files changed, 100 insertions(+)
 create mode 100644 include/dt-bindings/memory/mt6795-larb-port.h

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml 
b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index d5e3272a54e8..20902c387520 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -73,6 +73,7 @@ properties:
   - mediatek,mt2701-m4u  # generation one
   - mediatek,mt2712-m4u  # generation two
   - mediatek,mt6779-m4u  # generation two
+  - mediatek,mt6795-m4u  # generation two
   - mediatek,mt8167-m4u  # generation two
   - mediatek,mt8173-m4u  # generation two
   - mediatek,mt8183-m4u  # generation two
@@ -128,6 +129,7 @@ properties:
   dt-binding/memory/mt2701-larb-port.h for mt2701 and mt7623,
   dt-binding/memory/mt2712-larb-port.h for mt2712,
   dt-binding/memory/mt6779-larb-port.h for mt6779,
+  dt-binding/memory/mt6795-larb-port.h for mt6795,
   dt-binding/memory/mt8167-larb-port.h for mt8167,
   dt-binding/memory/mt8173-larb-port.h for mt8173,
   dt-binding/memory/mt8183-larb-port.h for mt8183,
@@ -152,6 +154,7 @@ allOf:
 enum:
   - mediatek,mt2701-m4u
   - mediatek,mt2712-m4u
+  - mediatek,mt6795-m4u
   - mediatek,mt8173-m4u
   - mediatek,mt8186-iommu-mm
   - mediatek,mt8192-m4u
@@ -181,6 +184,7 @@ allOf:
   contains:
 enum:
   - mediatek,mt2712-m4u
+  - mediatek,mt6795-m4u
   - mediatek,mt8173-m4u
 
 then:
diff --git a/include/dt-bindings/memory/mt6795-larb-port.h 
b/include/dt-bindings/memory/mt6795-larb-port.h
new file mode 100644
index ..2243bb6414f3
--- /dev/null
+++ b/include/dt-bindings/memory/mt6795-larb-port.h
@@ -0,0 +1,96 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2022 Collabora Ltd.
+ * Author: AngeloGioacchino Del Regno 
+ */
+
+#ifndef _DT_BINDINGS_MEMORY_MT6795_LARB_PORT_H_
+#define _DT_BINDINGS_MEMORY_MT6795_LARB_PORT_H_
+
+#include 
+
+#define M4U_LARB0_ID   0
+#define M4U_LARB1_ID   1
+#define M4U_LARB2_ID   2
+#define M4U_LARB3_ID   3
+#define M4U_LARB4_ID   4
+#define M4U_LARB5_ID   5
+
+/* larb0 */
+#define M4U_PORT_DISP_OVL0 MTK_M4U_ID(M4U_LARB0_ID, 0)
+#define M4U_PORT_DISP_RDMA0MTK_M4U_ID(M4U_LARB0_ID, 1)
+#define M4U_PORT_DISP_RDMA1MTK_M4U_ID(M4U_LARB0_ID, 2)
+#define M4U_PORT_DISP_WDMA0MTK_M4U_ID(M4U_LARB0_ID, 3)
+#define M4U_PORT_DISP_OVL1 MTK_M4U_ID(M4U_LARB0_ID, 4)
+#define M4U_PORT_DISP_RDMA2MTK_M4U_ID(M4U_LARB0_ID, 5)
+#define M4U_PORT_DISP_WDMA1MTK_M4U_ID(M4U_LARB0_ID, 6)
+#define M4U_PORT_DISP_OD_R MTK_M4U_ID(M4U_LARB0_ID, 7)
+#define M4U_PORT_DISP_OD_W MTK_M4U_ID(M4U_LARB0_ID, 8)
+#define M4U_PORT_MDP_RDMA0 MTK_M4U_ID(M4U_LARB0_ID, 9)
+#define M4U_PORT_MDP_RDMA1 MTK_M4U_ID(M4U_LARB0_ID, 10)
+#define M4U_PORT_MDP_WDMA  MTK_M4U_ID(M4U_LARB0_ID, 11)
+#define M4U_PORT_MDP_WROT0 MTK_M4U_ID(M4U_LARB0_ID, 12)
+#define M4U_PORT_MDP_WROT1 MTK_M4U_ID(M4U_LARB0_ID, 13)
+
+/* larb1 */
+#define M4U_PORT_VDEC_MC   MTK_M4U_ID(M4U_LARB1_ID, 0)
+#define M4U_PORT_VDEC_PP   MTK_M4U_ID(M4U_LARB1_ID, 1)
+#define M4U_PORT_VDEC_UFO  MTK_M4U_ID(M4U_LARB1_ID, 2)
+#define M4U_PORT_VDEC_VLD  MTK_M4U_ID(M4U_LARB1_ID, 3)
+#define M4U_PORT_VDEC_VLD2 MTK_M4U_ID(M4U_LARB1_ID, 4)
+#define M4U_PORT_VDEC_AVC_MV   MTK_M4U_ID(M4U_LARB1_ID, 5)
+#define M4U_PORT_VDEC_PRED_RD  MTK_M4U_ID(M4U_LARB1_ID, 6)
+#define M4U_PORT_VDEC_PRED_WR  MTK_M4U_ID(M4U_LARB1_ID, 7)
+#define M4U_PORT_VDEC_PPWRAP   MTK_M4U_ID(M4U_LARB1_ID, 8)
+
+/* larb2 */
+#define M4U_PORT_CAM_IMGO  MTK_M4U_ID(M4U_LARB2_ID, 0)
+#define M4U_PORT_CAM_RRZO  MTK_M4U_ID(M4U_LARB2_ID, 1)
+#define M4U_PORT_CAM_AAO   MTK_M4U_ID(M4U_LARB2_ID, 2)
+#define M4U_PORT_CAM_LCSO  MTK_M4U_ID(M4U_LARB2_ID, 3)
+#define M4U_PORT_CAM_ESFKO MTK_M4U_ID(M4U_LARB2_ID, 4)
+#define M4U_PORT_CAM_IMGO_SMTK_M4U_ID(M4U_LARB2_ID, 5)
+#define M4U_PORT_CAM_LSCI  MTK_M4U_ID(M4U_LARB2_ID, 6)
+#define M4U_PORT_CAM_LSCI_DMTK_M4U_ID(M4U_LARB2_ID, 7)
+#define M4U_PORT_CAM_BPCI  MTK_M4U_ID(M4U_LARB2_ID, 8)
+#define M4U_PORT_CAM_BPCI_DMTK_M4U_ID(M4U_LARB2_ID, 9)
+#define M4U

[PATCH v2 2/2] iommu: mtk_iommu: Add support for MT6795 Helio X10 M4Us

[AngeloGioacchino Del Regno]

Add support for the M4Us found in the MT6795 Helio X10 SoC.

Signed-off-by: AngeloGioacchino Del Regno 

---
 drivers/iommu/mtk_iommu.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 090cf6e15f85..97ff30ed2d0f 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -159,6 +159,7 @@
 enum mtk_iommu_plat {
M4U_MT2712,
M4U_MT6779,
+   M4U_MT6795,
M4U_MT8167,
M4U_MT8173,
M4U_MT8183,
@@ -954,7 +955,8 @@ static int mtk_iommu_hw_init(const struct mtk_iommu_data 
*data, unsigned int ban
 * Global control settings are in bank0. May re-init these global 
registers
 * since no sure if there is bank0 consumers.
 */
-   if (data->plat_data->m4u_plat == M4U_MT8173) {
+   if (data->plat_data->m4u_plat == M4U_MT6795 ||
+   data->plat_data->m4u_plat == M4U_MT8173) {
regval = F_MMU_PREFETCH_RT_REPLACE_MOD |
 F_MMU_TF_PROT_TO_PROGRAM_ADDR_MT8173;
} else {
@@ -1422,6 +1424,18 @@ static const struct mtk_iommu_plat_data mt6779_data = {
.larbid_remap  = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}},
 };
 
+static const struct mtk_iommu_plat_data mt6795_data = {
+   .m4u_plat = M4U_MT6795,
+   .flags= HAS_4GB_MODE | HAS_BCLK | RESET_AXI |
+   HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM,
+   .inv_sel_reg  = REG_MMU_INV_SEL_GEN1,
+   .banks_num= 1,
+   .banks_enable = {true},
+   .iova_region  = single_domain,
+   .iova_region_nr = ARRAY_SIZE(single_domain),
+   .larbid_remap = {{0}, {1}, {2}, {3}, {4}}, /* Linear mapping. */
+};
+
 static const struct mtk_iommu_plat_data mt8167_data = {
.m4u_plat = M4U_MT8167,
.flags= RESET_AXI | HAS_LEGACY_IVRP_PADDR | MTK_IOMMU_TYPE_MM,
@@ -1533,6 +1547,7 @@ static const struct mtk_iommu_plat_data mt8195_data_vpp = 
{
 static const struct of_device_id mtk_iommu_of_ids[] = {
{ .compatible = "mediatek,mt2712-m4u", .data = &mt2712_data},
{ .compatible = "mediatek,mt6779-m4u", .data = &mt6779_data},
+   { .compatible = "mediatek,mt6795-m4u", .data = &mt6795_data},
{ .compatible = "mediatek,mt8167-m4u", .data = &mt8167_data},
{ .compatible = "mediatek,mt8173-m4u", .data = &mt8173_data},
{ .compatible = "mediatek,mt8183-m4u", .data = &mt8183_data},
-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 0/2] MediaTek Helio X10 MT6795 - M4U/IOMMU Support

[AngeloGioacchino Del Regno]

In an effort to give some love to the apparently forgotten MT6795 SoC,
I am upstreaming more components that are necessary to support platforms
powered by this one apart from a simple boot to serial console.

This series introduces support for the IOMMUs found on this SoC.

Tested on a MT6795 Sony Xperia M5 (codename "Holly") smartphone.

Changes in v2:
 - Rebased on top of 
https://patchwork.kernel.org/project/linux-mediatek/list/?series=642681

AngeloGioacchino Del Regno (2):
  dt-bindings: mediatek: Add bindings for MT6795 M4U
  iommu: mtk_iommu: Add support for MT6795 Helio X10 M4Us

 .../bindings/iommu/mediatek,iommu.yaml|  4 +
 drivers/iommu/mtk_iommu.c | 17 +++-
 include/dt-bindings/memory/mt6795-larb-port.h | 96 +++
 3 files changed, 116 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/memory/mt6795-larb-port.h

-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 4/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to pericfg

[Matthias Brugger]

On 17/05/2022 15:21, AngeloGioacchino Del Regno wrote:

On some SoCs (of which only MT8195 is supported at the time of writing),
the "R" and "W" (I/O) enable bits for the IOMMUs are in the pericfg_ao
register space and not in the IOMMU space: as it happened already with
infracfg, it is expected that this list will grow.

Instead of specifying pericfg compatibles on a per-SoC basis, following
what was done with infracfg, let's lookup the syscon by phandle instead.
Also following the previous infracfg change, add a warning for outdated
devicetrees, in hope that the user will take action.

Signed-off-by: AngeloGioacchino Del Regno 



Reviewed-by: Matthias Brugger 


---
  drivers/iommu/mtk_iommu.c | 26 --
  1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index cfaaa98d2b50..c7e2d836199e 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -138,6 +138,8 @@
  /* PM and clock always on. e.g. infra iommu */
  #define PM_CLK_AO BIT(15)
  #define IFA_IOMMU_PCIE_SUPPORTBIT(16)
+/* IOMMU I/O (r/w) is enabled using PERICFG_IOMMU_1 register */
+#define HAS_PERI_IOMMU1_REGBIT(17)
  
  #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask)	\

pdata)->flags) & (mask)) == (_x))
@@ -187,7 +189,6 @@ struct mtk_iommu_plat_data {
u32 flags;
u32 inv_sel_reg;
  
-	char			*pericfg_comp_str;

struct list_head*hw_list;
unsigned intiova_region_nr;
const struct mtk_iommu_iova_region  *iova_region;
@@ -1214,14 +1215,19 @@ static int mtk_iommu_probe(struct platform_device *pdev)
goto out_runtime_disable;
}
} else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
-  data->plat_data->pericfg_comp_str) {
-   infracfg = 
syscon_regmap_lookup_by_compatible(data->plat_data->pericfg_comp_str);
-   if (IS_ERR(infracfg)) {
-   ret = PTR_ERR(infracfg);
-   goto out_runtime_disable;
-   }
+  MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_PERI_IOMMU1_REG)) {
+   data->pericfg = syscon_regmap_lookup_by_phandle(dev->of_node, 
"mediatek,pericfg");
+   if (IS_ERR(data->pericfg)) {
+   dev_warn(dev, "Cannot find phandle to mediatek,pericfg:"
+ " Please update your devicetree.\n");
  
-		data->pericfg = infracfg;

+   p = "mediatek,mt8195-pericfg_ao";
+   data->pericfg = syscon_regmap_lookup_by_compatible(p);
+   if (IS_ERR(data->pericfg)) {
+   ret = PTR_ERR(data->pericfg);
+   goto out_runtime_disable;
+   }
+   }
}
  
  	platform_set_drvdata(pdev, data);

@@ -1480,8 +1486,8 @@ static const struct mtk_iommu_plat_data mt8192_data = {
  static const struct mtk_iommu_plat_data mt8195_data_infra = {
.m4u_plat = M4U_MT8195,
.flags= WR_THROT_EN | DCM_DISABLE | STD_AXI_MODE | 
PM_CLK_AO |
-   MTK_IOMMU_TYPE_INFRA | IFA_IOMMU_PCIE_SUPPORT,
-   .pericfg_comp_str = "mediatek,mt8195-pericfg_ao",
+   HAS_PERI_IOMMU1_REG | MTK_IOMMU_TYPE_INFRA |
+   IFA_IOMMU_PCIE_SUPPORT,
.inv_sel_reg  = REG_MMU_INV_SEL_GEN2,
.banks_num= 5,
.banks_enable = {true, false, false, false, true},

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 6/7] dt-bindings: iommu: mediatek: Require mediatek, infracfg for mt2712/8173

[AngeloGioacchino Del Regno]

Both MT2712 and MT8173 got a mediatek,infracfg phandle: add that to
the required properties for these SoCs to deprecate the old way of
looking for SoC-specific infracfg compatible in the entire devicetree.

Signed-off-by: AngeloGioacchino Del Regno 

Acked-by: Rob Herring 
---
 .../devicetree/bindings/iommu/mediatek,iommu.yaml| 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml 
b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index c4af41947593..acc2d7e63a9f 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -175,6 +175,18 @@ allOf:
   required:
 - power-domains
 
+  - if:
+  properties:
+compatible:
+  contains:
+enum:
+  - mediatek,mt2712-m4u
+  - mediatek,mt8173-m4u
+
+then:
+  required:
+- mediatek,infracfg
+
   - if: # The IOMMUs don't have larbs.
   not:
 properties:
-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 3/7] iommu: mtk_iommu: Lookup phandle to retrieve syscon to pericfg

[AngeloGioacchino Del Regno]

On some SoCs (of which only MT8195 is supported at the time of writing),
the "R" and "W" (I/O) enable bits for the IOMMUs are in the pericfg_ao
register space and not in the IOMMU space: as it happened already with
infracfg, it is expected that this list will grow.

Instead of specifying pericfg compatibles on a per-SoC basis, following
what was done with infracfg, let's lookup the syscon by phandle instead.
Also following the previous infracfg change, add a warning for outdated
devicetrees, in hope that the user will take action.

Signed-off-by: AngeloGioacchino Del Regno 

---
 drivers/iommu/mtk_iommu.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index d16b95e71ded..090cf6e15f85 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -138,6 +138,8 @@
 /* PM and clock always on. e.g. infra iommu */
 #define PM_CLK_AO  BIT(15)
 #define IFA_IOMMU_PCIE_SUPPORT BIT(16)
+/* IOMMU I/O (r/w) is enabled using PERICFG_IOMMU_1 register */
+#define HAS_PERI_IOMMU1_REGBIT(17)
 
 #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask)   \
pdata)->flags) & (mask)) == (_x))
@@ -187,7 +189,6 @@ struct mtk_iommu_plat_data {
u32 flags;
u32 inv_sel_reg;
 
-   char*pericfg_comp_str;
struct list_head*hw_list;
unsigned intiova_region_nr;
const struct mtk_iommu_iova_region  *iova_region;
@@ -1214,14 +1215,19 @@ static int mtk_iommu_probe(struct platform_device *pdev)
goto out_runtime_disable;
}
} else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
-  data->plat_data->pericfg_comp_str) {
-   infracfg = 
syscon_regmap_lookup_by_compatible(data->plat_data->pericfg_comp_str);
-   if (IS_ERR(infracfg)) {
-   ret = PTR_ERR(infracfg);
-   goto out_runtime_disable;
-   }
+  MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_PERI_IOMMU1_REG)) {
+   data->pericfg = syscon_regmap_lookup_by_phandle(dev->of_node, 
"mediatek,pericfg");
+   if (IS_ERR(data->pericfg)) {
+   dev_info(dev, "Cannot find phandle to mediatek,pericfg:"
+ " Please update your devicetree.\n");
 
-   data->pericfg = infracfg;
+   p = "mediatek,mt8195-pericfg_ao";
+   data->pericfg = syscon_regmap_lookup_by_compatible(p);
+   if (IS_ERR(data->pericfg)) {
+   ret = PTR_ERR(data->pericfg);
+   goto out_runtime_disable;
+   }
+   }
}
 
platform_set_drvdata(pdev, data);
@@ -1480,8 +1486,8 @@ static const struct mtk_iommu_plat_data mt8192_data = {
 static const struct mtk_iommu_plat_data mt8195_data_infra = {
.m4u_plat = M4U_MT8195,
.flags= WR_THROT_EN | DCM_DISABLE | STD_AXI_MODE | 
PM_CLK_AO |
-   MTK_IOMMU_TYPE_INFRA | IFA_IOMMU_PCIE_SUPPORT,
-   .pericfg_comp_str = "mediatek,mt8195-pericfg_ao",
+   HAS_PERI_IOMMU1_REG | MTK_IOMMU_TYPE_INFRA |
+   IFA_IOMMU_PCIE_SUPPORT,
.inv_sel_reg  = REG_MMU_INV_SEL_GEN2,
.banks_num= 5,
.banks_enable = {true, false, false, false, true},
-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 2/7] iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg

[AngeloGioacchino Del Regno]

This driver will get support for more SoCs and the list of infracfg
compatibles is expected to grow: in order to prevent getting this
situation out of control and see a long list of compatible strings,
add support to retrieve a handle to infracfg's regmap through a
new "mediatek,infracfg" phandle.

In order to keep retrocompatibility with older devicetrees, the old
way is kept in place, but also a dev_warn() was added to advertise
this change in hope that the user will see it and eventually update
the devicetree if this is possible.

Signed-off-by: AngeloGioacchino Del Regno 

---
 drivers/iommu/mtk_iommu.c | 40 +--
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 71b2ace74cd6..d16b95e71ded 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1134,22 +1134,34 @@ static int mtk_iommu_probe(struct platform_device *pdev)
data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
 
if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
-   switch (data->plat_data->m4u_plat) {
-   case M4U_MT2712:
-   p = "mediatek,mt2712-infracfg";
-   break;
-   case M4U_MT8173:
-   p = "mediatek,mt8173-infracfg";
-   break;
-   default:
-   p = NULL;
+   infracfg = syscon_regmap_lookup_by_phandle(dev->of_node, 
"mediatek,infracfg");
+   if (IS_ERR(infracfg)) {
+   dev_info(dev, "Cannot find phandle to 
mediatek,infracfg:"
+ " Please update your devicetree.\n");
+   /*
+* Legacy devicetrees will not specify a phandle to
+* mediatek,infracfg: in that case, we use the older
+* way to retrieve a syscon to infra.
+*
+* This is for retrocompatibility purposes only, hence
+* no more compatibles shall be added to this.
+*/
+   switch (data->plat_data->m4u_plat) {
+   case M4U_MT2712:
+   p = "mediatek,mt2712-infracfg";
+   break;
+   case M4U_MT8173:
+   p = "mediatek,mt8173-infracfg";
+   break;
+   default:
+   p = NULL;
+   }
+
+   infracfg = syscon_regmap_lookup_by_compatible(p);
+   if (IS_ERR(infracfg))
+   return PTR_ERR(infracfg);
}
 
-   infracfg = syscon_regmap_lookup_by_compatible(p);
-
-   if (IS_ERR(infracfg))
-   return PTR_ERR(infracfg);
-
ret = regmap_read(infracfg, REG_INFRA_MISC, &val);
if (ret)
return ret;
-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 0/7] mtk_iommu: Specify phandles to infracfg and pericfg

[AngeloGioacchino Del Regno]

The IOMMU has registers in the infracfg and/or pericfg iospaces: as
for the currently supported SoCs, MT2712 and MT8173 need a phandle to
infracfg, while MT8195 needs one to pericfg.

Before this change, the driver was checking for a SoC-specific infra/peri
compatible but, sooner or later, these lists are going to grow a lot...
...and this is why it was chosen to add phandles (as it was done with
some other drivers already - look at mtk-pm-domains, mt8192-afe

Please note that, while it was necessary to update the devicetrees for
MT8173 and MT2712e, there was no update for MT8195 because there is no
IOMMU node in there yet.

Changes in v2:
 - Squashed dt-bindings patches as suggested by Matthias
 - Removed quotes from infra/peri phandle refs
 - Changed dev_warn to dev_info in patches [2/7], [3/7]

AngeloGioacchino Del Regno (7):
  dt-bindings: iommu: mediatek: Add phandles for mediatek infra/pericfg
  iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg
  iommu: mtk_iommu: Lookup phandle to retrieve syscon to pericfg
  arm64: dts: mediatek: mt8173: Add mediatek,infracfg phandle for IOMMU
  arm64: dts: mediatek: mt2712e: Add mediatek,infracfg phandle for IOMMU
  dt-bindings: iommu: mediatek: Require mediatek,infracfg for
mt2712/8173
  dt-bindings: iommu: mediatek: Require mediatek,pericfg for
mt8195-infra

 .../bindings/iommu/mediatek,iommu.yaml| 30 +
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi |  2 +
 arch/arm64/boot/dts/mediatek/mt8173.dtsi  |  1 +
 drivers/iommu/mtk_iommu.c | 66 ---
 4 files changed, 75 insertions(+), 24 deletions(-)

-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 4/7] arm64: dts: mediatek: mt8173: Add mediatek, infracfg phandle for IOMMU

[AngeloGioacchino Del Regno]

The IOMMU driver now looks for the "mediatek,infracfg" phandle as a
new way to retrieve a syscon to that:
even though the old way is retained, it has been deprecated and the
driver will write a message in kmsg advertising to use the phandle
way instead.

For this reason, assign the right phandle to mediatek,infracfg in
the iommu node.

Signed-off-by: AngeloGioacchino Del Regno 

---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi 
b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 40d7b47fc52e..825a3c670373 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -588,6 +588,7 @@ iommu: iommu@10205000 {
interrupts = ;
clocks = <&infracfg CLK_INFRA_M4U>;
clock-names = "bclk";
+   mediatek,infracfg = <&infracfg>;
mediatek,larbs = <&larb0>, <&larb1>, <&larb2>,
 <&larb3>, <&larb4>, <&larb5>;
#iommu-cells = <1>;
-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 5/7] arm64: dts: mediatek: mt2712e: Add mediatek, infracfg phandle for IOMMU

[AngeloGioacchino Del Regno]

The IOMMU driver now looks for the "mediatek,infracfg" phandle as a
new way to retrieve a syscon to that:
even though the old way is retained, it has been deprecated and the
driver will write a message in kmsg advertising to use the phandle
way instead.

For this reason, assign the right phandle to mediatek,infracfg in
the iommu node.

Signed-off-by: AngeloGioacchino Del Regno 

---
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi 
b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 623eb3beabf2..4797537cb368 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -329,6 +329,7 @@ iommu0: iommu@10205000 {
interrupts = ;
clocks = <&infracfg CLK_INFRA_M4U>;
clock-names = "bclk";
+   mediatek,infracfg = <&infracfg>;
mediatek,larbs = <&larb0>, <&larb1>, <&larb2>,
 <&larb3>, <&larb6>;
#iommu-cells = <1>;
@@ -346,6 +347,7 @@ iommu1: iommu@1020a000 {
interrupts = ;
clocks = <&infracfg CLK_INFRA_M4U>;
clock-names = "bclk";
+   mediatek,infracfg = <&infracfg>;
mediatek,larbs = <&larb4>, <&larb5>, <&larb7>;
#iommu-cells = <1>;
};
-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 7/7] dt-bindings: iommu: mediatek: Require mediatek, pericfg for mt8195-infra

[AngeloGioacchino Del Regno]

The MT8195 SoC has IOMMU related registers in the pericfg_ao iospace:
require a phandle to that.

Signed-off-by: AngeloGioacchino Del Regno 

Acked-by: Rob Herring 
---
 .../devicetree/bindings/iommu/mediatek,iommu.yaml  | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml 
b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index acc2d7e63a9f..d5e3272a54e8 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -187,6 +187,16 @@ allOf:
   required:
 - mediatek,infracfg
 
+  - if:
+  properties:
+compatible:
+  contains:
+const: mediatek,mt8195-iommu-infra
+
+then:
+  required:
+- mediatek,pericfg
+
   - if: # The IOMMUs don't have larbs.
   not:
 properties:
-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 1/7] dt-bindings: iommu: mediatek: Add phandles for mediatek infra/pericfg

[AngeloGioacchino Del Regno]

Add properties "mediatek,infracfg" and "mediatek,pericfg" to let the
mtk_iommu driver retrieve phandles to the infracfg and pericfg syscon(s)
instead of performing a per-soc compatible lookup.

Signed-off-by: AngeloGioacchino Del Regno 

---
 .../devicetree/bindings/iommu/mediatek,iommu.yaml | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml 
b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index 2ae3bbad7f1a..c4af41947593 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -101,6 +101,10 @@ properties:
 items:
   - const: bclk
 
+  mediatek,infracfg:
+$ref: /schemas/types.yaml#/definitions/phandle
+description: The phandle to the mediatek infracfg syscon
+
   mediatek,larbs:
 $ref: /schemas/types.yaml#/definitions/phandle-array
 minItems: 1
@@ -112,6 +116,10 @@ properties:
   Refer to bindings/memory-controllers/mediatek,smi-larb.yaml. It must sort
   according to the local arbiter index, like larb0, larb1, larb2...
 
+  mediatek,pericfg:
+$ref: /schemas/types.yaml#/definitions/phandle
+description: The phandle to the mediatek pericfg syscon
+
   '#iommu-cells':
 const: 1
 description: |
-- 
2.35.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/8] dt-bindings: iommu: mediatek: Add mediatek,infracfg phandle

[Matthias Brugger]

On 17/05/2022 15:21, AngeloGioacchino Del Regno wrote:

Add property "mediatek,infracfg" to let the mtk_iommu driver retrieve
a phandle to the infracfg syscon instead of performing a per-soc
compatible lookup.

Signed-off-by: AngeloGioacchino Del Regno 

---
  Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml | 4 
  1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml 
b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
index 2ae3bbad7f1a..78c72c22740b 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
@@ -101,6 +101,10 @@ properties:
  items:
- const: bclk
  
+  mediatek,infracfg:

+$ref: "/schemas/types.yaml#/definitions/phandle"
+description: The phandle to the mediatek infracfg syscon
+
mediatek,larbs:
  $ref: /schemas/types.yaml#/definitions/phandle-array
  minItems: 1


I think we can squash patch 7 in here. Same holds for pericfg

Regards,
Matthias
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/8] iommu: mtk_iommu: Lookup phandle to retrieve syscon to infracfg

[AngeloGioacchino Del Regno]

Il 17/05/22 16:12, Robin Murphy ha scritto:

On 2022-05-17 14:21, AngeloGioacchino Del Regno wrote:

This driver will get support for more SoCs and the list of infracfg
compatibles is expected to grow: in order to prevent getting this
situation out of control and see a long list of compatible strings,
add support to retrieve a handle to infracfg's regmap through a
new "mediatek,infracfg" phandle.

In order to keep retrocompatibility with older devicetrees, the old
way is kept in place, but also a dev_warn() was added to advertise
this change in hope that the user will see it and eventually update
the devicetree if this is possible.

Signed-off-by: AngeloGioacchino Del Regno 

---
  drivers/iommu/mtk_iommu.c | 40 +--
  1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 71b2ace74cd6..cfaaa98d2b50 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1134,22 +1134,34 @@ static int mtk_iommu_probe(struct platform_device *pdev)
  data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
  if (MTK_IOMMU_HAS_FLAG(data->plat_data, HAS_4GB_MODE)) {
-    switch (data->plat_data->m4u_plat) {
-    case M4U_MT2712:
-    p = "mediatek,mt2712-infracfg";
-    break;
-    case M4U_MT8173:
-    p = "mediatek,mt8173-infracfg";
-    break;
-    default:
-    p = NULL;
+    infracfg = syscon_regmap_lookup_by_phandle(dev->of_node, 
"mediatek,infracfg");

+    if (IS_ERR(infracfg)) {
+    dev_warn(dev, "Cannot find phandle to mediatek,infracfg:"
+  " Please update your devicetree.\n");


Is this really a dev_warn-level problem? There's no functional impact, given that 
we can't stop supporting the original binding any time soon, if ever, so I suspect 
this is more likely to just annoy users and CI systems than effect any significant 
change.




The upstream devicetrees were updated to use the new handle and this is a way to
warn about having outdated DTs... besides, I believe that CIs will always get 
the
devicetree from the same tree that the kernel was compiled from (hence no 
message
will be thrown).

In any case, if you think that a dev_info would be more appropriate, I can 
change
that no problem.


+    /*
+ * Legacy devicetrees will not specify a phandle to
+ * mediatek,infracfg: in that case, we use the older
+ * way to retrieve a syscon to infra.
+ *
+ * This is for retrocompatibility purposes only, hence
+ * no more compatibles shall be added to this.
+ */
+    switch (data->plat_data->m4u_plat) {
+    case M4U_MT2712:
+    p = "mediatek,mt2712-infracfg";
+    break;
+    case M4U_MT8173:
+    p = "mediatek,mt8173-infracfg";
+    break;
+    default:
+    p = NULL;
+    }
+
+    infracfg = syscon_regmap_lookup_by_compatible(p);


Would it not make sense to punt this over to the same mechanism as for pericfg, 
such that it simplifies down to something like:


 if (IS_ERR(infracfg) && plat_data->infracfg) {
     infracfg = syscon_regmap_lookup_by_compatible(plat_data->infracfg);
     ...
 }

?

TBH if we're still going to have a load of per-SoC data in the driver anyway then I 
don't see that we really gain much by delegating one aspect of it to DT, but meh. I 
would note that with the phandle approach, you still need some *other* flag in the 
driver to know whether a phandle is expected to be present or not, whereas a NULL 
vs. non-NULL string is at least neatly self-describing.




That would be possible but, as Yong also pointed out, we should try to reduce 
the
per-SoC data in the driver by commonizing as much as possible, because this 
driver
supports a very long list of SoCs (even though they're not all upstreamed yet),
and the list is going to grow even more with time: this is also why I have 
changed
the MT8195 pericfg regmap lookup with a phandle like I've done for infra.

There would also be another way, which would imply adding a generic compatible
"mediatek,infracfg" to the infra syscon node, but I really don't like that for
more than one reason, one of which is that this poses an issue, for which it's
not guaranteed that the registers are in infracfg and not infracfg_ao (even
though the offsets are the same), so then we would be back to ground zero.

Regards,
Angelo

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 7/8] dt-bindings: iommu: mediatek: Require mediatek,infracfg for mt2712/8173

[AngeloGioacchino Del Regno]

Il 18/05/22 03:41, Rob Herring ha scritto:

On Tue, May 17, 2022 at 03:21:06PM +0200, AngeloGioacchino Del Regno wrote:

Both MT2712 and MT8173 got a mediatek,infracfg phandle: add that to
the required properties for these SoCs to deprecate the old way of
looking for SoC-specific infracfg compatible in the entire devicetree.


Wait, what? If there's only one possible node that can match, I prefer
the 'old way'. Until we implemented a phandle cache, searching the
entire tree was how phandle lookups worked too, so not any better.

But if this makes things more consistent,

Acked-by: Rob Herring 



Hello Rob,

This makes things definitely more consistent, as it's done like that on
mtk-pm-domains and other mtk drivers as well.

The main reason why this phandle is useful, here and in other drivers, is
that we're seeing a list of compatibles that is growing more and more, so
you see stuff like (mockup names warning):

switch (some_model)
case MT1000:
p = "mediatek,mt1000-infracfg";
break;
case MT1001:
p = "mediatek,mt1001-infracfg";
break;
case MT1002:
p = "mediatek,mt1002-infracfg";
break;
.add another 20 SoCs, replicate this switch for 4/5 drivers

and this is why I want the mtk_iommu driver to also get that phandle like
some other drivers are already doing.

By the way, thanks for the ack!

Regards,
Angelo
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 6/7] x86/boot/tboot: Move tboot_force_iommu() to Intel IOMMU

[Baolu Lu]

On 2022/5/17 19:13, Jason Gunthorpe wrote:

On Tue, May 17, 2022 at 10:05:43AM +0800, Baolu Lu wrote:

Hi Jason,

On 2022/5/17 02:06, Jason Gunthorpe wrote:

+static __init int tboot_force_iommu(void)
+{
+   if (!tboot_enabled())
+   return 0;
+
+   if (no_iommu || dmar_disabled)
+   pr_warn("Forcing Intel-IOMMU to enabled\n");

Unrelated, but when we are in the special secure IOMMU modes, do we
force ATS off? Specifically does the IOMMU reject TLPs that are marked
as translated?


Good question. From IOMMU point of view, I don't see a point to force
ATS off, but trust boot involves lots of other things that I am not
familiar with. Anybody else could help to answer?


ATS is inherently not secure, if a rouge device can issue a TLP with
the translated bit set then it has unlimited access to host memory.


Agreed. The current logic is that the platform lets the OS know such
devices through firmware (ACPI/DT) and OS sets the untrusted flag in
their device structures. The IOMMU subsystem will disable ATS on devices
with the untrusted flag set.

There is some discussion about allowing the supervisor users to set the
trust policy through the sysfs ABI, but I don't think this has happened
in upstream kernel.


Many of these trusted iommu scenarios rely on the idea that a rouge
device cannot DMA to arbitary system memory.


I am not sure whether tboot has the same requirement.

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu