| 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: <b54bae8c-5106-41aa-60ab-27146660a16e@huawei.com>
Date: Tue, 11 Feb 2025 19:32:58 +0800
From: Jinjie Ruan <ruanjinjie@...wei.com>
To: Mark Rutland <mark.rutland@....com>
CC: <catalin.marinas@....com>, <will@...nel.org>, <oleg@...hat.com>,
<sstabellini@...nel.org>, <tglx@...utronix.de>, <peterz@...radead.org>,
<luto@...nel.org>, <mingo@...hat.com>, <juri.lelli@...hat.com>,
<vincent.guittot@...aro.org>, <dietmar.eggemann@....com>,
<rostedt@...dmis.org>, <bsegall@...gle.com>, <mgorman@...e.de>,
<vschneid@...hat.com>, <kees@...nel.org>, <wad@...omium.org>,
<akpm@...ux-foundation.org>, <samitolvanen@...gle.com>,
<masahiroy@...nel.org>, <hca@...ux.ibm.com>, <aliceryhl@...gle.com>,
<rppt@...nel.org>, <xur@...gle.com>, <paulmck@...nel.org>, <arnd@...db.de>,
<mbenes@...e.cz>, <puranjay@...nel.org>, <pcc@...gle.com>, <ardb@...nel.org>,
<sudeep.holla@....com>, <guohanjun@...wei.com>, <rafael@...nel.org>,
<liuwei09@...tc.cn>, <dwmw@...zon.co.uk>, <Jonathan.Cameron@...wei.com>,
<liaochang1@...wei.com>, <kristina.martsenko@....com>, <ptosi@...gle.com>,
<broonie@...nel.org>, <thiago.bauermann@...aro.org>, <kevin.brodsky@....com>,
<joey.gouly@....com>, <liuyuntao12@...wei.com>, <leobras@...hat.com>,
<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
<xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH -next v5 11/22] arm64: entry: Switch to generic IRQ entry
On 2025/2/10 20:24, Mark Rutland wrote:
> On Fri, Dec 06, 2024 at 06:17:33PM +0800, Jinjie Ruan wrote:
>> Currently, x86, Riscv, Loongarch use the generic entry. Convert arm64
>> to use the generic entry infrastructure from kernel/entry/*.
>> The generic entry makes maintainers' work easier and codes
>> more elegant.
>>
>> Switch arm64 to generic IRQ entry first, which removed duplicate 100+
>> LOC, and it will switch to generic entry completely later. Switch to
>> generic entry in two steps according to Mark's suggestion will make
>> it easier to review.
>>
>> The changes are below:
>> - Remove *enter_from/exit_to_kernel_mode(), and wrap with generic
>> irqentry_enter/exit(). Also remove *enter_from/exit_to_user_mode(),
>> and wrap with generic enter_from/exit_to_user_mode() because they
>> are exactly the same so far.
>>
>> - Remove arm64_enter/exit_nmi() and use generic irqentry_nmi_enter/exit()
>> because they're exactly the same, so the temporary arm64 version
>> irqentry_state can also be removed.
>>
>> - Remove PREEMPT_DYNAMIC code, as generic entry do the same thing
>> if arm64 implement arch_irqentry_exit_need_resched().
>>
>> Suggested-by: Mark Rutland <mark.rutland@....com>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@...wei.com>
>> ---
>> arch/arm64/Kconfig | 1 +
>> arch/arm64/include/asm/entry-common.h | 64 ++++++
>> arch/arm64/include/asm/preempt.h | 6 -
>> arch/arm64/kernel/entry-common.c | 307 ++++++--------------------
>> arch/arm64/kernel/signal.c | 3 +-
>> 5 files changed, 129 insertions(+), 252 deletions(-)
>> create mode 100644 arch/arm64/include/asm/entry-common.h
>
> Superficially this looks nice, but to be clear I have *not* looked at
> this in great detail; minor comments below.
>
> [...]
>
>> +static inline void arch_exit_to_user_mode_prepare(struct pt_regs *regs,
>> + unsigned long ti_work)
>> +{
>> + local_daif_mask();
>> +}
>> +
>> +#define arch_exit_to_user_mode_prepare arch_exit_to_user_mode_prepare
>
> I'm a little worried that this may be fragile having been hidden in the
> common code, as it's not clear exactly when this will occur during the
> return sequence, and the ordering requirements could easily be broken by
> refactoring there.
>
> I suspect we'll want to pull this later in the arm64 exit sequence so
> that we can have it explicit in entry-common.c.
Yes, this key function is hidden in generic entry code and is not easy
to clear and see when it is executed. But placing it directly in
entry-common.c in arm64 may change the order in which lockdep_sys_exit()
and local_daif_mask() are called, it's not clear what the potential
impact is.
Before:
exit_to_user_mode_prepare()
...
-> local_daif_mask()
-> lockdep_sys_exit()
arm64_exit_to_user_mode()
...
-> exit_to_user_mode_prepare()
-> lockdep_sys_exit()
-> local_daif_mask()
>
> [...]
>
>> index 14ac6fdb872b..84b6628647c7 100644
>> --- a/arch/arm64/kernel/signal.c
>> +++ b/arch/arm64/kernel/signal.c
>> @@ -9,6 +9,7 @@
>> #include <linux/cache.h>
>> #include <linux/compat.h>
>> #include <linux/errno.h>
>> +#include <linux/irq-entry-common.h>
>> #include <linux/kernel.h>
>> #include <linux/signal.h>
>> #include <linux/freezer.h>
>> @@ -1603,7 +1604,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
>> * the kernel can handle, and then we build all the user-level signal handling
>> * stack-frames in one go after that.
>> */
>> -void do_signal(struct pt_regs *regs)
>> +void arch_do_signal_or_restart(struct pt_regs *regs)
>> {
>> unsigned long continue_addr = 0, restart_addr = 0;
>> int retval = 0;
>
> Is the expected semantic the same here, or is those more than just a
> name change?
Yes, the expected semantic is the same here, they both handle
_TIF_SIGPENDING and _TIF_NOTIFY_SIGNAL thread flags before
exit to user.
In arm64 the code call sequence is:
exit_to_user_mode()
-> exit_to_user_mode_prepare()
-> do_notify_resume(regs, flags)
-> if (thread_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
do_signal(regs)
In generic entry code, the logic is the same:
exit_to_user_mode_prepare()
-> exit_to_user_mode_loop()
-> if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
arch_do_signal_or_restart(regs)
>
> Mark.
>
Powered by blists - more mailing lists