Re: [Xen-devel] [PATCH v3 08/10] nEPT: handle invept instruction from L1 VMM

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.12.12 at 16:43, Xiantao Zhang <xiantao.zhang@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2573,10 +2573,14 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>              update_guest_eip();
>          break;
>  
> +    case EXIT_REASON_INVEPT:
> +        if ( nvmx_handle_invept(regs) == X86EMUL_OKAY )
> +            update_guest_eip();
> +        break;
> +

I realize that you're just copying code written the same way
elsewhere, but: What if (here and elsewhere) X86EMUL_OKAY
is not being returned (e.g. in the non-nested case)? Without
the nested VMX code, all of these would have ended up at
the default case (crashing the guest). Iiuc the correct action
would be to inject an exception at least when X86EMUL_EXCEPTION
is being returned here - whether that's done here or (perhaps
better, as only it can know _what_ exception to inject) by the
callee is another thing to decide.

Also, at the example of nvmx_handle_vmclear() I see that it
produces exceptions in most of the cases, but I think all of the
related code needs auditing that things are being handled
consistently _and_ completely (constructs like

    if ( ... != X86EMUL_OKAY )
        return X86EMUL_EXCEPTION;

are definitely not okay, as there are further X86EMUL_* values
that can occur; if you know only the two must ever occur at a
given place, ASSERT() so, making things clear to the reader
without having to follow all code paths).

Jan


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