linux-kernel - Re: [PATCH] arch: loongarch: fix loongarch S3 WARNING

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAhV-H5A144ubJ6WD68y7f6sN3j=g-eWVqZ6mhko9nzTgM=xYA@mail.gmail.com>
Date: Thu, 7 Nov 2024 20:36:51 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Qunqin Zhao <zhaoqunqin@...ngson.cn>
Cc: kernel@...0n.name, loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arch: loongarch: fix loongarch S3 WARNING

Hi, Qunqin,

At first, the subject should be "LoongArch: Fix warnings during S3 suspend".

On Thu, Nov 7, 2024 at 11:29 AM Qunqin Zhao <zhaoqunqin@...ngson.cn> wrote:
>
> The enable_gpe_wakeup() function may call the preempt_schedule_common()
> function, resulting in a thread switch and causing the CPU to be in an
> interrupt enable state after the enable_gpe_wakeup() function returns,
> leading to the warning as follow. Calling enable_gap_wakeup() before
Second, I think enable_gap_wakeup() should be enable_gpe_wakeup() here.

> local_irq_disable() to fix this waring.
>
> [ C0] WARNING: ... at kernel/time/timekeeping.c:845 ktime_get+0xbc/0xc8
> [ C0]         ...
> [ C0] Call Trace:
> [ C0] [<90000000002243b4>] show_stack+0x64/0x188
> [ C0] [<900000000164673c>] dump_stack_lvl+0x60/0x88
> [ C0] [<90000000002687e4>] __warn+0x8c/0x148
> [ C0] [<90000000015e9978>] report_bug+0x1c0/0x2b0
> [ C0] [<90000000016478e4>] do_bp+0x204/0x3b8
> [ C0] [<90000000025b1924>] exception_handlers+0x1924/0x10000
> [ C0] [<9000000000343bbc>] ktime_get+0xbc/0xc8
> [ C0] [<9000000000354c08>] tick_sched_timer+0x30/0xb0
> [ C0] [<90000000003408e0>] __hrtimer_run_queues+0x160/0x378
> [ C0] [<9000000000341f14>] hrtimer_interrupt+0x144/0x388
> [ C0] [<9000000000228348>] constant_timer_interrupt+0x38/0x48
> [ C0] [<90000000002feba4>] __handle_irq_event_percpu+0x64/0x1e8
> [ C0] [<90000000002fed48>] handle_irq_event_percpu+0x20/0x80
> [ C0] [<9000000000306b9c>] handle_percpu_irq+0x5c/0x98
> [ C0] [<90000000002fd4a0>] generic_handle_domain_irq+0x30/0x48
> [ C0] [<9000000000d0c7b0>] handle_cpu_irq+0x70/0xa8
> [ C0] [<9000000001646b30>] handle_loongarch_irq+0x30/0x48
> [ C0] [<9000000001646bc8>] do_vint+0x80/0xe0
> [ C0] [<90000000002aea1c>] finish_task_switch.isra.0+0x8c/0x2a8
> [ C0] [<900000000164e34c>] __schedule+0x314/0xa48
> [ C0] [<900000000164ead8>] schedule+0x58/0xf0
> [ C0] [<9000000000294a2c>] worker_thread+0x224/0x498
> [ C0] [<900000000029d2f0>] kthread+0xf8/0x108
> [ C0] [<9000000000221f28>] ret_from_kernel_thread+0xc/0xa4
> [ C0]
> [ C0] ---[ end trace 0000000000000000 ]---
Finally, I think you should enable some config options to produce such
warnings? Because I haven't observed it.


Huacai

>
> And move enable_pci_wakeup() before local_irq_disable(), just in
> case interrupt is enabled after the execution of enable_pci_wakeup()
>
> Fixes: 366bb35a8e48 ("LoongArch: Add suspend (ACPI S3) support")
> Signed-off-by: Qunqin Zhao <zhaoqunqin@...ngson.cn>
> ---
>  arch/loongarch/power/suspend.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/loongarch/power/suspend.c b/arch/loongarch/power/suspend.c
> index c9e594925..d0dc375b0 100644
> --- a/arch/loongarch/power/suspend.c
> +++ b/arch/loongarch/power/suspend.c
> @@ -28,6 +28,13 @@ struct saved_registers {
>  };
>  static struct saved_registers saved_regs;
>
> +void arch_suspend_disable_irqs(void)
> +{
> +       enable_gpe_wakeup();
> +       enable_pci_wakeup();
> +       local_irq_disable();
> +}
> +
>  void loongarch_common_suspend(void)
>  {
>         save_counter();
> @@ -61,9 +68,6 @@ void loongarch_common_resume(void)
>
>  int loongarch_acpi_suspend(void)
>  {
> -       enable_gpe_wakeup();
> -       enable_pci_wakeup();
> -
>         loongarch_common_suspend();
>
>         /* processor specific suspend */
>
> base-commit: 73adbd92f3223dc0c3506822b71c6b259d5d537b
> --
> 2.43.0
>