Re: [PATCH 2/5] x86/syscall/32: Move 32-bit syscall dispatch code

Brian Gerst
2025-03-14T01:25:20Z

On Thu, Mar 13, 2025 at 7:45 PM Sohil Mehta <sohil.mehta@intel.com> wrote:
>
> On 3/13/2025 11:22 AM, Brian Gerst wrote:
>
> > SYSCALL_DEFINE0(ni_syscall)
> > {
> > return -ENOSYS;
> > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c
> > index 8cc9950d7104..7c286e89fd04 100644
> > --- a/arch/x86/entry/syscall_32.c
> > +++ b/arch/x86/entry/syscall_32.c
> > @@ -1,10 +1,23 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -/* System call table for i386. */
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * 32-bit system call dispatch
> > + *
> > + * Copyright (c) 2015 Andrew Lutomirski
> > + *
> > + * Based on asm and ptrace code by many authors. The code here originated
> > + * in ptrace.c and signal.c.
>
> Wondering if we can skip copying over some of these old comments? As the
> file grows, it's hard to discerne what specific code originated where.
> For example, the FRED code is all new.

I wasn't sure what the appropriate thing would be so I just copied it
over. The git history is probably a better way to attribute this
though.

>
> > +#ifdef CONFIG_IA32_EMULATION
> > +static __always_inline bool int80_is_external(void)
> > +{
> > + const unsigned int offs = (0x80 / 32) * 0x10;
> > + const u32 bit = BIT(0x80 % 32);
> > +
> > + /* The local APIC on XENPV guests is fake */
> > + if (cpu_feature_enabled(X86_FEATURE_XENPV))
> > + return false;
> > +
> > + /*
> > + * If vector 0x80 is set in the APIC ISR then this is an external
> > + * interrupt. Either from broken hardware or injected by a VMM.
> > + *
> > + * Note: In guest mode this is only valid for secure guests where
> > + * the secure module fully controls the vAPIC exposed to the guest.
> > + */
> > + return apic_read(APIC_ISR + offs) & bit;
> > +}
> > +
> > +/**
> > + * do_int80_emulation - 32-bit legacy syscall C entry from asm
> > + * @regs: syscall arguments in struct pt_args on the stack.
> > + *
> > + * This entry point can be used by 32-bit and 64-bit programs to perform
> > + * 32-bit system calls. Instances of INT $0x80 can be found inline in
> > + * various programs and libraries. It is also used by the vDSO's
> > + * __kernel_vsyscall fallback for hardware that doesn't support a faster
> > + * entry method. Restarted 32-bit system calls also fall back to INT
> > + * $0x80 regardless of what instruction was originally used to do the
> > + * system call.
> > + *
> > + * This is considered a slow path. It is not used by most libc
> > + * implementations on modern hardware except during process startup.
> > + *
> > + * The arguments for the INT $0x80 based syscall are on stack in the
> > + * pt_regs structure:
> > + * eax: system call number
> > + * ebx, ecx, edx, esi, edi, ebp: arg1 - arg 6
> > + */
> > +__visible noinstr void do_int80_emulation(struct pt_regs *regs)
> > +{
> > + int nr;
> > +
> > + /* Kernel does not use INT $0x80! */
> > + if (unlikely(!user_mode(regs))) {
> > + irqentry_enter(regs);
> > + instrumentation_begin();
> > + panic("Unexpected external interrupt 0x80\n");
> > + }
> > +
> > + /*
> > + * Establish kernel context for instrumentation, including for
> > + * int80_is_external() below which calls into the APIC driver.
> > + * Identical for soft and external interrupts.
> > + */
> > + enter_from_user_mode(regs);
> > +
> > + instrumentation_begin();
> > + add_random_kstack_offset();
> > +
> > + /* Validate that this is a soft interrupt to the extent possible */
> > + if (unlikely(int80_is_external()))
> > + panic("Unexpected external interrupt 0x80\n");
> > +
> > + /*
> > + * The low level idtentry code pushed -1 into regs::orig_ax
> > + * and regs::ax contains the syscall number.
> > + *
> > + * User tracing code (ptrace or signal handlers) might assume
> > + * that the regs::orig_ax contains a 32-bit number on invoking
> > + * a 32-bit syscall.
> > + *
> > + * Establish the syscall convention by saving the 32bit truncated
> > + * syscall number in regs::orig_ax and by invalidating regs::ax.
> > + */
> > + regs->orig_ax = regs->ax & GENMASK(31, 0);
> > + regs->ax = -ENOSYS;
> > +
> > + nr = syscall_32_enter(regs);
> > +
> > + local_irq_enable();
> > + nr = syscall_enter_from_user_mode_work(regs, nr);
> > + do_syscall_32_irqs_on(regs, nr);
> > +
> > + instrumentation_end();
> > + syscall_exit_to_user_mode(regs);
> > +}
> > +
> > +#ifdef CONFIG_X86_FRED
> > +/*
> > + * A FRED-specific INT80 handler is warranted for the follwing reasons:
> > + *
> > + * 1) As INT instructions and hardware interrupts are separate event
> > + * types, FRED does not preclude the use of vector 0x80 for external
> > + * interrupts. As a result, the FRED setup code does not reserve
> > + * vector 0x80 and calling int80_is_external() is not merely
> > + * suboptimal but actively incorrect: it could cause a system call
> > + * to be incorrectly ignored.
> > + *
> > + * 2) It is called only for handling vector 0x80 of event type
> > + * EVENT_TYPE_SWINT and will never be called to handle any external
> > + * interrupt (event type EVENT_TYPE_EXTINT).
> > + *
> > + * 3) FRED has separate entry flows depending on if the event came from
> > + * user space or kernel space, and because the kernel does not use
> > + * INT insns, the FRED kernel entry handler fred_entry_from_kernel()
> > + * falls through to fred_bad_type() if the event type is
> > + * EVENT_TYPE_SWINT, i.e., INT insns. So if the kernel is handling
> > + * an INT insn, it can only be from a user level.
> > + *
> > + * 4) int80_emulation() does a CLEAR_BRANCH_HISTORY. While FRED will
> > + * likely take a different approach if it is ever needed: it
> > + * probably belongs in either fred_intx()/ fred_other() or
> > + * asm_fred_entrypoint_user(), depending on if this ought to be done
> > + * for all entries from userspace or only system
> > + * calls.
> > + *
> > + * 5) INT $0x80 is the fast path for 32-bit system calls under FRED.
> > + */
> > +DEFINE_FREDENTRY_RAW(int80_emulation)
> > +{
> > + int nr;
> > +
> > + enter_from_user_mode(regs);
> > +
> > + instrumentation_begin();
> > + add_random_kstack_offset();
> > +
> > + /*
> > + * FRED pushed 0 into regs::orig_ax and regs::ax contains the
> > + * syscall number.
> > + *
> > + * User tracing code (ptrace or signal handlers) might assume
> > + * that the regs::orig_ax contains a 32-bit number on invoking
> > + * a 32-bit syscall.
> > + *
> > + * Establish the syscall convention by saving the 32bit truncated
> > + * syscall number in regs::orig_ax and by invalidating regs::ax.
> > + */
> > + regs->orig_ax = regs->ax & GENMASK(31, 0);
> > + regs->ax = -ENOSYS;
> > +
> > + nr = syscall_32_enter(regs);
> > +
> > + local_irq_enable();
> > + nr = syscall_enter_from_user_mode_work(regs, nr);
> > + do_syscall_32_irqs_on(regs, nr);
> > +
> > + instrumentation_end();
> > + syscall_exit_to_user_mode(regs);
> > +}
> > +#endif
>
> Nit: Would it be useful to add /* CONFIG_X86_FRED */ here since there
> are nested #ifdefs?

These patches should be as close to a copy and paste as possible, so
that it's easy to prove that nothing changes. This could be a
followup patch later though.

Brian Gerst