[PATCH v3 9/9] efi: libstub: Simplify interfaces for primary_display
Ard Biesheuvel
ardb at kernel.org
Tue Dec 16 05:23:28 PST 2025
Hi Thomas
On Wed, 26 Nov 2025 at 17:09, Thomas Zimmermann <tzimmermann at suse.de> wrote:
>
> Rename alloc_primary_display() and __alloc_primary_display(), clarify
> free semantics to make interfaces easier to understand.
>
> Rename alloc_primary_display() to lookup_primary_display() as it
> does not necessarily allocate. Then rename __alloc_primary_display()
> to the new alloc_primary_display(). The helper belongs to
> free_primary_display), so it should be named without underscores.
>
> The lookup helper does not necessarily allocate, so the output
> parameter needs_free to indicate when free should be called.
I don't understand why we need this. Whether or not the helper
allocates is a compile time decision, and in builds where it doesn't,
the free helper doesn't do anything.
I'm all for making things simpler, but I don't think this patch
achieves that tbh.
I've queued up this series now up until this patch - once we converge
on the simplification, I'm happy to apply it on top.
Thanks,
> Pass
> an argument through the calls to track this state. Put the free
> handling into release_primary_display() for simplificy.
>
> Also move the comment fro primary_display.c to efi-stub-entry.c,
> where it now describes lookup_primary_display().
>
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
> drivers/firmware/efi/libstub/efi-stub-entry.c | 23 +++++++++++++++++--
> drivers/firmware/efi/libstub/efi-stub.c | 22 ++++++++++++------
> drivers/firmware/efi/libstub/efistub.h | 2 +-
> .../firmware/efi/libstub/primary_display.c | 17 +-------------
> drivers/firmware/efi/libstub/zboot.c | 6 +++--
> 5 files changed, 42 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-entry.c b/drivers/firmware/efi/libstub/efi-stub-entry.c
> index aa85e910fe59..3077b51fe0b2 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-entry.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-entry.c
> @@ -14,10 +14,29 @@ static void *kernel_image_addr(void *addr)
> return addr + kernel_image_offset;
> }
>
> -struct sysfb_display_info *alloc_primary_display(void)
> +/*
> + * There are two ways of populating the core kernel's sysfb_primary_display
> + * via the stub:
> + *
> + * - using a configuration table, which relies on the EFI init code to
> + * locate the table and copy the contents; or
> + *
> + * - 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.
> + */
> +
> +struct sysfb_display_info *lookup_primary_display(bool *needs_free)
> {
> + *needs_free = true;
> +
> if (IS_ENABLED(CONFIG_ARM))
> - return __alloc_primary_display();
> + return alloc_primary_display();
> +
> + *needs_free = false;
>
> if (IS_ENABLED(CONFIG_X86) ||
> IS_ENABLED(CONFIG_EFI_EARLYCON) ||
> diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
> index 42d6073bcd06..dc545f62c62b 100644
> --- a/drivers/firmware/efi/libstub/efi-stub.c
> +++ b/drivers/firmware/efi/libstub/efi-stub.c
> @@ -51,14 +51,14 @@ static bool flat_va_mapping = (EFI_RT_VIRTUAL_OFFSET != 0);
> void __weak free_primary_display(struct sysfb_display_info *dpy)
> { }
>
> -static struct sysfb_display_info *setup_primary_display(void)
> +static struct sysfb_display_info *setup_primary_display(bool *dpy_needs_free)
> {
> struct sysfb_display_info *dpy;
> struct screen_info *screen = NULL;
> struct edid_info *edid = NULL;
> efi_status_t status;
>
> - dpy = alloc_primary_display();
> + dpy = lookup_primary_display(dpy_needs_free);
> if (!dpy)
> return NULL;
> screen = &dpy->screen;
> @@ -68,15 +68,22 @@ static struct sysfb_display_info *setup_primary_display(void)
>
> status = efi_setup_graphics(screen, edid);
> if (status != EFI_SUCCESS)
> - goto err_free_primary_display;
> + goto err___free_primary_display;
>
> return dpy;
>
> -err_free_primary_display:
> - free_primary_display(dpy);
> +err___free_primary_display:
> + if (*dpy_needs_free)
> + free_primary_display(dpy);
> return NULL;
> }
>
> +static void release_primary_display(struct sysfb_display_info *dpy, bool dpy_needs_free)
> +{
> + if (dpy && dpy_needs_free)
> + free_primary_display(dpy);
> +}
> +
> static void install_memreserve_table(void)
> {
> struct linux_efi_memreserve *rsv;
> @@ -156,13 +163,14 @@ efi_status_t efi_stub_common(efi_handle_t handle,
> char *cmdline_ptr)
> {
> struct sysfb_display_info *dpy;
> + bool dpy_needs_free;
> efi_status_t status;
>
> status = check_platform_features();
> if (status != EFI_SUCCESS)
> return status;
>
> - dpy = setup_primary_display();
> + dpy = setup_primary_display(&dpy_needs_free);
>
> efi_retrieve_eventlog();
>
> @@ -182,7 +190,7 @@ efi_status_t efi_stub_common(efi_handle_t handle,
>
> status = efi_boot_kernel(handle, image, image_addr, cmdline_ptr);
>
> - free_primary_display(dpy);
> + release_primary_display(dpy, dpy_needs_free);
>
> return status;
> }
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 979a21818cc1..1503ffb82903 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -1176,8 +1176,8 @@ efi_enable_reset_attack_mitigation(void) { }
>
> void efi_retrieve_eventlog(void);
>
> +struct sysfb_display_info *lookup_primary_display(bool *needs_free);
> struct sysfb_display_info *alloc_primary_display(void);
> -struct sysfb_display_info *__alloc_primary_display(void);
> void free_primary_display(struct sysfb_display_info *dpy);
>
> void efi_cache_sync_image(unsigned long image_base,
> diff --git a/drivers/firmware/efi/libstub/primary_display.c b/drivers/firmware/efi/libstub/primary_display.c
> index cdaebab26514..34c54ac1e02a 100644
> --- a/drivers/firmware/efi/libstub/primary_display.c
> +++ b/drivers/firmware/efi/libstub/primary_display.c
> @@ -7,24 +7,9 @@
>
> #include "efistub.h"
>
> -/*
> - * There are two ways of populating the core kernel's sysfb_primary_display
> - * via the stub:
> - *
> - * - using a configuration table, which relies on the EFI init code to
> - * locate the table and copy the contents; or
> - *
> - * - 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.
> - */
> -
> static efi_guid_t primary_display_guid = LINUX_EFI_PRIMARY_DISPLAY_TABLE_GUID;
>
> -struct sysfb_display_info *__alloc_primary_display(void)
> +struct sysfb_display_info *alloc_primary_display(void)
> {
> struct sysfb_display_info *dpy;
> efi_status_t status;
> diff --git a/drivers/firmware/efi/libstub/zboot.c b/drivers/firmware/efi/libstub/zboot.c
> index 4b76f74c56da..c1fd1fdbcb08 100644
> --- a/drivers/firmware/efi/libstub/zboot.c
> +++ b/drivers/firmware/efi/libstub/zboot.c
> @@ -26,9 +26,11 @@ void __weak efi_cache_sync_image(unsigned long image_base,
> // executable code loaded into memory to be safe for execution.
> }
>
> -struct sysfb_display_info *alloc_primary_display(void)
> +struct sysfb_display_info *lookup_primary_display(bool *needs_free)
> {
> - return __alloc_primary_display();
> + *needs_free = true;
> +
> + return alloc_primary_display();
> }
>
> asmlinkage efi_status_t __efiapi
> --
> 2.51.1
>
More information about the linux-riscv
mailing list