| Message ID | 1551095970-14645-1-git-send-email-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block/nvme: Remove QEMU_PACKED from naturally aligned NVMeRegs struct | expand |
On Mon, 25 Feb 2019 at 11:59, Thomas Huth <thuth@redhat.com> wrote: > > The QEMU_PACKED is causing a compiler warning/error with GCC 9: > > CC block/nvme.o > block/nvme.c: In function ‘nvme_create_queue_pair’: > block/nvme.c:209:22: error: taking address of packed member of > ‘struct <anonymous>’ may result in an unaligned pointer value > [-Werror=address-of-packed-member] > 209 | q->sq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale]; > > All members of the struct are naturally aligned, so there should > not be the need for QEMU_PACKED here, and the following QEMU_BUILD_BUG_ON > also ensures that there is no padding. Thus simply remove the QEMU_PACKED > here. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1817525 > Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > block/nvme.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/nvme.c b/block/nvme.c > index b5952c9..6c2ce7d 100644 > --- a/block/nvme.c > +++ b/block/nvme.c > @@ -82,7 +82,7 @@ typedef volatile struct { > uint8_t reserved1[0xec0]; > uint8_t cmd_set_specfic[0x100]; > uint32_t doorbells[]; > -} QEMU_PACKED NVMeRegs; > +} NVMeRegs; > > QEMU_BUILD_BUG_ON(offsetof(NVMeRegs, doorbells) != 0x1000); > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
Am 25.02.2019 um 12:59 hat Thomas Huth geschrieben: > The QEMU_PACKED is causing a compiler warning/error with GCC 9: > > CC block/nvme.o > block/nvme.c: In function ‘nvme_create_queue_pair’: > block/nvme.c:209:22: error: taking address of packed member of > ‘struct <anonymous>’ may result in an unaligned pointer value > [-Werror=address-of-packed-member] > 209 | q->sq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale]; > > All members of the struct are naturally aligned, so there should > not be the need for QEMU_PACKED here, and the following QEMU_BUILD_BUG_ON > also ensures that there is no padding. Thus simply remove the QEMU_PACKED > here. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1817525 > Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> Thanks, applied to the block branch. Though I'm not sure why a compiler should warn if packed and non-packed result in the exact same layout anyway... Kevin
On Mon, 25 Feb 2019 at 14:09, Kevin Wolf <kwolf@redhat.com> wrote: > Though I'm not sure why a compiler should warn if packed and non-packed > result in the exact same layout anyway... Packed implies "and any time you see a pointer to this struct it might not be aligned". Non-packed implies "you can assume that the base of the struct is aligned". thanks -- PMM
On 25/02/2019 15.30, Peter Maydell wrote: > On Mon, 25 Feb 2019 at 14:09, Kevin Wolf <kwolf@redhat.com> wrote: >> Though I'm not sure why a compiler should warn if packed and non-packed >> result in the exact same layout anyway... > > Packed implies "and any time you see a pointer to this struct > it might not be aligned". Non-packed implies "you can assume > that the base of the struct is aligned". Right. FWIW, consider this small program: #include <stdio.h> #include <stdint.h> typedef struct { uint64_t a; uint64_t b; uint64_t c; uint64_t d; } __attribute__((packed)) s1_t; struct { char x; s1_t s1; } s2; int main() { printf("address of s2.s1.d = %p\n", &s2.s1.d); return 0; } Though all members of s1_t look like they are naturally aligned, I get an odd pointer for one of its members in this case. So passing along that pointer to other functions will certainly cause crashes on architectures like Sparc, thus the compiler warning is indeed justified here. Thomas
diff --git a/block/nvme.c b/block/nvme.c index b5952c9..6c2ce7d 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -82,7 +82,7 @@ typedef volatile struct { uint8_t reserved1[0xec0]; uint8_t cmd_set_specfic[0x100]; uint32_t doorbells[]; -} QEMU_PACKED NVMeRegs; +} NVMeRegs; QEMU_BUILD_BUG_ON(offsetof(NVMeRegs, doorbells) != 0x1000);
The QEMU_PACKED is causing a compiler warning/error with GCC 9: CC block/nvme.o block/nvme.c: In function ‘nvme_create_queue_pair’: block/nvme.c:209:22: error: taking address of packed member of ‘struct <anonymous>’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 209 | q->sq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale]; All members of the struct are naturally aligned, so there should not be the need for QEMU_PACKED here, and the following QEMU_BUILD_BUG_ON also ensures that there is no padding. Thus simply remove the QEMU_PACKED here. Buglink: https://bugs.launchpad.net/qemu/+bug/1817525 Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)