Re: [PATCH 2/2] x86/efi: Implement support for embedding SBAT data for x86

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Ard Biesheuvel <ardb@kernel.org> writes:

> On Tue, 29 Apr 2025 at 11:55, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> Ard Biesheuvel <ardb@kernel.org> writes:
>>
>> > On Thu, 24 Apr 2025 at 10:10, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> ...
>>
>> >> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
>> >> index fdbce022db55..b9b80eccdc02 100644
>> >> --- a/arch/x86/boot/compressed/Makefile
>> >> +++ b/arch/x86/boot/compressed/Makefile
>> >> @@ -107,6 +107,8 @@ vmlinux-objs-$(CONFIG_UNACCEPTED_MEMORY) += $(obj)/mem.o
>> >>  vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o
>> >>  vmlinux-libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
>> >>
>> >> +vmlinux-objs-$(CONFIG_EFI_SBAT) += $(objtree)/drivers/firmware/efi/libstub/sbat.o
>> >> +
>> >
>> > Please drop this, and put the .incbin directly into header.S
>> >
>>
>> I'm sorry I'm probably missing something important but my understanding
>> is that that header.S is compiled into setup.elf:
>>
>>  ld -m elf_x86_64 -z noexecstack --no-warn-rwx-segments  -m elf_i386 -z
>>  noexecstack -T arch/x86/boot/setup.ld  ... arch/x86/boot/header.o ...  -o arch/x86/boot/setup.elf
>>
>> and then the result gets concatenated with vmlinux.bin to get bzImage:
>>
>>  objcopy  -O binary arch/x86/boot/setup.elf arch/x86/boot/setup.bin
>>  cp arch/x86/boot/setup.bin arch/x86/boot/bzImage; truncate -s %4K arch/x86/boot/bzImage; cat arch/x86/boot/vmlinux.bin >>arch/x86/boot/bzImage
>>
>> so if we want to have SBAT at the very end of bzImage without dirty
>> tricks it must be at the very end of vmlinux.bin, not setup.bin. I can,
>> of course, use some existing compilation unit but to be honest I can't
>> find anything suitable.
>>
>
> Yeah, you're right. I keep forgetting the insane way the bzImage is
> put together.
>
> So you'll need to incorporate $(CONFIG_EFI_SBAT_FILE) into
> arch/x86/boot/vmlinux. But that does not mean it needs to be
> constructed under drivers/firmware/efi/libstub, and it also doesn't
> mean you need filechk and a separate .o file, right?

Right, it just needs to be somewhere and this somewhere needs to depend
on the SBAT data to track its possible updates. E.g. looking at asm
files in arch/x86/boot/compressed/ (which go to vmlinux) I see:

arch/x86/boot/compressed/head_32.S
arch/x86/boot/compressed/head_64.S
arch/x86/boot/compressed/idt_handlers_64.S
arch/x86/boot/compressed/kernel_info.S
arch/x86/boot/compressed/la57toggle.S
arch/x86/boot/compressed/mem_encrypt.S
arch/x86/boot/compressed/piggy.S
arch/x86/boot/compressed/tdcall.S

and honestly I don't know which one to pick :-( An alternative would be
to create separate 3-line sbat.S files for x86 and zboot and then make
sbat.o dependent on CONFIG_EFI_SBAT_FILE but that would not satisfy all
the requirements as sbat.o stays)

-- 
Vitaly