Re: [PATCH 1/2] KVM: PPC: BookE: Disable preemption before loading guest FP and Altivec
flat view
From: Narayana Murty N <hidden>
Date: 2026-10-01 11:27:13
Also in:
lkml
Hi Shrikanth, Thanks for the fixes. I had one question on patch 1. On 28/09/26 4:34 PM, Shrikanth Hegde wrote:
quoted hunk ↗ jump to hunk
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 <redacted> Closes: https://lore.kernel.org/all/33342fbf-eb7b-bde6-2c8c-254fe8bfb993@xenosoft.de/ (local) Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com> --- 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.
} }