[PATCH v2 0/3] KVM: PPC: BookE: Fix boot hang on preemptible kernels

[PATCH v2 0/3] KVM: PPC: BookE: Fix boot hang on preemptible kernels

[Shrikanth Hegde]

Christian Zigotzky had reported that preemptible kernels don't boot on
his FSL Cyrus+ board. The board freezes on running either full/lazy
preemption and has been an issue for a while now (years!).

https://lore.kernel.org/all/b897b0fd-90f2-4215-bcd4-3714e497d773@xenosoft.de/#t

After 7.0, there is only preempt=full/lazy which removed the
preempt=none/voluntary workaround used to mask the issue. There was
challenge is getting the console logs which made the fixes difficult since
one couldn't know where the issue is.

Thanks to Michal for helping in getting the logs. That pointed at few
places where the issue could be. This series is an attempt at fixing
those.

Christian, I haven't taken the tested-by tag since patches have
changed quite a bit.
Please help in *testing* the patches again.

v1->v2:
- Re-order to avoid preempt count dance (Narayana Murty N)
- Do only hard_irq_disable on PPC64 to avoid second warning. On PPC32
  use local_irq_disable to acheive the same. (Ritesh Harjani)

v1: https://lore.kernel.org/all/20260928110411.844582-1-sshegde@linux.ibm.com/

New API ppc_hard_irq_disable could potentially be used in other places.
There is similar pattern of doing both local and hard irq disable and
likely not all callsites need both. But it is a larger effort and due to
my time limit, it will be taken up subsequently.

Shrikanth Hegde (3):
  powerpc/irq: Add ppc_hard_irq_disable()
  KVM: PPC: BookE: Load FP and Altivec before enabling the interrupts
  KVM: PPC: Use ppc_hard_irq_disable before guest entry

 arch/powerpc/include/asm/hw_irq.h | 17 +++++++++++++++++
 arch/powerpc/kvm/booke.c          |  7 +++++--
 arch/powerpc/kvm/powerpc.c        | 14 +++-----------
 3 files changed, 25 insertions(+), 13 deletions(-)

-- 
2.52.0

[PATCH v2 1/3] powerpc/irq: Add ppc_hard_irq_disable()

[Shrikanth Hegde]

Add a flavour of interrupt disable which could be used in
multiple places which also takes care of lockdep in both 32 and
64 bit archs.

On PPC64:
- Disable hard interrupts using hard_irq_disable. It takes care of
  lockdep and tracing. local_irq_disable only alters softmask's state.

On PPC32:
- Interrupts are hard disabled with local_irq_disable. It also takes
  care of lockdep and tacing.

Subsequent patch uses this API in KVM code to avoid the warnings.
Wider use of it is possible, but it will be taken subsequently.

Suggested-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 arch/powerpc/include/asm/hw_irq.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
index b7eee6385ae5..075442366e1e 100644
--- a/arch/powerpc/include/asm/hw_irq.h
+++ b/arch/powerpc/include/asm/hw_irq.h
@@ -516,6 +516,23 @@ static inline unsigned long mtmsr_isync_irqsafe(unsigned long msr)
 
 #define ARCH_IRQ_INIT_FLAGS	IRQ_NOREQUEST
 
+/*
+ * Hard-disable interrupts while keeping the IRQ tracing state
+ * consistent.
+ *
+ * On PPC64, local_irq_disable() only soft-disables interrupts, leaving
+ * a window where an interrupt can be recorded in irq_happened before
+ * interrupts are hard-disabled.
+ *
+ * On PPC32, hard_irq_disable() does not update the IRQ tracing state, so
+ * use local_irq_disable().
+ */
+#ifdef CONFIG_PPC64
+#define ppc_hard_irq_disable()	hard_irq_disable()
+#else
+#define ppc_hard_irq_disable()	local_irq_disable()
+#endif
+
 #endif  /* __ASSEMBLER__ */
 #endif	/* __KERNEL__ */
 #endif	/* _ASM_POWERPC_HW_IRQ_H */
-- 
2.52.0

[PATCH v2 2/3] KVM: PPC: BookE: Load FP and Altivec before enabling the interrupts

[Shrikanth Hegde]

Christian reported that booting preemptible kernel on FSL Cyrus+ board
causes boot hang.

The logs pointed that system was busy in printing below warning.

WARNING: at .enable_kernel_fp+0x30/0x78, CPU#3: qemu-system-ppc/4884
Modules linked in:
CPU: 3 UID: 1000 PID: 4884 Comm: qemu-system-ppc Not tainted 7.3.0-rc1-powerpc64-smp-preempt #1 PREEMPT
NIP [c000000000003338] .enable_kernel_fp+0x30/0x78
LR [c00000000005de84] .kvmppc_load_guest_fp+0x30/0x80
Call Trace:
[c000000085ca7700] [c00000000005de84] .kvmppc_load_guest_fp+0x30/0x80
[c000000085ca7780] [c00000000005f2a0] .kvmppc_handle_exit+0x5bc/0x5cc
[c000000085ca7830] [c00000000006204c] .kvmppc_resume_host+0xb8/0x10c

