* [PATCH v2 0/2] x86/tsx: Improve handling of the tsx= kernel parameter
@ 2025-09-26 18:01 Petr Tesarik
2025-09-26 18:01 ` [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static Petr Tesarik
2025-09-26 18:01 ` [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param() Petr Tesarik
0 siblings, 2 replies; 13+ messages in thread
From: Petr Tesarik @ 2025-09-26 18:01 UTC (permalink / raw)
To: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen
Cc: Pawan Gupta, 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.
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 | 47 +++++++++++++++++++++++++--------------
2 files changed, 30 insertions(+), 26 deletions(-)
--
2.50.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static
2025-09-26 18:01 [PATCH v2 0/2] x86/tsx: Improve handling of the tsx= kernel parameter Petr Tesarik
@ 2025-09-26 18:01 ` Petr Tesarik
2025-10-07 12:40 ` Nikolay Borisov
2025-10-09 17:58 ` Pawan Gupta
2025-09-26 18:01 ` [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param() Petr Tesarik
1 sibling, 2 replies; 13+ messages in thread
From: Petr Tesarik @ 2025-09-26 18:01 UTC (permalink / raw)
To: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen
Cc: Pawan Gupta, 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>
---
arch/x86/kernel/cpu/cpu.h | 9 ---------
arch/x86/kernel/cpu/tsx.c | 10 +++++++++-
2 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h
index bc38b2d56f26a..5c7a3a71191a1 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 49782724a9430..167dfd38b87a2 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -19,7 +19,15 @@
#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.50.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()
2025-09-26 18:01 [PATCH v2 0/2] x86/tsx: Improve handling of the tsx= kernel parameter Petr Tesarik
2025-09-26 18:01 ` [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static Petr Tesarik
@ 2025-09-26 18:01 ` Petr Tesarik
2025-10-07 12:50 ` Nikolay Borisov
2025-10-09 18:51 ` Pawan Gupta
1 sibling, 2 replies; 13+ messages in thread
From: Petr Tesarik @ 2025-09-26 18:01 UTC (permalink / raw)
To: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen
Cc: Pawan Gupta, 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.
Although cmdline_find_option() 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 parameter is ignored. The new
behavior is consistent with other parameters, e.g. "tsx_async_abort".
Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
arch/x86/kernel/cpu/tsx.c | 41 ++++++++++++++++++++++-----------------
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
index 167dfd38b87a2..bb407331f64b5 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -20,14 +20,14 @@
#define pr_fmt(fmt) "tsx: " fmt
enum tsx_ctrl_states {
+ TSC_CTRL_UNSPECIFIED,
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;
static void tsx_disable(void)
{
@@ -164,11 +164,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 = x86_get_tsx_auto_mode();
+ } 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();
/*
@@ -202,19 +219,7 @@ 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 {
+ if (tsx_ctrl_state == TSC_CTRL_UNSPECIFIED) {
/* tsx= not provided */
if (IS_ENABLED(CONFIG_X86_INTEL_TSX_MODE_AUTO))
tsx_ctrl_state = x86_get_tsx_auto_mode();
--
2.50.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static
2025-09-26 18:01 ` [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static Petr Tesarik
@ 2025-10-07 12:40 ` Nikolay Borisov
2025-10-09 17:58 ` Pawan Gupta
1 sibling, 0 replies; 13+ messages in thread
From: Nikolay Borisov @ 2025-10-07 12:40 UTC (permalink / raw)
To: Petr Tesarik, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen
Cc: Pawan Gupta, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On 26.09.25 г. 21:01 ч., Petr Tesarik wrote:
> 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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()
2025-09-26 18:01 ` [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param() Petr Tesarik
@ 2025-10-07 12:50 ` Nikolay Borisov
2025-10-08 14:03 ` Petr Tesarik
2025-10-09 18:51 ` Pawan Gupta
1 sibling, 1 reply; 13+ messages in thread
From: Nikolay Borisov @ 2025-10-07 12:50 UTC (permalink / raw)
To: Petr Tesarik, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen
Cc: Pawan Gupta, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On 26.09.25 г. 21:01 ч., Petr Tesarik wrote:
> Use early_param() to get the value of the tsx= command line parameter.
> Although cmdline_find_option() 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 parameter is ignored. The new
> behavior is consistent with other parameters, e.g. "tsx_async_abort".
>
> Signed-off-by: Petr Tesarik <ptesarik@suse.com>
LGTM, also could you include some rationale why early_param vs __setup
for example, or arch_param_cb (which by the way has yet to find its
first user).
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()
2025-10-07 12:50 ` Nikolay Borisov
@ 2025-10-08 14:03 ` Petr Tesarik
2025-10-22 16:31 ` Petr Tesarik
0 siblings, 1 reply; 13+ messages in thread
From: Petr Tesarik @ 2025-10-08 14:03 UTC (permalink / raw)
To: Nikolay Borisov, Borislav Petkov
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Dave Hansen,
Pawan Gupta, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Tue, 7 Oct 2025 15:50:02 +0300
Nikolay Borisov <nik.borisov@suse.com> wrote:
> On 26.09.25 г. 21:01 ч., Petr Tesarik wrote:
> > Use early_param() to get the value of the tsx= command line parameter.
> > Although cmdline_find_option() 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 parameter is ignored. The new
> > behavior is consistent with other parameters, e.g. "tsx_async_abort".
> >
> > Signed-off-by: Petr Tesarik <ptesarik@suse.com>
>
> LGTM, also could you include some rationale why early_param vs __setup
> for example, or arch_param_cb (which by the way has yet to find its
> first user).
I'd love to, but I'm a noob myself, so I simply followed Borislav's
advice here:
https://lore.kernel.org/all/20250915143909.GAaMglDd5oRSPDDuqu@fat_crate.local/
@Borislav: Would you mind explaining your request to use early_param()?
Petr T
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static
2025-09-26 18:01 ` [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static Petr Tesarik
2025-10-07 12:40 ` Nikolay Borisov
@ 2025-10-09 17:58 ` Pawan Gupta
1 sibling, 0 replies; 13+ messages in thread
From: Pawan Gupta @ 2025-10-09 17:58 UTC (permalink / raw)
To: Petr Tesarik
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Fri, Sep 26, 2025 at 08:01:01PM +0200, Petr Tesarik wrote:
> 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>
> ---
> arch/x86/kernel/cpu/cpu.h | 9 ---------
> arch/x86/kernel/cpu/tsx.c | 10 +++++++++-
> 2 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h
> index bc38b2d56f26a..5c7a3a71191a1 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 49782724a9430..167dfd38b87a2 100644
> --- a/arch/x86/kernel/cpu/tsx.c
> +++ b/arch/x86/kernel/cpu/tsx.c
> @@ -19,7 +19,15 @@
> #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;
Nit, this can be on the same line:
static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init = TSX_CTRL_NOT_SUPPORTED;
It is still under the 100 character limit.
Reviewed-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()
2025-09-26 18:01 ` [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param() Petr Tesarik
2025-10-07 12:50 ` Nikolay Borisov
@ 2025-10-09 18:51 ` Pawan Gupta
2025-10-10 0:33 ` Pawan Gupta
2025-10-10 7:40 ` Petr Tesarik
1 sibling, 2 replies; 13+ messages in thread
From: Pawan Gupta @ 2025-10-09 18:51 UTC (permalink / raw)
To: Petr Tesarik
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Fri, Sep 26, 2025 at 08:01:02PM +0200, Petr Tesarik wrote:
> Use early_param() to get the value of the tsx= command line parameter.
> Although cmdline_find_option() 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 parameter is ignored. The new
> behavior is consistent with other parameters, e.g. "tsx_async_abort".
>
> Signed-off-by: Petr Tesarik <ptesarik@suse.com>
> ---
> arch/x86/kernel/cpu/tsx.c | 41 ++++++++++++++++++++++-----------------
> 1 file changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
> index 167dfd38b87a2..bb407331f64b5 100644
> --- a/arch/x86/kernel/cpu/tsx.c
> +++ b/arch/x86/kernel/cpu/tsx.c
> @@ -20,14 +20,14 @@
> #define pr_fmt(fmt) "tsx: " fmt
>
> enum tsx_ctrl_states {
> + TSC_CTRL_UNSPECIFIED,
s/TSC/TSX/
> 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;
>
> static void tsx_disable(void)
> {
> @@ -164,11 +164,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 = x86_get_tsx_auto_mode();
NACK, this introduces a subtle bug. With this change x86_get_tsx_auto_mode()
would return TSX_CTRL_ENABLE always, irrespective of whether the CPU has
X86_BUG_TAA or not. This is because early_param() is executed before
cpu_set_bug_bits().
> + } else {
> + tsx_ctrl_state = TSX_CTRL_DISABLE;
> + pr_err("invalid option, defaulting to off\n");
> + }
>
> + return 0;
> +}
> +early_param("tsx", tsx_parse_cmdline);
Rather, a patch to add a comment would be better.
---
diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
index 49782724a943..8fd1c16a38ec 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -194,6 +194,11 @@ void __init tsx_init(void)
return;
}
+ /*
+ * Cmdline is not processed via early_param() to ensure
+ * cpu_set_bug_bits() is executed already. And x86_get_tsx_auto_mode()
+ * returns the appropriate default based on X86_BUG_TAA.
+ */
ret = cmdline_find_option(boot_command_line, "tsx", arg, sizeof(arg));
if (ret >= 0) {
if (!strcmp(arg, "on")) {
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()
2025-10-09 18:51 ` Pawan Gupta
@ 2025-10-10 0:33 ` Pawan Gupta
2025-10-10 7:45 ` Petr Tesarik
2025-10-10 7:40 ` Petr Tesarik
1 sibling, 1 reply; 13+ messages in thread
From: Pawan Gupta @ 2025-10-10 0:33 UTC (permalink / raw)
To: Petr Tesarik
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Thu, Oct 09, 2025 at 11:51:40AM -0700, Pawan Gupta wrote:
> On Fri, Sep 26, 2025 at 08:01:02PM +0200, Petr Tesarik wrote:
> > Use early_param() to get the value of the tsx= command line parameter.
> > Although cmdline_find_option() 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.
If this is too much of an annoyance, I would suggest to move
x86_get_tsx_auto_mode() from tsx_parse_cmdline() to tsx_init().
---
diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
index 167dfd38b87a..805be8beb37a 100644
--- a/arch/x86/kernel/cpu/tsx.c
+++ b/arch/x86/kernel/cpu/tsx.c
@@ -20,6 +20,7 @@
#define pr_fmt(fmt) "tsx: " fmt
enum tsx_ctrl_states {
+ TSX_CTRL_AUTO,
TSX_CTRL_ENABLE,
TSX_CTRL_DISABLE,
TSX_CTRL_RTM_ALWAYS_ABORT,
@@ -27,7 +28,8 @@ enum tsx_ctrl_states {
};
static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init =
- TSX_CTRL_NOT_SUPPORTED;
+ 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)
{
@@ -164,11 +166,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();
/*
@@ -202,27 +221,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();
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()
2025-10-09 18:51 ` Pawan Gupta
2025-10-10 0:33 ` Pawan Gupta
@ 2025-10-10 7:40 ` Petr Tesarik
1 sibling, 0 replies; 13+ messages in thread
From: Petr Tesarik @ 2025-10-10 7:40 UTC (permalink / raw)
To: Pawan Gupta
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Thu, 9 Oct 2025 11:51:34 -0700
Pawan Gupta <pawan.kumar.gupta@linux.intel.com> wrote:
> On Fri, Sep 26, 2025 at 08:01:02PM +0200, Petr Tesarik wrote:
> > Use early_param() to get the value of the tsx= command line parameter.
> > Although cmdline_find_option() 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 parameter is ignored. The new
> > behavior is consistent with other parameters, e.g. "tsx_async_abort".
> >
> > Signed-off-by: Petr Tesarik <ptesarik@suse.com>
> > ---
> > arch/x86/kernel/cpu/tsx.c | 41 ++++++++++++++++++++++-----------------
> > 1 file changed, 23 insertions(+), 18 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
> > index 167dfd38b87a2..bb407331f64b5 100644
> > --- a/arch/x86/kernel/cpu/tsx.c
> > +++ b/arch/x86/kernel/cpu/tsx.c
> > @@ -20,14 +20,14 @@
> > #define pr_fmt(fmt) "tsx: " fmt
> >
> > enum tsx_ctrl_states {
> > + TSC_CTRL_UNSPECIFIED,
>
> s/TSC/TSX/
Ouch. Yes. ;-)
> > 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;
> >
> > static void tsx_disable(void)
> > {
> > @@ -164,11 +164,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 = x86_get_tsx_auto_mode();
>
> NACK, this introduces a subtle bug. With this change x86_get_tsx_auto_mode()
> would return TSX_CTRL_ENABLE always, irrespective of whether the CPU has
> X86_BUG_TAA or not. This is because early_param() is executed before
> cpu_set_bug_bits().
Thank you. I was afraid of some sort of init ordering issues, and I
could not test on a CPU with X86_BUG_TAA, unfortunately (because I have
none at hand).
> > + } else {
> > + tsx_ctrl_state = TSX_CTRL_DISABLE;
> > + pr_err("invalid option, defaulting to off\n");
> > + }
> >
> > + return 0;
> > +}
> > +early_param("tsx", tsx_parse_cmdline);
>
> Rather, a patch to add a comment would be better.
Well, some explanation would also have to go into
Documentation/admin-guide/kernel-parameters.txt
I'll consider the other proposed approach.
Thank you!
Petr T
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()
2025-10-10 0:33 ` Pawan Gupta
@ 2025-10-10 7:45 ` Petr Tesarik
2025-10-10 17:52 ` Pawan Gupta
0 siblings, 1 reply; 13+ messages in thread
From: Petr Tesarik @ 2025-10-10 7:45 UTC (permalink / raw)
To: Pawan Gupta
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Thu, 9 Oct 2025 17:33:45 -0700
Pawan Gupta <pawan.kumar.gupta@linux.intel.com> wrote:
> On Thu, Oct 09, 2025 at 11:51:40AM -0700, Pawan Gupta wrote:
> > On Fri, Sep 26, 2025 at 08:01:02PM +0200, Petr Tesarik wrote:
> > > Use early_param() to get the value of the tsx= command line parameter.
> > > Although cmdline_find_option() 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.
>
> If this is too much of an annoyance, I would suggest to move
> x86_get_tsx_auto_mode() from tsx_parse_cmdline() to tsx_init().
>
> ---
> diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
> index 167dfd38b87a..805be8beb37a 100644
> --- a/arch/x86/kernel/cpu/tsx.c
> +++ b/arch/x86/kernel/cpu/tsx.c
> @@ -20,6 +20,7 @@
> #define pr_fmt(fmt) "tsx: " fmt
>
> enum tsx_ctrl_states {
> + TSX_CTRL_AUTO,
> TSX_CTRL_ENABLE,
> TSX_CTRL_DISABLE,
> TSX_CTRL_RTM_ALWAYS_ABORT,
> @@ -27,7 +28,8 @@ enum tsx_ctrl_states {
> };
>
> static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init =
> - TSX_CTRL_NOT_SUPPORTED;
> + 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;
I like this approach, because it converts runtime initialization code
to a build-time initializer, based on build-time config options.
Can I add your Signed-off-by: for a v3?
Petr T
> static void tsx_disable(void)
> {
> @@ -164,11 +166,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();
>
> /*
> @@ -202,27 +221,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();
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()
2025-10-10 7:45 ` Petr Tesarik
@ 2025-10-10 17:52 ` Pawan Gupta
0 siblings, 0 replies; 13+ messages in thread
From: Pawan Gupta @ 2025-10-10 17:52 UTC (permalink / raw)
To: Petr Tesarik
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Fri, Oct 10, 2025 at 09:45:19AM +0200, Petr Tesarik wrote:
> On Thu, 9 Oct 2025 17:33:45 -0700
> Pawan Gupta <pawan.kumar.gupta@linux.intel.com> wrote:
>
> > On Thu, Oct 09, 2025 at 11:51:40AM -0700, Pawan Gupta wrote:
> > > On Fri, Sep 26, 2025 at 08:01:02PM +0200, Petr Tesarik wrote:
> > > > Use early_param() to get the value of the tsx= command line parameter.
> > > > Although cmdline_find_option() 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.
> >
> > If this is too much of an annoyance, I would suggest to move
> > x86_get_tsx_auto_mode() from tsx_parse_cmdline() to tsx_init().
> >
> > ---
> > diff --git a/arch/x86/kernel/cpu/tsx.c b/arch/x86/kernel/cpu/tsx.c
> > index 167dfd38b87a..805be8beb37a 100644
> > --- a/arch/x86/kernel/cpu/tsx.c
> > +++ b/arch/x86/kernel/cpu/tsx.c
> > @@ -20,6 +20,7 @@
> > #define pr_fmt(fmt) "tsx: " fmt
> >
> > enum tsx_ctrl_states {
> > + TSX_CTRL_AUTO,
> > TSX_CTRL_ENABLE,
> > TSX_CTRL_DISABLE,
> > TSX_CTRL_RTM_ALWAYS_ABORT,
> > @@ -27,7 +28,8 @@ enum tsx_ctrl_states {
> > };
> >
> > static enum tsx_ctrl_states tsx_ctrl_state __ro_after_init =
> > - TSX_CTRL_NOT_SUPPORTED;
> > + 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;
>
> I like this approach, because it converts runtime initialization code
> to a build-time initializer, based on build-time config options.
>
> Can I add your Signed-off-by: for a v3?
You may keep the authorship and add my Suggested-by:
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()
2025-10-08 14:03 ` Petr Tesarik
@ 2025-10-22 16:31 ` Petr Tesarik
0 siblings, 0 replies; 13+ messages in thread
From: Petr Tesarik @ 2025-10-22 16:31 UTC (permalink / raw)
To: Nikolay Borisov, Borislav Petkov
Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, Dave Hansen,
Pawan Gupta, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)
On Wed, 8 Oct 2025 16:03:43 +0200
Petr Tesarik <ptesarik@suse.com> wrote:
> On Tue, 7 Oct 2025 15:50:02 +0300
> Nikolay Borisov <nik.borisov@suse.com> wrote:
>
> > On 26.09.25 г. 21:01 ч., Petr Tesarik wrote:
> > > Use early_param() to get the value of the tsx= command line parameter.
> > > Although cmdline_find_option() 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 parameter is ignored. The new
> > > behavior is consistent with other parameters, e.g. "tsx_async_abort".
> > >
> > > Signed-off-by: Petr Tesarik <ptesarik@suse.com>
> >
> > LGTM, also could you include some rationale why early_param vs __setup
> > for example, or arch_param_cb (which by the way has yet to find its
> > first user).
>
> I'd love to, but I'm a noob myself, so I simply followed Borislav's
> advice here:
>
> https://lore.kernel.org/all/20250915143909.GAaMglDd5oRSPDDuqu@fat_crate.local/
>
> @Borislav: Would you mind explaining your request to use early_param()?
Never mind, I have spent some time on it, so here's a dump of how I
understand it:
https://sigillatum.tesarici.cz/2025-10-22-kernel-parameters.html
HTH
Petr T
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-10-22 16:31 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-26 18:01 [PATCH v2 0/2] x86/tsx: Improve handling of the tsx= kernel parameter Petr Tesarik
2025-09-26 18:01 ` [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static Petr Tesarik
2025-10-07 12:40 ` Nikolay Borisov
2025-10-09 17:58 ` Pawan Gupta
2025-09-26 18:01 ` [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param() Petr Tesarik
2025-10-07 12:50 ` Nikolay Borisov
2025-10-08 14:03 ` Petr Tesarik
2025-10-22 16:31 ` Petr Tesarik
2025-10-09 18:51 ` Pawan Gupta
2025-10-10 0:33 ` Pawan Gupta
2025-10-10 7:45 ` Petr Tesarik
2025-10-10 17:52 ` Pawan Gupta
2025-10-10 7:40 ` Petr Tesarik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox