Hi Konrad,
On 12/09/17 01:37, Konrad Rzeszutek Wilk wrote:
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 get:
(XEN) livepatch.c:501: livepatch: xen_bye_world: .livepatch.depends is not
aligned properly!
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: Strip note_depends symbol from
test-cases."
which fixes this.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
---
v2: First version
---
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(-)
diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
index 505dc37cda..922a64436f 100644
--- a/docs/misc/livepatch.markdown
+++ b/docs/misc/livepatch.markdown
@@ -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).
diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index 6831383db1..89ad89dfd5 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -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/firmware/hvmloader/mkhex $(NOTE_DEPENDS) $^ >
$@))
It looks quite odd to use a file in firmware/hvmloader for livepatch.
Would it be possible to move mkhex to a generic place?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
