Message-ID: <a03d7530-2f47-41a6-b79b-cc7eac8ffd84@arm.com>
Date: Mon, 26 Aug 2024 17:56:31 +0200
From: Kevin Brodsky <kevin.brodsky@....com>
To: Jinjie Ruan <ruanjinjie@...wei.com>
Cc: catalin.marinas@....com, will@...nel.org, oleg@...hat.com,
 tglx@...utronix.de, peterz@...radead.org, luto@...nel.org, kees@...nel.org,
 wad@...omium.org, rostedt@...dmis.org, arnd@...db.de, ardb@...nel.org,
 broonie@...nel.org, mark.rutland@....com, rick.p.edgecombe@...el.com,
 leobras@...hat.com, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 3/3] arm64: entry: Convert to generic entry

On 22/08/2024 14:30, Jinjie Ruan wrote:
> On 2024/8/20 19:43, Kevin Brodsky wrote:
>> On 29/06/2024 10:56, 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, which also
>>> removed duplicate 150+ LOC. The changes are below:
>>>
>>>  - Remove TIF_SYSCALL_* flag, _TIF_WORK_MASK, _TIF_SYSCALL_WORK
>>>
>>>  - Remove syscall_trace_enter/exit() and use generic one.
>>>
>>>  - Remove *enter_from/exit_to_kernel_mode(), and wrap with generic
>>>    irqentry_enter/exit().
>>>
>>>  - Remove *enter_from/exit_to_user_mode(), and wrap with generic
>>>    irqentry_enter_from/exit_to_user_mode().
>>>
>>>  - Remove arm64_enter/exit_nmi() and use generic irqentry_nmi_enter/exit().
>>>
>>>  - Remove PREEMPT_DYNAMIC code, as generic entry will do it ok by
>>>    implementing arch_irqentry_exit_need_resched().
>> This is looking good to me overall, moving to using the generic helpers
>> is a clear improvement. I've tried my best to check that the new
> Thank you very much for the review, It does make the code more clear.
>
>> implementation is functionally equivalent to the old. Ignoring additions
>> in the generic code (such as additional instrumentation_{begin,end}()
>> pairs or assertions), this seems to be the case, with one exception. The
>> MTE operations are done in a slightly different order on entry:
>>
>> * mte_disable_tco_entry() was called *after* the generic lockdep/CT
>> functions in __enter_from_user_mode(). It is now called before those
>> generic operations - arch_enter_from_user_mode() called at the beginning
>> of enter_from_user_mode().
> The most special for arm64 are these MTE operations, the comment for
> __enter_from_kernel_mode() and __enter_from_user_mode() said:
>
>    " Handle IRQ/context state management when entering from user/kernel
> mode. Before this function is called it is not safe to call regular
> kernel code "
>
> I guess it is the reason why mte_disable_tco_entry() was called *after*
> the generic lockdep/CT functions, but the first version of commit
> 38ddf7dafaeaf ("arm64: mte: avoid clearing PSTATE.TCO on entry unless
> necessary") call it in arch/arm64/kernel/entry.S, so I think the order
> is not so important.

Good point, before that commit TCO was disabled before any generic
exception code was run so changing the ordering of
mte_disable_tco_entry() is unlikely to be a problem (the current
implementation doesn't do anything complex).

>
>> * Similarly mte_disable_tco_entry() and mte_check_tfsr_entry() was
>> called after the generic operations in enter_from_kernel_mode(), and
>> they are now called after - arch_enter_from_kernel_mode() called at the
>> beginning of irqentry_enter().
> This can be adjusted to be consistent.

I wonder if moving mte_check_tfsr_entry() is as harmless as
mte_disable_tco_entry(), because the former can cause a stack dump. Is
that safe to do before the lockdep/RCU handling? I would guess so but
would be good for someone to confirm.

Kevin

Powered by blists - more mailing lists