Re: [PATCH 2/4] arm64: mte: Add asynchronous mode support

On Thu, Jan 7, 2021 at 6:25 PM Vincenzo Frascino
[off-list ref] wrote:

Hi Andrey,

On 1/7/21 4:29 PM, Andrey Konovalov wrote:

quoted

On Wed, Jan 6, 2021 at 12:56 PM Vincenzo Frascino
[off-list ref] wrote:

quoted

MTE provides an asynchronous mode for detecting tag exceptions. In
particular instead of triggering a fault the arm64 core updates a
register which is checked by the kernel at the first entry after the tag
exception has occurred.

Add support for MTE asynchronous mode.

The exception handling mechanism will be added with a future patch.

Note: KASAN HW activates async mode via kasan.mode kernel parameter.
The default mode is set to synchronous.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <redacted>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
 arch/arm64/kernel/mte.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
index 24a273d47df1..5d992e16b420 100644
--- a/arch/arm64/kernel/mte.c
+++ b/arch/arm64/kernel/mte.c

@@ -153,8 +153,35 @@ void mte_init_tags(u64 max_tag)

 void mte_enable_kernel(enum kasan_arg_mode mode)
 {
-       /* Enable MTE Sync Mode for EL1. */
-       sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
+       const char *m;
+
+       /* Preset parameter values based on the mode. */
+       switch (mode) {
+       case KASAN_ARG_MODE_OFF:
+               return;
+       case KASAN_ARG_MODE_LIGHT:
+               /* Enable MTE Async Mode for EL1. */
+               sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_ASYNC);
+               m = "asynchronous";
+               break;
+       case KASAN_ARG_MODE_DEFAULT:
+       case KASAN_ARG_MODE_PROD:
+       case KASAN_ARG_MODE_FULL:
+               /* Enable MTE Sync Mode for EL1. */
+               sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_SYNC);
+               m = "synchronous";
+               break;
+       default:
+               /*
+                * kasan mode should be always set hence we should
+                * not reach this condition.
+                */
+               WARN_ON_ONCE(1);
+               return;
+       }
+
+       pr_info_once("MTE: enabled in %s mode at EL1\n", m);
+
        isb();
 }

--

2.29.2

Hi Vincenzo,

It would be cleaner to pass a bool to mte_enable_kernel() and have it
indicate sync/async mode. This way you don't have to pull all these
KASAN constants into the arm64 code.

Boolean arguments are generally bad for legibility, hence I tend to avoid them.
In this case exposing the constants does not seem a big issue especially because
the only user of this code is "KASAN_HW_TAGS" and definitely improves its
legibility hence I would prefer to keep it as is.

I don't like that this spills KASAN internals to the arm64 code. Let's
add another enum with two values and pass it as an argument then.
Something like:

enum mte_mode {
  ARM_MTE_SYNC,
  ARM_MTE_ASYNC
}

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