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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Tue, Dec 3, 2019 at 1:31 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> 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".
>

OK, thanks for the explanation. SGTM.

Tamas

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