[PATCH v2 0/2] x86/tsx: Improve handling of the tsx= kernel parameter

[PATCH v2 0/2] x86/tsx: Improve handling of the tsx= kernel parameter

[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

[PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static

[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

Re: [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static

[Nikolay Borisov]

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>

Re: [PATCH v2 1/2] x86/tsx: Make tsx_ctrl_state static

[Pawan Gupta]

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>

[PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()

[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

Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()

[Nikolay Borisov]

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>

Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()

[Petr Tesarik]

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

Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()

[Petr Tesarik]

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

Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()

[Pawan Gupta]

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")) {

Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()

[Pawan Gupta]

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();

Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()

[Petr Tesarik]

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();

Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()

[Pawan Gupta]

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:

Re: [PATCH v2 2/2] x86/tsx: Get the tsx= command line parameter with early_param()

[Petr Tesarik]

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