* [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
@ 2025-03-07 9:10 Uros Bizjak
2025-03-07 11:58 ` H. Peter Anvin
2025-03-08 19:51 ` [tip: x86/boot] " tip-bot2 for Uros Bizjak
0 siblings, 2 replies; 10+ messages in thread
From: Uros Bizjak @ 2025-03-07 9:10 UTC (permalink / raw)
To: x86, linux-kernel
Cc: Uros Bizjak, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, H. Peter Anvin
The test for the changeabitily of AC and ID eflags is used to
distinguish between i386 and i486 processors (AC) and to test
for cpuid instruction support (ID).
Skip these tests on x86_64 processors as they always supports cpuid.
Also change the return type of has_eflag() to bool.
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 | 26 +++++++++-----------------
arch/x86/boot/cpuflags.h | 6 +++++-
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
index d75237ba7ce9..2150a016176f 100644
--- a/arch/x86/boot/cpuflags.c
+++ b/arch/x86/boot/cpuflags.c
@@ -29,40 +29,32 @@ static int has_fpu(void)
return fsw == 0 && (fcw & 0x103f) == 0x003f;
}
+#ifdef CONFIG_X86_32
/*
* For building the 16-bit code we want to explicitly specify 32-bit
* push/pop operations, rather than just saying 'pushf' or 'popf' and
- * letting the compiler choose. But this is also included from the
- * compressed/ directory where it may be 64-bit code, and thus needs
- * to be 'pushfq' or 'popfq' in that case.
+ * letting the compiler choose.
*/
-#ifdef __x86_64__
-#define PUSHF "pushfq"
-#define POPF "popfq"
-#else
-#define PUSHF "pushfl"
-#define POPF "popfl"
-#endif
-
-int has_eflag(unsigned long mask)
+bool has_eflag(unsigned long mask)
{
unsigned long f0, f1;
- asm volatile(PUSHF " \n\t"
- PUSHF " \n\t"
+ asm volatile("pushfl \n\t"
+ "pushfl \n\t"
"pop %0 \n\t"
"mov %0,%1 \n\t"
"xor %2,%1 \n\t"
"push %1 \n\t"
- POPF " \n\t"
- PUSHF " \n\t"
+ "popfl \n\t"
+ "pushfl \n\t"
"pop %1 \n\t"
- POPF
+ "popfl"
: "=&r" (f0), "=&r" (f1)
: "ri" (mask));
return !!((f0^f1) & mask);
}
+#endif
void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
{
diff --git a/arch/x86/boot/cpuflags.h b/arch/x86/boot/cpuflags.h
index fdcc2aa4c3c4..a398d9204ad0 100644
--- a/arch/x86/boot/cpuflags.h
+++ b/arch/x86/boot/cpuflags.h
@@ -15,7 +15,11 @@ struct cpu_features {
extern struct cpu_features cpu;
extern u32 cpu_vendor[3];
-int has_eflag(unsigned long mask);
+#ifdef CONFIG_X86_32
+bool has_eflag(unsigned long mask);
+#else
+static inline bool has_eflag(unsigned long mask) { return true; }
+#endif
void get_cpuflags(void);
void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d);
bool has_cpuflag(int flag);
--
2.48.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
2025-03-07 9:10 [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64 Uros Bizjak
@ 2025-03-07 11:58 ` H. Peter Anvin
2025-03-07 12:15 ` Uros Bizjak
2025-03-08 19:51 ` [tip: x86/boot] " tip-bot2 for Uros Bizjak
1 sibling, 1 reply; 10+ messages in thread
From: H. Peter Anvin @ 2025-03-07 11:58 UTC (permalink / raw)
To: Uros Bizjak, x86, linux-kernel
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen
On March 7, 2025 1:10:03 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
>The test for the changeabitily of AC and ID eflags is used to
>distinguish between i386 and i486 processors (AC) and to test
>for cpuid instruction support (ID).
>
>Skip these tests on x86_64 processors as they always supports cpuid.
>
>Also change the return type of has_eflag() to bool.
>
>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 | 26 +++++++++-----------------
> arch/x86/boot/cpuflags.h | 6 +++++-
> 2 files changed, 14 insertions(+), 18 deletions(-)
>
>diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
>index d75237ba7ce9..2150a016176f 100644
>--- a/arch/x86/boot/cpuflags.c
>+++ b/arch/x86/boot/cpuflags.c
>@@ -29,40 +29,32 @@ static int has_fpu(void)
> return fsw == 0 && (fcw & 0x103f) == 0x003f;
> }
>
>+#ifdef CONFIG_X86_32
> /*
> * For building the 16-bit code we want to explicitly specify 32-bit
> * push/pop operations, rather than just saying 'pushf' or 'popf' and
>- * letting the compiler choose. But this is also included from the
>- * compressed/ directory where it may be 64-bit code, and thus needs
>- * to be 'pushfq' or 'popfq' in that case.
>+ * letting the compiler choose.
> */
>-#ifdef __x86_64__
>-#define PUSHF "pushfq"
>-#define POPF "popfq"
>-#else
>-#define PUSHF "pushfl"
>-#define POPF "popfl"
>-#endif
>-
>-int has_eflag(unsigned long mask)
>+bool has_eflag(unsigned long mask)
> {
> unsigned long f0, f1;
>
>- asm volatile(PUSHF " \n\t"
>- PUSHF " \n\t"
>+ asm volatile("pushfl \n\t"
>+ "pushfl \n\t"
> "pop %0 \n\t"
> "mov %0,%1 \n\t"
> "xor %2,%1 \n\t"
> "push %1 \n\t"
>- POPF " \n\t"
>- PUSHF " \n\t"
>+ "popfl \n\t"
>+ "pushfl \n\t"
> "pop %1 \n\t"
>- POPF
>+ "popfl"
> : "=&r" (f0), "=&r" (f1)
> : "ri" (mask));
>
> return !!((f0^f1) & mask);
> }
>+#endif
>
> void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
> {
>diff --git a/arch/x86/boot/cpuflags.h b/arch/x86/boot/cpuflags.h
>index fdcc2aa4c3c4..a398d9204ad0 100644
>--- a/arch/x86/boot/cpuflags.h
>+++ b/arch/x86/boot/cpuflags.h
>@@ -15,7 +15,11 @@ struct cpu_features {
> extern struct cpu_features cpu;
> extern u32 cpu_vendor[3];
>
>-int has_eflag(unsigned long mask);
>+#ifdef CONFIG_X86_32
>+bool has_eflag(unsigned long mask);
>+#else
>+static inline bool has_eflag(unsigned long mask) { return true; }
>+#endif
> void get_cpuflags(void);
> void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d);
> bool has_cpuflag(int flag);
PUSF et al → pushf
The -l and -q suffixes have been optional for a long time.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
2025-03-07 11:58 ` H. Peter Anvin
@ 2025-03-07 12:15 ` Uros Bizjak
2025-03-07 13:13 ` H. Peter Anvin
0 siblings, 1 reply; 10+ messages in thread
From: Uros Bizjak @ 2025-03-07 12:15 UTC (permalink / raw)
To: H. Peter Anvin
Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen
On Fri, Mar 7, 2025 at 12:58 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On March 7, 2025 1:10:03 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
> >The test for the changeabitily of AC and ID eflags is used to
> >distinguish between i386 and i486 processors (AC) and to test
> >for cpuid instruction support (ID).
> >
> >Skip these tests on x86_64 processors as they always supports cpuid.
> >
> >Also change the return type of has_eflag() to bool.
> >
> >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 | 26 +++++++++-----------------
> > arch/x86/boot/cpuflags.h | 6 +++++-
> > 2 files changed, 14 insertions(+), 18 deletions(-)
> >
> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
> >index d75237ba7ce9..2150a016176f 100644
> >--- a/arch/x86/boot/cpuflags.c
> >+++ b/arch/x86/boot/cpuflags.c
> >@@ -29,40 +29,32 @@ static int has_fpu(void)
> > return fsw == 0 && (fcw & 0x103f) == 0x003f;
> > }
> >
> >+#ifdef CONFIG_X86_32
> > /*
> > * For building the 16-bit code we want to explicitly specify 32-bit
> > * push/pop operations, rather than just saying 'pushf' or 'popf' and
> >- * letting the compiler choose. But this is also included from the
> >- * compressed/ directory where it may be 64-bit code, and thus needs
> >- * to be 'pushfq' or 'popfq' in that case.
> >+ * letting the compiler choose.
> > */
> >-#ifdef __x86_64__
> >-#define PUSHF "pushfq"
> >-#define POPF "popfq"
> >-#else
> >-#define PUSHF "pushfl"
> >-#define POPF "popfl"
> >-#endif
> >-
> >-int has_eflag(unsigned long mask)
> >+bool has_eflag(unsigned long mask)
> > {
> > unsigned long f0, f1;
> >
> >- asm volatile(PUSHF " \n\t"
> >- PUSHF " \n\t"
> >+ asm volatile("pushfl \n\t"
> >+ "pushfl \n\t"
> > "pop %0 \n\t"
> > "mov %0,%1 \n\t"
> > "xor %2,%1 \n\t"
> > "push %1 \n\t"
> >- POPF " \n\t"
> >- PUSHF " \n\t"
> >+ "popfl \n\t"
> >+ "pushfl \n\t"
> > "pop %1 \n\t"
> >- POPF
> >+ "popfl"
> > : "=&r" (f0), "=&r" (f1)
> > : "ri" (mask));
> >
> > return !!((f0^f1) & mask);
> > }
> >+#endif
> >
> > void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
> > {
> >diff --git a/arch/x86/boot/cpuflags.h b/arch/x86/boot/cpuflags.h
> >index fdcc2aa4c3c4..a398d9204ad0 100644
> >--- a/arch/x86/boot/cpuflags.h
> >+++ b/arch/x86/boot/cpuflags.h
> >@@ -15,7 +15,11 @@ struct cpu_features {
> > extern struct cpu_features cpu;
> > extern u32 cpu_vendor[3];
> >
> >-int has_eflag(unsigned long mask);
> >+#ifdef CONFIG_X86_32
> >+bool has_eflag(unsigned long mask);
> >+#else
> >+static inline bool has_eflag(unsigned long mask) { return true; }
> >+#endif
> > void get_cpuflags(void);
> > void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d);
> > bool has_cpuflag(int flag);
>
> PUSF et al → pushf
>
> The -l and -q suffixes have been optional for a long time.
No, not in this case. Please see the comment:
/*
* For building the 16-bit code we want to explicitly specify 32-bit
* push/pop operations, rather than just saying 'pushf' or 'popf' and
* letting the compiler choose.
*/
We are building 16-bit code here, and we want PUSHFL, the one with
operand size prefix 0x66.
Please consider the following code:
.code16
pushf
pushfl
as -o push.o push.s
objdump -dr -Mdata16 push.o
0000000000000000 <.text>:
0: 9c pushf
1: 66 9c pushfl
Uros.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
2025-03-07 12:15 ` Uros Bizjak
@ 2025-03-07 13:13 ` H. Peter Anvin
2025-03-07 13:45 ` Uros Bizjak
2025-03-08 19:34 ` Ingo Molnar
0 siblings, 2 replies; 10+ messages in thread
From: H. Peter Anvin @ 2025-03-07 13:13 UTC (permalink / raw)
To: Uros Bizjak
Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen
On March 7, 2025 4:15:42 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
>On Fri, Mar 7, 2025 at 12:58 PM H. Peter Anvin <hpa@zytor.com> wrote:
>>
>> On March 7, 2025 1:10:03 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
>> >The test for the changeabitily of AC and ID eflags is used to
>> >distinguish between i386 and i486 processors (AC) and to test
>> >for cpuid instruction support (ID).
>> >
>> >Skip these tests on x86_64 processors as they always supports cpuid.
>> >
>> >Also change the return type of has_eflag() to bool.
>> >
>> >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 | 26 +++++++++-----------------
>> > arch/x86/boot/cpuflags.h | 6 +++++-
>> > 2 files changed, 14 insertions(+), 18 deletions(-)
>> >
>> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
>> >index d75237ba7ce9..2150a016176f 100644
>> >--- a/arch/x86/boot/cpuflags.c
>> >+++ b/arch/x86/boot/cpuflags.c
>> >@@ -29,40 +29,32 @@ static int has_fpu(void)
>> > return fsw == 0 && (fcw & 0x103f) == 0x003f;
>> > }
>> >
>> >+#ifdef CONFIG_X86_32
>> > /*
>> > * For building the 16-bit code we want to explicitly specify 32-bit
>> > * push/pop operations, rather than just saying 'pushf' or 'popf' and
>> >- * letting the compiler choose. But this is also included from the
>> >- * compressed/ directory where it may be 64-bit code, and thus needs
>> >- * to be 'pushfq' or 'popfq' in that case.
>> >+ * letting the compiler choose.
>> > */
>> >-#ifdef __x86_64__
>> >-#define PUSHF "pushfq"
>> >-#define POPF "popfq"
>> >-#else
>> >-#define PUSHF "pushfl"
>> >-#define POPF "popfl"
>> >-#endif
>> >-
>> >-int has_eflag(unsigned long mask)
>> >+bool has_eflag(unsigned long mask)
>> > {
>> > unsigned long f0, f1;
>> >
>> >- asm volatile(PUSHF " \n\t"
>> >- PUSHF " \n\t"
>> >+ asm volatile("pushfl \n\t"
>> >+ "pushfl \n\t"
>> > "pop %0 \n\t"
>> > "mov %0,%1 \n\t"
>> > "xor %2,%1 \n\t"
>> > "push %1 \n\t"
>> >- POPF " \n\t"
>> >- PUSHF " \n\t"
>> >+ "popfl \n\t"
>> >+ "pushfl \n\t"
>> > "pop %1 \n\t"
>> >- POPF
>> >+ "popfl"
>> > : "=&r" (f0), "=&r" (f1)
>> > : "ri" (mask));
>> >
>> > return !!((f0^f1) & mask);
>> > }
>> >+#endif
>> >
>> > void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
>> > {
>> >diff --git a/arch/x86/boot/cpuflags.h b/arch/x86/boot/cpuflags.h
>> >index fdcc2aa4c3c4..a398d9204ad0 100644
>> >--- a/arch/x86/boot/cpuflags.h
>> >+++ b/arch/x86/boot/cpuflags.h
>> >@@ -15,7 +15,11 @@ struct cpu_features {
>> > extern struct cpu_features cpu;
>> > extern u32 cpu_vendor[3];
>> >
>> >-int has_eflag(unsigned long mask);
>> >+#ifdef CONFIG_X86_32
>> >+bool has_eflag(unsigned long mask);
>> >+#else
>> >+static inline bool has_eflag(unsigned long mask) { return true; }
>> >+#endif
>> > void get_cpuflags(void);
>> > void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d);
>> > bool has_cpuflag(int flag);
>>
>> PUSF et al → pushf
>>
>> The -l and -q suffixes have been optional for a long time.
>
>No, not in this case. Please see the comment:
>
>/*
>* For building the 16-bit code we want to explicitly specify 32-bit
>* push/pop operations, rather than just saying 'pushf' or 'popf' and
>* letting the compiler choose.
>*/
>
>We are building 16-bit code here, and we want PUSHFL, the one with
>operand size prefix 0x66.
>
>Please consider the following code:
>
> .code16
> pushf
> pushfl
>
>as -o push.o push.s
>
>objdump -dr -Mdata16 push.o
>
>0000000000000000 <.text>:
> 0: 9c pushf
> 1: 66 9c pushfl
>
>Uros.
>
*plonk* I should have remembered (.code16gcc is different then .code16 though.) I wrote the damned things after all...
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
2025-03-07 13:13 ` H. Peter Anvin
@ 2025-03-07 13:45 ` Uros Bizjak
2025-03-07 14:46 ` H. Peter Anvin
2025-03-08 19:34 ` Ingo Molnar
1 sibling, 1 reply; 10+ messages in thread
From: Uros Bizjak @ 2025-03-07 13:45 UTC (permalink / raw)
To: H. Peter Anvin
Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen
On Fri, Mar 7, 2025 at 2:13 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >> PUSF et al → pushf
> >>
> >> The -l and -q suffixes have been optional for a long time.
> >
> >No, not in this case. Please see the comment:
> >
> >/*
> >* For building the 16-bit code we want to explicitly specify 32-bit
> >* push/pop operations, rather than just saying 'pushf' or 'popf' and
> >* letting the compiler choose.
> >*/
> >
> >We are building 16-bit code here, and we want PUSHFL, the one with
> >operand size prefix 0x66.
> >
> >Please consider the following code:
> >
> > .code16
> > pushf
> > pushfl
> >
> >as -o push.o push.s
> >
> >objdump -dr -Mdata16 push.o
> >
> >0000000000000000 <.text>:
> > 0: 9c pushf
> > 1: 66 9c pushfl
> >
> >Uros.
> >
>
> *plonk* I should have remembered (.code16gcc is different then .code16 though.) I wrote the damned things after all...
Please note that while "gcc -m16" emits .code16gcc, "clang -m16" emits
.code16, so in the latter case we don't have ‘pushf’, and ‘popf’
instructions default to 32-bit size. So, the only solution is to
decorate pushfl with operand size prefix in this specific case.
Uros.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
2025-03-07 13:45 ` Uros Bizjak
@ 2025-03-07 14:46 ` H. Peter Anvin
2025-03-08 10:12 ` Uros Bizjak
0 siblings, 1 reply; 10+ messages in thread
From: H. Peter Anvin @ 2025-03-07 14:46 UTC (permalink / raw)
To: Uros Bizjak
Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen
On March 7, 2025 5:45:42 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
>On Fri, Mar 7, 2025 at 2:13 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
>> >> PUSF et al → pushf
>> >>
>> >> The -l and -q suffixes have been optional for a long time.
>> >
>> >No, not in this case. Please see the comment:
>> >
>> >/*
>> >* For building the 16-bit code we want to explicitly specify 32-bit
>> >* push/pop operations, rather than just saying 'pushf' or 'popf' and
>> >* letting the compiler choose.
>> >*/
>> >
>> >We are building 16-bit code here, and we want PUSHFL, the one with
>> >operand size prefix 0x66.
>> >
>> >Please consider the following code:
>> >
>> > .code16
>> > pushf
>> > pushfl
>> >
>> >as -o push.o push.s
>> >
>> >objdump -dr -Mdata16 push.o
>> >
>> >0000000000000000 <.text>:
>> > 0: 9c pushf
>> > 1: 66 9c pushfl
>> >
>> >Uros.
>> >
>>
>> *plonk* I should have remembered (.code16gcc is different then .code16 though.) I wrote the damned things after all...
>
>Please note that while "gcc -m16" emits .code16gcc, "clang -m16" emits
>.code16, so in the latter case we don't have ‘pushf’, and ‘popf’
>instructions default to 32-bit size. So, the only solution is to
>decorate pushfl with operand size prefix in this specific case.
>
>Uros.
>
Can you please beat up the clang people who do gratuitously incompatible things like this?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
2025-03-07 14:46 ` H. Peter Anvin
@ 2025-03-08 10:12 ` Uros Bizjak
0 siblings, 0 replies; 10+ messages in thread
From: Uros Bizjak @ 2025-03-08 10:12 UTC (permalink / raw)
To: H. Peter Anvin
Cc: x86, linux-kernel, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen
On Fri, Mar 7, 2025 at 3:46 PM H. Peter Anvin <hpa@zytor.com> wrote:
>
> On March 7, 2025 5:45:42 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
> >On Fri, Mar 7, 2025 at 2:13 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >
> >> >> PUSF et al → pushf
> >> >>
> >> >> The -l and -q suffixes have been optional for a long time.
> >> >
> >> >No, not in this case. Please see the comment:
> >> >
> >> >/*
> >> >* For building the 16-bit code we want to explicitly specify 32-bit
> >> >* push/pop operations, rather than just saying 'pushf' or 'popf' and
> >> >* letting the compiler choose.
> >> >*/
> >> >
> >> >We are building 16-bit code here, and we want PUSHFL, the one with
> >> >operand size prefix 0x66.
> >> >
> >> >Please consider the following code:
> >> >
> >> > .code16
> >> > pushf
> >> > pushfl
> >> >
> >> >as -o push.o push.s
> >> >
> >> >objdump -dr -Mdata16 push.o
> >> >
> >> >0000000000000000 <.text>:
> >> > 0: 9c pushf
> >> > 1: 66 9c pushfl
> >> >
> >> >Uros.
> >> >
> >>
> >> *plonk* I should have remembered (.code16gcc is different then .code16 though.) I wrote the damned things after all...
> >
> >Please note that while "gcc -m16" emits .code16gcc, "clang -m16" emits
> >.code16, so in the latter case we don't have ‘pushf’, and ‘popf’
> >instructions default to 32-bit size. So, the only solution is to
> >decorate pushfl with operand size prefix in this specific case.
> >
> >Uros.
> >
>
> Can you please beat up the clang people who do gratuitously incompatible things like this?
:)
BTW: This patch is actually the implementation you requested at some
previous discussion [1], but I took the approach similar to the
definition of have_cpuid_p() in cpuid.h [2].
[1] https://lore.kernel.org/lkml/0A743606-067F-450C-85A5-55ECE4378E22@zytor.com/#t
[2] https://elixir.bootlin.com/linux/v6.14-rc5/source/arch/x86/include/asm/cpuid.h#L32
Uros.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
2025-03-07 13:13 ` H. Peter Anvin
2025-03-07 13:45 ` Uros Bizjak
@ 2025-03-08 19:34 ` Ingo Molnar
2025-03-09 0:33 ` H. Peter Anvin
1 sibling, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2025-03-08 19:34 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Uros Bizjak, x86, linux-kernel, Thomas Gleixner, Borislav Petkov,
Dave Hansen
* H. Peter Anvin <hpa@zytor.com> wrote:
> On March 7, 2025 4:15:42 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
> >On Fri, Mar 7, 2025 at 12:58 PM H. Peter Anvin <hpa@zytor.com> wrote:
> >>
> >> On March 7, 2025 1:10:03 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
> >> >The test for the changeabitily of AC and ID eflags is used to
> >> >distinguish between i386 and i486 processors (AC) and to test
> >> >for cpuid instruction support (ID).
> >> >
> >> >Skip these tests on x86_64 processors as they always supports cpuid.
> >> >
> >> >Also change the return type of has_eflag() to bool.
> >> >
> >> >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 | 26 +++++++++-----------------
> >> > arch/x86/boot/cpuflags.h | 6 +++++-
> >> > 2 files changed, 14 insertions(+), 18 deletions(-)
> >> >
> >> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
> >> >index d75237ba7ce9..2150a016176f 100644
> >> >--- a/arch/x86/boot/cpuflags.c
> >> >+++ b/arch/x86/boot/cpuflags.c
> >> >@@ -29,40 +29,32 @@ static int has_fpu(void)
> >> > return fsw == 0 && (fcw & 0x103f) == 0x003f;
> >> > }
> >> >
> >> >+#ifdef CONFIG_X86_32
> >> > /*
> >> > * For building the 16-bit code we want to explicitly specify 32-bit
> >> > * push/pop operations, rather than just saying 'pushf' or 'popf' and
> >> >- * letting the compiler choose. But this is also included from the
> >> >- * compressed/ directory where it may be 64-bit code, and thus needs
> >> >- * to be 'pushfq' or 'popfq' in that case.
> >> >+ * letting the compiler choose.
> >> > */
> >> >-#ifdef __x86_64__
> >> >-#define PUSHF "pushfq"
> >> >-#define POPF "popfq"
> >> >-#else
> >> >-#define PUSHF "pushfl"
> >> >-#define POPF "popfl"
> >> >-#endif
> >> >-
> >> >-int has_eflag(unsigned long mask)
> >> >+bool has_eflag(unsigned long mask)
> >> > {
> >> > unsigned long f0, f1;
> >> >
> >> >- asm volatile(PUSHF " \n\t"
> >> >- PUSHF " \n\t"
> >> >+ asm volatile("pushfl \n\t"
> >> >+ "pushfl \n\t"
> >> > "pop %0 \n\t"
> >> > "mov %0,%1 \n\t"
> >> > "xor %2,%1 \n\t"
> >> > "push %1 \n\t"
> >> >- POPF " \n\t"
> >> >- PUSHF " \n\t"
> >> >+ "popfl \n\t"
> >> >+ "pushfl \n\t"
> >> > "pop %1 \n\t"
> >> >- POPF
> >> >+ "popfl"
> >> > : "=&r" (f0), "=&r" (f1)
> >> > : "ri" (mask));
> >> >
> >> > return !!((f0^f1) & mask);
> >> > }
> >> >+#endif
> >> >
> >> > void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
> >> > {
> >> >diff --git a/arch/x86/boot/cpuflags.h b/arch/x86/boot/cpuflags.h
> >> >index fdcc2aa4c3c4..a398d9204ad0 100644
> >> >--- a/arch/x86/boot/cpuflags.h
> >> >+++ b/arch/x86/boot/cpuflags.h
> >> >@@ -15,7 +15,11 @@ struct cpu_features {
> >> > extern struct cpu_features cpu;
> >> > extern u32 cpu_vendor[3];
> >> >
> >> >-int has_eflag(unsigned long mask);
> >> >+#ifdef CONFIG_X86_32
> >> >+bool has_eflag(unsigned long mask);
> >> >+#else
> >> >+static inline bool has_eflag(unsigned long mask) { return true; }
> >> >+#endif
> >> > void get_cpuflags(void);
> >> > void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d);
> >> > bool has_cpuflag(int flag);
> >>
> >> PUSF et al → pushf
> >>
> >> The -l and -q suffixes have been optional for a long time.
> >
> >No, not in this case. Please see the comment:
> >
> >/*
> >* For building the 16-bit code we want to explicitly specify 32-bit
> >* push/pop operations, rather than just saying 'pushf' or 'popf' and
> >* letting the compiler choose.
> >*/
> >
> >We are building 16-bit code here, and we want PUSHFL, the one with
> >operand size prefix 0x66.
> >
> >Please consider the following code:
> >
> > .code16
> > pushf
> > pushfl
> >
> >as -o push.o push.s
> >
> >objdump -dr -Mdata16 push.o
> >
> >0000000000000000 <.text>:
> > 0: 9c pushf
> > 1: 66 9c pushfl
> >
> >Uros.
> >
>
> *plonk* I should have remembered (.code16gcc is different then
> .code16 though.) I wrote the damned things after all...
So I'll read this as a tentative:
Acked-by: H. Peter Anvin <hpa@zytor.com>
Let me know if I'm not reading the room right. :-)
Ingo
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
2025-03-08 19:34 ` Ingo Molnar
@ 2025-03-09 0:33 ` H. Peter Anvin
0 siblings, 0 replies; 10+ messages in thread
From: H. Peter Anvin @ 2025-03-09 0:33 UTC (permalink / raw)
To: Ingo Molnar
Cc: Uros Bizjak, x86, linux-kernel, Thomas Gleixner, Borislav Petkov,
Dave Hansen
On March 8, 2025 11:34:49 AM PST, Ingo Molnar <mingo@kernel.org> wrote:
>
>* H. Peter Anvin <hpa@zytor.com> wrote:
>
>> On March 7, 2025 4:15:42 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
>> >On Fri, Mar 7, 2025 at 12:58 PM H. Peter Anvin <hpa@zytor.com> wrote:
>> >>
>> >> On March 7, 2025 1:10:03 AM PST, Uros Bizjak <ubizjak@gmail.com> wrote:
>> >> >The test for the changeabitily of AC and ID eflags is used to
>> >> >distinguish between i386 and i486 processors (AC) and to test
>> >> >for cpuid instruction support (ID).
>> >> >
>> >> >Skip these tests on x86_64 processors as they always supports cpuid.
>> >> >
>> >> >Also change the return type of has_eflag() to bool.
>> >> >
>> >> >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 | 26 +++++++++-----------------
>> >> > arch/x86/boot/cpuflags.h | 6 +++++-
>> >> > 2 files changed, 14 insertions(+), 18 deletions(-)
>> >> >
>> >> >diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
>> >> >index d75237ba7ce9..2150a016176f 100644
>> >> >--- a/arch/x86/boot/cpuflags.c
>> >> >+++ b/arch/x86/boot/cpuflags.c
>> >> >@@ -29,40 +29,32 @@ static int has_fpu(void)
>> >> > return fsw == 0 && (fcw & 0x103f) == 0x003f;
>> >> > }
>> >> >
>> >> >+#ifdef CONFIG_X86_32
>> >> > /*
>> >> > * For building the 16-bit code we want to explicitly specify 32-bit
>> >> > * push/pop operations, rather than just saying 'pushf' or 'popf' and
>> >> >- * letting the compiler choose. But this is also included from the
>> >> >- * compressed/ directory where it may be 64-bit code, and thus needs
>> >> >- * to be 'pushfq' or 'popfq' in that case.
>> >> >+ * letting the compiler choose.
>> >> > */
>> >> >-#ifdef __x86_64__
>> >> >-#define PUSHF "pushfq"
>> >> >-#define POPF "popfq"
>> >> >-#else
>> >> >-#define PUSHF "pushfl"
>> >> >-#define POPF "popfl"
>> >> >-#endif
>> >> >-
>> >> >-int has_eflag(unsigned long mask)
>> >> >+bool has_eflag(unsigned long mask)
>> >> > {
>> >> > unsigned long f0, f1;
>> >> >
>> >> >- asm volatile(PUSHF " \n\t"
>> >> >- PUSHF " \n\t"
>> >> >+ asm volatile("pushfl \n\t"
>> >> >+ "pushfl \n\t"
>> >> > "pop %0 \n\t"
>> >> > "mov %0,%1 \n\t"
>> >> > "xor %2,%1 \n\t"
>> >> > "push %1 \n\t"
>> >> >- POPF " \n\t"
>> >> >- PUSHF " \n\t"
>> >> >+ "popfl \n\t"
>> >> >+ "pushfl \n\t"
>> >> > "pop %1 \n\t"
>> >> >- POPF
>> >> >+ "popfl"
>> >> > : "=&r" (f0), "=&r" (f1)
>> >> > : "ri" (mask));
>> >> >
>> >> > return !!((f0^f1) & mask);
>> >> > }
>> >> >+#endif
>> >> >
>> >> > void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
>> >> > {
>> >> >diff --git a/arch/x86/boot/cpuflags.h b/arch/x86/boot/cpuflags.h
>> >> >index fdcc2aa4c3c4..a398d9204ad0 100644
>> >> >--- a/arch/x86/boot/cpuflags.h
>> >> >+++ b/arch/x86/boot/cpuflags.h
>> >> >@@ -15,7 +15,11 @@ struct cpu_features {
>> >> > extern struct cpu_features cpu;
>> >> > extern u32 cpu_vendor[3];
>> >> >
>> >> >-int has_eflag(unsigned long mask);
>> >> >+#ifdef CONFIG_X86_32
>> >> >+bool has_eflag(unsigned long mask);
>> >> >+#else
>> >> >+static inline bool has_eflag(unsigned long mask) { return true; }
>> >> >+#endif
>> >> > void get_cpuflags(void);
>> >> > void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d);
>> >> > bool has_cpuflag(int flag);
>> >>
>> >> PUSF et al → pushf
>> >>
>> >> The -l and -q suffixes have been optional for a long time.
>> >
>> >No, not in this case. Please see the comment:
>> >
>> >/*
>> >* For building the 16-bit code we want to explicitly specify 32-bit
>> >* push/pop operations, rather than just saying 'pushf' or 'popf' and
>> >* letting the compiler choose.
>> >*/
>> >
>> >We are building 16-bit code here, and we want PUSHFL, the one with
>> >operand size prefix 0x66.
>> >
>> >Please consider the following code:
>> >
>> > .code16
>> > pushf
>> > pushfl
>> >
>> >as -o push.o push.s
>> >
>> >objdump -dr -Mdata16 push.o
>> >
>> >0000000000000000 <.text>:
>> > 0: 9c pushf
>> > 1: 66 9c pushfl
>> >
>> >Uros.
>> >
>>
>> *plonk* I should have remembered (.code16gcc is different then
>> .code16 though.) I wrote the damned things after all...
>
>So I'll read this as a tentative:
>
> Acked-by: H. Peter Anvin <hpa@zytor.com>
>
>Let me know if I'm not reading the room right. :-)
>
> Ingo
Indeed.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [tip: x86/boot] x86/boot: Do not test if AC and ID eflags are changeable on x86_64
2025-03-07 9:10 [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64 Uros Bizjak
2025-03-07 11:58 ` H. Peter Anvin
@ 2025-03-08 19:51 ` tip-bot2 for Uros Bizjak
1 sibling, 0 replies; 10+ messages in thread
From: tip-bot2 for Uros Bizjak @ 2025-03-08 19:51 UTC (permalink / raw)
To: linux-tip-commits
Cc: Uros Bizjak, Ingo Molnar, H. Peter Anvin, Linus Torvalds,
Brian Gerst, Nathan Chancellor, Nick Desaulniers, x86,
linux-kernel
The following commit has been merged into the x86/boot branch of tip:
Commit-ID: 558fc8e1869ca6e1eb99a1e2b52f6c35424d4adf
Gitweb: https://git.kernel.org/tip/558fc8e1869ca6e1eb99a1e2b52f6c35424d4adf
Author: Uros Bizjak <ubizjak@gmail.com>
AuthorDate: Fri, 07 Mar 2025 10:10:03 +01:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Sat, 08 Mar 2025 20:36:26 +01:00
x86/boot: Do not test if AC and ID eflags are changeable on x86_64
The test for the changeabitily of AC and ID EFLAGS is used to
distinguish between i386 and i486 processors (AC) and to test
for CPUID instruction support (ID).
Skip these tests on x86_64 processors as they always supports CPUID.
Also change the return type of has_eflag() to bool.
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Acked-by: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Link: https://lore.kernel.org/r/20250307091022.181136-1-ubizjak@gmail.com
---
arch/x86/boot/cpuflags.c | 26 +++++++++-----------------
arch/x86/boot/cpuflags.h | 6 +++++-
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/arch/x86/boot/cpuflags.c b/arch/x86/boot/cpuflags.c
index d75237b..2150a01 100644
--- a/arch/x86/boot/cpuflags.c
+++ b/arch/x86/boot/cpuflags.c
@@ -29,40 +29,32 @@ static int has_fpu(void)
return fsw == 0 && (fcw & 0x103f) == 0x003f;
}
+#ifdef CONFIG_X86_32
/*
* For building the 16-bit code we want to explicitly specify 32-bit
* push/pop operations, rather than just saying 'pushf' or 'popf' and
- * letting the compiler choose. But this is also included from the
- * compressed/ directory where it may be 64-bit code, and thus needs
- * to be 'pushfq' or 'popfq' in that case.
+ * letting the compiler choose.
*/
-#ifdef __x86_64__
-#define PUSHF "pushfq"
-#define POPF "popfq"
-#else
-#define PUSHF "pushfl"
-#define POPF "popfl"
-#endif
-
-int has_eflag(unsigned long mask)
+bool has_eflag(unsigned long mask)
{
unsigned long f0, f1;
- asm volatile(PUSHF " \n\t"
- PUSHF " \n\t"
+ asm volatile("pushfl \n\t"
+ "pushfl \n\t"
"pop %0 \n\t"
"mov %0,%1 \n\t"
"xor %2,%1 \n\t"
"push %1 \n\t"
- POPF " \n\t"
- PUSHF " \n\t"
+ "popfl \n\t"
+ "pushfl \n\t"
"pop %1 \n\t"
- POPF
+ "popfl"
: "=&r" (f0), "=&r" (f1)
: "ri" (mask));
return !!((f0^f1) & mask);
}
+#endif
void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d)
{
diff --git a/arch/x86/boot/cpuflags.h b/arch/x86/boot/cpuflags.h
index fdcc2aa..a398d92 100644
--- a/arch/x86/boot/cpuflags.h
+++ b/arch/x86/boot/cpuflags.h
@@ -15,7 +15,11 @@ struct cpu_features {
extern struct cpu_features cpu;
extern u32 cpu_vendor[3];
-int has_eflag(unsigned long mask);
+#ifdef CONFIG_X86_32
+bool has_eflag(unsigned long mask);
+#else
+static inline bool has_eflag(unsigned long mask) { return true; }
+#endif
void get_cpuflags(void);
void cpuid_count(u32 id, u32 count, u32 *a, u32 *b, u32 *c, u32 *d);
bool has_cpuflag(int flag);
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-03-09 0:34 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-07 9:10 [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86_64 Uros Bizjak
2025-03-07 11:58 ` H. Peter Anvin
2025-03-07 12:15 ` Uros Bizjak
2025-03-07 13:13 ` H. Peter Anvin
2025-03-07 13:45 ` Uros Bizjak
2025-03-07 14:46 ` H. Peter Anvin
2025-03-08 10:12 ` Uros Bizjak
2025-03-08 19:34 ` Ingo Molnar
2025-03-09 0:33 ` H. Peter Anvin
2025-03-08 19:51 ` [tip: x86/boot] " 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.