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: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.

>
> ~Andrew
>
> P.S. What is the point of tail-padding a struct inside a union?

To always make sure all structures used by the interface are 64-bit
aligned without having to keep in mind which one is used in a union
and which one isn't or having to remember their relative position in
the overall structure. So it just reduces the cognitive load.

Tamas

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