diff mbox series

[v2,3/6] hw/riscv: simplify riscv_compute_fdt_addr()

Message ID 20230116173420.1146808-4-dbarboza@ventanamicro.com (mailing list archive)
State New, archived
Headers show
Series riscv: fdt related cleanups | expand

Commit Message

Daniel Henrique Barboza Jan. 16, 2023, 5:34 p.m. UTC
All callers are using attributes from the MachineState object. Use a
pointer to it instead of passing dram_size (which is always
machine->ram_size) and fdt (always machine->fdt).

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 hw/riscv/boot.c            | 6 +++---
 hw/riscv/microchip_pfsoc.c | 4 ++--
 hw/riscv/sifive_u.c        | 4 ++--
 hw/riscv/spike.c           | 4 ++--
 hw/riscv/virt.c            | 3 +--
 include/hw/riscv/boot.h    | 2 +-
 6 files changed, 11 insertions(+), 12 deletions(-)

Comments

Alistair Francis Jan. 19, 2023, 2:23 a.m. UTC | #1
On Tue, Jan 17, 2023 at 3:34 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> All callers are using attributes from the MachineState object. Use a
> pointer to it instead of passing dram_size (which is always
> machine->ram_size) and fdt (always machine->fdt).
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  hw/riscv/boot.c            | 6 +++---
>  hw/riscv/microchip_pfsoc.c | 4 ++--
>  hw/riscv/sifive_u.c        | 4 ++--
>  hw/riscv/spike.c           | 4 ++--
>  hw/riscv/virt.c            | 3 +--
>  include/hw/riscv/boot.h    | 2 +-
>  6 files changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index b213a32157..508da3f5c7 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -255,11 +255,11 @@ void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
>   *
>   * The FDT is fdt_packed() during the calculation.
>   */
> -uint32_t riscv_compute_fdt_addr(hwaddr dram_base, uint64_t mem_size,
> -                                void *fdt)
> +uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_base)
>  {
> +    void *fdt = machine->fdt;
>      uint64_t temp;
> -    hwaddr dram_end = dram_base + mem_size;
> +    hwaddr dram_end = dram_base + machine->ram_size;
>      int ret = fdt_pack(fdt);
>      int fdtsize;
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index dcdbc2cac3..a53e48e996 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -641,8 +641,8 @@ 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,
> -                                              machine->ram_size, machine->fdt);
> +        fdt_load_addr = riscv_compute_fdt_addr(machine,
> +                                               memmap[MICROCHIP_PFSOC_DRAM_LO].base);

I don't think this is correct here.

So, first up I understand we don't correctly handle this today, *but*
I see this change as a step in the wrong direction.

The problem here is that ram is split over two areas. So if
machine->ram_size is larger then 0x40000000 it is going to overflow
MICROCHIP_PFSOC_DRAM_LO and jump to MICROCHIP_PFSOC_DRAM_HI
(0x1000000000).

So we really want something like this

        /* Compute the fdt load address in dram */
        if (machine->ram_size > memmap[MICROCHIP_PFSOC_DRAM_LO].size) {
            fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_HI].base,
                                           machine->ram_size -
memmap[MICROCHIP_PFSOC_DRAM_LO].size,
                                           machine->fdt);
        } else {
            fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
                                           machine->ram_size,
                                           machine->fdt);
        }

to handle overflowing MICROCHIP_PFSOC_DRAM_LO. While this patch is
going in the wrong direction and making that more difficult

Alistair



