Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: KVM: arm64: Regression in at least linux-6.1.y tree with recent FPSIMD/SVE/SME fix
       [not found] <010001999bae0958-4d80d25d-8dda-4006-a6b9-798f3e774f6c-000000@email.amazonses.com>
@ 2025-10-01  9:32 ` Will Deacon
  2025-10-01 17:47   ` Kenneth Van Alstyne
  0 siblings, 1 reply; 2+ messages in thread
From: Will Deacon @ 2025-10-01  9:32 UTC (permalink / raw)
  To: Kenneth Van Alstyne
  Cc: stable, regressions, catalin.marinas, ardb, mark.rutland,
	linux-arm-kernel, maz

Hi Kenneth,

Thanks for the report.

On Tue, Sep 30, 2025 at 05:31:38PM +0000, Kenneth Van Alstyne wrote:
> Sending via plain text email -- apologies if you receive this twice.
> 
> If this isn't the process for reporting a regression in a LTS kernel per
> https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html,
> I'm happy to follow another process.
> 
> Kernel 6.1.149 introduced a regression, at least on our ARM Cortex
> A57-based platforms, via commit 8f4dc4e54eed4bebb18390305eb1f721c00457e1
> in arch/arm64/kernel/fpsimd.c where booting KVM VMs eventually leads to a
> spinlock recursion BUG and crash of the box.
> 
> Reverting that commit via the below reverts to the old (working) behavior:
> 
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 837d1937300a57..bc42163a7fd1f0 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -1851,10 +1851,10 @@ void fpsimd_save_and_flush_cpu_state(void)
>   if (!system_supports_fpsimd())
>   return;
>   WARN_ON(preemptible());
> - get_cpu_fpsimd_context();
> + __get_cpu_fpsimd_context();
>   fpsimd_save();
>   fpsimd_flush_cpu_state();
> - put_cpu_fpsimd_context();
> + __put_cpu_fpsimd_context();
>  }
>   #ifdef CONFIG_KERNEL_MODE_NEON

Hmm, the problem with doing that is it will reintroduce the bug that
8f4dc4e54eed ("KVM: arm64: Fix kernel BUG() due to bad backport of
FPSIMD/SVE/SME fix") was trying to fix (see the backtrace in the commit
message). So the old behaviour is still broken, just in a slightly
different way.

> It's not entirely clear to me if this is specific to our firmware,
> specific to ARM Cortex A57, or more systemic as we lack sufficiently
> differentiated hardware to know.  I've tested on the latest 6.1 kernel in
> addition to the one in the log below and have also tested a number of
> firmware versions available for these boxes.
> 
> Steps to reproduce:
> 
> Boot VM in qemu-system-aarch64 with "-accel kvm" and "-cpu host" flags set -- no other arguments seem to matter
> Generate CPU load in VM
> 
> Kernel log:
> 
> [sjc1] root@si-compute-kvm-e0fff70016b4:/# [  805.905413] BUG: spinlock recursion on CPU#7, CPU 3/KVM/57616
> [  805.905452]  lock: 0xffff3045ef850240, .magic: dead4ead, .owner: CPU 3/KVM/57616, .owner_cpu: 7
> [  805.905477] CPU: 7 PID: 57616 Comm: CPU 3/KVM Tainted: G           O       6.1.152 #1
> [  805.905495] Hardware name: SoftIron SoftIron Platform Mainboard/SoftIron Platform Mainboard, BIOS 1.31 May 11 2023
> [  805.905516] Call trace:
> [  805.905524]  dump_backtrace+0xe4/0x110
> [  805.905538]  show_stack+0x20/0x30
> [  805.905548]  dump_stack_lvl+0x6c/0x88
> [  805.905561]  dump_stack+0x18/0x34
> [  805.905571]  spin_dump+0x98/0xac
> [  805.905583]  do_raw_spin_lock+0x70/0x128
> [  805.905596]  _raw_spin_lock+0x18/0x28
> [  805.905607]  raw_spin_rq_lock_nested+0x18/0x28
> [  805.905620]  update_blocked_averages+0x70/0x550
> [  805.905634]  run_rebalance_domains+0x50/0x70
> [  805.905645]  handle_softirqs+0x198/0x328
> [  805.905659]  __do_softirq+0x1c/0x28
> [  805.905669]  ____do_softirq+0x18/0x28
> [  805.905680]  call_on_irq_stack+0x30/0x48
> [  805.905691]  do_softirq_own_stack+0x24/0x30
> [  805.905703]  do_softirq+0x74/0x90
> [  805.905714]  __local_bh_enable_ip+0x64/0x80

