On Tue, Jun 18, 2024 at 04:37:16PM -0400, Joe Lawrence wrote: > On Mon, Feb 19, 2024 at 02:27:30PM +0100, Sumanth Korikkar wrote: > > Hi All, > > > > This is a rebased version of Josh's patch series with a few fixups. > > https://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git/log/?h=s390 > > > > This introduces the capability to compile the s390 relocatable kernel > > with and without the -fPIE option. > > > > When utilizing the kpatch functionality, it is advisable to compile the > > kernel without the -fPIE option. This is particularly important if the > > kernel is built with the -ffunction-sections and -fdata-sections flags. > > The linker imposes a restriction on the number of sections (limited to > > 64k), necessitating the omission of -fPIE. > > > > [1] https://gcc.gnu.org/pipermail/gcc-patches/2023-June/622872.html > > [2] https://gcc.gnu.org/pipermail/gcc-patches/2023-August/625986.html > > > > Gcc recently implemented an optimization [1] for loading symbols without > > explicit alignment, aligning with the IBM Z ELF ABI. This ABI mandates > > symbols to reside on a 2-byte boundary, enabling the use of the larl > > instruction. However, kernel linker scripts may still generate unaligned > > symbols. To address this, a new -munaligned-symbols option has been > > introduced [2] in recent gcc versions. This option has to be used with > > future gcc versions. > > > > Older Clang lacks support for handling unaligned symbols generated > > by kernel linker scripts when the kernel is built without -fPIE. However, > > future versions of Clang will include support for the -munaligned-symbols > > option. When the support is unavailable, compile the kernel with -fPIE > > to maintain the existing behavior. > > > > Patch 1 filters out -munaligned-symbol flag for vdso code. This is beneficial > > when compiling kernel with -fno-PIE and -munaligned-symbols combination. > > > > Patch 2 introduces the 'relocs' tool, which reads the vmlinux file and > > generates a vmlinux.relocs_64 section, containing offsets for all > > R_390_64 relocations. > > > > Patch 3 enables the compilation of a relocatable kernel with or without > > the -fPIE option. It allows for building the relocatable kernel without > > -fPIE. However, if compiler cannot handle unaligned symbols, the kernel > > is built with -fPIE. > > > > Patch 4 handles orphan .rela sections when kernel is built with > > -fno-PIE. > > > > kpatch tools changes: > > * -mno-pic-data-is-text-relative prevents relative addressing between > > code and data. This is needed to avoid relocation error when klp text > > and data are too far apart. kpatch already includes this flag. > > However, with these changes, ARCH_KFLAGS+="-fPIC" should be added to > > s390 kpatch tools, As -mno-pic-data-is-text-relative can be used only > > with -fPIC. The corresponding pull request will be sent to kpatch > > tools. > > Hi Sumanth, > > I noticed interesting compiler differences when adding -fPIC build > option and not. The difference in resulting output can confuse > kpatch-build when it tries to verify that its reference build (with the > mentioned options, plus --ffunction-sections and -fdata-sections), > doesn't line up closely enough with the original vmlinux source (sans > all these options). Hi Joe, kpatch for s390 already uses extra compiler flag -mno-pic-data-is-text-relative inorder to prevent relative addressing between code and data. Also, includes -ffunction-sections and -fdata-sections along with it to identify modified functions and its relocations. Both the source code and modified code are built with the same options during kpatch-build (-fPIC added to -mno-pic-data-is-text-relative). kpatch-build was able to identify modified functions and its associated relocations and include these changes in the final kpatch module. May be I am missing some info: Does this deviation cause confusion to kpatch? Adding -fPIC flag would end up with extra indirections via GOT for eg: when accessing global vars. But, this is similar in case of previous kpatch build as well (-fPIE + -mno-pic-data-is-text-relative). Rela comparision which I performed on .rela.text.mmput: 1. with -fno-PIE kernel: .rela.text.mmput: 0000000000000022 000001a800000014 R_390_PLT32DBL 0000000000000000 __cond_resched + 2 0000000000000040 0000017800000013 R_390_PC32DBL 0000000000000000 __s390_indirect_jump_r14 + 2 000000000000004a 000001a000000014 R_390_PLT32DBL 0000000000000000 uprobe_clear_state + 2 0000000000000054 000001a100000014 R_390_PLT32DBL 0000000000000000 exit_aio + 2 000000000000006c 000001a200000014 R_390_PLT32DBL 0000000000000000 exit_mmap + 2 000000000000009a 000001a300000014 R_390_PLT32DBL 0000000000000000 fput + 2 00000000000000b8 000001a400000013 R_390_PC32DBL 0000000000000000 mmlist_lock + 2 <<< 00000000000000cc 000001a500000014 R_390_PLT32DBL 0000000000000000 __list_del_entry_valid_or_report + 2 0000000000000100 000001a400000013 R_390_PC32DBL 0000000000000000 mmlist_lock + 4 000000000000011e 000001a600000014 R_390_PLT32DBL 0000000000000000 module_put + 2 0000000000000140 0000016f00000014 R_390_PLT32DBL 0000000000000000 __mmdrop + 2 000000000000014a 000001a700000014 R_390_PLT32DBL 0000000000000000 __ksm_exit + 2 0000000000000154 0000019700000014 R_390_PLT32DBL 0000000000000000 arch_spin_lock_wait + 2 2. With -fPIC flag added: With Relocation section '.rela.text.mmput' at offset 0x84c80 contains 12 entries: Offset Info Type Symbol's Value Symbol's Name + Addend 0000000000000022 000001d500000014 R_390_PLT32DBL 0000000000000000 __cond_resched + 2 0000000000000040 0000017800000013 R_390_PC32DBL 0000000000000000 __s390_indirect_jump_r14 + 2 000000000000004a 000001d600000014 R_390_PLT32DBL 0000000000000000 uprobe_clear_state + 2 0000000000000054 000001d700000014 R_390_PLT32DBL 0000000000000000 exit_aio + 2 0000000000000070 000001d800000014 R_390_PLT32DBL 0000000000000000 exit_mmap + 2 000000000000007e 000001d300000014 R_390_PLT32DBL 0000000000000000 set_mm_exe_file + 2 0000000000000090 000001d90000001a R_390_GOTENT 0000000000000000 mmlist_lock + 2 <<< Difference GOTENT 00000000000000ac 000001da00000014 R_390_PLT32DBL 0000000000000000 __list_del_entry_valid_or_report + 2 00000000000000f8 000001db00000014 R_390_PLT32DBL 0000000000000000 module_put + 2 000000000000011a 0000016f00000014 R_390_PLT32DBL 0000000000000000 __mmdrop + 2 0000000000000124 000001dc00000014 R_390_PLT32DBL 0000000000000000 __ksm_exit + 2 0000000000000132 0000019500000014 R_390_PLT32DBL 0000000000000000 arch_spin_lock_wait + 2 3. built with -fPIE commit + -mno-pic-data-is-text-relative (Previous kpatch version): Relocation section '.rela.text.__mmput' at offset 0x95bc8 contains 13 entries: Offset Info Type Symbol's Value Symbol's Name + Addend 000000000000001c 000001c000000014 R_390_PLT32DBL 0000000000000000 uprobe_clear_state + 2 0000000000000026 000001c100000014 R_390_PLT32DBL 0000000000000000 exit_aio + 2 000000000000004c 000001c200000014 R_390_PLT32DBL 0000000000000000 exit_mmap + 2 0000000000000056 000001c300000014 R_390_PLT32DBL 0000000000000000 mm_put_huge_zero_page + 2 0000000000000084 000001c400000014 R_390_PLT32DBL 0000000000000000 fput + 2 000000000000009a 000001c50000001a R_390_GOTENT 0000000000000000 mmlist_lock + 2 <<<< 0000000000000106 000001c600000014 R_390_PLT32DBL 0000000000000000 module_put + 2 0000000000000124 0000019000000013 R_390_PC32DBL 0000000000000000 __s390_indirect_jump_r14 + 2 000000000000012e 000001c700000014 R_390_PLT32DBL 0000000000000000 __khugepaged_exit + 2 000000000000013c 000001c800000014 R_390_PLT32DBL 0000000000000000 __ksm_exit + 2 000000000000014a 000001c900000014 R_390_PLT32DBL 0000000000000000 __list_del_entry_valid_or_report + 2 0000000000000158 000001a200000014 R_390_PLT32DBL 0000000000000000 arch_spin_lock_wait + 2 000000000000016c 0000018700000014 R_390_PLT32DBL 0000000000000000 __mmdrop + 2 > > I don't think a kpatch-build PR was ever opened to add "-fPIC", but the > compiler now warns that its required. Have you ever seen optimization / > build output changes when adding this option and if so, were there > additional kpatch-build implications? I missed adding "-fPIC" KCFLAGS in kpatch tool. I will send a PR request. I expect and assume similar implications with (-fPIC added) vs (-fPIE + -mno-pic-data-is-text-relative) build, as shown above in the rela comparisions. Other Note: The latest kernel is built with -fPIC and linked with -no-pie (reference commit: ca888b17da9b ("s390: Compile kernel with -fPIC and link with -no-pie")) which also avoids generation of dynamic symbols and helps kpatch usecases (when num of sections >=64k sections). Also the build options would be similar (-fPIC in kernel and -fPIC in kpatch-build) For latest kernel, there is no need to add explicit -fPIC again in kpatch tool. But for the intermediate commits, yes, makes sense to add it in kpatch-build tools and will create one PR. > Here is a quick example that I stumbled on while investigating the > kpatch-build shadow-newpid.patch integration test that modifies > kernel/fork.c. I couldn't reproduce with the s390 defconfig, but it > shows up when building with RHEL-10 config. Reproducer steps and > disassembly examples follows. Sorry Joe, I didnt get this question. "I couldnt reproduce with the s390 defconfig". Could you please clarify it ? Thank you, Sumanth
