> -----Original Message-----
> From: Jan Beulich [mailto:[email protected]]
> Sent: 22 April 2016 10:31
> To: Andrew Cooper
> Cc: Paul Durrant; Wei Liu; Xen-devel
> Subject: Re: [PATCH for-4.7] x86/hvm: Correct emulation of invlpg instruction
>
> >>> On 22.04.16 at 10:59, <[email protected]> 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 <[email protected]>
> > ---
> > CC: Jan Beulich <[email protected]>
> > CC: Wei Liu <[email protected]>
>
> CC: Paul Durrant <[email protected]>
>
> > --- 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.
...in which case squashing the #GP would be incorrect, right?
>
> > + * 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?
Still probably good practise to clear the trap, but memset() would indeed be
more readable.
Paul
>
> Jan
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
