All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
@ 2022-11-04 12:45 Uros Bizjak
  2022-11-28 22:20 ` Borislav Petkov
  2022-11-29 19:44 ` [tip: x86/boot] x86/boot: Remove x86_32 PIC using %ebx workaround tip-bot2 for Uros Bizjak
  0 siblings, 2 replies; 13+ messages in thread
From: Uros Bizjak @ 2022-11-04 12:45 UTC (permalink / raw)
  To: x86, linux-kernel
  Cc: Uros Bizjak, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H. Peter Anvin

Current minimum required version of GCC is version 5.1 which allows
reuse of PIC hard register on x86/x86-64 targets [1]. Remove
obsolete workaround that was needed for earlier GCC versions.

[1] https://gcc.gnu.org/gcc-5/changes.html

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
---
 arch/x86/boot/cpuflags.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
index a83d67ec627d..d75237ba7ce9 100644
--- a/arch/x86/boot/cpuflags.c
+++ b/arch/x86/boot/cpuflags.c
@@ -64,20 +64,11 @@ int has_eflag(unsigned long mask)
 	return !!((f0^f1) & mask);
 }
 
-/* Handle x86_32 PIC using ebx. */
-#if defined(__i386__) && defined(__PIC__)
-# define EBX_REG "=r"
-#else
-# define EBX_REG "=b"
-#endif
-
 void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
 {
-	asm volatile(".ifnc %%ebx,%3 ; movl  %%ebx,%3 ; .endif	\n\t"
-		     "cpuid					\n\t"
-		     ".ifnc %%ebx,%3 ; xchgl %%ebx,%3 ; .endif	\n\t"
-		    : "=a" (*a), "=c" (*c), "=d" (*d), EBX_REG (*b)
-		    : "a" (id), "c" (count)
+	asm volatile("cpuid"
+		     : "=a" (*a), "=b" (*b), "=c" (*c), "=d" (*d)
+		     : "0" (id), "2" (count)
 	);
 }
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-04 12:45 [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround Uros Bizjak
@ 2022-11-28 22:20 ` Borislav Petkov
  2022-11-29  3:41   ` Brian Gerst
  2022-11-29  7:39   ` Uros Bizjak
  2022-11-29 19:44 ` [tip: x86/boot] x86/boot: Remove x86_32 PIC using %ebx workaround tip-bot2 for Uros Bizjak
  1 sibling, 2 replies; 13+ messages in thread
From: Borislav Petkov @ 2022-11-28 22:20 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	H. Peter Anvin

On Fri, Nov 04, 2022 at 01:45:46PM +0100, Uros Bizjak wrote:
> Current minimum required version of GCC is version 5.1 which allows
> reuse of PIC hard register on x86/x86-64 targets [1]. Remove
> obsolete workaround that was needed for earlier GCC versions.
> 
> [1] https://gcc.gnu.org/gcc-5/changes.html

Thanks for the doc pointer.

Lemme see if I understand this commit message correctly:

SysV i386 ABI says that %ebx is used as the base reg in PIC. gcc 5 and
newer can handle all possible cases properly where inline asm could
clobber the PIC reg. I.e., it is able to deal with the "=b" constraint
where an insn can overwrite %ebx and it'll push and pop around that
statement.

So far so good.

Why then does this matter for x86-64 where PIC addressing is done
rip-relative so %rbx is normal reg there?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-28 22:20 ` Borislav Petkov
@ 2022-11-29  3:41   ` Brian Gerst
  2022-11-29  8:32     ` Borislav Petkov
  2022-11-29  7:39   ` Uros Bizjak
  1 sibling, 1 reply; 13+ messages in thread
From: Brian Gerst @ 2022-11-29  3:41 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Uros Bizjak, x86, linux-kernel, Thomas Gleixner, Ingo Molnar,
	Dave Hansen, H. Peter Anvin

On Mon, Nov 28, 2022 at 5:57 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Fri, Nov 04, 2022 at 01:45:46PM +0100, Uros Bizjak wrote:
> > Current minimum required version of GCC is version 5.1 which allows
> > reuse of PIC hard register on x86/x86-64 targets [1]. Remove
> > obsolete workaround that was needed for earlier GCC versions.
> >
> > [1] https://gcc.gnu.org/gcc-5/changes.html
>
> Thanks for the doc pointer.
>
> Lemme see if I understand this commit message correctly:
>
> SysV i386 ABI says that %ebx is used as the base reg in PIC. gcc 5 and
> newer can handle all possible cases properly where inline asm could
> clobber the PIC reg. I.e., it is able to deal with the "=b" constraint
> where an insn can overwrite %ebx and it'll push and pop around that
> statement.
>
> So far so good.
>
> Why then does this matter for x86-64 where PIC addressing is done
> rip-relative so %rbx is normal reg there?

x86-64 uses a PIC register for the medium and large PIC code models,
where offsets can be larger than +/- 2GB.

--
Brian Gerst

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-28 22:20 ` Borislav Petkov
  2022-11-29  3:41   ` Brian Gerst
@ 2022-11-29  7:39   ` Uros Bizjak
  2022-11-29  8:31     ` Borislav Petkov
  1 sibling, 1 reply; 13+ messages in thread
From: Uros Bizjak @ 2022-11-29  7:39 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	H. Peter Anvin

On Mon, Nov 28, 2022 at 11:20 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Fri, Nov 04, 2022 at 01:45:46PM +0100, Uros Bizjak wrote:
> > Current minimum required version of GCC is version 5.1 which allows
> > reuse of PIC hard register on x86/x86-64 targets [1]. Remove
> > obsolete workaround that was needed for earlier GCC versions.
> >
> > [1] https://gcc.gnu.org/gcc-5/changes.html
>
> Thanks for the doc pointer.
>
> Lemme see if I understand this commit message correctly:
>
> SysV i386 ABI says that %ebx is used as the base reg in PIC. gcc 5 and
> newer can handle all possible cases properly where inline asm could
> clobber the PIC reg. I.e., it is able to deal with the "=b" constraint
> where an insn can overwrite %ebx and it'll push and pop around that
> statement.

gcc-5 considers PIC register as a pseudo-register and reloads it
before instruction that requires the value in %ebx. This way, PIC
register is no more special and does not need any special handling.
This includes inline asm which can clobber %ebx.
>
> So far so good.
>
> Why then does this matter for x86-64 where PIC addressing is done
> rip-relative so %rbx is normal reg there?

x86_64 does not use PIC register for small code models. Also, it uses
%r15 instead of %rbx for PIC register, so the removed workaround
applies only to x86_32.

Uros.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-29  7:39   ` Uros Bizjak
@ 2022-11-29  8:31     ` Borislav Petkov
  2022-11-29  8:50       ` Uros Bizjak
  0 siblings, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2022-11-29  8:31 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	H. Peter Anvin

On Tue, Nov 29, 2022 at 08:39:23AM +0100, Uros Bizjak wrote:
> On Mon, Nov 28, 2022 at 11:20 PM Borislav Petkov <bp@alien8.de> wrote:
> >
> > On Fri, Nov 04, 2022 at 01:45:46PM +0100, Uros Bizjak wrote:
> > > Current minimum required version of GCC is version 5.1 which allows
> > > reuse of PIC hard register on x86/x86-64 targets [1]. Remove
> > > obsolete workaround that was needed for earlier GCC versions.
> > >
> > > [1] https://gcc.gnu.org/gcc-5/changes.html
> >
> > Thanks for the doc pointer.
> >
> > Lemme see if I understand this commit message correctly:
> >
> > SysV i386 ABI says that %ebx is used as the base reg in PIC. gcc 5 and
> > newer can handle all possible cases properly where inline asm could
> > clobber the PIC reg. I.e., it is able to deal with the "=b" constraint
> > where an insn can overwrite %ebx and it'll push and pop around that
> > statement.
> 
> gcc-5 considers PIC register as a pseudo-register and reloads it

So not a "hard" register as you say above?

> x86_64 does not use PIC register for small code models. Also, it uses
> %r15 instead of %rbx for PIC register, so the removed workaround
> applies only to x86_32.

Let's see:

arch/x86/Makefile:
        # Never want PIC in a 32-bit kernel, prevent breakage with GCC built
        # with nonstandard options
        KBUILD_CFLAGS += -fno-pic

$ gcc -Wp,-MMD,arch/x86/boot/.cpuflags.o.d ... -fno-pic ... -D__KBUILD_MODNAME=kmod_cpuflags -c -o arch/x86/boot/cpuflags.o arch/x86/boot/cpuflags.c

So this workaround applies to nothing, I'd say. :)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-29  3:41   ` Brian Gerst
@ 2022-11-29  8:32     ` Borislav Petkov
  2022-11-29 13:24       ` Brian Gerst
  0 siblings, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2022-11-29  8:32 UTC (permalink / raw)
  To: Brian Gerst
  Cc: Uros Bizjak, x86, linux-kernel, Thomas Gleixner, Ingo Molnar,
	Dave Hansen, H. Peter Anvin

On Mon, Nov 28, 2022 at 10:41:11PM -0500, Brian Gerst wrote:
> x86-64 uses a PIC register for the medium and large PIC code models,
> where offsets can be larger than +/- 2GB.

Right but 64-bit is built with -mcmodel=kernel which obviously generates
rip-relative.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-29  8:31     ` Borislav Petkov
@ 2022-11-29  8:50       ` Uros Bizjak
  2022-11-29 14:46         ` Borislav Petkov
  0 siblings, 1 reply; 13+ messages in thread
From: Uros Bizjak @ 2022-11-29  8:50 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	H. Peter Anvin

On Tue, Nov 29, 2022 at 9:31 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Tue, Nov 29, 2022 at 08:39:23AM +0100, Uros Bizjak wrote:
> > On Mon, Nov 28, 2022 at 11:20 PM Borislav Petkov <bp@alien8.de> wrote:
> > >
> > > On Fri, Nov 04, 2022 at 01:45:46PM +0100, Uros Bizjak wrote:
> > > > Current minimum required version of GCC is version 5.1 which allows
> > > > reuse of PIC hard register on x86/x86-64 targets [1]. Remove
> > > > obsolete workaround that was needed for earlier GCC versions.
> > > >
> > > > [1] https://gcc.gnu.org/gcc-5/changes.html
> > >
> > > Thanks for the doc pointer.
> > >
> > > Lemme see if I understand this commit message correctly:
> > >
> > > SysV i386 ABI says that %ebx is used as the base reg in PIC. gcc 5 and
> > > newer can handle all possible cases properly where inline asm could
> > > clobber the PIC reg. I.e., it is able to deal with the "=b" constraint
> > > where an insn can overwrite %ebx and it'll push and pop around that
> > > statement.
> >
> > gcc-5 considers PIC register as a pseudo-register and reloads it
>
> So not a "hard" register as you say above?

There are registers of different "hardness" as far as gcc is
concerned. Before gcc-5.1, PIC register was considered as "fixed", it
was simply unavailable to a register allocator, decreasing the
miniscule available x86_32 register set by one register. Also, RA was
unable to satisfy the "=b" constraint. OTOH, a pseudo register belongs
to a certain class of registers (e.g. integer registers) and when
allocating real ("hard" ) registers, RA uses instruction (or inline
asm) register constraints to allocate correct "hard" register. In this
aspect, PIC access "requires" %ebx register in the same way as e.g.
shift instructions "require" %cl register. gcc-5.1 allows RA much more
freedom of how to use %ebx register - it can be used for other
purposes , as long as it is reloaded with correct value before insn
with PIC access.

> > x86_64 does not use PIC register for small code models. Also, it uses
> > %r15 instead of %rbx for PIC register, so the removed workaround
> > applies only to x86_32.
>
> Let's see:
>
> arch/x86/Makefile:
>         # Never want PIC in a 32-bit kernel, prevent breakage with GCC built
>         # with nonstandard options
>         KBUILD_CFLAGS += -fno-pic
>
> $ gcc -Wp,-MMD,arch/x86/boot/.cpuflags.o.d ... -fno-pic ... -D__KBUILD_MODNAME=kmod_cpuflags -c -o arch/x86/boot/cpuflags.o arch/x86/boot/cpuflags.c
>
> So this workaround applies to nothing, I'd say. :)

It looks like the workaround serves no purpose even when compiled with
gcc < 5.1.

Uros.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-29  8:32     ` Borislav Petkov
@ 2022-11-29 13:24       ` Brian Gerst
  2022-11-29 13:26         ` Uros Bizjak
  0 siblings, 1 reply; 13+ messages in thread
From: Brian Gerst @ 2022-11-29 13:24 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Uros Bizjak, x86, linux-kernel, Thomas Gleixner, Ingo Molnar,
	Dave Hansen, H. Peter Anvin

On Tue, Nov 29, 2022 at 3:32 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Mon, Nov 28, 2022 at 10:41:11PM -0500, Brian Gerst wrote:
> > x86-64 uses a PIC register for the medium and large PIC code models,
> > where offsets can be larger than +/- 2GB.
>
> Right but 64-bit is built with -mcmodel=kernel which obviously generates
> rip-relative.

Correct.  He quoted the specific part of the GCC changelog that fixed
the issue for the 32-bit kernel, but it's not applicable to the 64-bit
kernel.  Perhaps the commit message should be more apparent that it's
quoting the GCC changelog verbatim, or better yet, reference the
specific commit or bug number instead of the general release notes.

--
Brian Gerst

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-29 13:24       ` Brian Gerst
@ 2022-11-29 13:26         ` Uros Bizjak
  0 siblings, 0 replies; 13+ messages in thread
From: Uros Bizjak @ 2022-11-29 13:26 UTC (permalink / raw)
  To: Brian Gerst
  Cc: Borislav Petkov, x86, linux-kernel, Thomas Gleixner, Ingo Molnar,
	Dave Hansen, H. Peter Anvin

On Tue, Nov 29, 2022 at 2:24 PM Brian Gerst <brgerst@gmail.com> wrote:
>
> On Tue, Nov 29, 2022 at 3:32 AM Borislav Petkov <bp@alien8.de> wrote:
> >
> > On Mon, Nov 28, 2022 at 10:41:11PM -0500, Brian Gerst wrote:
> > > x86-64 uses a PIC register for the medium and large PIC code models,
> > > where offsets can be larger than +/- 2GB.
> >
> > Right but 64-bit is built with -mcmodel=kernel which obviously generates
> > rip-relative.
>
> Correct.  He quoted the specific part of the GCC changelog that fixed
> the issue for the 32-bit kernel, but it's not applicable to the 64-bit
> kernel.

It says so right in the summary.

Uros.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-29  8:50       ` Uros Bizjak
@ 2022-11-29 14:46         ` Borislav Petkov
  2022-11-29 14:56           ` Uros Bizjak
  0 siblings, 1 reply; 13+ messages in thread
From: Borislav Petkov @ 2022-11-29 14:46 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	H. Peter Anvin

Ok,

I tried to summarize what we talked on the thread and have the whole
deal explained in greater detail.

Holler if something's missing:

---
From: Uros Bizjak <ubizjak@gmail.com>
Date: Fri, 4 Nov 2022 13:45:46 +0100
Subject: [PATCH] x86/boot: Remove x86_32 PIC using %ebx workaround

The currently supported minimum gcc version is 5.1. Before that, the
PIC register, when generating Position Independent Code, was considered
"fixed" in the sense that it wasn't in the set of registers available to
the compiler's register allocator. Which, on x86-32, is already a very
small set.

What is more, the register allocator was unable to satisfy extended asm
"=b" constraints. (Yes, PIC code uses %ebx on 32-bit as the base reg.)

With gcc5:

"Reuse of the PIC hard register, instead of using a fixed register,
was implemented on x86/x86-64 targets. This improves generated PIC
code performance as more hard registers can be used. Shared libraries
can significantly benefit from this optimization. Currently it is
switched on only for x86/x86-64 targets. As RA infrastructure is already
implemented for PIC register reuse, other targets might follow this in
the future."

  (from: https://gcc.gnu.org/gcc-5/changes.html)

which basically means that the register allocator has a higher degree
of freedom when handling %ebx, including reloading it with the correct
value before a PIC access.

Furthermore:

  arch/x86/Makefile:
          # Never want PIC in a 32-bit kernel, prevent breakage with GCC built
          # with nonstandard options
          KBUILD_CFLAGS += -fno-pic

  $ gcc -Wp,-MMD,arch/x86/boot/.cpuflags.o.d ... -fno-pic ... -D__KBUILD_MODNAME=kmod_cpuflags -c -o arch/x86/boot/cpuflags.o arch/x86/boot/cpuflags.c

so the 32-bit workaround in cpuid_count() is fixing exactly nothing
because 32-bit configs don't even allow PIC builds.

As to 64-bit builds: they're done using -mcmodel=kernel which produces
RIP-relative addressing for PIC builds and thus does not apply here
either.

So get rid of the thing and make cpuid_count() nice and simple.

There should be no functional changes resulting from this.

  [ bp: Expand commit message. ]

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lore.kernel.org/r/20221104124546.196077-1-ubizjak@gmail.com
---
 arch/x86/boot/cpuflags.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
index a83d67ec627d..d75237ba7ce9 100644
--- a/arch/x86/boot/cpuflags.c
+++ b/arch/x86/boot/cpuflags.c
@@ -64,20 +64,11 @@ int has_eflag(unsigned long mask)
 	return !!((f0^f1) & mask);
 }
 
-/* Handle x86_32 PIC using ebx. */
-#if defined(__i386__) && defined(__PIC__)
-# define EBX_REG "=r"
-#else
-# define EBX_REG "=b"
-#endif
-
 void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
 {
-	asm volatile(".ifnc %%ebx,%3 ; movl  %%ebx,%3 ; .endif	\n\t"
-		     "cpuid					\n\t"
-		     ".ifnc %%ebx,%3 ; xchgl %%ebx,%3 ; .endif	\n\t"
-		    : "=a" (*a), "=c" (*c), "=d" (*d), EBX_REG (*b)
-		    : "a" (id), "c" (count)
+	asm volatile("cpuid"
+		     : "=a" (*a), "=b" (*b), "=c" (*c), "=d" (*d)
+		     : "0" (id), "2" (count)
 	);
 }
 
-- 
2.35.1

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-29 14:46         ` Borislav Petkov
@ 2022-11-29 14:56           ` Uros Bizjak
  2022-11-29 15:27             ` Borislav Petkov
  0 siblings, 1 reply; 13+ messages in thread
From: Uros Bizjak @ 2022-11-29 14:56 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	H. Peter Anvin

On Tue, Nov 29, 2022 at 3:46 PM Borislav Petkov <bp@alien8.de> wrote:
>
> Ok,
>
> I tried to summarize what we talked on the thread and have the whole
> deal explained in greater detail.
>
> Holler if something's missing:
>
> ---
> From: Uros Bizjak <ubizjak@gmail.com>
> Date: Fri, 4 Nov 2022 13:45:46 +0100
> Subject: [PATCH] x86/boot: Remove x86_32 PIC using %ebx workaround
>
> The currently supported minimum gcc version is 5.1. Before that, the
> PIC register, when generating Position Independent Code, was considered
> "fixed" in the sense that it wasn't in the set of registers available to
> the compiler's register allocator. Which, on x86-32, is already a very
> small set.
>
> What is more, the register allocator was unable to satisfy extended asm
> "=b" constraints. (Yes, PIC code uses %ebx on 32-bit as the base reg.)
>
> With gcc5:

Please say gcc 5.1 here.

Otherwise a really comprehensive and detailed explanation of the issue!

Uros.

>
> "Reuse of the PIC hard register, instead of using a fixed register,
> was implemented on x86/x86-64 targets. This improves generated PIC
> code performance as more hard registers can be used. Shared libraries
> can significantly benefit from this optimization. Currently it is
> switched on only for x86/x86-64 targets. As RA infrastructure is already
> implemented for PIC register reuse, other targets might follow this in
> the future."
>
>   (from: https://gcc.gnu.org/gcc-5/changes.html)
>
> which basically means that the register allocator has a higher degree
> of freedom when handling %ebx, including reloading it with the correct
> value before a PIC access.
>
> Furthermore:
>
>   arch/x86/Makefile:
>           # Never want PIC in a 32-bit kernel, prevent breakage with GCC built
>           # with nonstandard options
>           KBUILD_CFLAGS += -fno-pic
>
>   $ gcc -Wp,-MMD,arch/x86/boot/.cpuflags.o.d ... -fno-pic ... -D__KBUILD_MODNAME=kmod_cpuflags -c -o arch/x86/boot/cpuflags.o arch/x86/boot/cpuflags.c
>
> so the 32-bit workaround in cpuid_count() is fixing exactly nothing
> because 32-bit configs don't even allow PIC builds.
>
> As to 64-bit builds: they're done using -mcmodel=kernel which produces
> RIP-relative addressing for PIC builds and thus does not apply here
> either.
>
> So get rid of the thing and make cpuid_count() nice and simple.
>
> There should be no functional changes resulting from this.
>
>   [ bp: Expand commit message. ]
>
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Link: https://lore.kernel.org/r/20221104124546.196077-1-ubizjak@gmail.com
> ---
>  arch/x86/boot/cpuflags.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
> index a83d67ec627d..d75237ba7ce9 100644
> --- a/arch/x86/boot/cpuflags.c
> +++ b/arch/x86/boot/cpuflags.c
> @@ -64,20 +64,11 @@ int has_eflag(unsigned long mask)
>         return !!((f0^f1) & mask);
>  }
>
> -/* Handle x86_32 PIC using ebx. */
> -#if defined(__i386__) && defined(__PIC__)
> -# define EBX_REG "=r"
> -#else
> -# define EBX_REG "=b"
> -#endif
> -
>  void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
>  {
> -       asm volatile(".ifnc %%ebx,%3 ; movl  %%ebx,%3 ; .endif  \n\t"
> -                    "cpuid                                     \n\t"
> -                    ".ifnc %%ebx,%3 ; xchgl %%ebx,%3 ; .endif  \n\t"
> -                   : "=a" (*a), "=c" (*c), "=d" (*d), EBX_REG (*b)
> -                   : "a" (id), "c" (count)
> +       asm volatile("cpuid"
> +                    : "=a" (*a), "=b" (*b), "=c" (*c), "=d" (*d)
> +                    : "0" (id), "2" (count)
>         );
>  }
>
> --
> 2.35.1
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround
  2022-11-29 14:56           ` Uros Bizjak
@ 2022-11-29 15:27             ` Borislav Petkov
  0 siblings, 0 replies; 13+ messages in thread
From: Borislav Petkov @ 2022-11-29 15:27 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Dave Hansen,
	H. Peter Anvin

On Tue, Nov 29, 2022 at 03:56:37PM +0100, Uros Bizjak wrote:
> Please say gcc 5.1 here.

Done.

> Otherwise a really comprehensive and detailed explanation of the issue!

Thanks.

Lemme run it through the build tests just in case.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [tip: x86/boot] x86/boot: Remove x86_32 PIC using %ebx workaround
  2022-11-04 12:45 [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround Uros Bizjak
  2022-11-28 22:20 ` Borislav Petkov
@ 2022-11-29 19:44 ` tip-bot2 for Uros Bizjak
  1 sibling, 0 replies; 13+ messages in thread
From: tip-bot2 for Uros Bizjak @ 2022-11-29 19:44 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Uros Bizjak, Borislav Petkov, x86, linux-kernel

The following commit has been merged into the x86/boot branch of tip:

Commit-ID:     60253f100c5846029f1370e51be6ebaeb160dcec
Gitweb:        https://git.kernel.org/tip/60253f100c5846029f1370e51be6ebaeb160dcec
Author:        Uros Bizjak <ubizjak@gmail.com>
AuthorDate:    Fri, 04 Nov 2022 13:45:46 +01:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Tue, 29 Nov 2022 16:26:53 +01:00

x86/boot: Remove x86_32 PIC using %ebx workaround

The currently supported minimum gcc version is 5.1. Before that, the
PIC register, when generating Position Independent Code, was considered
"fixed" in the sense that it wasn't in the set of registers available to
the compiler's register allocator. Which, on x86-32, is already a very
small set.

What is more, the register allocator was unable to satisfy extended asm
"=b" constraints. (Yes, PIC code uses %ebx on 32-bit as the base reg.)

With gcc 5.1:

"Reuse of the PIC hard register, instead of using a fixed register,
was implemented on x86/x86-64 targets. This improves generated PIC
code performance as more hard registers can be used. Shared libraries
can significantly benefit from this optimization. Currently it is
switched on only for x86/x86-64 targets. As RA infrastructure is already
implemented for PIC register reuse, other targets might follow this in
the future."

  (from: https://gcc.gnu.org/gcc-5/changes.html)

which basically means that the register allocator has a higher degree
of freedom when handling %ebx, including reloading it with the correct
value before a PIC access.

Furthermore:

  arch/x86/Makefile:
          # Never want PIC in a 32-bit kernel, prevent breakage with GCC built
          # with nonstandard options
          KBUILD_CFLAGS += -fno-pic

  $ gcc -Wp,-MMD,arch/x86/boot/.cpuflags.o.d ... -fno-pic ... -D__KBUILD_MODNAME=kmod_cpuflags -c -o arch/x86/boot/cpuflags.o arch/x86/boot/cpuflags.c

so the 32-bit workaround in cpuid_count() is fixing exactly nothing
because 32-bit configs don't even allow PIC builds.

As to 64-bit builds: they're done using -mcmodel=kernel which produces
RIP-relative addressing for PIC builds and thus does not apply here
either.

So get rid of the thing and make cpuid_count() nice and simple.

There should be no functional changes resulting from this.

  [ bp: Expand commit message. ]

Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lore.kernel.org/r/20221104124546.196077-1-ubizjak@gmail.com
---
 arch/x86/boot/cpuflags.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
index a83d67e..d75237b 100644
--- a/arch/x86/boot/cpuflags.c
+++ b/arch/x86/boot/cpuflags.c
@@ -64,20 +64,11 @@ int has_eflag(unsigned long mask)
 	return !!((f0^f1) & mask);
 }
 
-/* Handle x86_32 PIC using ebx. */
-#if defined(__i386__) && defined(__PIC__)
-# define EBX_REG "=r"
-#else
-# define EBX_REG "=b"
-#endif
-
 void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
 {
-	asm volatile(".ifnc %%ebx,%3 ; movl  %%ebx,%3 ; .endif	\n\t"
-		     "cpuid					\n\t"
-		     ".ifnc %%ebx,%3 ; xchgl %%ebx,%3 ; .endif	\n\t"
-		    : "=a" (*a), "=c" (*c), "=d" (*d), EBX_REG (*b)
-		    : "a" (id), "c" (count)
+	asm volatile("cpuid"
+		     : "=a" (*a), "=b" (*b), "=c" (*c), "=d" (*d)
+		     : "0" (id), "2" (count)
 	);
 }
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-11-29 19:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-04 12:45 [PATCH] x86/boot: Remove x86_32 PIC using ebx workaround Uros Bizjak
2022-11-28 22:20 ` Borislav Petkov
2022-11-29  3:41   ` Brian Gerst
2022-11-29  8:32     ` Borislav Petkov
2022-11-29 13:24       ` Brian Gerst
2022-11-29 13:26         ` Uros Bizjak
2022-11-29  7:39   ` Uros Bizjak
2022-11-29  8:31     ` Borislav Petkov
2022-11-29  8:50       ` Uros Bizjak
2022-11-29 14:46         ` Borislav Petkov
2022-11-29 14:56           ` Uros Bizjak
2022-11-29 15:27             ` Borislav Petkov
2022-11-29 19:44 ` [tip: x86/boot] x86/boot: Remove x86_32 PIC using %ebx workaround tip-bot2 for Uros Bizjak

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.