Skip to contentSkip to conversation

powerpc/xive: Fix trying to "push" an already active pool VP

PATCH Applied Benjamin Herrenschmidt 2 participants 7 messages Apr 11, 2018 – Aug 8, 2018 PowerPC

lore
Share & tools
lore

Benjamin Herrenschmidt

author
#0 lore raw

PATCH 1/3 powerpc/xive: Fix trying to "push" an already active pool VP

When setting up a CPU, we "push" (activate) a pool VP for it.

However it's an error to do so if it already has an active pool VP.

This happens when doing soft CPU hotplug on powernv since we don't tear down the CPU on unplug. The HW flags the error which gets captured by the diagnostics.

Fix this by making sure to "pull" out any already active pool first.

Signed-off-by Benjamin Herrenschmidt <[email protected]>
---
 arch/powerpc/sysdev/xive/native.c | 4 ++++
 1 file changed, 4 insertions(+)
arch/powerpc/sysdev/xive/native.c +4 -0
index d22aeb0b69e1..b48454be5b98 100644
@@ -389,6 +389,10 @@ static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 	if (xive_pool_vps == XIVE_INVALID_VP)
 		return;
 
+	/* Check if pool VP already active, if it is, pull it */
+	if (in_be32(xive_tima + TM_QW2_HV_POOL + TM_WORD2) & TM_QW2W2_VP)
+		in_be64(xive_tima + TM_SPC_PULL_POOL_CTX);
+
 	/* Enable the pool VP */
 	vp = xive_pool_vps + cpu;
 	pr_debug("CPU %d setting up pool VP 0x%x\n", cpu, vp);
Signature · 1 line
2.14.3
author
#1 lore raw

PATCH 2/3 powerpc/xive: Remove now useless pr_debug statements

Those overly verbose statement in the setup of the pool VP aren't particularly useful (esp. considering we don't actually use the pool, we configure it bcs HW requires it only). So remove them which improves the code readability.

Signed-off-by Benjamin Herrenschmidt <[email protected]>
---
 arch/powerpc/sysdev/xive/native.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)
arch/powerpc/sysdev/xive/native.c +1 -9
index b48454be5b98..c7088a35eb89 100644
@@ -395,7 +395,6 @@ static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 
 	/* Enable the pool VP */
 	vp = xive_pool_vps + cpu;
-	pr_debug("CPU %d setting up pool VP 0x%x\n", cpu, vp);
 	for (;;) {
 		rc = opal_xive_set_vp_info(vp, OPAL_XIVE_VP_ENABLED, 0);
 		if (rc != OPAL_BUSY)

@@ -415,16 +414,9 @@ static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 	}
 	vp_cam = be64_to_cpu(vp_cam_be);
 
-	pr_debug("VP CAM = %llx\n", vp_cam);
-
 	/* Push it on the CPU (set LSMFB to 0xff to skip backlog scan) */
-	pr_debug("(Old HW value: %08x)\n",
-		 in_be32(xive_tima + TM_QW2_HV_POOL + TM_WORD2));
 	out_be32(xive_tima + TM_QW2_HV_POOL + TM_WORD0, 0xff);
-	out_be32(xive_tima + TM_QW2_HV_POOL + TM_WORD2,
-		 TM_QW2W2_VP | vp_cam);
-	pr_debug("(New HW value: %08x)\n",
-		 in_be32(xive_tima + TM_QW2_HV_POOL + TM_WORD2));
+	out_be32(xive_tima + TM_QW2_HV_POOL + TM_WORD2, TM_QW2W2_VP | vp_cam);
 }
 
 static void xive_native_teardown_cpu(unsigned int cpu, struct xive_cpu *xc)
Signature · 1 line
2.14.3
Applied
#2 lore raw

Re: [2/3] powerpc/xive: Remove now useless pr_debug statements

Quoted 6 lines · Benjamin Herrenschmidt

Those overly verbose statement in the setup of the pool VP aren't particularly useful (esp. considering we don't actually use the pool, we configure it bcs HW requires it only). So remove them which improves the code readability.

Signed-off-by Benjamin Herrenschmidt <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/dbc5740247961d6b060737620520dc

cheers

author
#3 lore raw

PATCH 3/3 powerpc/xive: Remove xive_kexec_teardown_cpu()

In reply to Benjamin Herrenschmidt

It's identical to xive_teardown_cpu() so just use the latter

Signed-off-by Benjamin Herrenschmidt <[email protected]>
---
 arch/powerpc/include/asm/xive.h        |  1 -
 arch/powerpc/platforms/powernv/setup.c |  2 +-
 arch/powerpc/platforms/pseries/kexec.c |  2 +-
 arch/powerpc/sysdev/xive/common.c      | 22 ----------------------
 4 files changed, 2 insertions(+), 25 deletions(-)
