[U-Boot] [PATCH v2 05/11] spl: support booting via RISC-V OpenSBI

Auer, Lukas lukas.auer at aisec.fraunhofer.de
Mon Jul 29 15:51:30 UTC 2019


Hi Anup,

On Mon, 2019-07-29 at 14:02 +0530, Anup Patel wrote:
> On Sun, Jul 28, 2019 at 9:55 PM Lukas Auer
> <lukas.auer at aisec.fraunhofer.de> wrote:
> > RISC-V OpenSBI is an open-source implementation of the RISC-V Supervisor
> > Binary Interface (SBI) specification. It is required by Linux and U-Boot
> > running in supervisor mode. This patch adds support for booting via the
> > OpenSBI FW_DYNAMIC firmware.
> > 
> > In this configuration, U-Boot SPL starts in machine mode. After loading
> > OpenSBI and U-Boot proper, it will start OpenSBI. All necessary
> > parameters are generated by U-Boot SPL and passed to OpenSBI. U-Boot
> > proper is started in supervisor mode by OpenSBI. Support for OpenSBI is
> > enabled with CONFIG_SPL_OPENSBI. An additional configuration entry,
> > CONFIG_SPL_OPENSBI_LOAD_ADDR, is used to specify the load address of the
> > OpenSBI firmware binary. It is not used directly in U-Boot and instead
> > is intended to make the value available to scripts such as FIT
> > configuration generators.
> > 
> > The header file include/opensbi.h is based on header files from the
> > OpenSBI project. They are recent, as of commit bae54f764570 ("firmware:
> > Add fw_dynamic firmware").
> 
> Instead of OpenSBI commit, may be mention that we need OpenSBI v0.4
> or higher.
> 

If there are no other comments on the series I would prefer to keep the
commit message as is, if that is ok for you.
Adding the minimum OpenSBI version is a good idea, however. I can send
a follow-up patch to this series to add the minimum version to the QEMU
board documentation.

> > Signed-off-by: Lukas Auer <lukas.auer at aisec.fraunhofer.de>
> > Reviewed-by: Bin Meng <bmeng.cn at gmail.com>
> > Tested-by: Bin Meng <bmeng.cn at gmail.com>
> > ---
> > 
> > Changes in v2: None
> > 
> >  common/image.c           |  1 +
> >  common/spl/Kconfig       | 17 ++++++++
> >  common/spl/Makefile      |  1 +
> >  common/spl/spl.c         |  6 +++
> >  common/spl/spl_opensbi.c | 85 ++++++++++++++++++++++++++++++++++++++++
> >  include/image.h          |  1 +
> >  include/opensbi.h        | 40 +++++++++++++++++++
> >  include/spl.h            |  5 +++
> >  8 files changed, 156 insertions(+)
> >  create mode 100644 common/spl/spl_opensbi.c
> >  create mode 100644 include/opensbi.h
> > 
> > diff --git a/common/image.c b/common/image.c
> > index 9f9538fac2..7c7353a989 100644
> > --- a/common/image.c
> > +++ b/common/image.c
> > @@ -125,6 +125,7 @@ static const table_entry_t uimage_os[] = {
> >  #if defined(CONFIG_BOOTM_OPENRTOS) || defined(USE_HOSTCC)
> >         {       IH_OS_OPENRTOS, "openrtos",     "OpenRTOS",             },
> >  #endif
> > +       {       IH_OS_OPENSBI,  "opensbi",      "RISC-V OpenSBI",       },
> > 
> >         {       -1,             "",             "",                     },
> >  };
> > diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> > index 5d6da5db89..939c8517cd 100644
> > --- a/common/spl/Kconfig
> > +++ b/common/spl/Kconfig
> > @@ -1126,6 +1126,23 @@ config SPL_OPTEE
> >           OP-TEE is an open source Trusted OS  which is loaded by SPL.
> >           More detail at: https://github.com/OP-TEE/optee_os
> > 
> > +config SPL_OPENSBI
> > +       bool "Support RISC-V OpenSBI"
> > +       depends on RISCV && SPL_RISCV_MMODE && RISCV_SMODE
> > +       help
> > +         OpenSBI is an open-source implementation of the RISC-V Supervisor Binary
> > +         Interface (SBI) specification. U-Boot supports the OpenSBI FW_DYNAMIC
> > +         firmware. It is loaded and started by U-Boot SPL.
> > +
> > +         More details are available at https://github.com/riscv/opensbi and
> > +         https://github.com/riscv/riscv-sbi-doc
> > +
> > +config SPL_OPENSBI_LOAD_ADDR
> > +       hex "OpenSBI load address"
> > +       depends on SPL_OPENSBI
> > +       help
> > +         Load address of the OpenSBI binary.
> > +
> >  config TPL
> >         bool
> >         depends on SUPPORT_TPL
> > diff --git a/common/spl/Makefile b/common/spl/Makefile
> > index d28de692dd..5ce6f4ae48 100644
> > --- a/common/spl/Makefile
> > +++ b/common/spl/Makefile
> > @@ -22,6 +22,7 @@ obj-$(CONFIG_$(SPL_TPL_)NET_SUPPORT) += spl_net.o
> >  obj-$(CONFIG_$(SPL_TPL_)MMC_SUPPORT) += spl_mmc.o
> >  obj-$(CONFIG_$(SPL_TPL_)ATF) += spl_atf.o
> >  obj-$(CONFIG_$(SPL_TPL_)OPTEE) += spl_optee.o
> > +obj-$(CONFIG_$(SPL_TPL_)OPENSBI) += spl_opensbi.o
> >  obj-$(CONFIG_$(SPL_TPL_)USB_STORAGE) += spl_usb.o
> >  obj-$(CONFIG_$(SPL_TPL_)FS_FAT) += spl_fat.o
> >  obj-$(CONFIG_$(SPL_TPL_)FS_EXT4) += spl_ext.o
> > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > index d5e3f680f4..1ed4741bdc 100644
> > --- a/common/spl/spl.c
> > +++ b/common/spl/spl.c
> > @@ -659,6 +659,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> >                                 (void *)spl_image.entry_point);
> >                 break;
> >  #endif
> > +#if CONFIG_IS_ENABLED(OPENSBI)
> 
> Should this be "CONFIG_IS_ENABLED(SPL_OPENSBI)" ?
> 
> Am I missing something here ?
> 

