]> xenbits.xensource.com Git - xen.git/commitdiff
x86/boot: Rework how 32bit C is linked/included for early boot
authorFrediano Ziglio <frediano.ziglio@cloud.com>
Tue, 29 Oct 2024 10:29:38 +0000 (10:29 +0000)
committerAndrew Cooper <andrew.cooper3@citrix.com>
Tue, 29 Oct 2024 15:10:16 +0000 (15:10 +0000)
Right now, the two functions which were really too complicated to write
in asm are compiled as 32bit PIC, linked to a blob and included
directly, using global asm() to arrange for them to have function semantics.

This is limiting and fragile; the use of data relocations will compile
fine but malfunction when used, creating hard-to-debug bugs.

Furthermore, we would like to increase the amount of C, to
deduplicate/unify Xen's boot logic, as well as making it easier to
follow.  Therefore, rework how the 32bit objects are included.

Link all 32bit objects together first.  This allows for sharing of logic
between translation units.  Use differential linking and explicit
imports/exports to confirm that we only have the expected relocations,
and write the object back out as an assembly file so it can be linked
again as if it were 64bit, to integrate with the rest of Xen.

This allows for the use of external references (e.g. access to global
variables) with reasonable assurance of doing so safely.

No functional change.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
xen/arch/x86/boot/.gitignore
xen/arch/x86/boot/Makefile
xen/arch/x86/boot/build32.lds [deleted file]
xen/arch/x86/boot/build32.lds.S [new file with mode: 0644]
xen/arch/x86/boot/cmdline.c
xen/arch/x86/boot/head.S
xen/arch/x86/boot/reloc.c
xen/tools/combine_two_binaries.py [new file with mode: 0755]

index a379db79888e3a617e48c4a5fc1b321cf4f1df1d..595cef6a2c4082fd923e36dc4176055a16e57bec 100644 (file)
@@ -1,3 +1,5 @@
 /mkelf32