Which is...

void enable_kernel_fp(void)
{
        unsigned long cpumsr;
        WARN_ON(preemptible());

And...

Though irq's are hard disabled after kvmppc_prepare_to_enter, but
kvmppc_fix_ee_before_entry enables the softmask's IRQ state.
That causes the irqs_disabled to return false.
Hence leading to the warnings.

Fix it by re-ordering the code to load the FP and Altivec
before enabling the softmask IRQ state.

Fixes: 3efc7da61f6c ("KVM: PPC: Book3E: Increase FPU laziness")
Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
Closes: https://lore.kernel.org/all/33342fbf-eb7b-bde6-2c8c-254fe8bfb993@xenosoft.de/
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 arch/powerpc/kvm/booke.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 13ad4cf5fa71..c35e98d544ec 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1404,10 +1404,13 @@ int kvmppc_handle_exit(struct kvm_vcpu *vcpu, unsigned int exit_nr)
 		if (s <= 0)
 			r = (s << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
 		else {
-			/* interrupts now hard-disabled */
-			kvmppc_fix_ee_before_entry();
+			/*
+			 * Interrupts are still hard disabled and soft enabled
+			 * after loading the FP and Altivec.
+			 */
 			kvmppc_load_guest_fp(vcpu);
 			kvmppc_load_guest_altivec(vcpu);
+			kvmppc_fix_ee_before_entry();
 		}
 	}
 
-- 
2.52.0

[PATCH v2 3/3] KVM: PPC: Use ppc_hard_irq_disable before guest entry

[Shrikanth Hegde]

After applying preempt disable patch, i.e PATCH 2/3, Christian reported
a subsequent warning stopping his board to boot properly.

WARNING: at .kvmppc_fix_ee_before_entry+0x10/0x28, CPU#0: qemu-system-ppc/4667
CPU: 0 UID: 1000 PID: 4667 Comm: qemu-system-ppc Tainted: G        W           7.3.0-rc4-2-powerpc64-smp #1 PREEMPT
Tainted: [W]=WARN
Hardware name: varisys,CYRUS5040 e5500 0x80240012 CoreNet Generic
NIP [c00000000005dc3c] .kvmppc_fix_ee_before_entry+0x10/0x28
LR [c00000000005f298] .kvmppc_handle_exit+0x5b4/0x5e8
Call Trace:
[c000000086587780] [c00000000005ee10] .kvmppc_handle_exit+0x12c/0x5e8 (unreliable)
[c000000086587830] [c000000000062068] .kvmppc_resume_host+0xb8/0x10c

It triggers below warning...

static inline void kvmppc_fix_ee_before_entry(void)
{
        trace_hardirqs_on();

        /*
         * To avoid races, the caller must have gone directly from having
         * interrupts fully-enabled to hard-disabled.
         */
        WARN_ON(local_paca->irq_happened != PACA_IRQ_HARD_DIS);

This happens since kvmppc_prepare_to_enter does first local_irq_disable
followed by hard_irq_disable. This leaves a small window where interrupt
may occur and it could set the irq pending bit in PACA.

That local_irq_disable was necessary for PPC32 to track the lockdep
state etc. On PPC64 hard_irq_disable is good enough. There is no need to
do both. Use the new irq api to do the necessary one.

Fixes: 12013e3d4695 ("KVM: powerpc: Use generic xfer to guest work function")
Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
Closes: https://lore.kernel.org/all/b8f82519-9ae5-c247-e020-e79c627e57df@xenosoft.de/
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 arch/powerpc/kvm/powerpc.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 9194cf492d1c..4ee3d7bfe917 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -81,13 +81,7 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
 	int r;
 
 	WARN_ON(irqs_disabled());
-	/*
-	 * local_irq_disable() first: on 32-bit, hard_irq_disable() alone is a
-	 * raw MSR[EE] clear that bypasses the lockdep/irq-tracing state, and
-	 * the xfer_to_guest_mode helpers assert IRQs are seen as disabled.
-	 */
-	local_irq_disable();
-	hard_irq_disable();
+	ppc_hard_irq_disable();
 
 	while (true) {
 		xfer_to_guest_mode_prepare();
@@ -101,8 +95,7 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
 			 */
 			local_irq_enable();
 			r = kvm_xfer_to_guest_mode_handle_work(vcpu);
-			local_irq_disable();
-			hard_irq_disable();
+			ppc_hard_irq_disable();
 			if (r) {
 				/*
 				 * The generic helper does not set the exit
@@ -134,8 +127,7 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
 			local_irq_enable();
 			trace_kvm_check_requests(vcpu);
 			r = kvmppc_core_check_requests(vcpu);
-			local_irq_disable();
-			hard_irq_disable();
+			ppc_hard_irq_disable();
 			if (r > 0)
 				continue;
 			break;
-- 
2.52.0