| Message ID | 20190411191809.8131-5-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nVMX Add IA32_PAT consistency checks | expand |
On 04/11/2019 12:18 PM, Sean Christopherson wrote: > Rename the top-level consistency check functions to (loosely) align with > the SDM. Historically, KVM has used the terms "prereq" and "postreq" to > differentiate between consistency checks that lead to VM-Fail and those > that lead to VM-Exit. The terms are vague and potentially misleading, > e.g. "postreq" might be interpreted as occurring after VM-Entry. > > Note, while the SDM lumps controls and host state into a single section, > "Checks on VMX Controls and Host-State Area", split them into separate > top-level functions as the two categories of checks result in different > VM instruction errors. This split will allow for additional cleanup. > > Note #2, "vmentry" is intentionally dropped from the new function names > to avoid confusion with nested_check_vm_entry_controls(), and to keep > the length of the functions names somewhat manageable. > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kvm/vmx/nested.c | 39 +++++++++++++++++++++++++-------------- > 1 file changed, 25 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index fe1323ab6894..b22605d5ee9e 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -2573,6 +2573,17 @@ static int nested_check_vm_entry_controls(struct kvm_vcpu *vcpu, > return 0; > } > > +static int nested_vmx_check_controls(struct kvm_vcpu *vcpu, > + struct vmcs12 *vmcs12) > +{ > + if (nested_check_vm_execution_controls(vcpu, vmcs12) || > + nested_check_vm_exit_controls(vcpu, vmcs12) || > + nested_check_vm_entry_controls(vcpu, vmcs12)) > + return VMXERR_ENTRY_INVALID_CONTROL_FIELD; > + > + return 0; > +} > + > /* > * Checks related to Host Control Registers and MSRs > */ > @@ -2612,14 +2623,9 @@ static int nested_check_host_control_regs(struct kvm_vcpu *vcpu, > return 0; > } > > -static int nested_vmx_check_vmentry_prereqs(struct kvm_vcpu *vcpu, > - struct vmcs12 *vmcs12) > +static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, > + struct vmcs12 *vmcs12) > { > - if (nested_check_vm_execution_controls(vcpu, vmcs12) || > - nested_check_vm_exit_controls(vcpu, vmcs12) || > - nested_check_vm_entry_controls(vcpu, vmcs12)) > - return VMXERR_ENTRY_INVALID_CONTROL_FIELD; > - > if (nested_check_host_control_regs(vcpu, vmcs12)) > return VMXERR_ENTRY_INVALID_HOST_STATE_FIELD; > > @@ -2665,9 +2671,9 @@ static int nested_check_guest_non_reg_state(struct vmcs12 *vmcs12) > return 0; > } > > -static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu, > - struct vmcs12 *vmcs12, > - u32 *exit_qual) > +static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu, > + struct vmcs12 *vmcs12, > + u32 *exit_qual) > { > bool ia32e; > > @@ -2985,7 +2991,7 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry) > return -1; > } > > - if (nested_vmx_check_vmentry_postreqs(vcpu, vmcs12, &exit_qual)) > + if (nested_vmx_check_guest_state(vcpu, vmcs12, &exit_qual)) > goto vmentry_fail_vmexit; > } > > @@ -3130,7 +3136,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) > launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS > : VMXERR_VMRESUME_NONLAUNCHED_VMCS); > > - ret = nested_vmx_check_vmentry_prereqs(vcpu, vmcs12); > + ret = nested_vmx_check_controls(vcpu, vmcs12); > + if (ret) > + return nested_vmx_failValid(vcpu, ret); > + > + ret = nested_vmx_check_host_state(vcpu, vmcs12); > if (ret) > return nested_vmx_failValid(vcpu, ret); > > @@ -5455,8 +5465,9 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > return -EINVAL; > } > > - if (nested_vmx_check_vmentry_prereqs(vcpu, vmcs12) || > - nested_vmx_check_vmentry_postreqs(vcpu, vmcs12, &exit_qual)) > + if (nested_vmx_check_controls(vcpu, vmcs12) || > + nested_vmx_check_host_state(vcpu, vmcs12) || > + nested_vmx_check_guest_state(vcpu, vmcs12, &exit_qual)) > return -EINVAL; > > vmx->nested.dirty_vmcs12 = true; It's probably a good idea to add a one-line comment above each of the top-level check functions. Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index fe1323ab6894..b22605d5ee9e 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -2573,6 +2573,17 @@ static int nested_check_vm_entry_controls(struct kvm_vcpu *vcpu, return 0; } +static int nested_vmx_check_controls(struct kvm_vcpu *vcpu, + struct vmcs12 *vmcs12) +{ + if (nested_check_vm_execution_controls(vcpu, vmcs12) || + nested_check_vm_exit_controls(vcpu, vmcs12) || + nested_check_vm_entry_controls(vcpu, vmcs12)) + return VMXERR_ENTRY_INVALID_CONTROL_FIELD; + + return 0; +} + /* * Checks related to Host Control Registers and MSRs */ @@ -2612,14 +2623,9 @@ static int nested_check_host_control_regs(struct kvm_vcpu *vcpu, return 0; } -static int nested_vmx_check_vmentry_prereqs(struct kvm_vcpu *vcpu, - struct vmcs12 *vmcs12) +static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu, + struct vmcs12 *vmcs12) { - if (nested_check_vm_execution_controls(vcpu, vmcs12) || - nested_check_vm_exit_controls(vcpu, vmcs12) || - nested_check_vm_entry_controls(vcpu, vmcs12)) - return VMXERR_ENTRY_INVALID_CONTROL_FIELD; - if (nested_check_host_control_regs(vcpu, vmcs12)) return VMXERR_ENTRY_INVALID_HOST_STATE_FIELD; @@ -2665,9 +2671,9 @@ static int nested_check_guest_non_reg_state(struct vmcs12 *vmcs12) return 0; } -static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu, - struct vmcs12 *vmcs12, - u32 *exit_qual) +static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu, + struct vmcs12 *vmcs12, + u32 *exit_qual) { bool ia32e; @@ -2985,7 +2991,7 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry) return -1; } - if (nested_vmx_check_vmentry_postreqs(vcpu, vmcs12, &exit_qual)) + if (nested_vmx_check_guest_state(vcpu, vmcs12, &exit_qual)) goto vmentry_fail_vmexit; } @@ -3130,7 +3136,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS : VMXERR_VMRESUME_NONLAUNCHED_VMCS); - ret = nested_vmx_check_vmentry_prereqs(vcpu, vmcs12); + ret = nested_vmx_check_controls(vcpu, vmcs12); + if (ret) + return nested_vmx_failValid(vcpu, ret); + + ret = nested_vmx_check_host_state(vcpu, vmcs12); if (ret) return nested_vmx_failValid(vcpu, ret); @@ -5455,8 +5465,9 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, return -EINVAL; } - if (nested_vmx_check_vmentry_prereqs(vcpu, vmcs12) || - nested_vmx_check_vmentry_postreqs(vcpu, vmcs12, &exit_qual)) + if (nested_vmx_check_controls(vcpu, vmcs12) || + nested_vmx_check_host_state(vcpu, vmcs12) || + nested_vmx_check_guest_state(vcpu, vmcs12, &exit_qual)) return -EINVAL; vmx->nested.dirty_vmcs12 = true;
Rename the top-level consistency check functions to (loosely) align with the SDM. Historically, KVM has used the terms "prereq" and "postreq" to differentiate between consistency checks that lead to VM-Fail and those that lead to VM-Exit. The terms are vague and potentially misleading, e.g. "postreq" might be interpreted as occurring after VM-Entry. Note, while the SDM lumps controls and host state into a single section, "Checks on VMX Controls and Host-State Area", split them into separate top-level functions as the two categories of checks result in different VM instruction errors. This split will allow for additional cleanup. Note #2, "vmentry" is intentionally dropped from the new function names to avoid confusion with nested_check_vm_entry_controls(), and to keep the length of the functions names somewhat manageable. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/vmx/nested.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)