-/*.bin
-/*.lnk
+/build32.*.lds
+/built-in-32.*.bin
+/built-in-32.*.map
+/built-in-32.S
index 1199291d2b064a703efe238bec3d26f7b1fc19b0..230a99a1e55f739892f63fdfc5a9862a4b65edb8 100644 (file)
@@ -1,4 +1,5 @@
 obj-bin-y += head.o
+obj-bin-y += built-in-32.o
 
 obj32 := cmdline.32.o
 obj32 += reloc.32.o
@@ -9,9 +10,6 @@ targets   += $(obj32)
 
 obj32 := $(addprefix $(obj)/,$(obj32))
 
-$(obj)/head.o: AFLAGS-y += -Wa$(comma)-I$(obj)
-$(obj)/head.o: $(obj32:.32.o=.bin)
-
 CFLAGS_x86_32 := $(subst -m64,-m32 -march=i686,$(XEN_TREEWIDE_CFLAGS))
 $(call cc-options-add,CFLAGS_x86_32,CC,$(EMBEDDED_EXTRA_CFLAGS))
 CFLAGS_x86_32 += -Werror -fno-builtin -g0 -msoft-float -mregparm=3
@@ -25,14 +23,66 @@ $(obj32): XEN_CFLAGS := $(CFLAGS_x86_32) -fpic
 $(obj)/%.32.o: $(src)/%.c FORCE
        $(call if_changed_rule,cc_o_c)
 
+orphan-handling-$(call ld-option,--orphan-handling=error) := --orphan-handling=error
 LDFLAGS_DIRECT-$(call ld-option,--warn-rwx-segments) := --no-warn-rwx-segments
 LDFLAGS_DIRECT += $(LDFLAGS_DIRECT-y)
 LD32 := $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT))
 
-%.bin: %.lnk
-       $(OBJCOPY) -j .text -O binary $< $@
+# The parameters below tweak the generated linker scripts:
+# - text_gap: padding between .text section external symbols and code.
+# - text_diff: address of the .text section.
+#
+# Note external symbols are only affected by text_diff, while internal symbols
+# are affected by both text_diff and text_gap.  Ensure the sum of gap and diff
+# is greater than 2^16 so that any 16bit relocations if present in the object
+# file turns into a build-time error.
+text_gap := 0x010200
+text_diff := 0x408020
+
+$(obj)/build32.base.lds: AFLAGS-y += -DGAP=$(text_gap) -DTEXT_DIFF=$(text_diff)
+$(obj)/build32.offset.lds: AFLAGS-y += -DGAP=$(text_gap) -DTEXT_DIFF=$(text_diff) -DFINAL
+$(obj)/build32.base.lds $(obj)/build32.offset.lds: $(src)/build32.lds.S FORCE
+       $(call if_changed_dep,cpp_lds_S)
+
+targets += build32.offset.lds build32.base.lds
+
+# Generate a single 32bit object.
+#
+# Resolve any relocations resulting from references between the translation
+# units.  This ensures the same combined object file can be used to generate
+# multiple images with slightly different linker scripts.
+$(obj)/built-in-32.tmp.o: $(obj32)
+       $(LD32) -r -o $@ $^
+
+# Link bundle with a given layout and extract a binary from it.
+# The linker will allocate GOP and resolve symbols specified in the linker
+# script.
+# The conversion to binary avoids polluting global symbols not used externally;
+# also removes conflict with needed 64 bit GOP.
+# If possible we use --orphan-handling=error option to make sure we account
+# for all possible sections from C code.
+$(obj)/built-in-32.%.bin: $(obj)/build32.%.lds $(obj)/built-in-32.tmp.o
+       $(LD32) $(orphan-handling-y) -N -T $< -Map $(@:bin=map) -o $(@:bin=o) $(filter %.o,$^)
+       $(OBJCOPY) -j .text -O binary $(@:bin=o) $@
+       rm -f $(@:bin=o)
+
+quiet_cmd_combine = GEN     $@
+cmd_combine = \
+    $(PYTHON) $(srctree)/tools/combine_two_binaries.py \
+              --gap       $(text_gap) \
+              --text-diff $(text_diff) \
+              --script    $(obj)/build32.offset.lds \
+              --bin1      $(obj)/built-in-32.base.bin \
+              --bin2      $(obj)/built-in-32.offset.bin \
+              --map       $(obj)/built-in-32.offset.map \
+              --exports   cmdline_parse_early,reloc \
+              --output    $@
+
+targets += built-in-32.S
 
-%.lnk: %.32.o $(src)/build32.lds
-       $(LD32) -N -T $(filter %.lds,$^) -o $@ $<
+# generate final object file combining and checking above binaries
+$(obj)/built-in-32.S: $(obj)/built-in-32.base.bin $(obj)/built-in-32.offset.bin \
+                      $(srctree)/tools/combine_two_binaries.py FORCE
+       $(call if_changed,combine)
 
-clean-files := *.lnk *.bin
+clean-files := built-in-32.*.bin built-in-32.*.map build32.*.lds
diff --git a/xen/arch/x86/boot/build32.lds b/xen/arch/x86/boot/build32.lds
deleted file mode 100644 (file)
index 56edaa7..0000000
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-ENTRY(_start)
-
-SECTIONS
-{
-  /* Merge code and data into one section. */
-  .text : {
-        *(.text)
-        *(.text.*)
-        *(.data)
-        *(.data.*)
-        *(.rodata)
-        *(.rodata.*)
-        *(.bss)
-        *(.bss.*)
-  }
-
-  /* Dynamic linkage sections.  Collected simply so we can check they're empty. */
-  .got : {
-        *(.got)
-  }
-  .got.plt : {
-        *(.got.plt)
-  }
-  .igot.plt : {
-        *(.igot.plt)
-  }
-  .iplt : {
-        *(.iplt)
-  }
-  .plt : {
-        *(.plt)
-  }
-  .rel : {
-        *(.rel.*)
-  }
-}
-
-ASSERT(SIZEOF(.got) == 0,         ".got non-empty")
-/*
- * At least GNU ld 2.30 and earlier fail to discard the generic part of
- * .got.plt when no actual entries were allocated. Permit this case alongside
- * the section being empty.
- */
-ASSERT(SIZEOF(.got.plt) == 0 ||
-       SIZEOF(.got.plt) == 3 * 4, "unexpected .got.plt size")
-ASSERT(SIZEOF(.igot.plt) == 0,    ".igot.plt non-empty")
-ASSERT(SIZEOF(.iplt) == 0,        ".iplt non-empty")
-ASSERT(SIZEOF(.plt) == 0,         ".plt non-empty")
-ASSERT(SIZEOF(.rel) == 0,         "leftover relocations")
diff --git a/xen/arch/x86/boot/build32.lds.S b/xen/arch/x86/boot/build32.lds.S
new file mode 100644 (file)
index 0000000..d1b9dc6
--- /dev/null
@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifdef FINAL
+# undef GAP
+# define GAP 0
+# define MULT 0
+# define TEXT_START
+#else
+# define MULT 1
+# define TEXT_START TEXT_DIFF
+#endif
+#define DECLARE_IMPORT(name) name = . + (__LINE__ * MULT)
+
+ENTRY(dummy_start)
+
+SECTIONS
+{
+  /* Merge code and data into one section. */
+  .text TEXT_START : {
+        /* Silence linker warning, we are not going to use it */
+        dummy_start = .;
+
+        /*
+         * Any symbols used should be declared below, this ensures which
+         * symbols are visible to the 32bit C boot code.
+         * With the exception of using external 32 bit function (maybe
+         * exported by head.S) they should point all to variables.
+         * Attention should be paid to pointers taking care of possible later
+         * relocation.
+         */
+        DECLARE_IMPORT(__base_relocs_start);
+        DECLARE_IMPORT(__base_relocs_end);
+        . = . + GAP;
+        *(.text)
+        *(.text.*)
+        *(.data)
+        *(.data.*)
+        *(.rodata)
+        *(.rodata.*)
+        *(.bss)
+        *(.bss.*)
+  }
+  /DISCARD/ : {
+       *(.comment)
+       *(.comment.*)
+       *(.note.*)
+  }
+  /* Dynamic linkage sections.  Collected simply so we can check they're empty. */
+  .got : {
+        *(.got)
+  }
+  .got.plt : {
+        *(.got.plt)
+  }
+  .igot.plt : {
+        *(.igot.plt)
+  }
+  .iplt : {
+        *(.iplt)
+  }
+  .plt : {
+        *(.plt)
+  }
+  .rel : {
+        *(.rel.*)
+  }
+}
+
+ASSERT(SIZEOF(.got) == 0,         ".got non-empty")
+/*
+ * At least GNU ld 2.30 and earlier fail to discard the generic part of
+ * .got.plt when no actual entries were allocated. Permit this case alongside
+ * the section being empty.
+ */
+ASSERT(SIZEOF(.got.plt) == 0 ||
+       SIZEOF(.got.plt) == 3 * 4, "unexpected .got.plt size")
+ASSERT(SIZEOF(.igot.plt) == 0,    ".igot.plt non-empty")
+ASSERT(SIZEOF(.iplt) == 0,        ".iplt non-empty")
+ASSERT(SIZEOF(.plt) == 0,         ".plt non-empty")
+ASSERT(SIZEOF(.rel) == 0,         "leftover relocations")
index fc9241ede9a0475d2cfb7b6523e4479581f39f56..196c580e9151b7ea19d71075c4736e8392826205 100644 (file)
  * Linux kernel source (linux/lib/string.c).
  */
 