Argh, this is because we can't simply mask/unmask softirqs and so when
they get re-enabled we process anything pending. I _think_ irqs are
disabled at this point, so perhaps we should only bother with
disabling/enabling softirqs if hardirqs are enabled, a bit like the hack
Ard had in:

https://lore.kernel.org/all/20250924152651.3328941-13-ardb+git@google.com/

Hacky diff at the end.

> [  805.905727]  fpsimd_save_and_flush_cpu_state+0x5c/0x68
> [  805.905740]  kvm_arch_vcpu_put_fp+0x4c/0x88
> [  805.905752]  kvm_arch_vcpu_put+0x28/0x88
> [  805.905764]  kvm_sched_out+0x38/0x58

(I think we run context_switch() => prepare_task_switch() here, so irqs
are disabled)

Will

--->8

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index fc51cdd5aaa7..a79df0804d67 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -184,7 +184,8 @@ static void __get_cpu_fpsimd_context(void)
  */
 static void get_cpu_fpsimd_context(void)
 {
-       local_bh_disable();
+       if (!irqs_disabled())
+               local_bh_disable();
        __get_cpu_fpsimd_context();
 }
 
@@ -205,7 +206,8 @@ static void __put_cpu_fpsimd_context(void)
 static void put_cpu_fpsimd_context(void)
 {
        __put_cpu_fpsimd_context();
-       local_bh_enable();
+       if (!irqs_disabled())
+               local_bh_enable();
 }
 
 static bool have_cpu_fpsimd_context(void)



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: KVM: arm64: Regression in at least linux-6.1.y tree with recent FPSIMD/SVE/SME fix
  2025-10-01  9:32 ` KVM: arm64: Regression in at least linux-6.1.y tree with recent FPSIMD/SVE/SME fix Will Deacon
@ 2025-10-01 17:47   ` Kenneth Van Alstyne
  0 siblings, 0 replies; 2+ messages in thread
From: Kenneth Van Alstyne @ 2025-10-01 17:47 UTC (permalink / raw)
  To: Will Deacon
  Cc: stable, regressions, catalin.marinas, ardb, mark.rutland,
	linux-arm-kernel, maz

Hi Will, thanks for the prompt reply and thorough explanation!

On the 6.1 branch, those functions have additional logic around realtime kernels, like so:

static void get_cpu_fpsimd_context(void)
{
        if (!IS_ENABLED(CONFIG_PREEMPT_RT))
                local_bh_disable();
        else
                preempt_disable();
        __get_cpu_fpsimd_context();
}

...

static void put_cpu_fpsimd_context(void)
{
        __put_cpu_fpsimd_context();
        if (!IS_ENABLED(CONFIG_PREEMPT_RT))
                local_bh_enable();
        else
                preempt_enable();
}


Honestly, we don't even set CONFIG_PREEMPT_RT for our ARM64 kernels, but in the interest of being complete, should we also not call preempt_enable/disable if (!irqs_disabled()) ?

I'm assuming this will get queued up for a fix in the stable branches soon in any case.

Thanks,

--
Kenneth Van Alstyne, Jr.
(228) 547-8045

