linux-kernel - Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu

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]

Date:   Thu, 19 Oct 2023 18:32:21 +0200
From:   Uros Bizjak <ubizjak@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Nadav Amit <namit@...are.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Brian Gerst <brgerst@...il.com>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr()

On Wed, Oct 18, 2023 at 10:22 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Wed, 18 Oct 2023 at 12:33, Uros Bizjak <ubizjak@...il.com> wrote:
> >
> > This pach works for me:
>
> Looks fine.
>
> But you actually bring up another issue:
>
> > BTW: I also don't understand the comment from include/linux/smp.h:
> >
> > /*
> >  * Allow the architecture to differentiate between a stable and unstable read.
> >  * For example, x86 uses an IRQ-safe asm-volatile read for the unstable but a
> >  * regular asm read for the stable.
>
> I think the comment is badly worded, but I think the issue may actually be real.
>
> One word: rematerialization.
>
> The thing is, turning inline asm accesses to regular compiler loads
> has a *very* bad semantic problem: the compiler may now feel like it
> can not only combine the loads (ok), but also possibly rematerialize
> values by re-doing the loads (NOT OK!).
>
> IOW, the kernel often has very strict requirements of "at most once"
> behavior, because doing two loads might give different results.
>
> The cpu number is a good example of this.
>
> And yes, sometimes we use actual volatile accesses for them
> (READ_ONCE() and WRITE_ONCE()) but those are *horrendous* in general,
> and are much too strict. Not only does gcc generally lose its mind
> when it sees volatile (ie it stops doing various sane combinations
> that would actually be perfectly valid), but it obviously also stops
> doing CSE on the loads (as it has to).
>
> So the "non-volatile asm" has been a great way to get the "at most
> one" behavior: it's safe wrt interrupts changing the value, because
> you will see *one* value, not two. As far as we know, gcc never
> rematerializes the output of an inline asm. So when you use an inline
> asm, you may have the result CSE'd, but you'll never see it generate
> more than *one* copy of the inline asm.
>
> (Of course, as with so much about inline asm, that "knowledge" is not
> necessarily explicitly spelled out anywhere, and it's just "that's how
> it has always worked").

Perhaps you will be interested in chapter 6.47.2.1, "Volatile" of GCC
manual that says:

" Under certain circumstances, GCC may duplicate (or remove duplicates
of) your assembly code when optimizing."

The compiler may re-materialize non-volatile asm in the same way it
may re-materialize arguments from non-volatile memory. To avoid this,
volatile asm is necessary when unstable memory arguments are accessed
using this_* variants.

Uros.