diff mbox series

[v4,4/9] KVM: nVMX: check GUEST_IA32_PERF_GLOBAL_CTRL on VM-Entry

Message ID 20190906210313.128316-5-oupton@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: VMX: Add full nested support for IA32_PERF_GLOBAL_CTRL | expand

Commit Message

Oliver Upton Sept. 6, 2019, 9:03 p.m. UTC
Add condition to nested_vmx_check_guest_state() to check the validity of
GUEST_IA32_PERF_GLOBAL_CTRL. Per Intel's SDM Vol 3 26.3.1.1:

  If the "load IA32_PERF_GLOBAL_CTRL" VM-entry control is 1, bits
  reserved in the IA32_PERF_GLOBAL_CTRL MSR must be 0 in the field for that
  register.

Suggested-by: Jim Mattson <jmattson@google.com>
Co-developed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Oliver Upton <oupton@google.com>
Reviewed-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
---
 arch/x86/kvm/vmx/nested.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Sean Christopherson Sept. 16, 2019, 6:10 p.m. UTC | #1
On Fri, Sep 06, 2019 at 02:03:08PM -0700, Oliver Upton wrote:
> Add condition to nested_vmx_check_guest_state() to check the validity of
> GUEST_IA32_PERF_GLOBAL_CTRL. Per Intel's SDM Vol 3 26.3.1.1:
> 
>   If the "load IA32_PERF_GLOBAL_CTRL" VM-entry control is 1, bits
>   reserved in the IA32_PERF_GLOBAL_CTRL MSR must be 0 in the field for that
>   register.
> 
> Suggested-by: Jim Mattson <jmattson@google.com>
> Co-developed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Signed-off-by: Oliver Upton <oupton@google.com>
> Reviewed-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 9ba90b38d74b..6c3aa3bcede3 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -10,6 +10,7 @@
>  #include "hyperv.h"
>  #include "mmu.h"
>  #include "nested.h"
> +#include "pmu.h"
>  #include "trace.h"
>  #include "x86.h"
>  
> @@ -2732,6 +2733,7 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
>  					u32 *exit_qual)
>  {
>  	bool ia32e;
> +	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);

Nit: I wouldn't bother with a local variable, just call vcpu_to_pmu() when
invoking kvm_is_valid_perf_global_ctrl(), especially since you need a line
break anyways.

>  
>  	*exit_qual = ENTRY_FAIL_DEFAULT;
>  
> @@ -2748,6 +2750,11 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
>  		return -EINVAL;
>  	}
>  
> +	if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL &&
> +	    !kvm_is_valid_perf_global_ctrl(pmu,
> +					   vmcs12->guest_ia32_perf_global_ctrl))
> +		return -EINVAL;
> +
>  	/*
>  	 * If the load IA32_EFER VM-entry control is 1, the following checks
>  	 * are performed on the field for the IA32_EFER MSR:
> -- 
> 2.23.0.187.g17f5b7556c-goog
>
Oliver Upton Sept. 16, 2019, 9:19 p.m. UTC | #2
On Mon, Sep 16, 2019 at 11:10:03AM -0700, Sean Christopherson wrote:
> On Fri, Sep 06, 2019 at 02:03:08PM -0700, Oliver Upton wrote:
> > Add condition to nested_vmx_check_guest_state() to check the validity of
> > GUEST_IA32_PERF_GLOBAL_CTRL. Per Intel's SDM Vol 3 26.3.1.1:
> > 
> >   If the "load IA32_PERF_GLOBAL_CTRL" VM-entry control is 1, bits
> >   reserved in the IA32_PERF_GLOBAL_CTRL MSR must be 0 in the field for that
> >   register.
> > 
> > Suggested-by: Jim Mattson <jmattson@google.com>
> > Co-developed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> > Signed-off-by: Oliver Upton <oupton@google.com>
> > Reviewed-by: Jim Mattson <jmattson@google.com>
> > Reviewed-by: Peter Shier <pshier@google.com>
> > ---
> >  arch/x86/kvm/vmx/nested.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> > index 9ba90b38d74b..6c3aa3bcede3 100644
> > --- a/arch/x86/kvm/vmx/nested.c
> > +++ b/arch/x86/kvm/vmx/nested.c
> > @@ -10,6 +10,7 @@
> >  #include "hyperv.h"
> >  #include "mmu.h"
> >  #include "nested.h"
> > +#include "pmu.h"
> >  #include "trace.h"
> >  #include "x86.h"
> >  
> > @@ -2732,6 +2733,7 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
> >  					u32 *exit_qual)
> >  {
> >  	bool ia32e;
> > +	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> 
> Nit: I wouldn't bother with a local variable, just call vcpu_to_pmu() when
> invoking kvm_is_valid_perf_global_ctrl(), especially since you need a line
> break anyways.

Ack to both (here and on 5/9).

> >  
> >  	*exit_qual = ENTRY_FAIL_DEFAULT;
> >  
> > @@ -2748,6 +2750,11 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
> >  		return -EINVAL;
> >  	}
> >  
> > +	if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL &&
> > +	    !kvm_is_valid_perf_global_ctrl(pmu,
> > +					   vmcs12->guest_ia32_perf_global_ctrl))
> > +		return -EINVAL;
> > +
> >  	/*
> >  	 * If the load IA32_EFER VM-entry control is 1, the following checks
> >  	 * are performed on the field for the IA32_EFER MSR:
> > -- 
> > 2.23.0.187.g17f5b7556c-goog
> >
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 9ba90b38d74b..6c3aa3bcede3 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -10,6 +10,7 @@ 
 #include "hyperv.h"
 #include "mmu.h"
 #include "nested.h"
+#include "pmu.h"
 #include "trace.h"
 #include "x86.h"
 
@@ -2732,6 +2733,7 @@  static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
 					u32 *exit_qual)
 {
 	bool ia32e;
+	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
 
 	*exit_qual = ENTRY_FAIL_DEFAULT;
 
@@ -2748,6 +2750,11 @@  static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
 		return -EINVAL;
 	}
 
+	if (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL &&
+	    !kvm_is_valid_perf_global_ctrl(pmu,
+					   vmcs12->guest_ia32_perf_global_ctrl))
+		return -EINVAL;
+
 	/*
 	 * If the load IA32_EFER VM-entry control is 1, the following checks
 	 * are performed on the field for the IA32_EFER MSR: