| Message ID | 20190722204447.7935-1-alistair.francis@wdc.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/2] boot/opensbi: Add support for including Linux payload | expand |
Allistair, All, On 2019-07-22 13:44 -0700, Alistair Francis spake thusly: > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> [--SNIP--] > diff --git a/boot/opensbi/opensbi.mk b/boot/opensbi/opensbi.mk > index 83552a5442..14657cac06 100644 > --- a/boot/opensbi/opensbi.mk > +++ b/boot/opensbi/opensbi.mk > @@ -19,19 +19,35 @@ 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 In Kconfig, OPENSBI_LINUX_PAYLOAD is conditional to OPENSBI_PLAT != "" so I would expect to see the same layout in the .mk (see below for what I mean). > define OPENSBI_BUILD_CMDS > $(TARGET_MAKE_ENV) $(OPENSBI_MAKE_ENV) $(MAKE) -C $(@D) > endef > > ifneq ($(OPENSBI_PLAT),) > OPENSBI_INSTALL_IMAGES = YES > +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; \ > + $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.bin $(BINARIES_DIR)/fw_dynamic.bin; \ > + $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.elf $(BINARIES_DIR)/fw_dynamic.elf; > +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 Why can't we have something like (untested): OPENSBI_FW_DIR = $(@D)/build/platform/$(OPENSBI_PLAT)/firmware ifneq ($(OPENSBI_PLAT),) OPENSBI_FW_FILES = fw_jump.bin fw_jump.elf fw_dynamic.bin fw_dynamic.elf ifeq ($(OPENSBI_LINUX_PAYLOAD),y) OPENSBI_FW_FILES += fw_jump.bin fw_jump.elf fw_dynamic.bin fw_dynamic.elf endif endif define OPENSBI_INSTALL_IMAGES_CMDS $(foreach fw,$(OPENSBI_FW_FILES), \ $(INSTALL) -m 0644 -D $(OPENSBI_FW_DIR)/$(fw) $(BINARIES_DIR)/$(fw) ) endef (notice also how OPENSBI_LINUX_PAYLOAD is inside the OPENSBI_PLAT != "" block, like it is in Config.in.) Regards, Yann E. MORIN. > define OPENSBI_INSTALL_IMAGES_CMDS > - $(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 > - $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.bin $(BINARIES_DIR)/fw_dynamic.bin > - $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.elf $(BINARIES_DIR)/fw_dynamic.elf > + $(OPENSBI_INSTALL_IMAGES_CMDS_PLAT) > + $(OPENSBI_INSTALL_IMAGES_CMDS_PAYLOAD) > endef > -endif > > # 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 > -- > 2.22.0 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Hello, On Mon, 22 Jul 2019 13:44:46 -0700 Alistair Francis <alistair.francis@wdc.com> wrote: > +if BR2_TARGET_OPENSBI_PLAT != "" I move this if to a simple "depends on" under the BR2_TARGET_OPENSBI_LINUX_PAYLOAD optio. > +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 This option doesn't exist anywhere, so I dropped this depends on. > +OPENSBI_LINUX_PAYLOAD = $(call qstrip,$(BR2_TARGET_OPENSBI_LINUX_PAYLOAD)) qstrip is only useful for string options. For boolean options, it's useless, so I dropped this and simply used BR2_TARGET_OPENSBI_LINUX_PAYLOAD everywhere. > +ifeq ($(OPENSBI_LINUX_PAYLOAD), y) No space before "y". > +OPENSBI_DEPENDENCIES = linux += for this addition, so that we don't mistakenly overwrite any previous dependency that might have been added. > +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 > +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; \ > + $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.bin $(BINARIES_DIR)/fw_dynamic.bin; \ > + $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.elf $(BINARIES_DIR)/fw_dynamic.elf; > +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 > - $(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 > - $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.bin $(BINARIES_DIR)/fw_dynamic.bin > - $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.elf $(BINARIES_DIR)/fw_dynamic.elf > + $(OPENSBI_INSTALL_IMAGES_CMDS_PLAT) > + $(OPENSBI_INSTALL_IMAGES_CMDS_PAYLOAD) As suggested by Yann, I've refactored all of that with a loop over a OPENSBI_FW_IMAGES variable that contains jump, dynamic and optionally payload. I fixed up these different aspects, and merged the patch. See the final commit at: https://git.buildroot.org/buildroot/commit/?id=9b5b7165deb91bad3cdb1ea0dd8f940eff75f3a9 Thanks! Thomas
On Sat, Aug 3, 2019 at 1:37 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > On Mon, 22 Jul 2019 13:44:46 -0700 > Alistair Francis <alistair.francis@wdc.com> wrote: > > > +if BR2_TARGET_OPENSBI_PLAT != "" > > I move this if to a simple "depends on" under the > BR2_TARGET_OPENSBI_LINUX_PAYLOAD optio. > > > +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 > > This option doesn't exist anywhere, so I dropped this depends on. > > > +OPENSBI_LINUX_PAYLOAD = $(call qstrip,$(BR2_TARGET_OPENSBI_LINUX_PAYLOAD)) > > qstrip is only useful for string options. For boolean options, it's > useless, so I dropped this and simply used > BR2_TARGET_OPENSBI_LINUX_PAYLOAD everywhere. > > > +ifeq ($(OPENSBI_LINUX_PAYLOAD), y) > > No space before "y". > > > +OPENSBI_DEPENDENCIES = linux > > += for this addition, so that we don't mistakenly overwrite any > previous dependency that might have been added. > > > +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 > > +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; \ > > + $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.bin $(BINARIES_DIR)/fw_dynamic.bin; \ > > + $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.elf $(BINARIES_DIR)/fw_dynamic.elf; > > +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 > > - $(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 > > - $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.bin $(BINARIES_DIR)/fw_dynamic.bin > > - $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.elf $(BINARIES_DIR)/fw_dynamic.elf > > + $(OPENSBI_INSTALL_IMAGES_CMDS_PLAT) > > + $(OPENSBI_INSTALL_IMAGES_CMDS_PAYLOAD) > > As suggested by Yann, I've refactored all of that with a loop over a > OPENSBI_FW_IMAGES variable that contains jump, dynamic and optionally > payload. > > I fixed up these different aspects, and merged the patch. See the final > commit at: > > https://git.buildroot.org/buildroot/commit/?id=9b5b7165deb91bad3cdb1ea0dd8f940eff75f3a9 Thanks for that Thomas. Sorry I didn't update it myself, I was travelling. Alistair > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
diff --git a/boot/opensbi/Config.in b/boot/opensbi/Config.in index 5f3cc13312..686aac8798 100644 --- a/boot/opensbi/Config.in +++ b/boot/opensbi/Config.in @@ -22,4 +22,14 @@ 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 + depends on !BR2_TARGET_OPENSBI_LIBRARY_ONLY + help + Build OpenSBI with the Linux kernel as a Payload. +endif endif diff --git a/boot/opensbi/opensbi.mk b/boot/opensbi/opensbi.mk index 83552a5442..14657cac06 100644 --- a/boot/opensbi/opensbi.mk +++ b/boot/opensbi/opensbi.mk @@ -19,19 +19,35 @@ 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 +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; \ + $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.bin $(BINARIES_DIR)/fw_dynamic.bin; \ + $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.elf $(BINARIES_DIR)/fw_dynamic.elf; +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 - $(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 - $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.bin $(BINARIES_DIR)/fw_dynamic.bin - $(INSTALL) -m 0644 -D $(@D)/build/platform/$(OPENSBI_PLAT)/firmware/fw_dynamic.elf $(BINARIES_DIR)/fw_dynamic.elf + $(OPENSBI_INSTALL_IMAGES_CMDS_PLAT) + $(OPENSBI_INSTALL_IMAGES_CMDS_PAYLOAD) endef -endif # 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
Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- boot/opensbi/Config.in | 10 ++++++++++ boot/opensbi/opensbi.mk | 26 +++++++++++++++++++++----- 2 files changed, 31 insertions(+), 5 deletions(-)