[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.
Hi Konrad,
On 17/08/2016 19:57, Konrad Rzeszutek Wilk wrote:
+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));
+
+ ASSERT(vmap_of_xen_text);
+
+ /* Save old one. */
+ old_ptr = func->old_addr;
+ memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE);
+
+ if ( func->new_size )
+ insn = aarch64_insn_gen_branch_imm((unsigned long)func->old_addr,
+ (unsigned long)func->new_addr,
+ AARCH64_INSN_BRANCH_NOLINK);
The function aarch64_insn_gen_branch_imm will fail to create the branch if
the difference between old_addr and new_addr is greater than 128MB.
In this case, the function will return AARCH64_BREAK_FAULT which will crash
Xen when the instruction is executed.
I cannot find any sanity check in the hypervisor code. So are we trusting
the payload?
Ugh. This is a thorny one. I concur we need to check this - and better of
do it when we load the payload to make sure the distance is correct.
And that should also be on x86, so will spin of a seperate patch.
And in here add an ASSERT that the insn != AARCH64_BREAK_FAULT
It sounds good to me.
[...]
+
+ modify_xen_mappings(start, start + pages * PAGE_SIZE, flags);
+
+ return 0;
}
void __init arch_livepatch_init(void)
{
+ void *start, *end;
+
+ start = (void *)LIVEPATCH_VMAP;
+ end = start + MB(2);
Could you define the in config.h? So in the future we can rework more easily
the memory layout.
LIVEPATCH_VMAP_START and LIVEPATCH_VMAP_END ?
Something like that.
+
+ vm_init_type(VMAP_XEN, start, end);
}
/*
diff --git a/xen/arch/arm/livepatch.h b/xen/arch/arm/livepatch.h
new file mode 100644
index 0000000..8c8d625
--- /dev/null
+++ b/xen/arch/arm/livepatch.h
I am not sure why this header is living in arch/arm/ and not
include/asm-arm/
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#ifndef __XEN_ARM_LIVEPATCH_H__
+#define __XEN_ARM_LIVEPATCH_H__
+
+/* On ARM32,64 instructions are always 4 bytes long. */
+#define PATCH_INSN_SIZE 4
+
+/*
+ * The va of the hypervisor .text region. We need this as the
+ * normal va are write protected.
+ */
+extern void *vmap_of_xen_text;
+
+#endif /* __XEN_ARM_LIVEPATCH_H__ */
+
+/*
+ * 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/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 06c67bc..e3f3f37 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -15,10 +15,12 @@
#define PATCH_INSN_SIZE 5
-void arch_livepatch_quiesce(void)
+int arch_livepatch_quiesce(void)
{
/* Disable WP to allow changes to read-only pages. */
write_cr0(read_cr0() & ~X86_CR0_WP);
+
+ return 0;
}
void arch_livepatch_revive(void)
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 51afa24..2fc76b6 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 (X86 || ARM_64) && 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 9c45270..af9443d 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -682,7 +682,7 @@ static int prepare_payload(struct payload *payload,
sizeof(*region->frame[i].bugs);
}
-#ifndef CONFIG_ARM
+#ifndef CONFIG_ARM_32
I would prefer if you use CONFIG_ALTERNATIVE rather than CONFIG_ARM_32.
That is not exposed on x86 thought.
I can expose HAVE_ALTERNATIVE on x86 and use that instead.
True. I would like to avoid using CONFIG_ARM_* in the common code as it
is a call to forget to remove the #ifdef when ARM32 will gain
alternative patching.
You are the maintainer of this code, so it is your call :).
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
