[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom
From: |
Stefano Garzarella |
Subject: |
Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom |
Date: |
Mon, 22 Mar 2021 11:59:10 +0100 |
On Fri, Mar 19, 2021 at 7:25 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Fri, Mar 19, 2021 at 7:20 PM Stefano Garzarella <sgarzare@redhat.com>
> wrote:
> >
> > On Fri, Mar 19, 2021 at 06:52:39PM +0100, Paolo Bonzini wrote:
> > >It's likely that the compiler will online it. But indeed it's better to add
> > >-minline-all-stringops to the compiler command line.
> > >
> >
> > Cool, I didn't know that one!
> > I tried but I did something wrong because the linker is not happy, next
> > week I'll check better:
> > ld: pvh_main.o: in function `search_rsdp':
> > /home/stefano/repos/qemu/pc-bios/optionrom/pvh_main.c:62: undefined
> > reference to `memcmp'
> > ld: /home/stefano/repos/qemu/pc-bios/optionrom/pvh_main.c:62: undefined
> > reference to `memcmp'
> >
> >
> > In the mean time, I simply tried to assign the RSDP_SIGNATURE macro to
> > an uint64_t variable and I have this new warning, using gcc 10.2.1 "cc
> > (Alpine 10.2.1_pre2) 10.2.1 20210313":
> >
> > In file included from /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:25:
> > /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c: In function 'search_rsdp':
> > /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:30:42: warning: conversion
> > from 'long long unsigned int' to 'uint64_t' {aka 'long unsigned int'}
> > changes value from '2329016660419433298' to '541348690' [-Woverflow]
> > 30 | #define RSDP_SIGNATURE UINT64_C(0x2052545020445352) /*
> > "RSD PTR " */
> > | ^~~~~~~~~~~~~~~~~~
> > /tmp/qemu-test/src/pc-bios/optionrom/pvh_main.c:69:31: note: in expansion
> > of macro 'RSDP_SIGNATURE'
> > 69 | uint64_t rsdp_signature = RSDP_SIGNATURE;
> > |
> >
> > Using gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9) I don't have it.
> >
> > It seems a bit strange, I don't know if it's related to the fact that we
> > compile with -m16, I'll check better.
>
> Anyway I think that using memcmp() I can switch to a character array to
> store the signature, but this gcc warnings confused me a bit...
>
I'll send a patch to add a simple implementation of memcmp and use it
since I wasn't able to embedded it with -minline-all-stringops.
In the mean time, I played a bit with sizeof() to understand what's
going on and I think there is something strange with Alpine gcc
10.2.1_pre2.
I added this 3 lines in pvh_main.c to print the sizes at compile time
(maybe there is a better way :-):
+ char (*__size1)[sizeof(uint64_t)] = 1;
+ char (*__size2)[sizeof(UINT64_C(1))] = 1;
+ char (*__size3)[sizeof(UINT64_C(0x2052545020445352))] = 1;
If I build with gcc 10.2.1 20201125 (Red Hat 10.2.1-9) everything looks
normal, since they are all 8 bytes:
warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from
integer without a cast [-Wint-conversion]
74 | char (*__size1)[sizeof(uint64_t)] = 1;
| ^
warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from
integer without a cast [-Wint-conversion]
75 | char (*__size2)[sizeof(UINT64_C(1))] = 1;
| ^
warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from
integer without a cast [-Wint-conversion]
76 | char (*__size3)[sizeof(UINT64_C(0x2052545020445352))] = 1;
If I build with gcc 10.2.1 20210313 (Alpine 10.2.1_pre2) uint64_t and
UINT64_C(1) have a size of 4 bytes, while UINT64_C(0x2052545020445352)
has a size of 8 bytes:
warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer from
integer without a cast [-Wint-conversion]
74 | char (*__size1)[sizeof(uint64_t)] = 1;
| ^
warning: initialization of ‘char (*)[4]’ from ‘int’ makes pointer from
integer without a cast [-Wint-conversion]
75 | char (*__size2)[sizeof(UINT64_C(1))] = 1;
| ^
warning: initialization of ‘char (*)[8]’ from ‘int’ makes pointer from
integer without a cast [-Wint-conversion]
76 | char (*__size3)[sizeof(UINT64_C(0x2052545020445352))] = 1;
This is a bit strange...
Thanks,
Stefano
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Philippe Mathieu-Daudé, 2021/03/19
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Stefano Garzarella, 2021/03/19
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Stefano Garzarella, 2021/03/19
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Paolo Bonzini, 2021/03/19
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Stefano Garzarella, 2021/03/19
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Paolo Bonzini, 2021/03/19
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Stefano Garzarella, 2021/03/19
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Stefano Garzarella, 2021/03/19
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom,
Stefano Garzarella <=
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Paolo Bonzini, 2021/03/22
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Stefano Garzarella, 2021/03/22
- Re: [Qemu-devel] [PULL 18/76] optionrom: add new PVH option rom, Paolo Bonzini, 2021/03/22