Hi Konrad,
On 21/09/16 18:32, Konrad Rzeszutek Wilk wrote:
The test-case is quite simple - we NOP the 'xen_minor_version'.
The amount of NOPs depends on the architecture.
On x86 the function is 11 bytes long:
55 push %rbp <- NOP
48 89 e5 mov %rsp,%rbp <- NOP
b8 04 00 00 00 mov $0x4,%eax <- NOP
5d pop %rbp <- NOP
c3 retq
We can NOP everything but the last instruction (so 10 bytes).
On ARM64 its 8 bytes:
52800100 mov w0, #0x8 <- NOP
d65f03c0 ret
We can NOP the first instruction.
While on ARM32 there are 24 bytes:
e52db004 push {fp}
e28db000 add fp, sp, #0 <- NOP
e3a00008 mov r0, #8 <- NOP
e24bd000 sub sp, fp, #0 <- NOP
e49db004 pop {fp}
e12fff1e bx lr
And we can NOP instruction 2,3, and 4.
Why don't you NOP push {fp} and pop {fp}? At first glance, I am a bit
surprised that the compiler is generating them (maybe it is because you
have debug enabled?) and would have expect to have:
mov r0, #8
bx lr
NOPing all the instructions but the last one would simplify at lot the
test case below.
[...]
--- /dev/null
+++ b/xen/test/livepatch/xen_nop.c
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include <xen/types.h>
+
+#include <public/sysctl.h>
+
+/*
+ * All of the .new_size and .old_addr are based on assumptions that the
+ * code for 'xen_minor_version' is compiled in specific way. Before
+ * running this test-case you MUST verify that the assumptions are
+ * correct (Hint: make debug and look in xen.s).
+ */
+struct livepatch_func __section(".livepatch.funcs") livepatch_nop = {
+ .version = LIVEPATCH_PAYLOAD_VERSION,
+ .old_size = MINOR_VERSION_SZ,
+
+#ifdef CONFIG_X86
+ .old_addr = (void *)MINOR_VERSION_ADDR,
+ /* Everything but the last instruction: "req". */
+ .new_size = MINOR_VERSION_SZ-1,
+#endif
+
+#ifdef CONFIG_ARM_64
+ .old_addr = (void *)MINOR_VERSION_ADDR,
+ /* Replace the first one: "mov w0, #0x8". */
+ .new_size = 4,
+#endif
+
+#ifdef CONFIG_ARM_32
+ /* Skip the first instruction: "push {fp}". */
+ .old_addr = (void *)(MINOR_VERSION_ADDR + 4),
+ /* And replace the next three instructions. */
+ .new_size = 3 * 4,
+#endif
+};
With my suggestion above, the two ARM code could become:
#ifdef CONFIG_ARM
.old_addr = (void *)MINOR_VERSION_ADDR,
.new_size = MINOR_VERSION_SIZE - 4,
#endif
The "- 4" is to avoid replacing the return.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