>          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 626d4dc2f3..ebfddf161d 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -616,8 +616,8 @@ static void sifive_u_machine_init(MachineState *machine)
>          kernel_entry = 0;
>      }
>
> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base,
> -                                           machine->ram_size, machine->fdt);
> +    fdt_load_addr = riscv_compute_fdt_addr(machine,
> +                                           memmap[SIFIVE_U_DEV_DRAM].base);
>      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 88b9fdfc36..afd581436b 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -324,8 +324,8 @@ static void spike_board_init(MachineState *machine)
>          kernel_entry = 0;
>      }
>
> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base,
> -                                           machine->ram_size, machine->fdt);
> +    fdt_load_addr = riscv_compute_fdt_addr(machine,
> +                                           memmap[SPIKE_DRAM].base);
>      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 839dfaa125..cbba0b8930 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1307,8 +1307,7 @@ static void virt_machine_done(Notifier *notifier, void *data)
>          start_addr = virt_memmap[VIRT_FLASH].base;
>      }
>
> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base,
> -                                           machine->ram_size, machine->fdt);
> +    fdt_load_addr = riscv_compute_fdt_addr(machine, memmap[VIRT_DRAM].base);
>      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 9aea7b9c46..f933de88fb 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -47,7 +47,7 @@ target_ulong riscv_load_kernel(MachineState *machine,
>                                 target_ulong firmware_end_addr,
>                                 symbol_fn_t sym_cb);
>  void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry);
> -uint32_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, void *fdt);
> +uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_start);
>  void riscv_load_fdt(uint32_t fdt_addr, void *fdt);
>  void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts,
>                                 hwaddr saddr,
> --
> 2.39.0
>
>
Daniel Henrique Barboza Jan. 19, 2023, 1 p.m. UTC | #2
On 1/18/23 23:23, Alistair Francis wrote:
> On Tue, Jan 17, 2023 at 3:34 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>> All callers are using attributes from the MachineState object. Use a
>> pointer to it instead of passing dram_size (which is always
>> machine->ram_size) and fdt (always machine->fdt).
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   hw/riscv/boot.c            | 6 +++---
>>   hw/riscv/microchip_pfsoc.c | 4 ++--
>>   hw/riscv/sifive_u.c        | 4 ++--
>>   hw/riscv/spike.c           | 4 ++--
>>   hw/riscv/virt.c            | 3 +--
>>   include/hw/riscv/boot.h    | 2 +-
>>   6 files changed, 11 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
>> index b213a32157..508da3f5c7 100644
>> --- a/hw/riscv/boot.c
>> +++ b/hw/riscv/boot.c
>> @@ -255,11 +255,11 @@ void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
>>    *
>>    * The FDT is fdt_packed() during the calculation.
>>    */
>> -uint32_t riscv_compute_fdt_addr(hwaddr dram_base, uint64_t mem_size,
>> -                                void *fdt)
>> +uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_base)
>>   {
>> +    void *fdt = machine->fdt;
>>       uint64_t temp;
>> -    hwaddr dram_end = dram_base + mem_size;
>> +    hwaddr dram_end = dram_base + machine->ram_size;
>>       int ret = fdt_pack(fdt);
>>       int fdtsize;
>>
>> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
>> index dcdbc2cac3..a53e48e996 100644
>> --- a/hw/riscv/microchip_pfsoc.c
>> +++ b/hw/riscv/microchip_pfsoc.c
>> @@ -641,8 +641,8 @@ 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,
>> -                                              machine->ram_size, machine->fdt);
>> +        fdt_load_addr = riscv_compute_fdt_addr(machine,
>> +                                               memmap[MICROCHIP_PFSOC_DRAM_LO].base);
> I don't think this is correct here.
>
> So, first up I understand we don't correctly handle this today, *but*
> I see this change as a step in the wrong direction.
>
> The problem here is that ram is split over two areas. So if
> machine->ram_size is larger then 0x40000000 it is going to overflow
> MICROCHIP_PFSOC_DRAM_LO and jump to MICROCHIP_PFSOC_DRAM_HI
> (0x1000000000).

Yeah .... I'll add a new helper just to handle the microchip_pfsoc case which seems to
be the only RISC-V board that has sparse RAM.




>
> So we really want something like this
>
>          /* Compute the fdt load address in dram */
>          if (machine->ram_size > memmap[MICROCHIP_PFSOC_DRAM_LO].size) {
>              fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_HI].base,
>                                             machine->ram_size -
> memmap[MICROCHIP_PFSOC_DRAM_LO].size,
>                                             machine->fdt);
>          } else {
>              fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
>                                             machine->ram_size,
>                                             machine->fdt);
>          }
>
> to handle overflowing MICROCHIP_PFSOC_DRAM_LO. While this patch is
> going in the wrong direction and making that more difficult
>
> Alistair
>
>
>
>>           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 626d4dc2f3..ebfddf161d 100644
>> --- a/hw/riscv/sifive_u.c
>> +++ b/hw/riscv/sifive_u.c
>> @@ -616,8 +616,8 @@ static void sifive_u_machine_init(MachineState *machine)
>>           kernel_entry = 0;
>>       }
>>
>> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base,
>> -                                           machine->ram_size, machine->fdt);
>> +    fdt_load_addr = riscv_compute_fdt_addr(machine,
>> +                                           memmap[SIFIVE_U_DEV_DRAM].base);
>>       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 88b9fdfc36..afd581436b 100644
>> --- a/hw/riscv/spike.c
>> +++ b/hw/riscv/spike.c
>> @@ -324,8 +324,8 @@ static void spike_board_init(MachineState *machine)
>>           kernel_entry = 0;
>>       }
>>
>> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base,
>> -                                           machine->ram_size, machine->fdt);
>> +    fdt_load_addr = riscv_compute_fdt_addr(machine,
>> +                                           memmap[SPIKE_DRAM].base);
>>       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 839dfaa125..cbba0b8930 100644
>> --- a/hw/riscv/virt.c
>> +++ b/hw/riscv/virt.c
>> @@ -1307,8 +1307,7 @@ static void virt_machine_done(Notifier *notifier, void *data)
>>           start_addr = virt_memmap[VIRT_FLASH].base;
>>       }
>>
>> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base,
>> -                                           machine->ram_size, machine->fdt);
>> +    fdt_load_addr = riscv_compute_fdt_addr(machine, memmap[VIRT_DRAM].base);
>>       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 9aea7b9c46..f933de88fb 100644
>> --- a/include/hw/riscv/boot.h
>> +++ b/include/hw/riscv/boot.h
>> @@ -47,7 +47,7 @@ target_ulong riscv_load_kernel(MachineState *machine,
>>                                  target_ulong firmware_end_addr,
>>                                  symbol_fn_t sym_cb);
>>   void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry);
>> -uint32_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, void *fdt);
>> +uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_start);
>>   void riscv_load_fdt(uint32_t fdt_addr, void *fdt);
>>   void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts,
>>                                  hwaddr saddr,
>> --
>> 2.39.0
>>
>>
Daniel Henrique Barboza Jan. 19, 2023, 4:47 p.m. UTC | #3
On 1/18/23 23:23, Alistair Francis wrote:
> On Tue, Jan 17, 2023 at 3:34 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>> All callers are using attributes from the MachineState object. Use a
>> pointer to it instead of passing dram_size (which is always
>> machine->ram_size) and fdt (always machine->fdt).
>>
>> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> ---
>>   hw/riscv/boot.c            | 6 +++---
>>   hw/riscv/microchip_pfsoc.c | 4 ++--
>>   hw/riscv/sifive_u.c        | 4 ++--
>>   hw/riscv/spike.c           | 4 ++--
>>   hw/riscv/virt.c            | 3 +--
>>   include/hw/riscv/boot.h    | 2 +-
>>   6 files changed, 11 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
>> index b213a32157..508da3f5c7 100644
>> --- a/hw/riscv/boot.c
>> +++ b/hw/riscv/boot.c
>> @@ -255,11 +255,11 @@ void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
>>    *
>>    * The FDT is fdt_packed() during the calculation.
>>    */
>> -uint32_t riscv_compute_fdt_addr(hwaddr dram_base, uint64_t mem_size,
>> -                                void *fdt)
>> +uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_base)
>>   {
>> +    void *fdt = machine->fdt;
>>       uint64_t temp;
>> -    hwaddr dram_end = dram_base + mem_size;
>> +    hwaddr dram_end = dram_base + machine->ram_size;
>>       int ret = fdt_pack(fdt);
>>       int fdtsize;
>>
>> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
>> index dcdbc2cac3..a53e48e996 100644
>> --- a/hw/riscv/microchip_pfsoc.c
>> +++ b/hw/riscv/microchip_pfsoc.c
>> @@ -641,8 +641,8 @@ 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,
>> -                                              machine->ram_size, machine->fdt);
>> +        fdt_load_addr = riscv_compute_fdt_addr(machine,
>> +                                               memmap[MICROCHIP_PFSOC_DRAM_LO].base);
> I don't think this is correct here.
>
> So, first up I understand we don't correctly handle this today, *but*
> I see this change as a step in the wrong direction.
>
> The problem here is that ram is split over two areas. So if
> machine->ram_size is larger then 0x40000000 it is going to overflow
> MICROCHIP_PFSOC_DRAM_LO and jump to MICROCHIP_PFSOC_DRAM_HI
> (0x1000000000).
>
> So we really want something like this
>
>          /* Compute the fdt load address in dram */
>          if (machine->ram_size > memmap[MICROCHIP_PFSOC_DRAM_LO].size) {
>              fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_HI].base,
>                                             machine->ram_size -
> memmap[MICROCHIP_PFSOC_DRAM_LO].size,
>                                             machine->fdt);
>          } else {
>              fdt_load_addr = riscv_load_fdt(memmap[MICROCHIP_PFSOC_DRAM_LO].base,
>                                             machine->ram_size,
>                                             machine->fdt);
>          }

