Re: [PATCH v3 0/3] arm64: entry: Convert to generic entry

[Mark Rutland <mark.rutland@arm.com>]

On Mon, Oct 21, 2024 at 04:30:51PM +0800, Jinjie Ruan wrote:
> On 2024/10/17 23:25, Mark Rutland wrote:
> > On Sat, Jun 29, 2024 at 04:55:58PM +0800, Jinjie Ruan wrote:

> > Patch 3 in particular is very hard to follow because several unrelated
> > complex systems are updated simultaneously. It would be really nice if
> > we could move to the generic sycall code separately from moving the rest
> > of the entry code, as the sycall handling code is a particularly
> > important ABI concern, and it's difficult to see whether we're making
> > ABI changes (accidentaly or knowingly).
> > 
> > Can we split that up (e.g. splitting the generic code first into
> > separate entry and syscall files), or are those too tightly coupled for
> > that to be possible?
> 
> It will be hard, but I will try to split it, they are surely tightly
> coupled which make the 3th patch too big when I try to switch to generic
> entry.

I'm confused. The point I'm making is don't try to switch to *all* the
generic entry code in one go: split the 3rd patch into smaller,
logically-distinct separate steps. The 3rd patch shouldn't get larger as
you should be changing fewer lines in any individual patch.

The regular entry state management (e.g. enter_from_user_mode() and
exit_to_user_mode()) is largely separate from the syscall state
management, which is pretty clear given
syscall_enter_from_user_mode_prepare() and syscall_exit_to_user_mode()
wrap the regular entry logic:

| noinstr void syscall_enter_from_user_mode_prepare(struct pt_regs *regs)
| {
|         enter_from_user_mode(regs);
|         instrumentation_begin();
|         local_irq_enable();
|         instrumentation_end();
| }

| __visible noinstr void syscall_exit_to_user_mode(struct pt_regs *regs)
| {
|         instrumentation_begin();
|         __syscall_exit_to_user_mode_work(regs);
|         instrumentation_end();
|         exit_to_user_mode();
| }

... and while exit_to_user_mode_prepare() is called by
irqentry_exit_to_user_mode(), that's also just a wrapper around
exit_to_user_mode():

| noinstr void irqentry_exit_to_user_mode(struct pt_regs *regs)
| {
|         instrumentation_begin();
|         exit_to_user_mode_prepare(regs);
|         instrumentation_end();
|         exit_to_user_mode();
| }

... so AFAICT we could move arm64 over to enter_from_user_mode() and
exit_to_user_mode() without needing to use any of the generic syscall
logic.

Doing that first, *then* moving over to the generic syscall handling
would be much easier to review/test/bisect, and if there are any ABI
issues with the syscall handling in particular (which I think is
likely), it will be easier to handle those in isolation.

Thanks,
Mark.