[PATCHv4 0/5] Legacy instruction emulation for arm64

[PATCHv4 0/5] Legacy instruction emulation for arm64

[Punit Agrawal]

Hi,

This is the fourth posting of the legacy instruction support for
arm64. Previous postings can be found at [1][2][3].

The patchset creates the infrastructure to support legacy instruction
emulation and then uses this to add support for the emulation of
SWP{B} and CP15 Barrier instructions from ARMv7 to the v8 port of
Linux. When available, the common infrastructure allows for enabling
the hardware execution of legacy instructions. Runtime support for
changing the execution mode - undef, legacy, or hw execution is
provided via nodes in sysctl. The default behaviour varies with the
state of the instruction in the architecture (deprecated or obsolete)
and is documented as part of this series.

Patches 1-2/5 add infrastructure code to add support for undefined
instruction hooks and decoding condition checks.

Patch 3-4/5 adds infrastructure to support legacy instruction
emulation and uses this to then add support for SWP and CP15 barriers
respectively.

Patch 5/5 introduces a trace point to log instruction emulation and then
uses this to trace the usage of the above instructions when using
emulation.


Cheers,
Punit

Changes since [3]:
* Shortened function name aarch32_insn_is_wide_instruction ->
aarch32_insn_is_wide
* Removed unnecessary addprefix
* Changed default suggestion from 'N' to 'Y'
* Allow deprecated instructions to potentially only implement
emulation or hardware execution - should help when certain
instructions are difficult to emulate.
* Disable CP15 barriers when hot-unplugging the CPUs
* Added assembler comment to trace the equivalent barrier when CP15
Barriers are used.

Changes since [2]:
* Made trace point local to arm64
* Re-factored infrastructure to handle registration / un-registration
of undefined hooks as well as handle enabling hardware execution when
supported
* Added documentation for default execution modes and how to change it
via sysctl

Changes since [1]:
* Added support for Thumb instructions when registering undefined
hooks as well
* Emulation support is now added to armv8_deprecated.c (was previously
v7_obsolete.c)
* Instruction support level - Off, Emulate or Enable (when supported
in hardware) - is now controlled through sysctl
* Using trace points instead of debugfs for stats reporting

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/351054
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/292213.html
[3] http://thread.gmane.org/gmane.linux.ports.arm.kernel/366728

Punit Agrawal (5):
  arm64: Add support for hooks to handle undefined instructions
  arm64: Add AArch32 instruction set condition code checks
  arm64: Port SWP/SWPB emulation support from arm
  arm64: Emulate CP15 Barrier instructions
  arm64: Trace emulation of AArch32 legacy instructions

 Documentation/arm64/legacy_instructions.txt |  45 +++
 arch/arm64/Kconfig                          |  54 +++
 arch/arm64/include/asm/insn.h               |  10 +
 arch/arm64/include/asm/opcodes.h            |   1 +
 arch/arm64/include/asm/traps.h              |  16 +
 arch/arm64/kernel/Makefile                  |   5 +-
 arch/arm64/kernel/armv8_deprecated.c        | 565 ++++++++++++++++++++++++++++
 arch/arm64/kernel/insn.c                    |  26 ++
 arch/arm64/kernel/trace-events-emulation.h  |  40 ++
 arch/arm64/kernel/traps.c                   |  68 ++++
 10 files changed, 829 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/arm64/legacy_instructions.txt
 create mode 100644 arch/arm64/include/asm/opcodes.h
 create mode 100644 arch/arm64/kernel/armv8_deprecated.c
 create mode 100644 arch/arm64/kernel/trace-events-emulation.h

-- 
2.1.1

[PATCHv4 1/5] arm64: Add support for hooks to handle undefined instructions

[Punit Agrawal]

Add support to register hooks for undefined instructions. The handlers
will be called when the undefined instruction and the processor state
(as contained in pstate) match criteria used at registration.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/insn.h  |  2 ++
 arch/arm64/include/asm/traps.h | 16 ++++++++++
 arch/arm64/kernel/insn.c       |  5 ++++
 arch/arm64/kernel/traps.c      | 68 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 91 insertions(+)

diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 56a9e63..1bb0430 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -354,6 +354,8 @@ bool aarch64_insn_hotpatch_safe(u32 old_insn, u32 new_insn);
 int aarch64_insn_patch_text_nosync(void *addr, u32 insn);
 int aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt);
 int aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt);
+
+bool aarch32_insn_is_wide(u32 insn);
 #endif /* __ASSEMBLY__ */
 
 #endif	/* __ASM_INSN_H */
diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 10ca8ff..4faaf03 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -18,6 +18,22 @@
 #ifndef __ASM_TRAP_H
 #define __ASM_TRAP_H
 
+#include <linux/list.h>
+
+struct pt_regs;
+
+struct undef_hook {
+	struct list_head node;
+	u32 instr_mask;
+	u32 instr_val;
+	u64 pstate_mask;
+	u64 pstate_val;
+	int (*fn)(struct pt_regs *regs, u32 instr);
+};
+
+int register_undef_hook(struct undef_hook *hook);
+void unregister_undef_hook(struct undef_hook *hook);
+
 static inline int in_exception_text(unsigned long ptr)
 {
 	extern char __exception_text_start[];
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index e007714..ab00eb5 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -959,3 +959,8 @@ u32 aarch64_insn_gen_logical_shifted_reg(enum aarch64_insn_register dst,
 
 	return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_6, insn, shift);
 }
+
+bool aarch32_insn_is_wide(u32 insn)
+{
+	return insn >= 0xe800;
+}
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index de1b085..c051d6c 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -259,6 +259,71 @@ void arm64_notify_die(const char *str, struct pt_regs *regs,
 	}
 }
 
