@@ -430,6 +430,8 @@ checksum, MD5 checksum or any unique value.
The size of these structures varies with the --build-id linker option.
+On ARM32 this section must by four-byte aligned.
+
## Hypercalls
We will employ the sub operations of the system management hypercall (sysctl).
@@ -1,15 +1,7 @@
include $(XEN_ROOT)/Config.mk
-ifeq ($(XEN_TARGET_ARCH),x86_64)
-OBJCOPY_MAGIC := -I binary -O elf64-x86-64 -B i386:x86-64
-endif
-ifeq ($(XEN_TARGET_ARCH),arm64)
-OBJCOPY_MAGIC := -I binary -O elf64-littleaarch64 -B aarch64
-endif
-ifeq ($(XEN_TARGET_ARCH),arm32)
-OBJCOPY_MAGIC := -I binary -O elf32-littlearm -B arm
-endif
-
+NOTE_SYMBOL = "note_depends"
+NOTE_DEPENDS = "const __section(\".livepatch.depends\") $(NOTE_SYMBOL)"
CODE_ADDR=$(shell nm --defined $(1) | grep $(2) | awk '{print "0x"$$1}')
CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{ print "0x"$$2}')
@@ -38,7 +30,7 @@ uninstall:
.PHONY: clean
clean::
- rm -f *.o .*.o.d *.livepatch config.h
+ rm -f *.o .*.o.d *.livepatch config.h livepatch_depends.h hello_world_livepatch_depends.h *.bin
#
# To compute these values we need the binary files: xen-syms
@@ -56,10 +48,10 @@ config.h: xen_hello_world_func.o
echo "#define MINOR_VERSION_ADDR $(MINOR_VERSION_ADDR)"; \
echo "#define OLD_CODE_SZ $(OLD_CODE_SZ)") > $@
-xen_hello_world.o: config.h
+xen_hello_world.o: config.h livepatch_depends.h
.PHONY: $(LIVEPATCH)
-$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
+$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o
$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH) $^
#
@@ -71,40 +63,42 @@ $(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
# not be built (it is for EFI builds), and that we do not have
# the note.o.bin to muck with (as it gets deleted)
#
-.PHONY: note.o
-note.o:
- $(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@.bin
- $(OBJCOPY) $(OBJCOPY_MAGIC) \
- --rename-section=.data=.livepatch.depends,alloc,load,readonly,data,contents -S $@.bin $@
- rm -f $@.bin
+.PHONY: note.bin
+note.bin:
+ $(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@
+
+.PHONY: livepatch_depends.h
+livepatch_depends.h: note.bin
+ $(shell (../../../tools/misc/mkhex $(NOTE_DEPENDS) $^ > $@))
#
# Extract the build-id of the xen_hello_world.livepatch
# (which xen_bye_world will depend on).
#
-.PHONY: hello_world_note.o
-hello_world_note.o: $(LIVEPATCH)
- $(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(LIVEPATCH) $@.bin
- $(OBJCOPY) $(OBJCOPY_MAGIC) \
- --rename-section=.data=.livepatch.depends,alloc,load,readonly,data,contents -S $@.bin $@
- rm -f $@.bin
+.PHONY: hello_world_note.bin
+hello_world_note.bin: $(LIVEPATCH)
+ $(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(LIVEPATCH) $@
+
+.PHONY: hello_world_livepatch_depends.h
+hello_world_livepatch_depends.h: hello_world_note.bin
+ $(shell (../../../tools/misc/mkhex $(NOTE_DEPENDS) $^ > $@))
-xen_bye_world.o: config.h
+xen_bye_world.o: config.h hello_world_livepatch_depends.h
.PHONY: $(LIVEPATCH_BYE)
-$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o
+$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o
$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_BYE) $^
-xen_replace_world.o: config.h
+xen_replace_world.o: config.h livepatch_depends.h
.PHONY: $(LIVEPATCH_REPLACE)
-$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o
+$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o
$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_REPLACE) $^
-xen_nop.o: config.h
+xen_nop.o: config.h livepatch_depends.h
.PHONY: $(LIVEPATCH_NOP)
-$(LIVEPATCH_NOP): xen_nop.o note.o
+$(LIVEPATCH_NOP): xen_nop.o
$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NOP) $^
.PHONY: livepatch
@@ -10,6 +10,7 @@
#include <xen/livepatch.h>
#include <public/sysctl.h>
+#include "hello_world_livepatch_depends.h"
static const char bye_world_patch_this_fnc[] = "xen_extra_version";
extern const char *xen_bye_world(void);
@@ -11,6 +11,7 @@
#include <xen/livepatch_payload.h>
#include <public/sysctl.h>
+#include "livepatch_depends.h"
static const char hello_world_patch_this_fnc[] = "xen_extra_version";
extern const char *xen_hello_world(void);
@@ -7,6 +7,7 @@
#include <xen/types.h>
#include <public/sysctl.h>
+#include "livepatch_depends.h"
/*
* All of the .new_size and .old_addr are based on assumptions that the
@@ -9,6 +9,7 @@
#include <xen/livepatch.h>
#include <public/sysctl.h>
+#include "livepatch_depends.h"
static const char xen_replace_world_name[] = "xen_extra_version";
extern const char *xen_replace_world(void);
By default when using objcopy we lose the alignment when we copy it from xen-syms - with the result that alignment (on ARM32 for example) can be 1: [Nr] Name Type Addr Off Size ES Flg Lk Inf Al .. [ 6] .livepatch.depend PROGBITS 00000000 000093 000024 00 A 0 0 1 That, combined with wacky offset means it will be loaded in memory with the wrong alignment: (XEN) livepatch.c:425: livepatch: xen_bye_world: Loaded .livepatch.depends at 000a08043 And later we crash as the .livepatch.depends is not aligned to four bytes, while the xen_build_id_check expects the code to be four byte aligned and we get an hypervisor crash (on ARM32): (XEN) CPU0: Unexpected Trap: Data Abort (XEN) ----[ Xen-4.10Hello World arm32 debug=y Not tainted ]---- (XEN) CPU: 0 (XEN) PC: 002400a0 xen_build_id_check+0x8/0xe8 ..snip.. (XEN) Xen call trace: (XEN) [<002400a0>] xen_build_id_check+0x8/0xe8 (PC) (XEN) [<0021a9c0>] livepatch_op+0x768/0x1610 (LR) (XEN) [<0023bbe4>] do_sysctl+0x9c8/0xa9c (XEN) [<002673c4>] do_trap_guest_sync+0x11e0/0x177c (XEN) [<0026b6a0>] entry.o#return_from_trap+0/0x4 (XEN) (XEN) (XEN) **************************************** (XEN) Panic on CPU 0: (XEN) CPU0: Unexpected Trap: Data Abort This fix forces all the test-cases to be built with a .livepatch.depends structure containing the build-id extracted from the hypervisor (except the xen_bye_world test-case). We use the 'mkhex' tool instead of 'xxd' as the end result is an 'unsigned' instead of 'char' type array - which naturally forces the alignment to be of four. Also the 'mkhex' tools allows us to pass the section name as parameter. The end result is much better alignment: [ 7] .livepatch.depend PROGBITS 00000000 000094 000024 00 A 0 0 4 Note that thanks to 'unsigned int .. __note_depends' the symbol becomes global: $ readelf --symbols *.livepatch | grep depen 23: 0000000000000000 36 OBJECT GLOBAL HIDDEN 6 note_depends 49: 0000000000000000 36 OBJECT GLOBAL HIDDEN 17 note_depends 16: 0000000000000000 36 OBJECT GLOBAL HIDDEN 3 note_depends 21: 0000000000000000 36 OBJECT GLOBAL HIDDEN 6 note_depends See patch titled: "livepatch/arm/x86: Rename note_depends symbol from test-cases." which fixes this. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> v2: First posting. v3: - Used mkhex from tools/misc instead of tools/firmware/hvmloader/ - Include the XEN crash --- docs/misc/livepatch.markdown | 2 ++ xen/test/livepatch/Makefile | 56 +++++++++++++++------------------- xen/test/livepatch/xen_bye_world.c | 1 + xen/test/livepatch/xen_hello_world.c | 1 + xen/test/livepatch/xen_nop.c | 1 + xen/test/livepatch/xen_replace_world.c | 1 + 6 files changed, 31 insertions(+), 31 deletions(-)