> On Oct 1, 2025, at 05:32, Will Deacon <will@kernel.org> wrote:
> 
> Hi Kenneth,
> 
> Thanks for the report.
> 
> On Tue, Sep 30, 2025 at 05:31:38PM +0000, Kenneth Van Alstyne wrote:
>> Sending via plain text email -- apologies if you receive this twice.
>> 
>> If this isn't the process for reporting a regression in a LTS kernel per
>> https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html,
>> I'm happy to follow another process.
>> 
>> Kernel 6.1.149 introduced a regression, at least on our ARM Cortex
>> A57-based platforms, via commit 8f4dc4e54eed4bebb18390305eb1f721c00457e1
>> in arch/arm64/kernel/fpsimd.c where booting KVM VMs eventually leads to a
>> spinlock recursion BUG and crash of the box.
>> 
>> Reverting that commit via the below reverts to the old (working) behavior:
>> 
>> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
>> index 837d1937300a57..bc42163a7fd1f0 100644
>> --- a/arch/arm64/kernel/fpsimd.c
>> +++ b/arch/arm64/kernel/fpsimd.c
>> @@ -1851,10 +1851,10 @@ void fpsimd_save_and_flush_cpu_state(void)
>>  if (!system_supports_fpsimd())
>>  return;
>>  WARN_ON(preemptible());
>> - get_cpu_fpsimd_context();
>> + __get_cpu_fpsimd_context();
>>  fpsimd_save();
>>  fpsimd_flush_cpu_state();
>> - put_cpu_fpsimd_context();
>> + __put_cpu_fpsimd_context();
>> }
>>  #ifdef CONFIG_KERNEL_MODE_NEON
> 
> Hmm, the problem with doing that is it will reintroduce the bug that
> 8f4dc4e54eed ("KVM: arm64: Fix kernel BUG() due to bad backport of
> FPSIMD/SVE/SME fix") was trying to fix (see the backtrace in the commit
> message). So the old behaviour is still broken, just in a slightly
> different way.
> 
>> It's not entirely clear to me if this is specific to our firmware,
>> specific to ARM Cortex A57, or more systemic as we lack sufficiently
>> differentiated hardware to know.  I've tested on the latest 6.1 kernel in
>> addition to the one in the log below and have also tested a number of
>> firmware versions available for these boxes.
>> 
>> Steps to reproduce:
>> 
>> Boot VM in qemu-system-aarch64 with "-accel kvm" and "-cpu host" flags set -- no other arguments seem to matter
>> Generate CPU load in VM
>> 
>> Kernel log:
>> 
>> [sjc1] root@si-compute-kvm-e0fff70016b4:/# [  805.905413] BUG: spinlock recursion on CPU#7, CPU 3/KVM/57616
>> [  805.905452]  lock: 0xffff3045ef850240, .magic: dead4ead, .owner: CPU 3/KVM/57616, .owner_cpu: 7
>> [  805.905477] CPU: 7 PID: 57616 Comm: CPU 3/KVM Tainted: G           O       6.1.152 #1
>> [  805.905495] Hardware name: SoftIron SoftIron Platform Mainboard/SoftIron Platform Mainboard, BIOS 1.31 May 11 2023
>> [  805.905516] Call trace:
>> [  805.905524]  dump_backtrace+0xe4/0x110
>> [  805.905538]  show_stack+0x20/0x30
>> [  805.905548]  dump_stack_lvl+0x6c/0x88
>> [  805.905561]  dump_stack+0x18/0x34
>> [  805.905571]  spin_dump+0x98/0xac
>> [  805.905583]  do_raw_spin_lock+0x70/0x128
>> [  805.905596]  _raw_spin_lock+0x18/0x28
>> [  805.905607]  raw_spin_rq_lock_nested+0x18/0x28
>> [  805.905620]  update_blocked_averages+0x70/0x550
>> [  805.905634]  run_rebalance_domains+0x50/0x70
>> [  805.905645]  handle_softirqs+0x198/0x328
>> [  805.905659]  __do_softirq+0x1c/0x28
>> [  805.905669]  ____do_softirq+0x18/0x28
>> [  805.905680]  call_on_irq_stack+0x30/0x48
>> [  805.905691]  do_softirq_own_stack+0x24/0x30
>> [  805.905703]  do_softirq+0x74/0x90
>> [  805.905714]  __local_bh_enable_ip+0x64/0x80
> 
> Argh, this is because we can't simply mask/unmask softirqs and so when
> they get re-enabled we process anything pending. I _think_ irqs are
> disabled at this point, so perhaps we should only bother with
> disabling/enabling softirqs if hardirqs are enabled, a bit like the hack
> Ard had in:
> 
> https://lore.kernel.org/all/20250924152651.3328941-13-ardb+git@google.com/
> 
> Hacky diff at the end.
> 
>> [  805.905727]  fpsimd_save_and_flush_cpu_state+0x5c/0x68
>> [  805.905740]  kvm_arch_vcpu_put_fp+0x4c/0x88
>> [  805.905752]  kvm_arch_vcpu_put+0x28/0x88
>> [  805.905764]  kvm_sched_out+0x38/0x58
> 
> (I think we run context_switch() => prepare_task_switch() here, so irqs
> are disabled)
> 
> Will
> 
> --->8
> 
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index fc51cdd5aaa7..a79df0804d67 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -184,7 +184,8 @@ static void __get_cpu_fpsimd_context(void)
>  */
> static void get_cpu_fpsimd_context(void)
> {
> -       local_bh_disable();
> +       if (!irqs_disabled())
> +               local_bh_disable();
>        __get_cpu_fpsimd_context();
> }
> 
> @@ -205,7 +206,8 @@ static void __put_cpu_fpsimd_context(void)
> static void put_cpu_fpsimd_context(void)
> {
>        __put_cpu_fpsimd_context();
> -       local_bh_enable();
> +       if (!irqs_disabled())
> +               local_bh_enable();
> }
> 
> static bool have_cpu_fpsimd_context(void)



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-10-01 17:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <010001999bae0958-4d80d25d-8dda-4006-a6b9-798f3e774f6c-000000@email.amazonses.com>
2025-10-01  9:32 ` KVM: arm64: Regression in at least linux-6.1.y tree with recent FPSIMD/SVE/SME fix Will Deacon
2025-10-01 17:47   ` Kenneth Van Alstyne

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