All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes
@ 2021-06-28  7:49 Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 1/8] powerpc/64e: fix CONFIG_RELOCATABLE build Nicholas Piggin
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin

This is a bunch of fixes for powerpc next, mostly a nasty hole in fast
interrupt exit code found by Sachin and some other bits along the way
while looking at it.

So far this survives about 5 hours of stress testing with a workload
that would trigger it in a few seconds (guest 128 vcpus running kernel
compile loops with perf record -ag running in the background).

Thanks,
Nick

Nicholas Piggin (8):
  powerpc/64e: fix CONFIG_RELOCATABLE build
  powerpc/64e: remove implicit soft-masking and interrupt exit restart
    logic
  powerpc/64s: add a table of implicit soft-masked addresses
  powerpc/64s/interrupt: preserve regs->softe for NMI interrupts
  powerpc/64: enable MSR[EE] in irq replay pt_regs
  powerpc/64/interrupts: add missing kprobe annotations on interrupt
    exit symbols
  powerpc/64s/interrupt: clean up interrupt return labels
  powerpc/64s: move ret_from_fork etc above __end_soft_masked

 arch/powerpc/include/asm/interrupt.h | 41 ++++++++++---
 arch/powerpc/include/asm/ppc_asm.h   |  7 +++
 arch/powerpc/kernel/exceptions-64e.S | 23 +++----
 arch/powerpc/kernel/exceptions-64s.S | 55 ++++++++++++++---
 arch/powerpc/kernel/interrupt_64.S   | 90 ++++++++++++++++++----------
 arch/powerpc/kernel/irq.c            |  1 +
 arch/powerpc/kernel/vmlinux.lds.S    |  9 +++
 arch/powerpc/lib/restart_table.c     | 26 ++++++++
 8 files changed, 194 insertions(+), 58 deletions(-)

-- 
2.23.0


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

* [PATCH 1/8] powerpc/64e: fix CONFIG_RELOCATABLE build
  2021-06-28  7:49 [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes Nicholas Piggin
@ 2021-06-28  7:49 ` Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 2/8] powerpc/64e: remove implicit soft-masking and interrupt exit restart logic Nicholas Piggin
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin

Commit 24d33ac5b8ff ("powerpc/64s: Make prom_init require RELOCATABLE")
also made my 64e config require RELOCATABLE, which results in compile
failures.

Whether or not that's the right thing to do for prom_init for 64e, this
fixes CONFIG_RELOCATABLE=y compile errors. That commit is marked as
being fixed, but only because that's what caused the compile error to
show up for a given config.

This passes basic qemu testing.

Fixes: 24d33ac5b8ff ("powerpc/64s: Make prom_init require RELOCATABLE")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/exceptions-64e.S | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 22fcd95dd8dc..d634bfceed2c 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -912,8 +912,14 @@ kernel_dbg_exc:
 	b	interrupt_return
 
 .macro SEARCH_RESTART_TABLE
+#ifdef CONFIG_RELOCATABLE
+	ld	r11,PACATOC(r13)
+	ld	r14,__start___restart_table@got(r11)
+	ld	r15,__stop___restart_table@got(r11)
+#else
 	LOAD_REG_IMMEDIATE_SYM(r14, r11, __start___restart_table)
 	LOAD_REG_IMMEDIATE_SYM(r15, r11, __stop___restart_table)
+#endif
 300:
 	cmpd	r14,r15
 	beq	302f
@@ -1329,7 +1335,12 @@ a2_tlbinit_code_start:
 a2_tlbinit_after_linear_map:
 
 	/* Now we branch the new virtual address mapped by this entry */
+#ifdef CONFIG_RELOCATABLE
+	ld	r5,PACATOC(r13)
+	ld	r3,1f@got(r5)
+#else
 	LOAD_REG_IMMEDIATE_SYM(r3, r5, 1f)
+#endif
 	mtctr	r3
 	bctr
 
-- 
2.23.0


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

* [PATCH 2/8] powerpc/64e: remove implicit soft-masking and interrupt exit restart logic
  2021-06-28  7:49 [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 1/8] powerpc/64e: fix CONFIG_RELOCATABLE build Nicholas Piggin
@ 2021-06-28  7:49 ` Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 3/8] powerpc/64s: add a table of implicit soft-masked addresses Nicholas Piggin
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin

The implicit soft-masking to speed up interrupt return was going to be
used by 64e as well, but it was not ready in time. 64e always disables
MSR[EE] when exiting from interrupt and syscall.

Disable it for now.

Fixes: 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs soft-masked")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/interrupt.h | 33 ++++++++++++++++++++--------
 arch/powerpc/kernel/exceptions-64e.S | 12 +---------
 arch/powerpc/kernel/interrupt_64.S   | 16 +++++++++++++-
 3 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index 8b4b1e84e110..f2481fac7f7f 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -73,20 +73,34 @@
 #include <asm/kprobes.h>
 #include <asm/runlatch.h>
 
-#ifdef CONFIG_PPC64
+#ifdef CONFIG_PPC_BOOK3S_64
 extern char __end_soft_masked[];
 unsigned long search_kernel_restart_table(unsigned long addr);
-#endif
 
-#ifdef CONFIG_PPC_BOOK3S_64
 DECLARE_STATIC_KEY_FALSE(interrupt_exit_not_reentrant);
 
+bool is_implicit_soft_masked(struct pt_regs *regs)
+{
+	if (regs->msr & MSR_PR)
+		return false;
+
+	if (regs->nip >= (unsigned long)__end_soft_masked)
+		return false;
+
+	return true;
+}
+
 static inline void srr_regs_clobbered(void)
 {
 	local_paca->srr_valid = 0;
 	local_paca->hsrr_valid = 0;
 }
 #else
+static inline bool is_implicit_soft_masked(struct pt_regs *regs)
+{
+	return false;
+}
+
 static inline void srr_regs_clobbered(void)
 {
 }
@@ -150,11 +164,13 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
 		 */
 		if (TRAP(regs) != INTERRUPT_PROGRAM) {
 			CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
-			BUG_ON(regs->nip < (unsigned long)__end_soft_masked);
+			BUG_ON(is_implicit_soft_masked(regs));
 		}
+#ifdef CONFIG_PPC_BOOK3S
 		/* Move this under a debugging check */
 		if (arch_irq_disabled_regs(regs))
 			BUG_ON(search_kernel_restart_table(regs->nip));
+#endif
 	}
 #endif
 
@@ -244,10 +260,9 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte
 	local_paca->irq_soft_mask = IRQS_ALL_DISABLED;
 	local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
 
