On 13/10/2023 9:18 pm, Alejandro Vallejo wrote:
> diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
> index 4f27187f92..085c4772d7 100644
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -1167,6 +1167,14 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
> if (c->x86 == 0x10)
> __clear_bit(X86_FEATURE_MONITOR, c->x86_capability);
>
> + /* Fix for AMD erratum #1485 */
> + if (!cpu_has_hypervisor && c->x86 == 0x19 && is_zen4_uarch()) {
> + rdmsrl(MSR_AMD64_BP_CFG, value);
> + #define ZEN4_BP_CFG_SHARED_BTB_FIX (1ull << 5)
> + wrmsrl(MSR_AMD64_BP_CFG,
> + value | ZEN4_BP_CFG_SHARED_BTB_FIX);
A #define indented like that is weird. I tend to either opencode it
directly in the "value |" expression, or have a local variable called
chickenbit.
This will surely be a core scope MSR rather than thread scope, at which
point it the write ought to be conditional on seeing the chickenbit
clear (hence needing to refer to the value at least twice, so use a
local variable).
It probably also wants a note about non-atomic RMW, and how it's safe in
practice. (See the Zenbleed comment).
Otherwise, LGTM.
As this is just cribbing from an existing example, I'm happy to adjust
on commit, but it's probably better to double check in the PPR and retest.
~Andrew
