All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps
@ 2022-10-19 14:41 Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 1/9] arm64: allow kprobes on EL0 handlers Mark Rutland
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

This series reworks the way UNDEFINED instruction traps are handled,
removing some dynamic data structure manipulation and related locking.
This makes the code a bit simpler and removes some unnecessary
bottlenecks when traps are handled.

My original aim for this series was to get rid of the RCU_NONIDLE() call
in cpu_suspend(), requiring the removal of the undef_hook list
manipulation. In the process of looking at that I figured we had a set
of related problems with UNDEF handling and deprecated instruction
handling, which this series addresses. This series does not remove the
RCU_NONIDLE() call from cpu_suspend() as removing that will require
further rework of the suspend code (e.g. for noinstr safety).

I've tested this series in VMs on ThunderX2 (for the ID reg emulation
changes) and a Raspberry Pi 4 (for the armv8_deprecated changes), as
described in the relevant commits.

Since v1 [1]:
* Rebase atop v6.1-rc1
* Fix typos

[1] https://lore.kernel.org/linux-arm-kernel/20220928102659.247510-1-mark.rutland@arm.com/

Thanks,
Mark.

Mark Rutland (9):
  arm64: allow kprobes on EL0 handlers
  arm64: split EL0/EL1 UNDEF handlers
  arm64: factor out EL1 SSBS emulation hook
  arm64: factor insn read out of call_undef_hook()
  arm64: rework EL0 MRS emulation
  arm64: armv8_deprecated: fold ops into insn_emulation
  arm64: armv8_deprecated move emulation functions
  arm64: armv8_deprecated: move aarch32 helper earlier
  arm64: armv8_deprecated: rework deprected instruction handling

 arch/arm64/include/asm/cpufeature.h  |   3 +-
 arch/arm64/include/asm/exception.h   |   7 +-
 arch/arm64/include/asm/spectre.h     |   2 +
 arch/arm64/include/asm/traps.h       |  19 +-
 arch/arm64/kernel/armv8_deprecated.c | 567 +++++++++++++--------------
 arch/arm64/kernel/cpufeature.c       |  23 +-
 arch/arm64/kernel/entry-common.c     |   8 +-
 arch/arm64/kernel/proton-pack.c      |  26 +-
 arch/arm64/kernel/traps.c            |  93 ++---
 9 files changed, 345 insertions(+), 403 deletions(-)

-- 
2.30.2


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

^ permalink raw reply	[flat|nested] 16+ messages in thread
* [PATCH v2 1/9] arm64: allow kprobes on EL0 handlers
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
@ 2022-10-19 14:41 ` Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 2/9] arm64: split EL0/EL1 UNDEF handlers Mark Rutland
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

Currently do_sysinstr() and do_cp15instr() are marked with
NOKPROBE_SYMBOL(). However, these are only called for exceptions taken
from EL0, and there is no risk of recursion in kprobes, so this is not
necessary.

Remove the NOKPROBE_SYMBOL() annotation, and rename the two functions to
more clearly indicate that these are solely for exceptions taken from
EL0, better matching the names used by the lower level entry points in
entry-common.c.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/exception.h | 4 ++--
 arch/arm64/kernel/entry-common.c   | 4 ++--
 arch/arm64/kernel/traps.c          | 6 ++----
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index 19713d0f013b..bc0dd002106d 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -67,10 +67,10 @@ void do_fpsimd_acc(unsigned long esr, struct pt_regs *regs);
 void do_sve_acc(unsigned long esr, struct pt_regs *regs);
 void do_sme_acc(unsigned long esr, struct pt_regs *regs);
 void do_fpsimd_exc(unsigned long esr, struct pt_regs *regs);
-void do_sysinstr(unsigned long esr, struct pt_regs *regs);
+void do_el0_sys(unsigned long esr, struct pt_regs *regs);
 void do_sp_pc_abort(unsigned long addr, unsigned long esr, struct pt_regs *regs);
 void bad_el0_sync(struct pt_regs *regs, int reason, unsigned long esr);
-void do_cp15instr(unsigned long esr, struct pt_regs *regs);
+void do_el0_cp15(unsigned long esr, struct pt_regs *regs);
 int do_compat_alignment_fixup(unsigned long addr, struct pt_regs *regs);
 void do_el0_svc(struct pt_regs *regs);
 void do_el0_svc_compat(struct pt_regs *regs);
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 9173fad279af..135d4a5868fe 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -569,7 +569,7 @@ static void noinstr el0_sys(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
 	local_daif_restore(DAIF_PROCCTX);
-	do_sysinstr(esr, regs);
+	do_el0_sys(esr, regs);
 	exit_to_user_mode(regs);
 }
 
@@ -761,7 +761,7 @@ static void noinstr el0_cp15(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
 	local_daif_restore(DAIF_PROCCTX);
-	do_cp15instr(esr, regs);
+	do_el0_cp15(esr, regs);
 	exit_to_user_mode(regs);
 }
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 23d281ed7621..ae184d661067 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -748,7 +748,7 @@ static const struct sys64_hook cp15_64_hooks[] = {
 	{},
 };
 
-void do_cp15instr(unsigned long esr, struct pt_regs *regs)
+void do_el0_cp15(unsigned long esr, struct pt_regs *regs)
 {
 	const struct sys64_hook *hook, *hook_base;
 
@@ -786,10 +786,9 @@ void do_cp15instr(unsigned long esr, struct pt_regs *regs)
 	 */
 	do_undefinstr(regs, esr);
 }
-NOKPROBE_SYMBOL(do_cp15instr);
 #endif
 
-void do_sysinstr(unsigned long esr, struct pt_regs *regs)
+void do_el0_sys(unsigned long esr, struct pt_regs *regs)
 {
 	const struct sys64_hook *hook;
 
@@ -806,7 +805,6 @@ void do_sysinstr(unsigned long esr, struct pt_regs *regs)
 	 */
 	do_undefinstr(regs, esr);
 }
-NOKPROBE_SYMBOL(do_sysinstr);
 
 static const char *esr_class_str[] = {
 	[0 ... ESR_ELx_EC_MAX]		= "UNRECOGNIZED EC",
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 2/9] arm64: split EL0/EL1 UNDEF handlers
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 1/9] arm64: allow kprobes on EL0 handlers Mark Rutland
@ 2022-10-19 14:41 ` Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 3/9] arm64: factor out EL1 SSBS emulation hook Mark Rutland
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

In general, exceptions taken from EL1 need to be handled separately from
exceptions taken from EL0, as the logic to handle the two cases can be
significantly divergent, and exceptions taken from EL1 typically have
more stringent requirements on locking and instrumentation.

Subsequent patches will rework the way EL1 UNDEFs are handled in order
to address longstanding soundness issues with instrumentation and RCU.
In preparation for that rework, this patch splits the existing
do_undefinstr() handler into separate do_el0_undef() and do_el1_undef()
handlers.

Prior to this patch, do_undefinstr() was marked with NOKPROBE_SYMBOL(),
preventing instrumentation via kprobes. However, do_undefinstr() invokes
other code which can be instrumented, and:

* For UNDEFINED exceptions taken from EL0, there is no risk of recursion
  within kprobes. Therefore it is safe for do_el0_undef to be
  instrumented with kprobes, and it does not need to be marked with
  NOKPROBE_SYMBOL().

* For UNDEFINED exceptions taken from EL1, either:

  (a) The exception is has been taken when manipulating SSBS; these cases
      are limited and do not occur within code that can be invoked
      recursively via kprobes. Hence, in these cases instrumentation
      with kprobes is benign.

  (b) The exception has been taken for an unknown reason, as other than
      manipulating SSBS we do not expect to take UNDEFINED exceptions
      from EL1. Any handling of these exception is best-effort.

  ... and in either case, marking do_el1_undef() with NOKPROBE_SYMBOL()
  isn't sufficient to prevent recursion via kprobes as functions it
  calls (including die()) are instrumentable via kprobes.

  Hence, it's not worthwhile to mark do_el1_undef() with
  NOKPROBE_SYMBOL(). The same applies to do_el1_bti() and do_el1_fpac(),
  so their NOKPROBE_SYMBOL() annotations are also removed.

Aside from the new instrumentability, there should be no functional
change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/exception.h |  3 ++-
 arch/arm64/kernel/entry-common.c   |  4 ++--
 arch/arm64/kernel/traps.c          | 22 ++++++++++++----------
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
index bc0dd002106d..92963f98afec 100644
--- a/arch/arm64/include/asm/exception.h
+++ b/arch/arm64/include/asm/exception.h
@@ -58,7 +58,8 @@ asmlinkage void call_on_irq_stack(struct pt_regs *regs,
 asmlinkage void asm_exit_to_user_mode(struct pt_regs *regs);
 
 void do_mem_abort(unsigned long far, unsigned long esr, struct pt_regs *regs);
-void do_undefinstr(struct pt_regs *regs, unsigned long esr);
+void do_el0_undef(struct pt_regs *regs, unsigned long esr);
+void do_el1_undef(struct pt_regs *regs, unsigned long esr);
 void do_el0_bti(struct pt_regs *regs);
 void do_el1_bti(struct pt_regs *regs, unsigned long esr);
 void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr,
diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
index 135d4a5868fe..a38c3c4f94c0 100644
--- a/arch/arm64/kernel/entry-common.c
+++ b/arch/arm64/kernel/entry-common.c
@@ -383,7 +383,7 @@ static void noinstr el1_undef(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_kernel_mode(regs);
 	local_daif_inherit(regs);
-	do_undefinstr(regs, esr);
+	do_el1_undef(regs, esr);
 	local_daif_mask();
 	exit_to_kernel_mode(regs);
 }
@@ -598,7 +598,7 @@ static void noinstr el0_undef(struct pt_regs *regs, unsigned long esr)
 {
 	enter_from_user_mode(regs);
 	local_daif_restore(DAIF_PROCCTX);
-	do_undefinstr(regs, esr);
+	do_el0_undef(regs, esr);
 	exit_to_user_mode(regs);
 }
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index ae184d661067..678720103388 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -486,7 +486,7 @@ void arm64_notify_segfault(unsigned long addr)
 	force_signal_inject(SIGSEGV, code, addr, 0);
 }
 
-void do_undefinstr(struct pt_regs *regs, unsigned long esr)
+void do_el0_undef(struct pt_regs *regs, unsigned long esr)
 {
 	/* check for AArch32 breakpoint instructions */
 	if (!aarch32_break_handler(regs))
@@ -495,12 +495,16 @@ void do_undefinstr(struct pt_regs *regs, unsigned long esr)
 	if (call_undef_hook(regs) == 0)
 		return;
 
-	if (!user_mode(regs))
-		die("Oops - Undefined instruction", regs, esr);
-
 	force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
 }
-NOKPROBE_SYMBOL(do_undefinstr);
+
+void do_el1_undef(struct pt_regs *regs, unsigned long esr)
+{
+	if (call_undef_hook(regs) == 0)
+		return;
+
+	die("Oops - Undefined instruction", regs, esr);
+}
 
 void do_el0_bti(struct pt_regs *regs)
 {
@@ -511,7 +515,6 @@ void do_el1_bti(struct pt_regs *regs, unsigned long esr)
 {
 	die("Oops - BTI", regs, esr);
 }
-NOKPROBE_SYMBOL(do_el1_bti);
 
 void do_el0_fpac(struct pt_regs *regs, unsigned long esr)
 {
@@ -526,7 +529,6 @@ void do_el1_fpac(struct pt_regs *regs, unsigned long esr)
 	 */
 	die("Oops - FPAC", regs, esr);
 }
-NOKPROBE_SYMBOL(do_el1_fpac)
 
 #define __user_cache_maint(insn, address, res)			\
 	if (address >= TASK_SIZE_MAX) {				\
@@ -769,7 +771,7 @@ void do_el0_cp15(unsigned long esr, struct pt_regs *regs)
 		hook_base = cp15_64_hooks;
 		break;
 	default:
-		do_undefinstr(regs, esr);
+		do_el0_undef(regs, esr);
 		return;
 	}
 
@@ -784,7 +786,7 @@ void do_el0_cp15(unsigned long esr, struct pt_regs *regs)
 	 * EL0. Fall back to our usual undefined instruction handler
 	 * so that we handle these consistently.
 	 */
-	do_undefinstr(regs, esr);
+	do_el0_undef(regs, esr);
 }
 #endif
 
@@ -803,7 +805,7 @@ void do_el0_sys(unsigned long esr, struct pt_regs *regs)
 	 * back to our usual undefined instruction handler so that we handle
 	 * these consistently.
 	 */
-	do_undefinstr(regs, esr);
+	do_el0_undef(regs, esr);
 }
 
 static const char *esr_class_str[] = {
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 3/9] arm64: factor out EL1 SSBS emulation hook
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 1/9] arm64: allow kprobes on EL0 handlers Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 2/9] arm64: split EL0/EL1 UNDEF handlers Mark Rutland
@ 2022-10-19 14:41 ` Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 4/9] arm64: factor insn read out of call_undef_hook() Mark Rutland
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

Currently call_undef_hook() is used to handle UNDEFINED exceptions from
EL0 and EL1. As support for deprecated instructions may be enabled
independently, the handlers for individual instructions are organised as
a linked list of struct undef_hook which can be manipulated dynamically.
As this can be manipulated dynamically, the list is protected with a
raw_spinlock which must be acquired when handling UNDEFINED exceptions
or when manipulating the list of handlers.

This locking is unfortunate as it serialises handling of UNDEFINED
exceptions, and requires RCU to be enabled for lockdep, requiring the
use of RCU_NONIDLE() in resume path of cpu_suspend() since commit:

  a2c42bbabbe260b7 ("arm64: spectre: Prevent lockdep splat on v4 mitigation enable path")

The list of UNDEFINED handlers largely consist of handlers for
exceptions taken from EL0, and the only handler for exceptions taken
from EL1 handles `MSR SSBS, #imm` on CPUs which feature PSTATE.SSBS but
lack the corresponding MSR (Immediate) instruction. Other than this we
never expect to take an UNDEFINED exception from EL1 in normal
operation.

This patch reworks do_el0_undef() to invoke the EL1 SSBS handler
directly, relegating call_undef_hook() to only handle EL0 UNDEFs. This
removes redundant work to iterate the list for EL1 UNDEFs, and removes
the need for locking, permitting EL1 UNDEFs to be handled in parallel
without contention.

The RCU_NONIDLE() call in cpu_suspend() will be removed in a subsequent
patch, as there are other potential issues with the use of
instrumentable code and RCU in the CPU suspend code.

I've tested this by forcing the detection of SSBS on a CPU that doesn't
have it, and verifying that the try_emulate_el1_ssbs() callback is
invoked.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/spectre.h |  2 ++
 arch/arm64/kernel/proton-pack.c  | 26 +++++++-------------------
 arch/arm64/kernel/traps.c        | 15 ++++++++-------
 3 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/arch/arm64/include/asm/spectre.h b/arch/arm64/include/asm/spectre.h
index aa3d3607d5c8..db7b371b367c 100644
--- a/arch/arm64/include/asm/spectre.h
+++ b/arch/arm64/include/asm/spectre.h
@@ -26,6 +26,7 @@ enum mitigation_state {
 	SPECTRE_VULNERABLE,
 };
 
+struct pt_regs;
 struct task_struct;
 
 /*
@@ -98,5 +99,6 @@ enum mitigation_state arm64_get_spectre_bhb_state(void);
 bool is_spectre_bhb_affected(const struct arm64_cpu_capabilities *entry, int scope);
 u8 spectre_bhb_loop_affected(int scope);
 void spectre_bhb_enable_mitigation(const struct arm64_cpu_capabilities *__unused);
+bool try_emulate_el1_ssbs(struct pt_regs *regs, u32 instr);
 #endif	/* __ASSEMBLY__ */
 #endif	/* __ASM_SPECTRE_H */
diff --git a/arch/arm64/kernel/proton-pack.c b/arch/arm64/kernel/proton-pack.c
index bfce41c2a53b..fca9cc6f5581 100644
--- a/arch/arm64/kernel/proton-pack.c
+++ b/arch/arm64/kernel/proton-pack.c
@@ -521,10 +521,13 @@ bool has_spectre_v4(const struct arm64_cpu_capabilities *cap, int scope)
 	return state != SPECTRE_UNAFFECTED;
 }
 
-static int ssbs_emulation_handler(struct pt_regs *regs, u32 instr)
+bool try_emulate_el1_ssbs(struct pt_regs *regs, u32 instr)
 {
-	if (user_mode(regs))
-		return 1;
+	const u32 instr_mask = ~(1U << PSTATE_Imm_shift);
+	const u32 instr_val = 0xd500401f | PSTATE_SSBS;
+
+	if ((instr & instr_mask) != instr_val)
+		return false;
 
 	if (instr & BIT(PSTATE_Imm_shift))
 		regs->pstate |= PSR_SSBS_BIT;
@@ -532,19 +535,11 @@ static int ssbs_emulation_handler(struct pt_regs *regs, u32 instr)
 		regs->pstate &= ~PSR_SSBS_BIT;
 
 	arm64_skip_faulting_instruction(regs, 4);
-	return 0;
+	return true;
 }
 
-static struct undef_hook ssbs_emulation_hook = {
-	.instr_mask	= ~(1U << PSTATE_Imm_shift),
-	.instr_val	= 0xd500401f | PSTATE_SSBS,
-	.fn		= ssbs_emulation_handler,
-};
-
 static enum mitigation_state spectre_v4_enable_hw_mitigation(void)
 {
-	static bool undef_hook_registered = false;
-	static DEFINE_RAW_SPINLOCK(hook_lock);
 	enum mitigation_state state;
 
 	/*
@@ -555,13 +550,6 @@ static enum mitigation_state spectre_v4_enable_hw_mitigation(void)
 	if (state != SPECTRE_MITIGATED || !this_cpu_has_cap(ARM64_SSBS))
 		return state;
 
-	raw_spin_lock(&hook_lock);
-	if (!undef_hook_registered) {
-		register_undef_hook(&ssbs_emulation_hook);
-		undef_hook_registered = true;
-	}
-	raw_spin_unlock(&hook_lock);
-
 	if (spectre_v4_mitigations_off()) {
 		sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_DSSBS);
 		set_pstate_ssbs(1);
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 678720103388..ccd7d773e5cd 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -402,12 +402,7 @@ static int call_undef_hook(struct pt_regs *regs)
 	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
 	unsigned long pc = instruction_pointer(regs);
 
-	if (!user_mode(regs)) {
-		__le32 instr_le;
-		if (get_kernel_nofault(instr_le, (__le32 *)pc))
-			goto exit;
-		instr = le32_to_cpu(instr_le);
-	} else if (compat_thumb_mode(regs)) {
+	if (compat_thumb_mode(regs)) {
 		/* 16-bit Thumb instruction */
 		__le16 instr_le;
 		if (get_user(instr_le, (__le16 __user *)pc))
@@ -500,9 +495,15 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
 
 void do_el1_undef(struct pt_regs *regs, unsigned long esr)
 {
-	if (call_undef_hook(regs) == 0)
+	u32 insn;
+
+	if (aarch64_insn_read((void *)regs->pc, &insn))
+		goto out_err;
+
+	if (try_emulate_el1_ssbs(regs, insn))
 		return;
 
+out_err:
 	die("Oops - Undefined instruction", regs, esr);
 }
 
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 4/9] arm64: factor insn read out of call_undef_hook()
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
                   ` (2 preceding siblings ...)
  2022-10-19 14:41 ` [PATCH v2 3/9] arm64: factor out EL1 SSBS emulation hook Mark Rutland
@ 2022-10-19 14:41 ` Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 5/9] arm64: rework EL0 MRS emulation Mark Rutland
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

Subsequent patches will rework EL0 UNDEF handling, removing the need for
struct undef_hook and call_undef_hook. In preparation for those changes,
this patch factors the logic for reading user instructions out of
call_undef_hook() and into a new user_insn_read() helper, matching the
style of the existing aarch64_insn_read() helper used for reading kernel
instructions.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/traps.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index ccd7d773e5cd..4d51afd010e1 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -394,25 +394,22 @@ void unregister_undef_hook(struct undef_hook *hook)
 	raw_spin_unlock_irqrestore(&undef_lock, flags);
 }
 
-static int call_undef_hook(struct pt_regs *regs)
+static int user_insn_read(struct pt_regs *regs, u32 *insnp)
 {
-	struct undef_hook *hook;
-	unsigned long flags;
 	u32 instr;
-	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
 	unsigned long pc = instruction_pointer(regs);
 
 	if (compat_thumb_mode(regs)) {
 		/* 16-bit Thumb instruction */
 		__le16 instr_le;
 		if (get_user(instr_le, (__le16 __user *)pc))
-			goto exit;
+			return -EFAULT;
 		instr = le16_to_cpu(instr_le);
 		if (aarch32_insn_is_wide(instr)) {
 			u32 instr2;
 
 			if (get_user(instr_le, (__le16 __user *)(pc + 2)))
-				goto exit;
+				return -EFAULT;
 			instr2 = le16_to_cpu(instr_le);
 			instr = (instr << 16) | instr2;
 		}
@@ -420,10 +417,20 @@ static int call_undef_hook(struct pt_regs *regs)
 		/* 32-bit ARM instruction */
 		__le32 instr_le;
 		if (get_user(instr_le, (__le32 __user *)pc))
-			goto exit;
+			return -EFAULT;
 		instr = le32_to_cpu(instr_le);
 	}
 
+	*insnp = instr;
+	return 0;
+}
+
+static int call_undef_hook(struct pt_regs *regs, u32 instr)
+{
+	struct undef_hook *hook;
+	unsigned long flags;
+	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
+
 	raw_spin_lock_irqsave(&undef_lock, flags);
 	list_for_each_entry(hook, &undef_hook, node)
 		if ((instr & hook->instr_mask) == hook->instr_val &&
@@ -431,7 +438,7 @@ static int call_undef_hook(struct pt_regs *regs)
 			fn = hook->fn;
 
 	raw_spin_unlock_irqrestore(&undef_lock, flags);
-exit:
+
 	return fn ? fn(regs, instr) : 1;
 }
 
@@ -483,13 +490,19 @@ void arm64_notify_segfault(unsigned long addr)
 
 void do_el0_undef(struct pt_regs *regs, unsigned long esr)
 {
+	u32 insn;
+
 	/* check for AArch32 breakpoint instructions */
 	if (!aarch32_break_handler(regs))
 		return;
 
-	if (call_undef_hook(regs) == 0)
+	if (user_insn_read(regs, &insn))
+		goto out_err;
+
+	if (call_undef_hook(regs, insn) == 0)
 		return;
 
+out_err:
 	force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc, 0);
 }
 
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 5/9] arm64: rework EL0 MRS emulation
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
                   ` (3 preceding siblings ...)
  2022-10-19 14:41 ` [PATCH v2 4/9] arm64: factor insn read out of call_undef_hook() Mark Rutland
@ 2022-10-19 14:41 ` Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 6/9] arm64: armv8_deprecated: fold ops into insn_emulation Mark Rutland
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

On CPUs without FEAT_IDST, ID register emulation is slower than it needs
to be, as all threads contend for the same lock to perform the
emulation. This patch reworks the emulation to avoid this unnecessary
contention.

On CPUs with FEAT_IDST (which is mandatory from ARMv8.4 onwards), EL0
accesses to ID registers result in a SYS trap, and emulation of these is
handled with a sys64_hook. These hooks are statically allocated, and no
locking is required to iterate through the hooks and perform the
emulation, allowing emulation to occur in parallel with no contention.