-	if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !(regs->msr & MSR_PR) &&
-				regs->nip < (unsigned long)__end_soft_masked) {
-		// Kernel code running below __end_soft_masked is
-		// implicitly soft-masked.
+	if (is_implicit_soft_masked(regs)) {
+		// Adjust regs->softe soft implicit soft-mask, so
+		// arch_irq_disabled_regs(regs) behaves as expected.
 		regs->softe = IRQS_ALL_DISABLED;
 	}
 
@@ -282,6 +297,7 @@ static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct inter
 	 */
 
 #ifdef CONFIG_PPC64
+#ifdef CONFIG_PPC_BOOK3S
 	if (arch_irq_disabled_regs(regs)) {
 		unsigned long rst = search_kernel_restart_table(regs->nip);
 		if (rst)
@@ -289,7 +305,6 @@ static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct inter
 	}
 #endif
 
-#ifdef CONFIG_PPC64
 	if (nmi_disables_ftrace(regs))
 		this_cpu_set_ftrace_enabled(state->ftrace_enabled);
 
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index d634bfceed2c..1401787b0b93 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -342,17 +342,7 @@ ret_from_mc_except:
 #define PROLOG_ADDITION_MASKABLE_GEN(n)					    \
 	lbz	r10,PACAIRQSOFTMASK(r13);	/* are irqs soft-masked? */ \
 	andi.	r10,r10,IRQS_DISABLED;	/* yes -> go out of line */ \
-	bne	masked_interrupt_book3e_##n;				    \
-	/* Kernel code below __end_soft_masked is implicitly masked */	    \
-	andi.	r10,r11,MSR_PR;						    \
-	bne	1f;			/* user -> not masked */	    \
-	std	r14,PACA_EXGEN+EX_R14(r13);				    \
-	LOAD_REG_IMMEDIATE_SYM(r14, r10, __end_soft_masked);		    \
-	mfspr	r10,SPRN_SRR0;						    \
-	cmpld	r10,r14;						    \
-	ld	r14,PACA_EXGEN+EX_R14(r13);				    \
-	blt	masked_interrupt_book3e_##n;				    \
-1:
+	bne	masked_interrupt_book3e_##n
 
 /*
  * Additional regs must be re-loaded from paca before EXCEPTION_COMMON* is
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index e7a50613a570..0a8afec6c07b 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -196,6 +196,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	RFI_TO_USER
 .Lsyscall_vectored_\name\()_rst_end:
 
+#ifdef CONFIG_PPC_BOOK3S
 syscall_vectored_\name\()_restart:
 	GET_PACA(r13)
 	ld	r1,PACA_EXIT_SAVE_R1(r13)
@@ -209,6 +210,7 @@ syscall_vectored_\name\()_restart:
 	b	.Lsyscall_vectored_\name\()_rst_start
 
 RESTART_TABLE(.Lsyscall_vectored_\name\()_rst_start, .Lsyscall_vectored_\name\()_rst_end, syscall_vectored_\name\()_restart)
+#endif
 
 .endm
 
@@ -320,10 +322,12 @@ END_BTB_FLUSH_SECTION
 	li	r5,0 /* !scv */
 	bl	syscall_exit_prepare
 	std	r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
+#ifdef CONFIG_PPC_BOOK3S
 .Lsyscall_rst_start:
 	lbz	r11,PACAIRQHAPPENED(r13)
 	andi.	r11,r11,(~PACA_IRQ_HARD_DIS)@l
 	bne-	syscall_restart
+#endif
 	li	r11,IRQS_ENABLED
 	stb	r11,PACAIRQSOFTMASK(r13)
 	li	r11,0
@@ -396,6 +400,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 	b	.Lsyscall_restore_regs_cont
 .Lsyscall_rst_end:
 
+#ifdef CONFIG_PPC_BOOK3S
 syscall_restart:
 	GET_PACA(r13)
 	ld	r1,PACA_EXIT_SAVE_R1(r13)
@@ -409,6 +414,7 @@ syscall_restart:
 	b	.Lsyscall_rst_start
 
 RESTART_TABLE(.Lsyscall_rst_start, .Lsyscall_rst_end, syscall_restart)
+#endif
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 tabort_syscall:
@@ -504,10 +510,12 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\())
 	bne-	.Lrestore_nvgprs_\srr
 .Lrestore_nvgprs_\srr\()_cont:
 	std	r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
+#ifdef CONFIG_PPC_BOOK3S
 .Linterrupt_return_\srr\()_user_rst_start:
 	lbz	r11,PACAIRQHAPPENED(r13)
 	andi.	r11,r11,(~PACA_IRQ_HARD_DIS)@l
 	bne-	interrupt_return_\srr\()_user_restart
+#endif
 	li	r11,IRQS_ENABLED
 	stb	r11,PACAIRQSOFTMASK(r13)
 	li	r11,0
@@ -590,6 +598,7 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 	REST_NVGPRS(r1)
 	b	.Lrestore_nvgprs_\srr\()_cont
 
+#ifdef CONFIG_PPC_BOOK3S
 interrupt_return_\srr\()_user_restart:
 	GET_PACA(r13)
 	ld	r1,PACA_EXIT_SAVE_R1(r13)
@@ -602,6 +611,7 @@ interrupt_return_\srr\()_user_restart:
 	b	.Linterrupt_return_\srr\()_user_rst_start
 
 RESTART_TABLE(.Linterrupt_return_\srr\()_user_rst_start, .Linterrupt_return_\srr\()_user_rst_end, interrupt_return_\srr\()_user_restart)
+#endif
 
 	.balign IFETCH_ALIGN_BYTES
 .Lkernel_interrupt_return_\srr\():
@@ -615,9 +625,11 @@ RESTART_TABLE(.Linterrupt_return_\srr\()_user_rst_start, .Linterrupt_return_\srr
 	cmpwi	r11,IRQS_ENABLED
 	stb	r11,PACAIRQSOFTMASK(r13)
 	bne	1f
+#ifdef CONFIG_PPC_BOOK3S
 	lbz	r11,PACAIRQHAPPENED(r13)
 	andi.	r11,r11,(~PACA_IRQ_HARD_DIS)@l
 	bne-	interrupt_return_\srr\()_kernel_restart
+#endif
 	li	r11,0
 	stb	r11,PACAIRQHAPPENED(r13) # clear out possible HARD_DIS
 1:
@@ -717,6 +729,7 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 	b	.	/* prevent speculative execution */
 .Linterrupt_return_\srr\()_kernel_rst_end:
 
+#ifdef CONFIG_PPC_BOOK3S
 interrupt_return_\srr\()_kernel_restart:
 	GET_PACA(r13)
 	ld	r1,PACA_EXIT_SAVE_R1(r13)
@@ -729,14 +742,15 @@ interrupt_return_\srr\()_kernel_restart:
 	b	.Linterrupt_return_\srr\()_kernel_rst_start
 
 RESTART_TABLE(.Linterrupt_return_\srr\()_kernel_rst_start, .Linterrupt_return_\srr\()_kernel_rst_end, interrupt_return_\srr\()_kernel_restart)
+#endif
 
 .endm
 
 interrupt_return_macro srr
 #ifdef CONFIG_PPC_BOOK3S
 interrupt_return_macro hsrr
-#endif /* CONFIG_PPC_BOOK3S */
 
 	.globl __end_soft_masked
 __end_soft_masked:
 DEFINE_FIXED_SYMBOL(__end_soft_masked)
+#endif /* CONFIG_PPC_BOOK3S */
-- 
2.23.0


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

* [PATCH 3/8] powerpc/64s: add a table of implicit soft-masked addresses
  2021-06-28  7:49 [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 1/8] powerpc/64e: fix CONFIG_RELOCATABLE build Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 2/8] powerpc/64e: remove implicit soft-masking and interrupt exit restart logic Nicholas Piggin
