diff mbox series

[2/2] x86/fred: Fix build with CONFIG_IA32_EMULATION=n

Message ID 20240127093728.1323-3-xin3.li@intel.com
State New
Headers
Series x86/fred: Fix two build issues |

Commit Message

Li, Xin3 Jan. 27, 2024, 9:37 a.m. UTC 
  When CONFIG_IA32_EMULATION=n, int80_emulation() is NOT defined, fix it.

Fixes: 5e0636a41485 ("x86/fred: FRED entry/exit and dispatch code")
Link: https://lore.kernel.org/lkml/20240126100519.GBZbOD3xFB0v3mp5B1@fat_crate.local/
Reported-by: Borislav Petkov (AMD) <bp@alien8.de>
Signed-off-by: Xin Li <xin3.li@intel.com>
---
 arch/x86/entry/entry_fred.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Borislav Petkov Jan. 30, 2024, 12:48 p.m. UTC | #1 
On Sat, Jan 27, 2024 at 01:37:28AM -0800, Xin Li wrote:
> When CONFIG_IA32_EMULATION=n, int80_emulation() is NOT defined, fix it.
> 
> Fixes: 5e0636a41485 ("x86/fred: FRED entry/exit and dispatch code")
> Link: https://lore.kernel.org/lkml/20240126100519.GBZbOD3xFB0v3mp5B1@fat_crate.local/
> Reported-by: Borislav Petkov (AMD) <bp@alien8.de>
> Signed-off-by: Xin Li <xin3.li@intel.com>
> ---
>  arch/x86/entry/entry_fred.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
> index 06d00c60ea64..ac120cbdaaf2 100644
> --- a/arch/x86/entry/entry_fred.c
> +++ b/arch/x86/entry/entry_fred.c
> @@ -62,11 +62,13 @@ static noinstr void fred_intx(struct pt_regs *regs)
>  	case X86_TRAP_OF:
>  		return exc_overflow(regs);
>  
> +#ifdef CONFIG_IA32_EMULATION
>  	/* INT80 */
>  	case IA32_SYSCALL_VECTOR:
>  		if (ia32_enabled())
>  			return int80_emulation(regs);
>  		fallthrough;
> +#endif
>  
>  	default:
>  		return exc_general_protection(regs, 0);
> -- 

That .config is still not happy after this:

ld: vmlinux.o: in function `fred_entry_from_user':
(.noinstr.text+0x177a): undefined reference to `do_fast_syscall_32'
make[2]: *** [scripts/Makefile.vmlinux:37: vmlinux] Error 1
make[1]: *** [/mnt/kernel/kernel/linux/Makefile:1158: vmlinux] Error 2
make: *** [Makefile:240: __sub-make] Error 2

I'm pushing the latest state I have here:

https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=tip-x86-fred

Thx.
Li, Xin3 Jan. 30, 2024, 3:22 p.m. UTC | #2 
> > When CONFIG_IA32_EMULATION=n, int80_emulation() is NOT defined, fix it.
> >
> > Fixes: 5e0636a41485 ("x86/fred: FRED entry/exit and dispatch code")
> > Link:
> > https://lore.kernel.org/lkml/20240126100519.GBZbOD3xFB0v3mp5B1@fat_cra
> > te.local/
> > Reported-by: Borislav Petkov (AMD) <bp@alien8.de>
> > Signed-off-by: Xin Li <xin3.li@intel.com>
> > ---
> >  arch/x86/entry/entry_fred.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
> > index 06d00c60ea64..ac120cbdaaf2 100644
> > --- a/arch/x86/entry/entry_fred.c
> > +++ b/arch/x86/entry/entry_fred.c
> > @@ -62,11 +62,13 @@ static noinstr void fred_intx(struct pt_regs *regs)
> >  	case X86_TRAP_OF:
> >  		return exc_overflow(regs);
> >
> > +#ifdef CONFIG_IA32_EMULATION
> >  	/* INT80 */
> >  	case IA32_SYSCALL_VECTOR:
> >  		if (ia32_enabled())
> >  			return int80_emulation(regs);
> >  		fallthrough;
> > +#endif
> >
> >  	default:
> >  		return exc_general_protection(regs, 0);
> > --
> 
> That .config is still not happy after this:
> 
> ld: vmlinux.o: in function `fred_entry_from_user':
> (.noinstr.text+0x177a): undefined reference to `do_fast_syscall_32'
> make[2]: *** [scripts/Makefile.vmlinux:37: vmlinux] Error 1
> make[1]: *** [/mnt/kernel/kernel/linux/Makefile:1158: vmlinux] Error 2
> make: *** [Makefile:240: __sub-make] Error 2

Sign, I'm sorry it still doesn’t work with GCC.

> I'm pushing the latest state I have here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=tip-x86-fred

Because clang can compile it, I checked the generated assembly code and
see that clang optimizes it by simply converting ia32_enabled() to false
when CONFIG_IA32_EMULATION=n thus never calling do_fast_syscall_32().

It looks to me that the following patch is better than adding another
#ifdef CONFIG_IA32_EMULATION around do_fast_syscall_32().

How do you think?

BTW, I have tested it with both GCC and clang with CONFIG_IA32_EMULATION=n.

Thanks!
        -Xin

diff --git a/arch/x86/include/asm/ia32.h b/arch/x86/include/asm/ia32.h
index c7ef6ea2fa99..01342d343c19 100644
--- a/arch/x86/include/asm/ia32.h
+++ b/arch/x86/include/asm/ia32.h
@@ -81,7 +81,7 @@ static inline void ia32_disable(void)

 #else /* !CONFIG_IA32_EMULATION */