Checking the code from microchip-icicle-kit I see that the minimal ram_size is
1.5Gb. In fact the machine would not allow anything less:

$ ./qemu-system-riscv64 -M microchip-icicle-kit -m 512M
qemu-system-riscv64: Invalid RAM size, should be bigger than 1.5 GiB

The reasoning is in its machine_class_init():

====
      * Map 513 MiB high memory, the mimimum required high memory size, because
      * HSS will do memory test against the high memory address range regardless
      * of physical memory installed.
====

This also means that this machine is putting the FDT in the wrong spot every time, since
1.5Gb is going to hit in the gap between the low and hi memory every time and the start
of the hi area is 64Gb away.


I believe this is yet another reason to create a specific helper to deal
with the FDT of this machine. I'll re-send with this change.


Daniel

>
> to handle overflowing MICROCHIP_PFSOC_DRAM_LO. While this patch is
> going in the wrong direction and making that more difficult
>
> Alistair
>
>
>
>>           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 626d4dc2f3..ebfddf161d 100644
>> --- a/hw/riscv/sifive_u.c
>> +++ b/hw/riscv/sifive_u.c
>> @@ -616,8 +616,8 @@ static void sifive_u_machine_init(MachineState *machine)
>>           kernel_entry = 0;
>>       }
>>
>> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base,
>> -                                           machine->ram_size, machine->fdt);
>> +    fdt_load_addr = riscv_compute_fdt_addr(machine,
>> +                                           memmap[SIFIVE_U_DEV_DRAM].base);
>>       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 88b9fdfc36..afd581436b 100644
>> --- a/hw/riscv/spike.c
>> +++ b/hw/riscv/spike.c
>> @@ -324,8 +324,8 @@ static void spike_board_init(MachineState *machine)
>>           kernel_entry = 0;
>>       }
>>
>> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base,
>> -                                           machine->ram_size, machine->fdt);
>> +    fdt_load_addr = riscv_compute_fdt_addr(machine,
>> +                                           memmap[SPIKE_DRAM].base);
>>       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 839dfaa125..cbba0b8930 100644
>> --- a/hw/riscv/virt.c
>> +++ b/hw/riscv/virt.c
>> @@ -1307,8 +1307,7 @@ static void virt_machine_done(Notifier *notifier, void *data)
>>           start_addr = virt_memmap[VIRT_FLASH].base;
>>       }
>>
>> -    fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base,
>> -                                           machine->ram_size, machine->fdt);
>> +    fdt_load_addr = riscv_compute_fdt_addr(machine, memmap[VIRT_DRAM].base);
>>       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 9aea7b9c46..f933de88fb 100644
>> --- a/include/hw/riscv/boot.h
>> +++ b/include/hw/riscv/boot.h
>> @@ -47,7 +47,7 @@ target_ulong riscv_load_kernel(MachineState *machine,
>>                                  target_ulong firmware_end_addr,
>>                                  symbol_fn_t sym_cb);
>>   void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry);
>> -uint32_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, void *fdt);
>> +uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_start);
>>   void riscv_load_fdt(uint32_t fdt_addr, void *fdt);
>>   void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts,
>>                                  hwaddr saddr,
>> --
>> 2.39.0
>>
>>
diff mbox series

