On Fri, 08/05 11:34, Paolo Bonzini wrote: > GCC compiles linuxboot_dma.c to 921 bytes, while clang needs 1527. > This causes the API to break between a GCC-compiled ROM and > one that was obtained with clang. > > First, this patch fixes this by preventing clang's happy inlining (which > -Os cannot prevent). This only requires adding a noinline attribute. > > Second, it makes sure that an unexpected guest ABI breakage cannot happen > in the future. The size must now hardcoded in the file that is passed to > signrom.py, as was the case before commit 6f71b77 ("scripts/signrom.py: > Allow option ROM checksum script to write the size header.", 2016-05-23); > signrom.py however will still pad the input to the requested size, to > avoid the need for -fno-toplevel-reorder which clang doesn't support. > signrom.py can then error out if the requested size is too small for > the actual size of the compiled ROM. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > pc-bios/optionrom/linuxboot_dma.c | 8 ++++++-- > scripts/signrom.py | 27 +++++++++++---------------- > 2 files changed, 17 insertions(+), 18 deletions(-) > > diff --git a/pc-bios/optionrom/linuxboot_dma.c > b/pc-bios/optionrom/linuxboot_dma.c > index 8509b28..8584a49 100644 > --- a/pc-bios/optionrom/linuxboot_dma.c > +++ b/pc-bios/optionrom/linuxboot_dma.c > @@ -25,7 +25,7 @@ asm( > ".global _start\n" > "_start:\n" > " .short 0xaa55\n" > -" .byte 0\n" /* size in 512 units, filled in by signrom.py */ > +" .byte 2\n" /* desired size in 512 units; signrom.py adds padding */ > " .byte 0xcb\n" /* far return without prefix */ > " .org 0x18\n" > " .short 0\n" > @@ -157,7 +157,11 @@ static inline uint32_t be32_to_cpu(uint32_t x) > return bswap32(x); > } > > -static void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len) > +/* clang is happy to inline this function, and bloats the > + * ROM. > + */ > +static __attribute__((__noinline__)) > +void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len) > { > FWCfgDmaAccess access; > uint32_t control = (entry << 16) | BIOS_CFG_DMA_CTL_SELECT > diff --git a/scripts/signrom.py b/scripts/signrom.py > index 5629bca..d1dabe0 100644 > --- a/scripts/signrom.py > +++ b/scripts/signrom.py > @@ -23,26 +23,21 @@ if magic != '\x55\xaa': > > size_byte = ord(fin.read(1)) > fin.seek(0) > +data = fin.read() > > -if size_byte == 0: > - # If the caller left the size field blank then we will fill it in, > - # also rounding the whole input to a multiple of 512 bytes. > - data = fin.read() > - # +1 because we need a byte to store the checksum. > - size = len(data) + 1 > - # Round up to next multiple of 512. > - size += 511 > - size -= size % 512 > - if size >= 65536: > - sys.exit("%s: option ROM size too large" % sys.argv[1]) > +size = size_byte * 512 > +if len(data) > size: > + sys.stderr.write('error: ROM is too large (%d > %d)\n' % (len(data), > size)) > + sys.exit(1) > +elif len(data) < size: > + # Add padding if necessary, rounding the whole input to a multiple of > + # 512 bytes according to the third byte of the input. > # size-1 because a final byte is added below to store the checksum. > data = data.ljust(size-1, '\0') > - data = data[:2] + chr(size/512) + data[3:] > else: > - # Otherwise the input file specifies the size so use it. > - # -1 because we overwrite the last byte of the file with the checksum. > - size = size_byte * 512 - 1 > - data = fin.read(size) > + if ord(data[-1:]) != 0: > + sys.stderr.write('WARNING: ROM includes nonzero checksum\n') > + data = data[:size-1] > > fout.write(data) > > -- > 2.7.4 > >
This size check fails for me on centos6 docker image: Signing optionrom/linuxboot_dma.bin error: ROM is too large (1029 > 1024) make[1]: *** [linuxboot_dma.bin] Error 1 make[1]: *** Deleting file `linuxboot_dma.bin' make: *** [romsubdir-optionrom] Error 2 make: *** Waiting for unfinished jobs.... tests/docker/Makefile.include:104: recipe for target 'docker-run-test-quick@centos6' failed Fam