Re: [PATCH v2 7/7] x86/syscall/32: Add comment to conditional

[Brian Gerst]

On Fri, Mar 14, 2025 at 1:19 PM Sohil Mehta <sohil.mehta@xxxxxxxxx> wrote:
>
> On 3/14/2025 8:12 AM, Brian Gerst wrote:
> > Add a CONFIG_X86_FRED comment, since this conditional is nested.
> >
> > Signed-off-by: Brian Gerst <brgerst@xxxxxxxxx>
> > Suggested-by: Sohil Mehta <sohil.mehta@xxxxxxxxx>
> > ---
> >  arch/x86/entry/syscall_32.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c
> > index 993d72504fc5..2b15ea17bb7c 100644
> > --- a/arch/x86/entry/syscall_32.c
> > +++ b/arch/x86/entry/syscall_32.c
> > @@ -238,7 +238,8 @@ DEFINE_FREDENTRY_RAW(int80_emulation)
> >       instrumentation_end();
> >       syscall_exit_to_user_mode(regs);
> >  }
> > -#endif
> > +#endif /* CONFIG_X86_FRED */
> > +
> >  #else /* CONFIG_IA32_EMULATION */
> >
> >  /* Handles int $0x80 on a 32bit kernel */
>
> Also, there seem to be a couple of adjacent CONFIG_IA32_EMULATION
> sections in the file. It might be better to bring them together under
> one section in this patch or a follow-up. Something like below:
>
> > diff --git a/arch/x86/entry/syscall_32.c b/arch/x86/entry/syscall_32.c
> > index 2b15ea17bb7c..a84f9d3c1695 100644
> > --- a/arch/x86/entry/syscall_32.c
> > +++ b/arch/x86/entry/syscall_32.c
> > @@ -57,16 +57,6 @@ static __always_inline int syscall_32_enter(struct pt_regs *regs)
> >         return (int)regs->orig_ax;
> >  }
> >
> > -#ifdef CONFIG_IA32_EMULATION
> > -bool __ia32_enabled __ro_after_init = !IS_ENABLED(CONFIG_IA32_EMULATION_DEFAULT_DISABLED);
> > -
> > -static int __init ia32_emulation_override_cmdline(char *arg)
> > -{
> > -       return kstrtobool(arg, &__ia32_enabled);
> > -}
> > -early_param("ia32_emulation", ia32_emulation_override_cmdline);
> > -#endif
> > -
> >  /*
> >   * Invoke a 32-bit syscall.  Called with IRQs on in CT_STATE_KERNEL.
> >   */
> > @@ -87,6 +77,14 @@ static __always_inline void do_syscall_32_irqs_on(struct pt_regs *regs, int nr)
> >  }
> >
> >  #ifdef CONFIG_IA32_EMULATION
> > +bool __ia32_enabled __ro_after_init = !IS_ENABLED(CONFIG_IA32_EMULATION_DEFAULT_DISABLED);
> > +
> > +static int __init ia32_emulation_override_cmdline(char *arg)
> > +{
> > +       return kstrtobool(arg, &__ia32_enabled);
> > +}
> > +early_param("ia32_emulation", ia32_emulation_override_cmdline);
> > +
> >  static __always_inline bool int80_is_external(void)
> >  {
> >         const unsigned int offs = (0x80 / 32) * 0x10;

The 32-bit syscall code is a bit of a mess right now, and it was hard
to see its state while it was lumped in with everything else in
common.c.  I'd like to see the INT80 code merged back into one
function, but that will probably have to wait until the dust settles
with FRED.


Brian Gerst