[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9268589d-9ccc-4cdd-9de2-5019407ef313@ghiti.fr>
Date: Tue, 18 Jun 2024 13:52:35 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: Andrea Parri <parri.andrea@...il.com>
Cc: Xu Lu <luxu.kernel@...edance.com>, Palmer Dabbelt <palmer@...belt.com>,
Paul Walmsley <paul.walmsley@...ive.com>, aou@...s.berkeley.edu,
linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [External] Re: [PATCH] riscv: Fix local irq restore when flags
indicates irq disabled
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
Powered by blists - more mailing lists