[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for 2.7] linuxboot_dma: avoid guest ABI breakage
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH for 2.7] linuxboot_dma: avoid guest ABI breakage on gcc vs. clang compilation |
Date: |
Sat, 6 Aug 2016 09:33:54 +0800 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
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 <address@hidden>
> ---
> 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 'address@hidden' failed
Fam
Re: [Qemu-devel] [PATCH for 2.7] linuxboot_dma: avoid guest ABI breakage on gcc vs. clang compilation,
Fam Zheng <=