linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Catalin Marinas <catalin.marinas@arm.com>,
	Peter Smith <Peter.Smith@arm.com>, Will Deacon <will@kernel.org>
Cc: Robin Murphy <Robin.Murphy@arm.com>,
	Naohiro Aota <naohiro.aota@wdc.com>,
	Stephen Boyd <swboyd@google.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Manoj Gupta <manojgupta@google.com>,
	Luis Lozano <llozano@google.com>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Vincenzo Frascino <Vincenzo.Frascino@arm.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>,
	Kristof Beyls <Kristof.Beyls@arm.com>,
	Victor Campos <Victor.Campos@arm.com>,
	"david.spickett@linaro.org" <david.spickett@linaro.org>,
	Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm
Date: Thu, 28 May 2020 12:06:46 -0700	[thread overview]
Message-ID: <CAKwvOdkBr9Y+J0ZgZT8RR60Kh-kG7Q_annQU+s=+RXg=qeLFNQ@mail.gmail.com> (raw)
In-Reply-To: <20200528094154.GB2961@gaia>

On Thu, May 28, 2020 at 12:20 AM Will Deacon <will@kernel.org> wrote:
> > Yes, I know that :)

Right, I forgot; you wrote the 64b one. :)

On Thu, May 28, 2020 at 2:41 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Thu, May 28, 2020 at 09:05:08AM +0100, Peter Smith wrote:
> > I suggest using Arm if you need a frame pointer, and disable the
> > frame pointer if you want/need to use Thumb. My understanding is that
> > runtime unwinding using the frame pointer in Thumb is already difficult
> > due to Arm and Thumb functions using different registers for the frame
> > pointer.
>
> IIRC from the Thumb-2 kernel porting days, even in the absence of
> ARM/Thumb interworking, the Thumb-2 frame pointer was pretty useless for
> unwinding since it points to the bottom of the current stack frame (the
> reason I think is that some LDR/STR instructions with negative indexing
> are not available). So finding the previous frame pointer was not
> possible and had to rely on the exception unwinding information.

Eureka!  That's it!  Implicit state of -fomit-frame-pointer.

Ok, forgetting ARCH=arm64 for a second, for ARCH=arm we have the
choice CONFIG_THUMB2_KERNEL.  Regardless of which is chosen, we
*always* explicitly set -mthumb or -marm, but we never rely on
implicit defaults.  Again for ARCH=arm, we have a choice of unwinders,
or at least we do when *not* targeting thumb.  If we select
CONFIG_THUMB2_KERNEL, then CONFIG_UNWINDER_FRAME_POINTER cannot be
selected.

arch/arm/Kconfig.debug:
  57 config UNWINDER_FRAME_POINTER
  58   bool "Frame pointer unwinder"
  59   depends on !THUMB2_KERNEL
...

CONFIG_UNWINDER_FRAME_POINTER selects CONFIG_FRAME_POINTER which sets
-fno-omit-frame-pointer.  Otherwise, it looks like the choice of
-f{no-}omit-frame-pointer is left unspecified, relying on compiler
defaults.

And that's just for ARCH=arm.  Returning to ARCH=arm64, and the compat
vdso in particular, it doesn't look like we ever set
-f{no-}omit-frame-pointer either, so again we're looking at the
compiler defaults.

And when we look at the default behavior for the implicit state of
-f{no-}omit-frame-pointer, we find differences.

Here's a test case you can play around with:
https://godbolt.org/z/0oY39t

For Clang, can you tell what the default state is if left off?
For GCC, can you tell what the default state is if left off?
Do they match?
Is this specific to -mthumb?
(Bonus: what happens when you remove `-O2`?)

Answers:
-fno-omit-frame-pointer
-fomit-frame-pointer
No.
No.
-fno-omit-frame-pointer in GCC (-fomit-frame-pointer is an optimization)