On CPUs without FEAT_IDST, EL0 accesses to ID registers result in an
UNDEFINED exception, and emulation of these accesses is handled with an
undef_hook. When an EL0 MRS instruction is trapped to EL1, the kernel
finds the relevant handler by iterating through all of the undef_hooks,
requiring undef_lock to be held during this lookup.

This locking is only required to safely traverse the list of undef_hooks
(as it can be concurrently modified), and the actual emulation of the
MRS does not require any mutual exclusion. This locking is an
unfortunate bottleneck, especially given that MRS emulation is enabled
unconditionally and is never disabled.

This patch reworks the non-FEAT_IDST MRS emulation logic so that it can
be invoked directly from do_el0_undef(). This removes the bottleneck,
allowing MRS traps to be handled entirely in parallel, and is a stepping
stone to making all of the undef_hooks lock-free.

I've tested this in a 64-vCPU VM on a 64-CPU ThunderX2 host, with a
benchmark which spawns a number of threads which each try to read
ID_AA64ISAR0_EL1 1000000 times. This is vastly more contention than will
ever be seen in realistic usage, but clearly demonstrates the removal of
the bottleneck:

  | Threads || Time (seconds)                       |
  |         || Before           || After            |
  |         || Real   | System  || Real   | System  |
  |---------++--------+---------++--------+---------|
  |       1 ||   0.29 |    0.20 ||   0.24 |    0.12 |
  |       2 ||   0.35 |    0.51 ||   0.23 |    0.27 |
  |       4 ||   1.08 |    3.87 ||   0.24 |    0.56 |
  |       8 ||   4.31 |   33.60 ||   0.24 |    1.11 |
  |      16 ||   9.47 |  149.39 ||   0.23 |    2.15 |
  |      32 ||  19.07 |  605.27 ||   0.24 |    4.38 |
  |      64 ||  65.40 | 3609.09 ||   0.33 |   11.27 |

Aside from the speedup, there should be no functional change as a result
of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/include/asm/cpufeature.h |  3 ++-
 arch/arm64/kernel/cpufeature.c      | 23 +++++------------------
 arch/arm64/kernel/traps.c           |  3 +++
 3 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index f73f11b55042..03d1c9d7af82 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -832,7 +832,8 @@ static inline bool system_supports_tlb_range(void)
 		cpus_have_const_cap(ARM64_HAS_TLB_RANGE);
 }
 
-extern int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
+int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
+bool try_emulate_mrs(struct pt_regs *regs, u32 isn);
 
 static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange)
 {
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 6062454a9067..5fc25cd9aedc 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -3411,35 +3411,22 @@ int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt)
 	return rc;
 }
 
-static int emulate_mrs(struct pt_regs *regs, u32 insn)
+bool try_emulate_mrs(struct pt_regs *regs, u32 insn)
 {
 	u32 sys_reg, rt;
 
+	if (compat_user_mode(regs) || !aarch64_insn_is_mrs(insn))
+		return false;
+
 	/*
 	 * sys_reg values are defined as used in mrs/msr instruction.
 	 * shift the imm value to get the encoding.
 	 */
 	sys_reg = (u32)aarch64_insn_decode_immediate(AARCH64_INSN_IMM_16, insn) << 5;
 	rt = aarch64_insn_decode_register(AARCH64_INSN_REGTYPE_RT, insn);
-	return do_emulate_mrs(regs, sys_reg, rt);
+	return do_emulate_mrs(regs, sys_reg, rt) == 0;
 }
 
-static struct undef_hook mrs_hook = {
-	.instr_mask = 0xffff0000,
-	.instr_val  = 0xd5380000,
-	.pstate_mask = PSR_AA32_MODE_MASK,
-	.pstate_val = PSR_MODE_EL0t,
-	.fn = emulate_mrs,
-};
-
-static int __init enable_mrs_emulation(void)
-{
-	register_undef_hook(&mrs_hook);
-	return 0;
-}
-
-core_initcall(enable_mrs_emulation);
-
 enum mitigation_state arm64_get_meltdown_state(void)
 {
 	if (__meltdown_safe)
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 4d51afd010e1..96eaf1aaec12 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -499,6 +499,9 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
 	if (user_insn_read(regs, &insn))
 		goto out_err;
 
+	if (try_emulate_mrs(regs, insn))
+		return;
+
 	if (call_undef_hook(regs, insn) == 0)
 		return;
 
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 6/9] arm64: armv8_deprecated: fold ops into insn_emulation
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
                   ` (4 preceding siblings ...)
  2022-10-19 14:41 ` [PATCH v2 5/9] arm64: rework EL0 MRS emulation Mark Rutland
@ 2022-10-19 14:41 ` Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 7/9] arm64: armv8_deprecated move emulation functions Mark Rutland
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

The code for emulating deprecated instructions has two related
structures: struct insn_emulation_ops and struct insn_emulation, where
each struct insn_emulation_ops is associated 1-1 with a struct
insn_emulation.

It would be simpler to combine the two into a single structure, removing
the need for (unconditional) dynamic allocation at boot time, and
simplifying some runtime pointer chasing.

This patch merges the two structures together.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/armv8_deprecated.c | 76 ++++++++++++----------------
 1 file changed, 33 insertions(+), 43 deletions(-)

diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index fb0e7c7b2e20..2a75e20a3913 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -41,16 +41,12 @@ enum legacy_insn_status {
 	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;
+	const char			*name;
+	struct list_head		node;
+	enum legacy_insn_status		status;
+	struct undef_hook		*hooks;
+	int				(*set_hw_mode)(bool enable);
 	int current_mode;
 	int min;
 	int max;
@@ -61,48 +57,48 @@ static int nr_insn_emulated __initdata;
 static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
 static DEFINE_MUTEX(insn_emulation_mutex);
 
-static void register_emulation_hooks(struct insn_emulation_ops *ops)
+static void register_emulation_hooks(struct insn_emulation *insn)
 {
 	struct undef_hook *hook;
 
-	BUG_ON(!ops->hooks);
+	BUG_ON(!insn->hooks);
 
-	for (hook = ops->hooks; hook->instr_mask; hook++)
+	for (hook = insn->hooks; hook->instr_mask; hook++)
 		register_undef_hook(hook);
 
-	pr_notice("Registered %s emulation handler\n", ops->name);
+	pr_notice("Registered %s emulation handler\n", insn->name);
 }
 
-static void remove_emulation_hooks(struct insn_emulation_ops *ops)
+static void remove_emulation_hooks(struct insn_emulation *insn)
 {
 	struct undef_hook *hook;
 
-	BUG_ON(!ops->hooks);
+	BUG_ON(!insn->hooks);
 
-	for (hook = ops->hooks; hook->instr_mask; hook++)
+	for (hook = insn->hooks; hook->instr_mask; hook++)
 		unregister_undef_hook(hook);
 
-	pr_notice("Removed %s emulation handler\n", ops->name);
+	pr_notice("Removed %s emulation handler\n", insn->name);
 }
 
 static void enable_insn_hw_mode(void *data)
 {
 	struct insn_emulation *insn = (struct insn_emulation *)data;
-	if (insn->ops->set_hw_mode)
-		insn->ops->set_hw_mode(true);
+	if (insn->set_hw_mode)
+		insn->set_hw_mode(true);
 }
 
 static void disable_insn_hw_mode(void *data)
 {
 	struct insn_emulation *insn = (struct insn_emulation *)data;
-	if (insn->ops->set_hw_mode)
-		insn->ops->set_hw_mode(false);
+	if (insn->set_hw_mode)
+		insn->set_hw_mode(false);
 }
 
 /* Run set_hw_mode(mode) on all active CPUs */
 static int run_all_cpu_set_hw_mode(struct insn_emulation *insn, bool enable)
 {
-	if (!insn->ops->set_hw_mode)
+	if (!insn->set_hw_mode)
 		return -EINVAL;
 	if (enable)
 		on_each_cpu(enable_insn_hw_mode, (void *)insn, true);
@@ -126,9 +122,9 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
 	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
 	list_for_each_entry(insn, &insn_emulation, node) {
 		bool enable = (insn->current_mode == INSN_HW);
-		if (insn->ops->set_hw_mode && insn->ops->set_hw_mode(enable)) {
+		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
 			pr_warn("CPU[%u] cannot support the emulation of %s",
-				cpu, insn->ops->name);
+				cpu, insn->name);
 			rc = -EINVAL;
 		}
 	}
@@ -145,11 +141,11 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
 	case INSN_UNDEF: /* Nothing to be done */
 		break;
 	case INSN_EMULATE:
-		remove_emulation_hooks(insn->ops);
+		remove_emulation_hooks(insn);
 		break;
 	case INSN_HW:
 		if (!run_all_cpu_set_hw_mode(insn, false))
-			pr_notice("Disabled %s support\n", insn->ops->name);
+			pr_notice("Disabled %s support\n", insn->name);
 		break;
 	}
 
@@ -157,31 +153,25 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
 	case INSN_UNDEF:
 		break;
 	case INSN_EMULATE:
-		register_emulation_hooks(insn->ops);
+		register_emulation_hooks(insn);
 		break;
 	case INSN_HW:
 		ret = run_all_cpu_set_hw_mode(insn, true);
 		if (!ret)
-			pr_notice("Enabled %s support\n", insn->ops->name);
+			pr_notice("Enabled %s support\n", insn->name);
 		break;
 	}
 
 	return ret;
 }
 
-static void __init register_insn_emulation(struct insn_emulation_ops *ops)
+static void __init register_insn_emulation(struct insn_emulation *insn)
 {
 	unsigned long flags;
-	struct insn_emulation *insn;
-
-	insn = kzalloc(sizeof(*insn), GFP_KERNEL);
-	if (!insn)
-		return;
 
-	insn->ops = ops;
 	insn->min = INSN_UNDEF;
 
-	switch (ops->status) {
+	switch (insn->status) {
 	case INSN_DEPRECATED:
 		insn->current_mode = INSN_EMULATE;
 		/* Disable the HW mode if it was turned on at early boot time */
@@ -247,7 +237,7 @@ static void __init register_insn_emulation_sysctl(void)
 		sysctl->mode = 0644;
 		sysctl->maxlen = sizeof(int);
 
-		sysctl->procname = insn->ops->name;
+		sysctl->procname = insn->name;
 		sysctl->data = &insn->current_mode;
 		sysctl->extra1 = &insn->min;
 		sysctl->extra2 = &insn->max;
@@ -445,7 +435,7 @@ static struct undef_hook swp_hooks[] = {
 	{ }
 };
 
-static struct insn_emulation_ops swp_ops = {
+static struct insn_emulation insn_swp = {
 	.name = "swp",
 	.status = INSN_OBSOLETE,
 	.hooks = swp_hooks,
@@ -532,7 +522,7 @@ static struct undef_hook cp15_barrier_hooks[] = {
 	{ }
 };
 
-static struct insn_emulation_ops cp15_barrier_ops = {
+static struct insn_emulation insn_cp15_barrier = {
 	.name = "cp15_barrier",
 	.status = INSN_DEPRECATED,
 	.hooks = cp15_barrier_hooks,
@@ -605,7 +595,7 @@ static struct undef_hook setend_hooks[] = {
 	{}
 };
 
-static struct insn_emulation_ops setend_ops = {
+static struct insn_emulation insn_setend = {
 	.name = "setend",
 	.status = INSN_DEPRECATED,
 	.hooks = setend_hooks,
@@ -619,14 +609,14 @@ static struct insn_emulation_ops setend_ops = {
 static int __init armv8_deprecated_init(void)
 {
 	if (IS_ENABLED(CONFIG_SWP_EMULATION))
-		register_insn_emulation(&swp_ops);
+		register_insn_emulation(&insn_swp);
 
 	if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
-		register_insn_emulation(&cp15_barrier_ops);
+		register_insn_emulation(&insn_cp15_barrier);
 
 	if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
 		if (system_supports_mixed_endian_el0())
-			register_insn_emulation(&setend_ops);
+			register_insn_emulation(&insn_setend);
 		else
 			pr_info("setend instruction emulation is not supported on this system\n");
 	}
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 7/9] arm64: armv8_deprecated move emulation functions
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
                   ` (5 preceding siblings ...)
  2022-10-19 14:41 ` [PATCH v2 6/9] arm64: armv8_deprecated: fold ops into insn_emulation Mark Rutland
@ 2022-10-19 14:41 ` Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 8/9] arm64: armv8_deprecated: move aarch32 helper earlier Mark Rutland
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

Subsequent patches will rework the logic in armv8_deprecated.c.

In preparation for subsequent changes, this patch moves the emulation
logic earlier in the file, and moves the infrastructure later in the
file. This will make subsequent diffs simpler and easier to read.

This is purely a move. There should be no functional change as a result
of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/armv8_deprecated.c | 394 +++++++++++++--------------
 1 file changed, 197 insertions(+), 197 deletions(-)

diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 2a75e20a3913..792b89929c04 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -52,203 +52,6 @@ struct insn_emulation {
 	int max;
 };
 
-static LIST_HEAD(insn_emulation);
-static int nr_insn_emulated __initdata;
-static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
-static DEFINE_MUTEX(insn_emulation_mutex);
-
-static void register_emulation_hooks(struct insn_emulation *insn)
-{
-	struct undef_hook *hook;
-
-	BUG_ON(!insn->hooks);
-
-	for (hook = insn->hooks; hook->instr_mask; hook++)
-		register_undef_hook(hook);
-
-	pr_notice("Registered %s emulation handler\n", insn->name);
-}
-
-static void remove_emulation_hooks(struct insn_emulation *insn)
-{
-	struct undef_hook *hook;
-
-	BUG_ON(!insn->hooks);
-
-	for (hook = insn->hooks; hook->instr_mask; hook++)
-		unregister_undef_hook(hook);
-
-	pr_notice("Removed %s emulation handler\n", insn->name);
-}
-
-static void enable_insn_hw_mode(void *data)
-{
-	struct insn_emulation *insn = (struct insn_emulation *)data;
-	if (insn->set_hw_mode)
-		insn->set_hw_mode(true);
-}
-
-static void disable_insn_hw_mode(void *data)
-{
-	struct insn_emulation *insn = (struct insn_emulation *)data;
-	if (insn->set_hw_mode)
-		insn->set_hw_mode(false);
-}
-
-/* Run set_hw_mode(mode) on all active CPUs */
-static int run_all_cpu_set_hw_mode(struct insn_emulation *insn, bool enable)
-{
-	if (!insn->set_hw_mode)
-		return -EINVAL;
-	if (enable)
-		on_each_cpu(enable_insn_hw_mode, (void *)insn, true);
-	else
-		on_each_cpu(disable_insn_hw_mode, (void *)insn, true);
-	return 0;
-}
-
-/*
- * Run set_hw_mode for all insns on a starting CPU.
- * Returns:
- *  0 		- If all the hooks ran successfully.
- * -EINVAL	- At least one hook is not supported by the CPU.
- */
-static int run_all_insn_set_hw_mode(unsigned int cpu)
-{
-	int rc = 0;
-	unsigned long flags;
-	struct insn_emulation *insn;
-
-	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
-	list_for_each_entry(insn, &insn_emulation, node) {
-		bool enable = (insn->current_mode == INSN_HW);
-		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
-			pr_warn("CPU[%u] cannot support the emulation of %s",
-				cpu, insn->name);
-			rc = -EINVAL;
-		}
-	}
-	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
-	return rc;
-}
-
-static int update_insn_emulation_mode(struct insn_emulation *insn,
-				       enum insn_emulation_mode prev)
-{
-	int ret = 0;
-
-	switch (prev) {
-	case INSN_UNDEF: /* Nothing to be done */
-		break;
-	case INSN_EMULATE:
-		remove_emulation_hooks(insn);
-		break;
-	case INSN_HW:
-		if (!run_all_cpu_set_hw_mode(insn, false))
-			pr_notice("Disabled %s support\n", insn->name);
-		break;
-	}
-
-	switch (insn->current_mode) {
-	case INSN_UNDEF:
-		break;
-	case INSN_EMULATE:
-		register_emulation_hooks(insn);
-		break;
-	case INSN_HW:
-		ret = run_all_cpu_set_hw_mode(insn, true);
-		if (!ret)
-			pr_notice("Enabled %s support\n", insn->name);
-		break;
-	}
-
-	return ret;
-}
-
-static void __init register_insn_emulation(struct insn_emulation *insn)
-{
-	unsigned long flags;
-
-	insn->min = INSN_UNDEF;
-
-	switch (insn->status) {
-	case INSN_DEPRECATED:
-		insn->current_mode = INSN_EMULATE;
-		/* Disable the HW mode if it was turned on at early boot time */
-		run_all_cpu_set_hw_mode(insn, false);
-		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 *buffer, size_t *lenp,
-				  loff_t *ppos)
-{
-	int ret = 0;
-	struct insn_emulation *insn = container_of(table->data, struct insn_emulation, current_mode);
-	enum insn_emulation_mode prev_mode = insn->current_mode;
-
-	mutex_lock(&insn_emulation_mutex);
-	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
-
-	if (ret || !write || prev_mode == insn->current_mode)
-		goto ret;
-
-	ret = update_insn_emulation_mode(insn, prev_mode);
-	if (ret) {
-		/* Mode change failed, revert to previous mode. */
-		insn->current_mode = prev_mode;
-		update_insn_emulation_mode(insn, INSN_UNDEF);
-	}
-ret:
-	mutex_unlock(&insn_emulation_mutex);
-	return ret;
-}
-
-static void __init register_insn_emulation_sysctl(void)
-{
-	unsigned long flags;
-	int i = 0;
-	struct insn_emulation *insn;
-	struct ctl_table *insns_sysctl, *sysctl;
-
-	insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
-			       GFP_KERNEL);
-	if (!insns_sysctl)
-		return;
-
-	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->name;
-		sysctl->data = &insn->current_mode;
-		sysctl->extra1 = &insn->min;
-		sysctl->extra2 = &insn->max;
-		sysctl->proc_handler = emulation_proc_handler;
-		i++;
-	}
-	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
-
-	register_sysctl("abi", insns_sysctl);
-}
-
 /*
  *  Implement emulation of the SWP/SWPB instructions using load-exclusive and
  *  store-exclusive.
@@ -602,6 +405,203 @@ static struct insn_emulation insn_setend = {
 	.set_hw_mode = setend_set_hw_mode,
 };
 
+static LIST_HEAD(insn_emulation);
+static int nr_insn_emulated __initdata;
+static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
+static DEFINE_MUTEX(insn_emulation_mutex);
+
+static void register_emulation_hooks(struct insn_emulation *insn)
+{
+	struct undef_hook *hook;
+
+	BUG_ON(!insn->hooks);
+
+	for (hook = insn->hooks; hook->instr_mask; hook++)
+		register_undef_hook(hook);
+
+	pr_notice("Registered %s emulation handler\n", insn->name);
+}
+
+static void remove_emulation_hooks(struct insn_emulation *insn)
+{
+	struct undef_hook *hook;
+
+	BUG_ON(!insn->hooks);
+
+	for (hook = insn->hooks; hook->instr_mask; hook++)
+		unregister_undef_hook(hook);
+
+	pr_notice("Removed %s emulation handler\n", insn->name);
+}
+
+static void enable_insn_hw_mode(void *data)
+{
+	struct insn_emulation *insn = (struct insn_emulation *)data;
+	if (insn->set_hw_mode)
+		insn->set_hw_mode(true);
+}
+
+static void disable_insn_hw_mode(void *data)
+{
+	struct insn_emulation *insn = (struct insn_emulation *)data;
+	if (insn->set_hw_mode)
+		insn->set_hw_mode(false);
+}
+
+/* Run set_hw_mode(mode) on all active CPUs */
+static int run_all_cpu_set_hw_mode(struct insn_emulation *insn, bool enable)
+{
+	if (!insn->set_hw_mode)
+		return -EINVAL;
+	if (enable)
+		on_each_cpu(enable_insn_hw_mode, (void *)insn, true);
+	else
+		on_each_cpu(disable_insn_hw_mode, (void *)insn, true);
+	return 0;
+}
+
+/*
+ * Run set_hw_mode for all insns on a starting CPU.
+ * Returns:
+ *  0 		- If all the hooks ran successfully.
+ * -EINVAL	- At least one hook is not supported by the CPU.
+ */
+static int run_all_insn_set_hw_mode(unsigned int cpu)
+{
+	int rc = 0;
+	unsigned long flags;
+	struct insn_emulation *insn;
+
+	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
+	list_for_each_entry(insn, &insn_emulation, node) {
+		bool enable = (insn->current_mode == INSN_HW);
+		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
+			pr_warn("CPU[%u] cannot support the emulation of %s",
+				cpu, insn->name);
+			rc = -EINVAL;
+		}
+	}
+	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+	return rc;
+}
+
+static int update_insn_emulation_mode(struct insn_emulation *insn,
+				       enum insn_emulation_mode prev)
+{
+	int ret = 0;
+
+	switch (prev) {
+	case INSN_UNDEF: /* Nothing to be done */
+		break;
+	case INSN_EMULATE:
+		remove_emulation_hooks(insn);
+		break;
+	case INSN_HW:
+		if (!run_all_cpu_set_hw_mode(insn, false))
+			pr_notice("Disabled %s support\n", insn->name);
+		break;
+	}
+
+	switch (insn->current_mode) {
+	case INSN_UNDEF:
+		break;
+	case INSN_EMULATE:
+		register_emulation_hooks(insn);
+		break;
+	case INSN_HW:
+		ret = run_all_cpu_set_hw_mode(insn, true);
+		if (!ret)
+			pr_notice("Enabled %s support\n", insn->name);
+		break;
+	}
+
+	return ret;
+}
+
+static void __init register_insn_emulation(struct insn_emulation *insn)
+{
+	unsigned long flags;
+
+	insn->min = INSN_UNDEF;
+
+	switch (insn->status) {
+	case INSN_DEPRECATED:
+		insn->current_mode = INSN_EMULATE;
+		/* Disable the HW mode if it was turned on at early boot time */
+		run_all_cpu_set_hw_mode(insn, false);
+		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 *buffer, size_t *lenp,
+				  loff_t *ppos)
+{
+	int ret = 0;
+	struct insn_emulation *insn = container_of(table->data, struct insn_emulation, current_mode);
+	enum insn_emulation_mode prev_mode = insn->current_mode;
+
+	mutex_lock(&insn_emulation_mutex);
+	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+
+	if (ret || !write || prev_mode == insn->current_mode)
+		goto ret;
+
+	ret = update_insn_emulation_mode(insn, prev_mode);
+	if (ret) {
+		/* Mode change failed, revert to previous mode. */
+		insn->current_mode = prev_mode;
+		update_insn_emulation_mode(insn, INSN_UNDEF);
+	}
+ret:
+	mutex_unlock(&insn_emulation_mutex);
+	return ret;
+}
+
+static void __init register_insn_emulation_sysctl(void)
+{
+	unsigned long flags;
+	int i = 0;
+	struct insn_emulation *insn;
+	struct ctl_table *insns_sysctl, *sysctl;
+
+	insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
+			       GFP_KERNEL);
+	if (!insns_sysctl)
+		return;
+
+	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->name;
+		sysctl->data = &insn->current_mode;
+		sysctl->extra1 = &insn->min;
+		sysctl->extra2 = &insn->max;
+		sysctl->proc_handler = emulation_proc_handler;
+		i++;
+	}
+	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+
+	register_sysctl("abi", insns_sysctl);
+}
+
 /*
  * Invoked as core_initcall, which guarantees that the instruction
  * emulation is ready for userspace.
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 8/9] arm64: armv8_deprecated: move aarch32 helper earlier
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
                   ` (6 preceding siblings ...)
  2022-10-19 14:41 ` [PATCH v2 7/9] arm64: armv8_deprecated move emulation functions Mark Rutland
@ 2022-10-19 14:41 ` Mark Rutland
  2022-10-19 14:41 ` [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling Mark Rutland
  2022-11-14 14:20 ` [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Will Deacon
  9 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

Subsequent patches will rework the logic in armv8_deprecated.c.

In preparation for subsequent changes, this patch moves some shared logic
earlier in the file. This will make subsequent diffs simpler and easier to
read.

At the same time, drop the `__kprobes` annotation from
aarch32_check_condition(), as this is only used for traps from compat
userspace, and has no risk of recursion within kprobes. As this is the
last kprobes annotation in armve8_deprecated.c, we no longer need to
include <asm/kprobes.h>.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
---
 arch/arm64/kernel/armv8_deprecated.c | 39 ++++++++++++++--------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 792b89929c04..7f2ce49dbf97 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -17,7 +17,6 @@
 #include <asm/sysreg.h>
 #include <asm/system_misc.h>
 #include <asm/traps.h>
-#include <asm/kprobes.h>
 
 #define CREATE_TRACE_POINTS
 #include "trace-events-emulation.h"
@@ -52,6 +51,25 @@ struct insn_emulation {
 	int max;
 };
 
+#define ARM_OPCODE_CONDTEST_FAIL   0
+#define ARM_OPCODE_CONDTEST_PASS   1
+#define ARM_OPCODE_CONDTEST_UNCOND 2
+
+#define	ARM_OPCODE_CONDITION_UNCOND	0xf
+
+static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
+{
+	u32 cc_bits  = opcode >> 28;
+
+	if (cc_bits != ARM_OPCODE_CONDITION_UNCOND) {
+		if ((*aarch32_opcode_cond_checks[cc_bits])(psr))
+			return ARM_OPCODE_CONDTEST_PASS;
+		else
+			return ARM_OPCODE_CONDTEST_FAIL;
+	}
+	return ARM_OPCODE_CONDTEST_UNCOND;
+}
+
 /*
  *  Implement emulation of the SWP/SWPB instructions using load-exclusive and
  *  store-exclusive.
@@ -132,25 +150,6 @@ static int emulate_swpX(unsigned int address, unsigned int *data,
 	return res;
 }
 
-#define ARM_OPCODE_CONDTEST_FAIL   0
-#define ARM_OPCODE_CONDTEST_PASS   1
-#define ARM_OPCODE_CONDTEST_UNCOND 2
-
-#define	ARM_OPCODE_CONDITION_UNCOND	0xf
-
-static unsigned int __kprobes aarch32_check_condition(u32 opcode, u32 psr)
-{
-	u32 cc_bits  = opcode >> 28;
-
-	if (cc_bits != ARM_OPCODE_CONDITION_UNCOND) {
-		if ((*aarch32_opcode_cond_checks[cc_bits])(psr))
-			return ARM_OPCODE_CONDTEST_PASS;
-		else
-			return ARM_OPCODE_CONDTEST_FAIL;
-	}
-	return ARM_OPCODE_CONDTEST_UNCOND;
-}
-
 /*
  * swp_handler logs the id of calling process, dissects the instruction, sanity
  * checks the memory location, calls emulate_swpX for the actual operation and
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
                   ` (7 preceding siblings ...)
  2022-10-19 14:41 ` [PATCH v2 8/9] arm64: armv8_deprecated: move aarch32 helper earlier Mark Rutland
@ 2022-10-19 14:41 ` Mark Rutland
  2023-02-03 10:08   ` Ruan Jinjie
  2022-11-14 14:20 ` [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Will Deacon
  9 siblings, 1 reply; 16+ messages in thread
From: Mark Rutland @ 2022-10-19 14:41 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: catalin.marinas, james.morse, joey.gouly, mark.rutland, peterz, will

Support for deprecated instructions can be enabled or disabled at
runtime. To handle this, the code in armv8_deprecated.c registers and
unregisters undef_hooks, and makes cross CPU calls to configure HW
support. This is rather complicated, and the synchronization required to
make this safe ends up serializing the handling of instructions which
have been trapped.

This patch simplifies the deprecated instruction handling by removing
the dynamic registration and unregistration, and changing the trap
handling code to determine whether a handler should be invoked. This
removes the need for dynamic list management, and simplifies the locking
requirements, making it possible to handle trapped instructions entirely
in parallel.

Where changing the emulation state requires a cross-call, this is
serialized by locally disabling interrupts, ensuring that the CPU is not
left in an inconsistent state.

To simplify sysctl management, each insn_emulation is given a separate
sysctl table, permitting these to be registered separately. The core
sysctl code will iterate over all of these when walking sysfs.

I've tested this with userspace programs which use each of the
deprecated instructions, and I've concurrently modified the support
level for each of the features back-and-forth between HW and emulated to
check that there are no spurious SIGILLs sent to userspace when the
support level is changed.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: James Morse <james.morse@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/include/asm/traps.h       |  19 +-
 arch/arm64/kernel/armv8_deprecated.c | 282 +++++++++++++--------------
 arch/arm64/kernel/traps.c            |  40 +---
 3 files changed, 149 insertions(+), 192 deletions(-)

diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 6e5826470bea..1f361e2da516 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -13,17 +13,16 @@
 
 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);
-};
+#ifdef CONFIG_ARMV8_DEPRECATED
+bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn);
+#else
+static inline bool
+try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
+{
+	return false;
+}
+#endif /* CONFIG_ARMV8_DEPRECATED */
 
