linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Harry Wentland <harry.wentland@amd.com>,
	"Deucher, Alexander" <alexander.deucher@amd.com>
Cc: yshuiv7@gmail.com, andrew.cooper3@citrix.com,
	Arnd Bergmann <arnd@arndb.de>,
	clang-built-linux <clang-built-linux@googlegroups.com>,
	Matthias Kaehlcke <mka@google.com>,
	"S, Shirish" <shirish.s@amd.com>,
	"Zhou, David(ChunMing)" <David1.Zhou@amd.com>,
	"Koenig, Christian" <christian.koenig@amd.com>,
	amd-gfx list <amd-gfx@lists.freedesktop.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: AMDGPU and 16B stack alignment
Date: Mon, 14 Oct 2019 15:22:09 -0700	[thread overview]
Message-ID: <CAKwvOdnDVe-dahZGnRtzMrx-AH_C+2Lf20qjFQHNtn9xh=Okzw@mail.gmail.com> (raw)

Hello!

The x86 kernel is compiled with an 8B stack alignment via
    `-mpreferred-stack-boundary=3` for GCC since 3.6-rc1 via
    commit d9b0cde91c60 ("x86-64, gcc: Use
-mpreferred-stack-boundary=3 if supported")
    or `-mstack-alignment=8` for Clang. Parts of the AMDGPU driver are
    compiled with 16B stack alignment.

    Generally, the stack alignment is part of the ABI. Linking together two
    different translation units with differing stack alignment is dangerous,
    particularly when the translation unit with the smaller stack alignment
    makes calls into the translation unit with the larger stack alignment.
    While 8B aligned stacks are sometimes also 16B aligned, they are not
    always.

    Multiple users have reported General Protection Faults (GPF) when using
    the AMDGPU driver compiled with Clang. Clang is placing objects in stack
    slots assuming the stack is 16B aligned, and selecting instructions that
    require 16B aligned memory operands. At runtime, syscalls handling 8B
    stack aligned code calls into code that assumes 16B stack alignment.
    When the stack is a multiple of 8B but not 16B, these instructions
    result in a GPF.

    GCC doesn't select instructions with alignment requirements, so the GPFs
    aren't observed, but it is still considered an ABI breakage to mix and
    match stack alignment.

I have patches that basically remove -mpreferred-stack-boundary=4 and
-mstack-alignment=16 from AMDGPU:
https://github.com/ClangBuiltLinux/linux/issues/735#issuecomment-541247601
Yuxuan has tested with Clang and GCC and reported it fixes the GPF's observed.

I've split the patch into 4; same commit message but different Fixes
tags so that they backport to stable on finer granularity. 2 questions
BEFORE I send the series:

1. Would you prefer 4 patches with unique `fixes` tags, or 1 patch?
2. Was there or is there still a good reason for the stack alignment mismatch?

(Further, I think we can use -msse2 for BOTH clang+gcc after my patch,
but I don't have hardware to test on. I'm happy to write/send the
follow up patch, but I'd need help testing).
-- 
Thanks,
~Nick Desaulniers

             reply	other threads:[~2019-10-14 22:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-14 22:22 Nick Desaulniers [this message]
     [not found] ` <9e4d6378-5032-8521-13a9-d9d9519d07de@amd.com>
2019-10-15  7:19   ` AMDGPU and 16B stack alignment Arnd Bergmann
2019-10-15 10:48     ` David Laight
2019-10-15 18:05     ` Nick Desaulniers
2019-10-15 18:11       ` Nick Desaulniers
2019-10-15 18:30       ` Alex Deucher
2019-10-15 20:15         ` Nick Desaulniers
2019-10-15 20:26       ` Arvind Sankar
2019-10-16  1:51         ` Nick Desaulniers
2019-10-16 18:55           ` Arvind Sankar
2019-10-16 23:05             ` Nick Desaulniers

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='CAKwvOdnDVe-dahZGnRtzMrx-AH_C+2Lf20qjFQHNtn9xh=Okzw@mail.gmail.com' \
    --to=ndesaulniers@google.com \
    --cc=David1.Zhou@amd.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=andrew.cooper3@citrix.com \
    --cc=arnd@arndb.de \
    --cc=christian.koenig@amd.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=harry.wentland@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mka@google.com \
    --cc=shirish.s@amd.com \
    --cc=yshuiv7@gmail.com \
    /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).