qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] riscv: Switch to use generic platform of opensbi bios images
@ 2020-05-01 15:50 Bin Meng
  2020-05-01 15:50 ` [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform Bin Meng
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Bin Meng @ 2020-05-01 15:50 UTC (permalink / raw)
  To: Alistair Francis, Bastian Koppelmann, Palmer Dabbelt,
	Sagar Karandikar, qemu-devel, qemu-riscv
  Cc: Bin Meng, Anup Patel

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.

This series updates QEMU to switch to use generic platform of opensbi
bios images.

The patch emails do not contain binary bits, please grab all updates
at https://github.com/lbmeng/qemu.git bios branch.


Bin Meng (5):
  roms/opensbi: Update to support building bios images for generic
    platform
  gitlab-ci/opensbi: Update GitLab CI to build generic platform
  riscv: Use pre-built bios image of generic platform for virt &
    sifive_u
  riscv/spike: Change the default bios to use generic platform image
  riscv: Suppress the error report for QEMU testing with
    riscv_find_firmware()

 .gitlab-ci-opensbi.yml                       |  26 +++++++----------------
 hw/riscv/boot.c                              |  14 ++++++++++---
 hw/riscv/sifive_u.c                          |   4 ++--
 hw/riscv/spike.c                             |   9 ++++++--
 hw/riscv/virt.c                              |   4 ++--
 pc-bios/opensbi-riscv32-generic-fw_jump.bin  | Bin 0 -> 58032 bytes
 pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin | Bin 49520 -> 0 bytes
 pc-bios/opensbi-riscv32-virt-fw_jump.bin     | Bin 49504 -> 0 bytes
 pc-bios/opensbi-riscv64-generic-fw_jump.bin  | Bin 0 -> 66680 bytes
 pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin | Bin 57936 -> 0 bytes
 pc-bios/opensbi-riscv64-virt-fw_jump.bin     | Bin 57920 -> 0 bytes
 roms/Makefile                                |  30 +++++++--------------------
 roms/opensbi                                 |   2 +-
 13 files changed, 39 insertions(+), 50 deletions(-)
 create mode 100644 pc-bios/opensbi-riscv32-generic-fw_jump.bin
 delete mode 100644 pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
 delete mode 100644 pc-bios/opensbi-riscv32-virt-fw_jump.bin
 create mode 100644 pc-bios/opensbi-riscv64-generic-fw_jump.bin
 delete mode 100644 pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
 delete mode 100644 pc-bios/opensbi-riscv64-virt-fw_jump.bin

-- 
2.7.4



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
  2020-05-01 15:50 [PATCH 0/5] riscv: Switch to use generic platform of opensbi bios images Bin Meng
@ 2020-05-01 15:50 ` Bin Meng
  2020-05-03  4:38   ` Anup Patel
  2020-05-01 15:50 ` [PATCH 2/5] gitlab-ci/opensbi: Update GitLab CI to build " Bin Meng
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2020-05-01 15:50 UTC (permalink / raw)
  To: Alistair Francis, Bastian Koppelmann, Palmer Dabbelt,
	Sagar Karandikar, qemu-devel, qemu-riscv
  Cc: Bin Meng, Anup Patel

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
 
-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
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/5] gitlab-ci/opensbi: Update GitLab CI to build generic platform
  2020-05-01 15:50 [PATCH 0/5] riscv: Switch to use generic platform of opensbi bios images Bin Meng
  2020-05-01 15:50 ` [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform Bin Meng
@ 2020-05-01 15:50 ` Bin Meng
  2020-05-03  4:40   ` Anup Patel
  2020-05-01 15:50 ` [PATCH 3/5] riscv: Use pre-built bios image of generic platform for virt & sifive_u Bin Meng
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2020-05-01 15:50 UTC (permalink / raw)
  To: Alistair Francis, Bastian Koppelmann, Palmer Dabbelt,
	Sagar Karandikar, qemu-devel, qemu-riscv
  Cc: Bin Meng, Anup Patel

From: Bin Meng <bin.meng@windriver.com>

This updates the GitLab CI opensbi job to build opensbi bios images
for the generic platform.

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

 .gitlab-ci-opensbi.yml | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/.gitlab-ci-opensbi.yml b/.gitlab-ci-opensbi.yml
index dd051c0..26092bb 100644
--- a/.gitlab-ci-opensbi.yml
+++ b/.gitlab-ci-opensbi.yml
@@ -34,18 +34,12 @@ build-opensbi:
    when: always
  artifacts:
    paths: # 'artifacts.zip' will contains the following files:
-   - pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
-   - pc-bios/opensbi-riscv32-virt-fw_jump.bin
-   - pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
-   - pc-bios/opensbi-riscv64-virt-fw_jump.bin
-   - opensbi32-virt-stdout.log
-   - opensbi32-virt-stderr.log
-   - opensbi64-virt-stdout.log
-   - opensbi64-virt-stderr.log
-   - opensbi32-sifive_u-stdout.log
-   - opensbi32-sifive_u-stderr.log
-   - opensbi64-sifive_u-stdout.log
-   - opensbi64-sifive_u-stderr.log
+   - pc-bios/opensbi-riscv32-generic-fw_jump.bin
+   - pc-bios/opensbi-riscv64-generic-fw_jump.bin
+   - opensbi32-generic-stdout.log
+   - opensbi32-generic-stderr.log
+   - opensbi64-generic-stdout.log
+   - opensbi64-generic-stderr.log
  image: $CI_REGISTRY_IMAGE:opensbi-cross-build
  variables:
    GIT_DEPTH: 3
@@ -54,10 +48,6 @@ build-opensbi:
  - export JOBS=$(($(getconf _NPROCESSORS_ONLN) + 1))
  - echo "=== Using ${JOBS} simultaneous jobs ==="
  - make -j${JOBS} -C roms/opensbi clean
- - make -j${JOBS} -C roms opensbi32-virt 2>&1 1>opensbi32-virt-stdout.log | tee -a opensbi32-virt-stderr.log >&2
+ - make -j${JOBS} -C roms opensbi32-generic 2>&1 1>opensbi32-generic-stdout.log | tee -a opensbi32-generic-stderr.log >&2
  - make -j${JOBS} -C roms/opensbi clean
- - make -j${JOBS} -C roms opensbi64-virt 2>&1 1>opensbi64-virt-stdout.log | tee -a opensbi64-virt-stderr.log >&2
- - make -j${JOBS} -C roms/opensbi clean
- - make -j${JOBS} -C roms opensbi32-sifive_u 2>&1 1>opensbi32-sifive_u-stdout.log | tee -a opensbi32-sifive_u-stderr.log >&2
- - make -j${JOBS} -C roms/opensbi clean
- - make -j${JOBS} -C roms opensbi64-sifive_u 2>&1 1>opensbi64-sifive_u-stdout.log | tee -a opensbi64-sifive_u-stderr.log >&2
+ - make -j${JOBS} -C roms opensbi64-generic 2>&1 1>opensbi64-generic-stdout.log | tee -a opensbi64-generic-stderr.log >&2
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/5] riscv: Use pre-built bios image of generic platform for virt & sifive_u
  2020-05-01 15:50 [PATCH 0/5] riscv: Switch to use generic platform of opensbi bios images Bin Meng
  2020-05-01 15:50 ` [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform Bin Meng
  2020-05-01 15:50 ` [PATCH 2/5] gitlab-ci/opensbi: Update GitLab CI to build " Bin Meng
