[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD
On Fri, Apr 29, 2022 at 05:49:42PM +0200, Roger Pau Monné wrote: > On Fri, Apr 29, 2022 at 12:59:58PM +0200, Jan Beulich wrote: > > On 27.04.2022 12:47, Roger Pau Monne wrote:> Changes since v3:> - Align > > ssbd per-core struct to a cache line.> - Open code a simple spinlock to > > avoid playing tricks with the lock> detector.> - > > s/ssbd_core/ssbd_ls_cfg/.> - Fix log message wording.> - Fix define name > > and remove comment.> - Also handle Hygon processors (Fam18h).> - Add > > changelog entry. > > What is this last line about? > > Hm, seems like I forgot to do a patch refresh... So new version will > have an entry about adding VIRT_SSBD support to HVM guests. Sorry for > spoiling the surprise. > > > > +bool __init amd_setup_legacy_ssbd(void) > > > +{ > > > + unsigned int i; > > > + > > > + if ((boot_cpu_data.x86 != 0x17 && boot_cpu_data.x86 != 0x18) || > > > + boot_cpu_data.x86_num_siblings <= 1) > > > + return true; > > > + > > > + /* > > > + * One could be forgiven for thinking that c->x86_max_cores is the > > > + * correct value to use here. > > > + * > > > + * However, that value is derived from the current configuration, and > > > + * c->cpu_core_id is sparse on all but the top end CPUs. Derive > > > + * max_cpus from ApicIdCoreIdSize which will cover any sparseness. > > > + */ > > > + if (boot_cpu_data.extended_cpuid_level >= 0x80000008) { > > > + ssbd_max_cores = 1u << MASK_EXTR(cpuid_ecx(0x80000008), 0xf000); > > > + ssbd_max_cores /= boot_cpu_data.x86_num_siblings; > > > + } > > > + if (!ssbd_max_cores) > > > + return false; > > > + > > > + ssbd_ls_cfg = xzalloc_array(struct ssbd_ls_cfg, > > > + ssbd_max_cores * AMD_FAM17H_MAX_SOCKETS); > > > + if (!ssbd_ls_cfg) > > > + return false; > > > + > > > + for (i = 0; i < ssbd_max_cores * AMD_FAM17H_MAX_SOCKETS; i++) > > > + /* Record initial state, also applies to any hotplug CPU. */ > > > + if (opt_ssbd) > > > + ssbd_ls_cfg[i].count = boot_cpu_data.x86_num_siblings; > > > > Perhaps flip if() and for()? > > Indeed, thanks. > > > > +void amd_set_legacy_ssbd(bool enable) > > > +{ > > > + const struct cpuinfo_x86 *c = ¤t_cpu_data; > > > + struct ssbd_ls_cfg *status; > > > + > > > + if (c->x86 != 0x17 || c->x86_num_siblings <= 1) { > > > + BUG_ON(!set_legacy_ssbd(c, enable)); > > > + return; > > > + } > > > + > > > + BUG_ON(c->phys_proc_id >= AMD_FAM17H_MAX_SOCKETS); > > > + BUG_ON(c->cpu_core_id >= ssbd_max_cores); > > > + status = &ssbd_ls_cfg[c->phys_proc_id * ssbd_max_cores + > > > + c->cpu_core_id]; > > > + > > > + /* > > > + * Open code a very simple spinlock: this function is used with GIF==0 > > > + * and different IF values, so would trigger the checklock detector. > > > + * Instead of trying to workaround the detector, use a very simple lock > > > + * implementation: it's better to reduce the amount of code executed > > > + * with GIF==0. > > > + */ > > > + while ( test_and_set_bool(status->locked) ) > > > + cpu_relax(); > > > + status->count += enable ? 1 : -1; > > > + ASSERT(status->count <= c->x86_num_siblings); > > > + if (enable ? status->count == 1 : !status->count) > > > + BUG_ON(!set_legacy_ssbd(c, enable)); > > > > What are the effects of ASSERT() or BUG_ON() triggering in a GIF=0 > > region? > > So AFAICT the BUG itself works, the usage of a crash kernel however > won't work as it's booted with GIF==0. > > Maybe we need to issue an stgi as part of BUG_FRAME if required? > (maybe that's too naive...) Well, better in panic() or kexec_crash() likely. Roger.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |