Re: [PATCH v3] evtchn/fifo: don't enforce higher than necessary alignment

[Julien Grall <julien@xxxxxxx>]

Hi,

On 21/04/2021 15:36, Jan Beulich wrote:
> Neither the code nor the original commit provide any justification for
> the need to 8-byte align the struct in all cases. Enforce just as much
> alignment as the structure actually needs - 4 bytes - by using alignof()
> instead of a literal number.
I had another fresh look today at this patch. The 32-bit padding is 
right after the field 'ready'.
I can't for sure tell how the second half is going to ever be used and how.

However, one possibility would be to extend the field 'ready' to 64-bit. 
With the current code, we could easily make a single 64-bit access 
without having to know whether the guest is able to interpret the top half.
With your approach, we may need to have different path depending on the 
padding and ensure the new extension cannot be enabled if the padding is 
4-byte. Otherwise, the atomicity would be broken.
> While relaxation of the requirements is intended here, the primary goal
> is to simply get rid of the hard coded number as well its lack of
> connection to the structure that is is meant to apply to.
Based on what I wrote above, I think the relaxation should not be done 
to give us more flexibility about possible extension to the structure.
Although, I would be worth documenting the reasoning in the code.

Cheers,

--
Julien Grall