Re: [PATCH] Makefile: Separate libsbi objects for each platform

[E Shattow <e@freeshell.de>]

Hi Samuel,

On 9/3/24 13:53, Samuel Holland wrote:
> Since commit 22f38ee6c658 ("lib: sbi_ecall: Add Kconfig option for each
> extension"), the libsbi object file contents depend on the Kconfig
> configuration, so these files may be different across platforms. As a
> result, each platform should get a separate copy of these object files,
> corresponding to that platform's configuration.
> 
> This change also allows building for multiple platforms in parallel.
> 
> Fixes: 22f38ee6c658 ("lib: sbi_ecall: Add Kconfig option for each extension")
> Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
> ---
> Note that Kconfig is not used for the generic (no PLATFORM set) libsbi
> build. But this is actually equivalent to all Kconfig options being
> disabled. As a result, since the referenced commit, the generic libsbi
> only supports the base SBI extension and none of the others. I don't
> know how we want to fix this -- does anyone even use libsbi anymore?
> 
>  Makefile | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index e5a0f19e..ccb2e138 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -264,11 +264,13 @@ include $(libsbiutils-object-mks)
>  include $(firmware-object-mks)
>  
>  # Setup list of objects
> -libsbi-objs-path-y=$(foreach obj,$(libsbi-objs-y),$(build_dir)/lib/sbi/$(obj))
>  ifdef PLATFORM
> +libsbi-objs-path-y=$(foreach obj,$(libsbi-objs-y),$(platform_build_dir)/lib/sbi/$(obj))
>  libsbiutils-objs-path-y=$(foreach obj,$(libsbiutils-objs-y),$(platform_build_dir)/lib/utils/$(obj))
>  platform-objs-path-y=$(foreach obj,$(platform-objs-y),$(platform_build_dir)/$(obj))
>  firmware-bins-path-y=$(foreach bin,$(firmware-bins-y),$(platform_build_dir)/firmware/$(bin))
> +else
> +libsbi-objs-path-y=$(foreach obj,$(libsbi-objs-y),$(build_dir)/lib/sbi/$(obj))
>  endif
>  firmware-elfs-path-y=$(firmware-bins-path-y:.bin=.elf)
>  firmware-objs-path-y=$(firmware-bins-path-y:.bin=.o)
> @@ -504,9 +506,10 @@ compile_gen_dep = $(CMD_PREFIX)mkdir -p `dirname $(1)`; \
>  	     echo " GEN-DEP   $(subst $(build_dir)/,,$(1))"; \
>  	     echo "$(1:.dep=$(2)): $(3)" >> $(1)
>  
> -targets-y  = $(build_dir)/lib/libsbi.a
>  ifdef PLATFORM
>  targets-y += $(platform_build_dir)/lib/libplatsbi.a
> +else
> +targets-y  = $(build_dir)/lib/libsbi.a
>  endif
>  targets-y += $(firmware-bins-path-y)
>  
> @@ -565,6 +568,11 @@ $(platform_build_dir)/%.dep: $(platform_src_dir)/%.c $(KCONFIG_AUTOHEADER)
>  $(platform_build_dir)/%.o: $(platform_src_dir)/%.c $(KCONFIG_AUTOHEADER)
>  	$(call compile_cc,$@,$<)
>  
> +ifeq ($(BUILD_INFO),y)
> +$(platform_build_dir)/lib/sbi/sbi_init.o: $(libsbi_dir)/sbi_init.c FORCE
> +	$(call compile_cc,$@,$<)
> +endif
> +
>  $(platform_build_dir)/%.dep: $(platform_src_dir)/%.S
>  	$(call compile_as_dep,$@,$<)
>  

Thanks for your reply [1] highlighting this patch. I recently
encountered the issue, here are the detailed steps to reproduce:

`git clone --branch v2025.10-rc4
https://source.denx.de/u-boot/u-boot.git /path/u-boot.git`
`make O=/path/u-boot -C /path/u-boot.git starfive_visionfive2_defconfig`
`make O=/path/u-boot -C /path/u-boot.git`

Image 'itb' is missing external blobs and is non-functional: opensbi
/binman/itb/fit/images/opensbi/opensbi (fw_dynamic.bin):
   See the documentation for your board. The OpenSBI git repo is at
   https://github.com/riscv/opensbi.git
   You may need to build fw_dynamic.bin first and re-build u-boot with
   OPENSBI=/path/to/fw_dynamic.bin
Image 'itb' has faked external blobs and is non-functional: fw_dynamic.bin

`git clone --branch v1.7
https://github.com/riscv-software-src/opensbi.git /path/opensbi.git`
`make O=/path/opensbi -C /path/opensbi.git`

# No such file fw_dynamic.bin, I am missing PLATFORM, try again:

`make O=/path/opensbi -C /path/opensbi.git PLATFORM=generic`
`make O=/path/u-boot -C /path/u-boot.git
OPENSBI=/path/opensbi/platform/generic/firmware/fw_payload.bin`

# U-Boot builds successfully however the result is defective in use

StarFive # reset
resetting ...
System reset not supported on this platform
### ERROR ### Please RESET the board ###

# Apply your patch, try again

`curl https://patchwork.ozlabs.org/series/422221/mbox/ | git -C
/path/opensbi.git am`
`make O=/path/opensbi -C /path/opensbi.git PLATFORM=generic`

# Result is no-op, must clean for patch to be effective

`make O=/path/opensbi -C /path/opensbi.git clean`
`make O=/path/opensbi -C /path/opensbi.git`
`make O=/path/opensbi -C /path/opensbi.git PLATFORM=generic`
`make O=/path/u-boot -C /path/u-boot.git
OPENSBI=/path/opensbi/platform/generic/firmware/fw_payload.bin`

# U-Boot builds successfully and result is valid

StarFive # reset
resetting ...
U-Boot SPL 2025.10-rc4...

1:
https://lore.kernel.org/opensbi/0d6bf7c7-7605-4ad6-a35c-54efc5230fb8@sifive.com/

I'm thinking this should get a mention in the documentation in addition
to the patch, if we're v1.7 and this is accepted for v1.8

"Releases prior to v1.8 would <technical description here>... when
building OpenSBI you must clean your build directory between v1.8 and
any earlier version for the new behavior to be effective."

With that, and without regard to uses other than PLATFORM=generic,

Tested-by: E Shattow <e@freeshell.de>

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi