Re: [qemu-s390x] [Qemu-devel] [PATCH 11/14] hw/vfio/ccw: avoid taking address members in packed structs

[Halil Pasic]

On Fri, 29 Mar 2019 11:11:01 +0000
Daniel P. Berrangé <address@hidden> wrote:

> The GCC 9 compiler complains about many places in s390 code
> that take the address of members of the 'struct SCHIB' which
> is marked packed:
> 
> hw/vfio/ccw.c: In function ‘vfio_ccw_io_notifier_handler’:
> hw/vfio/ccw.c:133:15: warning: taking address of packed member of ‘struct 
> SCHIB’ may result in an unaligned pointer value \
> [-Waddress-of-packed-member]
>   133 |     SCSW *s = &sch->curr_status.scsw;
>       |               ^~~~~~~~~~~~~~~~~~~~~~
> hw/vfio/ccw.c:134:15: warning: taking address of packed member of ‘struct 
> SCHIB’ may result in an unaligned pointer value \
> [-Waddress-of-packed-member]
>   134 |     PMCW *p = &sch->curr_status.pmcw;
>       |               ^~~~~~~~~~~~~~~~~~~~~~
> 
> ...snip many more...
> 
> Almost all of these are just done for convenience to avoid
> typing out long variable/field names when referencing struct
> members. We can get most of this convenience by taking the
> address of the 'struct SCHIB' instead, avoiding triggering
> the compiler warnings.
> 
> In a couple of places we copy via a local variable which is
> a technique already applied elsewhere in s390 code for this
> problem.
> 
> Signed-off-by: Daniel P. Berrangé <address@hidden>

Reviewed-by: Halil Pasic <address@hidden>

BTW the reason for SCHIB being packed is the padding that
would emerge at the end of the struct (sizeof(SCHIB) == 52
and non-packed sizeof(SCHIB) == 54). So getting rid of packed
seems to be viable as long as we write guest memory
with the correct size (52 bytes).

Regards,
Halil