Re: [Xen-devel] [RFC PATCH] vvmx: replace vmreturn() by vmsucceed() and vmfail*()

[Haozhong Zhang <haozhong.zhang@xxxxxxxxx>]

On 12/14/16 19:29 +0800, Haozhong Zhang wrote:
> Replace vmreturn() by vmsucceed(), vmfail(), vmfail_valid() and
> vmfail_invalid(), which are consistent to the pseudo code on Intel
> SDM, and allow to return VM instruction error numbers to L1
> hypervisor.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@xxxxxxxxx>
> ---
> * This patch is based on my patchset "[PATCH v2 0/4] vvmx: fix L1 vmxon".
>
> * I'm not sure whether this patch by itself makes sense or should be sent
>  with additional bugfix patches, so I tag it as RFC. Explanation: besides
>  returning error numbers, this patch does not fix other bugs in the individual
>  nvmx_handle_*() function, so the error numbers used (especially) for L1
>  vmclear, vmptrld and vmwrite are still inconsistent to Intel SDM, or even
>  incorrect.
> ---
> xen/arch/x86/hvm/vmx/vvmx.c        | 107 +++++++++++++++++++++----------------
> xen/include/asm-x86/hvm/vmx/vmcs.h |  15 ++++--
> 2 files changed, 74 insertions(+), 48 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index c4f19a0..f9ae756 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -479,28 +479,37 @@ gp_fault:
>     return X86EMUL_EXCEPTION;
> }
>
> -static void vmreturn(struct cpu_user_regs *regs, enum vmx_ops_result ops_res)
> +#define VMSUCCEED_EFLAGS_MASK \
> +    (X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | \
> +     X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF)
> +
> +static void vmsucceed(struct cpu_user_regs *regs)
> +{
> +    regs->eflags &= ~VMSUCCEED_EFLAGS_MASK;
> +}
> +
> +static void vmfail_valid(struct cpu_user_regs *regs, enum vmx_insn_errno errno)
> {
> +    struct vcpu *v = current;
>     unsigned long eflags = regs->eflags;
> -    unsigned long mask = X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF |
> -                         X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF;
>
> -    eflags &= ~mask;
> +    regs->eflags = (eflags & ~VMSUCCEED_EFLAGS_MASK) | X86_EFLAGS_ZF;
> +    set_vvmcs(v, VM_INSTRUCTION_ERROR, errno);
> +}
>
> -    switch ( ops_res ) {
> -    case VMSUCCEED:
> -        break;
> -    case VMFAIL_VALID:
> -        /* TODO: error number, useful for guest VMM debugging */
> -        eflags |= X86_EFLAGS_ZF;
> -        break;
> -    case VMFAIL_INVALID:
> -    default:
> -        eflags |= X86_EFLAGS_CF;
> -        break;
> -    }
> +static void vmfail_invalid(struct cpu_user_regs *regs)
> +{
> +    unsigned long eflags = regs->eflags;
> +
> +    regs->eflags = (eflags & ~VMSUCCEED_EFLAGS_MASK) | X86_EFLAGS_SF;
Sorry, I made a stupid error here. It should be X86_EFLAGS_CF. I'll
send another version later.

Haozhong

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel