All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Ard Biesheuvel <ardb@kernel.org>, Greg KH <gregkh@linuxfoundation.org>
Cc: "Sasha Levin" <sashal@kernel.org>,
	"# 3.4.x" <stable@vger.kernel.org>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	"Jian Cai" <jiancai@google.com>, "Stefan Agner" <stefan@agner.ch>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Sami Tolvanen" <samitolvanen@google.com>,
	candle.sun@unisoc.com,
	"Miles Chen (陳民樺)" <miles.chen@mediatek.com>,
	"Stephen Hines" <srhines@google.com>,
	"Luis Lozano" <llozano@google.com>,
	"Sandeep Patil" <sspatil@google.com>,
	"Marc Zyngier" <maz@kernel.org>
Subject: Re: ARCH=arm LLVM_IAS=1 patches for 5.10, 5.4, and 4.19
Date: Mon, 15 Mar 2021 15:58:07 -0700	[thread overview]
Message-ID: <CAKwvOdm6FXWVu-9YkQNNyoYmw+hkj1a7MQrRbWyUxsO2vDcnQA@mail.gmail.com> (raw)
In-Reply-To: <CAMj1kXHfQmObPZaVOZu+0M3SKFKNg5vcKmyJMXQ3RTBCqho9WQ@mail.gmail.com>

Ok, I see what went wrong.  I had tested allyesconfig and allmodconfig
on 5.4.y, but neither of those selects CONFIG_KVM=y for arm;
axm55xx_defconfig is literally the only config (out of 109) that does.

32b ARM KVM support was ripped out in
commit 541ad0150ca4 ("arm: Remove 32bit KVM host support")
which landed in v5.7-rc1. So when
commit 2cbd1cc3dcd3 ("ARM: 8991/1: use VFP assembler mnemonics if available")
was written, arch/arm/kvm/ no longer existed. If it did, then
2cbd1cc3dcd3 would have needed something like
https://gist.github.com/nickdesaulniers/980e68e9c0680fff06b1b64f2b973171.
And allmodconfig/allyesconfig testing wouldn't have caught this, only
testing axm55xx_defconfig would have.  Before KVM support was dropped,
that was the only config that explicitly enabled the config that
failed.

On Mon, Mar 15, 2021 at 10:53 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 15 Mar 2021 at 18:43, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > f77ac2e378be doesn't apply cleanly to linux-5.4.y. There's a conflict
> > in arch/arm/vfp/vfphw.S due to 5.4 missing
> > commit 2cbd1cc3dcd3 ("ARM: 8991/1: use VFP assembler mnemonics if available")
> > which is one of the patches I sent in the 5.4 series in this thread.
> > That was 1 of a 3 patch series:
> > https://lore.kernel.org/linux-arm-kernel/cover.1593205699.git.stefan@agner.ch/
> >
> > Should I separate out just those 3 and f77ac2e378be and send that for
> > 5.4, or manually backport just f77ac2e378be and note in the commit
> > message the conflict?

I assume we still want a fix for THUMB2, so I'll send a backport of
just f77ac2e378be modified to note that there was a fixup against
2cbd1cc3dcd3, since 2cbd1cc3dcd3 is problematic for CONFIG_KVM=y on
5.4.

> You haven't explained why all this effort is justified to begin with.
>
> Who cares about being able to build 4.19 or 5.4 mainline with Clang 12
> and IAS?

