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

[Anup Patel]

On Mon, Jul 29, 2019 at 9:21 PM Auer, Lukas
<lukas.auer at aisec.fraunhofer.de> wrote:
>
> 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.

Sure, please have a separate patch for minimum OpenSBI version. Maybe
you can just update QEMU RISC-V 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.

Ahh, got it.

Reviewed-by: Anup Patel <anup.patel at wdc.com>

Regards,
Anup