+static LIST_HEAD(undef_hook);
+static DEFINE_RAW_SPINLOCK(undef_lock);
+
+int register_undef_hook(struct undef_hook *hook)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&undef_lock, flags);
+	list_add(&hook->node, &undef_hook);
+	raw_spin_unlock_irqrestore(&undef_lock, flags);
+
+	return 0;
+}
+
+void unregister_undef_hook(struct undef_hook *hook)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&undef_lock, flags);
+	list_del(&hook->node);
+	raw_spin_unlock_irqrestore(&undef_lock, flags);
+}
+
+static int call_undef_hook(struct pt_regs *regs)
+{
+	struct undef_hook *hook;
+	unsigned long flags;
+	u32 instr;
+	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
+	void __user *pc = (void __user *)instruction_pointer(regs);
+
+	if (!user_mode(regs))
+		return 1;
+
+	if (compat_thumb_mode(regs)) {
+		/* 16-bit Thumb instruction */
+		if (get_user(instr, (u16 __user *)pc))
+			goto exit;
+		instr = le16_to_cpu(instr);
+		if (aarch32_insn_is_wide(instr)) {
+			u32 instr2;
+
+			if (get_user(instr2, (u16 __user *)(pc + 2)))
+				goto exit;
+			instr2 = le16_to_cpu(instr2);
+			instr = (instr << 16) | instr2;
+		}
+	} else {
+		/* 32-bit ARM instruction */
+		if (get_user(instr, (u32 __user *)pc))
+			goto exit;
+		instr = le32_to_cpu(instr);
+	}
+
+	raw_spin_lock_irqsave(&undef_lock, flags);
+	list_for_each_entry(hook, &undef_hook, node)
+		if ((instr & hook->instr_mask) == hook->instr_val &&
+			(regs->pstate & hook->pstate_mask) == hook->pstate_val)
+			fn = hook->fn;
+
+	raw_spin_unlock_irqrestore(&undef_lock, flags);
+exit:
+	return fn ? fn(regs, instr) : 1;
+}
+
 asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
 {
 	siginfo_t info;
@@ -268,6 +333,9 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
 	if (!aarch32_break_handler(regs))
 		return;
 
+	if (call_undef_hook(regs) == 0)
+		return;
+
 	if (show_unhandled_signals && unhandled_signal(current, SIGILL) &&
 	    printk_ratelimit()) {
 		pr_info("%s[%d]: undefined instruction: pc=%p\n",
-- 
2.1.1

[PATCHv4 2/5] arm64: Add AArch32 instruction set condition code checks

[Punit Agrawal]

Port support for AArch32 instruction condition code checking from arm
to arm64.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/opcodes.h | 1 +
 arch/arm64/kernel/Makefile       | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/include/asm/opcodes.h

diff --git a/arch/arm64/include/asm/opcodes.h b/arch/arm64/include/asm/opcodes.h
new file mode 100644
index 0000000..4e603ea
--- /dev/null
+++ b/arch/arm64/include/asm/opcodes.h
@@ -0,0 +1 @@
+#include <../../arm/include/asm/opcodes.h>
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 5bd029b..a4d8671 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -18,7 +18,8 @@ arm64-obj-y		:= cputable.o debug-monitors.o entry.o irq.o fpsimd.o	\
 			   cpuinfo.o
 
 arm64-obj-$(CONFIG_COMPAT)		+= sys32.o kuser32.o signal32.o 	\
-					   sys_compat.o
+					   sys_compat.o 			\
+					   ../../arm/kernel/opcodes.o
 arm64-obj-$(CONFIG_FUNCTION_TRACER)	+= ftrace.o entry-ftrace.o
 arm64-obj-$(CONFIG_MODULES)		+= arm64ksyms.o module.o
 arm64-obj-$(CONFIG_SMP)			+= smp.o smp_spin_table.o topology.o
-- 
2.1.1

[PATCHv4 3/5] arm64: Port SWP/SWPB emulation support from arm

[Punit Agrawal]

Typically, providing support for legacy instructions requires
emulating the behaviour of instructions whose encodings have become
undefined. If the instructions haven't been removed from the
architecture, there maybe an option in the implementation to turn
on/off the support for these instructions.

Create common infrastructure to support legacy instruction
emulation. In addition to emulation, also provide an option to support
hardware execution when supported. The default execution mode (one of
undef, emulate, hw exeuction) is dependent on the state of the
instruction (deprecated or obsolete) in the architecture and
can specified at the time of registering the instruction handlers. The
runtime state of the emulation can be controlled by writing to
individual nodes in sysctl. The expected default behaviour is
documented as part of this patch.

Use the new infrastructure to port the emulation of the SWP and SWPB
instructions using LDXR/STXR sequences from the arm port to arm64.

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 Documentation/arm64/legacy_instructions.txt |  40 +++
 arch/arm64/Kconfig                          |  39 +++
 arch/arm64/include/asm/insn.h               |   6 +
 arch/arm64/kernel/Makefile                  |   1 +
 arch/arm64/kernel/armv8_deprecated.c        | 419 ++++++++++++++++++++++++++++
 arch/arm64/kernel/insn.c                    |   8 +
 6 files changed, 513 insertions(+)
 create mode 100644 Documentation/arm64/legacy_instructions.txt
 create mode 100644 arch/arm64/kernel/armv8_deprecated.c

diff --git a/Documentation/arm64/legacy_instructions.txt b/Documentation/arm64/legacy_instructions.txt
new file mode 100644
index 0000000..5ab5861
--- /dev/null
+++ b/Documentation/arm64/legacy_instructions.txt
@@ -0,0 +1,40 @@
+The arm64 port of the Linux kernel provides infrastructure to support
+emulation of instructions which have been deprecated, or obsoleted in
+the architecture. The infrastructure code uses undefined instruction
+hooks to support emulation. Where available it also allows turning on
+the instruction execution in hardware.
+
+The emulation mode can be controlled by writing to sysctl nodes
+(/proc/sys/abi). The following explains the different execution
+behaviours and the corresponding values of the sysctl nodes -
+
+* Undef
+  Value: 0
+  Generates undefined instruction abort. Default for instructions that
+  have been obsoleted in the architecture, e.g., SWP
+
+* Emulate
+  Value: 1
+  Uses software emulation. To aid migration of software, in this mode
+  usage of emulated instruction is traced as well as rate limited
+  warnings are issued. This is the default for deprecated
+  instructions, .e.g., CP15 barriers
+
+* Hardware Execution
+  Value: 2
+  Although marked as deprecated, some implementations may support the
+  enabling/disabling of hardware support for the execution of these
+  instructions. Using hardware execution generally provides better
+  performance, but at the loss of ability to gather runtime statistics
+  about the use of the deprecated instructions.
+
+The default mode depends on the status of the instruction in the
+architecture. Deprecated instructions should default to emulation
+while obsolete instructions must be undefined by default.
+
+Supported legacy instructions
+-----------------------------
+* SWP{B}
+Node: /proc/sys/abi/swp
+Status: Obsolete
+Default: Undef (0)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9532f8d..2d59bfc 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -162,6 +162,45 @@ config ARCH_XGENE
 	help
 	  This enables support for AppliedMicro X-Gene SOC Family
 
+comment "Processor Features"
+
+menuconfig ARMV8_DEPRECATED
+	bool "Emulate deprecated/obsolete ARMv8 instructions"
+	depends on COMPAT
+	help
+	  Legacy software support may require certain instructions
+	  that have been deprecated or obsoleted in the architecture.
+
+	  Enable this config to enable selective emulation of these
+	  features.
+
+	  If unsure, say Y
+
+if ARMV8_DEPRECATED
+
+config SWP_EMULATION
+	bool "Emulate SWP/SWPB instructions"
+	help
+	  ARMv8 obsoletes the use of A32 SWP/SWPB instructions such that
+	  they are always undefined. Say Y here to enable software
+	  emulation of these instructions for userspace using LDXR/STXR.
+
+	  In some older versions of glibc [<=2.8] SWP is used during futex
+	  trylock() operations with the assumption that the code will not
+	  be preempted. This invalid assumption may be more likely to fail
+	  with SWP emulation enabled, leading to deadlock of the user
+	  application.
+
+	  NOTE: when accessing uncached shared regions, LDXR/STXR rely
+	  on an external transaction monitoring block called a global
+	  monitor to maintain update atomicity. If your system does not
+	  implement a global monitor, this option can cause programs that
+	  perform SWP operations to uncached memory to deadlock.
+
+	  If unsure, say Y
+
+endif
+
 endmenu
 
 menu "Bus support"
diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 1bb0430..3ecc57c 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -356,6 +356,12 @@ int aarch64_insn_patch_text_sync(void *addrs[], u32 insns[], int cnt);
 int aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt);
 
 bool aarch32_insn_is_wide(u32 insn);
+
+#define A32_RN_OFFSET	16
+#define A32_RT_OFFSET	12
+#define A32_RT2_OFFSET	 0
+
+u32 aarch32_insn_extract_reg_num(u32 insn, int offset);
 #endif /* __ASSEMBLY__ */
 
 #endif	/* __ASM_INSN_H */
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index a4d8671..84e9e51 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -32,6 +32,7 @@ arm64-obj-$(CONFIG_JUMP_LABEL)		+= jump_label.o
 arm64-obj-$(CONFIG_KGDB)		+= kgdb.o
 arm64-obj-$(CONFIG_EFI)			+= efi.o efi-stub.o efi-entry.o
 arm64-obj-$(CONFIG_PCI)			+= pci.o
+arm64-obj-$(CONFIG_ARMV8_DEPRECATED)	+= armv8_deprecated.o
 
 obj-y					+= $(arm64-obj-y) vdso/
 obj-m					+= $(arm64-obj-m)
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
new file mode 100644
index 0000000..bedf52c
--- /dev/null
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -0,0 +1,419 @@
+/*
+ *  Copyright (C) 2014 ARM Limited
+ *
+ * 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.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/perf_event.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/sysctl.h>
+
+#include <asm/insn.h>
+#include <asm/opcodes.h>
+#include <asm/system_misc.h>
+#include <asm/traps.h>
+#include <asm/uaccess.h>
+
+/*
+ * The runtime support for deprecated instruction support can be in one of
+ * following three states -
+ *
+ * 0 = undef
+ * 1 = emulate (software emulation)
+ * 2 = hw (supported in hardware)
+ */
+enum insn_emulation_mode {
+	INSN_UNDEF,
+	INSN_EMULATE,
+	INSN_HW,
+};
+
+enum legacy_insn_status {
+	INSN_DEPRECATED,
+	INSN_OBSOLETE,
+};
+
+struct insn_emulation_ops {
+	const char		*name;
+	enum legacy_insn_status	status;
+	struct undef_hook	*hooks;
+	int			(*set_hw_mode)(bool enable);
+};
+
+struct insn_emulation {
+	struct list_head node;
+	struct insn_emulation_ops *ops;
+	int current_mode;
+	int min;
+	int max;
+};
+
+/*
+ *  Implement emulation of the SWP/SWPB instructions using load-exclusive and
+ *  store-exclusive.
+ *
+ *  Syntax of SWP{B} instruction: SWP{B}<c> <Rt>, <Rt2>, [<Rn>]
+ *  Where: Rt  = destination
+ *	   Rt2 = source
+ *	   Rn  = address
+ */
+
+/*
+ * Error-checking SWP macros implemented using ldxr{b}/stxr{b}
+ */
+#define __user_swpX_asm(data, addr, res, temp, B)		\
+	__asm__ __volatile__(					\
+	"	mov		%w2, %w1\n"			\
+	"0:	ldxr"B"		%w1, [%3]\n"			\
+	"1:	stxr"B"		%w0, %w2, [%3]\n"		\
+	"	cbz		%w0, 2f\n"			\
+	"	mov		%w0, %w4\n"			\
+	"2:\n"							\
+	"	.pushsection	 .fixup,\"ax\"\n"		\
+	"	.align		2\n"				\
+	"3:	mov		%w0, %w5\n"			\
+	"	b		2b\n"				\
+	"	.popsection"					\
+	"	.pushsection	 __ex_table,\"a\"\n"		\
+	"	.align		3\n"				\
+	"	.quad		0b, 3b\n"			\
+	"	.quad		1b, 3b\n"			\
+	"	.popsection"					\
+	: "=&r" (res), "+r" (data), "=&r" (temp)		\
+	: "r" (addr), "i" (-EAGAIN), "i" (-EFAULT)		\
+	: "memory")
+
+#define __user_swp_asm(data, addr, res, temp) \
+	__user_swpX_asm(data, addr, res, temp, "")
+#define __user_swpb_asm(data, addr, res, temp) \
+	__user_swpX_asm(data, addr, res, temp, "b")
+
+/*
+ * Bit 22 of the instruction encoding distinguishes between
+ * the SWP and SWPB variants (bit set means SWPB).
+ */
+#define TYPE_SWPB (1 << 22)
+
+/*
+ * Set up process info to signal segmentation fault - called on access error.
+ */
+static void set_segfault(struct pt_regs *regs, unsigned long addr)
+{
+	siginfo_t info;
+
+	down_read(&current->mm->mmap_sem);
+	if (find_vma(current->mm, addr) == NULL)
+		info.si_code = SEGV_MAPERR;
+	else
+		info.si_code = SEGV_ACCERR;
+	up_read(&current->mm->mmap_sem);
+
+	info.si_signo = SIGSEGV;
+	info.si_errno = 0;
+	info.si_addr  = (void *) instruction_pointer(regs);
+
+	pr_debug("SWP{B} emulation: access caused memory abort!\n");
+	arm64_notify_die("Illegal memory access", regs, &info, 0);
+}
+
+static int emulate_swpX(unsigned int address, unsigned int *data,
+			unsigned int type)
+{
+	unsigned int res = 0;
+
+	if ((type != TYPE_SWPB) && (address & 0x3)) {
+		/* SWP to unaligned address not permitted */
+		pr_debug("SWP instruction on unaligned pointer!\n");
+		return -EFAULT;
+	}
+
+	while (1) {
+		unsigned long temp;
+
+		if (type == TYPE_SWPB)
+			__user_swpb_asm(*data, address, res, temp);
+		else
+			__user_swp_asm(*data, address, res, temp);
+
+		if (likely(res != -EAGAIN) || signal_pending(current))
+			break;
+
+		cond_resched();
+	}
+
+	return res;
+}
+
+/*
+ * swp_handler logs the id of calling process, dissects the instruction, sanity
+ * checks the memory location, calls emulate_swpX for the actual operation and
+ * deals with fixup/error handling before returning
+ */
+static int swp_handler(struct pt_regs *regs, u32 instr)
+{
+	u32 destreg, data, type, address = 0;
+	int rn, rt2, res = 0;
+
+	perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
+
+	type = instr & TYPE_SWPB;
+
+	switch (arm_check_condition(instr, regs->pstate)) {
+	case ARM_OPCODE_CONDTEST_PASS:
+		break;
+	case ARM_OPCODE_CONDTEST_FAIL:
+		/* Condition failed - return to next instruction */
+		goto ret;
+	case ARM_OPCODE_CONDTEST_UNCOND:
+		/* If unconditional encoding - not a SWP, undef */
+		return -EFAULT;
+	default:
+		return -EINVAL;
+	}
+
+	rn = aarch32_insn_extract_reg_num(instr, A32_RN_OFFSET);
+	rt2 = aarch32_insn_extract_reg_num(instr, A32_RT2_OFFSET);
+
+	address = (u32)regs->user_regs.regs[rn];
+	data	= (u32)regs->user_regs.regs[rt2];
+	destreg = aarch32_insn_extract_reg_num(instr, A32_RT_OFFSET);
+
+	pr_debug("addr in r%d->0x%08x, dest is r%d, source in r%d->0x%08x)\n",
+		rn, address, destreg,
+		aarch32_insn_extract_reg_num(instr, A32_RT2_OFFSET), data);
+
+	/* Check access in reasonable access range for both SWP and SWPB */
+	if (!access_ok(VERIFY_WRITE, (address & ~3), 4)) {
+		pr_debug("SWP{B} emulation: access to 0x%08x not allowed!\n",
+			address);
+		goto fault;
+	}
+
+	res = emulate_swpX(address, &data, type);
+	if (res == -EFAULT)
+		goto fault;
+	else if (res == 0)
+		regs->user_regs.regs[destreg] = data;
+
+ret:
+	pr_warn_ratelimited("\"%s\" (%ld) uses obsolete SWP{B} instruction at 0x%llx\n",
+			current->comm, (unsigned long)current->pid, regs->pc);
+
+	regs->pc += 4;
+	return 0;
+
+fault:
+	set_segfault(regs, address);
+
+	return 0;
+}
+
+/*
+ * Only emulate SWP/SWPB executed in ARM state/User mode.
+ * The kernel must be SWP free and SWP{B} does not exist in Thumb.
+ */
+static struct undef_hook swp_hooks[] = {
+		{
+			.instr_mask	= 0x0fb00ff0,
+			.instr_val	= 0x01000090,
+			.pstate_mask	= COMPAT_PSR_MODE_MASK,
+			.pstate_val	= COMPAT_PSR_MODE_USR,
+			.fn		= swp_handler
+		},
+		{ }
+};
+
+static struct insn_emulation_ops swp_ops = {
+	.name = "swp",
+	.status = INSN_OBSOLETE,
+	.hooks = swp_hooks,
+	.set_hw_mode = NULL,
+};
+
+static LIST_HEAD(insn_emulation);
+static int nr_insn_emulated;
+static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
+
+static bool register_emulation_hooks(struct insn_emulation_ops *ops)
+{
+	bool success = true;
+	struct undef_hook *hook;
+
+	if (!ops->hooks) {
+		success = false;
+		goto ret;
+	}
+
+	for (hook = ops->hooks; hook->instr_mask; hook++)
+		if (register_undef_hook(hook)) {
+			success = false;
+			break;
+		}
+
+ret:
+	return success;
+}
+
+static void remove_emulation_hooks(struct insn_emulation_ops *ops)
+{
+	struct undef_hook *hook;
+
+	if (!ops->hooks)
+		return;
+
+	for (hook = ops->hooks; hook->instr_mask; hook++)
+		unregister_undef_hook(hook);
+
+	pr_notice("Removed %s emulation handler\n", ops->name);
+}
+
+static int update_insn_emulation_mode(struct insn_emulation *insn,
+				       enum insn_emulation_mode prev)
+{
+	int ret = 0;
+
+	switch (prev) {
+	case INSN_UNDEF: /* Nothing to be done */
+		break;
+	case INSN_EMULATE:
+		remove_emulation_hooks(insn->ops);
+		break;
+	case INSN_HW:
+		if (insn->ops->set_hw_mode) {
+			insn->ops->set_hw_mode(false);
+			pr_notice("Disabled %s support\n", insn->ops->name);
+		}
+		break;
+	}
+
+	switch (insn->current_mode) {
+	case INSN_UNDEF:
+		break;
+	case INSN_EMULATE:
+		if (!register_emulation_hooks(insn->ops)) {
+			insn->current_mode = INSN_UNDEF;
+			ret = -EINVAL;
+		} else
+			pr_notice("Registered %s emulation handler\n",
+				  insn->ops->name);
+		break;
+	case INSN_HW:
+		if (insn->ops->set_hw_mode && insn->ops->set_hw_mode(true)) {
+			pr_notice("Enabled %s support\n", insn->ops->name);
+		} else {
+			insn->current_mode = INSN_UNDEF;
+			ret = -EINVAL;
+		}
+		break;
+	}
+
+	return ret;
+}
+
+static void register_insn_emulation(struct insn_emulation_ops *ops)
+{
+	unsigned long flags;
+	struct insn_emulation *insn;
+
+	insn = kzalloc(sizeof(*insn), GFP_KERNEL);
+	insn->ops = ops;
+	insn->min = INSN_UNDEF;
+
+	switch (ops->status) {
+	case INSN_DEPRECATED:
+		insn->current_mode = INSN_EMULATE;
+		insn->max = INSN_HW;
+		break;
+	case INSN_OBSOLETE:
+		insn->current_mode = INSN_UNDEF;
+		insn->max = INSN_EMULATE;
+		break;
+	}
+
+	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
+	list_add(&insn->node, &insn_emulation);
+	nr_insn_emulated++;
+	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+
+	/* Register any handlers if required */
+	update_insn_emulation_mode(insn, INSN_UNDEF);
+}
+
+static int emulation_proc_handler(struct ctl_table *table, int write,
+				  void __user *buffer, size_t *lenp,
+				  loff_t *ppos)
+{
+	int ret = 0;
+	struct insn_emulation *insn = (struct insn_emulation *) table->data;
+	enum insn_emulation_mode prev_mode = insn->current_mode;
+
+	table->data = &insn->current_mode;
+	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+
+	if (ret || !write || prev_mode == insn->current_mode)
+		goto ret;
+
+	ret = update_insn_emulation_mode(insn, prev_mode);
+ret:
+	table->data = insn;
+	return ret;
+}
+
+static struct ctl_table ctl_abi[] = {
+	{
+		.procname = "abi",
+		.mode = 0555,
+	},
+	{ }
+};
+
+static void register_insn_emulation_sysctl(struct ctl_table *table)
+{
+	unsigned long flags;
+	int i = 0;
+	struct insn_emulation *insn;
+	struct ctl_table *insns_sysctl, *sysctl;
+
+	insns_sysctl = kzalloc(sizeof(*sysctl) * (nr_insn_emulated + 1),
+			      GFP_KERNEL);
+
+	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
+	list_for_each_entry(insn, &insn_emulation, node) {
+		sysctl = &insns_sysctl[i];
+
+		sysctl->mode = 0644;
+		sysctl->maxlen = sizeof(int);
+
+		sysctl->procname = insn->ops->name;
+		sysctl->data = insn;
+		sysctl->extra1 = &insn->min;
+		sysctl->extra2 = &insn->max;
+		sysctl->proc_handler = emulation_proc_handler;
+		i++;
+	}
+	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+
+	table->child = insns_sysctl;
+	register_sysctl_table(table);
+}
+
+/*
+ * Invoked as late_initcall, since not needed before init spawned.
+ */
+static int __init armv8_deprecated_init(void)
+{
+	if (IS_ENABLED(CONFIG_SWP_EMULATION))
+		register_insn_emulation(&swp_ops);
+
+	register_insn_emulation_sysctl(ctl_abi);
+
+	return 0;
+}
+
+late_initcall(armv8_deprecated_init);
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index ab00eb5..63122dc 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -964,3 +964,11 @@ bool aarch32_insn_is_wide(u32 insn)
 {
 	return insn >= 0xe800;
 }
+
+/*
+ * Macros/defines for extracting register numbers from instruction.
+ */
+u32 aarch32_insn_extract_reg_num(u32 insn, int offset)
+{
+	return (insn & (0xf << offset)) >> offset;
+}
-- 
2.1.1

[PATCHv4 4/5] arm64: Emulate CP15 Barrier instructions

[Punit Agrawal]

The CP15 barrier instructions (CP15ISB, CP15DSB and CP15DMB) are
deprecated in the ARMv7 architecture, superseded by ISB, DSB and DMB
instructions respectively. Some implementations may provide the
ability to disable the CP15 barriers by disabling the CP15BEN bit in
SCTLR_EL1. If not enabled, the encodings for these instructions become
undefined.

To support legacy software using these instructions, this patch
register hooks to -
* emulate CP15 barriers and warn the user about their use
* toggle CP15BEN in SCTLR_EL1

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 Documentation/arm64/legacy_instructions.txt |   5 ++
 arch/arm64/Kconfig                          |  15 ++++
 arch/arm64/include/asm/insn.h               |   2 +
 arch/arm64/kernel/armv8_deprecated.c        | 131 ++++++++++++++++++++++++++++
 arch/arm64/kernel/insn.c                    |  13 +++
 5 files changed, 166 insertions(+)

diff --git a/Documentation/arm64/legacy_instructions.txt b/Documentation/arm64/legacy_instructions.txt
index 5ab5861..a3b3da2 100644
--- a/Documentation/arm64/legacy_instructions.txt
+++ b/Documentation/arm64/legacy_instructions.txt
@@ -38,3 +38,8 @@ Supported legacy instructions
 Node: /proc/sys/abi/swp
 Status: Obsolete
 Default: Undef (0)
+
+* CP15 Barriers
+Node: /proc/sys/abi/cp15_barrier
+Status: Deprecated
+Default: Emulate (1)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 2d59bfc..250ca25 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -199,6 +199,21 @@ config SWP_EMULATION
 
 	  If unsure, say Y
 
+config CP15_BARRIER_EMULATION
+	bool "Emulate CP15 Barrier instructions"
+	help
+	  The CP15 barrier instructions - CP15ISB, CP15DSB, and
+	  CP15DMB - are deprecated in ARMv8 (and ARMv7). It is
+	  strongly recommended to use the ISB, DSB, and DMB
+	  instructions instead.
+
+	  Say Y here to enable software emulation of these
+	  instructions for AArch32 userspace code. When this option is
+	  enabled, CP15 barrier usage is traced which can help
+	  identify software that needs updating.
+
+	  If unsure, say Y
+
 endif
 
 endmenu
diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 3ecc57c..e2ff32a 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -362,6 +362,8 @@ bool aarch32_insn_is_wide(u32 insn);
 #define A32_RT2_OFFSET	 0
 
 u32 aarch32_insn_extract_reg_num(u32 insn, int offset);
+u32 aarch32_insn_mcr_extract_opc2(u32 insn);
+u32 aarch32_insn_mcr_extract_crm(u32 insn);
 #endif /* __ASSEMBLY__ */
 
 #endif	/* __ASM_INSN_H */
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index bedf52c..248b222 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -6,6 +6,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/cpu.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
@@ -236,6 +237,133 @@ static struct insn_emulation_ops swp_ops = {
 	.set_hw_mode = NULL,
 };
 
+static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
+{
+	perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
+
+	switch (arm_check_condition(instr, regs->pstate)) {
+	case ARM_OPCODE_CONDTEST_PASS:
+		break;
+	case ARM_OPCODE_CONDTEST_FAIL:
+		/* Condition failed - return to next instruction */
+		goto ret;
+	case ARM_OPCODE_CONDTEST_UNCOND:
+		/* If unconditional encoding - not a barrier instruction */
+		return -EFAULT;
+	default:
+		return -EINVAL;
+	}
+
+	switch (aarch32_insn_mcr_extract_crm(instr)) {
+	case 10:
+		/*
+		 * dmb - mcr p15, 0, Rt, c7, c10, 5
+		 * dsb - mcr p15, 0, Rt, c7, c10, 4
+		 */
+		if (aarch32_insn_mcr_extract_opc2(instr) == 5)
+			dmb(sy);
+		else
+			dsb(sy);
+		break;
+	case 5:
+		/*
+		 * isb - mcr p15, 0, Rt, c7, c5, 4
+		 *
+		 * Taking an exception or returning from one acts as an
+		 * instruction barrier. So no explicit barrier needed here.
+		 */
+		break;
+	}
+
+ret:
+	pr_warn_ratelimited("\"%s\" (%ld) uses deprecated CP15 Barrier instruction at 0x%llx\n",
+			current->comm, (unsigned long)current->pid, regs->pc);
+
+	regs->pc += 4;
+	return 0;
+}
+
+#define SCTLR_EL1_CP15BEN (1 << 5)
+
+static inline void config_sctlr_el1(u32 clear, u32 set)
+{
+	u32 val;
+
+	asm volatile("mrs %0, sctlr_el1" : "=r" (val));
+	val &= ~clear;
+	val |= set;
+	asm volatile("msr sctlr_el1, %0" : : "r" (val));
+}
+
+static void enable_cp15_ben(void *info)
+{
+	config_sctlr_el1(0, SCTLR_EL1_CP15BEN);
+}
+
+static void disable_cp15_ben(void *info)
+{
+	config_sctlr_el1(SCTLR_EL1_CP15BEN, 0);
+}
+
+static int cpu_hotplug_notify(struct notifier_block *b,
+			      unsigned long action, void *hcpu)
+{
+	switch (action) {
+	case CPU_STARTING:
+	case CPU_STARTING_FROZEN:
+		enable_cp15_ben(NULL);
+		return NOTIFY_DONE;
+	case CPU_DYING:
+	case CPU_DYING_FROZEN:
+		disable_cp15_ben(NULL);
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block cpu_hotplug_notifier = {
+	.notifier_call = cpu_hotplug_notify,
+};
+
+static int cp15_barrier_set_hw_mode(bool enable)
+{
+	if (enable) {
+		register_cpu_notifier(&cpu_hotplug_notifier);
+		on_each_cpu(enable_cp15_ben, NULL, true);
+	} else {
+		unregister_cpu_notifier(&cpu_hotplug_notifier);
+		on_each_cpu(disable_cp15_ben, NULL, true);
+	}
+
+	return true;
+}
+
+static struct undef_hook cp15_barrier_hooks[] = {
+	{
+		.instr_mask	= 0x0fff0fdf,
+		.instr_val	= 0x0e070f9a,
+		.pstate_mask	= COMPAT_PSR_MODE_MASK,
+		.pstate_val	= COMPAT_PSR_MODE_USR,
+		.fn		= cp15barrier_handler,
+	},
+	{
+		.instr_mask	= 0x0fff0fff,
+		.instr_val	= 0x0e070f95,
+		.pstate_mask	= COMPAT_PSR_MODE_MASK,
+		.pstate_val	= COMPAT_PSR_MODE_USR,
+		.fn		= cp15barrier_handler,
+	},
+	{ }
+};
+
+static struct insn_emulation_ops cp15_barrier_ops = {
+	.name = "cp15_barrier",
+	.status = INSN_DEPRECATED,
+	.hooks = cp15_barrier_hooks,
+	.set_hw_mode = cp15_barrier_set_hw_mode,
+};
+
 static LIST_HEAD(insn_emulation);
 static int nr_insn_emulated;
 static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
@@ -411,6 +539,9 @@ static int __init armv8_deprecated_init(void)
 	if (IS_ENABLED(CONFIG_SWP_EMULATION))
 		register_insn_emulation(&swp_ops);
 
+	if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
+		register_insn_emulation(&cp15_barrier_ops);
+
 	register_insn_emulation_sysctl(ctl_abi);
 
 	return 0;
diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
index 63122dc..819e409 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -972,3 +972,16 @@ u32 aarch32_insn_extract_reg_num(u32 insn, int offset)
 {
 	return (insn & (0xf << offset)) >> offset;
 }
+
+#define OPC2_MASK	0x7
+#define OPC2_OFFSET	5
+u32 aarch32_insn_mcr_extract_opc2(u32 insn)
+{
+	return (insn & (OPC2_MASK << OPC2_OFFSET)) >> OPC2_OFFSET;
+}
+
+#define CRM_MASK	0xf
+u32 aarch32_insn_mcr_extract_crm(u32 insn)
+{
+	return insn & CRM_MASK;
+}
-- 
2.1.1

[PATCHv4 5/5] arm64: Trace emulation of AArch32 legacy instructions

[Punit Agrawal]

Introduce an event to trace the usage of emulated instructions. The
trace event is intended to help identify and encourage the migration
of legacy software using the emulation features.

Use this event to trace usage of swp and CP15 barrier emulation.

Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 arch/arm64/kernel/Makefile                 |  1 +
 arch/arm64/kernel/armv8_deprecated.c       | 19 ++++++++++++--
 arch/arm64/kernel/trace-events-emulation.h | 40 ++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm64/kernel/trace-events-emulation.h

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 84e9e51..b36ebd0 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -5,6 +5,7 @@
 CPPFLAGS_vmlinux.lds	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 AFLAGS_head.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 CFLAGS_efi-stub.o 	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
+CFLAGS_armv8_deprecated.o := -I$(src)
 
 CFLAGS_REMOVE_ftrace.o = -pg
 CFLAGS_REMOVE_insn.o = -pg
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 248b222..62d0672 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -15,6 +15,9 @@
 #include <linux/slab.h>
 #include <linux/sysctl.h>
 
+#define CREATE_TRACE_POINTS
+#include "trace-events-emulation.h"
+
 #include <asm/insn.h>
 #include <asm/opcodes.h>
 #include <asm/system_misc.h>
@@ -203,6 +206,11 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
 		regs->user_regs.regs[destreg] = data;
 
 ret:
+	if (type == TYPE_SWPB)
+		trace_instruction_emulation("swpb", regs->pc);
+	else
+		trace_instruction_emulation("swp", regs->pc);
+
 	pr_warn_ratelimited("\"%s\" (%ld) uses obsolete SWP{B} instruction at 0x%llx\n",
 			current->comm, (unsigned long)current->pid, regs->pc);
 
@@ -260,10 +268,15 @@ static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
 		 * dmb - mcr p15, 0, Rt, c7, c10, 5
 		 * dsb - mcr p15, 0, Rt, c7, c10, 4
 		 */
-		if (aarch32_insn_mcr_extract_opc2(instr) == 5)
+		if (aarch32_insn_mcr_extract_opc2(instr) == 5) {
 			dmb(sy);
-		else
+			trace_instruction_emulation(
+				"mcr p15, 0, Rt, c7, c10, 5 ; dmb", regs->pc);
+		} else {
 			dsb(sy);
+			trace_instruction_emulation(
+				"mcr p15, 0, Rt, c7, c10, 4 ; dsb", regs->pc);
+		}
 		break;
 	case 5:
 		/*
@@ -272,6 +285,8 @@ static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
 		 * Taking an exception or returning from one acts as an
 		 * instruction barrier. So no explicit barrier needed here.
 		 */
+		trace_instruction_emulation(
+			"mcr p15, 0, Rt, c7, c5, 4 ; isb", regs->pc);
 		break;
 	}
 
diff --git a/arch/arm64/kernel/trace-events-emulation.h b/arch/arm64/kernel/trace-events-emulation.h
new file mode 100644
index 0000000..2ba3882
--- /dev/null
+++ b/arch/arm64/kernel/trace-events-emulation.h
@@ -0,0 +1,40 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM emulation
+
+#if !defined(_TRACE_EMULATION_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_EMULATION_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(instruction_emulation,
+
+	TP_PROTO(const char *instr, u64 addr),
+	TP_ARGS(instr, addr),
+
+	TP_STRUCT__entry(
+		__array(char, comm, TASK_COMM_LEN)
+		__field(pid_t, pid)
+		__string(instr, instr)
+		__field(u64, addr)
+	),
+
+	TP_fast_assign(
+		memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
+		__entry->pid = current->pid;
+		__assign_str(instr, instr);
+		__entry->addr = addr;
+	),
+
+	TP_printk("instr=\"%s\" comm=%s pid=%d addr=0x%llx", __get_str(instr),
+		__entry->comm, __entry->pid, __entry->addr)
+);
+
+#endif /* _TRACE_EMULATION_H */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_PATH .
+
+#define TRACE_INCLUDE_FILE trace-events-emulation
+#include <trace/define_trace.h>
-- 
2.1.1

[PATCHv4 1/5] arm64: Add support for hooks to handle undefined instructions

[Catalin Marinas]

On Wed, Nov 12, 2014 at 11:44:48AM +0000, Punit Agrawal wrote:
> +int register_undef_hook(struct undef_hook *hook)
> +{
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&undef_lock, flags);
> +	list_add(&hook->node, &undef_hook);
> +	raw_spin_unlock_irqrestore(&undef_lock, flags);
> +
> +	return 0;
> +}

I missed this. Why do we need this function to return something? I don't
see how it would fail.

-- 
Catalin

[PATCHv4 1/5] arm64: Add support for hooks to handle undefined instructions

[Punit Agrawal]

Hi Catalin,

Catalin Marinas <catalin.marinas@arm.com> writes:

> On Wed, Nov 12, 2014 at 11:44:48AM +0000, Punit Agrawal wrote:
>> +int register_undef_hook(struct undef_hook *hook)
>> +{
>> +	unsigned long flags;
>> +
>> +	raw_spin_lock_irqsave(&undef_lock, flags);
>> +	list_add(&hook->node, &undef_hook);
>> +	raw_spin_unlock_irqrestore(&undef_lock, flags);
>> +
>> +	return 0;
>> +}
>
> I missed this. Why do we need this function to return something? I don't
> see how it would fail.

You're right. I don't remember why I changed the return type when
porting this over from arm. I'll update this and affected code.

Cheers,
Punit

[PATCHv4 3/5] arm64: Port SWP/SWPB emulation support from arm

[Catalin Marinas]

On Wed, Nov 12, 2014 at 11:44:50AM +0000, Punit Agrawal wrote:
>  Documentation/arm64/legacy_instructions.txt |  40 +++
>  arch/arm64/Kconfig                          |  39 +++
>  arch/arm64/include/asm/insn.h               |   6 +
>  arch/arm64/kernel/Makefile                  |   1 +
>  arch/arm64/kernel/armv8_deprecated.c        | 419 ++++++++++++++++++++++++++++
>  arch/arm64/kernel/insn.c                    |   8 +

I would have split this in two patches, one adding the framework and
another the emulation. I'm not too bothered (can leave them as a single
patch) but I have some comments below for grouping the code a bit as it
looks like it's a mix of API for emulation, swp emulation, the API again
making it hard to follow.

