Re: [External] Re: [PATCH] riscv: Fix local irq restore when flags indicates irq disabled

From: Alexandre Ghiti
Date: Tue Jun 18 2024 - 08:01:54 EST


Hi Andrea,

On 18/06/2024 13:05, Andrea Parri wrote:
(merging replies)

However, if local_irq_save() is called when irq is disabled. The SR_IE bit of
flag returned is clear. If some code between local_irq_save() and
local_irq_restore() reenables irq, causing the SR_IE bit of CSR_STATUS
back to 1, then local_irq_restore() can not restore irq status back to disabled.
But doesn't that represent some bogus manipulation of the irq flags? cf.

config DEBUG_IRQFLAGS
bool "Debug IRQ flag manipulation"
help
Enables checks for potentially unsafe enabling or disabling of
interrupts, such as calling raw_local_irq_restore() when interrupts
are enabled.

in particular, raw_check_bogus_irq_restore() in raw_local_irq_restore().


This got lost but this is still correct and needed.
You mean because of the mentioned rtl8723e example? are there other such
instances? IOW, why do you say that the changes in question are needed?


Simply because the scenario in this driver and I looked at the arm64 implementation which restores flags unconditionally.

But if that's considered bogus, let's drop this. Sorry Xu for the noise, and thanks Andrea for pointing this.

Alex



Andrea