linux-kernel - Re: [PATCH] powerpc/irq: Modernise inline assembly in irq_soft_mask

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: <CN4BYDAY75PX.33LJ1P2VQJXD9@bobo>
Date:   Sat, 24 Sep 2022 14:00:55 +1000
From:   "Nicholas Piggin" <npiggin@...il.com>
To:     "Segher Boessenkool" <segher@...nel.crashing.org>
Cc:     "Christophe Leroy" <christophe.leroy@...roup.eu>,
        "Michael Ellerman" <mpe@...erman.id.au>,
        <linux-kernel@...r.kernel.org>, <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH] powerpc/irq: Modernise inline assembly in
 irq_soft_mask_{set,return}

On Sat Sep 24, 2022 at 8:15 AM AEST, Segher Boessenkool wrote:
> On Sat, Sep 24, 2022 at 02:26:52AM +1000, Nicholas Piggin wrote:
> > I still don't see what clauses guarantees asm("%0" ::"r"(foo)) to give
> > 13. It doesn't say access via inline assembly is special,
>
> But it is.  It is for all register variables, local and global.  I agree
> this isn't documented clearly.  For local register variables this is the
> *only* thing guaranteed; for global register vars there is more (it
> changes the ABI, there are safe/restore effects, that kind of thing).
>
> Never it is guaranteed that all accesses through this variable will use
> the register directly: this fundamentally cannot work on all archs, and
> also not at -O0.  More in general it doesn't work if some basic
> optimisations are not done, be it because of a compiler deficiency, or a
> straight out bug, or maybe it is a conscious choice in some cases.

Right, and we know better than to rely on a spec that is not 100% air
tight with no possibility of lawyering. This may be what the intention is,
it may be what gcc and clang do now, and everybody involved today agrees
with that interpretation. We still have to maintain the kernel tomorrow
though, so explicit r13 it must be.

>
> > I think if it was obviously guaranteed then this might be marginally
> > better than explicit r13 in the asm
> > 
> >        asm volatile(
> >                "stb %0,%2(%1)"
> >                :
> >                : "r" (mask),
> > 	         "r" (local_paca),
> >                  "i" (offsetof(struct paca_struct, irq_soft_mask))
> >                : "memory");
>
> (Please use "n" instead of "i".  Doesn't matter here, but it does in
> many other places.)

What is the difference? Just "i" allows assmebly-time constants?

How about "I"? that looks like it was made for it. Gives much better
errors.

Thanks,
Nick