diff mbox series

[1/5] roms/opensbi: Update to support building bios images for generic platform

Message ID 1588348254-7241-2-git-send-email-bmeng.cn@gmail.com
State New
Headers show
Series riscv: Switch to use generic platform of opensbi bios images | expand

Commit Message

Bin Meng May 1, 2020, 3:50 p.m. UTC
From: Bin Meng <bin.meng@windriver.com>

The RISC-V generic platform is a flattened device tree (FDT) based
platform where all platform specific functionality is provided based
on FDT passed by previous booting stage. The support was added in
upstream opensbi recently.

Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range limit override")
with the following changes since v0.7 release:

  1bb00ab lib: No need to provide default PMP region using platform callbacks
  a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback
  6585fab lib: utils: Add SiFive test device
  4781545 platform: Add Nuclei UX600 platform
  3a326af scripts: adapt binary archive script for Nuclei UX600
  5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
  e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
  01a8c8e lib: utils: Improve fdt_parse_uart8250() API
  0a0093b lib: utils: Add fdt_parse_uart8250_node() function
  243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
  e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
  a39cd6f lib: utils: Add FDT match table based node lookup
  dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
  66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
  19e966b lib: utils: Add fdt_parse_hart_id() function
  44dd7be lib: utils: Add fdt_parse_max_hart_id() API
  f0eb503 lib: utils: Add fdt_parse_plic_node() function
  1ac794c include: Add array_size() macro
  8ff2b94 lib: utils: Add simple FDT timer framework
  76f0f81 lib: utils: Add simple FDT ipi framework
  75322a6 lib: utils: Add simple FDT irqchip framework
  76a8940 lib: utils: Add simple FDT serial framework
  7cc6fa4 lib: utils: Add simple FDT reset framework
  4d06353 firmware: fw_base: Introduce optional fw_platform_init()
  f1aa9e5 platform: Add generic FDT based platform support
  1f21b99 lib: sbi: Print platform hart count at boot time
  2ba7087 scripts: Add generic platform to create-binary-archive.sh
  4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override

Update our Makefile to build the generic platform instead of building
virt and sifive_u separately.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 roms/Makefile | 30 ++++++++----------------------
 roms/opensbi  |  2 +-
 2 files changed, 9 insertions(+), 23 deletions(-)

Comments

Anup Patel May 3, 2020, 4:38 a.m. UTC | #1
On Fri, May 1, 2020 at 9:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> The RISC-V generic platform is a flattened device tree (FDT) based
> platform where all platform specific functionality is provided based
> on FDT passed by previous booting stage. The support was added in
> upstream opensbi recently.
>
> Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range limit override")
> with the following changes since v0.7 release:
>
>   1bb00ab lib: No need to provide default PMP region using platform callbacks
>   a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback
>   6585fab lib: utils: Add SiFive test device
>   4781545 platform: Add Nuclei UX600 platform
>   3a326af scripts: adapt binary archive script for Nuclei UX600
>   5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
>   e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
>   01a8c8e lib: utils: Improve fdt_parse_uart8250() API
>   0a0093b lib: utils: Add fdt_parse_uart8250_node() function
>   243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
>   e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
>   a39cd6f lib: utils: Add FDT match table based node lookup
>   dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
>   66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
>   19e966b lib: utils: Add fdt_parse_hart_id() function
>   44dd7be lib: utils: Add fdt_parse_max_hart_id() API
>   f0eb503 lib: utils: Add fdt_parse_plic_node() function
>   1ac794c include: Add array_size() macro
>   8ff2b94 lib: utils: Add simple FDT timer framework
>   76f0f81 lib: utils: Add simple FDT ipi framework
>   75322a6 lib: utils: Add simple FDT irqchip framework
>   76a8940 lib: utils: Add simple FDT serial framework
>   7cc6fa4 lib: utils: Add simple FDT reset framework
>   4d06353 firmware: fw_base: Introduce optional fw_platform_init()
>   f1aa9e5 platform: Add generic FDT based platform support
>   1f21b99 lib: sbi: Print platform hart count at boot time
>   2ba7087 scripts: Add generic platform to create-binary-archive.sh
>   4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override
>
> Update our Makefile to build the generic platform instead of building
> virt and sifive_u separately.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  roms/Makefile | 30 ++++++++----------------------
>  roms/opensbi  |  2 +-
>  2 files changed, 9 insertions(+), 23 deletions(-)
>
> diff --git a/roms/Makefile b/roms/Makefile
> index f9acf39..cb00628 100644
> --- a/roms/Makefile
> +++ b/roms/Makefile
> @@ -64,10 +64,8 @@ default help:
>         @echo "  u-boot.e500        -- update u-boot.e500"
>         @echo "  u-boot.sam460      -- update u-boot.sam460"
>         @echo "  efi                -- update UEFI (edk2) platform firmware"
> -       @echo "  opensbi32-virt     -- update OpenSBI for 32-bit virt machine"
> -       @echo "  opensbi64-virt     -- update OpenSBI for 64-bit virt machine"
> -       @echo "  opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine"
> -       @echo "  opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine"
> +       @echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic machine"
> +       @echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic machine"
>         @echo "  bios-microvm       -- update bios-microvm.bin (qboot)"
>         @echo "  clean              -- delete the files generated by the previous" \
>                                       "build targets"
> @@ -170,29 +168,17 @@ skiboot:
>  efi: edk2-basetools
>         $(MAKE) -f Makefile.edk2
>
> -opensbi32-virt:
> +opensbi32-generic:
>         $(MAKE) -C opensbi \
>                 CROSS_COMPILE=$(riscv32_cross_prefix) \
> -               PLATFORM="qemu/virt"
> -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> +               PLATFORM="generic"
> +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-generic-fw_jump.bin

I think you should copy fw_jump.elf as well because QEMU Spike
platform needs it.

>
> -opensbi64-virt:
> +opensbi64-generic:
>         $(MAKE) -C opensbi \
>                 CROSS_COMPILE=$(riscv64_cross_prefix) \
> -               PLATFORM="qemu/virt"
> -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-virt-fw_jump.bin
> -
> -opensbi32-sifive_u:
> -       $(MAKE) -C opensbi \
> -               CROSS_COMPILE=$(riscv32_cross_prefix) \
> -               PLATFORM="sifive/fu540"
> -       cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
> -
> -opensbi64-sifive_u:
> -       $(MAKE) -C opensbi \
> -               CROSS_COMPILE=$(riscv64_cross_prefix) \
> -               PLATFORM="sifive/fu540"
> -       cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
> +               PLATFORM="generic"
> +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-generic-fw_jump.bin

