[Qemu-devel] [PULL 01/14] linuxboot_dma: avoid guest ABI breakage on gcc vs. clang compilation

[Paolo Bonzini]

Recent GCC compiles linuxboot_dma.c to 921 bytes, while CentOS 6 needs
1029 and clang needs 1527.  Because the size of the ROM, rounded to the
next 512 bytes, must match, this causes the API to break between a <1K
ROM and one that is bigger.

We want to make the ROM 1.5 KB in size, but it's better to make clang
produce leaner ROMs, because currently it is worryingly close to the limit.
To fix this prevent clang's happy inlining (which -Os cannot prevent).
This only requires adding a noinline attribute.

Second, the patch makes sure that the ROM has enough padding to prevent
ABI breakage on different compilers.  The size is 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.  This ensures that the padding goes beyond the
next multiple of 512 if necessary, and also avoids 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/linuxboot_dma.bin         | Bin 1024 -> 1536 bytes
 pc-bios/optionrom/linuxboot_dma.c |   8 ++++++--
 scripts/signrom.py                |  27 +++++++++++----------------
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/pc-bios/linuxboot_dma.bin b/pc-bios/linuxboot_dma.bin
index 
e1f623a1245efff0d46ea5b685e0f18e4ef98d74..238a195d3869995067f158d243d852778d38a736
 100644
GIT binary patch
literal 1536
zcmeHFO=}ZT6n#mjn#9N?ZK{K0k;PaL-DD+#E`)TKK>address@hidden;pp*(SMO{f8vv`CI
z_!9&-x)zKBLZ%f_LNuh%S`|r(address@hidden)U07ybYjeTz4D&OPs(`|i7ihXW1v
z&y{3)emWlr%H&aYT!!qlh)#^<3M_khdgexI>gwdhOV?7FoX`3Gb8V4T4OV^17z*G`
address@hidden|K(7+ertc1X4;Vf$ie}J<ZLwEt^>j?IftcP?P
z=6ZtIvqam3<7z0a3aQ<ckN9L~BLt^Q$2W`MSv^F%p+{+-(iPej9lQfu8zf)#1vZMP
z5JZvL=^1gV`EOw68_Z`3oWXfqz&XrSAJM6CJW<GuZej27zvjA-bca39E25gwV_O7!
zub*~*^nBcRu{8oH%o4@@_y=}e#j2yWgBm?rtgvIj;Pp7{L-dOpX&xS>vV%fdk;)OF
zd{0P_V?yG)address@hidden;?lBo<)AHR4WO3UI?t{W35w~doH(lT=XJJbR$+43du
zx6Kjoh43q=nR%#VPPwh#*%zRfiBT!H$tO_6LEl;^Qi;M&6PFP6(lryXhx%9clkV4F
zTOA6?dUuL?mn5!9JS1>O;H0C$j!T^o+y{~$mHc7Bzbo;i#1jI)+1JVFQP?AWovfaO
zJK4t*I(6nOC3y=6=4q%Dn4e<S8>?=0EZ1s*zc=ft{1q{%>6iWxrfC#EH)uF?F&uL=
cH&7qfG012%;address@hidden

literal 1024
address@hidden>p(g$dKVIh%T9=qM-1)wGcJY!KxJW`(h~-Bt;!#jBtbm
z9lAI>address@hidden>JyP;dt+J&wZbtZ~C#n!Tz~o
zj3=j(KEJ*^#!l)_mQr7*PmQM8$hE2ITk*;3<5#ZUh})ymX8Y&b7go%$;tR&$u7;7u
zd7Q&ph$V;`5*|d8xQX0)4B`i}tBiRJVivhZ(1A#CL)y8mD6B{C<hEr(%)@Frcy05r
address@hidden)uLkKKDY|{mCMhBuck3cNL?L^_WO=M-NK^%}T5vY^F7&jdhZEm6x
z-9gZr!&e7e1U%2{W_I8Idzk))+^ZNa;address@hidden>jnC`{bTm=MXKl`$#}Z
address@hidden<mCzY4$Cm;yCQRg`y9;fI_8JMQ=296;address@hidden
zIjAHaC<#g?qItjKw|mkl9&ykoYg?9LBvVjT-1I!E2<address@hidden
zeQokAK0<tI4nn*Qe6aGvK3LB-iBK5#7$vW|oUM^NAGIx*Uy;oe3$W~22Zv-t%7f~N
zK`HnA%QUD46js5euqu00xbwgM6wcIX>{9%R=26W}#UCn8I#H!Vog7g7j<address@hidden
zXdcr%qWGDfj?F_Vsu(JRiXZLqh-}yxO&O(ia7lIvxlXvWT&address@hidden
address@hidden(-r*mtE5S57vp_DS}uK

diff --git a/pc-bios/optionrom/linuxboot_dma.c 
b/pc-bios/optionrom/linuxboot_dma.c
index 8509b28..7549797 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 3\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)
 
-- 
1.8.3.1