> --- /dev/null
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -0,0 +1,419 @@
> +/*
> + *  Copyright (C) 2014 ARM Limited
> + *
> + * 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.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/perf_event.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/sysctl.h>
> +
> +#include <asm/insn.h>
> +#include <asm/opcodes.h>
> +#include <asm/system_misc.h>
> +#include <asm/traps.h>
> +#include <asm/uaccess.h>
> +
> +/*
> + * The runtime support for deprecated instruction support can be in one of
> + * following three states -
> + *
> + * 0 = undef
> + * 1 = emulate (software emulation)
> + * 2 = hw (supported in hardware)
> + */
> +enum insn_emulation_mode {
> +       INSN_UNDEF,
> +       INSN_EMULATE,
> +       INSN_HW,
> +};
> +
> +enum legacy_insn_status {
> +       INSN_DEPRECATED,
> +       INSN_OBSOLETE,
> +};
> +
> +struct insn_emulation_ops {
> +       const char              *name;
> +       enum legacy_insn_status status;
> +       struct undef_hook       *hooks;
> +       int                     (*set_hw_mode)(bool enable);
> +};
> +
> +struct insn_emulation {
> +       struct list_head node;
> +       struct insn_emulation_ops *ops;
> +       int current_mode;
> +       int min;
> +       int max;
> +};

