* [PATCH v2] hw/riscv: Fix the bug of max size limit when put initrd to RAM
@ 2023-03-12 9:40 Hang Xu
2023-03-12 22:25 ` Daniel Henrique Barboza
0 siblings, 1 reply; 2+ messages in thread
From: Hang Xu @ 2023-03-12 9:40 UTC (permalink / raw)
To: qemu-riscv; +Cc: dbarboza, palmer, bmeng, alistair.francis, Hang Xu
Because the starting address of ram is not necessarily 0,
the remaining free space in ram is
ram_size - (start - ram_base) instead of ram_size-start.
The changes based on patch1 are as follows:
1. Rebase
2. Considering that the ram block may be discontinuous
Signed-off-by: Hang Xu <xuhang@eswincomputing.com>
---
hw/riscv/boot.c | 18 ++++++++++++------
hw/riscv/microchip_pfsoc.c | 5 ++++-
hw/riscv/opentitan.c | 2 +-
hw/riscv/sifive_e.c | 2 +-
hw/riscv/sifive_u.c | 5 ++++-
hw/riscv/spike.c | 5 ++++-
hw/riscv/virt.c | 5 ++++-
include/hw/riscv/boot.h | 2 ++
8 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 52bf8e67de..73b44a0c8b 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -173,10 +173,10 @@ target_ulong riscv_load_firmware(const char *firmware_filename,
exit(1);
}
-static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
+static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry,
+ uint64_t ram_base, uint64_t ram_size)
{
const char *filename = machine->initrd_filename;
- uint64_t mem_size = machine->ram_size;
void *fdt = machine->fdt;
hwaddr start, end;
ssize_t size;
@@ -193,12 +193,16 @@ static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
* So for boards with less than 256MB of RAM we put the initrd
* halfway into RAM, and for boards with 256MB of RAM or more we put
* the initrd at 128MB.
+ * A ram_size == 0, usually from a MemMapEntry[].size element,
+ * means that the RAM block goes all the way to ms->ram_size.
*/
- start = kernel_entry + MIN(mem_size / 2, 128 * MiB);
+ ram_size = ram_size ? MIN(machine->ram_size, ram_size) : machine->ram_size;
+ start = kernel_entry + MIN(ram_size / 2, 128 * MiB);
+ uint64_t max_initrd = ram_size - (start - ram_base);
- size = load_ramdisk(filename, start, mem_size - start);
+ size = load_ramdisk(filename, start, max_initrd);
if (size == -1) {
- size = load_image_targphys(filename, start, mem_size - start);
+ size = load_image_targphys(filename, start, max_initrd);
if (size == -1) {
error_report("could not load ramdisk '%s'", filename);
exit(1);
@@ -217,6 +221,8 @@ target_ulong riscv_load_kernel(MachineState *machine,
RISCVHartArrayState *harts,
target_ulong kernel_start_addr,
bool load_initrd,
+ uint64_t ram_base,
+ uint64_t ram_size,
symbol_fn_t sym_cb)
{
const char *kernel_filename = machine->kernel_filename;
@@ -263,7 +269,7 @@ out:
}
if (load_initrd && machine->initrd_filename) {
- riscv_load_initrd(machine, kernel_entry);
+ riscv_load_initrd(machine, kernel_entry, ram_base, ram_size);
}
if (fdt && machine->kernel_cmdline && *machine->kernel_cmdline) {
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index e81bbd12df..b42d90b89e 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -630,7 +630,10 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
firmware_end_addr);
kernel_entry = riscv_load_kernel(machine, &s->soc.u_cpus,
- kernel_start_addr, true, NULL);
+ kernel_start_addr, true,
+ memmap[MICROCHIP_PFSOC_DRAM_LO].base,
+ memmap[MICROCHIP_PFSOC_DRAM_LO].size,
+ NULL);
/* Compute the fdt load address in dram */
fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index b06944d382..bb663523d5 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -103,7 +103,7 @@ static void opentitan_board_init(MachineState *machine)
if (machine->kernel_filename) {
riscv_load_kernel(machine, &s->soc.cpus,
memmap[IBEX_DEV_RAM].base,
- false, NULL);
+ false, 0, 0, NULL);
}
}
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 04939b60c3..5b47d539a6 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -116,7 +116,7 @@ static void sifive_e_machine_init(MachineState *machine)
if (machine->kernel_filename) {
riscv_load_kernel(machine, &s->soc.cpus,
memmap[SIFIVE_E_DEV_DTIM].base,
- false, NULL);
+ false, 0, 0, NULL);
}
}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 35a335b8d0..b45fdc968c 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -599,7 +599,10 @@ static void sifive_u_machine_init(MachineState *machine)
firmware_end_addr);
kernel_entry = riscv_load_kernel(machine, &s->soc.u_cpus,
- kernel_start_addr, true, NULL);
+ kernel_start_addr, true,
+ memmap[SIFIVE_U_DEV_DRAM].base,
+ memmap[SIFIVE_U_DEV_DRAM].size,
+ NULL);
} else {
/*
* If dynamic firmware is used, it doesn't know where is the next mode
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index a584d5b3a2..e322ed8506 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -307,7 +307,10 @@ static void spike_board_init(MachineState *machine)
kernel_entry = riscv_load_kernel(machine, &s->soc[0],
kernel_start_addr,
- true, htif_symbol_callback);
+ true,
+ memmap[SPIKE_DRAM].base,
+ memmap[SPIKE_DRAM].size,
+ htif_symbol_callback);
} else {
/*
* If dynamic firmware is used, it doesn't know where is the next mode
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4e3efbee16..11f26b0dc0 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1287,7 +1287,10 @@ static void virt_machine_done(Notifier *notifier, void *data)
firmware_end_addr);
kernel_entry = riscv_load_kernel(machine, &s->soc[0],
- kernel_start_addr, true, NULL);
+ kernel_start_addr, true,
+ memmap[VIRT_DRAM].base,
+ memmap[VIRT_DRAM].size,
+ NULL);
} else {
/*
* If dynamic firmware is used, it doesn't know where is the next mode
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index a2e4ae9cb0..987e1add38 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -47,6 +47,8 @@ target_ulong riscv_load_kernel(MachineState *machine,
RISCVHartArrayState *harts,
target_ulong firmware_end_addr,
bool load_initrd,
+ uint64_t ram_base,
+ uint64_t ram_size,
symbol_fn_t sym_cb);
uint64_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size,
MachineState *ms);
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] hw/riscv: Fix the bug of max size limit when put initrd to RAM
2023-03-12 9:40 [PATCH v2] hw/riscv: Fix the bug of max size limit when put initrd to RAM Hang Xu
@ 2023-03-12 22:25 ` Daniel Henrique Barboza
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Henrique Barboza @ 2023-03-12 22:25 UTC (permalink / raw)
To: Hang Xu, qemu-riscv
Cc: palmer, bmeng, alistair.francis, qemu-devel@nongnu.org
Hi,
First, all patches in QEMU must also go to to the general mailing list (qemu-devel)
as well. I'm adding it in the CC in this reply. Sorry to not mentioning it in the
first version - I noticed that you didn't CC qemu-devel here, and in v1, just now
when replying to this patch.
As for the commit title, we don't need to mention that you're fixing a bug. I mean,
at least for me, if it needs fixing it's already a bug :D
I'd go with:
"hw/riscv: Fix max initrd size limit when put initrd to RAM"
On 3/12/23 06:40, Hang Xu wrote:
> Because the starting address of ram is not necessarily 0,
> the remaining free space in ram is
> ram_size - (start - ram_base) instead of ram_size-start.
> The changes based on patch1 are as follows:
> 1. Rebase
> 2. Considering that the ram block may be discontinuous
So, when documenting changes from one version to the other we usually do in one
of two ways:
- put it right after the "---" after your "Signed-off-by". Everything you
put there won't be converted into code or commit message when applying the
patch;
- create the patch with --cover-letter and document it there in separate.
What you did here will force the maintainer to remove this change history
from the commit message when applying it.
>
> Signed-off-by: Hang Xu <xuhang@eswincomputing.com>
> ---
> hw/riscv/boot.c | 18 ++++++++++++------
> hw/riscv/microchip_pfsoc.c | 5 ++++-
> hw/riscv/opentitan.c | 2 +-
> hw/riscv/sifive_e.c | 2 +-
> hw/riscv/sifive_u.c | 5 ++++-
> hw/riscv/spike.c | 5 ++++-
> hw/riscv/virt.c | 5 ++++-
> include/hw/riscv/boot.h | 2 ++
> 8 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 52bf8e67de..73b44a0c8b 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -173,10 +173,10 @@ target_ulong riscv_load_firmware(const char *firmware_filename,
> exit(1);
> }
>
> -static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
> +static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry,
> + uint64_t ram_base, uint64_t ram_size)
> {
> const char *filename = machine->initrd_filename;
> - uint64_t mem_size = machine->ram_size;
> void *fdt = machine->fdt;
> hwaddr start, end;
> ssize_t size;
> @@ -193,12 +193,16 @@ static void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
> * So for boards with less than 256MB of RAM we put the initrd
> * halfway into RAM, and for boards with 256MB of RAM or more we put
> * the initrd at 128MB.
> + * A ram_size == 0, usually from a MemMapEntry[].size element,
> + * means that the RAM block goes all the way to ms->ram_size.
> */
> - start = kernel_entry + MIN(mem_size / 2, 128 * MiB);
> + ram_size = ram_size ? MIN(machine->ram_size, ram_size) : machine->ram_size;
> + start = kernel_entry + MIN(ram_size / 2, 128 * MiB);
> + uint64_t max_initrd = ram_size - (start - ram_base);
Please declare 'max_initrd' at the start of the function with the other vars.
Everything else looks good to me.
Thanks,
Daniel
>
> - size = load_ramdisk(filename, start, mem_size - start);
> + size = load_ramdisk(filename, start, max_initrd);
> if (size == -1) {
> - size = load_image_targphys(filename, start, mem_size - start);
> + size = load_image_targphys(filename, start, max_initrd);
> if (size == -1) {
> error_report("could not load ramdisk '%s'", filename);
> exit(1);
> @@ -217,6 +221,8 @@ target_ulong riscv_load_kernel(MachineState *machine,
> RISCVHartArrayState *harts,
> target_ulong kernel_start_addr,
> bool load_initrd,
> + uint64_t ram_base,
> + uint64_t ram_size,
> symbol_fn_t sym_cb)
> {
> const char *kernel_filename = machine->kernel_filename;
> @@ -263,7 +269,7 @@ out:
> }
>
> if (load_initrd && machine->initrd_filename) {
> - riscv_load_initrd(machine, kernel_entry);
> + riscv_load_initrd(machine, kernel_entry, ram_base, ram_size);
> }
>
> if (fdt && machine->kernel_cmdline && *machine->kernel_cmdline) {
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index e81bbd12df..b42d90b89e 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -630,7 +630,10 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
> firmware_end_addr);
>
> kernel_entry = riscv_load_kernel(machine, &s->soc.u_cpus,
> - kernel_start_addr, true, NULL);
> + kernel_start_addr, true,
> + memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> + memmap[MICROCHIP_PFSOC_DRAM_LO].size,
> + NULL);
>
> /* Compute the fdt load address in dram */
> fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index b06944d382..bb663523d5 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -103,7 +103,7 @@ static void opentitan_board_init(MachineState *machine)
> if (machine->kernel_filename) {
> riscv_load_kernel(machine, &s->soc.cpus,
> memmap[IBEX_DEV_RAM].base,
> - false, NULL);
> + false, 0, 0, NULL);
> }
> }
>
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 04939b60c3..5b47d539a6 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -116,7 +116,7 @@ static void sifive_e_machine_init(MachineState *machine)
> if (machine->kernel_filename) {
> riscv_load_kernel(machine, &s->soc.cpus,
> memmap[SIFIVE_E_DEV_DTIM].base,
> - false, NULL);
> + false, 0, 0, NULL);
> }
> }
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 35a335b8d0..b45fdc968c 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -599,7 +599,10 @@ static void sifive_u_machine_init(MachineState *machine)
> firmware_end_addr);
>
> kernel_entry = riscv_load_kernel(machine, &s->soc.u_cpus,
> - kernel_start_addr, true, NULL);
> + kernel_start_addr, true,
> + memmap[SIFIVE_U_DEV_DRAM].base,
> + memmap[SIFIVE_U_DEV_DRAM].size,
> + NULL);
> } else {
> /*
> * If dynamic firmware is used, it doesn't know where is the next mode
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index a584d5b3a2..e322ed8506 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -307,7 +307,10 @@ static void spike_board_init(MachineState *machine)
>
> kernel_entry = riscv_load_kernel(machine, &s->soc[0],
> kernel_start_addr,
> - true, htif_symbol_callback);
> + true,
> + memmap[SPIKE_DRAM].base,
> + memmap[SPIKE_DRAM].size,
> + htif_symbol_callback);
> } else {
> /*
> * If dynamic firmware is used, it doesn't know where is the next mode
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 4e3efbee16..11f26b0dc0 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1287,7 +1287,10 @@ static void virt_machine_done(Notifier *notifier, void *data)
> firmware_end_addr);
>
> kernel_entry = riscv_load_kernel(machine, &s->soc[0],
> - kernel_start_addr, true, NULL);
> + kernel_start_addr, true,
> + memmap[VIRT_DRAM].base,
> + memmap[VIRT_DRAM].size,
> + NULL);
> } else {
> /*
> * If dynamic firmware is used, it doesn't know where is the next mode
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index a2e4ae9cb0..987e1add38 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -47,6 +47,8 @@ target_ulong riscv_load_kernel(MachineState *machine,
> RISCVHartArrayState *harts,
> target_ulong firmware_end_addr,
> bool load_initrd,
> + uint64_t ram_base,
> + uint64_t ram_size,
> symbol_fn_t sym_cb);
> uint64_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size,
> MachineState *ms);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-03-12 22:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-12 9:40 [PATCH v2] hw/riscv: Fix the bug of max size limit when put initrd to RAM Hang Xu
2023-03-12 22:25 ` Daniel Henrique Barboza
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.