arch/x86/boot/cpuflags.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
Use __ASM_SIZE() macro to add correct insn suffix to pushf/popf.
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
arch/x86/boot/cpuflags.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
index d75237ba7ce9..aacabe431fd5 100644
--- a/arch/x86/boot/cpuflags.c
+++ b/arch/x86/boot/cpuflags.c
@@ -2,6 +2,7 @@
#include <linux/types.h>
#include "bitops.h"
+#include <asm/asm.h>
#include <asm/processor-flags.h>
#include <asm/required-features.h>
#include <asm/msr-index.h>
@@ -36,13 +37,8 @@ static int has_fpu(void)
* compressed/ directory where it may be 64-bit code, and thus needs
* to be 'pushfq' or 'popfq' in that case.
*/
-#ifdef __x86_64__
-#define PUSHF "pushfq"
-#define POPF "popfq"
-#else
-#define PUSHF "pushfl"
-#define POPF "popfl"
-#endif
+#define PUSHF __ASM_SIZE(pushf)
+#define POPF __ASM_SIZE(popf)
int has_eflag(unsigned long mask)
{
--
2.45.2On July 17, 2024 11:32:18 PM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>Use __ASM_SIZE() macro to add correct insn suffix to pushf/popf.
>
>Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
>Cc: Thomas Gleixner <tglx@linutronix.de>
>Cc: Ingo Molnar <mingo@kernel.org>
>Cc: Borislav Petkov <bp@alien8.de>
>Cc: Dave Hansen <dave.hansen@linux.intel.com>
>Cc: "H. Peter Anvin" <hpa@zytor.com>
>---
> arch/x86/boot/cpuflags.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
>diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
>index d75237ba7ce9..aacabe431fd5 100644
>--- a/arch/x86/boot/cpuflags.c
>+++ b/arch/x86/boot/cpuflags.c
>@@ -2,6 +2,7 @@
> #include <linux/types.h>
> #include "bitops.h"
>
>+#include <asm/asm.h>
> #include <asm/processor-flags.h>
> #include <asm/required-features.h>
> #include <asm/msr-index.h>
>@@ -36,13 +37,8 @@ static int has_fpu(void)
> * compressed/ directory where it may be 64-bit code, and thus needs
> * to be 'pushfq' or 'popfq' in that case.
> */
>-#ifdef __x86_64__
>-#define PUSHF "pushfq"
>-#define POPF "popfq"
>-#else
>-#define PUSHF "pushfl"
>-#define POPF "popfl"
>-#endif
>+#define PUSHF __ASM_SIZE(pushf)
>+#define POPF __ASM_SIZE(popf)
>
> int has_eflag(unsigned long mask)
> {
Just use pushf/popf. gas hasn't needed that suffix for a long time as far as I know.
On Thu, Jul 18, 2024 at 8:36 AM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On July 17, 2024 11:32:18 PM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
> >Use __ASM_SIZE() macro to add correct insn suffix to pushf/popf.
> >
> >Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> >Cc: Thomas Gleixner <tglx@linutronix.de>
> >Cc: Ingo Molnar <mingo@kernel.org>
> >Cc: Borislav Petkov <bp@alien8.de>
> >Cc: Dave Hansen <dave.hansen@linux.intel.com>
> >Cc: "H. Peter Anvin" <hpa@zytor.com>
> >---
> > arch/x86/boot/cpuflags.c | 10 +++-------
> > 1 file changed, 3 insertions(+), 7 deletions(-)
> >
> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
> >index d75237ba7ce9..aacabe431fd5 100644
> >--- a/arch/x86/boot/cpuflags.c
> >+++ b/arch/x86/boot/cpuflags.c
> >@@ -2,6 +2,7 @@
> > #include <linux/types.h>
> > #include "bitops.h"
> >
> >+#include <asm/asm.h>
> > #include <asm/processor-flags.h>
> > #include <asm/required-features.h>
> > #include <asm/msr-index.h>
> >@@ -36,13 +37,8 @@ static int has_fpu(void)
> > * compressed/ directory where it may be 64-bit code, and thus needs
> > * to be 'pushfq' or 'popfq' in that case.
> > */
> >-#ifdef __x86_64__
> >-#define PUSHF "pushfq"
> >-#define POPF "popfq"
> >-#else
> >-#define PUSHF "pushfl"
> >-#define POPF "popfl"
> >-#endif
> >+#define PUSHF __ASM_SIZE(pushf)
> >+#define POPF __ASM_SIZE(popf)
> >
> > int has_eflag(unsigned long mask)
> > {
>
> Just use pushf/popf. gas hasn't needed that suffix for a long time as far as I know.
Unfortunately, clang does not do the right thing when pushf/popf
without suffix are used.
arch/x86/boot/cpuflags.c compiles to:
00000000 <has_eflag>:
0: 9c pushf
1: 9c pushf
2: 66 5a pop %edx
4: 66 89 d1 mov %edx,%ecx
7: 66 31 c1 xor %eax,%ecx
a: 66 51 push %ecx
c: 9d popf
d: 9c pushf
e: 66 59 pop %ecx
10: 9d popf
11: 66 31 ca xor %ecx,%edx
14: 66 31 c9 xor %ecx,%ecx
17: 66 85 c2 test %eax,%edx
1a: 0f 95 c1 setne %cl
1d: 66 89 c8 mov %ecx,%eax
20: 66 c3 retl
instead of:
00000000 <has_eflag>:
0: 66 9c pushfl
2: 66 9c pushfl
4: 66 5a pop %edx
6: 66 89 d1 mov %edx,%ecx
9: 66 31 c1 xor %eax,%ecx
c: 66 51 push %ecx
e: 66 9d popfl
10: 66 9c pushfl
12: 66 59 pop %ecx
14: 66 9d popfl
16: 66 31 ca xor %ecx,%edx
19: 66 31 c9 xor %ecx,%ecx
1c: 66 85 c2 test %eax,%edx
1f: 0f 95 c1 setne %cl
22: 66 89 c8 mov %ecx,%eax
25: 66 c3 retl
Please note missing 0x66 operand size override prefixes with pushfl
and popfl. This is 16bit code, operand prefixes are mandatory to push
32-bit EFLAGS register (ID flag lives in bit 21).
So, the original patch is the way to go.
Uros.
On July 18, 2024 1:52:17 AM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>On Thu, Jul 18, 2024 at 8:36 AM H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On July 17, 2024 11:32:18 PM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>> >Use __ASM_SIZE() macro to add correct insn suffix to pushf/popf.
>> >
>> >Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
>> >Cc: Thomas Gleixner <tglx@linutronix.de>
>> >Cc: Ingo Molnar <mingo@kernel.org>
>> >Cc: Borislav Petkov <bp@alien8.de>
>> >Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> >Cc: "H. Peter Anvin" <hpa@zytor.com>
>> >---
>> > arch/x86/boot/cpuflags.c | 10 +++-------
>> > 1 file changed, 3 insertions(+), 7 deletions(-)
>> >
>> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
>> >index d75237ba7ce9..aacabe431fd5 100644
>> >--- a/arch/x86/boot/cpuflags.c
>> >+++ b/arch/x86/boot/cpuflags.c
>> >@@ -2,6 +2,7 @@
>> > #include <linux/types.h>
>> > #include "bitops.h"
>> >
>> >+#include <asm/asm.h>
>> > #include <asm/processor-flags.h>
>> > #include <asm/required-features.h>
>> > #include <asm/msr-index.h>
>> >@@ -36,13 +37,8 @@ static int has_fpu(void)
>> > * compressed/ directory where it may be 64-bit code, and thus needs
>> > * to be 'pushfq' or 'popfq' in that case.
>> > */
>> >-#ifdef __x86_64__
>> >-#define PUSHF "pushfq"
>> >-#define POPF "popfq"
>> >-#else
>> >-#define PUSHF "pushfl"
>> >-#define POPF "popfl"
>> >-#endif
>> >+#define PUSHF __ASM_SIZE(pushf)
>> >+#define POPF __ASM_SIZE(popf)
>> >
>> > int has_eflag(unsigned long mask)
>> > {
>>
>> Just use pushf/popf. gas hasn't needed that suffix for a long time as far as I know.
>
>Unfortunately, clang does not do the right thing when pushf/popf
>without suffix are used.
>
>arch/x86/boot/cpuflags.c compiles to:
>
>00000000 <has_eflag>:
> 0: 9c pushf
> 1: 9c pushf
> 2: 66 5a pop %edx
> 4: 66 89 d1 mov %edx,%ecx
> 7: 66 31 c1 xor %eax,%ecx
> a: 66 51 push %ecx
> c: 9d popf
> d: 9c pushf
> e: 66 59 pop %ecx
> 10: 9d popf
> 11: 66 31 ca xor %ecx,%edx
> 14: 66 31 c9 xor %ecx,%ecx
> 17: 66 85 c2 test %eax,%edx
> 1a: 0f 95 c1 setne %cl
> 1d: 66 89 c8 mov %ecx,%eax
> 20: 66 c3 retl
>
>instead of:
>
>00000000 <has_eflag>:
> 0: 66 9c pushfl
> 2: 66 9c pushfl
> 4: 66 5a pop %edx
> 6: 66 89 d1 mov %edx,%ecx
> 9: 66 31 c1 xor %eax,%ecx
> c: 66 51 push %ecx
> e: 66 9d popfl
> 10: 66 9c pushfl
> 12: 66 59 pop %ecx
> 14: 66 9d popfl
> 16: 66 31 ca xor %ecx,%edx
> 19: 66 31 c9 xor %ecx,%ecx
> 1c: 66 85 c2 test %eax,%edx
> 1f: 0f 95 c1 setne %cl
> 22: 66 89 c8 mov %ecx,%eax
> 25: 66 c3 retl
>
>Please note missing 0x66 operand size override prefixes with pushfl
>and popfl. This is 16bit code, operand prefixes are mandatory to push
>32-bit EFLAGS register (ID flag lives in bit 21).
>
>So, the original patch is the way to go.
>
>Uros.
>
You do know that has_eflag can be completely elided on x86-64, or you can use %z with one of the register operands.
One more reason why clang really needs to shape up.
On Thu, Jul 18, 2024 at 10:56 AM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On July 18, 2024 1:52:17 AM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
> >On Thu, Jul 18, 2024 at 8:36 AM H. Peter Anvin <hpa@zytor.com> wrote:
> >>
> >> On July 17, 2024 11:32:18 PM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
> >> >Use __ASM_SIZE() macro to add correct insn suffix to pushf/popf.
> >> >
> >> >Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> >> >Cc: Thomas Gleixner <tglx@linutronix.de>
> >> >Cc: Ingo Molnar <mingo@kernel.org>
> >> >Cc: Borislav Petkov <bp@alien8.de>
> >> >Cc: Dave Hansen <dave.hansen@linux.intel.com>
> >> >Cc: "H. Peter Anvin" <hpa@zytor.com>
> >> >---
> >> > arch/x86/boot/cpuflags.c | 10 +++-------
> >> > 1 file changed, 3 insertions(+), 7 deletions(-)
> >> >
> >> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
> >> >index d75237ba7ce9..aacabe431fd5 100644
> >> >--- a/arch/x86/boot/cpuflags.c
> >> >+++ b/arch/x86/boot/cpuflags.c
> >> >@@ -2,6 +2,7 @@
> >> > #include <linux/types.h>
> >> > #include "bitops.h"
> >> >
> >> >+#include <asm/asm.h>
> >> > #include <asm/processor-flags.h>
> >> > #include <asm/required-features.h>
> >> > #include <asm/msr-index.h>
> >> >@@ -36,13 +37,8 @@ static int has_fpu(void)
> >> > * compressed/ directory where it may be 64-bit code, and thus needs
> >> > * to be 'pushfq' or 'popfq' in that case.
> >> > */
> >> >-#ifdef __x86_64__
> >> >-#define PUSHF "pushfq"
> >> >-#define POPF "popfq"
> >> >-#else
> >> >-#define PUSHF "pushfl"
> >> >-#define POPF "popfl"
> >> >-#endif
> >> >+#define PUSHF __ASM_SIZE(pushf)
> >> >+#define POPF __ASM_SIZE(popf)
> >> >
> >> > int has_eflag(unsigned long mask)
> >> > {
> >>
> >> Just use pushf/popf. gas hasn't needed that suffix for a long time as far as I know.
> >
> >Unfortunately, clang does not do the right thing when pushf/popf
> >without suffix are used.
> >
> >arch/x86/boot/cpuflags.c compiles to:
> >
> >00000000 <has_eflag>:
> > 0: 9c pushf
> > 1: 9c pushf
> > 2: 66 5a pop %edx
> > 4: 66 89 d1 mov %edx,%ecx
> > 7: 66 31 c1 xor %eax,%ecx
> > a: 66 51 push %ecx
> > c: 9d popf
> > d: 9c pushf
> > e: 66 59 pop %ecx
> > 10: 9d popf
> > 11: 66 31 ca xor %ecx,%edx
> > 14: 66 31 c9 xor %ecx,%ecx
> > 17: 66 85 c2 test %eax,%edx
> > 1a: 0f 95 c1 setne %cl
> > 1d: 66 89 c8 mov %ecx,%eax
> > 20: 66 c3 retl
> >
> >instead of:
> >
> >00000000 <has_eflag>:
> > 0: 66 9c pushfl
> > 2: 66 9c pushfl
> > 4: 66 5a pop %edx
> > 6: 66 89 d1 mov %edx,%ecx
> > 9: 66 31 c1 xor %eax,%ecx
> > c: 66 51 push %ecx
> > e: 66 9d popfl
> > 10: 66 9c pushfl
> > 12: 66 59 pop %ecx
> > 14: 66 9d popfl
> > 16: 66 31 ca xor %ecx,%edx
> > 19: 66 31 c9 xor %ecx,%ecx
> > 1c: 66 85 c2 test %eax,%edx
> > 1f: 0f 95 c1 setne %cl
> > 22: 66 89 c8 mov %ecx,%eax
> > 25: 66 c3 retl
> >
> >Please note missing 0x66 operand size override prefixes with pushfl
> >and popfl. This is 16bit code, operand prefixes are mandatory to push
> >32-bit EFLAGS register (ID flag lives in bit 21).
> >
> >So, the original patch is the way to go.
> >
> >Uros.
> >
>
> You do know that has_eflag can be completely elided on x86-64, or you can use %z with one of the register operands.
It is 32-bit PUSHFL insn that requires the "L" suffix in 16-bit code.
This is x86_32 issue and clang was just lucky that the instruction was
always defined with explicit L suffix. Please note that PUSHF has no
register operand, so %z can't be used.
> One more reason why clang really needs to shape up.
Indeed.
Thanks,
Uros.
On July 18, 2024 2:12:07 AM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>On Thu, Jul 18, 2024 at 10:56 AM H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On July 18, 2024 1:52:17 AM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>> >On Thu, Jul 18, 2024 at 8:36 AM H. Peter Anvin <hpa@zytor.com> wrote:
>> >>
>> >> On July 17, 2024 11:32:18 PM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>> >> >Use __ASM_SIZE() macro to add correct insn suffix to pushf/popf.
>> >> >
>> >> >Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
>> >> >Cc: Thomas Gleixner <tglx@linutronix.de>
>> >> >Cc: Ingo Molnar <mingo@kernel.org>
>> >> >Cc: Borislav Petkov <bp@alien8.de>
>> >> >Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> >> >Cc: "H. Peter Anvin" <hpa@zytor.com>
>> >> >---
>> >> > arch/x86/boot/cpuflags.c | 10 +++-------
>> >> > 1 file changed, 3 insertions(+), 7 deletions(-)
>> >> >
>> >> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
>> >> >index d75237ba7ce9..aacabe431fd5 100644
>> >> >--- a/arch/x86/boot/cpuflags.c
>> >> >+++ b/arch/x86/boot/cpuflags.c
>> >> >@@ -2,6 +2,7 @@
>> >> > #include <linux/types.h>
>> >> > #include "bitops.h"
>> >> >
>> >> >+#include <asm/asm.h>
>> >> > #include <asm/processor-flags.h>
>> >> > #include <asm/required-features.h>
>> >> > #include <asm/msr-index.h>
>> >> >@@ -36,13 +37,8 @@ static int has_fpu(void)
>> >> > * compressed/ directory where it may be 64-bit code, and thus needs
>> >> > * to be 'pushfq' or 'popfq' in that case.
>> >> > */
>> >> >-#ifdef __x86_64__
>> >> >-#define PUSHF "pushfq"
>> >> >-#define POPF "popfq"
>> >> >-#else
>> >> >-#define PUSHF "pushfl"
>> >> >-#define POPF "popfl"
>> >> >-#endif
>> >> >+#define PUSHF __ASM_SIZE(pushf)
>> >> >+#define POPF __ASM_SIZE(popf)
>> >> >
>> >> > int has_eflag(unsigned long mask)
>> >> > {
>> >>
>> >> Just use pushf/popf. gas hasn't needed that suffix for a long time as far as I know.
>> >
>> >Unfortunately, clang does not do the right thing when pushf/popf
>> >without suffix are used.
>> >
>> >arch/x86/boot/cpuflags.c compiles to:
>> >
>> >00000000 <has_eflag>:
>> > 0: 9c pushf
>> > 1: 9c pushf
>> > 2: 66 5a pop %edx
>> > 4: 66 89 d1 mov %edx,%ecx
>> > 7: 66 31 c1 xor %eax,%ecx
>> > a: 66 51 push %ecx
>> > c: 9d popf
>> > d: 9c pushf
>> > e: 66 59 pop %ecx
>> > 10: 9d popf
>> > 11: 66 31 ca xor %ecx,%edx
>> > 14: 66 31 c9 xor %ecx,%ecx
>> > 17: 66 85 c2 test %eax,%edx
>> > 1a: 0f 95 c1 setne %cl
>> > 1d: 66 89 c8 mov %ecx,%eax
>> > 20: 66 c3 retl
>> >
>> >instead of:
>> >
>> >00000000 <has_eflag>:
>> > 0: 66 9c pushfl
>> > 2: 66 9c pushfl
>> > 4: 66 5a pop %edx
>> > 6: 66 89 d1 mov %edx,%ecx
>> > 9: 66 31 c1 xor %eax,%ecx
>> > c: 66 51 push %ecx
>> > e: 66 9d popfl
>> > 10: 66 9c pushfl
>> > 12: 66 59 pop %ecx
>> > 14: 66 9d popfl
>> > 16: 66 31 ca xor %ecx,%edx
>> > 19: 66 31 c9 xor %ecx,%ecx
>> > 1c: 66 85 c2 test %eax,%edx
>> > 1f: 0f 95 c1 setne %cl
>> > 22: 66 89 c8 mov %ecx,%eax
>> > 25: 66 c3 retl
>> >
>> >Please note missing 0x66 operand size override prefixes with pushfl
>> >and popfl. This is 16bit code, operand prefixes are mandatory to push
>> >32-bit EFLAGS register (ID flag lives in bit 21).
>> >
>> >So, the original patch is the way to go.
>> >
>> >Uros.
>> >
>>
>> You do know that has_eflag can be completely elided on x86-64, or you can use %z with one of the register operands.
>
>It is 32-bit PUSHFL insn that requires the "L" suffix in 16-bit code.
>This is x86_32 issue and clang was just lucky that the instruction was
>always defined with explicit L suffix. Please note that PUSHF has no
>register operand, so %z can't be used.
>
>> One more reason why clang really needs to shape up.
>
>Indeed.
>
>Thanks,
>Uros.
>
Oh, I see – it is a bug in clang -m16 (.code16gcc or its equivalent.)
However, there you can always do pushfl/popfl since that doesn't apply to 64-bit mode: both AC and ID always exist in 64-bit mode and any future features should be detected by cpuid rather than by frobbing random bits in xFLAGS.
I suggest have_eflag_ac() and have_eflag_id() and just have them return true on 64 bits.
On Thu, Jul 18, 2024 at 8:36 AM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On July 17, 2024 11:32:18 PM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
> >Use __ASM_SIZE() macro to add correct insn suffix to pushf/popf.
> >
> >Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> >Cc: Thomas Gleixner <tglx@linutronix.de>
> >Cc: Ingo Molnar <mingo@kernel.org>
> >Cc: Borislav Petkov <bp@alien8.de>
> >Cc: Dave Hansen <dave.hansen@linux.intel.com>
> >Cc: "H. Peter Anvin" <hpa@zytor.com>
> >---
> > arch/x86/boot/cpuflags.c | 10 +++-------
> > 1 file changed, 3 insertions(+), 7 deletions(-)
> >
> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
> >index d75237ba7ce9..aacabe431fd5 100644
> >--- a/arch/x86/boot/cpuflags.c
> >+++ b/arch/x86/boot/cpuflags.c
> >@@ -2,6 +2,7 @@
> > #include <linux/types.h>
> > #include "bitops.h"
> >
> >+#include <asm/asm.h>
> > #include <asm/processor-flags.h>
> > #include <asm/required-features.h>
> > #include <asm/msr-index.h>
> >@@ -36,13 +37,8 @@ static int has_fpu(void)
> > * compressed/ directory where it may be 64-bit code, and thus needs
> > * to be 'pushfq' or 'popfq' in that case.
> > */
> >-#ifdef __x86_64__
> >-#define PUSHF "pushfq"
> >-#define POPF "popfq"
> >-#else
> >-#define PUSHF "pushfl"
> >-#define POPF "popfl"
> >-#endif
> >+#define PUSHF __ASM_SIZE(pushf)
> >+#define POPF __ASM_SIZE(popf)
> >
> > int has_eflag(unsigned long mask)
> > {
>
> Just use pushf/popf. gas hasn't needed that suffix for a long time as far as I know.
Yes, this works, too. So I guess we can also remove the comment
explaining the reason for explicit suffixes?
Thanks,
Uros.
On July 17, 2024 11:46:57 PM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>On Thu, Jul 18, 2024 at 8:36 AM H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On July 17, 2024 11:32:18 PM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>> >Use __ASM_SIZE() macro to add correct insn suffix to pushf/popf.
>> >
>> >Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
>> >Cc: Thomas Gleixner <tglx@linutronix.de>
>> >Cc: Ingo Molnar <mingo@kernel.org>
>> >Cc: Borislav Petkov <bp@alien8.de>
>> >Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> >Cc: "H. Peter Anvin" <hpa@zytor.com>
>> >---
>> > arch/x86/boot/cpuflags.c | 10 +++-------
>> > 1 file changed, 3 insertions(+), 7 deletions(-)
>> >
>> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
>> >index d75237ba7ce9..aacabe431fd5 100644
>> >--- a/arch/x86/boot/cpuflags.c
>> >+++ b/arch/x86/boot/cpuflags.c
>> >@@ -2,6 +2,7 @@
>> > #include <linux/types.h>
>> > #include "bitops.h"
>> >
>> >+#include <asm/asm.h>
>> > #include <asm/processor-flags.h>
>> > #include <asm/required-features.h>
>> > #include <asm/msr-index.h>
>> >@@ -36,13 +37,8 @@ static int has_fpu(void)
>> > * compressed/ directory where it may be 64-bit code, and thus needs
>> > * to be 'pushfq' or 'popfq' in that case.
>> > */
>> >-#ifdef __x86_64__
>> >-#define PUSHF "pushfq"
>> >-#define POPF "popfq"
>> >-#else
>> >-#define PUSHF "pushfl"
>> >-#define POPF "popfl"
>> >-#endif
>> >+#define PUSHF __ASM_SIZE(pushf)
>> >+#define POPF __ASM_SIZE(popf)
>> >
>> > int has_eflag(unsigned long mask)
>> > {
>>
>> Just use pushf/popf. gas hasn't needed that suffix for a long time as far as I know.
>
>Yes, this works, too. So I guess we can also remove the comment
>explaining the reason for explicit suffixes?
>
>Thanks,
>Uros.
>
Yeah. You may want to check the version of binutils that fixed it and put that in the comments.
On Thu, Jul 18, 2024 at 8:59 AM H. Peter Anvin <hpa@zytor.com> wrote:
>
> >> >-#ifdef __x86_64__
> >> >-#define PUSHF "pushfq"
> >> >-#define POPF "popfq"
> >> >-#else
> >> >-#define PUSHF "pushfl"
> >> >-#define POPF "popfl"
> >> >-#endif
> >> >+#define PUSHF __ASM_SIZE(pushf)
> >> >+#define POPF __ASM_SIZE(popf)
> >> >
> >> > int has_eflag(unsigned long mask)
> >> > {
> >>
> >> Just use pushf/popf. gas hasn't needed that suffix for a long time as far as I know.
> >
> >Yes, this works, too. So I guess we can also remove the comment
> >explaining the reason for explicit suffixes?
> >
> >Thanks,
> >Uros.
> >
>
> Yeah. You may want to check the version of binutils that fixed it and put that in the comments.
I have checked that the build works with 9 year old binutils-2.25
(minimal required version), so the fix was applied to an even earlier
version. I guess we don't want to burden unsuspecting readers with
historic toolchain oddities, so I propose to just remove the comment
for good.
Uros.
On July 18, 2024 12:27:04 AM PDT, Uros Bizjak <ubizjak@gmail.com> wrote:
>On Thu, Jul 18, 2024 at 8:59 AM H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> >> >-#ifdef __x86_64__
>> >> >-#define PUSHF "pushfq"
>> >> >-#define POPF "popfq"
>> >> >-#else
>> >> >-#define PUSHF "pushfl"
>> >> >-#define POPF "popfl"
>> >> >-#endif
>> >> >+#define PUSHF __ASM_SIZE(pushf)
>> >> >+#define POPF __ASM_SIZE(popf)
>> >> >
>> >> > int has_eflag(unsigned long mask)
>> >> > {
>> >>
>> >> Just use pushf/popf. gas hasn't needed that suffix for a long time as far as I know.
>> >
>> >Yes, this works, too. So I guess we can also remove the comment
>> >explaining the reason for explicit suffixes?
>> >
>> >Thanks,
>> >Uros.
>> >
>>
>> Yeah. You may want to check the version of binutils that fixed it and put that in the comments.
>
>I have checked that the build works with 9 year old binutils-2.25
>(minimal required version), so the fix was applied to an even earlier
>version. I guess we don't want to burden unsuspecting readers with
>historic toolchain oddities, so I propose to just remove the comment
>for good.
>
>Uros.
>
Yeah, I meant the commit message not comments, sorry.
© 2016 - 2025 Red Hat, Inc.