linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/1] arm64/cpufeature: Support optionally disable MTE
@ 2021-07-30 14:49 yee.lee
  2021-07-30 14:49 ` [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line yee.lee
  0 siblings, 1 reply; 8+ messages in thread
From: yee.lee @ 2021-07-30 14:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: nicholas.Tang, Kuan-Ying.lee, chinwen.chang, Yee Lee,
	Matthias Brugger, moderated list:ARM/Mediatek SoC support,
	moderated list:ARM/Mediatek SoC support

From: Yee Lee <yee.lee@mediatek.com>

For some case of low-end devices with limited resources,
an option to disable MTE support is necessary.

This patch supoorts to override id-reg on the 
shadow capability via comandline and suppress MTE feature.

=== Test ===
QEMU5.2 + MTE
passed "arm64.nomte" in cmdline
boot log:
..(skip)
[    0.000000] CPU features: SYS_ID_AA64PFR1_EL1[11:8]: forced to 0

Changed since v3:
 - Add documentation text

Changed since v2:
 - Use id-reg override machanism to suppress feature.

Yee Lee (1):
  arm64/cpufeature: Optionally disable MTE via command-line

 Documentation/admin-guide/kernel-parameters.txt | 3 +++
 arch/arm64/kernel/idreg-override.c              | 2 ++
 2 files changed, 5 insertions(+)

-- 
2.18.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line
  2021-07-30 14:49 [PATCH v3 0/1] arm64/cpufeature: Support optionally disable MTE yee.lee
@ 2021-07-30 14:49 ` yee.lee
  2021-08-02 15:30   ` Catalin Marinas
  0 siblings, 1 reply; 8+ messages in thread
From: yee.lee @ 2021-07-30 14:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: nicholas.Tang, Kuan-Ying.lee, chinwen.chang, Yee Lee,
	Jonathan Corbet, Catalin Marinas, Will Deacon, Matthias Brugger,
	Paul E. McKenney, Randy Dunlap, Andrew Morton, Thomas Gleixner,
	Maciej W. Rozycki, Viresh Kumar, Peter Zijlstra, Vlastimil Babka,
	Marc Zyngier, David Brazdil, Ard Biesheuvel, Fuad Tabba,
	open list:DOCUMENTATION,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE),
	moderated list:ARM/Mediatek SoC support

From: Yee Lee <yee.lee@mediatek.com>

For some low-end devices with limited resources,
MTE needs to be optionally disabled to save system
costs such as tag memory and firmware controls.

This allows ID_AA64PFR1_EL1.MTE to be overridden on 
its shadow value by giving "arm64.nomte" on cmdline,
and to suppress MTE feature.

Suggested-by: Marc Zyngier <maz@kernel.org>
Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Yee Lee <yee.lee@mediatek.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 +++
 arch/arm64/kernel/idreg-override.c              | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index bdb22006f713..6f257e39d89e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -380,6 +380,9 @@
 	arm64.nopauth	[ARM64] Unconditionally disable Pointer Authentication
 			support
 
+	arm64.nomte	[ARM64] Unconditionally disable Memory Tagging Extension
+			support
+
 	ataflop=	[HW,M68k]
 
 	atarimouse=	[HW,MOUSE] Atari Mouse
diff --git a/arch/arm64/kernel/idreg-override.c b/arch/arm64/kernel/idreg-override.c
index 53a381a7f65d..d8e606fe3c21 100644
--- a/arch/arm64/kernel/idreg-override.c
+++ b/arch/arm64/kernel/idreg-override.c
@@ -54,6 +54,7 @@ static const struct ftr_set_desc pfr1 __initconst = {
 	.override	= &id_aa64pfr1_override,
 	.fields		= {
 	        { "bt", ID_AA64PFR1_BT_SHIFT },
+		{ "mte", ID_AA64PFR1_MTE_SHIFT},
 		{}
 	},
 };
@@ -100,6 +101,7 @@ static const struct {
 	{ "arm64.nopauth",
 	  "id_aa64isar1.gpi=0 id_aa64isar1.gpa=0 "
 	  "id_aa64isar1.api=0 id_aa64isar1.apa=0"	   },
+	{ "arm64.nomte",		"id_aa64pfr1.mte=0" },
 	{ "nokaslr",			"kaslr.disabled=1" },
 };
 
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line
  2021-07-30 14:49 ` [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line yee.lee
@ 2021-08-02 15:30   ` Catalin Marinas
  2021-08-03  5:55     ` Yee Lee
  2021-11-25 10:19     ` Yee Lee
  0 siblings, 2 replies; 8+ messages in thread
From: Catalin Marinas @ 2021-08-02 15:30 UTC (permalink / raw)
  To: yee.lee
  Cc: linux-kernel, nicholas.Tang, Kuan-Ying.lee, chinwen.chang,
	Jonathan Corbet, Will Deacon, Matthias Brugger, Paul E. McKenney,
	Randy Dunlap, Andrew Morton, Thomas Gleixner, Maciej W. Rozycki,
	Viresh Kumar, Peter Zijlstra, Vlastimil Babka, Marc Zyngier,
	David Brazdil, Ard Biesheuvel, Fuad Tabba,
	open list:DOCUMENTATION,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE),
	moderated list:ARM/Mediatek SoC support

On Fri, Jul 30, 2021 at 10:49:53PM +0800, yee.lee@mediatek.com wrote:
> From: Yee Lee <yee.lee@mediatek.com>
> 
> For some low-end devices with limited resources,
> MTE needs to be optionally disabled to save system
> costs such as tag memory and firmware controls.

I understand the cost of using MTE but I don't fully get what you mean
by firmware controls. If the ID_AA64PFR1_EL1.MTE reports that MTE is
present, the firmware should have initialised MTE correctly (e.g. tag
allocation storage, SCR_EL3.ATA) and not rely on a kernel command line
argument that may or may not be present.

> This allows ID_AA64PFR1_EL1.MTE to be overridden on 
> its shadow value by giving "arm64.nomte" on cmdline,
> and to suppress MTE feature.
> 
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Yee Lee <yee.lee@mediatek.com>

While this patch appears to disable MTE, I don't think it can fully
prevent the access to the allocation tag storage, so the firmware must
still initialise it correctly.

The issue is that __cpu_setup already configures the MAIR_EL1 register
to use Normal Tagged memory for the kernel mapping and SCTLR_EL1.ATA is
set. The TCF field is zero, so no tag checking, but I couldn't figure
out from the ARM ARM whether this also prevents LDR/STR from attempting
to fetch the allocation tags. I think it's only the ATA bit and the MAIR
configuration.

With this patch, KASAN_HW_TAGS (if configured) won't be used and MTE
will not be presented to user applications, if that's what you want, but
does not fully disable MTE.

Since May this year, the ARM ARM was updated so that SCTLR_EL1.ATA/ATA0
are not permitted to be cached in the TLB. We could therefore move the
setting to cpu_enable_mte(). Something like below, untested (to be
folded into your patch):

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index aa53954c2f6b..cac23455a2b5 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -698,8 +698,7 @@
 	(SCTLR_ELx_M    | SCTLR_ELx_C    | SCTLR_ELx_SA   | SCTLR_EL1_SA0   | \
 	 SCTLR_EL1_SED  | SCTLR_ELx_I    | SCTLR_EL1_DZE  | SCTLR_EL1_UCT   | \
 	 SCTLR_EL1_NTWE | SCTLR_ELx_IESB | SCTLR_EL1_SPAN | SCTLR_ELx_ITFSB | \
-	 SCTLR_ELx_ATA  | SCTLR_EL1_ATA0 | ENDIAN_SET_EL1 | SCTLR_EL1_UCI   | \
-	 SCTLR_EL1_EPAN | SCTLR_EL1_RES1)
+	 ENDIAN_SET_EL1 | SCTLR_EL1_UCI  | SCTLR_EL1_EPAN | SCTLR_EL1_RES1)

 /* MAIR_ELx memory attributes (used by Linux) */
 #define MAIR_ATTR_DEVICE_nGnRnE		UL(0x00)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9035c367d08b..23b1e3d83603 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1841,6 +1841,9 @@ static void bti_enable(const struct arm64_cpu_capabilities *__unused)
 #ifdef CONFIG_ARM64_MTE
 static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap)
 {
+	sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ATA | SCTLR_EL1_ATA0);
+	isb();
+
 	/*
 	 * Clear the tags in the zero page. This needs to be done via the
 	 * linear map which has the Tagged attribute.

-- 
Catalin

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line
  2021-08-02 15:30   ` Catalin Marinas
@ 2021-08-03  5:55     ` Yee Lee
  2021-11-25 10:19     ` Yee Lee
  1 sibling, 0 replies; 8+ messages in thread
From: Yee Lee @ 2021-08-03  5:55 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-kernel, nicholas.Tang, Kuan-Ying.lee, chinwen.chang,
	Jonathan Corbet, Will Deacon, Matthias Brugger, Paul E. McKenney,
	Randy Dunlap, Andrew Morton, Thomas Gleixner, Maciej W. Rozycki,
	Viresh Kumar, Peter Zijlstra, Vlastimil Babka, Marc Zyngier,
	David Brazdil, Ard Biesheuvel, Fuad Tabba,
	open list:DOCUMENTATION,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE),
	moderated list:ARM/Mediatek SoC support

