[v5,2/4] boot/opensbi: new package
diff mbox series

Message ID 20190320003706.1126-2-alistair.francis@wdc.com
State Changes Requested
Headers show
Series
  • [v5,1/4] board/qemu/riscv32-virt: Update linux config
Related show

Commit Message

Alistair Francis March 20, 2019, 12:38 a.m. UTC
OpenSBI is a much improved alternative to BBL (riscv-pk). Add OpenSBI
support to buildroot.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 DEVELOPERS              |  1 +
 boot/Config.in          |  1 +
 boot/opensbi/Config.in  | 25 +++++++++++++++++++++++++
 boot/opensbi/opensbi.mk | 37 +++++++++++++++++++++++++++++++++++++
 4 files changed, 64 insertions(+)
 create mode 100644 boot/opensbi/Config.in
 create mode 100644 boot/opensbi/opensbi.mk

Comments

Thomas Petazzoni March 20, 2019, 9:12 p.m. UTC | #1
Hello Francis,

You are respining the patches too fast, without answering some of the
questions, and therefore there's still an issue, see below.

On Wed, 20 Mar 2019 00:38:05 +0000
Alistair Francis <Alistair.Francis@wdc.com> wrote:

> +ifneq ($(OPENSBI_PLAT),)
> +OPENSBI_PLAT_INSTALL = \
> +	$(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
> +endif
> +
> +# Install libsbi.a in the host lib dir so it can be linked in the future.
> +define OPENSBI_INSTALL_IMAGES_CMDS
> +	$(OPENSBI_PLAT_INSTALL)
> +	$(INSTALL) -m 0644 -D $(@D)/build/lib/libsbi.a $(HOST_DIR)/usr/lib

I suppose that libsbi.a contains code cross-compiled for RISC-V, so
installing it in $(HOST_DIR) is wrong in this case. It should go in
$(STAGING_DIR).

The second question, which I asked in my review of your v4 is whether
this library is meant to be linked into bare-metal code (which is what
I would expect for something related to firmware) or to Linux
user-space applications. In the latter case, installing to
$(STAGING_DIR)/usr/lib is the right thing to do. In the former case
however, it should probably be installed elsewhere, maybe
$(STAGING_DIR)/usr/share/opensbi (but I'm not a FHS expert).

Also, when -D is used, the destination path should include the filename.

Best regards,

Thomas
Alistair Francis March 20, 2019, 9:28 p.m. UTC | #2
On Wed, Mar 20, 2019 at 2:12 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Francis,
>
> You are respining the patches too fast, without answering some of the
> questions, and therefore there's still an issue, see below.

Sorry, I didn't realise I didn't respond to a question.

>
> On Wed, 20 Mar 2019 00:38:05 +0000
> Alistair Francis <Alistair.Francis@wdc.com> wrote:
>
> > +ifneq ($(OPENSBI_PLAT),)
> > +OPENSBI_PLAT_INSTALL = \
> > +     $(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
> > +endif
> > +
> > +# Install libsbi.a in the host lib dir so it can be linked in the future.
> > +define OPENSBI_INSTALL_IMAGES_CMDS
> > +     $(OPENSBI_PLAT_INSTALL)
> > +     $(INSTALL) -m 0644 -D $(@D)/build/lib/libsbi.a $(HOST_DIR)/usr/lib
>
> I suppose that libsbi.a contains code cross-compiled for RISC-V, so
> installing it in $(HOST_DIR) is wrong in this case. It should go in
> $(STAGING_DIR).

Yes, it does contain cross compiled code. I have changed it to the STAGING_DIR.

>
> The second question, which I asked in my review of your v4 is whether
> this library is meant to be linked into bare-metal code (which is what

Sorry, I missed your question. It is meant to be linked into bare metal code.

> I would expect for something related to firmware) or to Linux
> user-space applications. In the latter case, installing to
> $(STAGING_DIR)/usr/lib is the right thing to do. In the former case
> however, it should probably be installed elsewhere, maybe
> $(STAGING_DIR)/usr/share/opensbi (but I'm not a FHS expert).

$(STAGING_DIR)/usr/share/opensbi looks good to me. I will use that.

>
> Also, when -D is used, the destination path should include the filename.

Ok, fixed.

Alistair

>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Thomas Petazzoni March 20, 2019, 9:44 p.m. UTC | #3
Hello,

On Wed, 20 Mar 2019 14:28:59 -0700
Alistair Francis <alistair23@gmail.com> wrote:

> > I would expect for something related to firmware) or to Linux
> > user-space applications. In the latter case, installing to
> > $(STAGING_DIR)/usr/lib is the right thing to do. In the former case
> > however, it should probably be installed elsewhere, maybe
> > $(STAGING_DIR)/usr/share/opensbi (but I'm not a FHS expert).  
> 
> $(STAGING_DIR)/usr/share/opensbi looks good to me. I will use that.

Then it should go to a OPENSBI_INSTALL_STAGING_CMDS variable, not in
OPENSBI_INSTALL_IMAGES_CMDS.

Thanks!

Thomas
Mark Corbin March 21, 2019, 4:36 p.m. UTC | #4
Hello Thomas

On 20/03/2019 21:12, Thomas Petazzoni wrote:
> Hello Francis,
>
> You are respining the patches too fast, without answering some of the
> questions, and therefore there's still an issue, see below.

Just a heads-up that things have got a bit out of sync for RISC-V
architecture on the master branch...

a) commit 'allow BR2_LINUX_KERNEL_IMAGE on RISC-V'
(a3a4d4d4d307fd21f19ae43e77ac21b85adad7f2) is causing riscv builds to
fail for the existing qemu defconfigs because OpenSBI hasn't been added
yet and riscv-pk expects a 'vmlinux' file.

b) commit 'configs/qemu_riscv32_virt: upgrade to 4.20 kernel'
(e92d5624236e47e1abe44f2ace24fceb1882c0a5) results in a kernel that
won't boot with QEMU due to lack of OpenSBI and/or required kernel
configuration updates.

Might be worth backing these out if the other updates are going to take
a while?

Regards

Mark
Thomas Petazzoni March 21, 2019, 4:57 p.m. UTC | #5
Hello Mark,

Thanks for the heads-up!

On Thu, 21 Mar 2019 16:36:22 +0000
Mark Corbin <mark.corbin@embecosm.com> wrote:

> a) commit 'allow BR2_LINUX_KERNEL_IMAGE on RISC-V'
> (a3a4d4d4d307fd21f19ae43e77ac21b85adad7f2) is causing riscv builds to
> fail for the existing qemu defconfigs because OpenSBI hasn't been added
> yet and riscv-pk expects a 'vmlinux' file.

This one should be fixed by adding:

BR2_LINUX_KERNEL_VMLINUX=y

in the defconfig.

> b) commit 'configs/qemu_riscv32_virt: upgrade to 4.20 kernel'
> (e92d5624236e47e1abe44f2ace24fceb1882c0a5) results in a kernel that
> won't boot with QEMU due to lack of OpenSBI and/or required kernel
> configuration updates.