Please move the register_* etc. functions here and let the swp emulation
grouped together.

> +/*
> + * Only emulate SWP/SWPB executed in ARM state/User mode.
> + * The kernel must be SWP free and SWP{B} does not exist in Thumb.
> + */
> +static struct undef_hook swp_hooks[] = {
> +               {
> +                       .instr_mask     = 0x0fb00ff0,
> +                       .instr_val      = 0x01000090,
> +                       .pstate_mask    = COMPAT_PSR_MODE_MASK,
> +                       .pstate_val     = COMPAT_PSR_MODE_USR,
> +                       .fn             = swp_handler
> +               },
> +               { }
> +};

It may be the email client but it looks like too much indentation here.

> +
> +static struct insn_emulation_ops swp_ops = {
> +       .name = "swp",
> +       .status = INSN_OBSOLETE,
> +       .hooks = swp_hooks,
> +       .set_hw_mode = NULL,
> +};
> +

Code below up to armv8_deprecated_init should be moved earlier.

> +static LIST_HEAD(insn_emulation);
> +static int nr_insn_emulated;
> +static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
> +
> +static bool register_emulation_hooks(struct insn_emulation_ops *ops)
> +{
> +       bool success = true;
> +       struct undef_hook *hook;
> +
> +       if (!ops->hooks) {
> +               success = false;
> +               goto ret;
> +       }
> +
> +       for (hook = ops->hooks; hook->instr_mask; hook++)
> +               if (register_undef_hook(hook)) {
> +                       success = false;
> +                       break;
> +               }
> +
> +ret:
> +       return success;
> +}