The CONFIG_IS_ENABLED(option) macro automatically handles the SPL_*
symbols. On SPL builds, it evaluates to one if CONFIG_SPL_option is
defined. On normal builds, it evaluates to one if CONFIG_option is
defined.

Thanks,
Lukas

> > +       case IH_OS_OPENSBI:
> > +               debug("Jumping to U-Boot via RISC-V OpenSBI\n");
> > +               spl_invoke_opensbi(&spl_image);
> > +               break;
> > +#endif
> >  #ifdef CONFIG_SPL_OS_BOOT
> >         case IH_OS_LINUX:
> >                 debug("Jumping to Linux\n");
> > diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
> > new file mode 100644
> > index 0000000000..a6b4480ed2
> > --- /dev/null
> > +++ b/common/spl/spl_opensbi.c
> > @@ -0,0 +1,85 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2019 Fraunhofer AISEC,
> > + * Lukas Auer <lukas.auer at aisec.fraunhofer.de>
> > + *
> > + * Based on common/spl/spl_atf.c
> > + */
> > +#include <common.h>
> > +#include <errno.h>
> > +#include <spl.h>
> > +#include <asm/smp.h>
> > +#include <opensbi.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +struct fw_dynamic_info opensbi_info;
> > +
> > +static int spl_opensbi_find_uboot_node(void *blob, int *uboot_node)
> > +{
> > +       int fit_images_node, node;
> > +       const char *fit_os;
> > +
> > +       fit_images_node = fdt_path_offset(blob, "/fit-images");
> > +       if (fit_images_node < 0)
> > +               return -ENODEV;
> > +
> > +       fdt_for_each_subnode(node, blob, fit_images_node) {
> > +               fit_os = fdt_getprop(blob, node, FIT_OS_PROP, NULL);
> > +               if (!fit_os)
> > +                       continue;
> > +
> > +               if (genimg_get_os_id(fit_os) == IH_OS_U_BOOT) {
> > +                       *uboot_node = node;
> > +                       return 0;
> > +               }
> > +       }
> > +
> > +       return -ENODEV;
> > +}
> > +
> > +void spl_invoke_opensbi(struct spl_image_info *spl_image)
> > +{
> > +       int ret, uboot_node;
> > +       ulong uboot_entry;
> > +       void (*opensbi_entry)(ulong hartid, ulong dtb, ulong info);
> > +
> > +       if (!spl_image->fdt_addr) {
> > +               pr_err("No device tree specified in SPL image\n");
> > +               hang();
> > +       }
> > +
> > +       /* Find U-Boot image in /fit-images */
> > +       ret = spl_opensbi_find_uboot_node(spl_image->fdt_addr, &uboot_node);
> > +       if (ret) {
> > +               pr_err("Can't find U-Boot node, %d", ret);
> > +               hang();
> > +       }
> > +
> > +       /* Get U-Boot entry point */
> > +       uboot_entry = fdt_getprop_u32(spl_image->fdt_addr, uboot_node,
> > +                                     "entry-point");
> > +       if (uboot_entry == FDT_ERROR)
> > +               uboot_entry = fdt_getprop_u32(spl_image->fdt_addr, uboot_node,
> > +                                             "load-addr");
> > +
> > +       /* Prepare obensbi_info object */
> > +       opensbi_info.magic = FW_DYNAMIC_INFO_MAGIC_VALUE;
> > +       opensbi_info.version = FW_DYNAMIC_INFO_VERSION;
> > +       opensbi_info.next_addr = uboot_entry;
> > +       opensbi_info.next_mode = FW_DYNAMIC_INFO_NEXT_MODE_S;
> > +       opensbi_info.options = SBI_SCRATCH_NO_BOOT_PRINTS;
> > +
> > +       opensbi_entry = (void (*)(ulong, ulong, ulong))spl_image->entry_point;
> > +       invalidate_icache_all();
> > +
> > +#ifdef CONFIG_SMP
> > +       ret = smp_call_function((ulong)spl_image->entry_point,
> > +                               (ulong)spl_image->fdt_addr,
> > +                               (ulong)&opensbi_info);
> > +       if (ret)
> > +               hang();
> > +#endif
> > +       opensbi_entry(gd->arch.boot_hart, (ulong)spl_image->fdt_addr,
> > +                     (ulong)&opensbi_info);
> > +}
> > diff --git a/include/image.h b/include/image.h
> > index 5f82194951..78a2dffe54 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -156,6 +156,7 @@ enum {
> >         IH_OS_OPENRTOS,         /* OpenRTOS     */
> >         IH_OS_ARM_TRUSTED_FIRMWARE,     /* ARM Trusted Firmware */
> >         IH_OS_TEE,                      /* Trusted Execution Environment */
> > +       IH_OS_OPENSBI,                  /* RISC-V OpenSBI */
> > 
> >         IH_OS_COUNT,
> >  };
> > diff --git a/include/opensbi.h b/include/opensbi.h
> > new file mode 100644
> > index 0000000000..9f1d62e7dd
> > --- /dev/null
> > +++ b/include/opensbi.h
> > @@ -0,0 +1,40 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (c) 2019 Western Digital Corporation or its affiliates.
> > + *
> > + * Based on include/sbi/{fw_dynamic.h,sbi_scratch.h} from the OpenSBI project.
> > + */
> > +#ifndef OPENSBI_H
> > +#define OPENSBI_H
> > +
> > +/** Expected value of info magic ('OSBI' ascii string in hex) */
> > +#define FW_DYNAMIC_INFO_MAGIC_VALUE            0x4942534f
> > +
> > +/** Maximum supported info version */
> > +#define FW_DYNAMIC_INFO_VERSION                        0x1
> > +
> > +/** Possible next mode values */
> > +#define FW_DYNAMIC_INFO_NEXT_MODE_U            0x0
> > +#define FW_DYNAMIC_INFO_NEXT_MODE_S            0x1
> > +#define FW_DYNAMIC_INFO_NEXT_MODE_M            0x3
> > +
> > +enum sbi_scratch_options {
> > +       /** Disable prints during boot */
> > +       SBI_SCRATCH_NO_BOOT_PRINTS = (1 << 0),
> > +};
> > +
> > +/** Representation dynamic info passed by previous booting stage */
> > +struct fw_dynamic_info {
> > +       /** Info magic */
> > +       unsigned long magic;
> > +       /** Info version */
> > +       unsigned long version;
> > +       /** Next booting stage address */
> > +       unsigned long next_addr;
> > +       /** Next booting stage mode */
> > +       unsigned long next_mode;
> > +       /** Options for OpenSBI library */
> > +       unsigned long options;
> > +} __packed;
> > +
> > +#endif
> > diff --git a/include/spl.h b/include/spl.h
> > index a90f971a23..e4640f3830 100644
> > --- a/include/spl.h
> > +++ b/include/spl.h
> > @@ -374,6 +374,11 @@ void spl_invoke_atf(struct spl_image_info *spl_image);
> >   */
> >  void spl_optee_entry(void *arg0, void *arg1, void *arg2, void *arg3);
> > 
> > +/**
> > + * spl_invoke_opensbi - boot using a RISC-V OpenSBI image
> > + */
> > +void spl_invoke_opensbi(struct spl_image_info *spl_image);
> > +
> >  /**
> >   * board_return_to_bootrom - allow for boards to continue with the boot ROM
> >   *
> > --
> > 2.21.0
> > 
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> 
> Apart from minor queries/comments above, looks good to me.
> 
> Reviewed-by: Anup Patel <anup.patel at wdc.com>
> 
> Regards,
> Anup


More information about the U-Boot mailing list