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, 12 Oct 2023 10:10:08 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Uros Bizjak <ubizjak@...il.com>
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 Thu, 12 Oct 2023 at 09:55, Uros Bizjak <ubizjak@...il.com> wrote:
>
> An example:

Oh, I'm convinced.

The fix seems to be a simple one-liner, ie just

-       asm(__pcpu_op2_##size(op, __percpu_arg(P[var]), "%[val]")       \
+       asm(__pcpu_op2_##size(op, __percpu_arg(a[var]), "%[val]")       \

and it turns out that we have other places where I think we could use that '%a',

For example, we have things like this:

        asm ("lea sme_cmdline_arg(%%rip), %0"
             : "=r" (cmdline_arg)
             : "p" (sme_cmdline_arg));

and I think the only reason we do that ridiculous asm is that the code
in question really does want that (%rip) encoding. It sounds like this
could just do

        asm ("lea %a1, %0"
             : "=r" (cmdline_arg)
             : "p" (sme_cmdline_arg));

instead. Once again, I claim ignorance of the operand modifiers as the
reason for these kinds of things.

But coming back to the stable op thing, I do wonder if there is some
way we could avoid the unnecessary reload.

I don't hate Nadav's patch, so that part is fine, but I'd like to
understand what it is that makes gcc think it needs to reload. We have
other cases (like the ALTERNATIVE() uses) where we *have* to use
inline asm, so it would be good to know...

Is it just that "p" (in the constraint, not "P" in the modifier) ends
up always being seen as a memory access, even when we only use the
address?

That part has never really been something we've been entirely clear
on. We *are* passing in just the address, so the hope in *that* place
is that it's only an address dependency, not a memory one.

(Of course, we use 'p' constraints in other places, and may
expect/assume that those have memory dependency. Again - this has
never been entirely clear)

              Linus