* Re: [PATCH v5 2/2] x86/purgatory: Add linker script [not found] ` <20230330113127.3011e021@gandalf.local.home> @ 2023-03-30 16:00 ` Borislav Petkov 2023-04-07 23:22 ` Nick Desaulniers 0 siblings, 1 reply; 4+ messages in thread From: Borislav Petkov @ 2023-03-30 16:00 UTC (permalink / raw) To: Steven Rostedt Cc: Ricardo Ribalda, Eric Biederman, Baoquan He, Philipp Rudo, kexec, linux-kernel, Ross Zwisler, Simon Horman, x86, linux-toolchains On Thu, Mar 30, 2023 at 11:31:27AM -0400, Steven Rostedt wrote: > On Thu, 30 Mar 2023 17:18:26 +0200 > Borislav Petkov <bp@alien8.de> wrote: > > > On Thu, Mar 30, 2023 at 11:15:23AM -0400, Steven Rostedt wrote: > > > > Make sure that the .text section is not divided in multiple overlapping > > > > sections. This is not supported by kexec_file. > > > > And? > > > > What is the failure scenario? Why are you fixing it? Why do we care? > > > > This is way too laconic. > > > > Yeah, I think the change log in patch 1 needs to be in this patch too, > which gives better context. Just read it. Why did it work with clang version < 16? + toolchains ML. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v5 2/2] x86/purgatory: Add linker script 2023-03-30 16:00 ` [PATCH v5 2/2] x86/purgatory: Add linker script Borislav Petkov @ 2023-04-07 23:22 ` Nick Desaulniers 2023-04-11 21:45 ` Ricardo Ribalda 0 siblings, 1 reply; 4+ messages in thread From: Nick Desaulniers @ 2023-04-07 23:22 UTC (permalink / raw) To: Ricardo Ribalda Cc: Steven Rostedt, Eric Biederman, Baoquan He, Philipp Rudo, kexec, linux-kernel, Ross Zwisler, Simon Horman, x86, linux-toolchains, clang-built-linux, Borislav Petkov Hi Ricardo, Thanks for the patch! Please make sure to cc our mailing list <llvm@lists.linux.dev> for llvm specific issues. scripts/get_maintainer.pl should recommend it, or you can find it from clangbuiltlinux.github.io. You can also ping me internally for toolchain related issues. Start of thread. https://lore.kernel.org/lkml/20230321-kexec_clang16-v5-0-5563bf7c4173@chromium.org/ On Thu, Mar 30, 2023 at 9:00 AM Borislav Petkov <bp@alien8.de> wrote: > > On Thu, Mar 30, 2023 at 11:31:27AM -0400, Steven Rostedt wrote: > > On Thu, 30 Mar 2023 17:18:26 +0200 > > Borislav Petkov <bp@alien8.de> wrote: > > > > > On Thu, Mar 30, 2023 at 11:15:23AM -0400, Steven Rostedt wrote: > > > > > Make sure that the .text section is not divided in multiple overlapping > > > > > sections. This is not supported by kexec_file. Perhaps this is related to CrOS' use of AutoFDO creating .text.hot? If so, it's probably more straightforward to straight up disable PGO for kexec. See also: commit bde971a83bbf ("KVM: arm64: nvhe: Fix build with profile optimization") > > > > > > And? > > > > > > What is the failure scenario? Why are you fixing it? Why do we care? > > > > > > This is way too laconic. > > > > > > > Yeah, I think the change log in patch 1 needs to be in this patch too, > > which gives better context. > > Just read it. > > Why did it work with clang version < 16? I'll bet if we bisect llvm, we can spot what might have changed, which may give us a clue on how to get the old behavior back; maybe without the need for a linker script. Ricardo, how did you verify that your fix was correct? Surely we can check using command line utilities without needing a full blown kexec setup? If you can share more info, I can bisect llvm quickly. If it requires profile data, you'll need to share it, since CrOS engineers still have not posted public documentation on AutoFDO as I have repeatedly asked for. > > + toolchains ML. > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v5 2/2] x86/purgatory: Add linker script 2023-04-07 23:22 ` Nick Desaulniers @ 2023-04-11 21:45 ` Ricardo Ribalda 2023-04-18 17:49 ` Nick Desaulniers 0 siblings, 1 reply; 4+ messages in thread From: Ricardo Ribalda @ 2023-04-11 21:45 UTC (permalink / raw) To: Nick Desaulniers Cc: Steven Rostedt, Eric Biederman, Baoquan He, Philipp Rudo, kexec, linux-kernel, Ross Zwisler, Simon Horman, x86, linux-toolchains, clang-built-linux, Borislav Petkov Hi Nick On Sat, 8 Apr 2023 at 01:22, Nick Desaulniers <ndesaulniers@google.com> wrote: > > Hi Ricardo, > Thanks for the patch! Please make sure to cc our mailing list > <llvm@lists.linux.dev> for llvm specific issues. > scripts/get_maintainer.pl should recommend it, or you can find it from > clangbuiltlinux.github.io. You can also ping me internally for > toolchain related issues. > > Start of thread. > https://lore.kernel.org/lkml/20230321-kexec_clang16-v5-0-5563bf7c4173@chromium.org/ > > On Thu, Mar 30, 2023 at 9:00 AM Borislav Petkov <bp@alien8.de> wrote: > > > > On Thu, Mar 30, 2023 at 11:31:27AM -0400, Steven Rostedt wrote: > > > On Thu, 30 Mar 2023 17:18:26 +0200 > > > Borislav Petkov <bp@alien8.de> wrote: > > > > > > > On Thu, Mar 30, 2023 at 11:15:23AM -0400, Steven Rostedt wrote: > > > > > > Make sure that the .text section is not divided in multiple overlapping > > > > > > sections. This is not supported by kexec_file. > > Perhaps this is related to CrOS' use of AutoFDO creating .text.hot? > If so, it's probably more straightforward to straight up disable PGO > for kexec. See also: > > commit bde971a83bbf ("KVM: arm64: nvhe: Fix build with profile optimization") It was indeed due to the AutoFDO, adding KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=% -fprofile-use=%, $(KBUILD_CFLAGS)) to arch/x86/purgatory/Makefile It is definitely simpler than adding a linker script, but I am not sure if it is the correct way to fix this... Seems like splitting .text in multiple sections is an implementation detail of the compiler and the only way to force it is with a linker script... Or am I missing something? Shall I send a new version with the KBUILD_CFLAGS ? Thanks! > > > > > > > > > And? > > > > > > > > What is the failure scenario? Why are you fixing it? Why do we care? > > > > > > > > This is way too laconic. > > > > > > > > > > Yeah, I think the change log in patch 1 needs to be in this patch too, > > > which gives better context. > > > > Just read it. > > > > Why did it work with clang version < 16? > > I'll bet if we bisect llvm, we can spot what might have changed, which > may give us a clue on how to get the old behavior back; maybe without > the need for a linker script. > > Ricardo, how did you verify that your fix was correct? Surely we can > check using command line utilities without needing a full blown kexec > setup? If you can share more info, I can bisect llvm quickly. If it > requires profile data, you'll need to share it, since CrOS engineers > still have not posted public documentation on AutoFDO as I have > repeatedly asked for. The simplest test is to run: $readelf -S arch/x86/purgatory/purgatory.ro | grep "] \.text" [ 3] .text PROGBITS 0000000000000000 000002a0 If there is only one .text section then that kernel will be load properly via kexec_file(). > > > > > + toolchains ML. > > > > -- > > Regards/Gruss, > > Boris. > > > > https://people.kernel.org/tglx/notes-about-netiquette > > > > -- > Thanks, > ~Nick Desaulniers -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v5 2/2] x86/purgatory: Add linker script 2023-04-11 21:45 ` Ricardo Ribalda @ 2023-04-18 17:49 ` Nick Desaulniers 0 siblings, 0 replies; 4+ messages in thread From: Nick Desaulniers @ 2023-04-18 17:49 UTC (permalink / raw) To: Ricardo Ribalda Cc: Steven Rostedt, Eric Biederman, Baoquan He, Philipp Rudo, kexec, linux-kernel, Ross Zwisler, Simon Horman, x86, linux-toolchains, clang-built-linux, Borislav Petkov On Tue, Apr 11, 2023 at 2:46 PM Ricardo Ribalda <ribalda@chromium.org> wrote: > > Hi Nick > > On Sat, 8 Apr 2023 at 01:22, Nick Desaulniers <ndesaulniers@google.com> wrote: > > > > Hi Ricardo, > > Thanks for the patch! Please make sure to cc our mailing list > > <llvm@lists.linux.dev> for llvm specific issues. > > scripts/get_maintainer.pl should recommend it, or you can find it from > > clangbuiltlinux.github.io. You can also ping me internally for > > toolchain related issues. > > > > Start of thread. > > https://lore.kernel.org/lkml/20230321-kexec_clang16-v5-0-5563bf7c4173@chromium.org/ > > > > On Thu, Mar 30, 2023 at 9:00 AM Borislav Petkov <bp@alien8.de> wrote: > > > > > > On Thu, Mar 30, 2023 at 11:31:27AM -0400, Steven Rostedt wrote: > > > > On Thu, 30 Mar 2023 17:18:26 +0200 > > > > Borislav Petkov <bp@alien8.de> wrote: > > > > > > > > > On Thu, Mar 30, 2023 at 11:15:23AM -0400, Steven Rostedt wrote: > > > > > > > Make sure that the .text section is not divided in multiple overlapping > > > > > > > sections. This is not supported by kexec_file. > > > > Perhaps this is related to CrOS' use of AutoFDO creating .text.hot? > > If so, it's probably more straightforward to straight up disable PGO > > for kexec. See also: > > > > commit bde971a83bbf ("KVM: arm64: nvhe: Fix build with profile optimization") > > It was indeed due to the AutoFDO, adding > > KBUILD_CFLAGS := $(filter-out -fprofile-sample-use=% -fprofile-use=%, > $(KBUILD_CFLAGS)) > > to arch/x86/purgatory/Makefile > > It is definitely simpler than adding a linker script, but I am not > sure if it is the correct way to fix this... Seems like splitting > .text in multiple sections is an implementation detail of the compiler > and the only way to force it is with a linker script... Or am I > missing something? I think with the use of `unlikely` GCC will put code in .text.cold, so it is possible to trigger this using simpler means, but... > > Shall I send a new version with the KBUILD_CFLAGS ? I still think the cflags approach is way simpler. If someone tries to use unlikely in purgatory: "don't do that." Same for PGO. > > Thanks! > > > > > > > > > > > > > And? > > > > > > > > > > What is the failure scenario? Why are you fixing it? Why do we care? > > > > > > > > > > This is way too laconic. > > > > > > > > > > > > > Yeah, I think the change log in patch 1 needs to be in this patch too, > > > > which gives better context. > > > > > > Just read it. > > > > > > Why did it work with clang version < 16? > > > > I'll bet if we bisect llvm, we can spot what might have changed, which > > may give us a clue on how to get the old behavior back; maybe without > > the need for a linker script. > > > > Ricardo, how did you verify that your fix was correct? Surely we can > > check using command line utilities without needing a full blown kexec > > setup? If you can share more info, I can bisect llvm quickly. If it > > requires profile data, you'll need to share it, since CrOS engineers > > still have not posted public documentation on AutoFDO as I have > > repeatedly asked for. > > The simplest test is to run: > > $readelf -S arch/x86/purgatory/purgatory.ro | grep "] \.text" > [ 3] .text PROGBITS 0000000000000000 000002a0 > > If there is only one .text section then that kernel will be load > properly via kexec_file(). Got it, profile data will be required to reproduce then. If you can share. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-18 17:49 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20230321-kexec_clang16-v5-0-5563bf7c4173@chromium.org> [not found] ` <20230321-kexec_clang16-v5-2-5563bf7c4173@chromium.org> [not found] ` <20230330111523.4b98c8ce@gandalf.local.home> [not found] ` <20230330151826.GDZCWoQkQBj4BYbwWw@fat_crate.local> [not found] ` <20230330113127.3011e021@gandalf.local.home> 2023-03-30 16:00 ` [PATCH v5 2/2] x86/purgatory: Add linker script Borislav Petkov 2023-04-07 23:22 ` Nick Desaulniers 2023-04-11 21:45 ` Ricardo Ribalda 2023-04-18 17:49 ` Nick Desaulniers
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).