Deja vu, I fixed a very similar discrepancy reported by Linus not too long ago.
https://reviews.llvm.org/D74698
Looking at the relevant logic in Clang:
https://github.com/llvm/llvm-project/blob/58beb76b7bd2f7caa1df461b9db6629521c3b60b/clang/lib/Driver/ToolChains/Clang.cpp#L527-L591
Noticely absent are arm, thumb, aarch64, and big endian variants,
specifically here:
https://github.com/llvm/llvm-project/blob/58beb76b7bd2f7caa1df461b9db6629521c3b60b/clang/lib/Driver/ToolChains/Clang.cpp#L556-L571

I should fix that in Clang.

That should also speed up our 32b ARM kernels that select
CONFIG_THUMB2_KERNEL (ie. CrOS veyron-minnie platform, rk3288).
Shouldn't make a difference for 64b ARM kernels since those select
frame pointers.  Though I am curious about the userspaces now for CrOS
and Android...

On Thu, May 28, 2020 at 1:05 AM Peter Smith <Peter.Smith@arm.com> wrote:
> > Hope this helps

Always, m8, you're the expert.

So r11 will be the frame pointer for arm and thumb according to latest
aapcs, but the compilers haven't yet made any changes related to this,
and can still use r7 in a bunch of cases (-mthumb
--target=arm-linux-gnueabi being the most relevant one for our
discussion).

> On Thu, May 28, 2020 at 12:20 AM Will Deacon <will@kernel.org> wrote:
> your
> patch is papering over a deeper issue.

Ah, your right.  Sorry, I was wrong.  I owe you (another) beer?  I'm
going into debt over these and will have to take out a loan, soon!
-- 
Thanks,
~Nick Desaulniers

      reply	other threads:[~2020-05-28 19:07 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-26 17:31 [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm Nick Desaulniers
2020-05-26 18:55 ` Stephen Boyd
2020-05-26 20:45 ` Will Deacon
2020-05-27 13:53   ` Dave Martin
2020-05-27 17:58     ` Nick Desaulniers
2020-05-27 13:45 ` Robin Murphy
2020-05-27 17:55   ` Nick Desaulniers
2020-05-27 18:08     ` Will Deacon
2020-05-27 18:17       ` Nick Desaulniers
2020-05-28  7:20         ` Will Deacon
2020-06-08 20:57           ` [PATCH v2] arm64: vdso32: add CONFIG_THUMB2_COMPAT_VDSO Nick Desaulniers
2020-06-09 20:35             ` Catalin Marinas
2020-06-09 23:55               ` Nick Desaulniers
2020-06-10  8:47                 ` Will Deacon
2020-06-10 10:29                   ` Catalin Marinas
2020-06-10 10:32                     ` Will Deacon
2020-06-10 11:21             ` Will Deacon
2020-05-27 19:28     ` [PATCH] arm64: vdso32: force vdso32 to be compiled as -marm Robin Murphy
2020-05-27 20:02       ` Robin Murphy
2020-05-27 20:14       ` Nick Desaulniers
2020-05-27 20:31         ` Nick Desaulniers
2020-05-27 21:47           ` Nick Desaulniers
2020-05-28  8:05           ` Peter Smith
2020-05-28  9:41             ` Catalin Marinas
2020-05-28 19:06               ` Nick Desaulniers [this message]

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='CAKwvOdkBr9Y+J0ZgZT8RR60Kh-kG7Q_annQU+s=+RXg=qeLFNQ@mail.gmail.com' \
    --to=ndesaulniers@google.com \
    --cc=Kristof.Beyls@arm.com \
    --cc=Peter.Smith@arm.com \
    --cc=Robin.Murphy@arm.com \
    --cc=Victor.Campos@arm.com \
    --cc=Vincenzo.Frascino@arm.com \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=david.spickett@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llozano@google.com \
    --cc=manojgupta@google.com \
    --cc=masahiroy@kernel.org \
    --cc=naohiro.aota@wdc.com \
    --cc=natechancellor@gmail.com \
    --cc=swboyd@google.com \
    --cc=will@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).