-void register_undef_hook(struct undef_hook *hook);
-void unregister_undef_hook(struct undef_hook *hook);
 void force_signal_inject(int signal, int code, unsigned long address, unsigned long err);
 void arm64_notify_segfault(unsigned long addr);
 void arm64_force_sig_fault(int signo, int code, unsigned long far, const char *str);
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 7f2ce49dbf97..ed0788cf6bbb 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -38,17 +38,24 @@ enum insn_emulation_mode {
 enum legacy_insn_status {
 	INSN_DEPRECATED,
 	INSN_OBSOLETE,
+	INSN_UNAVAILABLE,
 };
 
 struct insn_emulation {
 	const char			*name;
-	struct list_head		node;
 	enum legacy_insn_status		status;
-	struct undef_hook		*hooks;
+	bool				(*try_emulate)(struct pt_regs *regs,
+						       u32 insn);
 	int				(*set_hw_mode)(bool enable);
+
 	int current_mode;
 	int min;
 	int max;
+
+	/*
+	 * sysctl for this emulation + a sentinal entry.
+	 */
+	struct ctl_table sysctl[2];
 };
 
 #define ARM_OPCODE_CONDTEST_FAIL   0
@@ -70,6 +77,7 @@ static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
 	return ARM_OPCODE_CONDTEST_UNCOND;
 }
 
+#ifdef CONFIG_SWP_EMULATION
 /*
  *  Implement emulation of the SWP/SWPB instructions using load-exclusive and
  *  store-exclusive.
@@ -222,28 +230,27 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
 	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	= PSR_AA32_MODE_MASK,
-		.pstate_val	= PSR_AA32_MODE_USR,
-		.fn		= swp_handler
-	},
-	{ }
-};
+static bool try_emulate_swp(struct pt_regs *regs, u32 insn)
+{
+	/* SWP{B} only exists in ARM state and does not exist in Thumb */
+	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
+		return false;
+
+	if ((insn & 0x0fb00ff0) != 0x01000090)
+		return false;
+
+	return swp_handler(regs, insn) == 0;
+}
 
 static struct insn_emulation insn_swp = {
 	.name = "swp",
 	.status = INSN_OBSOLETE,
-	.hooks = swp_hooks,
+	.try_emulate = try_emulate_swp,
 	.set_hw_mode = NULL,
 };
+#endif /* CONFIG_SWP_EMULATION */
 
+#ifdef CONFIG_CP15_BARRIER_EMULATION
 static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
 {
 	perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
@@ -306,31 +313,29 @@ static int cp15_barrier_set_hw_mode(bool enable)
 	return 0;
 }
 
-static struct undef_hook cp15_barrier_hooks[] = {
-	{
-		.instr_mask	= 0x0fff0fdf,
-		.instr_val	= 0x0e070f9a,
-		.pstate_mask	= PSR_AA32_MODE_MASK,
-		.pstate_val	= PSR_AA32_MODE_USR,
-		.fn		= cp15barrier_handler,
-	},
-	{
-		.instr_mask	= 0x0fff0fff,
-		.instr_val	= 0x0e070f95,
-		.pstate_mask	= PSR_AA32_MODE_MASK,
-		.pstate_val	= PSR_AA32_MODE_USR,
-		.fn		= cp15barrier_handler,
-	},
-	{ }
-};
+static bool try_emulate_cp15_barrier(struct pt_regs *regs, u32 insn)
+{
+	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
+		return false;
+
+	if ((insn & 0x0fff0fdf) == 0x0e070f9a)
+		return cp15barrier_handler(regs, insn) == 0;
+
+	if ((insn & 0x0fff0fff) == 0x0e070f95)
+		return cp15barrier_handler(regs, insn) == 0;
+
+	return false;
+}
 
 static struct insn_emulation insn_cp15_barrier = {
 	.name = "cp15_barrier",
 	.status = INSN_DEPRECATED,
-	.hooks = cp15_barrier_hooks,
+	.try_emulate = try_emulate_cp15_barrier,
 	.set_hw_mode = cp15_barrier_set_hw_mode,
 };
+#endif /* CONFIG_CP15_BARRIER_EMULATION */
 
+#ifdef CONFIG_SETEND_EMULATION
 static int setend_set_hw_mode(bool enable)
 {
 	if (!cpu_supports_mixed_endian_el0())
@@ -378,61 +383,41 @@ static int t16_setend_handler(struct pt_regs *regs, u32 instr)
 	return rc;
 }
 
-static struct undef_hook setend_hooks[] = {
-	{
-		.instr_mask	= 0xfffffdff,
-		.instr_val	= 0xf1010000,
-		.pstate_mask	= PSR_AA32_MODE_MASK,
-		.pstate_val	= PSR_AA32_MODE_USR,
-		.fn		= a32_setend_handler,
-	},
-	{
-		/* Thumb mode */
-		.instr_mask	= 0xfffffff7,
-		.instr_val	= 0x0000b650,
-		.pstate_mask	= (PSR_AA32_T_BIT | PSR_AA32_MODE_MASK),
-		.pstate_val	= (PSR_AA32_T_BIT | PSR_AA32_MODE_USR),
-		.fn		= t16_setend_handler,
-	},
-	{}
-};
+static bool try_emulate_setend(struct pt_regs *regs, u32 insn)
+{
+	if (compat_thumb_mode(regs) &&
+	    (insn & 0xfffffff7) == 0x0000b650)
+		return t16_setend_handler(regs, insn) == 0;
+
+	if (compat_user_mode(regs) &&
+	    (insn & 0xfffffdff) == 0xf1010000)
+		return a32_setend_handler(regs, insn) == 0;
+
+	return false;
+}
 
 static struct insn_emulation insn_setend = {
 	.name = "setend",
 	.status = INSN_DEPRECATED,
-	.hooks = setend_hooks,
+	.try_emulate = try_emulate_setend,
 	.set_hw_mode = setend_set_hw_mode,
 };
+#endif /* CONFIG_SETEND_EMULATION */
+
+static struct insn_emulation *insn_emulations[] = {
+#ifdef CONFIG_SWP_EMULATION
+	&insn_swp,
+#endif
+#ifdef CONFIG_CP15_BARRIER_EMULATION
+	&insn_cp15_barrier,
+#endif
+#ifdef CONFIG_SETEND_EMULATION
+	&insn_setend,
+#endif
+};
 
-static LIST_HEAD(insn_emulation);
-static int nr_insn_emulated __initdata;
-static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
 static DEFINE_MUTEX(insn_emulation_mutex);
 
-static void register_emulation_hooks(struct insn_emulation *insn)
-{
-	struct undef_hook *hook;
-
-	BUG_ON(!insn->hooks);
-
-	for (hook = insn->hooks; hook->instr_mask; hook++)
-		register_undef_hook(hook);
-
-	pr_notice("Registered %s emulation handler\n", insn->name);
-}
-
-static void remove_emulation_hooks(struct insn_emulation *insn)
-{
-	struct undef_hook *hook;
-
-	BUG_ON(!insn->hooks);
-
-	for (hook = insn->hooks; hook->instr_mask; hook++)
-		unregister_undef_hook(hook);
-
-	pr_notice("Removed %s emulation handler\n", insn->name);
-}
-
 static void enable_insn_hw_mode(void *data)
 {
 	struct insn_emulation *insn = (struct insn_emulation *)data;
@@ -469,18 +454,24 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
 {
 	int rc = 0;
 	unsigned long flags;
-	struct insn_emulation *insn;
 
-	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
-	list_for_each_entry(insn, &insn_emulation, node) {
-		bool enable = (insn->current_mode == INSN_HW);
+	/*
+	 * Disable IRQs to serialize against an IPI from
+	 * run_all_cpu_set_hw_mode(), ensuring the HW is programmed to the most
+	 * recent enablement state if the two race with one another.
+	 */
+	local_irq_save(flags);
+	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
+		struct insn_emulation *insn = insn_emulations[i];
+		bool enable = READ_ONCE(insn->current_mode) == INSN_HW;
 		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
 			pr_warn("CPU[%u] cannot support the emulation of %s",
 				cpu, insn->name);
 			rc = -EINVAL;
 		}
 	}
-	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
+	local_irq_restore(flags);
+
 	return rc;
 }
 
@@ -493,7 +484,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
 	case INSN_UNDEF: /* Nothing to be done */
 		break;
 	case INSN_EMULATE:
-		remove_emulation_hooks(insn);
 		break;
 	case INSN_HW:
 		if (!run_all_cpu_set_hw_mode(insn, false))
@@ -505,7 +495,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
 	case INSN_UNDEF:
 		break;
 	case INSN_EMULATE:
-		register_emulation_hooks(insn);
 		break;
 	case INSN_HW:
 		ret = run_all_cpu_set_hw_mode(insn, true);
@@ -517,34 +506,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
 	return ret;
 }
 
-static void __init register_insn_emulation(struct insn_emulation *insn)
-{
-	unsigned long flags;
-
-	insn->min = INSN_UNDEF;
-
-	switch (insn->status) {
-	case INSN_DEPRECATED:
-		insn->current_mode = INSN_EMULATE;
-		/* Disable the HW mode if it was turned on at early boot time */
-		run_all_cpu_set_hw_mode(insn, false);
-		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 *buffer, size_t *lenp,
 				  loff_t *ppos)
@@ -562,7 +523,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
 	ret = update_insn_emulation_mode(insn, prev_mode);
 	if (ret) {
 		/* Mode change failed, revert to previous mode. */
-		insn->current_mode = prev_mode;
+		WRITE_ONCE(insn->current_mode, prev_mode);
 		update_insn_emulation_mode(insn, INSN_UNDEF);
 	}
 ret:
@@ -570,21 +531,34 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
 	return ret;
 }
 
-static void __init register_insn_emulation_sysctl(void)
+static void __init register_insn_emulation(struct insn_emulation *insn)
 {
-	unsigned long flags;
-	int i = 0;
-	struct insn_emulation *insn;
-	struct ctl_table *insns_sysctl, *sysctl;
+	struct ctl_table *sysctl;
 
-	insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
-			       GFP_KERNEL);
-	if (!insns_sysctl)
-		return;
+	insn->min = INSN_UNDEF;
 
-	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
-	list_for_each_entry(insn, &insn_emulation, node) {
-		sysctl = &insns_sysctl[i];
+	switch (insn->status) {
+	case INSN_DEPRECATED:
+		insn->current_mode = INSN_EMULATE;
+		/* Disable the HW mode if it was turned on at early boot time */
+		run_all_cpu_set_hw_mode(insn, false);
+		insn->max = INSN_HW;
+		break;
+	case INSN_OBSOLETE:
+		insn->current_mode = INSN_UNDEF;
+		insn->max = INSN_EMULATE;
+		break;
+	case INSN_UNAVAILABLE:
+		insn->current_mode = INSN_UNDEF;
+		insn->max = INSN_UNDEF;
+		break;
+	}
+
+	/* Program the HW if required */
+	update_insn_emulation_mode(insn, INSN_UNDEF);
+
+	if (insn->status != INSN_UNAVAILABLE) {
+		sysctl = &insn->sysctl[0];
 
 		sysctl->mode = 0644;
 		sysctl->maxlen = sizeof(int);
@@ -594,11 +568,32 @@ static void __init register_insn_emulation_sysctl(void)
 		sysctl->extra1 = &insn->min;
 		sysctl->extra2 = &insn->max;
 		sysctl->proc_handler = emulation_proc_handler;
-		i++;
+
+		register_sysctl("abi", sysctl);
+	}
+}
+
+bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
+{
+	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
+		struct insn_emulation *ie = insn_emulations[i];
+
+		if (ie->status == INSN_UNAVAILABLE)
+			continue;
+
+		/*
+		 * A trap may race with the mode being changed
+		 * INSN_EMULATE<->INSN_HW. Try to emulate the instruction to
+		 * avoid a spurious UNDEF.
+		 */
+		if (READ_ONCE(ie->current_mode) == INSN_UNDEF)
+			continue;
+
+		if (ie->try_emulate(regs, insn))
+			return true;
 	}
-	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
 
-	register_sysctl("abi", insns_sysctl);
+	return false;
 }
 
 /*
@@ -607,24 +602,25 @@ static void __init register_insn_emulation_sysctl(void)
  */
 static int __init armv8_deprecated_init(void)
 {
-	if (IS_ENABLED(CONFIG_SWP_EMULATION))
-		register_insn_emulation(&insn_swp);
+#ifdef CONFIG_SETEND_EMULATION
+	if (!system_supports_mixed_endian_el0()) {
+		insn_setend.status = INSN_UNAVAILABLE;
+		pr_info("setend instruction emulation is not supported on this system\n");
+	}
 
-	if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
-		register_insn_emulation(&insn_cp15_barrier);
+#endif
+	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
+		struct insn_emulation *ie = insn_emulations[i];
 
-	if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
-		if (system_supports_mixed_endian_el0())
-			register_insn_emulation(&insn_setend);
-		else
-			pr_info("setend instruction emulation is not supported on this system\n");
+		if (ie->status == INSN_UNAVAILABLE)
+			continue;
+
+		register_insn_emulation(ie);
 	}
 
 	cpuhp_setup_state_nocalls(CPUHP_AP_ARM64_ISNDEP_STARTING,
 				  "arm64/isndep:starting",
 				  run_all_insn_set_hw_mode, NULL);
-	register_insn_emulation_sysctl();
-
 	return 0;
 }
 
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 96eaf1aaec12..4c0caa589e12 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -373,27 +373,6 @@ void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
 		regs->pstate &= ~PSR_BTYPE_MASK;
 }
 
-static LIST_HEAD(undef_hook);
-static DEFINE_RAW_SPINLOCK(undef_lock);
-
-void 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);
-}
-
-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 user_insn_read(struct pt_regs *regs, u32 *insnp)
 {
 	u32 instr;
@@ -425,23 +404,6 @@ static int user_insn_read(struct pt_regs *regs, u32 *insnp)
 	return 0;
 }
 
-static int call_undef_hook(struct pt_regs *regs, u32 instr)
-{
-	struct undef_hook *hook;
-	unsigned long flags;
-	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
-
-	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);
-
-	return fn ? fn(regs, instr) : 1;
-}
-
 void force_signal_inject(int signal, int code, unsigned long address, unsigned long err)
 {
 	const char *desc;
@@ -502,7 +464,7 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
 	if (try_emulate_mrs(regs, insn))
 		return;
 
-	if (call_undef_hook(regs, insn) == 0)
+	if (try_emulate_armv8_deprecated(regs, insn))
 		return;
 
 out_err:
-- 
2.30.2


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps
  2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
                   ` (8 preceding siblings ...)
  2022-10-19 14:41 ` [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling Mark Rutland
@ 2022-11-14 14:20 ` Will Deacon
  9 siblings, 0 replies; 16+ messages in thread
From: Will Deacon @ 2022-11-14 14:20 UTC (permalink / raw)
  To: Mark Rutland, linux-arm-kernel
  Cc: catalin.marinas, kernel-team, Will Deacon, peterz, joey.gouly,
	james.morse

On Wed, 19 Oct 2022 15:41:14 +0100, Mark Rutland wrote:
> This series reworks the way UNDEFINED instruction traps are handled,
> removing some dynamic data structure manipulation and related locking.
> This makes the code a bit simpler and removes some unnecessary
> bottlenecks when traps are handled.
> 
> My original aim for this series was to get rid of the RCU_NONIDLE() call
> in cpu_suspend(), requiring the removal of the undef_hook list
> manipulation. In the process of looking at that I figured we had a set
> of related problems with UNDEF handling and deprecated instruction
> handling, which this series addresses. This series does not remove the
> RCU_NONIDLE() call from cpu_suspend() as removing that will require
> further rework of the suspend code (e.g. for noinstr safety).
> 
> [...]

Applied to arm64 (for-next/undef-traps), thanks!

[1/9] arm64: allow kprobes on EL0 handlers
      https://git.kernel.org/arm64/c/5111047c6c82
[2/9] arm64: split EL0/EL1 UNDEF handlers
      https://git.kernel.org/arm64/c/59a75653d8f9
[3/9] arm64: factor out EL1 SSBS emulation hook
      https://git.kernel.org/arm64/c/e3bee62254e1
[4/9] arm64: factor insn read out of call_undef_hook()
      https://git.kernel.org/arm64/c/2bef997bb4d4
[5/9] arm64: rework EL0 MRS emulation
      https://git.kernel.org/arm64/c/796ab8bb33c5
[6/9] arm64: armv8_deprecated: fold ops into insn_emulation
      https://git.kernel.org/arm64/c/e86cf2e60826
[7/9] arm64: armv8_deprecated move emulation functions
      https://git.kernel.org/arm64/c/270af901e2c2
[8/9] arm64: armv8_deprecated: move aarch32 helper earlier
      https://git.kernel.org/arm64/c/37402c31aa9c
[9/9] arm64: armv8_deprecated: rework deprected instruction handling
      https://git.kernel.org/arm64/c/b6d6c0923f59

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

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

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling
  2022-10-19 14:41 ` [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling Mark Rutland
@ 2023-02-03 10:08   ` Ruan Jinjie
  2023-02-03 17:27     ` Mark Rutland
  0 siblings, 1 reply; 16+ messages in thread
From: Ruan Jinjie @ 2023-02-03 10:08 UTC (permalink / raw)
  To: linux-arm-kernel



On 2022/10/19 22:41, Mark Rutland wrote:
> Support for deprecated instructions can be enabled or disabled at
> runtime. To handle this, the code in armv8_deprecated.c registers and
> unregisters undef_hooks, and makes cross CPU calls to configure HW
> support. This is rather complicated, and the synchronization required to
> make this safe ends up serializing the handling of instructions which
> have been trapped.
> 
> This patch simplifies the deprecated instruction handling by removing
> the dynamic registration and unregistration, and changing the trap
> handling code to determine whether a handler should be invoked. This
> removes the need for dynamic list management, and simplifies the locking
> requirements, making it possible to handle trapped instructions entirely
> in parallel.
> 
> Where changing the emulation state requires a cross-call, this is
> serialized by locally disabling interrupts, ensuring that the CPU is not
> left in an inconsistent state.
> 
> To simplify sysctl management, each insn_emulation is given a separate
> sysctl table, permitting these to be registered separately. The core
> sysctl code will iterate over all of these when walking sysfs.
> 
> I've tested this with userspace programs which use each of the
> deprecated instructions, and I've concurrently modified the support
> level for each of the features back-and-forth between HW and emulated to
> check that there are no spurious SIGILLs sent to userspace when the
> support level is changed.

Hi, Mark, I want to merge this patch to linux-5.10.y to solve a list_add
double problem. However, I can not trigger the emulation of these
deprecated instructions in qemu with userspace program as below, but
cause IABT and DABT, so that I can not verify the correctness of the
patch.Can you give me some help to test the emulation of these
deprecated instructions?

#include<stdio.h>

#define isb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
                                    : : "r" (0) : "memory")
#define dsb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
                                    : : "r" (0) : "memory")
#define dmb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
                                    : : "r" (0) : "memory")

