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

[Pawan Gupta <pawan.kumar.gupta@linux.intel.com>]

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]);
 }