LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 10/11] powerpc/40x: Add EXC_XFER_TEMPLATE_CRITICAL()
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

This patch adds EXC_XFER_TEMPLATE_CRITICAL() for handling
transfer to critical exception handler. This will allow
to move the normal exception to using the standard
EXC_XFER_TEMPLATE() defined in head_32.h

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_40x.S | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index 1203075c0c1a..d52e460ea85e 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -219,9 +219,7 @@ label:
 	START_EXCEPTION(n, label);				\
 	CRITICAL_EXCEPTION_PROLOG;				\
 	addi	r3,r1,STACK_FRAME_OVERHEAD;			\
-	EXC_XFER_TEMPLATE(hdlr, n+2, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \
-			  NOCOPY, crit_transfer_to_handler,	\
-			  ret_from_crit_exc)
+	EXC_XFER_TEMPLATE_CRITICAL(hdlr, n+2)
 
 #define EXC_XFER_TEMPLATE(hdlr, trap, msr, copyee, tfer, ret)	\
 	li	r10,trap;					\
@@ -233,6 +231,14 @@ label:
 	.long	hdlr;						\
 	.long	ret
 
+#define EXC_XFER_TEMPLATE_CRITICAL(hdlr, trap)			\
+	li	r10,trap;					\
+	stw	r10,_TRAP(r11);					\
+	li	r10, MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE);	\
+	bl	crit_transfer_to_handler;		 	\
+	.long	hdlr;						\
+	.long	ret_from_crit_exc
+
 #define COPY_EE(d, s)		rlwimi d,s,0,16,16
 #define NOCOPY(d, s)
 
@@ -733,9 +739,7 @@ label:
 	/* continue normal handling for a critical exception... */
 2:	mfspr	r4,SPRN_DBSR
 	addi	r3,r1,STACK_FRAME_OVERHEAD
-	EXC_XFER_TEMPLATE(DebugException, 0x2002, \
-		(MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \
-		NOCOPY, crit_transfer_to_handler, ret_from_crit_exc)
+	EXC_XFER_TEMPLATE_CRITICAL(DebugException, 0x2002)
 
 	/* Programmable Interval Timer (PIT) Exception. (from 0x1000) */
 Decrementer:
@@ -755,10 +759,7 @@ FITException:
 WDTException:
 	CRITICAL_EXCEPTION_PROLOG;
 	addi	r3,r1,STACK_FRAME_OVERHEAD;
-	EXC_XFER_TEMPLATE(WatchdogException, 0x1020+2,
-	                  (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)),
-			  NOCOPY, crit_transfer_to_handler,
-			  ret_from_crit_exc)
+	EXC_XFER_TEMPLATE_CRITICAL(WatchdogException, 0x1020+2)
 
 /*
  * The other Data TLB exceptions bail out to this point
-- 
2.13.3


^ permalink raw reply related

* [PATCH 09/11] powerpc/40x: Split and rename NORMAL_EXCEPTION_PROLOG
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

This patch splits NORMAL_EXCEPTION_PROLOG in the same way as in
head_8xx.S and head_32.S and renames it EXCEPTION_PROLOG() as well
to match head_32.h

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_40x.S | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index f3bfb695f952..1203075c0c1a 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -103,10 +103,14 @@ _ENTRY(saved_ksp_limit)
  * turned off (i.e. using physical addresses). We assume SPRG_THREAD has
  * the physical address of the current task thread_struct.
  */
-#define NORMAL_EXCEPTION_PROLOG						     \
+#define EXCEPTION_PROLOG						     \
 	mtspr	SPRN_SPRG_SCRATCH0,r10;	/* save two registers to work with */\
 	mtspr	SPRN_SPRG_SCRATCH1,r11;					     \
 	mfcr	r10;			/* save CR in r10 for now	   */\
+	EXCEPTION_PROLOG_1;						     \
+	EXCEPTION_PROLOG_2
+
+#define EXCEPTION_PROLOG_1						     \
 	mfspr	r11,SPRN_SRR1;		/* check whether user or kernel    */\
 	andi.	r11,r11,MSR_PR;						     \
 	tophys(r11,r1);							     \
@@ -115,7 +119,9 @@ _ENTRY(saved_ksp_limit)
 	lwz	r11,TASK_STACK-THREAD(r11); /* this thread's kernel stack */\
 	addi	r11,r11,THREAD_SIZE;					     \
 	tophys(r11,r11);						     \
-1:	subi	r11,r11,INT_FRAME_SIZE;	/* Allocate an exception frame     */\
+1:	subi	r11,r11,INT_FRAME_SIZE;	/* Allocate an exception frame     */
+
+#define EXCEPTION_PROLOG_2						     \
 	stw	r10,_CCR(r11);          /* save various registers	   */\
 	stw	r12,GPR12(r11);						     \
 	stw	r9,GPR9(r11);						     \
@@ -205,7 +211,7 @@ label:
 
 #define EXCEPTION(n, label, hdlr, xfer)				\
 	START_EXCEPTION(n, label);				\
-	NORMAL_EXCEPTION_PROLOG;				\
+	EXCEPTION_PROLOG;				\
 	addi	r3,r1,STACK_FRAME_OVERHEAD;			\
 	xfer(n, hdlr)
 
@@ -396,7 +402,7 @@ label:
  * This is caused by a fetch from non-execute or guarded pages.
  */
 	START_EXCEPTION(0x0400, InstructionAccess)
-	NORMAL_EXCEPTION_PROLOG
+	EXCEPTION_PROLOG
 	mr	r4,r12			/* Pass SRR0 as arg2 */
 	li	r5,0			/* Pass zero as arg3 */
 	EXC_XFER_LITE(0x400, handle_page_fault)
@@ -406,7 +412,7 @@ label:
 
 /* 0x0600 - Alignment Exception */
 	START_EXCEPTION(0x0600, Alignment)
-	NORMAL_EXCEPTION_PROLOG
+	EXCEPTION_PROLOG
 	mfspr	r4,SPRN_DEAR		/* Grab the DEAR and save it */
 	stw	r4,_DEAR(r11)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
@@ -414,7 +420,7 @@ label:
 
 /* 0x0700 - Program Exception */
 	START_EXCEPTION(0x0700, ProgramCheck)
-	NORMAL_EXCEPTION_PROLOG
+	EXCEPTION_PROLOG
 	mfspr	r4,SPRN_ESR		/* Grab the ESR and save it */
 	stw	r4,_ESR(r11)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
@@ -427,7 +433,7 @@ label:
 
 /* 0x0C00 - System Call Exception */
 	START_EXCEPTION(0x0C00,	SystemCall)
-	NORMAL_EXCEPTION_PROLOG
+	EXCEPTION_PROLOG
 	EXC_XFER_EE_LITE(0xc00, DoSyscall)
 
 	EXCEPTION(0x0D00, Trap_0D, unknown_exception, EXC_XFER_EE)
@@ -733,7 +739,7 @@ label:
 
 	/* Programmable Interval Timer (PIT) Exception. (from 0x1000) */
 Decrementer:
-	NORMAL_EXCEPTION_PROLOG
+	EXCEPTION_PROLOG
 	lis	r0,TSR_PIS@h
 	mtspr	SPRN_TSR,r0		/* Clear the PIT exception */
 	addi	r3,r1,STACK_FRAME_OVERHEAD
@@ -741,7 +747,7 @@ Decrementer:
 
 	/* Fixed Interval Timer (FIT) Exception. (from 0x1010) */
 FITException:
-	NORMAL_EXCEPTION_PROLOG
+	EXCEPTION_PROLOG
 	addi	r3,r1,STACK_FRAME_OVERHEAD;
 	EXC_XFER_EE(0x1010, unknown_exception)
 
@@ -759,7 +765,7 @@ WDTException:
  * if they can't resolve the lightweight TLB fault.
  */
 DataAccess:
-	NORMAL_EXCEPTION_PROLOG
+	EXCEPTION_PROLOG
 	mfspr	r5,SPRN_ESR		/* Grab the ESR, save it, pass arg3 */
 	stw	r5,_ESR(r11)
 	mfspr	r4,SPRN_DEAR		/* Grab the DEAR, save it, pass arg2 */
-- 
2.13.3


^ permalink raw reply related

* [PATCH 08/11] powerpc/40x: add exception frame marker
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

This patch adds STACK_FRAME_REGS_MARKER in the stack at exception entry
in order to see interrupts in call traces as below:

[    0.013964] Call Trace:
[    0.014014] [c0745db0] [c007a9d4] tick_periodic.constprop.5+0xd8/0x104 (unreliable)
[    0.014086] [c0745dc0] [c007aa20] tick_handle_periodic+0x20/0x9c
[    0.014181] [c0745de0] [c0009cd0] timer_interrupt+0xa0/0x264
[    0.014258] [c0745e10] [c000e484] ret_from_except+0x0/0x14
[    0.014390] --- interrupt: 901 at console_unlock.part.7+0x3f4/0x528
[    0.014390]     LR = console_unlock.part.7+0x3f0/0x528
[    0.014455] [c0745ee0] [c0050334] console_unlock.part.7+0x114/0x528 (unreliable)
[    0.014542] [c0745f30] [c00524e0] register_console+0x3d8/0x44c
[    0.014625] [c0745f60] [c0675aac] cpm_uart_console_init+0x18/0x2c
[    0.014709] [c0745f70] [c06614f4] console_init+0x114/0x1cc
[    0.014795] [c0745fb0] [c0658b68] start_kernel+0x300/0x3d8
[    0.014864] [c0745ff0] [c00022cc] start_here+0x44/0x98

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_40x.S | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index 59f6f53f1ac2..f3bfb695f952 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -132,6 +132,9 @@ _ENTRY(saved_ksp_limit)
 	tovirt(r1,r11);			/* set new kernel sp */	\
 	rlwinm	r9,r9,0,14,12;		/* clear MSR_WE (necessary?)	   */\
 	stw	r0,GPR0(r11);						     \
+	lis	r10, STACK_FRAME_REGS_MARKER@ha; /* exception frame marker */\
+	addi	r10, r10, STACK_FRAME_REGS_MARKER@l;			     \
+	stw	r10, 8(r11);						     \
 	SAVE_4GPRS(3, r11);						     \
 	SAVE_2GPRS(7, r11)
 
@@ -174,6 +177,9 @@ _ENTRY(saved_ksp_limit)
 	tovirt(r1,r11);							     \
 	rlwinm	r9,r9,0,14,12;		/* clear MSR_WE (necessary?)	   */\
 	stw	r0,GPR0(r11);						     \
+	lis	r10, STACK_FRAME_REGS_MARKER@ha; /* exception frame marker */\
+	addi	r10, r10, STACK_FRAME_REGS_MARKER@l;			     \
+	stw	r10, 8(r11);						     \
 	SAVE_4GPRS(3, r11);						     \
 	SAVE_2GPRS(7, r11)
 
-- 
2.13.3


^ permalink raw reply related

* [PATCH 07/11] powerpc/40x: Don't use SPRN_SPRG_SCRATCH2 in EXCEPTION_PROLOG
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

Unlike said in the comment, r1 is not reused by the critical
exception handler, as it uses a dedicated critirq_ctx stack.
Decrementing r1 early is then unneeded.

Should the above be valid, the code is crap buggy anyway as
r1 gets some intermediate values that would jeopardise the
whole process (for instance after mfspr   r1,SPRN_SPRG_THREAD)

Using SPRN_SPRG_SCRATCH2 to save r1 is then not needed, r11 can be
used instead. This avoids one mtspr and one mfspr and makes the
prolog closer to what's done on 6xx and 8xx.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_40x.S | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index 3088c9f29f5e..59f6f53f1ac2 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -102,23 +102,20 @@ _ENTRY(saved_ksp_limit)
  * Exception vector entry code. This code runs with address translation
  * turned off (i.e. using physical addresses). We assume SPRG_THREAD has
  * the physical address of the current task thread_struct.
- * Note that we have to have decremented r1 before we write to any fields
- * of the exception frame, since a critical interrupt could occur at any
- * time, and it will write to the area immediately below the current r1.
  */
 #define NORMAL_EXCEPTION_PROLOG						     \
 	mtspr	SPRN_SPRG_SCRATCH0,r10;	/* save two registers to work with */\
 	mtspr	SPRN_SPRG_SCRATCH1,r11;					     \
-	mtspr	SPRN_SPRG_SCRATCH2,r1;					     \
 	mfcr	r10;			/* save CR in r10 for now	   */\
 	mfspr	r11,SPRN_SRR1;		/* check whether user or kernel    */\
 	andi.	r11,r11,MSR_PR;						     \
-	beq	1f;							     \
-	mfspr	r1,SPRN_SPRG_THREAD;	/* if from user, start at top of   */\
-	lwz	r1,TASK_STACK-THREAD(r1); /* this thread's kernel stack   */\
-	addi	r1,r1,THREAD_SIZE;					     \
-1:	subi	r1,r1,INT_FRAME_SIZE;	/* Allocate an exception frame     */\
 	tophys(r11,r1);							     \
+	beq	1f;							     \
+	mfspr	r11,SPRN_SPRG_THREAD;	/* if from user, start at top of   */\
+	lwz	r11,TASK_STACK-THREAD(r11); /* this thread's kernel stack */\
+	addi	r11,r11,THREAD_SIZE;					     \
+	tophys(r11,r11);						     \
+1:	subi	r11,r11,INT_FRAME_SIZE;	/* Allocate an exception frame     */\
 	stw	r10,_CCR(r11);          /* save various registers	   */\
 	stw	r12,GPR12(r11);						     \
 	stw	r9,GPR9(r11);						     \
@@ -128,11 +125,11 @@ _ENTRY(saved_ksp_limit)
 	stw	r12,GPR11(r11);						     \
 	mflr	r10;							     \
 	stw	r10,_LINK(r11);						     \
-	mfspr	r10,SPRN_SPRG_SCRATCH2;					     \
 	mfspr	r12,SPRN_SRR0;						     \
-	stw	r10,GPR1(r11);						     \
+	stw	r1,GPR1(r11);						     \
 	mfspr	r9,SPRN_SRR1;						     \
-	stw	r10,0(r11);						     \
+	stw	r1,0(r11);						     \
+	tovirt(r1,r11);			/* set new kernel sp */	\
 	rlwinm	r9,r9,0,14,12;		/* clear MSR_WE (necessary?)	   */\
 	stw	r0,GPR0(r11);						     \
 	SAVE_4GPRS(3, r11);						     \
-- 
2.13.3


^ permalink raw reply related

* [PATCH 06/11] powerpc/32: move LOAD_MSR_KERNEL() into head_32.h and use it
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

As preparation for using head_32.h for head_40x.S, move
LOAD_MSR_KERNEL() there and use it to load r10 with MSR_KERNEL value.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/entry_32.S |  9 +--------
 arch/powerpc/kernel/head_32.h  | 11 ++++++++++-
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 3f83e71ae43f..24d93231665b 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -37,14 +37,7 @@
 #include <asm/feature-fixups.h>
 #include <asm/barrier.h>
 
