On Thu, Nov 15, 2018 at 12:45:54AM -0500, Krish Sadhukhan wrote:
> Separate out the checks in nested_check_vmentry_prereqs(), that are related
> to Guest Non-Register State, to a separate function. The re-organized code is
> easier for readability, enhancement and maintenance.
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> Reviewed-by: Mihai Carabas <mihai.carabas@xxxxxxxxxx>
> Reviewed-by: Mark Kanda <mark.kanda@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 93e49aa..d60c529 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -12531,12 +12531,21 @@ static int nested_check_host_ctl_regs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs
> return 0;
> }
>
> -static int nested_check_vmentry_prereqs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
> +/*
> + * Checks related to Guest Non-register State
> + */
> +static int nested_check_guest_non_reg_state(struct vmcs12 *vmcs12)
> {
> + /* Failed "Guest Activity State" checks cause VMExit */
If you're going to add a comment it should either be a "TODO" or explain
that KVM diverges from the SDM for an unknown reason, or better yet find
someone that can explain why KVM diverges :). My guess is it's
intentional as KVM is preventing WFS and Shutdown states, which also
diverges from the SDM.
Stating that the checks cause VMExits but not actually signalling a VMExit
makes it look like you introduced the bug.
> if (vmcs12->guest_activity_state != GUEST_ACTIVITY_ACTIVE &&
> vmcs12->guest_activity_state != GUEST_ACTIVITY_HLT)
> - return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
> + return -EINVAL;
> +
> + return 0;
> +}
>
> +static int nested_check_vmentry_prereqs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
> +{
> if (nested_check_vm_execution_ctls(vcpu, vmcs12) ||
> nested_check_vm_exit_ctls(vcpu, vmcs12) ||
> nested_check_vm_entry_ctls(vcpu, vmcs12))
> @@ -12545,6 +12554,9 @@ static int nested_check_vmentry_prereqs(struct kvm_vcpu *vcpu, struct vmcs12 *vm
> if (nested_check_host_ctl_regs(vcpu, vmcs12))
> return VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
>
> + if (nested_check_guest_non_reg_state(vmcs12))
> + return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
> +
> return 0;
> }
>
> --
> 2.9.5
>
