* [PATCH] x86/boot: efistub: Assign global boot_params variable @ 2023-10-16 16:46 Ard Biesheuvel 2023-10-17 6:57 ` Ard Biesheuvel 0 siblings, 1 reply; 5+ messages in thread From: Ard Biesheuvel @ 2023-10-16 16:46 UTC (permalink / raw) To: linux-efi; +Cc: x86, Ard Biesheuvel From: Ard Biesheuvel <ardb@kernel.org> Now that the x86 EFI stub calls into some APIs exposed by the decompressor (e.g., kaslr_get_random_long()), it is necessary to ensure that the global boot_params variable is set correctly before doing so. Note that the decompressor and the kernel proper carry conflicting declarations for the global variable 'boot_params' so refer to it via an alias to work around this. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- drivers/firmware/efi/libstub/x86-stub.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index 3bfc59637dd1..51407207ded5 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -812,12 +812,15 @@ void __noreturn efi_stub_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg, struct boot_params *boot_params) { + extern struct boot_params *bpp asm("boot_params"); efi_guid_t guid = EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID; struct setup_header *hdr = &boot_params->hdr; const struct linux_efi_initrd *initrd = NULL; unsigned long kernel_entry; efi_status_t status; + bpp = boot_params; + efi_system_table = sys_table_arg; /* Check if we were booted by the EFI firmware */ if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) -- 2.42.0.655.g421f12c284-goog ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/boot: efistub: Assign global boot_params variable 2023-10-16 16:46 [PATCH] x86/boot: efistub: Assign global boot_params variable Ard Biesheuvel @ 2023-10-17 6:57 ` Ard Biesheuvel 2023-10-17 11:34 ` Ingo Molnar 0 siblings, 1 reply; 5+ messages in thread From: Ard Biesheuvel @ 2023-10-17 6:57 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: linux-efi, x86 On Mon, 16 Oct 2023 at 18:46, Ard Biesheuvel <ardb@google.com> wrote: > > From: Ard Biesheuvel <ardb@kernel.org> > > Now that the x86 EFI stub calls into some APIs exposed by the > decompressor (e.g., kaslr_get_random_long()), it is necessary to ensure > that the global boot_params variable is set correctly before doing so. > > Note that the decompressor and the kernel proper carry conflicting > declarations for the global variable 'boot_params' so refer to it via an > alias to work around this. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > drivers/firmware/efi/libstub/x86-stub.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c > index 3bfc59637dd1..51407207ded5 100644 > --- a/drivers/firmware/efi/libstub/x86-stub.c > +++ b/drivers/firmware/efi/libstub/x86-stub.c > @@ -812,12 +812,15 @@ void __noreturn efi_stub_entry(efi_handle_t handle, > efi_system_table_t *sys_table_arg, > struct boot_params *boot_params) > { > + extern struct boot_params *bpp asm("boot_params"); I am getting hatemail from the bots about this extern declaration, so I will move that into x86-stub.h. Unless anyone minds, I'll queue this up and send it to Linus myself via the EFI fixes branch. > efi_guid_t guid = EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID; > struct setup_header *hdr = &boot_params->hdr; > const struct linux_efi_initrd *initrd = NULL; > unsigned long kernel_entry; > efi_status_t status; > > + bpp = boot_params; > + > efi_system_table = sys_table_arg; > /* Check if we were booted by the EFI firmware */ > if (efi_system_table->hdr.signature != EFI_SYSTEM_TABLE_SIGNATURE) > -- > 2.42.0.655.g421f12c284-goog > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/boot: efistub: Assign global boot_params variable 2023-10-17 6:57 ` Ard Biesheuvel @ 2023-10-17 11:34 ` Ingo Molnar 2023-10-17 11:38 ` Ard Biesheuvel 0 siblings, 1 reply; 5+ messages in thread From: Ingo Molnar @ 2023-10-17 11:34 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: Ard Biesheuvel, linux-efi, x86 * Ard Biesheuvel <ardb@kernel.org> wrote: > On Mon, 16 Oct 2023 at 18:46, Ard Biesheuvel <ardb@google.com> wrote: > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > Now that the x86 EFI stub calls into some APIs exposed by the > > decompressor (e.g., kaslr_get_random_long()), it is necessary to ensure > > that the global boot_params variable is set correctly before doing so. > > > > Note that the decompressor and the kernel proper carry conflicting > > declarations for the global variable 'boot_params' so refer to it via an > > alias to work around this. > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > drivers/firmware/efi/libstub/x86-stub.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c > > index 3bfc59637dd1..51407207ded5 100644 > > --- a/drivers/firmware/efi/libstub/x86-stub.c > > +++ b/drivers/firmware/efi/libstub/x86-stub.c > > @@ -812,12 +812,15 @@ void __noreturn efi_stub_entry(efi_handle_t handle, > > efi_system_table_t *sys_table_arg, > > struct boot_params *boot_params) > > { > > + extern struct boot_params *bpp asm("boot_params"); > > I am getting hatemail from the bots about this extern declaration, so > I will move that into x86-stub.h. Hm, it would be better if we moved all of x86-stub.h into arch/x86/include/asm/ or so. We don't really encourage "deep" driver-level exports like that, and there's little reason why this couldn't be in the x86 headers, right? > Unless anyone minds, I'll queue this up and send it to Linus myself > via the EFI fixes branch. Other than the above: Acked-by: Ingo Molnar <mingo@kernel.org> Thanks, Ingo ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/boot: efistub: Assign global boot_params variable 2023-10-17 11:34 ` Ingo Molnar @ 2023-10-17 11:38 ` Ard Biesheuvel 2023-10-17 13:06 ` Ingo Molnar 0 siblings, 1 reply; 5+ messages in thread From: Ard Biesheuvel @ 2023-10-17 11:38 UTC (permalink / raw) To: Ingo Molnar; +Cc: Ard Biesheuvel, linux-efi, x86 On Tue, 17 Oct 2023 at 13:34, Ingo Molnar <mingo@kernel.org> wrote: > > > * Ard Biesheuvel <ardb@kernel.org> wrote: > > > On Mon, 16 Oct 2023 at 18:46, Ard Biesheuvel <ardb@google.com> wrote: > > > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > > > Now that the x86 EFI stub calls into some APIs exposed by the > > > decompressor (e.g., kaslr_get_random_long()), it is necessary to ensure > > > that the global boot_params variable is set correctly before doing so. > > > > > > Note that the decompressor and the kernel proper carry conflicting > > > declarations for the global variable 'boot_params' so refer to it via an > > > alias to work around this. > > > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > > --- > > > drivers/firmware/efi/libstub/x86-stub.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c > > > index 3bfc59637dd1..51407207ded5 100644 > > > --- a/drivers/firmware/efi/libstub/x86-stub.c > > > +++ b/drivers/firmware/efi/libstub/x86-stub.c > > > @@ -812,12 +812,15 @@ void __noreturn efi_stub_entry(efi_handle_t handle, > > > efi_system_table_t *sys_table_arg, > > > struct boot_params *boot_params) > > > { > > > + extern struct boot_params *bpp asm("boot_params"); > > > > I am getting hatemail from the bots about this extern declaration, so > > I will move that into x86-stub.h. > > Hm, it would be better if we moved all of x86-stub.h into > arch/x86/include/asm/ or so. We don't really encourage "deep" driver-level > exports like that, and there's little reason why this couldn't be in the > x86 headers, right? > Yes, there is. arch/x86/boot/compressed/misc.h:extern struct boot_params *boot_params; arch/x86/include/asm/setup.h:extern struct boot_params boot_params; Ideally, we should get rid of the first one. Happy to send a patch that replaces all conflicting uses in the decompressor, so we can just access the global pointer without these tricks. But I'd like to get the fix in for v6.6 in any case. > > Unless anyone minds, I'll queue this up and send it to Linus myself > > via the EFI fixes branch. > > Other than the above: > > Acked-by: Ingo Molnar <mingo@kernel.org> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/boot: efistub: Assign global boot_params variable 2023-10-17 11:38 ` Ard Biesheuvel @ 2023-10-17 13:06 ` Ingo Molnar 0 siblings, 0 replies; 5+ messages in thread From: Ingo Molnar @ 2023-10-17 13:06 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: Ard Biesheuvel, linux-efi, x86 * Ard Biesheuvel <ardb@kernel.org> wrote: > On Tue, 17 Oct 2023 at 13:34, Ingo Molnar <mingo@kernel.org> wrote: > > > > > > * Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > On Mon, 16 Oct 2023 at 18:46, Ard Biesheuvel <ardb@google.com> wrote: > > > > > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > > > > > Now that the x86 EFI stub calls into some APIs exposed by the > > > > decompressor (e.g., kaslr_get_random_long()), it is necessary to ensure > > > > that the global boot_params variable is set correctly before doing so. > > > > > > > > Note that the decompressor and the kernel proper carry conflicting > > > > declarations for the global variable 'boot_params' so refer to it via an > > > > alias to work around this. > > > > > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > > > --- > > > > drivers/firmware/efi/libstub/x86-stub.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c > > > > index 3bfc59637dd1..51407207ded5 100644 > > > > --- a/drivers/firmware/efi/libstub/x86-stub.c > > > > +++ b/drivers/firmware/efi/libstub/x86-stub.c > > > > @@ -812,12 +812,15 @@ void __noreturn efi_stub_entry(efi_handle_t handle, > > > > efi_system_table_t *sys_table_arg, > > > > struct boot_params *boot_params) > > > > { > > > > + extern struct boot_params *bpp asm("boot_params"); > > > > > > I am getting hatemail from the bots about this extern declaration, so > > > I will move that into x86-stub.h. > > > > Hm, it would be better if we moved all of x86-stub.h into > > arch/x86/include/asm/ or so. We don't really encourage "deep" driver-level > > exports like that, and there's little reason why this couldn't be in the > > x86 headers, right? > > > > Yes, there is. > > arch/x86/boot/compressed/misc.h:extern struct boot_params *boot_params; > arch/x86/include/asm/setup.h:extern struct boot_params boot_params; > > Ideally, we should get rid of the first one. > > Happy to send a patch that replaces all conflicting uses in the > decompressor, so we can just access the global pointer without these > tricks. Yeah, that would be lovely, thanks. > But I'd like to get the fix in for v6.6 in any case. Sure: Acked-by: Ingo Molnar <mingo@kernel.org> Thanks, Ingo ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-10-17 13:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-16 16:46 [PATCH] x86/boot: efistub: Assign global boot_params variable Ard Biesheuvel 2023-10-17 6:57 ` Ard Biesheuvel 2023-10-17 11:34 ` Ingo Molnar 2023-10-17 11:38 ` Ard Biesheuvel 2023-10-17 13:06 ` Ingo Molnar
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).