| Message ID | 20241021040942.400-2-jim.shu@sifive.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support 64-bit address of initrd | expand |
On 10/21/24 1:09 AM, Jim Shu wrote: > Larger initrd image will overlap the DTB at 3GB address. Since 64-bit > system doesn't have 32-bit addressable issue, we just load DTB to the end > of dram in 64-bit system. > > Signed-off-by: Jim Shu <jim.shu@sifive.com> > --- > hw/riscv/boot.c | 8 ++++++-- > hw/riscv/microchip_pfsoc.c | 4 ++-- > hw/riscv/sifive_u.c | 4 ++-- > hw/riscv/spike.c | 4 ++-- > hw/riscv/virt.c | 2 +- > include/hw/riscv/boot.h | 2 +- > 6 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > index 9115ecd91f..ad45bd7a6a 100644 > --- a/hw/riscv/boot.c > +++ b/hw/riscv/boot.c > @@ -293,7 +293,7 @@ out: > * The FDT is fdt_packed() during the calculation. > */ > uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size, > - MachineState *ms) > + MachineState *ms, RISCVHartArrayState *harts) > { > int ret = fdt_pack(ms->fdt); > hwaddr dram_end, temp; > @@ -321,7 +321,11 @@ uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size, > * Thus, put it at an 2MB aligned address that less than fdt size from the > * end of dram or 3GB whichever is lesser. > */ > - temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; > + if (!riscv_is_32bit(harts)) { > + temp = dram_end; > + } else { > + temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; > + } I think it's a fine idea to do a different computation if we're running a 64 bit CPU but this alone won't fix the issue you're reporting. Running a 64-bit CPU does not guarantee that 'dram_end' will be enough to fit kernel+initrd and the fdt. There's no correlation between running 64 bits and having enough RAM to fit everything, i.e. you can run a 64-bit CPU with short RAM and end up with the same problem. There's also the board mem topology to account for: the Microchip board will always use a 1GB ram bank ('dram_end' will be always 1Gb max) running 32 or 64 bit CPUs. It seems that we also need to consider the end of kernel+initrd to properly tune where to put the fdt, erroring out if we don't have enough size for everything. This would make the logic work regardless of the word size of the CPU. This information is calculated in riscv_load_kernel(), which is currently returning only the kernel start addr. We have the information we need written in the FDT as "/chosen/linux,initrd-end" but that isn't enough because this attribute is written only if we have an initrd. We would hit the same problem again if we have a big enough kernel in an low enough RAM env. First thing that comes to mind is to use an abstraction like typedef struct RISCVBootProps { uint64_t kernel_start; uint64_t kernel_end; bool is_64bit; } RISCVKernelProps; And use it to allow riscv_load_kernel() to write both kernel_start (the current return value) and kernel_end (that can be either kernel_start + kernel_size or, if we have an initrd, the value written in linux,initrd-end). riscv_compute_fdt_addr() would then use this as an extra arg and then we have the 'kernel_end' value to calculate the fdt_addr too. In fact, I think this can be further extended to fully encapsulate the boot process of all boards, that are kind of doing the same thing with all these helpers, into a single 'super function' that would receive a struct like the one from above (with more properties), setting all the boot parameters the board needs, pass it to a single function that does everything and just use the result as each board wants. This is something that we might play around with but it'll be something for the next release. Thanks, Daniel > > return QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB); > } > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c > index f9a3b43d2e..ba8b0a2c26 100644 > --- a/hw/riscv/microchip_pfsoc.c > +++ b/hw/riscv/microchip_pfsoc.c > @@ -519,7 +519,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) > bool kernel_as_payload = false; > target_ulong firmware_end_addr, kernel_start_addr; > uint64_t kernel_entry; > - uint32_t fdt_load_addr; > + uint64_t fdt_load_addr; > DriveInfo *dinfo = drive_get(IF_SD, 0, 0); > > /* Sanity check on RAM size */ > @@ -625,7 +625,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) > /* Compute the fdt load address in dram */ > fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base, > memmap[MICROCHIP_PFSOC_DRAM_LO].size, > - machine); > + machine, &s->soc.u_cpus); > riscv_load_fdt(fdt_load_addr, machine->fdt); > > /* Load the reset vector */ > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > index 9b3dcf3a7a..fd974f2357 100644 > --- a/hw/riscv/sifive_u.c > +++ b/hw/riscv/sifive_u.c > @@ -519,7 +519,7 @@ static void sifive_u_machine_init(MachineState *machine) > const char *firmware_name; > uint32_t start_addr_hi32 = 0x00000000; > int i; > - uint32_t fdt_load_addr; > + uint64_t fdt_load_addr; > uint64_t kernel_entry; > DriveInfo *dinfo; > BlockBackend *blk; > @@ -606,7 +606,7 @@ static void sifive_u_machine_init(MachineState *machine) > > fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base, > memmap[SIFIVE_U_DEV_DRAM].size, > - machine); > + machine, &s->soc.u_cpus); > riscv_load_fdt(fdt_load_addr, machine->fdt); > > if (!riscv_is_32bit(&s->soc.u_cpus)) { > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c > index fceb91d946..acd7ab1ae1 100644 > --- a/hw/riscv/spike.c > +++ b/hw/riscv/spike.c > @@ -201,7 +201,7 @@ static void spike_board_init(MachineState *machine) > hwaddr firmware_load_addr = memmap[SPIKE_DRAM].base; > target_ulong kernel_start_addr; > char *firmware_name; > - uint32_t fdt_load_addr; > + uint64_t fdt_load_addr; > uint64_t kernel_entry; > char *soc_name; > int i, base_hartid, hart_count; > @@ -317,7 +317,7 @@ static void spike_board_init(MachineState *machine) > > fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base, > memmap[SPIKE_DRAM].size, > - machine); > + machine, &s->soc[0]); > riscv_load_fdt(fdt_load_addr, machine->fdt); > > /* load the reset vector */ > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index ee3129f3b3..cfbeeaf7d5 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -1399,7 +1399,7 @@ static void virt_machine_done(Notifier *notifier, void *data) > > fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base, > memmap[VIRT_DRAM].size, > - machine); > + machine, &s->soc[0]); > riscv_load_fdt(fdt_load_addr, machine->fdt); > > /* load the reset vector */ > diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h > index 18bfe9f7bf..7ccbe5f62a 100644 > --- a/include/hw/riscv/boot.h > +++ b/include/hw/riscv/boot.h > @@ -49,7 +49,7 @@ target_ulong riscv_load_kernel(MachineState *machine, > bool load_initrd, > symbol_fn_t sym_cb); > uint64_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, > - MachineState *ms); > + MachineState *ms, RISCVHartArrayState *harts); > void riscv_load_fdt(hwaddr fdt_addr, void *fdt); > void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts, > hwaddr saddr,
On Mon, Oct 21, 2024 at 9:42 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > > > On 10/21/24 1:09 AM, Jim Shu wrote: > > Larger initrd image will overlap the DTB at 3GB address. Since 64-bit > > system doesn't have 32-bit addressable issue, we just load DTB to the end > > of dram in 64-bit system. > > > > Signed-off-by: Jim Shu <jim.shu@sifive.com> > > --- > > hw/riscv/boot.c | 8 ++++++-- > > hw/riscv/microchip_pfsoc.c | 4 ++-- > > hw/riscv/sifive_u.c | 4 ++-- > > hw/riscv/spike.c | 4 ++-- > > hw/riscv/virt.c | 2 +- > > include/hw/riscv/boot.h | 2 +- > > 6 files changed, 14 insertions(+), 10 deletions(-) > > > > diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > > index 9115ecd91f..ad45bd7a6a 100644 > > --- a/hw/riscv/boot.c > > +++ b/hw/riscv/boot.c > > @@ -293,7 +293,7 @@ out: > > * The FDT is fdt_packed() during the calculation. > > */ > > uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size, > > - MachineState *ms) > > + MachineState *ms, RISCVHartArrayState *harts) > > { > > int ret = fdt_pack(ms->fdt); > > hwaddr dram_end, temp; > > @@ -321,7 +321,11 @@ uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size, > > * Thus, put it at an 2MB aligned address that less than fdt size from the > > * end of dram or 3GB whichever is lesser. > > */ > > - temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; > > + if (!riscv_is_32bit(harts)) { > > + temp = dram_end; > > + } else { > > + temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; > > + } > > I think it's a fine idea to do a different computation if we're running a 64 bit > CPU but this alone won't fix the issue you're reporting. > > Running a 64-bit CPU does not guarantee that 'dram_end' will be enough to fit > kernel+initrd and the fdt. There's no correlation between running 64 bits and having > enough RAM to fit everything, i.e. you can run a 64-bit CPU with short RAM and end > up with the same problem. There's also the board mem topology to account for: the > Microchip board will always use a 1GB ram bank ('dram_end' will be always 1Gb max) > running 32 or 64 bit CPUs. > > It seems that we also need to consider the end of kernel+initrd to properly tune > where to put the fdt, erroring out if we don't have enough size for everything. > This would make the logic work regardless of the word size of the CPU. > > This information is calculated in riscv_load_kernel(), which is currently > returning only the kernel start addr. We have the information we need written in > the FDT as "/chosen/linux,initrd-end" but that isn't enough because this attribute > is written only if we have an initrd. We would hit the same problem again if we > have a big enough kernel in an low enough RAM env. > > First thing that comes to mind is to use an abstraction like > > typedef struct RISCVBootProps { > uint64_t kernel_start; > uint64_t kernel_end; > bool is_64bit; > } RISCVKernelProps; > > And use it to allow riscv_load_kernel() to write both kernel_start (the current > return value) and kernel_end (that can be either kernel_start + kernel_size or, > if we have an initrd, the value written in linux,initrd-end). riscv_compute_fdt_addr() > would then use this as an extra arg and then we have the 'kernel_end' value to > calculate the fdt_addr too. > > > > In fact, I think this can be further extended to fully encapsulate the boot process > of all boards, that are kind of doing the same thing with all these helpers, into a > single 'super function' that would receive a struct like the one from above (with > more properties), setting all the boot parameters the board needs, pass it to a single > function that does everything and just use the result as each board wants. This > is something that we might play around with but it'll be something for the next > release. > > > Thanks, > > Daniel Thanks for the feedback. I can try to add the checking of overlapping between initrd and dtb in the V2 patch. I think it may change some boot APIs in "hw/riscv/boot.c" ARM has an abstraction of the most info in `struct arm_boot_info`, so it can add checkings in the functions like `arm_setup_direct_kernel_boot()`. We may do the similar work but be simpler at first. Thanks, Jim Shu > > > > > > > return QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB); > > } > > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c > > index f9a3b43d2e..ba8b0a2c26 100644 > > --- a/hw/riscv/microchip_pfsoc.c > > +++ b/hw/riscv/microchip_pfsoc.c > > @@ -519,7 +519,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) > > bool kernel_as_payload = false; > > target_ulong firmware_end_addr, kernel_start_addr; > > uint64_t kernel_entry; > > - uint32_t fdt_load_addr; > > + uint64_t fdt_load_addr; > > DriveInfo *dinfo = drive_get(IF_SD, 0, 0); > > > > /* Sanity check on RAM size */ > > @@ -625,7 +625,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) > > /* Compute the fdt load address in dram */ > > fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base, > > memmap[MICROCHIP_PFSOC_DRAM_LO].size, > > - machine); > > + machine, &s->soc.u_cpus); > > riscv_load_fdt(fdt_load_addr, machine->fdt); > > > > /* Load the reset vector */ > > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > > index 9b3dcf3a7a..fd974f2357 100644 > > --- a/hw/riscv/sifive_u.c > > +++ b/hw/riscv/sifive_u.c > > @@ -519,7 +519,7 @@ static void sifive_u_machine_init(MachineState *machine) > > const char *firmware_name; > > uint32_t start_addr_hi32 = 0x00000000; > > int i; > > - uint32_t fdt_load_addr; > > + uint64_t fdt_load_addr; > > uint64_t kernel_entry; > > DriveInfo *dinfo; > > BlockBackend *blk; > > @@ -606,7 +606,7 @@ static void sifive_u_machine_init(MachineState *machine) > > > > fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base, > > memmap[SIFIVE_U_DEV_DRAM].size, > > - machine); > > + machine, &s->soc.u_cpus); > > riscv_load_fdt(fdt_load_addr, machine->fdt); > > > > if (!riscv_is_32bit(&s->soc.u_cpus)) { > > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c > > index fceb91d946..acd7ab1ae1 100644 > > --- a/hw/riscv/spike.c > > +++ b/hw/riscv/spike.c > > @@ -201,7 +201,7 @@ static void spike_board_init(MachineState *machine) > > hwaddr firmware_load_addr = memmap[SPIKE_DRAM].base; > > target_ulong kernel_start_addr; > > char *firmware_name; > > - uint32_t fdt_load_addr; > > + uint64_t fdt_load_addr; > > uint64_t kernel_entry; > > char *soc_name; > > int i, base_hartid, hart_count; > > @@ -317,7 +317,7 @@ static void spike_board_init(MachineState *machine) > > > > fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base, > > memmap[SPIKE_DRAM].size, > > - machine); > > + machine, &s->soc[0]); > > riscv_load_fdt(fdt_load_addr, machine->fdt); > > > > /* load the reset vector */ > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > > index ee3129f3b3..cfbeeaf7d5 100644 > > --- a/hw/riscv/virt.c > > +++ b/hw/riscv/virt.c > > @@ -1399,7 +1399,7 @@ static void virt_machine_done(Notifier *notifier, void *data) > > > > fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base, > > memmap[VIRT_DRAM].size, > > - machine); > > + machine, &s->soc[0]); > > riscv_load_fdt(fdt_load_addr, machine->fdt); > > > > /* load the reset vector */ > > diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h > > index 18bfe9f7bf..7ccbe5f62a 100644 > > --- a/include/hw/riscv/boot.h > > +++ b/include/hw/riscv/boot.h > > @@ -49,7 +49,7 @@ target_ulong riscv_load_kernel(MachineState *machine, > > bool load_initrd, > > symbol_fn_t sym_cb); > > uint64_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, > > - MachineState *ms); > > + MachineState *ms, RISCVHartArrayState *harts); > > void riscv_load_fdt(hwaddr fdt_addr, void *fdt); > > void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts, > > hwaddr saddr,
On 10/23/24 7:19 AM, Jim Shu wrote: > On Mon, Oct 21, 2024 at 9:42 PM Daniel Henrique Barboza > <dbarboza@ventanamicro.com> wrote: >> >> >> >> On 10/21/24 1:09 AM, Jim Shu wrote: >>> Larger initrd image will overlap the DTB at 3GB address. Since 64-bit >>> system doesn't have 32-bit addressable issue, we just load DTB to the end >>> of dram in 64-bit system. >>> >>> Signed-off-by: Jim Shu <jim.shu@sifive.com> >>> --- >>> hw/riscv/boot.c | 8 ++++++-- >>> hw/riscv/microchip_pfsoc.c | 4 ++-- >>> hw/riscv/sifive_u.c | 4 ++-- >>> hw/riscv/spike.c | 4 ++-- >>> hw/riscv/virt.c | 2 +- >>> include/hw/riscv/boot.h | 2 +- >>> 6 files changed, 14 insertions(+), 10 deletions(-) >>> >>> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c >>> index 9115ecd91f..ad45bd7a6a 100644 >>> --- a/hw/riscv/boot.c >>> +++ b/hw/riscv/boot.c >>> @@ -293,7 +293,7 @@ out: >>> * The FDT is fdt_packed() during the calculation. >>> */ >>> uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size, >>> - MachineState *ms) >>> + MachineState *ms, RISCVHartArrayState *harts) >>> { >>> int ret = fdt_pack(ms->fdt); >>> hwaddr dram_end, temp; >>> @@ -321,7 +321,11 @@ uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size, >>> * Thus, put it at an 2MB aligned address that less than fdt size from the >>> * end of dram or 3GB whichever is lesser. >>> */ >>> - temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; >>> + if (!riscv_is_32bit(harts)) { >>> + temp = dram_end; >>> + } else { >>> + temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; >>> + } >> >> I think it's a fine idea to do a different computation if we're running a 64 bit >> CPU but this alone won't fix the issue you're reporting. >> >> Running a 64-bit CPU does not guarantee that 'dram_end' will be enough to fit >> kernel+initrd and the fdt. There's no correlation between running 64 bits and having >> enough RAM to fit everything, i.e. you can run a 64-bit CPU with short RAM and end >> up with the same problem. There's also the board mem topology to account for: the >> Microchip board will always use a 1GB ram bank ('dram_end' will be always 1Gb max) >> running 32 or 64 bit CPUs. >> >> It seems that we also need to consider the end of kernel+initrd to properly tune >> where to put the fdt, erroring out if we don't have enough size for everything. >> This would make the logic work regardless of the word size of the CPU. >> >> This information is calculated in riscv_load_kernel(), which is currently >> returning only the kernel start addr. We have the information we need written in >> the FDT as "/chosen/linux,initrd-end" but that isn't enough because this attribute >> is written only if we have an initrd. We would hit the same problem again if we >> have a big enough kernel in an low enough RAM env. >> >> First thing that comes to mind is to use an abstraction like >> >> typedef struct RISCVBootProps { >> uint64_t kernel_start; >> uint64_t kernel_end; >> bool is_64bit; >> } RISCVKernelProps; >> >> And use it to allow riscv_load_kernel() to write both kernel_start (the current >> return value) and kernel_end (that can be either kernel_start + kernel_size or, >> if we have an initrd, the value written in linux,initrd-end). riscv_compute_fdt_addr() >> would then use this as an extra arg and then we have the 'kernel_end' value to >> calculate the fdt_addr too. >> >> >> >> In fact, I think this can be further extended to fully encapsulate the boot process >> of all boards, that are kind of doing the same thing with all these helpers, into a >> single 'super function' that would receive a struct like the one from above (with >> more properties), setting all the boot parameters the board needs, pass it to a single >> function that does everything and just use the result as each board wants. This >> is something that we might play around with but it'll be something for the next >> release. >> >> >> Thanks, >> >> Daniel > > Thanks for the feedback. I can try to add the checking of overlapping > between initrd and dtb in the V2 patch. > I think it may change some boot APIs in "hw/riscv/boot.c" > > ARM has an abstraction of the most info in `struct arm_boot_info`, so > it can add checkings in the functions like > `arm_setup_direct_kernel_boot()`. > We may do the similar work but be simpler at first. Don't need to go too deep right now. You can focus on fixing the problem you have at hand first, adding a 'struct riscv_boot_info' if you want to take an inspiration from ARM, and passing just the kernel_start and kernel_end attrs around. In a second step we can use the same struct, extend it and refactor hw/riscv/boot.c (and its callers) accordingly. If we could make the board populate a riscv_boot_info struct and then just do a single call to a riscv_machine_boot() function that would do everything, returning the updated struct to the board, that would be great. ps: all function names I'm using up there are tentative. Feel free to name the functions whatever you think fits best. Thanks, Daniel > > > Thanks, > Jim Shu > >> >> >> >>> >>> return QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB); >>> } >>> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c >>> index f9a3b43d2e..ba8b0a2c26 100644 >>> --- a/hw/riscv/microchip_pfsoc.c >>> +++ b/hw/riscv/microchip_pfsoc.c >>> @@ -519,7 +519,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) >>> bool kernel_as_payload = false; >>> target_ulong firmware_end_addr, kernel_start_addr; >>> uint64_t kernel_entry; >>> - uint32_t fdt_load_addr; >>> + uint64_t fdt_load_addr; >>> DriveInfo *dinfo = drive_get(IF_SD, 0, 0); >>> >>> /* Sanity check on RAM size */ >>> @@ -625,7 +625,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) >>> /* Compute the fdt load address in dram */ >>> fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base, >>> memmap[MICROCHIP_PFSOC_DRAM_LO].size, >>> - machine); >>> + machine, &s->soc.u_cpus); >>> riscv_load_fdt(fdt_load_addr, machine->fdt); >>> >>> /* Load the reset vector */ >>> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c >>> index 9b3dcf3a7a..fd974f2357 100644 >>> --- a/hw/riscv/sifive_u.c >>> +++ b/hw/riscv/sifive_u.c >>> @@ -519,7 +519,7 @@ static void sifive_u_machine_init(MachineState *machine) >>> const char *firmware_name; >>> uint32_t start_addr_hi32 = 0x00000000; >>> int i; >>> - uint32_t fdt_load_addr; >>> + uint64_t fdt_load_addr; >>> uint64_t kernel_entry; >>> DriveInfo *dinfo; >>> BlockBackend *blk; >>> @@ -606,7 +606,7 @@ static void sifive_u_machine_init(MachineState *machine) >>> >>> fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base, >>> memmap[SIFIVE_U_DEV_DRAM].size, >>> - machine); >>> + machine, &s->soc.u_cpus); >>> riscv_load_fdt(fdt_load_addr, machine->fdt); >>> >>> if (!riscv_is_32bit(&s->soc.u_cpus)) { >>> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c >>> index fceb91d946..acd7ab1ae1 100644 >>> --- a/hw/riscv/spike.c >>> +++ b/hw/riscv/spike.c >>> @@ -201,7 +201,7 @@ static void spike_board_init(MachineState *machine) >>> hwaddr firmware_load_addr = memmap[SPIKE_DRAM].base; >>> target_ulong kernel_start_addr; >>> char *firmware_name; >>> - uint32_t fdt_load_addr; >>> + uint64_t fdt_load_addr; >>> uint64_t kernel_entry; >>> char *soc_name; >>> int i, base_hartid, hart_count; >>> @@ -317,7 +317,7 @@ static void spike_board_init(MachineState *machine) >>> >>> fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base, >>> memmap[SPIKE_DRAM].size, >>> - machine); >>> + machine, &s->soc[0]); >>> riscv_load_fdt(fdt_load_addr, machine->fdt); >>> >>> /* load the reset vector */ >>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c >>> index ee3129f3b3..cfbeeaf7d5 100644 >>> --- a/hw/riscv/virt.c >>> +++ b/hw/riscv/virt.c >>> @@ -1399,7 +1399,7 @@ static void virt_machine_done(Notifier *notifier, void *data) >>> >>> fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base, >>> memmap[VIRT_DRAM].size, >>> - machine); >>> + machine, &s->soc[0]); >>> riscv_load_fdt(fdt_load_addr, machine->fdt); >>> >>> /* load the reset vector */ >>> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h >>> index 18bfe9f7bf..7ccbe5f62a 100644 >>> --- a/include/hw/riscv/boot.h >>> +++ b/include/hw/riscv/boot.h >>> @@ -49,7 +49,7 @@ target_ulong riscv_load_kernel(MachineState *machine, >>> bool load_initrd, >>> symbol_fn_t sym_cb); >>> uint64_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, >>> - MachineState *ms); >>> + MachineState *ms, RISCVHartArrayState *harts); >>> void riscv_load_fdt(hwaddr fdt_addr, void *fdt); >>> void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts, >>> hwaddr saddr,
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c index 9115ecd91f..ad45bd7a6a 100644 --- a/hw/riscv/boot.c +++ b/hw/riscv/boot.c @@ -293,7 +293,7 @@ out: * The FDT is fdt_packed() during the calculation. */ uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size, - MachineState *ms) + MachineState *ms, RISCVHartArrayState *harts) { int ret = fdt_pack(ms->fdt); hwaddr dram_end, temp; @@ -321,7 +321,11 @@ uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size, * Thus, put it at an 2MB aligned address that less than fdt size from the * end of dram or 3GB whichever is lesser. */ - temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; + if (!riscv_is_32bit(harts)) { + temp = dram_end; + } else { + temp = (dram_base < 3072 * MiB) ? MIN(dram_end, 3072 * MiB) : dram_end; + } return QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB); } diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c index f9a3b43d2e..ba8b0a2c26 100644 --- a/hw/riscv/microchip_pfsoc.c +++ b/hw/riscv/microchip_pfsoc.c @@ -519,7 +519,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) bool kernel_as_payload = false; target_ulong firmware_end_addr, kernel_start_addr; uint64_t kernel_entry; - uint32_t fdt_load_addr; + uint64_t fdt_load_addr; DriveInfo *dinfo = drive_get(IF_SD, 0, 0); /* Sanity check on RAM size */ @@ -625,7 +625,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) /* Compute the fdt load address in dram */ fdt_load_addr = riscv_compute_fdt_addr(memmap[MICROCHIP_PFSOC_DRAM_LO].base, memmap[MICROCHIP_PFSOC_DRAM_LO].size, - machine); + machine, &s->soc.u_cpus); riscv_load_fdt(fdt_load_addr, machine->fdt); /* Load the reset vector */ diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index 9b3dcf3a7a..fd974f2357 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -519,7 +519,7 @@ static void sifive_u_machine_init(MachineState *machine) const char *firmware_name; uint32_t start_addr_hi32 = 0x00000000; int i; - uint32_t fdt_load_addr; + uint64_t fdt_load_addr; uint64_t kernel_entry; DriveInfo *dinfo; BlockBackend *blk; @@ -606,7 +606,7 @@ static void sifive_u_machine_init(MachineState *machine) fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base, memmap[SIFIVE_U_DEV_DRAM].size, - machine); + machine, &s->soc.u_cpus); riscv_load_fdt(fdt_load_addr, machine->fdt); if (!riscv_is_32bit(&s->soc.u_cpus)) { diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c index fceb91d946..acd7ab1ae1 100644 --- a/hw/riscv/spike.c +++ b/hw/riscv/spike.c @@ -201,7 +201,7 @@ static void spike_board_init(MachineState *machine) hwaddr firmware_load_addr = memmap[SPIKE_DRAM].base; target_ulong kernel_start_addr; char *firmware_name; - uint32_t fdt_load_addr; + uint64_t fdt_load_addr; uint64_t kernel_entry; char *soc_name; int i, base_hartid, hart_count; @@ -317,7 +317,7 @@ static void spike_board_init(MachineState *machine) fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base, memmap[SPIKE_DRAM].size, - machine); + machine, &s->soc[0]); riscv_load_fdt(fdt_load_addr, machine->fdt); /* load the reset vector */ diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index ee3129f3b3..cfbeeaf7d5 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -1399,7 +1399,7 @@ static void virt_machine_done(Notifier *notifier, void *data) fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base, memmap[VIRT_DRAM].size, - machine); + machine, &s->soc[0]); riscv_load_fdt(fdt_load_addr, machine->fdt); /* load the reset vector */ diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h index 18bfe9f7bf..7ccbe5f62a 100644 --- a/include/hw/riscv/boot.h +++ b/include/hw/riscv/boot.h @@ -49,7 +49,7 @@ target_ulong riscv_load_kernel(MachineState *machine, bool load_initrd, symbol_fn_t sym_cb); uint64_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, - MachineState *ms); + MachineState *ms, RISCVHartArrayState *harts); void riscv_load_fdt(hwaddr fdt_addr, void *fdt); void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts, hwaddr saddr,
Larger initrd image will overlap the DTB at 3GB address. Since 64-bit system doesn't have 32-bit addressable issue, we just load DTB to the end of dram in 64-bit system. Signed-off-by: Jim Shu <jim.shu@sifive.com> --- hw/riscv/boot.c | 8 ++++++-- hw/riscv/microchip_pfsoc.c | 4 ++-- hw/riscv/sifive_u.c | 4 ++-- hw/riscv/spike.c | 4 ++-- hw/riscv/virt.c | 2 +- include/hw/riscv/boot.h | 2 +- 6 files changed, 14 insertions(+), 10 deletions(-)