| Message ID | 20231114143816.71079-10-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/xen: Have most of Xen files become target-agnostic | expand |
On 14 November 2023 09:38:05 GMT-05:00, "Philippe Mathieu-Daudé" <philmd@linaro.org> wrote: >Except imported source files, QEMU code base uses >the QEMU_ALIGNED() macro to align its structures. > >Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Can we have a BUILD_BUG_ON(sizeof==) for these please?
On Tue, Nov 14, 2023 at 03:38:05PM +0100, Philippe Mathieu-Daudé wrote: > Except imported source files, QEMU code base uses > the QEMU_ALIGNED() macro to align its structures. This patch only convert the alignment, but discard pack. We need both or the struct is changed. > --- > hw/block/xen_blkif.h | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h > index 99733529c1..c1d154d502 100644 > --- a/hw/block/xen_blkif.h > +++ b/hw/block/xen_blkif.h > @@ -18,7 +18,6 @@ struct blkif_common_response { > }; > > /* i386 protocol version */ > -#pragma pack(push, 4) > struct blkif_x86_32_request { > uint8_t operation; /* BLKIF_OP_??? */ > uint8_t nr_segments; /* number of segments */ > @@ -26,7 +25,7 @@ struct blkif_x86_32_request { > uint64_t id; /* private guest value, echoed in resp */ > blkif_sector_t sector_number; /* start sector idx on disk (r/w only) */ > struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; > -}; > +} QEMU_ALIGNED(4); E.g. for this one, I've compare the output of `pahole --class_name=blkif_x86_32_request build/qemu-system-i386`: --- before +++ after @@ -1,11 +1,15 @@ struct blkif_x86_32_request { uint8_t operation; /* 0 1 */ uint8_t nr_segments; /* 1 1 */ uint16_t handle; /* 2 2 */ - uint64_t id; /* 4 8 */ - uint64_t sector_number; /* 12 8 */ - struct blkif_request_segment seg[11]; /* 20 88 */ - /* size: 108, cachelines: 2, members: 6 */ - /* last cacheline: 44 bytes */ -} __attribute__((__packed__)); + /* XXX 4 bytes hole, try to pack */ + + uint64_t id; /* 8 8 */ + uint64_t sector_number; /* 16 8 */ + struct blkif_request_segment seg[11]; /* 24 88 */ + + /* size: 112, cachelines: 2, members: 6 */ + /* sum members: 108, holes: 1, sum holes: 4 */ + /* last cacheline: 48 bytes */ +} __attribute__((__aligned__(8))); Thanks,
On 27 March 2024 13:31:52 GMT, Anthony PERARD <anthony.perard@cloud.com> wrote: >On Tue, Nov 14, 2023 at 03:38:05PM +0100, Philippe Mathieu-Daudé wrote: >> Except imported source files, QEMU code base uses >> the QEMU_ALIGNED() macro to align its structures. > >This patch only convert the alignment, but discard pack. We need both or >the struct is changed. Which means we need some build-time asserts on struct size and field offsets. That should never have passed a build test.
diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h index 99733529c1..c1d154d502 100644 --- a/hw/block/xen_blkif.h +++ b/hw/block/xen_blkif.h @@ -18,7 +18,6 @@ struct blkif_common_response { }; /* i386 protocol version */ -#pragma pack(push, 4) struct blkif_x86_32_request { uint8_t operation; /* BLKIF_OP_??? */ uint8_t nr_segments; /* number of segments */ @@ -26,7 +25,7 @@ struct blkif_x86_32_request { uint64_t id; /* private guest value, echoed in resp */ blkif_sector_t sector_number; /* start sector idx on disk (r/w only) */ struct blkif_request_segment seg[BLKIF_MAX_SEGMENTS_PER_REQUEST]; -}; +} QEMU_ALIGNED(4); struct blkif_x86_32_request_discard { uint8_t operation; /* BLKIF_OP_DISCARD */ uint8_t flag; /* nr_segments in request struct */ @@ -34,15 +33,14 @@ struct blkif_x86_32_request_discard { uint64_t id; /* private guest value, echoed in resp */ blkif_sector_t sector_number; /* start sector idx on disk (r/w only) */ uint64_t nr_sectors; /* # of contiguous sectors to discard */ -}; +} QEMU_ALIGNED(4); struct blkif_x86_32_response { uint64_t id; /* copied from request */ uint8_t operation; /* copied from request */ int16_t status; /* BLKIF_RSP_??? */ -}; +} QEMU_ALIGNED(4); typedef struct blkif_x86_32_request blkif_x86_32_request_t; typedef struct blkif_x86_32_response blkif_x86_32_response_t; -#pragma pack(pop) /* x86_64 protocol version */ struct blkif_x86_64_request {
Except imported source files, QEMU code base uses the QEMU_ALIGNED() macro to align its structures. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/block/xen_blkif.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)