[PATCH v2 1/7] x86/retbleed: Check for AUTO in all cases

Pawan Gupta posted 7 patches 6 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 1/7] x86/retbleed: Check for AUTO in all cases
Posted by Pawan Gupta 6 months, 3 weeks ago
When none of mitigation option is selected, AUTO gets converted to NONE.
This is currently only being done for Intel. The check is useful in
general, make it common.

Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/kernel/cpu/bugs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 7f94e6a5497d9a2d312a76095e48d6b364565777..19ff705b3128eacad5659990ed345d7a19bcb0f4 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1294,15 +1294,15 @@ static void __init retbleed_update_mitigation(void)
 			if (retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
 				pr_err(RETBLEED_INTEL_MSG);
 		}
-		/* If nothing has set the mitigation yet, default to NONE. */
-		if (retbleed_mitigation == RETBLEED_MITIGATION_AUTO)
-			retbleed_mitigation = RETBLEED_MITIGATION_NONE;
 	}
+
+	/* If nothing has set the mitigation yet, default to NONE. */
+	if (retbleed_mitigation == RETBLEED_MITIGATION_AUTO)
+		retbleed_mitigation = RETBLEED_MITIGATION_NONE;
 out:
 	pr_info("%s\n", retbleed_strings[retbleed_mitigation]);
 }
 
-
 static void __init retbleed_apply_mitigation(void)
 {
 	bool mitigate_smt = false;

-- 
2.34.1
Re: [PATCH v2 1/7] x86/retbleed: Check for AUTO in all cases
Posted by Borislav Petkov 6 months, 3 weeks ago
On Wed, May 21, 2025 at 07:44:22PM -0700, Pawan Gupta wrote:
> When none of mitigation option is selected, AUTO gets converted to NONE.
> This is currently only being done for Intel. The check is useful in
> general, make it common.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/bugs.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 7f94e6a5497d9a2d312a76095e48d6b364565777..19ff705b3128eacad5659990ed345d7a19bcb0f4 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -1294,15 +1294,15 @@ static void __init retbleed_update_mitigation(void)
>  			if (retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
>  				pr_err(RETBLEED_INTEL_MSG);
>  		}
> -		/* If nothing has set the mitigation yet, default to NONE. */
> -		if (retbleed_mitigation == RETBLEED_MITIGATION_AUTO)
> -			retbleed_mitigation = RETBLEED_MITIGATION_NONE;
>  	}
> +
> +	/* If nothing has set the mitigation yet, default to NONE. */
> +	if (retbleed_mitigation == RETBLEED_MITIGATION_AUTO)
> +		retbleed_mitigation = RETBLEED_MITIGATION_NONE;
>  out:
>  	pr_info("%s\n", retbleed_strings[retbleed_mitigation]);
>  }

So, the way I see it is, AUTO means user didn't select anything so we will
select the default thing.
 
And we do that in the select function.

But then in the update function we bring back AUTO from the dead again,
forcing us to having to deal with it, well, again.

So can we simply set to RETBLEED_MITIGATION_NONE in the retbleed + its = stuff
option when SPECTRE_V2_RETPOLINE not selected?

This'll get rid of the AUTO crap.

For that, the select function should probably select something else from AUTO
on Intel too.

My point is, let's deal with AUTO in the select functions only and then forget
it from then on...

