[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250319172935.GMZ9r-_zzXhyhHBLfj@fat_crate.local>
Date: Wed, 19 Mar 2025 18:29:35 +0100
From: Borislav Petkov <bp@...en8.de>
To: Brendan Jackman <jackmanb@...gle.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>, Andy Lutomirski <luto@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Richard Henderson <richard.henderson@...aro.org>,
Matt Turner <mattst88@...il.com>, Vineet Gupta <vgupta@...nel.org>,
Russell King <linux@...linux.org.uk>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Guo Ren <guoren@...nel.org>,
Brian Cain <bcain@...cinc.com>, Huacai Chen <chenhuacai@...nel.org>,
WANG Xuerui <kernel@...0n.name>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Michal Simek <monstr@...str.eu>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Dinh Nguyen <dinguyen@...nel.org>, Jonas Bonn <jonas@...thpole.se>,
Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>,
Stafford Horne <shorne@...il.com>,
"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
Helge Deller <deller@....de>, Michael Ellerman <mpe@...erman.id.au>,
Nicholas Piggin <npiggin@...il.com>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Naveen N Rao <naveen@...nel.org>,
Madhavan Srinivasan <maddy@...ux.ibm.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Heiko Carstens <hca@...ux.ibm.com>,
Vasily Gorbik <gor@...ux.ibm.com>,
Alexander Gordeev <agordeev@...ux.ibm.com>,
Christian Borntraeger <borntraeger@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>,
Yoshinori Sato <ysato@...rs.sourceforge.jp>,
Rich Felker <dalias@...c.org>,
John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>,
"David S. Miller" <davem@...emloft.net>,
Andreas Larsson <andreas@...sler.com>,
Richard Weinberger <richard@....at>,
Anton Ivanov <anton.ivanov@...bridgegreys.com>,
Johannes Berg <johannes@...solutions.net>,
Chris Zankel <chris@...kel.net>, Max Filippov <jcmvbkbc@...il.com>,
Arnd Bergmann <arnd@...db.de>,
Andrew Morton <akpm@...ux-foundation.org>,
Juri Lelli <juri.lelli@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Steven Rostedt <rostedt@...dmis.org>,
Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
Valentin Schneider <vschneid@...hat.com>,
Uladzislau Rezki <urezki@...il.com>,
Christoph Hellwig <hch@...radead.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Mike Rapoport <rppt@...nel.org>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Dennis Zhou <dennis@...nel.org>, Tejun Heo <tj@...nel.org>,
Christoph Lameter <cl@...ux.com>,
Sean Christopherson <seanjc@...gle.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Ard Biesheuvel <ardb@...nel.org>,
Josh Poimboeuf <jpoimboe@...nel.org>,
Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>, x86@...nel.org,
linux-kernel@...r.kernel.org, linux-alpha@...r.kernel.org,
linux-snps-arc@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-csky@...r.kernel.org,
linux-hexagon@...r.kernel.org, loongarch@...ts.linux.dev,
linux-m68k@...ts.linux-m68k.org, linux-mips@...r.kernel.org,
linux-openrisc@...r.kernel.org, linux-parisc@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, linux-riscv@...ts.infradead.org,
linux-s390@...r.kernel.org, linux-sh@...r.kernel.org,
sparclinux@...r.kernel.org, linux-um@...ts.infradead.org,
linux-arch@...r.kernel.org, linux-mm@...ck.org,
linux-trace-kernel@...r.kernel.org,
linux-perf-users@...r.kernel.org, kvm@...r.kernel.org,
linux-efi@...r.kernel.org, Junaid Shahid <junaids@...gle.com>,
Yosry Ahmed <yosryahmed@...gle.com>
Subject: Re: [PATCH RFC v2 04/29] mm: asi: Add infrastructure for boot-time
enablement
On Fri, Jan 10, 2025 at 06:40:30PM +0000, Brendan Jackman wrote:
> Add a boot time parameter to control the newly added X86_FEATURE_ASI.
> "asi=on" or "asi=off" can be used in the kernel command line to enable
> or disable ASI at boot time. If not specified, ASI enablement depends
> on CONFIG_ADDRESS_SPACE_ISOLATION_DEFAULT_ON, which is off by default.
I don't know yet why we need this default-on thing...
> asi_check_boottime_disable() is modeled after
> pti_check_boottime_disable().
>
> The boot parameter is currently ignored until ASI is fully functional.
>
> Once we have a set of ASI features checked in that we have actually
> tested, we will stop ignoring the flag. But for now let's just add the
> infrastructure so we can implement the usage code.
>
> Ignoring checkpatch.pl CONFIG_DESCRIPTION because the _DEFAULT_ON
> Kconfig is trivial to explain.
Those last two paragraphs go...
> Checkpatch-args: --ignore CONFIG_DESCRIPTION
> Co-developed-by: Junaid Shahid <junaids@...gle.com>
> Signed-off-by: Junaid Shahid <junaids@...gle.com>
> Co-developed-by: Yosry Ahmed <yosryahmed@...gle.com>
> Signed-off-by: Yosry Ahmed <yosryahmed@...gle.com>
> Signed-off-by: Brendan Jackman <jackmanb@...gle.com>
> ---
... here as that's text not really pertaining to the contents of the patch.
> arch/x86/Kconfig | 9 +++++
> arch/x86/include/asm/asi.h | 19 ++++++++--
> arch/x86/include/asm/cpufeatures.h | 1 +
> arch/x86/include/asm/disabled-features.h | 8 ++++-
> arch/x86/mm/asi.c | 61 +++++++++++++++++++++++++++-----
> arch/x86/mm/init.c | 4 ++-
> include/asm-generic/asi.h | 4 +++
> 7 files changed, 92 insertions(+), 14 deletions(-)
...
> * the N ASI classes.
> */
>
> +#define static_asi_enabled() cpu_feature_enabled(X86_FEATURE_ASI)
Yeah, as already mentioned somewhere else, whack that thing pls.
> +
> /*
> * ASI uses a per-CPU tainting model to track what mitigation actions are
> * required on domain transitions. Taints exist along two dimensions:
> @@ -131,6 +134,8 @@ struct asi {
>
> DECLARE_PER_CPU_ALIGNED(struct asi *, curr_asi);
>
> +void asi_check_boottime_disable(void);
> +
> void asi_init_mm_state(struct mm_struct *mm);
>
> int asi_init_class(enum asi_class_id class_id, struct asi_taint_policy *taint_policy);
> @@ -155,7 +160,9 @@ void asi_exit(void);
> /* The target is the domain we'll enter when returning to process context. */
> static __always_inline struct asi *asi_get_target(struct task_struct *p)
> {
> - return p->thread.asi_state.target;
> + return static_asi_enabled()
> + ? p->thread.asi_state.target
> + : NULL;
Waaay too fancy for old people:
if ()
return...
else
return NULL;
:-)
The others too pls.
> static __always_inline void asi_set_target(struct task_struct *p,
> @@ -166,7 +173,9 @@ static __always_inline void asi_set_target(struct task_struct *p,
>
> static __always_inline struct asi *asi_get_current(void)
> {
> - return this_cpu_read(curr_asi);
> + return static_asi_enabled()
> + ? this_cpu_read(curr_asi)
> + : NULL;
> }
>
> /* Are we currently in a restricted address space? */
> @@ -175,7 +184,11 @@ static __always_inline bool asi_is_restricted(void)
> return (bool)asi_get_current();
> }
>
> -/* If we exit/have exited, can we stay that way until the next asi_enter? */
> +/*
> + * If we exit/have exited, can we stay that way until the next asi_enter?
What is that supposed to mean here?
> + *
> + * When ASI is disabled, this returns true.
> + */
> static __always_inline bool asi_is_relaxed(void)
> {
> return !asi_get_target(current);
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 913fd3a7bac6506141de65f33b9ee61c615c7d7d..d6a808d10c3b8900d190ea01c66fc248863f05e2 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -474,6 +474,7 @@
> #define X86_FEATURE_CLEAR_BHB_HW (21*32+ 3) /* BHI_DIS_S HW control enabled */
> #define X86_FEATURE_CLEAR_BHB_LOOP_ON_VMEXIT (21*32+ 4) /* Clear branch history at vmexit using SW loop */
> #define X86_FEATURE_FAST_CPPC (21*32 + 5) /* AMD Fast CPPC */
> +#define X86_FEATURE_ASI (21*32+6) /* Kernel Address Space Isolation */
>
> /*
> * BUG word(s)
> diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
> index c492bdc97b0595ec77f89dc9b0cefe5e3e64be41..c7964ed4fef8b9441e1c0453da587787d8008d9d 100644
> --- a/arch/x86/include/asm/disabled-features.h
> +++ b/arch/x86/include/asm/disabled-features.h
> @@ -50,6 +50,12 @@
> # define DISABLE_PTI (1 << (X86_FEATURE_PTI & 31))
> #endif
>
> +#ifdef CONFIG_MITIGATION_ADDRESS_SPACE_ISOLATION
> +# define DISABLE_ASI 0
> +#else
> +# define DISABLE_ASI (1 << (X86_FEATURE_ASI & 31))
> +#endif
> +
> #ifdef CONFIG_MITIGATION_RETPOLINE
> # define DISABLE_RETPOLINE 0
> #else
> @@ -154,7 +160,7 @@
> #define DISABLED_MASK17 0
> #define DISABLED_MASK18 (DISABLE_IBT)
> #define DISABLED_MASK19 (DISABLE_SEV_SNP)
> -#define DISABLED_MASK20 0
> +#define DISABLED_MASK20 (DISABLE_ASI)
> #define DISABLED_MASK21 0
> #define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 22)
>
Right, that hunk is done this way now:
diff --git a/arch/x86/Kconfig.cpufeatures b/arch/x86/Kconfig.cpufeatures
index e12d5b7e39a2..f219eaf664fb 100644
--- a/arch/x86/Kconfig.cpufeatures
+++ b/arch/x86/Kconfig.cpufeatures
@@ -199,3 +199,7 @@ config X86_DISABLED_FEATURE_SEV_SNP
config X86_DISABLED_FEATURE_INVLPGB
def_bool y
depends on !BROADCAST_TLB_FLUSH
+
+config X86_DISABLED_FEATURE_ASI
+ def_bool y
+ depends on !MITIGATION_ADDRESS_SPACE_ISOLATION
> diff --git a/arch/x86/mm/asi.c b/arch/x86/mm/asi.c
> index 105cd8b43eaf5c20acc80d4916b761559fb95d74..5baf563a078f5b3a6cd4b9f5e92baaf81b0774c4 100644
> --- a/arch/x86/mm/asi.c
> +++ b/arch/x86/mm/asi.c
> @@ -4,6 +4,7 @@
> #include <linux/percpu.h>
> #include <linux/spinlock.h>
>
> +#include <linux/init.h>
> #include <asm/asi.h>
> #include <asm/cmdline.h>
> #include <asm/cpufeature.h>
> @@ -29,6 +30,9 @@ static inline bool asi_class_id_valid(enum asi_class_id class_id)
>
> static inline bool asi_class_initialized(enum asi_class_id class_id)
> {
> + if (!boot_cpu_has(X86_FEATURE_ASI))
check_for_deprecated_apis: WARNING: arch/x86/mm/asi.c:33: Do not use boot_cpu_has() - use cpu_feature_enabled() instead.
Check your whole set pls.
> + return 0;
> +
> if (WARN_ON(!asi_class_id_valid(class_id)))
> return false;
>
> @@ -51,6 +55,9 @@ EXPORT_SYMBOL_GPL(asi_init_class);
>
> void asi_uninit_class(enum asi_class_id class_id)
> {
> + if (!boot_cpu_has(X86_FEATURE_ASI))
> + return;
> +
> if (!asi_class_initialized(class_id))
> return;
>
> @@ -66,10 +73,36 @@ const char *asi_class_name(enum asi_class_id class_id)
> return asi_class_names[class_id];
> }
>
> +void __init asi_check_boottime_disable(void)
> +{
> + bool enabled = IS_ENABLED(CONFIG_MITIGATION_ADDRESS_SPACE_ISOLATION_DEFAULT_ON);
> + char arg[4];
> + int ret;
> +
> + ret = cmdline_find_option(boot_command_line, "asi", arg, sizeof(arg));
> + if (ret == 3 && !strncmp(arg, "off", 3)) {
> + enabled = false;
> + pr_info("ASI disabled through kernel command line.\n");
> + } else if (ret == 2 && !strncmp(arg, "on", 2)) {
> + enabled = true;
> + pr_info("Ignoring asi=on param while ASI implementation is incomplete.\n");
> + } else {
> + pr_info("ASI %s by default.\n",
> + enabled ? "enabled" : "disabled");
> + }
> +
> + if (enabled)
> + pr_info("ASI enablement ignored due to incomplete implementation.\n");
Incomplete how?
> +}
> +
> static void __asi_destroy(struct asi *asi)
> {
> - lockdep_assert_held(&asi->mm->asi_init_lock);
> + WARN_ON_ONCE(asi->ref_count <= 0);
> + if (--(asi->ref_count) > 0)
Switch that to
include/linux/kref.h
It gives you a sanity-checking functionality too so you don't need the WARN...
> + return;
>
> + free_pages((ulong)asi->pgd, PGD_ALLOCATION_ORDER);
> + memset(asi, 0, sizeof(struct asi));
And then you can do:
if (kref_put())
free_pages...
and so on.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Powered by blists - more mailing lists