On Tue, Nov 13, 2018 at 01:12:05AM -0500, Krish Sadhukhan wrote:
> Separate out the checks in nested_check_vmentry_prereqs(), that are related
> to Guest State Area, 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 | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 0f6c38f..d9f3bc7 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -12313,12 +12313,23 @@ static int nested_vmx_check_nmi_controls(struct vmcs12 *vmcs12)
> return 0;
> }
>
> +/*
> + * Checks related to Guest State Area
> + */
> +static int nested_check_guest_state_area(struct vmcs12 *vmcs12)
> +{
> + if (vmcs12->guest_activity_state != GUEST_ACTIVITY_ACTIVE &&
> + vmcs12->guest_activity_state != GUEST_ACTIVITY_HLT)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static int nested_check_vmentry_prereqs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
>
> - if (vmcs12->guest_activity_state != GUEST_ACTIVITY_ACTIVE &&
> - vmcs12->guest_activity_state != GUEST_ACTIVITY_HLT)
> + if (nested_check_guest_state_area(vmcs12))
> return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
Hmm, guest state is a bit of a mess since many of the checks result in
VMExit. The basic rule of thumb is that "Guest Non-Register State"
checks cause a failed VMEntry while "Guest Register State" checks cause
VMExit, though sadly even that delineation doesn't always hold true.
Maybe the best approach is to go one level deeper in the SDM, e.g.
nested_check_guest_non_register_state(), and add a comment calling out
that some "Guest Non-Register State" checks are deliberately omitted
because they cause VMExit.
>
> if (nested_vmx_check_io_bitmap_controls(vcpu, vmcs12))
> --
> 2.9.5
>