-/*
- * MSR_KERNEL is > 0x10000 on 4xx/Book-E since it include MSR_CE.
- */
-#if MSR_KERNEL >= 0x10000
-#define LOAD_MSR_KERNEL(r, x)	lis r,(x)@h; ori r,r,(x)@l
-#else
-#define LOAD_MSR_KERNEL(r, x)	li r,(x)
-#endif
+#include "head_32.h"
 
 /*
  * Align to 4k in order to ensure that all functions modyfing srr0/srr1
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index f77f13142410..e302afd40d0a 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -13,6 +13,15 @@
 #endif
 
 /*
+ * MSR_KERNEL is > 0x10000 on 4xx/Book-E since it include MSR_CE.
+ */
+#if MSR_KERNEL >= 0x10000
+#define LOAD_MSR_KERNEL(r, x)	lis r,(x)@h; ori r,r,(x)@l
+#else
+#define LOAD_MSR_KERNEL(r, x)	li r,(x)
+#endif
+
+/*
  * Exception entry code.  This code runs with address translation
  * turned off, i.e. using physical addresses.
  * We assume sprg3 has the physical address of the current
@@ -86,7 +95,7 @@
 #define EXC_XFER_TEMPLATE(n, hdlr, trap, copyee, tfer, ret)	\
 	li	r10,trap;					\
 	stw	r10,_TRAP(r11);					\
-	li	r10,MSR_KERNEL;					\
+	LOAD_MSR_KERNEL(r10, MSR_KERNEL);			\
 	copyee(r10, r9);					\
 	bl	tfer;						\
 i##n:								\
-- 
2.13.3


^ permalink raw reply related

* [PATCH 05/11] powerpc/32: add START_EXCEPTION() in head_32.h
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

Add START_EXCEPTION() in head_32.h for preparing the use
of head_32.h in head_40x.S

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_32.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 643dd8d34aac..f77f13142410 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -72,10 +72,13 @@
 /*
  * Exception vectors.
  */
-#define EXCEPTION(n, label, hdlr, xfer)		\
+#define	START_EXCEPTION(n, label)		\
 	. = n;					\
 	DO_KVM n;				\
-label:						\
+label:
+
+#define EXCEPTION(n, label, hdlr, xfer)		\
+	START_EXCEPTION(n, label)		\
 	EXCEPTION_PROLOG;			\
 	addi	r3,r1,STACK_FRAME_OVERHEAD;	\
 	xfer(n, hdlr)
-- 
2.13.3


^ permalink raw reply related

* [PATCH 04/11] powerpc/32: add CLR_MSR_WE() in EXCEPTION_PROLOG in head_32.h
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

Add CLR_MSR_WE() macro to allow 40x to clear
that bit from the register containing msr value.

This is the only difference between common EXCEPTION_PROLOG
and 40x one. This patch will allow 40x to use the common one.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_32.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 9e6fb9d468f0..643dd8d34aac 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -8,6 +8,10 @@
 #define SET_AND_STORE_MSR_RI(reg)
 #endif
 
+#ifndef CLR_MSR_WE
+#define CLR_MSR_WE(reg)
+#endif
+
 /*
  * Exception entry code.  This code runs with address translation
  * turned off, i.e. using physical addresses.
@@ -49,6 +53,7 @@
 	stw	r1,0(r11);	\
 	tovirt(r1,r11);			/* set new kernel sp */	\
 	SET_AND_STORE_MSR_RI(r10);	/* can take exceptions */ \
+	CLR_MSR_WE(r9);		\
 	stw	r0,GPR0(r11);	\
 	lis	r10,STACK_FRAME_REGS_MARKER@ha; /* exception frame marker */ \
 	addi	r10,r10,STACK_FRAME_REGS_MARKER@l; \
-- 
2.13.3


^ permalink raw reply related

* [PATCH 03/11] powerpc/32: Add a macro for setting MSR_RI in EXCEPTION_PROLOG_2
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

Setting MSR_RI applies to head_32 and head_8xx, but not to
head_40x. So in order to refactor EXCEPTION_PROLOG for 40x too,
this patch adds a macro for setting MSR_RI.

In the meantime, this gives the opportunity to make it
simpler on the 8xx as writing in SPRN_EID sets MSR_RI.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_32.S  | 2 ++
 arch/powerpc/kernel/head_32.h  | 7 +++++--
 arch/powerpc/kernel/head_8xx.S | 2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index a5efcdc78e8e..edc5e78faf08 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -37,6 +37,8 @@
 #include <asm/export.h>
 #include <asm/feature-fixups.h>
 
+#define SET_AND_STORE_MSR_RI(reg)	li reg,MSR_KERNEL & ~(MSR_IR|MSR_DR); MTMSRD(reg)
+
 #include "head_32.h"
 
 /* 601 only have IBAT; cr0.eq is set on 601 when using this macro */
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 7356c27d2136..9e6fb9d468f0 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -4,6 +4,10 @@
 
 #include <asm/ptrace.h>	/* for STACK_FRAME_REGS_MARKER */
 
+#ifndef SET_AND_STORE_MSR_RI
+#define SET_AND_STORE_MSR_RI(reg)
+#endif
+
 /*
  * Exception entry code.  This code runs with address translation
  * turned off, i.e. using physical addresses.
@@ -44,8 +48,7 @@
 	stw	r1,GPR1(r11);	\
 	stw	r1,0(r11);	\
 	tovirt(r1,r11);			/* set new kernel sp */	\
-	li	r10,MSR_KERNEL & ~(MSR_IR|MSR_DR); /* can take exceptions */ \
-	MTMSRD(r10);			/* (except for mach check in rtas) */ \
+	SET_AND_STORE_MSR_RI(r10);	/* can take exceptions */ \
 	stw	r0,GPR0(r11);	\
 	lis	r10,STACK_FRAME_REGS_MARKER@ha; /* exception frame marker */ \
 	addi	r10,r10,STACK_FRAME_REGS_MARKER@l; \
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index d9c5bc48bef0..00dacec2b0d1 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -36,6 +36,8 @@
 .macro DO_KVM intno
 .endm
 
+#define SET_AND_STORE_MSR_RI(reg)	mtspr	SPRN_EID, reg
+
 #include "head_32.h"
 
 #if CONFIG_TASK_SIZE <= 0x80000000 && CONFIG_PAGE_OFFSET >= 0x80000000
-- 
2.13.3


^ permalink raw reply related

* [PATCH 02/11] powerpc/32: Refactor EXCEPTION entry macros for head_8xx.S and head_32.S
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

EXCEPTION_PROLOG is similar in head_8xx.S and head_32.S

This patch creates head_32.h and moves EXCEPTION_PROLOG macro
into it.

It also moves EXCEPTION() and EXC_XFER_XXXX() macros which are also
similar. For that, the 8xx needs to define a dummy DO_KVM asm macro.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_32.S  |  99 +--------------------------------------
 arch/powerpc/kernel/head_32.h  | 104 +++++++++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/head_8xx.S | 101 ++-------------------------------------
 3 files changed, 111 insertions(+), 193 deletions(-)
 create mode 100644 arch/powerpc/kernel/head_32.h

diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 9268e5e87949..a5efcdc78e8e 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -37,6 +37,8 @@
 #include <asm/export.h>
 #include <asm/feature-fixups.h>
 
+#include "head_32.h"
+
 /* 601 only have IBAT; cr0.eq is set on 601 when using this macro */
 #define LOAD_BAT(n, reg, RA, RB)	\
 	/* see the comment for clear_bats() -- Cort */ \
@@ -242,103 +244,6 @@ __secondary_hold_spinloop:
 __secondary_hold_acknowledge:
 	.long	-1
 
-/*
- * Exception entry code.  This code runs with address translation
- * turned off, i.e. using physical addresses.
- * We assume sprg3 has the physical address of the current
- * task's thread_struct.
- */
-#define EXCEPTION_PROLOG	\
-	mtspr	SPRN_SPRG_SCRATCH0,r10;	\
-	mtspr	SPRN_SPRG_SCRATCH1,r11;	\
-	mfcr	r10;		\
-	EXCEPTION_PROLOG_1;	\
-	EXCEPTION_PROLOG_2
-
-#define EXCEPTION_PROLOG_1	\
-	mfspr	r11,SPRN_SRR1;		/* check whether user or kernel */ \
-	andi.	r11,r11,MSR_PR;	\
-	tophys(r11,r1);			/* use tophys(r1) if kernel */ \
-	beq	1f;		\
-	mfspr	r11,SPRN_SPRG_THREAD;	\
-	lwz	r11,TASK_STACK-THREAD(r11);	\
-	addi	r11,r11,THREAD_SIZE;	\
-	tophys(r11,r11);	\
-1:	subi	r11,r11,INT_FRAME_SIZE	/* alloc exc. frame */
-
-
-#define EXCEPTION_PROLOG_2	\
-	stw	r10,_CCR(r11);		/* save registers */ \
-	stw	r12,GPR12(r11);	\
-	stw	r9,GPR9(r11);	\
-	mfspr	r10,SPRN_SPRG_SCRATCH0;	\
-	stw	r10,GPR10(r11);	\
-	mfspr	r12,SPRN_SPRG_SCRATCH1;	\
-	stw	r12,GPR11(r11);	\
-	mflr	r10;		\
-	stw	r10,_LINK(r11);	\
-	mfspr	r12,SPRN_SRR0;	\
-	mfspr	r9,SPRN_SRR1;	\
-	stw	r1,GPR1(r11);	\
-	stw	r1,0(r11);	\
-	tovirt(r1,r11);			/* set new kernel sp */	\
-	li	r10,MSR_KERNEL & ~(MSR_IR|MSR_DR); /* can take exceptions */ \
-	MTMSRD(r10);			/* (except for mach check in rtas) */ \
-	stw	r0,GPR0(r11);	\
-	lis	r10,STACK_FRAME_REGS_MARKER@ha; /* exception frame marker */ \
-	addi	r10,r10,STACK_FRAME_REGS_MARKER@l; \
-	stw	r10,8(r11);	\
-	SAVE_4GPRS(3, r11);	\
-	SAVE_2GPRS(7, r11)
-
-/*
- * Note: code which follows this uses cr0.eq (set if from kernel),
- * r11, r12 (SRR0), and r9 (SRR1).
- *
- * Note2: once we have set r1 we are in a position to take exceptions
- * again, and we could thus set MSR:RI at that point.
- */
-
-/*
- * Exception vectors.
- */
-#define EXCEPTION(n, label, hdlr, xfer)		\
-	. = n;					\
-	DO_KVM n;				\
-label:						\
-	EXCEPTION_PROLOG;			\
-	addi	r3,r1,STACK_FRAME_OVERHEAD;	\
-	xfer(n, hdlr)
-
-#define EXC_XFER_TEMPLATE(n, hdlr, trap, copyee, tfer, ret)	\
-	li	r10,trap;					\
-	stw	r10,_TRAP(r11);					\
-	li	r10,MSR_KERNEL;					\
-	copyee(r10, r9);					\
-	bl	tfer;						\
-i##n:								\
-	.long	hdlr;						\
-	.long	ret
-
-#define COPY_EE(d, s)		rlwimi d,s,0,16,16
-#define NOCOPY(d, s)
-
-#define EXC_XFER_STD(n, hdlr)		\
-	EXC_XFER_TEMPLATE(n, hdlr, n, NOCOPY, transfer_to_handler_full,	\
-			  ret_from_except_full)
-
-#define EXC_XFER_LITE(n, hdlr)		\
-	EXC_XFER_TEMPLATE(n, hdlr, n+1, NOCOPY, transfer_to_handler, \
-			  ret_from_except)
-
-#define EXC_XFER_EE(n, hdlr)		\
-	EXC_XFER_TEMPLATE(n, hdlr, n, COPY_EE, transfer_to_handler_full, \
-			  ret_from_except_full)
-
-#define EXC_XFER_EE_LITE(n, hdlr)	\
-	EXC_XFER_TEMPLATE(n, hdlr, n+1, COPY_EE, transfer_to_handler, \
-			  ret_from_except)
-
 /* System reset */
 /* core99 pmac starts the seconary here by changing the vector, and
    putting it back to what it was (unknown_exception) when done.  */
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
new file mode 100644
index 000000000000..7356c27d2136
--- /dev/null
+++ b/arch/powerpc/kernel/head_32.h
@@ -0,0 +1,104 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __HEAD_32_H__
+#define __HEAD_32_H__
+
+#include <asm/ptrace.h>	/* for STACK_FRAME_REGS_MARKER */
+
+/*
+ * Exception entry code.  This code runs with address translation
+ * turned off, i.e. using physical addresses.
+ * We assume sprg3 has the physical address of the current
+ * task's thread_struct.
+ */
+#define EXCEPTION_PROLOG	\
+	mtspr	SPRN_SPRG_SCRATCH0,r10;	\
+	mtspr	SPRN_SPRG_SCRATCH1,r11;	\
+	mfcr	r10;		\
+	EXCEPTION_PROLOG_1;	\
+	EXCEPTION_PROLOG_2
+
+#define EXCEPTION_PROLOG_1	\
+	mfspr	r11,SPRN_SRR1;		/* check whether user or kernel */ \
+	andi.	r11,r11,MSR_PR;	\
+	tophys(r11,r1);			/* use tophys(r1) if kernel */ \
+	beq	1f;		\
+	mfspr	r11,SPRN_SPRG_THREAD;	\
+	lwz	r11,TASK_STACK-THREAD(r11);	\
+	addi	r11,r11,THREAD_SIZE;	\
+	tophys(r11,r11);	\
+1:	subi	r11,r11,INT_FRAME_SIZE	/* alloc exc. frame */
+
+
+#define EXCEPTION_PROLOG_2	\
+	stw	r10,_CCR(r11);		/* save registers */ \
+	stw	r12,GPR12(r11);	\
+	stw	r9,GPR9(r11);	\
+	mfspr	r10,SPRN_SPRG_SCRATCH0;	\
+	stw	r10,GPR10(r11);	\
+	mfspr	r12,SPRN_SPRG_SCRATCH1;	\
+	stw	r12,GPR11(r11);	\
+	mflr	r10;		\
+	stw	r10,_LINK(r11);	\
+	mfspr	r12,SPRN_SRR0;	\
+	mfspr	r9,SPRN_SRR1;	\
+	stw	r1,GPR1(r11);	\
+	stw	r1,0(r11);	\
+	tovirt(r1,r11);			/* set new kernel sp */	\
+	li	r10,MSR_KERNEL & ~(MSR_IR|MSR_DR); /* can take exceptions */ \
+	MTMSRD(r10);			/* (except for mach check in rtas) */ \
+	stw	r0,GPR0(r11);	\
+	lis	r10,STACK_FRAME_REGS_MARKER@ha; /* exception frame marker */ \
+	addi	r10,r10,STACK_FRAME_REGS_MARKER@l; \
+	stw	r10,8(r11);	\
+	SAVE_4GPRS(3, r11);	\
+	SAVE_2GPRS(7, r11)
+
+/*
+ * Note: code which follows this uses cr0.eq (set if from kernel),
+ * r11, r12 (SRR0), and r9 (SRR1).
+ *
+ * Note2: once we have set r1 we are in a position to take exceptions
+ * again, and we could thus set MSR:RI at that point.
+ */
+
+/*
+ * Exception vectors.
+ */
+#define EXCEPTION(n, label, hdlr, xfer)		\
+	. = n;					\
+	DO_KVM n;				\
+label:						\
+	EXCEPTION_PROLOG;			\
+	addi	r3,r1,STACK_FRAME_OVERHEAD;	\
+	xfer(n, hdlr)
+
+#define EXC_XFER_TEMPLATE(n, hdlr, trap, copyee, tfer, ret)	\
+	li	r10,trap;					\
+	stw	r10,_TRAP(r11);					\
+	li	r10,MSR_KERNEL;					\
+	copyee(r10, r9);					\
+	bl	tfer;						\
+i##n:								\
+	.long	hdlr;						\
+	.long	ret
+
+#define COPY_EE(d, s)		rlwimi d,s,0,MSR_EE
+#define NOCOPY(d, s)
+
+#define EXC_XFER_STD(n, hdlr)		\
+	EXC_XFER_TEMPLATE(n, hdlr, n, NOCOPY, transfer_to_handler_full,	\
+			  ret_from_except_full)
+
+#define EXC_XFER_LITE(n, hdlr)		\
+	EXC_XFER_TEMPLATE(n, hdlr, n+1, NOCOPY, transfer_to_handler, \
+			  ret_from_except)
+
+#define EXC_XFER_EE(n, hdlr)		\
+	EXC_XFER_TEMPLATE(n, hdlr, n, COPY_EE, transfer_to_handler_full, \
+			  ret_from_except_full)
+
+#define EXC_XFER_EE_LITE(n, hdlr)	\
+	EXC_XFER_TEMPLATE(n, hdlr, n+1, COPY_EE, transfer_to_handler, \
+			  ret_from_except)
+
+#endif /* __HEAD_32_H__ */
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index ca9207013579..d9c5bc48bef0 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -33,6 +33,11 @@
 #include <asm/export.h>
 #include <asm/code-patching-asm.h>
 
