diff mbox series

[1/4] x86/spec-ctrl: Rename retpoline_safe() to retpoline_calculations()

Message ID 20230526110656.4018711-2-andrew.cooper3@citrix.com (mailing list archive)
State Superseded
Headers show
Series x86: RSBA and RRSBA handling | expand

Commit Message

Andrew Cooper May 26, 2023, 11:06 a.m. UTC 
This is prep work, split out to simply the diff on the following change.

 * Rename to retpoline_calculations(), and call unconditionally.  It is
   shortly going to synthesize missing enumerations required for guest safety.
 * For Broadwell, store the ucode revision calculation in a variable and fall
   out of the bottom of the switch statement.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/spec_ctrl.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

Comments

Jan Beulich May 30, 2023, 9:07 a.m. UTC | #1 
On 26.05.2023 13:06, Andrew Cooper wrote:
> This is prep work, split out to simply the diff on the following change.
> 
>  * Rename to retpoline_calculations(), and call unconditionally.  It is
>    shortly going to synthesize missing enumerations required for guest safety.
>  * For Broadwell, store the ucode revision calculation in a variable and fall
>    out of the bottom of the switch statement.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

I guess subsequent patches will teach me why ...

> @@ -681,6 +682,12 @@ static bool __init retpoline_safe(void)
>                 boot_cpu_data.x86_model);
>          return false;
>      }
> +
> +    /* Only Broadwell gets here. */
> +    if ( safe )
> +        return true;
> +
> +    return false;

... this isn't just "return safe;".

Jan
Andrew Cooper May 30, 2023, 9:15 a.m. UTC | #2 
On 30/05/2023 10:07 am, Jan Beulich wrote:
> On 26.05.2023 13:06, Andrew Cooper wrote:
>> This is prep work, split out to simply the diff on the following change.
>>
>>  * Rename to retpoline_calculations(), and call unconditionally.  It is
>>    shortly going to synthesize missing enumerations required for guest safety.
>>  * For Broadwell, store the ucode revision calculation in a variable and fall
>>    out of the bottom of the switch statement.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

>
> I guess subsequent patches will teach me why ...
>
>> @@ -681,6 +682,12 @@ static bool __init retpoline_safe(void)
>>                 boot_cpu_data.x86_model);
>>          return false;
>>      }
>> +
>> +    /* Only Broadwell gets here. */
>> +    if ( safe )
>> +        return true;
>> +
>> +    return false;
> ... this isn't just "return safe;".

Indeed they will.

~Andrew
diff mbox series

Patch

diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 50d467f74cf8..0774d40627dd 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -579,9 +579,10 @@  static bool __init check_smt_enabled(void)
 }
 
 /* Calculate whether Retpoline is known-safe on this CPU. */
-static bool __init retpoline_safe(void)
+static bool __init retpoline_calculations(void)
 {
     unsigned int ucode_rev = this_cpu(cpu_sig).rev;
+    bool safe = false;
 
     if ( boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
         return true;
@@ -626,18 +627,18 @@  static bool __init retpoline_safe(void)
          * versions.
          */
     case 0x3d: /* Broadwell */
-        return ucode_rev >= 0x2a;
+        safe = ucode_rev >= 0x2a;      break;
     case 0x47: /* Broadwell H */
-        return ucode_rev >= 0x1d;
+        safe = ucode_rev >= 0x1d;      break;
     case 0x4f: /* Broadwell EP/EX */
-        return ucode_rev >= 0xb000021;
+        safe = ucode_rev >= 0xb000021; break;
     case 0x56: /* Broadwell D */
         switch ( boot_cpu_data.x86_mask )
         {
-        case 2:  return ucode_rev >= 0x15;
-        case 3:  return ucode_rev >= 0x7000012;
-        case 4:  return ucode_rev >= 0xf000011;
-        case 5:  return ucode_rev >= 0xe000009;
+        case 2:  safe = ucode_rev >= 0x15;      break;
+        case 3:  safe = ucode_rev >= 0x7000012; break;
+        case 4:  safe = ucode_rev >= 0xf000011; break;
+        case 5:  safe = ucode_rev >= 0xe000009; break;
         default:
             printk("Unrecognised CPU stepping %#x - assuming not reptpoline safe\n",
                    boot_cpu_data.x86_mask);
@@ -681,6 +682,12 @@  static bool __init retpoline_safe(void)
                boot_cpu_data.x86_model);
         return false;
     }
+
+    /* Only Broadwell gets here. */
+    if ( safe )
+        return true;
+
+    return false;
 }
 
 /*
@@ -1113,7 +1120,7 @@  void __init init_speculation_mitigations(void)
 {
     enum ind_thunk thunk = THUNK_DEFAULT;
     bool has_spec_ctrl, ibrs = false, hw_smt_enabled;
-    bool cpu_has_bug_taa;
+    bool cpu_has_bug_taa, retpoline_safe;
 
     hw_smt_enabled = check_smt_enabled();
 
@@ -1139,6 +1146,9 @@  void __init init_speculation_mitigations(void)
             thunk = THUNK_JMP;
     }
 
+    /* Determine if retpoline is safe on this CPU. */
+    retpoline_safe = retpoline_calculations();
+
     /*
      * Has the user specified any custom BTI mitigations?  If so, follow their
      * instructions exactly and disable all heuristics.
@@ -1160,7 +1170,7 @@  void __init init_speculation_mitigations(void)
              * On all hardware, we'd like to use retpoline in preference to
              * IBRS, but only if it is safe on this hardware.
              */
-            if ( retpoline_safe() )
+            if ( retpoline_safe )
                 thunk = THUNK_RETPOLINE;
             else if ( has_spec_ctrl )
                 ibrs = true;