Which configuration updates are needed ? The ones that Alistair was
pushing ? If they were need for the 4.20 bump to work, then they should
have been part of the same commit :-/

Anyway, Alistair has been respinning his series very frequently, so I
assume we can get the remainder of the work merged very soon.

Best regards,

Thomas
Alistair Francis March 23, 2019, 6:44 a.m. UTC | #6
On Thu, Mar 21, 2019 at 6:57 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Mark,
>
> Thanks for the heads-up!

I am on holidays for the next few days so I won't be able to send any
more patches until next week.

>
> On Thu, 21 Mar 2019 16:36:22 +0000
> Mark Corbin <mark.corbin@embecosm.com> wrote:
>
> > a) commit 'allow BR2_LINUX_KERNEL_IMAGE on RISC-V'
> > (a3a4d4d4d307fd21f19ae43e77ac21b85adad7f2) is causing riscv builds to
> > fail for the existing qemu defconfigs because OpenSBI hasn't been added
> > yet and riscv-pk expects a 'vmlinux' file.
>
> This one should be fixed by adding:
>
> BR2_LINUX_KERNEL_VMLINUX=y
>
> in the defconfig.

I didn't realise that enabling Image would change the default.

>
> > b) commit 'configs/qemu_riscv32_virt: upgrade to 4.20 kernel'
> > (e92d5624236e47e1abe44f2ace24fceb1882c0a5) results in a kernel that
> > won't boot with QEMU due to lack of OpenSBI and/or required kernel
> > configuration updates.
>
> Which configuration updates are needed ? The ones that Alistair was
> pushing ? If they were need for the 4.20 bump to work, then they should
> have been part of the same commit :-/

