Re: [PATCH 2/4] x86/spec-ctrl: Synthesize RSBA/RRSBA bits with older microcode

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.05.2023 13:06, Andrew Cooper wrote:
> @@ -687,6 +697,32 @@ static bool __init retpoline_calculations(void)
>      if ( safe )
>          return true;
>  
> +    /*
> +     * The meaning of the RSBA and RRSBA bits have evolved over time.  The
> +     * agreed upon meaning at the time of writing (May 2023) is thus:
> +     *
> +     * - RSBA (RSB Alterantive) means that an RSB may fall back to an
> +     *   alternative predictor on underflow.  Skylake uarch and later all 
> have
> +     *   this property.  Broadwell too, when running microcode versions prior
> +     *   to Jan 2018.
> +     *
> +     * - All eIBRS-capable processors suffer RSBA, but eIBRS also introduces
> +     *   tagging of predictions with the mode in which they were learned.  So
> +     *   when eIBRS is active, RSBA becomes RRSBA (Restricted RSBA).
> +     *
> +     * Some parts (Broadwell) are not expected to ever enumerate this
> +     * behaviour directly.  Other parts have differing enumeration with
> +     * microcode version.  Fix up Xen's idea, so we can advertise them safely
> +     * to guests, and so toolstacks can level a VM safelty for migration.
> +     */

If the difference between the two is whether eIBRS is active (as you did
word it yet more explicitly in e.g. [1]), then ...

> + unsafe_maybe_fixup_rrsba:
> +    if ( !cpu_has_rrsba )
> +        setup_force_cpu_cap(X86_FEATURE_RRSBA);
> +
> + unsafe_maybe_fixup_rsba:
> +    if ( !cpu_has_rsba )
> +        setup_force_cpu_cap(X86_FEATURE_RSBA);
> +
>      return false;
>  }

... can both actually be active at the same time? IOW is there a "return
false" missing ahead of the 2nd label?

Not having looked at further patches yet it also strikes me as odd that
each of the two labels is used exactly once only. Leaving the shared
comment aside, imo this would then better avoid "goto".

Finally, what use are the two if()s? There's nothing wrong with forcing
a feature which is already available.

Jan

[1] 
https://lore.kernel.org/lkml/f43c3c33-f8b9-e764-709d-b3864d2bd9f8@xxxxxxxxxx/