Re: [Xen-devel] [PATCH for-4.7] x86/hvm: Correct emulation of invlpg instruction

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.04.16 at 10:59, <andrew.cooper3@xxxxxxxxxx> wrote:
> `invlpg` and `invlpga` are specified to be NOPs when issued on non-canonical
> addresses.
> 
> These instructions are not normally intercepted.  They are however 
> intercepted
> for HVM guests running in shadow paging mode.  AMD hardware lacking decode
> hardware assistance uses the general instruction emulator to handle the
> interception.
> 
> Alter hvmemul_invlpg() to swallow the #GP exception resulting from a
> non-canonical address, rather than reporting it back to the guest.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>

CC: Paul Durrant <paul.durrant@xxxxxxxxxx>

> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1598,8 +1598,27 @@ static int hvmemul_invlpg(
>      rc = hvmemul_virtual_to_linear(
>          seg, offset, 1, &reps, hvm_access_none, hvmemul_ctxt, &addr);
>  
> -    if ( rc == X86EMUL_OKAY )
> +    switch ( rc )
> +    {
> +    case X86EMUL_OKAY:
>          hvm_funcs.invlpg_intercept(addr);
> +        break;
> +
> +    case X86EMUL_EXCEPTION:
> +        ASSERT(hvmemul_ctxt->trap.vector == TRAP_gp_fault);
> +        /*
> +         * `invlpg` and `invlpga` are specified to be NOPs when issued on a
> +         * non-canonical address.  hvmemul_virtual_to_linear() latches a #GP
> +         * which is the useful behaviour for most of its callers.

Here and in the description I'd prefer you to not exclusively refer
to non-canonical addresses - segment limit violations in 32-bit or
compatibility modes are affected as much.

> +         * Clear the pending exception to match avoid delivering a #GP fault
> +         * to the guest.
> +         */
> +        hvmemul_ctxt->exn_pending = 0;
> +        hvmemul_ctxt->trap = (struct hvm_trap){};

memset() would in this case look more natural I think, but is this
field really meaningful in the first place for exn_pending cleared?

Jan


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