Re: [PATCH v3 3/4] x86/spec-ctrl: Fix up the RSBA/RRSBA bits as appropriate

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.06.2023 19:25, Andrew Cooper wrote:
> On 13/06/2023 10:30 am, Jan Beulich wrote:
>> On 12.06.2023 18:13, Andrew Cooper wrote:
>>> @@ -593,15 +596,93 @@ static bool __init retpoline_calculations(void)
>>>          return false;
>>>  
>>>      /*
>>> -     * RSBA may be set by a hypervisor to indicate that we may move to a
>>> -     * processor which isn't retpoline-safe.
>>> +     * 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 Alternative) 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).
>>> +     *
>>> +     * - CPUs are not expected to enumerate both RSBA and RRSBA.
>>> +     *
>>> +     * 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 safety for migration.
>>> +     *
>>> +     * The following states exist:
>>> +     *
>>> +     * |   | RSBA | EIBRS | RRSBA | Notes              | Action        |
>>> +     * |---+------+-------+-------+--------------------+---------------|
>>> +     * | 1 |    0 |     0 |     0 | OK (older parts)   | Maybe +RSBA   |
>>> +     * | 2 |    0 |     0 |     1 | Broken             | +RSBA, -RRSBA |
>>> +     * | 3 |    0 |     1 |     0 | OK (pre-Aug ucode) | +RRSBA        |
>>> +     * | 4 |    0 |     1 |     1 | OK                 |               |
>>> +     * | 5 |    1 |     0 |     0 | OK                 |               |
>>> +     * | 6 |    1 |     0 |     1 | Broken             | -RRSBA        |
>>> +     * | 7 |    1 |     1 |     0 | Broken             | -RSBA, +RRSBA |
>>> +     * | 8 |    1 |     1 |     1 | Broken             | -RSBA         |
>> You've kept the Action column as you had it originally, despite no longer
>> applying all the fixups. Wouldn't it make sense to mark those we don't do,
>> e.g. by enclosing in parentheses?
> 
> Hmm, yes.  How does this look?
> 
> |   | RSBA | EIBRS | RRSBA | Notes              | Action (in principle) |
> |---+------+-------+-------+--------------------+-----------------------|
> | 1 |    0 |     0 |     0 | OK (older parts)   | Maybe +RSBA           |
> | 2 |    0 |     0 |     1 | Broken             | (+RSBA, -RRSBA)       |
> | 3 |    0 |     1 |     0 | OK (pre-Aug ucode) | +RRSBA                |
> | 4 |    0 |     1 |     1 | OK                 |                       |
> | 5 |    1 |     0 |     0 | OK                 |                       |
> | 6 |    1 |     0 |     1 | Broken             | (-RRSBA)              |
> | 7 |    1 |     1 |     0 | Broken             | (-RSBA, +RRSBA)       |
> | 8 |    1 |     1 |     1 | Broken             | (-RSBA)               |

Yes, I think it's better to have it this way, thanks.

Jan