arch/powerpc/include/asm/xive.h +0 -1
index 8d1a2792484f..3c704f5dd3ae 100644
@@ -87,7 +87,6 @@ extern int  xive_smp_prepare_cpu(unsigned int cpu);
 extern void xive_smp_setup_cpu(void);
 extern void xive_smp_disable_cpu(void);
 extern void xive_teardown_cpu(void);
-extern void xive_kexec_teardown_cpu(int secondary);
 extern void xive_shutdown(void);
 extern void xive_flush_interrupt(void);
arch/powerpc/platforms/powernv/setup.c +1 -1
index 092715b9674b..5b4b09816791 100644
@@ -282,7 +282,7 @@ static void pnv_kexec_cpu_down(int crash_shutdown, int secondary)
 	u64 reinit_flags;
 
 	if (xive_enabled())
-		xive_kexec_teardown_cpu(secondary);
+		xive_teardown_cpu();
 	else
 		xics_kexec_teardown_cpu(secondary);
arch/powerpc/platforms/pseries/kexec.c +1 -1
index eeb13429d685..9dabf019556b 100644
@@ -53,7 +53,7 @@ void pseries_kexec_cpu_down(int crash_shutdown, int secondary)
 	}
 
 	if (xive_enabled())
-		xive_kexec_teardown_cpu(secondary);
+		xive_teardown_cpu();
 	else
 		xics_kexec_teardown_cpu(secondary);
 }
arch/powerpc/sysdev/xive/common.c +0 -22
index 40c06110821c..c8db51b60b4b 100644
@@ -1408,28 +1408,6 @@ void xive_teardown_cpu(void)
 	xive_cleanup_cpu_queues(cpu, xc);
 }
 
-void xive_kexec_teardown_cpu(int secondary)
-{
-	struct xive_cpu *xc = __this_cpu_read(xive_cpu);
-	unsigned int cpu = smp_processor_id();
-
-	/* Set CPPR to 0 to disable flow of interrupts */
-	xc->cppr = 0;
-	out_8(xive_tima + xive_tima_offset + TM_CPPR, 0);
-
-	/* Backend cleanup if any */
-	if (xive_ops->teardown_cpu)
-		xive_ops->teardown_cpu(cpu, xc);
-
-#ifdef CONFIG_SMP
-	/* Get rid of IPI */
-	xive_cleanup_cpu_ipi(cpu, xc);
-#endif
-
-	/* Disable and free the queues */
-	xive_cleanup_cpu_queues(cpu, xc);
-}
-
 void xive_shutdown(void)
 {
 	xive_ops->shutdown();
Signature · 1 line
2.14.3
author
#5 lore raw
In reply to Benjamin Herrenschmidt
Quoted 5 lines · Benjamin Herrenschmidt

When setting up a CPU, we "push" (activate) a pool VP for it.

However it's an error to do so if it already has an active pool VP.

Quoted 8 lines

This happens when doing soft CPU hotplug on powernv since we don't tear down the CPU on unplug. The HW flags the error which gets captured by the diagnostics.

Fix this by making sure to "pull" out any already active pool first.

Signed-off-by Benjamin Herrenschmidt <[email protected]>
CC [email protected]...
Quoted 19 lines
---
 arch/powerpc/sysdev/xive/native.c | 4 ++++
 1 file changed, 4 insertions(+)
arch/powerpc/sysdev/xive/native.c +4 -0
index d22aeb0b69e1..b48454be5b98 100644
@@ -389,6 +389,10 @@ static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
 	if (xive_pool_vps == XIVE_INVALID_VP)
 		return;
 
+	/* Check if pool VP already active, if it is, pull it */
+	if (in_be32(xive_tima + TM_QW2_HV_POOL + TM_WORD2) & TM_QW2W2_VP)
+		in_be64(xive_tima + TM_SPC_PULL_POOL_CTX);
+
 	/* Enable the pool VP */
 	vp = xive_pool_vps + cpu;
 	pr_debug("CPU %d setting up pool VP 0x%x\n", cpu, vp);
#6 lore raw
In reply to Benjamin Herrenschmidt
Quoted 5 lines · Benjamin Herrenschmidt

When setting up a CPU, we "push" (activate) a pool VP for it.

However it's an error to do so if it already has an active pool VP.

Quoted 8 lines

This happens when doing soft CPU hotplug on powernv since we don't tear down the CPU on unplug. The HW flags the error which gets captured by the diagnostics.

Fix this by making sure to "pull" out any already active pool first.

Signed-off-by Benjamin Herrenschmidt <[email protected]>

Patch 1 applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/b32e56e5a87a1f9243db92bc7a5df0

cheers