* [Very RFC PATCH] Livepatch - initial ARM64/32 support.
@ 2016-08-09 4:18 Konrad Rzeszutek Wilk
2016-08-09 4:18 ` [Very RFC PATCH 1/3] mm/arm: Introduce modify_xen_mappings Konrad Rzeszutek Wilk
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-08-09 4:18 UTC (permalink / raw)
To: xen-devel, konrad, sstabellini, julien.grall
Hey!
Over the last couple of months in my spare time I was playing
with making livepatch work with ARM64 (using the FoundationModel
simulator) and I finally got it working tonight.
Sending out the patches just in case they don't work tomorrow :-)
The ARM32 part is going slowly - as I don't have a simulator
and using a real board takes time.
As you can see from the diffstat there is some room
for improvement:
- The 'tests' - they are now in x86 and arm - and they are quite
similar. My thinking is to move them to 'common' ?
- There is room to unify some of the ELF relocation checks as
they are exactly the same across architectures.
There is also an ugly implementation of modify_xen_mappings.
I am hoping the ARM maintainers could provide some input on
how they would like me to implement this.
Thanks!
xen/arch/arm/Makefile | 14 +-
xen/arch/arm/arm32/Makefile | 2 +-
xen/arch/arm/arm32/livepatch.c | 150 ++++++++++++++++
xen/arch/arm/arm64/Makefile | 1 +
xen/arch/arm/arm64/insn.c | 41 +++++
xen/arch/arm/arm64/livepatch.c | 268 +++++++++++++++++++++++++++++
xen/arch/arm/livepatch.c | 84 ++++++---
xen/arch/arm/mm.c | 28 ++-
xen/arch/arm/test/Makefile | 85 +++++++++
xen/arch/arm/test/xen_bye_world.c | 34 ++++
xen/arch/arm/test/xen_bye_world_func.c | 22 +++
xen/arch/arm/test/xen_hello_world.c | 69 ++++++++
xen/arch/arm/test/xen_hello_world_func.c | 26 +++
xen/arch/arm/test/xen_replace_world.c | 33 ++++
xen/arch/arm/test/xen_replace_world_func.c | 22 +++
xen/common/Kconfig | 2 +-
xen/common/livepatch.c | 12 +-
xen/include/asm-arm/arm64/insn.h | 2 +
xen/include/asm-arm/current.h | 7 +
xen/include/asm-arm/mm.h | 1 +
xen/include/xen/elfstructs.h | 35 ++++
21 files changed, 905 insertions(+), 33 deletions(-)
Konrad Rzeszutek Wilk (3):
mm/arm: Introduce modify_xen_mappings
insn: Add arch64_insn_gen_branch_imm to generate branch
livepatch: Initial ARM32/64 support.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Very RFC PATCH 1/3] mm/arm: Introduce modify_xen_mappings
2016-08-09 4:18 [Very RFC PATCH] Livepatch - initial ARM64/32 support Konrad Rzeszutek Wilk
@ 2016-08-09 4:18 ` Konrad Rzeszutek Wilk
2016-08-09 4:18 ` [Very RFC PATCH 2/3] insn: Add arch64_insn_gen_branch_imm to generate branch Konrad Rzeszutek Wilk
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-08-09 4:18 UTC (permalink / raw)
To: xen-devel, konrad, sstabellini, julien.grall
Cc: Ross Lagerwall, Andrew Cooper, Konrad Rzeszutek Wilk
Which is only used by Livepatch code. The purpose behind
this call is to modify the page table entries flags.
Specifically the .ro and .nx flags. The current mechanism
puts cache attributes in the flags and the .ro and .nx are
locked down and assumed to be .ro=0, nx=1.
Livepatch needs .nx=0 and also .ro to be set to 1.
We introduce a new 'flags' where bit 0 determines the .ro
and bit 1 determines the .nx.
TODO:
- Get ARM64 idea of how they want to do this.
- Add #define for R and NX bits for flag (if ARM folks like my idea).
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
--
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
xen/arch/arm/mm.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 4e256c2..eca7cdd 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -836,6 +836,7 @@ static int create_xen_table(lpae_t *entry)
enum xenmap_operation {
INSERT,
REMOVE,
+ MODIFY,
RESERVE
};
@@ -877,18 +878,27 @@ static int create_xen_entries(enum xenmap_operation op,
}
if ( op == RESERVE )
break;
+
pte = mfn_to_xen_entry(mfn, ai);
pte.pt.table = 1;
write_pte(&third[third_table_offset(addr)], pte);
break;
+ case MODIFY:
case REMOVE:
if ( !third[third_table_offset(addr)].pt.valid )
{
- printk("create_xen_entries: trying to remove a non-existing mapping addr=%lx\n",
- addr);
+ printk("create_xen_entries: trying to %s a non-existing mapping addr=%lx\n",
+ op == REMOVE ? "remove" : "modify", addr);
return -EINVAL;
}
- pte.bits = 0;
+ if ( op == REMOVE )
+ pte.bits = 0;
+ else
+ {
+ pte = third[third_table_offset(addr)];
+ pte.pt.ro = (ai >> 1) & 0x1;
+ pte.pt.xn = ai & 0x1;
+ }
write_pte(&third[third_table_offset(addr)], pte);
break;
default:
@@ -922,6 +932,13 @@ void destroy_xen_mappings(unsigned long v, unsigned long e)
create_xen_entries(REMOVE, v, 0, (e - v) >> PAGE_SHIFT, 0);
}
+void modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
+{
+ /* TODO: #define for ro and nx flags. */
+ ASSERT((flags & 0x3) == flags);
+ create_xen_entries(MODIFY, s, 0, (e - s) >> PAGE_SHIFT, flags);
+}
+
enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg)
{
--
2.4.11
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Very RFC PATCH 2/3] insn: Add arch64_insn_gen_branch_imm to generate branch
2016-08-09 4:18 [Very RFC PATCH] Livepatch - initial ARM64/32 support Konrad Rzeszutek Wilk
2016-08-09 4:18 ` [Very RFC PATCH 1/3] mm/arm: Introduce modify_xen_mappings Konrad Rzeszutek Wilk
@ 2016-08-09 4:18 ` Konrad Rzeszutek Wilk
2016-08-11 16:03 ` Julien Grall
2016-08-09 4:19 ` [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support Konrad Rzeszutek Wilk
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-08-09 4:18 UTC (permalink / raw)
To: xen-devel, konrad, sstabellini, julien.grall
Cc: Ross Lagerwall, Konrad Rzeszutek Wilk
We copied from Linux code (v4.7) the code that generates
the unconditional branch instructions. This is mostly
from Linux commit c0cafbae20d2878883ec3c06d6ea30ff38a6bf92
"arm64: introduce aarch64_insn_gen_branch_reg()"
However the branch-link instructions was omitted, only
the branch instruction is generated.
This lays the groundwork for Livepatch to generate the
trampoline to jump to the new replacement function.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
--
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
xen/arch/arm/arm64/insn.c | 41 ++++++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/arm64/insn.h | 2 ++
2 files changed, 43 insertions(+)
diff --git a/xen/arch/arm/arm64/insn.c b/xen/arch/arm/arm64/insn.c
index 12b4d96..15dd7bc 100644
--- a/xen/arch/arm/arm64/insn.c
+++ b/xen/arch/arm/arm64/insn.c
@@ -209,6 +209,47 @@ u32 aarch64_set_branch_offset(u32 insn, s32 offset)
BUG();
}
+static inline long branch_imm_common(unsigned long pc, unsigned long addr,
+ long range)
+{
+ long offset;
+
+ if ((pc & 0x3) || (addr & 0x3)) {
+ pr_err("%s: A64 instructions must be word aligned\n", __func__);
+ return range;
+ }
+
+ offset = ((long)addr - (long)pc);
+
+ if (offset < -range || offset >= range) {
+ pr_err("%s: offset out of range\n", __func__);
+ return range;
+ }
+
+ return offset;
+}
+
+u32 __kprobes aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr)
+{
+ u32 insn;
+ long offset;
+
+ /*
+ * B/BL support [-128M, 128M) offset
+ * ARM64 virtual address arrangement guarantees all kernel and module
+ * texts are within +/-128M.
+ */
+ offset = branch_imm_common(pc, addr, SZ_128M);
+ if (offset >= SZ_128M)
+ return AARCH64_BREAK_FAULT;
+
+ insn = aarch64_insn_get_b_value();
+
+ return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
+ offset >> 2);
+}
+
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
index 6ce37be..378ba11 100644
--- a/xen/include/asm-arm/arm64/insn.h
+++ b/xen/include/asm-arm/arm64/insn.h
@@ -61,6 +61,8 @@ u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
s32 aarch64_get_branch_offset(u32 insn);
u32 aarch64_set_branch_offset(u32 insn, s32 offset);
+u32 aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr);
+
/* Wrapper for common code */
static inline bool insn_is_branch_imm(u32 insn)
{
--
2.4.11
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support.
2016-08-09 4:18 [Very RFC PATCH] Livepatch - initial ARM64/32 support Konrad Rzeszutek Wilk
2016-08-09 4:18 ` [Very RFC PATCH 1/3] mm/arm: Introduce modify_xen_mappings Konrad Rzeszutek Wilk
2016-08-09 4:18 ` [Very RFC PATCH 2/3] insn: Add arch64_insn_gen_branch_imm to generate branch Konrad Rzeszutek Wilk
@ 2016-08-09 4:19 ` Konrad Rzeszutek Wilk
2016-08-12 15:00 ` Julien Grall
2016-08-09 4:24 ` [Very RFC PATCH] Livepatch - initial ARM64/32 support Konrad Rzeszutek Wilk
2016-08-11 16:28 ` Julien Grall
4 siblings, 1 reply; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-08-09 4:19 UTC (permalink / raw)
To: xen-devel, konrad, sstabellini, julien.grall
Cc: Ross Lagerwall, Andrew Cooper, Konrad Rzeszutek Wilk
The initial support for ARM64 - and livepatching
works:
(XEN) livepatch: xen_hello_world: Applying 1 functions
(XEN) hi_func: Hi! (called 1 times)
(XEN) Hook executing.
(XEN) livepatch: xen_hello_world finished APPLY with rc=0
(XEN) livepatch.c:1687: livepatch: load_payload_fnc: rc=0 (p->rc=0)
(XEN) livepatch: Hello World
(XEN) 'x' pressed - Dumping all livepatch patches
(XEN) build-id: e144bafc4ee8635eee5bed8e3988b484765c46c8
(XEN) name=xen_hello_world state=APPLIED(2) 0000000000318000 (.data=0000000000319000, .rodata=000000000031a000) using 3 pages.
(XEN) xen_extra_version patch 0000000000233fac(12) with 0000000000318000 (16)
(XEN) build-id=c4b842c276be43adbe4db788598b1e11bce04dc6
(XEN) depend-on=9affa110481e8e13606c61b21e5f6a357a3c8ef9
ARM32 still has some issues.
The are some TODOs left to be done:
General:
- Bubble ALT_ORIG_PTR macro for both x86/ARM.
- Unify the ELF RELA checks - they are the same on x86/ARM[32,64].
- Makefile generating .livepatch.depends needs to ingest the
-O argument based on architecture
- Test cases should move to common/ ? [Needs Jan's Ack]
ARM32 issues:
- vm_init_type: Assertion 'is_xen_heap_mfn(ma >> PAGE_SHIFT)' failed at xen/include/asm/mm.h:23
- Need to check R_ARM_CALL, R_ARM_JUMP24: Overflow check.
- Need to implement the rest of ELF RELA
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
RFC: Wholy cow! It works!
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
xen/arch/arm/Makefile | 14 +-
xen/arch/arm/arm32/Makefile | 2 +-
xen/arch/arm/arm32/livepatch.c | 150 ++++++++++++++++
xen/arch/arm/arm64/Makefile | 1 +
xen/arch/arm/arm64/livepatch.c | 268 +++++++++++++++++++++++++++++
xen/arch/arm/livepatch.c | 84 ++++++---
xen/arch/arm/mm.c | 5 +
xen/arch/arm/test/Makefile | 85 +++++++++
xen/arch/arm/test/xen_bye_world.c | 34 ++++
xen/arch/arm/test/xen_bye_world_func.c | 22 +++
xen/arch/arm/test/xen_hello_world.c | 69 ++++++++
xen/arch/arm/test/xen_hello_world_func.c | 26 +++
xen/arch/arm/test/xen_replace_world.c | 33 ++++
xen/arch/arm/test/xen_replace_world_func.c | 22 +++
xen/common/Kconfig | 2 +-
xen/common/livepatch.c | 12 +-
xen/include/asm-arm/current.h | 7 +
xen/include/asm-arm/mm.h | 1 +
xen/include/xen/elfstructs.h | 35 ++++
19 files changed, 842 insertions(+), 30 deletions(-)
create mode 100644 xen/arch/arm/arm32/livepatch.c
create mode 100644 xen/arch/arm/arm64/livepatch.c
create mode 100644 xen/arch/arm/test/Makefile
create mode 100644 xen/arch/arm/test/xen_bye_world.c
create mode 100644 xen/arch/arm/test/xen_bye_world_func.c
create mode 100644 xen/arch/arm/test/xen_hello_world.c
create mode 100644 xen/arch/arm/test/xen_hello_world_func.c
create mode 100644 xen/arch/arm/test/xen_replace_world.c
create mode 100644 xen/arch/arm/test/xen_replace_world_func.c
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 0a96713..95cd8af 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -66,6 +66,16 @@ endif
.PHONY: tests
tests:
+ $(MAKE) -f $(BASEDIR)/Rules.mk -C test livepatch
+
+ifdef CONFIG_LIVEPATCH
+all_symbols = --all-symbols
+ifdef CONFIG_FAST_SYMBOL_LOOKUP
+all_symbols = --all-symbols --sort-by-name
+endif
+else
+all_symbols =
+endif
$(TARGET).axf: $(TARGET)-syms
# XXX: VE model loads by VMA so instead of
@@ -93,12 +103,12 @@ $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
$(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
- | $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0.S
+ | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S
$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
$(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
- | $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).1.S
+ | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S
$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
$(@D)/.$(@F).1.o -o $@
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index b20db64..5966de0 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -4,8 +4,8 @@ obj-$(EARLY_PRINTK) += debug.o
obj-y += domctl.o
obj-y += domain.o
obj-y += entry.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o
obj-y += proc-v7.o proc-caxx.o
obj-y += smpboot.o
obj-y += traps.o
obj-y += vfp.o
-
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
new file mode 100644
index 0000000..14a4e12
--- /dev/null
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -0,0 +1,150 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ */
+
+#include <xen/lib.h>
+#include <xen/errno.h>
+#include <xen/livepatch_elf.h>
+#include <xen/livepatch.h>
+
+void arch_livepatch_apply_jmp(struct livepatch_func *func)
+{
+}
+
+void arch_livepatch_revert_jmp(const struct livepatch_func *func)
+{
+}
+
+int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
+{
+ const Elf_Ehdr *hdr = elf->hdr;
+
+ if ( hdr->e_machine != EM_ARM ||
+ hdr->e_ident[EI_CLASS] != ELFCLASS32 )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Unsupported ELF Machine type!\n",
+ elf->name);
+ return -EOPNOTSUPP;
+ }
+
+ if ( (hdr->e_flags & EF_ARM_EABI_MASK) != EF_ARM_EABI_VER5 )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Unsupported ELF EABI(%x)!\n",
+ elf->name, hdr->e_flags);
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+int arch_livepatch_perform_rel(struct livepatch_elf *elf,
+ const struct livepatch_elf_sec *base,
+ const struct livepatch_elf_sec *rela)
+{
+ return -ENOSYS;
+}
+
+int arch_livepatch_perform_rela(struct livepatch_elf *elf,
+ const struct livepatch_elf_sec *base,
+ const struct livepatch_elf_sec *rela)
+{
+ const Elf_RelA *r;
+ unsigned int symndx, i;
+ uint32_t val;
+ void *dest;
+
+
+ if ( !rela->sec->sh_size )
+ return 0;
+
+ if ( rela->sec->sh_entsize < sizeof(Elf_RelA) ||
+ rela->sec->sh_size % rela->sec->sh_entsize )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Section relative header is corrupted!\n",
+ elf->name);
+ return -EINVAL;
+ }
+
+ for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
+ {
+ s32 offset;
+
+ symndx = ELF32_R_SYM(r->r_info);
+ if ( symndx > elf->nsym )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative symbol wants symbol@%u which is past end!\n",
+ elf->name, symndx);
+ return -EINVAL;
+ }
+
+ dest = base->load_addr + r->r_offset; /* P */
+ val = elf->sym[symndx].sym->st_value; /* S */
+
+ /* r->r_addend is computed below. */
+ switch ( ELF32_R_TYPE(r->r_info) ) {
+ case R_ARM_NONE:
+ /* ignore */
+ break;
+
+ case R_ARM_MOVW_ABS_NC:
+ /* MOVW loads 16 bits into the bottom half of a register */
+ /* ResultMask(X) = X & 0xFFFF */
+ case R_ARM_MOVT_ABS:
+ /* MOVT loads 16 bits into the top half of a register.*/
+ /* ResultMask(X)= X & 0xFFFF0000 */
+ if ( ELF32_R_TYPE(r->r_info) == R_ARM_MOVT_ABS )
+ val &= 0xFFFF0000;
+ else
+ val &= 0xFFFF;
+ /*
+ * insn[19:16] = Result_Mask(X) >> 12
+ * insn[11:0] = Result_Mask(X) & 0xFFF
+ */
+ *(u32 *)dest |= val & 0xFFF;
+ *(u32 *)dest |= (val >> 12) << 16;
+ break;
+
+ case R_ARM_ABS32: /* (S + A) | T */
+ *(u32 *)dest = val + r->r_addend;
+ break;
+
+ case R_ARM_CALL:
+ case R_ARM_JUMP24:
+ offset = *(u32 *)dest;
+ /* addend = sign_extend (insn[23:0] << 2) */
+ offset = (offset & 0x00ffffff) << 2;
+ /* (S + A) - P */
+ offset += val - (unsigned long)dest;
+ /* X & 0x03FFFFFE */
+ offset &= 0x03FFFFFE;
+ *(u32 *)dest = offset;
+ /* TODO: Check overflow. */
+ if ( 0 )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Overflow in relocation %u in %s for %s!\n",
+ elf->name, i, rela->name, base->name);
+ return -EOVERFLOW;
+ }
+ break;
+ case R_ARM_REL32: /* ((S + A) | T) – P */
+ *(u32 *)dest = *(u32 *)dest + val - (unsigned long)dest;
+ break;
+
+ default:
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Unhandled relocation #%x\n",
+ elf->name, ELF32_R_TYPE(r->r_info));
+ return -EOPNOTSUPP;
+ }
+ }
+ return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index c1fa43f..149b6b3 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -6,6 +6,7 @@ obj-y += domctl.o
obj-y += domain.o
obj-y += entry.o
obj-y += insn.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o
obj-y += smpboot.o
obj-y += traps.o
obj-y += vfp.o
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
new file mode 100644
index 0000000..cc2e0a1
--- /dev/null
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -0,0 +1,268 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ */
+
+#include <xen/bitops.h>
+#include <xen/errno.h>
+#include <xen/lib.h>
+#include <xen/livepatch_elf.h>
+#include <xen/livepatch.h>
+#include <xen/mm.h>
+#include <xen/vmap.h>
+#include "../livepatch.h"
+
+#include <asm/bitops.h>
+#include <asm/byteorder.h>
+#include <asm/insn.h>
+
+void arch_livepatch_apply_jmp(struct livepatch_func *func)
+{
+ uint32_t insn;
+ uint32_t *old_ptr;
+ uint32_t *new_ptr;
+
+ BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque));
+ BUILD_BUG_ON(PATCH_INSN_SIZE != sizeof(insn));
+
+ if ( !vmap_of_xen_text )
+ return;
+
+ /* Save old one. */
+ old_ptr = func->old_addr;
+ memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE);
+
+ /* Branch, no link. */
+ insn = aarch64_insn_gen_branch_imm((unsigned long)func->old_addr,
+ (unsigned long)func->new_addr);
+
+ new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+
+ /* PATCH! */
+ *(new_ptr) = cpu_to_le32(insn);
+
+ clean_and_invalidate_dcache_va_range(new_ptr, sizeof(*new_ptr));
+}
+
+void arch_livepatch_revert_jmp(const struct livepatch_func *func)
+{
+ uint32_t *new_ptr;
+ uint32_t insn;
+
+ memcpy(&insn, func->opaque, PATCH_INSN_SIZE);
+
+ new_ptr = (uint32_t *)func->old_addr - (u32 *)_start + vmap_of_xen_text;
+
+ /* PATCH! */
+ *(new_ptr) = cpu_to_le32(insn);
+
+ clean_and_invalidate_dcache_va_range(new_ptr, sizeof(*new_ptr));
+}
+
+int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
+{
+ const Elf_Ehdr *hdr = elf->hdr;
+
+ if ( hdr->e_machine != EM_AARCH64 ||
+ hdr->e_ident[EI_CLASS] != ELFCLASS64 )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Unsupported ELF Machine type!\n",
+ elf->name);
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+int arch_livepatch_perform_rel(struct livepatch_elf *elf,
+ const struct livepatch_elf_sec *base,
+ const struct livepatch_elf_sec *rela)
+{
+ return -EOPNOTSUPP;
+}
+
+
+static int reloc_insn_imm(void *dest, u64 val, int lsb, int len,
+ enum aarch64_insn_imm_type imm_type)
+{
+ u64 imm, imm_mask;
+ s64 sval = val;
+ u32 insn = *(u32 *)dest;
+
+ /* Calculate the relocation value. */
+ sval >>= lsb;
+
+ /* Extract the value bits and shift them to bit 0. */
+ imm_mask = (BIT(lsb + len) - 1) >> lsb;
+ imm = sval & imm_mask;
+
+ /* Update the instruction's immediate field. */
+ insn = aarch64_insn_encode_immediate(imm_type, insn, imm);
+ *(u32 *)dest = insn;
+
+ /*
+ * Extract the upper value bits (including the sign bit) and
+ * shift them to bit 0.
+ */
+ sval = (s64)(sval & ~(imm_mask >> 1)) >> (len - 1);
+
+ /*
+ * Overflow has occurred if the upper bits are not all equal to
+ * the sign bit of the value.
+ */
+ if ((u64)(sval + 1) >= 2)
+ return -EOVERFLOW;
+
+ return 0;
+}
+
+int arch_livepatch_perform_rela(struct livepatch_elf *elf,
+ const struct livepatch_elf_sec *base,
+ const struct livepatch_elf_sec *rela)
+{
+ const Elf_RelA *r;
+ unsigned int symndx, i;
+ uint64_t val;
+ void *dest;
+
+ if ( !rela->sec->sh_size )
+ return 0;
+
+ if ( rela->sec->sh_entsize < sizeof(Elf_RelA) ||
+ rela->sec->sh_size % rela->sec->sh_entsize )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Section relative header is corrupted!\n",
+ elf->name);
+ return -EINVAL;
+ }
+
+ for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
+ {
+ int err = 0;
+
+ r = rela->data + i * rela->sec->sh_entsize;
+
+ symndx = ELF64_R_SYM(r->r_info);
+
+ if ( symndx > elf->nsym )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation wants symbol@%u which is past end!\n",
+ elf->name, symndx);
+ return -EINVAL;
+ }
+
+ dest = base->load_addr + r->r_offset; /* P */
+ val = elf->sym[symndx].sym->st_value + r->r_addend; /* S+A */
+
+ /* ARM64 operations at minimum are always 32-bit. */
+ if ( r->r_offset >= base->sec->sh_size ||
+ (r->r_offset + sizeof(uint32_t)) > base->sec->sh_size )
+ goto bad_offset;
+
+ dprintk(XENLOG_DEBUG, LIVEPATCH "%s: %s @%p val=%#lx, type=%ld\n",
+ elf->name, elf->sym[symndx].name, dest, val, ELF64_R_TYPE(r->r_info));
+
+ switch ( ELF64_R_TYPE(r->r_info) ) {
+ /* Data */
+ case R_AARCH64_ABS64:
+ if ( r->r_offset + sizeof(uint64_t) > base->sec->sh_size )
+ goto bad_offset;
+ *(int64_t *)dest = val;
+ break;
+
+ case R_AARCH64_ABS32:
+ *(int32_t *)dest = val;
+ if ( (int64_t)val != *(int32_t *)dest )
+ err = -EOVERFLOW;
+ break;
+
+ case R_AARCH64_PREL64:
+ if ( r->r_offset + sizeof(uint64_t) > base->sec->sh_size )
+ goto bad_offset;
+
+ val -= (uint64_t)dest;
+ *(int64_t *)dest = val;
+ break;
+
+ case R_AARCH64_PREL32:
+ val -= (uint64_t)dest;
+ *(int32_t *)dest = val;
+ if ( (int64_t)val != *(int32_t *)dest )
+ err = -EOVERFLOW;
+ break;
+
+ /* Instructions. */
+ case R_AARCH64_ADR_PREL_LO21:
+ val -= (uint64_t)dest;
+ err = reloc_insn_imm(dest, val, 0, 21, AARCH64_INSN_IMM_ADR);
+ break;
+
+ case R_AARCH64_ADR_PREL_PG_HI21:
+ val = (val & ~0xfff) - ((u64)dest & ~0xfff);
+ err = reloc_insn_imm(dest, val, 12, 21, AARCH64_INSN_IMM_ADR);
+ break;
+
+ case R_AARCH64_LDST8_ABS_LO12_NC:
+ case R_AARCH64_ADD_ABS_LO12_NC:
+ err = reloc_insn_imm(dest, val, 0, 12, AARCH64_INSN_IMM_12);
+ if ( err == -EOVERFLOW )
+ err = 0;
+ break;
+
+ case R_AARCH64_LDST16_ABS_LO12_NC:
+ err = reloc_insn_imm(dest, val, 1, 11, AARCH64_INSN_IMM_12);
+ if ( err == -EOVERFLOW )
+ err = 0;
+ break;
+
+ case R_AARCH64_LDST32_ABS_LO12_NC:
+ err = reloc_insn_imm(dest, val, 2, 10, AARCH64_INSN_IMM_12);
+ if ( err == -EOVERFLOW )
+ err = 0;
+ break;
+
+ case R_AARCH64_LDST64_ABS_LO12_NC:
+ err = reloc_insn_imm(dest, val, 3, 9, AARCH64_INSN_IMM_12);
+ if ( err == -EOVERFLOW )
+ err = 0;
+ break;
+
+ case R_AARCH64_CONDBR19:
+ err = reloc_insn_imm(dest, val, 2, 19, AARCH64_INSN_IMM_19);
+ break;
+
+ case R_AARCH64_JUMP26:
+ case R_AARCH64_CALL26:
+ val -= (uint64_t)dest;
+ err = reloc_insn_imm(dest, val, 2, 26, AARCH64_INSN_IMM_26);
+ break;
+
+ default:
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Unhandled relocation %lu\n",
+ elf->name, ELF64_R_TYPE(r->r_info));
+ return -EOPNOTSUPP;
+ }
+
+ if ( err )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Overflow in relocation %u in %s for %s!\n",
+ elf->name, i, rela->name, base->name);
+ return err;
+ }
+ }
+ return 0;
+
+ bad_offset:
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation offset is past %s section!\n",
+ elf->name, base->name);
+ return -EINVAL;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index aba1320..67749ed 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -6,66 +6,100 @@
#include <xen/lib.h>
#include <xen/livepatch_elf.h>
#include <xen/livepatch.h>
+#include <xen/vmap.h>
+#include "livepatch.h"
+
+#include <asm/mm.h>
+
+void *vmap_of_xen_text;
void arch_livepatch_quiesce(void)
{
+ mfn_t text_mfn;
+ unsigned int text_order;
+
+ if ( vmap_of_xen_text )
+ return;
+
+ text_mfn = _mfn(virt_to_mfn(_stext));
+ text_order = get_order_from_bytes(_end - _start);
+
+ /*
+ * The text section is read-only. So re-map Xen to be able to patch
+ * the code.
+ */
+ vmap_of_xen_text = __vmap(&text_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR,
+ VMAP_DEFAULT);
}
void arch_livepatch_revive(void)
{
+ /* Nuke the instruction cache */
+ invalidate_icache();
+
+ if ( vmap_of_xen_text )
+ vunmap(vmap_of_xen_text);
+
+ vmap_of_xen_text = NULL;
}
int arch_livepatch_verify_func(const struct livepatch_func *func)
{
- return -ENOSYS;
-}
+ /* No NOP patching yet. */
+ if ( !func->new_size )
+ return -EOPNOTSUPP;
-void arch_livepatch_apply_jmp(struct livepatch_func *func)
-{
-}
+ if ( func->old_size < PATCH_INSN_SIZE )
+ return -EINVAL;
-void arch_livepatch_revert_jmp(const struct livepatch_func *func)
-{
+ return 0;
}
void arch_livepatch_post_action(void)
{
+ /* arch_livepatch_revive has nuked the instruction cache. */
}
void arch_livepatch_mask(void)
{
+ /* TODO: No NMI on ARM? */
}
void arch_livepatch_unmask(void)
{
+ /* TODO: No NMI on ARM? */
}
-int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
+int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type type)
{
- return -ENOSYS;
-}
+ unsigned long start = (unsigned long)va;
+ unsigned int flags;
-int arch_livepatch_perform_rel(struct livepatch_elf *elf,
- const struct livepatch_elf_sec *base,
- const struct livepatch_elf_sec *rela)
-{
- return -ENOSYS;
-}
+ ASSERT(va);
+ ASSERT(pages);
-int arch_livepatch_perform_rela(struct livepatch_elf *elf,
- const struct livepatch_elf_sec *base,
- const struct livepatch_elf_sec *rela)
-{
- return -ENOSYS;
-}
+ if ( type == LIVEPATCH_VA_RX )
+ flags = 0x2; /* R set,NX clear */
+ else if ( type == LIVEPATCH_VA_RW )
+ flags = 0x1; /* R clear, NX set */
+ else
+ flags = 0x3; /* R set,NX set */
-int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type type)
-{
- return -ENOSYS;
+ modify_xen_mappings(start, start + pages * PAGE_SIZE, flags);
+
+ return 0;
}
void __init arch_livepatch_init(void)
{
+ void *start, *end;
+
+ start = (void *)xen_virt_end;
+ end = (void *)FIXMAP_ADDR(0);
+
+ BUG_ON(start >= end);
+
+ vm_init_type(VMAP_XEN, start, end);
}
/*
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index eca7cdd..94b4911 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -143,6 +143,8 @@ vaddr_t xenheap_virt_end __read_mostly;
vaddr_t xenheap_virt_start __read_mostly;
#endif
+vaddr_t xen_virt_end;
+
unsigned long frametable_base_pdx __read_mostly;
unsigned long frametable_virt_end __read_mostly;
@@ -540,6 +542,9 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
/* No flush required here as page table is not hooked in yet. */
}
+ if ( i < LPAE_ENTRIES )
+ xen_virt_end = XEN_VIRT_START + (i << PAGE_SHIFT);
+
pte = pte_of_xenaddr((vaddr_t)xen_xenmap);
pte.pt.table = 1;
write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte);
diff --git a/xen/arch/arm/test/Makefile b/xen/arch/arm/test/Makefile
new file mode 100644
index 0000000..c302f6a
--- /dev/null
+++ b/xen/arch/arm/test/Makefile
@@ -0,0 +1,85 @@
+include $(XEN_ROOT)/Config.mk
+
+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}')
+
+.PHONY: default
+
+LIVEPATCH := xen_hello_world.livepatch
+LIVEPATCH_BYE := xen_bye_world.livepatch
+LIVEPATCH_REPLACE := xen_replace_world.livepatch
+
+default: livepatch
+
+install: livepatch
+ $(INSTALL_DATA) $(LIVEPATCH) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH)
+ $(INSTALL_DATA) $(LIVEPATCH_BYE) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_BYE)
+ $(INSTALL_DATA) $(LIVEPATCH_REPLACE) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_REPLACE)
+uninstall:
+ rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH)
+ rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_BYE)
+ rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_REPLACE)
+
+.PHONY: clean
+clean::
+ rm -f *.o .*.o.d *.livepatch config.h
+
+#
+# To compute these values we need the binary files: xen-syms
+# and xen_hello_world_func.o to be already compiled.
+#
+.PHONY: config.h
+config.h: OLD_CODE_SZ=$(call CODE_SZ,$(BASEDIR)/xen-syms,xen_extra_version)
+config.h: NEW_CODE_SZ=$(call CODE_SZ,$<,xen_hello_world)
+config.h: xen_hello_world_func.o
+ (set -e; \
+ echo "#define NEW_CODE_SZ $(NEW_CODE_SZ)"; \
+ echo "#define OLD_CODE_SZ $(OLD_CODE_SZ)") > $@
+
+xen_hello_world.o: config.h
+
+.PHONY: $(LIVEPATCH)
+$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
+ $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH) $^
+
+#
+# This target is only accessible if CONFIG_LIVEPATCH is defined, which
+# depends on $(build_id_linker) being available. Hence we do not
+# need any checks.
+#
+# N.B. The reason we don't use arch/x86/note.o is that it may
+# 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) -I binary -O elf64-littleaarch64 -B aarch64 \
+ --rename-section=.data=.livepatch.depends -S $@.bin $@
+ rm -f $@.bin
+
+#
+# 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) -I binary -O elf64-littleaarch64 -B aarch64 \
+ --rename-section=.data=.livepatch.depends -S $@.bin $@
+ rm -f $@.bin
+
+xen_bye_world.o: config.h
+
+.PHONY: $(LIVEPATCH_BYE)
+$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o
+ $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_BYE) $^
+
+xen_replace_world.o: config.h
+
+.PHONY: $(LIVEPATCH_REPLACE)
+$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o
+ $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_REPLACE) $^
+
+.PHONY: livepatch
+livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE)
diff --git a/xen/arch/arm/test/xen_bye_world.c b/xen/arch/arm/test/xen_bye_world.c
new file mode 100644
index 0000000..b75e0b1
--- /dev/null
+++ b/xen/arch/arm/test/xen_bye_world.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <xen/version.h>
+#include <xen/livepatch.h>
+
+#include <public/sysctl.h>
+
+static char bye_world_patch_this_fnc[] = "xen_extra_version";
+extern const char *xen_bye_world(void);
+
+struct livepatch_func __section(".livepatch.funcs") livepatch_xen_bye_world = {
+ .version = LIVEPATCH_PAYLOAD_VERSION,
+ .name = bye_world_patch_this_fnc,
+ .new_addr = xen_bye_world,
+ .old_addr = xen_extra_version,
+ .new_size = NEW_CODE_SZ,
+ .old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/test/xen_bye_world_func.c b/xen/arch/arm/test/xen_bye_world_func.c
new file mode 100644
index 0000000..32ef341
--- /dev/null
+++ b/xen/arch/arm/test/xen_bye_world_func.c
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include <xen/types.h>
+
+/* Our replacement function for xen_hello_world. */
+const char *xen_bye_world(void)
+{
+ return "Bye World!";
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/test/xen_hello_world.c b/xen/arch/arm/test/xen_hello_world.c
new file mode 100644
index 0000000..e6a095b
--- /dev/null
+++ b/xen/arch/arm/test/xen_hello_world.c
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <xen/version.h>
+#include <xen/livepatch.h>
+#include <xen/livepatch_payload.h>
+
+#include <public/sysctl.h>
+
+static char hello_world_patch_this_fnc[] = "xen_extra_version";
+extern const char *xen_hello_world(void);
+static unsigned int cnt;
+
+static void apply_hook(void)
+{
+ printk(KERN_DEBUG "Hook executing.\n");
+}
+
+static void revert_hook(void)
+{
+ printk(KERN_DEBUG "Hook unloaded.\n");
+}
+
+static void hi_func(void)
+{
+ printk(KERN_DEBUG "%s: Hi! (called %u times)\n", __func__, ++cnt);
+};
+
+/* If we are sorted we _MUST_ be the last .livepatch.hook section. */
+static void Z_check_fnc(void)
+{
+ printk(KERN_DEBUG "%s: Hi func called %u times\n", __func__, cnt);
+ BUG_ON(cnt == 0 || cnt > 2);
+ cnt = 0; /* Otherwise if you revert, apply, revert the value will be 4! */
+}
+
+LIVEPATCH_LOAD_HOOK(apply_hook);
+LIVEPATCH_UNLOAD_HOOK(revert_hook);
+
+/* Imbalance here. Two load and three unload. */
+
+LIVEPATCH_LOAD_HOOK(hi_func);
+LIVEPATCH_UNLOAD_HOOK(hi_func);
+
+LIVEPATCH_UNLOAD_HOOK(Z_check_fnc);
+
+struct livepatch_func __section(".livepatch.funcs") livepatch_xen_hello_world = {
+ .version = LIVEPATCH_PAYLOAD_VERSION,
+ .name = hello_world_patch_this_fnc,
+ .new_addr = xen_hello_world,
+ .old_addr = xen_extra_version,
+ .new_size = NEW_CODE_SZ,
+ .old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/test/xen_hello_world_func.c b/xen/arch/arm/test/xen_hello_world_func.c
new file mode 100644
index 0000000..6a0c440
--- /dev/null
+++ b/xen/arch/arm/test/xen_hello_world_func.c
@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include <xen/types.h>
+#include <asm/alternative.h>
+
+
+/* Our replacement function for xen_extra_version. */
+const char *xen_hello_world(void)
+{
+ asm(ALTERNATIVE("nop", "nop", 1));
+
+ return "Hello World";
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/test/xen_replace_world.c b/xen/arch/arm/test/xen_replace_world.c
new file mode 100644
index 0000000..a2a221a
--- /dev/null
+++ b/xen/arch/arm/test/xen_replace_world.c
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <xen/livepatch.h>
+
+#include <public/sysctl.h>
+
+static char xen_replace_world_name[] = "xen_extra_version";
+extern const char *xen_replace_world(void);
+
+struct livepatch_func __section(".livepatch.funcs") livepatch_xen_replace_world = {
+ .version = LIVEPATCH_PAYLOAD_VERSION,
+ .name = xen_replace_world_name,
+ .old_addr = 0, /* Forces the hypervisor to lookup .name */
+ .new_addr = xen_replace_world,
+ .new_size = NEW_CODE_SZ,
+ .old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/test/xen_replace_world_func.c b/xen/arch/arm/test/xen_replace_world_func.c
new file mode 100644
index 0000000..afb5cda
--- /dev/null
+++ b/xen/arch/arm/test/xen_replace_world_func.c
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include <xen/types.h>
+
+/* Our replacement function for xen_hello_world. */
+const char *xen_replace_world(void)
+{
+ return "Hello Again World!";
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 51afa24..8b4dfbc 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -222,7 +222,7 @@ endmenu
config LIVEPATCH
bool "Live patching support (TECH PREVIEW)"
default n
- depends on X86 && HAS_BUILD_ID = "y"
+ depends on HAS_BUILD_ID = "y"
---help---
Allows a running Xen hypervisor to be dynamically patched using
binary patches without rebooting. This is primarily used to binarily
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 88a79d8..6ffd2d0 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -618,7 +618,6 @@ static int prepare_payload(struct payload *payload,
sizeof(*region->frame[i].bugs);
}
-#ifndef CONFIG_ARM
sec = livepatch_elf_sec_by_name(elf, ".altinstructions");
if ( sec )
{
@@ -636,8 +635,14 @@ static int prepare_payload(struct payload *payload,
for ( a = start; a < end; a++ )
{
+#ifndef CONFIG_ARM
+ /* TODO: Bubble ALT_ORIG_PTR up. */
const void *instr = &a->instr_offset + a->instr_offset;
const void *replacement = &a->repl_offset + a->repl_offset;
+#else
+ const void *instr = &a->orig_offset + a->orig_offset;
+ const void *replacement = &a->alt_offset + a->alt_offset;
+#endif
if ( (instr < region->start && instr >= region->end) ||
(replacement < region->start && replacement >= region->end) )
@@ -647,9 +652,14 @@ static int prepare_payload(struct payload *payload,
return -EINVAL;
}
}
+#ifndef CONFIG_ARM
apply_alternatives_nocheck(start, end);
+#else
+ apply_alternatives(start, sec->sec->sh_size);
+#endif
}
+#ifndef CONFIG_ARM
sec = livepatch_elf_sec_by_name(elf, ".ex_table");
if ( sec )
{
diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
index 65c0cdf..f4fcfd6 100644
--- a/xen/include/asm-arm/current.h
+++ b/xen/include/asm-arm/current.h
@@ -33,8 +33,15 @@ static inline struct cpu_info *get_cpu_info(void)
#define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
+#ifdef CONFIG_LIVEPATCH
+#define switch_stack_and_jump(stack, fn) \
+ asm volatile ("mov sp,%0;" \
+ "bl check_for_livepatch_work;" \
+ "b " STR(fn) : : "r" (stack) : "memory" )
+#else
#define switch_stack_and_jump(stack, fn) \
asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" )
+#endif
#define reset_stack_and_jump(fn) switch_stack_and_jump(get_cpu_info(), fn)
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 19eadd2..f3e8f7e 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -120,6 +120,7 @@ extern vaddr_t xenheap_virt_end;
extern vaddr_t xenheap_virt_start;
#endif
+extern vaddr_t xen_virt_end;
#ifdef CONFIG_ARM_32
#define is_xen_heap_page(page) is_xen_heap_mfn(page_to_mfn(page))
#define is_xen_heap_mfn(mfn) ({ \
diff --git a/xen/include/xen/elfstructs.h b/xen/include/xen/elfstructs.h
index 5f2082e..43a7060 100644
--- a/xen/include/xen/elfstructs.h
+++ b/xen/include/xen/elfstructs.h
@@ -103,6 +103,15 @@ typedef uint64_t Elf64_Xword;
(ehdr).e_ident[EI_MAG2] == ELFMAG2 && \
(ehdr).e_ident[EI_MAG3] == ELFMAG3)
+/* e_flags */
+#define EF_ARM_EABI_MASK 0xff000000
+#define EF_ARM_EABI_UNKNOWN 0x00000000
+#define EF_ARM_EABI_VER1 0x01000000
+#define EF_ARM_EABI_VER2 0x02000000
+#define EF_ARM_EABI_VER3 0x03000000
+#define EF_ARM_EABI_VER4 0x04000000
+#define EF_ARM_EABI_VER5 0x05000000
+
/* ELF Header */
typedef struct elfhdr {
unsigned char e_ident[EI_NIDENT]; /* ELF Identification */
@@ -171,6 +180,7 @@ typedef struct {
#define EM_PPC 20 /* PowerPC */
#define EM_PPC64 21 /* PowerPC 64-bit */
#define EM_ARM 40 /* Advanced RISC Machines ARM */
+#define EM_AARCH64 183
#define EM_ALPHA 41 /* DEC ALPHA */
#define EM_SPARCV9 43 /* SPARC version 9 */
#define EM_ALPHA_EXP 0x9026 /* DEC ALPHA */
@@ -359,6 +369,31 @@ typedef struct {
#define R_X86_64_PC32 2 /* PC relative 32 bit signed */
#define R_X86_64_PLT32 4 /* 32 bit PLT address */
+/*
+ * S - address of symbol.
+ * A - addend for relocation (r_addend)
+ * P - address of the dest being relocated (derieved from r_offset)
+ * NC - No check for overflow.
+ *
+ * The defines also use _PREL for PC-relative address, and _NC is No Check.
+ */
+#define R_AARCH64_ABS64 257 /* Direct 64 bit. S+A, NC*/
+#define R_AARCH64_ABS32 258 /* Direct 32 bit. S+A */
+#define R_AARCH64_PREL64 260 /* S+A-P, NC */
+#define R_AARCH64_PREL32 261 /* S+A-P */
+
+#define R_AARCH64_ADR_PREL_LO21 274 /* ADR imm, [20:0]. S+A-P */
+#define R_AARCH64_ADR_PREL_PG_HI21 275 /* ADRP imm, [32:12]. Page(S+A) - Page(P).*/
+#define R_AARCH64_ADD_ABS_LO12_NC 277 /* ADD imm. from bits 11:0. S+A, NC */
+
+#define R_AARCH64_CONDBR19 280 /* Bits 20:2, S+A-P */
+#define R_AARCH64_JUMP26 282 /* Bits 27:2, S+A-P */
+#define R_AARCH64_CALL26 283 /* Bits 27:2, S+A-P */
+#define R_AARCH64_LDST16_ABS_LO12_NC 284 /* LD/ST to bits 11:1, S+A, NC */
+#define R_AARCH64_LDST32_ABS_LO12_NC 285 /* LD/ST to bits 11:2, S+A, NC */
+#define R_AARCH64_LDST64_ABS_LO12_NC 286 /* LD/ST to bits 11:3, S+A, NC */
+#define R_AARCH64_LDST8_ABS_LO12_NC 278 /* LD/ST to bits 11:0, S+A, NC */
+
/* Program Header */
typedef struct {
Elf32_Word p_type; /* segment type */
--
2.4.11
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH] Livepatch - initial ARM64/32 support.
2016-08-09 4:18 [Very RFC PATCH] Livepatch - initial ARM64/32 support Konrad Rzeszutek Wilk
` (2 preceding siblings ...)
2016-08-09 4:19 ` [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support Konrad Rzeszutek Wilk
@ 2016-08-09 4:24 ` Konrad Rzeszutek Wilk
2016-08-11 16:28 ` Julien Grall
4 siblings, 0 replies; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-08-09 4:24 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel, julien.grall, sstabellini
On Tue, Aug 09, 2016 at 12:18:57AM -0400, Konrad Rzeszutek Wilk wrote:
> Hey!
>
> Over the last couple of months in my spare time I was playing
> with making livepatch work with ARM64 (using the FoundationModel
> simulator) and I finally got it working tonight.
The git tree is
git://xenbits.xen.org/people/konradwilk/xen.git livepatch.v4.8.v2
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH 2/3] insn: Add arch64_insn_gen_branch_imm to generate branch
2016-08-09 4:18 ` [Very RFC PATCH 2/3] insn: Add arch64_insn_gen_branch_imm to generate branch Konrad Rzeszutek Wilk
@ 2016-08-11 16:03 ` Julien Grall
0 siblings, 0 replies; 14+ messages in thread
From: Julien Grall @ 2016-08-11 16:03 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, sstabellini, julien.grall
Cc: Ross Lagerwall
Hi Konrad,
On 09/08/2016 06:18, Konrad Rzeszutek Wilk wrote:
> We copied from Linux code (v4.7) the code that generates
> the unconditional branch instructions. This is mostly
> from Linux commit c0cafbae20d2878883ec3c06d6ea30ff38a6bf92
> "arm64: introduce aarch64_insn_gen_branch_reg()"
>
> However the branch-link instructions was omitted, only
> the branch instruction is generated.
I would prefer to have the function fully copied (i.e with the
branch-link instructions). This will make easier to re-sync the code
later on.
>
> This lays the groundwork for Livepatch to generate the
> trampoline to jump to the new replacement function.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> --
> Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
> xen/arch/arm/arm64/insn.c | 41 ++++++++++++++++++++++++++++++++++++++++
> xen/include/asm-arm/arm64/insn.h | 2 ++
> 2 files changed, 43 insertions(+)
>
> diff --git a/xen/arch/arm/arm64/insn.c b/xen/arch/arm/arm64/insn.c
> index 12b4d96..15dd7bc 100644
> --- a/xen/arch/arm/arm64/insn.c
> +++ b/xen/arch/arm/arm64/insn.c
> @@ -209,6 +209,47 @@ u32 aarch64_set_branch_offset(u32 insn, s32 offset)
> BUG();
> }
>
> +static inline long branch_imm_common(unsigned long pc, unsigned long addr,
> + long range)
> +{
> + long offset;
> +
> + if ((pc & 0x3) || (addr & 0x3)) {
> + pr_err("%s: A64 instructions must be word aligned\n", __func__);
> + return range;
> + }
> +
> + offset = ((long)addr - (long)pc);
> +
> + if (offset < -range || offset >= range) {
> + pr_err("%s: offset out of range\n", __func__);
> + return range;
> + }
> +
> + return offset;
> +}
> +
> +u32 __kprobes aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr)
> +{
> + u32 insn;
> + long offset;
> +
> + /*
> + * B/BL support [-128M, 128M) offset
> + * ARM64 virtual address arrangement guarantees all kernel and module
> + * texts are within +/-128M.
> + */
> + offset = branch_imm_common(pc, addr, SZ_128M);
> + if (offset >= SZ_128M)
> + return AARCH64_BREAK_FAULT;
> +
> + insn = aarch64_insn_get_b_value();
> +
> + return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
> + offset >> 2);
> +}
I would prefer to have the functions added at the same place as in Linux
for similar reason as stated previously.
> +
> +
I think you can drop one empty line here.
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
> index 6ce37be..378ba11 100644
> --- a/xen/include/asm-arm/arm64/insn.h
> +++ b/xen/include/asm-arm/arm64/insn.h
> @@ -61,6 +61,8 @@ u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
> s32 aarch64_get_branch_offset(u32 insn);
> u32 aarch64_set_branch_offset(u32 insn, s32 offset);
>
> +u32 aarch64_insn_gen_branch_imm(unsigned long pc, unsigned long addr);
> +
> /* Wrapper for common code */
> static inline bool insn_is_branch_imm(u32 insn)
> {
>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH] Livepatch - initial ARM64/32 support.
2016-08-09 4:18 [Very RFC PATCH] Livepatch - initial ARM64/32 support Konrad Rzeszutek Wilk
` (3 preceding siblings ...)
2016-08-09 4:24 ` [Very RFC PATCH] Livepatch - initial ARM64/32 support Konrad Rzeszutek Wilk
@ 2016-08-11 16:28 ` Julien Grall
2016-08-11 19:05 ` Stefano Stabellini
4 siblings, 1 reply; 14+ messages in thread
From: Julien Grall @ 2016-08-11 16:28 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, sstabellini, julien.grall
Cc: Steve Capper
On 09/08/2016 06:18, Konrad Rzeszutek Wilk wrote:
> Hey!
Hi Konrad,
> Over the last couple of months in my spare time I was playing
> with making livepatch work with ARM64 (using the FoundationModel
> simulator) and I finally got it working tonight.
Congrats!
>
> Sending out the patches just in case they don't work tomorrow :-)
>
> The ARM32 part is going slowly - as I don't have a simulator
> and using a real board takes time.
Whilst I can see usage of livepatch for Xen ARM64 (e.g server), I am
less convinced for ARM32. I am bit worry to check-in code that will get
rotten in the long term. So do you see any usage on ARM32?
>
> As you can see from the diffstat there is some room
> for improvement:
> - The 'tests' - they are now in x86 and arm - and they are quite
> similar. My thinking is to move them to 'common' ?
> - There is room to unify some of the ELF relocation checks as
> they are exactly the same across architectures.
>
> There is also an ugly implementation of modify_xen_mappings.
> I am hoping the ARM maintainers could provide some input on
> how they would like me to implement this.
I will give a look and comment on the patch.
[...]
> Konrad Rzeszutek Wilk (3):
> mm/arm: Introduce modify_xen_mappings
> insn: Add arch64_insn_gen_branch_imm to generate branch
> livepatch: Initial ARM32/64 support.
Do you plan to split the last patch in smaller patches?
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH] Livepatch - initial ARM64/32 support.
2016-08-11 16:28 ` Julien Grall
@ 2016-08-11 19:05 ` Stefano Stabellini
2016-08-11 23:08 ` Konrad Rzeszutek Wilk
2016-08-12 8:08 ` Julien Grall
0 siblings, 2 replies; 14+ messages in thread
From: Stefano Stabellini @ 2016-08-11 19:05 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, sstabellini, Steve Capper
On Thu, 11 Aug 2016, Julien Grall wrote:
> On 09/08/2016 06:18, Konrad Rzeszutek Wilk wrote:
> > Hey!
>
> Hi Konrad,
>
> > Over the last couple of months in my spare time I was playing
> > with making livepatch work with ARM64 (using the FoundationModel
> > simulator) and I finally got it working tonight.
>
> Congrats!
Indeed, congratulations! I hope it was fun :-)
> > Sending out the patches just in case they don't work tomorrow :-)
> >
> > The ARM32 part is going slowly - as I don't have a simulator
> > and using a real board takes time.
>
> Whilst I can see usage of livepatch for Xen ARM64 (e.g server), I am less
> convinced for ARM32. I am bit worry to check-in code that will get rotten in
> the long term. So do you see any usage on ARM32?
Actually I think it is useful for embedded use cases too: think about
security updates for any embedded devices which cannot easily be
rebooted. Or even if they can be rebooted, it might create a
competitive advantage to build a car that can be updated for critical
security fixes without requesting the driver to stop.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH] Livepatch - initial ARM64/32 support.
2016-08-11 19:05 ` Stefano Stabellini
@ 2016-08-11 23:08 ` Konrad Rzeszutek Wilk
2016-08-12 8:08 ` Julien Grall
1 sibling, 0 replies; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-08-11 23:08 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, Julien Grall, Steve Capper
On Thu, Aug 11, 2016 at 12:05:33PM -0700, Stefano Stabellini wrote:
> On Thu, 11 Aug 2016, Julien Grall wrote:
> > On 09/08/2016 06:18, Konrad Rzeszutek Wilk wrote:
> > > Hey!
> >
> > Hi Konrad,
> >
> > > Over the last couple of months in my spare time I was playing
> > > with making livepatch work with ARM64 (using the FoundationModel
> > > simulator) and I finally got it working tonight.
> >
> > Congrats!
>
> Indeed, congratulations! I hope it was fun :-)
Yes!
>
>
> > > Sending out the patches just in case they don't work tomorrow :-)
> > >
> > > The ARM32 part is going slowly - as I don't have a simulator
> > > and using a real board takes time.
> >
> > Whilst I can see usage of livepatch for Xen ARM64 (e.g server), I am less
> > convinced for ARM32. I am bit worry to check-in code that will get rotten in
> > the long term. So do you see any usage on ARM32?
>
> Actually I think it is useful for embedded use cases too: think about
> security updates for any embedded devices which cannot easily be
> rebooted. Or even if they can be rebooted, it might create a
> competitive advantage to build a car that can be updated for critical
> security fixes without requesting the driver to stop.
My 'usage' is that ARM32 is the only board ARM board I've that
boots and runs without issues :-)
Either way the idea is that the test-cases will work on both
ARM32/ARM64/x86 - and I do plan to have them as part of OSSTest - so
I am not that afraid of the bitrotten part.
However for simplicity and review I will redo the patches without the
ARM32 part (the ELF part is the headache one) and keep on poking
at them until I get ARM32 working all the way (probably Xen 4.9 material).
And it was fun learning the opcodes, thought I still have much to learn.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH] Livepatch - initial ARM64/32 support.
2016-08-11 19:05 ` Stefano Stabellini
2016-08-11 23:08 ` Konrad Rzeszutek Wilk
@ 2016-08-12 8:08 ` Julien Grall
1 sibling, 0 replies; 14+ messages in thread
From: Julien Grall @ 2016-08-12 8:08 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, Steve Capper
Hi
On 11/08/2016 21:05, Stefano Stabellini wrote:
> On Thu, 11 Aug 2016, Julien Grall wrote:
>> On 09/08/2016 06:18, Konrad Rzeszutek Wilk wrote:
>>> Hey!
>>
>> Hi Konrad,
>>
>>> Over the last couple of months in my spare time I was playing
>>> with making livepatch work with ARM64 (using the FoundationModel
>>> simulator) and I finally got it working tonight.
>>
>> Congrats!
>
> Indeed, congratulations! I hope it was fun :-)
>
>
>>> Sending out the patches just in case they don't work tomorrow :-)
>>>
>>> The ARM32 part is going slowly - as I don't have a simulator
>>> and using a real board takes time.
>>
>> Whilst I can see usage of livepatch for Xen ARM64 (e.g server), I am less
>> convinced for ARM32. I am bit worry to check-in code that will get rotten in
>> the long term. So do you see any usage on ARM32?
>
> Actually I think it is useful for embedded use cases too: think about
> security updates for any embedded devices which cannot easily be
> rebooted. Or even if they can be rebooted, it might create a
> competitive advantage to build a car that can be updated for critical
> security fixes without requesting the driver to stop.
I see your point, although I think updating Xen while driving sounds
really risky :). There are still a small window (few ms/ns) where
nothing can run. That might be an issue with real-time operating system.
This leads to few questions as I am not very familiar with livepatching.
Who decide when to patch? Is it the scheduler or the user requesting the
patching?
Also, could this be aborted because a task with higher priority is incoming?
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support.
2016-08-09 4:19 ` [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support Konrad Rzeszutek Wilk
@ 2016-08-12 15:00 ` Julien Grall
2016-08-12 20:50 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 14+ messages in thread
From: Julien Grall @ 2016-08-12 15:00 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, sstabellini
Cc: Ross Lagerwall, Andrew Cooper
Hi Konrad,
On 09/08/2016 06:19, Konrad Rzeszutek Wilk wrote:
> The initial support for ARM64 - and livepatching
> works:
As it is a very RFC I only gave a quick look. I have few comments on it
(see below).
>
> (XEN) livepatch: xen_hello_world: Applying 1 functions
> (XEN) hi_func: Hi! (called 1 times)
> (XEN) Hook executing.
> (XEN) livepatch: xen_hello_world finished APPLY with rc=0
> (XEN) livepatch.c:1687: livepatch: load_payload_fnc: rc=0 (p->rc=0)
> (XEN) livepatch: Hello World
> (XEN) 'x' pressed - Dumping all livepatch patches
> (XEN) build-id: e144bafc4ee8635eee5bed8e3988b484765c46c8
> (XEN) name=xen_hello_world state=APPLIED(2) 0000000000318000 (.data=0000000000319000, .rodata=000000000031a000) using 3 pages.
> (XEN) xen_extra_version patch 0000000000233fac(12) with 0000000000318000 (16)
> (XEN) build-id=c4b842c276be43adbe4db788598b1e11bce04dc6
> (XEN) depend-on=9affa110481e8e13606c61b21e5f6a357a3c8ef9
>
> ARM32 still has some issues.
>
> The are some TODOs left to be done:
>
> General:
> - Bubble ALT_ORIG_PTR macro for both x86/ARM.
> - Unify the ELF RELA checks - they are the same on x86/ARM[32,64].
> - Makefile generating .livepatch.depends needs to ingest the
> -O argument based on architecture
> - Test cases should move to common/ ? [Needs Jan's Ack]
In general, I would be in favor of sharing as much as possible the code.
> ARM32 issues:
> - vm_init_type: Assertion 'is_xen_heap_mfn(ma >> PAGE_SHIFT)' failed at xen/include/asm/mm.h:23
xen/include/asm/mm.h:23 points to the beginning of struct page_info. Did
you mean to write instead 233?
This would point to maddr_virt. This would mean someone is trying to get
the virtual address of MFN that is not in the xenheap (only the xenheap
is mapped on ARM32). Do you have the full call stack?
[...]
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
[...]
> diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
> index aba1320..67749ed 100644
> --- a/xen/arch/arm/livepatch.c
> +++ b/xen/arch/arm/livepatch.c
> @@ -6,66 +6,100 @@
> #include <xen/lib.h>
> #include <xen/livepatch_elf.h>
> #include <xen/livepatch.h>
> +#include <xen/vmap.h>
> +#include "livepatch.h"
> +
> +#include <asm/mm.h>
> +
> +void *vmap_of_xen_text;
>
> void arch_livepatch_quiesce(void)
> {
> + mfn_t text_mfn;
> + unsigned int text_order;
> +
> + if ( vmap_of_xen_text )
> + return;
> +
> + text_mfn = _mfn(virt_to_mfn(_stext));
> + text_order = get_order_from_bytes(_end - _start);
> +
> + /*
> + * The text section is read-only. So re-map Xen to be able to patch
> + * the code.
> + */
> + vmap_of_xen_text = __vmap(&text_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR,
> + VMAP_DEFAULT);
Shouldn't we return an error if we fail to re-map the region?
Otherwise the patch may silently be ignored (see arch_livepatch_apply_jmp).
> }
>
> void arch_livepatch_revive(void)
> {
> + /* Nuke the instruction cache */
> + invalidate_icache();
I was about to say that this is not enough. But you called
clean_and_invalidate_* every time you rewrite the jump. It might be
worth to add a comment here, explain that the cache has been cleaned
before hand.
However, I can't find any data cache flush for the payload. Did I miss
anything?
Lastly, before I forgot, you will need to invalidate the branch
predictor for ARMv7 as it may be architecturally visible to the software
(see B2.2.4 in ARM DDI 0406C.b).
> +
> + if ( vmap_of_xen_text )
> + vunmap(vmap_of_xen_text);
> +
> + vmap_of_xen_text = NULL;
> }
>
> int arch_livepatch_verify_func(const struct livepatch_func *func)
> {
> - return -ENOSYS;
> -}
> + /* No NOP patching yet. */
> + if ( !func->new_size )
> + return -EOPNOTSUPP;
>
> -void arch_livepatch_apply_jmp(struct livepatch_func *func)
> -{
> -}
> + if ( func->old_size < PATCH_INSN_SIZE )
> + return -EINVAL;
>
> -void arch_livepatch_revert_jmp(const struct livepatch_func *func)
> -{
> + return 0;
> }
>
> void arch_livepatch_post_action(void)
> {
> + /* arch_livepatch_revive has nuked the instruction cache. */
> }
>
> void arch_livepatch_mask(void)
> {
> + /* TODO: No NMI on ARM? */
All interrupts can be masked on ARM so far. Although, local_irq_disable
will only mask IRQ (i.e interrupt from the interrupt controller).
We may want to mask SError (System Error) via
local_abort_{disable,enable} to avoid receiving asynchronous abort
whilst patching Xen. The interrupt will stay pending until it will be
re-enabled in arch_livepatch_unmask.
> }
>
> void arch_livepatch_unmask(void)
> {
> + /* TODO: No NMI on ARM? */
> }
>
> -int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
> +int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type type)
> {
> - return -ENOSYS;
> -}
> + unsigned long start = (unsigned long)va;
> + unsigned int flags;
>
> -int arch_livepatch_perform_rel(struct livepatch_elf *elf,
> - const struct livepatch_elf_sec *base,
> - const struct livepatch_elf_sec *rela)
> -{
> - return -ENOSYS;
> -}
> + ASSERT(va);
> + ASSERT(pages);
>
> -int arch_livepatch_perform_rela(struct livepatch_elf *elf,
> - const struct livepatch_elf_sec *base,
> - const struct livepatch_elf_sec *rela)
> -{
> - return -ENOSYS;
> -}
> + if ( type == LIVEPATCH_VA_RX )
> + flags = 0x2; /* R set,NX clear */
> + else if ( type == LIVEPATCH_VA_RW )
> + flags = 0x1; /* R clear, NX set */
> + else
> + flags = 0x3; /* R set,NX set */
>
> -int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type type)
> -{
> - return -ENOSYS;
> + modify_xen_mappings(start, start + pages * PAGE_SIZE, flags);
> +
> + return 0;
> }
>
> void __init arch_livepatch_init(void)
> {
> + void *start, *end;
> +
> + start = (void *)xen_virt_end;
> + end = (void *)FIXMAP_ADDR(0);
The space between xen_virt_end and FIXMAP_ADDR may be really small
depending on the size of the Xen binary.
I would introduce a specific region in the layout of few megabytes (not
sure how much me need). Or re-use "early relocation address" (8M - 10M)
as the region is only used during early boot.
> +
> + BUG_ON(start >= end);
> +
> + vm_init_type(VMAP_XEN, start, end);
> }
>
> /*
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index eca7cdd..94b4911 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -143,6 +143,8 @@ vaddr_t xenheap_virt_end __read_mostly;
> vaddr_t xenheap_virt_start __read_mostly;
> #endif
>
> +vaddr_t xen_virt_end;
> +
> unsigned long frametable_base_pdx __read_mostly;
> unsigned long frametable_virt_end __read_mostly;
>
> @@ -540,6 +542,9 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
> /* No flush required here as page table is not hooked in yet. */
> }
>
> + if ( i < LPAE_ENTRIES )
Why this check?
> + xen_virt_end = XEN_VIRT_START + (i << PAGE_SHIFT);
> +
> pte = pte_of_xenaddr((vaddr_t)xen_xenmap);
> pte.pt.table = 1;
> write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte);
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index 88a79d8..6ffd2d0 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -618,7 +618,6 @@ static int prepare_payload(struct payload *payload,
> sizeof(*region->frame[i].bugs);
> }
>
> -#ifndef CONFIG_ARM
> sec = livepatch_elf_sec_by_name(elf, ".altinstructions");
> if ( sec )
> {
> @@ -636,8 +635,14 @@ static int prepare_payload(struct payload *payload,
>
> for ( a = start; a < end; a++ )
> {
> +#ifndef CONFIG_ARM
> + /* TODO: Bubble ALT_ORIG_PTR up. */
> const void *instr = &a->instr_offset + a->instr_offset;
> const void *replacement = &a->repl_offset + a->repl_offset;
> +#else
> + const void *instr = &a->orig_offset + a->orig_offset;
> + const void *replacement = &a->alt_offset + a->alt_offset;
> +#endif
>
> if ( (instr < region->start && instr >= region->end) ||
> (replacement < region->start && replacement >= region->end) )
> @@ -647,9 +652,14 @@ static int prepare_payload(struct payload *payload,
> return -EINVAL;
> }
> }
> +#ifndef CONFIG_ARM
> apply_alternatives_nocheck(start, end);
> +#else
> + apply_alternatives(start, sec->sec->sh_size);
> +#endif
> }
>
> +#ifndef CONFIG_ARM
> sec = livepatch_elf_sec_by_name(elf, ".ex_table");
> if ( sec )
> {
> diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
> index 65c0cdf..f4fcfd6 100644
> --- a/xen/include/asm-arm/current.h
> +++ b/xen/include/asm-arm/current.h
> @@ -33,8 +33,15 @@ static inline struct cpu_info *get_cpu_info(void)
>
> #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
>
> +#ifdef CONFIG_LIVEPATCH
> +#define switch_stack_and_jump(stack, fn) \
> + asm volatile ("mov sp,%0;" \
> + "bl check_for_livepatch_work;" \
May I ask why check_for_livepatch_work is called in switch_stack_and_jump?
> + "b " STR(fn) : : "r" (stack) : "memory" )
> +#else
> #define switch_stack_and_jump(stack, fn) \
> asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" )
> +#endif
>
> #define reset_stack_and_jump(fn) switch_stack_and_jump(get_cpu_info(), fn)
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support.
2016-08-12 15:00 ` Julien Grall
@ 2016-08-12 20:50 ` Konrad Rzeszutek Wilk
2016-08-15 13:27 ` Julien Grall
0 siblings, 1 reply; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-08-12 20:50 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, sstabellini, Ross Lagerwall, Andrew Cooper
On Fri, Aug 12, 2016 at 05:00:47PM +0200, Julien Grall wrote:
> Hi Konrad,
>
> On 09/08/2016 06:19, Konrad Rzeszutek Wilk wrote:
> > The initial support for ARM64 - and livepatching
> > works:
>
> As it is a very RFC I only gave a quick look. I have few comments on it (see
> below).
>
> >
> > (XEN) livepatch: xen_hello_world: Applying 1 functions
> > (XEN) hi_func: Hi! (called 1 times)
> > (XEN) Hook executing.
> > (XEN) livepatch: xen_hello_world finished APPLY with rc=0
> > (XEN) livepatch.c:1687: livepatch: load_payload_fnc: rc=0 (p->rc=0)
> > (XEN) livepatch: Hello World
> > (XEN) 'x' pressed - Dumping all livepatch patches
> > (XEN) build-id: e144bafc4ee8635eee5bed8e3988b484765c46c8
> > (XEN) name=xen_hello_world state=APPLIED(2) 0000000000318000 (.data=0000000000319000, .rodata=000000000031a000) using 3 pages.
> > (XEN) xen_extra_version patch 0000000000233fac(12) with 0000000000318000 (16)
> > (XEN) build-id=c4b842c276be43adbe4db788598b1e11bce04dc6
> > (XEN) depend-on=9affa110481e8e13606c61b21e5f6a357a3c8ef9
> >
> > ARM32 still has some issues.
> >
> > The are some TODOs left to be done:
> >
> > General:
> > - Bubble ALT_ORIG_PTR macro for both x86/ARM.
> > - Unify the ELF RELA checks - they are the same on x86/ARM[32,64].
> > - Makefile generating .livepatch.depends needs to ingest the
> > -O argument based on architecture
> > - Test cases should move to common/ ? [Needs Jan's Ack]
>
> In general, I would be in favor of sharing as much as possible the code.
>
> > ARM32 issues:
> > - vm_init_type: Assertion 'is_xen_heap_mfn(ma >> PAGE_SHIFT)' failed at xen/include/asm/mm.h:23
>
> xen/include/asm/mm.h:23 points to the beginning of struct page_info. Did you
> mean to write instead 233?
Probably.
>
> This would point to maddr_virt. This would mean someone is trying to get the
> virtual address of MFN that is not in the xenheap (only the xenheap is
> mapped on ARM32). Do you have the full call stack?
No :-( I need to rebuild it and put it on the board. Maybe in a week?
>
> [...]
>
> >
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> [...]
>
> > diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
> > index aba1320..67749ed 100644
> > --- a/xen/arch/arm/livepatch.c
> > +++ b/xen/arch/arm/livepatch.c
> > @@ -6,66 +6,100 @@
> > #include <xen/lib.h>
> > #include <xen/livepatch_elf.h>
> > #include <xen/livepatch.h>
> > +#include <xen/vmap.h>
> > +#include "livepatch.h"
> > +
> > +#include <asm/mm.h>
> > +
> > +void *vmap_of_xen_text;
> >
> > void arch_livepatch_quiesce(void)
> > {
> > + mfn_t text_mfn;
> > + unsigned int text_order;
> > +
> > + if ( vmap_of_xen_text )
> > + return;
> > +
> > + text_mfn = _mfn(virt_to_mfn(_stext));
> > + text_order = get_order_from_bytes(_end - _start);
> > +
> > + /*
> > + * The text section is read-only. So re-map Xen to be able to patch
> > + * the code.
> > + */
> > + vmap_of_xen_text = __vmap(&text_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR,
> > + VMAP_DEFAULT);
>
> Shouldn't we return an error if we fail to re-map the region?
>
> Otherwise the patch may silently be ignored (see arch_livepatch_apply_jmp).
We do actually bail out of patching if vmap_of_xen_text is NULL.
But let me add a return (-ENOMEM) to this so that we don't attempt to do any
patching and print a nice message.
>
> > }
> >
> > void arch_livepatch_revive(void)
> > {
> > + /* Nuke the instruction cache */
> > + invalidate_icache();
>
> I was about to say that this is not enough. But you called
> clean_and_invalidate_* every time you rewrite the jump. It might be worth to
> add a comment here, explain that the cache has been cleaned before hand.
>
> However, I can't find any data cache flush for the payload. Did I miss
> anything?
No just didn't occur to me - as we patch the .text not the .data.
I will take a look at the existing code and see if I can find out how
to do the data cache flush.
>
> Lastly, before I forgot, you will need to invalidate the branch predictor
> for ARMv7 as it may be architecturally visible to the software (see B2.2.4
> in ARM DDI 0406C.b).
OK.
>
> > +
> > + if ( vmap_of_xen_text )
> > + vunmap(vmap_of_xen_text);
> > +
> > + vmap_of_xen_text = NULL;
> > }
> >
> > int arch_livepatch_verify_func(const struct livepatch_func *func)
> > {
> > - return -ENOSYS;
> > -}
> > + /* No NOP patching yet. */
> > + if ( !func->new_size )
> > + return -EOPNOTSUPP;
> >
> > -void arch_livepatch_apply_jmp(struct livepatch_func *func)
> > -{
> > -}
> > + if ( func->old_size < PATCH_INSN_SIZE )
> > + return -EINVAL;
> >
> > -void arch_livepatch_revert_jmp(const struct livepatch_func *func)
> > -{
> > + return 0;
> > }
> >
> > void arch_livepatch_post_action(void)
> > {
> > + /* arch_livepatch_revive has nuked the instruction cache. */
> > }
> >
> > void arch_livepatch_mask(void)
> > {
> > + /* TODO: No NMI on ARM? */
>
> All interrupts can be masked on ARM so far. Although, local_irq_disable will
> only mask IRQ (i.e interrupt from the interrupt controller).
>
> We may want to mask SError (System Error) via local_abort_{disable,enable}
> to avoid receiving asynchronous abort whilst patching Xen. The interrupt
> will stay pending until it will be re-enabled in arch_livepatch_unmask.
/me nods.
Thanks!
>
> > }
> >
> > void arch_livepatch_unmask(void)
> > {
> > + /* TODO: No NMI on ARM? */
> > }
> >
> > -int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
> > +int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type type)
> > {
> > - return -ENOSYS;
> > -}
> > + unsigned long start = (unsigned long)va;
> > + unsigned int flags;
> >
> > -int arch_livepatch_perform_rel(struct livepatch_elf *elf,
> > - const struct livepatch_elf_sec *base,
> > - const struct livepatch_elf_sec *rela)
> > -{
> > - return -ENOSYS;
> > -}
> > + ASSERT(va);
> > + ASSERT(pages);
> >
> > -int arch_livepatch_perform_rela(struct livepatch_elf *elf,
> > - const struct livepatch_elf_sec *base,
> > - const struct livepatch_elf_sec *rela)
> > -{
> > - return -ENOSYS;
> > -}
> > + if ( type == LIVEPATCH_VA_RX )
> > + flags = 0x2; /* R set,NX clear */
> > + else if ( type == LIVEPATCH_VA_RW )
> > + flags = 0x1; /* R clear, NX set */
> > + else
> > + flags = 0x3; /* R set,NX set */
> >
> > -int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type type)
> > -{
> > - return -ENOSYS;
> > + modify_xen_mappings(start, start + pages * PAGE_SIZE, flags);
> > +
> > + return 0;
> > }
> >
> > void __init arch_livepatch_init(void)
> > {
> > + void *start, *end;
> > +
> > + start = (void *)xen_virt_end;
> > + end = (void *)FIXMAP_ADDR(0);
>
> The space between xen_virt_end and FIXMAP_ADDR may be really small depending
> on the size of the Xen binary.
>
> I would introduce a specific region in the layout of few megabytes (not sure
> how much me need). Or re-use "early relocation address" (8M - 10M) as the
> region is only used during early boot.
OK, will take a look. Thanks!
>
> > +
> > + BUG_ON(start >= end);
> > +
> > + vm_init_type(VMAP_XEN, start, end);
> > }
> >
> > /*
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index eca7cdd..94b4911 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -143,6 +143,8 @@ vaddr_t xenheap_virt_end __read_mostly;
> > vaddr_t xenheap_virt_start __read_mostly;
> > #endif
> >
> > +vaddr_t xen_virt_end;
> > +
> > unsigned long frametable_base_pdx __read_mostly;
> > unsigned long frametable_virt_end __read_mostly;
> >
> > @@ -540,6 +542,9 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr)
> > /* No flush required here as page table is not hooked in yet. */
> > }
> >
> > + if ( i < LPAE_ENTRIES )
>
> Why this check?
/me scratches his head. That was for when I was also printing the entries.
Let me remove it.
>
> > + xen_virt_end = XEN_VIRT_START + (i << PAGE_SHIFT);
> > +
> > pte = pte_of_xenaddr((vaddr_t)xen_xenmap);
> > pte.pt.table = 1;
> > write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte);
>
> > diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> > index 88a79d8..6ffd2d0 100644
> > --- a/xen/common/livepatch.c
> > +++ b/xen/common/livepatch.c
> > @@ -618,7 +618,6 @@ static int prepare_payload(struct payload *payload,
> > sizeof(*region->frame[i].bugs);
> > }
> >
> > -#ifndef CONFIG_ARM
> > sec = livepatch_elf_sec_by_name(elf, ".altinstructions");
> > if ( sec )
> > {
> > @@ -636,8 +635,14 @@ static int prepare_payload(struct payload *payload,
> >
> > for ( a = start; a < end; a++ )
> > {
> > +#ifndef CONFIG_ARM
> > + /* TODO: Bubble ALT_ORIG_PTR up. */
> > const void *instr = &a->instr_offset + a->instr_offset;
> > const void *replacement = &a->repl_offset + a->repl_offset;
> > +#else
> > + const void *instr = &a->orig_offset + a->orig_offset;
> > + const void *replacement = &a->alt_offset + a->alt_offset;
> > +#endif
> >
> > if ( (instr < region->start && instr >= region->end) ||
> > (replacement < region->start && replacement >= region->end) )
> > @@ -647,9 +652,14 @@ static int prepare_payload(struct payload *payload,
> > return -EINVAL;
> > }
> > }
> > +#ifndef CONFIG_ARM
> > apply_alternatives_nocheck(start, end);
> > +#else
> > + apply_alternatives(start, sec->sec->sh_size);
> > +#endif
> > }
> >
> > +#ifndef CONFIG_ARM
> > sec = livepatch_elf_sec_by_name(elf, ".ex_table");
> > if ( sec )
> > {
> > diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
> > index 65c0cdf..f4fcfd6 100644
> > --- a/xen/include/asm-arm/current.h
> > +++ b/xen/include/asm-arm/current.h
> > @@ -33,8 +33,15 @@ static inline struct cpu_info *get_cpu_info(void)
> >
> > #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
> >
> > +#ifdef CONFIG_LIVEPATCH
> > +#define switch_stack_and_jump(stack, fn) \
> > + asm volatile ("mov sp,%0;" \
> > + "bl check_for_livepatch_work;" \
>
> May I ask why check_for_livepatch_work is called in switch_stack_and_jump?
From 29f4ab0b0a4ff62589af35b7cbc2334e1d9acdcd:
To perform and action on a payload, the hypercall sets up a data
structure to schedule the work. A hook is added in the reset_stack_and_jump
to check for work and execute it if needed (specifically we check an
per-cpu flag to make this as quick as possible).
In this way, patches can be applied with all CPUs idle and without
stacks. The first CPU to run check_for_xsplice_work() becomes the
master and triggers a reschedule softirq to trigger all the other CPUs
to enter check_for_xsplice_work() with no stack. Once all CPUs
have rendezvoused, all CPUs disable their IRQs and NMIs are ignored.
The system is then quiscient and the master performs the action.
After this, all CPUs enable IRQs and NMIs are re-enabled.
>
> > + "b " STR(fn) : : "r" (stack) : "memory" )
> > +#else
> > #define switch_stack_and_jump(stack, fn) \
> > asm volatile ("mov sp,%0; b " STR(fn) : : "r" (stack) : "memory" )
> > +#endif
> >
> > #define reset_stack_and_jump(fn) switch_stack_and_jump(get_cpu_info(), fn)
>
> Regards,
>
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support.
2016-08-12 20:50 ` Konrad Rzeszutek Wilk
@ 2016-08-15 13:27 ` Julien Grall
2016-08-15 15:04 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 14+ messages in thread
From: Julien Grall @ 2016-08-15 13:27 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, sstabellini, Ross Lagerwall, Andrew Cooper
Hi Konrad,
On 12/08/2016 22:50, Konrad Rzeszutek Wilk wrote:
> On Fri, Aug 12, 2016 at 05:00:47PM +0200, Julien Grall wrote:
>>> diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
>>> index 65c0cdf..f4fcfd6 100644
>>> --- a/xen/include/asm-arm/current.h
>>> +++ b/xen/include/asm-arm/current.h
>>> @@ -33,8 +33,15 @@ static inline struct cpu_info *get_cpu_info(void)
>>>
>>> #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
>>>
>>> +#ifdef CONFIG_LIVEPATCH
>>> +#define switch_stack_and_jump(stack, fn) \
>>> + asm volatile ("mov sp,%0;" \
>>> + "bl check_for_livepatch_work;" \
>>
>> May I ask why check_for_livepatch_work is called in switch_stack_and_jump?
>
> From 29f4ab0b0a4ff62589af35b7cbc2334e1d9acdcd:
> To perform and action on a payload, the hypercall sets up a data
> structure to schedule the work. A hook is added in the reset_stack_and_jump
> to check for work and execute it if needed (specifically we check an
> per-cpu flag to make this as quick as possible).
>
> In this way, patches can be applied with all CPUs idle and without
> stacks. The first CPU to run check_for_xsplice_work() becomes the
> master and triggers a reschedule softirq to trigger all the other CPUs
> to enter check_for_xsplice_work() with no stack. Once all CPUs
> have rendezvoused, all CPUs disable their IRQs and NMIs are ignored.
> The system is then quiscient and the master performs the action.
> After this, all CPUs enable IRQs and NMIs are re-enabled.
I am a bit confused, switch_stack_and_jump will only be called on ARM
when a vCPU is created. So why do you want to check livepatch at that time?
After looking at the x86 version, I noticed that reset_stack_and_jump is
called every time Xen returns to the guest, correct?
If so, you may want to call check_for_livepatch_work in
leave_hypervisor_tail instead. The latter will be call every time Xen
returns to the guest.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support.
2016-08-15 13:27 ` Julien Grall
@ 2016-08-15 15:04 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 14+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-08-15 15:04 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, sstabellini, Ross Lagerwall, Andrew Cooper
On Mon, Aug 15, 2016 at 03:27:08PM +0200, Julien Grall wrote:
> Hi Konrad,
>
> On 12/08/2016 22:50, Konrad Rzeszutek Wilk wrote:
> > On Fri, Aug 12, 2016 at 05:00:47PM +0200, Julien Grall wrote:
> > > > diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h
> > > > index 65c0cdf..f4fcfd6 100644
> > > > --- a/xen/include/asm-arm/current.h
> > > > +++ b/xen/include/asm-arm/current.h
> > > > @@ -33,8 +33,15 @@ static inline struct cpu_info *get_cpu_info(void)
> > > >
> > > > #define guest_cpu_user_regs() (&get_cpu_info()->guest_cpu_user_regs)
> > > >
> > > > +#ifdef CONFIG_LIVEPATCH
> > > > +#define switch_stack_and_jump(stack, fn) \
> > > > + asm volatile ("mov sp,%0;" \
> > > > + "bl check_for_livepatch_work;" \
> > >
> > > May I ask why check_for_livepatch_work is called in switch_stack_and_jump?
> >
> > From 29f4ab0b0a4ff62589af35b7cbc2334e1d9acdcd:
> > To perform and action on a payload, the hypercall sets up a data
> > structure to schedule the work. A hook is added in the reset_stack_and_jump
> > to check for work and execute it if needed (specifically we check an
> > per-cpu flag to make this as quick as possible).
> >
> > In this way, patches can be applied with all CPUs idle and without
> > stacks. The first CPU to run check_for_xsplice_work() becomes the
> > master and triggers a reschedule softirq to trigger all the other CPUs
> > to enter check_for_xsplice_work() with no stack. Once all CPUs
> > have rendezvoused, all CPUs disable their IRQs and NMIs are ignored.
> > The system is then quiscient and the master performs the action.
> > After this, all CPUs enable IRQs and NMIs are re-enabled.
>
>
> I am a bit confused, switch_stack_and_jump will only be called on ARM when a
> vCPU is created. So why do you want to check livepatch at that time?
>
> After looking at the x86 version, I noticed that reset_stack_and_jump is
> called every time Xen returns to the guest, correct?
Yes. I assumed ARM was the same.
>
> If so, you may want to call check_for_livepatch_work in
> leave_hypervisor_tail instead. The latter will be call every time Xen
> returns to the guest.
Oooh. Thanks!
>
> Regards,
>
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-08-15 15:04 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09 4:18 [Very RFC PATCH] Livepatch - initial ARM64/32 support Konrad Rzeszutek Wilk
2016-08-09 4:18 ` [Very RFC PATCH 1/3] mm/arm: Introduce modify_xen_mappings Konrad Rzeszutek Wilk
2016-08-09 4:18 ` [Very RFC PATCH 2/3] insn: Add arch64_insn_gen_branch_imm to generate branch Konrad Rzeszutek Wilk
2016-08-11 16:03 ` Julien Grall
2016-08-09 4:19 ` [Very RFC PATCH 3/3] livepatch: Initial ARM32/64 support Konrad Rzeszutek Wilk
2016-08-12 15:00 ` Julien Grall
2016-08-12 20:50 ` Konrad Rzeszutek Wilk
2016-08-15 13:27 ` Julien Grall
2016-08-15 15:04 ` Konrad Rzeszutek Wilk
2016-08-09 4:24 ` [Very RFC PATCH] Livepatch - initial ARM64/32 support Konrad Rzeszutek Wilk
2016-08-11 16:28 ` Julien Grall
2016-08-11 19:05 ` Stefano Stabellini
2016-08-11 23:08 ` Konrad Rzeszutek Wilk
2016-08-12 8:08 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.