Messages in this thread | | | Subject | Re: [PATCH v3 2/4] arm64: mte: Add asynchronous mode support | From | Vincenzo Frascino <> | Date | Sat, 16 Jan 2021 13:49:14 +0000 |
| |
On 1/15/21 3:13 PM, Mark Rutland wrote: > On Fri, Jan 15, 2021 at 12:00:41PM +0000, Vincenzo Frascino wrote: >> 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 after the asynchronous tag >> check fault 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. >> The code that verifies the status of TFSR_EL1 will be added with a >> future patch. >> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Will Deacon <will@kernel.org> >> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> >> --- >> arch/arm64/kernel/mte.c | 26 ++++++++++++++++++++++++-- >> 1 file changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c >> index 53a6d734e29b..df7a1ae26d7c 100644 >> --- a/arch/arm64/kernel/mte.c >> +++ b/arch/arm64/kernel/mte.c >> @@ -153,8 +153,30 @@ void mte_init_tags(u64 max_tag) >> >> void mte_enable_kernel(enum kasan_hw_tags_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_HW_TAGS_ASYNC: >> + /* Enable MTE Async Mode for EL1. */ >> + sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, SCTLR_ELx_TCF_ASYNC); >> + m = "asynchronous"; >> + break; >> + case KASAN_HW_TAGS_SYNC: >> + 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(); >> } > > For clarity, we should have that ISB before the pr_info_once(). >
Good point, I will fix it in v4.
> As with my comment on patch 1, I think with separate functions this > would be much clearer and simpler: > > static inline void __mte_enable_kernel(const char *mode, unsigned long tcf) > { > sysreg_clear_set(sctlr_el1, SCTLR_ELx_TCF_MASK, tcf); > isb(); > > pr_info_once("MTE: enabled in %s mode at EL1\n", mode); > } > > void mte_enable_kernel_sync(void) > { > __mte_enable_kernel("synchronous", SCTLR_ELx_TCF_SYNC); > } > > void mte_enable_kernel_async(void) > { > __mte_enable_kernel("asynchronous", SCTLR_ELx_TCF_ASYNC); > } >
Ok, seems cleaner like this, will adapt my code accordingly.
> Thanks, > Mark. >
-- Regards, Vincenzo
| |