Received-SPF: pass (google.com: domain of linux-kernel+bounces-59510-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Fri, 9 Feb 2024 08:08:42 -0700
From: Nathan Chancellor <nathan@kernel.org>
To: Heiko Carstens <hca@linux.ibm.com>
Cc: gor@linux.ibm.com, agordeev@linux.ibm.com, borntraeger@linux.ibm.com,
	svens@linux.ibm.com, maskray@google.com, ndesaulniers@google.com,
	linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev, patches@lists.linux.dev
Subject: Re: [PATCH 00/11] s390: Support linking with ld.lld
Message-ID: <20240209150842.GA666821@dev-arch.thelio-3990X>
References: <20240207-s390-lld-and-orphan-warn-v1-0-8a665b3346ab@kernel.org>
 <20240209112046.13241-C-hca@linux.ibm.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240209112046.13241-C-hca@linux.ibm.com>

Hi Heiko,

On Fri, Feb 09, 2024 at 12:20:46PM +0100, Heiko Carstens wrote:
> > This series allows the s390 kernel to be linked with ld.lld (support for
> > s390 is under review at [1]). This implicitly depends on [2], which was
> > created and sent before it was realized that this series was necessary.
> ...
> > There is one outstanding issue due to something that ld.lld does not
> > support that the kernel relies on:
> > 
> >   ld.lld: error: drivers/nvme/host/fc.o:(__bug_table): writable SHF_MERGE section is not supported
> > 
> > This was changed in the kernel in commit e21f8baf8d9a ("s390/bug: add
> > entry size to the __bug_table section"). Is this change truly necessary?
> > I selectively applied a revert on top of current mainline and I did not
> > observe any issues with either Clang or GCC.
> 
> No it is not necessary. As the original patch stated this was a pre-req
> patch for objtool, for which we still don't have support. This (or
> something different) might be needed. But for now this can easily be
> reverted.
> 
> > Then build the kernel with 'LD=ld.lld' in addition to whatever command
> > line you use (I tested both Clang and GCC). I can boot an ld.lld linked
> > kernel built with both compilers in QEMU with this series.
> > 
> > [    1.386970] Linux version 6.8.0-rc3-00043-g05761ede85d6-dirty (nathan@dev-fedora.aadp) (s390-linux-gcc (GCC) 13.2.0, ClangBuiltLinux LLD 19.0.0) #1 SMP Wed Feb  7 16:51:12 MST 2024
> > 
> > [    0.871923] Linux version 6.8.0-rc3-00043-g05761ede85d6-dirty (nathan@dev-fedora.aadp) (ClangBuiltLinux clang version 19.0.0git (https://github.com/llvm/llvm-project 417075e56aeba5a5b20301c7bfeba9c2a800982b), ClangBuiltLinux LLD 19.0.0) #1 SMP Wed Feb  7 17:01:22 MST 2024
> 
> Tested, and works for me. Thanks a lot for your work. This is highly
> appreciated!
> 
> I applied this series internally to get some CI runs over the weekend, and
> push it to our external repository beginning of next week. As suggested by

Excellent, please let me know if anything comes up from that!

> you I reverted the commit you mentioned above, and also removed ENTRY from
> our vdso linker scripts, similar to what you did already for powerpc (see
> patches below).

Ah thanks for spotting that, I was so focused on the errors I forgot to
clean up the warning too.

> Please feel free to send patches for both of the issues, and I'll replace
> my patches with your patches.

Honestly, I am not sure that I will write much better commit messages
than the ones you already have and those solutions seem acceptable to
me. Consider them:

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

But if you would prefer patches from myself though, I can send them next
week.

Cheers,
Nathan

> From 30a0a88d0e6c4802b748a942bb3f6f1b223f53ba Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <hca@linux.ibm.com>
> Date: Fri, 9 Feb 2024 11:48:25 +0100
> Subject: [PATCH 1/2] s390/bug: remove entry size from __bug_table section
> 
> Commit e21f8baf8d9a ("s390/bug: add entry size to the __bug_table section")
> changed the __EMIT_BUG() inline assembly to emit mergeable __bug_table
> entries. This is at least currently not needed, but causes problems with
> the upcoming s390 ld.lld support:
> 
>   ld.lld: error: drivers/nvme/host/fc.o:(__bug_table): writable SHF_MERGE section is not supported
> 
> Therefore revert the change for now.
> 
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Closes: https://lore.kernel.org/all/20240207-s390-lld-and-orphan-warn-v1-0-8a665b3346ab@kernel.org/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/include/asm/bug.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
> index aebe1e22c7be..c500d45fb465 100644
> --- a/arch/s390/include/asm/bug.h
> +++ b/arch/s390/include/asm/bug.h
> @@ -14,7 +14,7 @@
>  		".section .rodata.str,\"aMS\",@progbits,1\n"	\
>  		"1:	.asciz	\""__FILE__"\"\n"		\
>  		".previous\n"					\
> -		".section __bug_table,\"awM\",@progbits,%2\n"	\
> +		".section __bug_table,\"aw\"\n"			\
>  		"2:	.long	0b-.\n"				\
>  		"	.long	1b-.\n"				\
>  		"	.short	%0,%1\n"			\
> @@ -30,7 +30,7 @@
>  #define __EMIT_BUG(x) do {					\
>  	asm_inline volatile(					\
>  		"0:	mc	0,0\n"				\
> -		".section __bug_table,\"awM\",@progbits,%1\n"	\
> +		".section __bug_table,\"aw\"\n"			\
>  		"1:	.long	0b-.\n"				\
>  		"	.short	%0\n"				\
>  		"	.org	1b+%1\n"			\
> -- 
> 2.40.1
> 
> From bdca9b8dcf3f0884341f491d54502d4cbe660446 Mon Sep 17 00:00:00 2001
> From: Heiko Carstens <hca@linux.ibm.com>
> Date: Fri, 9 Feb 2024 11:54:01 +0100
> Subject: [PATCH 2/2] s390/vdso: remove unused ENTRY in linker scripts
> 
> When linking vdso64.so.dbg with ld.lld, there is a warning about not
> finding _start for the starting address:
> 
>   ld.lld: warning: cannot find entry symbol _start; not setting start address
> 
> Fix this be removing the unused ENTRY in both vdso linker scripts. See
> commit e247172854a5 ("powerpc/vdso: Remove unused ENTRY in linker
> scripts"), which solved the same problem for powerpc, for further details.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>  arch/s390/kernel/vdso32/vdso32.lds.S | 1 -
>  arch/s390/kernel/vdso64/vdso64.lds.S | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/arch/s390/kernel/vdso32/vdso32.lds.S b/arch/s390/kernel/vdso32/vdso32.lds.S
> index edf5ff1debe1..65b9513a5a0e 100644
> --- a/arch/s390/kernel/vdso32/vdso32.lds.S
> +++ b/arch/s390/kernel/vdso32/vdso32.lds.S
> @@ -9,7 +9,6 @@
>  
>  OUTPUT_FORMAT("elf32-s390", "elf32-s390", "elf32-s390")
>  OUTPUT_ARCH(s390:31-bit)
> -ENTRY(_start)
>  
>  SECTIONS
>  {
> diff --git a/arch/s390/kernel/vdso64/vdso64.lds.S b/arch/s390/kernel/vdso64/vdso64.lds.S
> index 4461ea151e49..37e2a505e81d 100644
> --- a/arch/s390/kernel/vdso64/vdso64.lds.S
> +++ b/arch/s390/kernel/vdso64/vdso64.lds.S
> @@ -9,7 +9,6 @@
>  
>  OUTPUT_FORMAT("elf64-s390", "elf64-s390", "elf64-s390")
>  OUTPUT_ARCH(s390:64-bit)
> -ENTRY(_start)
>  
>  SECTIONS
>  {
> -- 
> 2.40.1
>