| Message ID | 20250224132132.1765115-6-ardb+git@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | x86/build: Get rid of vmlinux postlink step | expand |
* Ard Biesheuvel <ardb+git@google.com> wrote: > From: Ard Biesheuvel <ardb@kernel.org> > > Kbuild supports an architecture specific Makefile.postlink file that is > invoked for the vmlinux target after it has been built. This Makefile > takes 'vmlinux' (which has just been built) as the target, and mangles > the file and/or constructs other intermediate artifacts from it. > > This violates the general philosophy of Make, which is based on rules > and dependencies, and artifacts that are rebuilt only when any of their > dependencies have been updated. > > Instead, the different incarnations of vmlinux that are consumed by > different stages of the build should be emitted as distinct files, where > rules and dependencies are used to define one in terms of the other. > > This also works around an error observed here [0], where vmlinux is > deleted by Make because a subsequent step that consumes it as input > throws an error. > > So refactor the vmlinux shell scripts and build rules so that > architectures that rely on --emit-relocs to construct vmlinux with > static relocations preserved will get a separate vmlinux.unstripped file > carrying those relocations. This removes the need for an imperative > postlink step, given that any rules that depend on the unstripped > vmlinux can now simply depend on vmlinux.unstripped, rather than inject > a build step into Makefile.postlink Nice! Does this also result in any measurable speedup of the build, or are the steps still similar? But in terms of justification for upstreaming, the reduction in complexity alone makes it worth it IMO: 19 files changed, 52 insertions(+), 87 deletions(-) Thanks, Ingo
On Mon, 24 Feb 2025 at 10:51, Ingo Molnar <mingo@kernel.org> wrote: > > But in terms of justification for upstreaming, the reduction in > complexity alone makes it worth it IMO: > > 19 files changed, 52 insertions(+), 87 deletions(-) Yeah, absolutely. Our fancy make build rules still have too many of the phony forced targets, but this is a few less of them and makes the build confirm (more) to the usual rules. I do wonder if we could just get rid of that CONFIG_ARCH_VMLINUX_NEEDS_RELOCS entirely and make it just be how all architectures do it. Yes, it was apparently "just" riscv/s390/x86/mips that did that 'strip_relocs' hack, but at the same time that whole pass *feels* entirely generic. IOW, this all makes me just wonder why other architectures don't do it? Linus
On Mon, 24 Feb 2025 at 21:00, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Mon, 24 Feb 2025 at 10:51, Ingo Molnar <mingo@kernel.org> wrote: > > > > But in terms of justification for upstreaming, the reduction in > > complexity alone makes it worth it IMO: > > > > 19 files changed, 52 insertions(+), 87 deletions(-) > > Yeah, absolutely. Our fancy make build rules still have too many of > the phony forced targets, but this is a few less of them and makes the > build confirm (more) to the usual rules. > > I do wonder if we could just get rid of that > CONFIG_ARCH_VMLINUX_NEEDS_RELOCS entirely and make it just be how all > architectures do it. > > Yes, it was apparently "just" riscv/s390/x86/mips that did that > 'strip_relocs' hack, but at the same time that whole pass *feels* > entirely generic. > TL;DR it is not It is only needed on architectures that use --emit-relocs in the first place, e.g., to construct bespoke KASLR tables. This is actually a somewhat dubious practice, because these are static relocations, i.e., what the linker consumes as input, and they are emitted along with vmlinux as output. [*] This feature was (AFAIK) never really intended for constructing dynamic relocation tables as some architectures in Linux do. On those architectures, these static relocations need to be stripped again, to avoid bloating vmlinux with useless data. On architectures that rely on PIE linking (such as arm64), the linker will emit a dynamic relocation table that is more suitable for use at boot time, i.e., it only contains absolute relocations (as RIP-relative ones never require any fixing up at boot), and uses RELR format to pack them very densely, removing the need for our own special format. Architectures that do not implement KASLR in the first place have no need for these static relocations either. PIE linking is generally a better choice than relying on --emit-relocs, but it is highly ISA dependent whether that requires a costlier kind of codegen. On arm64, we don't even bother generating -fPIE code because ordinary code can be linked in PIE mode. OTOH, on x86, we'd need full 64-bit PIC/PIE codegen in order to link with PIE, whereas we currently rely on the 'kernel' code model to generate 32-bit wide absolute symbol references that can only refer to the top 2G of the 64-bit address space. [*] Using static relocations to describe a fully linked binary such as vmlinux is problematic because a) it covers all external symbol references, including relative ones that don't need fixing up, but more importantly, b) the linker may perform relaxations that result in the code going out of sync with the relocation that annotates it (this is not entirely avoidable if the relaxed version of the code cannot even be described by any relocation specified by the ELF psABI)
On Mon, 24 Feb 2025 at 19:51, Ingo Molnar <mingo@kernel.org> wrote: > ... > > > > So refactor the vmlinux shell scripts and build rules so that > > architectures that rely on --emit-relocs to construct vmlinux with > > static relocations preserved will get a separate vmlinux.unstripped file > > carrying those relocations. This removes the need for an imperative > > postlink step, given that any rules that depend on the unstripped > > vmlinux can now simply depend on vmlinux.unstripped, rather than inject > > a build step into Makefile.postlink > > Nice! Does this also result in any measurable speedup of the build, or > are the steps still similar? > The steps are essentially the same, only described in a slightly better way (and with an additional artifact so the used disk space goes up slightly too)
On Mon, 24 Feb 2025 at 14:21, Ard Biesheuvel <ardb+git@google.com> wrote: > > From: Ard Biesheuvel <ardb@kernel.org> > > Kbuild supports an architecture specific Makefile.postlink file that is > invoked for the vmlinux target after it has been built. This Makefile > takes 'vmlinux' (which has just been built) as the target, and mangles > the file and/or constructs other intermediate artifacts from it. > > This violates the general philosophy of Make, which is based on rules > and dependencies, and artifacts that are rebuilt only when any of their > dependencies have been updated. > > Instead, the different incarnations of vmlinux that are consumed by > different stages of the build should be emitted as distinct files, where > rules and dependencies are used to define one in terms of the other. > > This also works around an error observed here [0], where vmlinux is > deleted by Make because a subsequent step that consumes it as input > throws an error. > > So refactor the vmlinux shell scripts and build rules so that > architectures that rely on --emit-relocs to construct vmlinux with > static relocations preserved will get a separate vmlinux.unstripped file > carrying those relocations. This removes the need for an imperative > postlink step, given that any rules that depend on the unstripped > vmlinux can now simply depend on vmlinux.unstripped, rather than inject > a build step into Makefile.postlink > > S390 should be able to do the same. MIPS and RISC-V perform some > post-build checks on vmlinux, which is reasonable in principle for a > postlink step, although deleting vmlinux when the check fails is equally > unhelpful. > > [0] https://lore.kernel.org/all/Z5ARucnUgqjwBnrp@gmail.com/T/#m731ed0206949fc3f39fcc8a7b82fe348a8fc80c4 > > Cc: Masahiro Yamada <masahiroy@kernel.org> > Cc: Ingo Molnar <mingo@kernel.org> > > Ard Biesheuvel (4): > Kbuild/link-vmlinux.sh: Make output file name configurable > Kbuild: Introduce Kconfig symbol for linking vmlinux with relocations > Kbuild: Create intermediate vmlinux build with relocations preserved > x86: Get rid of Makefile.postlink > Ping?
On Fri, Mar 7, 2025 at 1:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Mon, 24 Feb 2025 at 14:21, Ard Biesheuvel <ardb+git@google.com> wrote: > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > Kbuild supports an architecture specific Makefile.postlink file that is > > invoked for the vmlinux target after it has been built. This Makefile > > takes 'vmlinux' (which has just been built) as the target, and mangles > > the file and/or constructs other intermediate artifacts from it. > > > > This violates the general philosophy of Make, which is based on rules > > and dependencies, and artifacts that are rebuilt only when any of their > > dependencies have been updated. > > > > Instead, the different incarnations of vmlinux that are consumed by > > different stages of the build should be emitted as distinct files, where > > rules and dependencies are used to define one in terms of the other. In my understanding, the build rule of vmlinux is atomic because vmlinux embeds a timestamp and a build version. Now, you are splitting it into two stages. vmlinux.unstripped (this includes timestamp and the build version) --(cmd_strip_relocs)--> vmlinux When cmd_strip_relocs is changed, only vmlinux is updated. This changes the content of vmlinux, but its timestamp and build version remain the same. So, I am not sure if this is the right direction. You can see more steps for updating vmlinux. Do you believe the build rule should be further split into more fine-grained stages? For example, vmlinux.pre-sort (this includes timestamp and the build version) --(scripts/sortable)--> vmlinux.unstripped --(cmd_strip_relocs)--> vmlinux But, again, even when sorttable is changed, the timestamp and the build version remain the same. Yeah, arch/*/Makefile.postlink is a crap where arch maintainers build a fence and start whatever they want to do. If they completely disappear, I would love it. However, this seems a partial clean-up within the scope you are interested in. (more specifically your motivation is because Linus pointed out a failure in arch/x86/Makefile.postlink deleted vmlinux) > > This also works around an error observed here [0], where vmlinux is > > deleted by Make because a subsequent step that consumes it as input > > throws an error. > > > > So refactor the vmlinux shell scripts and build rules so that > > architectures that rely on --emit-relocs to construct vmlinux with > > static relocations preserved will get a separate vmlinux.unstripped file > > carrying those relocations. This removes the need for an imperative > > postlink step, given that any rules that depend on the unstripped > > vmlinux can now simply depend on vmlinux.unstripped, rather than inject > > a build step into Makefile.postlink > > > > S390 should be able to do the same. MIPS and RISC-V perform some > > post-build checks on vmlinux, which is reasonable in principle for a > > postlink step, although deleting vmlinux when the check fails is equally > > unhelpful. > > > > [0] https://lore.kernel.org/all/Z5ARucnUgqjwBnrp@gmail.com/T/#m731ed0206949fc3f39fcc8a7b82fe348a8fc80c4 > > > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > Cc: Ingo Molnar <mingo@kernel.org> > > > > Ard Biesheuvel (4): > > Kbuild/link-vmlinux.sh: Make output file name configurable > > Kbuild: Introduce Kconfig symbol for linking vmlinux with relocations > > Kbuild: Create intermediate vmlinux build with relocations preserved > > x86: Get rid of Makefile.postlink > > > > Ping?
On Fri, 7 Mar 2025 at 19:54, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Fri, Mar 7, 2025 at 1:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Mon, 24 Feb 2025 at 14:21, Ard Biesheuvel <ardb+git@google.com> wrote: > > > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > > > Kbuild supports an architecture specific Makefile.postlink file that is > > > invoked for the vmlinux target after it has been built. This Makefile > > > takes 'vmlinux' (which has just been built) as the target, and mangles > > > the file and/or constructs other intermediate artifacts from it. > > > > > > This violates the general philosophy of Make, which is based on rules > > > and dependencies, and artifacts that are rebuilt only when any of their > > > dependencies have been updated. > > > > > > Instead, the different incarnations of vmlinux that are consumed by > > > different stages of the build should be emitted as distinct files, where > > > rules and dependencies are used to define one in terms of the other. > > > In my understanding, the build rule of vmlinux is atomic > because vmlinux embeds a timestamp and a build version. > > Now, you are splitting it into two stages. > > vmlinux.unstripped (this includes timestamp and the build version) > --(cmd_strip_relocs)--> vmlinux > > > When cmd_strip_relocs is changed, only vmlinux is updated. > This changes the content of vmlinux, but its timestamp and build version > remain the same. > > So, I am not sure if this is the right direction. > You are saying that, when we change the contents of the cmd_strip_relocs build rule and rebuild without cleaning, vmlinux will have an older timestamp? I think there are many more cases where the version is not updated, so I don't see this as a problem at all. > > You can see more steps for updating vmlinux. > Do you believe the build rule should be further split into > more fine-grained stages? > No. The problem is that vmlinux.relocs (in arch/x86/boot/compressed) depends on vmlinux, but not the version of vmlinux that is ultimately produced. It should depend on an artifact that is always suitable to generate the relocation table, not only during the short time between the initial creation of vmlinux and the point during the execution of Makefile.postlink where the relocations that are needed to generate vmlinux.relocs are stripped from vmlinux. > For example, > > vmlinux.pre-sort (this includes timestamp and the build version) > --(scripts/sortable)--> Why? Which other artifact depends on the unsorted tables, and can no longer be generated correctly after the tables have been sorted? > vmlinux.unstripped > --(cmd_strip_relocs)--> > vmlinux > > But, again, even when sorttable is changed, > the timestamp and the build version remain the same. > Again, there are many other ways in which the final vmlinux can be newer than the internal version fields suggest. I really don't think this is an issue. > > Yeah, arch/*/Makefile.postlink is a crap > where arch maintainers build a fence > and start whatever they want to do. > > If they completely disappear, I would love it. > Good. > However, this seems a partial clean-up > within the scope you are interested in. > (more specifically your motivation is because Linus pointed out > a failure in arch/x86/Makefile.postlink deleted vmlinux) > Yes. Makefile.postlink failures propagate back to the build rule that generates vmlinux, and so the file is deleted again. For sanity checks performed on vmlinux, this is fine. But for generating another file that takes vmlinux as its input, this is completely broken.
On Sat, Mar 8, 2025 at 7:49 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Fri, 7 Mar 2025 at 19:54, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Fri, Mar 7, 2025 at 1:47 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > On Mon, 24 Feb 2025 at 14:21, Ard Biesheuvel <ardb+git@google.com> wrote: > > > > > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > > > > > Kbuild supports an architecture specific Makefile.postlink file that is > > > > invoked for the vmlinux target after it has been built. This Makefile > > > > takes 'vmlinux' (which has just been built) as the target, and mangles > > > > the file and/or constructs other intermediate artifacts from it. > > > > > > > > This violates the general philosophy of Make, which is based on rules > > > > and dependencies, and artifacts that are rebuilt only when any of their > > > > dependencies have been updated. > > > > > > > > Instead, the different incarnations of vmlinux that are consumed by > > > > different stages of the build should be emitted as distinct files, where > > > > rules and dependencies are used to define one in terms of the other. > > > > > > In my understanding, the build rule of vmlinux is atomic > > because vmlinux embeds a timestamp and a build version. > > > > Now, you are splitting it into two stages. > > > > vmlinux.unstripped (this includes timestamp and the build version) > > --(cmd_strip_relocs)--> vmlinux > > > > > > When cmd_strip_relocs is changed, only vmlinux is updated. > > This changes the content of vmlinux, but its timestamp and build version > > remain the same. > > > > So, I am not sure if this is the right direction. > > > > You are saying that, when we change the contents of the > cmd_strip_relocs build rule and rebuild without cleaning, vmlinux will > have an older timestamp? I think there are many more cases where the > version is not updated, so I don't see this as a problem at all. For example please? > > > > > You can see more steps for updating vmlinux. > > Do you believe the build rule should be further split into > > more fine-grained stages? > > > > No. > > The problem is that vmlinux.relocs (in arch/x86/boot/compressed) > depends on vmlinux, but not the version of vmlinux that is ultimately > produced. Why? Is there any case where vmlinux and vmlinux.relocs become out of sync? > It should depend on an artifact that is always suitable to generate > the relocation table, not only during the short time between the > initial creation of vmlinux and the point during the execution of > Makefile.postlink where the relocations that are needed to generate > vmlinux.relocs are stripped from vmlinux. I still do not understand why this is a problem. arch/*/Makefile.postlink is invoked from $(call if_changed_dep,link_vmlinux) From Make's perspective, this is a single build rule to generate vmlinux. For x86, vmlinux.relocs is a byproduct. I do not see this as a problem. > > For example, > > > > vmlinux.pre-sort (this includes timestamp and the build version) > > --(scripts/sortable)--> > > Why? Which other artifact depends on the unsorted tables, and can no > longer be generated correctly after the tables have been sorted? > > > vmlinux.unstripped > > --(cmd_strip_relocs)--> > > vmlinux > > > > But, again, even when sorttable is changed, > > the timestamp and the build version remain the same. > > > > Again, there are many other ways in which the final vmlinux can be > newer than the internal version fields suggest. I really don't think > this is an issue. Again, for example please. The build rule of vmlinux is atomic. When vmlinux is updated, its timestamp is updated. When any part of the build rule fails, vmlinux is deleted because we cannot keep an invalid vmlinux. This process is quite simple. > > > > > Yeah, arch/*/Makefile.postlink is a crap > > where arch maintainers build a fence > > and start whatever they want to do. > > > > If they completely disappear, I would love it. > > > > Good. > > > However, this seems a partial clean-up > > within the scope you are interested in. > > (more specifically your motivation is because Linus pointed out > > a failure in arch/x86/Makefile.postlink deleted vmlinux) > > > > Yes. Makefile.postlink failures propagate back to the build rule that > generates vmlinux, and so the file is deleted again. > > For sanity checks performed on vmlinux, this is fine. But for > generating another file that takes vmlinux as its input, this is > completely broken. I do not think it is broken. As I mentioned above, I regard vmlinux.relocs as a byproduct of the atomic build rule of vmlinux. This works. -- Best Regards Masahiro Yamada
On Sat, 8 Mar 2025 at 17:17, Masahiro Yamada <masahiroy@kernel.org> wrote: > ... > I do not think it is broken. > As I mentioned above, I regard vmlinux.relocs as a byproduct > of the atomic build rule of vmlinux. This works. > There is no make rule for vmlinux.relocs, and so - if it gets deleted, it cannot be rebuilt and even though the build does not break, the relocation data is missing from the compressed image, and this could potentially break the kaslr startup code, - it vmlinux.relocs is older than vmlinux for some reason, make will not notice and silently reuse the outdated version, - when creating vmlinux.relocs from vmlinux and an error occurs, vmlinux is deleted, making it difficult to diagnose the problem. I think this is badly broken, but if you think this is all working as it should, I am not going to debate this further, and you can consider the patch series withdrawn.
* Ard Biesheuvel <ardb@kernel.org> wrote: > On Sat, 8 Mar 2025 at 17:17, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > ... > > I do not think it is broken. > > As I mentioned above, I regard vmlinux.relocs as a byproduct > > of the atomic build rule of vmlinux. This works. Except when it doesn't work, such as when an intermediate linking step fails, and intermediate build products are lost and cannot be recreated easily (or at all without modifying the source)? And the thing is, there should be no such thing as an 'atomic build rule of vmlinux' if it means lost information when the build is broken at an intermediate step. What purpose does it have? > There is no make rule for vmlinux.relocs, and so > > - if it gets deleted, it cannot be rebuilt and even though the build > does not break, the relocation data is missing from the compressed > image, and this could potentially break the kaslr startup code, > - it vmlinux.relocs is older than vmlinux for some reason, make will > not notice and silently reuse the outdated version, > - when creating vmlinux.relocs from vmlinux and an error occurs, > vmlinux is deleted, making it difficult to diagnose the problem. > > I think this is badly broken, but if you think this is all working as > it should, I am not going to debate this further, and you can consider > the patch series withdrawn. That's very sad, as both the simplification is substantial: 19 files changed, 52 insertions(+), 87 deletions(-) and the increase in debuggability is substantial as well. Thanks, Ingo
On Sun, Mar 9, 2025 at 6:48 PM Ingo Molnar <mingo@kernel.org> wrote: > > > * Ard Biesheuvel <ardb@kernel.org> wrote: > > > On Sat, 8 Mar 2025 at 17:17, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > ... > > > I do not think it is broken. > > > As I mentioned above, I regard vmlinux.relocs as a byproduct > > > of the atomic build rule of vmlinux. This works. > > Except when it doesn't work, such as when an intermediate linking step > fails, and intermediate build products are lost and cannot be recreated > easily (or at all without modifying the source)? > > And the thing is, there should be no such thing as an 'atomic build > rule of vmlinux' if it means lost information when the build is broken > at an intermediate step. What purpose does it have? > > > There is no make rule for vmlinux.relocs, and so > > > > - if it gets deleted, it cannot be rebuilt and even though the build > > does not break, the relocation data is missing from the compressed > > image, and this could potentially break the kaslr startup code, > > - it vmlinux.relocs is older than vmlinux for some reason, make will > > not notice and silently reuse the outdated version, > > - when creating vmlinux.relocs from vmlinux and an error occurs, > > vmlinux is deleted, making it difficult to diagnose the problem. > > > > I think this is badly broken, but if you think this is all working as > > it should, I am not going to debate this further, and you can consider > > the patch series withdrawn. > > That's very sad, as both the simplification is substantial: > > 19 files changed, 52 insertions(+), 87 deletions(-) > > and the increase in debuggability is substantial as well. > > Thanks, > > Ingo When a byproduct is accidentally lost (for example, manually deleted), it is not automatically restored. Running 'rm vmlinux' or 'make clean' is needed. vmlinux.relocs is one such byproduct. Another is the map file when CONFIG_VMLINUX_MAP=y is enabled. I am a bit concerned about having more and more intermediate vmlinux.* files, but maybe only me. I hope vmlinux.unstripped is the only/last one. OK, let's do this. Ard, please send v2 with .gitignore and 'clean' target updates. -- Best Regards Masahiro Yamada
On Tue, 11 Mar 2025 at 03:39, Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Sun, Mar 9, 2025 at 6:48 PM Ingo Molnar <mingo@kernel.org> wrote: > > > > > > * Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > On Sat, 8 Mar 2025 at 17:17, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > ... > > > > I do not think it is broken. > > > > As I mentioned above, I regard vmlinux.relocs as a byproduct > > > > of the atomic build rule of vmlinux. This works. > > > > Except when it doesn't work, such as when an intermediate linking step > > fails, and intermediate build products are lost and cannot be recreated > > easily (or at all without modifying the source)? > > > > And the thing is, there should be no such thing as an 'atomic build > > rule of vmlinux' if it means lost information when the build is broken > > at an intermediate step. What purpose does it have? > > > > > There is no make rule for vmlinux.relocs, and so > > > > > > - if it gets deleted, it cannot be rebuilt and even though the build > > > does not break, the relocation data is missing from the compressed > > > image, and this could potentially break the kaslr startup code, > > > - it vmlinux.relocs is older than vmlinux for some reason, make will > > > not notice and silently reuse the outdated version, > > > - when creating vmlinux.relocs from vmlinux and an error occurs, > > > vmlinux is deleted, making it difficult to diagnose the problem. > > > > > > I think this is badly broken, but if you think this is all working as > > > it should, I am not going to debate this further, and you can consider > > > the patch series withdrawn. > > > > That's very sad, as both the simplification is substantial: > > > > 19 files changed, 52 insertions(+), 87 deletions(-) > > > > and the increase in debuggability is substantial as well. > > > > Thanks, > > > > Ingo > > When a byproduct is accidentally lost > (for example, manually deleted), it is not automatically restored. > Running 'rm vmlinux' or 'make clean' is needed. > Exactly. Make cannot detect this situation, and so the build breaks in some way. > vmlinux.relocs is one such byproduct. > Another is the map file when CONFIG_VMLINUX_MAP=y is enabled. > The linker map is not depended upon by other build targets, and is typically for human debug consumption, so while not ideal, it is not as broken as for the unstripped vmlinux. > I am a bit concerned about having more and more > intermediate vmlinux.* files, but maybe only me. > I hope vmlinux.unstripped is the only/last one. > Maybe we should not strip vmlinux at all, but only remove any static relocations when packaging? E.g., tar-pkg, etc > > OK, let's do this. > > Ard, please send v2 with .gitignore and 'clean' target updates. > Thanks, I'll prepare a v2, but in the meantime, can we think about not removing the relocations in the first place?
On Tue, Mar 11, 2025 at 3:40 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 11 Mar 2025 at 03:39, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Sun, Mar 9, 2025 at 6:48 PM Ingo Molnar <mingo@kernel.org> wrote: > > > > > > > > > * Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > On Sat, 8 Mar 2025 at 17:17, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > ... > > > > > I do not think it is broken. > > > > > As I mentioned above, I regard vmlinux.relocs as a byproduct > > > > > of the atomic build rule of vmlinux. This works. > > > > > > Except when it doesn't work, such as when an intermediate linking step > > > fails, and intermediate build products are lost and cannot be recreated > > > easily (or at all without modifying the source)? > > > > > > And the thing is, there should be no such thing as an 'atomic build > > > rule of vmlinux' if it means lost information when the build is broken > > > at an intermediate step. What purpose does it have? > > > > > > > There is no make rule for vmlinux.relocs, and so > > > > > > > > - if it gets deleted, it cannot be rebuilt and even though the build > > > > does not break, the relocation data is missing from the compressed > > > > image, and this could potentially break the kaslr startup code, > > > > - it vmlinux.relocs is older than vmlinux for some reason, make will > > > > not notice and silently reuse the outdated version, > > > > - when creating vmlinux.relocs from vmlinux and an error occurs, > > > > vmlinux is deleted, making it difficult to diagnose the problem. > > > > > > > > I think this is badly broken, but if you think this is all working as > > > > it should, I am not going to debate this further, and you can consider > > > > the patch series withdrawn. > > > > > > That's very sad, as both the simplification is substantial: > > > > > > 19 files changed, 52 insertions(+), 87 deletions(-) > > > > > > and the increase in debuggability is substantial as well. > > > > > > Thanks, > > > > > > Ingo > > > > When a byproduct is accidentally lost > > (for example, manually deleted), it is not automatically restored. > > Running 'rm vmlinux' or 'make clean' is needed. > > > > Exactly. Make cannot detect this situation, and so the build breaks in some way. > > > vmlinux.relocs is one such byproduct. > > Another is the map file when CONFIG_VMLINUX_MAP=y is enabled. > > > > The linker map is not depended upon by other build targets, and is > typically for human debug consumption, so while not ideal, it is not > as broken as for the unstripped vmlinux. > > > > I am a bit concerned about having more and more > > intermediate vmlinux.* files, but maybe only me. > > I hope vmlinux.unstripped is the only/last one. > > > > Maybe we should not strip vmlinux at all, but only remove any static > relocations when packaging? E.g., tar-pkg, etc > > > > OK, let's do this. > > > > Ard, please send v2 with .gitignore and 'clean' target updates. > > > > Thanks, I'll prepare a v2, but in the meantime, can we think about not > removing the relocations in the first place? OK, let's try this approach. -- Best Regards Masahiro Yamada
On 3/11/25 07:40, Ard Biesheuvel wrote: > On Tue, 11 Mar 2025 at 03:39, Masahiro Yamada <masahiroy@kernel.org> wrote: >> I am a bit concerned about having more and more >> intermediate vmlinux.* files, but maybe only me. >> I hope vmlinux.unstripped is the only/last one. >> > > Maybe we should not strip vmlinux at all, but only remove any static > relocations when packaging? E.g., tar-pkg, etc Distributions typically have their own package recipes. The idea of stripping these relocations from vmlinux in one place was that everyone would trivially benefit from it. I'm not sure what you have exactly in mind implementation-wise here, but I think this would be useful to keep.
On Tue, Mar 11, 2025 at 3:40 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 11 Mar 2025 at 03:39, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Sun, Mar 9, 2025 at 6:48 PM Ingo Molnar <mingo@kernel.org> wrote: > > > > > > > > > * Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > On Sat, 8 Mar 2025 at 17:17, Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > ... > > > > > I do not think it is broken. > > > > > As I mentioned above, I regard vmlinux.relocs as a byproduct > > > > > of the atomic build rule of vmlinux. This works. > > > > > > Except when it doesn't work, such as when an intermediate linking step > > > fails, and intermediate build products are lost and cannot be recreated > > > easily (or at all without modifying the source)? > > > > > > And the thing is, there should be no such thing as an 'atomic build > > > rule of vmlinux' if it means lost information when the build is broken > > > at an intermediate step. What purpose does it have? > > > > > > > There is no make rule for vmlinux.relocs, and so > > > > > > > > - if it gets deleted, it cannot be rebuilt and even though the build > > > > does not break, the relocation data is missing from the compressed > > > > image, and this could potentially break the kaslr startup code, > > > > - it vmlinux.relocs is older than vmlinux for some reason, make will > > > > not notice and silently reuse the outdated version, > > > > - when creating vmlinux.relocs from vmlinux and an error occurs, > > > > vmlinux is deleted, making it difficult to diagnose the problem. > > > > > > > > I think this is badly broken, but if you think this is all working as > > > > it should, I am not going to debate this further, and you can consider > > > > the patch series withdrawn. > > > > > > That's very sad, as both the simplification is substantial: > > > > > > 19 files changed, 52 insertions(+), 87 deletions(-) > > > > > > and the increase in debuggability is substantial as well. > > > > > > Thanks, > > > > > > Ingo > > > > When a byproduct is accidentally lost > > (for example, manually deleted), it is not automatically restored. > > Running 'rm vmlinux' or 'make clean' is needed. > > > > Exactly. Make cannot detect this situation, and so the build breaks in some way. > > > vmlinux.relocs is one such byproduct. > > Another is the map file when CONFIG_VMLINUX_MAP=y is enabled. > > > > The linker map is not depended upon by other build targets, and is > typically for human debug consumption, so while not ideal, it is not > as broken as for the unstripped vmlinux. Now you learned this statement is wrong.
From: Ard Biesheuvel <ardb@kernel.org> Kbuild supports an architecture specific Makefile.postlink file that is invoked for the vmlinux target after it has been built. This Makefile takes 'vmlinux' (which has just been built) as the target, and mangles the file and/or constructs other intermediate artifacts from it. This violates the general philosophy of Make, which is based on rules and dependencies, and artifacts that are rebuilt only when any of their dependencies have been updated. Instead, the different incarnations of vmlinux that are consumed by different stages of the build should be emitted as distinct files, where rules and dependencies are used to define one in terms of the other. This also works around an error observed here [0], where vmlinux is deleted by Make because a subsequent step that consumes it as input throws an error. So refactor the vmlinux shell scripts and build rules so that architectures that rely on --emit-relocs to construct vmlinux with static relocations preserved will get a separate vmlinux.unstripped file carrying those relocations. This removes the need for an imperative postlink step, given that any rules that depend on the unstripped vmlinux can now simply depend on vmlinux.unstripped, rather than inject a build step into Makefile.postlink S390 should be able to do the same. MIPS and RISC-V perform some post-build checks on vmlinux, which is reasonable in principle for a postlink step, although deleting vmlinux when the check fails is equally unhelpful. [0] https://lore.kernel.org/all/Z5ARucnUgqjwBnrp@gmail.com/T/#m731ed0206949fc3f39fcc8a7b82fe348a8fc80c4 Cc: Masahiro Yamada <masahiroy@kernel.org> Cc: Ingo Molnar <mingo@kernel.org> Ard Biesheuvel (4): Kbuild/link-vmlinux.sh: Make output file name configurable Kbuild: Introduce Kconfig symbol for linking vmlinux with relocations Kbuild: Create intermediate vmlinux build with relocations preserved x86: Get rid of Makefile.postlink Makefile | 4 ++ arch/Kconfig | 7 ++++ arch/mips/Kconfig | 1 + arch/mips/Makefile | 4 -- arch/mips/Makefile.postlink | 2 +- arch/riscv/Kconfig | 1 + arch/riscv/Makefile | 2 +- arch/riscv/Makefile.postlink | 11 +----- arch/riscv/boot/Makefile | 5 +-- arch/s390/Kconfig | 1 + arch/s390/Makefile | 2 +- arch/s390/Makefile.postlink | 4 +- arch/x86/Kconfig | 1 + arch/x86/Makefile | 6 --- arch/x86/Makefile.postlink | 40 -------------------- arch/x86/boot/compressed/Makefile | 8 ++-- scripts/Makefile.lib | 2 +- scripts/Makefile.vmlinux | 27 +++++++++---- scripts/link-vmlinux.sh | 11 +++--- 19 files changed, 52 insertions(+), 87 deletions(-) delete mode 100644 arch/x86/Makefile.postlink