Hi Shrikanth,
Thanks for the fixes. I had one question on patch 1.

On 28/09/26 4:34 PM, Shrikanth Hegde wrote:
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 disabling the preemption using the preempt disable.
Note, it is calling noresched variant of preempt enable, since hard
irq are disabled. It is likely not a good idea to call schedule.

Fixes: 3efc7da61f6c ("KVM: PPC: Book3E: Increase FPU laziness")
Reported-by: Christian Zigotzky <[email protected]>
Closes: 
https://lore.kernel.org/all/[email protected]/
Signed-off-by: Shrikanth Hegde <[email protected]>
---
  arch/powerpc/kvm/booke.c | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 13ad4cf5fa71..5b9118eefe1d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1404,10 +1404,17 @@ 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() marks the software
+                        * IRQ state enabled while interrupts are still
+                        * hard-disabled. So disable preemption while loading
+                        * guest FP and Altivec.
+                        */
                        kvmppc_fix_ee_before_entry();
+                       preempt_disable();
                        kvmppc_load_guest_fp(vcpu);
                        kvmppc_load_guest_altivec(vcpu);
+                       preempt_enable_no_resched();
Would it be simpler to move kvmppc_fix_ee_before_entry() after the
FP/Altivec loads instead?

The normal kvmppc_vcpu_run() entry path already loads the guest
FP/Altivec state while interrupts are still disabled and calls
kvmppc_fix_ee_before_entry() immediately before entering the guest.

So could this path follow the same ordering:

kvmppc_load_guest_fp(vcpu);
kvmppc_load_guest_altivec(vcpu);
kvmppc_fix_ee_before_entry();

That would avoid making the software IRQ state enabled before loading
the guest FP/Altivec state, and also avoid the additional
preempt_disable()/preempt_enable_no_resched() pair.

Thanks,
Narayana Murty.
                }
        }


Reply via email to