On 01/02/2019 16:55, Jan Beulich wrote:
>>>> On 01.02.19 at 17:25, <andrew.coop...@citrix.com> wrote:
>> On 01/02/2019 15:58, Jan Beulich wrote:
>>>>>> On 01.02.19 at 15:49, <andrew.coop...@citrix.com> wrote:
>>>> c/s 9338a37d "x86/svm: implement debug events" added support for 
>>>> introspecting
>>>> ICEBP debug exceptions, but didn't account for the fact that
>>>> svm_get_insn_len() (previously __get_instruction_length) can fail and may
>>>> already raise #GP for the guest.
>>>>
>>>> If svm_get_insn_len() fails, return back to guest context rather than
>>>> continuing and mistaking a trap-style VMExit for a fault-style one.
>>> My reading of the last part of this sentence is that the exit in
>>> question is a trap-style one. Is my English failing me here?
>> Your reading of my sentence is correct, but I was confused when writing it.
>>
>> ICEBP is a fault-style intercept.
>>
>> However, when svm_get_insn_len() fails, it will inject #GP and return
> s/will/may/ afaict

Oh true - this is also problematic for all other cases.  I'll need to
fix that up as well,

>
>> 0.  This then gets passed into hvm_monitor_debug() or the #DB
>> re-injected as-was.
>>
>> If it were just getting insn_len incorrectly as 0, then the guest would
>> livelock as we wouldn't inject the #DB with trap semantics it requires,
> I'm confused again: Why trap semantics? The ICEBP has fault
> semantics as you confirmed above.

The ICEBP intercept has fault semantics.  An ICEBP instruction executing
in the guest has trap semantics.

Xen has to make up the difference as part of re-injecting the #DB (which
is extra complicated due to AMD's even injection not handing
X86_EVENTTYPE_PRI_SW_EXCEPTION in a sensible way).

>
>> but as the #GP is already raised, this will combine to #DF.
> How that? #DB is a benign exception, so according to the table on the
> #DF page in the SDM, with #GP it shouldn't combine to #DF.

#GP is raised first.  It is contributory.

A subsequent #DB getting raised causes #GP to turn into #DF.

> Also, if live-locking the guest is a concern, then (as said before) how
> can simply re-entering the guest be the right solution?

It isn't, given your observation that there is a path out of
svm_get_insn_len() which return 0 and don't raise #GP, but that is an
orthogonal bug to fix.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Reply via email to