Ah, sorry, ChromeOS and Android very much do so.  (Google's production
kernels as well, though I don't think they have any 32b ARM machines).
Android is already building 4.19+ with LLVM_IAS=1 for
ARCH=arm64,x86_64,i686. ChromeOS is doing so for 5.4+ for
ARCH=arm64,x86_64 as well.  I'm not sure precisely what's going on in
prodkernel land, but I know they have LLVM_IAS=1 enabled for x86_64.
So when Greg says this is "for no real users" I disagree.  Maybe no
one is using LLVM_IAS=1 for ARCH=arm at this moment, but that was the
point of the backports, to enable more distros to do so.

Stable has already accepted patches to 4.19+ for these architectures
where it was made explicit that this was for LLVM_IAS=1 support.
https://lore.kernel.org/stable/CAKwvOdk_U6SEwOC-ykaVTMu1ZmEjWC8cCiTetvU2k2dQ6WPCoQ@mail.gmail.com/
https://lore.kernel.org/stable/CAKwvOd=F_wWLxhnV3J8jx1L3SXPd8NFYyOKzAh7rL0iRb_aNyA@mail.gmail.com/
https://lore.kernel.org/stable/CAKwvOdmEcjjw78K0Avj-7s5BBXcT7ARhEMMEYqpCP-ZT=2dAJw@mail.gmail.com/
https://lore.kernel.org/stable/CAKwvOdnGDHn+Y+g5AsKvOFiuF7iVAJ8+x53SgWxH9ejqEZwY9w@mail.gmail.com/
https://lore.kernel.org/stable/CAKwvOdkK1LgLC4ChptzUTC45WvE9-Sn0OqtgF7-odNSw8xLTYA@mail.gmail.com/
https://lore.kernel.org/stable/CAKwvOd=x+fVo1_mMJUGHYXpmGf8UM5yx+uWD-Ci=y=0oFX2ktg@mail.gmail.com/
https://lore.kernel.org/stable/CAKwvOdn78WAUiRtyPxW7oEhUz8GN6MkL=Jt+n17jEQXPPZE77g@mail.gmail.com/

Now it's just down to ARM and THUMB2 support.  Then we will be using a
similar toolchain regardless of ISA.  We will also then have an
evergreen toolchain, rather than one that will not be receiving future
updates and is unsupported (which becomes a problem when folks need
new things and is a liability to be removed), and more wood behind
fewer arrows so we can focus on starting on the feature requests we
have piling up (like kernel GCC plugin-like features in LLVM, like a
code model for the kernel, etc).

This has been communicated to Android OEMs
(https://android.googlesource.com/platform/prebuilts/clang/host/linux-x86/+/master/BINUTILS_KERNEL_DEPRECATION.md)
for them to help test and report issues and likely will also happen
for the next release of the NDK
(https://github.com/android/ndk/wiki/Changelog-r23#announcements).

> I am aware that Clang enablement is a prerequisite for CFI
> and LTO etcetera, and so I am fully on board with this activity for
> current and future kernels.

LTO does require LLVM_IAS=1 at the moment; there are no plans I know
of yet to port LTO to ARCH=arm, but maybe if Sami is bored? :P

> Stable kernels are a different matter, though. I tend to get
> stable-kernel-rules.rst thrown in my face for proposing backports that
> aren't nearly as large or intrusive as this stuff, but for some
> reason, those rules do not seem to apply here.

I understand.  I'm also balancing shipping patches for toolchain
support out of tree, vs upstreaming.  Everything so far has been
upstreamed, but 32b ARM support has been...more involved.  But I'm
hopeful that we will be able to expand our staff soon to better
improve that.

I think all of these patches would be useful to CrOS, so my plan was
to send the series upstream.  We can keep it downstream, where the
number of supported configurations and toolchains is more limited.
4.19+ is of interest for new Android devices this year, but 5.4 in
particular will live much longer, so we will have to carry the
divergence for longer.  I think some of the strictly UAL related
changes are relatively lower risk.

> So my suggestion would be to forget about 4.19 and 5.4 entirely for
> these changes, unless there is an obvious benefit to all consumers of
> these stable trees. Otherwise, exposing them to ongoing breakage like
> this is indefensible IMHO.
-- 
Thanks,
~Nick Desaulniers

  reply	other threads:[~2021-03-15 22:59 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-11 19:32 ARCH=arm LLVM_IAS=1 patches for 5.10, 5.4, and 4.19 Nick Desaulniers
2021-03-12 10:12 ` Greg KH
2021-03-12 17:28   ` Nick Desaulniers
2021-03-13  4:07     ` Sasha Levin
2021-03-15  9:08       ` Greg KH
2021-03-15  9:12         ` Greg KH
2021-03-15  9:16           ` Greg KH
2021-03-15 10:37             ` Ard Biesheuvel
2021-03-15 17:43               ` Nick Desaulniers
2021-03-15 17:53                 ` Ard Biesheuvel
2021-03-15 22:58                   ` Nick Desaulniers [this message]
2021-03-15 23:19                     ` [PATCH 5.4.y] ARM: 9030/1: entry: omit FP emulation for UND exceptions taken in kernel mode Nick Desaulniers
2021-03-16  6:20                       ` Ard Biesheuvel
2021-03-16 16:59                         ` [PATCH 5.4 2/2] ARM: 9044/1: vfp: use undef hook for VFP support detection Nick Desaulniers
2021-03-19 10:06                           ` Greg KH
2021-03-19 20:14                             ` Nick Desaulniers
2021-03-20 11:04                               ` Greg KH
2021-03-16 14:02                     ` ARCH=arm LLVM_IAS=1 patches for 5.10, 5.4, and 4.19 Sasha Levin
2021-03-15 19:06                 ` Greg KH
2021-03-15 20:43                   ` Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKwvOdm6FXWVu-9YkQNNyoYmw+hkj1a7MQrRbWyUxsO2vDcnQA@mail.gmail.com \
    --to=ndesaulniers@google.com \
    --cc=ardb@kernel.org \
    --cc=candle.sun@unisoc.com \
    --cc=catalin.marinas@arm.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jiancai@google.com \
    --cc=llozano@google.com \
    --cc=maz@kernel.org \
    --cc=miles.chen@mediatek.com \
    --cc=samitolvanen@google.com \
    --cc=sashal@kernel.org \
    --cc=srhines@google.com \
    --cc=sspatil@google.com \
    --cc=stable@vger.kernel.org \
    --cc=stefan@agner.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.