| Message ID | 1461315563-2862-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> 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
> -----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
>>> 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
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
>>> 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
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
>>> 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
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
>>> 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
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; }
`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(-)