On 08/05/19 18:08, Sean Christopherson 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.
>
> 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@xxxxxxxxxxxxxxx>
> Cc: Saar Amar <saaramar@xxxxxxxxxxxxx>
> Cc: Mihai Carabas <mihai.carabas@xxxxxxxxxx>
> Cc: Jim Mattson <jmattson@xxxxxxxxxx>
> Cc: Liran Alon <liran.alon@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> ---
> arch/x86/kvm/vmx/vmx.c | 25 -------------------------
> 1 file changed, 25 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 60306f19105d..0db7ded18951 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -6866,30 +6866,6 @@ static void nested_vmx_entry_exit_ctls_update(struct kvm_vcpu *vcpu)
> }
> }
>
> -static bool guest_cpuid_has_pmu(struct kvm_vcpu *vcpu)
> -{
> - struct kvm_cpuid_entry2 *entry;
> - union cpuid10_eax eax;
> -
> - entry = kvm_find_cpuid_entry(vcpu, 0xa, 0);
> - if (!entry)
> - return false;
> -
> - eax.full = entry->eax;
> - return (eax.split.version_id > 0);
> -}
> -
> -static void nested_vmx_procbased_ctls_update(struct kvm_vcpu *vcpu)
> -{
> - struct vcpu_vmx *vmx = to_vmx(vcpu);
> - bool pmu_enabled = guest_cpuid_has_pmu(vcpu);
> -
> - if (pmu_enabled)
> - vmx->nested.msrs.procbased_ctls_high |= CPU_BASED_RDPMC_EXITING;
> - else
> - vmx->nested.msrs.procbased_ctls_high &= ~CPU_BASED_RDPMC_EXITING;
> -}
> -
> static void update_intel_pt_cfg(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> @@ -6978,7 +6954,6 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> if (nested_vmx_allowed(vcpu)) {
> nested_vmx_cr_fixed1_bits_update(vcpu);
> nested_vmx_entry_exit_ctls_update(vcpu);
> - nested_vmx_procbased_ctls_update(vcpu);
> }
>
> if (boot_cpu_has(X86_FEATURE_INTEL_PT) &&
>
Queued, thanks.
Paolo
