On Mon, 13 Mar 2023 at 08:56, Huacai Chen <chenhuacai@xxxxxxxxxx> wrote: > > Hi, Ard, > > I'm a bit confused, It seems this patch cannot solve the problem > reported in [1] for LoongArch. > [1] https://lore.kernel.org/linux-efi/20230310021749.921041-1-chenhuacai@xxxxxxxxxxx/ > The intent was to always pass the screen_info contents via the configuration table, so I am surprised that this does not work. However, I am preparing a v2 that preserves the old behavior - I'll send it out in a minute. > > On Mon, Mar 13, 2023 at 3:45 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > On Sun, 12 Mar 2023 at 23:58, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > > > > > In some cases, we expose the kernel's struct screen_info to the EFI stub > > > directly, so it gets populated before even entering the kernel. This > > > means the early console is available as soon as the early param parsing > > > happens, which is nice. It also means we need two different ways to pass > > > this information, as this trick only works if the EFI stub is baked into > > > the core kernel image, which is not always the case. > > > > > > Huacai reports that the preparatory refactoring that was needed to > > > implement this alternative method for zboot resulted in a non-functional > > > efifb earlycon for other cases as well, due to the reordering of the > > > kernel image relocation with the population of the screen_info struct, > > > and the latter now takes place after copying the image to its new > > > location, which means we copy the old, uninitialized state. > > > > > > To fix this and simplify things at the same time, let's just always use > > > the config table method. > > > > > > Cc: loongarch@xxxxxxxxxxxxxxx > > > Cc: Xuefeng Li <lixuefeng@xxxxxxxxxxx> > > > Cc: Xuerui Wang <kernel@xxxxxxxxxx> > > > Cc: loongson-kernel@xxxxxxxxxxxxxxxxx > > > Reported-by: Huacai Chen <chenhuacai@xxxxxxxxxxx> > > > Link: https://lore.kernel.org/linux-efi/20230310021749.921041-1-chenhuacai@xxxxxxxxxxx/ > > > Fixes: 42c8ea3dca094ab8 ("efi: libstub: Factor out EFI stub entrypoint into separate file") > > > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx> > > > > Please disregard - I think I can simplify this without losing the very > > early earlycon. > > > > > > > --- > > > arch/arm64/kernel/image-vars.h | 1 - > > > arch/loongarch/kernel/image-vars.h | 1 - > > > arch/riscv/kernel/image-vars.h | 1 - > > > drivers/firmware/efi/libstub/efi-stub.c | 9 --------- > > > drivers/firmware/efi/libstub/screen_info.c | 19 ------------------- > > > 5 files changed, 31 deletions(-) > > > > > > diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h > > > index 8309197c0ebd4a8e..01382bd99d0addea 100644 > > > --- a/arch/arm64/kernel/image-vars.h > > > +++ b/arch/arm64/kernel/image-vars.h > > > @@ -27,7 +27,6 @@ PROVIDE(__efistub__text = _text); > > > PROVIDE(__efistub__end = _end); > > > PROVIDE(__efistub___inittext_end = __inittext_end); > > > PROVIDE(__efistub__edata = _edata); > > > -PROVIDE(__efistub_screen_info = screen_info); > > > PROVIDE(__efistub__ctype = _ctype); > > > > > > PROVIDE(__pi___memcpy = __pi_memcpy); > > > diff --git a/arch/loongarch/kernel/image-vars.h b/arch/loongarch/kernel/image-vars.h > > > index e561989d02de93c5..110cdc32cba5f28b 100644 > > > --- a/arch/loongarch/kernel/image-vars.h > > > +++ b/arch/loongarch/kernel/image-vars.h > > > @@ -12,7 +12,6 @@ __efistub_kernel_entry = kernel_entry; > > > __efistub_kernel_asize = kernel_asize; > > > __efistub_kernel_fsize = kernel_fsize; > > > __efistub_kernel_offset = kernel_offset; > > > -__efistub_screen_info = screen_info; > > > > > > #endif > > > > > > diff --git a/arch/riscv/kernel/image-vars.h b/arch/riscv/kernel/image-vars.h > > > index 7e2962ef73f92e95..5e2515fa013495cb 100644 > > > --- a/arch/riscv/kernel/image-vars.h > > > +++ b/arch/riscv/kernel/image-vars.h > > > @@ -29,7 +29,6 @@ __efistub__start = _start; > > > __efistub__start_kernel = _start_kernel; > > > __efistub__end = _end; > > > __efistub__edata = _edata; > > > -__efistub_screen_info = screen_info; > > > > > > #endif > > > > > > diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c > > > index 2955c1ac6a36ee00..c4b9eccad0f103dc 100644 > > > --- a/drivers/firmware/efi/libstub/efi-stub.c > > > +++ b/drivers/firmware/efi/libstub/efi-stub.c > > > @@ -47,15 +47,6 @@ > > > static u64 virtmap_base = EFI_RT_VIRTUAL_BASE; > > > static bool flat_va_mapping = (EFI_RT_VIRTUAL_OFFSET != 0); > > > > > > -struct screen_info * __weak alloc_screen_info(void) > > > -{ > > > - return &screen_info; > > > -} > > > - > > > -void __weak free_screen_info(struct screen_info *si) > > > -{ > > > -} > > > - > > > static struct screen_info *setup_graphics(void) > > > { > > > efi_guid_t gop_proto = EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID; > > > diff --git a/drivers/firmware/efi/libstub/screen_info.c b/drivers/firmware/efi/libstub/screen_info.c > > > index 8e76a8b384ba142d..7b791541a1ad1b48 100644 > > > --- a/drivers/firmware/efi/libstub/screen_info.c > > > +++ b/drivers/firmware/efi/libstub/screen_info.c > > > @@ -5,25 +5,6 @@ > > > > > > #include "efistub.h" > > > > > > -/* > > > - * There are two ways of populating the core kernel's struct screen_info via the stub: > > > - * - using a configuration table, like below, which relies on the EFI init code > > > - * to locate the table and copy the contents; > > > - * - by linking directly to the core kernel's copy of the global symbol. > > > - * > > > - * The latter is preferred because it makes the EFIFB earlycon available very > > > - * early, but it only works if the EFI stub is part of the core kernel image > > > - * itself. The zboot decompressor can only use the configuration table > > > - * approach. > > > - * > > > - * In order to support both methods from the same build of the EFI stub > > > - * library, provide this dummy global definition of struct screen_info. If it > > > - * is required to satisfy a link dependency, it means we need to override the > > > - * __weak alloc and free methods with the ones below, and those will be pulled > > > - * in as well. > > > - */ > > > -struct screen_info screen_info; > > > - > > > static efi_guid_t screen_info_guid = LINUX_EFI_SCREEN_INFO_TABLE_GUID; > > > > > > struct screen_info *alloc_screen_info(void) > > > -- > > > 2.39.2 > > > > >
