[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFULd4a2wXkA0dBVzZnJomsghi8X0i98LfaJ7TTdVQgJzF65dw@mail.gmail.com>
Date: Wed, 21 Jan 2026 17:07:04 +0100
From: Uros Bizjak <ubizjak@...il.com>
To: "H. Peter Anvin" <hpa@...or.com>
Cc: Thomas Gleixner <tglx@...nel.org>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, Petr Mladek <pmladek@...e.com>,
Andrew Morton <akpm@...ux-foundation.org>, Kees Cook <kees@...nel.org>,
"Peter Zijlstra (Intel)" <peterz@...radead.org>, Nathan Chancellor <nathan@...nel.org>,
Kiryl Shutsemau <kas@...nel.org>, Rick Edgecombe <rick.p.edgecombe@...el.com>,
linux-kernel@...r.kernel.org, linux-coco@...ts.linux.dev, x86@...nel.org
Subject: Re: [PATCH v1 11/14] x86/boot: use __seg_fs and __seg_gs in the
real-mode boot code
On Tue, Jan 20, 2026 at 8:54 PM H. Peter Anvin <hpa@...or.com> wrote:
>
> All supported versions of gcc support __seg_fs and __seg_gs now.
> All supported versions of clang support __seg_fs and __seg_gs too,
> except for two bugs (as of clang 21, at least):
>
> 1. The %fs: and %gs: prefix does not get emitted in inline assembly.
> 2. An internal compiler error when addressing symbols directly.
>
> However, none of these are required in the boot code. Furthermore,
> this makes it possible to remove the absolute_pointer() hack in the
> fs/gs access functions.
>
> This requires adding a barrier() to a20.c, to prevent the compiler
> from eliding the load from the aliased memory address.
>
> Remove the unused memcmp_[fg]s() functions.
>
> Finally, ds() is by necessity constant, so mark the function as such.
>
> Signed-off-by: H. Peter Anvin (Intel) <hpa@...or.com>
Reviewed-by: Uros Bizjak <ubizjak@...il.com>
> ---
> arch/x86/boot/a20.c | 1 +
> arch/x86/boot/boot.h | 81 ++++++++++++++------------------------------
> 2 files changed, 27 insertions(+), 55 deletions(-)
>
> diff --git a/arch/x86/boot/a20.c b/arch/x86/boot/a20.c
> index 3ab6cd8eaa31..52c3fccdcb70 100644
> --- a/arch/x86/boot/a20.c
> +++ b/arch/x86/boot/a20.c
> @@ -63,6 +63,7 @@ static int a20_test(int loops)
> while (loops--) {
> wrgs32(++ctr, A20_TEST_ADDR);
> io_delay(); /* Serialize and make delay constant */
> + barrier(); /* Compiler won't know about fs/gs overlap */
> ok = rdfs32(A20_TEST_ADDR+0x10) ^ ctr;
> if (ok)
> break;
> diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
> index b4eb8405ba55..4d3549ed7987 100644
> --- a/arch/x86/boot/boot.h
> +++ b/arch/x86/boot/boot.h
> @@ -45,7 +45,7 @@ static inline void io_delay(void)
>
> /* These functions are used to reference data in other segments. */
>
> -static inline u16 ds(void)
> +static inline __attribute_const__ u16 ds(void)
> {
> u16 seg;
> asm("movw %%ds,%0" : "=rm" (seg));
> @@ -54,7 +54,7 @@ static inline u16 ds(void)
>
> static inline void set_fs(u16 seg)
> {
> - asm volatile("movw %0,%%fs" : : "rm" (seg));
> + asm volatile("movw %0,%%fs" : : "rm" (seg) : "memory");
> }
> static inline u16 fs(void)
> {
> @@ -65,7 +65,7 @@ static inline u16 fs(void)
>
> static inline void set_gs(u16 seg)
> {
> - asm volatile("movw %0,%%gs" : : "rm" (seg));
> + asm volatile("movw %0,%%gs" : : "rm" (seg) : "memory");
> }
> static inline u16 gs(void)
> {
> @@ -76,96 +76,67 @@ static inline u16 gs(void)
>
> typedef unsigned int addr_t;
>
> +/*
> + * WARNING: as of clang 21, clang has the following two bugs related
> + * to __seg_fs and __seg_gs:
> + *
> + * 1. The %fs: and %gs: prefix does not get emitted in inline assembly.
> + * 2. An internal compiler error when addressing symbols directly.
> + *
> + * Neither of those constructs are currently used in the boot code.
> + * If they ever are, and those bugs still remain, then those bugs will
> + * need to be worked around.
> + */
> static inline u8 rdfs8(addr_t addr)
> {
> - u8 *ptr = (u8 *)absolute_pointer(addr);
> - u8 v;
> - asm volatile("movb %%fs:%1,%0" : "=q" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_fs const u8 *)addr;
> }
> static inline u16 rdfs16(addr_t addr)
> {
> - u16 *ptr = (u16 *)absolute_pointer(addr);
> - u16 v;
> - asm volatile("movw %%fs:%1,%0" : "=r" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_fs const u16 *)addr;
> }
> static inline u32 rdfs32(addr_t addr)
> {
> - u32 *ptr = (u32 *)absolute_pointer(addr);
> - u32 v;
> - asm volatile("movl %%fs:%1,%0" : "=r" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_fs const u32 *)addr;
> }
>
> static inline void wrfs8(u8 v, addr_t addr)
> {
> - u8 *ptr = (u8 *)absolute_pointer(addr);
> - asm volatile("movb %1,%%fs:%0" : "+m" (*ptr) : "qi" (v));
> + *(__seg_fs u8 *)addr = v;
> }
> static inline void wrfs16(u16 v, addr_t addr)
> {
> - u16 *ptr = (u16 *)absolute_pointer(addr);
> - asm volatile("movw %1,%%fs:%0" : "+m" (*ptr) : "ri" (v));
> + *(__seg_fs u16 *)addr = v;
> }
> static inline void wrfs32(u32 v, addr_t addr)
> {
> - u32 *ptr = (u32 *)absolute_pointer(addr);
> - asm volatile("movl %1,%%fs:%0" : "+m" (*ptr) : "ri" (v));
> + *(__seg_fs u32 *)addr = v;
> }
>
> static inline u8 rdgs8(addr_t addr)
> {
> - u8 *ptr = (u8 *)absolute_pointer(addr);
> - u8 v;
> - asm volatile("movb %%gs:%1,%0" : "=q" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_gs const u8 *)addr;
> }
> static inline u16 rdgs16(addr_t addr)
> {
> - u16 *ptr = (u16 *)absolute_pointer(addr);
> - u16 v;
> - asm volatile("movw %%gs:%1,%0" : "=r" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_gs const u16 *)addr;
> }
> static inline u32 rdgs32(addr_t addr)
> {
> - u32 *ptr = (u32 *)absolute_pointer(addr);
> - u32 v;
> - asm volatile("movl %%gs:%1,%0" : "=r" (v) : "m" (*ptr));
> - return v;
> + return *(__seg_gs const u32 *)addr;
> }
>
> static inline void wrgs8(u8 v, addr_t addr)
> {
> - u8 *ptr = (u8 *)absolute_pointer(addr);
> - asm volatile("movb %1,%%gs:%0" : "+m" (*ptr) : "qi" (v));
> + *(__seg_gs u8 *)addr = v;
> }
> static inline void wrgs16(u16 v, addr_t addr)
> {
> - u16 *ptr = (u16 *)absolute_pointer(addr);
> - asm volatile("movw %1,%%gs:%0" : "+m" (*ptr) : "ri" (v));
> + *(__seg_gs u16 *)addr = v;
> }
> static inline void wrgs32(u32 v, addr_t addr)
> {
> - u32 *ptr = (u32 *)absolute_pointer(addr);
> - asm volatile("movl %1,%%gs:%0" : "+m" (*ptr) : "ri" (v));
> -}
> -
> -/* Note: these only return true/false, not a signed return value! */
> -static inline bool memcmp_fs(const void *s1, addr_t s2, size_t len)
> -{
> - bool diff;
> - asm volatile("fs repe cmpsb"
> - : "=@...z" (diff), "+D" (s1), "+S" (s2), "+c" (len));
> - return diff;
> -}
> -static inline bool memcmp_gs(const void *s1, addr_t s2, size_t len)
> -{
> - bool diff;
> - asm volatile("gs repe cmpsb"
> - : "=@...z" (diff), "+D" (s1), "+S" (s2), "+c" (len));
> - return diff;
> + *(__seg_gs u32 *)addr = v;
> }
>
> /* Heap -- available for dynamic lists. */
> --
> 2.52.0
>
Powered by blists - more mailing lists