Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Will Deacon <will@kernel.org>
To: Yong Wu <yong.wu@mediatek.com>
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>,
	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>,
	ming-fan.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 v9 08/21] iommu/io-pgtable-arm-v7s: Extend MediaTek 4GB Mode
Date: Wed, 14 Aug 2019 15:41:00 +0100
Message-ID: <20190814144059.ruyc45yoqkwpbuga@willie-the-truck> (raw)
In-Reply-To: <1565423901-17008-9-git-send-email-yong.wu@mediatek.com>

Hi Yong Wu,

Sorry, but I'm still deeply confused by this patch.

On Sat, Aug 10, 2019 at 03:58:08PM +0800, Yong Wu wrote:
> MediaTek extend the arm v7s descriptor to support the dram over 4GB.
> 
> In the mt2712 and mt8173, it's called "4GB mode", the physical address
> is from 0x4000_0000 to 0x1_3fff_ffff, but from EMI point of view, it
> is remapped to high address from 0x1_0000_0000 to 0x1_ffff_ffff, the
> bit32 is always enabled. thus, in the M4U, we always enable the bit9
> for all PTEs which means to enable bit32 of physical address. Here is
> the detailed remap relationship in the "4GB mode":
> CPU PA         ->    HW PA
> 0x4000_0000          0x1_4000_0000 (Add bit32)
> 0x8000_0000          0x1_8000_0000 ...
> 0xc000_0000          0x1_c000_0000 ...
> 0x1_0000_0000        0x1_0000_0000 (No change)

So in this example, there are no PAs below 0x4000_0000 yet you later
add code to deal with that:

