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

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

[Andrew Cooper]

`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@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>

Note: Ideally this should be caught in the instruction emulator itself, but it
is the hvmemul_virtual_to_linear() which completes the memory calculation
including a possible non-zero %fs/%gs base.
---
 xen/arch/x86/hvm/emulate.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index cc0b841..897724e 100644
--- 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.
+         *
+         * 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){};
+        rc = X86EMUL_OKAY;
+        break;
+    }
 
     return rc;
 }
-- 
2.1.4


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

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

[Jan Beulich]

>>> On 22.04.16 at 10:59, <andrew.cooper3@citrix.com> 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@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>

CC: Paul Durrant <paul.durrant@citrix.com>

> --- 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@lists.xen.org
http://lists.xen.org/xen-devel

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

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> 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, <andrew.cooper3@citrix.com> 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@citrix.com>
> > ---
> > CC: Jan Beulich <JBeulich@suse.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> 
> CC: Paul Durrant <paul.durrant@citrix.com>
> 
> > --- 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
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Jan Beulich]

>>> On 22.04.16 at 11:48, <Paul.Durrant@citrix.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 22 April 2016 10:31
>> >>> On 22.04.16 at 10:59, <andrew.cooper3@citrix.com> wrote:
>> > --- 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?

No, not according to the SDM.

Jan


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

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

[Andrew Cooper]

On 22/04/16 10:57, Jan Beulich wrote:
>>>> On 22.04.16 at 11:48, <Paul.Durrant@citrix.com> wrote:
>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>> Sent: 22 April 2016 10:31
>>>>>> On 22.04.16 at 10:59, <andrew.cooper3@citrix.com> wrote:
>>>> --- 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?
> No, not according to the SDM.

I should check and only squash a #GP(0)

#GP(sel) or #SS(sel) should not be squashed.

v2 on its way.

~Andrew

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

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

[Jan Beulich]

>>> On 22.04.16 at 12:16, <andrew.cooper3@citrix.com> wrote:
> On 22/04/16 10:57, Jan Beulich wrote:
>>>>> On 22.04.16 at 11:48, <Paul.Durrant@citrix.com> wrote:
>>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>>> Sent: 22 April 2016 10:31
>>>>>>> On 22.04.16 at 10:59, <andrew.cooper3@citrix.com> wrote:
>>>>> --- 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?
>> No, not according to the SDM.
> 
> I should check and only squash a #GP(0)
> 
> #GP(sel) or #SS(sel) should not be squashed.

Which also can't happen here (these only occur when selectors
get loaded via some the various mechanisms allowing that).

Jan


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

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

[Andrew Cooper]

On 22/04/16 11:30, Jan Beulich wrote:
>>>> On 22.04.16 at 12:16, <andrew.cooper3@citrix.com> wrote:
>> On 22/04/16 10:57, Jan Beulich wrote:
>>>>>> On 22.04.16 at 11:48, <Paul.Durrant@citrix.com> wrote:
>>>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>>>> Sent: 22 April 2016 10:31
>>>>>>>> On 22.04.16 at 10:59, <andrew.cooper3@citrix.com> wrote:
>>>>>> --- 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?
>>> No, not according to the SDM.
>> I should check and only squash a #GP(0)
>>
>> #GP(sel) or #SS(sel) should not be squashed.
> Which also can't happen here (these only occur when selectors
> get loaded via some the various mechanisms allowing that).

#GP(sel) or #SS(sel) also occur for a memory access which causes a
segment limit violation.  (SDM Vol 3, 5.3 "Limit Checking")

It is not clear whether invlpg falls into this category; it does take a
memory operand, but doesn't use it in the usual manor.

~Andrew

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

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

[Jan Beulich]

>>> On 22.04.16 at 13:18, <andrew.cooper3@citrix.com> wrote:
> On 22/04/16 11:30, Jan Beulich wrote:
>>>>> On 22.04.16 at 12:16, <andrew.cooper3@citrix.com> wrote:
>>> On 22/04/16 10:57, Jan Beulich wrote:
>>>>>>> On 22.04.16 at 11:48, <Paul.Durrant@citrix.com> wrote:
>>>>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>>>>> Sent: 22 April 2016 10:31
>>>>>>>>> On 22.04.16 at 10:59, <andrew.cooper3@citrix.com> wrote:
>>>>>>> --- 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?
>>>> No, not according to the SDM.
>>> I should check and only squash a #GP(0)
>>>
>>> #GP(sel) or #SS(sel) should not be squashed.
>> Which also can't happen here (these only occur when selectors
>> get loaded via some the various mechanisms allowing that).
> 
> #GP(sel) or #SS(sel) also occur for a memory access which causes a
> segment limit violation.  (SDM Vol 3, 5.3 "Limit Checking")

I can't find any mention of error codes in this section at all. Not
sure which version of it you're looking at, mine is 057US. And
even if it said so, I'd be 99.999% certain this is in error, as all
instruction pages always say #GP(0) and #SS(0) for limit
violations.

Jan


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

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

[Andrew Cooper]

On 22/04/16 12:47, Jan Beulich wrote:
>>>> On 22.04.16 at 13:18, <andrew.cooper3@citrix.com> wrote:
>> On 22/04/16 11:30, Jan Beulich wrote:
>>>>>> On 22.04.16 at 12:16, <andrew.cooper3@citrix.com> wrote:
>>>> On 22/04/16 10:57, Jan Beulich wrote:
>>>>>>>> On 22.04.16 at 11:48, <Paul.Durrant@citrix.com> wrote:
>>>>>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>>>>>> Sent: 22 April 2016 10:31
>>>>>>>>>> On 22.04.16 at 10:59, <andrew.cooper3@citrix.com> wrote:
>>>>>>>> --- 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?
>>>>> No, not according to the SDM.
>>>> I should check and only squash a #GP(0)
>>>>
>>>> #GP(sel) or #SS(sel) should not be squashed.
>>> Which also can't happen here (these only occur when selectors
>>> get loaded via some the various mechanisms allowing that).
>> #GP(sel) or #SS(sel) also occur for a memory access which causes a
>> segment limit violation.  (SDM Vol 3, 5.3 "Limit Checking")
> I can't find any mention of error codes in this section at all. Not
> sure which version of it you're looking at, mine is 057US. And
> even if it said so, I'd be 99.999% certain this is in error, as all
> instruction pages always say #GP(0) and #SS(0) for limit
> violations.

Hmm - Section 6.15 is rather more clear, and does state  #GP(0) and
#SS(0) for limit violations.

In which case I am going to have to untangle hvmemul_virtual_to_linear()
and hvm_virtual_to_linear_addr() to distinguish the two cases, and also
to raise #SS for %ss limit violations.

~Andrew

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

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

[Jan Beulich]

>>> On 22.04.16 at 15:40, <andrew.cooper3@citrix.com> wrote:
> Hmm - Section 6.15 is rather more clear, and does state  #GP(0) and
> #SS(0) for limit violations.
> 
> In which case I am going to have to untangle hvmemul_virtual_to_linear()
> and hvm_virtual_to_linear_addr() to distinguish the two cases, and also
> to raise #SS for %ss limit violations.

Oh, that's a good point.

Jan


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