Right?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 1/7] x86/retbleed: Check for AUTO in all cases
Posted by Pawan Gupta 6 months, 3 weeks ago
On Thu, May 22, 2025 at 01:27:30PM +0200, Borislav Petkov wrote:
> On Wed, May 21, 2025 at 07:44:22PM -0700, Pawan Gupta wrote:
> > When none of mitigation option is selected, AUTO gets converted to NONE.
> > This is currently only being done for Intel. The check is useful in
> > general, make it common.
> > 
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> > ---
> >  arch/x86/kernel/cpu/bugs.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> > index 7f94e6a5497d9a2d312a76095e48d6b364565777..19ff705b3128eacad5659990ed345d7a19bcb0f4 100644
> > --- a/arch/x86/kernel/cpu/bugs.c
> > +++ b/arch/x86/kernel/cpu/bugs.c
> > @@ -1294,15 +1294,15 @@ static void __init retbleed_update_mitigation(void)
> >  			if (retbleed_mitigation != RETBLEED_MITIGATION_STUFF)
> >  				pr_err(RETBLEED_INTEL_MSG);
> >  		}
> > -		/* If nothing has set the mitigation yet, default to NONE. */
> > -		if (retbleed_mitigation == RETBLEED_MITIGATION_AUTO)
> > -			retbleed_mitigation = RETBLEED_MITIGATION_NONE;
> >  	}
> > +
> > +	/* If nothing has set the mitigation yet, default to NONE. */
> > +	if (retbleed_mitigation == RETBLEED_MITIGATION_AUTO)
> > +		retbleed_mitigation = RETBLEED_MITIGATION_NONE;
> >  out:
> >  	pr_info("%s\n", retbleed_strings[retbleed_mitigation]);
> >  }
> 
> So, the way I see it is, AUTO means user didn't select anything so we will
> select the default thing.
>  
> And we do that in the select function.
> 
> But then in the update function we bring back AUTO from the dead again,
> forcing us to having to deal with it, well, again.
> 
> So can we simply set to RETBLEED_MITIGATION_NONE in the retbleed + its = stuff
> option when SPECTRE_V2_RETPOLINE not selected?

That can be done.

> This'll get rid of the AUTO crap.

Yup.

> For that, the select function should probably select something else from AUTO
> on Intel too.
> 
> My point is, let's deal with AUTO in the select functions only and then forget
> it from then on...
> 
> Right?

Yes, makes sense to me.

This is how it is looking:

---
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index ad04da8711b2..156fee146228 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -1264,6 +1264,13 @@ static void __init retbleed_select_mitigation(void)
 			retbleed_mitigation = RETBLEED_MITIGATION_IBPB;
 		else
 			retbleed_mitigation = RETBLEED_MITIGATION_NONE;
+	} else if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) {
+		if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
+			retbleed_mitigation = RETBLEED_MITIGATION_EIBRS;
+		else if (boot_cpu_has(X86_FEATURE_IBRS))
+			retbleed_mitigation = RETBLEED_MITIGATION_IBRS;
+		else
+			retbleed_mitigation = RETBLEED_MITIGATION_NONE;
 	}
 }
 
@@ -1272,9 +1279,6 @@ static void __init retbleed_update_mitigation(void)
 	if (!boot_cpu_has_bug(X86_BUG_RETBLEED) || cpu_mitigations_off())
 		return;
 
-	if (retbleed_mitigation == RETBLEED_MITIGATION_NONE)
-		goto out;
-
 	 /* ITS can also enable stuffing */
 	if (its_mitigation == ITS_MITIGATION_RETPOLINE_STUFF)
 		retbleed_mitigation = RETBLEED_MITIGATION_STUFF;
@@ -1282,7 +1286,7 @@ static void __init retbleed_update_mitigation(void)
 	if (retbleed_mitigation == RETBLEED_MITIGATION_STUFF &&
 	    !cdt_possible(spectre_v2_enabled)) {
 		pr_err("WARNING: retbleed=stuff depends on retpoline\n");
-		retbleed_mitigation = RETBLEED_MITIGATION_AUTO;
+		retbleed_mitigation = RETBLEED_MITIGATION_NONE;
 	}
 
 	/*
@@ -1305,10 +1309,6 @@ static void __init retbleed_update_mitigation(void)
 		}
 	}
 
-	/* If nothing has set the mitigation yet, default to NONE. */
-	if (retbleed_mitigation == RETBLEED_MITIGATION_AUTO)
-		retbleed_mitigation = RETBLEED_MITIGATION_NONE;
-out:
 	pr_info("%s\n", retbleed_strings[retbleed_mitigation]);
 }
Re: [PATCH v2 1/7] x86/retbleed: Check for AUTO in all cases
Posted by Nikolay Borisov 6 months, 3 weeks ago

On 5/22/25 05:44, Pawan Gupta wrote:
> When none of mitigation option is selected, AUTO gets converted to NONE.
> This is currently only being done for Intel. The check is useful in
> general, make it common.
> 
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>

Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>