[Buildroot] [PATCH 1/2] boot/opensbi: Add support for including Linux payload

[Mark Corbin]

Hello Alistair

On 08/07/2019 22:53, Alistair Francis wrote:
> Signed-off-by: Alistair Francis <alistair.francis at wdc.com>
> ---
>  boot/opensbi/Config.in  |  9 +++++++++
>  boot/opensbi/opensbi.mk | 22 +++++++++++++++++++---
>  2 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/boot/opensbi/Config.in b/boot/opensbi/Config.in
> index 5f3cc13312..5cb8aace3e 100644
> --- a/boot/opensbi/Config.in
> +++ b/boot/opensbi/Config.in
> @@ -22,4 +22,13 @@ config BR2_TARGET_OPENSBI_PLAT
>  	  library libsbi.a is built. If a platform is specified then
>  	  the platform specific static library libplatsbi.a and firmware
>  	  examples are built.
> +
> +if BR2_TARGET_OPENSBI_PLAT != ""
> +config BR2_TARGET_OPENSBI_LINUX_PAYLOAD
> +	bool "Include Linux as OpenSBI Payload"
> +	depends on BR2_LINUX_KERNEL
> +	depends on BR2_LINUX_KERNEL_IMAGE
> +	help
> +	  Build OpenSBI with the Linux kernel as a Payload.
> +endif

I've tested this patch and it works okay with my HiFive Unleashed board,
but I'm still not a great fan of using a free-form string to specify the
OpenSBI platform (BR2_TARGET_OPENSBI_PLAT). I think that a) this always
increases the chance of errors due to typos and b) it doesn't let me see
what other options are available for my build without looking at the
OpenSBI source tree.

Perhaps a list of options would be better (similar to the current grub2
implementation)? There are only 5 OpenSBI platforms at the moment and I
don't think that it would be a big deal to add others as they appear (We
only actually need to add 3 platforms to support QEMU and HiFive
Unleashed - plus a default library only mode).

So with, for example, the following options:
    BR2_TARGET_OPENSBI_LIBRARY_ONLY
    BR2_TARGET_OPENSBI_QEMU_VIRT
    BR2_TARGET_OPENSBI_QEMU_SIFIVE_U
    BR2_TARGET_OPENSBI_HIFIVE_UNLEASHED

The configuration snippet above for BR2_TARGET_OPENSBI_LINUX_PAYLOAD
would become:

config BR2_TARGET_OPENSBI_LINUX_PAYLOAD
    bool "Include Linux as OpenSBI Payload"
    depends on BR2_LINUX_KERNEL
    depends on BR2_LINUX_KERNEL_IMAGE
    depends on !BR2_TARGET_OPENSBI_LIBRARY_ONLY

i.e. it loses the ' if BR2_TARGET_OPENSBI_PLAT != "" '

Thoughts?

Mark
>  endif
> diff --git a/boot/opensbi/opensbi.mk b/boot/opensbi/opensbi.mk
> index 45a3fc4859..2179d846e3 100644
> --- a/boot/opensbi/opensbi.mk
> +++ b/boot/opensbi/opensbi.mk
> @@ -19,18 +19,34 @@ ifneq ($(OPENSBI_PLAT),)
>  OPENSBI_MAKE_ENV += PLATFORM=$(OPENSBI_PLAT)
>  endif
>  
> +OPENSBI_LINUX_PAYLOAD = $(call qstrip,$(BR2_TARGET_OPENSBI_LINUX_PAYLOAD))
> +ifeq ($(OPENSBI_LINUX_PAYLOAD), y)
> +OPENSBI_DEPENDENCIES = linux
> +OPENSBI_MAKE_ENV += FW_PAYLOAD_PATH="$(BINARIES_DIR)/Image"
> +endif
> +
>  define OPENSBI_BUILD_CMDS
>  	$(TARGET_MAKE_ENV) $(OPENSBI_MAKE_ENV) $(MAKE) -C $(@D)
>  endef
>  
>  ifneq ($(OPENSBI_PLAT),)
>  OPENSBI_INSTALL_IMAGES = YES
> -define OPENSBI_INSTALL_IMAGES_CMDS
> -	$(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_jump.bin $(BINARIES_DIR)/fw_jump.bin
> +OPENSBI_INSTALL_IMAGES_CMDS_PLAT = \
> +	$(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_jump.bin $(BINARIES_DIR)/fw_jump.bin; \
>  	$(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_jump.elf $(BINARIES_DIR)/fw_jump.elf
> -endef
>  endif
>  
> +ifeq ($(OPENSBI_LINUX_PAYLOAD), y)
> +OPENSBI_INSTALL_IMAGES_CMDS_PAYLOAD = \
> +	$(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_payload.bin $(BINARIES_DIR)/fw_payload.bin; \
> +	$(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_payload.elf $(BINARIES_DIR)/fw_payload.elf
> +endif
> +
> +define OPENSBI_INSTALL_IMAGES_CMDS
> +	$(OPENSBI_INSTALL_IMAGES_CMDS_PLAT)
> +	$(OPENSBI_INSTALL_IMAGES_CMDS_PAYLOAD)
> +endef
> +
>  # libsbi.a is not a library meant to be linked in user-space code, but
>  # with bare metal code, which is why we don't install it in
>  # $(STAGING_DIR)/usr/lib

-- 
Mark Corbin
Embecosm Ltd.
https://www.embecosm.com