linux-kernel - Re: [PATCH v3 11/24] iommu/io-pgtable-arm-v7s: Quad lvl1 pgtable for MediaTek

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]

Message-ID: <1603698334.26323.88.camel@mhfsdcap03>
Date:   Mon, 26 Oct 2020 15:45:34 +0800
From:   Yong Wu <yong.wu@...iatek.com>
To:     Will Deacon <will@...nel.org>
CC:     Joerg Roedel <joro@...tes.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Evan Green <evgreen@...omium.org>,
        Tomasz Figa <tfiga@...gle.com>,
        <linux-mediatek@...ts.infradead.org>,
        <srv_heupstream@...iatek.com>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <iommu@...ts.linux-foundation.org>, <youlin.pei@...iatek.com>,
        Nicolas Boichat <drinkcat@...omium.org>,
        <anan.sun@...iatek.com>, <chao.hao@...iatek.com>,
        <ming-fan.chen@...iatek.com>,
        Greg Kroah-Hartman <gregkh@...gle.com>,
        <kernel-team@...roid.com>
Subject: Re: [PATCH v3 11/24] iommu/io-pgtable-arm-v7s: Quad lvl1 pgtable
 for MediaTek

On Fri, 2020-10-23 at 12:21 +0100, Will Deacon wrote:
> On Wed, Sep 30, 2020 at 03:06:34PM +0800, Yong Wu wrote:
> > The standard input iova bits is 32. MediaTek quad the lvl1 pagetable
> > (4 * lvl1). No change for lvl2 pagetable. Then the iova bits can reach
> > 34bit.
> > 
> > Signed-off-by: Yong Wu <yong.wu@...iatek.com>
> > ---
> >  drivers/iommu/io-pgtable-arm-v7s.c | 13 ++++++++++---
> >  drivers/iommu/mtk_iommu.c          |  2 +-
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
> > index 8362fdf76657..306bae2755ed 100644
> > --- a/drivers/iommu/io-pgtable-arm-v7s.c
> > +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> > @@ -50,10 +50,17 @@
> >   */
> >  #define ARM_V7S_ADDR_BITS		32
> 
> If we rename this to _ARM_V7S_ADDR_BITS can we then have ARM_V7S_ADDR_BITS
> take a cfg parameter and move the arm_v7s_is_mtk_enabled() check in there?
> Same for _ARM_V7S_LVL_BITS.
> 
> That would avoid scattering arm_v7s_is_mtk_enabled() into all the users.

I added "cfg" for _ARM_V7S_LVL_BITS in Robin's mail. is that ok?

Regarding ARM_V7S_ADDR_BITS, I'd like to keep it as is(Don't add cfg),
this macro only is used in ARM_V7S_LVL_SHIFT and checking the value of
ias/oas.

a) ARM_V7S_LVL_SHIFT always expect ARM_V7S_ADDR_BITS is 32.

b) our ias/oas is different(ias is 34 while oas is 35). If we define a
new macro, we need two about this, like:
#define ARM_V7S_IADDR_BITS(cfg)	(!arm_v7s_is_mtk_enabled(cfg) ? 32 : 34)
#define ARM_V7S_OADDR_BITS(cfg)	(!arm_v7s_is_mtk_enabled(cfg) ? 32 : 35)
and the two will only are used in the checking of ias/oas.

thus it looks unnecessary?

> 
> Will