1. Patchew
  2. linux
  3. [PATCH v2 0/7] x86/entry: Break up common.c
  4. View patch

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

Brian Gerst posted 7 patches 9 months, 1 week ago
[PATCH v2 7/7] x86/syscall/32: Add comment to conditional
Posted by Brian Gerst 9 months, 1 week ago 
Add a CONFIG_X86_FRED comment, since this conditional is nested.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
Suggested-by: Sohil Mehta <sohil.mehta@intel.com>
---
 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 */
-- 
2.48.1
Re: [PATCH v2 7/7] x86/syscall/32: Add comment to conditional
Posted by Sohil Mehta 9 months, 1 week ago 
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@gmail.com>
> Suggested-by: Sohil Mehta <sohil.mehta@intel.com>
> ---
>  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;
Re: [PATCH v2 7/7] x86/syscall/32: Add comment to conditional
Posted by Brian Gerst 9 months, 1 week ago 
On Fri, Mar 14, 2025 at 1:19 PM Sohil Mehta <sohil.mehta@intel.com> 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@gmail.com>
> > Suggested-by: Sohil Mehta <sohil.mehta@intel.com>
> > ---
> >  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
Re: [PATCH v2 7/7] x86/syscall/32: Add comment to conditional
Posted by H. Peter Anvin 9 months, 1 week ago 
On March 14, 2025 10:43:34 AM PDT, Brian Gerst <brgerst@gmail.com> wrote:
>On Fri, Mar 14, 2025 at 1:19 PM Sohil Mehta <sohil.mehta@intel.com> 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@gmail.com>
>> > Suggested-by: Sohil Mehta <sohil.mehta@intel.com>
>> > ---
>> >  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
>

That function gets really, really ugly. The reason we factored out the FRED code is that the number of conditionals ended up grotesque, *and* that they are part of completely separate flows – they should have separate every points, not dynamic conditionals.
[tip: x86/core] x86/syscall/32: Add comment to conditional
Posted by tip-bot2 for Brian Gerst 9 months ago 
The following commit has been merged into the x86/core branch of tip:

Commit-ID:     82070bc0425db949b406ede1c7f066346f5b3eb9
Gitweb:        https://git.kernel.org/tip/82070bc0425db949b406ede1c7f066346f5b3eb9
Author:        Brian Gerst <brgerst@gmail.com>
AuthorDate:    Fri, 14 Mar 2025 11:12:20 -04:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 19 Mar 2025 11:19:20 +01:00

x86/syscall/32: Add comment to conditional

Add a CONFIG_X86_FRED comment, since this conditional is nested.

Suggested-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Brian Gerst <brgerst@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Link: https://lore.kernel.org/r/20250314151220.862768-8-brgerst@gmail.com
---
 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 993d725..2b15ea1 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 */
[tip: x86/cpu] x86/syscall/32: Add comment to conditional
Posted by tip-bot2 for Brian Gerst 9 months ago 
The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     fe35c87ad8e7795e1ab020ce2023e952806353d0
Gitweb:        https://git.kernel.org/tip/fe35c87ad8e7795e1ab020ce2023e952806353d0
Author:        Brian Gerst <brgerst@gmail.com>
AuthorDate:    Fri, 14 Mar 2025 11:12:20 -04:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Sun, 16 Mar 2025 11:49:42 +01:00

x86/syscall/32: Add comment to conditional

Add a CONFIG_X86_FRED comment, since this conditional is nested.

Suggested-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Brian Gerst <brgerst@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Sohil Mehta <sohil.mehta@intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Link: https://lore.kernel.org/r/20250314151220.862768-8-brgerst@gmail.com
---
 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 993d725..2b15ea1 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 */

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.