Re: [Xen-devel] [PATCH] x86/debug: Plumb pending_dbg through the monitor and devicemodel interfaces

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/12/2019 18:24, Tamas K Lengyel wrote:
> On Tue, Dec 3, 2019 at 1:22 PM Tamas K Lengyel <tamas@xxxxxxxxxxxxx> wrote:
>> On Tue, Dec 3, 2019 at 1:18 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
>> wrote:
>>> On 03/12/2019 18:09, Tamas K Lengyel wrote:
>>>> On Tue, Dec 3, 2019 at 1:05 PM Tamas K Lengyel <tamas@xxxxxxxxxxxxx> wrote:
>>>>>> diff --git a/xen/include/public/vm_event.h 
>>>>>> b/xen/include/public/vm_event.h
>>>>>> index 959083d8c4..76676ff4c0 100644
>>>>>> --- a/xen/include/public/vm_event.h
>>>>>> +++ b/xen/include/public/vm_event.h
>>>>>> @@ -281,6 +281,7 @@ struct vm_event_debug {
>>>>>>      uint32_t insn_length;
>>>>>>      uint8_t type;        /* HVMOP_TRAP_* */
>>>>>>      uint8_t _pad[3];
>>>>>> +    uint64_t pending_dbg;
>>>>> This is just a nitpick but I would prefer if we had the _pad field as
>>>>> the last element in the struct and keep all 64-bit members up in the
>>>>> front.
>>>> Also, since pending_dbg uses unsigned int in Xen, do we need uint64_t
>>>> for it here? Seems to me a uint32_t would suffice.
>>> Its %dr6 (but not quite, due to complexity with exception priorities,
>>> interrupt shadows, and backwards compatibility of the RTM bit with
>>> inverted polarity).  All other registers have 64 bit fields in the
>>> interface.
>>>
>>> The only interesting bits in it fall within the first 32 which is why it
>>> is handled in a shorter way within Xen.  Like %cr0, I don't expect
>>> anything interesting to appear in the upper 32 bits.
>>>
>> Perhaps it would be better to call it dr6 in the interface then to
>> make it more clear that this is a register value?
>>
> Which then begs the question, why not just use dr6 that's already
> present in the vm_event_regs_x86 struct?

Because it (specifically) isn't exactly %dr6.  The ABI it follows is
strictly like the VT-x's pending_dbg VMCS field.

All bits have positive polarity, and are specific to the debug exception
in question.

%dr6 accumulates some debug bits or-wise (and until the guest #DB
handler decides to clear them), some debug bits overwrite-wise, and some
bits with inverted polarity.

Providing %dr6 alone, either before or after merging pending_dbg, is
insufficient to disambiguate the debug exception.

pending_dbg is strictly "the new exception(s) to add into the %dr6 mix".

~Andrew

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