+.macro DO_KVM intno
+.endm
+
+#include "head_32.h"
+
 #if CONFIG_TASK_SIZE <= 0x80000000 && CONFIG_PAGE_OFFSET >= 0x80000000
 /* By simply checking Address >= 0x80000000, we know if its a kernel address */
 #define SIMPLE_KERNEL_ADDRESS		1
@@ -123,102 +128,6 @@ instruction_counter:
 	.space	4
 #endif
 
-/*
- * Exception entry code.  This code runs with address translation
- * turned off, i.e. using physical addresses.
- * We assume sprg3 has the physical address of the current
- * task's thread_struct.
- */
-#define EXCEPTION_PROLOG	\
-	mtspr	SPRN_SPRG_SCRATCH0, r10;	\
-	mtspr	SPRN_SPRG_SCRATCH1, r11;	\
-	mfcr	r10;		\
-	EXCEPTION_PROLOG_1;	\
-	EXCEPTION_PROLOG_2
-
-#define EXCEPTION_PROLOG_1	\
-	mfspr	r11,SPRN_SRR1;		/* check whether user or kernel */ \
-	andi.	r11,r11,MSR_PR;	\
-	tophys(r11,r1);			/* use tophys(r1) if kernel */ \
-	beq	1f;		\
-	mfspr	r11,SPRN_SPRG_THREAD;	\
-	lwz	r11,TASK_STACK-THREAD(r11);	\
-	addi	r11,r11,THREAD_SIZE;	\
-	tophys(r11,r11);	\
-1:	subi	r11,r11,INT_FRAME_SIZE	/* alloc exc. frame */
-
-
-#define EXCEPTION_PROLOG_2	\
-	stw	r10,_CCR(r11);		/* save registers */ \
-	stw	r12,GPR12(r11);	\
-	stw	r9,GPR9(r11);	\
-	mfspr	r10,SPRN_SPRG_SCRATCH0;	\
-	stw	r10,GPR10(r11);	\
-	mfspr	r12,SPRN_SPRG_SCRATCH1;	\
-	stw	r12,GPR11(r11);	\
-	mflr	r10;		\
-	stw	r10,_LINK(r11);	\
-	mfspr	r12,SPRN_SRR0;	\
-	mfspr	r9,SPRN_SRR1;	\
-	stw	r1,GPR1(r11);	\
-	stw	r1,0(r11);	\
-	tovirt(r1,r11);			/* set new kernel sp */	\
-	li	r10,MSR_KERNEL & ~(MSR_IR|MSR_DR); /* can take exceptions */ \
-	mtmsr	r10;		\
-	stw	r0,GPR0(r11);	\
-	lis	r10, STACK_FRAME_REGS_MARKER@ha; /* exception frame marker */ \
-	addi	r10, r10, STACK_FRAME_REGS_MARKER@l; \
-	stw	r10, 8(r11);	\
-	SAVE_4GPRS(3, r11);	\
-	SAVE_2GPRS(7, r11)
-
-/*
- * Note: code which follows this uses cr0.eq (set if from kernel),
- * r11, r12 (SRR0), and r9 (SRR1).
- *
- * Note2: once we have set r1 we are in a position to take exceptions
- * again, and we could thus set MSR:RI at that point.
- */
-
-/*
- * Exception vectors.
- */
-#define EXCEPTION(n, label, hdlr, xfer)		\
-	. = n;					\
-label:						\
-	EXCEPTION_PROLOG;			\
-	addi	r3,r1,STACK_FRAME_OVERHEAD;	\
-	xfer(n, hdlr)
-
-#define EXC_XFER_TEMPLATE(n, hdlr, trap, copyee, tfer, ret)	\
-	li	r10,trap;					\
-	stw	r10,_TRAP(r11);					\
-	li	r10,MSR_KERNEL;					\
-	copyee(r10, r9);					\
-	bl	tfer;						\
-i##n:								\
-	.long	hdlr;						\
-	.long	ret
-
-#define COPY_EE(d, s)		rlwimi d,s,0,16,16
-#define NOCOPY(d, s)
-
-#define EXC_XFER_STD(n, hdlr)		\
-	EXC_XFER_TEMPLATE(n, hdlr, n, NOCOPY, transfer_to_handler_full,	\
-			  ret_from_except_full)
-
-#define EXC_XFER_LITE(n, hdlr)		\
-	EXC_XFER_TEMPLATE(n, hdlr, n+1, NOCOPY, transfer_to_handler, \
-			  ret_from_except)
-
-#define EXC_XFER_EE(n, hdlr)		\
-	EXC_XFER_TEMPLATE(n, hdlr, n, COPY_EE, transfer_to_handler_full, \
-			  ret_from_except_full)
-
-#define EXC_XFER_EE_LITE(n, hdlr)	\
-	EXC_XFER_TEMPLATE(n, hdlr, n+1, COPY_EE, transfer_to_handler, \
-			  ret_from_except)
-
 /* System reset */
 	EXCEPTION(0x100, Reset, system_reset_exception, EXC_XFER_STD)
 
-- 
2.13.3


^ permalink raw reply related

* [PATCH 01/11] powerpc/32: Rename THREAD_INFO to TASK_STACK
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1548673552.git.christophe.leroy@c-s.fr>

This patch renames THREAD_INFO to TASK_STACK, because it is in fact
the offset of the pointer to the stack in task_struct so this pointer
will not be impacted by the move of THREAD_INFO.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/asm-offsets.c    | 2 +-
 arch/powerpc/kernel/entry_32.S       | 2 +-
 arch/powerpc/kernel/head_32.S        | 2 +-
 arch/powerpc/kernel/head_40x.S       | 4 ++--
 arch/powerpc/kernel/head_8xx.S       | 2 +-
 arch/powerpc/kernel/head_booke.h     | 4 ++--
 arch/powerpc/kernel/head_fsl_booke.S | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 9ffc72ded73a..23456ba3410a 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -90,7 +90,7 @@ int main(void)
 	DEFINE(SIGSEGV, SIGSEGV);
 	DEFINE(NMI_MASK, NMI_MASK);
 #else
-	OFFSET(THREAD_INFO, task_struct, stack);
+	OFFSET(TASK_STACK, task_struct, stack);
 	DEFINE(THREAD_INFO_GAP, _ALIGN_UP(sizeof(struct thread_info), 16));
 	OFFSET(KSP_LIMIT, thread_struct, ksp_limit);
 #endif /* CONFIG_PPC64 */
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 0768dfd8a64e..3f83e71ae43f 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -1166,7 +1166,7 @@ ret_from_debug_exc:
 	mfspr	r9,SPRN_SPRG_THREAD
 	lwz	r10,SAVED_KSP_LIMIT(r1)
 	stw	r10,KSP_LIMIT(r9)
-	lwz	r9,THREAD_INFO-THREAD(r9)
+	lwz	r9,TASK_STACK-THREAD(r9)
 	CURRENT_THREAD_INFO(r10, r1)
 	lwz	r10,TI_PREEMPT(r10)
 	stw	r10,TI_PREEMPT(r9)
diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 05b08db3901d..9268e5e87949 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -261,7 +261,7 @@ __secondary_hold_acknowledge:
 	tophys(r11,r1);			/* use tophys(r1) if kernel */ \
 	beq	1f;		\
 	mfspr	r11,SPRN_SPRG_THREAD;	\
-	lwz	r11,THREAD_INFO-THREAD(r11);	\
+	lwz	r11,TASK_STACK-THREAD(r11);	\
 	addi	r11,r11,THREAD_SIZE;	\
 	tophys(r11,r11);	\
 1:	subi	r11,r11,INT_FRAME_SIZE	/* alloc exc. frame */
diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index b19d78410511..3088c9f29f5e 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -115,7 +115,7 @@ _ENTRY(saved_ksp_limit)
 	andi.	r11,r11,MSR_PR;						     \
 	beq	1f;							     \
 	mfspr	r1,SPRN_SPRG_THREAD;	/* if from user, start at top of   */\
-	lwz	r1,THREAD_INFO-THREAD(r1); /* this thread's kernel stack   */\
+	lwz	r1,TASK_STACK-THREAD(r1); /* this thread's kernel stack   */\
 	addi	r1,r1,THREAD_SIZE;					     \
 1:	subi	r1,r1,INT_FRAME_SIZE;	/* Allocate an exception frame     */\
 	tophys(r11,r1);							     \
@@ -158,7 +158,7 @@ _ENTRY(saved_ksp_limit)
 	beq	1f;							     \
 	/* COMING FROM USER MODE */					     \
 	mfspr	r11,SPRN_SPRG_THREAD;	/* if from user, start at top of   */\
-	lwz	r11,THREAD_INFO-THREAD(r11); /* this thread's kernel stack */\
+	lwz	r11,TASK_STACK-THREAD(r11); /* this thread's kernel stack */\
 1:	addi	r11,r11,THREAD_SIZE-INT_FRAME_SIZE; /* Alloc an excpt frm  */\
 	tophys(r11,r11);						     \
 	stw	r10,_CCR(r11);          /* save various registers	   */\
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 20cc816b3508..ca9207013579 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -142,7 +142,7 @@ instruction_counter:
 	tophys(r11,r1);			/* use tophys(r1) if kernel */ \
 	beq	1f;		\
 	mfspr	r11,SPRN_SPRG_THREAD;	\
-	lwz	r11,THREAD_INFO-THREAD(r11);	\
+	lwz	r11,TASK_STACK-THREAD(r11);	\
 	addi	r11,r11,THREAD_SIZE;	\
 	tophys(r11,r11);	\
 1:	subi	r11,r11,INT_FRAME_SIZE	/* alloc exc. frame */
diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
index 306e26c073a0..69e80e6d0d16 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -55,7 +55,7 @@ END_BTB_FLUSH_SECTION
 	beq	1f;							     \
 	BOOKE_CLEAR_BTB(r11)						\
 	/* if from user, start at top of this thread's kernel stack */       \
-	lwz	r11, THREAD_INFO-THREAD(r10);				     \
+	lwz	r11, TASK_STACK - THREAD(r10);				     \
 	ALLOC_STACK_FRAME(r11, THREAD_SIZE);				     \
 1 :	subi	r11, r11, INT_FRAME_SIZE; /* Allocate exception frame */     \
 	stw	r13, _CCR(r11);		/* save various registers */	     \
@@ -142,7 +142,7 @@ END_BTB_FLUSH_SECTION
 	BOOKE_CLEAR_BTB(r10)						\
 	andi.	r11,r11,MSR_PR;						     \
 	mfspr	r11,SPRN_SPRG_THREAD;	/* if from user, start at top of   */\
-	lwz	r11,THREAD_INFO-THREAD(r11); /* this thread's kernel stack */\
+	lwz	r11, TASK_STACK - THREAD(r11); /* this thread's kernel stack */\
 	addi	r11,r11,EXC_LVL_FRAME_OVERHEAD;	/* allocate stack frame    */\
 	beq	1f;							     \
 	/* COMING FROM USER MODE */					     \
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 2386ce2a9c6e..acf9a58c3c00 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -717,7 +717,7 @@ finish_tlb_load:
 
 	/* Get the next_tlbcam_idx percpu var */
 #ifdef CONFIG_SMP
-	lwz	r12, THREAD_INFO-THREAD(r12)
+	lwz	r12, TASK_STACK-THREAD(r12)
 	lwz	r15, TI_CPU(r12)
 	lis     r14, __per_cpu_offset@h
 	ori     r14, r14, __per_cpu_offset@l
-- 
2.13.3


^ permalink raw reply related

* [PATCH 00/11] Refactor exception entry on 40x/6xx/8xx
From: Christophe Leroy @ 2019-01-28 11:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

This serie refactors exception entry macros for 40x, 6xx and 8xx

This serie will benefit to the implementation of CONFIG_VMAP, and also
to Ben's serie on MSR_EE.

First patch of this serie is a part of the CONFIG_THREAD_INFO_IN_TASK serie.
This avoids a conflict between the two series.

Christophe Leroy (11):
  powerpc/32: Rename THREAD_INFO to TASK_STACK
  powerpc/32: Refactor EXCEPTION entry macros for head_8xx.S and
    head_32.S
  powerpc/32: Add a macro for setting MSR_RI in EXCEPTION_PROLOG_2
  powerpc/32: add CLR_MSR_WE() in EXCEPTION_PROLOG in head_32.h
  powerpc/32: add START_EXCEPTION() in head_32.h
  powerpc/32: move LOAD_MSR_KERNEL() into head_32.h and use it
  powerpc/40x: Don't use SPRN_SPRG_SCRATCH2 in EXCEPTION_PROLOG
  powerpc/40x: add exception frame marker
  powerpc/40x: Split and rename NORMAL_EXCEPTION_PROLOG
  powerpc/40x: Add EXC_XFER_TEMPLATE_CRITICAL()
  powerpc/40x: Refactor exception entry macros by using head_32.h

 arch/powerpc/kernel/asm-offsets.c    |   2 +-
 arch/powerpc/kernel/entry_32.S       |  11 +---
 arch/powerpc/kernel/head_32.S        | 101 ++--------------------------
 arch/powerpc/kernel/head_32.h        | 124 +++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/head_40x.S       | 120 ++++++++-------------------------
 arch/powerpc/kernel/head_8xx.S       | 103 ++---------------------------
 arch/powerpc/kernel/head_booke.h     |   4 +-
 arch/powerpc/kernel/head_fsl_booke.S |   2 +-
 8 files changed, 167 insertions(+), 300 deletions(-)
 create mode 100644 arch/powerpc/kernel/head_32.h

-- 
2.13.3


^ permalink raw reply

* Re: [PATCH] perf mem/c2c: Fix perf_mem_events to support powerpc
From: Jiri Olsa @ 2019-01-28 11:07 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: dzickus, maddy, linux-kernel, acme, fowles, jmario, namhyung,
	linuxppc-dev
In-Reply-To: <20190114041402.14033-1-ravi.bangoria@linux.ibm.com>

On Mon, Jan 14, 2019 at 09:44:02AM +0530, Ravi Bangoria wrote:

SNIP

> diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
> new file mode 100644
> index 0000000..5b4dcfe
> --- /dev/null
> +++ b/tools/perf/arch/x86/util/mem-events.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "mem-events.h"
> +
> +struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
> +	PERF_MEM_EVENT("ldlat-loads", "cpu/mem-loads,ldlat=%u/P", "mem-loads"),
> +	PERF_MEM_EVENT("ldlat-stores", "cpu/mem-stores/P", "mem-stores"),
> +};
> +
> +static char mem_loads_name[100];
> +static bool mem_loads_name__init;
> +
> +char *perf_mem_events__name(int i)
> +{
> +	if (i == PERF_MEM_EVENTS__LOAD) {
> +		if (!mem_loads_name__init) {
> +			mem_loads_name__init = true;
> +			scnprintf(mem_loads_name, sizeof(mem_loads_name),
> +				  perf_mem_events[i].name,
> +				  perf_mem_events__loads_ldlat);
> +		}
> +		return mem_loads_name;
> +	}
> +
> +	return (char *)perf_mem_events[i].name;
> +}
> diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
> index 93f74d8..1ffefd3 100644
> --- a/tools/perf/util/mem-events.c
> +++ b/tools/perf/util/mem-events.c
> @@ -15,31 +15,13 @@
>  
>  unsigned int perf_mem_events__loads_ldlat = 30;
>  
> -#define E(t, n, s) { .tag = t, .name = n, .sysfs_name = s }
> -
> -struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
> -	E("ldlat-loads",	"cpu/mem-loads,ldlat=%u/P",	"mem-loads"),
> -	E("ldlat-stores",	"cpu/mem-stores/P",		"mem-stores"),
> +struct perf_mem_event __weak perf_mem_events[PERF_MEM_EVENTS__MAX] = {
> +	PERF_MEM_EVENT("ldlat-loads", "cpu/mem-loads/P", "mem-loads"),
> +	PERF_MEM_EVENT("ldlat-stores", "cpu/mem-stores/P", "mem-stores"),
>  };

I dont think perf_mem_events array needs to be overloaded as well,
the perf_mem_events__name function should be enough no?

thanks,
jirka

^ permalink raw reply

* Re: [PATCH] perf mem/c2c: Fix perf_mem_events to support powerpc
From: Ravi Bangoria @ 2019-01-28 10:08 UTC (permalink / raw)
  To: acme, Michael Ellerman
  Cc: dzickus, maddy, linuxppc-dev, fowles, linux-kernel, jmario,
	namhyung, jolsa
In-Reply-To: <20190114041402.14033-1-ravi.bangoria@linux.ibm.com>



On 1/14/19 9:44 AM, Ravi Bangoria wrote:
> Powerpc hw does not have inbuilt latency filter (--ldlat) for mem-load
> event and, perf_mem_events by default includes ldlat=30 which is
> causing failure on powerpc. Refactor code to support perf mem/c2c on
> powerpc.
> 
> This patch depends on kernel side changes done my Madhavan:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-December/182596.html
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>


Arnaldo / Michael, Any thoughts?

Thanks.


^ permalink raw reply

* Re: [PATCH] ucc_geth: Reset BQL queue when stopping device
From: Christophe Leroy @ 2019-01-28  9:48 UTC (permalink / raw)
  To: Mathias Thore, leoyang.li, netdev, linuxppc-dev, david.gounaris,
	joakim.tjernlund
In-Reply-To: <20190128090747.15851-1-mathias.thore@infinera.com>

Hi,

Le 28/01/2019 à 10:07, Mathias Thore a écrit :
> After a timeout event caused by for example a broadcast storm, when
> the MAC and PHY are reset, the BQL TX queue needs to be reset as
> well. Otherwise, the device will exhibit severe performance issues
> even after the storm has ended.

What are the symptomns ?

Is this reset needed on any network driver in that case, or is it 
something particular for the ucc_geth ?
For instance, the freescale fs_enet doesn't have that reset. Should it 
have it too ?

Christophe

> 
> Co-authored-by: David Gounaris <david.gounaris@infinera.com>
> Signed-off-by: Mathias Thore <mathias.thore@infinera.com>
> ---
>   drivers/net/ethernet/freescale/ucc_geth.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> index c3d539e209ed..eb3e65e8868f 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -1879,6 +1879,8 @@ static void ucc_geth_free_tx(struct ucc_geth_private *ugeth)
>   	u16 i, j;
>   	u8 __iomem *bd;
>   
> +	netdev_reset_queue(ugeth->ndev);
> +
>   	ug_info = ugeth->ug_info;
>   	uf_info = &ug_info->uf_info;
>   
> 

^ permalink raw reply

* Re: [PATCH v4] kbuild: Add support for DT binding schema checks
From: Geert Uytterhoeven @ 2019-01-28  9:43 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Kumar Gala, arm-soc, Sean Hudson, Jonathan Corbet, linuxppc-dev,
	open list:DOCUMENTATION, Linux Kernel Mailing List, linux-kbuild,
	Masahiro Yamada, Grant Likely, Michal Marek, Frank Rowand,
	Linux ARM
In-Reply-To: <20181211202406.27721-1-robh@kernel.org>

Hi Rob,

On Tue, Dec 11, 2018 at 9:24 PM Rob Herring <robh@kernel.org> wrote:
> This adds the build infrastructure for checking DT binding schema
> documents and validating dts files using the binding schema.
>
> Check DT binding schema documents:
> make dt_binding_check
>
> Build dts files and check using DT binding schema:
> make dtbs_check
>
> Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to
> use for validation. This makes it easier to find and fix errors
> generated by a specific schema.
>
> Currently, the validation targets are separate from a normal build to
> avoid a hard dependency on the external DT schema project and because
> there are lots of warnings generated.
>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <michal.lkml@markovi.net>
> Cc: linux-doc@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kbuild@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>

BTW, what are the CONFIG dependencies for this to work?
E.g. defconfig on x86_64 fails, even after enabling CONFIG_OF:

$ make dt_binding_check
  SCHEMA  Documentation/devicetree/bindings/processed-schema.yaml
  CHKDT   Documentation/devicetree/bindings/arm/primecell.yaml
  ...
  CHKDT   Documentation/devicetree/bindings/trivial-devices.yaml
make[1]: *** No rule to make target
'Documentation/devicetree/bindings/arm/primecell.example.dtb', needed
by '__build'.  Stop.

Obviously it does work for arm/arm64.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] ucc_geth: Reset BQL queue when stopping device
From: Mathias Thore @ 2019-01-28  9:07 UTC (permalink / raw)
  To: leoyang.li, netdev, linuxppc-dev, david.gounaris,
	joakim.tjernlund
  Cc: Mathias Thore

After a timeout event caused by for example a broadcast storm, when
the MAC and PHY are reset, the BQL TX queue needs to be reset as
well. Otherwise, the device will exhibit severe performance issues
even after the storm has ended.

Co-authored-by: David Gounaris <david.gounaris@infinera.com>
Signed-off-by: Mathias Thore <mathias.thore@infinera.com>
---
 drivers/net/ethernet/freescale/ucc_geth.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index c3d539e209ed..eb3e65e8868f 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1879,6 +1879,8 @@ static void ucc_geth_free_tx(struct ucc_geth_private *ugeth)
 	u16 i, j;
 	u8 __iomem *bd;
 
+	netdev_reset_queue(ugeth->ndev);
+
 	ug_info = ugeth->ug_info;
 	uf_info = &ug_info->uf_info;
 
-- 
2.19.2


^ permalink raw reply related

* Re: [PATCH v4] kbuild: Add support for DT binding schema checks
From: Geert Uytterhoeven @ 2019-01-28  8:59 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Kumar Gala, arm-soc, Sean Hudson, Jonathan Corbet, linuxppc-dev,
	open list:DOCUMENTATION, Linux Kernel Mailing List, linux-kbuild,
	Masahiro Yamada, Grant Likely, Michal Marek, Frank Rowand,
	Linux ARM
In-Reply-To: <CAL_JsqJKhOmp+UKS=B-_8txHhRbqheG3GuogcZpXJ+aYcO5Oqg@mail.gmail.com>

Hi Rob,

On Sun, Jan 27, 2019 at 4:00 AM Rob Herring <robh@kernel.org> wrote:
> On Wed, Jan 23, 2019 at 9:33 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Dec 11, 2018 at 9:24 PM Rob Herring <robh@kernel.org> wrote:
> > > This adds the build infrastructure for checking DT binding schema
> > > documents and validating dts files using the binding schema.
> > >
> > > Check DT binding schema documents:
> > > make dt_binding_check
> > >
> > > Build dts files and check using DT binding schema:
> > > make dtbs_check
> > >
> > > Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to
> > > use for validation. This makes it easier to find and fix errors
> > > generated by a specific schema.
> > >
> > > Currently, the validation targets are separate from a normal build to
> > > avoid a hard dependency on the external DT schema project and because
> > > there are lots of warnings generated.
> >
> > Thanks, I'm giving this a try, and get errors like:
> >
> >       DTC     arch/arm/boot/dts/emev2-kzm9d.dt.yaml
> >     FATAL ERROR: No markers present in property 'cpu0' value
> >
> > and
> >
> >       DTC     arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml
> >     FATAL ERROR: No markers present in property 'audio_clk_a' value
> >
> > Do you have a clue?
>
> That's really strange because those aren't even properties. Are other
> dts files okay? This is the in tree dtc?
>
> The only time you should be missing markers is if you did a dts -> dts
> -> dt.yaml.

Found it: make dtbs_check doesn't play well with my local change to
add symbols for DT overlays:

    --- a/scripts/Makefile.lib
    +++ b/scripts/Makefile.lib
    @@ -285,6 +285,10 @@ cmd_dt_S_dtb=
             \
     $(obj)/%.dtb.S: $(obj)/%.dtb FORCE
            $(call if_changed,dt_S_dtb)

    +ifeq ($(CONFIG_OF_OVERLAY),y)
    +DTC_FLAGS += -@
    +endif
    +
     quiet_cmd_dtc = DTC     $@
     cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
            $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o
$(dtc-tmp) $< ; \

Do you see a way to handle that better?

Apart from a few expected issues, I'm seeing one other strange message:

    arch/arm/boot/dts/sh73a0-kzm9g.dt.yaml: interrupts: [[2, 4], [3,
4]] is too long

This is the interrupts property in the adi,adxl345 node in
arch/arm/boot/dts/sh73a0-kzm9g.dts.
Apparently the check complains if more than one interrupt is listed here.
Is this a known issue?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2] cxl: Wrap iterations over afu slices inside 'afu_list_lock'
From: Frederic Barrat @ 2019-01-28  8:42 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev
  Cc: Philippe Bergheaud, Alastair D'Silva, Christophe Lombard,
	Andrew Donnellan
In-Reply-To: <20190126114603.29278-1-vaibhav@linux.ibm.com>

Hi Vaibhav,

2 comments below (one of which I missed on the previous iteration, sorry).


