On Wed, 2021-02-10 at 10:26 -0800, Sean Christopherson wrote:
> Add a 2 byte pad to struct compat_vcpu_info so that the sum size of its
> fields is actually 64 bytes. The effective size without the padding is
> also 64 bytes due to the compiler aligning evtchn_pending_sel to a 4-byte
> boundary, but depending on compiler alignment is subtle and unnecessary.
I think there's at least one BUILD_BUG_ON() which would have triggered
if the compiler ever did stop honouring the ELF ABI. And in fact in a
parallel universe where the ABI permits such packing, the padding would
be *wrong*, since the original Xen struct doesn't have the padding.
It *does* have an explicit uint8_t to replace evtchn_upcall_mask but it
doesn't have the following two bytes; canonically we *are* supposed to
take our chances with the ABI there. Although of course the relevant
ABI is the *32-bit* ABI in the compat case, not the 64-bit ABI. They
both align 32-bit values to 32 bits though.
uint8_t evtchn_upcall_pending;
#ifdef XEN_HAVE_PV_UPCALL_MASK
uint8_t evtchn_upcall_mask;
#else /* XEN_HAVE_PV_UPCALL_MASK */
uint8_t pad0;
#endif /* XEN_HAVE_PV_UPCALL_MASK */
xen_ulong_t evtchn_pending_sel;
struct arch_vcpu_info arch;
struct vcpu_time_info time;
}; /* 64 bytes (x86) */
So it isn't clear the additionally padding really buys us anything; if
we play this game without knowing the ABI we'd be screwed anyway. But
it doesn't hurt.
> Opportunistically replace spaces with tables in the other fields.
That part I certainly approve of.
Reviewed-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.
