[PATCH] lib: lmb: conditionally include EFI loader notification

[Heinrich Schuchardt]

On 2/15/25 04:11, 1425075683 at qq.com wrote:
> From: Liya Huang <1425075683 at qq.com>
>
> Compiling with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y
> without CONFIG_EFI_LOADER enabled will result in an
> undefined reference to 'efi_add_memory_map_pg'.

Hello Liva,

Thank you for reporting the issue. It is reproducible with
qemu_arm64_defconfig.

We try to avoid #ifdef as far as possible.

I think the listener should be implemented on the EFI side of the code
to have a clear separation of duties.

I created a git branch implementing this idea:
https://source.denx.de/u-boot/custodians/u-boot-efi/-/commits/lmb

Best regards

Heinrich

>
> The make command is as follows:
>      export CROSS_COMPILE=arm-none-eabi- ARCH=arm
>      make stm32h750-art-pi_defconfig
>      -> General setup -> Optimization level -> Optimize for debugging
>      make -j 12
> The error message is as follows:
>      arm-none-eabi-ld.bfd: lib/lmb.o: in function `lmb_map_update_notify':
>      u-boot/lib/lmb.c:458: undefined reference to `efi_add_memory_map_pg'
>      make: *** [Makefile:1824：u-boot] Error 1
>
> Using the CONFIG_CC_OPTIMIZE_FOR_SIZE configuration
> will not cause an error.My guess is that the compiler
> recognizes that this function will not be executed and optimizes it.
>
> As you can see from the lmb_should_notify function,
> EFI_LOADER will exit without calling efi_add_memory_map_pg().
> So splitting with the CONFIG_EFI_LOADER macro would be a fix.
>
>      ```c
>      static bool lmb_should_notify(u32 flags)
>      {
>          return !lmb.test && !(flags & LMB_NONOTIFY) &&
>              CONFIG_IS_ENABLED(EFI_LOADER);
>      }
>      static int lmb_map_update_notify()
>      {
>      #if defined(CONFIG_EFI_LOADER)
>          if (!lmb_should_notify(flags))
>              return 0;
>      #endif
>      }
>      ```
>
> Signed-off-by: Liya Huang <1425075683 at qq.com>
> ---
>
>   lib/lmb.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 7ca44591e1d..28d31308a79 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -430,27 +430,30 @@ long io_lmb_free(struct lmb *io_lmb, phys_addr_t base, phys_size_t size)
>
>   static struct lmb lmb;
>
> +#if defined(CONFIG_EFI_LOADER)
>   static bool lmb_should_notify(u32 flags)
>   {
>   	return !lmb.test && !(flags & LMB_NONOTIFY) &&
>   		CONFIG_IS_ENABLED(EFI_LOADER);
>   }
> +#endif
>
>   static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op,
>   				 u32 flags)
>   {
> -	u64 efi_addr;
> -	u64 pages;
> -	efi_status_t status;
> -
>   	if (op != MAP_OP_RESERVE && op != MAP_OP_FREE && op != MAP_OP_ADD) {
>   		log_err("Invalid map update op received (%d)\n", op);
>   		return -1;
>   	}
>
> +#if defined(CONFIG_EFI_LOADER)
>   	if (!lmb_should_notify(flags))
>   		return 0;
>
> +	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;
> @@ -466,6 +469,7 @@ static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op,
>   		return -1;
>   	}
>   	unmap_sysmem((void *)(uintptr_t)efi_addr);
> +#endif
>
>   	return 0;
>   }