Re: [PATCH] Revert "KVM: nVMX: Expose RDPMC-exiting only when guest supports PMU"

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Wed, May 08, 2019 at 09:57:11AM -0700, Jim Mattson wrote:
> On Wed, May 8, 2019 at 9:08 AM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > The RDPMC-exiting control is dependent on the existence of the RDPMC
> > instruction itself, i.e. is not tied to the "Architectural Performance
> > Monitoring" feature.  For all intents and purposes, the control exists
> > on all CPUs with VMX support since RDPMC also exists on all VCPUs with
> > VMX supported.  Per Intel's SDM:
> >
> >   The RDPMC instruction was introduced into the IA-32 Architecture in
> >   the Pentium Pro processor and the Pentium processor with MMX technology.
> >   The earlier Pentium processors have performance-monitoring counters, but
> >   they must be read with the RDMSR instruction.
> >
> > Because RDPMC-exiting always exists, KVM requires the control and refuses
> > to load if it's not available.  As a result, hiding the PMU from a guest
> > breaks nested virtualization if the guest attemts to use KVM.
> 
> Is it true that the existence of instruction <X> implies the
> availaibility of the VM-execution control <X>-exiting (if such a
> VM-execution control exists)? What about WBINVD? That instruction has
> certainly been around forever, but there were VMX-capable processors
> that did not support WBINVD-exiting.

Technically no, but 99% of the time yes.  It's kind of similar to KVM's
live migration requirements: new features with "dangerous" instructions
need an associated VMCS control, but there are some legacy cases where
a VMCS control was added after the fact, WBINVD being the obvious example.

> Having said that, I think our hands are tied by the assumptions made
> by existing hypervisors, whether or not those assumptions are true.
> (VMware's VMM, for instance, requires MONITOR-exiting and
> MWAIT-exiting even when MONITOR/MWAIT are not enumerated by CPUID.)

I'd say it's more of a requirement than an assumption, e.g. KVM
*requires* RDPMC-exiting so that the guest can't glean info about the
host.  I guess technically KVM is assuming RDPMC itself exists, but
it's existence is effectively guaranteed by the SDM.

I can't speak to the VMWare behavior, e.g. it might be nothing more
than a simple oversight that isn't worth fixing, or maybe it's paranoid
and really wants to ensure the guest can't execute MONITOR/MWAIT :-)

> > While it's not explicitly stated in the RDPMC pseudocode, the VM-Exit
> > check for RDPMC-exiting follows standard fault vs. VM-Exit prioritization
> > for privileged instructions, e.g. occurs after the CPL/CR0.PE/CR4.PCE
> > checks, but before the counter referenced in ECX is checked for validity.
> >
> > In other words, the original KVM behavior of injecting a #GP was correct,
> > and the KVM unit test needs to be adjusted accordingly, e.g. eat the #GP
> > when the unit test guest (L3 in this case) executes RDPMC without
> > RDPMC-exiting set in the unit test host (L2).
> >
> > This reverts commit e51bfdb68725dc052d16241ace40ea3140f938aa.
> >
> > Fixes: e51bfdb68725 ("KVM: nVMX: Expose RDPMC-exiting only when guest supports PMU")
> > Reported-by: David Hill <hilld@binarystorm.net>
> > Cc: Saar Amar <saaramar@microsoft.com>
> > Cc: Mihai Carabas <mihai.carabas@oracle.com>
> > Cc: Jim Mattson <jmattson@google.com>
> > Cc: Liran Alon <liran.alon@oracle.com>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Reviewed-by: Jim Mattson <jmattson@google.com>