| Message ID | 20240528225131.3577704-1-pcc@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: mte: Make mte_check_tfsr_*() conditional on KASAN instead of MTE | expand |
On Tue, May 28, 2024 at 3:51 PM Peter Collingbourne <pcc@google.com> wrote: > > The check in mte_check_tfsr_el1() is only necessary if HW tag > based KASAN is enabled. However, we were also executing the check > if MTE is enabled and KASAN is enabled at build time but disabled > at runtime. This turned out to cause a measurable increase in > power consumption on a specific microarchitecture after enabling > MTE. Moreover, on the same system, an increase in invalid syscall > latency (as measured by [1]) of around 20-30% (depending on the > cluster) was observed after enabling MTE; this almost entirely goes > away after removing this check. Therefore, make the check conditional > on whether KASAN is enabled rather than on whether MTE is enabled. Ping. Peter
Hi, On Tue, May 28, 2024 at 03:51:30PM -0700, Peter Collingbourne wrote: > The check in mte_check_tfsr_el1() is only necessary if HW tag > based KASAN is enabled. However, we were also executing the check > if MTE is enabled and KASAN is enabled at build time but disabled > at runtime. This turned out to cause a measurable increase in > power consumption on a specific microarchitecture after enabling > MTE. Moreover, on the same system, an increase in invalid syscall > latency (as measured by [1]) of around 20-30% (depending on the > cluster) was observed after enabling MTE; this almost entirely goes > away after removing this check. Therefore, make the check conditional > on whether KASAN is enabled rather than on whether MTE is enabled. > > [1] https://lore.kernel.org/all/CAMn1gO4MwRV8bmFJ_SeY5tsYNPn2ZP56LjAhafygjFaKuu5ouw@mail.gmail.com/ > > Signed-off-by: Peter Collingbourne <pcc@google.com> > Link: https://linux-review.googlesource.com/id/I22d98d1483dd400a95595946552b769a5a1ad7bd > --- > arch/arm64/include/asm/mte.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h > index 91fbd5c8a3911..0f84518632b4a 100644 > --- a/arch/arm64/include/asm/mte.h > +++ b/arch/arm64/include/asm/mte.h > @@ -182,7 +182,7 @@ void mte_check_tfsr_el1(void); > > static inline void mte_check_tfsr_entry(void) > { > - if (!system_supports_mte()) > + if (!kasan_hw_tags_enabled()) > return; > > mte_check_tfsr_el1(); > @@ -190,7 +190,7 @@ static inline void mte_check_tfsr_entry(void) > > static inline void mte_check_tfsr_exit(void) > { > - if (!system_supports_mte()) > + if (!kasan_hw_tags_enabled()) > return; The change looks correct to me - tag check faults for the kernel are enabled only when hw kasan is enabled. The functions are already compiled only when CONFIG_KASAN_HW_TAGS=y, so it makes sense to further restrict them based on kasan_hw_tags_enabled(): Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com> Can't speak for the performance results, altough skipping the dsb + isb on the exit path sure looks nice. Thanks, Alex > > /* > -- > 2.45.1.288.g0e0cd299f1-goog >
On Tue, 28 May 2024 15:51:30 -0700, Peter Collingbourne wrote: > The check in mte_check_tfsr_el1() is only necessary if HW tag > based KASAN is enabled. However, we were also executing the check > if MTE is enabled and KASAN is enabled at build time but disabled > at runtime. This turned out to cause a measurable increase in > power consumption on a specific microarchitecture after enabling > MTE. Moreover, on the same system, an increase in invalid syscall > latency (as measured by [1]) of around 20-30% (depending on the > cluster) was observed after enabling MTE; this almost entirely goes > away after removing this check. Therefore, make the check conditional > on whether KASAN is enabled rather than on whether MTE is enabled. > > [...] Applied to arm64 (for-next/mte), thanks! [1/1] arm64: mte: Make mte_check_tfsr_*() conditional on KASAN instead of MTE https://git.kernel.org/arm64/c/26ca4423604f
diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h index 91fbd5c8a3911..0f84518632b4a 100644 --- a/arch/arm64/include/asm/mte.h +++ b/arch/arm64/include/asm/mte.h @@ -182,7 +182,7 @@ void mte_check_tfsr_el1(void); static inline void mte_check_tfsr_entry(void) { - if (!system_supports_mte()) + if (!kasan_hw_tags_enabled()) return; mte_check_tfsr_el1(); @@ -190,7 +190,7 @@ static inline void mte_check_tfsr_entry(void) static inline void mte_check_tfsr_exit(void) { - if (!system_supports_mte()) + if (!kasan_hw_tags_enabled()) return; /*
The check in mte_check_tfsr_el1() is only necessary if HW tag based KASAN is enabled. However, we were also executing the check if MTE is enabled and KASAN is enabled at build time but disabled at runtime. This turned out to cause a measurable increase in power consumption on a specific microarchitecture after enabling MTE. Moreover, on the same system, an increase in invalid syscall latency (as measured by [1]) of around 20-30% (depending on the cluster) was observed after enabling MTE; this almost entirely goes away after removing this check. Therefore, make the check conditional on whether KASAN is enabled rather than on whether MTE is enabled. [1] https://lore.kernel.org/all/CAMn1gO4MwRV8bmFJ_SeY5tsYNPn2ZP56LjAhafygjFaKuu5ouw@mail.gmail.com/ Signed-off-by: Peter Collingbourne <pcc@google.com> Link: https://linux-review.googlesource.com/id/I22d98d1483dd400a95595946552b769a5a1ad7bd --- arch/arm64/include/asm/mte.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)