[PATCH 0/3] Add support of unmanaged domain to mediatek IOMMU

[PATCH 0/3] Add support of unmanaged domain to mediatek IOMMU

[Alexandre Bailon]

Mediatek mt8365 SoC APU support (AI Processor Unit) is being upstremed.
This device, behind an IOMMU uses remoteproc which requires unmanaged domain.
This updates the iommu driver to support unmanaged iommu domain and let
remoteproc use the IOMMU.

Alexandre Bailon (3):
  dt-bindings: memory: mediatek: Add support of unmanaged iommu domain
  iommu: mediatek: Add support of unmanaged iommu domain
  dt-bindings: iommu: memory: Use unmanaged iommu domain for the APU

 drivers/iommu/mtk_iommu.c                              | 6 ++++++
 include/dt-bindings/memory/mediatek,mt8365-larb-port.h | 4 ++--
 include/dt-bindings/memory/mtk-memory-port.h           | 3 +++
 3 files changed, 11 insertions(+), 2 deletions(-)

-- 
2.38.2

[PATCH 1/3] dt-bindings: memory: mediatek: Add support of unmanaged iommu domain

[Alexandre Bailon]

Currently, the only way to have an unmanaged domain for a device
is having a dedicated iova region or bank.
This adds a new bit to MTK_M4U_ID that allows defining devices
that needs unmanaged iommu domain.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 include/dt-bindings/memory/mtk-memory-port.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/dt-bindings/memory/mtk-memory-port.h b/include/dt-bindings/memory/mtk-memory-port.h
index 2f68a0511a257..86896c776691a 100644
--- a/include/dt-bindings/memory/mtk-memory-port.h
+++ b/include/dt-bindings/memory/mtk-memory-port.h
@@ -7,11 +7,14 @@
 #define __DT_BINDINGS_MEMORY_MTK_MEMORY_PORT_H_
 
 #define MTK_LARB_NR_MAX			32
+#define MTK_UNMANAGED_DEVICE            (1 << 31)
 
 #define MTK_M4U_ID(larb, port)		(((larb) << 5) | (port))
 #define MTK_M4U_TO_LARB(id)		(((id) >> 5) & 0x1f)
 #define MTK_M4U_TO_PORT(id)		((id) & 0x1f)
+#define MTK_M4U_UNMANAGED(id)		((id) & MTK_UNMANAGED_DEVICE)
 
 #define MTK_IFAIOMMU_PERI_ID(port)	MTK_M4U_ID(0, port)
+#define MTK_M4U_UNMANAGED_ID(larb, port) (MTK_M4U_ID(larb, port) | MTK_UNMANAGED_DEVICE)
 
 #endif
-- 
2.38.2

[PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu domain

[Alexandre Bailon]

Currently, the driver can allocate an unmanaged iommu domain.
But, this only works for SoC having multiple bank or multiple iova region.
This updates the driver to also support unmanaged iommu domain if
MTK_UNMANAGED_DEVICE is set in the iommu id.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 drivers/iommu/mtk_iommu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2badd6acfb23d..5884a1ad409a0 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -858,9 +858,15 @@ static int mtk_iommu_get_group_id(struct device *dev, const struct mtk_iommu_pla
 static struct iommu_group *mtk_iommu_device_group(struct device *dev)
 {
 	struct mtk_iommu_data *c_data = dev_iommu_priv_get(dev), *data;
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 	struct list_head *hw_list = c_data->hw_list;
 	struct iommu_group *group;
 	int groupid;
+	int i;
+
+	for (i = 0; i < fwspec->num_ids; i++)
+		if (MTK_M4U_UNMANAGED(fwspec->ids[i]))
+			return iommu_group_alloc();
 
 	data = mtk_iommu_get_frst_data(hw_list);
 	if (!data)
-- 
2.38.2

[PATCH 3/3] dt-bindings: iommu: memory: Use unmanaged iommu domain for the APU

[Alexandre Bailon]

This updates the iommu id to use unmanaged iommu domain for the APU.
This is required by remoteproc to use the iommu.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 include/dt-bindings/memory/mediatek,mt8365-larb-port.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/dt-bindings/memory/mediatek,mt8365-larb-port.h b/include/dt-bindings/memory/mediatek,mt8365-larb-port.h
index 56d5a5dd519e7..86d82a8f6017d 100644
--- a/include/dt-bindings/memory/mediatek,mt8365-larb-port.h
+++ b/include/dt-bindings/memory/mediatek,mt8365-larb-port.h
@@ -24,8 +24,8 @@
 #define M4U_PORT_MDP_WROT0		MTK_M4U_ID(M4U_LARB0_ID, 7)
 #define M4U_PORT_MDP_RDMA1		MTK_M4U_ID(M4U_LARB0_ID, 8)
 #define M4U_PORT_DISP_FAKE0		MTK_M4U_ID(M4U_LARB0_ID, 9)
-#define M4U_PORT_APU_READ		MTK_M4U_ID(M4U_LARB0_ID, 10)
-#define M4U_PORT_APU_WRITE		MTK_M4U_ID(M4U_LARB0_ID, 11)
+#define M4U_PORT_APU_READ		MTK_M4U_UNMANAGED_ID(M4U_LARB0_ID, 10)
+#define M4U_PORT_APU_WRITE		MTK_M4U_UNMANAGED_ID(M4U_LARB0_ID, 11)
 
 /* larb1 */
 #define M4U_PORT_VENC_RCPU		MTK_M4U_ID(M4U_LARB1_ID, 0)
-- 
2.38.2

Re: [PATCH 1/3] dt-bindings: memory: mediatek: Add support of unmanaged iommu domain

[Alexandre Mergnat]

On 30/01/2023 11:27, Alexandre Bailon wrote:
> Currently, the only way to have an unmanaged domain for a device
> is having a dedicated iova region or bank.
> This adds a new bit to MTK_M4U_ID that allows defining devices
> that needs unmanaged iommu domain.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>   include/dt-bindings/memory/mtk-memory-port.h | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/include/dt-bindings/memory/mtk-memory-port.h b/include/dt-bindings/memory/mtk-memory-port.h
> index 2f68a0511a257..86896c776691a 100644
> --- a/include/dt-bindings/memory/mtk-memory-port.h
> +++ b/include/dt-bindings/memory/mtk-memory-port.h
> @@ -7,11 +7,14 @@
>   #define __DT_BINDINGS_MEMORY_MTK_MEMORY_PORT_H_
>   
>   #define MTK_LARB_NR_MAX			32
> +#define MTK_UNMANAGED_DEVICE            (1 << 31)
>   
>   #define MTK_M4U_ID(larb, port)		(((larb) << 5) | (port))
>   #define MTK_M4U_TO_LARB(id)		(((id) >> 5) & 0x1f)
>   #define MTK_M4U_TO_PORT(id)		((id) & 0x1f)
> +#define MTK_M4U_UNMANAGED(id)		((id) & MTK_UNMANAGED_DEVICE)
>   
>   #define MTK_IFAIOMMU_PERI_ID(port)	MTK_M4U_ID(0, port)
> +#define MTK_M4U_UNMANAGED_ID(larb, port) (MTK_M4U_ID(larb, port) | MTK_UNMANAGED_DEVICE)
>   
>   #endif

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

Re: [PATCH 1/3] dt-bindings: memory: mediatek: Add support of unmanaged iommu domain

[Robin Murphy]

On 2023-01-30 10:27, Alexandre Bailon wrote:
> Currently, the only way to have an unmanaged domain for a device
> is having a dedicated iova region or bank.
> This adds a new bit to MTK_M4U_ID that allows defining devices
> that needs unmanaged iommu domain.

These are details of Linux-specific usage policy, so don't belong in DT 
bindings. I believe the driver already has some internal data to make 
its own assignment of particular devices into particular groups, so 
please just add to that.

Thanks,
Robin.

> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>   include/dt-bindings/memory/mtk-memory-port.h | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/include/dt-bindings/memory/mtk-memory-port.h b/include/dt-bindings/memory/mtk-memory-port.h
> index 2f68a0511a257..86896c776691a 100644
> --- a/include/dt-bindings/memory/mtk-memory-port.h
> +++ b/include/dt-bindings/memory/mtk-memory-port.h
> @@ -7,11 +7,14 @@
>   #define __DT_BINDINGS_MEMORY_MTK_MEMORY_PORT_H_
>   
>   #define MTK_LARB_NR_MAX			32
> +#define MTK_UNMANAGED_DEVICE            (1 << 31)
>   
>   #define MTK_M4U_ID(larb, port)		(((larb) << 5) | (port))
>   #define MTK_M4U_TO_LARB(id)		(((id) >> 5) & 0x1f)
>   #define MTK_M4U_TO_PORT(id)		((id) & 0x1f)
> +#define MTK_M4U_UNMANAGED(id)		((id) & MTK_UNMANAGED_DEVICE)
>   
>   #define MTK_IFAIOMMU_PERI_ID(port)	MTK_M4U_ID(0, port)
> +#define MTK_M4U_UNMANAGED_ID(larb, port) (MTK_M4U_ID(larb, port) | MTK_UNMANAGED_DEVICE)
>   
>   #endif

Re: [PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu domain

[Alexandre Mergnat]

On 30/01/2023 11:27, Alexandre Bailon wrote:
> Currently, the driver can allocate an unmanaged iommu domain.
> But, this only works for SoC having multiple bank or multiple iova region.
> This updates the driver to also support unmanaged iommu domain if
> MTK_UNMANAGED_DEVICE is set in the iommu id.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

Re: [PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu domain

[Robin Murphy]

On 2023-01-30 10:27, Alexandre Bailon wrote:
> Currently, the driver can allocate an unmanaged iommu domain.
> But, this only works for SoC having multiple bank or multiple iova region.

That is for good reason - there is only a single pagetable per bank, so 
if there are multiple devices assigned to a single bank, they cannot 
possibly be attached to different domains at the same time. Hence why 
the banks are modelled as groups.

Thanks,
Robin.

> This updates the driver to also support unmanaged iommu domain if
> MTK_UNMANAGED_DEVICE is set in the iommu id.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>   drivers/iommu/mtk_iommu.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2badd6acfb23d..5884a1ad409a0 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -858,9 +858,15 @@ static int mtk_iommu_get_group_id(struct device *dev, const struct mtk_iommu_pla
>   static struct iommu_group *mtk_iommu_device_group(struct device *dev)
>   {
>   	struct mtk_iommu_data *c_data = dev_iommu_priv_get(dev), *data;
> +	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>   	struct list_head *hw_list = c_data->hw_list;
>   	struct iommu_group *group;
>   	int groupid;
> +	int i;
> +
> +	for (i = 0; i < fwspec->num_ids; i++)
> +		if (MTK_M4U_UNMANAGED(fwspec->ids[i]))
> +			return iommu_group_alloc();
>   
>   	data = mtk_iommu_get_frst_data(hw_list);
>   	if (!data)

Re: [PATCH 3/3] dt-bindings: iommu: memory: Use unmanaged iommu domain for the APU

[Alexandre Mergnat]

On 30/01/2023 11:27, Alexandre Bailon wrote:
> This updates the iommu id to use unmanaged iommu domain for the APU.
> This is required by remoteproc to use the iommu.
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>

Re: [PATCH 3/3] dt-bindings: iommu: memory: Use unmanaged iommu domain for the APU

[Rob Herring]

On Mon, Jan 30, 2023 at 11:27:22AM +0100, Alexandre Bailon wrote:
> This updates the iommu id to use unmanaged iommu domain for the APU.
> This is required by remoteproc to use the iommu.

Explain how a mixture of DTs with or without this value changed would 
work with kernels with or without support for unmanaged domains? Looks 
like a compatibility problem to me.

> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  include/dt-bindings/memory/mediatek,mt8365-larb-port.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/dt-bindings/memory/mediatek,mt8365-larb-port.h b/include/dt-bindings/memory/mediatek,mt8365-larb-port.h
> index 56d5a5dd519e7..86d82a8f6017d 100644
> --- a/include/dt-bindings/memory/mediatek,mt8365-larb-port.h
> +++ b/include/dt-bindings/memory/mediatek,mt8365-larb-port.h
> @@ -24,8 +24,8 @@
>  #define M4U_PORT_MDP_WROT0		MTK_M4U_ID(M4U_LARB0_ID, 7)
>  #define M4U_PORT_MDP_RDMA1		MTK_M4U_ID(M4U_LARB0_ID, 8)
>  #define M4U_PORT_DISP_FAKE0		MTK_M4U_ID(M4U_LARB0_ID, 9)
> -#define M4U_PORT_APU_READ		MTK_M4U_ID(M4U_LARB0_ID, 10)
> -#define M4U_PORT_APU_WRITE		MTK_M4U_ID(M4U_LARB0_ID, 11)
> +#define M4U_PORT_APU_READ		MTK_M4U_UNMANAGED_ID(M4U_LARB0_ID, 10)
> +#define M4U_PORT_APU_WRITE		MTK_M4U_UNMANAGED_ID(M4U_LARB0_ID, 11)
>  
>  /* larb1 */
>  #define M4U_PORT_VENC_RCPU		MTK_M4U_ID(M4U_LARB1_ID, 0)
> -- 
> 2.38.2
> 

Re: [PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu domain

[Alexandre Bailon]

Hi Robin

On 1/30/23 13:04, Robin Murphy wrote:
> On 2023-01-30 10:27, Alexandre Bailon wrote:
>> Currently, the driver can allocate an unmanaged iommu domain.
>> But, this only works for SoC having multiple bank or multiple iova 
>> region.
> 
> That is for good reason - there is only a single pagetable per bank, so 
> if there are multiple devices assigned to a single bank, they cannot 
> possibly be attached to different domains at the same time. Hence why 
> the banks are modelled as groups.
I understand.
I am trying to upstream a remoteproc driver but the remote processor is
behind the iommu.
remoteproc can manage the iommu but it requires an unmanaged domain.
I tried a couple of things but this cause code duplication,
implies many hacks and not always reliable.
Do you have any suggestion ?

Thanks,
Alexandre
> 
> Thanks,
> Robin.
> 
>> This updates the driver to also support unmanaged iommu domain if
>> MTK_UNMANAGED_DEVICE is set in the iommu id.
>>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>>   drivers/iommu/mtk_iommu.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
>> index 2badd6acfb23d..5884a1ad409a0 100644
>> --- a/drivers/iommu/mtk_iommu.c
>> +++ b/drivers/iommu/mtk_iommu.c
>> @@ -858,9 +858,15 @@ static int mtk_iommu_get_group_id(struct device 
>> *dev, const struct mtk_iommu_pla
>>   static struct iommu_group *mtk_iommu_device_group(struct device *dev)
>>   {
>>       struct mtk_iommu_data *c_data = dev_iommu_priv_get(dev), *data;
>> +    struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>>       struct list_head *hw_list = c_data->hw_list;
>>       struct iommu_group *group;
>>       int groupid;
>> +    int i;
>> +
>> +    for (i = 0; i < fwspec->num_ids; i++)
>> +        if (MTK_M4U_UNMANAGED(fwspec->ids[i]))
>> +            return iommu_group_alloc();
>>       data = mtk_iommu_get_frst_data(hw_list);
>>       if (!data)

Re: [PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu domain

[Robin Murphy]

On 31/01/2023 1:08 pm, Alexandre Bailon wrote:
> Hi Robin
> 
> On 1/30/23 13:04, Robin Murphy wrote:
>> On 2023-01-30 10:27, Alexandre Bailon wrote:
>>> Currently, the driver can allocate an unmanaged iommu domain.
>>> But, this only works for SoC having multiple bank or multiple iova 
>>> region.
>>
>> That is for good reason - there is only a single pagetable per bank, 
>> so if there are multiple devices assigned to a single bank, they 
>> cannot possibly be attached to different domains at the same time. 
>> Hence why the banks are modelled as groups.
> I understand.
> I am trying to upstream a remoteproc driver but the remote processor is
> behind the iommu.
> remoteproc can manage the iommu but it requires an unmanaged domain.
> I tried a couple of things but this cause code duplication,
> implies many hacks and not always reliable.
> Do you have any suggestion ?

If there are other active devices behind the same IOMMU, and the 
remoteproc device cannot be isolated into its own bank using the 
existing IOMMU driver logic, then the remoteproc driver cannot manage 
the IOMMU directly, and must just use the regular DMA API. There's no 
way around it; you can't have two different parts of the kernel both 
thinking they have exclusive control of a single IOMMU address space at 
the same time. Similarly, remoteproc also cannot take explicit control 
of a multi-device group if it's not actually in control of the other 
devices, since their drivers will not be expecting the DMA address space 
to suddenly change underfoot - that's why iommu_attach_device() has the 
check which you presumably ran into.

Thanks,
Robin.

Re: [PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu domain

[Alexandre Bailon]

On 1/31/23 15:15, Robin Murphy wrote:
> On 31/01/2023 1:08 pm, Alexandre Bailon wrote:
>> Hi Robin
>>
>> On 1/30/23 13:04, Robin Murphy wrote:
>>> On 2023-01-30 10:27, Alexandre Bailon wrote:
>>>> Currently, the driver can allocate an unmanaged iommu domain.
>>>> But, this only works for SoC having multiple bank or multiple iova 
>>>> region.
>>>
>>> That is for good reason - there is only a single pagetable per bank, 
>>> so if there are multiple devices assigned to a single bank, they 
>>> cannot possibly be attached to different domains at the same time. 
>>> Hence why the banks are modelled as groups.
>> I understand.
>> I am trying to upstream a remoteproc driver but the remote processor is
>> behind the iommu.
>> remoteproc can manage the iommu but it requires an unmanaged domain.
>> I tried a couple of things but this cause code duplication,
>> implies many hacks and not always reliable.
>> Do you have any suggestion ?
> 
> If there are other active devices behind the same IOMMU, and the 
> remoteproc device cannot be isolated into its own bank using the 
> existing IOMMU driver logic, then the remoteproc driver cannot manage 
> the IOMMU directly, and must just use the regular DMA API. There's no 
> way around it; you can't have two different parts of the kernel both 
> thinking they have exclusive control of a single IOMMU address space at 
> the same time. Similarly, remoteproc also cannot take explicit control 
> of a multi-device group if it's not actually in control of the other 
> devices, since their drivers will not be expecting the DMA address space 
> to suddenly change underfoot - that's why iommu_attach_device() has the 
> check which you presumably ran into.
Unfortunately, we can't just use the regular DMA API.
Basically, the firmware use static addresses (and the remote core is 
only supposed to access addresses between 0x60000000 and 0x70000000).
When we use DMA API, we get a random address that doesn't match what the
firmware would expect.
remoteproc use directly the iommu API to map physical address to the
static address expected by the firmware when DMA API can't be use.

Thanks,
Alexandre

Re: [PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu domain

[Yong Wu (吴勇)]

On Tue, 2023-01-31 at 16:31 +0100, Alexandre Bailon wrote:
> 
> On 1/31/23 15:15, Robin Murphy wrote:
> > On 31/01/2023 1:08 pm, Alexandre Bailon wrote:
> > > Hi Robin
> > > 
> > > On 1/30/23 13:04, Robin Murphy wrote:
> > > > On 2023-01-30 10:27, Alexandre Bailon wrote:
> > > > > Currently, the driver can allocate an unmanaged iommu domain.
> > > > > But, this only works for SoC having multiple bank or multiple
> > > > > iova 
> > > > > region.
> > > > 
> > > > That is for good reason - there is only a single pagetable per
> > > > bank, 
> > > > so if there are multiple devices assigned to a single bank,
> > > > they 
> > > > cannot possibly be attached to different domains at the same
> > > > time. 
> > > > Hence why the banks are modelled as groups.
> > > 
> > > I understand.
> > > I am trying to upstream a remoteproc driver but the remote
> > > processor is
> > > behind the iommu.
> > > remoteproc can manage the iommu but it requires an unmanaged
> > > domain.
> > > I tried a couple of things but this cause code duplication,
> > > implies many hacks and not always reliable.
> > > Do you have any suggestion ?
> > 
> > If there are other active devices behind the same IOMMU, and the 
> > remoteproc device cannot be isolated into its own bank using the 
> > existing IOMMU driver logic, then the remoteproc driver cannot
> > manage 
> > the IOMMU directly, and must just use the regular DMA API. There's
> > no 
> > way around it; you can't have two different parts of the kernel
> > both 
> > thinking they have exclusive control of a single IOMMU address
> > space at 
> > the same time. Similarly, remoteproc also cannot take explicit
> > control 
> > of a multi-device group if it's not actually in control of the
> > other 
> > devices, since their drivers will not be expecting the DMA address
> > space 
> > to suddenly change underfoot - that's why iommu_attach_device() has
> > the 
> > check which you presumably ran into.
> 
> Unfortunately, we can't just use the regular DMA API.
> Basically, the firmware use static addresses (and the remote core is 
> only supposed to access addresses between 0x60000000 and 0x70000000).
> When we use DMA API, we get a random address that doesn't match what
> the
> firmware would expect.
> remoteproc use directly the iommu API to map physical address to the
> static address expected by the firmware when DMA API can't be use.

If this master can only support this special address, We could handle
it inside this driver.

Could you help try to add these two patches [3/11] and [4/11]?

[3/11] 
https://patchwork.kernel.org/project/linux-mediatek/patch/20230214031114.926-4-yong.wu@mediatek.com/
[4/11] 
https://patchwork.kernel.org/project/linux-mediatek/patch/20230214031114.926-5-yong.wu@mediatek.com/


and, then add the logical for mt8365(I see the APU is larb0 port10/11
in the binding):
--------------------------------

+#define MT8365_REGION_NR		2
+
+static const struct mtk_iommu_iova_region
mt8365_multi_rgn[MT8365_REGION_NR] = {
+	{ .iova_base = 0x0,		.size = SZ_4G},	       /* 0 ~
4G. */
+	{ .iova_base = 0x60000000,	.size = SZ_256M},      /* APU
*/
+};
+

xxxxxxxxxxx

+static const unsigned int
mt8365_larb_region_msk[MT8365_REGION_NR][MTK_LARB_NR_MAX] = {
+	[0] = {~(u32)(BIT(10) | BIT(11)), ~0, ~0, ~0, ~0, ~0},
+	[1] = {[0] = BIT(10) | BIT(11)},
+};
+
 static const struct mtk_iommu_plat_data mt8365_data = {
 	.m4u_plat	= M4U_MT8365,
 	.flags		= RESET_AXI | INT_ID_PORT_WIDTH_6,
 	.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),
+	.iova_region	= mt8365_multi_rgn,
+	.iova_region_nr	= ARRAY_SIZE(mt8365_multi_rgn),
+	.iova_region_larb_msk = mt8365_larb_region_msk,
 	.larbid_remap	= {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear
mapping. */
 };
--------------------------------

After that, If we call DMA API with the device whose dtsi has
M4U_PORT_APU_READ/M4U_PORT_APU_WRITE. The iova should be located at
that special address. Sorry, I have no board to test.

Thanks. 

> 
> Thanks,
> Alexandre
> 

Re: [PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu domain

[Alexandre Bailon]

Hi,

Sorry for long delay before the response.

On 2/14/23 06:48, Yong Wu (吴勇) wrote:
> On Tue, 2023-01-31 at 16:31 +0100, Alexandre Bailon wrote:
>>
>> On 1/31/23 15:15, Robin Murphy wrote:
>>> On 31/01/2023 1:08 pm, Alexandre Bailon wrote:
>>>> Hi Robin
>>>>
>>>> On 1/30/23 13:04, Robin Murphy wrote:
>>>>> On 2023-01-30 10:27, Alexandre Bailon wrote:
>>>>>> Currently, the driver can allocate an unmanaged iommu domain.
>>>>>> But, this only works for SoC having multiple bank or multiple
>>>>>> iova
>>>>>> region.
>>>>>
>>>>> That is for good reason - there is only a single pagetable per
>>>>> bank,
>>>>> so if there are multiple devices assigned to a single bank,
>>>>> they
>>>>> cannot possibly be attached to different domains at the same
>>>>> time.
>>>>> Hence why the banks are modelled as groups.
>>>>
>>>> I understand.
>>>> I am trying to upstream a remoteproc driver but the remote
>>>> processor is
>>>> behind the iommu.
>>>> remoteproc can manage the iommu but it requires an unmanaged
>>>> domain.
>>>> I tried a couple of things but this cause code duplication,
>>>> implies many hacks and not always reliable.
>>>> Do you have any suggestion ?
>>>
>>> If there are other active devices behind the same IOMMU, and the
>>> remoteproc device cannot be isolated into its own bank using the
>>> existing IOMMU driver logic, then the remoteproc driver cannot
>>> manage
>>> the IOMMU directly, and must just use the regular DMA API. There's
>>> no
>>> way around it; you can't have two different parts of the kernel
>>> both
>>> thinking they have exclusive control of a single IOMMU address
>>> space at
>>> the same time. Similarly, remoteproc also cannot take explicit
>>> control
>>> of a multi-device group if it's not actually in control of the
>>> other
>>> devices, since their drivers will not be expecting the DMA address
>>> space
>>> to suddenly change underfoot - that's why iommu_attach_device() has
>>> the
>>> check which you presumably ran into.
>>
>> Unfortunately, we can't just use the regular DMA API.
>> Basically, the firmware use static addresses (and the remote core is
>> only supposed to access addresses between 0x60000000 and 0x70000000).
>> When we use DMA API, we get a random address that doesn't match what
>> the
>> firmware would expect.
>> remoteproc use directly the iommu API to map physical address to the
>> static address expected by the firmware when DMA API can't be use.
> 
> If this master can only support this special address, We could handle
> it inside this driver.
> 
> Could you help try to add these two patches [3/11] and [4/11]?
> 
> [3/11]
> https://patchwork.kernel.org/project/linux-mediatek/patch/20230214031114.926-4-yong.wu@mediatek.com/
> [4/11]
> https://patchwork.kernel.org/project/linux-mediatek/patch/20230214031114.926-5-yong.wu@mediatek.com/
> 
> 
> and, then add the logical for mt8365(I see the APU is larb0 port10/11
> in the binding):
> --------------------------------
> 
> +#define MT8365_REGION_NR		2
> +
> +static const struct mtk_iommu_iova_region
> mt8365_multi_rgn[MT8365_REGION_NR] = {
> +	{ .iova_base = 0x0,		.size = SZ_4G},	       /* 0 ~
> 4G. */
> +	{ .iova_base = 0x60000000,	.size = SZ_256M},      /* APU
> */
> +};
> +
> 
> xxxxxxxxxxx
> 
> +static const unsigned int
> mt8365_larb_region_msk[MT8365_REGION_NR][MTK_LARB_NR_MAX] = {
> +	[0] = {~(u32)(BIT(10) | BIT(11)), ~0, ~0, ~0, ~0, ~0},
> +	[1] = {[0] = BIT(10) | BIT(11)},
> +};
> +
>   static const struct mtk_iommu_plat_data mt8365_data = {
>   	.m4u_plat	= M4U_MT8365,
>   	.flags		= RESET_AXI | INT_ID_PORT_WIDTH_6,
>   	.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),
> +	.iova_region	= mt8365_multi_rgn,
> +	.iova_region_nr	= ARRAY_SIZE(mt8365_multi_rgn),
> +	.iova_region_larb_msk = mt8365_larb_region_msk,
>   	.larbid_remap	= {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear
> mapping. */
>   };
> --------------------------------
> 
> After that, If we call DMA API with the device whose dtsi has
> M4U_PORT_APU_READ/M4U_PORT_APU_WRITE. The iova should be located at
> that special address. Sorry, I have no board to test.
> 

I have not yet tested the patches but it will only address one part of 
the problem.
Using your patches, I could allocate some shared memory using DMA API 
but the main issue still remain.
The firmware is not relocatable at all. So, once the firmware is built, 
it is expected to be loaded at a specific address.
Remoteproc framework support this use case. Using the resource table, 
the firmware expose to remoteproc what device address is expect and 
remoteproc manually call iommu_map to satisfy this requirement.
Using DMA_API, I could allocate the memory to load the firmware but I 
could not be sure that the DMA address will be the one expected by firmware.

Thanks,
Alexandre

Re: [PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu domain

[Robin Murphy]

On 2023-09-14 09:07, Alexandre Bailon wrote:
> Hi,
> 
> Sorry for long delay before the response.
> 
> On 2/14/23 06:48, Yong Wu (吴勇) wrote:
>> On Tue, 2023-01-31 at 16:31 +0100, Alexandre Bailon wrote:
>>>
>>> On 1/31/23 15:15, Robin Murphy wrote:
>>>> On 31/01/2023 1:08 pm, Alexandre Bailon wrote:
>>>>> Hi Robin
>>>>>
>>>>> On 1/30/23 13:04, Robin Murphy wrote:
>>>>>> On 2023-01-30 10:27, Alexandre Bailon wrote:
>>>>>>> Currently, the driver can allocate an unmanaged iommu domain.
>>>>>>> But, this only works for SoC having multiple bank or multiple
>>>>>>> iova
>>>>>>> region.
>>>>>>
>>>>>> That is for good reason - there is only a single pagetable per
>>>>>> bank,
>>>>>> so if there are multiple devices assigned to a single bank,
>>>>>> they
>>>>>> cannot possibly be attached to different domains at the same
>>>>>> time.
>>>>>> Hence why the banks are modelled as groups.
>>>>>
>>>>> I understand.
>>>>> I am trying to upstream a remoteproc driver but the remote
>>>>> processor is
>>>>> behind the iommu.
>>>>> remoteproc can manage the iommu but it requires an unmanaged
>>>>> domain.
>>>>> I tried a couple of things but this cause code duplication,
>>>>> implies many hacks and not always reliable.
>>>>> Do you have any suggestion ?
>>>>
>>>> If there are other active devices behind the same IOMMU, and the
>>>> remoteproc device cannot be isolated into its own bank using the
>>>> existing IOMMU driver logic, then the remoteproc driver cannot
>>>> manage
>>>> the IOMMU directly, and must just use the regular DMA API. There's
>>>> no
>>>> way around it; you can't have two different parts of the kernel
>>>> both
>>>> thinking they have exclusive control of a single IOMMU address
>>>> space at
>>>> the same time. Similarly, remoteproc also cannot take explicit
>>>> control
>>>> of a multi-device group if it's not actually in control of the
>>>> other
>>>> devices, since their drivers will not be expecting the DMA address
>>>> space
>>>> to suddenly change underfoot - that's why iommu_attach_device() has
>>>> the
>>>> check which you presumably ran into.
>>>
>>> Unfortunately, we can't just use the regular DMA API.
>>> Basically, the firmware use static addresses (and the remote core is
>>> only supposed to access addresses between 0x60000000 and 0x70000000).
>>> When we use DMA API, we get a random address that doesn't match what
>>> the
>>> firmware would expect.
>>> remoteproc use directly the iommu API to map physical address to the
>>> static address expected by the firmware when DMA API can't be use.
>>
>> If this master can only support this special address, We could handle
>> it inside this driver.
>>
>> Could you help try to add these two patches [3/11] and [4/11]?
>>
>> [3/11]
>> https://patchwork.kernel.org/project/linux-mediatek/patch/20230214031114.926-4-yong.wu@mediatek.com/
>> [4/11]
>> https://patchwork.kernel.org/project/linux-mediatek/patch/20230214031114.926-5-yong.wu@mediatek.com/
>>
>>
>> and, then add the logical for mt8365(I see the APU is larb0 port10/11
>> in the binding):
>> --------------------------------
>>
>> +#define MT8365_REGION_NR        2
>> +
>> +static const struct mtk_iommu_iova_region
>> mt8365_multi_rgn[MT8365_REGION_NR] = {
>> +    { .iova_base = 0x0,        .size = SZ_4G},           /* 0 ~
>> 4G. */
>> +    { .iova_base = 0x60000000,    .size = SZ_256M},      /* APU
>> */
>> +};
>> +
>>
>> xxxxxxxxxxx
>>
>> +static const unsigned int
>> mt8365_larb_region_msk[MT8365_REGION_NR][MTK_LARB_NR_MAX] = {
>> +    [0] = {~(u32)(BIT(10) | BIT(11)), ~0, ~0, ~0, ~0, ~0},
>> +    [1] = {[0] = BIT(10) | BIT(11)},
>> +};
>> +
>>   static const struct mtk_iommu_plat_data mt8365_data = {
>>       .m4u_plat    = M4U_MT8365,
>>       .flags        = RESET_AXI | INT_ID_PORT_WIDTH_6,
>>       .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),
>> +    .iova_region    = mt8365_multi_rgn,
>> +    .iova_region_nr    = ARRAY_SIZE(mt8365_multi_rgn),
>> +    .iova_region_larb_msk = mt8365_larb_region_msk,
>>       .larbid_remap    = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear
>> mapping. */
>>   };
>> --------------------------------
>>
>> After that, If we call DMA API with the device whose dtsi has
>> M4U_PORT_APU_READ/M4U_PORT_APU_WRITE. The iova should be located at
>> that special address. Sorry, I have no board to test.
>>
> 
> I have not yet tested the patches but it will only address one part of 
> the problem.
> Using your patches, I could allocate some shared memory using DMA API 
> but the main issue still remain.
> The firmware is not relocatable at all. So, once the firmware is built, 
> it is expected to be loaded at a specific address.
> Remoteproc framework support this use case. Using the resource table, 
> the firmware expose to remoteproc what device address is expect and 
> remoteproc manually call iommu_map to satisfy this requirement.
> Using DMA_API, I could allocate the memory to load the firmware but I 
> could not be sure that the DMA address will be the one expected by 
> firmware.

I think the solution to this is now to use the new DT binding to require 
an IOMMU identity mapping of the firmware region[1], such that it's all 
taken care of for you.

Thanks,
Robin.

[1] https://git.kernel.org/torvalds/c/af0d81357cc5