> +	/* 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);

Why? Mainline currently doesn't do anything like this for the "4gb mode"
support as far as I can tell. In fact, we currently unconditionally set
bit 32 in the physical address returned by iova_to_phys() which wouldn't
match your CPU PAs listed above, so I'm confused about how this is supposed
to work.

The way I would like this quirk to work is that the io-pgtable code
basically sets bit 9 in the pte when bit 32 is set in the physical address,
and sets bit 4 in the pte when bit 33 is set in the physical address. It
would then do the opposite when converting a pte to a physical address.

That way, your driver can call the page table code directly with the high
addresses and we don't have to do any manual offsetting or range checking
in the page table code.

Please can you explain to me why the diff below doesn't work on top of
this series? I'm happy to chat on IRC if you think it would be easier,
because I have a horrible feeling that we've been talking past each other
and I'd like to see this support merged for 5.4.

Will

--->8

diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
index ab12ef5f8b03..d8d84617c822 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -184,7 +184,7 @@ static arm_v7s_iopte paddr_to_iopte(phys_addr_t paddr, int lvl,
 	arm_v7s_iopte pte = paddr & ARM_V7S_LVL_MASK(lvl);
 
 	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
-		if ((paddr & BIT_ULL(32)) || cfg->oas == ARM_V7S_MTK_4GB_OAS)
+		if (paddr & BIT_ULL(32))
 			pte |= ARM_V7S_ATTR_MTK_PA_BIT32;
 		if (paddr & BIT_ULL(33))
 			pte |= ARM_V7S_ATTR_MTK_PA_BIT33;
@@ -206,17 +206,14 @@ static phys_addr_t iopte_to_paddr(arm_v7s_iopte pte, int lvl,
 		mask = ARM_V7S_LVL_MASK(lvl);
 
 	paddr = pte & mask;
-	if (cfg->oas == 32 || !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT))
-		return paddr;
 
-	if (pte & ARM_V7S_ATTR_MTK_PA_BIT33)
-		paddr |= BIT_ULL(33);
+	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
+		if (pte & ARM_V7S_ATTR_MTK_PA_BIT32)
+			paddr |= BIT_ULL(32);
+		if (pte & ARM_V7S_ATTR_MTK_PA_BIT33)
+			paddr |= BIT_ULL(33);
+	}
 
-	/* 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;
 }
 
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index d5b9454352fd..3ae54dedede0 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -286,7 +286,7 @@ static int mtk_iommu_domain_finalise(struct mtk_iommu_domain *dom)
 	if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
 		dom->cfg.oas = 32;
 	else if (data->enable_4GB)
-		dom->cfg.oas = ARM_V7S_MTK_4GB_OAS;
+		dom->cfg.oas = 33;
 	else
 		dom->cfg.oas = 34;
 
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index 27337395bd42..a2a52c349fe4 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -113,8 +113,6 @@ struct io_pgtable_cfg {
 	};
 };
 
-#define ARM_V7S_MTK_4GB_OAS			33
-
 /**
  * struct io_pgtable_ops - Page table manipulation API for IOMMU drivers.
  *

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-10  7:58 [PATCH v9 00/21] MT8183 IOMMU SUPPORT Yong Wu
2019-08-10  7:58 ` [PATCH v9 01/21] dt-bindings: mediatek: Add binding for mt8183 IOMMU and SMI Yong Wu
2019-08-10  7:58 ` [PATCH v9 02/21] iommu/mediatek: Use a struct as the platform data Yong Wu
2019-08-10  7:58 ` [PATCH v9 03/21] memory: mtk-smi: Use a general config_port interface Yong Wu
2019-08-10  7:58 ` [PATCH v9 04/21] memory: mtk-smi: Use a struct for the platform data for smi-common Yong Wu
2019-08-10  7:58 ` [PATCH v9 05/21] iommu/io-pgtable-arm-v7s: Add paddr_to_iopte and iopte_to_paddr helpers Yong Wu
2019-08-10  7:58 ` [PATCH v9 06/21] iommu/io-pgtable-arm-v7s: Use ias/oas to check the valid iova/pa Yong Wu
2019-08-10  7:58 ` [PATCH v9 07/21] iommu/io-pgtable-arm-v7s: Rename the quirk from MTK_4GB to MTK_EXT Yong Wu
2019-08-10  7:58 ` [PATCH v9 08/21] iommu/io-pgtable-arm-v7s: Extend MediaTek 4GB Mode Yong Wu
2019-08-14 14:41   ` Will Deacon [this message]
2019-08-15  8:47     ` Yong Wu
2019-08-15  9:51       ` Will Deacon
2019-08-15 10:03         ` Yong Wu
2019-08-15 10:04           ` Will Deacon
2019-08-15 10:18         ` Yong Wu
2019-08-15 11:50           ` Will Deacon
2019-08-16  7:22             ` Yong Wu
2019-08-19 11:24               ` Will Deacon
2019-08-10  7:58 ` [PATCH v9 09/21] iommu/mediatek: Add bclk can be supported optionally Yong Wu
2019-08-10  7:58 ` [PATCH v9 10/21] iommu/mediatek: Add larb-id remapped support Yong Wu
2019-08-10  7:58 ` [PATCH v9 11/21] iommu/mediatek: Refine protect memory definition Yong Wu
2019-08-10  7:58 ` [PATCH v9 12/21] iommu/mediatek: Move reset_axi into plat_data Yong Wu
2019-08-10  7:58 ` [PATCH v9 13/21] iommu/mediatek: Move vld_pa_rng " Yong Wu
2019-08-10  7:58 ` [PATCH v9 14/21] memory: mtk-smi: Add gals support Yong Wu
2019-08-10  7:58 ` [PATCH v9 15/21] iommu/mediatek: Add mt8183 IOMMU support Yong Wu
2019-08-10  7:58 ` [PATCH v9 16/21] iommu/mediatek: Add mmu1 support Yong Wu
2019-08-10  7:58 ` [PATCH v9 17/21] memory: mtk-smi: Invoke pm runtime_callback to enable clocks Yong Wu
2019-08-10  7:58 ` [PATCH v9 18/21] memory: mtk-smi: Add bus_sel for mt8183 Yong Wu
2019-08-10  7:58 ` [PATCH v9 19/21] iommu/mediatek: Fix VLD_PA_RNG register backup when suspend Yong Wu
2019-08-10  7:58 ` [PATCH v9 20/21] memory: mtk-smi: Get rid of need_larbid Yong Wu
2019-08-10  7:58 ` [PATCH v9 21/21] iommu/mediatek: Clean up struct mtk_smi_iommu Yong Wu
2019-08-14  8:18 ` [PATCH v9 00/21] MT8183 IOMMU SUPPORT Joerg Roedel
2019-08-14  8:24   ` Will Deacon

Reply instructions:

You may reply publically 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=20190814144059.ruyc45yoqkwpbuga@willie-the-truck \
    --to=will@kernel.org \
    --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=ming-fan.chen@mediatek.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=yong.wu@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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox