[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 1/2] xen/arm32: Introduce alternative runtime patching
Hi Wei,
On 03/16/2017 09:53 AM, Wei Chen wrote:
This patch is based on the implementation of ARM64, it introduces
alternative runtime patching to ARM32. This allows to patch assembly
instruction at runtime to either fix hardware bugs or optimize for
certain hardware features on ARM32 platform.
Xen hypervisor is using ARM execution state only on ARM32 platform,
Thumb is not used. So, the Thumb only branch instructions (CBZ, CBNZ,
TBB and TBH) are not considered in alternatives.
The left ARM32 branch instructions are BX, BLX, BL and B. The
instruction BX is taking a register in parameter, so we don't need to
rewrite it. The instructions BLX, BL and B are using the similar
encoding for the offset and will avoid specific case when extracting
and updating the offset.
In this patch, we include alternative.h header file to livepatch.c
directly for ARM32 compilation issues. When the alternative patching
config is enabled, the livepatch.c will use the alternative functions.
In this case, we should include the alternative header file to this
file. But for ARM64, it does not include this header file directly.
It includes this header file indirectly through:
sched.h->domain.h->page.h->alternative.h.
But, unfortunately, the page.h of ARM32 doesn't include alternative.h,
and we don't have the reason to include it to ARM32 page.h now. So we
have to include the alternative.h directly in livepatch.c.
Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
---
xen/arch/arm/Kconfig | 2 +-
xen/arch/arm/arm32/Makefile | 1 +
xen/arch/arm/arm32/insn.c | 99 ++++++++++++++++++++++++++++++++++++++++
xen/common/livepatch.c | 1 +
CC Ross and Konrad for the livepatch part.
xen/include/asm-arm/arm32/insn.h | 57 +++++++++++++++++++++++
xen/include/asm-arm/insn.h | 2 +
6 files changed, 161 insertions(+), 1 deletion(-)
create mode 100644 xen/arch/arm/arm32/insn.c
create mode 100644 xen/include/asm-arm/arm32/insn.h
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 2e023d1..43123e6 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -12,11 +12,11 @@ config ARM_32
config ARM_64
def_bool y
depends on 64BIT
- select HAS_ALTERNATIVE
select HAS_GICV3
config ARM
def_bool y
+ select HAS_ALTERNATIVE
select HAS_ARM_HDLCD
select HAS_DEVICE_TREE
select HAS_MEM_ACCESS
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 4395693..0ac254f 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -4,6 +4,7 @@ obj-$(EARLY_PRINTK) += debug.o
obj-y += domctl.o
obj-y += domain.o
obj-y += entry.o
+obj-y += insn.o
obj-$(CONFIG_LIVEPATCH) += livepatch.o
obj-y += proc-v7.o proc-caxx.o
obj-y += smpboot.o
diff --git a/xen/arch/arm/arm32/insn.c b/xen/arch/arm/arm32/insn.c
new file mode 100644
index 0000000..91a3010
--- /dev/null
+++ b/xen/arch/arm/arm32/insn.c
@@ -0,0 +1,99 @@
+/*
+ * Copyright (C) 2017 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <xen/lib.h>
+#include <xen/bitops.h>
+#include <xen/sizes.h>
+#include <asm/bug.h>
This is already included by xen/lib.h.
+#include <asm/insn.h>
+
+/* Mask of branch instructions' immediate. */
+#define BRANCH_INSN_IMM_MASK GENMASK(23, 0)
+/* Shift of branch instructions' immediate. */
+#define BRANCH_INSN_IMM_SHIFT 0
+
+static uint32_t branch_insn_encode_immediate(uint32_t insn, int32_t offset)
+{
+ uint32_t imm;
+
+ /*
+ * Encode the offset to imm. All ARM32 instructions must be word aligned.
+ * Therefore the offset value's bits [1:0] equal to zero.
+ * (see ARM DDI 0406C.b A8.8.18/A8.8.25 for more encode/decode details
DDI 0406C.b is an old version (July 2012) of the ARM ARM. Please try to
use the latest version (e.g 0406C.c released in May 2014) when possible.
+ * about ARM32 branch instructions)
+ */
+ imm = ((offset >> 2) & BRANCH_INSN_IMM_MASK) << BRANCH_INSN_IMM_SHIFT;
+
+ /* Update the immediate field. */
+ insn &= ~(BRANCH_INSN_IMM_MASK << BRANCH_INSN_IMM_SHIFT);
+ insn |= imm;
+
+ return insn;
+}
+
+/*
+ * Decode the branch offset from a branch instruction's imm field.
+ * The branch offset is a signed value, so it can be used to compute
+ * a new branch target.
+ */
+int32_t aarch32_get_branch_offset(uint32_t insn)
+{
+ uint32_t imm;
+
+ /* Retrieve imm from branch instruction. */
+ imm = ( insn >> BRANCH_INSN_IMM_SHIFT ) & BRANCH_INSN_IMM_MASK;
+
+ /*
+ * Check the imm signed bit. If the imm is a negative value, we
+ * have to extend the imm to a full 32 bit negative value.
+ */
+ if ( imm & BIT(23) )
+ imm |= GENMASK(31, 24);
+
+ return (int32_t)(imm << 2);
+}
+
+/*
+ * Encode the displacement of a branch in the imm field and return the
+ * updated instruction.
+ */
+uint32_t aarch32_set_branch_offset(uint32_t insn, int32_t offset)
+{
+ if ( offset & 0x3 )
+ {
+ printk(XENLOG_ERR
+ "%s: ARM32 instructions must be word aligned.\n", __func__);
This error message looks wrong to me. The offset is not an instruction.
But do we really care about checking that offset is aligned to 4-bit?
After all we will shift the value later on.
+ return BUG_OPCODE;
+ }
+
+ /* B/BL support [-32M, 32M) offset (see ARM DDI 0406C.b A4.3). */
+ if ( offset < -SZ_32M || offset >= SZ_32M )
+ {
+ printk(XENLOG_ERR
+ "%s: new branch offset out of range.\n", __func__);
+ return BUG_OPCODE;
+ }
+
+ return branch_insn_encode_immediate(insn, offset);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 246e673..f14bcbc 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -25,6 +25,7 @@
#include <xen/livepatch.h>
#include <xen/livepatch_payload.h>
+#include <asm/alternative.h>
#include <asm/event.h>
/*
diff --git a/xen/include/asm-arm/arm32/insn.h b/xen/include/asm-arm/arm32/insn.h
new file mode 100644
index 0000000..045acc3
--- /dev/null
+++ b/xen/include/asm-arm/arm32/insn.h
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2017 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __ARCH_ARM_ARM32_INSN
+#define __ARCH_ARM_ARM32_INSN
+
+#include <xen/types.h>
+
+#define __AARCH32_INSN_FUNCS(abbr, mask, val) \
+static always_inline bool_t aarch32_insn_is_##abbr(uint32_t code) \
+{ \
+ return (code & (mask)) == (val); \
+}
+
+__AARCH32_INSN_FUNCS(b, 0x0F000000, 0x0A000000)
Looking at the ARM ARM (A8.8.18 in DDI406C.c) when cond = 0b1111, this
will be a bx. Thankfully we also want to catch them.
I think this needs to be spelled out in the code to help the reader
understand how bx is handled.
+__AARCH32_INSN_FUNCS(bl, 0x0F000000, 0x0B000000)
+
+int32_t aarch32_get_branch_offset(uint32_t insn);
+uint32_t aarch32_set_branch_offset(uint32_t insn, int32_t offset);
+
+/* Wrapper for common code */
+static inline bool insn_is_branch_imm(uint32_t insn)
+{
+ return ( aarch32_insn_is_b(insn) || aarch32_insn_is_bl(insn) );
+}
+
+static inline int32_t insn_get_branch_offset(uint32_t insn)
+{
+ return aarch32_get_branch_offset(insn);
+}
+
+static inline uint32_t insn_set_branch_offset(uint32_t insn, int32_t offset)
+{
+ return aarch32_set_branch_offset(insn, offset);
+}
+
+#endif /* !__ARCH_ARM_ARM32_INSN */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
index a205ceb..3489179 100644
--- a/xen/include/asm-arm/insn.h
+++ b/xen/include/asm-arm/insn.h
@@ -5,6 +5,8 @@
#if defined(CONFIG_ARM_64)
# include <asm/arm64/insn.h>
+#elif defined(CONFIG_ARM_32)
+# include <asm/arm32/insn.h>
#else
# error "unknown ARM variant"
#endif
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