Sorry about this. I tested them with a custom config that wasn't
commuted in the same patch.

>
> Anyway, Alistair has been respinning his series very frequently, so I
> assume we can get the remainder of the work merged very soon.

Are there any comments on the latest series? Or can they be merged to fix this.

Alistair

>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Patch
diff mbox series

diff --git a/DEVELOPERS b/DEVELOPERS
index 3e0ac08e11..91eda42949 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -122,6 +122,7 @@  F:	package/kvazaar/
 F:	package/v4l2loopback/
 
 N:	Alistair Francis <alistair@alistair23.me>
+F:	boot/opensbi/
 F:	package/xen/
 
 N:	Alvaro G. M <alvaro.gamez@hazent.com>
diff --git a/boot/Config.in b/boot/Config.in
index 74481e7545..97bd3de6e9 100644
--- a/boot/Config.in
+++ b/boot/Config.in
@@ -14,6 +14,7 @@  source "boot/lpc32xxcdl/Config.in"
 source "boot/mv-ddr-marvell/Config.in"
 source "boot/mxs-bootlets/Config.in"
 source "boot/optee-os/Config.in"
+source "boot/opensbi/Config.in"
 source "boot/riscv-pk/Config.in"
 source "boot/s500-bootloader/Config.in"
 source "boot/shim/Config.in"
diff --git a/boot/opensbi/Config.in b/boot/opensbi/Config.in
new file mode 100644
index 0000000000..5f3cc13312
--- /dev/null
+++ b/boot/opensbi/Config.in
@@ -0,0 +1,25 @@ 
+config BR2_TARGET_OPENSBI
+	bool "opensbi"
+	depends on BR2_riscv
+	help
+	  OpenSBI aims to provide an open-source and extensible
+	  implementation of the RISC-V SBI specification for a platform
+	  specific firmware (M-mode) and a general purpose OS,
+	  hypervisor or bootloader (S-mode or HS-mode). OpenSBI
+	  implementation can be easily extended by RISC-V platform or
+	  System-on-Chip vendors to fit a particular hadware
+	  configuration.
+
+	  https://github.com/riscv/opensbi.git
+
+if BR2_TARGET_OPENSBI
+config BR2_TARGET_OPENSBI_PLAT
+	string "OpenSBI Platform"
+	default ""
+	help
+	  Specifies the OpenSBI platform to build. If no platform is
+	  specified only the OpenSBI platform independent static
+	  library libsbi.a is built. If a platform is specified then
+	  the platform specific static library libplatsbi.a and firmware
+	  examples are built.
+endif
diff --git a/boot/opensbi/opensbi.mk b/boot/opensbi/opensbi.mk
new file mode 100644
index 0000000000..341e2dcb19
--- /dev/null
+++ b/boot/opensbi/opensbi.mk
@@ -0,0 +1,37 @@ 
+################################################################################
+#
+# opensbi
+#
+################################################################################
+
+OPENSBI_VERSION = v0.3
+OPENSBI_SITE = $(call github,riscv,opensbi,$(OPENSBI_VERSION))
+OPENSBI_LICENSE = BSD-2-Clause
+OPENSBI_LICENSE_FILES = COPYING.BSD
+OPENSBI_INSTALL_IMAGES = YES
+
+OPENSBI_MAKE_ENV = \
+	CROSS_COMPILE=$(TARGET_CROSS)
+
+OPENSBI_PLAT = $(call qstrip,$(BR2_TARGET_OPENSBI_PLAT))
+ifneq ($(OPENSBI_PLAT),)
+OPENSBI_MAKE_ENV += PLATFORM=$(OPENSBI_PLAT)
+endif
+
+define OPENSBI_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(OPENSBI_MAKE_ENV) $(MAKE) -C $(@D)
+endef
+
+ifneq ($(OPENSBI_PLAT),)
+OPENSBI_PLAT_INSTALL = \
+	$(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
+endif
+
+# Install libsbi.a in the host lib dir so it can be linked in the future.
+define OPENSBI_INSTALL_IMAGES_CMDS
+	$(OPENSBI_PLAT_INSTALL)
+	$(INSTALL) -m 0644 -D $(@D)/build/lib/libsbi.a $(HOST_DIR)/usr/lib
+endef
+
+$(eval $(generic-package))