Same as above.

>
>  bios-microvm:
>         $(MAKE) -C qboot
> diff --git a/roms/opensbi b/roms/opensbi
> index 9f1b72c..4f18c6e 160000
> --- a/roms/opensbi
> +++ b/roms/opensbi
> @@ -1 +1 @@
> -Subproject commit 9f1b72ce66d659e91013b358939e832fb27223f5
> +Subproject commit 4f18c6e55049d858c62e87d2605dd41c06956e4e
> --
> 2.7.4
>
>

Otherwise looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup
Bin Meng May 4, 2020, 7:16 a.m. UTC | #2
Hi Anup,

On Sun, May 3, 2020 at 12:38 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Fri, May 1, 2020 at 9:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > The RISC-V generic platform is a flattened device tree (FDT) based
> > platform where all platform specific functionality is provided based
> > on FDT passed by previous booting stage. The support was added in
> > upstream opensbi recently.
> >
> > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range limit override")
> > with the following changes since v0.7 release:
> >
> >   1bb00ab lib: No need to provide default PMP region using platform callbacks
> >   a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback
> >   6585fab lib: utils: Add SiFive test device
> >   4781545 platform: Add Nuclei UX600 platform
> >   3a326af scripts: adapt binary archive script for Nuclei UX600
> >   5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> >   e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
> >   01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> >   0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> >   243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> >   e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
> >   a39cd6f lib: utils: Add FDT match table based node lookup
> >   dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> >   66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> >   19e966b lib: utils: Add fdt_parse_hart_id() function
> >   44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> >   f0eb503 lib: utils: Add fdt_parse_plic_node() function
> >   1ac794c include: Add array_size() macro
> >   8ff2b94 lib: utils: Add simple FDT timer framework
> >   76f0f81 lib: utils: Add simple FDT ipi framework
> >   75322a6 lib: utils: Add simple FDT irqchip framework
> >   76a8940 lib: utils: Add simple FDT serial framework
> >   7cc6fa4 lib: utils: Add simple FDT reset framework
> >   4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> >   f1aa9e5 platform: Add generic FDT based platform support
> >   1f21b99 lib: sbi: Print platform hart count at boot time
> >   2ba7087 scripts: Add generic platform to create-binary-archive.sh
> >   4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override
> >
> > Update our Makefile to build the generic platform instead of building
> > virt and sifive_u separately.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  roms/Makefile | 30 ++++++++----------------------
> >  roms/opensbi  |  2 +-
> >  2 files changed, 9 insertions(+), 23 deletions(-)
> >
> > diff --git a/roms/Makefile b/roms/Makefile
> > index f9acf39..cb00628 100644
> > --- a/roms/Makefile
> > +++ b/roms/Makefile
> > @@ -64,10 +64,8 @@ default help:
> >         @echo "  u-boot.e500        -- update u-boot.e500"
> >         @echo "  u-boot.sam460      -- update u-boot.sam460"
> >         @echo "  efi                -- update UEFI (edk2) platform firmware"
> > -       @echo "  opensbi32-virt     -- update OpenSBI for 32-bit virt machine"
> > -       @echo "  opensbi64-virt     -- update OpenSBI for 64-bit virt machine"
> > -       @echo "  opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine"
> > -       @echo "  opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine"
> > +       @echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic machine"
> > +       @echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic machine"
> >         @echo "  bios-microvm       -- update bios-microvm.bin (qboot)"
> >         @echo "  clean              -- delete the files generated by the previous" \
> >                                       "build targets"
> > @@ -170,29 +168,17 @@ skiboot:
> >  efi: edk2-basetools
> >         $(MAKE) -f Makefile.edk2
> >
> > -opensbi32-virt:
> > +opensbi32-generic:
> >         $(MAKE) -C opensbi \
> >                 CROSS_COMPILE=$(riscv32_cross_prefix) \
> > -               PLATFORM="qemu/virt"
> > -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > +               PLATFORM="generic"
> > +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
>
> I think you should copy fw_jump.elf as well because QEMU Spike
> platform needs it.
>

I believe we intended only to ship default bios images for virt and
sifive_u. Spike bios image was not shipped in previous QEMU version
too.

