Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Arvind Sankar <nivedita@alum.mit.edu>
Date:   Fri, 7 Aug 2020 17:29:14 -0400
To:     Nick Desaulniers <ndesaulniers@google.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Fangrui Song <maskray@google.com>,
        clang-built-linux@googlegroups.com, e5ten.arch@gmail.com,
        stable@vger.kernel.org, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>,
        Masahiro Yamada <masahiroy@kernel.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Arvind Sankar <nivedita@alum.mit.edu>,
        Dmitry Golovin <dima@golovin.in>,
        Marco Elver <elver@google.com>, Nick Terrell <terrelln@fb.com>,
        Daniel Kiper <daniel.kiper@oracle.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/boot: avoid relaxable symbols with Clang
Message-ID: <20200807212914.GB1454138@rani.riverdale.lan>
References: <20200807194100.3570838-1-ndesaulniers@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20200807194100.3570838-1-ndesaulniers@google.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Aug 07, 2020 at 12:41:00PM -0700, Nick Desaulniers wrote:
> A recent change to a default value of configuration variable
> (ENABLE_X86_RELAX_RELOCATIONS OFF -> ON) in LLVM now causes Clang's
> integrated assembler to emit R_X86_64_GOTPCRELX/R_X86_64_REX_GOTPCRELX
> relocations. LLD will relax instructions with these relocations based on
> whether the image is being linked as position independent or not.  When
> not, then LLD will relax these instructions to use absolute addressing
> mode (R_RELAX_GOT_PC_NOPIC). This causes kernels built with Clang
> and linked with LLD to fail to boot.

It could also cause kernels compiled with gcc and linked with LLD to
fail in the same way, no? The gcc/gas combination will generate the
relaxed relocations from I think gas-2.26 onward. Although the only
troublesome symbol in the case of gcc/gas is trampoline_32bit_src,
referenced from pgtable_64.c (gcc doesn't use a GOTPC reloc for _pgtable
etc).

I'm a bit surprised you were able to boot with just _pgtable fixed
(looking at the CBL issue), there are quite a few more GOTPC relocs with
clang -- maybe LLD isn't doing all the optimizations it could yet.

This potential issue was mentioned [0] in one of the earlier threads
(see last paragraph).

[0] https://lore.kernel.org/lkml/20200526191411.GA2380966@rani.riverdale.lan/

> 
> Also, the LLVM commit notes that these relocation types aren't supported
> until binutils 2.26. Since we support binutils 2.23+, avoid the
> relocations regardless of linker.

Note that the GNU assembler won't support the option to disable the
relaxations until 2.26, when they were added.

However, it turns out that clang always uses the integrated assembler
for the decompressor (and the EFI stub) because the no-integrated-as
option gets dropped when building these pieces, due to redefinition of
KBUILD_CFLAGS. You might want to mention this in the commit log or a
comment to explain why using the option unconditionally is safe. It
might need to be made conditional if the CFLAGS ever gets fixed to
maintain no-integrated-as.

Thanks.