Re: [PATCH v6 6/7] KVM: nVMX: Collapse nested_check_host_control_regs() into its caller

Krish Sadhukhan
2019-04-11T22:02:35Z

On 04/11/2019 12:18 PM, Sean Christopherson wrote:
> ...now that nested_vmx_check_host_state() returns -EINVAL to indicate
> failure, i.e. moving the checks doesn't require changing their return
> values to be 'VMXERR_ENTRY_INVALID_HOST_STATE_FIELD'. This eliminates
> a misleading function name since nested_check_host_control_regs() was
> checking MSRs as well as control registers.
>
> Alternatively, nested_check_host_control_regs() could be split up,
> e.g. into _control_regs(), _debug_regs() and _msrs(), but that's a
> bit gratutitous at this time given that the entire function is 30 LoC.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
> arch/x86/kvm/vmx/nested.c | 26 ++++++--------------------
> 1 file changed, 6 insertions(+), 20 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 16cff40456ee..98afbe7c15a1 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -2590,13 +2590,11 @@ static int nested_vmx_check_controls(struct kvm_vcpu *vcpu,
> return 0;
> }
>
> -/*
> - * Checks related to Host Control Registers and MSRs
> - */
> -static int nested_check_host_control_regs(struct kvm_vcpu *vcpu,
> - struct vmcs12 *vmcs12)
> +static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
> + struct vmcs12 *vmcs12,
> + u32 *vm_instruction_error)
> {
> - bool ia32e;
> + *vm_instruction_error = VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
>
> if (!nested_host_cr0_valid(vcpu, vmcs12->host_cr0) ||
> !nested_host_cr4_valid(vcpu, vmcs12->host_cr4) ||
> @@ -2618,8 +2616,8 @@ static int nested_check_host_control_regs(struct kvm_vcpu *vcpu,
> * the host address-space size VM-exit control.
> */
> if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) {
> - ia32e = (vmcs12->vm_exit_controls &
> - VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
> + bool ia32e = (vmcs12->vm_exit_controls &
> + VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
> if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) ||
> ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) ||
> ia32e != !!(vmcs12->host_ia32_efer & EFER_LME))
> @@ -2629,18 +2627,6 @@ static int nested_check_host_control_regs(struct kvm_vcpu *vcpu,
> return 0;
> }
>
> -static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
> - struct vmcs12 *vmcs12,
> - u32 *vm_instruction_error)
> -{
> - *vm_instruction_error = VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
> -
> - if (nested_check_host_control_regs(vcpu, vmcs12))
> - return -EINVAL;
> -
> - return 0;
> -}
> -
> static int nested_vmx_check_vmcs_link_ptr(struct kvm_vcpu *vcpu,
> struct vmcs12 *vmcs12)
> {

Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>