int main(){
        isb();
        dsb();
        dmb();
        return 0;
}

> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/include/asm/traps.h       |  19 +-
>  arch/arm64/kernel/armv8_deprecated.c | 282 +++++++++++++--------------
>  arch/arm64/kernel/traps.c            |  40 +---
>  3 files changed, 149 insertions(+), 192 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
> index 6e5826470bea..1f361e2da516 100644
> --- a/arch/arm64/include/asm/traps.h
> +++ b/arch/arm64/include/asm/traps.h
> @@ -13,17 +13,16 @@
>  
>  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);
> -};
> +#ifdef CONFIG_ARMV8_DEPRECATED
> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn);
> +#else
> +static inline bool
> +try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
> +{
> +	return false;
> +}
> +#endif /* CONFIG_ARMV8_DEPRECATED */
>  
> -void register_undef_hook(struct undef_hook *hook);
> -void unregister_undef_hook(struct undef_hook *hook);
>  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err);
>  void arm64_notify_segfault(unsigned long addr);
>  void arm64_force_sig_fault(int signo, int code, unsigned long far, const char *str);
> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> index 7f2ce49dbf97..ed0788cf6bbb 100644
> --- a/arch/arm64/kernel/armv8_deprecated.c
> +++ b/arch/arm64/kernel/armv8_deprecated.c
> @@ -38,17 +38,24 @@ enum insn_emulation_mode {
>  enum legacy_insn_status {
>  	INSN_DEPRECATED,
>  	INSN_OBSOLETE,
> +	INSN_UNAVAILABLE,
>  };
>  
>  struct insn_emulation {
>  	const char			*name;
> -	struct list_head		node;
>  	enum legacy_insn_status		status;
> -	struct undef_hook		*hooks;
> +	bool				(*try_emulate)(struct pt_regs *regs,
> +						       u32 insn);
>  	int				(*set_hw_mode)(bool enable);
> +
>  	int current_mode;
>  	int min;
>  	int max;
> +
> +	/*
> +	 * sysctl for this emulation + a sentinal entry.
> +	 */
> +	struct ctl_table sysctl[2];
>  };
>  
>  #define ARM_OPCODE_CONDTEST_FAIL   0
> @@ -70,6 +77,7 @@ static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
>  	return ARM_OPCODE_CONDTEST_UNCOND;
>  }
>  
> +#ifdef CONFIG_SWP_EMULATION
>  /*
>   *  Implement emulation of the SWP/SWPB instructions using load-exclusive and
>   *  store-exclusive.
> @@ -222,28 +230,27 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
>  	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	= PSR_AA32_MODE_MASK,
> -		.pstate_val	= PSR_AA32_MODE_USR,
> -		.fn		= swp_handler
> -	},
> -	{ }
> -};
> +static bool try_emulate_swp(struct pt_regs *regs, u32 insn)
> +{
> +	/* SWP{B} only exists in ARM state and does not exist in Thumb */
> +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
> +		return false;
> +
> +	if ((insn & 0x0fb00ff0) != 0x01000090)
> +		return false;
> +
> +	return swp_handler(regs, insn) == 0;
> +}
>  
>  static struct insn_emulation insn_swp = {
>  	.name = "swp",
>  	.status = INSN_OBSOLETE,
> -	.hooks = swp_hooks,
> +	.try_emulate = try_emulate_swp,
>  	.set_hw_mode = NULL,
>  };
> +#endif /* CONFIG_SWP_EMULATION */
>  
> +#ifdef CONFIG_CP15_BARRIER_EMULATION
>  static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
>  {
>  	perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
> @@ -306,31 +313,29 @@ static int cp15_barrier_set_hw_mode(bool enable)
>  	return 0;
>  }
>  
> -static struct undef_hook cp15_barrier_hooks[] = {
> -	{
> -		.instr_mask	= 0x0fff0fdf,
> -		.instr_val	= 0x0e070f9a,
> -		.pstate_mask	= PSR_AA32_MODE_MASK,
> -		.pstate_val	= PSR_AA32_MODE_USR,
> -		.fn		= cp15barrier_handler,
> -	},
> -	{
> -		.instr_mask	= 0x0fff0fff,
> -		.instr_val	= 0x0e070f95,
> -		.pstate_mask	= PSR_AA32_MODE_MASK,
> -		.pstate_val	= PSR_AA32_MODE_USR,
> -		.fn		= cp15barrier_handler,
> -	},
> -	{ }
> -};
> +static bool try_emulate_cp15_barrier(struct pt_regs *regs, u32 insn)
> +{
> +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
> +		return false;
> +
> +	if ((insn & 0x0fff0fdf) == 0x0e070f9a)
> +		return cp15barrier_handler(regs, insn) == 0;
> +
> +	if ((insn & 0x0fff0fff) == 0x0e070f95)
> +		return cp15barrier_handler(regs, insn) == 0;
> +
> +	return false;
> +}
>  
>  static struct insn_emulation insn_cp15_barrier = {
>  	.name = "cp15_barrier",
>  	.status = INSN_DEPRECATED,
> -	.hooks = cp15_barrier_hooks,
> +	.try_emulate = try_emulate_cp15_barrier,
>  	.set_hw_mode = cp15_barrier_set_hw_mode,
>  };
> +#endif /* CONFIG_CP15_BARRIER_EMULATION */
>  
> +#ifdef CONFIG_SETEND_EMULATION
>  static int setend_set_hw_mode(bool enable)
>  {
>  	if (!cpu_supports_mixed_endian_el0())
> @@ -378,61 +383,41 @@ static int t16_setend_handler(struct pt_regs *regs, u32 instr)
>  	return rc;
>  }
>  
> -static struct undef_hook setend_hooks[] = {
> -	{
> -		.instr_mask	= 0xfffffdff,
> -		.instr_val	= 0xf1010000,
> -		.pstate_mask	= PSR_AA32_MODE_MASK,
> -		.pstate_val	= PSR_AA32_MODE_USR,
> -		.fn		= a32_setend_handler,
> -	},
> -	{
> -		/* Thumb mode */
> -		.instr_mask	= 0xfffffff7,
> -		.instr_val	= 0x0000b650,
> -		.pstate_mask	= (PSR_AA32_T_BIT | PSR_AA32_MODE_MASK),
> -		.pstate_val	= (PSR_AA32_T_BIT | PSR_AA32_MODE_USR),
> -		.fn		= t16_setend_handler,
> -	},
> -	{}
> -};
> +static bool try_emulate_setend(struct pt_regs *regs, u32 insn)
> +{
> +	if (compat_thumb_mode(regs) &&
> +	    (insn & 0xfffffff7) == 0x0000b650)
> +		return t16_setend_handler(regs, insn) == 0;
> +
> +	if (compat_user_mode(regs) &&
> +	    (insn & 0xfffffdff) == 0xf1010000)
> +		return a32_setend_handler(regs, insn) == 0;
> +
> +	return false;
> +}
>  
>  static struct insn_emulation insn_setend = {
>  	.name = "setend",
>  	.status = INSN_DEPRECATED,
> -	.hooks = setend_hooks,
> +	.try_emulate = try_emulate_setend,
>  	.set_hw_mode = setend_set_hw_mode,
>  };
> +#endif /* CONFIG_SETEND_EMULATION */
> +
> +static struct insn_emulation *insn_emulations[] = {
> +#ifdef CONFIG_SWP_EMULATION
> +	&insn_swp,
> +#endif
> +#ifdef CONFIG_CP15_BARRIER_EMULATION
> +	&insn_cp15_barrier,
> +#endif
> +#ifdef CONFIG_SETEND_EMULATION
> +	&insn_setend,
> +#endif
> +};
>  
> -static LIST_HEAD(insn_emulation);
> -static int nr_insn_emulated __initdata;
> -static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
>  static DEFINE_MUTEX(insn_emulation_mutex);
>  
> -static void register_emulation_hooks(struct insn_emulation *insn)
> -{
> -	struct undef_hook *hook;
> -
> -	BUG_ON(!insn->hooks);
> -
> -	for (hook = insn->hooks; hook->instr_mask; hook++)
> -		register_undef_hook(hook);
> -
> -	pr_notice("Registered %s emulation handler\n", insn->name);
> -}
> -
> -static void remove_emulation_hooks(struct insn_emulation *insn)
> -{
> -	struct undef_hook *hook;
> -
> -	BUG_ON(!insn->hooks);
> -
> -	for (hook = insn->hooks; hook->instr_mask; hook++)
> -		unregister_undef_hook(hook);
> -
> -	pr_notice("Removed %s emulation handler\n", insn->name);
> -}
> -
>  static void enable_insn_hw_mode(void *data)
>  {
>  	struct insn_emulation *insn = (struct insn_emulation *)data;
> @@ -469,18 +454,24 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
>  {
>  	int rc = 0;
>  	unsigned long flags;
> -	struct insn_emulation *insn;
>  
> -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> -	list_for_each_entry(insn, &insn_emulation, node) {
> -		bool enable = (insn->current_mode == INSN_HW);
> +	/*
> +	 * Disable IRQs to serialize against an IPI from
> +	 * run_all_cpu_set_hw_mode(), ensuring the HW is programmed to the most
> +	 * recent enablement state if the two race with one another.
> +	 */
> +	local_irq_save(flags);
> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> +		struct insn_emulation *insn = insn_emulations[i];
> +		bool enable = READ_ONCE(insn->current_mode) == INSN_HW;
>  		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
>  			pr_warn("CPU[%u] cannot support the emulation of %s",
>  				cpu, insn->name);
>  			rc = -EINVAL;
>  		}
>  	}
> -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
> +	local_irq_restore(flags);
> +
>  	return rc;
>  }
>  
> @@ -493,7 +484,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
>  	case INSN_UNDEF: /* Nothing to be done */
>  		break;
>  	case INSN_EMULATE:
> -		remove_emulation_hooks(insn);
>  		break;
>  	case INSN_HW:
>  		if (!run_all_cpu_set_hw_mode(insn, false))
> @@ -505,7 +495,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
>  	case INSN_UNDEF:
>  		break;
>  	case INSN_EMULATE:
> -		register_emulation_hooks(insn);
>  		break;
>  	case INSN_HW:
>  		ret = run_all_cpu_set_hw_mode(insn, true);
> @@ -517,34 +506,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
>  	return ret;
>  }
>  
> -static void __init register_insn_emulation(struct insn_emulation *insn)
> -{
> -	unsigned long flags;
> -
> -	insn->min = INSN_UNDEF;
> -
> -	switch (insn->status) {
> -	case INSN_DEPRECATED:
> -		insn->current_mode = INSN_EMULATE;
> -		/* Disable the HW mode if it was turned on at early boot time */
> -		run_all_cpu_set_hw_mode(insn, false);
> -		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 *buffer, size_t *lenp,
>  				  loff_t *ppos)
> @@ -562,7 +523,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
>  	ret = update_insn_emulation_mode(insn, prev_mode);
>  	if (ret) {
>  		/* Mode change failed, revert to previous mode. */
> -		insn->current_mode = prev_mode;
> +		WRITE_ONCE(insn->current_mode, prev_mode);
>  		update_insn_emulation_mode(insn, INSN_UNDEF);
>  	}
>  ret:
> @@ -570,21 +531,34 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
>  	return ret;
>  }
>  
> -static void __init register_insn_emulation_sysctl(void)
> +static void __init register_insn_emulation(struct insn_emulation *insn)
>  {
> -	unsigned long flags;
> -	int i = 0;
> -	struct insn_emulation *insn;
> -	struct ctl_table *insns_sysctl, *sysctl;
> +	struct ctl_table *sysctl;
>  
> -	insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
> -			       GFP_KERNEL);
> -	if (!insns_sysctl)
> -		return;
> +	insn->min = INSN_UNDEF;
>  
> -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> -	list_for_each_entry(insn, &insn_emulation, node) {
> -		sysctl = &insns_sysctl[i];
> +	switch (insn->status) {
> +	case INSN_DEPRECATED:
> +		insn->current_mode = INSN_EMULATE;
> +		/* Disable the HW mode if it was turned on at early boot time */
> +		run_all_cpu_set_hw_mode(insn, false);
> +		insn->max = INSN_HW;
> +		break;
> +	case INSN_OBSOLETE:
> +		insn->current_mode = INSN_UNDEF;
> +		insn->max = INSN_EMULATE;
> +		break;
> +	case INSN_UNAVAILABLE:
> +		insn->current_mode = INSN_UNDEF;
> +		insn->max = INSN_UNDEF;
> +		break;
> +	}
> +
> +	/* Program the HW if required */
> +	update_insn_emulation_mode(insn, INSN_UNDEF);
> +
> +	if (insn->status != INSN_UNAVAILABLE) {
> +		sysctl = &insn->sysctl[0];
>  
>  		sysctl->mode = 0644;
>  		sysctl->maxlen = sizeof(int);
> @@ -594,11 +568,32 @@ static void __init register_insn_emulation_sysctl(void)
>  		sysctl->extra1 = &insn->min;
>  		sysctl->extra2 = &insn->max;
>  		sysctl->proc_handler = emulation_proc_handler;
> -		i++;
> +
> +		register_sysctl("abi", sysctl);
> +	}
> +}
> +
> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
> +{
> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> +		struct insn_emulation *ie = insn_emulations[i];
> +
> +		if (ie->status == INSN_UNAVAILABLE)
> +			continue;
> +
> +		/*
> +		 * A trap may race with the mode being changed
> +		 * INSN_EMULATE<->INSN_HW. Try to emulate the instruction to
> +		 * avoid a spurious UNDEF.
> +		 */
> +		if (READ_ONCE(ie->current_mode) == INSN_UNDEF)
> +			continue;
> +
> +		if (ie->try_emulate(regs, insn))
> +			return true;
>  	}
> -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
>  
> -	register_sysctl("abi", insns_sysctl);
> +	return false;
>  }
>  
>  /*
> @@ -607,24 +602,25 @@ static void __init register_insn_emulation_sysctl(void)
>   */
>  static int __init armv8_deprecated_init(void)
>  {
> -	if (IS_ENABLED(CONFIG_SWP_EMULATION))
> -		register_insn_emulation(&insn_swp);
> +#ifdef CONFIG_SETEND_EMULATION
> +	if (!system_supports_mixed_endian_el0()) {
> +		insn_setend.status = INSN_UNAVAILABLE;
> +		pr_info("setend instruction emulation is not supported on this system\n");
> +	}
>  
> -	if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
> -		register_insn_emulation(&insn_cp15_barrier);
> +#endif
> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> +		struct insn_emulation *ie = insn_emulations[i];
>  
> -	if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
> -		if (system_supports_mixed_endian_el0())
> -			register_insn_emulation(&insn_setend);
> -		else
> -			pr_info("setend instruction emulation is not supported on this system\n");
> +		if (ie->status == INSN_UNAVAILABLE)
> +			continue;
> +
> +		register_insn_emulation(ie);
>  	}
>  
>  	cpuhp_setup_state_nocalls(CPUHP_AP_ARM64_ISNDEP_STARTING,
>  				  "arm64/isndep:starting",
>  				  run_all_insn_set_hw_mode, NULL);
> -	register_insn_emulation_sysctl();
> -
>  	return 0;
>  }
>  
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 96eaf1aaec12..4c0caa589e12 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -373,27 +373,6 @@ void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
>  		regs->pstate &= ~PSR_BTYPE_MASK;
>  }
>  
> -static LIST_HEAD(undef_hook);
> -static DEFINE_RAW_SPINLOCK(undef_lock);
> -
> -void 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);
> -}
> -
> -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 user_insn_read(struct pt_regs *regs, u32 *insnp)
>  {
>  	u32 instr;
> @@ -425,23 +404,6 @@ static int user_insn_read(struct pt_regs *regs, u32 *insnp)
>  	return 0;
>  }
>  
> -static int call_undef_hook(struct pt_regs *regs, u32 instr)
> -{
> -	struct undef_hook *hook;
> -	unsigned long flags;
> -	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
> -
> -	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);
> -
> -	return fn ? fn(regs, instr) : 1;
> -}
> -
>  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err)
>  {
>  	const char *desc;
> @@ -502,7 +464,7 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
>  	if (try_emulate_mrs(regs, insn))
>  		return;
>  
> -	if (call_undef_hook(regs, insn) == 0)
> +	if (try_emulate_armv8_deprecated(regs, insn))
>  		return;
>  
>  out_err:

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

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling
  2023-02-03 10:08   ` Ruan Jinjie
@ 2023-02-03 17:27     ` Mark Rutland
  2023-02-06  2:03       ` Ruan Jinjie
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Rutland @ 2023-02-03 17:27 UTC (permalink / raw)
  To: Ruan Jinjie; +Cc: linux-arm-kernel

On Fri, Feb 03, 2023 at 06:08:08PM +0800, Ruan Jinjie wrote:
> 
> 
> On 2022/10/19 22:41, Mark Rutland wrote:
> > Support for deprecated instructions can be enabled or disabled at
> > runtime. To handle this, the code in armv8_deprecated.c registers and
> > unregisters undef_hooks, and makes cross CPU calls to configure HW
> > support. This is rather complicated, and the synchronization required to
> > make this safe ends up serializing the handling of instructions which
> > have been trapped.
> > 
> > This patch simplifies the deprecated instruction handling by removing
> > the dynamic registration and unregistration, and changing the trap
> > handling code to determine whether a handler should be invoked. This
> > removes the need for dynamic list management, and simplifies the locking
> > requirements, making it possible to handle trapped instructions entirely
> > in parallel.
> > 
> > Where changing the emulation state requires a cross-call, this is
> > serialized by locally disabling interrupts, ensuring that the CPU is not
> > left in an inconsistent state.
> > 
> > To simplify sysctl management, each insn_emulation is given a separate
> > sysctl table, permitting these to be registered separately. The core
> > sysctl code will iterate over all of these when walking sysfs.
> > 
> > I've tested this with userspace programs which use each of the
> > deprecated instructions, and I've concurrently modified the support
> > level for each of the features back-and-forth between HW and emulated to
> > check that there are no spurious SIGILLs sent to userspace when the
> > support level is changed.
> 
> Hi, Mark, I want to merge this patch to linux-5.10.y to solve a list_add
> double problem. However, I can not trigger the emulation of these
> deprecated instructions in qemu with userspace program as below, but
> cause IABT and DABT,

Well that certainly sounds wrong!

When you say "in qemu", so you mean in a KVM VM, a TCG VM, or usermode
emulation?

Can you tell me exactly how you're invoking qemu?

Can you give an example of the IABT and DABT you see?

> so that I can not verify the correctness of the
> patch.Can you give me some help to test the emulation of these
> deprecated instructions?
> 
> #include<stdio.h>
> 
> #define isb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
>                                     : : "r" (0) : "memory")
> #define dsb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
>                                     : : "r" (0) : "memory")
> #define dmb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
>                                     : : "r" (0) : "memory")

FWIW, these encodings look correc to me.

Thanks,
Mark.

> 
> int main(){
>         isb();
>         dsb();
>         dmb();
>         return 0;
> }
> 
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Joey Gouly <joey.gouly@arm.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Will Deacon <will@kernel.org>
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  arch/arm64/include/asm/traps.h       |  19 +-
> >  arch/arm64/kernel/armv8_deprecated.c | 282 +++++++++++++--------------
> >  arch/arm64/kernel/traps.c            |  40 +---
> >  3 files changed, 149 insertions(+), 192 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
> > index 6e5826470bea..1f361e2da516 100644
> > --- a/arch/arm64/include/asm/traps.h
> > +++ b/arch/arm64/include/asm/traps.h
> > @@ -13,17 +13,16 @@
> >  
> >  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);
> > -};
> > +#ifdef CONFIG_ARMV8_DEPRECATED
> > +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn);
> > +#else
> > +static inline bool
> > +try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
> > +{
> > +	return false;
> > +}
> > +#endif /* CONFIG_ARMV8_DEPRECATED */
> >  
> > -void register_undef_hook(struct undef_hook *hook);
> > -void unregister_undef_hook(struct undef_hook *hook);
> >  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err);
> >  void arm64_notify_segfault(unsigned long addr);
> >  void arm64_force_sig_fault(int signo, int code, unsigned long far, const char *str);
> > diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> > index 7f2ce49dbf97..ed0788cf6bbb 100644
> > --- a/arch/arm64/kernel/armv8_deprecated.c
> > +++ b/arch/arm64/kernel/armv8_deprecated.c
> > @@ -38,17 +38,24 @@ enum insn_emulation_mode {
> >  enum legacy_insn_status {
> >  	INSN_DEPRECATED,
> >  	INSN_OBSOLETE,
> > +	INSN_UNAVAILABLE,
> >  };
> >  
> >  struct insn_emulation {
> >  	const char			*name;
> > -	struct list_head		node;
> >  	enum legacy_insn_status		status;
> > -	struct undef_hook		*hooks;
> > +	bool				(*try_emulate)(struct pt_regs *regs,
> > +						       u32 insn);
> >  	int				(*set_hw_mode)(bool enable);
> > +
> >  	int current_mode;
> >  	int min;
> >  	int max;
> > +
> > +	/*
> > +	 * sysctl for this emulation + a sentinal entry.
> > +	 */
> > +	struct ctl_table sysctl[2];
> >  };
> >  
> >  #define ARM_OPCODE_CONDTEST_FAIL   0
> > @@ -70,6 +77,7 @@ static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
> >  	return ARM_OPCODE_CONDTEST_UNCOND;
> >  }
> >  
> > +#ifdef CONFIG_SWP_EMULATION
> >  /*
> >   *  Implement emulation of the SWP/SWPB instructions using load-exclusive and
> >   *  store-exclusive.
> > @@ -222,28 +230,27 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
> >  	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	= PSR_AA32_MODE_MASK,
> > -		.pstate_val	= PSR_AA32_MODE_USR,
> > -		.fn		= swp_handler
> > -	},
> > -	{ }
> > -};
> > +static bool try_emulate_swp(struct pt_regs *regs, u32 insn)
> > +{
> > +	/* SWP{B} only exists in ARM state and does not exist in Thumb */
> > +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
> > +		return false;
> > +
> > +	if ((insn & 0x0fb00ff0) != 0x01000090)
> > +		return false;
> > +
> > +	return swp_handler(regs, insn) == 0;
> > +}
> >  
> >  static struct insn_emulation insn_swp = {
> >  	.name = "swp",
> >  	.status = INSN_OBSOLETE,
> > -	.hooks = swp_hooks,
> > +	.try_emulate = try_emulate_swp,
> >  	.set_hw_mode = NULL,
> >  };
> > +#endif /* CONFIG_SWP_EMULATION */
> >  
> > +#ifdef CONFIG_CP15_BARRIER_EMULATION
> >  static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
> >  {
> >  	perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
> > @@ -306,31 +313,29 @@ static int cp15_barrier_set_hw_mode(bool enable)
> >  	return 0;
> >  }
> >  
> > -static struct undef_hook cp15_barrier_hooks[] = {
> > -	{
> > -		.instr_mask	= 0x0fff0fdf,
> > -		.instr_val	= 0x0e070f9a,
> > -		.pstate_mask	= PSR_AA32_MODE_MASK,
> > -		.pstate_val	= PSR_AA32_MODE_USR,
> > -		.fn		= cp15barrier_handler,
> > -	},
> > -	{
> > -		.instr_mask	= 0x0fff0fff,
> > -		.instr_val	= 0x0e070f95,
> > -		.pstate_mask	= PSR_AA32_MODE_MASK,
> > -		.pstate_val	= PSR_AA32_MODE_USR,
> > -		.fn		= cp15barrier_handler,
> > -	},
> > -	{ }
> > -};
> > +static bool try_emulate_cp15_barrier(struct pt_regs *regs, u32 insn)
> > +{
> > +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
> > +		return false;
> > +
> > +	if ((insn & 0x0fff0fdf) == 0x0e070f9a)
> > +		return cp15barrier_handler(regs, insn) == 0;
> > +
> > +	if ((insn & 0x0fff0fff) == 0x0e070f95)
> > +		return cp15barrier_handler(regs, insn) == 0;
> > +
> > +	return false;
> > +}
> >  
> >  static struct insn_emulation insn_cp15_barrier = {
> >  	.name = "cp15_barrier",
> >  	.status = INSN_DEPRECATED,
> > -	.hooks = cp15_barrier_hooks,
> > +	.try_emulate = try_emulate_cp15_barrier,
> >  	.set_hw_mode = cp15_barrier_set_hw_mode,
> >  };
> > +#endif /* CONFIG_CP15_BARRIER_EMULATION */
> >  
> > +#ifdef CONFIG_SETEND_EMULATION
> >  static int setend_set_hw_mode(bool enable)
> >  {
> >  	if (!cpu_supports_mixed_endian_el0())
> > @@ -378,61 +383,41 @@ static int t16_setend_handler(struct pt_regs *regs, u32 instr)
> >  	return rc;
> >  }
> >  
> > -static struct undef_hook setend_hooks[] = {
> > -	{
> > -		.instr_mask	= 0xfffffdff,
> > -		.instr_val	= 0xf1010000,
> > -		.pstate_mask	= PSR_AA32_MODE_MASK,
> > -		.pstate_val	= PSR_AA32_MODE_USR,
> > -		.fn		= a32_setend_handler,
> > -	},
> > -	{
> > -		/* Thumb mode */
> > -		.instr_mask	= 0xfffffff7,
> > -		.instr_val	= 0x0000b650,
> > -		.pstate_mask	= (PSR_AA32_T_BIT | PSR_AA32_MODE_MASK),
> > -		.pstate_val	= (PSR_AA32_T_BIT | PSR_AA32_MODE_USR),
> > -		.fn		= t16_setend_handler,
> > -	},
> > -	{}
> > -};
> > +static bool try_emulate_setend(struct pt_regs *regs, u32 insn)
> > +{
> > +	if (compat_thumb_mode(regs) &&
> > +	    (insn & 0xfffffff7) == 0x0000b650)
> > +		return t16_setend_handler(regs, insn) == 0;
> > +
> > +	if (compat_user_mode(regs) &&
> > +	    (insn & 0xfffffdff) == 0xf1010000)
> > +		return a32_setend_handler(regs, insn) == 0;
> > +
> > +	return false;
> > +}
> >  
> >  static struct insn_emulation insn_setend = {
> >  	.name = "setend",
> >  	.status = INSN_DEPRECATED,
> > -	.hooks = setend_hooks,
> > +	.try_emulate = try_emulate_setend,
> >  	.set_hw_mode = setend_set_hw_mode,
> >  };
> > +#endif /* CONFIG_SETEND_EMULATION */
> > +
> > +static struct insn_emulation *insn_emulations[] = {
> > +#ifdef CONFIG_SWP_EMULATION
> > +	&insn_swp,
> > +#endif
> > +#ifdef CONFIG_CP15_BARRIER_EMULATION
> > +	&insn_cp15_barrier,
> > +#endif
> > +#ifdef CONFIG_SETEND_EMULATION
> > +	&insn_setend,
> > +#endif
> > +};
> >  
> > -static LIST_HEAD(insn_emulation);
> > -static int nr_insn_emulated __initdata;
> > -static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
> >  static DEFINE_MUTEX(insn_emulation_mutex);
> >  
> > -static void register_emulation_hooks(struct insn_emulation *insn)
> > -{
> > -	struct undef_hook *hook;
> > -
> > -	BUG_ON(!insn->hooks);
> > -
> > -	for (hook = insn->hooks; hook->instr_mask; hook++)
> > -		register_undef_hook(hook);
> > -
> > -	pr_notice("Registered %s emulation handler\n", insn->name);
> > -}
> > -
> > -static void remove_emulation_hooks(struct insn_emulation *insn)
> > -{
> > -	struct undef_hook *hook;
> > -
> > -	BUG_ON(!insn->hooks);
> > -
> > -	for (hook = insn->hooks; hook->instr_mask; hook++)
> > -		unregister_undef_hook(hook);
> > -
> > -	pr_notice("Removed %s emulation handler\n", insn->name);
> > -}
> > -
> >  static void enable_insn_hw_mode(void *data)
> >  {
> >  	struct insn_emulation *insn = (struct insn_emulation *)data;
> > @@ -469,18 +454,24 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
> >  {
> >  	int rc = 0;
> >  	unsigned long flags;
> > -	struct insn_emulation *insn;
> >  
> > -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> > -	list_for_each_entry(insn, &insn_emulation, node) {
> > -		bool enable = (insn->current_mode == INSN_HW);
> > +	/*
> > +	 * Disable IRQs to serialize against an IPI from
> > +	 * run_all_cpu_set_hw_mode(), ensuring the HW is programmed to the most
> > +	 * recent enablement state if the two race with one another.
> > +	 */
> > +	local_irq_save(flags);
> > +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> > +		struct insn_emulation *insn = insn_emulations[i];
> > +		bool enable = READ_ONCE(insn->current_mode) == INSN_HW;
> >  		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
> >  			pr_warn("CPU[%u] cannot support the emulation of %s",
> >  				cpu, insn->name);
> >  			rc = -EINVAL;
> >  		}
> >  	}
> > -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
> > +	local_irq_restore(flags);
> > +
> >  	return rc;
> >  }
> >  
> > @@ -493,7 +484,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
> >  	case INSN_UNDEF: /* Nothing to be done */
> >  		break;
> >  	case INSN_EMULATE:
> > -		remove_emulation_hooks(insn);
> >  		break;
> >  	case INSN_HW:
> >  		if (!run_all_cpu_set_hw_mode(insn, false))
> > @@ -505,7 +495,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
> >  	case INSN_UNDEF:
> >  		break;
> >  	case INSN_EMULATE:
> > -		register_emulation_hooks(insn);
> >  		break;
> >  	case INSN_HW:
> >  		ret = run_all_cpu_set_hw_mode(insn, true);
> > @@ -517,34 +506,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
> >  	return ret;
> >  }
> >  
> > -static void __init register_insn_emulation(struct insn_emulation *insn)
> > -{
> > -	unsigned long flags;
> > -
> > -	insn->min = INSN_UNDEF;
> > -
> > -	switch (insn->status) {
> > -	case INSN_DEPRECATED:
> > -		insn->current_mode = INSN_EMULATE;
> > -		/* Disable the HW mode if it was turned on at early boot time */
> > -		run_all_cpu_set_hw_mode(insn, false);
> > -		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 *buffer, size_t *lenp,
> >  				  loff_t *ppos)
> > @@ -562,7 +523,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
> >  	ret = update_insn_emulation_mode(insn, prev_mode);
> >  	if (ret) {
> >  		/* Mode change failed, revert to previous mode. */
> > -		insn->current_mode = prev_mode;
> > +		WRITE_ONCE(insn->current_mode, prev_mode);
> >  		update_insn_emulation_mode(insn, INSN_UNDEF);
> >  	}
> >  ret:
> > @@ -570,21 +531,34 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
> >  	return ret;
> >  }
> >  
> > -static void __init register_insn_emulation_sysctl(void)
> > +static void __init register_insn_emulation(struct insn_emulation *insn)
> >  {
> > -	unsigned long flags;
> > -	int i = 0;
> > -	struct insn_emulation *insn;
> > -	struct ctl_table *insns_sysctl, *sysctl;
> > +	struct ctl_table *sysctl;
> >  
> > -	insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
> > -			       GFP_KERNEL);
> > -	if (!insns_sysctl)
> > -		return;
> > +	insn->min = INSN_UNDEF;
> >  
> > -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> > -	list_for_each_entry(insn, &insn_emulation, node) {
> > -		sysctl = &insns_sysctl[i];
> > +	switch (insn->status) {
> > +	case INSN_DEPRECATED:
> > +		insn->current_mode = INSN_EMULATE;
> > +		/* Disable the HW mode if it was turned on at early boot time */
> > +		run_all_cpu_set_hw_mode(insn, false);
> > +		insn->max = INSN_HW;
> > +		break;
> > +	case INSN_OBSOLETE:
> > +		insn->current_mode = INSN_UNDEF;
> > +		insn->max = INSN_EMULATE;
> > +		break;
> > +	case INSN_UNAVAILABLE:
> > +		insn->current_mode = INSN_UNDEF;
> > +		insn->max = INSN_UNDEF;
> > +		break;
> > +	}
> > +
> > +	/* Program the HW if required */
> > +	update_insn_emulation_mode(insn, INSN_UNDEF);
> > +
> > +	if (insn->status != INSN_UNAVAILABLE) {
> > +		sysctl = &insn->sysctl[0];
> >  
> >  		sysctl->mode = 0644;
> >  		sysctl->maxlen = sizeof(int);
> > @@ -594,11 +568,32 @@ static void __init register_insn_emulation_sysctl(void)
> >  		sysctl->extra1 = &insn->min;
> >  		sysctl->extra2 = &insn->max;
> >  		sysctl->proc_handler = emulation_proc_handler;
> > -		i++;
> > +
> > +		register_sysctl("abi", sysctl);
> > +	}
> > +}
> > +
> > +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
> > +{
> > +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> > +		struct insn_emulation *ie = insn_emulations[i];
> > +
> > +		if (ie->status == INSN_UNAVAILABLE)
> > +			continue;
> > +
> > +		/*
> > +		 * A trap may race with the mode being changed
> > +		 * INSN_EMULATE<->INSN_HW. Try to emulate the instruction to
> > +		 * avoid a spurious UNDEF.
> > +		 */
> > +		if (READ_ONCE(ie->current_mode) == INSN_UNDEF)
> > +			continue;
> > +
> > +		if (ie->try_emulate(regs, insn))
> > +			return true;
> >  	}
> > -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
> >  
> > -	register_sysctl("abi", insns_sysctl);
> > +	return false;
> >  }
> >  
> >  /*
> > @@ -607,24 +602,25 @@ static void __init register_insn_emulation_sysctl(void)
> >   */
> >  static int __init armv8_deprecated_init(void)
> >  {
> > -	if (IS_ENABLED(CONFIG_SWP_EMULATION))
> > -		register_insn_emulation(&insn_swp);
> > +#ifdef CONFIG_SETEND_EMULATION
> > +	if (!system_supports_mixed_endian_el0()) {
> > +		insn_setend.status = INSN_UNAVAILABLE;
> > +		pr_info("setend instruction emulation is not supported on this system\n");
> > +	}
> >  
> > -	if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
> > -		register_insn_emulation(&insn_cp15_barrier);
> > +#endif
> > +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> > +		struct insn_emulation *ie = insn_emulations[i];
> >  
> > -	if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
> > -		if (system_supports_mixed_endian_el0())
> > -			register_insn_emulation(&insn_setend);
> > -		else
> > -			pr_info("setend instruction emulation is not supported on this system\n");
> > +		if (ie->status == INSN_UNAVAILABLE)
> > +			continue;
> > +
> > +		register_insn_emulation(ie);
> >  	}
> >  
> >  	cpuhp_setup_state_nocalls(CPUHP_AP_ARM64_ISNDEP_STARTING,
> >  				  "arm64/isndep:starting",
> >  				  run_all_insn_set_hw_mode, NULL);
> > -	register_insn_emulation_sysctl();
> > -
> >  	return 0;
> >  }
> >  
> > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> > index 96eaf1aaec12..4c0caa589e12 100644
> > --- a/arch/arm64/kernel/traps.c
> > +++ b/arch/arm64/kernel/traps.c
> > @@ -373,27 +373,6 @@ void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
> >  		regs->pstate &= ~PSR_BTYPE_MASK;
> >  }
> >  
> > -static LIST_HEAD(undef_hook);
> > -static DEFINE_RAW_SPINLOCK(undef_lock);
> > -
> > -void 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);
> > -}
> > -
> > -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 user_insn_read(struct pt_regs *regs, u32 *insnp)
> >  {
> >  	u32 instr;
> > @@ -425,23 +404,6 @@ static int user_insn_read(struct pt_regs *regs, u32 *insnp)
> >  	return 0;
> >  }
> >  
> > -static int call_undef_hook(struct pt_regs *regs, u32 instr)
> > -{
> > -	struct undef_hook *hook;
> > -	unsigned long flags;
> > -	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
> > -
> > -	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);
> > -
> > -	return fn ? fn(regs, instr) : 1;
> > -}
> > -
> >  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err)
> >  {
> >  	const char *desc;
> > @@ -502,7 +464,7 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
> >  	if (try_emulate_mrs(regs, insn))
> >  		return;
> >  
> > -	if (call_undef_hook(regs, insn) == 0)
> > +	if (try_emulate_armv8_deprecated(regs, insn))
> >  		return;
> >  
> >  out_err:
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling
  2023-02-03 17:27     ` Mark Rutland
@ 2023-02-06  2:03       ` Ruan Jinjie
  2023-02-06  9:13         ` Mark Rutland
  0 siblings, 1 reply; 16+ messages in thread
From: Ruan Jinjie @ 2023-02-06  2:03 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, liaochang (A)



On 2023/2/4 1:27, Mark Rutland wrote:
> On Fri, Feb 03, 2023 at 06:08:08PM +0800, Ruan Jinjie wrote:
>>
>>
>> On 2022/10/19 22:41, Mark Rutland wrote:
>>> Support for deprecated instructions can be enabled or disabled at
>>> runtime. To handle this, the code in armv8_deprecated.c registers and
>>> unregisters undef_hooks, and makes cross CPU calls to configure HW
>>> support. This is rather complicated, and the synchronization required to
>>> make this safe ends up serializing the handling of instructions which
>>> have been trapped.
>>>
>>> This patch simplifies the deprecated instruction handling by removing
>>> the dynamic registration and unregistration, and changing the trap
>>> handling code to determine whether a handler should be invoked. This
>>> removes the need for dynamic list management, and simplifies the locking
>>> requirements, making it possible to handle trapped instructions entirely
>>> in parallel.
>>>
>>> Where changing the emulation state requires a cross-call, this is
>>> serialized by locally disabling interrupts, ensuring that the CPU is not
>>> left in an inconsistent state.
>>>
>>> To simplify sysctl management, each insn_emulation is given a separate
>>> sysctl table, permitting these to be registered separately. The core
>>> sysctl code will iterate over all of these when walking sysfs.
>>>
>>> I've tested this with userspace programs which use each of the
>>> deprecated instructions, and I've concurrently modified the support
>>> level for each of the features back-and-forth between HW and emulated to
>>> check that there are no spurious SIGILLs sent to userspace when the
>>> support level is changed.
>>
>> Hi, Mark, I want to merge this patch to linux-5.10.y to solve a list_add
>> double problem. However, I can not trigger the emulation of these
>> deprecated instructions in qemu with userspace program as below, but
>> cause IABT and DABT,
> 
> Well that certainly sounds wrong!
> 
> When you say "in qemu", so you mean in a KVM VM, a TCG VM, or usermode
> emulation?
> 
> Can you tell me exactly how you're invoking qemu?

Sorry, the information given was incomplete.I invoke qemu 6.1.1 with
following commands, which is a TCG VM:

qemu-system-aarch64
	-M virt \
	-cpu cortex-a57 \
	-smp 4 \
	-m 4096 \
	-nographic \
	-kernel linux-stable/arch/arm64/boot/Image \
	-drive if=none,file=rootfs.img,format=raw,id=hd0 \
	-device virtio-net-device,netdev=net0 \
	-netdev user,id=net0,host=10.0.2.1,hostfwd=tcp::5555-:22 \
	-device virtio-blk-device,drive=hd0 \
	-append "root=/dev/vda console=ttyAMA0 rootfstype=ext4 init=/linuxrc rw
oops=panic nmi_watchdog=panic panic=1"

> 
> Can you give an example of the IABT and DABT you see?

Because the emulation of cp15 deprecated instructions is not triggered,I
add printk message in el0_sync_compat_handler() function to check the EC
code When the following user-space code is executed.The print is as
follows:

/home # ./cp15_barrier_test
[   72.469366] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
[   72.470477] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
[   72.471342] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
[   72.472341] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
[   72.474033] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
[   72.475145] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
[   72.476255] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
[   72.477861] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
[   72.478427] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
[   72.479111] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
[   72.479919] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
[   72.480467] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
[   72.481616] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
[   72.483386] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
[   72.484187] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
[   72.485215] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
[   72.485882] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
[   72.486628] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
[   72.487495] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
[   72.488548] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11

I went through "ARM® Architecture Reference Manual ARMv8, for ARMv8-A
architecture profile" and check the header arch/arm64/include/asm/esr.h,
find it's EC codes for SVC32, IABT and DABT.

 #define ESR_ELx_EC_SVC32    (0x11)
 #define ESR_ELx_EC_IABT_LOW (0x20)
 #define ESR_ELx_EC_DABT_LOW (0x24)

I also learned from the manual that the SCTLR_EL1.CP15BEN is critical,
which control the behavior of above deprecating memory barrier
instructions which can be set by the 'echo 2 >
/proc/sys/abi/cp15_barrier' command.However, the result is the same
whether it is set or not.

I think the code went into the wrong code flow because of the wrong EC
code, but I don't know why the wrong EC value was passed in.

> 
>> so that I can not verify the correctness of the
>> patch.Can you give me some help to test the emulation of these
>> deprecated instructions?
>>
>> #include<stdio.h>
>>
>> #define isb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
>>                                     : : "r" (0) : "memory")
>> #define dsb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
>>                                     : : "r" (0) : "memory")
>> #define dmb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
>>                                     : : "r" (0) : "memory")
> 
> FWIW, these encodings look correc to me.
> 
> Thanks,
> Mark.
> 
>>
>> int main(){
>>         isb();
>>         dsb();
>>         dmb();
>>         return 0;
>> }
>>
>>>
>>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: James Morse <james.morse@arm.com>
>>> Cc: Joey Gouly <joey.gouly@arm.com>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Will Deacon <will@kernel.org>
>>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>>> ---
>>>  arch/arm64/include/asm/traps.h       |  19 +-
>>>  arch/arm64/kernel/armv8_deprecated.c | 282 +++++++++++++--------------
>>>  arch/arm64/kernel/traps.c            |  40 +---
>>>  3 files changed, 149 insertions(+), 192 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
>>> index 6e5826470bea..1f361e2da516 100644
>>> --- a/arch/arm64/include/asm/traps.h
>>> +++ b/arch/arm64/include/asm/traps.h
>>> @@ -13,17 +13,16 @@
>>>  
>>>  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);
>>> -};
>>> +#ifdef CONFIG_ARMV8_DEPRECATED
>>> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn);
>>> +#else
>>> +static inline bool
>>> +try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
>>> +{
>>> +	return false;
>>> +}
>>> +#endif /* CONFIG_ARMV8_DEPRECATED */
>>>  
>>> -void register_undef_hook(struct undef_hook *hook);
>>> -void unregister_undef_hook(struct undef_hook *hook);
>>>  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err);
>>>  void arm64_notify_segfault(unsigned long addr);
>>>  void arm64_force_sig_fault(int signo, int code, unsigned long far, const char *str);
>>> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
>>> index 7f2ce49dbf97..ed0788cf6bbb 100644
>>> --- a/arch/arm64/kernel/armv8_deprecated.c
>>> +++ b/arch/arm64/kernel/armv8_deprecated.c
>>> @@ -38,17 +38,24 @@ enum insn_emulation_mode {
>>>  enum legacy_insn_status {
>>>  	INSN_DEPRECATED,
>>>  	INSN_OBSOLETE,
>>> +	INSN_UNAVAILABLE,
>>>  };
>>>  
>>>  struct insn_emulation {
>>>  	const char			*name;
>>> -	struct list_head		node;
>>>  	enum legacy_insn_status		status;
>>> -	struct undef_hook		*hooks;
>>> +	bool				(*try_emulate)(struct pt_regs *regs,
>>> +						       u32 insn);
>>>  	int				(*set_hw_mode)(bool enable);
>>> +
>>>  	int current_mode;
>>>  	int min;
>>>  	int max;
>>> +
>>> +	/*
>>> +	 * sysctl for this emulation + a sentinal entry.
>>> +	 */
>>> +	struct ctl_table sysctl[2];
>>>  };
>>>  
>>>  #define ARM_OPCODE_CONDTEST_FAIL   0
>>> @@ -70,6 +77,7 @@ static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
>>>  	return ARM_OPCODE_CONDTEST_UNCOND;
>>>  }
>>>  
>>> +#ifdef CONFIG_SWP_EMULATION
>>>  /*
>>>   *  Implement emulation of the SWP/SWPB instructions using load-exclusive and
>>>   *  store-exclusive.
>>> @@ -222,28 +230,27 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
>>>  	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	= PSR_AA32_MODE_MASK,
>>> -		.pstate_val	= PSR_AA32_MODE_USR,
>>> -		.fn		= swp_handler
>>> -	},
>>> -	{ }
>>> -};
>>> +static bool try_emulate_swp(struct pt_regs *regs, u32 insn)
>>> +{
>>> +	/* SWP{B} only exists in ARM state and does not exist in Thumb */
>>> +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
>>> +		return false;
>>> +
>>> +	if ((insn & 0x0fb00ff0) != 0x01000090)
>>> +		return false;
>>> +
>>> +	return swp_handler(regs, insn) == 0;
>>> +}
>>>  
>>>  static struct insn_emulation insn_swp = {
>>>  	.name = "swp",
>>>  	.status = INSN_OBSOLETE,
>>> -	.hooks = swp_hooks,
>>> +	.try_emulate = try_emulate_swp,
>>>  	.set_hw_mode = NULL,
>>>  };
>>> +#endif /* CONFIG_SWP_EMULATION */
>>>  
>>> +#ifdef CONFIG_CP15_BARRIER_EMULATION
>>>  static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
>>>  {
>>>  	perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
>>> @@ -306,31 +313,29 @@ static int cp15_barrier_set_hw_mode(bool enable)
>>>  	return 0;
>>>  }
>>>  
>>> -static struct undef_hook cp15_barrier_hooks[] = {
>>> -	{
>>> -		.instr_mask	= 0x0fff0fdf,
>>> -		.instr_val	= 0x0e070f9a,
>>> -		.pstate_mask	= PSR_AA32_MODE_MASK,
>>> -		.pstate_val	= PSR_AA32_MODE_USR,
>>> -		.fn		= cp15barrier_handler,
>>> -	},
>>> -	{
>>> -		.instr_mask	= 0x0fff0fff,
>>> -		.instr_val	= 0x0e070f95,
>>> -		.pstate_mask	= PSR_AA32_MODE_MASK,
>>> -		.pstate_val	= PSR_AA32_MODE_USR,
>>> -		.fn		= cp15barrier_handler,
>>> -	},
>>> -	{ }
>>> -};
>>> +static bool try_emulate_cp15_barrier(struct pt_regs *regs, u32 insn)
>>> +{
>>> +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
>>> +		return false;
>>> +
>>> +	if ((insn & 0x0fff0fdf) == 0x0e070f9a)
>>> +		return cp15barrier_handler(regs, insn) == 0;
>>> +
>>> +	if ((insn & 0x0fff0fff) == 0x0e070f95)
>>> +		return cp15barrier_handler(regs, insn) == 0;
>>> +
>>> +	return false;
>>> +}
>>>  
>>>  static struct insn_emulation insn_cp15_barrier = {
>>>  	.name = "cp15_barrier",
>>>  	.status = INSN_DEPRECATED,
>>> -	.hooks = cp15_barrier_hooks,
>>> +	.try_emulate = try_emulate_cp15_barrier,
>>>  	.set_hw_mode = cp15_barrier_set_hw_mode,
>>>  };
>>> +#endif /* CONFIG_CP15_BARRIER_EMULATION */
>>>  
>>> +#ifdef CONFIG_SETEND_EMULATION
>>>  static int setend_set_hw_mode(bool enable)
>>>  {
>>>  	if (!cpu_supports_mixed_endian_el0())
>>> @@ -378,61 +383,41 @@ static int t16_setend_handler(struct pt_regs *regs, u32 instr)
>>>  	return rc;
>>>  }
>>>  
>>> -static struct undef_hook setend_hooks[] = {
>>> -	{
>>> -		.instr_mask	= 0xfffffdff,
>>> -		.instr_val	= 0xf1010000,
>>> -		.pstate_mask	= PSR_AA32_MODE_MASK,
>>> -		.pstate_val	= PSR_AA32_MODE_USR,
>>> -		.fn		= a32_setend_handler,
>>> -	},
>>> -	{
>>> -		/* Thumb mode */
>>> -		.instr_mask	= 0xfffffff7,
>>> -		.instr_val	= 0x0000b650,
>>> -		.pstate_mask	= (PSR_AA32_T_BIT | PSR_AA32_MODE_MASK),
>>> -		.pstate_val	= (PSR_AA32_T_BIT | PSR_AA32_MODE_USR),
>>> -		.fn		= t16_setend_handler,
>>> -	},
>>> -	{}
>>> -};
>>> +static bool try_emulate_setend(struct pt_regs *regs, u32 insn)
>>> +{
>>> +	if (compat_thumb_mode(regs) &&
>>> +	    (insn & 0xfffffff7) == 0x0000b650)
>>> +		return t16_setend_handler(regs, insn) == 0;
>>> +
>>> +	if (compat_user_mode(regs) &&
>>> +	    (insn & 0xfffffdff) == 0xf1010000)
>>> +		return a32_setend_handler(regs, insn) == 0;
>>> +
>>> +	return false;
>>> +}
>>>  
>>>  static struct insn_emulation insn_setend = {
>>>  	.name = "setend",
>>>  	.status = INSN_DEPRECATED,
>>> -	.hooks = setend_hooks,
>>> +	.try_emulate = try_emulate_setend,
>>>  	.set_hw_mode = setend_set_hw_mode,
>>>  };
>>> +#endif /* CONFIG_SETEND_EMULATION */
>>> +
>>> +static struct insn_emulation *insn_emulations[] = {
>>> +#ifdef CONFIG_SWP_EMULATION
>>> +	&insn_swp,
>>> +#endif
>>> +#ifdef CONFIG_CP15_BARRIER_EMULATION
>>> +	&insn_cp15_barrier,
>>> +#endif
>>> +#ifdef CONFIG_SETEND_EMULATION
>>> +	&insn_setend,
>>> +#endif
>>> +};
>>>  
>>> -static LIST_HEAD(insn_emulation);
>>> -static int nr_insn_emulated __initdata;
>>> -static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
>>>  static DEFINE_MUTEX(insn_emulation_mutex);
>>>  
>>> -static void register_emulation_hooks(struct insn_emulation *insn)
>>> -{
>>> -	struct undef_hook *hook;
>>> -
>>> -	BUG_ON(!insn->hooks);
>>> -
>>> -	for (hook = insn->hooks; hook->instr_mask; hook++)
>>> -		register_undef_hook(hook);
>>> -
>>> -	pr_notice("Registered %s emulation handler\n", insn->name);
>>> -}
>>> -
>>> -static void remove_emulation_hooks(struct insn_emulation *insn)
>>> -{
>>> -	struct undef_hook *hook;
>>> -
>>> -	BUG_ON(!insn->hooks);
>>> -
>>> -	for (hook = insn->hooks; hook->instr_mask; hook++)
>>> -		unregister_undef_hook(hook);
>>> -
>>> -	pr_notice("Removed %s emulation handler\n", insn->name);
>>> -}
>>> -
>>>  static void enable_insn_hw_mode(void *data)
>>>  {
>>>  	struct insn_emulation *insn = (struct insn_emulation *)data;
>>> @@ -469,18 +454,24 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
>>>  {
>>>  	int rc = 0;
>>>  	unsigned long flags;
>>> -	struct insn_emulation *insn;
>>>  
>>> -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
>>> -	list_for_each_entry(insn, &insn_emulation, node) {
>>> -		bool enable = (insn->current_mode == INSN_HW);
>>> +	/*
>>> +	 * Disable IRQs to serialize against an IPI from
>>> +	 * run_all_cpu_set_hw_mode(), ensuring the HW is programmed to the most
>>> +	 * recent enablement state if the two race with one another.
>>> +	 */
>>> +	local_irq_save(flags);
>>> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
>>> +		struct insn_emulation *insn = insn_emulations[i];
>>> +		bool enable = READ_ONCE(insn->current_mode) == INSN_HW;
>>>  		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
>>>  			pr_warn("CPU[%u] cannot support the emulation of %s",
>>>  				cpu, insn->name);
>>>  			rc = -EINVAL;
>>>  		}
>>>  	}
>>> -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
>>> +	local_irq_restore(flags);
>>> +
>>>  	return rc;
>>>  }
>>>  
>>> @@ -493,7 +484,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
>>>  	case INSN_UNDEF: /* Nothing to be done */
>>>  		break;
>>>  	case INSN_EMULATE:
>>> -		remove_emulation_hooks(insn);
>>>  		break;
>>>  	case INSN_HW:
>>>  		if (!run_all_cpu_set_hw_mode(insn, false))
>>> @@ -505,7 +495,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
>>>  	case INSN_UNDEF:
>>>  		break;
>>>  	case INSN_EMULATE:
>>> -		register_emulation_hooks(insn);
>>>  		break;
>>>  	case INSN_HW:
>>>  		ret = run_all_cpu_set_hw_mode(insn, true);
>>> @@ -517,34 +506,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
>>>  	return ret;
>>>  }
>>>  
>>> -static void __init register_insn_emulation(struct insn_emulation *insn)
>>> -{
>>> -	unsigned long flags;
>>> -
>>> -	insn->min = INSN_UNDEF;
>>> -
>>> -	switch (insn->status) {
>>> -	case INSN_DEPRECATED:
>>> -		insn->current_mode = INSN_EMULATE;
>>> -		/* Disable the HW mode if it was turned on at early boot time */
>>> -		run_all_cpu_set_hw_mode(insn, false);
>>> -		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 *buffer, size_t *lenp,
>>>  				  loff_t *ppos)
>>> @@ -562,7 +523,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
>>>  	ret = update_insn_emulation_mode(insn, prev_mode);
>>>  	if (ret) {
>>>  		/* Mode change failed, revert to previous mode. */
>>> -		insn->current_mode = prev_mode;
>>> +		WRITE_ONCE(insn->current_mode, prev_mode);
>>>  		update_insn_emulation_mode(insn, INSN_UNDEF);
>>>  	}
>>>  ret:
>>> @@ -570,21 +531,34 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
>>>  	return ret;
>>>  }
>>>  
>>> -static void __init register_insn_emulation_sysctl(void)
>>> +static void __init register_insn_emulation(struct insn_emulation *insn)
>>>  {
>>> -	unsigned long flags;
>>> -	int i = 0;
>>> -	struct insn_emulation *insn;
>>> -	struct ctl_table *insns_sysctl, *sysctl;
>>> +	struct ctl_table *sysctl;
>>>  
>>> -	insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
>>> -			       GFP_KERNEL);
>>> -	if (!insns_sysctl)
>>> -		return;
>>> +	insn->min = INSN_UNDEF;
>>>  
>>> -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
>>> -	list_for_each_entry(insn, &insn_emulation, node) {
>>> -		sysctl = &insns_sysctl[i];
>>> +	switch (insn->status) {
>>> +	case INSN_DEPRECATED:
>>> +		insn->current_mode = INSN_EMULATE;
>>> +		/* Disable the HW mode if it was turned on at early boot time */
>>> +		run_all_cpu_set_hw_mode(insn, false);
>>> +		insn->max = INSN_HW;
>>> +		break;
>>> +	case INSN_OBSOLETE:
>>> +		insn->current_mode = INSN_UNDEF;
>>> +		insn->max = INSN_EMULATE;
>>> +		break;
>>> +	case INSN_UNAVAILABLE:
>>> +		insn->current_mode = INSN_UNDEF;
>>> +		insn->max = INSN_UNDEF;
>>> +		break;
>>> +	}
>>> +
>>> +	/* Program the HW if required */
>>> +	update_insn_emulation_mode(insn, INSN_UNDEF);
>>> +
>>> +	if (insn->status != INSN_UNAVAILABLE) {
>>> +		sysctl = &insn->sysctl[0];
>>>  
>>>  		sysctl->mode = 0644;
>>>  		sysctl->maxlen = sizeof(int);
>>> @@ -594,11 +568,32 @@ static void __init register_insn_emulation_sysctl(void)
>>>  		sysctl->extra1 = &insn->min;
>>>  		sysctl->extra2 = &insn->max;
>>>  		sysctl->proc_handler = emulation_proc_handler;
>>> -		i++;
>>> +
>>> +		register_sysctl("abi", sysctl);
>>> +	}
>>> +}
>>> +
>>> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
>>> +{
>>> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
>>> +		struct insn_emulation *ie = insn_emulations[i];
>>> +
>>> +		if (ie->status == INSN_UNAVAILABLE)
>>> +			continue;
>>> +
>>> +		/*
>>> +		 * A trap may race with the mode being changed
>>> +		 * INSN_EMULATE<->INSN_HW. Try to emulate the instruction to
>>> +		 * avoid a spurious UNDEF.
>>> +		 */
>>> +		if (READ_ONCE(ie->current_mode) == INSN_UNDEF)
>>> +			continue;
>>> +
>>> +		if (ie->try_emulate(regs, insn))
>>> +			return true;
>>>  	}
>>> -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
>>>  
>>> -	register_sysctl("abi", insns_sysctl);
>>> +	return false;
>>>  }
>>>  
>>>  /*
>>> @@ -607,24 +602,25 @@ static void __init register_insn_emulation_sysctl(void)
>>>   */
>>>  static int __init armv8_deprecated_init(void)
>>>  {
>>> -	if (IS_ENABLED(CONFIG_SWP_EMULATION))
>>> -		register_insn_emulation(&insn_swp);
>>> +#ifdef CONFIG_SETEND_EMULATION
>>> +	if (!system_supports_mixed_endian_el0()) {
>>> +		insn_setend.status = INSN_UNAVAILABLE;
>>> +		pr_info("setend instruction emulation is not supported on this system\n");
>>> +	}
>>>  
>>> -	if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
>>> -		register_insn_emulation(&insn_cp15_barrier);
>>> +#endif
>>> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
>>> +		struct insn_emulation *ie = insn_emulations[i];
>>>  
>>> -	if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
>>> -		if (system_supports_mixed_endian_el0())
>>> -			register_insn_emulation(&insn_setend);
>>> -		else
>>> -			pr_info("setend instruction emulation is not supported on this system\n");
>>> +		if (ie->status == INSN_UNAVAILABLE)
>>> +			continue;
>>> +
>>> +		register_insn_emulation(ie);
>>>  	}
>>>  
>>>  	cpuhp_setup_state_nocalls(CPUHP_AP_ARM64_ISNDEP_STARTING,
>>>  				  "arm64/isndep:starting",
>>>  				  run_all_insn_set_hw_mode, NULL);
>>> -	register_insn_emulation_sysctl();
>>> -
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>>> index 96eaf1aaec12..4c0caa589e12 100644
>>> --- a/arch/arm64/kernel/traps.c
>>> +++ b/arch/arm64/kernel/traps.c
>>> @@ -373,27 +373,6 @@ void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
>>>  		regs->pstate &= ~PSR_BTYPE_MASK;
>>>  }
>>>  
>>> -static LIST_HEAD(undef_hook);
>>> -static DEFINE_RAW_SPINLOCK(undef_lock);
>>> -
>>> -void 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);
>>> -}
>>> -
>>> -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 user_insn_read(struct pt_regs *regs, u32 *insnp)
>>>  {
>>>  	u32 instr;
>>> @@ -425,23 +404,6 @@ static int user_insn_read(struct pt_regs *regs, u32 *insnp)
>>>  	return 0;
>>>  }
>>>  
>>> -static int call_undef_hook(struct pt_regs *regs, u32 instr)
>>> -{
>>> -	struct undef_hook *hook;
>>> -	unsigned long flags;
>>> -	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
>>> -
>>> -	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);
>>> -
>>> -	return fn ? fn(regs, instr) : 1;
>>> -}
>>> -
>>>  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err)
>>>  {
>>>  	const char *desc;
>>> @@ -502,7 +464,7 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
>>>  	if (try_emulate_mrs(regs, insn))
>>>  		return;
>>>  
>>> -	if (call_undef_hook(regs, insn) == 0)
>>> +	if (try_emulate_armv8_deprecated(regs, insn))
>>>  		return;
>>>  
>>>  out_err:
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling
  2023-02-06  2:03       ` Ruan Jinjie
@ 2023-02-06  9:13         ` Mark Rutland
  2023-02-07  7:01           ` Ruan Jinjie
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Rutland @ 2023-02-06  9:13 UTC (permalink / raw)
  To: Ruan Jinjie; +Cc: linux-arm-kernel, liaochang (A)

On Mon, Feb 06, 2023 at 10:03:17AM +0800, Ruan Jinjie wrote:
> On 2023/2/4 1:27, Mark Rutland wrote:
> > On Fri, Feb 03, 2023 at 06:08:08PM +0800, Ruan Jinjie wrote:
> >> Hi, Mark, I want to merge this patch to linux-5.10.y to solve a list_add
> >> double problem. However, I can not trigger the emulation of these
> >> deprecated instructions in qemu with userspace program as below, but
> >> cause IABT and DABT,
> > 
> > Well that certainly sounds wrong!
> > 
> > When you say "in qemu", so you mean in a KVM VM, a TCG VM, or usermode
> > emulation?
> > 
> > Can you tell me exactly how you're invoking qemu?
> 
> Sorry, the information given was incomplete.I invoke qemu 6.1.1 with
> following commands, which is a TCG VM:
> 
> qemu-system-aarch64
> 	-M virt \
> 	-cpu cortex-a57 \
> 	-smp 4 \
> 	-m 4096 \
> 	-nographic \
> 	-kernel linux-stable/arch/arm64/boot/Image \
> 	-drive if=none,file=rootfs.img,format=raw,id=hd0 \
> 	-device virtio-net-device,netdev=net0 \
> 	-netdev user,id=net0,host=10.0.2.1,hostfwd=tcp::5555-:22 \
> 	-device virtio-blk-device,drive=hd0 \
> 	-append "root=/dev/vda console=ttyAMA0 rootfstype=ext4 init=/linuxrc rw
> oops=panic nmi_watchdog=panic panic=1"
> 
> > 
> > Can you give an example of the IABT and DABT you see?
> 
> Because the emulation of cp15 deprecated instructions is not triggered,I
> add printk message in el0_sync_compat_handler() function 

Ah! So you mean that those are the *only* exceptions you see, not that those
are *unexpected*. AFAICT, those are jsut the test code and data being faulted
in as usual, and are not related to the emulation.

I assume the test completes successfully, and returns 0?

I had a play with this locally (with QEMU 6.2.94), and I see that the
deprecated instructions work even when neither the emulation nor HW support is
enabled (in the relevant SCTLR_ELx bits), and I think that what's happening
here is the QEMU isn't correctly emulating the architecture:

* QEMU seems to ignore SCTLR_ELx.CP15BEN, and always imeplements those rather
  than making those UNDEFINED when SCTLR_ELx.CP15BEN == 0b0.

* QEMU seems to ignore SCTLR_ELx.SED, and always implements SETEND rather than
  making that UNDEFINED when SCTLR_ELx.SED == 0b1.

* QEMU seems to implement SWP*, even though those instructions don't exist in
  ARMv8-A and should always be UNDEFINED.
 
> to check the EC code When the following user-space code is executed.The print
> is as follows:
> 
> /home # ./cp15_barrier_test
> [   72.469366] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
> [   72.470477] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
> [   72.471342] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
> [   72.472341] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
> [   72.474033] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
> [   72.475145] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
> [   72.476255] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
> [   72.477861] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
> [   72.478427] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
> [   72.479111] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
> [   72.479919] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
> [   72.480467] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
> [   72.481616] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
> [   72.483386] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
> [   72.484187] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
> [   72.485215] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
> [   72.485882] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
> [   72.486628] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
> [   72.487495] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
> [   72.488548] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
> 
> I went through "ARM® Architecture Reference Manual ARMv8, for ARMv8-A
> architecture profile" and check the header arch/arm64/include/asm/esr.h,
> find it's EC codes for SVC32, IABT and DABT.
> 
>  #define ESR_ELx_EC_SVC32    (0x11)
>  #define ESR_ELx_EC_IABT_LOW (0x20)
>  #define ESR_ELx_EC_DABT_LOW (0x24)
> 
> I also learned from the manual that the SCTLR_EL1.CP15BEN is critical,
> which control the behavior of above deprecating memory barrier
> instructions which can be set by the 'echo 2 >
> /proc/sys/abi/cp15_barrier' command.However, the result is the same
> whether it is set or not.

As above, I think this is a QEMU bug, with QEMU erroneosuly implementing the
instructions when they should be UNDEFINED. The aborts are unrelated, and are
just the usual faulting in of code and data pages.

So I'd recommend reporting tha to QEMU folk, and in the mean time testing on
real HW rather than QEMU in TCG mode. When I wrote these patches I tested on a
Raspberry Pi 4, under a KVM virtual machine.

Thanks,
Mark.

> 
> I think the code went into the wrong code flow because of the wrong EC
> code, but I don't know why the wrong EC value was passed in.
> 
> > 
> >> so that I can not verify the correctness of the
> >> patch.Can you give me some help to test the emulation of these
> >> deprecated instructions?
> >>
> >> #include<stdio.h>
> >>
> >> #define isb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
> >>                                     : : "r" (0) : "memory")
> >> #define dsb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
> >>                                     : : "r" (0) : "memory")
> >> #define dmb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
> >>                                     : : "r" (0) : "memory")
> > 
> > FWIW, these encodings look correc to me.
> > 
> > Thanks,
> > Mark.
> > 
> >>
> >> int main(){
> >>         isb();
> >>         dsb();
> >>         dmb();
> >>         return 0;
> >> }
> >>
> >>>
> >>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> >>> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >>> Cc: James Morse <james.morse@arm.com>
> >>> Cc: Joey Gouly <joey.gouly@arm.com>
> >>> Cc: Peter Zijlstra <peterz@infradead.org>
> >>> Cc: Will Deacon <will@kernel.org>
> >>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> >>> ---
> >>>  arch/arm64/include/asm/traps.h       |  19 +-
> >>>  arch/arm64/kernel/armv8_deprecated.c | 282 +++++++++++++--------------
> >>>  arch/arm64/kernel/traps.c            |  40 +---
> >>>  3 files changed, 149 insertions(+), 192 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
> >>> index 6e5826470bea..1f361e2da516 100644
> >>> --- a/arch/arm64/include/asm/traps.h
> >>> +++ b/arch/arm64/include/asm/traps.h
> >>> @@ -13,17 +13,16 @@
> >>>  
> >>>  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);
> >>> -};
> >>> +#ifdef CONFIG_ARMV8_DEPRECATED
> >>> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn);
> >>> +#else
> >>> +static inline bool
> >>> +try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
> >>> +{
> >>> +	return false;
> >>> +}
> >>> +#endif /* CONFIG_ARMV8_DEPRECATED */
> >>>  
> >>> -void register_undef_hook(struct undef_hook *hook);
> >>> -void unregister_undef_hook(struct undef_hook *hook);
> >>>  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err);
> >>>  void arm64_notify_segfault(unsigned long addr);
> >>>  void arm64_force_sig_fault(int signo, int code, unsigned long far, const char *str);
> >>> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
> >>> index 7f2ce49dbf97..ed0788cf6bbb 100644
> >>> --- a/arch/arm64/kernel/armv8_deprecated.c
> >>> +++ b/arch/arm64/kernel/armv8_deprecated.c
> >>> @@ -38,17 +38,24 @@ enum insn_emulation_mode {
> >>>  enum legacy_insn_status {
> >>>  	INSN_DEPRECATED,
> >>>  	INSN_OBSOLETE,
> >>> +	INSN_UNAVAILABLE,
> >>>  };
> >>>  
> >>>  struct insn_emulation {
> >>>  	const char			*name;
> >>> -	struct list_head		node;
> >>>  	enum legacy_insn_status		status;
> >>> -	struct undef_hook		*hooks;
> >>> +	bool				(*try_emulate)(struct pt_regs *regs,
> >>> +						       u32 insn);
> >>>  	int				(*set_hw_mode)(bool enable);
> >>> +
> >>>  	int current_mode;
> >>>  	int min;
> >>>  	int max;
> >>> +
> >>> +	/*
> >>> +	 * sysctl for this emulation + a sentinal entry.
> >>> +	 */
> >>> +	struct ctl_table sysctl[2];
> >>>  };
> >>>  
> >>>  #define ARM_OPCODE_CONDTEST_FAIL   0
> >>> @@ -70,6 +77,7 @@ static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
> >>>  	return ARM_OPCODE_CONDTEST_UNCOND;
> >>>  }
> >>>  
> >>> +#ifdef CONFIG_SWP_EMULATION
> >>>  /*
> >>>   *  Implement emulation of the SWP/SWPB instructions using load-exclusive and
> >>>   *  store-exclusive.
> >>> @@ -222,28 +230,27 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
> >>>  	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	= PSR_AA32_MODE_MASK,
> >>> -		.pstate_val	= PSR_AA32_MODE_USR,
> >>> -		.fn		= swp_handler
> >>> -	},
> >>> -	{ }
> >>> -};
> >>> +static bool try_emulate_swp(struct pt_regs *regs, u32 insn)
> >>> +{
> >>> +	/* SWP{B} only exists in ARM state and does not exist in Thumb */
> >>> +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
> >>> +		return false;
> >>> +
> >>> +	if ((insn & 0x0fb00ff0) != 0x01000090)
> >>> +		return false;
> >>> +
> >>> +	return swp_handler(regs, insn) == 0;
> >>> +}
> >>>  
> >>>  static struct insn_emulation insn_swp = {
> >>>  	.name = "swp",
> >>>  	.status = INSN_OBSOLETE,
> >>> -	.hooks = swp_hooks,
> >>> +	.try_emulate = try_emulate_swp,
> >>>  	.set_hw_mode = NULL,
> >>>  };
> >>> +#endif /* CONFIG_SWP_EMULATION */
> >>>  
> >>> +#ifdef CONFIG_CP15_BARRIER_EMULATION
> >>>  static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
> >>>  {
> >>>  	perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
> >>> @@ -306,31 +313,29 @@ static int cp15_barrier_set_hw_mode(bool enable)
> >>>  	return 0;
> >>>  }
> >>>  
> >>> -static struct undef_hook cp15_barrier_hooks[] = {
> >>> -	{
> >>> -		.instr_mask	= 0x0fff0fdf,
> >>> -		.instr_val	= 0x0e070f9a,
> >>> -		.pstate_mask	= PSR_AA32_MODE_MASK,
> >>> -		.pstate_val	= PSR_AA32_MODE_USR,
> >>> -		.fn		= cp15barrier_handler,
> >>> -	},
> >>> -	{
> >>> -		.instr_mask	= 0x0fff0fff,
> >>> -		.instr_val	= 0x0e070f95,
> >>> -		.pstate_mask	= PSR_AA32_MODE_MASK,
> >>> -		.pstate_val	= PSR_AA32_MODE_USR,
> >>> -		.fn		= cp15barrier_handler,
> >>> -	},
> >>> -	{ }
> >>> -};
> >>> +static bool try_emulate_cp15_barrier(struct pt_regs *regs, u32 insn)
> >>> +{
> >>> +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
> >>> +		return false;
> >>> +
> >>> +	if ((insn & 0x0fff0fdf) == 0x0e070f9a)
> >>> +		return cp15barrier_handler(regs, insn) == 0;
> >>> +
> >>> +	if ((insn & 0x0fff0fff) == 0x0e070f95)
> >>> +		return cp15barrier_handler(regs, insn) == 0;
> >>> +
> >>> +	return false;
> >>> +}
> >>>  
> >>>  static struct insn_emulation insn_cp15_barrier = {
> >>>  	.name = "cp15_barrier",
> >>>  	.status = INSN_DEPRECATED,
> >>> -	.hooks = cp15_barrier_hooks,
> >>> +	.try_emulate = try_emulate_cp15_barrier,
> >>>  	.set_hw_mode = cp15_barrier_set_hw_mode,
> >>>  };
> >>> +#endif /* CONFIG_CP15_BARRIER_EMULATION */
> >>>  
> >>> +#ifdef CONFIG_SETEND_EMULATION
> >>>  static int setend_set_hw_mode(bool enable)
> >>>  {
> >>>  	if (!cpu_supports_mixed_endian_el0())
> >>> @@ -378,61 +383,41 @@ static int t16_setend_handler(struct pt_regs *regs, u32 instr)
> >>>  	return rc;
> >>>  }
> >>>  
> >>> -static struct undef_hook setend_hooks[] = {
> >>> -	{
> >>> -		.instr_mask	= 0xfffffdff,
> >>> -		.instr_val	= 0xf1010000,
> >>> -		.pstate_mask	= PSR_AA32_MODE_MASK,
> >>> -		.pstate_val	= PSR_AA32_MODE_USR,
> >>> -		.fn		= a32_setend_handler,
> >>> -	},
> >>> -	{
> >>> -		/* Thumb mode */
> >>> -		.instr_mask	= 0xfffffff7,
> >>> -		.instr_val	= 0x0000b650,
> >>> -		.pstate_mask	= (PSR_AA32_T_BIT | PSR_AA32_MODE_MASK),
> >>> -		.pstate_val	= (PSR_AA32_T_BIT | PSR_AA32_MODE_USR),
> >>> -		.fn		= t16_setend_handler,
> >>> -	},
> >>> -	{}
> >>> -};
> >>> +static bool try_emulate_setend(struct pt_regs *regs, u32 insn)
> >>> +{
> >>> +	if (compat_thumb_mode(regs) &&
> >>> +	    (insn & 0xfffffff7) == 0x0000b650)
> >>> +		return t16_setend_handler(regs, insn) == 0;
> >>> +
> >>> +	if (compat_user_mode(regs) &&
> >>> +	    (insn & 0xfffffdff) == 0xf1010000)
> >>> +		return a32_setend_handler(regs, insn) == 0;
> >>> +
> >>> +	return false;
> >>> +}
> >>>  
> >>>  static struct insn_emulation insn_setend = {
> >>>  	.name = "setend",
> >>>  	.status = INSN_DEPRECATED,
> >>> -	.hooks = setend_hooks,
> >>> +	.try_emulate = try_emulate_setend,
> >>>  	.set_hw_mode = setend_set_hw_mode,
> >>>  };
> >>> +#endif /* CONFIG_SETEND_EMULATION */
> >>> +
> >>> +static struct insn_emulation *insn_emulations[] = {
> >>> +#ifdef CONFIG_SWP_EMULATION
> >>> +	&insn_swp,
> >>> +#endif
> >>> +#ifdef CONFIG_CP15_BARRIER_EMULATION
> >>> +	&insn_cp15_barrier,
> >>> +#endif
> >>> +#ifdef CONFIG_SETEND_EMULATION
> >>> +	&insn_setend,
> >>> +#endif
> >>> +};
> >>>  
> >>> -static LIST_HEAD(insn_emulation);
> >>> -static int nr_insn_emulated __initdata;
> >>> -static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
> >>>  static DEFINE_MUTEX(insn_emulation_mutex);
> >>>  
> >>> -static void register_emulation_hooks(struct insn_emulation *insn)
> >>> -{
> >>> -	struct undef_hook *hook;
> >>> -
> >>> -	BUG_ON(!insn->hooks);
> >>> -
> >>> -	for (hook = insn->hooks; hook->instr_mask; hook++)
> >>> -		register_undef_hook(hook);
> >>> -
> >>> -	pr_notice("Registered %s emulation handler\n", insn->name);
> >>> -}
> >>> -
> >>> -static void remove_emulation_hooks(struct insn_emulation *insn)
> >>> -{
> >>> -	struct undef_hook *hook;
> >>> -
> >>> -	BUG_ON(!insn->hooks);
> >>> -
> >>> -	for (hook = insn->hooks; hook->instr_mask; hook++)
> >>> -		unregister_undef_hook(hook);
> >>> -
> >>> -	pr_notice("Removed %s emulation handler\n", insn->name);
> >>> -}
> >>> -
> >>>  static void enable_insn_hw_mode(void *data)
> >>>  {
> >>>  	struct insn_emulation *insn = (struct insn_emulation *)data;
> >>> @@ -469,18 +454,24 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
> >>>  {
> >>>  	int rc = 0;
> >>>  	unsigned long flags;
> >>> -	struct insn_emulation *insn;
> >>>  
> >>> -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> >>> -	list_for_each_entry(insn, &insn_emulation, node) {
> >>> -		bool enable = (insn->current_mode == INSN_HW);
> >>> +	/*
> >>> +	 * Disable IRQs to serialize against an IPI from
> >>> +	 * run_all_cpu_set_hw_mode(), ensuring the HW is programmed to the most
> >>> +	 * recent enablement state if the two race with one another.
> >>> +	 */
> >>> +	local_irq_save(flags);
> >>> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> >>> +		struct insn_emulation *insn = insn_emulations[i];
> >>> +		bool enable = READ_ONCE(insn->current_mode) == INSN_HW;
> >>>  		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
> >>>  			pr_warn("CPU[%u] cannot support the emulation of %s",
> >>>  				cpu, insn->name);
> >>>  			rc = -EINVAL;
> >>>  		}
> >>>  	}
> >>> -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
> >>> +	local_irq_restore(flags);
> >>> +
> >>>  	return rc;
> >>>  }
> >>>  
> >>> @@ -493,7 +484,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
> >>>  	case INSN_UNDEF: /* Nothing to be done */
> >>>  		break;
> >>>  	case INSN_EMULATE:
> >>> -		remove_emulation_hooks(insn);
> >>>  		break;
> >>>  	case INSN_HW:
> >>>  		if (!run_all_cpu_set_hw_mode(insn, false))
> >>> @@ -505,7 +495,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
> >>>  	case INSN_UNDEF:
> >>>  		break;
> >>>  	case INSN_EMULATE:
> >>> -		register_emulation_hooks(insn);
> >>>  		break;
> >>>  	case INSN_HW:
> >>>  		ret = run_all_cpu_set_hw_mode(insn, true);
> >>> @@ -517,34 +506,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
> >>>  	return ret;
> >>>  }
> >>>  
> >>> -static void __init register_insn_emulation(struct insn_emulation *insn)
> >>> -{
> >>> -	unsigned long flags;
> >>> -
> >>> -	insn->min = INSN_UNDEF;
> >>> -
> >>> -	switch (insn->status) {
> >>> -	case INSN_DEPRECATED:
> >>> -		insn->current_mode = INSN_EMULATE;
> >>> -		/* Disable the HW mode if it was turned on at early boot time */
> >>> -		run_all_cpu_set_hw_mode(insn, false);
> >>> -		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 *buffer, size_t *lenp,
> >>>  				  loff_t *ppos)
> >>> @@ -562,7 +523,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
> >>>  	ret = update_insn_emulation_mode(insn, prev_mode);
> >>>  	if (ret) {
> >>>  		/* Mode change failed, revert to previous mode. */
> >>> -		insn->current_mode = prev_mode;
> >>> +		WRITE_ONCE(insn->current_mode, prev_mode);
> >>>  		update_insn_emulation_mode(insn, INSN_UNDEF);
> >>>  	}
> >>>  ret:
> >>> @@ -570,21 +531,34 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
> >>>  	return ret;
> >>>  }
> >>>  
> >>> -static void __init register_insn_emulation_sysctl(void)
> >>> +static void __init register_insn_emulation(struct insn_emulation *insn)
> >>>  {
> >>> -	unsigned long flags;
> >>> -	int i = 0;
> >>> -	struct insn_emulation *insn;
> >>> -	struct ctl_table *insns_sysctl, *sysctl;
> >>> +	struct ctl_table *sysctl;
> >>>  
> >>> -	insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
> >>> -			       GFP_KERNEL);
> >>> -	if (!insns_sysctl)
> >>> -		return;
> >>> +	insn->min = INSN_UNDEF;
> >>>  
> >>> -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
> >>> -	list_for_each_entry(insn, &insn_emulation, node) {
> >>> -		sysctl = &insns_sysctl[i];
> >>> +	switch (insn->status) {
> >>> +	case INSN_DEPRECATED:
> >>> +		insn->current_mode = INSN_EMULATE;
> >>> +		/* Disable the HW mode if it was turned on at early boot time */
> >>> +		run_all_cpu_set_hw_mode(insn, false);
> >>> +		insn->max = INSN_HW;
> >>> +		break;
> >>> +	case INSN_OBSOLETE:
> >>> +		insn->current_mode = INSN_UNDEF;
> >>> +		insn->max = INSN_EMULATE;
> >>> +		break;
> >>> +	case INSN_UNAVAILABLE:
> >>> +		insn->current_mode = INSN_UNDEF;
> >>> +		insn->max = INSN_UNDEF;
> >>> +		break;
> >>> +	}
> >>> +
> >>> +	/* Program the HW if required */
> >>> +	update_insn_emulation_mode(insn, INSN_UNDEF);
> >>> +
> >>> +	if (insn->status != INSN_UNAVAILABLE) {
> >>> +		sysctl = &insn->sysctl[0];
> >>>  
> >>>  		sysctl->mode = 0644;
> >>>  		sysctl->maxlen = sizeof(int);
> >>> @@ -594,11 +568,32 @@ static void __init register_insn_emulation_sysctl(void)
> >>>  		sysctl->extra1 = &insn->min;
> >>>  		sysctl->extra2 = &insn->max;
> >>>  		sysctl->proc_handler = emulation_proc_handler;
> >>> -		i++;
> >>> +
> >>> +		register_sysctl("abi", sysctl);
> >>> +	}
> >>> +}
> >>> +
> >>> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
> >>> +{
> >>> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> >>> +		struct insn_emulation *ie = insn_emulations[i];
> >>> +
> >>> +		if (ie->status == INSN_UNAVAILABLE)
> >>> +			continue;
> >>> +
> >>> +		/*
> >>> +		 * A trap may race with the mode being changed
> >>> +		 * INSN_EMULATE<->INSN_HW. Try to emulate the instruction to
> >>> +		 * avoid a spurious UNDEF.
> >>> +		 */
> >>> +		if (READ_ONCE(ie->current_mode) == INSN_UNDEF)
> >>> +			continue;
> >>> +
> >>> +		if (ie->try_emulate(regs, insn))
> >>> +			return true;
> >>>  	}
> >>> -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
> >>>  
> >>> -	register_sysctl("abi", insns_sysctl);
> >>> +	return false;
> >>>  }
> >>>  
> >>>  /*
> >>> @@ -607,24 +602,25 @@ static void __init register_insn_emulation_sysctl(void)
> >>>   */
> >>>  static int __init armv8_deprecated_init(void)
> >>>  {
> >>> -	if (IS_ENABLED(CONFIG_SWP_EMULATION))
> >>> -		register_insn_emulation(&insn_swp);
> >>> +#ifdef CONFIG_SETEND_EMULATION
> >>> +	if (!system_supports_mixed_endian_el0()) {
> >>> +		insn_setend.status = INSN_UNAVAILABLE;
> >>> +		pr_info("setend instruction emulation is not supported on this system\n");
> >>> +	}
> >>>  
> >>> -	if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
> >>> -		register_insn_emulation(&insn_cp15_barrier);
> >>> +#endif
> >>> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
> >>> +		struct insn_emulation *ie = insn_emulations[i];
> >>>  
> >>> -	if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
> >>> -		if (system_supports_mixed_endian_el0())
> >>> -			register_insn_emulation(&insn_setend);
> >>> -		else
> >>> -			pr_info("setend instruction emulation is not supported on this system\n");
> >>> +		if (ie->status == INSN_UNAVAILABLE)
> >>> +			continue;
> >>> +
> >>> +		register_insn_emulation(ie);
> >>>  	}
> >>>  
> >>>  	cpuhp_setup_state_nocalls(CPUHP_AP_ARM64_ISNDEP_STARTING,
> >>>  				  "arm64/isndep:starting",
> >>>  				  run_all_insn_set_hw_mode, NULL);
> >>> -	register_insn_emulation_sysctl();
> >>> -
> >>>  	return 0;
> >>>  }
> >>>  
> >>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> >>> index 96eaf1aaec12..4c0caa589e12 100644
> >>> --- a/arch/arm64/kernel/traps.c
> >>> +++ b/arch/arm64/kernel/traps.c
> >>> @@ -373,27 +373,6 @@ void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
> >>>  		regs->pstate &= ~PSR_BTYPE_MASK;
> >>>  }
> >>>  
> >>> -static LIST_HEAD(undef_hook);
> >>> -static DEFINE_RAW_SPINLOCK(undef_lock);
> >>> -
> >>> -void 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);
> >>> -}
> >>> -
> >>> -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 user_insn_read(struct pt_regs *regs, u32 *insnp)
> >>>  {
> >>>  	u32 instr;
> >>> @@ -425,23 +404,6 @@ static int user_insn_read(struct pt_regs *regs, u32 *insnp)
> >>>  	return 0;
> >>>  }
> >>>  
> >>> -static int call_undef_hook(struct pt_regs *regs, u32 instr)
> >>> -{
> >>> -	struct undef_hook *hook;
> >>> -	unsigned long flags;
> >>> -	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
> >>> -
> >>> -	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);
> >>> -
> >>> -	return fn ? fn(regs, instr) : 1;
> >>> -}
> >>> -
> >>>  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err)
> >>>  {
> >>>  	const char *desc;
> >>> @@ -502,7 +464,7 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
> >>>  	if (try_emulate_mrs(regs, insn))
> >>>  		return;
> >>>  
> >>> -	if (call_undef_hook(regs, insn) == 0)
> >>> +	if (try_emulate_armv8_deprecated(regs, insn))
> >>>  		return;
> >>>  
> >>>  out_err:
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 

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

^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling
  2023-02-06  9:13         ` Mark Rutland
@ 2023-02-07  7:01           ` Ruan Jinjie
  0 siblings, 0 replies; 16+ messages in thread
From: Ruan Jinjie @ 2023-02-07  7:01 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, liaochang (A)



On 2023/2/6 17:13, Mark Rutland wrote:
> On Mon, Feb 06, 2023 at 10:03:17AM +0800, Ruan Jinjie wrote:
>> On 2023/2/4 1:27, Mark Rutland wrote:
>>> On Fri, Feb 03, 2023 at 06:08:08PM +0800, Ruan Jinjie wrote:
>>>> Hi, Mark, I want to merge this patch to linux-5.10.y to solve a list_add
>>>> double problem. However, I can not trigger the emulation of these
>>>> deprecated instructions in qemu with userspace program as below, but
>>>> cause IABT and DABT,
>>>
>>> Well that certainly sounds wrong!
>>>
>>> When you say "in qemu", so you mean in a KVM VM, a TCG VM, or usermode
>>> emulation?
>>>
>>> Can you tell me exactly how you're invoking qemu?
>>
>> Sorry, the information given was incomplete.I invoke qemu 6.1.1 with
>> following commands, which is a TCG VM:
>>
>> qemu-system-aarch64
>> 	-M virt \
>> 	-cpu cortex-a57 \
>> 	-smp 4 \
>> 	-m 4096 \
>> 	-nographic \
>> 	-kernel linux-stable/arch/arm64/boot/Image \
>> 	-drive if=none,file=rootfs.img,format=raw,id=hd0 \
>> 	-device virtio-net-device,netdev=net0 \
>> 	-netdev user,id=net0,host=10.0.2.1,hostfwd=tcp::5555-:22 \
>> 	-device virtio-blk-device,drive=hd0 \
>> 	-append "root=/dev/vda console=ttyAMA0 rootfstype=ext4 init=/linuxrc rw
>> oops=panic nmi_watchdog=panic panic=1"
>>
>>>
>>> Can you give an example of the IABT and DABT you see?
>>
>> Because the emulation of cp15 deprecated instructions is not triggered,I
>> add printk message in el0_sync_compat_handler() function 
> 
> Ah! So you mean that those are the *only* exceptions you see, not that those
> are *unexpected*. AFAICT, those are jsut the test code and data being faulted
> in as usual, and are not related to the emulation.
> 
> I assume the test completes successfully, and returns 0?
> 
> I had a play with this locally (with QEMU 6.2.94), and I see that the
> deprecated instructions work even when neither the emulation nor HW support is
> enabled (in the relevant SCTLR_ELx bits), and I think that what's happening
> here is the QEMU isn't correctly emulating the architecture:
> 
> * QEMU seems to ignore SCTLR_ELx.CP15BEN, and always imeplements those rather
>   than making those UNDEFINED when SCTLR_ELx.CP15BEN == 0b0.
> 
> * QEMU seems to ignore SCTLR_ELx.SED, and always implements SETEND rather than
>   making that UNDEFINED when SCTLR_ELx.SED == 0b1.
> 
> * QEMU seems to implement SWP*, even though those instructions don't exist in
>   ARMv8-A and should always be UNDEFINED.

I agree with you! It is common that the qemu implementation is
inconsistent with the architecture manual specification in some details.

>  
>> to check the EC code When the following user-space code is executed.The print
>> is as follows:
>>
>> /home # ./cp15_barrier_test
>> [   72.469366] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
>> [   72.470477] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
>> [   72.471342] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
>> [   72.472341] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
>> [   72.474033] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
>> [   72.475145] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
>> [   72.476255] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
>> [   72.477861] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
>> [   72.478427] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
>> [   72.479111] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
>> [   72.479919] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
>> [   72.480467] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
>> [   72.481616] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
>> [   72.483386] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
>> [   72.484187] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
>> [   72.485215] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
>> [   72.485882] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x24
>> [   72.486628] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
>> [   72.487495] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x20
>> [   72.488548] el0_sync_compat_handler, ESR_ELx_EC(esr)= 0x11
>>
>> I went through "ARM® Architecture Reference Manual ARMv8, for ARMv8-A
>> architecture profile" and check the header arch/arm64/include/asm/esr.h,
>> find it's EC codes for SVC32, IABT and DABT.
>>
>>  #define ESR_ELx_EC_SVC32    (0x11)
>>  #define ESR_ELx_EC_IABT_LOW (0x20)
>>  #define ESR_ELx_EC_DABT_LOW (0x24)
>>
>> I also learned from the manual that the SCTLR_EL1.CP15BEN is critical,
>> which control the behavior of above deprecating memory barrier
>> instructions which can be set by the 'echo 2 >
>> /proc/sys/abi/cp15_barrier' command.However, the result is the same
>> whether it is set or not.
> 
> As above, I think this is a QEMU bug, with QEMU erroneosuly implementing the
> instructions when they should be UNDEFINED. The aborts are unrelated, and are
> just the usual faulting in of code and data pages.
> 
> So I'd recommend reporting tha to QEMU folk, and in the mean time testing on
> real HW rather than QEMU in TCG mode. When I wrote these patches I tested on a
> Raspberry Pi 4, under a KVM virtual machine.

Thanks for your advice. I'll test it on real HW.

> 
> Thanks,
> Mark.
> 
>>
>> I think the code went into the wrong code flow because of the wrong EC
>> code, but I don't know why the wrong EC value was passed in.
>>
>>>
>>>> so that I can not verify the correctness of the
>>>> patch.Can you give me some help to test the emulation of these
>>>> deprecated instructions?
>>>>
>>>> #include<stdio.h>
>>>>
>>>> #define isb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c5, 4" \
>>>>                                     : : "r" (0) : "memory")
>>>> #define dsb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 4" \
>>>>                                     : : "r" (0) : "memory")
>>>> #define dmb(x) __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
>>>>                                     : : "r" (0) : "memory")
>>>
>>> FWIW, these encodings look correc to me.
>>>
>>> Thanks,
>>> Mark.
>>>
>>>>
>>>> int main(){
>>>>         isb();
>>>>         dsb();
>>>>         dmb();
>>>>         return 0;
>>>> }
>>>>
>>>>>
>>>>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>>> Cc: James Morse <james.morse@arm.com>
>>>>> Cc: Joey Gouly <joey.gouly@arm.com>
>>>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>>>> Cc: Will Deacon <will@kernel.org>
>>>>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>>>>> ---
>>>>>  arch/arm64/include/asm/traps.h       |  19 +-
>>>>>  arch/arm64/kernel/armv8_deprecated.c | 282 +++++++++++++--------------
>>>>>  arch/arm64/kernel/traps.c            |  40 +---
>>>>>  3 files changed, 149 insertions(+), 192 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
>>>>> index 6e5826470bea..1f361e2da516 100644
>>>>> --- a/arch/arm64/include/asm/traps.h
>>>>> +++ b/arch/arm64/include/asm/traps.h
>>>>> @@ -13,17 +13,16 @@
>>>>>  
>>>>>  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);
>>>>> -};
>>>>> +#ifdef CONFIG_ARMV8_DEPRECATED
>>>>> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn);
>>>>> +#else
>>>>> +static inline bool
>>>>> +try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
>>>>> +{
>>>>> +	return false;
>>>>> +}
>>>>> +#endif /* CONFIG_ARMV8_DEPRECATED */
>>>>>  
>>>>> -void register_undef_hook(struct undef_hook *hook);
>>>>> -void unregister_undef_hook(struct undef_hook *hook);
>>>>>  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err);
>>>>>  void arm64_notify_segfault(unsigned long addr);
>>>>>  void arm64_force_sig_fault(int signo, int code, unsigned long far, const char *str);
>>>>> diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
>>>>> index 7f2ce49dbf97..ed0788cf6bbb 100644
>>>>> --- a/arch/arm64/kernel/armv8_deprecated.c
>>>>> +++ b/arch/arm64/kernel/armv8_deprecated.c
>>>>> @@ -38,17 +38,24 @@ enum insn_emulation_mode {
>>>>>  enum legacy_insn_status {
>>>>>  	INSN_DEPRECATED,
>>>>>  	INSN_OBSOLETE,
>>>>> +	INSN_UNAVAILABLE,
>>>>>  };
>>>>>  
>>>>>  struct insn_emulation {
>>>>>  	const char			*name;
>>>>> -	struct list_head		node;
>>>>>  	enum legacy_insn_status		status;
>>>>> -	struct undef_hook		*hooks;
>>>>> +	bool				(*try_emulate)(struct pt_regs *regs,
>>>>> +						       u32 insn);
>>>>>  	int				(*set_hw_mode)(bool enable);
>>>>> +
>>>>>  	int current_mode;
>>>>>  	int min;
>>>>>  	int max;
>>>>> +
>>>>> +	/*
>>>>> +	 * sysctl for this emulation + a sentinal entry.
>>>>> +	 */
>>>>> +	struct ctl_table sysctl[2];
>>>>>  };
>>>>>  
>>>>>  #define ARM_OPCODE_CONDTEST_FAIL   0
>>>>> @@ -70,6 +77,7 @@ static unsigned int aarch32_check_condition(u32 opcode, u32 psr)
>>>>>  	return ARM_OPCODE_CONDTEST_UNCOND;
>>>>>  }
>>>>>  
>>>>> +#ifdef CONFIG_SWP_EMULATION
>>>>>  /*
>>>>>   *  Implement emulation of the SWP/SWPB instructions using load-exclusive and
>>>>>   *  store-exclusive.
>>>>> @@ -222,28 +230,27 @@ static int swp_handler(struct pt_regs *regs, u32 instr)
>>>>>  	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	= PSR_AA32_MODE_MASK,
>>>>> -		.pstate_val	= PSR_AA32_MODE_USR,
>>>>> -		.fn		= swp_handler
>>>>> -	},
>>>>> -	{ }
>>>>> -};
>>>>> +static bool try_emulate_swp(struct pt_regs *regs, u32 insn)
>>>>> +{
>>>>> +	/* SWP{B} only exists in ARM state and does not exist in Thumb */
>>>>> +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
>>>>> +		return false;
>>>>> +
>>>>> +	if ((insn & 0x0fb00ff0) != 0x01000090)
>>>>> +		return false;
>>>>> +
>>>>> +	return swp_handler(regs, insn) == 0;
>>>>> +}
>>>>>  
>>>>>  static struct insn_emulation insn_swp = {
>>>>>  	.name = "swp",
>>>>>  	.status = INSN_OBSOLETE,
>>>>> -	.hooks = swp_hooks,
>>>>> +	.try_emulate = try_emulate_swp,
>>>>>  	.set_hw_mode = NULL,
>>>>>  };
>>>>> +#endif /* CONFIG_SWP_EMULATION */
>>>>>  
>>>>> +#ifdef CONFIG_CP15_BARRIER_EMULATION
>>>>>  static int cp15barrier_handler(struct pt_regs *regs, u32 instr)
>>>>>  {
>>>>>  	perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->pc);
>>>>> @@ -306,31 +313,29 @@ static int cp15_barrier_set_hw_mode(bool enable)
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> -static struct undef_hook cp15_barrier_hooks[] = {
>>>>> -	{
>>>>> -		.instr_mask	= 0x0fff0fdf,
>>>>> -		.instr_val	= 0x0e070f9a,
>>>>> -		.pstate_mask	= PSR_AA32_MODE_MASK,
>>>>> -		.pstate_val	= PSR_AA32_MODE_USR,
>>>>> -		.fn		= cp15barrier_handler,
>>>>> -	},
>>>>> -	{
>>>>> -		.instr_mask	= 0x0fff0fff,
>>>>> -		.instr_val	= 0x0e070f95,
>>>>> -		.pstate_mask	= PSR_AA32_MODE_MASK,
>>>>> -		.pstate_val	= PSR_AA32_MODE_USR,
>>>>> -		.fn		= cp15barrier_handler,
>>>>> -	},
>>>>> -	{ }
>>>>> -};
>>>>> +static bool try_emulate_cp15_barrier(struct pt_regs *regs, u32 insn)
>>>>> +{
>>>>> +	if (!compat_user_mode(regs) || compat_thumb_mode(regs))
>>>>> +		return false;
>>>>> +
>>>>> +	if ((insn & 0x0fff0fdf) == 0x0e070f9a)
>>>>> +		return cp15barrier_handler(regs, insn) == 0;
>>>>> +
>>>>> +	if ((insn & 0x0fff0fff) == 0x0e070f95)
>>>>> +		return cp15barrier_handler(regs, insn) == 0;
>>>>> +
>>>>> +	return false;
>>>>> +}
>>>>>  
>>>>>  static struct insn_emulation insn_cp15_barrier = {
>>>>>  	.name = "cp15_barrier",
>>>>>  	.status = INSN_DEPRECATED,
>>>>> -	.hooks = cp15_barrier_hooks,
>>>>> +	.try_emulate = try_emulate_cp15_barrier,
>>>>>  	.set_hw_mode = cp15_barrier_set_hw_mode,
>>>>>  };
>>>>> +#endif /* CONFIG_CP15_BARRIER_EMULATION */
>>>>>  
>>>>> +#ifdef CONFIG_SETEND_EMULATION
>>>>>  static int setend_set_hw_mode(bool enable)
>>>>>  {
>>>>>  	if (!cpu_supports_mixed_endian_el0())
>>>>> @@ -378,61 +383,41 @@ static int t16_setend_handler(struct pt_regs *regs, u32 instr)
>>>>>  	return rc;
>>>>>  }
>>>>>  
>>>>> -static struct undef_hook setend_hooks[] = {
>>>>> -	{
>>>>> -		.instr_mask	= 0xfffffdff,
>>>>> -		.instr_val	= 0xf1010000,
>>>>> -		.pstate_mask	= PSR_AA32_MODE_MASK,
>>>>> -		.pstate_val	= PSR_AA32_MODE_USR,
>>>>> -		.fn		= a32_setend_handler,
>>>>> -	},
>>>>> -	{
>>>>> -		/* Thumb mode */
>>>>> -		.instr_mask	= 0xfffffff7,
>>>>> -		.instr_val	= 0x0000b650,
>>>>> -		.pstate_mask	= (PSR_AA32_T_BIT | PSR_AA32_MODE_MASK),
>>>>> -		.pstate_val	= (PSR_AA32_T_BIT | PSR_AA32_MODE_USR),
>>>>> -		.fn		= t16_setend_handler,
>>>>> -	},
>>>>> -	{}
>>>>> -};
>>>>> +static bool try_emulate_setend(struct pt_regs *regs, u32 insn)
>>>>> +{
>>>>> +	if (compat_thumb_mode(regs) &&
>>>>> +	    (insn & 0xfffffff7) == 0x0000b650)
>>>>> +		return t16_setend_handler(regs, insn) == 0;
>>>>> +
>>>>> +	if (compat_user_mode(regs) &&
>>>>> +	    (insn & 0xfffffdff) == 0xf1010000)
>>>>> +		return a32_setend_handler(regs, insn) == 0;
>>>>> +
>>>>> +	return false;
>>>>> +}
>>>>>  
>>>>>  static struct insn_emulation insn_setend = {
>>>>>  	.name = "setend",
>>>>>  	.status = INSN_DEPRECATED,
>>>>> -	.hooks = setend_hooks,
>>>>> +	.try_emulate = try_emulate_setend,
>>>>>  	.set_hw_mode = setend_set_hw_mode,
>>>>>  };
>>>>> +#endif /* CONFIG_SETEND_EMULATION */
>>>>> +
>>>>> +static struct insn_emulation *insn_emulations[] = {
>>>>> +#ifdef CONFIG_SWP_EMULATION
>>>>> +	&insn_swp,
>>>>> +#endif
>>>>> +#ifdef CONFIG_CP15_BARRIER_EMULATION
>>>>> +	&insn_cp15_barrier,
>>>>> +#endif
>>>>> +#ifdef CONFIG_SETEND_EMULATION
>>>>> +	&insn_setend,
>>>>> +#endif
>>>>> +};
>>>>>  
>>>>> -static LIST_HEAD(insn_emulation);
>>>>> -static int nr_insn_emulated __initdata;
>>>>> -static DEFINE_RAW_SPINLOCK(insn_emulation_lock);
>>>>>  static DEFINE_MUTEX(insn_emulation_mutex);
>>>>>  
>>>>> -static void register_emulation_hooks(struct insn_emulation *insn)
>>>>> -{
>>>>> -	struct undef_hook *hook;
>>>>> -
>>>>> -	BUG_ON(!insn->hooks);
>>>>> -
>>>>> -	for (hook = insn->hooks; hook->instr_mask; hook++)
>>>>> -		register_undef_hook(hook);
>>>>> -
>>>>> -	pr_notice("Registered %s emulation handler\n", insn->name);
>>>>> -}
>>>>> -
>>>>> -static void remove_emulation_hooks(struct insn_emulation *insn)
>>>>> -{
>>>>> -	struct undef_hook *hook;
>>>>> -
>>>>> -	BUG_ON(!insn->hooks);
>>>>> -
>>>>> -	for (hook = insn->hooks; hook->instr_mask; hook++)
>>>>> -		unregister_undef_hook(hook);
>>>>> -
>>>>> -	pr_notice("Removed %s emulation handler\n", insn->name);
>>>>> -}
>>>>> -
>>>>>  static void enable_insn_hw_mode(void *data)
>>>>>  {
>>>>>  	struct insn_emulation *insn = (struct insn_emulation *)data;
>>>>> @@ -469,18 +454,24 @@ static int run_all_insn_set_hw_mode(unsigned int cpu)
>>>>>  {
>>>>>  	int rc = 0;
>>>>>  	unsigned long flags;
>>>>> -	struct insn_emulation *insn;
>>>>>  
>>>>> -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
>>>>> -	list_for_each_entry(insn, &insn_emulation, node) {
>>>>> -		bool enable = (insn->current_mode == INSN_HW);
>>>>> +	/*
>>>>> +	 * Disable IRQs to serialize against an IPI from
>>>>> +	 * run_all_cpu_set_hw_mode(), ensuring the HW is programmed to the most
>>>>> +	 * recent enablement state if the two race with one another.
>>>>> +	 */
>>>>> +	local_irq_save(flags);
>>>>> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
>>>>> +		struct insn_emulation *insn = insn_emulations[i];
>>>>> +		bool enable = READ_ONCE(insn->current_mode) == INSN_HW;
>>>>>  		if (insn->set_hw_mode && insn->set_hw_mode(enable)) {
>>>>>  			pr_warn("CPU[%u] cannot support the emulation of %s",
>>>>>  				cpu, insn->name);
>>>>>  			rc = -EINVAL;
>>>>>  		}
>>>>>  	}
>>>>> -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
>>>>> +	local_irq_restore(flags);
>>>>> +
>>>>>  	return rc;
>>>>>  }
>>>>>  
>>>>> @@ -493,7 +484,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
>>>>>  	case INSN_UNDEF: /* Nothing to be done */
>>>>>  		break;
>>>>>  	case INSN_EMULATE:
>>>>> -		remove_emulation_hooks(insn);
>>>>>  		break;
>>>>>  	case INSN_HW:
>>>>>  		if (!run_all_cpu_set_hw_mode(insn, false))
>>>>> @@ -505,7 +495,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
>>>>>  	case INSN_UNDEF:
>>>>>  		break;
>>>>>  	case INSN_EMULATE:
>>>>> -		register_emulation_hooks(insn);
>>>>>  		break;
>>>>>  	case INSN_HW:
>>>>>  		ret = run_all_cpu_set_hw_mode(insn, true);
>>>>> @@ -517,34 +506,6 @@ static int update_insn_emulation_mode(struct insn_emulation *insn,
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> -static void __init register_insn_emulation(struct insn_emulation *insn)
>>>>> -{
>>>>> -	unsigned long flags;
>>>>> -
>>>>> -	insn->min = INSN_UNDEF;
>>>>> -
>>>>> -	switch (insn->status) {
>>>>> -	case INSN_DEPRECATED:
>>>>> -		insn->current_mode = INSN_EMULATE;
>>>>> -		/* Disable the HW mode if it was turned on at early boot time */
>>>>> -		run_all_cpu_set_hw_mode(insn, false);
>>>>> -		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 *buffer, size_t *lenp,
>>>>>  				  loff_t *ppos)
>>>>> @@ -562,7 +523,7 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
>>>>>  	ret = update_insn_emulation_mode(insn, prev_mode);
>>>>>  	if (ret) {
>>>>>  		/* Mode change failed, revert to previous mode. */
>>>>> -		insn->current_mode = prev_mode;
>>>>> +		WRITE_ONCE(insn->current_mode, prev_mode);
>>>>>  		update_insn_emulation_mode(insn, INSN_UNDEF);
>>>>>  	}
>>>>>  ret:
>>>>> @@ -570,21 +531,34 @@ static int emulation_proc_handler(struct ctl_table *table, int write,
>>>>>  	return ret;
>>>>>  }
>>>>>  
>>>>> -static void __init register_insn_emulation_sysctl(void)
>>>>> +static void __init register_insn_emulation(struct insn_emulation *insn)
>>>>>  {
>>>>> -	unsigned long flags;
>>>>> -	int i = 0;
>>>>> -	struct insn_emulation *insn;
>>>>> -	struct ctl_table *insns_sysctl, *sysctl;
>>>>> +	struct ctl_table *sysctl;
>>>>>  
>>>>> -	insns_sysctl = kcalloc(nr_insn_emulated + 1, sizeof(*sysctl),
>>>>> -			       GFP_KERNEL);
>>>>> -	if (!insns_sysctl)
>>>>> -		return;
>>>>> +	insn->min = INSN_UNDEF;
>>>>>  
>>>>> -	raw_spin_lock_irqsave(&insn_emulation_lock, flags);
>>>>> -	list_for_each_entry(insn, &insn_emulation, node) {
>>>>> -		sysctl = &insns_sysctl[i];
>>>>> +	switch (insn->status) {
>>>>> +	case INSN_DEPRECATED:
>>>>> +		insn->current_mode = INSN_EMULATE;
>>>>> +		/* Disable the HW mode if it was turned on at early boot time */
>>>>> +		run_all_cpu_set_hw_mode(insn, false);
>>>>> +		insn->max = INSN_HW;
>>>>> +		break;
>>>>> +	case INSN_OBSOLETE:
>>>>> +		insn->current_mode = INSN_UNDEF;
>>>>> +		insn->max = INSN_EMULATE;
>>>>> +		break;
>>>>> +	case INSN_UNAVAILABLE:
>>>>> +		insn->current_mode = INSN_UNDEF;
>>>>> +		insn->max = INSN_UNDEF;
>>>>> +		break;
>>>>> +	}
>>>>> +
>>>>> +	/* Program the HW if required */
>>>>> +	update_insn_emulation_mode(insn, INSN_UNDEF);
>>>>> +
>>>>> +	if (insn->status != INSN_UNAVAILABLE) {
>>>>> +		sysctl = &insn->sysctl[0];
>>>>>  
>>>>>  		sysctl->mode = 0644;
>>>>>  		sysctl->maxlen = sizeof(int);
>>>>> @@ -594,11 +568,32 @@ static void __init register_insn_emulation_sysctl(void)
>>>>>  		sysctl->extra1 = &insn->min;
>>>>>  		sysctl->extra2 = &insn->max;
>>>>>  		sysctl->proc_handler = emulation_proc_handler;
>>>>> -		i++;
>>>>> +
>>>>> +		register_sysctl("abi", sysctl);
>>>>> +	}
>>>>> +}
>>>>> +
>>>>> +bool try_emulate_armv8_deprecated(struct pt_regs *regs, u32 insn)
>>>>> +{
>>>>> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
>>>>> +		struct insn_emulation *ie = insn_emulations[i];
>>>>> +
>>>>> +		if (ie->status == INSN_UNAVAILABLE)
>>>>> +			continue;
>>>>> +
>>>>> +		/*
>>>>> +		 * A trap may race with the mode being changed
>>>>> +		 * INSN_EMULATE<->INSN_HW. Try to emulate the instruction to
>>>>> +		 * avoid a spurious UNDEF.
>>>>> +		 */
>>>>> +		if (READ_ONCE(ie->current_mode) == INSN_UNDEF)
>>>>> +			continue;
>>>>> +
>>>>> +		if (ie->try_emulate(regs, insn))
>>>>> +			return true;
>>>>>  	}
>>>>> -	raw_spin_unlock_irqrestore(&insn_emulation_lock, flags);
>>>>>  
>>>>> -	register_sysctl("abi", insns_sysctl);
>>>>> +	return false;
>>>>>  }
>>>>>  
>>>>>  /*
>>>>> @@ -607,24 +602,25 @@ static void __init register_insn_emulation_sysctl(void)
>>>>>   */
>>>>>  static int __init armv8_deprecated_init(void)
>>>>>  {
>>>>> -	if (IS_ENABLED(CONFIG_SWP_EMULATION))
>>>>> -		register_insn_emulation(&insn_swp);
>>>>> +#ifdef CONFIG_SETEND_EMULATION
>>>>> +	if (!system_supports_mixed_endian_el0()) {
>>>>> +		insn_setend.status = INSN_UNAVAILABLE;
>>>>> +		pr_info("setend instruction emulation is not supported on this system\n");
>>>>> +	}
>>>>>  
>>>>> -	if (IS_ENABLED(CONFIG_CP15_BARRIER_EMULATION))
>>>>> -		register_insn_emulation(&insn_cp15_barrier);
>>>>> +#endif
>>>>> +	for (int i = 0; i < ARRAY_SIZE(insn_emulations); i++) {
>>>>> +		struct insn_emulation *ie = insn_emulations[i];
>>>>>  
>>>>> -	if (IS_ENABLED(CONFIG_SETEND_EMULATION)) {
>>>>> -		if (system_supports_mixed_endian_el0())
>>>>> -			register_insn_emulation(&insn_setend);
>>>>> -		else
>>>>> -			pr_info("setend instruction emulation is not supported on this system\n");
>>>>> +		if (ie->status == INSN_UNAVAILABLE)
>>>>> +			continue;
>>>>> +
>>>>> +		register_insn_emulation(ie);
>>>>>  	}
>>>>>  
>>>>>  	cpuhp_setup_state_nocalls(CPUHP_AP_ARM64_ISNDEP_STARTING,
>>>>>  				  "arm64/isndep:starting",
>>>>>  				  run_all_insn_set_hw_mode, NULL);
>>>>> -	register_insn_emulation_sysctl();
>>>>> -
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
>>>>> index 96eaf1aaec12..4c0caa589e12 100644
>>>>> --- a/arch/arm64/kernel/traps.c
>>>>> +++ b/arch/arm64/kernel/traps.c
>>>>> @@ -373,27 +373,6 @@ void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
>>>>>  		regs->pstate &= ~PSR_BTYPE_MASK;
>>>>>  }
>>>>>  
>>>>> -static LIST_HEAD(undef_hook);
>>>>> -static DEFINE_RAW_SPINLOCK(undef_lock);
>>>>> -
>>>>> -void 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);
>>>>> -}
>>>>> -
>>>>> -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 user_insn_read(struct pt_regs *regs, u32 *insnp)
>>>>>  {
>>>>>  	u32 instr;
>>>>> @@ -425,23 +404,6 @@ static int user_insn_read(struct pt_regs *regs, u32 *insnp)
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> -static int call_undef_hook(struct pt_regs *regs, u32 instr)
>>>>> -{
>>>>> -	struct undef_hook *hook;
>>>>> -	unsigned long flags;
>>>>> -	int (*fn)(struct pt_regs *regs, u32 instr) = NULL;
>>>>> -
>>>>> -	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);
>>>>> -
>>>>> -	return fn ? fn(regs, instr) : 1;
>>>>> -}
>>>>> -
>>>>>  void force_signal_inject(int signal, int code, unsigned long address, unsigned long err)
>>>>>  {
>>>>>  	const char *desc;
>>>>> @@ -502,7 +464,7 @@ void do_el0_undef(struct pt_regs *regs, unsigned long esr)
>>>>>  	if (try_emulate_mrs(regs, insn))
>>>>>  		return;
>>>>>  
>>>>> -	if (call_undef_hook(regs, insn) == 0)
>>>>> +	if (try_emulate_armv8_deprecated(regs, insn))
>>>>>  		return;
>>>>>  
>>>>>  out_err:
>>>>
>>>> _______________________________________________
>>>> linux-arm-kernel mailing list
>>>> linux-arm-kernel@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-02-07  7:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-19 14:41 [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Mark Rutland
2022-10-19 14:41 ` [PATCH v2 1/9] arm64: allow kprobes on EL0 handlers Mark Rutland
2022-10-19 14:41 ` [PATCH v2 2/9] arm64: split EL0/EL1 UNDEF handlers Mark Rutland
2022-10-19 14:41 ` [PATCH v2 3/9] arm64: factor out EL1 SSBS emulation hook Mark Rutland
2022-10-19 14:41 ` [PATCH v2 4/9] arm64: factor insn read out of call_undef_hook() Mark Rutland
2022-10-19 14:41 ` [PATCH v2 5/9] arm64: rework EL0 MRS emulation Mark Rutland
2022-10-19 14:41 ` [PATCH v2 6/9] arm64: armv8_deprecated: fold ops into insn_emulation Mark Rutland
2022-10-19 14:41 ` [PATCH v2 7/9] arm64: armv8_deprecated move emulation functions Mark Rutland
2022-10-19 14:41 ` [PATCH v2 8/9] arm64: armv8_deprecated: move aarch32 helper earlier Mark Rutland
2022-10-19 14:41 ` [PATCH v2 9/9] arm64: armv8_deprecated: rework deprected instruction handling Mark Rutland
2023-02-03 10:08   ` Ruan Jinjie
2023-02-03 17:27     ` Mark Rutland
2023-02-06  2:03       ` Ruan Jinjie
2023-02-06  9:13         ` Mark Rutland
2023-02-07  7:01           ` Ruan Jinjie
2022-11-14 14:20 ` [PATCH v2 0/9] arm64: rework UNDEFINED instruction traps Will Deacon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.