| Message ID | 20250408085254.836788-9-ardb+git@google.com |
|---|---|
| Headers | show |
| Series | x86: Refactor and consolidate startup code | expand |
On Tue, Apr 8, 2025 at 5:01 AM Ard Biesheuvel <ardb+git@google.com> wrote: > > From: Ard Biesheuvel <ardb@kernel.org> > > Reorganize C code that is used during early boot, either in the > decompressor/EFI stub or the kernel proper, but before the kernel > virtual mapping is up. > > v3: > - keep rip_rel_ptr() around in PIC code - sadly, it is still needed in > some cases > - remove RIP_REL_REF() uses in separate patches > - keep __head annotations for now, they will all be removed later > - disable objtool validation for library objects (i.e., pieces that are > not linked into vmlinux) > > I will follow up with a series that gets rid of .head.text altogether, > as it will no longer be needed at all once the startup code is checked > for absolute relocations. > > The SEV startup code needs to be moved first, though, and this is a bit > more complicated, so I will decouple that effort from this series, also > because there is a known issue that needs to be fixed first related to > memory acceptance from the EFI stub. Is there anything to verify that the compiler doesn't do something unexpected with PIC code generation like create GOT references? Brian Gerst
On Tue, 8 Apr 2025 at 20:16, Brian Gerst <brgerst@gmail.com> wrote: > > On Tue, Apr 8, 2025 at 5:01 AM Ard Biesheuvel <ardb+git@google.com> wrote: > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > Reorganize C code that is used during early boot, either in the > > decompressor/EFI stub or the kernel proper, but before the kernel > > virtual mapping is up. > > > > v3: > > - keep rip_rel_ptr() around in PIC code - sadly, it is still needed in > > some cases > > - remove RIP_REL_REF() uses in separate patches > > - keep __head annotations for now, they will all be removed later > > - disable objtool validation for library objects (i.e., pieces that are > > not linked into vmlinux) > > > > I will follow up with a series that gets rid of .head.text altogether, > > as it will no longer be needed at all once the startup code is checked > > for absolute relocations. > > > > The SEV startup code needs to be moved first, though, and this is a bit > > more complicated, so I will decouple that effort from this series, also > > because there is a known issue that needs to be fixed first related to > > memory acceptance from the EFI stub. > > Is there anything to verify that the compiler doesn't do something > unexpected with PIC code generation like create GOT references? > I will propose something along the lines of what is already being done for the EFI stub: ------%<------ STUBCOPY_RELOC-$(CONFIG_X86_64) := R_X86_64_64 quiet_cmd_stubcopy = STUBCPY $@ cmd_stubcopy = \ $(STRIP) --strip-debug -o $@ $<; \ if $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y); then \ echo "$@: absolute symbol references not allowed in the EFI stub" >&2; \ /bin/false; \ fi; \ $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@
On 4/8/2025 2:22 PM, Ard Biesheuvel wrote: > From: Ard Biesheuvel <ardb@kernel.org> > > The library code built under arch/x86/boot/startup is not intended to be > linked into vmlinux but only into the decompressor and/or the EFI stub. > > This means objtool validation is not needed here, and may result in > false positive errors for things like missing retpolines. > > So disable it for all objects added to lib-y > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> Solves https://lore.kernel.org/intel-gfx/CAMj1kXEfBMczOmA2+dMMubuD-qE59GTAiV2E_9m8KNG4-rgP6Q@mail.gmail.com/T/#mbf2913e778475b70617390d4a5d0244295b9cb8c Tested-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> > --- > arch/x86/boot/startup/Makefile | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/x86/boot/startup/Makefile b/arch/x86/boot/startup/Makefile > index 73946a3f6b3b..8919a1cbcb5a 100644 > --- a/arch/x86/boot/startup/Makefile > +++ b/arch/x86/boot/startup/Makefile > @@ -4,3 +4,9 @@ KBUILD_AFLAGS += -D__DISABLE_EXPORTS > > lib-$(CONFIG_X86_64) += la57toggle.o > lib-$(CONFIG_EFI_MIXED) += efi-mixed.o > + > +# > +# Disable objtool validation for all library code, which is intended > +# to be linked into the decompressor or the EFI stub but not vmlinux > +# > +$(patsubst %.o,$(obj)/%.o,$(lib-y)): OBJECT_FILES_NON_STANDARD := y
* Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com> wrote: > > On 4/8/2025 2:22 PM, Ard Biesheuvel wrote: > > From: Ard Biesheuvel <ardb@kernel.org> > > > > The library code built under arch/x86/boot/startup is not intended to be > > linked into vmlinux but only into the decompressor and/or the EFI stub. > > > > This means objtool validation is not needed here, and may result in > > false positive errors for things like missing retpolines. > > > > So disable it for all objects added to lib-y > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > Solves https://lore.kernel.org/intel-gfx/CAMj1kXEfBMczOmA2+dMMubuD-qE59GTAiV2E_9m8KNG4-rgP6Q@mail.gmail.com/T/#mbf2913e778475b70617390d4a5d0244295b9cb8c > > Tested-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com> Thank you for the testing! Ingo
* Ard Biesheuvel <ardb+git@google.com> wrote: > From: Ard Biesheuvel <ardb@kernel.org> > > Move the early GDT/IDT setup code that runs long before the kernel > virtual mapping is up into arch/x86/boot/startup/, and build it in a way > that ensures that the code tolerates being called from the 1:1 mapping > of memory. The code itself is left unchanged by this patch. > > Also tweak the sed symbol matching pattern in the decompressor to match > on lower case 't' or 'b', as these will be emitted by Clang for symbols > with hidden linkage. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > arch/x86/boot/compressed/Makefile | 2 +- > arch/x86/boot/startup/Makefile | 15 ++++ > arch/x86/boot/startup/gdt_idt.c | 83 ++++++++++++++++++++ > arch/x86/kernel/head64.c | 73 ----------------- > 4 files changed, 99 insertions(+), 74 deletions(-) This causes the following build failure on x86-64-defconfig: arch/x86/boot/startup/gdt_idt.c:67:55: error: cast to generic address space pointer from disjoint ‘__seg_gs’ address space pointer [-Werror] Thanks, Ingo
* Ingo Molnar <mingo@kernel.org> wrote: > > * Ard Biesheuvel <ardb+git@google.com> wrote: > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > Move the early GDT/IDT setup code that runs long before the kernel > > virtual mapping is up into arch/x86/boot/startup/, and build it in a way > > that ensures that the code tolerates being called from the 1:1 mapping > > of memory. The code itself is left unchanged by this patch. > > > > Also tweak the sed symbol matching pattern in the decompressor to match > > on lower case 't' or 'b', as these will be emitted by Clang for symbols > > with hidden linkage. > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > arch/x86/boot/compressed/Makefile | 2 +- > > arch/x86/boot/startup/Makefile | 15 ++++ > > arch/x86/boot/startup/gdt_idt.c | 83 ++++++++++++++++++++ > > arch/x86/kernel/head64.c | 73 ----------------- > > 4 files changed, 99 insertions(+), 74 deletions(-) > > This causes the following build failure on x86-64-defconfig: > > arch/x86/boot/startup/gdt_idt.c:67:55: error: cast to generic address space pointer from disjoint ‘__seg_gs’ address space pointer [-Werror] Caused by the previous patch: x86/asm: Make rip_rel_ptr() usable from fPIC code Thanks, Ingo
On Wed, 9 Apr 2025 at 12:07, Ingo Molnar <mingo@kernel.org> wrote: > > > * Ingo Molnar <mingo@kernel.org> wrote: > > > > > * Ard Biesheuvel <ardb+git@google.com> wrote: > > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > > > Move the early GDT/IDT setup code that runs long before the kernel > > > virtual mapping is up into arch/x86/boot/startup/, and build it in a way > > > that ensures that the code tolerates being called from the 1:1 mapping > > > of memory. The code itself is left unchanged by this patch. > > > > > > Also tweak the sed symbol matching pattern in the decompressor to match > > > on lower case 't' or 'b', as these will be emitted by Clang for symbols > > > with hidden linkage. > > > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > > --- > > > arch/x86/boot/compressed/Makefile | 2 +- > > > arch/x86/boot/startup/Makefile | 15 ++++ > > > arch/x86/boot/startup/gdt_idt.c | 83 ++++++++++++++++++++ > > > arch/x86/kernel/head64.c | 73 ----------------- > > > 4 files changed, 99 insertions(+), 74 deletions(-) > > > > This causes the following build failure on x86-64-defconfig: > > > > arch/x86/boot/startup/gdt_idt.c:67:55: error: cast to generic address space pointer from disjoint ‘__seg_gs’ address space pointer [-Werror] > > Caused by the previous patch: > > x86/asm: Make rip_rel_ptr() usable from fPIC code > Oops, sorry about that. I saw that error and thought I had fixed it with the (__force void*) cast.
* Ard Biesheuvel <ardb@kernel.org> wrote: > On Wed, 9 Apr 2025 at 12:07, Ingo Molnar <mingo@kernel.org> wrote: > > > > > > * Ingo Molnar <mingo@kernel.org> wrote: > > > > > > > > * Ard Biesheuvel <ardb+git@google.com> wrote: > > > > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > > > > > Move the early GDT/IDT setup code that runs long before the kernel > > > > virtual mapping is up into arch/x86/boot/startup/, and build it in a way > > > > that ensures that the code tolerates being called from the 1:1 mapping > > > > of memory. The code itself is left unchanged by this patch. > > > > > > > > Also tweak the sed symbol matching pattern in the decompressor to match > > > > on lower case 't' or 'b', as these will be emitted by Clang for symbols > > > > with hidden linkage. > > > > > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > > > --- > > > > arch/x86/boot/compressed/Makefile | 2 +- > > > > arch/x86/boot/startup/Makefile | 15 ++++ > > > > arch/x86/boot/startup/gdt_idt.c | 83 ++++++++++++++++++++ > > > > arch/x86/kernel/head64.c | 73 ----------------- > > > > 4 files changed, 99 insertions(+), 74 deletions(-) > > > > > > This causes the following build failure on x86-64-defconfig: > > > > > > arch/x86/boot/startup/gdt_idt.c:67:55: error: cast to generic address space pointer from disjoint ‘__seg_gs’ address space pointer [-Werror] > > > > Caused by the previous patch: > > > > x86/asm: Make rip_rel_ptr() usable from fPIC code > > > > Oops, sorry about that. I saw that error and thought I had fixed it > with the (__force void*) cast. NP, caught it early enough. Thanks, Ingo
From: Ard Biesheuvel <ardb@kernel.org> Reorganize C code that is used during early boot, either in the decompressor/EFI stub or the kernel proper, but before the kernel virtual mapping is up. v3: - keep rip_rel_ptr() around in PIC code - sadly, it is still needed in some cases - remove RIP_REL_REF() uses in separate patches - keep __head annotations for now, they will all be removed later - disable objtool validation for library objects (i.e., pieces that are not linked into vmlinux) I will follow up with a series that gets rid of .head.text altogether, as it will no longer be needed at all once the startup code is checked for absolute relocations. The SEV startup code needs to be moved first, though, and this is a bit more complicated, so I will decouple that effort from this series, also because there is a known issue that needs to be fixed first related to memory acceptance from the EFI stub. Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Dionna Amalie Glaze <dionnaglaze@google.com> Cc: Kevin Loughlin <kevinloughlin@google.com> Ard Biesheuvel (7): x86/boot/startup: Disable objtool validation for library code x86/asm: Make rip_rel_ptr() usable from fPIC code x86/boot: Move the early GDT/IDT setup code into startup/ x86/boot: Move early kernel mapping code into startup/ x86/boot: Drop RIP_REL_REF() uses from early mapping code x86/boot: Move early SME init code into startup/ x86/boot: Drop RIP_REL_REF() uses from SME startup code arch/x86/boot/compressed/Makefile | 2 +- arch/x86/boot/startup/Makefile | 22 ++ arch/x86/boot/startup/gdt_idt.c | 83 ++++++ arch/x86/boot/startup/map_kernel.c | 225 ++++++++++++++++ arch/x86/{mm/mem_encrypt_identity.c => boot/startup/sme.c} | 19 +- arch/x86/coco/sev/core.c | 2 +- arch/x86/coco/sev/shared.c | 4 +- arch/x86/include/asm/asm.h | 2 +- arch/x86/include/asm/coco.h | 2 +- arch/x86/include/asm/mem_encrypt.h | 2 +- arch/x86/kernel/head64.c | 285 +------------------- arch/x86/mm/Makefile | 6 - 12 files changed, 346 insertions(+), 308 deletions(-) create mode 100644 arch/x86/boot/startup/gdt_idt.c create mode 100644 arch/x86/boot/startup/map_kernel.c rename arch/x86/{mm/mem_encrypt_identity.c => boot/startup/sme.c} (97%) base-commit: 4f2d1bbc2c92a32fd612e6c3b51832d5c1c3678e