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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 14 Feb 2023 05:48:07 +0000
From:   Yong Wu (吴勇) <Yong.Wu@...iatek.com>
To:     "robin.murphy@....com" <robin.murphy@....com>,
        "joro@...tes.org" <joro@...tes.org>,
        "will@...nel.org" <will@...nel.org>,
        "abailon@...libre.com" <abailon@...libre.com>
CC:     "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>
Subject: Re: [PATCH 2/3] iommu: mediatek: Add support of unmanaged iommu
 domain

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
>