Re: [PATCH v8 07/21] iommu/io-pgtable-arm-v7s: Extend MediaTek 4GB Mode
From: Will Deacon <hidden>
Date: 2019-07-15 09:52:07
Also in:
linux-arm-kernel, linux-iommu, linux-mediatek, lkml
On Sun, Jul 14, 2019 at 12:41:20PM +0800, Yong Wu wrote:
On Thu, 2019-07-11 at 13:31 +0100, Will Deacon wrote:quoted
This looks like the right sort of idea. Basically, I was thinking that you can use the oas in conjunction with the quirk to specify whether or not your two magic bits should be set. You could also then cap the oas using the size of phys_addr_t to deal with my other comment. Finally, I was hoping you could drop the |= BIT_ULL(32) and the &= ~BIT_ULL(32) bits of the mtk driver if the pgtable code now accepts higher addresses. Did that not work out?After the current patch, the pgtable has accepted the higher address. the " |= BIT_ULL(32)" and "& = ~ BIT_ULL(32)" is for a special case(we call it 4GB mode). Now MediaTek IOMMU support 2 kind memory: 1) normal case: PA is 0x4000_0000 - 0x3_ffff_ffff. the PA won't be remapped. mt8183 and the non-4GB mode of mt8173/mt2712 use this mode. 2) 4GB Mode: PA is 0x4000_0000 - 0x1_3fff_ffff. But the PA will remapped to 0x1_0000_0000 to 0x1_ffff_ffff. This is for the 4GB mode of mt8173/mt2712. This case is so special that we should change the PA manually(add bit32). (mt2712 and mt8173 have both mode: 4GB and non-4GB.) If we try to use oas and our quirk to cover this two case. Then I can use "oas == 33" only for this 4GB mode. and "oas == 34" for the normal case even though the PA mayn't reach 34bit. Also I should add some "workaround" for the 4GB mode(oas==33). I copy the new patch in the mail below(have dropped the "|= BIT_ULL(32)" and the "&= ~BIT_ULL(32)) in mtk iommu". please help have a look if it is ok. (another thing: Current the PA can support over 4GB. So the quirk name "MTK_4GB" looks not suitable, I used a new patch rename to "MTK_EXT").
Makes sense, thanks. One comment below.
quoted hunk ↗ jump to hunk
@@ -205,7 +216,20 @@ static phys_addr_t iopte_to_paddr(arm_v7s_ioptepte, int lvl, else mask = ARM_V7S_LVL_MASK(lvl); - return pte & mask; + paddr = pte & mask; + if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT) && + (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)) { + /* + * Workaround for MTK 4GB Mode: + * Add BIT32 only when PA < 0x4000_0000. + */ + if ((cfg->oas == 33 && paddr < 0x40000000UL) || + (cfg->oas > 33 && (pte & ARM_V7S_ATTR_MTK_PA_BIT32))) + paddr |= BIT_ULL(32); + if (pte & ARM_V7S_ATTR_MTK_PA_BIT33) + paddr |= BIT_ULL(33); + } + return paddr; } static arm_v7s_iopte *iopte_deref(arm_v7s_iopte pte, int lvl,@@ -326,9 +350,6 @@ static arm_v7s_iopte arm_v7s_prot_to_pte(int prot,int lvl, if (lvl == 1 && (cfg->quirks & IO_PGTABLE_QUIRK_ARM_NS)) pte |= ARM_V7S_ATTR_NS_SECTION; - if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) - pte |= ARM_V7S_ATTR_MTK_4GB; - return pte; }@@ -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?
Will