On Mon, Oct 18, 2021 at 09:06:07PM +0100, Andrew Cooper wrote:
... this is 0x18 for Hygon, and ...
Sure, whatever :)
/* All the remaining ones are affected */ set_cpu_bug(c, X86_BUG_NULL_SEG);
... hypervisor && !ncsb still needs to set BUG_NULL_SEG, so you really can't exit early.
Yeah, we had a session on IRC, we came up with this rough version, more polishing tomorrow:
static void early_probe_null_seg_clearing_base(struct cpuinfo_x86 *c) {
/* Zen3 CPUs advertise Null Selector Clears Base in CPUID. */ if (c->extended_cpuid_level >= 0x80000021 && cpuid_eax(0x80000021) & BIT(6)) return;
/* * CPUID bit above wasn't set. If this kernel is still running as a HV guest, * then the HV has decided not to advertize that CPUID bit for whatever reason. * For example, one member of the migration pool might be vulnerable. * Which means, the bug is present: set the BUG flag and return. */ if (cpu_has(c, X86_FEATURE_HYPERVISOR)) { set_cpu_bug(c, X86_BUG_NULL_SEG); return; }
/* Zen2 CPUs also have this behaviour, but no CPUID bit. 0x18 for Hygon. */ if ((c->x86 == 0x17 || c->x86 == 0x18) && check_null_seg_clears_base(c)) return;
/* All the remaining ones are affected */ set_cpu_bug(c, X86_BUG_NULL_SEG); }
So I really want to have those comments explaining each step in the complex check because we will forget why this crazy dance is being done and as I said in a previous thread, we're not all virtualizers. :)
No other CPU vendors are known to have this issue.
How do you know? Or should there be a comment along the lines of "Cooper says that..."
:-)
(And by "issue", even this is complicated. Back in the 32bit days, it was a plausible perf improvement, but it backfired massively for AMD64 where there was a possibility/expectation to use NULL segments.)
Andy only put the check in unilaterally just in case, and even that was fine-ish until AMD went and fixed it silently in Zen2.
Yeah, there's the context switch overhead too but that's for another thread.
Because if this null seg behavior detection should happen on all CPUs - and I think it should, because, well, it has been that way until now - then the vendor specific identification minus what detect_null_seg_behavior() does should run first and then after ->c_identify() is done, you should do something like:
if (!cpu_has_bug(c, X86_BUG_NULL_SEG)) { if (!check_null_seg_clears_base(c)) set_cpu_bug(c, X86_BUG_NULL_SEG); }
so that it still takes place on all CPUs.
This would only really work for boot cpu and setup_force_cap(), because no CPU is going to have X86_BUG_NULL_SEG set by default, but this still misses the point of the bugfix which is "check_null_seg_clears_base() must not be called when cpu_has_hypervisor".
In practice, the BSP is good enough. The behaviour predates the K8, which was the first CPU where it became observable without SMM/PUSHALL/etc, and quite possibly goes back to the dawn of time, and you can't mix a Zen1 and Zen2 in a 2-socket system.
Oh, I didn't express myself properly "should happen on all CPUs" was supposed to mean, if this detection should happen on all vendors like it does now. Not BSP vs AP.
It is made unused by this patch, so can't be pulled out earlier, but should be adjusted.
Right.