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:16, Tamas K Lengyel wrote:
> On Tue, Dec 3, 2019 at 1:09 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
> wrote:
>> On 03/12/2019 18:05, Tamas K Lengyel 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.
>> Well the reason I did it like this is that this version will continue to
>> function with older introspection code.  The extra field is within a
>> union and no other data moves.
>>
>> By repositioning to the start, it will almost certainly break older
>> introspection code even though it compiled correctly.
>>
>> Your choice.
> We are already bumping the interface version for the next release so
> old introspection code by design will stop working. We make no ABI
> stability guarantees between interface versions so this is a
> non-issue.

Ok fine.  Updated locally, but I won't send a new version of the patch
just for this delta.

diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 76676ff4c0..8c24a58964 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -278,10 +278,10 @@ struct vm_event_singlestep {
 
 struct vm_event_debug {
     uint64_t gfn;
+    uint64_t pending_dbg;
     uint32_t insn_length;
     uint8_t type;        /* HVMOP_TRAP_* */
     uint8_t _pad[3];
-    uint64_t pending_dbg;
 };
 
 struct vm_event_mov_to_msr {


However, this does raise the question of why insn_length is uint32_t. 
It has a value which is at most 15.

~Andrew

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