linux-kernel - Re: [PATCH v1 11/14] x86/boot: use __seg_fs and __seg

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4E5A81C0-1938-4FB0-A0ED-2CCF03B9C480@zytor.com>
Date: Wed, 21 Jan 2026 07:13:49 -0800
From: "H. Peter Anvin" <hpa@...or.com>
To: Uros Bizjak <ubizjak@...il.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 January 21, 2026 12:56:39 AM PST, Uros Bizjak <ubizjak@...il.com> wrote:
>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>
>> ---
>>  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;
>
>This particular issue is not due to the compiler not knowing about
>fs/gs overlap (if the compiler determines that write and read are to
>the same non-volatile address, it would simply remove both, load and
>store), but due to the compiler performing load hoisting and store
>sinking from the loop. The compiler considers these two addresses as
>two different addresses (they are also defined in two different named
>address spaces), and optimizes access to them. So, without barrier(),
>it simply loads the value from A20_TEST_ADDR and A20_TEST_ADDR+0x10
>before the loop, resulting in:
>
>  9:   65 8b 0d 00 02 00 00    mov    %gs:0x200,%ecx
> ...
> 16:   64 8b 35 10 02 00 00    mov    %fs:0x210,%esi
> ...
> 28:   83 ea 01                sub    $0x1,%edx
> 2b:   74 0a                   je     37 <a20_test_ref+0x37>
> 2d:   e6 80                   out    %al,$0x80
> 2f:   89 d9                   mov    %ebx,%ecx
> 31:   29 d1                   sub    %edx,%ecx
> 33:   39 ce                   cmp    %ecx,%esi
> 35:   74 f1                   je     28 <a20_test_ref+0x28>
>
>The solution with barrier() introduces memory clobber between store
>and load, so the compiler is now forced to load and store the values
>due to the side effects of the barrier() inbetween. This kind of
>works, but is just a workaround for what really happens. In reality,
>the value at the test address changes "behind the compiler back", IOW
>- variable’s value can change in ways the compiler cannot predict.
>
>My proposal is to use a volatile pointer to an absolute address, so
>the unwanted optimizations are suppressed. The generated code is the
>same as with barrier(), but now the code tells the compiler that every
>read and write to this address must happen exactly as written in the
>source code. Before your patch, the accessors were defined with
>volatile asm, and this is the place where volatile qualifier matters.
>So, my proposed code would read:
>
>#define A20_TEST_ADDR    (4*0x80)
>
>#define A20_TEST_GS (*(volatile __seg_gs u32 *)A20_TEST_ADDR)
>#define A20_TEST_FS (*(volatile __seg_fs u32 *)(A20_TEST_ADDR+0x10))
>
>static int a20_test(int loops)
>{
>    int saved, ctr;
>
>    set_fs(0xffff);
>
>    saved = ctr = A20_TEST_GS;
>
>    do {
>        A20_TEST_GS = ++ctr;
>        io_delay();    /* Make constant delay */
>        if (A20_TEST_FS != ctr)
>            break;
>    } while (--loops);
>
>    A20_TEST_GS = saved;
>    return loops;
>}
>
>BR,
>Uros.
>

I disagree with that being the preferred solution, and it isn't really the Linux style.

Not only does it require making more changes to the macros, but the barrier() construct is well established in Linux as the way to indicate that a memory variable is subject to examination and/or modification by another system agent, while still being a memory variable. 

It also generally produces better code. 

So no, your analysis is, in my opinion, incorrect in light of the way the Linux memory model is already used.