Re: [PATCH] KVM : Set success rflags when emulate VMXON/VMXOFF in nested virt

[Gleb Natapov <gleb@redhat.com>]

On Thu, Jul 04, 2013 at 12:09:32AM +0800, Arthur Chunqi Li wrote:
> Before moving the 3 functions ahead, they will be declared twice in
> the same file before handle_vmon(). I don't know if this small change
> can be committed together in one patch. If this change is needless or
> should be committed in a separate patch, tell me and I will commit
> another version.
> 
Yes please, move the code in a separate patch.

> Arthur
> 
> On Thu, Jul 4, 2013 at 12:05 AM, Arthur Chunqi Li <yzt356@gmail.com> wrote:
> > Set success rflags after emulating VMXON/VMXOFF in nested environment.
> >
> > Re-arrange the code sequence of 3 functions,
> > nested_vmx_succeed()/failValid()/failInvalid(), to avoid double
> > declaration in the same file.
> >
> > Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> > ---
> >  arch/x86/kvm/vmx.c |   85 ++++++++++++++++++++++++++--------------------------
> >  1 file changed, 42 insertions(+), 43 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 260a919..423bc411 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -5551,8 +5551,47 @@ static void nested_free_all_saved_vmcss(struct vcpu_vmx *vmx)
> >                 free_loaded_vmcs(&vmx->vmcs01);
> >  }
> >
> > +/*
> > + * The following 3 functions, nested_vmx_succeed()/failValid()/failInvalid(),
> > + * set the success or error code of an emulated VMX instruction, as specified
> > + * by Vol 2B, VMX Instruction Reference, "Conventions".
> > + */
> > +static void nested_vmx_succeed(struct kvm_vcpu *vcpu)
> > +{
> > +       vmx_set_rflags(vcpu, vmx_get_rflags(vcpu)
> > +                       & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF |
> > +                           X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF));
> > +}
> > +
> > +static void nested_vmx_failInvalid(struct kvm_vcpu *vcpu)
> > +{
> > +       vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu)
> > +                       & ~(X86_EFLAGS_PF | X86_EFLAGS_AF | X86_EFLAGS_ZF |
> > +                           X86_EFLAGS_SF | X86_EFLAGS_OF))
> > +                       | X86_EFLAGS_CF);
> > +}
> > +
> >  static void nested_vmx_failValid(struct kvm_vcpu *vcpu,
> > -                                u32 vm_instruction_error);
> > +                                       u32 vm_instruction_error)
> > +{
> > +       if (to_vmx(vcpu)->nested.current_vmptr == -1ull) {
> > +               /*
> > +                * failValid writes the error number to the current VMCS, which
> > +                * can't be done there isn't a current VMCS.
> > +                */
> > +               nested_vmx_failInvalid(vcpu);
> > +               return;
> > +       }
> > +       vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu)
> > +                       & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF |
> > +                           X86_EFLAGS_SF | X86_EFLAGS_OF))
> > +                       | X86_EFLAGS_ZF);
> > +       get_vmcs12(vcpu)->vm_instruction_error = vm_instruction_error;
> > +       /*
> > +        * We don't need to force a shadow sync because
> > +        * VM_INSTRUCTION_ERROR is not shadowed
> > +        */
> > +}
> >
> >  /*
> >   * Emulate the VMXON instruction.
> > @@ -5612,6 +5651,7 @@ static int handle_vmon(struct kvm_vcpu *vcpu)
> >         vmx->nested.vmxon = true;
> >
> >         skip_emulated_instruction(vcpu);
> > +       nested_vmx_succeed(vcpu);
> >         return 1;
> >  }
> >
> > @@ -5696,6 +5736,7 @@ static int handle_vmoff(struct kvm_vcpu *vcpu)
> >                 return 1;
> >         free_nested(to_vmx(vcpu));
> >         skip_emulated_instruction(vcpu);
> > +       nested_vmx_succeed(vcpu);
> >         return 1;
> >  }
> >
> > @@ -5752,48 +5793,6 @@ static int get_vmx_mem_address(struct kvm_vcpu *vcpu,
> >         return 0;
> >  }
> >
> > -/*
> > - * The following 3 functions, nested_vmx_succeed()/failValid()/failInvalid(),
> > - * set the success or error code of an emulated VMX instruction, as specified
> > - * by Vol 2B, VMX Instruction Reference, "Conventions".
> > - */
> > -static void nested_vmx_succeed(struct kvm_vcpu *vcpu)
> > -{
> > -       vmx_set_rflags(vcpu, vmx_get_rflags(vcpu)
> > -                       & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF |
> > -                           X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF));
> > -}
> > -
> > -static void nested_vmx_failInvalid(struct kvm_vcpu *vcpu)
> > -{
> > -       vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu)
> > -                       & ~(X86_EFLAGS_PF | X86_EFLAGS_AF | X86_EFLAGS_ZF |
> > -                           X86_EFLAGS_SF | X86_EFLAGS_OF))
> > -                       | X86_EFLAGS_CF);
> > -}
> > -
> > -static void nested_vmx_failValid(struct kvm_vcpu *vcpu,
> > -                                       u32 vm_instruction_error)
> > -{
> > -       if (to_vmx(vcpu)->nested.current_vmptr == -1ull) {
> > -               /*
> > -                * failValid writes the error number to the current VMCS, which
> > -                * can't be done there isn't a current VMCS.
> > -                */
> > -               nested_vmx_failInvalid(vcpu);
> > -               return;
> > -       }
> > -       vmx_set_rflags(vcpu, (vmx_get_rflags(vcpu)
> > -                       & ~(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF |
> > -                           X86_EFLAGS_SF | X86_EFLAGS_OF))
> > -                       | X86_EFLAGS_ZF);
> > -       get_vmcs12(vcpu)->vm_instruction_error = vm_instruction_error;
> > -       /*
> > -        * We don't need to force a shadow sync because
> > -        * VM_INSTRUCTION_ERROR is not shadowed
> > -        */
> > -}
> > -
> >  /* Emulate the VMCLEAR instruction */
> >  static int handle_vmclear(struct kvm_vcpu *vcpu)
> >  {
> > --
> > 1.7.9.5
> >

--
			Gleb.