* [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4)
@ 2018-05-08 10:17 Thomas Huth
2018-05-08 10:44 ` Cornelia Huck
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Thomas Huth @ 2018-05-08 10:17 UTC (permalink / raw)
To: qemu-devel, Cornelia Huck; +Cc: qemu-s390x, Christian Borntraeger
I've run into a compilation error today with the current version of GCC 8:
In file included from s390-ccw.h:49,
from main.c:12:
cio.h:128:1: error: alignment 1 of 'struct tpi_info' is less than 4 [-Werror=packed-not-aligned]
} __attribute__ ((packed));
^
cc1: all warnings being treated as errors
Since the struct tpi_info contains an element ("struct subchannel_id schid")
which is marked as aligned(4), we've got to mark the struct tpi_info as
aligned(4), too.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
pc-bios/s390-ccw/cio.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index 55eaeee..1a0795f 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -125,7 +125,7 @@ struct tpi_info {
__u32 reserved3 : 12;
__u32 int_type : 3;
__u32 reserved4 : 12;
-} __attribute__ ((packed));
+} __attribute__ ((packed, aligned(4)));
/* channel command word (type 1) */
struct ccw1 {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4)
2018-05-08 10:17 [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4) Thomas Huth
@ 2018-05-08 10:44 ` Cornelia Huck
2018-05-08 10:49 ` Thomas Huth
2018-05-08 12:20 ` Cornelia Huck
2018-05-16 9:36 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
2 siblings, 1 reply; 8+ messages in thread
From: Cornelia Huck @ 2018-05-08 10:44 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel, qemu-s390x, Christian Borntraeger
On Tue, 8 May 2018 12:17:52 +0200
Thomas Huth <thuth@redhat.com> wrote:
> I've run into a compilation error today with the current version of GCC 8:
>
> In file included from s390-ccw.h:49,
> from main.c:12:
> cio.h:128:1: error: alignment 1 of 'struct tpi_info' is less than 4 [-Werror=packed-not-aligned]
> } __attribute__ ((packed));
> ^
> cc1: all warnings being treated as errors
>
> Since the struct tpi_info contains an element ("struct subchannel_id schid")
> which is marked as aligned(4), we've got to mark the struct tpi_info as
> aligned(4), too.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> pc-bios/s390-ccw/cio.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
> index 55eaeee..1a0795f 100644
> --- a/pc-bios/s390-ccw/cio.h
> +++ b/pc-bios/s390-ccw/cio.h
> @@ -125,7 +125,7 @@ struct tpi_info {
> __u32 reserved3 : 12;
> __u32 int_type : 3;
> __u32 reserved4 : 12;
> -} __attribute__ ((packed));
> +} __attribute__ ((packed, aligned(4)));
>
> /* channel command word (type 1) */
> struct ccw1 {
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Alternatively, we could also ditch this struct and the tpi function...
but I have not given up hope yet that we might someday handle channel
I/O more canonically in the bios :)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4)
2018-05-08 10:44 ` Cornelia Huck
@ 2018-05-08 10:49 ` Thomas Huth
2018-05-08 11:06 ` Cornelia Huck
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2018-05-08 10:49 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-devel, qemu-s390x, Christian Borntraeger
On 08.05.2018 12:44, Cornelia Huck wrote:
> On Tue, 8 May 2018 12:17:52 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> I've run into a compilation error today with the current version of GCC 8:
>>
>> In file included from s390-ccw.h:49,
>> from main.c:12:
>> cio.h:128:1: error: alignment 1 of 'struct tpi_info' is less than 4 [-Werror=packed-not-aligned]
>> } __attribute__ ((packed));
>> ^
>> cc1: all warnings being treated as errors
>>
>> Since the struct tpi_info contains an element ("struct subchannel_id schid")
>> which is marked as aligned(4), we've got to mark the struct tpi_info as
>> aligned(4), too.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> pc-bios/s390-ccw/cio.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
>> index 55eaeee..1a0795f 100644
>> --- a/pc-bios/s390-ccw/cio.h
>> +++ b/pc-bios/s390-ccw/cio.h
>> @@ -125,7 +125,7 @@ struct tpi_info {
>> __u32 reserved3 : 12;
>> __u32 int_type : 3;
>> __u32 reserved4 : 12;
>> -} __attribute__ ((packed));
>> +} __attribute__ ((packed, aligned(4)));
>>
>> /* channel command word (type 1) */
>> struct ccw1 {
>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>
> Alternatively, we could also ditch this struct and the tpi function...
> but I have not given up hope yet that we might someday handle channel
> I/O more canonically in the bios :)
Yes, it's currently unused (so I think you could also pick up the patch
directly, without the need for recompiling the s390-ccw.img just because
of this) ... I don't mind too much if we fix it or remove it, but since
you've said that it might be useful in the future again, I think we can
simply keep it for now.
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4)
2018-05-08 10:49 ` Thomas Huth
@ 2018-05-08 11:06 ` Cornelia Huck
2018-05-08 11:09 ` Christian Borntraeger
2018-05-08 11:12 ` Thomas Huth
0 siblings, 2 replies; 8+ messages in thread
From: Cornelia Huck @ 2018-05-08 11:06 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel, qemu-s390x, Christian Borntraeger
On Tue, 8 May 2018 12:49:59 +0200
Thomas Huth <thuth@redhat.com> wrote:
> On 08.05.2018 12:44, Cornelia Huck wrote:
> > On Tue, 8 May 2018 12:17:52 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >
> >> I've run into a compilation error today with the current version of GCC 8:
> >>
> >> In file included from s390-ccw.h:49,
> >> from main.c:12:
> >> cio.h:128:1: error: alignment 1 of 'struct tpi_info' is less than 4 [-Werror=packed-not-aligned]
> >> } __attribute__ ((packed));
> >> ^
> >> cc1: all warnings being treated as errors
> >>
> >> Since the struct tpi_info contains an element ("struct subchannel_id schid")
> >> which is marked as aligned(4), we've got to mark the struct tpi_info as
> >> aligned(4), too.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >> pc-bios/s390-ccw/cio.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
> >> index 55eaeee..1a0795f 100644
> >> --- a/pc-bios/s390-ccw/cio.h
> >> +++ b/pc-bios/s390-ccw/cio.h
> >> @@ -125,7 +125,7 @@ struct tpi_info {
> >> __u32 reserved3 : 12;
> >> __u32 int_type : 3;
> >> __u32 reserved4 : 12;
> >> -} __attribute__ ((packed));
> >> +} __attribute__ ((packed, aligned(4)));
> >>
> >> /* channel command word (type 1) */
> >> struct ccw1 {
> >
> > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> >
> > Alternatively, we could also ditch this struct and the tpi function...
> > but I have not given up hope yet that we might someday handle channel
> > I/O more canonically in the bios :)
>
> Yes, it's currently unused (so I think you could also pick up the patch
> directly, without the need for recompiling the s390-ccw.img just because
> of this) ... I don't mind too much if we fix it or remove it, but since
> you've said that it might be useful in the future again, I think we can
> simply keep it for now.
Related question: Should this be cc:stable (without a rebuild)? The
same logic as for the just-merged e500 patch probably applies.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4)
2018-05-08 11:06 ` Cornelia Huck
@ 2018-05-08 11:09 ` Christian Borntraeger
2018-05-08 11:12 ` Thomas Huth
1 sibling, 0 replies; 8+ messages in thread
From: Christian Borntraeger @ 2018-05-08 11:09 UTC (permalink / raw)
To: Cornelia Huck, Thomas Huth; +Cc: qemu-devel, qemu-s390x
On 05/08/2018 01:06 PM, Cornelia Huck wrote:
> On Tue, 8 May 2018 12:49:59 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> On 08.05.2018 12:44, Cornelia Huck wrote:
>>> On Tue, 8 May 2018 12:17:52 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> I've run into a compilation error today with the current version of GCC 8:
>>>>
>>>> In file included from s390-ccw.h:49,
>>>> from main.c:12:
>>>> cio.h:128:1: error: alignment 1 of 'struct tpi_info' is less than 4 [-Werror=packed-not-aligned]
>>>> } __attribute__ ((packed));
>>>> ^
>>>> cc1: all warnings being treated as errors
>>>>
>>>> Since the struct tpi_info contains an element ("struct subchannel_id schid")
>>>> which is marked as aligned(4), we've got to mark the struct tpi_info as
>>>> aligned(4), too.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>> pc-bios/s390-ccw/cio.h | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
>>>> index 55eaeee..1a0795f 100644
>>>> --- a/pc-bios/s390-ccw/cio.h
>>>> +++ b/pc-bios/s390-ccw/cio.h
>>>> @@ -125,7 +125,7 @@ struct tpi_info {
>>>> __u32 reserved3 : 12;
>>>> __u32 int_type : 3;
>>>> __u32 reserved4 : 12;
>>>> -} __attribute__ ((packed));
>>>> +} __attribute__ ((packed, aligned(4)));
>>>>
>>>> /* channel command word (type 1) */
>>>> struct ccw1 {
>>>
>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>>>
>>> Alternatively, we could also ditch this struct and the tpi function...
>>> but I have not given up hope yet that we might someday handle channel
>>> I/O more canonically in the bios :)
>>
>> Yes, it's currently unused (so I think you could also pick up the patch
>> directly, without the need for recompiling the s390-ccw.img just because
>> of this) ... I don't mind too much if we fix it or remove it, but since
>> you've said that it might be useful in the future again, I think we can
>> simply keep it for now.
>
> Related question: Should this be cc:stable (without a rebuild)? The
> same logic as for the just-merged e500 patch probably applies.
Ack. Compile warnings/errors qualify for stable I think.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4)
2018-05-08 11:06 ` Cornelia Huck
2018-05-08 11:09 ` Christian Borntraeger
@ 2018-05-08 11:12 ` Thomas Huth
1 sibling, 0 replies; 8+ messages in thread
From: Thomas Huth @ 2018-05-08 11:12 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-devel, qemu-s390x, Christian Borntraeger
On 08.05.2018 13:06, Cornelia Huck wrote:
> On Tue, 8 May 2018 12:49:59 +0200
> Thomas Huth <thuth@redhat.com> wrote:
>
>> On 08.05.2018 12:44, Cornelia Huck wrote:
>>> On Tue, 8 May 2018 12:17:52 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> I've run into a compilation error today with the current version of GCC 8:
>>>>
>>>> In file included from s390-ccw.h:49,
>>>> from main.c:12:
>>>> cio.h:128:1: error: alignment 1 of 'struct tpi_info' is less than 4 [-Werror=packed-not-aligned]
>>>> } __attribute__ ((packed));
>>>> ^
>>>> cc1: all warnings being treated as errors
>>>>
>>>> Since the struct tpi_info contains an element ("struct subchannel_id schid")
>>>> which is marked as aligned(4), we've got to mark the struct tpi_info as
>>>> aligned(4), too.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>> pc-bios/s390-ccw/cio.h | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
>>>> index 55eaeee..1a0795f 100644
>>>> --- a/pc-bios/s390-ccw/cio.h
>>>> +++ b/pc-bios/s390-ccw/cio.h
>>>> @@ -125,7 +125,7 @@ struct tpi_info {
>>>> __u32 reserved3 : 12;
>>>> __u32 int_type : 3;
>>>> __u32 reserved4 : 12;
>>>> -} __attribute__ ((packed));
>>>> +} __attribute__ ((packed, aligned(4)));
>>>>
>>>> /* channel command word (type 1) */
>>>> struct ccw1 {
>>>
>>> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>>>
>>> Alternatively, we could also ditch this struct and the tpi function...
>>> but I have not given up hope yet that we might someday handle channel
>>> I/O more canonically in the bios :)
>>
>> Yes, it's currently unused (so I think you could also pick up the patch
>> directly, without the need for recompiling the s390-ccw.img just because
>> of this) ... I don't mind too much if we fix it or remove it, but since
>> you've said that it might be useful in the future again, I think we can
>> simply keep it for now.
>
> Related question: Should this be cc:stable (without a rebuild)? The
> same logic as for the just-merged e500 patch probably applies.
Since the stable releases are normally not built with -Werror, I think
it's ok to not include it there. OTOH it also does not hurt and silences
an annoying warning, and in case somebody tries to build a stable
release with -Werror, it also fixes the build there. So why not, please
add a "Cc: stable" when you pick up the patch.
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4)
2018-05-08 10:17 [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4) Thomas Huth
2018-05-08 10:44 ` Cornelia Huck
@ 2018-05-08 12:20 ` Cornelia Huck
2018-05-16 9:36 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
2 siblings, 0 replies; 8+ messages in thread
From: Cornelia Huck @ 2018-05-08 12:20 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel, qemu-s390x, Christian Borntraeger
On Tue, 8 May 2018 12:17:52 +0200
Thomas Huth <thuth@redhat.com> wrote:
> I've run into a compilation error today with the current version of GCC 8:
>
> In file included from s390-ccw.h:49,
> from main.c:12:
> cio.h:128:1: error: alignment 1 of 'struct tpi_info' is less than 4 [-Werror=packed-not-aligned]
> } __attribute__ ((packed));
> ^
> cc1: all warnings being treated as errors
>
> Since the struct tpi_info contains an element ("struct subchannel_id schid")
> which is marked as aligned(4), we've got to mark the struct tpi_info as
> aligned(4), too.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> pc-bios/s390-ccw/cio.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
> index 55eaeee..1a0795f 100644
> --- a/pc-bios/s390-ccw/cio.h
> +++ b/pc-bios/s390-ccw/cio.h
> @@ -125,7 +125,7 @@ struct tpi_info {
> __u32 reserved3 : 12;
> __u32 int_type : 3;
> __u32 reserved4 : 12;
> -} __attribute__ ((packed));
> +} __attribute__ ((packed, aligned(4)));
>
> /* channel command word (type 1) */
> struct ccw1 {
Thanks, queued to s390-next with a cc:stable.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [qemu-s390x] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4)
2018-05-08 10:17 [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4) Thomas Huth
2018-05-08 10:44 ` Cornelia Huck
2018-05-08 12:20 ` Cornelia Huck
@ 2018-05-16 9:36 ` David Hildenbrand
2 siblings, 0 replies; 8+ messages in thread
From: David Hildenbrand @ 2018-05-16 9:36 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Cornelia Huck; +Cc: Christian Borntraeger, qemu-s390x
On 08.05.2018 12:17, Thomas Huth wrote:
> I've run into a compilation error today with the current version of GCC 8:
>
> In file included from s390-ccw.h:49,
> from main.c:12:
> cio.h:128:1: error: alignment 1 of 'struct tpi_info' is less than 4 [-Werror=packed-not-aligned]
> } __attribute__ ((packed));
> ^
> cc1: all warnings being treated as errors
>
> Since the struct tpi_info contains an element ("struct subchannel_id schid")
> which is marked as aligned(4), we've got to mark the struct tpi_info as
> aligned(4), too.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> pc-bios/s390-ccw/cio.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
> index 55eaeee..1a0795f 100644
> --- a/pc-bios/s390-ccw/cio.h
> +++ b/pc-bios/s390-ccw/cio.h
> @@ -125,7 +125,7 @@ struct tpi_info {
> __u32 reserved3 : 12;
> __u32 int_type : 3;
> __u32 reserved4 : 12;
> -} __attribute__ ((packed));
> +} __attribute__ ((packed, aligned(4)));
>
> /* channel command word (type 1) */
> struct ccw1 {
>
Late but still
Reviewed-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-16 9:36 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08 10:17 [Qemu-devel] [PATCH] pc-bios/s390-ccw: struct tpi_info must be declared as aligned(4) Thomas Huth
2018-05-08 10:44 ` Cornelia Huck
2018-05-08 10:49 ` Thomas Huth
2018-05-08 11:06 ` Cornelia Huck
2018-05-08 11:09 ` Christian Borntraeger
2018-05-08 11:12 ` Thomas Huth
2018-05-08 12:20 ` Cornelia Huck
2018-05-16 9:36 ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.