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

[E Shattow <e@freeshell.de>]

On 9/17/25 15:52, Samuel Holland wrote:
> Hi E,
> 
> On 2025-09-17 2:50 PM, E Shattow wrote:
>> 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
> 
> Thanks for testing! Yes, unfortunately this is still a problem that changes to
> several files (especially objects.mk files) do not get picked up by the build
> system, and it's a larger job to untangle. So currently it is best to assume you
> need to clean the build directory after moving commits or applying a patch that
> touches non-C files.
> 
> Regards,
> Samuel
> 
>> `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>
> 

Yes, thank you. Without the patch 100% it makes that defective chimera.
Postscript n.b. I'd wrote 'fw_payload.bin' in all cases what I meant was
'fw_dynamic.bin' for use with U-Boot. T-by still applies, it was a
transcription error writing to e-mail.  -E

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