On Mon, 2021-08-02 at 16:30 +0100, Catalin Marinas wrote:
> On Fri, Jul 30, 2021 at 10:49:53PM +0800, yee.lee@mediatek.com wrote:
> > From: Yee Lee <yee.lee@mediatek.com>
> > 
> > For some low-end devices with limited resources,
> > MTE needs to be optionally disabled to save system
> > costs such as tag memory and firmware controls.
> 
> I understand the cost of using MTE but I don't fully get what you
> mean
> by firmware controls. If the ID_AA64PFR1_EL1.MTE reports that MTE is
> present, the firmware should have initialised MTE correctly (e.g. tag
> allocation storage, SCR_EL3.ATA) and not rely on a kernel command
> line
> argument that may or may not be present.

Thanks for the reminding. 
Yes, it seems not able to fully disable MTE.
This still provides an option in runtime for evaluation and test.
And it is also useful for firmware development and hw issue workaround.
> 
> > This allows ID_AA64PFR1_EL1.MTE to be overridden on 
> > its shadow value by giving "arm64.nomte" on cmdline,
> > and to suppress MTE feature.
> > 
> > Suggested-by: Marc Zyngier <maz@kernel.org>
> > Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> 
> While this patch appears to disable MTE, I don't think it can fully
> prevent the access to the allocation tag storage, so the firmware
> must
> still initialise it correctly.
> 
> The issue is that __cpu_setup already configures the MAIR_EL1
> register
> to use Normal Tagged memory for the kernel mapping and SCTLR_EL1.ATA
> is
> set. The TCF field is zero, so no tag checking, but I couldn't figure
> out from the ARM ARM whether this also prevents LDR/STR from
> attempting
> to fetch the allocation tags. I think it's only the ATA bit and the
> MAIR
> configuration.
> 
> With this patch, KASAN_HW_TAGS (if configured) won't be used and MTE
> will not be presented to user applications, if that's what you want,
> but
> does not fully disable MTE.
> 
> Since May this year, the ARM ARM was updated so that
> SCTLR_EL1.ATA/ATA0
> are not permitted to be cached in the TLB. We could therefore move
> the
> setting to cpu_enable_mte(). Something like below, untested (to be
> folded into your patch):
> 
ok. will be integrated in v4 and tested.

