From: Yong Wu <yong.wu@mediatek.com>
To: Will Deacon <will@kernel.org>
Cc: <youlin.pei@mediatek.com>, <devicetree@vger.kernel.org>,
Nicolas Boichat <drinkcat@chromium.org>, <cui.zhang@mediatek.com>,
<srv_heupstream@mediatek.com>, <chao.hao@mediatek.com>,
Joerg Roedel <joro@8bytes.org>, Will Deacon <will.deacon@arm.com>,
<linux-kernel@vger.kernel.org>, Evan Green <evgreen@chromium.org>,
"Tomasz Figa" <tfiga@google.com>,
<iommu@lists.linux-foundation.org>,
Rob Herring <robh+dt@kernel.org>,
<linux-mediatek@lists.infradead.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
<yingjoe.chen@mediatek.com>, <anan.sun@mediatek.com>,
Robin Murphy <robin.murphy@arm.com>,
"Matthias Kaehlcke" <mka@chromium.org>,
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v8 07/21] iommu/io-pgtable-arm-v7s: Extend MediaTek 4GB Mode
Date: Thu, 18 Jul 2019 13:37:23 +0800 [thread overview]
Message-ID: <1563428243.31342.39.camel@mhfsdcap03> (raw)
In-Reply-To: <20190717142339.wltamw6wktwixqqn@willie-the-truck>
On Wed, 2019-07-17 at 15:23 +0100, Will Deacon wrote:
> On Wed, Jul 17, 2019 at 08:44:19PM +0800, Yong Wu wrote:
> > On Mon, 2019-07-15 at 10:51 +0100, Will Deacon wrote:
> > > On Sun, Jul 14, 2019 at 12:41:20PM +0800, Yong Wu wrote:
> > > > @@ -742,7 +763,9 @@ static struct io_pgtable
> > > > *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> > > > {
> > > > struct arm_v7s_io_pgtable *data;
> > > >
> > > > - if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> > > > + if (cfg->ias > ARM_V7S_ADDR_BITS ||
> > > > + (cfg->oas > ARM_V7S_ADDR_BITS &&
> > > > + !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> > > > return NULL;
> > >
> > > I think you can rework this to do something like:
> > >
> > > if (cfg->ias > ARM_V7S_ADDR_BITS)
> > > return NULL;
> > >
> > > if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
> > > if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
> > > cfg->oas = min(cfg->oas, ARM_V7S_ADDR_BITS);
> > > else if (cfg->oas > 34)
> > > return NULL;
> > > } else if (cfg->oas > ARM_V7S_ADDR_BITS) {
> > > return NULL;
> > > }
> > >
> > > so that we clamp the oas when phys_addr_t is 32-bit for you. That should
> > > allow you to remove lots of the checking from iopte_to_paddr() too if you
> > > check against oas in the map() function.
> > >
> > > Does that make sense?
> >
> > Of course I'm ok for this. I'm only afraid that this function has
> > already 3 checking "if (x) return NULL", Here we add a new one and so
> > many lines... Maybe the user should guarantee the right value of oas.
> > How about move it into mtk_iommu.c?
> >
> > About the checking of iopte_to_paddr, I can not remove them. I know it
> > may be a bit special and not readable. Hmm, I guess I should use a MACRO
> > instead of the hard code 33 for the special 4GB mode case.
>
> Why can't you just do something like:
>
> if (!(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT))
> return paddr;
>
> if (pte & ARM_V7S_ATTR_MTK_PA_BIT33)
> paddr |= BIT_ULL(33);
OK here.
>
> if (pte & ARM_V&S_ATTR_MTK_PA_BIT32)
> paddr |= BIT_ULL(32);
No here, The flow is a bit special for 4GB mode here.
This is the detailed remap relationship for our 4GB mode.
CPU PA -> HW PA
register: 0x0 ~ 0x3fff_ffff
dram 1G:0x4000_0000~0x7fff_ffff ->0x1_4000_0000~0x1_7fff_ffff(Add bit32)
dram 2G:0x8000_0000~0xbfff_ffff ->0x1_8000_0000~0x1_bfff_ffff(Add bit32)
dram 3G:0xc000_0000~0xffff_ffff ->0x1_c000_0000~0x1_ffff_ffff(Add bit32)
dram 4G:0x1_0000_0000~0x1_3fff_ffff->0x1_0000_0000~0x1_3fff_ffff
Thus, in the 4GB mode, we should add always add bit9 in pte(for bit32
PA). But we can not always add bit32 in the iova_to_phys. The valid PA
range should be 0x4000_0000 - 0x1_3fff_ffff. Thus, we can only add bit32
when the PA in pte < 0x4000_0000, keep it as-is if the PA in pte located
from 0x4000_0000 to 0xffff_ffff.
This issue exist all the time after we added 4GB mode for mt8173.
Thus, I have to add a special flow for 4gb mode here:
/* Workaround for MTK 4GB Mode: Add BIT32 only when PA < 0x4000_0000.*/
if (cfg->oas == ARM_V7S_MTK_4GB_OAS && paddr < 0x40000000UL)
paddr |= BIT_ULL(32);
else if (pte & ARM_V7S_ATTR_MTK_PA_BIT32)
paddr |= BIT_ULL(32);
>
> return paddr;
>
> The diff I sent previously sanitises the oas at init time, and then you
> can just enforce it in map().
>
> Will
next prev parent reply other threads:[~2019-07-18 5:37 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-29 2:09 [PATCH v8 00/21] MT8183 IOMMU SUPPORT Yong Wu
2019-06-29 2:09 ` [PATCH v8 01/21] dt-bindings: mediatek: Add binding for mt8183 IOMMU and SMI Yong Wu
2019-06-29 2:09 ` [PATCH v8 02/21] iommu/mediatek: Use a struct as the platform data Yong Wu
2019-06-29 2:09 ` [PATCH v8 03/21] memory: mtk-smi: Use a general config_port interface Yong Wu
2019-06-29 2:09 ` [PATCH v8 04/21] memory: mtk-smi: Use a struct for the platform data for smi-common Yong Wu
2019-06-29 2:09 ` [PATCH v8 05/21] iommu/mediatek: Fix iova_to_phys PA start for 4GB mode Yong Wu
2019-06-29 2:09 ` [PATCH v8 06/21] iommu/io-pgtable-arm-v7s: Add paddr_to_iopte and iopte_to_paddr helpers Yong Wu
2019-06-29 2:09 ` [PATCH v8 07/21] iommu/io-pgtable-arm-v7s: Extend MediaTek 4GB Mode Yong Wu
2019-07-10 14:36 ` Will Deacon
2019-07-11 11:53 ` Yong Wu
2019-07-11 12:31 ` Will Deacon
2019-07-14 4:41 ` Yong Wu
2019-07-15 9:51 ` Will Deacon
2019-07-17 12:44 ` Yong Wu
2019-07-17 14:23 ` Will Deacon
2019-07-18 5:37 ` Yong Wu [this message]
2019-06-29 2:09 ` [PATCH v8 08/21] iommu/mediatek: Add bclk can be supported optionally Yong Wu
2019-06-29 2:09 ` [PATCH v8 09/21] iommu/mediatek: Add larb-id remapped support Yong Wu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1563428243.31342.39.camel@mhfsdcap03 \
--to=yong.wu@mediatek.com \
--cc=anan.sun@mediatek.com \
--cc=chao.hao@mediatek.com \
--cc=cui.zhang@mediatek.com \
--cc=devicetree@vger.kernel.org \
--cc=drinkcat@chromium.org \
--cc=evgreen@chromium.org \
--cc=iommu@lists.linux-foundation.org \
--cc=joro@8bytes.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=mka@chromium.org \
--cc=robh+dt@kernel.org \
--cc=robin.murphy@arm.com \
--cc=srv_heupstream@mediatek.com \
--cc=tfiga@google.com \
--cc=will.deacon@arm.com \
--cc=will@kernel.org \
--cc=yingjoe.chen@mediatek.com \
--cc=youlin.pei@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).