On Tue, Nov 13, 2018 at 08:48:53AM -0800, Sean Christopherson wrote:
> 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.
Actually, this is a KVM bug, or maybe KVM intentionally diverges from
the SDM. Guest activity state checks cause VMExits, not VMFail.
> >
> > if (nested_vmx_check_io_bitmap_controls(vcpu, vmcs12))
> > --
> > 2.9.5
> >