BR,
Yee

> diff --git a/arch/arm64/include/asm/sysreg.h
> b/arch/arm64/include/asm/sysreg.h
> index aa53954c2f6b..cac23455a2b5 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -698,8 +698,7 @@
>  	(SCTLR_ELx_M    | SCTLR_ELx_C    | SCTLR_ELx_SA   |
> SCTLR_EL1_SA0   | \
>  	 SCTLR_EL1_SED  | SCTLR_ELx_I    | SCTLR_EL1_DZE  |
> SCTLR_EL1_UCT   | \
>  	 SCTLR_EL1_NTWE | SCTLR_ELx_IESB | SCTLR_EL1_SPAN |
> SCTLR_ELx_ITFSB | \
> -	 SCTLR_ELx_ATA  | SCTLR_EL1_ATA0 | ENDIAN_SET_EL1 |
> SCTLR_EL1_UCI   | \
> -	 SCTLR_EL1_EPAN | SCTLR_EL1_RES1)
> +	 ENDIAN_SET_EL1 | SCTLR_EL1_UCI  | SCTLR_EL1_EPAN |
> SCTLR_EL1_RES1)
> 
>  /* MAIR_ELx memory attributes (used by Linux) */
>  #define MAIR_ATTR_DEVICE_nGnRnE		UL(0x00)
> diff --git a/arch/arm64/kernel/cpufeature.c
> b/arch/arm64/kernel/cpufeature.c
> index 9035c367d08b..23b1e3d83603 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1841,6 +1841,9 @@ static void bti_enable(const struct
> arm64_cpu_capabilities *__unused)
>  #ifdef CONFIG_ARM64_MTE
>  static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap)
>  {
> +	sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ATA | SCTLR_EL1_ATA0);
> +	isb();
> +
>  	/*
>  	 * Clear the tags in the zero page. This needs to be done via
> the
>  	 * linear map which has the Tagged attribute.
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line
  2021-08-02 15:30   ` Catalin Marinas
  2021-08-03  5:55     ` Yee Lee
@ 2021-11-25 10:19     ` Yee Lee
  2021-12-03 16:33       ` Catalin Marinas
  1 sibling, 1 reply; 8+ messages in thread
From: Yee Lee @ 2021-11-25 10:19 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-kernel, nicholas.Tang, Kuan-Ying.lee, chinwen.chang,
	Jonathan Corbet, Will Deacon, Matthias Brugger, Paul E. McKenney,
	Randy Dunlap, Andrew Morton, Thomas Gleixner, Maciej W. Rozycki,
	Viresh Kumar, Peter Zijlstra, Vlastimil Babka, Marc Zyngier,
	David Brazdil, Ard Biesheuvel, Fuad Tabba,
	open list:DOCUMENTATION,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE),
	moderated list:ARM/Mediatek SoC support

On Mon, 2021-08-02 at 16:30 +0100, Catalin Marinas wrote:
> On Fri, Jul 30, 2021 at 10:49:53PM +0800, yee.lee@mediatek.com wrote:
> > From: Yee Lee <yee.lee@mediatek.com>
> > 
> > For some low-end devices with limited resources,
> > MTE needs to be optionally disabled to save system
> > costs such as tag memory and firmware controls.
> 
> I understand the cost of using MTE but I don't fully get what you
> mean
> by firmware controls. If the ID_AA64PFR1_EL1.MTE reports that MTE is
> present, the firmware should have initialised MTE correctly (e.g. tag
> allocation storage, SCR_EL3.ATA) and not rely on a kernel command
> line
> argument that may or may not be present.
> 
> > This allows ID_AA64PFR1_EL1.MTE to be overridden on 
> > its shadow value by giving "arm64.nomte" on cmdline,
> > and to suppress MTE feature.
> > 
> > Suggested-by: Marc Zyngier <maz@kernel.org>
> > Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> 
> While this patch appears to disable MTE, I don't think it can fully
> prevent the access to the allocation tag storage, so the firmware
> must
> still initialise it correctly.
> 
> The issue is that __cpu_setup already configures the MAIR_EL1
> register
> to use Normal Tagged memory for the kernel mapping and SCTLR_EL1.ATA
> is
> set. The TCF field is zero, so no tag checking, but I couldn't figure
> out from the ARM ARM whether this also prevents LDR/STR from
> attempting
> to fetch the allocation tags. I think it's only the ATA bit and the
> MAIR
> configuration.
> 
> With this patch, KASAN_HW_TAGS (if configured) won't be used and MTE
> will not be presented to user applications, if that's what you want,
> but
> does not fully disable MTE.
> 

As pointed out earlier, the hardware has been verified that still has
transaction sending to DRAM due to mair_el1(Normal_tagged) is
setup.  That means the override in this patch would be incompleted and
cannot achieve to avoid undesired hardware confliction by disabling
MTE.

Do we have other options to delay the configuration on MAIR_EL1 after
the override? Or maybe another CONFIG to bypass the init in
__cpu_setup?


> Since May this year, the ARM ARM was updated so that
> SCTLR_EL1.ATA/ATA0
> are not permitted to be cached in the TLB. We could therefore move
> the
> setting to cpu_enable_mte(). Something like below, untested (to be
> folded into your patch):
> 
> diff --git a/arch/arm64/include/asm/sysreg.h
> b/arch/arm64/include/asm/sysreg.h
> index aa53954c2f6b..cac23455a2b5 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -698,8 +698,7 @@
>  	(SCTLR_ELx_M    | SCTLR_ELx_C    | SCTLR_ELx_SA   |
> SCTLR_EL1_SA0   | \
>  	 SCTLR_EL1_SED  | SCTLR_ELx_I    | SCTLR_EL1_DZE  |
> SCTLR_EL1_UCT   | \
>  	 SCTLR_EL1_NTWE | SCTLR_ELx_IESB | SCTLR_EL1_SPAN |
> SCTLR_ELx_ITFSB | \
> -	 SCTLR_ELx_ATA  | SCTLR_EL1_ATA0 | ENDIAN_SET_EL1 |
> SCTLR_EL1_UCI   | \
> -	 SCTLR_EL1_EPAN | SCTLR_EL1_RES1)
> +	 ENDIAN_SET_EL1 | SCTLR_EL1_UCI  | SCTLR_EL1_EPAN |
> SCTLR_EL1_RES1)
> 
>  /* MAIR_ELx memory attributes (used by Linux) */
>  #define MAIR_ATTR_DEVICE_nGnRnE		UL(0x00)
> diff --git a/arch/arm64/kernel/cpufeature.c
> b/arch/arm64/kernel/cpufeature.c
> index 9035c367d08b..23b1e3d83603 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1841,6 +1841,9 @@ static void bti_enable(const struct
> arm64_cpu_capabilities *__unused)
>  #ifdef CONFIG_ARM64_MTE
>  static void cpu_enable_mte(struct arm64_cpu_capabilities const *cap)
>  {
> +	sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ATA | SCTLR_EL1_ATA0);
> +	isb();
> +
>  	/*
>  	 * Clear the tags in the zero page. This needs to be done via
> the
>  	 * linear map which has the Tagged attribute.
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line
  2021-11-25 10:19     ` Yee Lee
@ 2021-12-03 16:33       ` Catalin Marinas
  2021-12-14  8:19         ` Yee Lee
  0 siblings, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2021-12-03 16:33 UTC (permalink / raw)
  To: Yee Lee
  Cc: linux-kernel, nicholas.Tang, Kuan-Ying.lee, chinwen.chang,
	Jonathan Corbet, Will Deacon, Matthias Brugger, Paul E. McKenney,
	Randy Dunlap, Andrew Morton, Thomas Gleixner, Maciej W. Rozycki,
	Viresh Kumar, Peter Zijlstra, Vlastimil Babka, Marc Zyngier,
	David Brazdil, Ard Biesheuvel, Fuad Tabba,
	open list:DOCUMENTATION,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE),
	moderated list:ARM/Mediatek SoC support

On Thu, Nov 25, 2021 at 06:19:29PM +0800, Yee Lee wrote:
> On Mon, 2021-08-02 at 16:30 +0100, Catalin Marinas wrote:
> > On Fri, Jul 30, 2021 at 10:49:53PM +0800, yee.lee@mediatek.com wrote:
> > > From: Yee Lee <yee.lee@mediatek.com>
> > > 
> > > For some low-end devices with limited resources,
> > > MTE needs to be optionally disabled to save system
> > > costs such as tag memory and firmware controls.
> > 
> > I understand the cost of using MTE but I don't fully get what you mean
> > by firmware controls. If the ID_AA64PFR1_EL1.MTE reports that MTE is
> > present, the firmware should have initialised MTE correctly (e.g. tag
> > allocation storage, SCR_EL3.ATA) and not rely on a kernel command line
> > argument that may or may not be present.
> > 
> > > This allows ID_AA64PFR1_EL1.MTE to be overridden on 
> > > its shadow value by giving "arm64.nomte" on cmdline,
> > > and to suppress MTE feature.
> > > 
> > > Suggested-by: Marc Zyngier <maz@kernel.org>
> > > Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> > 
> > While this patch appears to disable MTE, I don't think it can fully
> > prevent the access to the allocation tag storage, so the firmware must
> > still initialise it correctly.
> > 
> > The issue is that __cpu_setup already configures the MAIR_EL1 register
> > to use Normal Tagged memory for the kernel mapping and SCTLR_EL1.ATA is
> > set. The TCF field is zero, so no tag checking, but I couldn't figure
> > out from the ARM ARM whether this also prevents LDR/STR from attempting
> > to fetch the allocation tags. I think it's only the ATA bit and the MAIR
> > configuration.
> > 
> > With this patch, KASAN_HW_TAGS (if configured) won't be used and MTE
> > will not be presented to user applications, if that's what you want, but
> > does not fully disable MTE.
> 
> As pointed out earlier, the hardware has been verified that still has
> transaction sending to DRAM due to mair_el1(Normal_tagged) is
> setup.  That means the override in this patch would be incompleted and
> cannot achieve to avoid undesired hardware confliction by disabling
> MTE.
> 
> Do we have other options to delay the configuration on MAIR_EL1 after
> the override? Or maybe another CONFIG to bypass the init in
> __cpu_setup?

This register is trickier as it may be cached in the TLB (IIRC). I think
deferring the setting of SCTLR_EL1.ATA(0) should be sufficient. Can you
try the diff I sent in the previous email and confirm that the accesses
to the allocation tag storage are blocked?

-- 
Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line
  2021-12-03 16:33       ` Catalin Marinas
@ 2021-12-14  8:19         ` Yee Lee
  2021-12-14 12:02           ` Catalin Marinas
  0 siblings, 1 reply; 8+ messages in thread
From: Yee Lee @ 2021-12-14  8:19 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-kernel, nicholas.Tang, Kuan-Ying.lee, chinwen.chang,
	Jonathan Corbet, Will Deacon, Matthias Brugger, Paul E. McKenney,
	Randy Dunlap, Andrew Morton, Thomas Gleixner, Maciej W. Rozycki,
	Viresh Kumar, Peter Zijlstra, Vlastimil Babka, Marc Zyngier,
	David Brazdil, Ard Biesheuvel, Fuad Tabba,
	open list:DOCUMENTATION,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE),
	moderated list:ARM/Mediatek SoC support

On Fri, 2021-12-03 at 16:33 +0000, Catalin Marinas wrote:
> On Thu, Nov 25, 2021 at 06:19:29PM +0800, Yee Lee wrote:
> > On Mon, 2021-08-02 at 16:30 +0100, Catalin Marinas wrote:
> > > On Fri, Jul 30, 2021 at 10:49:53PM +0800, yee.lee@mediatek.com
> > > wrote:
> > > > From: Yee Lee <yee.lee@mediatek.com>
> > > > 
> > > > For some low-end devices with limited resources,
> > > > MTE needs to be optionally disabled to save system
> > > > costs such as tag memory and firmware controls.
> > > 
> > > I understand the cost of using MTE but I don't fully get what you
> > > mean
> > > by firmware controls. If the ID_AA64PFR1_EL1.MTE reports that MTE
> > > is
> > > present, the firmware should have initialised MTE correctly (e.g.
> > > tag
> > > allocation storage, SCR_EL3.ATA) and not rely on a kernel command
> > > line
> > > argument that may or may not be present.
> > > 
> > > > This allows ID_AA64PFR1_EL1.MTE to be overridden on 
> > > > its shadow value by giving "arm64.nomte" on cmdline,
> > > > and to suppress MTE feature.
> > > > 
> > > > Suggested-by: Marc Zyngier <maz@kernel.org>
> > > > Suggested-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > > Signed-off-by: Yee Lee <yee.lee@mediatek.com>
> > > 
> > > While this patch appears to disable MTE, I don't think it can
> > > fully
> > > prevent the access to the allocation tag storage, so the firmware
> > > must
> > > still initialise it correctly.
> > > 
> > > The issue is that __cpu_setup already configures the MAIR_EL1
> > > register
> > > to use Normal Tagged memory for the kernel mapping and
> > > SCTLR_EL1.ATA is
> > > set. The TCF field is zero, so no tag checking, but I couldn't
> > > figure
> > > out from the ARM ARM whether this also prevents LDR/STR from
> > > attempting
> > > to fetch the allocation tags. I think it's only the ATA bit and
> > > the MAIR
> > > configuration.
> > > 
> > > With this patch, KASAN_HW_TAGS (if configured) won't be used and
> > > MTE
> > > will not be presented to user applications, if that's what you
> > > want, but
> > > does not fully disable MTE.
> > 
> > As pointed out earlier, the hardware has been verified that still
> > has
> > transaction sending to DRAM due to mair_el1(Normal_tagged) is
> > setup.  That means the override in this patch would be incompleted
> > and
> > cannot achieve to avoid undesired hardware confliction by disabling
> > MTE.
> > 
> > Do we have other options to delay the configuration on MAIR_EL1
> > after
> > the override? Or maybe another CONFIG to bypass the init in
> > __cpu_setup?
> 
> This register is trickier as it may be cached in the TLB (IIRC). I
> think
> deferring the setting of SCTLR_EL1.ATA(0) should be sufficient. Can
> you
> try the diff I sent in the previous email and confirm that the
> accesses
> to the allocation tag storage are blocked?
> 

Yes, the previous diff is already online. 

In our experiment, with cmdline, "arm64.nomte", cpu_enable_mte() is
bypassed and the ATA0 is not set, but the access to tag memory still
dispatches. Only as MAIR_EL1 remains MAIR_ATTR_NORMAL, instead of
MAIR_ATTR_NORMAL_TAGGED, the access will stop.

From the manual, I think ATA only affects TAG instructions like STG,
IRG, but not the tag access within normal STR/LDR.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line
  2021-12-14  8:19         ` Yee Lee
@ 2021-12-14 12:02           ` Catalin Marinas
  0 siblings, 0 replies; 8+ messages in thread
From: Catalin Marinas @ 2021-12-14 12:02 UTC (permalink / raw)
  To: Yee Lee
  Cc: linux-kernel, nicholas.Tang, Kuan-Ying.lee, chinwen.chang,
	Jonathan Corbet, Will Deacon, Matthias Brugger, Paul E. McKenney,
	Randy Dunlap, Andrew Morton, Thomas Gleixner, Maciej W. Rozycki,
	Viresh Kumar, Peter Zijlstra, Vlastimil Babka, Marc Zyngier,
	David Brazdil, Ard Biesheuvel, Fuad Tabba,
	open list:DOCUMENTATION,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE),
	moderated list:ARM/Mediatek SoC support

On Tue, Dec 14, 2021 at 04:19:05PM +0800, Yee Lee wrote:
> On Fri, 2021-12-03 at 16:33 +0000, Catalin Marinas wrote:
> > On Thu, Nov 25, 2021 at 06:19:29PM +0800, Yee Lee wrote:
> > > As pointed out earlier, the hardware has been verified that still has
> > > transaction sending to DRAM due to mair_el1(Normal_tagged) is
> > > setup.  That means the override in this patch would be incompleted and
> > > cannot achieve to avoid undesired hardware confliction by disabling MTE.
> > > 
> > > Do we have other options to delay the configuration on MAIR_EL1 after
> > > the override? Or maybe another CONFIG to bypass the init in __cpu_setup?
> > 
> > This register is trickier as it may be cached in the TLB (IIRC). I think
> > deferring the setting of SCTLR_EL1.ATA(0) should be sufficient. Can you
> > try the diff I sent in the previous email and confirm that the accesses
> > to the allocation tag storage are blocked?
> 
> Yes, the previous diff is already online. 
> 
> In our experiment, with cmdline, "arm64.nomte", cpu_enable_mte() is
> bypassed and the ATA0 is not set, but the access to tag memory still
> dispatches. Only as MAIR_EL1 remains MAIR_ATTR_NORMAL, instead of
> MAIR_ATTR_NORMAL_TAGGED, the access will stop.
> 
> From the manual, I think ATA only affects TAG instructions like STG,
> IRG, but not the tag access within normal STR/LDR.

The ARM ARM states SCTLR_EL1.ATA0 == 0 means "access to allocation tags
is prevented". The AArch64.MemSingle[] pseudocode ends up with similar
checks:

https://developer.arm.com/documentation/ddi0596/2021-09/Shared-Pseudocode/AArch64-Functions?lang=en#AArch64.MemSingle.read.5

before reading the tags from memory in AArch64.CheckTag():

https://developer.arm.com/documentation/ddi0596/2021-09/Shared-Pseudocode/AArch64-Functions?lang=en#AArch64.CheckTag.4

My suggestion is to raise this with support@arm.com (feel free to cc me)
so that we clarify the hardware behaviour. I don't think it's entirely
correct (it's more like, is there a risk of external aborts caused by
access to allocation tag storage that's not present?)

-- 
Catalin

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-12-14 12:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-30 14:49 [PATCH v3 0/1] arm64/cpufeature: Support optionally disable MTE yee.lee
2021-07-30 14:49 ` [PATCH v3 1/1] arm64/cpufeature: Optionally disable MTE via command-line yee.lee
2021-08-02 15:30   ` Catalin Marinas
2021-08-03  5:55     ` Yee Lee
2021-11-25 10:19     ` Yee Lee
2021-12-03 16:33       ` Catalin Marinas
2021-12-14  8:19         ` Yee Lee
2021-12-14 12:02           ` Catalin Marinas

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).