* [PATCH v3 0/2] x86/tsx: Improve handling of the tsx= kernel parameter @ 2025-10-22 10:26 Petr Tesarik 2025-10-22 10:26 ` [PATCH v3 1/2] x86/tsx: Make tsx_ctrl_state static Petr Tesarik 2025-10-22 10:26 ` [PATCH v3 2/2] x86/tsx: Get the tsx= command line parameter with early_param() Petr Tesarik 0 siblings, 2 replies; 6+ messages in thread From: Petr Tesarik @ 2025-10-22 10:26 UTC (permalink / raw) To: Pawan Gupta, Thomas Gleixner, Ingo Molnar, Borislav Petkov Cc: H. Peter Anvin, Dave Hansen, Nikolay Borisov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:X86 ARCHITECTURE (32-BIT AND 64-BIT), Petr Tesarik Parse the tsx= kernel parameter with early_param(), so it is not reported as unknown. As a side note, it was not necessary to defer the handling of tsx=auto. Contrary to Pawan's claim, cpu_set_bug_bits() is called from setup_arch() via early_cpu_init() and early_identify_cpu() before parse_early_param(). But it is a nice cleanup, nevertheless, and I learned something about parsing the command line. Cf. the NAK here: https://lore.kernel.org/all/20251009185134.fb4evjrk76rwxv37@desk/ Changes from v2: * compile-time initialization with the configured default * defer tsx=auto handling to tsx_init() * add more detail to the commit message Changes from v1: * make tsx_ctrl_state local to tsx.c * use early_param() instead of core_param() Petr Tesarik (2): x86/tsx: Make tsx_ctrl_state static x86/tsx: Get the tsx= command line parameter with early_param() arch/x86/kernel/cpu/cpu.h | 9 ------ arch/x86/kernel/cpu/tsx.c | 59 ++++++++++++++++++++++----------------- 2 files changed, 34 insertions(+), 34 deletions(-) -- 2.51.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] x86/tsx: Make tsx_ctrl_state static 2025-10-22 10:26 [PATCH v3 0/2] x86/tsx: Improve handling of the tsx= kernel parameter Petr Tesarik @ 2025-10-22 10:26 ` Petr Tesarik 2025-10-22 10:26 ` [PATCH v3 2/2] x86/tsx: Get the tsx= command line parameter with early_param() Petr Tesarik 1 sibling, 0 replies; 6+ messages in thread From: Petr Tesarik @ 2025-10-22 10:26 UTC (permalink / raw) To: Pawan Gupta, Thomas Gleixner, Ingo Molnar, Borislav Petkov Cc: H. Peter Anvin, Dave Hansen, Nikolay Borisov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:X86 ARCHITECTURE (32-BIT AND 64-BIT), Petr Tesarik Move all definitions related to tsx_ctrl_state to tsx.c. They are never referenced outside this file. No functional change. Signed-off-by: Petr Tesarik <ptesarik@suse.com> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com> --- arch/x86/kernel/cpu/cpu.h | 9 --------- arch/x86/kernel/cpu/tsx.c | 9 ++++++++- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h index bc38b2d56f26..5c7a3a71191a 100644 --- a/arch/x86/kernel/cpu/cpu.h +++ b/arch/x86/kernel/cpu/cpu.h @@ -42,15 +42,6 @@ extern const struct cpu_dev *const __x86_cpu_dev_start[], *const __x86_cpu_dev_end[]; #ifdef CONFIG_CPU_SUP_INTEL -enum tsx_ctrl_states { - TSX_CTRL_ENABLE, - TSX_CTRL_DISABLE, - TSX_CTRL_RTM_ALWAYS_ABORT, - TSX_CTRL_NOT_SUPPORTED, -}; - -extern __ro_after_init enum tsx_ctrl_states tsx_ctrl_state; - extern void __init tsx_init(void); void tsx_ap_init(void); void intel_unlock_cpuid_leafs(struct cpuinfo_x86 *c); diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c index 49782724a943..8be08ece2214 100644 --- a/arch/x86/kernel/cpu/tsx.c +++ b/arch/x86/kernel/cpu/tsx.c @@ -19,7 +19,14 @@ #undef pr_fmt #define pr_fmt(fmt) "tsx: " fmt -enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = TSX_CTRL_NOT_SUPPORTED; +enum tsx_ctrl_states { + TSX_CTRL_ENABLE, + TSX_CTRL_DISABLE, + TSX_CTRL_RTM_ALWAYS_ABORT, + TSX_CTRL_NOT_SUPPORTED, +}; + +static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = TSX_CTRL_NOT_SUPPORTED; static void tsx_disable(void) { -- 2.51.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] x86/tsx: Get the tsx= command line parameter with early_param() 2025-10-22 10:26 [PATCH v3 0/2] x86/tsx: Improve handling of the tsx= kernel parameter Petr Tesarik 2025-10-22 10:26 ` [PATCH v3 1/2] x86/tsx: Make tsx_ctrl_state static Petr Tesarik @ 2025-10-22 10:26 ` Petr Tesarik 2025-10-22 17:46 ` Pawan Gupta 1 sibling, 1 reply; 6+ messages in thread From: Petr Tesarik @ 2025-10-22 10:26 UTC (permalink / raw) To: Pawan Gupta, Thomas Gleixner, Ingo Molnar, Borislav Petkov Cc: H. Peter Anvin, Dave Hansen, Nikolay Borisov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:X86 ARCHITECTURE (32-BIT AND 64-BIT), Petr Tesarik Use early_param() to get the value of the tsx= command line parameter. It is an early parameter, because it must be parsed before tsx_init(), which is called long before kernel_init(), where normal parameters are parsed. Although cmdline_find_option() from tsx_init() works fine, the option is later reported as unknown and passed to user space. The latter is not a real issue, but the former is confusing and makes people wonder if the tsx= parameter had any effect and double-check for typos unnecessarily. The behavior changes slightly if "tsx" is given without any argument (which is invalid syntax). Prior to this patch, the kernel logged an error message and disabled TSX. With this patch, the kernel still issues a warning (Malformed early option 'tsx'), but TSX state is unchanged. The new behavior is consistent with other parameters, e.g. "tsx_async_abort". Suggested-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> Signed-off-by: Petr Tesarik <ptesarik@suse.com> --- arch/x86/kernel/cpu/tsx.c | 52 ++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c index 8be08ece2214..74ba4abac7e9 100644 --- a/arch/x86/kernel/cpu/tsx.c +++ b/arch/x86/kernel/cpu/tsx.c @@ -20,13 +20,17 @@ #define pr_fmt(fmt) "tsx: " fmt enum tsx_ctrl_states { + TSX_CTRL_AUTO, TSX_CTRL_ENABLE, TSX_CTRL_DISABLE, TSX_CTRL_RTM_ALWAYS_ABORT, TSX_CTRL_NOT_SUPPORTED, }; -static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = TSX_CTRL_NOT_SUPPORTED; +static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = + IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_AUTO) ? TSX_CTRL_AUTO : + IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_OFF) ? TSX_CTRL_DISABLE : + TSX_CTRL_ENABLE; static void tsx_disable(void) { @@ -163,11 +167,28 @@ static void tsx_dev_mode_disable(void) } } -void __init tsx_init(void) +static int __init tsx_parse_cmdline(char *str) { - char arg[5] = {}; - int ret; + if (!str) + return -EINVAL; + + if (!strcmp(str, "on")) { + tsx_ctrl_state = TSX_CTRL_ENABLE; + } else if (!strcmp(str, "off")) { + tsx_ctrl_state = TSX_CTRL_DISABLE; + } else if (!strcmp(str, "auto")) { + tsx_ctrl_state = TSX_CTRL_AUTO; + } else { + tsx_ctrl_state = TSX_CTRL_DISABLE; + pr_err("invalid option, defaulting to off\n"); + } + + return 0; +} +early_param("tsx", tsx_parse_cmdline); +void __init tsx_init(void) +{ tsx_dev_mode_disable(); /* @@ -201,27 +222,8 @@ void __init tsx_init(void) return; } - ret = cmdline_find_option(boot_command_line, "tsx", arg, sizeof(arg)); - if (ret >= 0) { - if (!strcmp(arg, "on")) { - tsx_ctrl_state = TSX_CTRL_ENABLE; - } else if (!strcmp(arg, "off")) { - tsx_ctrl_state = TSX_CTRL_DISABLE; - } else if (!strcmp(arg, "auto")) { - tsx_ctrl_state = x86_get_tsx_auto_mode(); - } else { - tsx_ctrl_state = TSX_CTRL_DISABLE; - pr_err("invalid option, defaulting to off\n"); - } - } else { - /* tsx= not provided */ - if (IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_AUTO)) - tsx_ctrl_state = x86_get_tsx_auto_mode(); - else if (IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_OFF)) - tsx_ctrl_state = TSX_CTRL_DISABLE; - else - tsx_ctrl_state = TSX_CTRL_ENABLE; - } + if (tsx_ctrl_state == TSX_CTRL_AUTO) + tsx_ctrl_state = x86_get_tsx_auto_mode(); if (tsx_ctrl_state == TSX_CTRL_DISABLE) { tsx_disable(); -- 2.51.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] x86/tsx: Get the tsx= command line parameter with early_param() 2025-10-22 10:26 ` [PATCH v3 2/2] x86/tsx: Get the tsx= command line parameter with early_param() Petr Tesarik @ 2025-10-22 17:46 ` Pawan Gupta 2025-10-23 6:45 ` Petr Tesarik 0 siblings, 1 reply; 6+ messages in thread From: Pawan Gupta @ 2025-10-22 17:46 UTC (permalink / raw) To: Petr Tesarik Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, Dave Hansen, Nikolay Borisov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:X86 ARCHITECTURE (32-BIT AND 64-BIT) On Wed, Oct 22, 2025 at 12:26:13PM +0200, Petr Tesarik wrote: > Use early_param() to get the value of the tsx= command line parameter. It > is an early parameter, because it must be parsed before tsx_init(), which > is called long before kernel_init(), where normal parameters are parsed. > > Although cmdline_find_option() from tsx_init() works fine, the option is > later reported as unknown and passed to user space. The latter is not a > real issue, but the former is confusing and makes people wonder if the tsx= > parameter had any effect and double-check for typos unnecessarily. > > The behavior changes slightly if "tsx" is given without any argument (which > is invalid syntax). Prior to this patch, the kernel logged an error message > and disabled TSX. With this patch, the kernel still issues a warning > (Malformed early option 'tsx'), but TSX state is unchanged. The new > behavior is consistent with other parameters, e.g. "tsx_async_abort". > > Suggested-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> > Signed-off-by: Petr Tesarik <ptesarik@suse.com> > --- > arch/x86/kernel/cpu/tsx.c | 52 ++++++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 25 deletions(-) > > diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c > index 8be08ece2214..74ba4abac7e9 100644 > --- a/arch/x86/kernel/cpu/tsx.c > +++ b/arch/x86/kernel/cpu/tsx.c > @@ -20,13 +20,17 @@ > #define pr_fmt(fmt) "tsx: " fmt > > enum tsx_ctrl_states { > + TSX_CTRL_AUTO, > TSX_CTRL_ENABLE, > TSX_CTRL_DISABLE, > TSX_CTRL_RTM_ALWAYS_ABORT, > TSX_CTRL_NOT_SUPPORTED, > }; > > -static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = TSX_CTRL_NOT_SUPPORTED; > +static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = > + IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_AUTO) ? TSX_CTRL_AUTO : > + IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_OFF) ? TSX_CTRL_DISABLE : ^ The extra space I had in the version I sent was intentional. > + TSX_CTRL_ENABLE; Also this can stay on the same line. IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_AUTO) ? TSX_CTRL_AUTO : IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_OFF) ? TSX_CTRL_DISABLE : TSX_CTRL_ENABLE; IMO, this is so much more easier to read. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] x86/tsx: Get the tsx= command line parameter with early_param() 2025-10-22 17:46 ` Pawan Gupta @ 2025-10-23 6:45 ` Petr Tesarik 2025-10-23 16:05 ` Pawan Gupta 0 siblings, 1 reply; 6+ messages in thread From: Petr Tesarik @ 2025-10-23 6:45 UTC (permalink / raw) To: Pawan Gupta, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen Cc: H. Peter Anvin, Nikolay Borisov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:X86 ARCHITECTURE (32-BIT AND 64-BIT) On Wed, 22 Oct 2025 10:46:03 -0700 Pawan Gupta <pawan.kumar.gupta@linux.intel.com> wrote: > On Wed, Oct 22, 2025 at 12:26:13PM +0200, Petr Tesarik wrote: > > Use early_param() to get the value of the tsx= command line parameter. It > > is an early parameter, because it must be parsed before tsx_init(), which > > is called long before kernel_init(), where normal parameters are parsed. > > > > Although cmdline_find_option() from tsx_init() works fine, the option is > > later reported as unknown and passed to user space. The latter is not a > > real issue, but the former is confusing and makes people wonder if the tsx= > > parameter had any effect and double-check for typos unnecessarily. > > > > The behavior changes slightly if "tsx" is given without any argument (which > > is invalid syntax). Prior to this patch, the kernel logged an error message > > and disabled TSX. With this patch, the kernel still issues a warning > > (Malformed early option 'tsx'), but TSX state is unchanged. The new > > behavior is consistent with other parameters, e.g. "tsx_async_abort". > > > > Suggested-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> > > Signed-off-by: Petr Tesarik <ptesarik@suse.com> > > --- > > arch/x86/kernel/cpu/tsx.c | 52 ++++++++++++++++++++------------------- > > 1 file changed, 27 insertions(+), 25 deletions(-) > > > > diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c > > index 8be08ece2214..74ba4abac7e9 100644 > > --- a/arch/x86/kernel/cpu/tsx.c > > +++ b/arch/x86/kernel/cpu/tsx.c > > @@ -20,13 +20,17 @@ > > #define pr_fmt(fmt) "tsx: " fmt > > > > enum tsx_ctrl_states { > > + TSX_CTRL_AUTO, > > TSX_CTRL_ENABLE, > > TSX_CTRL_DISABLE, > > TSX_CTRL_RTM_ALWAYS_ABORT, > > TSX_CTRL_NOT_SUPPORTED, > > }; > > > > -static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = TSX_CTRL_NOT_SUPPORTED; > > +static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = > > + IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_AUTO) ? TSX_CTRL_AUTO : > > + IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_OFF) ? TSX_CTRL_DISABLE : > ^ > The extra space I had in > the version I sent was > intentional. > > > + TSX_CTRL_ENABLE; > > Also this can stay on the same line. > > IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_AUTO) ? TSX_CTRL_AUTO : > IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_OFF) ? TSX_CTRL_DISABLE : TSX_CTRL_ENABLE; > > IMO, this is so much more easier to read. Matter of taste if you ask me. I have no preference either way, so if you do have an opinion, let's write it your way. Do I have to resubmit, or can an x86 maintainer adjust it when applying the patch? Petr T ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] x86/tsx: Get the tsx= command line parameter with early_param() 2025-10-23 6:45 ` Petr Tesarik @ 2025-10-23 16:05 ` Pawan Gupta 0 siblings, 0 replies; 6+ messages in thread From: Pawan Gupta @ 2025-10-23 16:05 UTC (permalink / raw) To: Petr Tesarik Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin, Nikolay Borisov, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), open list:X86 ARCHITECTURE (32-BIT AND 64-BIT) On Thu, Oct 23, 2025 at 08:45:10AM +0200, Petr Tesarik wrote: > On Wed, 22 Oct 2025 10:46:03 -0700 > Pawan Gupta <pawan.kumar.gupta@linux.intel.com> wrote: > > > On Wed, Oct 22, 2025 at 12:26:13PM +0200, Petr Tesarik wrote: > > > Use early_param() to get the value of the tsx= command line parameter. It > > > is an early parameter, because it must be parsed before tsx_init(), which > > > is called long before kernel_init(), where normal parameters are parsed. > > > > > > Although cmdline_find_option() from tsx_init() works fine, the option is > > > later reported as unknown and passed to user space. The latter is not a > > > real issue, but the former is confusing and makes people wonder if the tsx= > > > parameter had any effect and double-check for typos unnecessarily. > > > > > > The behavior changes slightly if "tsx" is given without any argument (which > > > is invalid syntax). Prior to this patch, the kernel logged an error message > > > and disabled TSX. With this patch, the kernel still issues a warning > > > (Malformed early option 'tsx'), but TSX state is unchanged. The new > > > behavior is consistent with other parameters, e.g. "tsx_async_abort". > > > > > > Suggested-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> > > > Signed-off-by: Petr Tesarik <ptesarik@suse.com> > > > --- > > > arch/x86/kernel/cpu/tsx.c | 52 ++++++++++++++++++++------------------- > > > 1 file changed, 27 insertions(+), 25 deletions(-) > > > > > > diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c > > > index 8be08ece2214..74ba4abac7e9 100644 > > > --- a/arch/x86/kernel/cpu/tsx.c > > > +++ b/arch/x86/kernel/cpu/tsx.c > > > @@ -20,13 +20,17 @@ > > > #define pr_fmt(fmt) "tsx: " fmt > > > > > > enum tsx_ctrl_states { > > > + TSX_CTRL_AUTO, > > > TSX_CTRL_ENABLE, > > > TSX_CTRL_DISABLE, > > > TSX_CTRL_RTM_ALWAYS_ABORT, > > > TSX_CTRL_NOT_SUPPORTED, > > > }; > > > > > > -static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = TSX_CTRL_NOT_SUPPORTED; > > > +static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = > > > + IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_AUTO) ? TSX_CTRL_AUTO : > > > + IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_OFF) ? TSX_CTRL_DISABLE : > > ^ > > The extra space I had in > > the version I sent was > > intentional. > > > > > + TSX_CTRL_ENABLE; > > > > Also this can stay on the same line. > > > > IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_AUTO) ? TSX_CTRL_AUTO : > > IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_OFF) ? TSX_CTRL_DISABLE : TSX_CTRL_ENABLE; > > > > IMO, this is so much more easier to read. > > Matter of taste if you ask me. I have no preference either way, so if you > do have an opinion, let's write it your way. I would let the maintainers decide how they want it. > Do I have to resubmit, or can an x86 maintainer adjust it when applying > the patch? Borislav, Dave? ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-10-23 16:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-22 10:26 [PATCH v3 0/2] x86/tsx: Improve handling of the tsx= kernel parameter Petr Tesarik 2025-10-22 10:26 ` [PATCH v3 1/2] x86/tsx: Make tsx_ctrl_state static Petr Tesarik 2025-10-22 10:26 ` [PATCH v3 2/2] x86/tsx: Get the tsx= command line parameter with early_param() Petr Tesarik 2025-10-22 17:46 ` Pawan Gupta 2025-10-23 6:45 ` Petr Tesarik 2025-10-23 16:05 ` Pawan Gupta
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox