Messages in this thread Patch in this message | | | Date | Mon, 11 Sep 2023 11:22:56 +0200 | Subject | Re: [PATCH 1/4] iommu/mediatek: Initialise the secure bank | From | AngeloGioacchino Del Regno <> |
| |
Il 11/09/23 03:17, Yong Wu ha scritto: > The lastest IOMMU always have 5 banks, and we always use the last bank > (id:4) for the secure memory address translation. This patch add a new > flag (SECURE_BANK_ENABLE) for this feature. > > For the secure bank, its kernel va "base" is not helpful since the > secure bank registers has already been protected and can only be accessed > in the secure world. But we still record its register base, because we need > use it to determine which IOMMU HW the translation fault happen in the > secure world. > > Signed-off-by: Anan Sun <anan.sun@mediatek.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > --- > drivers/iommu/mtk_iommu.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c > index 640275873a27..4a2cffb28c61 100644 > --- a/drivers/iommu/mtk_iommu.c > +++ b/drivers/iommu/mtk_iommu.c > @@ -146,6 +146,7 @@ > #define TF_PORT_TO_ADDR_MT8173 BIT(18) > #define INT_ID_PORT_WIDTH_6 BIT(19) > #define CFG_IFA_MASTER_IN_ATF BIT(20) > +#define SECURE_BANK_ENABLE BIT(21) > > #define MTK_IOMMU_HAS_FLAG_MASK(pdata, _x, mask) \ > ((((pdata)->flags) & (mask)) == (_x)) > @@ -162,6 +163,8 @@ > #define MTK_IOMMU_GROUP_MAX 8 > #define MTK_IOMMU_BANK_MAX 5 > > +#define MTK_IOMMU_SEC_BANKID 4 > +
Is there any SoC (previous, current or future) that may have more than one secure context bank?
I'm thinking about implementing this differently...
static const struct mtk_iommu_plat_data mt8188_data_vdo = { .... .flags = ..flags.. | ATF_SECURE_BANKS_ENABLE .banks_num = 5, .banks_enable = {true, false, false, false, true}, .banks_secure = {false, false, false, false, true}, .... }
...this would means that you won't need to specify a static SEC_BANKID, as you'd get that from banks_secure... so that....
> enum mtk_iommu_plat { > M4U_MT2712, > M4U_MT6779, > @@ -240,9 +243,13 @@ struct mtk_iommu_plat_data { > }; > > struct mtk_iommu_bank_data { > - void __iomem *base; > + union { > + void __iomem *base; > + phys_addr_t sec_bank_base; > + }; > int irq; > u8 id; > + bool is_secure; > struct device *parent_dev; > struct mtk_iommu_data *parent_data; > spinlock_t tlb_lock; /* lock for tlb range flush */ > @@ -1309,7 +1316,15 @@ static int mtk_iommu_probe(struct platform_device *pdev) > continue; > bank = &data->bank[i]; > bank->id = i; > - bank->base = base + i * MTK_IOMMU_BANK_SZ;
....this would become:
bank->is_secure = MTK_IOMMU_HAS_FLAG(data->plat_data, ATF_SECURE_BANKS_ENABLE) && data->plat_data->banks_secure[i];
if (bank->is_secure) bank->sec_bank_base = res->start + i * MTK_IOMMU_BANK_SZ; else bank->base = base + i * MTK_IOMMU_BANK_SZ;
> + if (MTK_IOMMU_HAS_FLAG(data->plat_data, SECURE_BANK_ENABLE) && > + bank->id == MTK_IOMMU_SEC_BANKID) { > + /* Record the secure bank base to indicate which iommu TF in sec world */ > + bank->sec_bank_base = res->start + i * MTK_IOMMU_BANK_SZ; > + bank->is_secure = true; > + } else { > + bank->base = base + i * MTK_IOMMU_BANK_SZ; > + bank->is_secure = false; > + } > bank->m4u_dom = NULL; > > bank->irq = platform_get_irq(pdev, i);
What do you think?
Cheers, Angelo
| |