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) |
>> + * further investigation.
>> + */
>> + if ( cpu_has_eibrs ? cpu_has_rsba /* Rows 7, 8 */
>> + : cpu_has_rrsba /* Rows 2, 6 */ )
>> + {
>> + printk(XENLOG_ERR
>> + "FIRMWARE BUG: CPU %02x-%02x-%02x, ucode 0x%08x: RSBA %u,
>> EIBRS %u, RRSBA %u\n",
>> + boot_cpu_data.x86, boot_cpu_data.x86_model,
>> + boot_cpu_data.x86_mask, ucode_rev,
>> + cpu_has_rsba, cpu_has_eibrs, cpu_has_rrsba);
> Perhaps with adjustments (as you deem them sensible)
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
Thanks.
~Andrew
