linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv3 0/5] Legacy instruction emulation for arm64
@ 2014-10-27 18:40 Punit Agrawal
  2014-10-27 18:40 ` [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions Punit Agrawal
                   ` (5 more replies)
  0 siblings, 6 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-10-27 18:40 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

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

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 [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

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                  |   6 +-
 arch/arm64/kernel/armv8_deprecated.c        | 541 ++++++++++++++++++++++++++++
 arch/arm64/kernel/insn.c                    |  26 ++
 arch/arm64/kernel/trace-events-emulation.h  |  40 ++
 arch/arm64/kernel/traps.c                   |  68 ++++
 10 files changed, 806 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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions
  2014-10-27 18:40 [PATCHv3 0/5] Legacy instruction emulation for arm64 Punit Agrawal
@ 2014-10-27 18:40 ` Punit Agrawal
  2014-11-05 11:27   ` Catalin Marinas
  2014-10-27 18:40 ` Punit Agrawal
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: Punit Agrawal @ 2014-10-27 18:40 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---
 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..2bffdc3 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_instruction(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..9766a56 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_instruction(u32 insn)
+{
+	return insn >= 0xe800;
+}
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index de1b085..16dd73f 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -259,3 +259,36 @@ 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_instruction(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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions
  2014-10-27 18:40 [PATCHv3 0/5] Legacy instruction emulation for arm64 Punit Agrawal
  2014-10-27 18:40 ` [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions Punit Agrawal
@ 2014-10-27 18:40 ` Punit Agrawal
  2014-10-29 14:26   ` Punit Agrawal
  2014-10-27 18:40 ` [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks Punit Agrawal
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: Punit Agrawal @ 2014-10-27 18:40 UTC (permalink / raw)
  To: linux-arm-kernel

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>
---
 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..2bffdc3 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_instruction(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..9766a56 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_instruction(u32 insn)
+{
+	return insn >= 0xe800;
+}
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index de1b085..16dd73f 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_instruction(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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks
  2014-10-27 18:40 [PATCHv3 0/5] Legacy instruction emulation for arm64 Punit Agrawal
  2014-10-27 18:40 ` [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions Punit Agrawal
  2014-10-27 18:40 ` Punit Agrawal
@ 2014-10-27 18:40 ` Punit Agrawal
  2014-10-29 15:21   ` Russell King - ARM Linux
  2014-11-05 11:28   ` Catalin Marinas
  2014-10-27 18:40 ` [PATCHv3 3/5] arm64: Port SWP/SWPB emulation support from arm Punit Agrawal
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-10-27 18:40 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
---
 arch/arm64/include/asm/opcodes.h | 1 +
 arch/arm64/kernel/Makefile       | 4 +++-
 2 files changed, 4 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..798f7d7 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -18,7 +18,9 @@ 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 			\
+					   $(addprefix ../../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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCHv3 3/5] arm64: Port SWP/SWPB emulation support from arm
  2014-10-27 18:40 [PATCHv3 0/5] Legacy instruction emulation for arm64 Punit Agrawal
                   ` (2 preceding siblings ...)
  2014-10-27 18:40 ` [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks Punit Agrawal
@ 2014-10-27 18:40 ` Punit Agrawal
  2014-11-05 11:47   ` Catalin Marinas
  2014-10-27 18:40 ` [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions Punit Agrawal
  2014-10-27 18:40 ` [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions Punit Agrawal
  5 siblings, 1 reply; 27+ messages in thread
From: Punit Agrawal @ 2014-10-27 18:40 UTC (permalink / raw)
  To: linux-arm-kernel

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        | 402 ++++++++++++++++++++++++++++
 arch/arm64/kernel/insn.c                    |   8 +
 6 files changed, 496 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..6ae8079 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 N
+
+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 N
+
+endif
+
 endmenu
 
 menu "Bus support"
diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 2bffdc3..6b8fb3b 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_instruction(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 798f7d7..5362578 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -33,6 +33,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..6fdacc9
--- /dev/null
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -0,0 +1,402 @@
+/*
+ *  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 void register_emulation_hooks(struct insn_emulation_ops *ops)
+{
+	int fail = 0;
+	struct undef_hook *hook;
+
+	for (hook = ops->hooks; hook->instr_mask; hook++)
+		if (register_undef_hook(hook)) {
+			pr_notice("Error while registering %s emulation handler\n",
+				ops->name);
+			fail = 1;
+			break;
+		}
+
+	if (!fail)
+		pr_notice("Registered %s emulation handler\n", ops->name);
+}
+
+static void remove_emulation_hooks(struct insn_emulation_ops *ops)
+{
+	struct undef_hook *hook;
+
+	for (hook = ops->hooks; hook->instr_mask; hook++)
+		unregister_undef_hook(hook);
+
+	pr_notice("Removed %s emulation handler\n", ops->name);
+}
+
+static void update_insn_emulation_mode(struct insn_emulation *insn,
+				       enum insn_emulation_mode prev)
+{
+	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:
+		register_emulation_hooks(insn->ops);
+		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);
+		}
+		break;
+	}
+}
+
+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;
+
+	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 9766a56..58574c5 100644
--- a/arch/arm64/kernel/insn.c
+++ b/arch/arm64/kernel/insn.c
@@ -964,3 +964,11 @@ bool aarch32_insn_is_wide_instruction(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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions
  2014-10-27 18:40 [PATCHv3 0/5] Legacy instruction emulation for arm64 Punit Agrawal
                   ` (3 preceding siblings ...)
  2014-10-27 18:40 ` [PATCHv3 3/5] arm64: Port SWP/SWPB emulation support from arm Punit Agrawal
@ 2014-10-27 18:40 ` Punit Agrawal
  2014-11-05 13:05   ` Catalin Marinas
  2014-11-05 14:51   ` Catalin Marinas
  2014-10-27 18:40 ` [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions Punit Agrawal
  5 siblings, 2 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-10-27 18:40 UTC (permalink / raw)
  To: linux-arm-kernel

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        | 124 ++++++++++++++++++++++++++++
 arch/arm64/kernel/insn.c                    |  13 +++
 5 files changed, 159 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 6ae8079..2f7026e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -199,6 +199,21 @@ config SWP_EMULATION
 
 	  If unsure, say N
 
+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 N
+
 endif
 
 endmenu
diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index 6b8fb3b..e5a512b 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -362,6 +362,8 @@ bool aarch32_insn_is_wide_instruction(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 6fdacc9..fded15f 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,126 @@ 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
+		 */
+		isb();
+		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:
+		enable_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 0;
+}
+
+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);
@@ -394,6 +515,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 58574c5..056bedf 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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions
  2014-10-27 18:40 [PATCHv3 0/5] Legacy instruction emulation for arm64 Punit Agrawal
                   ` (4 preceding siblings ...)
  2014-10-27 18:40 ` [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions Punit Agrawal
@ 2014-10-27 18:40 ` Punit Agrawal
  2014-11-05 14:46   ` Catalin Marinas
  2014-11-10 18:27   ` Will Deacon
  5 siblings, 2 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-10-27 18:40 UTC (permalink / raw)
  To: linux-arm-kernel

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 5362578..1fc7abd 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 fded15f..d376fe2 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,16 +268,23 @@ 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", regs->pc);
+		} else {
 			dsb(sy);
+			trace_instruction_emulation(
+				"mcr p15, 0, Rt, c7, c10, 4", regs->pc);
+		}
 		break;
 	case 5:
 		/*
 		 * isb - mcr p15, 0, Rt, c7, c5, 4
 		 */
 		isb();
+		trace_instruction_emulation(
+			"mcr p15, 0, Rt, c7, c5, 4", 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..73aa750
--- /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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions
  2014-10-27 18:40 ` Punit Agrawal
@ 2014-10-29 14:26   ` Punit Agrawal
  0 siblings, 0 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-10-29 14:26 UTC (permalink / raw)
  To: linux-arm-kernel

Punit Agrawal <punit.agrawal@arm.com> writes:

> 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>
> ---
>  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(+)
>

This patch was sent twice by mistake. Please ignore this copy.

[...]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks
  2014-10-27 18:40 ` [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks Punit Agrawal
@ 2014-10-29 15:21   ` Russell King - ARM Linux
  2014-10-29 16:54     ` Punit Agrawal
  2014-11-05 16:14     ` Catalin Marinas
  2014-11-05 11:28   ` Catalin Marinas
  1 sibling, 2 replies; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-10-29 15:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 27, 2014 at 06:40:04PM +0000, Punit Agrawal wrote:
>  arm64-obj-$(CONFIG_COMPAT)		+= sys32.o kuser32.o signal32.o 	\
> -					   sys_compat.o
> +					   sys_compat.o 			\
> +					   $(addprefix ../../arm/kernel/,	\
> +						opcodes.o)

This is not particularly nice.  While it means that this file gets built
on both arm64 and arm, it means that it routinely won't get tested on
arm64 when changes to it happen.

The second reason it's not nice is that the whole $(addprefix thing
really isn't needed.  You're only adding the prefix to one name.  So,
a simpler way to write it is "../../arm/kernel/opcodes.o".

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks
  2014-10-29 15:21   ` Russell King - ARM Linux
@ 2014-10-29 16:54     ` Punit Agrawal
  2014-11-05 16:14     ` Catalin Marinas
  1 sibling, 0 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-10-29 16:54 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,

Thanks for having a look.

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> On Mon, Oct 27, 2014 at 06:40:04PM +0000, Punit Agrawal wrote:
>>  arm64-obj-$(CONFIG_COMPAT)		+= sys32.o kuser32.o signal32.o 	\
>> -					   sys_compat.o
>> +					   sys_compat.o 			\
>> +					   $(addprefix ../../arm/kernel/,	\
>> +						opcodes.o)
>
> This is not particularly nice.  While it means that this file gets built
> on both arm64 and arm, it means that it routinely won't get tested on
> arm64 when changes to it happen.

I agree. An earlier version of this patch copied opcodes.c to arm64 but
then it wouldn't benefit from any fixes that would go in the
original. The current approach misses out on the testing. Although
better than the previous approach, it is not optimal.

>
> The second reason it's not nice is that the whole $(addprefix thing
> really isn't needed.  You're only adding the prefix to one name.  So,
> a simpler way to write it is "../../arm/kernel/opcodes.o".

This one is easy to make nice. Fixed in my local copy.

Cheers,
Punit

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions
  2014-10-27 18:40 ` [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions Punit Agrawal
@ 2014-11-05 11:27   ` Catalin Marinas
  2014-11-06 12:20     ` Punit Agrawal
  0 siblings, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-11-05 11:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 27, 2014 at 06:40:02PM +0000, Punit Agrawal wrote:
> --- 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_instruction(u32 insn);

Nitpick: too wide function name ;). Maybe aarch32_insn_is_wide().

Otherwise the patch looks fine.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks
  2014-10-27 18:40 ` [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks Punit Agrawal
  2014-10-29 15:21   ` Russell King - ARM Linux
@ 2014-11-05 11:28   ` Catalin Marinas
  1 sibling, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2014-11-05 11:28 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 27, 2014 at 06:40:04PM +0000, Punit Agrawal wrote:
> Port support for AArch32 instruction condition code checking from arm
> to arm64.
> 
> Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> ---
>  arch/arm64/include/asm/opcodes.h | 1 +
>  arch/arm64/kernel/Makefile       | 4 +++-
>  2 files changed, 4 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..798f7d7 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -18,7 +18,9 @@ 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 			\
> +					   $(addprefix ../../arm/kernel/,	\
> +						opcodes.o)

Apart from the addprefix that Russell mentioned:

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 3/5] arm64: Port SWP/SWPB emulation support from arm
  2014-10-27 18:40 ` [PATCHv3 3/5] arm64: Port SWP/SWPB emulation support from arm Punit Agrawal
@ 2014-11-05 11:47   ` Catalin Marinas
  2014-11-07 18:15     ` Punit Agrawal
  0 siblings, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-11-05 11:47 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 27, 2014 at 06:40:05PM +0000, Punit Agrawal wrote:
> --- /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..6ae8079 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 N

I think if unsure we should say Y. It should probably be disabled if you
know for sure you are not going to have legacy applications using
deprecated instructions. We could even have a "default y".

> +
> +if ARMV8_DEPRECATED

Does menuconfig automatically imply an "if"? Not sure.

> +
> +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 N

Same here, I think we should default to y.

> +static void update_insn_emulation_mode(struct insn_emulation *insn,
> +                                      enum insn_emulation_mode prev)
> +{
> +       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:
> +               register_emulation_hooks(insn->ops);
> +               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);
> +               }
> +               break;

Here we should return -EINVAL all the way to the sysctl hook when
!set_hw_mode. SWP doesn't have a hw mode on ARMv8, so we should let the
user know. We even remove the emulation in this case falling back to
undef.

-- 
Catalin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions
  2014-10-27 18:40 ` [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions Punit Agrawal
@ 2014-11-05 13:05   ` Catalin Marinas
  2014-11-07 16:07     ` Punit Agrawal
  2014-11-05 14:51   ` Catalin Marinas
  1 sibling, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-11-05 13:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 27, 2014 at 06:40:06PM +0000, Punit Agrawal wrote:
> 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 6ae8079..2f7026e 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -199,6 +199,21 @@ config SWP_EMULATION
>  
>  	  If unsure, say N
>  
> +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 N
> +
>  endif

default y (I think we had a discussion in private whether deprecated
should default to y and obsolete to y or n but I don't remember the
conclusion; it's worth adding it to the Documentation/ file).

> +#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:
> +		enable_cp15_ben(NULL);
> +		return NOTIFY_DONE;
> +	}
> +
> +	return NOTIFY_OK;
> +}

Do we need CPU_STARTING_FROZEN as well?

This code always enables the CP15 barriers in hardware but it should
take into account the actual state (emulation, undef).

-- 
Catalin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions
  2014-10-27 18:40 ` [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions Punit Agrawal
@ 2014-11-05 14:46   ` Catalin Marinas
  2014-11-05 15:05     ` Steven Rostedt
  2014-11-10 18:27   ` Will Deacon
  1 sibling, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-11-05 14:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 27, 2014 at 06:40:07PM +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>
> ---
>  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 5362578..1fc7abd 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)

Why do you need this?

>  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 fded15f..d376fe2 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"

Using double quotes should be fine for the current directory.

-- 
Catalin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions
  2014-10-27 18:40 ` [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions Punit Agrawal
  2014-11-05 13:05   ` Catalin Marinas
@ 2014-11-05 14:51   ` Catalin Marinas
  2014-11-07 13:03     ` Punit Agrawal
  1 sibling, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-11-05 14:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 27, 2014 at 06:40:06PM +0000, Punit Agrawal wrote:
> +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
> +		 */
> +		isb();
> +		break;
> +	}

IIRC we concluded that an isb() isn't needed here as taking an exception
or returning from one would act as an instruction barrier.

-- 
Catalin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions
  2014-11-05 14:46   ` Catalin Marinas
@ 2014-11-05 15:05     ` Steven Rostedt
  2014-11-05 15:52       ` Catalin Marinas
  0 siblings, 1 reply; 27+ messages in thread
From: Steven Rostedt @ 2014-11-05 15:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 5 Nov 2014 14:46:19 +0000
Catalin Marinas <catalin.marinas@arm.com> wrote:

> On Mon, Oct 27, 2014 at 06:40:07PM +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>
> > ---
> >  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 5362578..1fc7abd 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)
> 
> Why do you need this?

It has to do with the TRACE_EVENT magic.

Read samples/trace_events/Makefile

> 
> >  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 fded15f..d376fe2 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"
> 
> Using double quotes should be fine for the current directory.
> 

No it is not enough. It not only gets included by this file, but also
gets included by include/trace/ftrace.h with:

#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)

Without that -I$(src) added, this include will fail.

-- Steve

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions
  2014-11-05 15:05     ` Steven Rostedt
@ 2014-11-05 15:52       ` Catalin Marinas
  0 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2014-11-05 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 05, 2014 at 03:05:11PM +0000, Steven Rostedt wrote:
> On Wed, 5 Nov 2014 14:46:19 +0000
> Catalin Marinas <catalin.marinas@arm.com> wrote:
> 
> > On Mon, Oct 27, 2014 at 06:40:07PM +0000, Punit Agrawal wrote:
> > > --- 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)
> > 
> > Why do you need this?
> 
> It has to do with the TRACE_EVENT magic.
> 
> Read samples/trace_events/Makefile
> 
> > 
> > >  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 fded15f..d376fe2 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"
> > 
> > Using double quotes should be fine for the current directory.
> 
> No it is not enough. It not only gets included by this file, but also
> gets included by include/trace/ftrace.h with:
> 
> #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
> 
> Without that -I$(src) added, this include will fail.

Ah, thanks for clarification.

-- 
Catalin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks
  2014-10-29 15:21   ` Russell King - ARM Linux
  2014-10-29 16:54     ` Punit Agrawal
@ 2014-11-05 16:14     ` Catalin Marinas
  2014-11-05 16:19       ` Will Deacon
  1 sibling, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-11-05 16:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Oct 29, 2014 at 03:21:48PM +0000, Russell King - ARM Linux wrote:
> On Mon, Oct 27, 2014 at 06:40:04PM +0000, Punit Agrawal wrote:
> >  arm64-obj-$(CONFIG_COMPAT)		+= sys32.o kuser32.o signal32.o 	\
> > -					   sys_compat.o
> > +					   sys_compat.o 			\
> > +					   $(addprefix ../../arm/kernel/,	\
> > +						opcodes.o)
> 
> This is not particularly nice.

Not that bad, you can look at it as a decoding library.

> While it means that this file gets built on both arm64 and arm, it
> means that it routinely won't get tested on arm64 when changes to it
> happen.

Low risk, it's a small file unlikely to change in the future.

Anyway, since you mention testing, I wonder how regularly SWP emulation
gets tested on arm32 (and in future arm64). If Punit has some tests
already, it may be good to add them somewhere like
tools/testing/selftests/arm/ (probably separately from this series).

-- 
Catalin

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks
  2014-11-05 16:14     ` Catalin Marinas
@ 2014-11-05 16:19       ` Will Deacon
  0 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2014-11-05 16:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Nov 05, 2014 at 04:14:54PM +0000, Catalin Marinas wrote:
> Anyway, since you mention testing, I wonder how regularly SWP emulation
> gets tested on arm32 (and in future arm64). If Punit has some tests
> already, it may be good to add them somewhere like
> tools/testing/selftests/arm/ (probably separately from this series).

Sounds like you want atomic.h implemented using SWP and CP15 barriers ;)

There *is* lib/atomic64_test.c but, perversely, it's single-threaded and
unlikely to spot issues even if we got it running in userspace.

Will

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions
  2014-11-05 11:27   ` Catalin Marinas
@ 2014-11-06 12:20     ` Punit Agrawal
  0 siblings, 0 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-11-06 12:20 UTC (permalink / raw)
  To: linux-arm-kernel

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

> On Mon, Oct 27, 2014 at 06:40:02PM +0000, Punit Agrawal wrote:
>> --- 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_instruction(u32 insn);
>
> Nitpick: too wide function name ;). Maybe aarch32_insn_is_wide().

Agreed. I'll change it to aarch32_insn_is_wide(). Please shout if you
can think of a better name.

>
> Otherwise the patch looks fine.
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions
  2014-11-05 14:51   ` Catalin Marinas
@ 2014-11-07 13:03     ` Punit Agrawal
  0 siblings, 0 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-11-07 13:03 UTC (permalink / raw)
  To: linux-arm-kernel

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

> On Mon, Oct 27, 2014 at 06:40:06PM +0000, Punit Agrawal wrote:
>> +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
>> +		 */
>> +		isb();
>> +		break;
>> +	}
>
> IIRC we concluded that an isb() isn't needed here as taking an exception
> or returning from one would act as an instruction barrier.

I don't remember what the conclusion was - the isb was introduced to err
on the side of caution. I've removed it now.

Thanks.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions
  2014-11-05 13:05   ` Catalin Marinas
@ 2014-11-07 16:07     ` Punit Agrawal
  2014-11-13 10:34       ` Catalin Marinas
  0 siblings, 1 reply; 27+ messages in thread
From: Punit Agrawal @ 2014-11-07 16:07 UTC (permalink / raw)
  To: linux-arm-kernel

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

> On Mon, Oct 27, 2014 at 06:40:06PM +0000, Punit Agrawal wrote:
>> 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 6ae8079..2f7026e 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -199,6 +199,21 @@ config SWP_EMULATION
>>  
>>  	  If unsure, say N
>>  
>> +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 N
>> +
>>  endif
>
> default y (I think we had a discussion in private whether deprecated
> should default to y and obsolete to y or n but I don't remember the
> conclusion; it's worth adding it to the Documentation/ file).

Based on the outcome of the discussion, I've documented the runtime
defaults ('undef' for obsolete and 'emulation' for deprecated instructions)
in the previous patch (3/5).

As for Kconfig, the conclusion was to keep them off by default. The
rationale was that they can be enabled quite easily and would form
another way to communicate that certain features are deprecated and
software might need updating.

>
>> +#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:
>> +		enable_cp15_ben(NULL);
>> +		return NOTIFY_DONE;
>> +	}
>> +
>> +	return NOTIFY_OK;
>> +}
>
> Do we need CPU_STARTING_FROZEN as well?

Added this now.

Thinking about this, I think I want to disable this bit for CPUs being
hot-unplugged otherwise we might end up with an inconsistent state
across cores. I'll fix this up for the next version.

>
> This code always enables the CP15 barriers in hardware but it should
> take into account the actual state (emulation, undef).

The actual state is taken into account in the previous patch (3/5
function: update_insn_emulation_mode) and the hotplug notifier is only
installed to handle offlined CPUs when hardware execution is requested.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 3/5] arm64: Port SWP/SWPB emulation support from arm
  2014-11-05 11:47   ` Catalin Marinas
@ 2014-11-07 18:15     ` Punit Agrawal
  0 siblings, 0 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-11-07 18:15 UTC (permalink / raw)
  To: linux-arm-kernel

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

> On Mon, Oct 27, 2014 at 06:40:05PM +0000, Punit Agrawal wrote:
>> --- /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..6ae8079 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 N
>
> I think if unsure we should say Y. It should probably be disabled if you
> know for sure you are not going to have legacy applications using
> deprecated instructions. We could even have a "default y".

I replied in another email to why this wasn't turned off by default. Let
me know if you'd like to change it.

>
>> +
>> +if ARMV8_DEPRECATED
>
> Does menuconfig automatically imply an "if"? Not sure.
>
>> +
>> +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 N
>
> Same here, I think we should default to y.

Again, same as above.

>
>> +static void update_insn_emulation_mode(struct insn_emulation *insn,
>> +                                      enum insn_emulation_mode prev)
>> +{
>> +       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:
>> +               register_emulation_hooks(insn->ops);
>> +               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);
>> +               }
>> +               break;
>
> Here we should return -EINVAL all the way to the sysctl hook when
> !set_hw_mode. SWP doesn't have a hw mode on ARMv8, so we should let the
> user know. We even remove the emulation in this case falling back to
> undef.

If set_hw_mode callback is not provided- which swp doesn't, then
the min-max check performed when parsing the value written to sysctl
will fail. So it already works as you suggest for swp - but I take your
point about erroring out all the way to userspace if something doesn't
succeed.

I'll audit the code to make sure all code paths behave accordingly.

Thanks for your feedback. I'll post a new version with the updates.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions
  2014-10-27 18:40 ` [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions Punit Agrawal
  2014-11-05 14:46   ` Catalin Marinas
@ 2014-11-10 18:27   ` Will Deacon
  2014-11-12 11:31     ` Punit Agrawal
  1 sibling, 1 reply; 27+ messages in thread
From: Will Deacon @ 2014-11-10 18:27 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Oct 27, 2014 at 06:40:07PM +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>
> ---
>  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 5362578..1fc7abd 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 fded15f..d376fe2 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,16 +268,23 @@ 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", regs->pc);
> +		} else {
>  			dsb(sy);
> +			trace_instruction_emulation(
> +				"mcr p15, 0, Rt, c7, c10, 4", regs->pc);
> +		}
>  		break;
>  	case 5:
>  		/*
>  		 * isb - mcr p15, 0, Rt, c7, c5, 4
>  		 */
>  		isb();
> +		trace_instruction_emulation(
> +			"mcr p15, 0, Rt, c7, c5, 4", regs->pc);

Any chance you can put the barrier name in here too please? Maybe as an asm
comment, e.g.:

  "mcr p15, 0, Rt, c7, c5, 4 ; isb"

Will

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions
  2014-11-10 18:27   ` Will Deacon
@ 2014-11-12 11:31     ` Punit Agrawal
  0 siblings, 0 replies; 27+ messages in thread
From: Punit Agrawal @ 2014-11-12 11:31 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Will,

Thanks for taking a look.

Will Deacon <will.deacon@arm.com> writes:

> On Mon, Oct 27, 2014 at 06:40:07PM +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>
>> ---

[...]

>> @@ -260,16 +268,23 @@ 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", regs->pc);
>> +		} else {
>>  			dsb(sy);
>> +			trace_instruction_emulation(
>> +				"mcr p15, 0, Rt, c7, c10, 4", regs->pc);
>> +		}
>>  		break;
>>  	case 5:
>>  		/*
>>  		 * isb - mcr p15, 0, Rt, c7, c5, 4
>>  		 */
>>  		isb();
>> +		trace_instruction_emulation(
>> +			"mcr p15, 0, Rt, c7, c5, 4", regs->pc);
>
> Any chance you can put the barrier name in here too please? Maybe as an asm
> comment, e.g.:
>
>   "mcr p15, 0, Rt, c7, c5, 4 ; isb"

Good point. I've added the barrier names as assembler comments.

Thanks.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions
  2014-11-07 16:07     ` Punit Agrawal
@ 2014-11-13 10:34       ` Catalin Marinas
  0 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2014-11-13 10:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Nov 07, 2014 at 04:07:51PM +0000, Punit Agrawal wrote:
> Catalin Marinas <catalin.marinas@arm.com> writes:
> > On Mon, Oct 27, 2014 at 06:40:06PM +0000, Punit Agrawal wrote:
> >> 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 6ae8079..2f7026e 100644
> >> --- a/arch/arm64/Kconfig
> >> +++ b/arch/arm64/Kconfig
> >> @@ -199,6 +199,21 @@ config SWP_EMULATION
> >>  
> >>  	  If unsure, say N
> >>  
> >> +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 N
> >> +
> >>  endif
> >
> > default y (I think we had a discussion in private whether deprecated
> > should default to y and obsolete to y or n but I don't remember the
> > conclusion; it's worth adding it to the Documentation/ file).
> 
> Based on the outcome of the discussion, I've documented the runtime
> defaults ('undef' for obsolete and 'emulation' for deprecated instructions)
> in the previous patch (3/5).
> 
> As for Kconfig, the conclusion was to keep them off by default. The
> rationale was that they can be enabled quite easily and would form
> another way to communicate that certain features are deprecated and
> software might need updating.

Sounds fine.

-- 
Catalin

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2014-11-13 10:34 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-27 18:40 [PATCHv3 0/5] Legacy instruction emulation for arm64 Punit Agrawal
2014-10-27 18:40 ` [PATCHv3 1/5] arm64: Add support for hooks to handle undefined instructions Punit Agrawal
2014-11-05 11:27   ` Catalin Marinas
2014-11-06 12:20     ` Punit Agrawal
2014-10-27 18:40 ` Punit Agrawal
2014-10-29 14:26   ` Punit Agrawal
2014-10-27 18:40 ` [PATCHv3 2/5] arm64: Add AArch32 instruction set condition code checks Punit Agrawal
2014-10-29 15:21   ` Russell King - ARM Linux
2014-10-29 16:54     ` Punit Agrawal
2014-11-05 16:14     ` Catalin Marinas
2014-11-05 16:19       ` Will Deacon
2014-11-05 11:28   ` Catalin Marinas
2014-10-27 18:40 ` [PATCHv3 3/5] arm64: Port SWP/SWPB emulation support from arm Punit Agrawal
2014-11-05 11:47   ` Catalin Marinas
2014-11-07 18:15     ` Punit Agrawal
2014-10-27 18:40 ` [PATCHv3 4/5] arm64: Emulate CP15 Barrier instructions Punit Agrawal
2014-11-05 13:05   ` Catalin Marinas
2014-11-07 16:07     ` Punit Agrawal
2014-11-13 10:34       ` Catalin Marinas
2014-11-05 14:51   ` Catalin Marinas
2014-11-07 13:03     ` Punit Agrawal
2014-10-27 18:40 ` [PATCHv3 5/5] arm64: Trace emulation of AArch32 legacy instructions Punit Agrawal
2014-11-05 14:46   ` Catalin Marinas
2014-11-05 15:05     ` Steven Rostedt
2014-11-05 15:52       ` Catalin Marinas
2014-11-10 18:27   ` Will Deacon
2014-11-12 11:31     ` Punit Agrawal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).