Re: [PATCH 2/3] lmb: move lmb_map_update_notify() to EFI

[Alexander Dahl <ada@thorsis.com>]

Hei hei,

Am Sun, Feb 16, 2025 at 12:12:40PM +0100 schrieb Heinrich Schuchardt:
> When building with qemu_arm64_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y
> and CONFIG_EFI_LOADER an error undefined reference to efi_add_memory_map_pg
> occurs.
> 
> Move the EFI dependent part of lmb_map_update_notify() to the EFI
> sub-system.
> 
> Reported-by: Liya Huang <1425075683@qq.com>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Acked-by: Liya Huang <1425075683@qq.com>

This seems to be the same underlying problem I reported back in January:

https://lore.kernel.org/u-boot/20250117-molecular-quicksand-e80bf947313e@thorsis.com/

FWIW, while your patch hit master, and the follow-up to my report did
not yet … you might be interested that master does not fail to build
the armv5 targets with CC_OPTIMIZE_FOR_DEBUG anymore.

Besides: keeping EFI code in the EFI subsystem seems the right
approach to me, too. ;-)

Thanks and Greets
Alex

> ---
>  include/efi_loader.h        | 15 +++++++++++++++
>  lib/efi_loader/efi_memory.c | 27 +++++++++++++++++++++++++++
>  lib/lmb.c                   | 31 +++----------------------------
>  3 files changed, 45 insertions(+), 28 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index dcae6a731a0..db3d20fd753 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -1263,6 +1263,21 @@ efi_status_t efi_disk_get_device_name(const efi_handle_t handle, char *buf, int
>   */
>  void efi_add_known_memory(void);
>  
> +/**
> + * efi_map_update_notify() - notify EFI of memory map changes
> + *
> + * @addr:	start of memory area
> + * @size:	size of memory area
> + * @op:		type of change
> + * Return:	0 if change could be processed
> + */
> +#ifdef CONFIG_EFI_LOADER
> +int efi_map_update_notify(phys_addr_t addr, phys_size_t size,
> +			  enum lmb_map_op op);
> +#else
> +#define efi_map_update_notify(addr, size, op) (0)
> +#endif
> +
>  /**
>   * efi_load_option_dp_join() - join device-paths for load option
>   *
> diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> index 1212772471e..11d092dc289 100644
> --- a/lib/efi_loader/efi_memory.c
> +++ b/lib/efi_loader/efi_memory.c
> @@ -865,3 +865,30 @@ int efi_memory_init(void)
>  
>  	return 0;
>  }
> +
> +int efi_map_update_notify(phys_addr_t addr, phys_size_t size,
> +			  enum lmb_map_op op)
> +{
> +	u64 efi_addr;
> +	u64 pages;
> +	efi_status_t status;
> +
> +	efi_addr = (uintptr_t)map_sysmem(addr, 0);
> +	pages = efi_size_in_pages(size + (efi_addr & EFI_PAGE_MASK));
> +	efi_addr &= ~EFI_PAGE_MASK;
> +
> +	status = efi_add_memory_map_pg(efi_addr, pages,
> +				       op == LMB_MAP_OP_RESERVE ?
> +				       EFI_BOOT_SERVICES_DATA :
> +				       EFI_CONVENTIONAL_MEMORY,
> +				       false);
> +	if (status != EFI_SUCCESS) {
> +		log_err("LMB Map notify failure %lu\n",
> +			status & ~EFI_ERROR_MASK);
> +		return -1;
> +	}
> +	unmap_sysmem((void *)(uintptr_t)efi_addr);
> +
> +	return 0;
> +}
> +
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 7534a231c99..93fc1bea07c 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -426,37 +426,12 @@ long io_lmb_free(struct lmb *io_lmb, phys_addr_t base, phys_size_t size)
>  
>  static struct lmb lmb;
>  
> -static bool lmb_should_notify(u32 flags)
> -{
> -	return !lmb.test && !(flags & LMB_NONOTIFY) &&
> -		CONFIG_IS_ENABLED(EFI_LOADER);
> -}
> -
>  static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size,
>  				 enum lmb_map_op op, u32 flags)
>  {
> -	u64 efi_addr;
> -	u64 pages;
> -	efi_status_t status;
> -
> -	if (!lmb_should_notify(flags))
> -		return 0;
> -
> -	efi_addr = (uintptr_t)map_sysmem(addr, 0);
> -	pages = efi_size_in_pages(size + (efi_addr & EFI_PAGE_MASK));
> -	efi_addr &= ~EFI_PAGE_MASK;
> -
> -	status = efi_add_memory_map_pg(efi_addr, pages,
> -				       op == LMB_MAP_OP_RESERVE ?
> -				       EFI_BOOT_SERVICES_DATA :
> -				       EFI_CONVENTIONAL_MEMORY,
> -				       false);
> -	if (status != EFI_SUCCESS) {
> -		log_err("%s: LMB Map notify failure %lu\n", __func__,
> -			status & ~EFI_ERROR_MASK);
> -		return -1;
> -	}
> -	unmap_sysmem((void *)(uintptr_t)efi_addr);
> +	if (CONFIG_IS_ENABLED(EFI_LOADER) &&
> +	    !lmb.test && !(flags & LMB_NONOTIFY))
> +		return efi_map_update_notify(addr, size, op);
>  
>  	return 0;
>  }
> -- 
> 2.47.1
>