Le 26/01/2019 à 12:46, Vaibhav Jain a écrit :
> Within cxl module, iteration over array 'adapter->slices' may be racy
> at few points as it might be simultaneously read during an EEH and its
> contents being set to NULL while driver is being unloaded or unbound
> from the adapter. This might result in a NULL pointer to 'struct afu'
> being de-referenced during an EEH thereby causing a kernel oops.
> 
> This patch fixes this by making sure that all access to the array
> 'adapter->slices' is wrapped within the context of spin-lock
> 'adapter->afu_list_lock'.
> 
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Changelog:
> 
> v2:
> * Fixed a wrong comparison of non-null pointer [Fred]
> * Moved a call to cxl_vphb_error_detected() within a branch that
>    checks for not null AFU pointer in 'adapter->slices' [Fred]
> * Removed a misleading comment in code.
> ---
>   drivers/misc/cxl/guest.c |  2 ++
>   drivers/misc/cxl/pci.c   | 43 ++++++++++++++++++++++++++++------------
>   2 files changed, 32 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> index 5d28d9e454f5..08f4a512afad 100644
> --- a/drivers/misc/cxl/guest.c
> +++ b/drivers/misc/cxl/guest.c
> @@ -267,6 +267,7 @@ static int guest_reset(struct cxl *adapter)
>   	int i, rc;
>   
>   	pr_devel("Adapter reset request\n");
> +	spin_lock(&adapter->afu_list_lock);
>   	for (i = 0; i < adapter->slices; i++) {
>   		if ((afu = adapter->afu[i])) {
>   			pci_error_handlers(afu, CXL_ERROR_DETECTED_EVENT,
> @@ -283,6 +284,7 @@ static int guest_reset(struct cxl *adapter)
>   			pci_error_handlers(afu, CXL_RESUME_EVENT, 0);
>   		}
>   	}
> +	spin_unlock(&adapter->afu_list_lock);
>   	return rc;
>   }
>   
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index c79ba1c699ad..ca968a889425 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -1805,7 +1805,7 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
>   	/* There should only be one entry, but go through the list
>   	 * anyway
>   	 */
> -	if (afu->phb == NULL)
> +	if (afu == NULL || afu->phb == NULL)
>   		return result;
>   
>   	list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
> @@ -1832,7 +1832,8 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>   {
>   	struct cxl *adapter = pci_get_drvdata(pdev);
>   	struct cxl_afu *afu;
> -	pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET, afu_result;
> +	pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET;
> +	pci_ers_result_t afu_result = PCI_ERS_RESULT_NEED_RESET;
>   	int i;
>   
>   	/* At this point, we could still have an interrupt pending.
> @@ -1843,6 +1844,7 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>   
>   	/* If we're permanently dead, give up. */
>   	if (state == pci_channel_io_perm_failure) {
> +		spin_lock(&adapter->afu_list_lock);
>   		for (i = 0; i < adapter->slices; i++) {
>   			afu = adapter->afu[i];
>   			/*
> @@ -1851,6 +1853,7 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>   			 */
>   			cxl_vphb_error_detected(afu, state);
>   		}
> +		spin_unlock(&adapter->afu_list_lock);
>   		return PCI_ERS_RESULT_DISCONNECT;
>   	}
>   
> @@ -1932,14 +1935,19 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>   	 *     * In slot_reset, free the old resources and allocate new ones.
>   	 *     * In resume, clear the flag to allow things to start.
>   	 */
> +
> +	/* Make sure no one else changes the afu list */
> +	spin_lock(&adapter->afu_list_lock);
> +
>   	for (i = 0; i < adapter->slices; i++) {
>   		afu = adapter->afu[i];
>   
> -		afu_result = cxl_vphb_error_detected(afu, state);
> -
> -		cxl_context_detach_all(afu);
> -		cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
> -		pci_deconfigure_afu(afu);
> +		if (afu != NULL) {
> +			afu_result = cxl_vphb_error_detected(afu, state);
> +			cxl_context_detach_all(afu);
> +			cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
> +			pci_deconfigure_afu(afu);
> +		}
>   
>   		/* Disconnect trumps all, NONE trumps NEED_RESET */
>   		if (afu_result == PCI_ERS_RESULT_DISCONNECT)


Thanks for moving the call to cxl_vphb_error_detected(), but now, the 
"if (afu_result == PCI_ERS_RESULT_DISCONNECT)" test looks like it should 
also be part of the "if (afu != NULL)" statement (and then you wouldn't 
have hit the warning about uninitialized afu_result). Current code would 
work, but looks awkward since there's no need to check afu_result at 
each iteration if afu is NULL.



> @@ -1948,6 +1956,7 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>   			 (result == PCI_ERS_RESULT_NEED_RESET))
>   			result = PCI_ERS_RESULT_NONE;
>   	}
> +	spin_unlock(&adapter->afu_list_lock);
>   
>   	/* should take the context lock here */
>   	if (cxl_adapter_context_lock(adapter) != 0)
> @@ -1980,14 +1989,15 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
>   	 */
>   	cxl_adapter_context_unlock(adapter);
>   
> +	spin_lock(&adapter->afu_list_lock);
>   	for (i = 0; i < adapter->slices; i++) {
>   		afu = adapter->afu[i];
>   

We're missing here:
if (afu == NULL)
	continue;

Not introduced with this patch, but since you're fixing a few similar 
cases...

Thanks,

   Fred


>   		if (pci_configure_afu(afu, adapter, pdev))
> -			goto err;
> +			goto err_unlock;
>   
>   		if (cxl_afu_select_best_mode(afu))
> -			goto err;
> +			goto err_unlock;
>   
>   		if (afu->phb == NULL)
>   			continue;
> @@ -1999,16 +2009,16 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
>   			ctx = cxl_get_context(afu_dev);
>   
>   			if (ctx && cxl_release_context(ctx))
> -				goto err;
> +				goto err_unlock;
>   
>   			ctx = cxl_dev_context_init(afu_dev);
>   			if (IS_ERR(ctx))
> -				goto err;
> +				goto err_unlock;
>   
>   			afu_dev->dev.archdata.cxl_ctx = ctx;
>   
>   			if (cxl_ops->afu_check_and_enable(afu))
> -				goto err;
> +				goto err_unlock;
>   
>   			afu_dev->error_state = pci_channel_io_normal;
>   
> @@ -2029,8 +2039,13 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
>   				result = PCI_ERS_RESULT_DISCONNECT;
>   		}
>   	}
> +
> +	spin_unlock(&adapter->afu_list_lock);
>   	return result;
>   
> +err_unlock:
> +	spin_unlock(&adapter->afu_list_lock);
> +
>   err:
>   	/* All the bits that happen in both error_detected and cxl_remove
>   	 * should be idempotent, so we don't need to worry about leaving a mix
> @@ -2051,10 +2066,11 @@ static void cxl_pci_resume(struct pci_dev *pdev)
>   	 * This is not the place to be checking if everything came back up
>   	 * properly, because there's no return value: do that in slot_reset.
>   	 */
> +	spin_lock(&adapter->afu_list_lock);
>   	for (i = 0; i < adapter->slices; i++) {
>   		afu = adapter->afu[i];
>   
> -		if (afu->phb == NULL)
> +		if (afu == NULL || afu->phb == NULL)
>   			continue;
>   
>   		list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
> @@ -2063,6 +2079,7 @@ static void cxl_pci_resume(struct pci_dev *pdev)
>   				afu_dev->driver->err_handler->resume(afu_dev);
>   		}
>   	}
> +	spin_unlock(&adapter->afu_list_lock);
>   }
>   
>   static const struct pci_error_handlers cxl_err_handler = {
> 


^ permalink raw reply

* Re: use generic DMA mapping code in powerpc V4
From: Christoph Hellwig @ 2019-01-28  7:04 UTC (permalink / raw)
  To: Christian Zigotzky
  Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
	linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev,
	Christoph Hellwig
In-Reply-To: <1dec2fbe-f654-dac7-392a-93a5d20e3602@xenosoft.de>

On Sun, Jan 27, 2019 at 02:13:09PM +0100, Christian Zigotzky wrote:
> Christoph,
>
> What shall I do next?

I'll need to figure out what went wrong with the new zone selection
on powerpc and give you another branch to test.

^ permalink raw reply

* Re: [PATCH 18/19] KVM: PPC: Book3S HV: add passthrough support
From: Paul Mackerras @ 2019-01-28  6:13 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <75762dbe-0f08-5b06-e376-744ff87ff4cb@kaod.org>

On Wed, Jan 23, 2019 at 12:07:19PM +0100, Cédric Le Goater wrote:
> On 1/23/19 11:30 AM, Paul Mackerras wrote:
> > On Wed, Jan 23, 2019 at 05:45:24PM +1100, Benjamin Herrenschmidt wrote:
> >> On Tue, 2019-01-22 at 16:26 +1100, Paul Mackerras wrote:
> >>> On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
> >>>> Clear the ESB pages from the VMA of the IRQ being pass through to the
> >>>> guest and let the fault handler repopulate the VMA when the ESB pages
> >>>> are accessed for an EOI or for a trigger.
> >>>
> >>> Why do we want to do this?
> >>>
> >>> I don't see any possible advantage to removing the PTEs from the
> >>> userspace mapping.  You'll need to explain further.
> >>
> >> Afaik bcs we change the mapping to point to the real HW irq ESB page
> >> instead of the "IPI" that was there at VM init time.
> 
> yes exactly. You need to clean up the pages each time.
>  
> > So that makes it sound like there is a whole lot going on that hasn't
> > even been hinted at in the patch descriptions...  It sounds like we
> > need a good description of how all this works and fits together
> > somewhere under Documentation/.
> 
> OK. I have started doing so for the models merged in QEMU but not yet 
> for KVM. I will work on it.
> 
> > In any case we need much more informative patch descriptions.  I
> > realize that it's all currently in Cedric's head, but I bet that in
> > two or three years' time when we come to try to debug something, it
> > won't be in anyone's head...
> 
> I agree. 
> 
> 
> So, storing the ESB VMA under the KVM device is not shocking anyone ?  

Actually, now that I think of it, why can't userspace (QEMU) manage
this using mmap()?  Based on what Ben has said, I assume there would
be a pair of pages for each interrupt that a PCI pass-through device
has.  Would we end up with too many VMAs if we just used mmap() to
change the mappings from the software-generated pages to the
hardware-generated interrupt pages?  Are the necessary pages for a PCI
passthrough device contiguous in both host real space and guest real
space?  If so we'd only need one mmap() for all the device's interrupt
pages.

Paul.

^ permalink raw reply

* Re: [PATCH 00/19] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: Paul Mackerras @ 2019-01-28  5:51 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <2f9b4420-ef90-20b8-d31b-dc547a6aa6b4@kaod.org>

On Wed, Jan 23, 2019 at 08:07:33PM +0100, Cédric Le Goater wrote:
> On 1/22/19 5:46 AM, Paul Mackerras wrote:
> > On Mon, Jan 07, 2019 at 07:43:12PM +0100, Cédric Le Goater wrote:
> >> Hello,
> >>
> >> On the POWER9 processor, the XIVE interrupt controller can control
> >> interrupt sources using MMIO to trigger events, to EOI or to turn off
> >> the sources. Priority management and interrupt acknowledgment is also
> >> controlled by MMIO in the CPU presenter subengine.
> >>
> >> PowerNV/baremetal Linux runs natively under XIVE but sPAPR guests need
> >> special support from the hypervisor to do the same. This is called the
> >> XIVE native exploitation mode and today, it can be activated under the
> >> PowerPC Hypervisor, pHyp. However, Linux/KVM lacks XIVE native support
> >> and still offers the old interrupt mode interface using a
> >> XICS-over-XIVE glue which implements the XICS hcalls.
> >>
> >> The following series is proposal to add the same support under KVM.
> >>
> >> A new KVM device is introduced for the XIVE native exploitation
> >> mode. It reuses most of the XICS-over-XIVE glue implementation
> >> structures which are internal to KVM but has a completely different
> >> interface. A set of Hypervisor calls configures the sources and the
> >> event queues and from there, all control is done by the guest through
> >> MMIOs.
> >>
> >> These MMIO regions (ESB and TIMA) are exposed to guests in QEMU,
> >> similarly to VFIO, and the associated VMAs are populated dynamically
> >> with the appropriate pages using a fault handler. This is implemented
> >> with a couple of KVM device ioctls.
> >>
> >> On a POWER9 sPAPR machine, the Client Architecture Support (CAS)
> >> negotiation process determines whether the guest operates with a
> >> interrupt controller using the XICS legacy model, as found on POWER8,
> >> or in XIVE exploitation mode. Which means that the KVM interrupt
> >> device should be created at runtime, after the machine as started.
> >> This requires extra KVM support to create/destroy KVM devices. The
> >> last patches are an attempt to solve that problem.
> >>
> >> Migration has its own specific needs. The patchset provides the
> >> necessary routines to quiesce XIVE, to capture and restore the state
> >> of the different structures used by KVM, OPAL and HW. Extra OPAL
> >> support is required for these.
> > 
> > Thanks for the patchset.  It mostly looks good, but there are some
> > more things we need to consider, and I think a v2 will be needed.
> >> One general comment I have is that there are a lot of acronyms in this
> > code and you mostly seem to assume that people will know what they all
> > mean.  It would make the code more readable if you provide the
> > expansion of the acronym on first use in a comment or whatever.  For
> > example, one of the patches in this series talks about the "EAS"
> 
>  Event Assignment Structure, a.k.a IVE (Interrupt Virtualization Entry)
> 
> All the names changed somewhere between XIVE v1 and XIVE v2. OPAL and
> Linux should be adjusted ...
> 
> > without ever expanding it in any comment or in the patch description,
> > and I have forgotten just at the moment what EAS stands for (I just
> > know that understanding the XIVE is not eas-y. :)
> ah ! yes. But we have great documentation :)
> 
> We pushed some high level description of XIVE in QEMU :
> 
>   https://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/ppc/xive.h;h=ec23253ba448e25c621356b55a7777119a738f8e;hb=HEAD
> 
> I should do the same for Linux with a KVM section to explain the 
> interfaces which do not directly expose the underlying XIVE concepts. 
> It's better to understand a little what is happening under the hood.
> 
> > Another general comment is that you seem to have written all this
> > code assuming we are using HV KVM in a host running bare-metal.
> 
> Yes. I didn't look at the other configurations. I thought that we could
> use the kernel_irqchip=off option to begin with. A couple of checks
> are indeed missing.

Using kernel_irqchip=off would mean that we would not be able to use
the in-kernel XICS emulation, which would have a performance impact.

We need an explicit capability for XIVE exploitation that can be
enabled or disabled on the qemu command line, so that we can enforce a
uniform set of capabilities across all the hosts in a migration
domain.  And it's no good to say we have the capability when all
attempts to use it will fail.  Therefore the kernel needs to say that
it doesn't have the capability in a PR KVM guest or in a nested HV
guest.

> > However, we could be using PR KVM (either in a bare-metal host or in a
> > guest), or we could be doing nested HV KVM where we are using the
> > kvm_hv module inside a KVM guest and using special hypercalls for
> > controlling our guests.
> 
> Yes. 
> 
> It would be good to talk a little about the nested support (offline 
> may be) to make sure that we are not missing some major interface that 
> would require a lot of change. If we need to prepare ground, I think
> the timing is good.
> 
> The size of the IRQ number space might be a problem. It seems we 
> would need to increase it considerably to support multiple nested 
> guests. That said I haven't look much how nested is designed.  

The current design of nested HV is that the entire non-volatile state
of all the nested guests is encapsulated within the state and
resources of the L1 hypervisor.  That means that if the L1 hypervisor
gets migrated, all of its guests go across inside it and there is no
extra state that L0 needs to be aware of.  That would imply that the
VP number space for the nested guests would need to come from within
the VP number space for L1; but the amount of VP space we allocate to
each guest doesn't seem to be large enough for that to be practical.

> > It would be perfectly acceptable for now to say that we don't yet
> > support XIVE exploitation in those scenarios, as long as we then make
> > sure that the new KVM capability reports false in those scenarios, and
> > any attempt to use the XIVE exploitation interfaces fails cleanly.
> 
> ok. That looks the best approach for now.
> 
> > I don't see that either of those is true in the patch set as it
> > stands, so that is one area that needs to be fixed.
> > 
> > A third general comment is that the new KVM interfaces you have added
> > need to be documented in the files under Documentation/virtual/kvm.
> 
> ok. 
> 
> Thanks,
> 
> C. 
> 

Paul.

^ permalink raw reply

* Re: [PATCH 18/19] KVM: PPC: Book3S HV: add passthrough support
From: Paul Mackerras @ 2019-01-28  4:43 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: kvm, kvm-ppc, Cédric Le Goater, linuxppc-dev, David Gibson
In-Reply-To: <bcb5d487547503ceaf0aefad4a63889a52ea0a02.camel@kernel.crashing.org>

On Thu, Jan 24, 2019 at 08:25:15AM +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2019-01-23 at 21:30 +1100, Paul Mackerras wrote:
> > > Afaik bcs we change the mapping to point to the real HW irq ESB page
> > > instead of the "IPI" that was there at VM init time.
> > 
> > So that makes it sound like there is a whole lot going on that hasn't
> > even been hinted at in the patch descriptions...  It sounds like we
> > need a good description of how all this works and fits together
> > somewhere under Documentation/.
> > 
> > In any case we need much more informative patch descriptions.  I
> > realize that it's all currently in Cedric's head, but I bet that in
> > two or three years' time when we come to try to debug something, it
> > won't be in anyone's head...
> 
> The main problem is understanding XIVE itself. It's not realistic to
> ask Cedric to write a proper documentation for XIVE as part of the
> patch series, but sadly IBM doesn't have a good one to provide either.

There are: (a) the XIVE hardware, (b) the definition of the XIVE
hypercalls that guests use, and (c) the design decisions around how to
implement that hypercall interface.  We need to get (b) published
somehow, but it is mostly (c) that I would expect the patch
descriptions to explain.

It sounds like there will be a mapping to userspace where the pages
can sometimes point to an IPI page and sometimes point to a real HW
irq ESB page.  That is, the same guest "hardware" irq number sometimes
refers to a software-generated interrupt (what you called an "IPI"
above) and sometimes to a hardware-generated interrupt.  That fact,
the reason why it is so and the consequences all need to be explained
somewhere.  They are really not obvious and I don't believe they are
part of either the XIVE hardware spec or the XIVE hypercall spec.

Paul.

^ permalink raw reply

* Re: [RFC/WIP] powerpc: Fix 32-bit handling of MSR_EE on exceptions
From: christophe leroy @ 2019-01-27 19:26 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev@lists.ozlabs.org
  Cc: Diana Craciun, Scott Wood, Nick Piggin, Laurentiu Tudor
In-Reply-To: <fe3a79e078c644d9c1379ef5f8c431d787b6d0c8.camel@kernel.crashing.org>