I don't really think this code needs so much checking and error
returning. As I already commented, register_undef_hook() should not
return anything and for the ops->hooks check I think we could just use
BUG_ON(!ops->hooks).

Do we need to add support for instructions that can't be emulated? I
wouldn't go there yet until we see some real use-case.

> +static void remove_emulation_hooks(struct insn_emulation_ops *ops)
> +{
> +       struct undef_hook *hook;
> +
> +       if (!ops->hooks)
> +               return;

Same here.

> +
> +       for (hook = ops->hooks; hook->instr_mask; hook++)
> +               unregister_undef_hook(hook);
> +
> +       pr_notice("Removed %s emulation handler\n", ops->name);
> +}
> +
> +static int update_insn_emulation_mode(struct insn_emulation *insn,
> +                                      enum insn_emulation_mode prev)
> +{
> +       int ret = 0;
> +
> +       switch (prev) {
> +       case INSN_UNDEF: /* Nothing to be done */
> +               break;
> +       case INSN_EMULATE:
> +               remove_emulation_hooks(insn->ops);
> +               break;
> +       case INSN_HW:
> +               if (insn->ops->set_hw_mode) {
> +                       insn->ops->set_hw_mode(false);
> +                       pr_notice("Disabled %s support\n", insn->ops->name);
> +               }
> +               break;
> +       }
> +
> +       switch (insn->current_mode) {
> +       case INSN_UNDEF:
> +               break;
> +       case INSN_EMULATE:
> +               if (!register_emulation_hooks(insn->ops)) {
> +                       insn->current_mode = INSN_UNDEF;
> +                       ret = -EINVAL;
> +               } else
> +                       pr_notice("Registered %s emulation handler\n",
> +                                 insn->ops->name);
> +               break;

If we assume that register_emulation_hooks() doesn't fail, we could
simplify slightly here.

> +       case INSN_HW:
> +               if (insn->ops->set_hw_mode && insn->ops->set_hw_mode(true)) {
> +                       pr_notice("Enabled %s support\n", insn->ops->name);
> +               } else {
> +                       insn->current_mode = INSN_UNDEF;
> +                       ret = -EINVAL;
> +               }
> +               break;

I would rather let it fall back to the previous. So in the caller, we
could re-instate the previous mode rather than undef.

> +static void register_insn_emulation(struct insn_emulation_ops *ops)
> +{
> +       unsigned long flags;
> +       struct insn_emulation *insn;
> +
> +       insn = kzalloc(sizeof(*insn), GFP_KERNEL);
> +       insn->ops = ops;
> +       insn->min = INSN_UNDEF;
> +
> +       switch (ops->status) {
> +       case INSN_DEPRECATED:
> +               insn->current_mode = INSN_EMULATE;
> +               insn->max = INSN_HW;
> +               break;
> +       case INSN_OBSOLETE:
> +               insn->current_mode = INSN_UNDEF;
> +               insn->max = INSN_EMULATE;
> +               break;
> +       }
> +
> +       raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> +       list_add(&insn->node, &insn_emulation);
> +       nr_insn_emulated++;
> +       raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
> +
> +       /* Register any handlers if required */
> +       update_insn_emulation_mode(insn, INSN_UNDEF);
> +}
> +
> +static int emulation_proc_handler(struct ctl_table *table, int write,
> +                                 void __user *buffer, size_t *lenp,
> +                                 loff_t *ppos)
> +{
> +       int ret = 0;
> +       struct insn_emulation *insn = (struct insn_emulation *) table->data;
> +       enum insn_emulation_mode prev_mode = insn->current_mode;
> +
> +       table->data = &insn->current_mode;
> +       ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> +
> +       if (ret || !write || prev_mode == insn->current_mode)
> +               goto ret;
> +
> +       ret = update_insn_emulation_mode(insn, prev_mode);

After this instruction, something like below to restore the previous
mode:

	if (!ret) {
		enum insn_emulation mode = insn_mode->current_mode;
		insn->current_mode = prev_mode;
		update_insn_emulation_mode(insn, prev_mode);
	}

--
Catalin

[PATCHv4 4/5] arm64: Emulate CP15 Barrier instructions

[Catalin Marinas]

On Wed, Nov 12, 2014 at 11:44:51AM +0000, Punit Agrawal wrote:
> The CP15 barrier instructions (CP15ISB, CP15DSB and CP15DMB) are
> deprecated in the ARMv7 architecture, superseded by ISB, DSB and DMB
> instructions respectively. Some implementations may provide the
> ability to disable the CP15 barriers by disabling the CP15BEN bit in
> SCTLR_EL1. If not enabled, the encodings for these instructions become
> undefined.
> 
> To support legacy software using these instructions, this patch
> register hooks to -
> * emulate CP15 barriers and warn the user about their use
> * toggle CP15BEN in SCTLR_EL1
> 
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCHv4 5/5] arm64: Trace emulation of AArch32 legacy instructions

[Catalin Marinas]

On Wed, Nov 12, 2014 at 11:44:52AM +0000, Punit Agrawal wrote:
> Introduce an event to trace the usage of emulated instructions. The
> trace event is intended to help identify and encourage the migration
> of legacy software using the emulation features.
> 
> Use this event to trace usage of swp and CP15 barrier emulation.
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>

I can't comment much on trace stuff but I'm ok with the patch:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCHv4 5/5] arm64: Trace emulation of AArch32 legacy instructions

[Steven Rostedt]

On Wed, 12 Nov 2014 11:44:52 +0000
Punit Agrawal <punit.agrawal@arm.com> wrote:

> Introduce an event to trace the usage of emulated instructions. The
> trace event is intended to help identify and encourage the migration
> of legacy software using the emulation features.
> 
> Use this event to trace usage of swp and CP15 barrier emulation.
> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
>  arch/arm64/kernel/Makefile                 |  1 +
>  arch/arm64/kernel/armv8_deprecated.c       | 19 ++++++++++++--
>  arch/arm64/kernel/trace-events-emulation.h | 40 ++++++++++++++++++++++++++++++
>  3 files changed, 58 insertions(+), 2 deletions(-)
>  create mode 100644 arch/arm64/kernel/trace-events-emulation.h
> 
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 84e9e51..b36ebd0 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -5,6 +5,7 @@
>  CPPFLAGS_vmlinux.lds	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>  AFLAGS_head.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>  CFLAGS_efi-stub.o 	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
> +CFLAGS_armv8_deprecated.o := -I$(src)
>  
>  CFLAGS_REMOVE_ftrace.o = -pg
>  CFLAGS_REMOVE_insn.o = -pg
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index 248b222..62d0672 100644
> --- a/arch/arm64/kernel/armv8_deprecated.c
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -15,6 +15,9 @@
>  #include <linux/slab.h>
>  #include <linux/sysctl.h>
>  
> +#define CREATE_TRACE_POINTS
> +#include "trace-events-emulation.h"
> +

Note, if possible, please put this include after all other includes.
Sometimes a header file may include a trace events file and the
CREATE_TRACE_POINTS define might cause issues.

I'm sure it's fine now, but we don't want to debug this if someone does
it in the future.


>  #include <asm/insn.h>
>  #include <asm/opcodes.h>
>  #include <asm/system_misc.h>
> @@ -203,6 +206,11 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
>  		regs->user_regs.regs[destreg] = data;
>  
>  ret:
> +	if (type == TYPE_SWPB)
> +		trace_instruction_emulation("swpb", regs->pc);
> +	else
> +		trace_instruction_emulation("swp", regs->pc);
> +

I don't know how critical a path the above is, but if you want to micro
optimize this, you can make it so that the branch conditional only does
the test if the tracepoint is enabled.

Now, this makes a difference if jump labels are implemented in arm64 or
not.

	if (trace_instruction_emulation_enabled()) {
		if (type == TYPE_SWPB)
			...
		else
			...
	}

That trace_instruction_emulation_enabled() is a static key (jump label)
that can turn the if into a unlikely "nop", making the code within it
in the very slow path.


>  	pr_warn_ratelimited("\"%s\" (%ld) uses obsolete SWP{B} instruction at 0x%llx\n",
>  			current->comm, (unsigned long)current->pid, regs->pc);
>  
> @@ -260,10 +268,15 @@ static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
>  		 * dmb - mcr p15, 0, Rt, c7, c10, 5
>  		 * dsb - mcr p15, 0, Rt, c7, c10, 4
>  		 */
> -		if (aarch32_insn_mcr_extract_opc2(instr) == 5)
> +		if (aarch32_insn_mcr_extract_opc2(instr) == 5) {
>  			dmb(sy);
> -		else
> +			trace_instruction_emulation(
> +				"mcr p15, 0, Rt, c7, c10, 5 ; dmb", regs->pc);
> +		} else {
>  			dsb(sy);
> +			trace_instruction_emulation(
> +				"mcr p15, 0, Rt, c7, c10, 4 ; dsb", regs->pc);
> +		}
>  		break;
>  	case 5:
>  		/*
> @@ -272,6 +285,8 @@ static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
>  		 * Taking an exception or returning from one acts as an
>  		 * instruction barrier. So no explicit barrier needed here.
>  		 */
> +		trace_instruction_emulation(
> +			"mcr p15, 0, Rt, c7, c5, 4 ; isb", regs->pc);
>  		break;
>  	}
>  
> diff --git a/arch/arm64/kernel/trace-events-emulation.h b/arch/arm64/kernel/trace-events-emulation.h
> new file mode 100644
> index 0000000..2ba3882
> --- /dev/null
> +++ b/arch/arm64/kernel/trace-events-emulation.h
> @@ -0,0 +1,40 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM emulation
> +
> +#if !defined(_TRACE_EMULATION_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_EMULATION_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(instruction_emulation,
> +
> +	TP_PROTO(const char *instr, u64 addr),
> +	TP_ARGS(instr, addr),
> +
> +	TP_STRUCT__entry(
> +		__array(char, comm, TASK_COMM_LEN)

Why do you save the comm? It gets saved by the tracer (most of the
time, but we got better at saving it).

> +		__field(pid_t, pid)

Same here. The pid is already saved by the tracing infrastructure (the
comm is stored in a table that matches pids with comms to which
explains my comment above).


-- Steve

> +		__string(instr, instr)
> +		__field(u64, addr)
> +	),
> +
> +	TP_fast_assign(
> +		memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
> +		__entry->pid = current->pid;
> +		__assign_str(instr, instr);
> +		__entry->addr = addr;
> +	),
> +
> +	TP_printk("instr=\"%s\" comm=%s pid=%d addr=0x%llx", __get_str(instr),
> +		__entry->comm, __entry->pid, __entry->addr)
> +);
> +
> +#endif /* _TRACE_EMULATION_H */
> +
> +/* This part must be outside protection */
> +#undef TRACE_INCLUDE_PATH
> +#undef TRACE_INCLUDE_FILE
> +#define TRACE_INCLUDE_PATH .
> +
> +#define TRACE_INCLUDE_FILE trace-events-emulation
> +#include <trace/define_trace.h>

