[PATCH 0/5] single-binary: Prepare hw/riscv for single compilation

[PATCH 0/5] single-binary: Prepare hw/riscv for single compilation

[Anton Johansson via]

Hello,

this is a small prerequisite patchset that removes target_[u]?long and
replaces a few target specific macros with target-info variants.

Next up is using the target-info style QOM filtering of boards and then
moving translation units to common code.  My current patches for this
are based on Philippes single-binary-hw-arm-rfc-v5 branch, is there some
other branch I should target here?

---
Anton Johansson (5):
      hw/riscv: Use generic hwaddr for firmware addressses
      hw/riscv: Replace target_ulong uses
      hw/riscv: Widen OpenSBI dynamic info struct
      target-info: Introduce runtime TARGET_PHYS_ADDR_SPACE_BITS
      hw/riscv: Use runtime target_phys_addr_space_bits()

 include/hw/riscv/boot.h         | 20 +++++++++----------
 include/hw/riscv/boot_opensbi.h | 14 ++++++-------
 include/qemu/target-info-impl.h |  2 ++
 include/qemu/target-info.h      |  8 ++++++++
 hw/riscv/boot.c                 | 44 +++++++++++++++++++++--------------------
 hw/riscv/microchip_pfsoc.c      |  2 +-
 hw/riscv/riscv-iommu.c          |  7 ++++---
 hw/riscv/riscv_hart.c           |  2 +-
 hw/riscv/sifive_u.c             |  2 +-
 hw/riscv/spike.c                |  4 ++--
 hw/riscv/virt.c                 |  2 +-
 target-info.c                   |  5 +++++
 12 files changed, 65 insertions(+), 47 deletions(-)

[PATCH 1/5] hw/riscv: Use generic hwaddr for firmware addressses

[Anton Johansson via]

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 include/hw/riscv/boot.h    | 20 ++++++++++----------
 hw/riscv/boot.c            | 22 +++++++++++-----------
 hw/riscv/microchip_pfsoc.c |  2 +-
 hw/riscv/sifive_u.c        |  2 +-
 hw/riscv/spike.c           |  4 ++--
 hw/riscv/virt.c            |  2 +-
 6 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index 7d59b2e6c6..d835594baa 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -43,21 +43,21 @@ bool riscv_is_32bit(RISCVHartArrayState *harts);
 char *riscv_plic_hart_config_string(int hart_count);
 
 void riscv_boot_info_init(RISCVBootInfo *info, RISCVHartArrayState *harts);
-target_ulong riscv_calc_kernel_start_addr(RISCVBootInfo *info,
-                                          target_ulong firmware_end_addr);
-target_ulong riscv_find_and_load_firmware(MachineState *machine,
-                                          const char *default_machine_firmware,
-                                          hwaddr *firmware_load_addr,
-                                          symbol_fn_t sym_cb);
+hwaddr riscv_calc_kernel_start_addr(RISCVBootInfo *info,
+                                    hwaddr firmware_end_addr);
+hwaddr riscv_find_and_load_firmware(MachineState *machine,
+                                    const char *default_machine_firmware,
+                                    hwaddr *firmware_load_addr,
+                                    symbol_fn_t sym_cb);
 const char *riscv_default_firmware_name(RISCVHartArrayState *harts);
 char *riscv_find_firmware(const char *firmware_filename,
                           const char *default_machine_firmware);
-target_ulong riscv_load_firmware(const char *firmware_filename,
-                                 hwaddr *firmware_load_addr,
-                                 symbol_fn_t sym_cb);
+hwaddr riscv_load_firmware(const char *firmware_filename,
+                           hwaddr *firmware_load_addr,
+                           symbol_fn_t sym_cb);
 void riscv_load_kernel(MachineState *machine,
                        RISCVBootInfo *info,
-                       target_ulong kernel_start_addr,
+                       hwaddr kernel_start_addr,
                        bool load_initrd,
                        symbol_fn_t sym_cb);
 uint64_t riscv_compute_fdt_addr(hwaddr dram_base, hwaddr dram_size,
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 828a867be3..4eadcff26c 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -74,8 +74,8 @@ void riscv_boot_info_init(RISCVBootInfo *info, RISCVHartArrayState *harts)
     info->is_32bit = riscv_is_32bit(harts);
 }
 