> >
> > -opensbi64-virt:
> > +opensbi64-generic:
> >         $(MAKE) -C opensbi \
> >                 CROSS_COMPILE=$(riscv64_cross_prefix) \
> > -               PLATFORM="qemu/virt"
> > -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-virt-fw_jump.bin
> > -
> > -opensbi32-sifive_u:
> > -       $(MAKE) -C opensbi \
> > -               CROSS_COMPILE=$(riscv32_cross_prefix) \
> > -               PLATFORM="sifive/fu540"
> > -       cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
> > -
> > -opensbi64-sifive_u:
> > -       $(MAKE) -C opensbi \
> > -               CROSS_COMPILE=$(riscv64_cross_prefix) \
> > -               PLATFORM="sifive/fu540"
> > -       cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
> > +               PLATFORM="generic"
> > +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-generic-fw_jump.bin
>
> Same as above.
>
> >
> >  bios-microvm:
> >         $(MAKE) -C qboot
> > diff --git a/roms/opensbi b/roms/opensbi
> > index 9f1b72c..4f18c6e 160000
> > --- a/roms/opensbi
> > +++ b/roms/opensbi
> > @@ -1 +1 @@
> > -Subproject commit 9f1b72ce66d659e91013b358939e832fb27223f5
> > +Subproject commit 4f18c6e55049d858c62e87d2605dd41c06956e4e
> > --
> > 2.7.4
> >
> >
>
> Otherwise looks good to me.
>
> Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Bin
Anup Patel May 4, 2020, 7:51 a.m. UTC | #3
On Mon, May 4, 2020 at 12:46 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Anup,
>
> On Sun, May 3, 2020 at 12:38 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Fri, May 1, 2020 at 9:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > platform where all platform specific functionality is provided based
> > > on FDT passed by previous booting stage. The support was added in
> > > upstream opensbi recently.
> > >
> > > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range limit override")
> > > with the following changes since v0.7 release:
> > >
> > >   1bb00ab lib: No need to provide default PMP region using platform callbacks
> > >   a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback
> > >   6585fab lib: utils: Add SiFive test device
> > >   4781545 platform: Add Nuclei UX600 platform
> > >   3a326af scripts: adapt binary archive script for Nuclei UX600
> > >   5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > >   e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
> > >   01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > >   0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > >   243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > >   e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
> > >   a39cd6f lib: utils: Add FDT match table based node lookup
> > >   dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > >   66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > >   19e966b lib: utils: Add fdt_parse_hart_id() function
> > >   44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > >   f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > >   1ac794c include: Add array_size() macro
> > >   8ff2b94 lib: utils: Add simple FDT timer framework
> > >   76f0f81 lib: utils: Add simple FDT ipi framework
> > >   75322a6 lib: utils: Add simple FDT irqchip framework
> > >   76a8940 lib: utils: Add simple FDT serial framework
> > >   7cc6fa4 lib: utils: Add simple FDT reset framework
> > >   4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > >   f1aa9e5 platform: Add generic FDT based platform support
> > >   1f21b99 lib: sbi: Print platform hart count at boot time
> > >   2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > >   4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override
> > >
> > > Update our Makefile to build the generic platform instead of building
> > > virt and sifive_u separately.
> > >
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > ---
> > >
> > >  roms/Makefile | 30 ++++++++----------------------
> > >  roms/opensbi  |  2 +-
> > >  2 files changed, 9 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/roms/Makefile b/roms/Makefile
> > > index f9acf39..cb00628 100644
> > > --- a/roms/Makefile
> > > +++ b/roms/Makefile
> > > @@ -64,10 +64,8 @@ default help:
> > >         @echo "  u-boot.e500        -- update u-boot.e500"
> > >         @echo "  u-boot.sam460      -- update u-boot.sam460"
> > >         @echo "  efi                -- update UEFI (edk2) platform firmware"
> > > -       @echo "  opensbi32-virt     -- update OpenSBI for 32-bit virt machine"
> > > -       @echo "  opensbi64-virt     -- update OpenSBI for 64-bit virt machine"
> > > -       @echo "  opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine"
> > > -       @echo "  opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine"
> > > +       @echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic machine"
> > > +       @echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic machine"
> > >         @echo "  bios-microvm       -- update bios-microvm.bin (qboot)"
> > >         @echo "  clean              -- delete the files generated by the previous" \
> > >                                       "build targets"
> > > @@ -170,29 +168,17 @@ skiboot:
> > >  efi: edk2-basetools
> > >         $(MAKE) -f Makefile.edk2
> > >
> > > -opensbi32-virt:
> > > +opensbi32-generic:
> > >         $(MAKE) -C opensbi \
> > >                 CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > -               PLATFORM="qemu/virt"
> > > -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > > +               PLATFORM="generic"
> > > +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
> >
> > I think you should copy fw_jump.elf as well because QEMU Spike
> > platform needs it.
> >
>
> I believe we intended only to ship default bios images for virt and
> sifive_u. Spike bios image was not shipped in previous QEMU version
> too.

Is there a specific reason for not shipping bios image for Spike machine ?

There were issues booting Linux on QEMU spike machine which are
now fixed and available in QEMU master. I think we should certainly
ship fw_jump.elf for Spike machine. This way we have OpenSBI generic
firmware available as a bios image for three QEMU machines(virt, sifive_u,
and spike).

Regards,
Anup

>
> > >
> > > -opensbi64-virt:
> > > +opensbi64-generic:
> > >         $(MAKE) -C opensbi \
> > >                 CROSS_COMPILE=$(riscv64_cross_prefix) \
> > > -               PLATFORM="qemu/virt"
> > > -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-virt-fw_jump.bin
> > > -
> > > -opensbi32-sifive_u:
> > > -       $(MAKE) -C opensbi \
> > > -               CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > -               PLATFORM="sifive/fu540"
> > > -       cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
> > > -
> > > -opensbi64-sifive_u:
> > > -       $(MAKE) -C opensbi \
> > > -               CROSS_COMPILE=$(riscv64_cross_prefix) \
> > > -               PLATFORM="sifive/fu540"
> > > -       cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
> > > +               PLATFORM="generic"
> > > +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-generic-fw_jump.bin
> >
> > Same as above.
> >
> > >
> > >  bios-microvm:
> > >         $(MAKE) -C qboot
> > > diff --git a/roms/opensbi b/roms/opensbi
> > > index 9f1b72c..4f18c6e 160000
> > > --- a/roms/opensbi
> > > +++ b/roms/opensbi
> > > @@ -1 +1 @@
> > > -Subproject commit 9f1b72ce66d659e91013b358939e832fb27223f5
> > > +Subproject commit 4f18c6e55049d858c62e87d2605dd41c06956e4e
> > > --
> > > 2.7.4
> > >
> > >
> >
> > Otherwise looks good to me.
> >
> > Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Regards,
> Bin
Bin Meng May 4, 2020, 8 a.m. UTC | #4
Hi Anup,