-/*
- * This entry point is entered from xen/arch/x86/boot/head.S with:
- *   - %eax      = &cmdline,
- *   - %edx      = &early_boot_opts.
- */
-asm (
-    "    .text                         \n"
-    "    .globl _start                 \n"
-    "_start:                           \n"
-    "    jmp  cmdline_parse_early      \n"
-    );
-
 #include <xen/compiler.h>
 #include <xen/kconfig.h>
 #include <xen/macros.h>
index c4de1dfab566c637dfd7f87f6a8b3e9439d97a43..e0776e389680150e58fd7f149f51f914a58d4fa0 100644 (file)
@@ -759,18 +759,6 @@ trampoline_setup:
         /* Jump into the relocated trampoline. */
         lret
 
-        /*
-         * cmdline and reloc are written in C, and linked to be 32bit PIC with
-         * entrypoints at 0 and using the fastcall convention.
-         */
-FUNC_LOCAL(cmdline_parse_early)
-        .incbin "cmdline.bin"
-END(cmdline_parse_early)
-
-FUNC_LOCAL(reloc)
-        .incbin "reloc.bin"
-END(reloc)
-
 ENTRY(trampoline_start)
 #include "trampoline.S"
 ENTRY(trampoline_end)
index 8c58affcd9b2a289848f88ce3127a9e70364b599..94b078d7b10d531e02596d09dcfad836d2202542 100644 (file)
  *    Daniel Kiper <daniel.kiper@oracle.com>
  */
 