@ 2020-05-01 15:50 ` Bin Meng
  2020-05-03  4:43   ` Anup Patel
  2020-05-04 15:53   ` Alistair Francis
  2020-05-01 15:50 ` [PATCH 4/5] riscv/spike: Change the default bios to use generic platform image Bin Meng
  2020-05-01 15:50 ` [PATCH 5/5] riscv: Suppress the error report for QEMU testing with riscv_find_firmware() Bin Meng
  4 siblings, 2 replies; 20+ messages in thread
From: Bin Meng @ 2020-05-01 15:50 UTC (permalink / raw)
  To: Alistair Francis, Bastian Koppelmann, Palmer Dabbelt,
	Sagar Karandikar, qemu-devel, qemu-riscv
  Cc: Bin Meng, Anup Patel

From: Bin Meng <bin.meng@windriver.com>

Update virt and sifive_u machines to use the opensbi bios image
built for the generic FDT platform.

Remove the out-of-date no longer used bios images.

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

 hw/riscv/sifive_u.c                          |   4 ++--
 hw/riscv/virt.c                              |   4 ++--
 pc-bios/opensbi-riscv32-generic-fw_jump.bin  | Bin 0 -> 58032 bytes
 pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin | Bin 49520 -> 0 bytes
 pc-bios/opensbi-riscv32-virt-fw_jump.bin     | Bin 49504 -> 0 bytes
 pc-bios/opensbi-riscv64-generic-fw_jump.bin  | Bin 0 -> 66680 bytes
 pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin | Bin 57936 -> 0 bytes
 pc-bios/opensbi-riscv64-virt-fw_jump.bin     | Bin 57920 -> 0 bytes
 8 files changed, 4 insertions(+), 4 deletions(-)
 create mode 100644 pc-bios/opensbi-riscv32-generic-fw_jump.bin
 delete mode 100644 pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
 delete mode 100644 pc-bios/opensbi-riscv32-virt-fw_jump.bin
 create mode 100644 pc-bios/opensbi-riscv64-generic-fw_jump.bin
 delete mode 100644 pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
 delete mode 100644 pc-bios/opensbi-riscv64-virt-fw_jump.bin

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index bed10fc..cf015cc 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -58,9 +58,9 @@
 #include <libfdt.h>
 
 #if defined(TARGET_RISCV32)
-# define BIOS_FILENAME "opensbi-riscv32-sifive_u-fw_jump.bin"
+# define BIOS_FILENAME "opensbi-riscv32-generic-fw_jump.bin"
 #else
-# define BIOS_FILENAME "opensbi-riscv64-sifive_u-fw_jump.bin"
+# define BIOS_FILENAME "opensbi-riscv64-generic-fw_jump.bin"
 #endif
 
 static const struct MemmapEntry {
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index daae3eb..0c00d93 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -46,9 +46,9 @@
 #include <libfdt.h>
 
 #if defined(TARGET_RISCV32)
-# define BIOS_FILENAME "opensbi-riscv32-virt-fw_jump.bin"
+# define BIOS_FILENAME "opensbi-riscv32-generic-fw_jump.bin"
 #else
-# define BIOS_FILENAME "opensbi-riscv64-virt-fw_jump.bin"
+# define BIOS_FILENAME "opensbi-riscv64-generic-fw_jump.bin"
 #endif
 
 static const struct MemmapEntry {
diff --git a/pc-bios/opensbi-riscv32-generic-fw_jump.bin b/pc-bios/opensbi-riscv32-generic-fw_jump.bin
new file mode 100644
index 0000000..dc39398
Binary files /dev/null and b/pc-bios/opensbi-riscv32-generic-fw_jump.bin differ
diff --git a/pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin b/pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
deleted file mode 100644
index 3e0da54..0000000
Binary files a/pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin and /dev/null differ
diff --git a/pc-bios/opensbi-riscv32-virt-fw_jump.bin b/pc-bios/opensbi-riscv32-virt-fw_jump.bin
deleted file mode 100644
index bc56ed6..0000000
Binary files a/pc-bios/opensbi-riscv32-virt-fw_jump.bin and /dev/null differ
diff --git a/pc-bios/opensbi-riscv64-generic-fw_jump.bin b/pc-bios/opensbi-riscv64-generic-fw_jump.bin
new file mode 100644
index 0000000..c8f209a
Binary files /dev/null and b/pc-bios/opensbi-riscv64-generic-fw_jump.bin differ
diff --git a/pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin b/pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
deleted file mode 100644
index 1acee86..0000000
Binary files a/pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin and /dev/null differ
diff --git a/pc-bios/opensbi-riscv64-virt-fw_jump.bin b/pc-bios/opensbi-riscv64-virt-fw_jump.bin
deleted file mode 100644
index c62f2b4..0000000
Binary files a/pc-bios/opensbi-riscv64-virt-fw_jump.bin and /dev/null differ
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/5] riscv/spike: Change the default bios to use generic platform image
  2020-05-01 15:50 [PATCH 0/5] riscv: Switch to use generic platform of opensbi bios images Bin Meng
                   ` (2 preceding siblings ...)
  2020-05-01 15:50 ` [PATCH 3/5] riscv: Use pre-built bios image of generic platform for virt & sifive_u Bin Meng
@ 2020-05-01 15:50 ` Bin Meng
  2020-05-03  4:41   ` Anup Patel
  2020-05-04 15:54   ` Alistair Francis
  2020-05-01 15:50 ` [PATCH 5/5] riscv: Suppress the error report for QEMU testing with riscv_find_firmware() Bin Meng
  4 siblings, 2 replies; 20+ messages in thread
From: Bin Meng @ 2020-05-01 15:50 UTC (permalink / raw)
  To: Alistair Francis, Bastian Koppelmann, Palmer Dabbelt,
	Sagar Karandikar, qemu-devel, qemu-riscv
  Cc: Bin Meng, Anup Patel

From: Bin Meng <bin.meng@windriver.com>

To keep sync with other RISC-V machines, change the default bios
to use generic platform image.

While we are here, add some comments to mention that keeping ELF
files here was intentional.

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

 hw/riscv/spike.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index d0c4843..6f26fcf 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -45,10 +45,15 @@
 
 #include <libfdt.h>
 
+/*
+ * Not like other RISC-V machines that use plain binary bios images,
+ * keeping ELF files here was intentional because BIN files don't work
+ * for the Spike machine as HTIF emulation depends on ELF parsing.
+ */
 #if defined(TARGET_RISCV32)
-# define BIOS_FILENAME "opensbi-riscv32-spike-fw_jump.elf"
+# define BIOS_FILENAME "opensbi-riscv32-generic-fw_jump.elf"
 #else
-# define BIOS_FILENAME "opensbi-riscv64-spike-fw_jump.elf"
+# define BIOS_FILENAME "opensbi-riscv64-generic-fw_jump.elf"
 #endif
 
 static const struct MemmapEntry {
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/5] riscv: Suppress the error report for QEMU testing with riscv_find_firmware()
  2020-05-01 15:50 [PATCH 0/5] riscv: Switch to use generic platform of opensbi bios images Bin Meng
                   ` (3 preceding siblings ...)
  2020-05-01 15:50 ` [PATCH 4/5] riscv/spike: Change the default bios to use generic platform image Bin Meng
@ 2020-05-01 15:50 ` Bin Meng
  2020-05-03  4:44   ` Anup Patel
  4 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2020-05-01 15:50 UTC (permalink / raw)
  To: Alistair Francis, Bastian Koppelmann, Palmer Dabbelt,
	Sagar Karandikar, qemu-devel, qemu-riscv
  Cc: Bin Meng, Anup Patel

From: Bin Meng <bin.meng@windriver.com>

We only ship plain binary bios images in the QEMU source. With Spike
machine that uses ELF images as the default bios, running QEMU test
will complain hence let's suppress the error report for QEMU testing.

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

---

 hw/riscv/boot.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index b76b2f3..adb421b 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -65,9 +65,17 @@ char *riscv_find_firmware(const char *firmware_filename)
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, firmware_filename);
     if (filename == NULL) {
-        error_report("Unable to load the RISC-V firmware \"%s\"",
-                     firmware_filename);
-        exit(1);
+        if (!qtest_enabled()) {
+            /*
+             * We only ship plain binary bios images in the QEMU source.
+             * With Spike machine that uses ELF images as the default bios,
+             * running QEMU test will complain hence let's suppress the error
+             * report for QEMU testing.
+             */
+            error_report("Unable to load the RISC-V firmware \"%s\"",
+                         firmware_filename);
+            exit(1);
+        }
     }
 
     return filename;
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
  2020-05-01 15:50 ` [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform Bin Meng
@ 2020-05-03  4:38   ` Anup Patel
  2020-05-04  7:16     ` Bin Meng
  0 siblings, 1 reply; 20+ messages in thread
From: Anup Patel @ 2020-05-03  4:38 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/5] gitlab-ci/opensbi: Update GitLab CI to build generic platform
  2020-05-01 15:50 ` [PATCH 2/5] gitlab-ci/opensbi: Update GitLab CI to build " Bin Meng
@ 2020-05-03  4:40   ` Anup Patel
  0 siblings, 0 replies; 20+ messages in thread
From: Anup Patel @ 2020-05-03  4:40 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

On Fri, May 1, 2020 at 9:24 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This updates the GitLab CI opensbi job to build opensbi bios images
> for the generic platform.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  .gitlab-ci-opensbi.yml | 26 ++++++++------------------
>  1 file changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/.gitlab-ci-opensbi.yml b/.gitlab-ci-opensbi.yml
> index dd051c0..26092bb 100644
> --- a/.gitlab-ci-opensbi.yml
> +++ b/.gitlab-ci-opensbi.yml
> @@ -34,18 +34,12 @@ build-opensbi:
>     when: always
>   artifacts:
>     paths: # 'artifacts.zip' will contains the following files:
> -   - pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
> -   - pc-bios/opensbi-riscv32-virt-fw_jump.bin
> -   - pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
> -   - pc-bios/opensbi-riscv64-virt-fw_jump.bin
> -   - opensbi32-virt-stdout.log
> -   - opensbi32-virt-stderr.log
> -   - opensbi64-virt-stdout.log
> -   - opensbi64-virt-stderr.log
> -   - opensbi32-sifive_u-stdout.log
> -   - opensbi32-sifive_u-stderr.log
> -   - opensbi64-sifive_u-stdout.log
> -   - opensbi64-sifive_u-stderr.log
> +   - pc-bios/opensbi-riscv32-generic-fw_jump.bin
> +   - pc-bios/opensbi-riscv64-generic-fw_jump.bin

Same comment as PATCH1

We should add generic-fw_jump.elf here as well.

