[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [for-4.18][PATCH v2] x86/amd: Address AMD erratum #1485
On Fri, Oct 13, 2023 at 09:40:52PM +0800, Andrew Cooper wrote: > 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. Ok, I don't mind either way. I'll just go with the chickenbit. > > This will surely be a core scope MSR rather than thread scope, It is, though I doubt it matters a whole lot. The writes are consistent anyway. > at which > point 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). I have serious doubts such a conditional would do much for boot times, but sure. > > It probably also wants a note about non-atomic RMW, and how it's safe in > practice. (See the Zenbleed comment). Fair enough. > > 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 Let me send a v3 after re-testing with the conditional in place Thanks, Alejandro
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |