* [Qemu-devel] [PATCH] virtio-scsi: Fix build with gcc 9
@ 2019-02-28 17:59 Greg Kurz
2019-02-28 18:31 ` Peter Maydell
0 siblings, 1 reply; 3+ messages in thread
From: Greg Kurz @ 2019-02-28 17:59 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz, Michael S. Tsirkin, Paolo Bonzini, Peter Maydell
Build fails with gcc 9:
CC ppc64-softmmu/hw/scsi/virtio-scsi.o
hw/scsi/virtio-scsi.c: In function ‘virtio_scsi_do_tmf’:
hw/scsi/virtio-scsi.c:265:39: error: taking address of packed member of ‘struct virtio_scsi_ctrl_tmf_req’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
265 | virtio_tswap32s(VIRTIO_DEVICE(s), &req->req.tmf.subtype);
| ^~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
All the fields in struct virtio_scsi_ctrl_tmf_req are naturally aligned,
so we could in theory drop QEMU_PACKED. Unfortunately, the header file
is imported from linux which already has the packed attribute. Trying to
fix that in the update-linux-headers.sh script is likely to produce
ugliness. Turn the call to virtio_tswap32s() into an assignment instead.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/scsi/virtio-scsi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index ce99d288b035..839f1202567d 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -262,7 +262,13 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
/* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE". */
req->resp.tmf.response = VIRTIO_SCSI_S_OK;
- virtio_tswap32s(VIRTIO_DEVICE(s), &req->req.tmf.subtype);
+ /*
+ * req->req.tmf has the QEMU_PACKED attribute. Don't use virtio_tswap32s()
+ * to avoid compiler errors.
+ */
+ req->req.tmf.subtype =
+ virtio_tswap32(VIRTIO_DEVICE(s), req->req.tmf.subtype);
+
switch (req->req.tmf.subtype) {
case VIRTIO_SCSI_T_TMF_ABORT_TASK:
case VIRTIO_SCSI_T_TMF_QUERY_TASK:
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] virtio-scsi: Fix build with gcc 9
2019-02-28 17:59 [Qemu-devel] [PATCH] virtio-scsi: Fix build with gcc 9 Greg Kurz
@ 2019-02-28 18:31 ` Peter Maydell
2019-03-01 13:19 ` Paolo Bonzini
0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2019-02-28 18:31 UTC (permalink / raw)
To: Greg Kurz; +Cc: QEMU Developers, Michael S. Tsirkin, Paolo Bonzini
On Thu, 28 Feb 2019 at 17:59, Greg Kurz <groug@kaod.org> wrote:
>
> Build fails with gcc 9:
>
> CC ppc64-softmmu/hw/scsi/virtio-scsi.o
> hw/scsi/virtio-scsi.c: In function ‘virtio_scsi_do_tmf’:
> hw/scsi/virtio-scsi.c:265:39: error: taking address of packed member of ‘struct virtio_scsi_ctrl_tmf_req’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
> 265 | virtio_tswap32s(VIRTIO_DEVICE(s), &req->req.tmf.subtype);
> | ^~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> All the fields in struct virtio_scsi_ctrl_tmf_req are naturally aligned,
> so we could in theory drop QEMU_PACKED. Unfortunately, the header file
> is imported from linux which already has the packed attribute. Trying to
> fix that in the update-linux-headers.sh script is likely to produce
> ugliness. Turn the call to virtio_tswap32s() into an assignment instead.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> hw/scsi/virtio-scsi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index ce99d288b035..839f1202567d 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -262,7 +262,13 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
> /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE". */
> req->resp.tmf.response = VIRTIO_SCSI_S_OK;
>
> - virtio_tswap32s(VIRTIO_DEVICE(s), &req->req.tmf.subtype);
> + /*
> + * req->req.tmf has the QEMU_PACKED attribute. Don't use virtio_tswap32s()
> + * to avoid compiler errors.
> + */
> + req->req.tmf.subtype =
> + virtio_tswap32(VIRTIO_DEVICE(s), req->req.tmf.subtype);
> +
> switch (req->req.tmf.subtype) {
> case VIRTIO_SCSI_T_TMF_ABORT_TASK:
> case VIRTIO_SCSI_T_TMF_QUERY_TASK:
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
I haven't generally bothered to add comments about packed
structs when I've switched away from the *s versions of byteswap
functions, but it doesn't hurt.
We should consider just replacing the other dozen uses
of virtio_tswap*s() with the non-s versions and dropping the
s functions entirely...
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] virtio-scsi: Fix build with gcc 9
2019-02-28 18:31 ` Peter Maydell
@ 2019-03-01 13:19 ` Paolo Bonzini
0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2019-03-01 13:19 UTC (permalink / raw)
To: Peter Maydell, Greg Kurz; +Cc: QEMU Developers, Michael S. Tsirkin
On 28/02/19 19:31, Peter Maydell wrote:
> On Thu, 28 Feb 2019 at 17:59, Greg Kurz <groug@kaod.org> wrote:
>>
>> Build fails with gcc 9:
>>
>> CC ppc64-softmmu/hw/scsi/virtio-scsi.o
>> hw/scsi/virtio-scsi.c: In function ‘virtio_scsi_do_tmf’:
>> hw/scsi/virtio-scsi.c:265:39: error: taking address of packed member of ‘struct virtio_scsi_ctrl_tmf_req’ may result in an unaligned pointer value [-Werror=address-of-packed-member]
>> 265 | virtio_tswap32s(VIRTIO_DEVICE(s), &req->req.tmf.subtype);
>> | ^~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>> All the fields in struct virtio_scsi_ctrl_tmf_req are naturally aligned,
>> so we could in theory drop QEMU_PACKED. Unfortunately, the header file
>> is imported from linux which already has the packed attribute. Trying to
>> fix that in the update-linux-headers.sh script is likely to produce
>> ugliness. Turn the call to virtio_tswap32s() into an assignment instead.
>>
>> Signed-off-by: Greg Kurz <groug@kaod.org>
>> ---
>> hw/scsi/virtio-scsi.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
>> index ce99d288b035..839f1202567d 100644
>> --- a/hw/scsi/virtio-scsi.c
>> +++ b/hw/scsi/virtio-scsi.c
>> @@ -262,7 +262,13 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req)
>> /* Here VIRTIO_SCSI_S_OK means "FUNCTION COMPLETE". */
>> req->resp.tmf.response = VIRTIO_SCSI_S_OK;
>>
>> - virtio_tswap32s(VIRTIO_DEVICE(s), &req->req.tmf.subtype);
>> + /*
>> + * req->req.tmf has the QEMU_PACKED attribute. Don't use virtio_tswap32s()
>> + * to avoid compiler errors.
>> + */
>> + req->req.tmf.subtype =
>> + virtio_tswap32(VIRTIO_DEVICE(s), req->req.tmf.subtype);
>> +
>> switch (req->req.tmf.subtype) {
>> case VIRTIO_SCSI_T_TMF_ABORT_TASK:
>> case VIRTIO_SCSI_T_TMF_QUERY_TASK:
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> I haven't generally bothered to add comments about packed
> structs when I've switched away from the *s versions of byteswap
> functions, but it doesn't hurt.
>
> We should consider just replacing the other dozen uses
> of virtio_tswap*s() with the non-s versions and dropping the
> s functions entirely...
Applied, thanks.
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-01 13:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-28 17:59 [Qemu-devel] [PATCH] virtio-scsi: Fix build with gcc 9 Greg Kurz
2019-02-28 18:31 ` Peter Maydell
2019-03-01 13:19 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).