-target_ulong riscv_calc_kernel_start_addr(RISCVBootInfo *info,
-                                          target_ulong firmware_end_addr) {
+hwaddr riscv_calc_kernel_start_addr(RISCVBootInfo *info,
+                                    hwaddr firmware_end_addr) {
     if (info->is_32bit) {
         return QEMU_ALIGN_UP(firmware_end_addr, 4 * MiB);
     } else {
@@ -133,13 +133,13 @@ char *riscv_find_firmware(const char *firmware_filename,
     return filename;
 }
 
-target_ulong riscv_find_and_load_firmware(MachineState *machine,
-                                          const char *default_machine_firmware,
-                                          hwaddr *firmware_load_addr,
-                                          symbol_fn_t sym_cb)
+hwaddr riscv_find_and_load_firmware(MachineState *machine,
+                                    const char *default_machine_firmware,
+                                    hwaddr *firmware_load_addr,
+                                    symbol_fn_t sym_cb)
 {
     char *firmware_filename;
-    target_ulong firmware_end_addr = *firmware_load_addr;
+    hwaddr firmware_end_addr = *firmware_load_addr;
 
     firmware_filename = riscv_find_firmware(machine->firmware,
                                             default_machine_firmware);
@@ -154,9 +154,9 @@ target_ulong riscv_find_and_load_firmware(MachineState *machine,
     return firmware_end_addr;
 }
 
-target_ulong riscv_load_firmware(const char *firmware_filename,
-                                 hwaddr *firmware_load_addr,
-                                 symbol_fn_t sym_cb)
+hwaddr riscv_load_firmware(const char *firmware_filename,
+                           hwaddr *firmware_load_addr,
+                           symbol_fn_t sym_cb)
 {
     uint64_t firmware_entry, firmware_end;
     ssize_t firmware_size;
@@ -227,7 +227,7 @@ static void riscv_load_initrd(MachineState *machine, RISCVBootInfo *info)
 
 void riscv_load_kernel(MachineState *machine,
                        RISCVBootInfo *info,
-                       target_ulong kernel_start_addr,
+                       hwaddr kernel_start_addr,
                        bool load_initrd,
                        symbol_fn_t sym_cb)
 {
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 2e74783fce..e5a0196a00 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -515,7 +515,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
     uint64_t mem_low_size, mem_high_size;
     hwaddr firmware_load_addr;
     const char *firmware_name;
-    target_ulong firmware_end_addr, kernel_start_addr;
+    hwaddr firmware_end_addr, kernel_start_addr;
     uint64_t kernel_entry;
     uint64_t fdt_load_addr;
     DriveInfo *dinfo = drive_get(IF_SD, 0, 0);
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index d69f942cfb..390f9b8d9a 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -515,7 +515,7 @@ static void sifive_u_machine_init(MachineState *machine)
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *flash0 = g_new(MemoryRegion, 1);
     hwaddr start_addr = memmap[SIFIVE_U_DEV_DRAM].base;
-    target_ulong firmware_end_addr, kernel_start_addr;
+    hwaddr firmware_end_addr, kernel_start_addr;
     const char *firmware_name;
     uint32_t start_addr_hi32 = 0x00000000;
     uint32_t fdt_load_addr_hi32 = 0x00000000;
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 641aae8c01..b0bab3fe00 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -197,9 +197,9 @@ static void spike_board_init(MachineState *machine)
     SpikeState *s = SPIKE_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
-    target_ulong firmware_end_addr = memmap[SPIKE_DRAM].base;
+    hwaddr firmware_end_addr = memmap[SPIKE_DRAM].base;
     hwaddr firmware_load_addr = memmap[SPIKE_DRAM].base;
-    target_ulong kernel_start_addr;
+    hwaddr kernel_start_addr;
     char *firmware_name;
     uint64_t fdt_load_addr;
     uint64_t kernel_entry;
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 47e573f85a..17909206c7 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1434,7 +1434,7 @@ static void virt_machine_done(Notifier *notifier, void *data)
                                      machine_done);
     MachineState *machine = MACHINE(s);
     hwaddr start_addr = s->memmap[VIRT_DRAM].base;
-    target_ulong firmware_end_addr, kernel_start_addr;
+    hwaddr firmware_end_addr, kernel_start_addr;
     const char *firmware_name = riscv_default_firmware_name(&s->soc[0]);
     uint64_t fdt_load_addr;
     uint64_t kernel_entry = 0;

-- 
2.51.0

[PATCH 2/5] hw/riscv: Replace target_ulong uses

[Anton Johansson via]

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 hw/riscv/riscv-iommu.c | 5 +++--
 hw/riscv/riscv_hart.c  | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index b33c7fe325..450285a850 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -26,6 +26,7 @@
 #include "migration/vmstate.h"
 #include "qapi/error.h"
 #include "qemu/timer.h"
+#include "qemu/target-info.h"
 
 #include "cpu_bits.h"
 #include "riscv-iommu.h"
@@ -391,9 +392,9 @@ static int riscv_iommu_spa_fetch(RISCVIOMMUState *s, RISCVIOMMUContext *ctx,
             const uint64_t va_mask = (1ULL << va_len) - 1;
 
             if (pass == S_STAGE && va_len > 32) {
-                target_ulong mask, masked_msbs;
+                uint64_t mask, masked_msbs;
 
-                mask = (1L << (TARGET_LONG_BITS - (va_len - 1))) - 1;
+                mask = (1L << (target_long_bits() - (va_len - 1))) - 1;
                 masked_msbs = (addr >> (va_len - 1)) & mask;
 
                 if (masked_msbs != 0 && masked_msbs != mask) {
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index c7e98a4308..65d2c92018 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -93,7 +93,7 @@ static bool csr_qtest_callback(CharBackend *chr, gchar **words)
         g_assert(rc == 0);
         csr_call(words[1], cpu, csr, &val);
 
-        qtest_sendf(chr, "OK 0 "TARGET_FMT_lx"\n", (target_ulong)val);
+        qtest_sendf(chr, "OK 0 %"PRIx64"\n", val);
 
         return true;
     }

-- 
2.51.0

[PATCH 3/5] hw/riscv: Widen OpenSBI dynamic info struct

[Anton Johansson via]

Since fw_dynamic_info is only used for non 32 bit targets, target_long
is int64_t anyway.  Rename struct to fw_dynamic_info64 and use int64_t.

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 include/hw/riscv/boot_opensbi.h | 14 +++++++-------
 hw/riscv/boot.c                 | 22 ++++++++++++----------
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/include/hw/riscv/boot_opensbi.h b/include/hw/riscv/boot_opensbi.h
index 18664a174b..ab9999be3f 100644
--- a/include/hw/riscv/boot_opensbi.h
+++ b/include/hw/riscv/boot_opensbi.h
@@ -29,17 +29,17 @@ enum sbi_scratch_options {
 };
 
 /** Representation dynamic info passed by previous booting stage */
-struct fw_dynamic_info {
+struct fw_dynamic_info64 {
     /** Info magic */
-    target_long magic;
+    int64_t magic;
     /** Info version */
-    target_long version;
+    int64_t version;
     /** Next booting stage address */
-    target_long next_addr;
+    int64_t next_addr;
     /** Next booting stage mode */
-    target_long next_mode;
+    int64_t next_mode;
     /** Options for OpenSBI library */
-    target_long options;
+    int64_t options;
     /**
      * Preferred boot HART id
      *
@@ -55,7 +55,7 @@ struct fw_dynamic_info {
      * stage can set it to -1UL which will force the FW_DYNAMIC firmware
      * to use the relocation lottery mechanism.
      */
-    target_long boot_hart;
+    int64_t boot_hart;
 };
 
 /** Representation dynamic info passed by previous booting stage */
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 4eadcff26c..64608e58c7 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -387,7 +387,8 @@ void riscv_rom_copy_firmware_info(MachineState *machine,
                                   uint64_t kernel_entry)
 {
     struct fw_dynamic_info32 dinfo32;
-    struct fw_dynamic_info dinfo;
+    struct fw_dynamic_info64 dinfo64;
+    void *dinfo_ptr = NULL;
     size_t dinfo_len;
 
     if (riscv_is_32bit(harts)) {
@@ -397,15 +398,17 @@ void riscv_rom_copy_firmware_info(MachineState *machine,
         dinfo32.next_addr = cpu_to_le32(kernel_entry);
         dinfo32.options = 0;
         dinfo32.boot_hart = 0;
+        dinfo_ptr = &dinfo32;
         dinfo_len = sizeof(dinfo32);
     } else {
-        dinfo.magic = cpu_to_le64(FW_DYNAMIC_INFO_MAGIC_VALUE);
-        dinfo.version = cpu_to_le64(FW_DYNAMIC_INFO_VERSION);
-        dinfo.next_mode = cpu_to_le64(FW_DYNAMIC_INFO_NEXT_MODE_S);
-        dinfo.next_addr = cpu_to_le64(kernel_entry);
-        dinfo.options = 0;
-        dinfo.boot_hart = 0;
-        dinfo_len = sizeof(dinfo);
+        dinfo64.magic = cpu_to_le64(FW_DYNAMIC_INFO_MAGIC_VALUE);
+        dinfo64.version = cpu_to_le64(FW_DYNAMIC_INFO_VERSION);
+        dinfo64.next_mode = cpu_to_le64(FW_DYNAMIC_INFO_NEXT_MODE_S);
+        dinfo64.next_addr = cpu_to_le64(kernel_entry);
+        dinfo64.options = 0;
+        dinfo64.boot_hart = 0;
+        dinfo_ptr = &dinfo64;
+        dinfo_len = sizeof(dinfo64);
     }
 
     /**
@@ -419,8 +422,7 @@ void riscv_rom_copy_firmware_info(MachineState *machine,
     }
 
     rom_add_blob_fixed_as("mrom.finfo",
-                           riscv_is_32bit(harts) ?
-                           (void *)&dinfo32 : (void *)&dinfo,
+                           dinfo_ptr,
                            dinfo_len,
                            rom_base + reset_vec_size,
                            &address_space_memory);

-- 
2.51.0

[PATCH 4/5] target-info: Introduce runtime TARGET_PHYS_ADDR_SPACE_BITS

[Anton Johansson via]

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 include/qemu/target-info-impl.h | 2 ++
 include/qemu/target-info.h      | 8 ++++++++
 target-info.c                   | 5 +++++
 3 files changed, 15 insertions(+)

diff --git a/include/qemu/target-info-impl.h b/include/qemu/target-info-impl.h
index 17887f64e2..80d1613128 100644
--- a/include/qemu/target-info-impl.h
+++ b/include/qemu/target-info-impl.h
@@ -18,6 +18,8 @@ typedef struct TargetInfo {
     SysEmuTarget target_arch;
     /* runtime equivalent of TARGET_LONG_BITS definition */
     unsigned long_bits;
+    /* runtime equivalent of TARGET_PHYS_ADDR_SPACE_BITS definition */
+    unsigned phys_addr_space_bits;
     /* runtime equivalent of CPU_RESOLVING_TYPE definition */
     const char *cpu_type;
     /* QOM typename machines for this binary must implement */
diff --git a/include/qemu/target-info.h b/include/qemu/target-info.h
index abcf25db6f..8474c43404 100644
--- a/include/qemu/target-info.h
+++ b/include/qemu/target-info.h
@@ -23,6 +23,14 @@ const char *target_name(void);
  */
 unsigned target_long_bits(void);
 
+/**
+ * target_phys_addr_space_bits:
+ *
+ * Returns: number of bits needed to represent the targets physical
+ *          address space.
+ */
+unsigned target_phys_addr_space_bits(void);
+
 /**
  * target_machine_typename:
  *
diff --git a/target-info.c b/target-info.c
index 3110ab32f7..3d696ae0b3 100644
--- a/target-info.c
+++ b/target-info.c
@@ -22,6 +22,11 @@ unsigned target_long_bits(void)
     return target_info()->long_bits;
 }
 
+unsigned target_phys_addr_space_bits(void)
+{
+    return target_info()->phys_addr_space_bits;
+}
+
 SysEmuTarget target_arch(void)
 {
     SysEmuTarget arch = target_info()->target_arch;

-- 
2.51.0

[PATCH 5/5] hw/riscv: Use runtime target_phys_addr_space_bits()

[Anton Johansson via]

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 hw/riscv/riscv-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index 450285a850..9ac37efc70 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -2449,7 +2449,7 @@ static void riscv_iommu_instance_init(Object *obj)
 
     /* Report QEMU target physical address space limits */
     s->cap = set_field(s->cap, RISCV_IOMMU_CAP_PAS,
-                       TARGET_PHYS_ADDR_SPACE_BITS);
+                       target_phys_addr_space_bits());
 
     /* TODO: method to report supported PID bits */
     s->pid_bits = 8; /* restricted to size of MemTxAttrs.pid */

-- 
2.51.0

Re: [PATCH 1/5] hw/riscv: Use generic hwaddr for firmware addressses

[Philippe Mathieu-Daudé]

On 15/10/25 15:27, Anton Johansson wrote:
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   include/hw/riscv/boot.h    | 20 ++++++++++----------
>   hw/riscv/boot.c            | 22 +++++++++++-----------
>   hw/riscv/microchip_pfsoc.c |  2 +-
>   hw/riscv/sifive_u.c        |  2 +-
>   hw/riscv/spike.c           |  4 ++--
>   hw/riscv/virt.c            |  2 +-
>   6 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index 7d59b2e6c6..d835594baa 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -43,21 +43,21 @@ bool riscv_is_32bit(RISCVHartArrayState *harts);
>   char *riscv_plic_hart_config_string(int hart_count);
>   
>   void riscv_boot_info_init(RISCVBootInfo *info, RISCVHartArrayState *harts);
> -target_ulong riscv_calc_kernel_start_addr(RISCVBootInfo *info,
> -                                          target_ulong firmware_end_addr);
> -target_ulong riscv_find_and_load_firmware(MachineState *machine,
> -                                          const char *default_machine_firmware,
> -                                          hwaddr *firmware_load_addr,
> -                                          symbol_fn_t sym_cb);
> +hwaddr riscv_calc_kernel_start_addr(RISCVBootInfo *info,
> +                                    hwaddr firmware_end_addr);
> +hwaddr riscv_find_and_load_firmware(MachineState *machine,
> +                                    const char *default_machine_firmware,
> +                                    hwaddr *firmware_load_addr,
> +                                    symbol_fn_t sym_cb);
>   const char *riscv_default_firmware_name(RISCVHartArrayState *harts);
>   char *riscv_find_firmware(const char *firmware_filename,
>                             const char *default_machine_firmware);
> -target_ulong riscv_load_firmware(const char *firmware_filename,
> -                                 hwaddr *firmware_load_addr,
> -                                 symbol_fn_t sym_cb);
> +hwaddr riscv_load_firmware(const char *firmware_filename,
> +                           hwaddr *firmware_load_addr,
> +                           symbol_fn_t sym_cb);
>   void riscv_load_kernel(MachineState *machine,
>                          RISCVBootInfo *info,
> -                       target_ulong kernel_start_addr,
> +                       hwaddr kernel_start_addr,

vaddr?

>                          bool load_initrd,
>                          symbol_fn_t sym_cb);

Re: [PATCH 2/5] hw/riscv: Replace target_ulong uses

[Philippe Mathieu-Daudé]

On 15/10/25 15:27, Anton Johansson wrote:
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   hw/riscv/riscv-iommu.c | 5 +++--
>   hw/riscv/riscv_hart.c  | 2 +-
>   2 files changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 4/5] target-info: Introduce runtime TARGET_PHYS_ADDR_SPACE_BITS

[Philippe Mathieu-Daudé]

On 15/10/25 15:27, Anton Johansson wrote:
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   include/qemu/target-info-impl.h | 2 ++
>   include/qemu/target-info.h      | 8 ++++++++
>   target-info.c                   | 5 +++++
>   3 files changed, 15 insertions(+)
> 
> diff --git a/include/qemu/target-info-impl.h b/include/qemu/target-info-impl.h
> index 17887f64e2..80d1613128 100644
> --- a/include/qemu/target-info-impl.h
> +++ b/include/qemu/target-info-impl.h
> @@ -18,6 +18,8 @@ typedef struct TargetInfo {
>       SysEmuTarget target_arch;
>       /* runtime equivalent of TARGET_LONG_BITS definition */
>       unsigned long_bits;
> +    /* runtime equivalent of TARGET_PHYS_ADDR_SPACE_BITS definition */
> +    unsigned phys_addr_space_bits;
>       /* runtime equivalent of CPU_RESOLVING_TYPE definition */
>       const char *cpu_type;
>       /* QOM typename machines for this binary must implement */


> diff --git a/target-info.c b/target-info.c
> index 3110ab32f7..3d696ae0b3 100644
> --- a/target-info.c
> +++ b/target-info.c
> @@ -22,6 +22,11 @@ unsigned target_long_bits(void)
>       return target_info()->long_bits;
>   }
>   
> +unsigned target_phys_addr_space_bits(void)
> +{
> +    return target_info()->phys_addr_space_bits;
> +}

Missing field initialization in target_info_stub[].

BTW this definition seems unused by common code since commit
2e8fe327eb6 ("accel/tcg: Simplify L1_MAP_ADDR_SPACE_BITS").

Do we still need to expose it to common components?

Re: [PATCH 2/5] hw/riscv: Replace target_ulong uses

[Richard Henderson]

On 10/15/25 06:27, Anton Johansson wrote:
> -                mask = (1L << (TARGET_LONG_BITS - (va_len - 1))) - 1;
> +                mask = (1L << (target_long_bits() - (va_len - 1))) - 1;

While you're at it, long, via 1L, is always wrong.

Here, use MAKE_64BIT_MASK.


r~

Re: [PATCH 2/5] hw/riscv: Replace target_ulong uses

[Philippe Mathieu-Daudé]

On 15/10/25 18:31, Richard Henderson wrote:
> On 10/15/25 06:27, Anton Johansson wrote:
>> -                mask = (1L << (TARGET_LONG_BITS - (va_len - 1))) - 1;
>> +                mask = (1L << (target_long_bits() - (va_len - 1))) - 1;
> 
> While you're at it, long, via 1L, is always wrong.
> 
> Here, use MAKE_64BIT_MASK.

Oh, right.

Re: [PATCH 3/5] hw/riscv: Widen OpenSBI dynamic info struct

[Alistair Francis]

On Wed, Oct 15, 2025 at 11:29 PM Anton Johansson via
<qemu-devel@nongnu.org> wrote:
>
> Since fw_dynamic_info is only used for non 32 bit targets, target_long
> is int64_t anyway.  Rename struct to fw_dynamic_info64 and use int64_t.
>
> Signed-off-by: Anton Johansson <anjo@rev.ng>

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

Alistair

> ---
>  include/hw/riscv/boot_opensbi.h | 14 +++++++-------
>  hw/riscv/boot.c                 | 22 ++++++++++++----------
>  2 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/riscv/boot_opensbi.h b/include/hw/riscv/boot_opensbi.h
> index 18664a174b..ab9999be3f 100644
> --- a/include/hw/riscv/boot_opensbi.h
> +++ b/include/hw/riscv/boot_opensbi.h
> @@ -29,17 +29,17 @@ enum sbi_scratch_options {
>  };
>
>  /** Representation dynamic info passed by previous booting stage */
> -struct fw_dynamic_info {
> +struct fw_dynamic_info64 {
>      /** Info magic */
> -    target_long magic;
> +    int64_t magic;
>      /** Info version */
> -    target_long version;
> +    int64_t version;
>      /** Next booting stage address */
> -    target_long next_addr;
> +    int64_t next_addr;
>      /** Next booting stage mode */
> -    target_long next_mode;
> +    int64_t next_mode;
>      /** Options for OpenSBI library */
> -    target_long options;
> +    int64_t options;
>      /**
>       * Preferred boot HART id
>       *
> @@ -55,7 +55,7 @@ struct fw_dynamic_info {
>       * stage can set it to -1UL which will force the FW_DYNAMIC firmware
>       * to use the relocation lottery mechanism.
>       */
> -    target_long boot_hart;
> +    int64_t boot_hart;
>  };
>
>  /** Representation dynamic info passed by previous booting stage */
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 4eadcff26c..64608e58c7 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -387,7 +387,8 @@ void riscv_rom_copy_firmware_info(MachineState *machine,
>                                    uint64_t kernel_entry)
>  {
>      struct fw_dynamic_info32 dinfo32;
> -    struct fw_dynamic_info dinfo;
> +    struct fw_dynamic_info64 dinfo64;
> +    void *dinfo_ptr = NULL;
>      size_t dinfo_len;
>
>      if (riscv_is_32bit(harts)) {
> @@ -397,15 +398,17 @@ void riscv_rom_copy_firmware_info(MachineState *machine,
>          dinfo32.next_addr = cpu_to_le32(kernel_entry);
>          dinfo32.options = 0;
>          dinfo32.boot_hart = 0;
> +        dinfo_ptr = &dinfo32;
>          dinfo_len = sizeof(dinfo32);
>      } else {
> -        dinfo.magic = cpu_to_le64(FW_DYNAMIC_INFO_MAGIC_VALUE);
> -        dinfo.version = cpu_to_le64(FW_DYNAMIC_INFO_VERSION);
> -        dinfo.next_mode = cpu_to_le64(FW_DYNAMIC_INFO_NEXT_MODE_S);
> -        dinfo.next_addr = cpu_to_le64(kernel_entry);
> -        dinfo.options = 0;
> -        dinfo.boot_hart = 0;
> -        dinfo_len = sizeof(dinfo);
> +        dinfo64.magic = cpu_to_le64(FW_DYNAMIC_INFO_MAGIC_VALUE);
> +        dinfo64.version = cpu_to_le64(FW_DYNAMIC_INFO_VERSION);
> +        dinfo64.next_mode = cpu_to_le64(FW_DYNAMIC_INFO_NEXT_MODE_S);
> +        dinfo64.next_addr = cpu_to_le64(kernel_entry);
> +        dinfo64.options = 0;
> +        dinfo64.boot_hart = 0;
> +        dinfo_ptr = &dinfo64;
> +        dinfo_len = sizeof(dinfo64);
>      }
>
>      /**
> @@ -419,8 +422,7 @@ void riscv_rom_copy_firmware_info(MachineState *machine,
>      }
>
>      rom_add_blob_fixed_as("mrom.finfo",
> -                           riscv_is_32bit(harts) ?
> -                           (void *)&dinfo32 : (void *)&dinfo,
> +                           dinfo_ptr,
>                             dinfo_len,
>                             rom_base + reset_vec_size,
>                             &address_space_memory);
>
> --
> 2.51.0
>
>

Re: [PATCH 5/5] hw/riscv: Use runtime target_phys_addr_space_bits()

[Alistair Francis]

On Wed, Oct 15, 2025 at 11:28 PM Anton Johansson via
<qemu-devel@nongnu.org> wrote:
>
> Signed-off-by: Anton Johansson <anjo@rev.ng>

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

Alistair

> ---
>  hw/riscv/riscv-iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index 450285a850..9ac37efc70 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -2449,7 +2449,7 @@ static void riscv_iommu_instance_init(Object *obj)
>
>      /* Report QEMU target physical address space limits */
>      s->cap = set_field(s->cap, RISCV_IOMMU_CAP_PAS,
> -                       TARGET_PHYS_ADDR_SPACE_BITS);
> +                       target_phys_addr_space_bits());
>
>      /* TODO: method to report supported PID bits */
>      s->pid_bits = 8; /* restricted to size of MemTxAttrs.pid */
>
> --
> 2.51.0
>
>

Re: [PATCH 4/5] target-info: Introduce runtime TARGET_PHYS_ADDR_SPACE_BITS

[Anton Johansson via]

On 15/10/25, Philippe Mathieu-Daudé wrote:
> On 15/10/25 15:27, Anton Johansson wrote:
> > Signed-off-by: Anton Johansson <anjo@rev.ng>
> > ---
> >   include/qemu/target-info-impl.h | 2 ++
> >   include/qemu/target-info.h      | 8 ++++++++
> >   target-info.c                   | 5 +++++
> >   3 files changed, 15 insertions(+)
> > 
> > diff --git a/include/qemu/target-info-impl.h b/include/qemu/target-info-impl.h
> > index 17887f64e2..80d1613128 100644
> > --- a/include/qemu/target-info-impl.h
> > +++ b/include/qemu/target-info-impl.h
> > @@ -18,6 +18,8 @@ typedef struct TargetInfo {
> >       SysEmuTarget target_arch;
> >       /* runtime equivalent of TARGET_LONG_BITS definition */
> >       unsigned long_bits;
> > +    /* runtime equivalent of TARGET_PHYS_ADDR_SPACE_BITS definition */
> > +    unsigned phys_addr_space_bits;
> >       /* runtime equivalent of CPU_RESOLVING_TYPE definition */
> >       const char *cpu_type;
> >       /* QOM typename machines for this binary must implement */
> 
> 
> > diff --git a/target-info.c b/target-info.c
> > index 3110ab32f7..3d696ae0b3 100644
> > --- a/target-info.c
> > +++ b/target-info.c
> > @@ -22,6 +22,11 @@ unsigned target_long_bits(void)
> >       return target_info()->long_bits;
> >   }
> > +unsigned target_phys_addr_space_bits(void)
> > +{
> > +    return target_info()->phys_addr_space_bits;
> > +}
> 
> Missing field initialization in target_info_stub[].

Ugh right, thanks!

> 
> BTW this definition seems unused by common code since commit
> 2e8fe327eb6 ("accel/tcg: Simplify L1_MAP_ADDR_SPACE_BITS").
> 
> Do we still need to expose it to common components?
> 

Hmm you're right looking at git grep -C1 TARGET_PHYS_ADDR_SPACE_BITS
(output below excluding the hw/riscv change in the following patch),
there are really aren't that many uses left and none in common code.

We still got to move it to a runtime value somewhere though, what
would be a more suitable location?  Maybe as a field in CPUArchState or
some parent QOM machine as only i386, hppa, loongarch, riscv, alpha
actually use the definition.

  hw/loongarch/boot.c:    return addr & MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS);
  --
  hw/loongarch/boot.c-    *kernel_entry = extract64(le64_to_cpu(hdr->kernel_entry),
  hw/loongarch/boot.c:                              0, TARGET_PHYS_ADDR_SPACE_BITS);
  hw/loongarch/boot.c-    *kernel_low = extract64(le64_to_cpu(hdr->load_offset),
  hw/loongarch/boot.c:                            0, TARGET_PHYS_ADDR_SPACE_BITS);
  --
  linux-user/alpha/target_proc.h-            "L3 cache\t\t: n/a\n",
  linux-user/alpha/target_proc.h:            model, TARGET_PAGE_SIZE, TARGET_PHYS_ADDR_SPACE_BITS,
  linux-user/alpha/target_proc.h-            max_cpus, num_cpus, cpu_mask);
  --
  target/hppa/mem_helper.c:    QEMU_BUILD_BUG_ON(TARGET_PHYS_ADDR_SPACE_BITS > 54);
  target/hppa/mem_helper.c:    return sextract64(addr, 0, TARGET_PHYS_ADDR_SPACE_BITS);
  --
  target/hppa/mem_helper.c:        addr |= -1ull << (TARGET_PHYS_ADDR_SPACE_BITS - 4);
  --
  target/hppa/mem_helper.c-    /* Ignore the bits beyond physical address space. */
  target/hppa/mem_helper.c:    ent->pa = sextract64(ent->pa, 0, TARGET_PHYS_ADDR_SPACE_BITS);
  --
  target/i386/cpu.c-        if (cpu->phys_bits &&
  target/i386/cpu.c:            (cpu->phys_bits > TARGET_PHYS_ADDR_SPACE_BITS ||
  target/i386/cpu.c-            cpu->phys_bits < 32)) {
  --
  target/i386/cpu.c-                             " (but is %u)",
  target/i386/cpu.c:                             TARGET_PHYS_ADDR_SPACE_BITS, cpu->phys_bits);
  --
  target/i386/kvm/kvm.c:        QEMU_BUILD_BUG_ON(TARGET_PHYS_ADDR_SPACE_BITS > 52);
  target/i386/kvm/kvm.c:        assert(cpu->phys_bits <= TARGET_PHYS_ADDR_SPACE_BITS);
  --
  target/i386/tcg/helper-tcg.h:QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS);
  --
  target/loongarch/internals.h:#define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)
  target/loongarch/internals.h-#define TARGET_VIRT_MASK MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS)

Re: [PATCH 4/5] target-info: Introduce runtime TARGET_PHYS_ADDR_SPACE_BITS

[Richard Henderson]

On 10/17/25 09:11, Anton Johansson wrote:
> Hmm you're right looking at git grep -C1 TARGET_PHYS_ADDR_SPACE_BITS
> (output below excluding the hw/riscv change in the following patch),
> there are really aren't that many uses left and none in common code.
> 
> We still got to move it to a runtime value somewhere though, what
> would be a more suitable location?  Maybe as a field in CPUArchState or
> some parent QOM machine as only i386, hppa, loongarch, riscv, alpha
> actually use the definition.

A fair few of these are arguably wrong.


>    hw/loongarch/boot.c:    return addr & MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS);
>    --
>    hw/loongarch/boot.c-    *kernel_entry = extract64(le64_to_cpu(hdr->kernel_entry),
>    hw/loongarch/boot.c:                              0, TARGET_PHYS_ADDR_SPACE_BITS);
>    hw/loongarch/boot.c-    *kernel_low = extract64(le64_to_cpu(hdr->load_offset),
>    hw/loongarch/boot.c:                            0, TARGET_PHYS_ADDR_SPACE_BITS);

This is cpu_loongarch_virt_to_phys, and some repetitions.

This should probably use a loongarch-specific runtime function to find the address space 
range supported by the chosen cpu.  Or perhaps just a target-specific constant mask.


>    linux-user/alpha/target_proc.h-            "L3 cache\t\t: n/a\n",
>    linux-user/alpha/target_proc.h:            model, TARGET_PAGE_SIZE, TARGET_PHYS_ADDR_SPACE_BITS,
>    linux-user/alpha/target_proc.h-            max_cpus, num_cpus, cpu_mask);

This is the alpha-linux-user implementation of /proc/cpuinfo.

Ideally this should be a target-specific function; see

/* ??? EV4 has 34 phys addr bits, EV5 has 40, EV6 has 44.  */
#define TARGET_PHYS_ADDR_SPACE_BITS  44

It's certainly not generic, and it's also not really important.

>    --
>    target/hppa/mem_helper.c:    QEMU_BUILD_BUG_ON(TARGET_PHYS_ADDR_SPACE_BITS > 54);
>    target/hppa/mem_helper.c:    return sextract64(addr, 0, TARGET_PHYS_ADDR_SPACE_BITS);
>    --
>    target/hppa/mem_helper.c:        addr |= -1ull << (TARGET_PHYS_ADDR_SPACE_BITS - 4);
>    --
>    target/hppa/mem_helper.c-    /* Ignore the bits beyond physical address space. */
>    target/hppa/mem_helper.c:    ent->pa = sextract64(ent->pa, 0, TARGET_PHYS_ADDR_SPACE_BITS);

Similarly

/* ??? PA-8000 through 8600 have 40 bits; PA-8700 and 8900 have 44 bits. */
# define TARGET_PHYS_ADDR_SPACE_BITS  40

While we don't actually name concrete cpu models, bios advertises the (32-bit) HP B160L 
machine, which originally had a 7300LC, and the (64-bit) which had a 8700.

I can't find definitive documentation, but I suspect the 7300LC has only 32 physical 
address bits.  And according to our own comment we get the 8700 value wrong.

In either case, it's not exposed to generic code.

>    --
>    target/i386/cpu.c-        if (cpu->phys_bits &&
>    target/i386/cpu.c:            (cpu->phys_bits > TARGET_PHYS_ADDR_SPACE_BITS ||
>    target/i386/cpu.c-            cpu->phys_bits < 32)) {
>    --
>    target/i386/cpu.c-                             " (but is %u)",
>    target/i386/cpu.c:                             TARGET_PHYS_ADDR_SPACE_BITS, cpu->phys_bits);
>    --
>    target/i386/kvm/kvm.c:        QEMU_BUILD_BUG_ON(TARGET_PHYS_ADDR_SPACE_BITS > 52);
>    target/i386/kvm/kvm.c:        assert(cpu->phys_bits <= TARGET_PHYS_ADDR_SPACE_BITS);

All of these are simply making sure that cpu->phys_bits is "in range", which is now 
irrelevant because TARGET_PHYS_ADDR_SPACE_BITS itself is no longer in use.  They can all 
be removed.

>    --
>    target/i386/tcg/helper-tcg.h:QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > TARGET_PHYS_ADDR_SPACE_BITS);

Likewise.


>    target/loongarch/internals.h:#define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)

This is used by target/loongarch/tcg/tlb_helper.c.

I'm not sure what the implications are.
Should it be using a common function with the loongarch boot virt-to-phys?
Is it re-using TARGET_PHYS_ADDR_SPACE_BITS just because it was convienient?

In either case, it's not exposed to generic code.


r~

Re: [PATCH 4/5] target-info: Introduce runtime TARGET_PHYS_ADDR_SPACE_BITS

[Bibo Mao]

On 2025/10/18 上午2:47, Richard Henderson wrote:
> On 10/17/25 09:11, Anton Johansson wrote:
>> Hmm you're right looking at git grep -C1 TARGET_PHYS_ADDR_SPACE_BITS
>> (output below excluding the hw/riscv change in the following patch),
>> there are really aren't that many uses left and none in common code.
>>
>> We still got to move it to a runtime value somewhere though, what
>> would be a more suitable location?  Maybe as a field in CPUArchState or
>> some parent QOM machine as only i386, hppa, loongarch, riscv, alpha
>> actually use the definition.
> 
> A fair few of these are arguably wrong.
> 
> 
>>    hw/loongarch/boot.c:    return addr & MAKE_64BIT_MASK(0, 
>> TARGET_PHYS_ADDR_SPACE_BITS);
>>    --
>>    hw/loongarch/boot.c-    *kernel_entry = 
>> extract64(le64_to_cpu(hdr->kernel_entry),
>>    hw/loongarch/boot.c:                              0, 
>> TARGET_PHYS_ADDR_SPACE_BITS);
>>    hw/loongarch/boot.c-    *kernel_low = 
>> extract64(le64_to_cpu(hdr->load_offset),
>>    hw/loongarch/boot.c:                            0, 
>> TARGET_PHYS_ADDR_SPACE_BITS);
> 
> This is cpu_loongarch_virt_to_phys, and some repetitions.
> 
> This should probably use a loongarch-specific runtime function to find 
> the address space range supported by the chosen cpu.  Or perhaps just a 
> target-specific constant mask.
> 
> 
>>    linux-user/alpha/target_proc.h-            "L3 cache\t\t: n/a\n",
>>    linux-user/alpha/target_proc.h:            model, TARGET_PAGE_SIZE, 
>> TARGET_PHYS_ADDR_SPACE_BITS,
>>    linux-user/alpha/target_proc.h-            max_cpus, num_cpus, 
>> cpu_mask);
> 
> This is the alpha-linux-user implementation of /proc/cpuinfo.
> 
> Ideally this should be a target-specific function; see
> 
> /* ??? EV4 has 34 phys addr bits, EV5 has 40, EV6 has 44.  */
> #define TARGET_PHYS_ADDR_SPACE_BITS  44
> 
> It's certainly not generic, and it's also not really important.
> 
>>    --
>>    target/hppa/mem_helper.c:    
>> QEMU_BUILD_BUG_ON(TARGET_PHYS_ADDR_SPACE_BITS > 54);
>>    target/hppa/mem_helper.c:    return sextract64(addr, 0, 
>> TARGET_PHYS_ADDR_SPACE_BITS);
>>    --
>>    target/hppa/mem_helper.c:        addr |= -1ull << 
>> (TARGET_PHYS_ADDR_SPACE_BITS - 4);
>>    --
>>    target/hppa/mem_helper.c-    /* Ignore the bits beyond physical 
>> address space. */
>>    target/hppa/mem_helper.c:    ent->pa = sextract64(ent->pa, 0, 
>> TARGET_PHYS_ADDR_SPACE_BITS);
> 
> Similarly
> 
> /* ??? PA-8000 through 8600 have 40 bits; PA-8700 and 8900 have 44 bits. */
> # define TARGET_PHYS_ADDR_SPACE_BITS  40
> 
> While we don't actually name concrete cpu models, bios advertises the 
> (32-bit) HP B160L machine, which originally had a 7300LC, and the 
> (64-bit) which had a 8700.
> 
> I can't find definitive documentation, but I suspect the 7300LC has only 
> 32 physical address bits.  And according to our own comment we get the 
> 8700 value wrong.
> 
> In either case, it's not exposed to generic code.
> 
>>    --
>>    target/i386/cpu.c-        if (cpu->phys_bits &&
>>    target/i386/cpu.c:            (cpu->phys_bits > 
>> TARGET_PHYS_ADDR_SPACE_BITS ||
>>    target/i386/cpu.c-            cpu->phys_bits < 32)) {
>>    --
>>    target/i386/cpu.c-                             " (but is %u)",
>>    target/i386/cpu.c:                             
>> TARGET_PHYS_ADDR_SPACE_BITS, cpu->phys_bits);
>>    --
>>    target/i386/kvm/kvm.c:        
>> QEMU_BUILD_BUG_ON(TARGET_PHYS_ADDR_SPACE_BITS > 52);
>>    target/i386/kvm/kvm.c:        assert(cpu->phys_bits <= 
>> TARGET_PHYS_ADDR_SPACE_BITS);
> 
> All of these are simply making sure that cpu->phys_bits is "in range", 
> which is now irrelevant because TARGET_PHYS_ADDR_SPACE_BITS itself is no 
> longer in use.  They can all be removed.
> 
>>    --
>>    target/i386/tcg/helper-tcg.h:QEMU_BUILD_BUG_ON(TCG_PHYS_ADDR_BITS > 
>> TARGET_PHYS_ADDR_SPACE_BITS);
> 
> Likewise.
> 
> 
>>    target/loongarch/internals.h:#define TARGET_PHYS_MASK 
>> MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)
> 
> This is used by target/loongarch/tcg/tlb_helper.c.
> 
> I'm not sure what the implications are.
It is to convert it to physical address for compatible issue. With page 
directory table, HW discards bits higher than 
TARGET_PHYS_ADDR_SPACE_BITS for compatible issue since SW has already 
used in this way.

SW sets higher bit and treats it as virtual address, software can use it 
directly with set_pXd() and needs not convert to physical address. In 
future SW can use page directory table with physical address method, 
however there is no obvious benefits and motivation :(

Regards
Bibo Mao

> Should it be using a common function with the loongarch boot virt-to-phys?
> Is it re-using TARGET_PHYS_ADDR_SPACE_BITS just because it was convienient?
> 
> In either case, it's not exposed to generic code.
> 
> 
> r~

Re: [PATCH 1/5] hw/riscv: Use generic hwaddr for firmware addressses

[Anton Johansson via]

On 15/10/25, Philippe Mathieu-Daudé wrote:
> On 15/10/25 15:27, Anton Johansson wrote:
> > Signed-off-by: Anton Johansson <anjo@rev.ng>
> > ---
> >   include/hw/riscv/boot.h    | 20 ++++++++++----------
> >   hw/riscv/boot.c            | 22 +++++++++++-----------
> >   hw/riscv/microchip_pfsoc.c |  2 +-
> >   hw/riscv/sifive_u.c        |  2 +-
> >   hw/riscv/spike.c           |  4 ++--
> >   hw/riscv/virt.c            |  2 +-
> >   6 files changed, 26 insertions(+), 26 deletions(-)
> > 
> > diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> > index 7d59b2e6c6..d835594baa 100644
> > --- a/include/hw/riscv/boot.h
> > +++ b/include/hw/riscv/boot.h
> > @@ -43,21 +43,21 @@ bool riscv_is_32bit(RISCVHartArrayState *harts);
> >   char *riscv_plic_hart_config_string(int hart_count);
> >   void riscv_boot_info_init(RISCVBootInfo *info, RISCVHartArrayState *harts);
> > -target_ulong riscv_calc_kernel_start_addr(RISCVBootInfo *info,
> > -                                          target_ulong firmware_end_addr);
> > -target_ulong riscv_find_and_load_firmware(MachineState *machine,
> > -                                          const char *default_machine_firmware,
> > -                                          hwaddr *firmware_load_addr,
> > -                                          symbol_fn_t sym_cb);
> > +hwaddr riscv_calc_kernel_start_addr(RISCVBootInfo *info,
> > +                                    hwaddr firmware_end_addr);
> > +hwaddr riscv_find_and_load_firmware(MachineState *machine,
> > +                                    const char *default_machine_firmware,
> > +                                    hwaddr *firmware_load_addr,
> > +                                    symbol_fn_t sym_cb);
> >   const char *riscv_default_firmware_name(RISCVHartArrayState *harts);
> >   char *riscv_find_firmware(const char *firmware_filename,
> >                             const char *default_machine_firmware);
> > -target_ulong riscv_load_firmware(const char *firmware_filename,
> > -                                 hwaddr *firmware_load_addr,
> > -                                 symbol_fn_t sym_cb);
> > +hwaddr riscv_load_firmware(const char *firmware_filename,
> > +                           hwaddr *firmware_load_addr,
> > +                           symbol_fn_t sym_cb);
> >   void riscv_load_kernel(MachineState *machine,
> >                          RISCVBootInfo *info,
> > -                       target_ulong kernel_start_addr,
> > +                       hwaddr kernel_start_addr,
> 
> vaddr?

Maybe vaddr would be more suitable, I went with hwaddr as
kernel_start_addr is fed into load_image_targphys_as()
which expects hwaddr, and hw/arm does the same.

Re: [PATCH 1/5] hw/riscv: Use generic hwaddr for firmware addressses

[Philippe Mathieu-Daudé]

On 23/10/25 19:14, Anton Johansson wrote:
> On 15/10/25, Philippe Mathieu-Daudé wrote:
>> On 15/10/25 15:27, Anton Johansson wrote:
>>> Signed-off-by: Anton Johansson <anjo@rev.ng>
>>> ---
>>>    include/hw/riscv/boot.h    | 20 ++++++++++----------
>>>    hw/riscv/boot.c            | 22 +++++++++++-----------
>>>    hw/riscv/microchip_pfsoc.c |  2 +-
>>>    hw/riscv/sifive_u.c        |  2 +-
>>>    hw/riscv/spike.c           |  4 ++--
>>>    hw/riscv/virt.c            |  2 +-
>>>    6 files changed, 26 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
>>> index 7d59b2e6c6..d835594baa 100644
>>> --- a/include/hw/riscv/boot.h
>>> +++ b/include/hw/riscv/boot.h
>>> @@ -43,21 +43,21 @@ bool riscv_is_32bit(RISCVHartArrayState *harts);
>>>    char *riscv_plic_hart_config_string(int hart_count);
>>>    void riscv_boot_info_init(RISCVBootInfo *info, RISCVHartArrayState *harts);
>>> -target_ulong riscv_calc_kernel_start_addr(RISCVBootInfo *info,
>>> -                                          target_ulong firmware_end_addr);
>>> -target_ulong riscv_find_and_load_firmware(MachineState *machine,
>>> -                                          const char *default_machine_firmware,
>>> -                                          hwaddr *firmware_load_addr,
>>> -                                          symbol_fn_t sym_cb);
>>> +hwaddr riscv_calc_kernel_start_addr(RISCVBootInfo *info,
>>> +                                    hwaddr firmware_end_addr);
>>> +hwaddr riscv_find_and_load_firmware(MachineState *machine,
>>> +                                    const char *default_machine_firmware,
>>> +                                    hwaddr *firmware_load_addr,
>>> +                                    symbol_fn_t sym_cb);
>>>    const char *riscv_default_firmware_name(RISCVHartArrayState *harts);
>>>    char *riscv_find_firmware(const char *firmware_filename,
>>>                              const char *default_machine_firmware);
>>> -target_ulong riscv_load_firmware(const char *firmware_filename,
>>> -                                 hwaddr *firmware_load_addr,
>>> -                                 symbol_fn_t sym_cb);
>>> +hwaddr riscv_load_firmware(const char *firmware_filename,
>>> +                           hwaddr *firmware_load_addr,
>>> +                           symbol_fn_t sym_cb);
>>>    void riscv_load_kernel(MachineState *machine,
>>>                           RISCVBootInfo *info,
>>> -                       target_ulong kernel_start_addr,
>>> +                       hwaddr kernel_start_addr,
>>
>> vaddr?
> 
> Maybe vaddr would be more suitable, I went with hwaddr as
> kernel_start_addr is fed into load_image_targphys_as()
> which expects hwaddr, and hw/arm does the same.

load_kernel() tries to load a file at a vaddr; when it fails
(because can not be parsed, i.e. ELF) the fallback is to try
to load as firmware at a hwaddr. My understanding anyway...