Re: [PATCH v9 2/7] iommu/mediatek: Fix two IOMMU share pagetable issue

["Yong Wu (吴勇)" <Yong.Wu@mediatek.com>]

On Fri, 2023-03-17 at 10:34 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/03/23 09:55, Yong Wu ha scritto:
> > From: "Chengci.Xu" <chengci.xu@mediatek.com>
> > 
> > Prepare for mt8188 to fix a two IOMMU HWs share pagetable issue.
> > 
> > We have two MM IOMMU HWs in mt8188, one is VPP-IOMMU, another is
> > VDO-IOMMU.
> > The 2 MM IOMMU HWs share pagetable don't work in this case:
> >   a) VPP-IOMMU probe firstly.
> >   b) VDO-IOMMU probe.
> >   c) The master for VDO-IOMMU probe (means frstdata is vpp-iommu).
> >   d) The master in another domain probe. No matter it is vdo or
> > vpp.
> > Then it still create a new pagetable in step d). The problem is
> > "frstdata->bank[0]->m4u_dom" was not initialized. Then when d)
> > enter, it
> > still create a new one.
> > 
> > In this patch, we create a new variable "share_dom" for this share
> > pgtable case, it should be helpful for readable. and put all the
> > share
> > pgtable logic in the mtk_iommu_domain_finalise.
> > 
> > In mt8195, the master of VPP-IOMMU probes before than VDO-IOMMU
> > from its dtsi node sequence, we don't see this issue in it. Prepare
> > for
> > mt8188.
> > 
> > Signed-off-by: Chengci.Xu <chengci.xu@mediatek.com>
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> 
> I'm not sure whether this is *not* a fix... if a specific platform
> wasn't
> affected, this may still be a logic mistake... to be cautious, I
> would
> still add a Fixes tag to this one.

I think you are right. If we need add the Fixes tag, it should fix this
one: 645b87c190c9 ("iommu/mediatek: Fix 2 HW sharing pgtable issue").

Before I thought the code flow was changed a lot. I added the bank
structure and removed the mtk_iommu.h, I'm a bit afraid that this fix
patch can not be applied clean, then it will introduce confuse when
applying to the previous version for the maintainers.

Meanwhile, After mt8195, mt8186/mt6795/m8365/6795 were merged in
upstream. All of them don't have this sharing case, thus I thought this
fix it is not so necessary.

What's your opinion? and should I send this one separately if I add the
fixes tag?


> Otherwise, I would reword the commit title and remove the "Fix" word,
> so
> that it would look something like...
> 
> iommu/mediatek: Prepare for pagetables domain sharing
> 
> I'm sorry for noticing that only at v9, but the previous versions had
> a bit
> more stuff to address and couldn't really focus on analyzing this
> one.
> 
> If there's anything I've misunderstood about this patch, please feel
> free
> to explain.
> 
> Regards,
> Angelo
>