diff mbox

[Xen-devel,RFC,09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions

Message ID 1462466065-30212-10-git-send-email-julien.grall@arm.com
State Superseded
Headers show

Commit Message

Julien Grall May 5, 2016, 4:34 p.m. UTC
We may need to update branch instruction when patching Xen.

The code has been imported from the files arch/arm64/kernel/insn.c
and arch/arm64/include/asm/insn.h in Linux v4.6-rc3.

Note that only the necessary helpers have been imported.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/arm64/Makefile      |   1 +
 xen/arch/arm/arm64/insn.c        | 213 +++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/insn.h |  72 +++++++++++++
 xen/include/asm-arm/insn.h       |  22 ++++
 4 files changed, 308 insertions(+)
 create mode 100644 xen/arch/arm/arm64/insn.c
 create mode 100644 xen/include/asm-arm/arm64/insn.h
 create mode 100644 xen/include/asm-arm/insn.h

Comments

Julien Grall May 9, 2016, 1:04 p.m. UTC | #1
Hi Stefano,

On 09/05/16 11:05, Stefano Stabellini wrote:
> On Thu, 5 May 2016, Julien Grall wrote:
>> +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
>> +				  u32 insn, u64 imm)
>> +{
>> +	u32 immlo, immhi, mask;
>> +	int shift;
>
> Here Linux checks for insn == AARCH64_BREAK_FAULT. Shouldn't we do the
> same?

I am not sure why I removed this check. I will re-add on the next version.

>> diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
>> new file mode 100644
>> index 0000000..fe9419c
>> --- /dev/null
>> +++ b/xen/include/asm-arm/insn.h
>> @@ -0,0 +1,22 @@
>> +#ifndef __ARCH_ARM_INSN
>> +#define __ARCH_ARM_INSN
>> +
>> +#include <xen/types.h>
>> +
>> +#if defined(CONFIG_ARM_32)
>> +# include <asm/arm32/insn.h>
>
> This is missing

I will drop the include as I don't plan to implement instruction 
patching for ARM32.

Regards,
Julien Grall May 14, 2016, 10:49 a.m. UTC | #2
Hi Konrad,

On 13/05/2016 21:35, Konrad Rzeszutek Wilk wrote:
>> diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
>> new file mode 100644
>> index 0000000..cfcdbe9
>> --- /dev/null
>> +++ b/xen/include/asm-arm/arm64/insn.h
>> @@ -0,0 +1,72 @@
>> +/*
>> + * Copyright (C) 2013 Huawei Ltd.
>> + * Author: Jiang Liu <liuj97@gmail.com>
>> + *
>> + * Copyright (C) 2014 Zi Shen Lim <zlim.lnx@gmail.com>
>> + *
>> + * 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_ARM64_INSN
>> +#define __ARCH_ARM_ARM64_INSN
>> +
>> +#include <xen/config.h>
>> +#include <xen/types.h>
>> +#include <xen/stdbool.h>
>> +
>> +enum aarch64_insn_imm_type {
>> +	AARCH64_INSN_IMM_ADR,
>> +	AARCH64_INSN_IMM_26,
>> +	AARCH64_INSN_IMM_19,
>> +	AARCH64_INSN_IMM_16,
>> +	AARCH64_INSN_IMM_14,
>> +	AARCH64_INSN_IMM_12,
>> +	AARCH64_INSN_IMM_9,
>> +	AARCH64_INSN_IMM_7,
>> +	AARCH64_INSN_IMM_6,
>> +	AARCH64_INSN_IMM_S,
>> +	AARCH64_INSN_IMM_R,
>> +	AARCH64_INSN_IMM_MAX
>> +};
>> +
>> +#define	__AARCH64_INSN_FUNCS(abbr, mask, val)	\
>> +static always_inline bool_t aarch64_insn_is_##abbr(u32 code) \
>> +{ return (code & (mask)) == (val); } \
>> +static always_inline u32 aarch64_insn_get_##abbr##_value(void) \
>> +{ return (val); }
>> +
>> +__AARCH64_INSN_FUNCS(b,		0xFC000000, 0x14000000)
>
> This looks odd here but in the file the aligment is OK.
>> +__AARCH64_INSN_FUNCS(bl,	0xFC000000, 0x94000000)
>> +__AARCH64_INSN_FUNCS(cbz,	0x7F000000, 0x34000000)
>> +__AARCH64_INSN_FUNCS(cbnz,	0x7F000000, 0x35000000)
>> +__AARCH64_INSN_FUNCS(tbz,	0x7F000000, 0x36000000)
>> +__AARCH64_INSN_FUNCS(tbnz,	0x7F000000, 0x37000000)
>> +__AARCH64_INSN_FUNCS(bcond,	0xFF000010, 0x54000000)
>> +
>> +bool aarch64_insn_is_branch_imm(u32 insn);
>> +
>> +u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn);
>> +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
>> +				  u32 insn, u64 imm);
>
> While this is off. I think you have tabs here instead of spaces?

The code is a verbatim copy from Linux, so I would prefer to not modify 
the code for future port/bug fixing.

Regards,
Julien Grall May 23, 2016, 10:52 a.m. UTC | #3
Hi Stefano,

On 09/05/16 14:04, Julien Grall wrote:
> On 09/05/16 11:05, Stefano Stabellini wrote:
>> On Thu, 5 May 2016, Julien Grall wrote:
>>> +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
>>> +                  u32 insn, u64 imm)
>>> +{
>>> +    u32 immlo, immhi, mask;
>>> +    int shift;
>>
>> Here Linux checks for insn == AARCH64_BREAK_FAULT. Shouldn't we do the
>> same?
>
> I am not sure why I removed this check. I will re-add on the next version.

Actually the patch was based on Linux 4.6-rc3 (see in commit message) 
which does not contain the check. It has been added in 4.6-rc4.

I will re-sync the code with Linux 4.6.

>
>>> diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
>>> new file mode 100644
>>> index 0000000..fe9419c
>>> --- /dev/null
>>> +++ b/xen/include/asm-arm/insn.h
>>> @@ -0,0 +1,22 @@
>>> +#ifndef __ARCH_ARM_INSN
>>> +#define __ARCH_ARM_INSN
>>> +
>>> +#include <xen/types.h>
>>> +
>>> +#if defined(CONFIG_ARM_32)
>>> +# include <asm/arm32/insn.h>
>>
>> This is missing
>
> I will drop the include as I don't plan to implement instruction
> patching for ARM32.

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 39c6ac6..c1fa43f 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -5,6 +5,7 @@  obj-$(EARLY_PRINTK) += debug.o
 obj-y += domctl.o
 obj-y += domain.o
 obj-y += entry.o
+obj-y += insn.o
 obj-y += smpboot.o
 obj-y += traps.o
 obj-y += vfp.o
diff --git a/xen/arch/arm/arm64/insn.c b/xen/arch/arm/arm64/insn.c
new file mode 100644
index 0000000..54a6b62
--- /dev/null
+++ b/xen/arch/arm/arm64/insn.c
@@ -0,0 +1,213 @@ 
+/*
+ * Based on Linux arch/arm64/kernel/insn.c
+ *
+ * Copyright (C) 2013 Huawei Ltd.
+ * Author: Jiang Liu <liuj97@gmail.com>
+ *
+ * Copyright (C) 2014-2016 Zi Shen Lim <zlim.lnx@gmail.com>
+ *
+ * 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/config.h>
+#include <xen/types.h>
+#include <xen/lib.h>
+#include <xen/errno.h>
+#include <xen/sizes.h>
+#include <xen/bitops.h>
+#include <asm/insn.h>
+#include <asm/arm64/insn.h>
+
+bool aarch64_insn_is_branch_imm(u32 insn)
+{
+	return (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn) ||
+		aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn) ||
+		aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
+		aarch64_insn_is_bcond(insn));
+}
+
+static int aarch64_get_imm_shift_mask(enum aarch64_insn_imm_type type,
+						u32 *maskp, int *shiftp)
+{
+	u32 mask;
+	int shift;
+
+	switch (type) {
+	case AARCH64_INSN_IMM_26:
+		mask = BIT(26) - 1;
+		shift = 0;
+		break;
+	case AARCH64_INSN_IMM_19:
+		mask = BIT(19) - 1;
+		shift = 5;
+		break;
+	case AARCH64_INSN_IMM_16:
+		mask = BIT(16) - 1;
+		shift = 5;
+		break;
+	case AARCH64_INSN_IMM_14:
+		mask = BIT(14) - 1;
+		shift = 5;
+		break;
+	case AARCH64_INSN_IMM_12:
+		mask = BIT(12) - 1;
+		shift = 10;
+		break;
+	case AARCH64_INSN_IMM_9:
+		mask = BIT(9) - 1;
+		shift = 12;
+		break;
+	case AARCH64_INSN_IMM_7:
+		mask = BIT(7) - 1;
+		shift = 15;
+		break;
+	case AARCH64_INSN_IMM_6:
+	case AARCH64_INSN_IMM_S:
+		mask = BIT(6) - 1;
+		shift = 10;
+		break;
+	case AARCH64_INSN_IMM_R:
+		mask = BIT(6) - 1;
+		shift = 16;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*maskp = mask;
+	*shiftp = shift;
+
+	return 0;
+}
+
+#define ADR_IMM_HILOSPLIT	2
+#define ADR_IMM_SIZE		SZ_2M
+#define ADR_IMM_LOMASK		((1 << ADR_IMM_HILOSPLIT) - 1)
+#define ADR_IMM_HIMASK		((ADR_IMM_SIZE >> ADR_IMM_HILOSPLIT) - 1)
+#define ADR_IMM_LOSHIFT		29
+#define ADR_IMM_HISHIFT		5
+
+u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn)
+{
+	u32 immlo, immhi, mask;
+	int shift;
+
+	switch (type) {
+	case AARCH64_INSN_IMM_ADR:
+		shift = 0;
+		immlo = (insn >> ADR_IMM_LOSHIFT) & ADR_IMM_LOMASK;
+		immhi = (insn >> ADR_IMM_HISHIFT) & ADR_IMM_HIMASK;
+		insn = (immhi << ADR_IMM_HILOSPLIT) | immlo;
+		mask = ADR_IMM_SIZE - 1;
+		break;
+	default:
+		if (aarch64_get_imm_shift_mask(type, &mask, &shift) < 0) {
+			printk(XENLOG_ERR "aarch64_insn_decode_immediate: unknown immediate encoding %d\n",
+			       type);
+			return 0;
+		}
+	}
+
+	return (insn >> shift) & mask;
+}
+
+u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
+				  u32 insn, u64 imm)
+{
+	u32 immlo, immhi, mask;
+	int shift;
+
+	switch (type) {
+	case AARCH64_INSN_IMM_ADR:
+		shift = 0;
+		immlo = (imm & ADR_IMM_LOMASK) << ADR_IMM_LOSHIFT;
+		imm >>= ADR_IMM_HILOSPLIT;
+		immhi = (imm & ADR_IMM_HIMASK) << ADR_IMM_HISHIFT;
+		imm = immlo | immhi;
+		mask = ((ADR_IMM_LOMASK << ADR_IMM_LOSHIFT) |
+			(ADR_IMM_HIMASK << ADR_IMM_HISHIFT));
+		break;
+	default:
+		if (aarch64_get_imm_shift_mask(type, &mask, &shift) < 0) {
+			printk(XENLOG_ERR "aarch64_insn_encode_immediate: unknown immediate encoding %d\n",
+			       type);
+			return AARCH64_BREAK_FAULT;
+		}
+	}
+
+	/* Update the immediate field. */
+	insn &= ~(mask << shift);
+	insn |= (imm & mask) << shift;
+
+	return insn;
+}
+
+/*
+ * Decode the imm field of a branch, and return the byte offset as a
+ * signed value (so it can be used when computing a new branch
+ * target).
+ */
+s32 aarch64_get_branch_offset(u32 insn)
+{
+	s32 imm;
+
+	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn)) {
+		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_26, insn);
+		return (imm << 6) >> 4;
+	}
+
+	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
+	    aarch64_insn_is_bcond(insn)) {
+		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_19, insn);
+		return (imm << 13) >> 11;
+	}
+
+	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn)) {
+		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_14, insn);
+		return (imm << 18) >> 16;
+	}
+
+	/* Unhandled instruction */
+	BUG();
+}
+
+/*
+ * Encode the displacement of a branch in the imm field and return the
+ * updated instruction.
+ */
+u32 aarch64_set_branch_offset(u32 insn, s32 offset)
+{
+	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn))
+		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
+						     offset >> 2);
+
+	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
+	    aarch64_insn_is_bcond(insn))
+		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_19, insn,
+						     offset >> 2);
+
+	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn))
+		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_14, insn,
+						     offset >> 2);
+
+	/* Unhandled instruction */
+	BUG();
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * indent-tabs-mode: t
+ * End:
+ */
diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
new file mode 100644
index 0000000..cfcdbe9
--- /dev/null
+++ b/xen/include/asm-arm/arm64/insn.h
@@ -0,0 +1,72 @@ 
+/*
+ * Copyright (C) 2013 Huawei Ltd.
+ * Author: Jiang Liu <liuj97@gmail.com>
+ *
+ * Copyright (C) 2014 Zi Shen Lim <zlim.lnx@gmail.com>
+ *
+ * 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_ARM64_INSN
+#define __ARCH_ARM_ARM64_INSN
+
+#include <xen/config.h>
+#include <xen/types.h>
+#include <xen/stdbool.h>
+
+enum aarch64_insn_imm_type {
+	AARCH64_INSN_IMM_ADR,
+	AARCH64_INSN_IMM_26,
+	AARCH64_INSN_IMM_19,
+	AARCH64_INSN_IMM_16,
+	AARCH64_INSN_IMM_14,
+	AARCH64_INSN_IMM_12,
+	AARCH64_INSN_IMM_9,
+	AARCH64_INSN_IMM_7,
+	AARCH64_INSN_IMM_6,
+	AARCH64_INSN_IMM_S,
+	AARCH64_INSN_IMM_R,
+	AARCH64_INSN_IMM_MAX
+};
+
+#define	__AARCH64_INSN_FUNCS(abbr, mask, val)	\
+static always_inline bool_t aarch64_insn_is_##abbr(u32 code) \
+{ return (code & (mask)) == (val); } \
+static always_inline u32 aarch64_insn_get_##abbr##_value(void) \
+{ return (val); }
+
+__AARCH64_INSN_FUNCS(b,		0xFC000000, 0x14000000)
+__AARCH64_INSN_FUNCS(bl,	0xFC000000, 0x94000000)
+__AARCH64_INSN_FUNCS(cbz,	0x7F000000, 0x34000000)
+__AARCH64_INSN_FUNCS(cbnz,	0x7F000000, 0x35000000)
+__AARCH64_INSN_FUNCS(tbz,	0x7F000000, 0x36000000)
+__AARCH64_INSN_FUNCS(tbnz,	0x7F000000, 0x37000000)
+__AARCH64_INSN_FUNCS(bcond,	0xFF000010, 0x54000000)
+
+bool aarch64_insn_is_branch_imm(u32 insn);
+
+u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn);
+u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
+				  u32 insn, u64 imm);
+
+s32 aarch64_get_branch_offset(u32 insn);
+u32 aarch64_set_branch_offset(u32 insn, s32 offset);
+
+#endif /* !__ARCH_ARM_ARM64_INSN */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * indent-tabs-mode: t
+ * End:
+ */
diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
new file mode 100644
index 0000000..fe9419c
--- /dev/null
+++ b/xen/include/asm-arm/insn.h
@@ -0,0 +1,22 @@ 
+#ifndef __ARCH_ARM_INSN
+#define __ARCH_ARM_INSN
+
+#include <xen/types.h>
+
+#if defined(CONFIG_ARM_32)
+# include <asm/arm32/insn.h>
+#elif defined(CONFIG_ARM_64)
+# include <asm/arm64/insn.h>
+#else
+# error "unknown ARM variant"
+#endif
+
+#endif /* !__ARCH_ARM_INSN */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * indent-tabs-mode: t
+ * End:
+ */