[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 15/08/2016 01:07, Konrad Rzeszutek Wilk wrote:
[...]
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
-
Spurious change?
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
new file mode 100644
index 0000000..89ee2f5
--- /dev/null
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ */
+
+#include <xen/errno.h>
+#include <xen/lib.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;
I would prefer if you introduce ARM32 implementation in one go. I.e can
you only return -EOPNOTSUPP here for now?
[...]
+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;
+
+ 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;
+
+ 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;
As you borrow the code from Linux, could we keep the abstraction with
reloc_data and defer the overflow check? It would avoid to have the same
if in multiple place in this code.
+ *(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;
+
It seems the implementation of R_AARCH64_ADR_PREL_PG_HI21_NC is missing.
+ 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;
Hmmm, I think we want to avoid the payload having
R_AARCH64_ADR_PREL_PG_HI21* relocations due to the erratum #843419 on
some Cortex-A53.
I haven't yet looked at how you build the payload, but this may also
affects the process to do it. I will give a look on it.
+
+ 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:
+ */
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