@ 2021-06-28  7:49 ` Nicholas Piggin
  2021-06-28 14:26   ` Sachin Sant
  2021-06-28  7:49 ` [PATCH 4/8] powerpc/64s/interrupt: preserve regs->softe for NMI interrupts Nicholas Piggin
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin

Commit 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs
soft-masked") ends up catching too much code, including ret_from_fork,
and parts of interrupt and syscall return that do not expect to be
interrupts to be soft-masked. If an interrupt gets marked pending,
and then the code proceeds out of the implicit soft-masked region it
will fail to deal with the pending interrupt.

Fix this by adding a new table of addresses which explicitly marks
the regions of code that are soft masked. This table is only checked
for interrupts that below __end_soft_masked, so most kernel interrupts
will not have the overhead of the table search.

Fixes: 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs soft-masked")
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/interrupt.h |  5 ++-
 arch/powerpc/include/asm/ppc_asm.h   |  7 ++++
 arch/powerpc/kernel/exceptions-64s.S | 55 ++++++++++++++++++++++++----
 arch/powerpc/kernel/interrupt_64.S   |  8 ++++
 arch/powerpc/kernel/vmlinux.lds.S    |  9 +++++
 arch/powerpc/lib/restart_table.c     | 26 +++++++++++++
 6 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index f2481fac7f7f..d7df247a149c 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -75,11 +75,12 @@
 
 #ifdef CONFIG_PPC_BOOK3S_64
 extern char __end_soft_masked[];
+bool search_kernel_soft_mask_table(unsigned long addr);
 unsigned long search_kernel_restart_table(unsigned long addr);
 
 DECLARE_STATIC_KEY_FALSE(interrupt_exit_not_reentrant);
 
-bool is_implicit_soft_masked(struct pt_regs *regs)
+static inline bool is_implicit_soft_masked(struct pt_regs *regs)
 {
 	if (regs->msr & MSR_PR)
 		return false;
@@ -87,7 +88,7 @@ bool is_implicit_soft_masked(struct pt_regs *regs)
 	if (regs->nip >= (unsigned long)__end_soft_masked)
 		return false;
 
-	return true;
+	return search_kernel_soft_mask_table(regs->nip);
 }
 
 static inline void srr_regs_clobbered(void)
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index c9c2c36c1f8f..116c1519728a 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -762,6 +762,13 @@ END_FTR_SECTION_NESTED(CPU_FTR_CELL_TB_BUG, CPU_FTR_CELL_TB_BUG, 96)
 	stringify_in_c(.long (_target) - . ;)	\
 	stringify_in_c(.previous)
 
+#define SOFT_MASK_TABLE(_start, _end)		\
+	stringify_in_c(.section __soft_mask_table,"a";)\
+	stringify_in_c(.balign 8;)		\
+	stringify_in_c(.llong (_start);)	\
+	stringify_in_c(.llong (_end);)		\
+	stringify_in_c(.previous)
+
 #define RESTART_TABLE(_start, _end, _target)	\
 	stringify_in_c(.section __restart_table,"a";)\
 	stringify_in_c(.balign 8;)		\
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index ecd07bf604c5..3a58c3fd6de4 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -428,21 +428,30 @@ DEFINE_FIXED_SYMBOL(\name\()_common_real)
 
 		/* If coming from user, skip soft-mask tests. */
 		andi.	r10,r12,MSR_PR
-		bne	2f
+		bne	3f
 
 		/*
-		 * Kernel code running below __end_soft_masked is implicitly
-		 * soft-masked
+		 * Kernel code running below __end_soft_masked may be
+		 * implicitly soft-masked if it is within the regions
+		 * in the soft mask table.
 		 */
 		LOAD_HANDLER(r10, __end_soft_masked)
 		cmpld	r11,r10
-
+		bge+	1f
+
+		/* SEARCH_SOFT_MASK_TABLE clobbers r9,r10,r12 */
+		stw	r9,PACA_EXGEN+EX_CCR(r13)
+		SEARCH_SOFT_MASK_TABLE
+		lwz	r9,PACA_EXGEN+EX_CCR(r13)
+		cmpdi	r12,0
+		mfspr	r12,SPRN_SRR1	/* Restore r12 to SRR1 */
+		beq	1f		/* Not in soft-mask table */
 		li	r10,IMASK
-		blt-	1f
+		b	2f		/* In soft-mask table, always mask */
 
 		/* Test the soft mask state against our interrupt's bit */
-		lbz	r10,PACAIRQSOFTMASK(r13)
-1:		andi.	r10,r10,IMASK
+1:		lbz	r10,PACAIRQSOFTMASK(r13)
+2:		andi.	r10,r10,IMASK
 		/* Associate vector numbers with bits in paca->irq_happened */
 		.if IVEC == 0x500 || IVEC == 0xea0
 		li	r10,PACA_IRQ_EE
@@ -473,7 +482,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_real)
 
 	.if ISTACK
 	andi.	r10,r12,MSR_PR		/* See if coming from user	*/
-2:	mr	r10,r1			/* Save r1			*/
+3:	mr	r10,r1			/* Save r1			*/
 	subi	r1,r1,INT_FRAME_SIZE	/* alloc frame on kernel stack	*/
 	beq-	100f
 	ld	r1,PACAKSAVE(r13)	/* kernel stack to use		*/
@@ -624,6 +633,36 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
 303:
 .endm
 
+.macro SEARCH_SOFT_MASK_TABLE
+#ifdef CONFIG_RELOCATABLE
+	mr	r12,r2
+	ld	r2,PACATOC(r13)
+	LOAD_REG_ADDR(r9, __start___soft_mask_table)
+	LOAD_REG_ADDR(r10, __stop___soft_mask_table)
+	mr	r2,r12
+#else
+	LOAD_REG_IMMEDIATE_SYM(r9, r12, __start___soft_mask_table)
+	LOAD_REG_IMMEDIATE_SYM(r10, r12, __stop___soft_mask_table)
+#endif
+300:
+	cmpd	r9,r10
+	beq	302f
+	ld	r12,0(r9)
+	cmpld	r11,r12
+	blt	301f
+	ld	r12,8(r9)
+	cmpld	r11,r12
+	bge	301f
+	li	r12,1
+	b	303f
+301:
+	addi	r9,r9,16
+	b	300b
+302:
+	li	r12,0
+303:
+.endm
+
 /*
  * Restore all registers including H/SRR0/1 saved in a stack frame of a
  * standard exception.
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 0a8afec6c07b..c06ed64541e1 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -208,7 +208,9 @@ syscall_vectored_\name\()_restart:
 	bl	syscall_exit_restart
 	std	r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
 	b	.Lsyscall_vectored_\name\()_rst_start
+1:
 
+SOFT_MASK_TABLE(.Lsyscall_vectored_\name\()_rst_start, 1b)
 RESTART_TABLE(.Lsyscall_vectored_\name\()_rst_start, .Lsyscall_vectored_\name\()_rst_end, syscall_vectored_\name\()_restart)
 #endif
 
@@ -412,7 +414,9 @@ syscall_restart:
 	bl	syscall_exit_restart
 	std	r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
 	b	.Lsyscall_rst_start
+1:
 
+SOFT_MASK_TABLE(.Lsyscall_rst_start, 1b)
 RESTART_TABLE(.Lsyscall_rst_start, .Lsyscall_rst_end, syscall_restart)
 #endif
 
@@ -609,7 +613,9 @@ interrupt_return_\srr\()_user_restart:
 	bl	interrupt_exit_user_restart
 	std	r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
 	b	.Linterrupt_return_\srr\()_user_rst_start
+1:
 
+SOFT_MASK_TABLE(.Linterrupt_return_\srr\()_user_rst_start, 1b)
 RESTART_TABLE(.Linterrupt_return_\srr\()_user_rst_start, .Linterrupt_return_\srr\()_user_rst_end, interrupt_return_\srr\()_user_restart)
 #endif
 
@@ -740,7 +746,9 @@ interrupt_return_\srr\()_kernel_restart:
 	bl	interrupt_exit_kernel_restart
 	std	r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
 	b	.Linterrupt_return_\srr\()_kernel_rst_start
+1:
 
+SOFT_MASK_TABLE(.Linterrupt_return_\srr\()_kernel_rst_start, 1b)
 RESTART_TABLE(.Linterrupt_return_\srr\()_kernel_rst_start, .Linterrupt_return_\srr\()_kernel_rst_end, interrupt_return_\srr\()_kernel_restart)
 #endif
 
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 16c5e13e00c4..40bdefe9caa7 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -9,6 +9,14 @@
 #define EMITS_PT_NOTE
 #define RO_EXCEPTION_TABLE_ALIGN	0
 
+#define SOFT_MASK_TABLE(align)						\
+	. = ALIGN(align);						\
+	__soft_mask_table : AT(ADDR(__soft_mask_table) - LOAD_OFFSET) {	\
+		__start___soft_mask_table = .;				\
+		KEEP(*(__soft_mask_table))				\
+		__stop___soft_mask_table = .;				\
+	}
+
 #define RESTART_TABLE(align)						\
 	. = ALIGN(align);						\
 	__restart_table : AT(ADDR(__restart_table) - LOAD_OFFSET) {	\
@@ -132,6 +140,7 @@ SECTIONS
 	RO_DATA(PAGE_SIZE)
 
 #ifdef CONFIG_PPC64
+	SOFT_MASK_TABLE(8)
 	RESTART_TABLE(8)
 
 	. = ALIGN(8);
diff --git a/arch/powerpc/lib/restart_table.c b/arch/powerpc/lib/restart_table.c
index 7cd20757cc33..bccb662c1b7b 100644
--- a/arch/powerpc/lib/restart_table.c
+++ b/arch/powerpc/lib/restart_table.c
@@ -1,15 +1,41 @@
 #include <asm/interrupt.h>
 #include <asm/kprobes.h>
 
+struct soft_mask_table_entry {
+	unsigned long start;
+	unsigned long end;
+};
+
 struct restart_table_entry {
 	unsigned long start;
 	unsigned long end;
 	unsigned long fixup;
 };
 
+extern struct soft_mask_table_entry __start___soft_mask_table[];
+extern struct soft_mask_table_entry __stop___soft_mask_table[];
+
 extern struct restart_table_entry __start___restart_table[];
 extern struct restart_table_entry __stop___restart_table[];
 
+/* Given an address, look for it in the soft mask table */
+bool search_kernel_soft_mask_table(unsigned long addr)
+{
+	struct soft_mask_table_entry *smte = __start___soft_mask_table;
+
+	while (smte < __stop___soft_mask_table) {
+		unsigned long start = smte->start;
+		unsigned long end = smte->end;
+
+		if (addr >= start && addr < end)
+			return true;
+
+		smte++;
+	}
+	return false;
+}
+NOKPROBE_SYMBOL(search_kernel_soft_mask_table);
+
 /* Given an address, look for it in the kernel exception table */
 unsigned long search_kernel_restart_table(unsigned long addr)
 {
-- 
2.23.0


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

* [PATCH 4/8] powerpc/64s/interrupt: preserve regs->softe for NMI interrupts
  2021-06-28  7:49 [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes Nicholas Piggin
                   ` (2 preceding siblings ...)
  2021-06-28  7:49 ` [PATCH 3/8] powerpc/64s: add a table of implicit soft-masked addresses Nicholas Piggin
@ 2021-06-28  7:49 ` Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 5/8] powerpc/64: enable MSR[EE] in irq replay pt_regs Nicholas Piggin
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin

If an NMI interrupt hits in an implicit soft-masked region, regs->softe
is modified to reflect that. This may not be necessary for correctness
at the moment, but it is less surprising and it's unhelpful when
debugging or adding checks.

Make sure this is changed back to how it was found before returning.

Fixes: 4ec5feec1ad0 ("powerpc/64s: Make NMI record implicitly soft-masked code as irqs disabled")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/interrupt.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index d7df247a149c..789311d1e283 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -227,6 +227,7 @@ struct interrupt_nmi_state {
 	u8 irq_soft_mask;
 	u8 irq_happened;
 	u8 ftrace_enabled;
+	u64 softe;
 #endif
 };
 
@@ -252,6 +253,7 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte
 #ifdef CONFIG_PPC64
 	state->irq_soft_mask = local_paca->irq_soft_mask;
 	state->irq_happened = local_paca->irq_happened;
+	state->softe = regs->softe;
 
 	/*
 	 * Set IRQS_ALL_DISABLED unconditionally so irqs_disabled() does
@@ -311,6 +313,7 @@ static inline void interrupt_nmi_exit_prepare(struct pt_regs *regs, struct inter
 
 	/* Check we didn't change the pending interrupt mask. */
 	WARN_ON_ONCE((state->irq_happened | PACA_IRQ_HARD_DIS) != local_paca->irq_happened);
+	regs->softe = state->softe;
 	local_paca->irq_happened = state->irq_happened;
 	local_paca->irq_soft_mask = state->irq_soft_mask;
 #endif
-- 
2.23.0


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

* [PATCH 5/8] powerpc/64: enable MSR[EE] in irq replay pt_regs
  2021-06-28  7:49 [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes Nicholas Piggin
                   ` (3 preceding siblings ...)
  2021-06-28  7:49 ` [PATCH 4/8] powerpc/64s/interrupt: preserve regs->softe for NMI interrupts Nicholas Piggin
@ 2021-06-28  7:49 ` Nicholas Piggin
  2021-06-28 14:37   ` Sachin Sant
  2021-06-28  7:49 ` [PATCH 6/8] powerpc/64/interrupts: add missing kprobe annotations on interrupt exit symbols Nicholas Piggin
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin

Similar to 2b48e96be2f9f ("powerpc/64: fix irq replay pt_regs->softe
value"), enable MSR_EE in pt_regs->msr, which makes the regs look a
bit more normal and allows the extra debug checks to be added to
interrupt handler entry.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/interrupt.h | 4 ++++
 arch/powerpc/kernel/irq.c            | 1 +
 2 files changed, 5 insertions(+)

diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index 789311d1e283..d4bdf7d274ac 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -173,6 +173,8 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
 			BUG_ON(search_kernel_restart_table(regs->nip));
 #endif
 	}
+	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
+		BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
 #endif
 
 	booke_restore_dbcr0();
@@ -268,6 +270,8 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte
 		// arch_irq_disabled_regs(regs) behaves as expected.
 		regs->softe = IRQS_ALL_DISABLED;
 	}
+	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
+		BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
 
 	/* Don't do any per-CPU operations until interrupt state is fixed */
 
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 8428caf3194e..91e63eac4e8f 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -121,6 +121,7 @@ void replay_soft_interrupts(void)
 
 	ppc_save_regs(&regs);
 	regs.softe = IRQS_ENABLED;
+	regs.msr |= MSR_EE;
 
 again:
 	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
-- 
2.23.0


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

* [PATCH 6/8] powerpc/64/interrupts: add missing kprobe annotations on interrupt exit symbols
  2021-06-28  7:49 [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes Nicholas Piggin
                   ` (4 preceding siblings ...)
  2021-06-28  7:49 ` [PATCH 5/8] powerpc/64: enable MSR[EE] in irq replay pt_regs Nicholas Piggin
@ 2021-06-28  7:49 ` Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 7/8] powerpc/64s/interrupt: clean up interrupt return labels Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 8/8] powerpc/64s: move ret_from_fork etc above __end_soft_masked Nicholas Piggin
  7 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin

If one interrupt exit symbol must not be kprobed, none of them can be,
really.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/interrupt_64.S | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index c06ed64541e1..06244b4df719 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -198,6 +198,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 
 #ifdef CONFIG_PPC_BOOK3S
 syscall_vectored_\name\()_restart:
+_ASM_NOKPROBE_SYMBOL(syscall_vectored_\name\()_restart)
 	GET_PACA(r13)
 	ld	r1,PACA_EXIT_SAVE_R1(r13)
 	ld	r2,PACATOC(r13)
@@ -240,6 +241,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_emulate)
 	.balign IFETCH_ALIGN_BYTES
 	.globl system_call_common_real
 system_call_common_real:
+_ASM_NOKPROBE_SYMBOL(system_call_common_real)
 	ld	r10,PACAKMSR(r13)	/* get MSR value for kernel */
 	mtmsrd	r10
 
@@ -404,6 +406,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
 
 #ifdef CONFIG_PPC_BOOK3S
 syscall_restart:
+_ASM_NOKPROBE_SYMBOL(syscall_restart)
 	GET_PACA(r13)
 	ld	r1,PACA_EXIT_SAVE_R1(r13)
 	ld	r2,PACATOC(r13)
@@ -422,6 +425,7 @@ RESTART_TABLE(.Lsyscall_rst_start, .Lsyscall_rst_end, syscall_restart)
 
 #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
 tabort_syscall:
+_ASM_NOKPROBE_SYMBOL(tabort_syscall)
 	/* Firstly we need to enable TM in the kernel */
 	mfmsr	r10
 	li	r9, 1
@@ -604,6 +608,7 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 
 #ifdef CONFIG_PPC_BOOK3S
 interrupt_return_\srr\()_user_restart:
+_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user_restart)
 	GET_PACA(r13)
 	ld	r1,PACA_EXIT_SAVE_R1(r13)
 	ld	r2,PACATOC(r13)
@@ -737,6 +742,7 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 
 #ifdef CONFIG_PPC_BOOK3S
 interrupt_return_\srr\()_kernel_restart:
+_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel_restart)
 	GET_PACA(r13)
 	ld	r1,PACA_EXIT_SAVE_R1(r13)
 	ld	r2,PACATOC(r13)
-- 
2.23.0


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

* [PATCH 7/8] powerpc/64s/interrupt: clean up interrupt return labels
  2021-06-28  7:49 [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes Nicholas Piggin
                   ` (5 preceding siblings ...)
  2021-06-28  7:49 ` [PATCH 6/8] powerpc/64/interrupts: add missing kprobe annotations on interrupt exit symbols Nicholas Piggin
@ 2021-06-28  7:49 ` Nicholas Piggin
  2021-06-28  7:49 ` [PATCH 8/8] powerpc/64s: move ret_from_fork etc above __end_soft_masked Nicholas Piggin
  7 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin

Normal kernel-interrupt exits can get interrupt_return_srr_user_restart
in their backtrace, which is an unusual and notable function, and it is
part of the user-interrupt exit path, which is doubly confusing.

Add symmetric non-local labels for user and kernel interrupt exit cases
to address this. Also get rid of an unused label.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/interrupt_64.S | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 06244b4df719..795c105850e4 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -511,7 +511,9 @@ interrupt_return_\srr\():
 _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\())
 	ld	r4,_MSR(r1)
 	andi.	r0,r4,MSR_PR
-	beq	.Lkernel_interrupt_return_\srr
+	beq	interrupt_return_\srr\()_kernel
+interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
+_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	interrupt_exit_user_prepare
 	cmpdi	r3,0