-static inline bool ia32_enabled(void)
+static __always_inline bool ia32_enabled(void)
 {
 	return IS_ENABLED(CONFIG_X86_32);
 }
Borislav Petkov Jan. 30, 2024, 3:31 p.m. UTC | #3 
On Tue, Jan 30, 2024 at 03:22:01PM +0000, Li, Xin3 wrote:
> How do you think?

Interesting. For some reason gcc doesn't constant-fold it away like
clang does.

> diff --git a/arch/x86/include/asm/ia32.h b/arch/x86/include/asm/ia32.h
> index c7ef6ea2fa99..01342d343c19 100644
> --- a/arch/x86/include/asm/ia32.h
> +++ b/arch/x86/include/asm/ia32.h
> @@ -81,7 +81,7 @@ static inline void ia32_disable(void)
> 
>  #else /* !CONFIG_IA32_EMULATION */
> 
> -static inline bool ia32_enabled(void)
> +static __always_inline bool ia32_enabled(void)
>  {
>  	return IS_ENABLED(CONFIG_X86_32);
>  }

Looks good to me. Lemme try it here.

Thx.
Li, Xin3 Jan. 30, 2024, 4:30 p.m. UTC | #4 
> On Tue, Jan 30, 2024 at 03:22:01PM +0000, Li, Xin3 wrote:
> > How do you think?
> 
> Interesting. For some reason gcc doesn't constant-fold it away like clang does.

Even more interesting, gcc doesn't complain it with the attached config
File in which CONFIG_X86_FRED=y and CONFIG_IA32_EMULATION not set.

I compared the 2 config files, nothing suspicious to me.

CCing PeterZ to give insights 😊

> > diff --git a/arch/x86/include/asm/ia32.h b/arch/x86/include/asm/ia32.h
> > index c7ef6ea2fa99..01342d343c19 100644
> > --- a/arch/x86/include/asm/ia32.h
> > +++ b/arch/x86/include/asm/ia32.h
> > @@ -81,7 +81,7 @@ static inline void ia32_disable(void)
> >
> >  #else /* !CONFIG_IA32_EMULATION */
> >
> > -static inline bool ia32_enabled(void)
> > +static __always_inline bool ia32_enabled(void)
> >  {
> >  	return IS_ENABLED(CONFIG_X86_32);
> >  }
> 
> Looks good to me. Lemme try it here.

Thank you very much!
        -Xin
Borislav Petkov Jan. 30, 2024, 4:34 p.m. UTC | #5 
On Tue, Jan 30, 2024 at 04:30:34PM +0000, Li, Xin3 wrote:
> Even more interesting, gcc doesn't complain it with the attached config
> File in which CONFIG_X86_FRED=y and CONFIG_IA32_EMULATION not set.

Yes, CONFIG_IA32_EMULATION=n alone is not enough. That .config which
triggers it has something else which is causing this but I'm not sure
I want to go chase down what it is...
Borislav Petkov Jan. 30, 2024, 5:17 p.m. UTC | #6 
On Tue, Jan 30, 2024 at 03:22:01PM +0000, Li, Xin3 wrote:
> diff --git a/arch/x86/include/asm/ia32.h b/arch/x86/include/asm/ia32.h
> index c7ef6ea2fa99..01342d343c19 100644
> --- a/arch/x86/include/asm/ia32.h
> +++ b/arch/x86/include/asm/ia32.h
> @@ -81,7 +81,7 @@ static inline void ia32_disable(void)
> 
>  #else /* !CONFIG_IA32_EMULATION */
> 
> -static inline bool ia32_enabled(void)
> +static __always_inline bool ia32_enabled(void)
>  {
>  	return IS_ENABLED(CONFIG_X86_32);

So I always-inlined both versions since they're small enough, see diff
below. That works with this .config, I'll run some more overnight to
make sure...

Thx.

diff --git a/arch/x86/include/asm/ia32.h b/arch/x86/include/asm/ia32.h
index c7ef6ea2fa99..4212c00c9708 100644
--- a/arch/x86/include/asm/ia32.h
+++ b/arch/x86/include/asm/ia32.h
@@ -69,7 +69,7 @@ extern void ia32_pick_mmap_layout(struct mm_struct *mm);
 
 extern bool __ia32_enabled;
 
-static inline bool ia32_enabled(void)
+static __always_inline bool ia32_enabled(void)
 {
 	return __ia32_enabled;
 }
@@ -81,7 +81,7 @@ static inline void ia32_disable(void)
 
 #else /* !CONFIG_IA32_EMULATION */
 
-static inline bool ia32_enabled(void)
+static __always_inline bool ia32_enabled(void)
 {
 	return IS_ENABLED(CONFIG_X86_32);
 }
diff mbox series

Patch

diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
index 06d00c60ea64..ac120cbdaaf2 100644
--- a/arch/x86/entry/entry_fred.c
+++ b/arch/x86/entry/entry_fred.c
@@ -62,11 +62,13 @@  static noinstr void fred_intx(struct pt_regs *regs)
 	case X86_TRAP_OF:
 		return exc_overflow(regs);
 
+#ifdef CONFIG_IA32_EMULATION
 	/* INT80 */
 	case IA32_SYSCALL_VECTOR:
 		if (ia32_enabled())
 			return int80_emulation(regs);
 		fallthrough;
+#endif
 
 	default:
 		return exc_general_protection(regs, 0);