[PATCHv4 5/5] arm64: Trace emulation of AArch32 legacy instructions

[Punit Agrawal]

Hi Steven,

Thanks for taking a look.

Steven Rostedt <rostedt@goodmis.org> writes:

> On Wed, 12 Nov 2014 11:44:52 +0000
> Punit Agrawal <punit.agrawal@arm.com> wrote:
>
>> Introduce an event to trace the usage of emulated instructions. The
>> trace event is intended to help identify and encourage the migration
>> of legacy software using the emulation features.
>> 
>> Use this event to trace usage of swp and CP15 barrier emulation.
>> 
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
>> ---
>>  arch/arm64/kernel/Makefile                 |  1 +
>>  arch/arm64/kernel/armv8_deprecated.c       | 19 ++++++++++++--
>>  arch/arm64/kernel/trace-events-emulation.h | 40 ++++++++++++++++++++++++++++++
>>  3 files changed, 58 insertions(+), 2 deletions(-)
>>  create mode 100644 arch/arm64/kernel/trace-events-emulation.h
>> 
>> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
>> index 84e9e51..b36ebd0 100644
>> --- a/arch/arm64/kernel/Makefile
>> +++ b/arch/arm64/kernel/Makefile
>> @@ -5,6 +5,7 @@
>>  CPPFLAGS_vmlinux.lds	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>>  AFLAGS_head.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>>  CFLAGS_efi-stub.o 	:= -DTEXT_OFFSET=$(TEXT_OFFSET)
>> +CFLAGS_armv8_deprecated.o := -I$(src)
>>  
>>  CFLAGS_REMOVE_ftrace.o = -pg
>>  CFLAGS_REMOVE_insn.o = -pg
>> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
>> index 248b222..62d0672 100644
>> --- a/arch/arm64/kernel/armv8_deprecated.c
>> +++ b/arch/arm64/kernel/armv8_deprecated.c
>> @@ -15,6 +15,9 @@
>>  #include <linux/slab.h>
>>  #include <linux/sysctl.h>
>>  
>> +#define CREATE_TRACE_POINTS
>> +#include "trace-events-emulation.h"
>> +
>
> Note, if possible, please put this include after all other includes.
> Sometimes a header file may include a trace events file and the
> CREATE_TRACE_POINTS define might cause issues.
>
> I'm sure it's fine now, but we don't want to debug this if someone does
> it in the future.