@@ -625,8 +627,8 @@ RESTART_TABLE(.Linterrupt_return_\srr\()_user_rst_start, .Linterrupt_return_\srr
 #endif
 
 	.balign IFETCH_ALIGN_BYTES
-.Lkernel_interrupt_return_\srr\():
-.Linterrupt_return_\srr\()_kernel:
+interrupt_return_\srr\()_kernel:
+_ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
 	bl	interrupt_exit_kernel_prepare
 
-- 
2.23.0


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

* [PATCH 8/8] powerpc/64s: move ret_from_fork etc above __end_soft_masked
  2021-06-28  7:49 [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes Nicholas Piggin
                   ` (6 preceding siblings ...)
  2021-06-28  7:49 ` [PATCH 7/8] powerpc/64s/interrupt: clean up interrupt return labels Nicholas Piggin
@ 2021-06-28  7:49 ` Nicholas Piggin
  7 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28  7:49 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sachin Sant, Nicholas Piggin

Code which runs with interrupts enabled should be moved above
__end_soft_masked where possible, because maskable interrupts that hit
below there need to consult the soft mask table.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/interrupt_64.S | 52 +++++++++++++++---------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index 795c105850e4..3ca3576690ce 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -451,32 +451,6 @@ _ASM_NOKPROBE_SYMBOL(tabort_syscall)
 	b	.	/* prevent speculative execution */
 #endif
 
-#ifdef CONFIG_PPC_BOOK3S
-_GLOBAL(ret_from_fork_scv)
-	bl	schedule_tail
-	REST_NVGPRS(r1)
-	li	r3,0	/* fork() return value */
-	b	.Lsyscall_vectored_common_exit
-#endif
-
-_GLOBAL(ret_from_fork)
-	bl	schedule_tail
-	REST_NVGPRS(r1)
-	li	r3,0	/* fork() return value */
-	b	.Lsyscall_exit
-
-_GLOBAL(ret_from_kernel_thread)
-	bl	schedule_tail
-	REST_NVGPRS(r1)
-	mtctr	r14
-	mr	r3,r15
-#ifdef PPC64_ELF_ABI_v2
-	mr	r12,r14
-#endif
-	bctrl
-	li	r3,0
-	b	.Lsyscall_exit
-
 	/*
 	 * If MSR EE/RI was never enabled, IRQs not reconciled, NVGPRs not
 	 * touched, no exit work created, then this can be used.
@@ -770,3 +744,29 @@ interrupt_return_macro hsrr
 __end_soft_masked:
 DEFINE_FIXED_SYMBOL(__end_soft_masked)
 #endif /* CONFIG_PPC_BOOK3S */
+
+#ifdef CONFIG_PPC_BOOK3S
+_GLOBAL(ret_from_fork_scv)
+	bl	schedule_tail
+	REST_NVGPRS(r1)
+	li	r3,0	/* fork() return value */
+	b	.Lsyscall_vectored_common_exit
+#endif
+
+_GLOBAL(ret_from_fork)
+	bl	schedule_tail
+	REST_NVGPRS(r1)
+	li	r3,0	/* fork() return value */
+	b	.Lsyscall_exit
+
+_GLOBAL(ret_from_kernel_thread)
+	bl	schedule_tail
+	REST_NVGPRS(r1)
+	mtctr	r14
+	mr	r3,r15
+#ifdef PPC64_ELF_ABI_v2
+	mr	r12,r14
+#endif
+	bctrl
+	li	r3,0
+	b	.Lsyscall_exit
-- 
2.23.0


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

* Re: [PATCH 3/8] powerpc/64s: add a table of implicit soft-masked addresses
  2021-06-28  7:49 ` [PATCH 3/8] powerpc/64s: add a table of implicit soft-masked addresses Nicholas Piggin
@ 2021-06-28 14:26   ` Sachin Sant
  0 siblings, 0 replies; 12+ messages in thread
From: Sachin Sant @ 2021-06-28 14:26 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev



> On 28-Jun-2021, at 1:19 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> 
> Commit 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs
> soft-masked") ends up catching too much code, including ret_from_fork,
> and parts of interrupt and syscall return that do not expect to be
> interrupts to be soft-masked. If an interrupt gets marked pending,
> and then the code proceeds out of the implicit soft-masked region it
> will fail to deal with the pending interrupt.
> 
> Fix this by adding a new table of addresses which explicitly marks
> the regions of code that are soft masked. This table is only checked
> for interrupts that below __end_soft_masked, so most kernel interrupts
> will not have the overhead of the table search.
> 
> Fixes: 9d1988ca87dd ("powerpc/64: treat low kernel text as irqs soft-masked")
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Thanks Nick for the fix.

I was able to verify this patch. 
Both kernel boot and test ran to completion without the reported warning.

Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>

-Sachin


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

* Re: [PATCH 5/8] powerpc/64: enable MSR[EE] in irq replay pt_regs
  2021-06-28  7:49 ` [PATCH 5/8] powerpc/64: enable MSR[EE] in irq replay pt_regs Nicholas Piggin
@ 2021-06-28 14:37   ` Sachin Sant
  2021-06-28 20:15     ` Nicholas Piggin
  0 siblings, 1 reply; 12+ messages in thread
From: Sachin Sant @ 2021-06-28 14:37 UTC (permalink / raw)
  To: Nicholas Piggin; +Cc: linuxppc-dev


> On 28-Jun-2021, at 1:19 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> 
> Similar to 2b48e96be2f9f ("powerpc/64: fix irq replay pt_regs->softe
> value"), enable MSR_EE in pt_regs->msr, which makes the regs look a
> bit more normal and allows the extra debug checks to be added to
> interrupt handler entry.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/interrupt.h | 4 ++++
> arch/powerpc/kernel/irq.c            | 1 +
> 2 files changed, 5 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index 789311d1e283..d4bdf7d274ac 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -173,6 +173,8 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
> 			BUG_ON(search_kernel_restart_table(regs->nip));
> #endif
> 	}
> +	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> +		BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
> #endif

I think this BUG_ON was triggered while running selftests (powerpc/mm/pkey_exec_prot)

[ 9741.254969] ------------[ cut here ]------------
[ 9741.254978] kernel BUG at arch/powerpc/include/asm/interrupt.h:177!
[ 9741.254985] Oops: Exception in kernel mode, sig: 5 [#1]
[ 9741.254990] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
[ 9741.254995] Modules linked in: rpadlpar_io rpaphp uinput sha512_generic vmac n_gsm pps_ldisc pps_core ppp_synctty ppp_async ppp_generic slcan slip slhc snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore authenc pcrypt crypto_user n_hdlc dummy veth nfsv3 nfs_acl nfs lockd grace fscache netfs tun brd overlay vfat fat btrfs blake2b_generic xor zstd_compress raid6_pq xfs loop sctp ip6_udp_tunnel udp_tunnel dm_mod bonding nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables libcrc32c nfnetlink sunrpc pseries_rng xts vmx_crypto uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sr_mod sd_mod cdrom t10_pi sg ibmvscsi ibmveth scsi_transport_srp fuse [last unloaded: test_cpuidle_latency]
[ 9741.255097] CPU: 17 PID: 3278920 Comm: pkey_exec_prot Tainted: G        W  OE     5.13.0-rc7-next-20210625-dirty #4
[ 9741.255106] NIP:  c0000000000300d8 LR: c000000000009604 CTR: c000000000009330
[ 9741.255111] REGS: c0000000347536f0 TRAP: 0700   Tainted: G        W  OE      (5.13.0-rc7-next-20210625-dirty)
[ 9741.255117] MSR:  8000000000021033 <SF,ME,IR,DR,RI,LE>  CR: 22004282  XER: 20040000
[ 9741.255130] CFAR: c00000000003007c IRQMASK: 3 
[ 9741.255130] GPR00: c000000000093cd0 c000000034753990 c0000000029bbe00 c000000034753a30 
[ 9741.255130] GPR04: 00007fff9ebb0000 0000000000200000 000000000000000a 000000000000002d 
[ 9741.255130] GPR08: 0000000000000000 0000000000000001 0000000000000000 7265677368657265 
[ 9741.255130] GPR12: 8000000000021033 c00000001ec27280 0000000000000000 0000000000000000 
[ 9741.255130] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
[ 9741.255130] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40 
[ 9741.255130] GPR24: 0000000000000000 0000000000000000 0000000000200000 c00000005e89d200 
[ 9741.255130] GPR28: 0000000000000300 00007fff9ebb0000 c000000034753e80 c000000034753a30 
[ 9741.255191] NIP [c0000000000300d8] program_check_exception+0xe8/0x1c0
[ 9741.255202] LR [c000000000009604] program_check_common_virt+0x2d4/0x320
[ 9741.255209] Call Trace:
[ 9741.255212] [c000000034753990] [0000000000000008] 0x8 (unreliable)
[ 9741.255219] [c0000000347539c0] [c000000034753a80] 0xc000000034753a80
[ 9741.255225] --- interrupt: 700 at arch_local_irq_restore+0x1d0/0x200
[ 9741.255231] NIP:  c000000000016790 LR: c000000000093388 CTR: c000000000008780
[ 9741.255236] REGS: c000000034753a30 TRAP: 0700   Tainted: G        W  OE      (5.13.0-rc7-next-20210625-dirty)
[ 9741.255242] MSR:  8000000000021033 <SF,ME,IR,DR,RI,LE>  CR: 24004288  XER: 20040000
[ 9741.255253] CFAR: c0000000000165ec IRQMASK: 0 
[ 9741.255253] GPR00: c000000000093cd0 c000000034753cd0 c0000000029bbe00 0000000000000000 
[ 9741.255253] GPR04: 00007fff9ebb0000 0000000000200000 000000000000000a 000000000000002d 
[ 9741.255253] GPR08: 0000000000000000 0000000000000000 c0000000bd77d400 7265677368657265 
[ 9741.255253] GPR12: 0000000044000282 c00000001ec27280 0000000000000000 0000000000000000 
[ 9741.255253] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
[ 9741.255253] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40 
[ 9741.255253] GPR24: 0000000000000000 0000000000000000 0000000000200000 c00000005e89d200 
[ 9741.255253] GPR28: 0000000000000300 00007fff9ebb0000 c000000034753e80 0000000000000001 
[ 9741.255313] NIP [c000000000016790] arch_local_irq_restore+0x1d0/0x200
[ 9741.255319] LR [c000000000093388] ___do_page_fault+0x438/0xb80
[ 9741.255325] --- interrupt: 700
[ 9741.255328] [c000000034753cd0] [c00000000009be74] hash_page_mm+0x5e4/0x800 (unreliable)
[ 9741.255335] [c000000034753d00] [000000000000002d] 0x2d
[ 9741.255340] [c000000034753db0] [c000000000093cd0] hash__do_page_fault+0x30/0x70
[ 9741.255348] [c000000034753de0] [c00000000009c438] do_hash_fault+0x78/0xb0
[ 9741.255354] [c000000034753e10] [c00000000000891c] data_access_common_virt+0x19c/0x1f0
[ 9741.255361] --- interrupt: 300 at 0x10001e8c
[ 9741.255365] NIP:  0000000010001e8c LR: 0000000010001e84 CTR: 00007fff9ea4eb60
[ 9741.255370] REGS: c000000034753e80 TRAP: 0300   Tainted: G        W  OE      (5.13.0-rc7-next-20210625-dirty)
[ 9741.255376] MSR:  800000000280f033 <SF,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE>  CR: 28000282  XER: 20040000
[ 9741.255391] CFAR: c00000000000ca44 DAR: 00007fff9ebb0000 DSISR: 00200000 IRQMASK: 0 
[ 9741.255391] GPR00: 0000000010001e84 00007fffd11fa7b0 0000000010027f00 0000000000000033 
[ 9741.255391] GPR04: 0000000010003c3d 0000000000000001 000000000000000a 000000000000002d 
[ 9741.255391] GPR08: 0000000000000000 00007fff9ebb0000 0000000000000000 000001002ab20337 
[ 9741.255391] GPR12: 0000000000000000 00007fff9ec4a270 0000000000000000 0000000000000000 
[ 9741.255391] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
[ 9741.255391] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40 
[ 9741.255391] GPR24: 0000000000000001 0000000010003c18 000000000000002d 0000000010020160 
[ 9741.255391] GPR28: 00c0000000000000 0000000010020230 0000000000000002 0000000000000004 
[ 9741.255455] NIP [0000000010001e8c] 0x10001e8c
[ 9741.255459] LR [0000000010001e84] 0x10001e84
[ 9741.255463] --- interrupt: 300
[ 9741.255466] Instruction dump:
[ 9741.255470] 60000000 e8010040 7c6a1b78 7c0803a6 0b0a0000 e93f0138 71290001 4082004c 
[ 9741.255481] e95f0108 39200001 714a8000 4082ff68 <0b090000> 38210030 7fe3fb78 ebe1fff8 
[ 9741.255494] ---[ end trace c668c70ea0d5061f ]—

Thanks
-Sachin

> 
> 	booke_restore_dbcr0();
> @@ -268,6 +270,8 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte
> 		// arch_irq_disabled_regs(regs) behaves as expected.
> 		regs->softe = IRQS_ALL_DISABLED;
> 	}
> +	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> +		BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
> 
> 	/* Don't do any per-CPU operations until interrupt state is fixed */
> 
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index 8428caf3194e..91e63eac4e8f 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -121,6 +121,7 @@ void replay_soft_interrupts(void)
> 
> 	ppc_save_regs(&regs);
> 	regs.softe = IRQS_ENABLED;
> +	regs.msr |= MSR_EE;
> 
> again:
> 	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
> -- 
> 2.23.0
> 


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

* Re: [PATCH 5/8] powerpc/64: enable MSR[EE] in irq replay pt_regs
  2021-06-28 14:37   ` Sachin Sant
@ 2021-06-28 20:15     ` Nicholas Piggin
  0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Piggin @ 2021-06-28 20:15 UTC (permalink / raw)
  To: Sachin Sant; +Cc: linuxppc-dev

Excerpts from Sachin Sant's message of June 29, 2021 12:37 am:
> 
>> On 28-Jun-2021, at 1:19 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>> 
>> Similar to 2b48e96be2f9f ("powerpc/64: fix irq replay pt_regs->softe
>> value"), enable MSR_EE in pt_regs->msr, which makes the regs look a
>> bit more normal and allows the extra debug checks to be added to
>> interrupt handler entry.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/interrupt.h | 4 ++++
>> arch/powerpc/kernel/irq.c            | 1 +
>> 2 files changed, 5 insertions(+)
>> 
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index 789311d1e283..d4bdf7d274ac 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -173,6 +173,8 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
>> 			BUG_ON(search_kernel_restart_table(regs->nip));
>> #endif
>> 	}
>> +	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
>> +		BUG_ON(!arch_irq_disabled_regs(regs) && !(regs->msr & MSR_EE));
>> #endif
> 
> I think this BUG_ON was triggered while running selftests (powerpc/mm/pkey_exec_prot)
> 
> [ 9741.254969] ------------[ cut here ]------------
> [ 9741.254978] kernel BUG at arch/powerpc/include/asm/interrupt.h:177!
> [ 9741.254985] Oops: Exception in kernel mode, sig: 5 [#1]
> [ 9741.254990] LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> [ 9741.254995] Modules linked in: rpadlpar_io rpaphp uinput sha512_generic vmac n_gsm pps_ldisc pps_core ppp_synctty ppp_async ppp_generic slcan slip slhc snd_hrtimer snd_seq snd_seq_device snd_timer snd soundcore authenc pcrypt crypto_user n_hdlc dummy veth nfsv3 nfs_acl nfs lockd grace fscache netfs tun brd overlay vfat fat btrfs blake2b_generic xor zstd_compress raid6_pq xfs loop sctp ip6_udp_tunnel udp_tunnel dm_mod bonding nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set rfkill nf_tables libcrc32c nfnetlink sunrpc pseries_rng xts vmx_crypto uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sr_mod sd_mod cdrom t10_pi sg ibmvscsi ibmveth scsi_transport_srp fuse [last unloaded: test_cpuidle_latency]
> [ 9741.255097] CPU: 17 PID: 3278920 Comm: pkey_exec_prot Tainted: G        W  OE     5.13.0-rc7-next-20210625-dirty #4
> [ 9741.255106] NIP:  c0000000000300d8 LR: c000000000009604 CTR: c000000000009330
> [ 9741.255111] REGS: c0000000347536f0 TRAP: 0700   Tainted: G        W  OE      (5.13.0-rc7-next-20210625-dirty)
> [ 9741.255117] MSR:  8000000000021033 <SF,ME,IR,DR,RI,LE>  CR: 22004282  XER: 20040000
> [ 9741.255130] CFAR: c00000000003007c IRQMASK: 3 
> [ 9741.255130] GPR00: c000000000093cd0 c000000034753990 c0000000029bbe00 c000000034753a30 
> [ 9741.255130] GPR04: 00007fff9ebb0000 0000000000200000 000000000000000a 000000000000002d 
> [ 9741.255130] GPR08: 0000000000000000 0000000000000001 0000000000000000 7265677368657265 
> [ 9741.255130] GPR12: 8000000000021033 c00000001ec27280 0000000000000000 0000000000000000 
> [ 9741.255130] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> [ 9741.255130] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40 
> [ 9741.255130] GPR24: 0000000000000000 0000000000000000 0000000000200000 c00000005e89d200 
> [ 9741.255130] GPR28: 0000000000000300 00007fff9ebb0000 c000000034753e80 c000000034753a30 
> [ 9741.255191] NIP [c0000000000300d8] program_check_exception+0xe8/0x1c0
> [ 9741.255202] LR [c000000000009604] program_check_common_virt+0x2d4/0x320
> [ 9741.255209] Call Trace:
> [ 9741.255212] [c000000034753990] [0000000000000008] 0x8 (unreliable)
> [ 9741.255219] [c0000000347539c0] [c000000034753a80] 0xc000000034753a80
> [ 9741.255225] --- interrupt: 700 at arch_local_irq_restore+0x1d0/0x200
> [ 9741.255231] NIP:  c000000000016790 LR: c000000000093388 CTR: c000000000008780
> [ 9741.255236] REGS: c000000034753a30 TRAP: 0700   Tainted: G        W  OE      (5.13.0-rc7-next-20210625-dirty)
> [ 9741.255242] MSR:  8000000000021033 <SF,ME,IR,DR,RI,LE>  CR: 24004288  XER: 20040000
> [ 9741.255253] CFAR: c0000000000165ec IRQMASK: 0 
> [ 9741.255253] GPR00: c000000000093cd0 c000000034753cd0 c0000000029bbe00 0000000000000000 
> [ 9741.255253] GPR04: 00007fff9ebb0000 0000000000200000 000000000000000a 000000000000002d 
> [ 9741.255253] GPR08: 0000000000000000 0000000000000000 c0000000bd77d400 7265677368657265 
> [ 9741.255253] GPR12: 0000000044000282 c00000001ec27280 0000000000000000 0000000000000000 
> [ 9741.255253] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> [ 9741.255253] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000010003c40 
> [ 9741.255253] GPR24: 0000000000000000 0000000000000000 0000000000200000 c00000005e89d200 
> [ 9741.255253] GPR28: 0000000000000300 00007fff9ebb0000 c000000034753e80 0000000000000001 
> [ 9741.255313] NIP [c000000000016790] arch_local_irq_restore+0x1d0/0x200
> [ 9741.255319] LR [c000000000093388] ___do_page_fault+0x438/0xb80
> [ 9741.255325] --- interrupt: 700
> [ 9741.255328] [c000000034753cd0] [c00000000009be74] hash_page_mm+0x5e4/0x800 (unreliable)
> [ 9741.255335] [c000000034753d00] [000000000000002d] 0x2d
> [ 9741.255340] [c000000034753db0] [c000000000093cd0] hash__do_page_fault+0x30/0x70
> [ 9741.255348] [c000000034753de0] [c00000000009c438] do_hash_fault+0x78/0xb0

This looks like it's probably running un-reconciled due to the first 
call to hash__do_page_fault not calling a real interrupt handler.  
Without this patch, the test must be causing a warning due to the same
thing probably (the bug triggered in the program check interrupt handler).

I think this patch is probably the right fix for it.

Thanks,
Nick

---

diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 96d9aa164007..ac5720371c0d 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1522,8 +1522,8 @@ int hash_page(unsigned long ea, unsigned long access, unsigned long trap,
 }
 EXPORT_SYMBOL_GPL(hash_page);
 
-DECLARE_INTERRUPT_HANDLER_RET(__do_hash_fault);
-DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
+DECLARE_INTERRUPT_HANDLER(__do_hash_fault);
+DEFINE_INTERRUPT_HANDLER(__do_hash_fault)
 {
 	unsigned long ea = regs->dar;
 	unsigned long dsisr = regs->dsisr;
@@ -1533,6 +1533,11 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
 	unsigned int region_id;
 	long err;
 
+	if (unlikely(dsisr & (DSISR_BAD_FAULT_64S | DSISR_KEYFAULT))) {
+		hash__do_page_fault(regs);
+		return;
+	}
+
 	region_id = get_region_id(ea);
 	if ((region_id == VMALLOC_REGION_ID) || (region_id == IO_REGION_ID))
 		mm = &init_mm;
@@ -1571,9 +1576,10 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
 			bad_page_fault(regs, SIGBUS);
 		}
 		err = 0;
-	}
 
-	return err;
+	} else if (err) {
+		hash__do_page_fault(regs);
+	}
 }
 
 /*
@@ -1582,13 +1588,6 @@ DEFINE_INTERRUPT_HANDLER_RET(__do_hash_fault)
  */
 DEFINE_INTERRUPT_HANDLER_RAW(do_hash_fault)
 {
-	unsigned long dsisr = regs->dsisr;
-
-	if (unlikely(dsisr & (DSISR_BAD_FAULT_64S | DSISR_KEYFAULT))) {
-		hash__do_page_fault(regs);
-		return 0;
-	}
-
 	/*
 	 * If we are in an "NMI" (e.g., an interrupt when soft-disabled), then
 	 * don't call hash_page, just fail the fault. This is required to
@@ -1607,8 +1606,7 @@ DEFINE_INTERRUPT_HANDLER_RAW(do_hash_fault)
 		return 0;
 	}
 
-	if (__do_hash_fault(regs))
-		hash__do_page_fault(regs);
+	__do_hash_fault(regs);
 
 	return 0;
 }

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

end of thread, other threads:[~2021-06-28 20:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-28  7:49 [PATCH 0/8] powerpc: fast interrupt exit bug and misc fixes Nicholas Piggin
2021-06-28  7:49 ` [PATCH 1/8] powerpc/64e: fix CONFIG_RELOCATABLE build Nicholas Piggin
2021-06-28  7:49 ` [PATCH 2/8] powerpc/64e: remove implicit soft-masking and interrupt exit restart logic Nicholas Piggin
2021-06-28  7:49 ` [PATCH 3/8] powerpc/64s: add a table of implicit soft-masked addresses Nicholas Piggin
2021-06-28 14:26   ` Sachin Sant
2021-06-28  7:49 ` [PATCH 4/8] powerpc/64s/interrupt: preserve regs->softe for NMI interrupts Nicholas Piggin
2021-06-28  7:49 ` [PATCH 5/8] powerpc/64: enable MSR[EE] in irq replay pt_regs Nicholas Piggin
2021-06-28 14:37   ` Sachin Sant
2021-06-28 20:15     ` Nicholas Piggin
2021-06-28  7:49 ` [PATCH 6/8] powerpc/64/interrupts: add missing kprobe annotations on interrupt exit symbols Nicholas Piggin
2021-06-28  7:49 ` [PATCH 7/8] powerpc/64s/interrupt: clean up interrupt return labels Nicholas Piggin
2021-06-28  7:49 ` [PATCH 8/8] powerpc/64s: move ret_from_fork etc above __end_soft_masked Nicholas Piggin

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.