Patch

diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index b213a32157..508da3f5c7 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -255,11 +255,11 @@  void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry)
  *
  * The FDT is fdt_packed() during the calculation.
  */
-uint32_t riscv_compute_fdt_addr(hwaddr dram_base, uint64_t mem_size,
-                                void *fdt)
+uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_base)
 {
+    void *fdt = machine->fdt;
     uint64_t temp;
-    hwaddr dram_end = dram_base + mem_size;
+    hwaddr dram_end = dram_base + machine->ram_size;
     int ret = fdt_pack(fdt);
     int fdtsize;
 
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index dcdbc2cac3..a53e48e996 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -641,8 +641,8 @@  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,
-                                              machine->ram_size, machine->fdt);
+        fdt_load_addr = riscv_compute_fdt_addr(machine,
+                                               memmap[MICROCHIP_PFSOC_DRAM_LO].base);
         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 626d4dc2f3..ebfddf161d 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -616,8 +616,8 @@  static void sifive_u_machine_init(MachineState *machine)
         kernel_entry = 0;
     }
 
-    fdt_load_addr = riscv_compute_fdt_addr(memmap[SIFIVE_U_DEV_DRAM].base,
-                                           machine->ram_size, machine->fdt);
+    fdt_load_addr = riscv_compute_fdt_addr(machine,
+                                           memmap[SIFIVE_U_DEV_DRAM].base);
     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 88b9fdfc36..afd581436b 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -324,8 +324,8 @@  static void spike_board_init(MachineState *machine)
         kernel_entry = 0;
     }
 
-    fdt_load_addr = riscv_compute_fdt_addr(memmap[SPIKE_DRAM].base,
-                                           machine->ram_size, machine->fdt);
+    fdt_load_addr = riscv_compute_fdt_addr(machine,
+                                           memmap[SPIKE_DRAM].base);
     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 839dfaa125..cbba0b8930 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1307,8 +1307,7 @@  static void virt_machine_done(Notifier *notifier, void *data)
         start_addr = virt_memmap[VIRT_FLASH].base;
     }
 
-    fdt_load_addr = riscv_compute_fdt_addr(memmap[VIRT_DRAM].base,
-                                           machine->ram_size, machine->fdt);
+    fdt_load_addr = riscv_compute_fdt_addr(machine, memmap[VIRT_DRAM].base);
     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 9aea7b9c46..f933de88fb 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -47,7 +47,7 @@  target_ulong riscv_load_kernel(MachineState *machine,
                                target_ulong firmware_end_addr,
                                symbol_fn_t sym_cb);
 void riscv_load_initrd(MachineState *machine, uint64_t kernel_entry);
-uint32_t riscv_compute_fdt_addr(hwaddr dram_start, uint64_t dram_size, void *fdt);
+uint32_t riscv_compute_fdt_addr(MachineState *machine, hwaddr dram_start);
 void riscv_load_fdt(uint32_t fdt_addr, void *fdt);
 void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState *harts,
                                hwaddr saddr,