On Mon, May 4, 2020 at 3:52 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, May 4, 2020 at 12:46 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Anup,
> >
> > On Sun, May 3, 2020 at 12:38 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Fri, May 1, 2020 at 9:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > From: Bin Meng <bin.meng@windriver.com>
> > > >
> > > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > > platform where all platform specific functionality is provided based
> > > > on FDT passed by previous booting stage. The support was added in
> > > > upstream opensbi recently.
> > > >
> > > > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range limit override")
> > > > with the following changes since v0.7 release:
> > > >
> > > >   1bb00ab lib: No need to provide default PMP region using platform callbacks
> > > >   a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback
> > > >   6585fab lib: utils: Add SiFive test device
> > > >   4781545 platform: Add Nuclei UX600 platform
> > > >   3a326af scripts: adapt binary archive script for Nuclei UX600
> > > >   5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > > >   e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
> > > >   01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > > >   0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > > >   243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > > >   e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
> > > >   a39cd6f lib: utils: Add FDT match table based node lookup
> > > >   dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > > >   66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > > >   19e966b lib: utils: Add fdt_parse_hart_id() function
> > > >   44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > > >   f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > > >   1ac794c include: Add array_size() macro
> > > >   8ff2b94 lib: utils: Add simple FDT timer framework
> > > >   76f0f81 lib: utils: Add simple FDT ipi framework
> > > >   75322a6 lib: utils: Add simple FDT irqchip framework
> > > >   76a8940 lib: utils: Add simple FDT serial framework
> > > >   7cc6fa4 lib: utils: Add simple FDT reset framework
> > > >   4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > > >   f1aa9e5 platform: Add generic FDT based platform support
> > > >   1f21b99 lib: sbi: Print platform hart count at boot time
> > > >   2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > > >   4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override
> > > >
> > > > Update our Makefile to build the generic platform instead of building
> > > > virt and sifive_u separately.
> > > >
> > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > ---
> > > >
> > > >  roms/Makefile | 30 ++++++++----------------------
> > > >  roms/opensbi  |  2 +-
> > > >  2 files changed, 9 insertions(+), 23 deletions(-)
> > > >
> > > > diff --git a/roms/Makefile b/roms/Makefile
> > > > index f9acf39..cb00628 100644
> > > > --- a/roms/Makefile
> > > > +++ b/roms/Makefile
> > > > @@ -64,10 +64,8 @@ default help:
> > > >         @echo "  u-boot.e500        -- update u-boot.e500"
> > > >         @echo "  u-boot.sam460      -- update u-boot.sam460"
> > > >         @echo "  efi                -- update UEFI (edk2) platform firmware"
> > > > -       @echo "  opensbi32-virt     -- update OpenSBI for 32-bit virt machine"
> > > > -       @echo "  opensbi64-virt     -- update OpenSBI for 64-bit virt machine"
> > > > -       @echo "  opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine"
> > > > -       @echo "  opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine"
> > > > +       @echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic machine"
> > > > +       @echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic machine"
> > > >         @echo "  bios-microvm       -- update bios-microvm.bin (qboot)"
> > > >         @echo "  clean              -- delete the files generated by the previous" \
> > > >                                       "build targets"
> > > > @@ -170,29 +168,17 @@ skiboot:
> > > >  efi: edk2-basetools
> > > >         $(MAKE) -f Makefile.edk2
> > > >
> > > > -opensbi32-virt:
> > > > +opensbi32-generic:
> > > >         $(MAKE) -C opensbi \
> > > >                 CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > > -               PLATFORM="qemu/virt"
> > > > -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > > > +               PLATFORM="generic"
> > > > +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
> > >
> > > I think you should copy fw_jump.elf as well because QEMU Spike
> > > platform needs it.
> > >
> >
> > I believe we intended only to ship default bios images for virt and
> > sifive_u. Spike bios image was not shipped in previous QEMU version
> > too.
>
> Is there a specific reason for not shipping bios image for Spike machine ?
>

That's only my guess.  Based on what I see from git history, adding
"-bios" support was added via:

commit 5b8a986350a9 ("hw/riscv/spike: Allow loading firmware
separately using -bios option")

with bios image as opensbi-riscv{32,64}-spike-fw_jump.elf, but the
images were not added to QEMU repo.

> There were issues booting Linux on QEMU spike machine which are
> now fixed and available in QEMU master. I think we should certainly
> ship fw_jump.elf for Spike machine. This way we have OpenSBI generic
> firmware available as a bios image for three QEMU machines(virt, sifive_u,
> and spike).
>

If everyone thinks shipping the ELF image is OK, I can do that in v2.

Regards,
Bin
Bin Meng May 4, 2020, 8:04 a.m. UTC | #5
On Mon, May 4, 2020 at 4:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Anup,
>
> On Mon, May 4, 2020 at 3:52 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Mon, May 4, 2020 at 12:46 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Anup,
> > >
> > > On Sun, May 3, 2020 at 12:38 PM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Fri, May 1, 2020 at 9:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > From: Bin Meng <bin.meng@windriver.com>
> > > > >
> > > > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > > > platform where all platform specific functionality is provided based
> > > > > on FDT passed by previous booting stage. The support was added in
> > > > > upstream opensbi recently.
> > > > >
> > > > > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > > > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range limit override")
> > > > > with the following changes since v0.7 release:
> > > > >
> > > > >   1bb00ab lib: No need to provide default PMP region using platform callbacks
> > > > >   a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback
> > > > >   6585fab lib: utils: Add SiFive test device
> > > > >   4781545 platform: Add Nuclei UX600 platform
> > > > >   3a326af scripts: adapt binary archive script for Nuclei UX600
> > > > >   5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > > > >   e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
> > > > >   01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > > > >   0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > > > >   243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > > > >   e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
> > > > >   a39cd6f lib: utils: Add FDT match table based node lookup
> > > > >   dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > > > >   66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > > > >   19e966b lib: utils: Add fdt_parse_hart_id() function
> > > > >   44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > > > >   f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > > > >   1ac794c include: Add array_size() macro
> > > > >   8ff2b94 lib: utils: Add simple FDT timer framework
> > > > >   76f0f81 lib: utils: Add simple FDT ipi framework
> > > > >   75322a6 lib: utils: Add simple FDT irqchip framework
> > > > >   76a8940 lib: utils: Add simple FDT serial framework
> > > > >   7cc6fa4 lib: utils: Add simple FDT reset framework
> > > > >   4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > > > >   f1aa9e5 platform: Add generic FDT based platform support
> > > > >   1f21b99 lib: sbi: Print platform hart count at boot time
> > > > >   2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > > > >   4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override
> > > > >
> > > > > Update our Makefile to build the generic platform instead of building
> > > > > virt and sifive_u separately.
> > > > >
> > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > > ---
> > > > >
> > > > >  roms/Makefile | 30 ++++++++----------------------
> > > > >  roms/opensbi  |  2 +-
> > > > >  2 files changed, 9 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/roms/Makefile b/roms/Makefile
> > > > > index f9acf39..cb00628 100644
> > > > > --- a/roms/Makefile
> > > > > +++ b/roms/Makefile
> > > > > @@ -64,10 +64,8 @@ default help:
> > > > >         @echo "  u-boot.e500        -- update u-boot.e500"
> > > > >         @echo "  u-boot.sam460      -- update u-boot.sam460"
> > > > >         @echo "  efi                -- update UEFI (edk2) platform firmware"
> > > > > -       @echo "  opensbi32-virt     -- update OpenSBI for 32-bit virt machine"
> > > > > -       @echo "  opensbi64-virt     -- update OpenSBI for 64-bit virt machine"
> > > > > -       @echo "  opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine"
> > > > > -       @echo "  opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine"
> > > > > +       @echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic machine"
> > > > > +       @echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic machine"
> > > > >         @echo "  bios-microvm       -- update bios-microvm.bin (qboot)"
> > > > >         @echo "  clean              -- delete the files generated by the previous" \
> > > > >                                       "build targets"
> > > > > @@ -170,29 +168,17 @@ skiboot:
> > > > >  efi: edk2-basetools
> > > > >         $(MAKE) -f Makefile.edk2
> > > > >
> > > > > -opensbi32-virt:
> > > > > +opensbi32-generic:
> > > > >         $(MAKE) -C opensbi \
> > > > >                 CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > > > -               PLATFORM="qemu/virt"
> > > > > -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > > > > +               PLATFORM="generic"
> > > > > +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
> > > >
> > > > I think you should copy fw_jump.elf as well because QEMU Spike
> > > > platform needs it.
> > > >
> > >
> > > I believe we intended only to ship default bios images for virt and
> > > sifive_u. Spike bios image was not shipped in previous QEMU version
> > > too.
> >
> > Is there a specific reason for not shipping bios image for Spike machine ?
> >
>
> That's only my guess.  Based on what I see from git history, adding
> "-bios" support was added via:
>
> commit 5b8a986350a9 ("hw/riscv/spike: Allow loading firmware
> separately using -bios option")
>
> with bios image as opensbi-riscv{32,64}-spike-fw_jump.elf, but the
> images were not added to QEMU repo.
>
> > There were issues booting Linux on QEMU spike machine which are
> > now fixed and available in QEMU master. I think we should certainly
> > ship fw_jump.elf for Spike machine. This way we have OpenSBI generic
> > firmware available as a bios image for three QEMU machines(virt, sifive_u,
> > and spike).
> >
>
> If everyone thinks shipping the ELF image is OK, I can do that in v2.

One additional note, that's why patch 5 in this series for. Without
the default bios image being shipped in QEMU, QEMU testing will
complain.

So in the future, when we have more QEMU RISC-V machines added, if
they are not using the generic firmware, do we want to ship all of
these different firmware images in QEMU?

Regards,
Bin
Anup Patel May 4, 2020, 8:16 a.m. UTC | #6
On Mon, May 4, 2020 at 1:35 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Mon, May 4, 2020 at 4:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Anup,
> >
> > On Mon, May 4, 2020 at 3:52 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Mon, May 4, 2020 at 12:46 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Anup,
> > > >
> > > > On Sun, May 3, 2020 at 12:38 PM Anup Patel <anup@brainfault.org> wrote:
> > > > >
> > > > > On Fri, May 1, 2020 at 9:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > From: Bin Meng <bin.meng@windriver.com>
> > > > > >
> > > > > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > > > > platform where all platform specific functionality is provided based
> > > > > > on FDT passed by previous booting stage. The support was added in
> > > > > > upstream opensbi recently.
> > > > > >
> > > > > > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > > > > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range limit override")
> > > > > > with the following changes since v0.7 release:
> > > > > >
> > > > > >   1bb00ab lib: No need to provide default PMP region using platform callbacks
> > > > > >   a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback
> > > > > >   6585fab lib: utils: Add SiFive test device
> > > > > >   4781545 platform: Add Nuclei UX600 platform
> > > > > >   3a326af scripts: adapt binary archive script for Nuclei UX600
> > > > > >   5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > > > > >   e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
> > > > > >   01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > > > > >   0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > > > > >   243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > > > > >   e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
> > > > > >   a39cd6f lib: utils: Add FDT match table based node lookup
> > > > > >   dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > > > > >   66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > > > > >   19e966b lib: utils: Add fdt_parse_hart_id() function
> > > > > >   44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > > > > >   f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > > > > >   1ac794c include: Add array_size() macro
> > > > > >   8ff2b94 lib: utils: Add simple FDT timer framework
> > > > > >   76f0f81 lib: utils: Add simple FDT ipi framework
> > > > > >   75322a6 lib: utils: Add simple FDT irqchip framework
> > > > > >   76a8940 lib: utils: Add simple FDT serial framework
> > > > > >   7cc6fa4 lib: utils: Add simple FDT reset framework
> > > > > >   4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > > > > >   f1aa9e5 platform: Add generic FDT based platform support
> > > > > >   1f21b99 lib: sbi: Print platform hart count at boot time
> > > > > >   2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > > > > >   4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override
> > > > > >
> > > > > > Update our Makefile to build the generic platform instead of building
> > > > > > virt and sifive_u separately.
> > > > > >
> > > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > > > ---
> > > > > >
> > > > > >  roms/Makefile | 30 ++++++++----------------------
> > > > > >  roms/opensbi  |  2 +-
> > > > > >  2 files changed, 9 insertions(+), 23 deletions(-)
> > > > > >
> > > > > > diff --git a/roms/Makefile b/roms/Makefile
> > > > > > index f9acf39..cb00628 100644
> > > > > > --- a/roms/Makefile
> > > > > > +++ b/roms/Makefile
> > > > > > @@ -64,10 +64,8 @@ default help:
> > > > > >         @echo "  u-boot.e500        -- update u-boot.e500"
> > > > > >         @echo "  u-boot.sam460      -- update u-boot.sam460"
> > > > > >         @echo "  efi                -- update UEFI (edk2) platform firmware"
> > > > > > -       @echo "  opensbi32-virt     -- update OpenSBI for 32-bit virt machine"
> > > > > > -       @echo "  opensbi64-virt     -- update OpenSBI for 64-bit virt machine"
> > > > > > -       @echo "  opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine"
> > > > > > -       @echo "  opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine"
> > > > > > +       @echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic machine"
> > > > > > +       @echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic machine"
> > > > > >         @echo "  bios-microvm       -- update bios-microvm.bin (qboot)"
> > > > > >         @echo "  clean              -- delete the files generated by the previous" \
> > > > > >                                       "build targets"
> > > > > > @@ -170,29 +168,17 @@ skiboot:
> > > > > >  efi: edk2-basetools
> > > > > >         $(MAKE) -f Makefile.edk2
> > > > > >
> > > > > > -opensbi32-virt:
> > > > > > +opensbi32-generic:
> > > > > >         $(MAKE) -C opensbi \
> > > > > >                 CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > > > > -               PLATFORM="qemu/virt"
> > > > > > -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > > > > > +               PLATFORM="generic"
> > > > > > +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
> > > > >
> > > > > I think you should copy fw_jump.elf as well because QEMU Spike
> > > > > platform needs it.
> > > > >
> > > >
> > > > I believe we intended only to ship default bios images for virt and
> > > > sifive_u. Spike bios image was not shipped in previous QEMU version
> > > > too.
> > >
> > > Is there a specific reason for not shipping bios image for Spike machine ?
> > >
> >
> > That's only my guess.  Based on what I see from git history, adding
> > "-bios" support was added via:
> >
> > commit 5b8a986350a9 ("hw/riscv/spike: Allow loading firmware
> > separately using -bios option")
> >
> > with bios image as opensbi-riscv{32,64}-spike-fw_jump.elf, but the
> > images were not added to QEMU repo.
> >
> > > There were issues booting Linux on QEMU spike machine which are
> > > now fixed and available in QEMU master. I think we should certainly
> > > ship fw_jump.elf for Spike machine. This way we have OpenSBI generic
> > > firmware available as a bios image for three QEMU machines(virt, sifive_u,
> > > and spike).
> > >
> >
> > If everyone thinks shipping the ELF image is OK, I can do that in v2.
>
> One additional note, that's why patch 5 in this series for. Without
> the default bios image being shipped in QEMU, QEMU testing will
> complain.
>
> So in the future, when we have more QEMU RISC-V machines added, if
> they are not using the generic firmware, do we want to ship all of
> these different firmware images in QEMU?

This would be a QEMU RISC-V policy decision which Palmer or Alistair
can decide.

IMHO, we can insist that newer QEMU RISC-V machines work fine with
OpenSBI generic firmwares so that we don't increase the number of bios
images shipped for QEMU RISC-V.

Regards,
Anup
Alistair Francis May 4, 2020, 3:51 p.m. UTC | #7
On Mon, May 4, 2020 at 1:05 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Mon, May 4, 2020 at 4:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Anup,
> >
> > On Mon, May 4, 2020 at 3:52 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Mon, May 4, 2020 at 12:46 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Anup,
> > > >
> > > > On Sun, May 3, 2020 at 12:38 PM Anup Patel <anup@brainfault.org> wrote:
> > > > >
> > > > > On Fri, May 1, 2020 at 9:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > From: Bin Meng <bin.meng@windriver.com>
> > > > > >
> > > > > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > > > > platform where all platform specific functionality is provided based
> > > > > > on FDT passed by previous booting stage. The support was added in
> > > > > > upstream opensbi recently.
> > > > > >
> > > > > > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > > > > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range limit override")
> > > > > > with the following changes since v0.7 release:
> > > > > >
> > > > > >   1bb00ab lib: No need to provide default PMP region using platform callbacks
> > > > > >   a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback
> > > > > >   6585fab lib: utils: Add SiFive test device
> > > > > >   4781545 platform: Add Nuclei UX600 platform
> > > > > >   3a326af scripts: adapt binary archive script for Nuclei UX600
> > > > > >   5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > > > > >   e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
> > > > > >   01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > > > > >   0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > > > > >   243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > > > > >   e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
> > > > > >   a39cd6f lib: utils: Add FDT match table based node lookup
> > > > > >   dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > > > > >   66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > > > > >   19e966b lib: utils: Add fdt_parse_hart_id() function
> > > > > >   44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > > > > >   f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > > > > >   1ac794c include: Add array_size() macro
> > > > > >   8ff2b94 lib: utils: Add simple FDT timer framework
> > > > > >   76f0f81 lib: utils: Add simple FDT ipi framework
> > > > > >   75322a6 lib: utils: Add simple FDT irqchip framework
> > > > > >   76a8940 lib: utils: Add simple FDT serial framework
> > > > > >   7cc6fa4 lib: utils: Add simple FDT reset framework
> > > > > >   4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > > > > >   f1aa9e5 platform: Add generic FDT based platform support
> > > > > >   1f21b99 lib: sbi: Print platform hart count at boot time
> > > > > >   2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > > > > >   4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override
> > > > > >
> > > > > > Update our Makefile to build the generic platform instead of building
> > > > > > virt and sifive_u separately.

Hey Bin,

Thanks for the patch!

I don't think we can take this update though, as we should stick with
OpenSBI releases.

> > > > > >
> > > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > > > ---
> > > > > >
> > > > > >  roms/Makefile | 30 ++++++++----------------------
> > > > > >  roms/opensbi  |  2 +-
> > > > > >  2 files changed, 9 insertions(+), 23 deletions(-)
> > > > > >
> > > > > > diff --git a/roms/Makefile b/roms/Makefile
> > > > > > index f9acf39..cb00628 100644
> > > > > > --- a/roms/Makefile
> > > > > > +++ b/roms/Makefile
> > > > > > @@ -64,10 +64,8 @@ default help:
> > > > > >         @echo "  u-boot.e500        -- update u-boot.e500"
> > > > > >         @echo "  u-boot.sam460      -- update u-boot.sam460"
> > > > > >         @echo "  efi                -- update UEFI (edk2) platform firmware"
> > > > > > -       @echo "  opensbi32-virt     -- update OpenSBI for 32-bit virt machine"
> > > > > > -       @echo "  opensbi64-virt     -- update OpenSBI for 64-bit virt machine"
> > > > > > -       @echo "  opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine"
> > > > > > -       @echo "  opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine"
> > > > > > +       @echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic machine"
> > > > > > +       @echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic machine"
> > > > > >         @echo "  bios-microvm       -- update bios-microvm.bin (qboot)"
> > > > > >         @echo "  clean              -- delete the files generated by the previous" \
> > > > > >                                       "build targets"
> > > > > > @@ -170,29 +168,17 @@ skiboot:
> > > > > >  efi: edk2-basetools
> > > > > >         $(MAKE) -f Makefile.edk2
> > > > > >
> > > > > > -opensbi32-virt:
> > > > > > +opensbi32-generic:
> > > > > >         $(MAKE) -C opensbi \
> > > > > >                 CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > > > > -               PLATFORM="qemu/virt"
> > > > > > -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > > > > > +               PLATFORM="generic"
> > > > > > +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
> > > > >
> > > > > I think you should copy fw_jump.elf as well because QEMU Spike
> > > > > platform needs it.
> > > > >
> > > >
> > > > I believe we intended only to ship default bios images for virt and
> > > > sifive_u. Spike bios image was not shipped in previous QEMU version
> > > > too.
> > >
> > > Is there a specific reason for not shipping bios image for Spike machine ?
> > >
> >
> > That's only my guess.  Based on what I see from git history, adding
> > "-bios" support was added via:
> >
> > commit 5b8a986350a9 ("hw/riscv/spike: Allow loading firmware
> > separately using -bios option")
> >
> > with bios image as opensbi-riscv{32,64}-spike-fw_jump.elf, but the
> > images were not added to QEMU repo.
> >
> > > There were issues booting Linux on QEMU spike machine which are
> > > now fixed and available in QEMU master. I think we should certainly
> > > ship fw_jump.elf for Spike machine. This way we have OpenSBI generic
> > > firmware available as a bios image for three QEMU machines(virt, sifive_u,
> > > and spike).
> > >
> >
> > If everyone thinks shipping the ELF image is OK, I can do that in v2.

I don't really mind either way.

On one hand it is nice to have all the boards "just work" with
OpenSBI. On the other hand Spike isn't used very often from what I can
tell and it's a larger maintenance burden to update another image.

>
> One additional note, that's why patch 5 in this series for. Without
> the default bios image being shipped in QEMU, QEMU testing will
> complain.
>
> So in the future, when we have more QEMU RISC-V machines added, if
> they are not using the generic firmware, do we want to ship all of
> these different firmware images in QEMU?

That's a good point.

I don't really want to be maintaining 20 different OpenSBI binaries.

I suspect the plan will be that we supply OpenSBI binaries for the
"key" boards. Which is the Virt board and sifive_u. If other boards
can use the same OpenSBI binary then that's great. If not we won't
ship a binary. The main reason to ship the binary is to allow people
to try RISC-V easily. The virt and sifive_u machines do that, we don't
need them all to do that.

So on that note, I think we should include the generic elf which will
support Spike and any future boards that need the elf as well.

So to summarise my ramblings:
 - We will swap to shipping generic ELF and BIN OpenSBI files
 - All boards that can use those should
 - Other boards won't have pre-built OpenSBI support
 - For future updates we just update the 4 files (32-bit and 64-bit)
on each release.

Alistair

>
> Regards,
> Bin
>
Bin Meng May 6, 2020, 10 a.m. UTC | #8
Hi Alistair,

On Tue, May 5, 2020 at 12:00 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Mon, May 4, 2020 at 1:05 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Mon, May 4, 2020 at 4:00 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Anup,
> > >
> > > On Mon, May 4, 2020 at 3:52 PM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Mon, May 4, 2020 at 12:46 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > Hi Anup,
> > > > >
> > > > > On Sun, May 3, 2020 at 12:38 PM Anup Patel <anup@brainfault.org> wrote:
> > > > > >
> > > > > > On Fri, May 1, 2020 at 9:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > >
> > > > > > > From: Bin Meng <bin.meng@windriver.com>
> > > > > > >
> > > > > > > The RISC-V generic platform is a flattened device tree (FDT) based
> > > > > > > platform where all platform specific functionality is provided based
> > > > > > > on FDT passed by previous booting stage. The support was added in
> > > > > > > upstream opensbi recently.
> > > > > > >
> > > > > > > Bring the QEMU roms/opensbi submodule to the upstream opensbi commit:
> > > > > > > commit 4f18c6e55049 ("platform: generic: Add Sifive FU540 TLB flush range limit override")
> > > > > > > with the following changes since v0.7 release:
> > > > > > >
> > > > > > >   1bb00ab lib: No need to provide default PMP region using platform callbacks
> > > > > > >   a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback
> > > > > > >   6585fab lib: utils: Add SiFive test device
> > > > > > >   4781545 platform: Add Nuclei UX600 platform
> > > > > > >   3a326af scripts: adapt binary archive script for Nuclei UX600
> > > > > > >   5bdf022 firmware: fw_base: Remove CSR_MTVEC update check
> > > > > > >   e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero
> > > > > > >   01a8c8e lib: utils: Improve fdt_parse_uart8250() API
> > > > > > >   0a0093b lib: utils: Add fdt_parse_uart8250_node() function
> > > > > > >   243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration
> > > > > > >   e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr()
> > > > > > >   a39cd6f lib: utils: Add FDT match table based node lookup
> > > > > > >   dd33b9e lib: utils: Make fdt_get_node_addr_size() public function
> > > > > > >   66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function
> > > > > > >   19e966b lib: utils: Add fdt_parse_hart_id() function
> > > > > > >   44dd7be lib: utils: Add fdt_parse_max_hart_id() API
> > > > > > >   f0eb503 lib: utils: Add fdt_parse_plic_node() function
> > > > > > >   1ac794c include: Add array_size() macro
> > > > > > >   8ff2b94 lib: utils: Add simple FDT timer framework
> > > > > > >   76f0f81 lib: utils: Add simple FDT ipi framework
> > > > > > >   75322a6 lib: utils: Add simple FDT irqchip framework
> > > > > > >   76a8940 lib: utils: Add simple FDT serial framework
> > > > > > >   7cc6fa4 lib: utils: Add simple FDT reset framework
> > > > > > >   4d06353 firmware: fw_base: Introduce optional fw_platform_init()
> > > > > > >   f1aa9e5 platform: Add generic FDT based platform support
> > > > > > >   1f21b99 lib: sbi: Print platform hart count at boot time
> > > > > > >   2ba7087 scripts: Add generic platform to create-binary-archive.sh
> > > > > > >   4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override
> > > > > > >
> > > > > > > Update our Makefile to build the generic platform instead of building
> > > > > > > virt and sifive_u separately.
>
> Hey Bin,
>
> Thanks for the patch!
>
> I don't think we can take this update though, as we should stick with
> OpenSBI releases.

Sure, will delay this series once OpenSBI v0.8 release is out.

>
> > > > > > >
> > > > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > > > > ---
> > > > > > >
> > > > > > >  roms/Makefile | 30 ++++++++----------------------
> > > > > > >  roms/opensbi  |  2 +-
> > > > > > >  2 files changed, 9 insertions(+), 23 deletions(-)
> > > > > > >
> > > > > > > diff --git a/roms/Makefile b/roms/Makefile
> > > > > > > index f9acf39..cb00628 100644
> > > > > > > --- a/roms/Makefile
> > > > > > > +++ b/roms/Makefile
> > > > > > > @@ -64,10 +64,8 @@ default help:
> > > > > > >         @echo "  u-boot.e500        -- update u-boot.e500"
> > > > > > >         @echo "  u-boot.sam460      -- update u-boot.sam460"
> > > > > > >         @echo "  efi                -- update UEFI (edk2) platform firmware"
> > > > > > > -       @echo "  opensbi32-virt     -- update OpenSBI for 32-bit virt machine"
> > > > > > > -       @echo "  opensbi64-virt     -- update OpenSBI for 64-bit virt machine"
> > > > > > > -       @echo "  opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine"
> > > > > > > -       @echo "  opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine"
> > > > > > > +       @echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic machine"
> > > > > > > +       @echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic machine"
> > > > > > >         @echo "  bios-microvm       -- update bios-microvm.bin (qboot)"
> > > > > > >         @echo "  clean              -- delete the files generated by the previous" \
> > > > > > >                                       "build targets"
> > > > > > > @@ -170,29 +168,17 @@ skiboot:
> > > > > > >  efi: edk2-basetools
> > > > > > >         $(MAKE) -f Makefile.edk2
> > > > > > >
> > > > > > > -opensbi32-virt:
> > > > > > > +opensbi32-generic:
> > > > > > >         $(MAKE) -C opensbi \
> > > > > > >                 CROSS_COMPILE=$(riscv32_cross_prefix) \
> > > > > > > -               PLATFORM="qemu/virt"
> > > > > > > -       cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
> > > > > > > +               PLATFORM="generic"
> > > > > > > +       cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
> > > > > >
> > > > > > I think you should copy fw_jump.elf as well because QEMU Spike
> > > > > > platform needs it.
> > > > > >
> > > > >
> > > > > I believe we intended only to ship default bios images for virt and
> > > > > sifive_u. Spike bios image was not shipped in previous QEMU version
> > > > > too.
> > > >
> > > > Is there a specific reason for not shipping bios image for Spike machine ?
> > > >
> > >
> > > That's only my guess.  Based on what I see from git history, adding
> > > "-bios" support was added via:
> > >
> > > commit 5b8a986350a9 ("hw/riscv/spike: Allow loading firmware
> > > separately using -bios option")
> > >
> > > with bios image as opensbi-riscv{32,64}-spike-fw_jump.elf, but the
> > > images were not added to QEMU repo.
> > >
> > > > There were issues booting Linux on QEMU spike machine which are
> > > > now fixed and available in QEMU master. I think we should certainly
> > > > ship fw_jump.elf for Spike machine. This way we have OpenSBI generic
> > > > firmware available as a bios image for three QEMU machines(virt, sifive_u,
> > > > and spike).
> > > >
> > >
> > > If everyone thinks shipping the ELF image is OK, I can do that in v2.
>
> I don't really mind either way.
>
> On one hand it is nice to have all the boards "just work" with
> OpenSBI. On the other hand Spike isn't used very often from what I can
> tell and it's a larger maintenance burden to update another image.
>
> >
> > One additional note, that's why patch 5 in this series for. Without
> > the default bios image being shipped in QEMU, QEMU testing will
> > complain.
> >
> > So in the future, when we have more QEMU RISC-V machines added, if
> > they are not using the generic firmware, do we want to ship all of
> > these different firmware images in QEMU?
>
> That's a good point.
>
> I don't really want to be maintaining 20 different OpenSBI binaries.
>
> I suspect the plan will be that we supply OpenSBI binaries for the
> "key" boards. Which is the Virt board and sifive_u. If other boards
> can use the same OpenSBI binary then that's great. If not we won't
> ship a binary. The main reason to ship the binary is to allow people
> to try RISC-V easily. The virt and sifive_u machines do that, we don't
> need them all to do that.
>
> So on that note, I think we should include the generic elf which will
> support Spike and any future boards that need the elf as well.
>
> So to summarise my ramblings:
>  - We will swap to shipping generic ELF and BIN OpenSBI files
>  - All boards that can use those should
>  - Other boards won't have pre-built OpenSBI support
>  - For future updates we just update the 4 files (32-bit and 64-bit)
> on each release.

Sounds good to me. Thanks for providing the guideline from the
maintainer's view!

Regards,
Bin
diff mbox series

Patch

diff --git a/roms/Makefile b/roms/Makefile
index f9acf39..cb00628 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -64,10 +64,8 @@  default help:
 	@echo "  u-boot.e500        -- update u-boot.e500"
 	@echo "  u-boot.sam460      -- update u-boot.sam460"
 	@echo "  efi                -- update UEFI (edk2) platform firmware"
-	@echo "  opensbi32-virt     -- update OpenSBI for 32-bit virt machine"
-	@echo "  opensbi64-virt     -- update OpenSBI for 64-bit virt machine"
-	@echo "  opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine"
-	@echo "  opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine"
+	@echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic machine"
+	@echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic machine"
 	@echo "  bios-microvm       -- update bios-microvm.bin (qboot)"
 	@echo "  clean              -- delete the files generated by the previous" \
 	                              "build targets"
@@ -170,29 +168,17 @@  skiboot:
 efi: edk2-basetools
 	$(MAKE) -f Makefile.edk2
 
-opensbi32-virt:
+opensbi32-generic:
 	$(MAKE) -C opensbi \
 		CROSS_COMPILE=$(riscv32_cross_prefix) \
-		PLATFORM="qemu/virt"
-	cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin
+		PLATFORM="generic"
+	cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-generic-fw_jump.bin
 
-opensbi64-virt:
+opensbi64-generic:
 	$(MAKE) -C opensbi \
 		CROSS_COMPILE=$(riscv64_cross_prefix) \
-		PLATFORM="qemu/virt"
-	cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-virt-fw_jump.bin
-
-opensbi32-sifive_u:
-	$(MAKE) -C opensbi \
-		CROSS_COMPILE=$(riscv32_cross_prefix) \
-		PLATFORM="sifive/fu540"
-	cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
-
-opensbi64-sifive_u:
-	$(MAKE) -C opensbi \
-		CROSS_COMPILE=$(riscv64_cross_prefix) \
-		PLATFORM="sifive/fu540"
-	cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
+		PLATFORM="generic"
+	cp opensbi/build/platform/generic/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-generic-fw_jump.bin
 
 bios-microvm:
 	$(MAKE) -C qboot
diff --git a/roms/opensbi b/roms/opensbi
index 9f1b72c..4f18c6e 160000
--- a/roms/opensbi
+++ b/roms/opensbi
@@ -1 +1 @@ 
-Subproject commit 9f1b72ce66d659e91013b358939e832fb27223f5
+Subproject commit 4f18c6e55049d858c62e87d2605dd41c06956e4e