Received-SPF: pass (google.com: domain of linux-kernel+bounces-60709-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240208220604.140859-1-seanjc@google.com> <CAKwvOdk_obRUkD6WQHhS9uoFVe3HrgqH5h+FpqsNNgmj4cmvCQ@mail.gmail.com>
 <DM6PR02MB40587AD6ABBF1814E9CCFA7CB84B2@DM6PR02MB4058.namprd02.prod.outlook.com>
 <CAHk-=wi3p5C1n03UYoQhgVDJbh_0ogCpwbgVGnOdGn6RJ6hnKA@mail.gmail.com>
 <ZcZyWrawr1NUCiQZ@google.com> <CAKwvOdmKaYYxf7vjvPf2vbn-Ly+4=JZ_zf+OcjYOkWCkgyU_kA@mail.gmail.com>
 <CAHk-=wgEABCwu7HkJufpWC=K7u_say8k6Tp9eHvAXFa4DNXgzQ@mail.gmail.com> <CAHk-=wgBt9SsYjyHWn1ZH5V0Q7P6thqv_urVCTYqyWNUWSJ6_g@mail.gmail.com>
In-Reply-To: <CAHk-=wgBt9SsYjyHWn1ZH5V0Q7P6thqv_urVCTYqyWNUWSJ6_g@mail.gmail.com>
From: Uros Bizjak <ubizjak@gmail.com>
Date: Sun, 11 Feb 2024 12:12:37 +0100
Message-ID: <CAFULd4ZUa56KDLXSoYjoQkX0BcJwaipy3ZrEW+0tbi_Lz3FYAw@mail.gmail.com>
Subject: Re: [PATCH] Kconfig: Explicitly disable asm goto w/ outputs on gcc-11
 (and earlier)
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>, Jakub Jelinek <jakub@redhat.com>, 
	Sean Christopherson <seanjc@google.com>, "Andrew Pinski (QUIC)" <quic_apinski@quicinc.com>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Masahiro Yamada <masahiroy@kernel.org>, 
	Peter Zijlstra <peterz@infradead.org>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 9, 2024 at 9:39=E2=80=AFPM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, 9 Feb 2024 at 11:01, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > We should also probably get rid of the existing "asm_volatile_goto()"
> > macro name entirely. That name was always pretty horrific, in that it
> > didn't even mark the asm as volatile even in the case where it did
> > anything.
> >
> > So the name of that macro made little sense, and the new workaround
> > should be only for asm goto with outputs. So I'd suggest jmaking a new
> > macro with that name:
> >
> >    #define asm_goto_output(x...)
> >
> > and on gcc use that old workaround, and on clang just make it be a
> > plain "asm goto".
>
> So here's a suggested patch that does this.
>
> It's largely done with "git grep" and "sed -i", plus some manual
> fixups for the (few) cases where we have outputs.
>
> It looks superficially sane to me, and it passed an allmodconfig build
> with gcc, but I'm not going to claim that it is really tested.
>
> Sean? Does this work for the case you noticed?
>
> Basically this gets rid of the old "asm_volatile_goto()" entirely as
> useless, but replaces it with "asm_goto_outputs()" for the places
> where we have outputs.
>
> And then for gcc, it makes those cases
>
>  (a) use "asm volatile goto" to fix the fact that some versions of gcc
> will have missed the "volatile"
>
>  (b) adds that extra empty asm as a second barrier after the "real"
> asm goto statement
>
> That (b) is very much voodoo programming, but it matches the old magic
> barrier thing that Jakub Jelinek suggested for the really *old* gcc
> bug wrt plain (non-output) "asm goto". The underlying bug for _that_
> was fixed long ago:
>
>     http://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D58670
>
> We removed that for plain "asm goto" workaround a couple of years ago,
> so "asm_volatile_goto()" has been a no-op since June 2022, but this
> now resurrects that hack for the output case.
>
> I'm not loving it, but Sean seemed to confirm that it fixes the code
> generation problem, so ...
>
> Adding Uros to the cc, since he is both involved with gcc and with the
> previous asm goto workaround removal, so maybe he has other
> suggestions. Uros, see
>
>     https://lore.kernel.org/all/20240208220604.140859-1-seanjc@google.com=
/

I'd suggest the original poster to file a bug report in the GCC
bugzilla. This way, the bug can be properly analysed and eventually
fixed. The detailed instructions are available at
https://gcc.gnu.org/bugs/

> for background.
>
> Also adding Jakub since I'm re-using the hack he suggested for a
> different - but similar - case. He may have strong opinions too, and
> may object to that particular monkey-see-monkey-do voodoo programming.

This big-hammer approach will effectively disable all optimizations
around the asm, and should really be used only as a last resort. As
learned from the PR58670 (linked above), these workarounds can stay in
the kernel forever, and even when the compiler is fixed, there is
little incentive to remove them - developers from the kernel world do
not want to touch them, because "the workaround just works", and
developers from the compiler world do not want to touch them either,
because of unknown hidden consequences of removal.

Please note, that the kernel is a heavy user of asm statements, and in
GCC some forms of asm statements were developed specifically for
kernel use, e.g. CC-output, and asm goto in all of its flavors. They
have little or no use outside the kernel. If the kernel disables all
optimizations around these statements, there actually remain no
real-world testcases of how compiler optimizations interact with asm
statements, and when new optimizations are introduced to the compiler,
asm statemets are left behind (*).

(*) When the GCC compiler nears its release, people start to test it
on their code bases, and the kernel is one of the most important code
bases as far as the compiler is concerned. We count on bugreports from
these tests to fix possible fall-out from new optimizations, and the
big-hammer workaround will just hide the possible problems.

So, I'd suggest at least limit the workaround to known-bad compilers.

Thanks,
Uros.