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