-/*
- * This entry point is entered from xen/arch/x86/boot/head.S with:
- *   - %eax       = MAGIC,
- *   - %edx       = INFORMATION_ADDRESS,
- *   - %ecx       = TOPMOST_LOW_MEMORY_STACK_ADDRESS.
- *   - 0x04(%esp) = BOOT_VIDEO_INFO_ADDRESS.
- */
-asm (
-    "    .text                         \n"
-    "    .globl _start                 \n"
-    "_start:                           \n"
-    "    jmp  reloc                    \n"
-    );
-
 #include <xen/compiler.h>
 #include <xen/macros.h>
 #include <xen/types.h>
diff --git a/xen/tools/combine_two_binaries.py b/xen/tools/combine_two_binaries.py
new file mode 100755 (executable)
index 0000000..447c0d3
--- /dev/null
@@ -0,0 +1,220 @@
+#!/usr/bin/env python3
+
+from __future__ import print_function
+import argparse
+import functools
+import re
+import struct
+import sys
+
+parser = argparse.ArgumentParser(description='Generate assembly file to merge into other code.')
+auto_int = functools.update_wrapper(lambda x: int(x, 0), int) # allows hex
+parser.add_argument('--script', dest='script',
+                    required=True,
+                    help='Linker script for extracting symbols')
+parser.add_argument('--bin1', dest='bin1',
+                    required=True,
+                    help='First binary')
+parser.add_argument('--bin2', dest='bin2',
+                    required=True,
+                    help='Second binary')
+parser.add_argument('--gap', dest='gap',
+                    required=True,
+                    type=auto_int,
+                    help='Gap inserted at the start of code section')
+parser.add_argument('--text-diff', dest='text_diff',
+                    required=True,
+                    type=auto_int,
+                    help='Difference between code section start')
+parser.add_argument('--output', dest='output',
+                    help='Output file')
+parser.add_argument('--map', dest='mapfile',
+                    help='Map file to read for symbols to export')
+parser.add_argument('--exports', dest='exports',
+                    help='Symbols to export')
+parser.add_argument('--section-header', dest='section_header',
+                    default='.section .init.text, "ax", @progbits',
+                    help='Section header declaration')
+parser.add_argument('-v', '--verbose',
+                    action='store_true')
+args = parser.parse_args()
+
+gap = args.gap
+text_diff = args.text_diff
+
+# Parse linker script for external symbols to use.
+# Next regex matches expanded DECLARE_IMPORT lines in linker script.
+symbol_re = re.compile(r'\s+(\S+)\s*=\s*\.\s*\+\s*\((\d+)\s*\*\s*0\s*\)\s*;')
+symbols = {}
+lines = {}
+for line in open(args.script):
+    m = symbol_re.match(line)
+    if not m:
+        continue
+    (name, line_num) = (m.group(1), int(m.group(2)))
+    if line_num == 0:
+        raise Exception("Invalid line number found:\n\t" + line)
+    if line_num in symbols:
+        raise Exception("Symbol with this line already present:\n\t" + line)
+    if name in lines:
+        raise Exception("Symbol with this name already present:\n\t" + name)
+    symbols[line_num] = name
+    lines[name] = line_num
+
+exports = []
+if args.exports is not None:
+    exports = dict([(name, None) for name in args.exports.split(',')])
+
+# Parse mapfile, look for ther symbols we want to export.
+if args.mapfile is not None:
+    symbol_re = re.compile(r'\s{15,}0x([0-9a-f]+)\s+(\S+)\n')
+    for line in open(args.mapfile):
+        m = symbol_re.match(line)
+        if not m or m.group(2) not in exports:
+            continue
+        addr = int(m.group(1), 16)
+        exports[m.group(2)] = addr
+for (name, addr) in exports.items():
+    if addr is None:
+        raise Exception("Required export symbols %s not found" % name)
+
+file1 = open(args.bin1, 'rb')
+file2 = open(args.bin2, 'rb')
+file1.seek(0, 2)
+size1 = file1.tell()
+file2.seek(0, 2)
+size2 = file2.tell()
+if size1 > size2:
+    file1, file2 = file2, file1
+    size1, size2 = size2, size1
+if size2 != size1 + gap:
+    raise Exception('File sizes do not match')
+del size2
+
+file1.seek(0, 0)
+data1 = file1.read(size1)
+del file1
+file2.seek(gap, 0)
+data2 = file2.read(size1)
+del file2
+
+max_line = max(symbols.keys())
+
+def to_int32(n):
+    '''Convert a number to signed 32 bit integer truncating if needed'''
+    mask = (1 << 32) - 1
+    h = 1 << 31
+    return (n & mask) ^ h - h
+
+i = 0
+references = {}
+internals = 0
+while i <= size1 - 4:
+    n1 = struct.unpack('<I', data1[i:i+4])[0]
+    n2 = struct.unpack('<I', data2[i:i+4])[0]
+    i += 1
+    # The two numbers are the same, no problems
+    if n1 == n2:
+        continue
+    # Try to understand why they are different
+    diff = to_int32(n1 - n2)
+    if diff == -gap: # this is an internal relocation
+        pos = i - 1
+        print("Internal relocation found at position %#x "
+              "n1=%#x n2=%#x diff=%#x" % (pos, n1, n2, diff),
+              file=sys.stderr)
+        i += 3
+        internals += 1
+        if internals >= 10:
+            break
+        continue
+    # This is a relative relocation to a symbol, accepted, code/data is
+    # relocatable.
+    if diff < gap and diff >= gap - max_line:
+        n = gap - diff
+        symbol = symbols.get(n)
+        # check we have a symbol
+        if symbol is None:
+            raise Exception("Cannot find symbol for line %d" % n)
+        pos = i - 1
+        if args.verbose:
+            print('Position %#x %d %s' % (pos, n, symbol), file=sys.stderr)
+        i += 3
+        references[pos] = symbol
+        continue
+    # First byte is the same, move to next byte
+    if diff & 0xff == 0 and i <= size1 - 4:
+       continue
+    # Probably a type of relocation we don't want or support
+    pos = i - 1
+    suggestion = ''
+    symbol = symbols.get(-diff - text_diff)
+    if symbol is not None:
+        suggestion = " Maybe %s is not defined as hidden?" % symbol
+    raise Exception("Unexpected difference found at %#x "
+                    "n1=%#x n2=%#x diff=%#x gap=%#x.%s" % \
+                    (pos, n1, n2, diff, gap, suggestion))
+if internals != 0:
+    raise Exception("Previous relocations found")
+
+def line_bytes(buf, out):
+    '''Output an assembly line with all bytes in "buf"'''
+    # Python 2 compatibility
+    if type(buf) == str:
+        print("\t.byte " + ','.join([str(ord(c)) for c in buf]), file=out)
+    else:
+        print("\t.byte " + ','.join([str(n) for n in buf]), file=out)
+
+def part(start, end, out):
+    '''Output bytes of "data" from "start" to "end"'''
+    while start < end:
+        e = min(start + 16, end)
+        line_bytes(data1[start:e], out)
+        start = e
+
+def reference(pos, out):
+    name = references[pos]
+    n = struct.unpack('<I', data1[pos:pos+4])[0]
+    sign = '+'
+    if n >= (1 << 31):
+        n -= (1 << 32)
+    n += pos
+    if n < 0:
+        n = -n
+        sign = '-'
+    print("\t.hidden %s\n"
+          "\t.long %s %s %#x - ." % (name, name, sign, n),
+          file=out)
+
+def output(out):
+    prev = 0
+    exports_by_addr = {}
+    for (sym, addr) in exports.items():
+        exports_by_addr.setdefault(addr, []).append(sym)
+    positions = list(references.keys())
+    positions += list(exports_by_addr.keys())
+    for pos in sorted(positions):
+        part(prev, pos, out)
+        prev = pos
+        if pos in references:
+            reference(pos, out)
+            prev = pos + 4
+        if pos in exports_by_addr:
+            for sym in exports_by_addr[pos]:
+                print("\t.global %s\n"
+                      "\t.hidden %s\n"
+                      "%s:" % (sym, sym, sym),
+                      file=out)
+    part(prev, size1, out)
+
+out = sys.stdout
+if args.output is not None:
+    out = open(args.output, 'w')
+print('''/*
+ * File autogenerated by combine_two_binaries.py DO NOT EDIT
+ */''', file=out)
+print('\t' + args.section_header, file=out)
+print('obj32_start:', file=out)
+output(out)
+print('\n\t.section .note.GNU-stack,"",@progbits', file=out)
+out.flush()