Makes sense. I've moved the include to the end of the list.

>
>
>>  #include <asm/insn.h>
>>  #include <asm/opcodes.h>
>>  #include <asm/system_misc.h>
>> @@ -203,6 +206,11 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
>>  		regs->user_regs.regs[destreg] = data;
>>  
>>  ret:
>> +	if (type == TYPE_SWPB)
>> +		trace_instruction_emulation("swpb", regs->pc);
>> +	else
>> +		trace_instruction_emulation("swp", regs->pc);
>> +
>
> I don't know how critical a path the above is, but if you want to micro
> optimize this, you can make it so that the branch conditional only does
> the test if the tracepoint is enabled.
>
> Now, this makes a difference if jump labels are implemented in arm64 or
> not.
>
> 	if (trace_instruction_emulation_enabled()) {
> 		if (type == TYPE_SWPB)
> 			...
> 		else
> 			...
> 	}
>
> That trace_instruction_emulation_enabled() is a static key (jump label)
> that can turn the if into a unlikely "nop", making the code within it
> in the very slow path.

I don't think the path above is a fast path - SWP{B} emulation will be
slow as it is. Unless anybody thinks otherwise, I will leave the above
micro-optimisation out.

>
>
>>  	pr_warn_ratelimited("\"%s\" (%ld) uses obsolete SWP{B} instruction at 0x%llx\n",
>>  			current->comm, (unsigned long)current->pid, regs->pc);
>>  
>> @@ -260,10 +268,15 @@ static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
>>  		 * dmb - mcr p15, 0, Rt, c7, c10, 5
>>  		 * dsb - mcr p15, 0, Rt, c7, c10, 4
>>  		 */
>> -		if (aarch32_insn_mcr_extract_opc2(instr) == 5)
>> +		if (aarch32_insn_mcr_extract_opc2(instr) == 5) {
>>  			dmb(sy);
>> -		else
>> +			trace_instruction_emulation(
>> +				"mcr p15, 0, Rt, c7, c10, 5 ; dmb", regs->pc);
>> +		} else {
>>  			dsb(sy);
>> +			trace_instruction_emulation(
>> +				"mcr p15, 0, Rt, c7, c10, 4 ; dsb", regs->pc);
>> +		}
>>  		break;
>>  	case 5:
>>  		/*
>> @@ -272,6 +285,8 @@ static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
>>  		 * Taking an exception or returning from one acts as an
>>  		 * instruction barrier. So no explicit barrier needed here.
>>  		 */
>> +		trace_instruction_emulation(
>> +			"mcr p15, 0, Rt, c7, c5, 4 ; isb", regs->pc);
>>  		break;
>>  	}
>>  
>> diff --git a/arch/arm64/kernel/trace-events-emulation.h b/arch/arm64/kernel/trace-events-emulation.h
>> new file mode 100644
>> index 0000000..2ba3882
>> --- /dev/null
>> +++ b/arch/arm64/kernel/trace-events-emulation.h
>> @@ -0,0 +1,40 @@
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM emulation
>> +
>> +#if !defined(_TRACE_EMULATION_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_EMULATION_H
>> +
>> +#include <linux/tracepoint.h>
>> +
>> +TRACE_EVENT(instruction_emulation,
>> +
>> +	TP_PROTO(const char *instr, u64 addr),
>> +	TP_ARGS(instr, addr),
>> +
>> +	TP_STRUCT__entry(
>> +		__array(char, comm, TASK_COMM_LEN)
>
> Why do you save the comm? It gets saved by the tracer (most of the
> time, but we got better at saving it).
>
>> +		__field(pid_t, pid)
>
> Same here. The pid is already saved by the tracing infrastructure (the
> comm is stored in a table that matches pids with comms to which
> explains my comment above).

Good point. I'd forgotten about the comm and pid already reported by the
tracing infrastructure. The emulation should typically occur in
application context and should be accurate enough. I'll zap their
explict usage here.

Cheers,
Punit

>
>
> -- Steve
>
>> +		__string(instr, instr)
>> +		__field(u64, addr)
>> +	),
>> +
>> +	TP_fast_assign(
>> +		memcpy(__entry->comm, current->comm, TASK_COMM_LEN);
>> +		__entry->pid = current->pid;
>> +		__assign_str(instr, instr);
>> +		__entry->addr = addr;
>> +	),
>> +
>> +	TP_printk("instr=\"%s\" comm=%s pid=%d addr=0x%llx", __get_str(instr),
>> +		__entry->comm, __entry->pid, __entry->addr)
>> +);
>> +
>> +#endif /* _TRACE_EMULATION_H */
>> +
>> +/* This part must be outside protection */
>> +#undef TRACE_INCLUDE_PATH
>> +#undef TRACE_INCLUDE_FILE
>> +#define TRACE_INCLUDE_PATH .
>> +
>> +#define TRACE_INCLUDE_FILE trace-events-emulation
>> +#include <trace/define_trace.h>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv4 3/5] arm64: Port SWP/SWPB emulation support from arm

