On Fri, Aug 21, 2020 at 10:22 AM Sean Christopherson <[email protected]> wrote: > > On Fri, Aug 21, 2020 at 06:52:29AM -0400, Paolo Bonzini wrote: > > From: Sean Christopherson <[email protected]> > > > > Don't use RDPID in the paranoid entry flow, as it can consume a KVM > > guest's MSR_TSC_AUX value if an NMI arrives during KVM's run loop. > > > > In general, the kernel does not need TSC_AUX because it can just use > > __this_cpu_read(cpu_number) to read the current processor id. It can > > also just block preemption and thread migration at its will, therefore > > it has no need for the atomic rdtsc+vgetcpu provided by RDTSCP. For this > > reason, as a performance optimization, KVM loads the guest's TSC_AUX when > > a CPU first enters its run loop. On AMD's SVM, it doesn't restore the > > host's value until the CPU exits the run loop; VMX is even more aggressive > > and defers restoring the host's value until the CPU returns to userspace. > > > > This optimization obviously relies on the kernel not consuming TSC_AUX, > > which falls apart if an NMI arrives during the run loop and uses RDPID. > > Removing it would be painful, as both SVM and VMX would need to context > > switch the MSR on every VM-Enter (for a cost of 2x WRMSR), whereas using > > LSL instead RDPID is a minor blip. > > > > Both SAVE_AND_SET_GSBASE and GET_PERCPU_BASE are only used in paranoid > > entry, > > therefore the patch can just remove the RDPID alternative. > > > > Fixes: eaad981291ee3 ("x86/entry/64: Introduce the FIND_PERCPU_BASE macro") > > Cc: Dave Hansen <[email protected]> > > Cc: Chang Seok Bae <[email protected]> > > Cc: Peter Zijlstra <[email protected]> > > Cc: Sasha Levin <[email protected]> > > Cc: Paolo Bonzini <[email protected]> > > Cc: [email protected] > > Reported-by: Tom Lendacky <[email protected]> > > Debugged-by: Tom Lendacky <[email protected]> > > Suggested-by: Andy Lutomirski <[email protected]> > > Suggested-by: Peter Zijlstra <[email protected]> > > Signed-off-by: Sean Christopherson <[email protected]> > > Signed-off-by: Paolo Bonzini <[email protected]> > > --- > > arch/x86/entry/calling.h | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h > > index 98e4d8886f11..ae9b0d4615b3 100644 > > --- a/arch/x86/entry/calling.h > > +++ b/arch/x86/entry/calling.h > > @@ -374,12 +374,14 @@ For 32-bit we have the following conventions - kernel > > is built with > > * Fetch the per-CPU GSBASE value for this processor and put it in @reg. > > * We normally use %gs for accessing per-CPU data, but we are setting up > > * %gs here and obviously can not use %gs itself to access per-CPU data. > > + * > > + * Do not use RDPID, because KVM loads guest's TSC_AUX on vm-entry and > > + * may not restore the host's value until the CPU returns to userspace. > > + * Thus the kernel would consume a guest's TSC_AUX if an NMI arrives > > + * while running KVM's run loop. > > */ > > .macro GET_PERCPU_BASE reg:req > > - ALTERNATIVE \ > > - "LOAD_CPU_AND_NODE_SEG_LIMIT \reg", \ > > - "RDPID \reg", \ > > This was the only user of the RDPID macro, I assume we want to yank that out > as well?
No. That one should be kept until the minimum binutils version is raised to one that supports the RDPID opcode. -- Brian Gerst