> +   - opensbi32-generic-stdout.log
> +   - opensbi32-generic-stderr.log
> +   - opensbi64-generic-stdout.log
> +   - opensbi64-generic-stderr.log
>   image: $CI_REGISTRY_IMAGE:opensbi-cross-build
>   variables:
>     GIT_DEPTH: 3
> @@ -54,10 +48,6 @@ build-opensbi:
>   - export JOBS=$(($(getconf _NPROCESSORS_ONLN) + 1))
>   - echo "=== Using ${JOBS} simultaneous jobs ==="
>   - make -j${JOBS} -C roms/opensbi clean
> - - make -j${JOBS} -C roms opensbi32-virt 2>&1 1>opensbi32-virt-stdout.log | tee -a opensbi32-virt-stderr.log >&2
> + - make -j${JOBS} -C roms opensbi32-generic 2>&1 1>opensbi32-generic-stdout.log | tee -a opensbi32-generic-stderr.log >&2
>   - make -j${JOBS} -C roms/opensbi clean
> - - make -j${JOBS} -C roms opensbi64-virt 2>&1 1>opensbi64-virt-stdout.log | tee -a opensbi64-virt-stderr.log >&2
> - - make -j${JOBS} -C roms/opensbi clean
> - - make -j${JOBS} -C roms opensbi32-sifive_u 2>&1 1>opensbi32-sifive_u-stdout.log | tee -a opensbi32-sifive_u-stderr.log >&2
> - - make -j${JOBS} -C roms/opensbi clean
> - - make -j${JOBS} -C roms opensbi64-sifive_u 2>&1 1>opensbi64-sifive_u-stdout.log | tee -a opensbi64-sifive_u-stderr.log >&2
> + - make -j${JOBS} -C roms opensbi64-generic 2>&1 1>opensbi64-generic-stdout.log | tee -a opensbi64-generic-stderr.log >&2
> --
> 2.7.4
>
>

Otherwise looks good to me.

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

Regards,
Anup


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/5] riscv/spike: Change the default bios to use generic platform image
  2020-05-01 15:50 ` [PATCH 4/5] riscv/spike: Change the default bios to use generic platform image Bin Meng
@ 2020-05-03  4:41   ` Anup Patel
  2020-05-04 15:54   ` Alistair Francis
  1 sibling, 0 replies; 20+ messages in thread
From: Anup Patel @ 2020-05-03  4:41 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

On Fri, May 1, 2020 at 9:25 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> To keep sync with other RISC-V machines, change the default bios
> to use generic platform image.
>
> While we are here, add some comments to mention that keeping ELF
> files here was intentional.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  hw/riscv/spike.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index d0c4843..6f26fcf 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -45,10 +45,15 @@
>
>  #include <libfdt.h>
>
> +/*
> + * Not like other RISC-V machines that use plain binary bios images,
> + * keeping ELF files here was intentional because BIN files don't work
> + * for the Spike machine as HTIF emulation depends on ELF parsing.
> + */
>  #if defined(TARGET_RISCV32)
> -# define BIOS_FILENAME "opensbi-riscv32-spike-fw_jump.elf"
> +# define BIOS_FILENAME "opensbi-riscv32-generic-fw_jump.elf"
>  #else
> -# define BIOS_FILENAME "opensbi-riscv64-spike-fw_jump.elf"
> +# define BIOS_FILENAME "opensbi-riscv64-generic-fw_jump.elf"
>  #endif
>
>  static const struct MemmapEntry {
> --
> 2.7.4
>
>

Looks good to me.

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

Regards,
Anup


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] riscv: Use pre-built bios image of generic platform for virt & sifive_u
  2020-05-01 15:50 ` [PATCH 3/5] riscv: Use pre-built bios image of generic platform for virt & sifive_u Bin Meng
@ 2020-05-03  4:43   ` Anup Patel
  2020-05-04 15:53   ` Alistair Francis
  1 sibling, 0 replies; 20+ messages in thread
From: Anup Patel @ 2020-05-03  4:43 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

On Fri, May 1, 2020 at 9:26 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Update virt and sifive_u machines to use the opensbi bios image
> built for the generic FDT platform.
>
> Remove the out-of-date no longer used bios images.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  hw/riscv/sifive_u.c                          |   4 ++--
>  hw/riscv/virt.c                              |   4 ++--
>  pc-bios/opensbi-riscv32-generic-fw_jump.bin  | Bin 0 -> 58032 bytes
>  pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin | Bin 49520 -> 0 bytes
>  pc-bios/opensbi-riscv32-virt-fw_jump.bin     | Bin 49504 -> 0 bytes
>  pc-bios/opensbi-riscv64-generic-fw_jump.bin  | Bin 0 -> 66680 bytes
>  pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin | Bin 57936 -> 0 bytes
>  pc-bios/opensbi-riscv64-virt-fw_jump.bin     | Bin 57920 -> 0 bytes
>  8 files changed, 4 insertions(+), 4 deletions(-)
>  create mode 100644 pc-bios/opensbi-riscv32-generic-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv32-virt-fw_jump.bin
>  create mode 100644 pc-bios/opensbi-riscv64-generic-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv64-virt-fw_jump.bin
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index bed10fc..cf015cc 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -58,9 +58,9 @@
>  #include <libfdt.h>
>
>  #if defined(TARGET_RISCV32)
> -# define BIOS_FILENAME "opensbi-riscv32-sifive_u-fw_jump.bin"
> +# define BIOS_FILENAME "opensbi-riscv32-generic-fw_jump.bin"
>  #else
> -# define BIOS_FILENAME "opensbi-riscv64-sifive_u-fw_jump.bin"
> +# define BIOS_FILENAME "opensbi-riscv64-generic-fw_jump.bin"
>  #endif
>
>  static const struct MemmapEntry {
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index daae3eb..0c00d93 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -46,9 +46,9 @@
>  #include <libfdt.h>
>
>  #if defined(TARGET_RISCV32)
> -# define BIOS_FILENAME "opensbi-riscv32-virt-fw_jump.bin"
> +# define BIOS_FILENAME "opensbi-riscv32-generic-fw_jump.bin"
>  #else
> -# define BIOS_FILENAME "opensbi-riscv64-virt-fw_jump.bin"
> +# define BIOS_FILENAME "opensbi-riscv64-generic-fw_jump.bin"
>  #endif
>
>  static const struct MemmapEntry {
> diff --git a/pc-bios/opensbi-riscv32-generic-fw_jump.bin b/pc-bios/opensbi-riscv32-generic-fw_jump.bin
> new file mode 100644
> index 0000000..dc39398
> Binary files /dev/null and b/pc-bios/opensbi-riscv32-generic-fw_jump.bin differ
> diff --git a/pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin b/pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
> deleted file mode 100644
> index 3e0da54..0000000
> Binary files a/pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin and /dev/null differ
> diff --git a/pc-bios/opensbi-riscv32-virt-fw_jump.bin b/pc-bios/opensbi-riscv32-virt-fw_jump.bin
> deleted file mode 100644
> index bc56ed6..0000000
> Binary files a/pc-bios/opensbi-riscv32-virt-fw_jump.bin and /dev/null differ
> diff --git a/pc-bios/opensbi-riscv64-generic-fw_jump.bin b/pc-bios/opensbi-riscv64-generic-fw_jump.bin
> new file mode 100644
> index 0000000..c8f209a
> Binary files /dev/null and b/pc-bios/opensbi-riscv64-generic-fw_jump.bin differ
> diff --git a/pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin b/pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
> deleted file mode 100644
> index 1acee86..0000000
> Binary files a/pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin and /dev/null differ
> diff --git a/pc-bios/opensbi-riscv64-virt-fw_jump.bin b/pc-bios/opensbi-riscv64-virt-fw_jump.bin
> deleted file mode 100644
> index c62f2b4..0000000
> Binary files a/pc-bios/opensbi-riscv64-virt-fw_jump.bin and /dev/null differ
> --
> 2.7.4
>
>

Looks good to me.

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

Regards,
Anup


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/5] riscv: Suppress the error report for QEMU testing with riscv_find_firmware()
  2020-05-01 15:50 ` [PATCH 5/5] riscv: Suppress the error report for QEMU testing with riscv_find_firmware() Bin Meng
@ 2020-05-03  4:44   ` Anup Patel
  0 siblings, 0 replies; 20+ messages in thread
From: Anup Patel @ 2020-05-03  4:44 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

On Fri, May 1, 2020 at 9:25 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> We only ship plain binary bios images in the QEMU source. With Spike
> machine that uses ELF images as the default bios, running QEMU test
> will complain hence let's suppress the error report for QEMU testing.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
>  hw/riscv/boot.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index b76b2f3..adb421b 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -65,9 +65,17 @@ char *riscv_find_firmware(const char *firmware_filename)
>
>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, firmware_filename);
>      if (filename == NULL) {
> -        error_report("Unable to load the RISC-V firmware \"%s\"",
> -                     firmware_filename);
> -        exit(1);
> +        if (!qtest_enabled()) {
> +            /*
> +             * We only ship plain binary bios images in the QEMU source.
> +             * With Spike machine that uses ELF images as the default bios,
> +             * running QEMU test will complain hence let's suppress the error
> +             * report for QEMU testing.
> +             */
> +            error_report("Unable to load the RISC-V firmware \"%s\"",
> +                         firmware_filename);
> +            exit(1);
> +        }
>      }
>
>      return filename;
> --
> 2.7.4
>
>

Looks good to me.

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

Regards,
Anup


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
  2020-05-03  4:38   ` Anup Patel
@ 2020-05-04  7:16     ` Bin Meng
  2020-05-04  7:51       ` Anup Patel
  0 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2020-05-04  7:16 UTC (permalink / raw)
  To: Anup Patel
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
  2020-05-04  7:16     ` Bin Meng
@ 2020-05-04  7:51       ` Anup Patel
  2020-05-04  8:00         ` Bin Meng
  0 siblings, 1 reply; 20+ messages in thread
From: Anup Patel @ 2020-05-04  7:51 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
  2020-05-04  7:51       ` Anup Patel
@ 2020-05-04  8:00         ` Bin Meng
  2020-05-04  8:04           ` Bin Meng
  0 siblings, 1 reply; 20+ messages in thread
From: Bin Meng @ 2020-05-04  8:00 UTC (permalink / raw)
  To: Anup Patel
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
  2020-05-04  8:00         ` Bin Meng
@ 2020-05-04  8:04           ` Bin Meng
  2020-05-04  8:16             ` Anup Patel
  2020-05-04 15:51             ` Alistair Francis
  0 siblings, 2 replies; 20+ messages in thread
From: Bin Meng @ 2020-05-04  8:04 UTC (permalink / raw)
  To: Anup Patel
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
  2020-05-04  8:04           ` Bin Meng
@ 2020-05-04  8:16             ` Anup Patel
  2020-05-04 15:51             ` Alistair Francis
  1 sibling, 0 replies; 20+ messages in thread
From: Anup Patel @ 2020-05-04  8:16 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
  2020-05-04  8:04           ` Bin Meng
  2020-05-04  8:16             ` Anup Patel
@ 2020-05-04 15:51             ` Alistair Francis
  2020-05-06 10:00               ` Bin Meng
  1 sibling, 1 reply; 20+ messages in thread
From: Alistair Francis @ 2020-05-04 15:51 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Anup Patel,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis,
	Bastian Koppelmann

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
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/5] riscv: Use pre-built bios image of generic platform for virt & sifive_u
  2020-05-01 15:50 ` [PATCH 3/5] riscv: Use pre-built bios image of generic platform for virt & sifive_u Bin Meng
  2020-05-03  4:43   ` Anup Patel
@ 2020-05-04 15:53   ` Alistair Francis
  1 sibling, 0 replies; 20+ messages in thread
From: Alistair Francis @ 2020-05-04 15:53 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, qemu-devel@nongnu.org Developers, Anup Patel,
	Alistair Francis

On Fri, May 1, 2020 at 8:56 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Update virt and sifive_u machines to use the opensbi bios image
> built for the generic FDT platform.
>
> Remove the out-of-date no longer used bios images.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
>  hw/riscv/sifive_u.c                          |   4 ++--
>  hw/riscv/virt.c                              |   4 ++--
>  pc-bios/opensbi-riscv32-generic-fw_jump.bin  | Bin 0 -> 58032 bytes
>  pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin | Bin 49520 -> 0 bytes
>  pc-bios/opensbi-riscv32-virt-fw_jump.bin     | Bin 49504 -> 0 bytes
>  pc-bios/opensbi-riscv64-generic-fw_jump.bin  | Bin 0 -> 66680 bytes
>  pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin | Bin 57936 -> 0 bytes
>  pc-bios/opensbi-riscv64-virt-fw_jump.bin     | Bin 57920 -> 0 bytes
>  8 files changed, 4 insertions(+), 4 deletions(-)
>  create mode 100644 pc-bios/opensbi-riscv32-generic-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv32-virt-fw_jump.bin
>  create mode 100644 pc-bios/opensbi-riscv64-generic-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv64-virt-fw_jump.bin
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index bed10fc..cf015cc 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -58,9 +58,9 @@
>  #include <libfdt.h>
>
>  #if defined(TARGET_RISCV32)
> -# define BIOS_FILENAME "opensbi-riscv32-sifive_u-fw_jump.bin"
> +# define BIOS_FILENAME "opensbi-riscv32-generic-fw_jump.bin"
>  #else
> -# define BIOS_FILENAME "opensbi-riscv64-sifive_u-fw_jump.bin"
> +# define BIOS_FILENAME "opensbi-riscv64-generic-fw_jump.bin"
>  #endif
>
>  static const struct MemmapEntry {
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index daae3eb..0c00d93 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -46,9 +46,9 @@
>  #include <libfdt.h>
>
>  #if defined(TARGET_RISCV32)
> -# define BIOS_FILENAME "opensbi-riscv32-virt-fw_jump.bin"
> +# define BIOS_FILENAME "opensbi-riscv32-generic-fw_jump.bin"
>  #else
> -# define BIOS_FILENAME "opensbi-riscv64-virt-fw_jump.bin"
> +# define BIOS_FILENAME "opensbi-riscv64-generic-fw_jump.bin"
>  #endif
>
>  static const struct MemmapEntry {
> diff --git a/pc-bios/opensbi-riscv32-generic-fw_jump.bin b/pc-bios/opensbi-riscv32-generic-fw_jump.bin
> new file mode 100644
> index 0000000..dc39398
> Binary files /dev/null and b/pc-bios/opensbi-riscv32-generic-fw_jump.bin differ
> diff --git a/pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin b/pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
> deleted file mode 100644
> index 3e0da54..0000000
> Binary files a/pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin and /dev/null differ
> diff --git a/pc-bios/opensbi-riscv32-virt-fw_jump.bin b/pc-bios/opensbi-riscv32-virt-fw_jump.bin
> deleted file mode 100644
> index bc56ed6..0000000
> Binary files a/pc-bios/opensbi-riscv32-virt-fw_jump.bin and /dev/null differ
> diff --git a/pc-bios/opensbi-riscv64-generic-fw_jump.bin b/pc-bios/opensbi-riscv64-generic-fw_jump.bin
> new file mode 100644
> index 0000000..c8f209a
> Binary files /dev/null and b/pc-bios/opensbi-riscv64-generic-fw_jump.bin differ
> diff --git a/pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin b/pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
> deleted file mode 100644
> index 1acee86..0000000
> Binary files a/pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin and /dev/null differ
> diff --git a/pc-bios/opensbi-riscv64-virt-fw_jump.bin b/pc-bios/opensbi-riscv64-virt-fw_jump.bin
> deleted file mode 100644
> index c62f2b4..0000000
> Binary files a/pc-bios/opensbi-riscv64-virt-fw_jump.bin and /dev/null differ
> --
> 2.7.4
>
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/5] riscv/spike: Change the default bios to use generic platform image
  2020-05-01 15:50 ` [PATCH 4/5] riscv/spike: Change the default bios to use generic platform image Bin Meng
  2020-05-03  4:41   ` Anup Patel
@ 2020-05-04 15:54   ` Alistair Francis
  1 sibling, 0 replies; 20+ messages in thread
From: Alistair Francis @ 2020-05-04 15:54 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
	Palmer Dabbelt, qemu-devel@nongnu.org Developers, Anup Patel,
	Alistair Francis

On Fri, May 1, 2020 at 8:51 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> To keep sync with other RISC-V machines, change the default bios
> to use generic platform image.
>
> While we are here, add some comments to mention that keeping ELF
> files here was intentional.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
>  hw/riscv/spike.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index d0c4843..6f26fcf 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -45,10 +45,15 @@
>
>  #include <libfdt.h>
>
> +/*
> + * Not like other RISC-V machines that use plain binary bios images,
> + * keeping ELF files here was intentional because BIN files don't work
> + * for the Spike machine as HTIF emulation depends on ELF parsing.
> + */
>  #if defined(TARGET_RISCV32)
> -# define BIOS_FILENAME "opensbi-riscv32-spike-fw_jump.elf"
> +# define BIOS_FILENAME "opensbi-riscv32-generic-fw_jump.elf"
>  #else
> -# define BIOS_FILENAME "opensbi-riscv64-spike-fw_jump.elf"
> +# define BIOS_FILENAME "opensbi-riscv64-generic-fw_jump.elf"
>  #endif
>
>  static const struct MemmapEntry {
> --
> 2.7.4
>
>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform
  2020-05-04 15:51             ` Alistair Francis
@ 2020-05-06 10:00               ` Bin Meng
  0 siblings, 0 replies; 20+ messages in thread
From: Bin Meng @ 2020-05-06 10:00 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Bin Meng, open list:RISC-V, Sagar Karandikar, Anup Patel,
	Palmer Dabbelt, QEMU Developers, Anup Patel, Alistair Francis,
	Bastian Koppelmann

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-05-06 10:03 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-01 15:50 [PATCH 0/5] riscv: Switch to use generic platform of opensbi bios images Bin Meng
2020-05-01 15:50 ` [PATCH 1/5] roms/opensbi: Update to support building bios images for generic platform Bin Meng
2020-05-03  4:38   ` Anup Patel
2020-05-04  7:16     ` Bin Meng
2020-05-04  7:51       ` Anup Patel
2020-05-04  8:00         ` Bin Meng
2020-05-04  8:04           ` Bin Meng
2020-05-04  8:16             ` Anup Patel
2020-05-04 15:51             ` Alistair Francis
2020-05-06 10:00               ` Bin Meng
2020-05-01 15:50 ` [PATCH 2/5] gitlab-ci/opensbi: Update GitLab CI to build " Bin Meng
2020-05-03  4:40   ` Anup Patel
2020-05-01 15:50 ` [PATCH 3/5] riscv: Use pre-built bios image of generic platform for virt & sifive_u Bin Meng
2020-05-03  4:43   ` Anup Patel
2020-05-04 15:53   ` Alistair Francis
2020-05-01 15:50 ` [PATCH 4/5] riscv/spike: Change the default bios to use generic platform image Bin Meng
2020-05-03  4:41   ` Anup Patel
2020-05-04 15:54   ` Alistair Francis
2020-05-01 15:50 ` [PATCH 5/5] riscv: Suppress the error report for QEMU testing with riscv_find_firmware() Bin Meng
2020-05-03  4:44   ` Anup Patel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).