[Punit Agrawal]

Hi Catalin,

Thanks for reviewing the series.

Catalin Marinas <catalin.marinas@arm.com> writes:

> On Wed, Nov 12, 2014 at 11:44:50AM +0000, Punit Agrawal wrote:
>>  Documentation/arm64/legacy_instructions.txt |  40 +++
>>  arch/arm64/Kconfig                          |  39 +++
>>  arch/arm64/include/asm/insn.h               |   6 +
>>  arch/arm64/kernel/Makefile                  |   1 +
>>  arch/arm64/kernel/armv8_deprecated.c        | 419 ++++++++++++++++++++++++++++
>>  arch/arm64/kernel/insn.c                    |   8 +
>
> I would have split this in two patches, one adding the framework and
> another the emulation. I'm not too bothered (can leave them as a single
> patch) but I have some comments below for grouping the code a bit as it
> looks like it's a mix of API for emulation, swp emulation, the API again
> making it hard to follow.

I will split them out. It's easier to follow.

>
>> --- /dev/null
>> +++ b/arch/arm64/kernel/armv8_deprecated.c
>> @@ -0,0 +1,419 @@
>> +/*
>> + *  Copyright (C) 2014 ARM Limited
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/list.h>
>> +#include <linux/perf_event.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/sysctl.h>
>> +
>> +#include <asm/insn.h>
>> +#include <asm/opcodes.h>
>> +#include <asm/system_misc.h>
>> +#include <asm/traps.h>
>> +#include <asm/uaccess.h>
>> +
>> +/*
>> + * The runtime support for deprecated instruction support can be in one of
>> + * following three states -
>> + *
>> + * 0 = undef
>> + * 1 = emulate (software emulation)
>> + * 2 = hw (supported in hardware)
>> + */
>> +enum insn_emulation_mode {
>> +       INSN_UNDEF,
>> +       INSN_EMULATE,
>> +       INSN_HW,
>> +};
>> +
>> +enum legacy_insn_status {
>> +       INSN_DEPRECATED,
>> +       INSN_OBSOLETE,
>> +};
>> +
>> +struct insn_emulation_ops {
>> +       const char              *name;
>> +       enum legacy_insn_status status;
>> +       struct undef_hook       *hooks;
>> +       int                     (*set_hw_mode)(bool enable);
>> +};
>> +
>> +struct insn_emulation {
>> +       struct list_head node;
>> +       struct insn_emulation_ops *ops;
>> +       int current_mode;
>> +       int min;
>> +       int max;
>> +};
>
> Please move the register_* etc. functions here and let the swp emulation
> grouped together.
>

Done.

>> +/*
>> + * Only emulate SWP/SWPB executed in ARM state/User mode.
>> + * The kernel must be SWP free and SWP{B} does not exist in Thumb.
>> + */
>> +static struct undef_hook swp_hooks[] = {
>> +               {
>> +                       .instr_mask     = 0x0fb00ff0,
>> +                       .instr_val      = 0x01000090,
>> +                       .pstate_mask    = COMPAT_PSR_MODE_MASK,
>> +                       .pstate_val     = COMPAT_PSR_MODE_USR,
>> +                       .fn             = swp_handler
>> +               },
>> +               { }
>> +};
>
> It may be the email client but it looks like too much indentation
> here.

Fixed.

>
>> +
>> +static struct insn_emulation_ops swp_ops = {
>> +       .name = "swp",
>> +       .status = INSN_OBSOLETE,
>> +       .hooks = swp_hooks,
>> +       .set_hw_mode = NULL,
>> +};
>> +
>
> Code below up to armv8_deprecated_init should be moved earlier.
>
>> +static LIST_HEAD(insn_emulation);
>> +static int nr_insn_emulated;
>> +static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
>> +
>> +static bool register_emulation_hooks(struct insn_emulation_ops *ops)
>> +{
>> +       bool success = true;
>> +       struct undef_hook *hook;
>> +
>> +       if (!ops->hooks) {
>> +               success = false;
>> +               goto ret;
>> +       }
>> +
>> +       for (hook = ops->hooks; hook->instr_mask; hook++)
>> +               if (register_undef_hook(hook)) {
>> +                       success = false;
>> +                       break;
>> +               }
>> +
>> +ret:
>> +       return success;
>> +}
>
> I don't really think this code needs so much checking and error
> returning. As I already commented, register_undef_hook() should not
> return anything and for the ops->hooks check I think we could just use
> BUG_ON(!ops->hooks).

I've simplified both the above function and the one below with the BUG_ON.

>
> Do we need to add support for instructions that can't be emulated? I
> wouldn't go there yet until we see some real use-case.

Thanks! Some of the checks were put in anticipation of exactly such
features.

>
>> +static void remove_emulation_hooks(struct insn_emulation_ops *ops)
>> +{
>> +       struct undef_hook *hook;
>> +
>> +       if (!ops->hooks)
>> +               return;
>
> Same here.
>
>> +
>> +       for (hook = ops->hooks; hook->instr_mask; hook++)
>> +               unregister_undef_hook(hook);
>> +
>> +       pr_notice("Removed %s emulation handler\n", ops->name);
>> +}
>> +
>> +static int update_insn_emulation_mode(struct insn_emulation *insn,
>> +                                      enum insn_emulation_mode prev)
>> +{
>> +       int ret = 0;
>> +
>> +       switch (prev) {
>> +       case INSN_UNDEF: /* Nothing to be done */
>> +               break;
>> +       case INSN_EMULATE:
>> +               remove_emulation_hooks(insn->ops);
>> +               break;
>> +       case INSN_HW:
>> +               if (insn->ops->set_hw_mode) {
>> +                       insn->ops->set_hw_mode(false);
>> +                       pr_notice("Disabled %s support\n", insn->ops->name);
>> +               }
>> +               break;
>> +       }
>> +
>> +       switch (insn->current_mode) {
>> +       case INSN_UNDEF:
>> +               break;
>> +       case INSN_EMULATE:
>> +               if (!register_emulation_hooks(insn->ops)) {
>> +                       insn->current_mode = INSN_UNDEF;
>> +                       ret = -EINVAL;
>> +               } else
>> +                       pr_notice("Registered %s emulation handler\n",
>> +                                 insn->ops->name);
>> +               break;
>
> If we assume that register_emulation_hooks() doesn't fail, we could
> simplify slightly here.

Done.

>
>> +       case INSN_HW:
>> +               if (insn->ops->set_hw_mode && insn->ops->set_hw_mode(true)) {
>> +                       pr_notice("Enabled %s support\n", insn->ops->name);
>> +               } else {
>> +                       insn->current_mode = INSN_UNDEF;
>> +                       ret = -EINVAL;
>> +               }
>> +               break;
>
> I would rather let it fall back to the previous. So in the caller, we
> could re-instate the previous mode rather than undef.

Ok.

>
>> +static void register_insn_emulation(struct insn_emulation_ops *ops)
>> +{
>> +       unsigned long flags;
>> +       struct insn_emulation *insn;
>> +
>> +       insn = kzalloc(sizeof(*insn), GFP_KERNEL);
>> +       insn->ops = ops;
>> +       insn->min = INSN_UNDEF;
>> +
>> +       switch (ops->status) {
>> +       case INSN_DEPRECATED:
>> +               insn->current_mode = INSN_EMULATE;
>> +               insn->max = INSN_HW;
>> +               break;
>> +       case INSN_OBSOLETE:
>> +               insn->current_mode = INSN_UNDEF;
>> +               insn->max = INSN_EMULATE;
>> +               break;
>> +       }
>> +
>> +       raw_spin_lock_irqsave(&insn_emulation_lock, flags);
>> +       list_add(&insn->node, &insn_emulation);
>> +       nr_insn_emulated++;
>> +       raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
>> +
>> +       /* Register any handlers if required */
>> +       update_insn_emulation_mode(insn, INSN_UNDEF);
>> +}
>> +
>> +static int emulation_proc_handler(struct ctl_table *table, int write,
>> +                                 void __user *buffer, size_t *lenp,
>> +                                 loff_t *ppos)
>> +{
>> +       int ret = 0;
>> +       struct insn_emulation *insn = (struct insn_emulation *) table->data;
>> +       enum insn_emulation_mode prev_mode = insn->current_mode;
>> +
>> +       table->data = &insn->current_mode;
>> +       ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
>> +
>> +       if (ret || !write || prev_mode == insn->current_mode)
>> +               goto ret;
>> +
>> +       ret = update_insn_emulation_mode(insn, prev_mode);
>
> After this instruction, something like below to restore the previous
> mode:
>
> 	if (!ret) {
> 		enum insn_emulation mode = insn_mode->current_mode;
> 		insn->current_mode = prev_mode;
> 		update_insn_emulation_mode(insn, prev_mode);
> 	}

I've added code similar to your suggestion to revert to previous mode.

I'll post a new version with your's (and Steven's) comments addressed
soon.

Thanks,
Punit

>
> --
> Catalin
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel