| 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
| ||
|
Message-ID: <06cf8750-901d-2f65-fb9e-81980688e017@loongson.cn>
Date: Mon, 14 Aug 2023 10:58:16 +0800
From: bibo mao <maobibo@...ngson.cn>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Anna-Maria Behnsen <anna-maria@...utronix.de>,
WANG Xuerui <kernel@...0n.name>,
LKML <linux-kernel@...r.kernel.org>,
Huacai Chen <chenhuacai@...ngson.cn>
Subject: Re: [PATCH 04/10] loongson: Fix idle VS timer enqueue
+Add huacai
asm instruction move should be replaced with li.w, the other looks good to me.
diff --git a/arch/loongarch/kernel/genex.S b/arch/loongarch/kernel/genex.S
index 359d693f112e..8a98023ecafd 100644
--- a/arch/loongarch/kernel/genex.S
+++ b/arch/loongarch/kernel/genex.S
@@ -19,7 +19,7 @@
.align 5
SYM_FUNC_START(__arch_cpu_idle)
/* start of idle interrupt region */
- move t0, CSR_CRMD_IE
+ li.w t0, CSR_CRMD_IE
csrxchg t0, t0, LOONGARCH_CSR_CRMD
/*
* If an interrupt lands here; between enabling interrupts above and
By the way __arch_cpu_idle is called by machine_halt/machine_power_off etc,
irq will be enabled with new patch. Need we add something like this?
diff --git a/arch/loongarch/kernel/reset.c b/arch/loongarch/kernel/reset.c
index 1ef8c6383535..9ecd42c0c804 100644
--- a/arch/loongarch/kernel/reset.c
+++ b/arch/loongarch/kernel/reset.c
@@ -20,6 +20,11 @@
void (*pm_power_off)(void);
EXPORT_SYMBOL(pm_power_off);
+static __always_inline void native_halt(void)
+{
+ asm volatile("idle 0": : :"memory");
+}
+
void machine_halt(void)
{
#ifdef CONFIG_SMP
@@ -32,9 +37,9 @@ void machine_halt(void)
pr_notice("\n\n** You can safely turn off the power now **\n\n");
console_flush_on_panic(CONSOLE_FLUSH_PENDING);
- while (true) {
- __arch_cpu_idle();
- }
+ while (1) {
+ native_halt();
+ };
}
void machine_power_off(void)
@@ -52,9 +57,9 @@ void machine_power_off(void)
efi.reset_system(EFI_RESET_SHUTDOWN, EFI_SUCCESS, 0, NULL);
#endif
- while (true) {
- __arch_cpu_idle();
- }
+ while (1) {
+ native_halt();
+ };
}
void machine_restart(char *command)
@@ -73,7 +78,7 @@ void machine_restart(char *command)
if (!acpi_disabled)
acpi_reboot();
- while (true) {
- __arch_cpu_idle();
- }
+ while (1) {
+ native_halt();
+ };
}
Regards
Bibo Mao
在 2023/8/12 01:00, Frederic Weisbecker 写道:
> From: Peter Zijlstra <peterz@...radead.org>
>
> Loongson re-enables interrupts on its idle routine and performs a
> TIF_NEED_RESCHED check afterwards before putting the CPU to sleep.
>
> The IRQs firing between the check and the idling instruction may set the
> TIF_NEED_RESCHED flag. In order to deal with the such a race, IRQs
> interrupting __arch_cpu_idle() rollback their return address to the
> beginning of __arch_cpu_idle() so that TIF_NEED_RESCHED is checked
> again before going back to sleep.
>
> However idle IRQs can also queue timers that may require a tick
> reprogramming through a new generic idle loop iteration but those timers
> would go unnoticed here because __arch_cpu_idle() only checks
> TIF_NEED_RESCHED. It doesn't check for pending timers.
>
> Fix this with fast-forwarding idle IRQs return value to the end of the
> idle routine instead of the beginning, so that the generic idle loop
> handles both TIF_NEED_RESCHED and pending timers.
>
> Fixes: 0603839b18f4 (LoongArch: Add exception/interrupt handling)
> Tested-by: Bibo, Mao <maobibo@...ngson.cn>
> Not-yet-signed-off-by: Peter Zijlstra <peterz@...radead.org>
> Cc: WANG Xuerui <kernel@...0n.name>
> Signed-off-by: Frederic Weisbecker <frederic@...nel.org>
> ---
> arch/loongarch/kernel/genex.S | 32 ++++++++++++++++++--------------
> arch/loongarch/kernel/idle.c | 1 -
> 2 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/arch/loongarch/kernel/genex.S b/arch/loongarch/kernel/genex.S
> index 78f066384657..359d693f112e 100644
> --- a/arch/loongarch/kernel/genex.S
> +++ b/arch/loongarch/kernel/genex.S
> @@ -18,27 +18,31 @@
>
> .align 5
> SYM_FUNC_START(__arch_cpu_idle)
> - /* start of rollback region */
> - LONG_L t0, tp, TI_FLAGS
> - nop
> - andi t0, t0, _TIF_NEED_RESCHED
> - bnez t0, 1f
> - nop
> - nop
> - nop
> + /* start of idle interrupt region */
> + move t0, CSR_CRMD_IE
> + csrxchg t0, t0, LOONGARCH_CSR_CRMD
> + /*
> + * If an interrupt lands here; between enabling interrupts above and
> + * going idle on the next instruction, we must *NOT* go idle since the
> + * interrupt could have set TIF_NEED_RESCHED or caused an timer to need
> + * reprogramming. Fall through -- see handle_vint() below -- and have
> + * the idle loop take care of things.
> + */
> idle 0
> - /* end of rollback region */
> -1: jr ra
> + nop
> + /* end of idle interrupt region */
> +SYM_INNER_LABEL(__arch_cpu_idle_exit, SYM_L_LOCAL)
> + jr ra
> SYM_FUNC_END(__arch_cpu_idle)
>
> SYM_FUNC_START(handle_vint)
> BACKUP_T0T1
> SAVE_ALL
> - la_abs t1, __arch_cpu_idle
> + la_abs t1, __arch_cpu_idle_exit
> LONG_L t0, sp, PT_ERA
> - /* 32 byte rollback region */
> - ori t0, t0, 0x1f
> - xori t0, t0, 0x1f
> + /* 16 byte idle interrupt region */
> + ori t0, t0, 0x0f
> + addi.d t0, t0, 1
> bne t0, t1, 1f
> LONG_S t0, sp, PT_ERA
> 1: move a0, sp
> diff --git a/arch/loongarch/kernel/idle.c b/arch/loongarch/kernel/idle.c
> index 0b5dd2faeb90..5ba72d229920 100644
> --- a/arch/loongarch/kernel/idle.c
> +++ b/arch/loongarch/kernel/idle.c
> @@ -11,7 +11,6 @@
>
> void __cpuidle arch_cpu_idle(void)
> {
> - raw_local_irq_enable();
> __arch_cpu_idle(); /* idle instruction needs irq enabled */
> raw_local_irq_disable();
> }
Powered by blists - more mailing lists