linux-kernel - Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable on x86

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFULd4aMkxYp6L=grE7TrvvfdX7gTGOTAJgojJ=mjHfDLJ=kVQ@mail.gmail.com>
Date: Fri, 7 Mar 2025 13:15:42 +0100
From: Uros Bizjak <ubizjak@...il.com>
To: "H. Peter Anvin" <hpa@...or.com>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH] x86/boot: Do not test if AC and ID eflags are changeable
 on x86_64

On Fri, Mar 7, 2025 at 12:58 PM H. Peter Anvin <hpa@...or.com> wrote:
>
> On March 7, 2025 1:10:03 AM PST, Uros Bizjak <ubizjak@...il.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@...il.com>
> >Cc: Thomas Gleixner <tglx@...utronix.de>
> >Cc: Ingo Molnar <mingo@...nel.org>
> >Cc: Borislav Petkov <bp@...en8.de>
> >Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> >Cc: "H. Peter Anvin" <hpa@...or.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.