Le 20/12/2018 à 06:40, Benjamin Herrenschmidt a écrit :
> Hi folks !
> 
> Why trying to figure out why we had occasionally lockdep barf about
> interrupt state on ppc32 (440 in my case but I could reproduce on e500
> as well using qemu), I realized that we are still doing something
> rather gothic and wrong on 32-bit which we stopped doing on 64-bit
> a while ago.
> 
> We have that thing where some handlers "copy" the EE value from the
> original stack frame into the new MSR before transferring to the
> handler.
> 
> Thus for a number of exceptions, we enter the handlers with interrupts
> enabled.
> 
> This is rather fishy, some of the stuff that handlers might do early
> on such as irq_enter/exit or user_exit, context tracking, etc... should
> be run with interrupts off afaik.
> 
> Generally our handlers know when to re-enable interrupts if needed
> (though some of the FSL specific SPE ones don't).
> 
> The problem we were having is that we assumed these interrupts would
> return with interrupts enabled. However that isn't the case.
> 
> Instead, this changes things so that we always enter exception handlers
> with interrupts *off* with the notable exception of syscalls which are
> special (and get a fast path).
> 
> Currently, the patch only changes BookE (440 and E5xx tested in qemu),
> the same recipe needs to be applied to 6xx, 8xx and 40x.

In the scope of the implementation of vmapped stacks, I'm preparing 
serie to refactor the EXCEPTION_PROLOG on the 40x/8xx/6xx. I plan to 
send it out this week.

Christophe

> 
> Also I'm not sure whether we need to create a stack frame around some
> of the calls to trace_hardirqs_* in asm. ppc64 does it, due to problems
> with the irqsoff tracer, but I haven't managed to reproduce those
> issues. We need to look into it a bit more.
> 
> I'll work more on this in the next few days, comments appreciated.
> 
> Not-signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> ---
>   arch/powerpc/kernel/entry_32.S       | 113 ++++++++++++++++++++++-------------
>   arch/powerpc/kernel/head_44x.S       |   9 +--
>   arch/powerpc/kernel/head_booke.h     |  34 ++++-------
>   arch/powerpc/kernel/head_fsl_booke.S |  28 ++++-----
>   arch/powerpc/kernel/traps.c          |   8 +++
>   5 files changed, 111 insertions(+), 81 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 3841d74..39b4cb5 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -34,6 +34,9 @@
>   #include <asm/ftrace.h>
>   #include <asm/ptrace.h>
>   #include <asm/export.h>
> +#include <asm/feature-fixups.h>
> +#include <asm/barrier.h>
> +#include <asm/bug.h>
>   
>   /*
>    * MSR_KERNEL is > 0x10000 on 4xx/Book-E since it include MSR_CE.
> @@ -205,20 +208,46 @@ transfer_to_handler_cont:
>   	mflr	r9
>   	lwz	r11,0(r9)		/* virtual address of handler */
>   	lwz	r9,4(r9)		/* where to go when done */
> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PERF_EVENTS)
> +	mtspr	SPRN_NRI, r0
> +#endif
> +
>   #ifdef CONFIG_TRACE_IRQFLAGS
> +	/*
> +	 * When tracing IRQ state (lockdep) we enable the MMU before we call
> +	 * the IRQ tracing functions as they might access vmalloc space or
> +	 * perform IOs for console output.
> +	 *
> +	 * To speed up the syscall path where interrupts stay on, let's check
> +	 * first if we are changing the MSR value at all.
> +	 */
> +	lwz	r12,_MSR(r1)
> +	xor	r0,r10,r12
> +	andi.	r0,r0,MSR_EE
> +	bne	1f
> +
> +	/* MSR isn't changing, just transition directly */
> +	lwz	r0,GPR0(r1)
> +	mtspr	SPRN_SRR0,r11
> +	mtspr	SPRN_SRR1,r10
> +	mtlr	r9
> +	SYNC
> +	RFI
> +
> +1:	/* MSR is changing, re-enable MMU so we can notify lockdep. We need to
> +	 * keep interrupts disabled at this point otherwise we might risk
> +	 * taking an interrupt before we tell lockdep they are enabled.
> +	 */
>   	lis	r12,reenable_mmu@h
>   	ori	r12,r12,reenable_mmu@l
> +	lis	r0,MSR_KERNEL@h
> +	ori	r0,r0,MSR_KERNEL@l
>   	mtspr	SPRN_SRR0,r12
> -	mtspr	SPRN_SRR1,r10
> +	mtspr	SPRN_SRR1,r0
>   	SYNC
>   	RFI
> -reenable_mmu:				/* re-enable mmu so we can */
> -	mfmsr	r10
> -	lwz	r12,_MSR(r1)
> -	xor	r10,r10,r12
> -	andi.	r10,r10,MSR_EE		/* Did EE change? */
> -	beq	1f
>   
> +reenable_mmu:
>   	/*
>   	 * The trace_hardirqs_off will use CALLER_ADDR0 and CALLER_ADDR1.
>   	 * If from user mode there is only one stack frame on the stack, and
> @@ -239,8 +268,29 @@ reenable_mmu:				/* re-enable mmu so we can */
>   	stw	r3,16(r1)
>   	stw	r4,20(r1)
>   	stw	r5,24(r1)
> -	bl	trace_hardirqs_off
> -	lwz	r5,24(r1)
> +
> +	/* Are we enabling or disabling interrupts ? */
> +	andi.	r0,r10,MSR_EE
> +	beq	1f
> +
> +	/* If we are enabling interrupt, this is a syscall. They shouldn't
> +	 * happen while interrupts are disabled, so let's do a warning here.
> +	 */
> +0:	trap
> +	EMIT_BUG_ENTRY 0b,__FILE__,__LINE__, BUGFLAG_WARNING
> +	bl	trace_hardirqs_on
> +
> +	/* Now enable for real */
> +	mfmsr	r10
> +	ori	r10,r10,MSR_EE
> +	mtmsr	r10
> +	b	2f
> +
> +	/* If we are disabling interrupts (normal case), simply log it with
> +	 * lockdep
> +	 */
> +1:	bl	trace_hardirqs_off
> +2:	lwz	r5,24(r1)
>   	lwz	r4,20(r1)
>   	lwz	r3,16(r1)
>   	lwz	r11,12(r1)
> @@ -250,7 +300,7 @@ reenable_mmu:				/* re-enable mmu so we can */
>   	lwz	r6,GPR6(r1)
>   	lwz	r7,GPR7(r1)
>   	lwz	r8,GPR8(r1)
> -1:	mtctr	r11
> +	mtctr	r11
>   	mtlr	r9
>   	bctr				/* jump to handler */
>   #else /* CONFIG_TRACE_IRQFLAGS */
> @@ -312,31 +362,19 @@ _GLOBAL(DoSyscall)
>   	lwz	r11,_CCR(r1)	/* Clear SO bit in CR */
>   	rlwinm	r11,r11,0,4,2
>   	stw	r11,_CCR(r1)
> +
>   #ifdef CONFIG_TRACE_IRQFLAGS
> -	/* Return from syscalls can (and generally will) hard enable
> -	 * interrupts. You aren't supposed to call a syscall with
> -	 * interrupts disabled in the first place. However, to ensure
> -	 * that we get it right vs. lockdep if it happens, we force
> -	 * that hard enable here with appropriate tracing if we see
> -	 * that we have been called with interrupts off
> -	 */
> +	/* Make sure interrupts are enabled */
>   	mfmsr	r11
>   	andi.	r12,r11,MSR_EE
>   	bne+	1f
> -	/* We came in with interrupts disabled, we enable them now */
> -	bl	trace_hardirqs_on
> -	mfmsr	r11
> -	lwz	r0,GPR0(r1)
> -	lwz	r3,GPR3(r1)
> -	lwz	r4,GPR4(r1)
> -	ori	r11,r11,MSR_EE
> -	lwz	r5,GPR5(r1)
> -	lwz	r6,GPR6(r1)
> -	lwz	r7,GPR7(r1)
> -	lwz	r8,GPR8(r1)
> -	mtmsr	r11
> +	/* We came in with interrupts disabled, we WARN and mark them enabled
> +	 * for lockdep now */
> +0:	trap
> +	EMIT_BUG_ENTRY 0b,__FILE__,__LINE__, BUGFLAG_WARNING
>   1:
>   #endif /* CONFIG_TRACE_IRQFLAGS */
> +
>   	CURRENT_THREAD_INFO(r10, r1)
>   	lwz	r11,TI_FLAGS(r10)
>   	andi.	r11,r11,_TIF_SYSCALL_DOTRACE
> @@ -375,8 +413,7 @@ syscall_exit_cont:
>   	lwz	r8,_MSR(r1)
>   #ifdef CONFIG_TRACE_IRQFLAGS
>   	/* If we are going to return from the syscall with interrupts
> -	 * off, we trace that here. It shouldn't happen though but we
> -	 * want to catch the bugger if it does right ?
> +	 * off, we trace that here. It shouldn't normally happen.
>   	 */
>   	andi.	r10,r8,MSR_EE
>   	bne+	1f
> @@ -881,13 +918,6 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_47x)
>   	 * off in this assembly code while peeking at TI_FLAGS() and such. However
>   	 * we need to inform it if the exception turned interrupts off, and we
>   	 * are about to trun them back on.
> -	 *
> -	 * The problem here sadly is that we don't know whether the exceptions was
> -	 * one that turned interrupts off or not. So we always tell lockdep about
> -	 * turning them on here when we go back to wherever we came from with EE
> -	 * on, even if that may meen some redudant calls being tracked. Maybe later
> -	 * we could encode what the exception did somewhere or test the exception
> -	 * type in the pt_regs but that sounds overkill
>   	 */
>   	andi.	r10,r9,MSR_EE
>   	beq	1f
> @@ -1175,9 +1205,10 @@ do_work:			/* r10 contains MSR_KERNEL here */
>   	beq	do_user_signal
>   
>   do_resched:			/* r10 contains MSR_KERNEL here */
> -	/* Note: We don't need to inform lockdep that we are enabling
> -	 * interrupts here. As far as it knows, they are already enabled
> -	 */
> +#ifdef CONFIG_TRACE_IRQFLAGS
> +	bl	trace_hardirqs_on
> +	mfmsr	r10
> +#endif
>   	ori	r10,r10,MSR_EE
>   	SYNC
>   	MTMSRD(r10)		/* hard-enable interrupts */
> diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
> index 37e4a7c..891a048 100644
> --- a/arch/powerpc/kernel/head_44x.S
> +++ b/arch/powerpc/kernel/head_44x.S
> @@ -40,6 +40,7 @@
>   #include <asm/ptrace.h>
>   #include <asm/synch.h>
>   #include <asm/export.h>
> +
>   #include "head_booke.h"
>   
>   
> @@ -277,16 +278,16 @@ interrupt_base:
>   	FP_UNAVAILABLE_EXCEPTION
>   #else
>   	EXCEPTION(0x2010, BOOKE_INTERRUPT_FP_UNAVAIL, \
> -		  FloatingPointUnavailable, unknown_exception, EXC_XFER_EE)
> +		  FloatingPointUnavailable, unknown_exception, EXC_XFER_STD)
>   #endif
>   	/* System Call Interrupt */
>   	START_EXCEPTION(SystemCall)
>   	NORMAL_EXCEPTION_PROLOG(BOOKE_INTERRUPT_SYSCALL)
> -	EXC_XFER_EE_LITE(0x0c00, DoSyscall)
> +	EXC_XFER_SYS(0x0c00, DoSyscall)
>   
>   	/* Auxiliary Processor Unavailable Interrupt */
>   	EXCEPTION(0x2020, BOOKE_INTERRUPT_AP_UNAVAIL, \
> -		  AuxillaryProcessorUnavailable, unknown_exception, EXC_XFER_EE)
> +		  AuxillaryProcessorUnavailable, unknown_exception, EXC_XFER_STD)
>   
>   	/* Decrementer Interrupt */
>   	DECREMENTER_EXCEPTION
> @@ -294,7 +295,7 @@ interrupt_base:
>   	/* Fixed Internal Timer Interrupt */
>   	/* TODO: Add FIT support */
>   	EXCEPTION(0x1010, BOOKE_INTERRUPT_FIT, FixedIntervalTimer, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   
>   	/* Watchdog Timer Interrupt */
>   	/* TODO: Add watchdog support */
> diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
> index a620203..46be533 100644
> --- a/arch/powerpc/kernel/head_booke.h
> +++ b/arch/powerpc/kernel/head_booke.h
> @@ -210,8 +210,7 @@
>   	CRITICAL_EXCEPTION_PROLOG(intno);				\
>   	addi	r3,r1,STACK_FRAME_OVERHEAD;				\
>   	EXC_XFER_TEMPLATE(hdlr, n+2, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \
> -			  NOCOPY, crit_transfer_to_handler, \
> -			  ret_from_crit_exc)
> +			  crit_transfer_to_handler, ret_from_crit_exc)
>   
>   #define MCHECK_EXCEPTION(n, label, hdlr)			\
>   	START_EXCEPTION(label);					\
> @@ -220,36 +219,27 @@
>   	stw	r5,_ESR(r11);					\
>   	addi	r3,r1,STACK_FRAME_OVERHEAD;			\
>   	EXC_XFER_TEMPLATE(hdlr, n+4, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \
> -			  NOCOPY, mcheck_transfer_to_handler,   \
> -			  ret_from_mcheck_exc)
> +			  mcheck_transfer_to_handler, ret_from_mcheck_exc)
>   
> -#define EXC_XFER_TEMPLATE(hdlr, trap, msr, copyee, tfer, ret)	\
> +#define EXC_XFER_TEMPLATE(hdlr, trap, msr, tfer, ret)	\
>   	li	r10,trap;					\
>   	stw	r10,_TRAP(r11);					\
>   	lis	r10,msr@h;					\
>   	ori	r10,r10,msr@l;					\
> -	copyee(r10, r9);					\
>   	bl	tfer;		 				\
>   	.long	hdlr;						\
>   	.long	ret
>   
> -#define COPY_EE(d, s)		rlwimi d,s,0,16,16
> -#define NOCOPY(d, s)
> -
>   #define EXC_XFER_STD(n, hdlr)		\
> -	EXC_XFER_TEMPLATE(hdlr, n, MSR_KERNEL, NOCOPY, transfer_to_handler_full, \
> +	EXC_XFER_TEMPLATE(hdlr, n, MSR_KERNEL, transfer_to_handler_full, \
>   			  ret_from_except_full)
>   
> -#define EXC_XFER_LITE(n, hdlr)		\
> -	EXC_XFER_TEMPLATE(hdlr, n+1, MSR_KERNEL, NOCOPY, transfer_to_handler, \
> +#define EXC_XFER_SYS(n, hdlr)						\
> +	EXC_XFER_TEMPLATE(hdlr, n+1, MSR_KERNEL | MSR_EE, transfer_to_handler, \
>   			  ret_from_except)
>   
> -#define EXC_XFER_EE(n, hdlr)		\
> -	EXC_XFER_TEMPLATE(hdlr, n, MSR_KERNEL, COPY_EE, transfer_to_handler_full, \
> -			  ret_from_except_full)
> -
> -#define EXC_XFER_EE_LITE(n, hdlr)	\
> -	EXC_XFER_TEMPLATE(hdlr, n+1, MSR_KERNEL, COPY_EE, transfer_to_handler, \
> +#define EXC_XFER_LITE(n, hdlr)		\
> +	EXC_XFER_TEMPLATE(hdlr, n+1, MSR_KERNEL, transfer_to_handler, \
>   			  ret_from_except)
>   
>   /* Check for a single step debug exception while in an exception
> @@ -316,7 +306,7 @@
>   	/* continue normal handling for a debug exception... */		      \
>   2:	mfspr	r4,SPRN_DBSR;						      \
>   	addi	r3,r1,STACK_FRAME_OVERHEAD;				      \
> -	EXC_XFER_TEMPLATE(DebugException, 0x2008, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), NOCOPY, debug_transfer_to_handler, ret_from_debug_exc)
> +	EXC_XFER_TEMPLATE(DebugException, 0x2008, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), debug_transfer_to_handler, ret_from_debug_exc)
>   
>   #define DEBUG_CRIT_EXCEPTION						      \
>   	START_EXCEPTION(DebugCrit);					      \
> @@ -369,7 +359,7 @@
>   	/* continue normal handling for a critical exception... */	      \
>   2:	mfspr	r4,SPRN_DBSR;						      \
>   	addi	r3,r1,STACK_FRAME_OVERHEAD;				      \
> -	EXC_XFER_TEMPLATE(DebugException, 0x2002, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), NOCOPY, crit_transfer_to_handler, ret_from_crit_exc)
> +	EXC_XFER_TEMPLATE(DebugException, 0x2002, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), crit_transfer_to_handler, ret_from_crit_exc)
>   
>   #define DATA_STORAGE_EXCEPTION						      \
>   	START_EXCEPTION(DataStorage)					      \
> @@ -394,7 +384,7 @@
>   	mfspr   r4,SPRN_DEAR;           /* Grab the DEAR and save it */	      \
>   	stw     r4,_DEAR(r11);						      \
>   	addi    r3,r1,STACK_FRAME_OVERHEAD;				      \
> -	EXC_XFER_EE(0x0600, alignment_exception)
> +	EXC_XFER_STD(0x0600, alignment_exception)
>   
>   #define PROGRAM_EXCEPTION						      \
>   	START_EXCEPTION(Program)					      \
> @@ -419,7 +409,7 @@
>   	bl	load_up_fpu;		/* if from user, just load it up */   \
>   	b	fast_exception_return;					      \
>   1:	addi	r3,r1,STACK_FRAME_OVERHEAD;				      \
> -	EXC_XFER_EE_LITE(0x800, kernel_fp_unavailable_exception)
> +	EXC_XFER_STD(0x800, kernel_fp_unavailable_exception)
>   
>   #ifndef __ASSEMBLY__
>   struct exception_regs {
> diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> index bf4c602..556cffb 100644
> --- a/arch/powerpc/kernel/head_fsl_booke.S
> +++ b/arch/powerpc/kernel/head_fsl_booke.S
> @@ -385,7 +385,7 @@ interrupt_base:
>   	EXC_XFER_LITE(0x0300, handle_page_fault)
>   1:
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
> -	EXC_XFER_EE_LITE(0x0300, CacheLockingException)
> +	EXC_XFER_LITE(0x0300, CacheLockingException)
>   
>   	/* Instruction Storage Interrupt */
>   	INSTRUCTION_STORAGE_EXCEPTION
> @@ -406,21 +406,21 @@ interrupt_base:
>   #ifdef CONFIG_E200
>   	/* E200 treats 'normal' floating point instructions as FP Unavail exception */
>   	EXCEPTION(0x0800, FP_UNAVAIL, FloatingPointUnavailable, \
> -		  program_check_exception, EXC_XFER_EE)
> +		  program_check_exception, EXC_XFER_STD)
>   #else
>   	EXCEPTION(0x0800, FP_UNAVAIL, FloatingPointUnavailable, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   #endif
>   #endif
>   
>   	/* System Call Interrupt */
>   	START_EXCEPTION(SystemCall)
>   	NORMAL_EXCEPTION_PROLOG(SYSCALL)
> -	EXC_XFER_EE_LITE(0x0c00, DoSyscall)
> +	EXC_XFER_SYS(0x0c00, DoSyscall)
>   
>   	/* Auxiliary Processor Unavailable Interrupt */
>   	EXCEPTION(0x2900, AP_UNAVAIL, AuxillaryProcessorUnavailable, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   
>   	/* Decrementer Interrupt */
>   	DECREMENTER_EXCEPTION
> @@ -428,7 +428,7 @@ interrupt_base:
>   	/* Fixed Internal Timer Interrupt */
>   	/* TODO: Add FIT support */
>   	EXCEPTION(0x3100, FIT, FixedIntervalTimer, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   
>   	/* Watchdog Timer Interrupt */
>   #ifdef CONFIG_BOOKE_WDT
> @@ -623,25 +623,25 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>   	bl	load_up_spe
>   	b	fast_exception_return
>   1:	addi	r3,r1,STACK_FRAME_OVERHEAD
> -	EXC_XFER_EE_LITE(0x2010, KernelSPE)
> +	EXC_XFER_LITE(0x2010, KernelSPE)
>   #elif defined(CONFIG_SPE_POSSIBLE)
>   	EXCEPTION(0x2020, SPE_UNAVAIL, SPEUnavailable, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   #endif /* CONFIG_SPE_POSSIBLE */
>   
>   	/* SPE Floating Point Data */
>   #ifdef CONFIG_SPE
>   	EXCEPTION(0x2030, SPE_FP_DATA, SPEFloatingPointData,
> -		  SPEFloatingPointException, EXC_XFER_EE)
> +		  SPEFloatingPointException, EXC_XFER_STD)
>   
>   	/* SPE Floating Point Round */
>   	EXCEPTION(0x2050, SPE_FP_ROUND, SPEFloatingPointRound, \
> -		  SPEFloatingPointRoundException, EXC_XFER_EE)
> +		  SPEFloatingPointRoundException, EXC_XFER_STD)
>   #elif defined(CONFIG_SPE_POSSIBLE)
>   	EXCEPTION(0x2040, SPE_FP_DATA, SPEFloatingPointData,
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   	EXCEPTION(0x2050, SPE_FP_ROUND, SPEFloatingPointRound, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   #endif /* CONFIG_SPE_POSSIBLE */
>   
>   
> @@ -664,10 +664,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>   			   unknown_exception)
>   
>   	/* Hypercall */
> -	EXCEPTION(0, HV_SYSCALL, Hypercall, unknown_exception, EXC_XFER_EE)
> +	EXCEPTION(0, HV_SYSCALL, Hypercall, unknown_exception, EXC_XFER_STD)
>   
>   	/* Embedded Hypervisor Privilege */
> -	EXCEPTION(0, HV_PRIV, Ehvpriv, unknown_exception, EXC_XFER_EE)
> +	EXCEPTION(0, HV_PRIV, Ehvpriv, unknown_exception, EXC_XFER_STD)
>   
>   interrupt_end:
>   
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 023a462..5ee0b90 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1858,6 +1858,10 @@ void SPEFloatingPointException(struct pt_regs *regs)
>   	int code = 0;
>   	int err;
>   
> +	/* We restore the interrupt state now */
> +	if (!arch_irq_disabled_regs(regs))
> +		local_irq_enable();
> +
>   	flush_spe_to_thread(current);
>   
>   	spefscr = current->thread.spefscr;
> @@ -1903,6 +1907,10 @@ void SPEFloatingPointRoundException(struct pt_regs *regs)
>   	extern int speround_handler(struct pt_regs *regs);
>   	int err;
>   
> +	/* We restore the interrupt state now */
> +	if (!arch_irq_disabled_regs(regs))
> +		local_irq_enable();
> +
>   	preempt_disable();
>   	if (regs->msr & MSR_SPE)
>   		giveup_spe(current);
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus


^ permalink raw reply

* Re: [RFC/WIP] powerpc: Fix 32-bit handling of MSR_EE on exceptions
From: christophe leroy @ 2019-01-27 18:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev@lists.ozlabs.org
  Cc: Diana Craciun, Scott Wood, Nick Piggin, Laurentiu Tudor
In-Reply-To: <fe3a79e078c644d9c1379ef5f8c431d787b6d0c8.camel@kernel.crashing.org>



Le 20/12/2018 à 06:40, Benjamin Herrenschmidt a écrit :
> Hi folks !
> 
> Why trying to figure out why we had occasionally lockdep barf about
> interrupt state on ppc32 (440 in my case but I could reproduce on e500
> as well using qemu), I realized that we are still doing something
> rather gothic and wrong on 32-bit which we stopped doing on 64-bit
> a while ago.
> 
> We have that thing where some handlers "copy" the EE value from the
> original stack frame into the new MSR before transferring to the
> handler.
> 
> Thus for a number of exceptions, we enter the handlers with interrupts
> enabled.
> 
> This is rather fishy, some of the stuff that handlers might do early
> on such as irq_enter/exit or user_exit, context tracking, etc... should
> be run with interrupts off afaik.
> 
> Generally our handlers know when to re-enable interrupts if needed
> (though some of the FSL specific SPE ones don't).
> 
> The problem we were having is that we assumed these interrupts would
> return with interrupts enabled. However that isn't the case.
> 
> Instead, this changes things so that we always enter exception handlers
> with interrupts *off* with the notable exception of syscalls which are
> special (and get a fast path).
> 
> Currently, the patch only changes BookE (440 and E5xx tested in qemu),
> the same recipe needs to be applied to 6xx, 8xx and 40x.
> 
> Also I'm not sure whether we need to create a stack frame around some
> of the calls to trace_hardirqs_* in asm. ppc64 does it, due to problems
> with the irqsoff tracer, but I haven't managed to reproduce those
> issues. We need to look into it a bit more.
> 
> I'll work more on this in the next few days, comments appreciated.
> 
> Not-signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> ---
>   arch/powerpc/kernel/entry_32.S       | 113 ++++++++++++++++++++++-------------
>   arch/powerpc/kernel/head_44x.S       |   9 +--
>   arch/powerpc/kernel/head_booke.h     |  34 ++++-------
>   arch/powerpc/kernel/head_fsl_booke.S |  28 ++++-----
>   arch/powerpc/kernel/traps.c          |   8 +++
>   5 files changed, 111 insertions(+), 81 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 3841d74..39b4cb5 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -34,6 +34,9 @@
>   #include <asm/ftrace.h>
>   #include <asm/ptrace.h>
>   #include <asm/export.h>
> +#include <asm/feature-fixups.h>
> +#include <asm/barrier.h>
> +#include <asm/bug.h>
>   
>   /*
>    * MSR_KERNEL is > 0x10000 on 4xx/Book-E since it include MSR_CE.
> @@ -205,20 +208,46 @@ transfer_to_handler_cont:
>   	mflr	r9
>   	lwz	r11,0(r9)		/* virtual address of handler */
>   	lwz	r9,4(r9)		/* where to go when done */
> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PERF_EVENTS)
> +	mtspr	SPRN_NRI, r0
> +#endif

Was already there before your patch.

> +
>   #ifdef CONFIG_TRACE_IRQFLAGS
> +	/*
> +	 * When tracing IRQ state (lockdep) we enable the MMU before we call
> +	 * the IRQ tracing functions as they might access vmalloc space or
> +	 * perform IOs for console output.
> +	 *
> +	 * To speed up the syscall path where interrupts stay on, let's check
> +	 * first if we are changing the MSR value at all.
> +	 */
> +	lwz	r12,_MSR(r1)
> +	xor	r0,r10,r12
> +	andi.	r0,r0,MSR_EE
> +	bne	1f
> +
> +	/* MSR isn't changing, just transition directly */
> +	lwz	r0,GPR0(r1)
> +	mtspr	SPRN_SRR0,r11
> +	mtspr	SPRN_SRR1,r10
> +	mtlr	r9
> +	SYNC
> +	RFI
> +
> +1:	/* MSR is changing, re-enable MMU so we can notify lockdep. We need to
> +	 * keep interrupts disabled at this point otherwise we might risk
> +	 * taking an interrupt before we tell lockdep they are enabled.
> +	 */
>   	lis	r12,reenable_mmu@h
>   	ori	r12,r12,reenable_mmu@l
> +	lis	r0,MSR_KERNEL@h
> +	ori	r0,r0,MSR_KERNEL@l

You should use LOAD_MSR_KERNEL(), not all targets need an upper part.

>   	mtspr	SPRN_SRR0,r12
> -	mtspr	SPRN_SRR1,r10
> +	mtspr	SPRN_SRR1,r0
>   	SYNC
>   	RFI
> -reenable_mmu:				/* re-enable mmu so we can */
> -	mfmsr	r10
> -	lwz	r12,_MSR(r1)
> -	xor	r10,r10,r12
> -	andi.	r10,r10,MSR_EE		/* Did EE change? */
> -	beq	1f
>   
> +reenable_mmu:
>   	/*
>   	 * The trace_hardirqs_off will use CALLER_ADDR0 and CALLER_ADDR1.
>   	 * If from user mode there is only one stack frame on the stack, and
> @@ -239,8 +268,29 @@ reenable_mmu:				/* re-enable mmu so we can */
>   	stw	r3,16(r1)
>   	stw	r4,20(r1)
>   	stw	r5,24(r1)
> -	bl	trace_hardirqs_off
> -	lwz	r5,24(r1)
> +
> +	/* Are we enabling or disabling interrupts ? */
> +	andi.	r0,r10,MSR_EE
> +	beq	1f

This branch is likely, could we avoid it ?

For instance by moving the below part after the bctr and branching to it 
with a bne- ?

> +
> +	/* If we are enabling interrupt, this is a syscall. They shouldn't
> +	 * happen while interrupts are disabled, so let's do a warning here.
> +	 */
> +0:	trap
> +	EMIT_BUG_ENTRY 0b,__FILE__,__LINE__, BUGFLAG_WARNING
> +	bl	trace_hardirqs_on
> +
> +	/* Now enable for real */
> +	mfmsr	r10
> +	ori	r10,r10,MSR_EE
> +	mtmsr	r10
> +	b	2f
> +
> +	/* If we are disabling interrupts (normal case), simply log it with
> +	 * lockdep
> +	 */
> +1:	bl	trace_hardirqs_off
> +2:	lwz	r5,24(r1)
>   	lwz	r4,20(r1)
>   	lwz	r3,16(r1)
>   	lwz	r11,12(r1)
> @@ -250,7 +300,7 @@ reenable_mmu:				/* re-enable mmu so we can */
>   	lwz	r6,GPR6(r1)
>   	lwz	r7,GPR7(r1)
>   	lwz	r8,GPR8(r1)
> -1:	mtctr	r11
> +	mtctr	r11
>   	mtlr	r9
>   	bctr				/* jump to handler */
>   #else /* CONFIG_TRACE_IRQFLAGS */
> @@ -312,31 +362,19 @@ _GLOBAL(DoSyscall)
>   	lwz	r11,_CCR(r1)	/* Clear SO bit in CR */
>   	rlwinm	r11,r11,0,4,2
>   	stw	r11,_CCR(r1)
> +
>   #ifdef CONFIG_TRACE_IRQFLAGS
> -	/* Return from syscalls can (and generally will) hard enable
> -	 * interrupts. You aren't supposed to call a syscall with
> -	 * interrupts disabled in the first place. However, to ensure
> -	 * that we get it right vs. lockdep if it happens, we force
> -	 * that hard enable here with appropriate tracing if we see
> -	 * that we have been called with interrupts off
> -	 */
> +	/* Make sure interrupts are enabled */
>   	mfmsr	r11
>   	andi.	r12,r11,MSR_EE
>   	bne+	1f

Same here, can we avoid the branch on the general case ? For instance by 
moving the below part after the blrl and doing a beq- to it ?

> -	/* We came in with interrupts disabled, we enable them now */
> -	bl	trace_hardirqs_on
> -	mfmsr	r11
> -	lwz	r0,GPR0(r1)
> -	lwz	r3,GPR3(r1)
> -	lwz	r4,GPR4(r1)
> -	ori	r11,r11,MSR_EE
> -	lwz	r5,GPR5(r1)
> -	lwz	r6,GPR6(r1)
> -	lwz	r7,GPR7(r1)
> -	lwz	r8,GPR8(r1)
> -	mtmsr	r11
> +	/* We came in with interrupts disabled, we WARN and mark them enabled
> +	 * for lockdep now */
> +0:	trap
> +	EMIT_BUG_ENTRY 0b,__FILE__,__LINE__, BUGFLAG_WARNING
>   1:
>   #endif /* CONFIG_TRACE_IRQFLAGS */
> +
>   	CURRENT_THREAD_INFO(r10, r1)
>   	lwz	r11,TI_FLAGS(r10)
>   	andi.	r11,r11,_TIF_SYSCALL_DOTRACE
> @@ -375,8 +413,7 @@ syscall_exit_cont:
>   	lwz	r8,_MSR(r1)
>   #ifdef CONFIG_TRACE_IRQFLAGS
>   	/* If we are going to return from the syscall with interrupts
> -	 * off, we trace that here. It shouldn't happen though but we
> -	 * want to catch the bugger if it does right ?
> +	 * off, we trace that here. It shouldn't normally happen.
>   	 */
>   	andi.	r10,r8,MSR_EE
>   	bne+	1f
> @@ -881,13 +918,6 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_47x)
>   	 * off in this assembly code while peeking at TI_FLAGS() and such. However
>   	 * we need to inform it if the exception turned interrupts off, and we
>   	 * are about to trun them back on.
> -	 *
> -	 * The problem here sadly is that we don't know whether the exceptions was
> -	 * one that turned interrupts off or not. So we always tell lockdep about
> -	 * turning them on here when we go back to wherever we came from with EE
> -	 * on, even if that may meen some redudant calls being tracked. Maybe later
> -	 * we could encode what the exception did somewhere or test the exception
> -	 * type in the pt_regs but that sounds overkill
>   	 */
>   	andi.	r10,r9,MSR_EE
>   	beq	1f
> @@ -1175,9 +1205,10 @@ do_work:			/* r10 contains MSR_KERNEL here */
>   	beq	do_user_signal
>   
>   do_resched:			/* r10 contains MSR_KERNEL here */
> -	/* Note: We don't need to inform lockdep that we are enabling
> -	 * interrupts here. As far as it knows, they are already enabled
> -	 */
> +#ifdef CONFIG_TRACE_IRQFLAGS
> +	bl	trace_hardirqs_on
> +	mfmsr	r10
> +#endif
>   	ori	r10,r10,MSR_EE
>   	SYNC
>   	MTMSRD(r10)		/* hard-enable interrupts */

On the 8xx, the above can be done by writing any value to SPRN_EID in a 
single mtspr instead of the sequence mfmsr/ori/mtmsr. Should we use a 
macro for that ?

Christophe

> diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
> index 37e4a7c..891a048 100644
> --- a/arch/powerpc/kernel/head_44x.S
> +++ b/arch/powerpc/kernel/head_44x.S
> @@ -40,6 +40,7 @@
>   #include <asm/ptrace.h>
>   #include <asm/synch.h>
>   #include <asm/export.h>
> +
>   #include "head_booke.h"
>   
>   
> @@ -277,16 +278,16 @@ interrupt_base:
>   	FP_UNAVAILABLE_EXCEPTION
>   #else
>   	EXCEPTION(0x2010, BOOKE_INTERRUPT_FP_UNAVAIL, \
> -		  FloatingPointUnavailable, unknown_exception, EXC_XFER_EE)
> +		  FloatingPointUnavailable, unknown_exception, EXC_XFER_STD)
>   #endif
>   	/* System Call Interrupt */
>   	START_EXCEPTION(SystemCall)
>   	NORMAL_EXCEPTION_PROLOG(BOOKE_INTERRUPT_SYSCALL)
> -	EXC_XFER_EE_LITE(0x0c00, DoSyscall)
> +	EXC_XFER_SYS(0x0c00, DoSyscall)
>   
>   	/* Auxiliary Processor Unavailable Interrupt */
>   	EXCEPTION(0x2020, BOOKE_INTERRUPT_AP_UNAVAIL, \
> -		  AuxillaryProcessorUnavailable, unknown_exception, EXC_XFER_EE)
> +		  AuxillaryProcessorUnavailable, unknown_exception, EXC_XFER_STD)
>   
>   	/* Decrementer Interrupt */
>   	DECREMENTER_EXCEPTION
> @@ -294,7 +295,7 @@ interrupt_base:
>   	/* Fixed Internal Timer Interrupt */
>   	/* TODO: Add FIT support */
>   	EXCEPTION(0x1010, BOOKE_INTERRUPT_FIT, FixedIntervalTimer, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   
>   	/* Watchdog Timer Interrupt */
>   	/* TODO: Add watchdog support */
> diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
> index a620203..46be533 100644
> --- a/arch/powerpc/kernel/head_booke.h
> +++ b/arch/powerpc/kernel/head_booke.h
> @@ -210,8 +210,7 @@
>   	CRITICAL_EXCEPTION_PROLOG(intno);				\
>   	addi	r3,r1,STACK_FRAME_OVERHEAD;				\
>   	EXC_XFER_TEMPLATE(hdlr, n+2, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \
> -			  NOCOPY, crit_transfer_to_handler, \
> -			  ret_from_crit_exc)
> +			  crit_transfer_to_handler, ret_from_crit_exc)
>   
>   #define MCHECK_EXCEPTION(n, label, hdlr)			\
>   	START_EXCEPTION(label);					\
> @@ -220,36 +219,27 @@
>   	stw	r5,_ESR(r11);					\
>   	addi	r3,r1,STACK_FRAME_OVERHEAD;			\
>   	EXC_XFER_TEMPLATE(hdlr, n+4, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), \
> -			  NOCOPY, mcheck_transfer_to_handler,   \
> -			  ret_from_mcheck_exc)
> +			  mcheck_transfer_to_handler, ret_from_mcheck_exc)
>   
> -#define EXC_XFER_TEMPLATE(hdlr, trap, msr, copyee, tfer, ret)	\
> +#define EXC_XFER_TEMPLATE(hdlr, trap, msr, tfer, ret)	\
>   	li	r10,trap;					\
>   	stw	r10,_TRAP(r11);					\
>   	lis	r10,msr@h;					\
>   	ori	r10,r10,msr@l;					\
> -	copyee(r10, r9);					\
>   	bl	tfer;		 				\
>   	.long	hdlr;						\
>   	.long	ret
>   
> -#define COPY_EE(d, s)		rlwimi d,s,0,16,16
> -#define NOCOPY(d, s)
> -
>   #define EXC_XFER_STD(n, hdlr)		\
> -	EXC_XFER_TEMPLATE(hdlr, n, MSR_KERNEL, NOCOPY, transfer_to_handler_full, \
> +	EXC_XFER_TEMPLATE(hdlr, n, MSR_KERNEL, transfer_to_handler_full, \
>   			  ret_from_except_full)
>   
> -#define EXC_XFER_LITE(n, hdlr)		\
> -	EXC_XFER_TEMPLATE(hdlr, n+1, MSR_KERNEL, NOCOPY, transfer_to_handler, \
> +#define EXC_XFER_SYS(n, hdlr)						\
> +	EXC_XFER_TEMPLATE(hdlr, n+1, MSR_KERNEL | MSR_EE, transfer_to_handler, \
>   			  ret_from_except)
>   
> -#define EXC_XFER_EE(n, hdlr)		\
> -	EXC_XFER_TEMPLATE(hdlr, n, MSR_KERNEL, COPY_EE, transfer_to_handler_full, \
> -			  ret_from_except_full)
> -
> -#define EXC_XFER_EE_LITE(n, hdlr)	\
> -	EXC_XFER_TEMPLATE(hdlr, n+1, MSR_KERNEL, COPY_EE, transfer_to_handler, \
> +#define EXC_XFER_LITE(n, hdlr)		\
> +	EXC_XFER_TEMPLATE(hdlr, n+1, MSR_KERNEL, transfer_to_handler, \
>   			  ret_from_except)
>   
>   /* Check for a single step debug exception while in an exception
> @@ -316,7 +306,7 @@
>   	/* continue normal handling for a debug exception... */		      \
>   2:	mfspr	r4,SPRN_DBSR;						      \
>   	addi	r3,r1,STACK_FRAME_OVERHEAD;				      \
> -	EXC_XFER_TEMPLATE(DebugException, 0x2008, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), NOCOPY, debug_transfer_to_handler, ret_from_debug_exc)
> +	EXC_XFER_TEMPLATE(DebugException, 0x2008, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), debug_transfer_to_handler, ret_from_debug_exc)
>   
>   #define DEBUG_CRIT_EXCEPTION						      \
>   	START_EXCEPTION(DebugCrit);					      \
> @@ -369,7 +359,7 @@
>   	/* continue normal handling for a critical exception... */	      \
>   2:	mfspr	r4,SPRN_DBSR;						      \
>   	addi	r3,r1,STACK_FRAME_OVERHEAD;				      \
> -	EXC_XFER_TEMPLATE(DebugException, 0x2002, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), NOCOPY, crit_transfer_to_handler, ret_from_crit_exc)
> +	EXC_XFER_TEMPLATE(DebugException, 0x2002, (MSR_KERNEL & ~(MSR_ME|MSR_DE|MSR_CE)), crit_transfer_to_handler, ret_from_crit_exc)
>   
>   #define DATA_STORAGE_EXCEPTION						      \
>   	START_EXCEPTION(DataStorage)					      \
> @@ -394,7 +384,7 @@
>   	mfspr   r4,SPRN_DEAR;           /* Grab the DEAR and save it */	      \
>   	stw     r4,_DEAR(r11);						      \
>   	addi    r3,r1,STACK_FRAME_OVERHEAD;				      \
> -	EXC_XFER_EE(0x0600, alignment_exception)
> +	EXC_XFER_STD(0x0600, alignment_exception)
>   
>   #define PROGRAM_EXCEPTION						      \
>   	START_EXCEPTION(Program)					      \
> @@ -419,7 +409,7 @@
>   	bl	load_up_fpu;		/* if from user, just load it up */   \
>   	b	fast_exception_return;					      \
>   1:	addi	r3,r1,STACK_FRAME_OVERHEAD;				      \
> -	EXC_XFER_EE_LITE(0x800, kernel_fp_unavailable_exception)
> +	EXC_XFER_STD(0x800, kernel_fp_unavailable_exception)
>   
>   #ifndef __ASSEMBLY__
>   struct exception_regs {
> diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> index bf4c602..556cffb 100644
> --- a/arch/powerpc/kernel/head_fsl_booke.S
> +++ b/arch/powerpc/kernel/head_fsl_booke.S
> @@ -385,7 +385,7 @@ interrupt_base:
>   	EXC_XFER_LITE(0x0300, handle_page_fault)
>   1:
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
> -	EXC_XFER_EE_LITE(0x0300, CacheLockingException)
> +	EXC_XFER_LITE(0x0300, CacheLockingException)
>   
>   	/* Instruction Storage Interrupt */
>   	INSTRUCTION_STORAGE_EXCEPTION
> @@ -406,21 +406,21 @@ interrupt_base:
>   #ifdef CONFIG_E200
>   	/* E200 treats 'normal' floating point instructions as FP Unavail exception */
>   	EXCEPTION(0x0800, FP_UNAVAIL, FloatingPointUnavailable, \
> -		  program_check_exception, EXC_XFER_EE)
> +		  program_check_exception, EXC_XFER_STD)
>   #else
>   	EXCEPTION(0x0800, FP_UNAVAIL, FloatingPointUnavailable, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   #endif
>   #endif
>   
>   	/* System Call Interrupt */
>   	START_EXCEPTION(SystemCall)
>   	NORMAL_EXCEPTION_PROLOG(SYSCALL)
> -	EXC_XFER_EE_LITE(0x0c00, DoSyscall)
> +	EXC_XFER_SYS(0x0c00, DoSyscall)
>   
>   	/* Auxiliary Processor Unavailable Interrupt */
>   	EXCEPTION(0x2900, AP_UNAVAIL, AuxillaryProcessorUnavailable, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   
>   	/* Decrementer Interrupt */
>   	DECREMENTER_EXCEPTION
> @@ -428,7 +428,7 @@ interrupt_base:
>   	/* Fixed Internal Timer Interrupt */
>   	/* TODO: Add FIT support */
>   	EXCEPTION(0x3100, FIT, FixedIntervalTimer, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   
>   	/* Watchdog Timer Interrupt */
>   #ifdef CONFIG_BOOKE_WDT
> @@ -623,25 +623,25 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>   	bl	load_up_spe
>   	b	fast_exception_return
>   1:	addi	r3,r1,STACK_FRAME_OVERHEAD
> -	EXC_XFER_EE_LITE(0x2010, KernelSPE)
> +	EXC_XFER_LITE(0x2010, KernelSPE)
>   #elif defined(CONFIG_SPE_POSSIBLE)
>   	EXCEPTION(0x2020, SPE_UNAVAIL, SPEUnavailable, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   #endif /* CONFIG_SPE_POSSIBLE */
>   
>   	/* SPE Floating Point Data */
>   #ifdef CONFIG_SPE
>   	EXCEPTION(0x2030, SPE_FP_DATA, SPEFloatingPointData,
> -		  SPEFloatingPointException, EXC_XFER_EE)
> +		  SPEFloatingPointException, EXC_XFER_STD)
>   
>   	/* SPE Floating Point Round */
>   	EXCEPTION(0x2050, SPE_FP_ROUND, SPEFloatingPointRound, \
> -		  SPEFloatingPointRoundException, EXC_XFER_EE)
> +		  SPEFloatingPointRoundException, EXC_XFER_STD)
>   #elif defined(CONFIG_SPE_POSSIBLE)
>   	EXCEPTION(0x2040, SPE_FP_DATA, SPEFloatingPointData,
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   	EXCEPTION(0x2050, SPE_FP_ROUND, SPEFloatingPointRound, \
> -		  unknown_exception, EXC_XFER_EE)
> +		  unknown_exception, EXC_XFER_STD)
>   #endif /* CONFIG_SPE_POSSIBLE */
>   
>   
> @@ -664,10 +664,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_EMB_HV)
>   			   unknown_exception)
>   
>   	/* Hypercall */
> -	EXCEPTION(0, HV_SYSCALL, Hypercall, unknown_exception, EXC_XFER_EE)
> +	EXCEPTION(0, HV_SYSCALL, Hypercall, unknown_exception, EXC_XFER_STD)
>   
>   	/* Embedded Hypervisor Privilege */
> -	EXCEPTION(0, HV_PRIV, Ehvpriv, unknown_exception, EXC_XFER_EE)
> +	EXCEPTION(0, HV_PRIV, Ehvpriv, unknown_exception, EXC_XFER_STD)
>   
>   interrupt_end:
>   
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 023a462..5ee0b90 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1858,6 +1858,10 @@ void SPEFloatingPointException(struct pt_regs *regs)
>   	int code = 0;
>   	int err;
>   
> +	/* We restore the interrupt state now */
> +	if (!arch_irq_disabled_regs(regs))
> +		local_irq_enable();
> +
>   	flush_spe_to_thread(current);
>   
>   	spefscr = current->thread.spefscr;
> @@ -1903,6 +1907,10 @@ void SPEFloatingPointRoundException(struct pt_regs *regs)
>   	extern int speround_handler(struct pt_regs *regs);
>   	int err;
>   
> +	/* We restore the interrupt state now */
> +	if (!arch_irq_disabled_regs(regs))
> +		local_irq_enable();
> +
>   	preempt_disable();
>   	if (regs->msr & MSR_SPE)
>   		giveup_spe(current);
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus


^ permalink raw reply

* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-01-27 13:13 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
	linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <4d8d4854-dac9-a78e-77e5-0455e8ca56c4@xenosoft.de>

Christoph,

What shall I do next?

Cheers,
Christian


On 25 January 2019 at 2:37PM, Christian Zigotzky wrote:
> Next step just with the first patch: 
> 5c532d07c2f3c3972104de505d06b8d85f403f06 (use powerpc zone selection)
>
> git clone git://git.infradead.org/users/hch/misc.git -b 
> powerpc-dma.6-debug a
>
> git checkout 5c532d07c2f3c3972104de505d06b8d85f403f06
>
> Link to the Git: 
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6-debug
>
> Results:
>
> X5000: The kernel detects the SATA hard disk drive and boots without 
> any problems.
>
> X1000: The kernel boots and the P.A. Semi Ethernet works!
>
> -- Christian
>
>
> On 23 January 2019 at 3:34PM, Christian Zigotzky wrote:
>> Hi Christoph,
>>
>> I also compiled a kernel (zImage) for the X1000  from your Git 
>> 'powerpc-dma.6-debug' (both patches) today.
>>
>> It boots and the P.A. Semi Ethernet works!
>>
>> I will test just the first patch tomorrow.
>>
>> Thanks,
>> Christian
>>
>>
>> On 21 January 2019 at 3:38PM, Christian Zigotzky wrote:
>>> Hello Christoph,
>>>
>>> Thanks for your reply. I successfully compiled a kernel (uImage) for 
>>> the X5000 from your Git 'powerpc-dma.6-debug' (both patches) today.
>>>
>>> It detects the SATA hard disk drive and boots without any problems.
>>>
>>
>>
>
>


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox