* [PATCH 1/3] riscv: Unify Qemu's reset vector code path
2020-06-16 19:26 [PATCH 0/3] Add OpenSBI dynamic firmware support Atish Patra
@ 2020-06-16 19:26 ` Atish Patra
2020-06-18 8:02 ` Bin Meng
2020-06-19 16:37 ` Alexander Richardson
2020-06-16 19:26 ` [PATCH 2/3] RISC-V: Copy the fdt in dram instead of ROM Atish Patra
` (3 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Atish Patra @ 2020-06-16 19:26 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann, Atish Patra,
Alistair Francis, Palmer Dabbelt
Currently, all riscv machines have identical reset vector code
implementations with memory addresses being different for all machines.
They can be easily combined into a single function in common code.
Move it to common function and let all the machines use the common function.
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
hw/riscv/boot.c | 46 +++++++++++++++++++++++++++++++++++++++++
hw/riscv/sifive_u.c | 38 +++-------------------------------
hw/riscv/spike.c | 38 +++-------------------------------
hw/riscv/virt.c | 37 +++------------------------------
include/hw/riscv/boot.h | 2 ++
5 files changed, 57 insertions(+), 104 deletions(-)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index adb421b91b68..8ed96da600c9 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -22,12 +22,16 @@
#include "qemu/units.h"
#include "qemu/error-report.h"
#include "exec/cpu-defs.h"
+#include "exec/address-spaces.h"
#include "hw/boards.h"
#include "hw/loader.h"
#include "hw/riscv/boot.h"
#include "elf.h"
+#include "sysemu/device_tree.h"
#include "sysemu/qtest.h"
+#include <libfdt.h>
+
#if defined(TARGET_RISCV32)
# define KERNEL_BOOT_ADDRESS 0x80400000
#else
@@ -155,3 +159,45 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
return *start + size;
}
+
+void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
+ hwaddr rom_size, void *fdt)
+{
+ int i;
+ /* reset vector */
+ uint32_t reset_vec[8] = {
+ 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
+ 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
+ 0xf1402573, /* csrr a0, mhartid */
+#if defined(TARGET_RISCV32)
+ 0x0182a283, /* lw t0, 24(t0) */
+#elif defined(TARGET_RISCV64)
+ 0x0182b283, /* ld t0, 24(t0) */
+#endif
+ 0x00028067, /* jr t0 */
+ 0x00000000,
+ start_addr, /* start: .dword */
+ 0x00000000,
+ /* dtb: */
+ };
+
+ /* copy in the reset vector in little_endian byte order */
+ for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
+ reset_vec[i] = cpu_to_le32(reset_vec[i]);
+ }
+ rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
+ rom_base, &address_space_memory);
+
+ /* copy in the device tree */
+ if (fdt_pack(fdt) || fdt_totalsize(fdt) >
+ rom_size - sizeof(reset_vec)) {
+ error_report("not enough space to store device-tree");
+ exit(1);
+ }
+ qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
+ rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
+ rom_base + sizeof(reset_vec),
+ &address_space_memory);
+
+ return;
+}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index f9fef2be9170..c2712570e0d9 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -325,7 +325,6 @@ static void sifive_u_machine_init(MachineState *machine)
MemoryRegion *main_mem = g_new(MemoryRegion, 1);
MemoryRegion *flash0 = g_new(MemoryRegion, 1);
target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
- int i;
/* Initialize SoC */
object_initialize_child(OBJECT(machine), "soc", &s->soc,
@@ -374,40 +373,9 @@ static void sifive_u_machine_init(MachineState *machine)
start_addr = memmap[SIFIVE_U_FLASH0].base;
}
- /* reset vector */
- uint32_t reset_vec[8] = {
- 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
- 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
- 0xf1402573, /* csrr a0, mhartid */
-#if defined(TARGET_RISCV32)
- 0x0182a283, /* lw t0, 24(t0) */
-#elif defined(TARGET_RISCV64)
- 0x0182b283, /* ld t0, 24(t0) */
-#endif
- 0x00028067, /* jr t0 */
- 0x00000000,
- start_addr, /* start: .dword */
- 0x00000000,
- /* dtb: */
- };
-
- /* copy in the reset vector in little_endian byte order */
- for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
- reset_vec[i] = cpu_to_le32(reset_vec[i]);
- }
- rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
- memmap[SIFIVE_U_MROM].base, &address_space_memory);
-
- /* copy in the device tree */
- if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
- memmap[SIFIVE_U_MROM].size - sizeof(reset_vec)) {
- error_report("not enough space to store device-tree");
- exit(1);
- }
- qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
- rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
- memmap[SIFIVE_U_MROM].base + sizeof(reset_vec),
- &address_space_memory);
+ /* load the reset vector */
+ riscv_setup_rom_reset_vec(start_addr, memmap[SIFIVE_U_MROM].base,
+ memmap[SIFIVE_U_MROM].size, s->fdt);
}
static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 7bbbdb50363d..238eae48716a 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -165,7 +165,6 @@ static void spike_board_init(MachineState *machine)
MemoryRegion *system_memory = get_system_memory();
MemoryRegion *main_mem = g_new(MemoryRegion, 1);
MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
- int i;
unsigned int smp_cpus = machine->smp.cpus;
/* Initialize SOC */
@@ -213,40 +212,9 @@ static void spike_board_init(MachineState *machine)
}
}
- /* reset vector */
- uint32_t reset_vec[8] = {
- 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
- 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
- 0xf1402573, /* csrr a0, mhartid */
-#if defined(TARGET_RISCV32)
- 0x0182a283, /* lw t0, 24(t0) */
-#elif defined(TARGET_RISCV64)
- 0x0182b283, /* ld t0, 24(t0) */
-#endif
- 0x00028067, /* jr t0 */
- 0x00000000,
- memmap[SPIKE_DRAM].base, /* start: .dword DRAM_BASE */
- 0x00000000,
- /* dtb: */
- };
-
- /* copy in the reset vector in little_endian byte order */
- for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
- reset_vec[i] = cpu_to_le32(reset_vec[i]);
- }
- rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
- memmap[SPIKE_MROM].base, &address_space_memory);
-
- /* copy in the device tree */
- if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
- memmap[SPIKE_MROM].size - sizeof(reset_vec)) {
- error_report("not enough space to store device-tree");
- exit(1);
- }
- qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
- rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
- memmap[SPIKE_MROM].base + sizeof(reset_vec),
- &address_space_memory);
+ /* load the reset vector */
+ riscv_setup_rom_reset_vec(memmap[SPIKE_DRAM].base, memmap[SPIKE_MROM].base,
+ memmap[SPIKE_MROM].size, s->fdt);
/* initialize HTIF using symbols found in load_kernel */
htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hd(0));
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4e4c494a7050..a8e2d58cc067 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -536,40 +536,9 @@ static void virt_machine_init(MachineState *machine)
start_addr = virt_memmap[VIRT_FLASH].base;
}
- /* reset vector */
- uint32_t reset_vec[8] = {
- 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
- 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
- 0xf1402573, /* csrr a0, mhartid */
-#if defined(TARGET_RISCV32)
- 0x0182a283, /* lw t0, 24(t0) */
-#elif defined(TARGET_RISCV64)
- 0x0182b283, /* ld t0, 24(t0) */
-#endif
- 0x00028067, /* jr t0 */
- 0x00000000,
- start_addr, /* start: .dword */
- 0x00000000,
- /* dtb: */
- };
-
- /* copy in the reset vector in little_endian byte order */
- for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
- reset_vec[i] = cpu_to_le32(reset_vec[i]);
- }
- rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
- memmap[VIRT_MROM].base, &address_space_memory);
-
- /* copy in the device tree */
- if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
- memmap[VIRT_MROM].size - sizeof(reset_vec)) {
- error_report("not enough space to store device-tree");
- exit(1);
- }
- qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
- rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
- memmap[VIRT_MROM].base + sizeof(reset_vec),
- &address_space_memory);
+ /* load the reset vector */
+ riscv_setup_rom_reset_vec(start_addr, virt_memmap[VIRT_MROM].base,
+ virt_memmap[VIRT_MROM].size, s->fdt);
/* create PLIC hart topology configuration string */
plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus;
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index 9daa98da08d7..3e9759c89aa2 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -35,5 +35,7 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
symbol_fn_t sym_cb);
hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
uint64_t kernel_entry, hwaddr *start);
+void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
+ hwaddr rom_size, void *fdt);
#endif /* RISCV_BOOT_H */
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] riscv: Unify Qemu's reset vector code path
2020-06-16 19:26 ` [PATCH 1/3] riscv: Unify Qemu's reset vector code path Atish Patra
@ 2020-06-18 8:02 ` Bin Meng
2020-06-19 1:55 ` Atish Patra
2020-06-19 16:37 ` Alexander Richardson
1 sibling, 1 reply; 14+ messages in thread
From: Bin Meng @ 2020-06-18 8:02 UTC (permalink / raw)
To: Atish Patra
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt
On Wed, Jun 17, 2020 at 3:30 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> Currently, all riscv machines have identical reset vector code
> implementations with memory addresses being different for all machines.
> They can be easily combined into a single function in common code.
>
> Move it to common function and let all the machines use the common function.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
> hw/riscv/boot.c | 46 +++++++++++++++++++++++++++++++++++++++++
> hw/riscv/sifive_u.c | 38 +++-------------------------------
sifive_u's reset vector has to be different to emulate the real
hardware MSEL pin state.
Please rebase this on top of the following series:
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=183567
> hw/riscv/spike.c | 38 +++-------------------------------
> hw/riscv/virt.c | 37 +++------------------------------
> include/hw/riscv/boot.h | 2 ++
> 5 files changed, 57 insertions(+), 104 deletions(-)
>
Regards,
Bin
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] riscv: Unify Qemu's reset vector code path
2020-06-18 8:02 ` Bin Meng
@ 2020-06-19 1:55 ` Atish Patra
0 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2020-06-19 1:55 UTC (permalink / raw)
To: Bin Meng
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Atish Patra, Alistair Francis,
Palmer Dabbelt
On Thu, Jun 18, 2020 at 1:03 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jun 17, 2020 at 3:30 AM Atish Patra <atish.patra@wdc.com> wrote:
> >
> > Currently, all riscv machines have identical reset vector code
> > implementations with memory addresses being different for all machines.
> > They can be easily combined into a single function in common code.
> >
> > Move it to common function and let all the machines use the common function.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> > hw/riscv/boot.c | 46 +++++++++++++++++++++++++++++++++++++++++
> > hw/riscv/sifive_u.c | 38 +++-------------------------------
>
> sifive_u's reset vector has to be different to emulate the real
> hardware MSEL pin state.
> Please rebase this on top of the following series:
> http://patchwork.ozlabs.org/project/qemu-devel/list/?series=183567
>
Sure. I will rebase. I think sifive_u may be used in future to emulate
other sifive boards in future.
This may require additional data in rom. That's why, it's better to
keep the reset vector code in
sifive_u and just unify spike & virt.
> > hw/riscv/spike.c | 38 +++-------------------------------
> > hw/riscv/virt.c | 37 +++------------------------------
> > include/hw/riscv/boot.h | 2 ++
> > 5 files changed, 57 insertions(+), 104 deletions(-)
> >
>
> Regards,
> Bin
>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] riscv: Unify Qemu's reset vector code path
2020-06-16 19:26 ` [PATCH 1/3] riscv: Unify Qemu's reset vector code path Atish Patra
2020-06-18 8:02 ` Bin Meng
@ 2020-06-19 16:37 ` Alexander Richardson
2020-06-19 20:34 ` Atish Patra
1 sibling, 1 reply; 14+ messages in thread
From: Alexander Richardson @ 2020-06-19 16:37 UTC (permalink / raw)
To: Atish Patra
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt
On Tue, 16 Jun 2020 at 20:30, Atish Patra <atish.patra@wdc.com> wrote:
>
> Currently, all riscv machines have identical reset vector code
> implementations with memory addresses being different for all machines.
> They can be easily combined into a single function in common code.
>
> Move it to common function and let all the machines use the common function.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
> hw/riscv/boot.c | 46 +++++++++++++++++++++++++++++++++++++++++
> hw/riscv/sifive_u.c | 38 +++-------------------------------
> hw/riscv/spike.c | 38 +++-------------------------------
> hw/riscv/virt.c | 37 +++------------------------------
> include/hw/riscv/boot.h | 2 ++
> 5 files changed, 57 insertions(+), 104 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index adb421b91b68..8ed96da600c9 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -22,12 +22,16 @@
> #include "qemu/units.h"
> #include "qemu/error-report.h"
> #include "exec/cpu-defs.h"
> +#include "exec/address-spaces.h"
> #include "hw/boards.h"
> #include "hw/loader.h"
> #include "hw/riscv/boot.h"
> #include "elf.h"
> +#include "sysemu/device_tree.h"
> #include "sysemu/qtest.h"
>
> +#include <libfdt.h>
> +
> #if defined(TARGET_RISCV32)
> # define KERNEL_BOOT_ADDRESS 0x80400000
> #else
> @@ -155,3 +159,45 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
>
> return *start + size;
> }
> +
> +void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
> + hwaddr rom_size, void *fdt)
> +{
> + int i;
> + /* reset vector */
> + uint32_t reset_vec[8] = {
> + 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> + 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> + 0xf1402573, /* csrr a0, mhartid */
> +#if defined(TARGET_RISCV32)
> + 0x0182a283, /* lw t0, 24(t0) */
> +#elif defined(TARGET_RISCV64)
> + 0x0182b283, /* ld t0, 24(t0) */
> +#endif
> + 0x00028067, /* jr t0 */
> + 0x00000000,
> + start_addr, /* start: .dword */
> + 0x00000000,
> + /* dtb: */
> + };
> +
> + /* copy in the reset vector in little_endian byte order */
> + for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> + reset_vec[i] = cpu_to_le32(reset_vec[i]);
> + }
Maybe use ARRAY_SIZE(reset_vec) instead of sizeof(reset_vec) >> 2 ?
> + rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> + rom_base, &address_space_memory);
> +
> + /* copy in the device tree */
> + if (fdt_pack(fdt) || fdt_totalsize(fdt) >
> + rom_size - sizeof(reset_vec)) {
> + error_report("not enough space to store device-tree");
> + exit(1);
> + }
> + qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> + rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
> + rom_base + sizeof(reset_vec),
> + &address_space_memory);
> +
> + return;
> +}
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index f9fef2be9170..c2712570e0d9 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -325,7 +325,6 @@ static void sifive_u_machine_init(MachineState *machine)
> MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> MemoryRegion *flash0 = g_new(MemoryRegion, 1);
> target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
> - int i;
>
> /* Initialize SoC */
> object_initialize_child(OBJECT(machine), "soc", &s->soc,
> @@ -374,40 +373,9 @@ static void sifive_u_machine_init(MachineState *machine)
> start_addr = memmap[SIFIVE_U_FLASH0].base;
> }
>
> - /* reset vector */
> - uint32_t reset_vec[8] = {
> - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> - 0xf1402573, /* csrr a0, mhartid */
> -#if defined(TARGET_RISCV32)
> - 0x0182a283, /* lw t0, 24(t0) */
> -#elif defined(TARGET_RISCV64)
> - 0x0182b283, /* ld t0, 24(t0) */
> -#endif
> - 0x00028067, /* jr t0 */
> - 0x00000000,
> - start_addr, /* start: .dword */
> - 0x00000000,
> - /* dtb: */
> - };
> -
> - /* copy in the reset vector in little_endian byte order */
> - for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> - reset_vec[i] = cpu_to_le32(reset_vec[i]);
> - }
> - rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> - memmap[SIFIVE_U_MROM].base, &address_space_memory);
> -
> - /* copy in the device tree */
> - if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
> - memmap[SIFIVE_U_MROM].size - sizeof(reset_vec)) {
> - error_report("not enough space to store device-tree");
> - exit(1);
> - }
> - qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
> - rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
> - memmap[SIFIVE_U_MROM].base + sizeof(reset_vec),
> - &address_space_memory);
> + /* load the reset vector */
> + riscv_setup_rom_reset_vec(start_addr, memmap[SIFIVE_U_MROM].base,
> + memmap[SIFIVE_U_MROM].size, s->fdt);
> }
>
> static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 7bbbdb50363d..238eae48716a 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -165,7 +165,6 @@ static void spike_board_init(MachineState *machine)
> MemoryRegion *system_memory = get_system_memory();
> MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> - int i;
> unsigned int smp_cpus = machine->smp.cpus;
>
> /* Initialize SOC */
> @@ -213,40 +212,9 @@ static void spike_board_init(MachineState *machine)
> }
> }
>
> - /* reset vector */
> - uint32_t reset_vec[8] = {
> - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> - 0xf1402573, /* csrr a0, mhartid */
> -#if defined(TARGET_RISCV32)
> - 0x0182a283, /* lw t0, 24(t0) */
> -#elif defined(TARGET_RISCV64)
> - 0x0182b283, /* ld t0, 24(t0) */
> -#endif
> - 0x00028067, /* jr t0 */
> - 0x00000000,
> - memmap[SPIKE_DRAM].base, /* start: .dword DRAM_BASE */
> - 0x00000000,
> - /* dtb: */
> - };
> -
> - /* copy in the reset vector in little_endian byte order */
> - for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> - reset_vec[i] = cpu_to_le32(reset_vec[i]);
> - }
> - rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> - memmap[SPIKE_MROM].base, &address_space_memory);
> -
> - /* copy in the device tree */
> - if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
> - memmap[SPIKE_MROM].size - sizeof(reset_vec)) {
> - error_report("not enough space to store device-tree");
> - exit(1);
> - }
> - qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
> - rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
> - memmap[SPIKE_MROM].base + sizeof(reset_vec),
> - &address_space_memory);
> + /* load the reset vector */
> + riscv_setup_rom_reset_vec(memmap[SPIKE_DRAM].base, memmap[SPIKE_MROM].base,
> + memmap[SPIKE_MROM].size, s->fdt);
>
> /* initialize HTIF using symbols found in load_kernel */
> htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hd(0));
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 4e4c494a7050..a8e2d58cc067 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -536,40 +536,9 @@ static void virt_machine_init(MachineState *machine)
> start_addr = virt_memmap[VIRT_FLASH].base;
> }
>
> - /* reset vector */
> - uint32_t reset_vec[8] = {
> - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> - 0xf1402573, /* csrr a0, mhartid */
> -#if defined(TARGET_RISCV32)
> - 0x0182a283, /* lw t0, 24(t0) */
> -#elif defined(TARGET_RISCV64)
> - 0x0182b283, /* ld t0, 24(t0) */
> -#endif
> - 0x00028067, /* jr t0 */
> - 0x00000000,
> - start_addr, /* start: .dword */
> - 0x00000000,
> - /* dtb: */
> - };
> -
> - /* copy in the reset vector in little_endian byte order */
> - for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> - reset_vec[i] = cpu_to_le32(reset_vec[i]);
> - }
> - rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> - memmap[VIRT_MROM].base, &address_space_memory);
> -
> - /* copy in the device tree */
> - if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
> - memmap[VIRT_MROM].size - sizeof(reset_vec)) {
> - error_report("not enough space to store device-tree");
> - exit(1);
> - }
> - qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
> - rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
> - memmap[VIRT_MROM].base + sizeof(reset_vec),
> - &address_space_memory);
> + /* load the reset vector */
> + riscv_setup_rom_reset_vec(start_addr, virt_memmap[VIRT_MROM].base,
> + virt_memmap[VIRT_MROM].size, s->fdt);
>
> /* create PLIC hart topology configuration string */
> plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus;
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index 9daa98da08d7..3e9759c89aa2 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -35,5 +35,7 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
> symbol_fn_t sym_cb);
> hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
> uint64_t kernel_entry, hwaddr *start);
> +void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
> + hwaddr rom_size, void *fdt);
>
> #endif /* RISCV_BOOT_H */
> --
> 2.26.2
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/3] riscv: Unify Qemu's reset vector code path
2020-06-19 16:37 ` Alexander Richardson
@ 2020-06-19 20:34 ` Atish Patra
0 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2020-06-19 20:34 UTC (permalink / raw)
To: Alexander Richardson
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Atish Patra, Alistair Francis,
Palmer Dabbelt
On Fri, Jun 19, 2020 at 10:11 AM Alexander Richardson
<Alexander.Richardson@cl.cam.ac.uk> wrote:
>
> On Tue, 16 Jun 2020 at 20:30, Atish Patra <atish.patra@wdc.com> wrote:
> >
> > Currently, all riscv machines have identical reset vector code
> > implementations with memory addresses being different for all machines.
> > They can be easily combined into a single function in common code.
> >
> > Move it to common function and let all the machines use the common function.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> > hw/riscv/boot.c | 46 +++++++++++++++++++++++++++++++++++++++++
> > hw/riscv/sifive_u.c | 38 +++-------------------------------
> > hw/riscv/spike.c | 38 +++-------------------------------
> > hw/riscv/virt.c | 37 +++------------------------------
> > include/hw/riscv/boot.h | 2 ++
> > 5 files changed, 57 insertions(+), 104 deletions(-)
> >
> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > index adb421b91b68..8ed96da600c9 100644
> > --- a/hw/riscv/boot.c
> > +++ b/hw/riscv/boot.c
> > @@ -22,12 +22,16 @@
> > #include "qemu/units.h"
> > #include "qemu/error-report.h"
> > #include "exec/cpu-defs.h"
> > +#include "exec/address-spaces.h"
> > #include "hw/boards.h"
> > #include "hw/loader.h"
> > #include "hw/riscv/boot.h"
> > #include "elf.h"
> > +#include "sysemu/device_tree.h"
> > #include "sysemu/qtest.h"
> >
> > +#include <libfdt.h>
> > +
> > #if defined(TARGET_RISCV32)
> > # define KERNEL_BOOT_ADDRESS 0x80400000
> > #else
> > @@ -155,3 +159,45 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
> >
> > return *start + size;
> > }
> > +
> > +void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
> > + hwaddr rom_size, void *fdt)
> > +{
> > + int i;
> > + /* reset vector */
> > + uint32_t reset_vec[8] = {
> > + 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> > + 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> > + 0xf1402573, /* csrr a0, mhartid */
> > +#if defined(TARGET_RISCV32)
> > + 0x0182a283, /* lw t0, 24(t0) */
> > +#elif defined(TARGET_RISCV64)
> > + 0x0182b283, /* ld t0, 24(t0) */
> > +#endif
> > + 0x00028067, /* jr t0 */
> > + 0x00000000,
> > + start_addr, /* start: .dword */
> > + 0x00000000,
> > + /* dtb: */
> > + };
> > +
> > + /* copy in the reset vector in little_endian byte order */
> > + for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> > + reset_vec[i] = cpu_to_le32(reset_vec[i]);
> > + }
> Maybe use ARRAY_SIZE(reset_vec) instead of sizeof(reset_vec) >> 2 ?
>
Yeah. That's better. Thanks.
> > + rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> > + rom_base, &address_space_memory);
> > +
> > + /* copy in the device tree */
> > + if (fdt_pack(fdt) || fdt_totalsize(fdt) >
> > + rom_size - sizeof(reset_vec)) {
> > + error_report("not enough space to store device-tree");
> > + exit(1);
> > + }
> > + qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> > + rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
> > + rom_base + sizeof(reset_vec),
> > + &address_space_memory);
> > +
> > + return;
> > +}
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index f9fef2be9170..c2712570e0d9 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -325,7 +325,6 @@ static void sifive_u_machine_init(MachineState *machine)
> > MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> > MemoryRegion *flash0 = g_new(MemoryRegion, 1);
> > target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
> > - int i;
> >
> > /* Initialize SoC */
> > object_initialize_child(OBJECT(machine), "soc", &s->soc,
> > @@ -374,40 +373,9 @@ static void sifive_u_machine_init(MachineState *machine)
> > start_addr = memmap[SIFIVE_U_FLASH0].base;
> > }
> >
> > - /* reset vector */
> > - uint32_t reset_vec[8] = {
> > - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> > - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> > - 0xf1402573, /* csrr a0, mhartid */
> > -#if defined(TARGET_RISCV32)
> > - 0x0182a283, /* lw t0, 24(t0) */
> > -#elif defined(TARGET_RISCV64)
> > - 0x0182b283, /* ld t0, 24(t0) */
> > -#endif
> > - 0x00028067, /* jr t0 */
> > - 0x00000000,
> > - start_addr, /* start: .dword */
> > - 0x00000000,
> > - /* dtb: */
> > - };
> > -
> > - /* copy in the reset vector in little_endian byte order */
> > - for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> > - reset_vec[i] = cpu_to_le32(reset_vec[i]);
> > - }
> > - rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> > - memmap[SIFIVE_U_MROM].base, &address_space_memory);
> > -
> > - /* copy in the device tree */
> > - if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
> > - memmap[SIFIVE_U_MROM].size - sizeof(reset_vec)) {
> > - error_report("not enough space to store device-tree");
> > - exit(1);
> > - }
> > - qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
> > - rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
> > - memmap[SIFIVE_U_MROM].base + sizeof(reset_vec),
> > - &address_space_memory);
> > + /* load the reset vector */
> > + riscv_setup_rom_reset_vec(start_addr, memmap[SIFIVE_U_MROM].base,
> > + memmap[SIFIVE_U_MROM].size, s->fdt);
> > }
> >
> > static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
> > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> > index 7bbbdb50363d..238eae48716a 100644
> > --- a/hw/riscv/spike.c
> > +++ b/hw/riscv/spike.c
> > @@ -165,7 +165,6 @@ static void spike_board_init(MachineState *machine)
> > MemoryRegion *system_memory = get_system_memory();
> > MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> > MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> > - int i;
> > unsigned int smp_cpus = machine->smp.cpus;
> >
> > /* Initialize SOC */
> > @@ -213,40 +212,9 @@ static void spike_board_init(MachineState *machine)
> > }
> > }
> >
> > - /* reset vector */
> > - uint32_t reset_vec[8] = {
> > - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> > - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> > - 0xf1402573, /* csrr a0, mhartid */
> > -#if defined(TARGET_RISCV32)
> > - 0x0182a283, /* lw t0, 24(t0) */
> > -#elif defined(TARGET_RISCV64)
> > - 0x0182b283, /* ld t0, 24(t0) */
> > -#endif
> > - 0x00028067, /* jr t0 */
> > - 0x00000000,
> > - memmap[SPIKE_DRAM].base, /* start: .dword DRAM_BASE */
> > - 0x00000000,
> > - /* dtb: */
> > - };
> > -
> > - /* copy in the reset vector in little_endian byte order */
> > - for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> > - reset_vec[i] = cpu_to_le32(reset_vec[i]);
> > - }
> > - rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> > - memmap[SPIKE_MROM].base, &address_space_memory);
> > -
> > - /* copy in the device tree */
> > - if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
> > - memmap[SPIKE_MROM].size - sizeof(reset_vec)) {
> > - error_report("not enough space to store device-tree");
> > - exit(1);
> > - }
> > - qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
> > - rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
> > - memmap[SPIKE_MROM].base + sizeof(reset_vec),
> > - &address_space_memory);
> > + /* load the reset vector */
> > + riscv_setup_rom_reset_vec(memmap[SPIKE_DRAM].base, memmap[SPIKE_MROM].base,
> > + memmap[SPIKE_MROM].size, s->fdt);
> >
> > /* initialize HTIF using symbols found in load_kernel */
> > htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hd(0));
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index 4e4c494a7050..a8e2d58cc067 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -536,40 +536,9 @@ static void virt_machine_init(MachineState *machine)
> > start_addr = virt_memmap[VIRT_FLASH].base;
> > }
> >
> > - /* reset vector */
> > - uint32_t reset_vec[8] = {
> > - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> > - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> > - 0xf1402573, /* csrr a0, mhartid */
> > -#if defined(TARGET_RISCV32)
> > - 0x0182a283, /* lw t0, 24(t0) */
> > -#elif defined(TARGET_RISCV64)
> > - 0x0182b283, /* ld t0, 24(t0) */
> > -#endif
> > - 0x00028067, /* jr t0 */
> > - 0x00000000,
> > - start_addr, /* start: .dword */
> > - 0x00000000,
> > - /* dtb: */
> > - };
> > -
> > - /* copy in the reset vector in little_endian byte order */
> > - for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
> > - reset_vec[i] = cpu_to_le32(reset_vec[i]);
> > - }
> > - rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
> > - memmap[VIRT_MROM].base, &address_space_memory);
> > -
> > - /* copy in the device tree */
> > - if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
> > - memmap[VIRT_MROM].size - sizeof(reset_vec)) {
> > - error_report("not enough space to store device-tree");
> > - exit(1);
> > - }
> > - qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
> > - rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
> > - memmap[VIRT_MROM].base + sizeof(reset_vec),
> > - &address_space_memory);
> > + /* load the reset vector */
> > + riscv_setup_rom_reset_vec(start_addr, virt_memmap[VIRT_MROM].base,
> > + virt_memmap[VIRT_MROM].size, s->fdt);
> >
> > /* create PLIC hart topology configuration string */
> > plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> > index 9daa98da08d7..3e9759c89aa2 100644
> > --- a/include/hw/riscv/boot.h
> > +++ b/include/hw/riscv/boot.h
> > @@ -35,5 +35,7 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
> > symbol_fn_t sym_cb);
> > hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
> > uint64_t kernel_entry, hwaddr *start);
> > +void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
> > + hwaddr rom_size, void *fdt);
> >
> > #endif /* RISCV_BOOT_H */
> > --
> > 2.26.2
> >
> >
>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] RISC-V: Copy the fdt in dram instead of ROM
2020-06-16 19:26 [PATCH 0/3] Add OpenSBI dynamic firmware support Atish Patra
2020-06-16 19:26 ` [PATCH 1/3] riscv: Unify Qemu's reset vector code path Atish Patra
@ 2020-06-16 19:26 ` Atish Patra
2020-06-18 8:25 ` Bin Meng
2020-06-16 19:27 ` [PATCH 3/3] riscv: Add opensbi firmware dynamic support Atish Patra
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Atish Patra @ 2020-06-16 19:26 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann, Atish Patra,
Alistair Francis, Palmer Dabbelt
Currently, the fdt is copied to the ROM after the reset vector. The firmware
has to copy it to DRAM. Instead of this, directly copy the device tree to a
pre-computed dram address. The device tree load address should be as far as
possible from kernel and initrd images. That's why it is kept at the end of
the DRAM or 4GB whichever is lesser.
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
hw/riscv/boot.c | 45 ++++++++++++++++++++++++++++++-----------
hw/riscv/sifive_u.c | 14 ++++++++++++-
hw/riscv/spike.c | 14 ++++++++++++-
hw/riscv/virt.c | 13 +++++++++++-
include/hw/riscv/boot.h | 5 ++++-
5 files changed, 75 insertions(+), 16 deletions(-)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 8ed96da600c9..0378b7f1bd58 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -160,25 +160,51 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
return *start + size;
}
+hwaddr riscv_calc_fdt_load_addr(hwaddr dram_base, uint64_t mem_size, void *fdt)
+{
+ hwaddr temp, fdt_addr;
+ hwaddr dram_end = dram_base + mem_size;
+ int fdtsize = fdt_totalsize(fdt);
+
+ if (fdtsize <= 0) {
+ error_report("invalid device-tree");
+ exit(1);
+ }
+ /*
+ * We should put fdt as far as possible to avoid kernel/initrd overwriting
+ * its content. But it should be addressable by 32 bit system as well.
+ * Thus, put it at an aligned address that less than fdt size from end of
+ * dram or 4GB whichever is lesser.
+ */
+ temp = MIN(dram_end, 4096 * MiB);
+ fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
+
+ return fdt_addr;
+}
+
void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
- hwaddr rom_size, void *fdt)
+ hwaddr rom_size,
+ hwaddr fdt_load_addr, void *fdt)
{
int i;
/* reset vector */
- uint32_t reset_vec[8] = {
- 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
- 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
+ uint32_t reset_vec[10] = {
+ 0x00000297, /* 1: auipc t0, %pcrel_hi(fw_dyn) */
0xf1402573, /* csrr a0, mhartid */
#if defined(TARGET_RISCV32)
+ 0x0202a583, /* lw a1, 32(t0) */
0x0182a283, /* lw t0, 24(t0) */
#elif defined(TARGET_RISCV64)
+ 0x0202b583, /* ld a1, 32(t0) */
0x0182b283, /* ld t0, 24(t0) */
#endif
0x00028067, /* jr t0 */
0x00000000,
start_addr, /* start: .dword */
0x00000000,
- /* dtb: */
+ fdt_load_addr, /* fdt_laddr: .dword */
+ 0x00000000,
+ /* fw_dyn: */
};
/* copy in the reset vector in little_endian byte order */
@@ -189,14 +215,9 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
rom_base, &address_space_memory);
/* copy in the device tree */
- if (fdt_pack(fdt) || fdt_totalsize(fdt) >
- rom_size - sizeof(reset_vec)) {
- error_report("not enough space to store device-tree");
- exit(1);
- }
qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
- rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
- rom_base + sizeof(reset_vec),
+
+ rom_add_blob_fixed_as("fdt", fdt, fdt_totalsize(fdt), fdt_load_addr,
&address_space_memory);
return;
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index c2712570e0d9..1a1540c7f98d 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -31,6 +31,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "qemu/log.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
@@ -325,6 +326,7 @@ static void sifive_u_machine_init(MachineState *machine)
MemoryRegion *main_mem = g_new(MemoryRegion, 1);
MemoryRegion *flash0 = g_new(MemoryRegion, 1);
target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
+ hwaddr fdt_load_addr;
/* Initialize SoC */
object_initialize_child(OBJECT(machine), "soc", &s->soc,
@@ -369,13 +371,23 @@ static void sifive_u_machine_init(MachineState *machine)
}
}
+ /* Compute the fdt load address in dram */
+ fdt_load_addr = riscv_calc_fdt_load_addr(memmap[SIFIVE_U_DRAM].base,
+ machine->ram_size, s->fdt);
+
+ if (fdt_load_addr >= (memmap[SIFIVE_U_DRAM].base + machine->ram_size)) {
+ error_report("Not enough space for FDT after kernel + initrd");
+ exit(1);
+ }
+
if (s->start_in_flash) {
start_addr = memmap[SIFIVE_U_FLASH0].base;
}
/* load the reset vector */
riscv_setup_rom_reset_vec(start_addr, memmap[SIFIVE_U_MROM].base,
- memmap[SIFIVE_U_MROM].size, s->fdt);
+ memmap[SIFIVE_U_MROM].size,
+ fdt_load_addr, s->fdt);
}
static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 238eae48716a..2a34a1382487 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -24,6 +24,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "qemu/log.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
@@ -166,6 +167,7 @@ static void spike_board_init(MachineState *machine)
MemoryRegion *main_mem = g_new(MemoryRegion, 1);
MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
unsigned int smp_cpus = machine->smp.cpus;
+ hwaddr fdt_load_addr;
/* Initialize SOC */
object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
@@ -212,9 +214,19 @@ static void spike_board_init(MachineState *machine)
}
}
+ /* Compute the fdt load address in dram */
+ fdt_load_addr = riscv_calc_fdt_load_addr(memmap[SPIKE_DRAM].base,
+ machine->ram_size, s->fdt);
+
+ if (fdt_load_addr >= (memmap[SPIKE_DRAM].base + machine->ram_size)) {
+ error_report("Not enough space for FDT after kernel + initrd");
+ exit(1);
+ }
+
/* load the reset vector */
riscv_setup_rom_reset_vec(memmap[SPIKE_DRAM].base, memmap[SPIKE_MROM].base,
- memmap[SPIKE_MROM].size, s->fdt);
+ memmap[SPIKE_MROM].size,
+ fdt_load_addr, s->fdt);
/* initialize HTIF using symbols found in load_kernel */
htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hd(0));
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index a8e2d58cc067..ebb5dd5c8c1c 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -481,6 +481,7 @@ static void virt_machine_init(MachineState *machine)
char *plic_hart_config;
size_t plic_hart_config_len;
target_ulong start_addr = memmap[VIRT_DRAM].base;
+ hwaddr fdt_load_addr;
int i;
unsigned int smp_cpus = machine->smp.cpus;
@@ -536,9 +537,19 @@ static void virt_machine_init(MachineState *machine)
start_addr = virt_memmap[VIRT_FLASH].base;
}
+ /* Compute the fdt load address in dram */
+ fdt_load_addr = riscv_calc_fdt_load_addr(memmap[VIRT_DRAM].base,
+ machine->ram_size, s->fdt);
+ if (fdt_load_addr >= (memmap[VIRT_DRAM].base + machine->ram_size)) {
+ error_report("Not enough space for FDT after kernel + initrd");
+ exit(1);
+ }
+
+
/* load the reset vector */
riscv_setup_rom_reset_vec(start_addr, virt_memmap[VIRT_MROM].base,
- virt_memmap[VIRT_MROM].size, s->fdt);
+ virt_memmap[VIRT_MROM].size,
+ fdt_load_addr, s->fdt);
/* create PLIC hart topology configuration string */
plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus;
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index 3e9759c89aa2..b6289a05d952 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -35,7 +35,10 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
symbol_fn_t sym_cb);
hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
uint64_t kernel_entry, hwaddr *start);
+hwaddr riscv_calc_fdt_load_addr(hwaddr dram_start, uint64_t dram_size,
+ void *fdt);
void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
- hwaddr rom_size, void *fdt);
+ hwaddr rom_size,
+ hwaddr fdt_load_addr, void *fdt);
#endif /* RISCV_BOOT_H */
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] RISC-V: Copy the fdt in dram instead of ROM
2020-06-16 19:26 ` [PATCH 2/3] RISC-V: Copy the fdt in dram instead of ROM Atish Patra
@ 2020-06-18 8:25 ` Bin Meng
2020-06-19 1:53 ` Atish Patra
0 siblings, 1 reply; 14+ messages in thread
From: Bin Meng @ 2020-06-18 8:25 UTC (permalink / raw)
To: Atish Patra
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt
On Wed, Jun 17, 2020 at 3:29 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> Currently, the fdt is copied to the ROM after the reset vector. The firmware
> has to copy it to DRAM. Instead of this, directly copy the device tree to a
> pre-computed dram address. The device tree load address should be as far as
> possible from kernel and initrd images. That's why it is kept at the end of
> the DRAM or 4GB whichever is lesser.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
> hw/riscv/boot.c | 45 ++++++++++++++++++++++++++++++-----------
> hw/riscv/sifive_u.c | 14 ++++++++++++-
> hw/riscv/spike.c | 14 ++++++++++++-
> hw/riscv/virt.c | 13 +++++++++++-
> include/hw/riscv/boot.h | 5 ++++-
> 5 files changed, 75 insertions(+), 16 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 8ed96da600c9..0378b7f1bd58 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -160,25 +160,51 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
> return *start + size;
> }
>
> +hwaddr riscv_calc_fdt_load_addr(hwaddr dram_base, uint64_t mem_size, void *fdt)
> +{
> + hwaddr temp, fdt_addr;
> + hwaddr dram_end = dram_base + mem_size;
> + int fdtsize = fdt_totalsize(fdt);
> +
> + if (fdtsize <= 0) {
> + error_report("invalid device-tree");
> + exit(1);
> + }
> + /*
> + * We should put fdt as far as possible to avoid kernel/initrd overwriting
> + * its content. But it should be addressable by 32 bit system as well.
> + * Thus, put it at an aligned address that less than fdt size from end of
> + * dram or 4GB whichever is lesser.
> + */
> + temp = MIN(dram_end, 4096 * MiB);
> + fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> +
> + return fdt_addr;
> +}
> +
> void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
> - hwaddr rom_size, void *fdt)
> + hwaddr rom_size,
> + hwaddr fdt_load_addr, void *fdt)
> {
> int i;
> /* reset vector */
> - uint32_t reset_vec[8] = {
> - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> + uint32_t reset_vec[10] = {
> + 0x00000297, /* 1: auipc t0, %pcrel_hi(fw_dyn) */
> 0xf1402573, /* csrr a0, mhartid */
> #if defined(TARGET_RISCV32)
> + 0x0202a583, /* lw a1, 32(t0) */
> 0x0182a283, /* lw t0, 24(t0) */
> #elif defined(TARGET_RISCV64)
> + 0x0202b583, /* ld a1, 32(t0) */
> 0x0182b283, /* ld t0, 24(t0) */
> #endif
> 0x00028067, /* jr t0 */
> 0x00000000,
> start_addr, /* start: .dword */
> 0x00000000,
> - /* dtb: */
> + fdt_load_addr, /* fdt_laddr: .dword */
> + 0x00000000,
> + /* fw_dyn: */
> };
>
> /* copy in the reset vector in little_endian byte order */
> @@ -189,14 +215,9 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
> rom_base, &address_space_memory);
>
> /* copy in the device tree */
> - if (fdt_pack(fdt) || fdt_totalsize(fdt) >
> - rom_size - sizeof(reset_vec)) {
> - error_report("not enough space to store device-tree");
> - exit(1);
> - }
> qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> - rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
> - rom_base + sizeof(reset_vec),
> +
> + rom_add_blob_fixed_as("fdt", fdt, fdt_totalsize(fdt), fdt_load_addr,
> &address_space_memory);
>
> return;
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index c2712570e0d9..1a1540c7f98d 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -31,6 +31,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> #include "qemu/log.h"
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> @@ -325,6 +326,7 @@ static void sifive_u_machine_init(MachineState *machine)
> MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> MemoryRegion *flash0 = g_new(MemoryRegion, 1);
> target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
> + hwaddr fdt_load_addr;
>
> /* Initialize SoC */
> object_initialize_child(OBJECT(machine), "soc", &s->soc,
> @@ -369,13 +371,23 @@ static void sifive_u_machine_init(MachineState *machine)
> }
> }
>
> + /* Compute the fdt load address in dram */
> + fdt_load_addr = riscv_calc_fdt_load_addr(memmap[SIFIVE_U_DRAM].base,
> + machine->ram_size, s->fdt);
> +
> + if (fdt_load_addr >= (memmap[SIFIVE_U_DRAM].base + machine->ram_size)) {
This check is unnecessary as the parameter passed to
riscv_calc_fdt_load_addr() guarantees that this won't happen.
> + error_report("Not enough space for FDT after kernel + initrd");
> + exit(1);
> + }
> +
> if (s->start_in_flash) {
> start_addr = memmap[SIFIVE_U_FLASH0].base;
> }
>
> /* load the reset vector */
> riscv_setup_rom_reset_vec(start_addr, memmap[SIFIVE_U_MROM].base,
> - memmap[SIFIVE_U_MROM].size, s->fdt);
> + memmap[SIFIVE_U_MROM].size,
> + fdt_load_addr, s->fdt);
> }
>
> static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 238eae48716a..2a34a1382487 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -24,6 +24,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> #include "qemu/log.h"
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> @@ -166,6 +167,7 @@ static void spike_board_init(MachineState *machine)
> MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> unsigned int smp_cpus = machine->smp.cpus;
> + hwaddr fdt_load_addr;
>
> /* Initialize SOC */
> object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
> @@ -212,9 +214,19 @@ static void spike_board_init(MachineState *machine)
> }
> }
>
> + /* Compute the fdt load address in dram */
> + fdt_load_addr = riscv_calc_fdt_load_addr(memmap[SPIKE_DRAM].base,
> + machine->ram_size, s->fdt);
> +
> + if (fdt_load_addr >= (memmap[SPIKE_DRAM].base + machine->ram_size)) {
> + error_report("Not enough space for FDT after kernel + initrd");
> + exit(1);
> + }
> +
> /* load the reset vector */
> riscv_setup_rom_reset_vec(memmap[SPIKE_DRAM].base, memmap[SPIKE_MROM].base,
> - memmap[SPIKE_MROM].size, s->fdt);
> + memmap[SPIKE_MROM].size,
> + fdt_load_addr, s->fdt);
>
> /* initialize HTIF using symbols found in load_kernel */
> htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hd(0));
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index a8e2d58cc067..ebb5dd5c8c1c 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -481,6 +481,7 @@ static void virt_machine_init(MachineState *machine)
> char *plic_hart_config;
> size_t plic_hart_config_len;
> target_ulong start_addr = memmap[VIRT_DRAM].base;
> + hwaddr fdt_load_addr;
> int i;
> unsigned int smp_cpus = machine->smp.cpus;
>
> @@ -536,9 +537,19 @@ static void virt_machine_init(MachineState *machine)
> start_addr = virt_memmap[VIRT_FLASH].base;
> }
>
> + /* Compute the fdt load address in dram */
> + fdt_load_addr = riscv_calc_fdt_load_addr(memmap[VIRT_DRAM].base,
> + machine->ram_size, s->fdt);
> + if (fdt_load_addr >= (memmap[VIRT_DRAM].base + machine->ram_size)) {
> + error_report("Not enough space for FDT after kernel + initrd");
> + exit(1);
> + }
> +
> +
> /* load the reset vector */
> riscv_setup_rom_reset_vec(start_addr, virt_memmap[VIRT_MROM].base,
> - virt_memmap[VIRT_MROM].size, s->fdt);
> + virt_memmap[VIRT_MROM].size,
> + fdt_load_addr, s->fdt);
>
> /* create PLIC hart topology configuration string */
> plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus;
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index 3e9759c89aa2..b6289a05d952 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -35,7 +35,10 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
> symbol_fn_t sym_cb);
> hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
> uint64_t kernel_entry, hwaddr *start);
> +hwaddr riscv_calc_fdt_load_addr(hwaddr dram_start, uint64_t dram_size,
> + void *fdt);
> void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
> - hwaddr rom_size, void *fdt);
> + hwaddr rom_size,
> + hwaddr fdt_load_addr, void *fdt);
>
> #endif /* RISCV_BOOT_H */
Regards,
Bin
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 2/3] RISC-V: Copy the fdt in dram instead of ROM
2020-06-18 8:25 ` Bin Meng
@ 2020-06-19 1:53 ` Atish Patra
0 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2020-06-19 1:53 UTC (permalink / raw)
To: Bin Meng
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Atish Patra, Alistair Francis,
Palmer Dabbelt
On Thu, Jun 18, 2020 at 1:26 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jun 17, 2020 at 3:29 AM Atish Patra <atish.patra@wdc.com> wrote:
> >
> > Currently, the fdt is copied to the ROM after the reset vector. The firmware
> > has to copy it to DRAM. Instead of this, directly copy the device tree to a
> > pre-computed dram address. The device tree load address should be as far as
> > possible from kernel and initrd images. That's why it is kept at the end of
> > the DRAM or 4GB whichever is lesser.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> > hw/riscv/boot.c | 45 ++++++++++++++++++++++++++++++-----------
> > hw/riscv/sifive_u.c | 14 ++++++++++++-
> > hw/riscv/spike.c | 14 ++++++++++++-
> > hw/riscv/virt.c | 13 +++++++++++-
> > include/hw/riscv/boot.h | 5 ++++-
> > 5 files changed, 75 insertions(+), 16 deletions(-)
> >
> > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> > index 8ed96da600c9..0378b7f1bd58 100644
> > --- a/hw/riscv/boot.c
> > +++ b/hw/riscv/boot.c
> > @@ -160,25 +160,51 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
> > return *start + size;
> > }
> >
> > +hwaddr riscv_calc_fdt_load_addr(hwaddr dram_base, uint64_t mem_size, void *fdt)
> > +{
> > + hwaddr temp, fdt_addr;
> > + hwaddr dram_end = dram_base + mem_size;
> > + int fdtsize = fdt_totalsize(fdt);
> > +
> > + if (fdtsize <= 0) {
> > + error_report("invalid device-tree");
> > + exit(1);
> > + }
> > + /*
> > + * We should put fdt as far as possible to avoid kernel/initrd overwriting
> > + * its content. But it should be addressable by 32 bit system as well.
> > + * Thus, put it at an aligned address that less than fdt size from end of
> > + * dram or 4GB whichever is lesser.
> > + */
> > + temp = MIN(dram_end, 4096 * MiB);
> > + fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> > +
> > + return fdt_addr;
> > +}
> > +
> > void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
> > - hwaddr rom_size, void *fdt)
> > + hwaddr rom_size,
> > + hwaddr fdt_load_addr, void *fdt)
> > {
> > int i;
> > /* reset vector */
> > - uint32_t reset_vec[8] = {
> > - 0x00000297, /* 1: auipc t0, %pcrel_hi(dtb) */
> > - 0x02028593, /* addi a1, t0, %pcrel_lo(1b) */
> > + uint32_t reset_vec[10] = {
> > + 0x00000297, /* 1: auipc t0, %pcrel_hi(fw_dyn) */
> > 0xf1402573, /* csrr a0, mhartid */
> > #if defined(TARGET_RISCV32)
> > + 0x0202a583, /* lw a1, 32(t0) */
> > 0x0182a283, /* lw t0, 24(t0) */
> > #elif defined(TARGET_RISCV64)
> > + 0x0202b583, /* ld a1, 32(t0) */
> > 0x0182b283, /* ld t0, 24(t0) */
> > #endif
> > 0x00028067, /* jr t0 */
> > 0x00000000,
> > start_addr, /* start: .dword */
> > 0x00000000,
> > - /* dtb: */
> > + fdt_load_addr, /* fdt_laddr: .dword */
> > + 0x00000000,
> > + /* fw_dyn: */
> > };
> >
> > /* copy in the reset vector in little_endian byte order */
> > @@ -189,14 +215,9 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
> > rom_base, &address_space_memory);
> >
> > /* copy in the device tree */
> > - if (fdt_pack(fdt) || fdt_totalsize(fdt) >
> > - rom_size - sizeof(reset_vec)) {
> > - error_report("not enough space to store device-tree");
> > - exit(1);
> > - }
> > qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
> > - rom_add_blob_fixed_as("mrom.fdt", fdt, fdt_totalsize(fdt),
> > - rom_base + sizeof(reset_vec),
> > +
> > + rom_add_blob_fixed_as("fdt", fdt, fdt_totalsize(fdt), fdt_load_addr,
> > &address_space_memory);
> >
> > return;
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index c2712570e0d9..1a1540c7f98d 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -31,6 +31,7 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qemu/units.h"
> > #include "qemu/log.h"
> > #include "qemu/error-report.h"
> > #include "qapi/error.h"
> > @@ -325,6 +326,7 @@ static void sifive_u_machine_init(MachineState *machine)
> > MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> > MemoryRegion *flash0 = g_new(MemoryRegion, 1);
> > target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
> > + hwaddr fdt_load_addr;
> >
> > /* Initialize SoC */
> > object_initialize_child(OBJECT(machine), "soc", &s->soc,
> > @@ -369,13 +371,23 @@ static void sifive_u_machine_init(MachineState *machine)
> > }
> > }
> >
> > + /* Compute the fdt load address in dram */
> > + fdt_load_addr = riscv_calc_fdt_load_addr(memmap[SIFIVE_U_DRAM].base,
> > + machine->ram_size, s->fdt);
> > +
> > + if (fdt_load_addr >= (memmap[SIFIVE_U_DRAM].base + machine->ram_size)) {
>
> This check is unnecessary as the parameter passed to
> riscv_calc_fdt_load_addr() guarantees that this won't happen.
>
Sure. I will remove that.
> > + error_report("Not enough space for FDT after kernel + initrd");
> > + exit(1);
> > + }
> > +
> > if (s->start_in_flash) {
> > start_addr = memmap[SIFIVE_U_FLASH0].base;
> > }
> >
> > /* load the reset vector */
> > riscv_setup_rom_reset_vec(start_addr, memmap[SIFIVE_U_MROM].base,
> > - memmap[SIFIVE_U_MROM].size, s->fdt);
> > + memmap[SIFIVE_U_MROM].size,
> > + fdt_load_addr, s->fdt);
> > }
> >
> > static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
> > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> > index 238eae48716a..2a34a1382487 100644
> > --- a/hw/riscv/spike.c
> > +++ b/hw/riscv/spike.c
> > @@ -24,6 +24,7 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qemu/units.h"
> > #include "qemu/log.h"
> > #include "qemu/error-report.h"
> > #include "qapi/error.h"
> > @@ -166,6 +167,7 @@ static void spike_board_init(MachineState *machine)
> > MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> > MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> > unsigned int smp_cpus = machine->smp.cpus;
> > + hwaddr fdt_load_addr;
> >
> > /* Initialize SOC */
> > object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
> > @@ -212,9 +214,19 @@ static void spike_board_init(MachineState *machine)
> > }
> > }
> >
> > + /* Compute the fdt load address in dram */
> > + fdt_load_addr = riscv_calc_fdt_load_addr(memmap[SPIKE_DRAM].base,
> > + machine->ram_size, s->fdt);
> > +
> > + if (fdt_load_addr >= (memmap[SPIKE_DRAM].base + machine->ram_size)) {
> > + error_report("Not enough space for FDT after kernel + initrd");
> > + exit(1);
> > + }
> > +
> > /* load the reset vector */
> > riscv_setup_rom_reset_vec(memmap[SPIKE_DRAM].base, memmap[SPIKE_MROM].base,
> > - memmap[SPIKE_MROM].size, s->fdt);
> > + memmap[SPIKE_MROM].size,
> > + fdt_load_addr, s->fdt);
> >
> > /* initialize HTIF using symbols found in load_kernel */
> > htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hd(0));
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index a8e2d58cc067..ebb5dd5c8c1c 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -481,6 +481,7 @@ static void virt_machine_init(MachineState *machine)
> > char *plic_hart_config;
> > size_t plic_hart_config_len;
> > target_ulong start_addr = memmap[VIRT_DRAM].base;
> > + hwaddr fdt_load_addr;
> > int i;
> > unsigned int smp_cpus = machine->smp.cpus;
> >
> > @@ -536,9 +537,19 @@ static void virt_machine_init(MachineState *machine)
> > start_addr = virt_memmap[VIRT_FLASH].base;
> > }
> >
> > + /* Compute the fdt load address in dram */
> > + fdt_load_addr = riscv_calc_fdt_load_addr(memmap[VIRT_DRAM].base,
> > + machine->ram_size, s->fdt);
> > + if (fdt_load_addr >= (memmap[VIRT_DRAM].base + machine->ram_size)) {
> > + error_report("Not enough space for FDT after kernel + initrd");
> > + exit(1);
> > + }
> > +
> > +
> > /* load the reset vector */
> > riscv_setup_rom_reset_vec(start_addr, virt_memmap[VIRT_MROM].base,
> > - virt_memmap[VIRT_MROM].size, s->fdt);
> > + virt_memmap[VIRT_MROM].size,
> > + fdt_load_addr, s->fdt);
> >
> > /* create PLIC hart topology configuration string */
> > plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus;
> > diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> > index 3e9759c89aa2..b6289a05d952 100644
> > --- a/include/hw/riscv/boot.h
> > +++ b/include/hw/riscv/boot.h
> > @@ -35,7 +35,10 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
> > symbol_fn_t sym_cb);
> > hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
> > uint64_t kernel_entry, hwaddr *start);
> > +hwaddr riscv_calc_fdt_load_addr(hwaddr dram_start, uint64_t dram_size,
> > + void *fdt);
> > void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
> > - hwaddr rom_size, void *fdt);
> > + hwaddr rom_size,
> > + hwaddr fdt_load_addr, void *fdt);
> >
> > #endif /* RISCV_BOOT_H */
>
> Regards,
> Bin
>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] riscv: Add opensbi firmware dynamic support
2020-06-16 19:26 [PATCH 0/3] Add OpenSBI dynamic firmware support Atish Patra
2020-06-16 19:26 ` [PATCH 1/3] riscv: Unify Qemu's reset vector code path Atish Patra
2020-06-16 19:26 ` [PATCH 2/3] RISC-V: Copy the fdt in dram instead of ROM Atish Patra
@ 2020-06-16 19:27 ` Atish Patra
2020-06-16 22:39 ` [PATCH 0/3] Add OpenSBI dynamic firmware support no-reply
2020-06-18 8:56 ` Bin Meng
4 siblings, 0 replies; 14+ messages in thread
From: Atish Patra @ 2020-06-16 19:27 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann, Atish Patra,
Alistair Francis, Palmer Dabbelt
OpenSBI is the default firmware in Qemu and has various firmware loading
options. Currently, qemu loader uses fw_jump which has a compile time
pre-defined address where fdt & kernel image must reside. This puts a
constraint on image size of the Linux kernel depending on the fdt location
and available memory. However, fw_dynamic allows the loader to specify
the next stage location (i.e. Linux kernel/U-boot) in memory and other
configurable boot options available in OpenSBI.
Add support for OpenSBI dynamic firmware loading support. This doesn't
break existing setup and fw_jump will continue to work as it is. Any
other firmware will continue to work without any issues as long as it
doesn't expect anything specific from loader in "a2" register.
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
hw/riscv/boot.c | 32 ++++++++++++++++--
hw/riscv/sifive_u.c | 11 +++++--
hw/riscv/spike.c | 11 +++++--
hw/riscv/virt.c | 11 +++++--
include/hw/riscv/boot.h | 2 +-
include/hw/riscv/boot_opensbi.h | 58 +++++++++++++++++++++++++++++++++
6 files changed, 116 insertions(+), 9 deletions(-)
create mode 100644 include/hw/riscv/boot_opensbi.h
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 0378b7f1bd58..47530f2732cf 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -26,6 +26,7 @@
#include "hw/boards.h"
#include "hw/loader.h"
#include "hw/riscv/boot.h"
+#include "hw/riscv/boot_opensbi.h"
#include "elf.h"
#include "sysemu/device_tree.h"
#include "sysemu/qtest.h"
@@ -34,8 +35,10 @@
#if defined(TARGET_RISCV32)
# define KERNEL_BOOT_ADDRESS 0x80400000
+#define fw_dynamic_info_data(__val) cpu_to_le32(__val)
#else
# define KERNEL_BOOT_ADDRESS 0x80200000
+#define fw_dynamic_info_data(__val) cpu_to_le64(__val)
#endif
void riscv_find_and_load_firmware(MachineState *machine,
@@ -183,13 +186,25 @@ hwaddr riscv_calc_fdt_load_addr(hwaddr dram_base, uint64_t mem_size, void *fdt)
}
void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
- hwaddr rom_size,
+ hwaddr rom_size, uint64_t kernel_entry,
hwaddr fdt_load_addr, void *fdt)
{
int i;
+ struct fw_dynamic_info dinfo;
+ uint64_t dinfo_len;
+
+ dinfo.magic = fw_dynamic_info_data(FW_DYNAMIC_INFO_MAGIC_VALUE);
+ dinfo.version = fw_dynamic_info_data(FW_DYNAMIC_INFO_VERSION);
+ dinfo.next_mode = fw_dynamic_info_data(FW_DYNAMIC_INFO_NEXT_MODE_S);
+ dinfo.next_addr = fw_dynamic_info_data(kernel_entry);
+ dinfo.options = 0;
+ dinfo.boot_hart = 0;
+ dinfo_len = sizeof(dinfo);
+
/* reset vector */
uint32_t reset_vec[10] = {
0x00000297, /* 1: auipc t0, %pcrel_hi(fw_dyn) */
+ 0x02828613, /* addi a2, t0, %pcrel_lo(1b) */
0xf1402573, /* csrr a0, mhartid */
#if defined(TARGET_RISCV32)
0x0202a583, /* lw a1, 32(t0) */
@@ -199,7 +214,6 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
0x0182b283, /* ld t0, 24(t0) */
#endif
0x00028067, /* jr t0 */
- 0x00000000,
start_addr, /* start: .dword */
0x00000000,
fdt_load_addr, /* fdt_laddr: .dword */
@@ -214,6 +228,20 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
rom_base, &address_space_memory);
+ /**
+ * copy the dynamic firmware info. This information is specific to
+ * OpenSBI but doesn't break any other firmware as long as they don't
+ * expect any certain value in "a2" register.
+ */
+ if (fdt_pack(fdt) || fdt_totalsize(fdt) >
+ rom_size - dinfo_len) {
+ error_report("not enough space to store device-tree");
+ exit(1);
+ }
+
+ rom_add_blob_fixed_as("mrom.finfo", &dinfo, dinfo_len,
+ rom_base + sizeof(reset_vec),
+ &address_space_memory);
/* copy in the device tree */
qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 1a1540c7f98d..11b8814b9240 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -327,6 +327,7 @@ static void sifive_u_machine_init(MachineState *machine)
MemoryRegion *flash0 = g_new(MemoryRegion, 1);
target_ulong start_addr = memmap[SIFIVE_U_DRAM].base;
hwaddr fdt_load_addr;
+ uint64_t kernel_entry;
/* Initialize SoC */
object_initialize_child(OBJECT(machine), "soc", &s->soc,
@@ -356,7 +357,7 @@ static void sifive_u_machine_init(MachineState *machine)
memmap[SIFIVE_U_DRAM].base, NULL);
if (machine->kernel_filename) {
- uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
+ kernel_entry = riscv_load_kernel(machine->kernel_filename,
NULL);
if (machine->initrd_filename) {
@@ -369,6 +370,12 @@ static void sifive_u_machine_init(MachineState *machine)
qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
end);
}
+ } else {
+ /*
+ * If dynamic firmware is used, it doesn't know where is the next mode
+ * if kernel argument is not set.
+ */
+ kernel_entry = 0;
}
/* Compute the fdt load address in dram */
@@ -386,7 +393,7 @@ static void sifive_u_machine_init(MachineState *machine)
/* load the reset vector */
riscv_setup_rom_reset_vec(start_addr, memmap[SIFIVE_U_MROM].base,
- memmap[SIFIVE_U_MROM].size,
+ memmap[SIFIVE_U_MROM].size, kernel_entry,
fdt_load_addr, s->fdt);
}
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 2a34a1382487..bd29a2b38611 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -168,6 +168,7 @@ static void spike_board_init(MachineState *machine)
MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
unsigned int smp_cpus = machine->smp.cpus;
hwaddr fdt_load_addr;
+ uint64_t kernel_entry;
/* Initialize SOC */
object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
@@ -199,7 +200,7 @@ static void spike_board_init(MachineState *machine)
htif_symbol_callback);
if (machine->kernel_filename) {
- uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
+ kernel_entry = riscv_load_kernel(machine->kernel_filename,
htif_symbol_callback);
if (machine->initrd_filename) {
@@ -212,6 +213,12 @@ static void spike_board_init(MachineState *machine)
qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
end);
}
+ } else {
+ /*
+ * If dynamic firmware is used, it doesn't know where is the next mode
+ * if kernel argument is not set.
+ */
+ kernel_entry = 0;
}
/* Compute the fdt load address in dram */
@@ -225,7 +232,7 @@ static void spike_board_init(MachineState *machine)
/* load the reset vector */
riscv_setup_rom_reset_vec(memmap[SPIKE_DRAM].base, memmap[SPIKE_MROM].base,
- memmap[SPIKE_MROM].size,
+ memmap[SPIKE_MROM].size, kernel_entry,
fdt_load_addr, s->fdt);
/* initialize HTIF using symbols found in load_kernel */
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index ebb5dd5c8c1c..7897eaa97301 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -482,6 +482,7 @@ static void virt_machine_init(MachineState *machine)
size_t plic_hart_config_len;
target_ulong start_addr = memmap[VIRT_DRAM].base;
hwaddr fdt_load_addr;
+ uint64_t kernel_entry;
int i;
unsigned int smp_cpus = machine->smp.cpus;
@@ -514,7 +515,7 @@ static void virt_machine_init(MachineState *machine)
memmap[VIRT_DRAM].base, NULL);
if (machine->kernel_filename) {
- uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
+ kernel_entry = riscv_load_kernel(machine->kernel_filename,
NULL);
if (machine->initrd_filename) {
@@ -527,6 +528,12 @@ static void virt_machine_init(MachineState *machine)
qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
end);
}
+ } else {
+ /*
+ * If dynamic firmware is used, it doesn't know where is the next mode
+ * if kernel argument is not set.
+ */
+ kernel_entry = 0;
}
if (drive_get(IF_PFLASH, 0, 0)) {
@@ -548,7 +555,7 @@ static void virt_machine_init(MachineState *machine)
/* load the reset vector */
riscv_setup_rom_reset_vec(start_addr, virt_memmap[VIRT_MROM].base,
- virt_memmap[VIRT_MROM].size,
+ virt_memmap[VIRT_MROM].size, kernel_entry,
fdt_load_addr, s->fdt);
/* create PLIC hart topology configuration string */
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index b6289a05d952..2e7163e3fef2 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -38,7 +38,7 @@ hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
hwaddr riscv_calc_fdt_load_addr(hwaddr dram_start, uint64_t dram_size,
void *fdt);
void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
- hwaddr rom_size,
+ hwaddr rom_size, hwaddr kernel_entry,
hwaddr fdt_load_addr, void *fdt);
#endif /* RISCV_BOOT_H */
diff --git a/include/hw/riscv/boot_opensbi.h b/include/hw/riscv/boot_opensbi.h
new file mode 100644
index 000000000000..0d5ddd6c3daf
--- /dev/null
+++ b/include/hw/riscv/boot_opensbi.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (c) 2019 Western Digital Corporation or its affiliates.
+ *
+ * Based on include/sbi/{fw_dynamic.h,sbi_scratch.h} from the OpenSBI project.
+ */
+#ifndef OPENSBI_H
+#define OPENSBI_H
+
+/** Expected value of info magic ('OSBI' ascii string in hex) */
+#define FW_DYNAMIC_INFO_MAGIC_VALUE 0x4942534f
+
+/** Maximum supported info version */
+#define FW_DYNAMIC_INFO_VERSION 0x2
+
+/** Possible next mode values */
+#define FW_DYNAMIC_INFO_NEXT_MODE_U 0x0
+#define FW_DYNAMIC_INFO_NEXT_MODE_S 0x1
+#define FW_DYNAMIC_INFO_NEXT_MODE_M 0x3
+
+enum sbi_scratch_options {
+ /** Disable prints during boot */
+ SBI_SCRATCH_NO_BOOT_PRINTS = (1 << 0),
+ /** Enable runtime debug prints */
+ SBI_SCRATCH_DEBUG_PRINTS = (1 << 1),
+};
+
+/** Representation dynamic info passed by previous booting stage */
+struct fw_dynamic_info {
+ /** Info magic */
+ target_long magic;
+ /** Info version */
+ target_long version;
+ /** Next booting stage address */
+ target_long next_addr;
+ /** Next booting stage mode */
+ target_long next_mode;
+ /** Options for OpenSBI library */
+ target_long options;
+ /**
+ * Preferred boot HART id
+ *
+ * It is possible that the previous booting stage uses same link
+ * address as the FW_DYNAMIC firmware. In this case, the relocation
+ * lottery mechanism can potentially overwrite the previous booting
+ * stage while other HARTs are still running in the previous booting
+ * stage leading to boot-time crash. To avoid this boot-time crash,
+ * the previous booting stage can specify last HART that will jump
+ * to the FW_DYNAMIC firmware as the preferred boot HART.
+ *
+ * To avoid specifying a preferred boot HART, the previous booting
+ * stage can set it to -1UL which will force the FW_DYNAMIC firmware
+ * to use the relocation lottery mechanism.
+ */
+ target_long boot_hart;
+};
+
+#endif
--
2.26.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 0/3] Add OpenSBI dynamic firmware support
2020-06-16 19:26 [PATCH 0/3] Add OpenSBI dynamic firmware support Atish Patra
` (2 preceding siblings ...)
2020-06-16 19:27 ` [PATCH 3/3] riscv: Add opensbi firmware dynamic support Atish Patra
@ 2020-06-16 22:39 ` no-reply
2020-06-18 8:56 ` Bin Meng
4 siblings, 0 replies; 14+ messages in thread
From: no-reply @ 2020-06-16 22:39 UTC (permalink / raw)
To: atish.patra
Cc: qemu-riscv, sagark, kbastian, qemu-devel, atish.patra,
Alistair.Francis, palmer
Patchew URL: https://patchew.org/QEMU/20200616192700.1900260-1-atish.patra@wdc.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
CC qga/commands.o
CC qga/guest-agent-command-state.o
CC qga/main.o
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of ` CC qga/commands-posix.o
__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
CC qga/channel-posix.o
CC qga/qapi-generated/qga-qapi-types.o
---
GEN docs/interop/qemu-ga-ref.html
GEN docs/interop/qemu-ga-ref.txt
GEN docs/interop/qemu-ga-ref.7
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
LINK qemu-ga
LINK qemu-keymap
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
LINK ivshmem-client
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
LINK ivshmem-server
LINK qemu-nbd
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
LINK qemu-storage-daemon
AS pc-bios/optionrom/multiboot.o
AS pc-bios/optionrom/linuxboot.o
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
CC pc-bios/optionrom/linuxboot_dma.o
AS pc-bios/optionrom/kvmvapic.o
AS pc-bios/optionrom/pvh.o
---
BUILD pc-bios/optionrom/multiboot.raw
BUILD pc-bios/optionrom/linuxboot.raw
BUILD pc-bios/optionrom/kvmvapic.raw
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
SIGN pc-bios/optionrom/multiboot.bin
SIGN pc-bios/optionrom/linuxboot.bin
SIGN pc-bios/optionrom/kvmvapic.bin
LINK fsdev/virtfs-proxy-helper
LINK scsi/qemu-pr-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
BUILD pc-bios/optionrom/linuxboot_dma.img
BUILD pc-bios/optionrom/linuxboot_dma.raw
SIGN pc-bios/optionrom/linuxboot_dma.bin
LINK qemu-bridge-helper
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
BUILD pc-bios/optionrom/pvh.img
LINK virtiofsd
BUILD pc-bios/optionrom/pvh.raw
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
SIGN pc-bios/optionrom/pvh.bin
LINK vhost-user-input
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
/usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o)
GEN x86_64-softmmu/hmp-commands.h
GEN x86_64-softmmu/hmp-commands-info.h
GEN x86_64-softmmu/config-devices.h
---
CC x86_64-softmmu/hw/display/virtio-gpu.o
CC x86_64-softmmu/hw/display/virtio-gpu-3d.o
CC x86_64-softmmu/hw/display/vhost-user-gpu.o
/tmp/qemu-test/src/fpu/softfloat.c:3365:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
absZ &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!
/tmp/qemu-test/src/fpu/softfloat.c:3423:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
absZ0 &= ~ ( ( (uint64_t) ( absZ1<<1 ) == 0 ) & roundNearestEven );
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!
/tmp/qemu-test/src/fpu/softfloat.c:3483:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
absZ0 &= ~(((uint64_t)(absZ1<<1) == 0) & roundNearestEven);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!
/tmp/qemu-test/src/fpu/softfloat.c:3606:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
zSig &= ~ ( ( ( roundBits ^ 0x40 ) == 0 ) & roundNearestEven );
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!
/tmp/qemu-test/src/fpu/softfloat.c:3760:13: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
zSig &= ~ ( ( ( roundBits ^ 0x200 ) == 0 ) & roundNearestEven );
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!
/tmp/qemu-test/src/fpu/softfloat.c:3987:21: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!
/tmp/qemu-test/src/fpu/softfloat.c:4003:22: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
zSig0 &= ~ ( ( (uint64_t) ( zSig1<<1 ) == 0 ) & roundNearestEven );
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!
/tmp/qemu-test/src/fpu/softfloat.c:4273:18: error: bitwise negation of a boolean expression; did you mean logical negation? [-Werror,-Wbool-operation]
zSig1 &= ~ ( ( zSig2 + zSig2 == 0 ) & roundNearestEven );
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!
8 errors generated.
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: fpu/softfloat.o] Error 1
make[1]: *** Waiting for unfinished jobs....
/tmp/qemu-test/src/migration/ram.c:919:45: error: implicit conversion from 'unsigned long' to 'double' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion]
xbzrle_counters.encoding_rate = UINT64_MAX;
~ ^~~~~~~~~~
/usr/include/stdint.h:130:23: note: expanded from macro 'UINT64_MAX'
---
18446744073709551615UL
^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: migration/ram.o] Error 1
make: *** [Makefile:527: x86_64-softmmu/all] Error 2
Traceback (most recent call last):
File "./tests/docker/docker.py", line 669, in <module>
sys.exit(main())
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=87c0cd21d942481f9c8b0deaf8f61398', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-yoca3tbl/src/docker-src.2020-06-16-18.35.31.23981:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=87c0cd21d942481f9c8b0deaf8f61398
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-yoca3tbl/src'
make: *** [docker-run-test-debug@fedora] Error 2
real 4m15.706s
user 0m9.061s
The full log is available at
http://patchew.org/logs/20200616192700.1900260-1-atish.patra@wdc.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 0/3] Add OpenSBI dynamic firmware support
2020-06-16 19:26 [PATCH 0/3] Add OpenSBI dynamic firmware support Atish Patra
` (3 preceding siblings ...)
2020-06-16 22:39 ` [PATCH 0/3] Add OpenSBI dynamic firmware support no-reply
@ 2020-06-18 8:56 ` Bin Meng
2020-06-18 18:19 ` Atish Patra
4 siblings, 1 reply; 14+ messages in thread
From: Bin Meng @ 2020-06-18 8:56 UTC (permalink / raw)
To: Atish Patra
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt
On Wed, Jun 17, 2020 at 3:29 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> This series adds support OpenSBI dynamic firmware support to Qemu.
> Qemu loader passes the information about the DT and next stage (i.e. kernel
> or U-boot) via "a2" register. It allows the user to build bigger OS images
> without worrying about overwriting DT. It also unifies the reset vector code
I am not sure in what situation overwriting DT could happen. Could you
please elaborate?
> in rom and dt placement. Now, the DT is copied directly in DRAM instead of ROM.
>
> The changes have been verified on following qemu machines.
>
> 64bit:
> - spike, sifive_u, virt
> 32bit:
> - virt
Any test instructions?
>
> I have also verified fw_jump on all the above platforms to ensure that this
> series doesn't break the existing setup.
>
Regards,
Bin
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 0/3] Add OpenSBI dynamic firmware support
2020-06-18 8:56 ` Bin Meng
@ 2020-06-18 18:19 ` Atish Patra
2020-06-19 16:32 ` Alexander Richardson
0 siblings, 1 reply; 14+ messages in thread
From: Atish Patra @ 2020-06-18 18:19 UTC (permalink / raw)
To: Bin Meng
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Atish Patra, Alistair Francis,
Palmer Dabbelt
On Thu, Jun 18, 2020 at 1:56 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jun 17, 2020 at 3:29 AM Atish Patra <atish.patra@wdc.com> wrote:
> >
> > This series adds support OpenSBI dynamic firmware support to Qemu.
> > Qemu loader passes the information about the DT and next stage (i.e. kernel
> > or U-boot) via "a2" register. It allows the user to build bigger OS images
> > without worrying about overwriting DT. It also unifies the reset vector code
>
> I am not sure in what situation overwriting DT could happen. Could you
> please elaborate?
>
Currently, the DT is loaded 0x82200000 (34MB offset) for fw_jump.
Thus, a bigger kernel image
would overwrite the DT. In fact, it was reported by FreeBSD folks.
https://github.com/riscv/opensbi/issues/169
There are temporary solutions that can put DT a little bit further or
put it within 2MB offset. But that's
just delaying the inevitable.
> > in rom and dt placement. Now, the DT is copied directly in DRAM instead of ROM.
> >
> > The changes have been verified on following qemu machines.
> >
> > 64bit:
> > - spike, sifive_u, virt
> > 32bit:
> > - virt
>
> Any test instructions?
>
you just need to provide fw_dynamic instead of fw_jump in bios argument.
For example: Here is my qemu commandline for testing
qemu-system-riscv64 -M virt -smp 4 -m 2g -display none -serial
mon:stdio -bios
~/workspace/opensbi/build/platform/generic/firmware/fw_dynamic.bin \
-kernel /home/atish/workspace/linux/arch/riscv/boot/Image -initrd
/home/atish/workspace/rootfs_images/riscv64_busybox_rootfs.img -object
rng-random,filename=/dev/urandom,id=rng0 \
-device virtio-rng-device,rng=rng0 -device
virtio-net-device,netdev=usernet -netdev
user,id=usernet,hostfwd=tcp::10000-:22 -d in_asm -D log -append 'rw
console=ttyS0 earlycon'
> >
> > I have also verified fw_jump on all the above platforms to ensure that this
> > series doesn't break the existing setup.
> >
>
> Regards,
> Bin
>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] Add OpenSBI dynamic firmware support
2020-06-18 18:19 ` Atish Patra
@ 2020-06-19 16:32 ` Alexander Richardson
0 siblings, 0 replies; 14+ messages in thread
From: Alexander Richardson @ 2020-06-19 16:32 UTC (permalink / raw)
To: Atish Patra
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Atish Patra, Alistair Francis,
Palmer Dabbelt, Bin Meng
On Thu, 18 Jun 2020 at 19:22, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Thu, Jun 18, 2020 at 1:56 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Wed, Jun 17, 2020 at 3:29 AM Atish Patra <atish.patra@wdc.com> wrote:
> > >
> > > This series adds support OpenSBI dynamic firmware support to Qemu.
> > > Qemu loader passes the information about the DT and next stage (i.e. kernel
> > > or U-boot) via "a2" register. It allows the user to build bigger OS images
> > > without worrying about overwriting DT. It also unifies the reset vector code
> >
> > I am not sure in what situation overwriting DT could happen. Could you
> > please elaborate?
> >
>
> Currently, the DT is loaded 0x82200000 (34MB offset) for fw_jump.
> Thus, a bigger kernel image
> would overwrite the DT. In fact, it was reported by FreeBSD folks.
> https://github.com/riscv/opensbi/issues/169
>
The problem is that the DT overwrites the kernel image. Usually this
is not noticeable since it's so small and rarely overwrites something
useful, but in my case it was overwriting program memory which
resulted in invalid instruction crashes.
Since this is quite awkward to debug, I added a kernel assertion to
FreeBSD to abort boot in that case.
> There are temporary solutions that can put DT a little bit further or
> put it within 2MB offset. But that's
> just delaying the inevitable.
>
I've changed OpenSBI locally to use a 1MB offset (i.e. place the DT
between OpenSBI and the kernel), but I think the fw_dynamic approach
is much nicer.
Thanks,
Alex
^ permalink raw reply [flat|nested] 14+ messages in thread