qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Improve Microchip Polarfire SoC customization
@ 2025-02-14  6:24 Sebastian Huber
  2025-02-14  6:24 ` [PATCH 1/5] hw/misc: Add MPFS system reset support Sebastian Huber
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Sebastian Huber @ 2025-02-14  6:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Conor Dooley, Bin Meng

Booting the microchip-icicle-kit machine using the latest PolarFire SoC
Hart Software Services (HSS) no longer works since Qemu lacks support
for several registers (clocks, DRAM controller). Also reading from the
SDCard does not work currently.

In order to allow tests runs for real-time kernels such as RTEMS and
Zephyr, improve the boot customization. This patch set enables a direct
run of kernel executables, for example:

qemu-system-riscv64 -no-reboot -nographic \
  -serial null -serial mon:stdio \
  -smp 2 \
  -bios none \
  -machine microchip-icicle-kit,clint-timebase-frequency=10000000 \
  -kernel rtos.elf

Sebastian Huber (5):
  hw/misc: Add MPFS system reset support
  hw/riscv: More flexible FDT placement for MPFS
  hw/riscv: Make FDT optional for MPFS
  hw/riscv: Allow direct start of kernel for MPFS
  hw/riscv: Configurable MPFS CLINT timebase freq

 hw/misc/mchp_pfsoc_sysreg.c        |   7 ++
 hw/riscv/microchip_pfsoc.c         | 147 +++++++++++++++++++++--------
 include/hw/riscv/microchip_pfsoc.h |   1 +
 3 files changed, 115 insertions(+), 40 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/5] hw/misc: Add MPFS system reset support
  2025-02-14  6:24 [PATCH 0/5] Improve Microchip Polarfire SoC customization Sebastian Huber
@ 2025-02-14  6:24 ` Sebastian Huber
  2025-02-14  6:24 ` [PATCH 2/5] hw/riscv: More flexible FDT placement for MPFS Sebastian Huber
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Sebastian Huber @ 2025-02-14  6:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Conor Dooley, Bin Meng

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 hw/misc/mchp_pfsoc_sysreg.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/misc/mchp_pfsoc_sysreg.c b/hw/misc/mchp_pfsoc_sysreg.c
index 7876fe0c5b..08196525aa 100644
--- a/hw/misc/mchp_pfsoc_sysreg.c
+++ b/hw/misc/mchp_pfsoc_sysreg.c
@@ -27,7 +27,9 @@
 #include "hw/irq.h"
 #include "hw/sysbus.h"
 #include "hw/misc/mchp_pfsoc_sysreg.h"
+#include "system/runstate.h"
 
+#define MSS_RESET_CR    0x18
 #define ENVM_CR         0xb8
 #define MESSAGE_INT     0x118c
 
@@ -56,6 +58,11 @@ static void mchp_pfsoc_sysreg_write(void *opaque, hwaddr offset,
 {
     MchpPfSoCSysregState *s = opaque;
     switch (offset) {
+    case MSS_RESET_CR:
+        if (value == 0xdead) {
+            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+        }
+        break;
     case MESSAGE_INT:
         qemu_irq_lower(s->irq);
         break;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/5] hw/riscv: More flexible FDT placement for MPFS
  2025-02-14  6:24 [PATCH 0/5] Improve Microchip Polarfire SoC customization Sebastian Huber
  2025-02-14  6:24 ` [PATCH 1/5] hw/misc: Add MPFS system reset support Sebastian Huber
@ 2025-02-14  6:24 ` Sebastian Huber
  2025-02-24  5:15   ` Alistair Francis
  2025-02-14  6:24 ` [PATCH 3/5] hw/riscv: Make FDT optional " Sebastian Huber
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Sebastian Huber @ 2025-02-14  6:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Conor Dooley, Bin Meng

If the kernel entry is in the high DRAM area, place the FDT into this
area.

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 hw/riscv/microchip_pfsoc.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index ec7e2e4226..2ddc3464bb 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -626,8 +626,15 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
         kernel_entry = boot_info.image_low_addr;
 
         /* 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,
+        hwaddr kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
+        hwaddr kernel_ram_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
+
+        if (kernel_entry - kernel_ram_base >= kernel_ram_size) {
+            kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
+            kernel_ram_size = mem_high_size;
+        }
+
+        fdt_load_addr = riscv_compute_fdt_addr(kernel_ram_base, kernel_ram_size,
                                                machine, &boot_info);
         riscv_load_fdt(fdt_load_addr, machine->fdt);
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/5] hw/riscv: Make FDT optional for MPFS
  2025-02-14  6:24 [PATCH 0/5] Improve Microchip Polarfire SoC customization Sebastian Huber
  2025-02-14  6:24 ` [PATCH 1/5] hw/misc: Add MPFS system reset support Sebastian Huber
  2025-02-14  6:24 ` [PATCH 2/5] hw/riscv: More flexible FDT placement for MPFS Sebastian Huber
@ 2025-02-14  6:24 ` Sebastian Huber
  2025-02-24  5:22   ` Alistair Francis
  2025-02-14  6:24 ` [PATCH 4/5] hw/riscv: Allow direct start of kernel " Sebastian Huber
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Sebastian Huber @ 2025-02-14  6:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Conor Dooley, Bin Meng

Real-time kernels such as RTEMS or Zephyr may use a static device tree
built into the kernel image.  Do not require to use the -dtb option if
-kernel is used for the microchip-icicle-kit machine.

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 hw/riscv/microchip_pfsoc.c | 53 ++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 28 deletions(-)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 2ddc3464bb..f9e256df52 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -516,7 +516,6 @@ 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;
-    bool kernel_as_payload = false;
     target_ulong firmware_end_addr, kernel_start_addr;
     uint64_t kernel_entry;
     uint64_t fdt_load_addr;
@@ -589,25 +588,12 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
      *
      * This ensures backwards compatibility with how we used to expose -bios
      * to users but allows them to run through direct kernel booting as well.
-     *
-     * When -kernel is used for direct boot, -dtb must be present to provide
-     * a valid device tree for the board, as we don't generate device tree.
      */
 
-    if (machine->kernel_filename && machine->dtb) {
-        int fdt_size;
-        machine->fdt = load_device_tree(machine->dtb, &fdt_size);
-        if (!machine->fdt) {
-            error_report("load_device_tree() failed");
-            exit(1);
-        }
-
+    if (machine->kernel_filename) {
         firmware_name = RISCV64_BIOS_BIN;
         firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
-        kernel_as_payload = true;
-    }
-
-    if (!kernel_as_payload) {
+    } else {
         firmware_name = BIOS_FILENAME;
         firmware_load_addr = RESET_VECTOR;
     }
@@ -617,7 +603,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
                                                      &firmware_load_addr, NULL);
 
     riscv_boot_info_init(&boot_info, &s->soc.u_cpus);
-    if (kernel_as_payload) {
+    if (machine->kernel_filename) {
         kernel_start_addr = riscv_calc_kernel_start_addr(&boot_info,
                                                          firmware_end_addr);
 
@@ -625,19 +611,30 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
                           true, NULL);
         kernel_entry = boot_info.image_low_addr;
 
-        /* Compute the fdt load address in dram */
-        hwaddr kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
-        hwaddr kernel_ram_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
-
-        if (kernel_entry - kernel_ram_base >= kernel_ram_size) {
-            kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
-            kernel_ram_size = mem_high_size;
+        if (machine->dtb) {
+            int fdt_size;
+            machine->fdt = load_device_tree(machine->dtb, &fdt_size);
+            if (!machine->fdt) {
+                error_report("load_device_tree() failed");
+                exit(1);
+            }
+
+            /* Compute the FDT load address in DRAM */
+            hwaddr kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
+            hwaddr kernel_ram_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
+
+            if (kernel_entry - kernel_ram_base >= kernel_ram_size) {
+                kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
+                kernel_ram_size = mem_high_size;
+            }
+
+            fdt_load_addr = riscv_compute_fdt_addr(kernel_ram_base, kernel_ram_size,
+                                                   machine, &boot_info);
+            riscv_load_fdt(fdt_load_addr, machine->fdt);
+        } else {
+            fdt_load_addr = 0;
         }
 
-        fdt_load_addr = riscv_compute_fdt_addr(kernel_ram_base, kernel_ram_size,
-                                               machine, &boot_info);
-        riscv_load_fdt(fdt_load_addr, machine->fdt);
-
         /* Load the reset vector */
         riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr,
                                   memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/5] hw/riscv: Allow direct start of kernel for MPFS
  2025-02-14  6:24 [PATCH 0/5] Improve Microchip Polarfire SoC customization Sebastian Huber
                   ` (2 preceding siblings ...)
  2025-02-14  6:24 ` [PATCH 3/5] hw/riscv: Make FDT optional " Sebastian Huber
@ 2025-02-14  6:24 ` Sebastian Huber
  2025-02-21  0:11   ` [PATCH v2] " Sebastian Huber
  2025-02-14  6:24 ` [PATCH 5/5] hw/riscv: Configurable MPFS CLINT timebase freq Sebastian Huber
  2025-02-20 18:30 ` [PATCH 0/5] Improve Microchip Polarfire SoC customization Conor Dooley
  5 siblings, 1 reply; 18+ messages in thread
From: Sebastian Huber @ 2025-02-14  6:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Conor Dooley, Bin Meng

Further customize the -bios and -kernel options behaviour for the
microchip-icicle-kit machine.  If "-bios none -kernel filename" is
specified, then do not load a firmware and instead only load and start
the kernel image.

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 hw/riscv/microchip_pfsoc.c | 56 ++++++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index f9e256df52..76a2c56419 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -578,29 +578,44 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
     }
 
     /*
-     * We follow the following table to select which payload we execute.
+     * We follow the following table to select which firmware we use.
      *
-     *  -bios |    -kernel | payload
-     * -------+------------+--------
-     *      N |          N | HSS
-     *      Y | don't care | HSS
-     *      N |          Y | kernel
-     *
-     * This ensures backwards compatibility with how we used to expose -bios
-     * to users but allows them to run through direct kernel booting as well.
+     * -bios         | -kernel    | firmware
+     * --------------+------------+--------
+     * none          |          N | error
+     * none          |          Y | kernel
+     * NULL, default |          N | BIOS_FILENAME
+     * NULL, default |          Y | RISCV64_BIOS_BIN
+     * other         | don't care | other
      */
+    if (machine->firmware && !strcmp(machine->firmware, "none")) {
+        if (!machine->kernel_filename) {
+            error_report("for -bios none, a kernel is required");
+            exit(1);
+        }
 
-    if (machine->kernel_filename) {
-        firmware_name = RISCV64_BIOS_BIN;
-        firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
+        firmware_name = NULL;
+        firmware_load_addr = RESET_VECTOR;
+    } else if (!machine->firmware || !strcmp(machine->firmware, "default")) {
+        if (machine->kernel_filename) {
+            firmware_name = RISCV64_BIOS_BIN;
+            firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
+        } else {
+            firmware_name = BIOS_FILENAME;
+            firmware_load_addr = RESET_VECTOR;
+        }
     } else {
-        firmware_name = BIOS_FILENAME;
+        firmware_name = machine->firmware;
         firmware_load_addr = RESET_VECTOR;
     }
 
-    /* Load the firmware */
-    firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
-                                                     &firmware_load_addr, NULL);
+    /* Load the firmware if necessary */
+    if (firmware_name) {
+        firmware_end_addr = riscv_load_firmware(firmware_name,
+                                                &firmware_load_addr, NULL);
+    } else {
+        firmware_end_addr = firmware_load_addr;
+    }
 
     riscv_boot_info_init(&boot_info, &s->soc.u_cpus);
     if (machine->kernel_filename) {
@@ -635,8 +650,15 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
             fdt_load_addr = 0;
         }
 
+        hwaddr start_addr;
+        if (firmware_name) {
+            start_addr = firmware_load_addr;
+        } else {
+            start_addr = kernel_entry;
+        }
+
         /* Load the reset vector */
-        riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr,
+        riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, start_addr,
                                   memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
                                   memmap[MICROCHIP_PFSOC_ENVM_DATA].size,
                                   kernel_entry, fdt_load_addr);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/5] hw/riscv: Configurable MPFS CLINT timebase freq
  2025-02-14  6:24 [PATCH 0/5] Improve Microchip Polarfire SoC customization Sebastian Huber
                   ` (3 preceding siblings ...)
  2025-02-14  6:24 ` [PATCH 4/5] hw/riscv: Allow direct start of kernel " Sebastian Huber
@ 2025-02-14  6:24 ` Sebastian Huber
  2025-02-16 14:06   ` Philippe Mathieu-Daudé
  2025-02-20 18:30 ` [PATCH 0/5] Improve Microchip Polarfire SoC customization Conor Dooley
  5 siblings, 1 reply; 18+ messages in thread
From: Sebastian Huber @ 2025-02-14  6:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: Conor Dooley, Bin Meng

This property enables the setting of the CLINT timebase frequency
through the command line, for example:

  -machine microchip-icicle-kit,clint-timebase-frequency=10000000

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---
 hw/riscv/microchip_pfsoc.c         | 49 +++++++++++++++++++++++++++---
 include/hw/riscv/microchip_pfsoc.h |  1 +
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 76a2c56419..c83d588962 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -39,6 +39,7 @@
 #include "qemu/units.h"
 #include "qemu/cutils.h"
 #include "qapi/error.h"
+#include "qapi/visitor.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "hw/sysbus.h"
@@ -61,9 +62,6 @@
 #define BIOS_FILENAME   "hss.bin"
 #define RESET_VECTOR    0x20220000
 
-/* CLINT timebase frequency */
-#define CLINT_TIMEBASE_FREQ 1000000
-
 /* GEM version */
 #define GEM_REVISION    0x0107010c
 
@@ -193,6 +191,7 @@ static void microchip_pfsoc_soc_instance_init(Object *obj)
 static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
+    MicrochipIcicleKitState *iks = MICROCHIP_ICICLE_KIT_MACHINE(ms);
     MicrochipPFSoCState *s = MICROCHIP_PFSOC(dev);
     const MemMapEntry *memmap = microchip_pfsoc_memmap;
     MemoryRegion *system_memory = get_system_memory();
@@ -253,7 +252,7 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
         memmap[MICROCHIP_PFSOC_CLINT].base + RISCV_ACLINT_SWI_SIZE,
         RISCV_ACLINT_DEFAULT_MTIMER_SIZE, 0, ms->smp.cpus,
         RISCV_ACLINT_DEFAULT_MTIMECMP, RISCV_ACLINT_DEFAULT_MTIME,
-        CLINT_TIMEBASE_FREQ, false);
+        iks->clint_timebase_freq, false);
 
     /* L2 cache controller */
     create_unimplemented_device("microchip.pfsoc.l2cc",
@@ -665,6 +664,40 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
     }
 }
 
+static void microchip_icicle_kit_set_clint_timebase_freq(Object *obj,
+                                                         Visitor *v,
+                                                         const char *name,
+                                                         void *opaque,
+                                                         Error **errp)
+{
+    MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(obj);
+    uint32_t value;
+
+    if (!visit_type_uint32(v, name, &value, errp)) {
+        return;
+    }
+
+    s->clint_timebase_freq = value;
+}
+
+static void microchip_icicle_kit_get_clint_timebase_freq(Object *obj,
+                                                         Visitor *v,
+                                                         const char *name,
+                                                         void *opaque,
+                                                         Error **errp)
+{
+    MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(obj);
+    uint32_t value = s->clint_timebase_freq;
+
+    visit_type_uint32(v, name, &value, errp);
+}
+
+static void microchip_icicle_kit_machine_instance_init(Object *obj)
+{
+    MicrochipIcicleKitState *m = MICROCHIP_ICICLE_KIT_MACHINE(obj);
+    m->clint_timebase_freq = 1000000;
+}
+
 static void microchip_icicle_kit_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -685,12 +718,20 @@ static void microchip_icicle_kit_machine_class_init(ObjectClass *oc, void *data)
      * See memory_tests() in mss_ddr.c in the HSS source code.
      */
     mc->default_ram_size = 1537 * MiB;
+
+    object_class_property_add(oc, "clint-timebase-frequency", "uint32_t",
+                              microchip_icicle_kit_get_clint_timebase_freq,
+                              microchip_icicle_kit_set_clint_timebase_freq,
+                              NULL, NULL);
+    object_class_property_set_description(oc, "clint-timebase-frequency",
+                                  "Set CLINT timebase frequency in Hz.");
 }
 
 static const TypeInfo microchip_icicle_kit_machine_typeinfo = {
     .name       = MACHINE_TYPE_NAME("microchip-icicle-kit"),
     .parent     = TYPE_MACHINE,
     .class_init = microchip_icicle_kit_machine_class_init,
+    .instance_init = microchip_icicle_kit_machine_instance_init,
     .instance_size = sizeof(MicrochipIcicleKitState),
 };
 
diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
index daef086da6..7ca9b976c1 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -67,6 +67,7 @@ typedef struct MicrochipIcicleKitState {
     MachineState parent_obj;
 
     /*< public >*/
+    uint32_t clint_timebase_freq;
     MicrochipPFSoCState soc;
 } MicrochipIcicleKitState;
 
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/5] hw/riscv: Configurable MPFS CLINT timebase freq
  2025-02-14  6:24 ` [PATCH 5/5] hw/riscv: Configurable MPFS CLINT timebase freq Sebastian Huber
@ 2025-02-16 14:06   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-02-16 14:06 UTC (permalink / raw)
  To: Sebastian Huber, qemu-devel
  Cc: Conor Dooley, Bin Meng, Markus Armbruster, Paolo Bonzini,
	Peter Maydell

On 14/2/25 07:24, Sebastian Huber wrote:
> This property enables the setting of the CLINT timebase frequency
> through the command line, for example:
> 
>    -machine microchip-icicle-kit,clint-timebase-frequency=10000000
> 
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
> ---
>   hw/riscv/microchip_pfsoc.c         | 49 +++++++++++++++++++++++++++---
>   include/hw/riscv/microchip_pfsoc.h |  1 +
>   2 files changed, 46 insertions(+), 4 deletions(-)

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

Although few comments related to pre-existing code inlined.

> 
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 76a2c56419..c83d588962 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -39,6 +39,7 @@
>   #include "qemu/units.h"
>   #include "qemu/cutils.h"
>   #include "qapi/error.h"
> +#include "qapi/visitor.h"
>   #include "hw/boards.h"
>   #include "hw/loader.h"
>   #include "hw/sysbus.h"
> @@ -61,9 +62,6 @@
>   #define BIOS_FILENAME   "hss.bin"
>   #define RESET_VECTOR    0x20220000
>   
> -/* CLINT timebase frequency */
> -#define CLINT_TIMEBASE_FREQ 1000000
> -
>   /* GEM version */
>   #define GEM_REVISION    0x0107010c
>   
> @@ -193,6 +191,7 @@ static void microchip_pfsoc_soc_instance_init(Object *obj)
>   static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>   {
>       MachineState *ms = MACHINE(qdev_get_machine());
> +    MicrochipIcicleKitState *iks = MICROCHIP_ICICLE_KIT_MACHINE(ms);
>       MicrochipPFSoCState *s = MICROCHIP_PFSOC(dev);
>       const MemMapEntry *memmap = microchip_pfsoc_memmap;
>       MemoryRegion *system_memory = get_system_memory();
> @@ -253,7 +252,7 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>           memmap[MICROCHIP_PFSOC_CLINT].base + RISCV_ACLINT_SWI_SIZE,
>           RISCV_ACLINT_DEFAULT_MTIMER_SIZE, 0, ms->smp.cpus,
>           RISCV_ACLINT_DEFAULT_MTIMECMP, RISCV_ACLINT_DEFAULT_MTIME,
> -        CLINT_TIMEBASE_FREQ, false);
> +        iks->clint_timebase_freq, false);

1/ Ideally TYPE_RISCV_ACLINT_MTIMER should take a Clock input.

>   
>       /* L2 cache controller */
>       create_unimplemented_device("microchip.pfsoc.l2cc",
> @@ -665,6 +664,40 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>       }
>   }
>   
> +static void microchip_icicle_kit_set_clint_timebase_freq(Object *obj,
> +                                                         Visitor *v,
> +                                                         const char *name,
> +                                                         void *opaque,
> +                                                         Error **errp)
> +{
> +    MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(obj);
> +    uint32_t value;
> +
> +    if (!visit_type_uint32(v, name, &value, errp)) {

2/ Could we use qemu_strtosz_metric() here?

Better would be to implement visit_type_frequency(), similar
to visit_type_size(), to parse a 'Hz' suffix.

> +        return;
> +    }
> +
> +    s->clint_timebase_freq = value;
> +}
> +
> +static void microchip_icicle_kit_get_clint_timebase_freq(Object *obj,
> +                                                         Visitor *v,
> +                                                         const char *name,
> +                                                         void *opaque,
> +                                                         Error **errp)
> +{
> +    MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(obj);
> +    uint32_t value = s->clint_timebase_freq;
> +
> +    visit_type_uint32(v, name, &value, errp);
> +}
> +
> +static void microchip_icicle_kit_machine_instance_init(Object *obj)
> +{
> +    MicrochipIcicleKitState *m = MICROCHIP_ICICLE_KIT_MACHINE(obj);
> +    m->clint_timebase_freq = 1000000;
> +}

3/ If MachineState were inheriting QDev, we could use qdev-properties.h
API. I'm not sure why we have to use duplicated QOM boiler plate code
here.

Peter, Paolo, Markus, do you have views on this?

Thanks,

Phil.

>   static void microchip_icicle_kit_machine_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> @@ -685,12 +718,20 @@ static void microchip_icicle_kit_machine_class_init(ObjectClass *oc, void *data)
>        * See memory_tests() in mss_ddr.c in the HSS source code.
>        */
>       mc->default_ram_size = 1537 * MiB;
> +
> +    object_class_property_add(oc, "clint-timebase-frequency", "uint32_t",
> +                              microchip_icicle_kit_get_clint_timebase_freq,
> +                              microchip_icicle_kit_set_clint_timebase_freq,
> +                              NULL, NULL);
> +    object_class_property_set_description(oc, "clint-timebase-frequency",
> +                                  "Set CLINT timebase frequency in Hz.");
>   }
>   
>   static const TypeInfo microchip_icicle_kit_machine_typeinfo = {
>       .name       = MACHINE_TYPE_NAME("microchip-icicle-kit"),
>       .parent     = TYPE_MACHINE,
>       .class_init = microchip_icicle_kit_machine_class_init,
> +    .instance_init = microchip_icicle_kit_machine_instance_init,
>       .instance_size = sizeof(MicrochipIcicleKitState),
>   };
>   
> diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
> index daef086da6..7ca9b976c1 100644
> --- a/include/hw/riscv/microchip_pfsoc.h
> +++ b/include/hw/riscv/microchip_pfsoc.h
> @@ -67,6 +67,7 @@ typedef struct MicrochipIcicleKitState {
>       MachineState parent_obj;
>   
>       /*< public >*/
> +    uint32_t clint_timebase_freq;
>       MicrochipPFSoCState soc;
>   } MicrochipIcicleKitState;
>   



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] Improve Microchip Polarfire SoC customization
  2025-02-14  6:24 [PATCH 0/5] Improve Microchip Polarfire SoC customization Sebastian Huber
                   ` (4 preceding siblings ...)
  2025-02-14  6:24 ` [PATCH 5/5] hw/riscv: Configurable MPFS CLINT timebase freq Sebastian Huber
@ 2025-02-20 18:30 ` Conor Dooley
  2025-02-20 22:29   ` Philippe Mathieu-Daudé
  2025-02-24  5:14   ` Alistair Francis
  5 siblings, 2 replies; 18+ messages in thread
From: Conor Dooley @ 2025-02-20 18:30 UTC (permalink / raw)
  To: Sebastian Huber
  Cc: qemu-devel, Conor Dooley, Bin Meng, alistair.francis, qemu-riscv

[-- Attachment #1: Type: text/plain, Size: 3034 bytes --]

+cc qemu-riscv, Alistar.

On Fri, Feb 14, 2025 at 07:24:37AM +0100, Sebastian Huber wrote:
> Booting the microchip-icicle-kit machine using the latest PolarFire SoC
> Hart Software Services (HSS) no longer works since Qemu lacks support
> for several registers (clocks, DRAM controller). Also reading from the
> SDCard does not work currently.

On that note, I think the inaccurate docs about polarfire should be
removed. There's a wiki page here with dead links, or links to things
that do not work anymore:
https://wiki.qemu.org/Documentation/Platforms/RISCV#Microchip_PolarFire_SoC_Icicle_Kit
I think the whole section should be removed, find it kinda odd that
there's a polarfire section but not for any other board. Either way,
it's talking about something that just does not work, the current HSS
and Yocto don't boot.

There's also a docs page here:
https://www.qemu.org/docs/master/system/riscv/microchip-icicle-kit.html
that has a copy of the table your patch 4 modifies, that probably should
be updated to match your changes.

In a similar vein to the wiki, it talks about the HSS and booting a
yocto wic image. I think those should be deleted since they don't work.

Alistar/Other RISC-V folks, what do you think? Bin wrote the port but
seems to be AFK and I don't have the capacity to fix any of that stuff
on top of what I already do in my spare time - do you agree that
deleting the now inaccurate docs makes sense?

> In order to allow tests runs for real-time kernels such as RTEMS and
> Zephyr, improve the boot customization. This patch set enables a direct
> run of kernel executables, for example:
> 
> qemu-system-riscv64 -no-reboot -nographic \
>   -serial null -serial mon:stdio \
>   -smp 2 \
>   -bios none \
>   -machine microchip-icicle-kit,clint-timebase-frequency=10000000 \
>   -kernel rtos.elf

The series breaks my usage:
qemu//build/qemu-system-riscv64 -M microchip-icicle-kit \
        -m 3G -smp 5 \
        -kernel vmlinux.bin \
        -dtb riscvpc.dtb \
        -initrd initramfs.cpio.gz \
        -display none -serial null \
        -serial mon:stdio \
        -D qemu.log -d unimp
opensbi-riscv64-generic-fw_dynamic.bin: No such file or directory
qemu-system-riscv64: could not load firmware 'opensbi-riscv64-generic-fw_dynamic.bin'
make: *** [Makefile:305: qemu-icicle] Error 1

Figure it is likely to be your patch 4? The file does exist, so probably
some sort of path-to-it issues?

Cheers,
Conor.

> 
> Sebastian Huber (5):
>   hw/misc: Add MPFS system reset support
>   hw/riscv: More flexible FDT placement for MPFS
>   hw/riscv: Make FDT optional for MPFS
>   hw/riscv: Allow direct start of kernel for MPFS
>   hw/riscv: Configurable MPFS CLINT timebase freq
> 
>  hw/misc/mchp_pfsoc_sysreg.c        |   7 ++
>  hw/riscv/microchip_pfsoc.c         | 147 +++++++++++++++++++++--------
>  include/hw/riscv/microchip_pfsoc.h |   1 +
>  3 files changed, 115 insertions(+), 40 deletions(-)
> 
> -- 
> 2.43.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] Improve Microchip Polarfire SoC customization
  2025-02-20 18:30 ` [PATCH 0/5] Improve Microchip Polarfire SoC customization Conor Dooley
@ 2025-02-20 22:29   ` Philippe Mathieu-Daudé
  2025-02-21  0:13     ` Sebastian Huber
  2025-02-24  5:14   ` Alistair Francis
  1 sibling, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-02-20 22:29 UTC (permalink / raw)
  To: Conor Dooley, Sebastian Huber
  Cc: qemu-devel, Conor Dooley, Bin Meng, alistair.francis, qemu-riscv

Hi Conor,

On 20/2/25 19:30, Conor Dooley wrote:
> +cc qemu-riscv, Alistar.
> 
> On Fri, Feb 14, 2025 at 07:24:37AM +0100, Sebastian Huber wrote:
>> Booting the microchip-icicle-kit machine using the latest PolarFire SoC
>> Hart Software Services (HSS) no longer works since Qemu lacks support
>> for several registers (clocks, DRAM controller). Also reading from the
>> SDCard does not work currently.
> 
> On that note, I think the inaccurate docs about polarfire should be
> removed. There's a wiki page here with dead links, or links to things
> that do not work anymore:
> https://wiki.qemu.org/Documentation/Platforms/RISCV#Microchip_PolarFire_SoC_Icicle_Kit
> I think the whole section should be removed, find it kinda odd that
> there's a polarfire section but not for any other board. Either way,
> it's talking about something that just does not work, the current HSS
> and Yocto don't boot.
> 
> There's also a docs page here:
> https://www.qemu.org/docs/master/system/riscv/microchip-icicle-kit.html
> that has a copy of the table your patch 4 modifies, that probably should
> be updated to match your changes.
> 
> In a similar vein to the wiki, it talks about the HSS and booting a
> yocto wic image. I think those should be deleted since they don't work.
> 
> Alistar/Other RISC-V folks, what do you think? Bin wrote the port but
> seems to be AFK and I don't have the capacity to fix any of that stuff
> on top of what I already do in my spare time - do you agree that
> deleting the now inaccurate docs makes sense?
> 
>> In order to allow tests runs for real-time kernels such as RTEMS and
>> Zephyr, improve the boot customization. This patch set enables a direct
>> run of kernel executables, for example:
>>
>> qemu-system-riscv64 -no-reboot -nographic \
>>    -serial null -serial mon:stdio \
>>    -smp 2 \
>>    -bios none \
>>    -machine microchip-icicle-kit,clint-timebase-frequency=10000000 \
>>    -kernel rtos.elf
> 
> The series breaks my usage:
> qemu//build/qemu-system-riscv64 -M microchip-icicle-kit \
>          -m 3G -smp 5 \
>          -kernel vmlinux.bin \
>          -dtb riscvpc.dtb \
>          -initrd initramfs.cpio.gz \
>          -display none -serial null \
>          -serial mon:stdio \
>          -D qemu.log -d unimp
> opensbi-riscv64-generic-fw_dynamic.bin: No such file or directory
> qemu-system-riscv64: could not load firmware 'opensbi-riscv64-generic-fw_dynamic.bin'
> make: *** [Makefile:305: qemu-icicle] Error 1
> 
> Figure it is likely to be your patch 4? The file does exist, so probably
> some sort of path-to-it issues?

Maybe missing the -L option?

   -L path         set the directory for the BIOS, VGA BIOS and keymaps

Regards,

Phil.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] hw/riscv: Allow direct start of kernel for MPFS
  2025-02-14  6:24 ` [PATCH 4/5] hw/riscv: Allow direct start of kernel " Sebastian Huber
@ 2025-02-21  0:11   ` Sebastian Huber
  0 siblings, 0 replies; 18+ messages in thread
From: Sebastian Huber @ 2025-02-21  0:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-riscv, Conor Dooley

Further customize the -bios and -kernel options behaviour for the
microchip-icicle-kit machine.  If "-bios none -kernel filename" is
specified, then do not load a firmware and instead only load and start
the kernel image.

Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
---

v2: Use riscv_find_firmware() to locate the firmware shipped with Qemu.

 hw/riscv/microchip_pfsoc.c | 57 ++++++++++++++++++++++++++------------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 4d3c5c8e20..b5ac057a1d 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -578,29 +578,45 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
     }
 
     /*
-     * We follow the following table to select which payload we execute.
+     * We follow the following table to select which firmware we use.
      *
-     *  -bios |    -kernel | payload
-     * -------+------------+--------
-     *      N |          N | HSS
-     *      Y | don't care | HSS
-     *      N |          Y | kernel
-     *
-     * This ensures backwards compatibility with how we used to expose -bios
-     * to users but allows them to run through direct kernel booting as well.
+     * -bios         | -kernel    | firmware
+     * --------------+------------+--------
+     * none          |          N | error
+     * none          |          Y | kernel
+     * NULL, default |          N | BIOS_FILENAME
+     * NULL, default |          Y | RISCV64_BIOS_BIN
+     * other         | don't care | other
      */
+    if (machine->firmware && !strcmp(machine->firmware, "none")) {
+        if (!machine->kernel_filename) {
+            error_report("for -bios none, a kernel is required");
+            exit(1);
+        }
 
-    if (machine->kernel_filename) {
-        firmware_name = RISCV64_BIOS_BIN;
-        firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
+        firmware_name = NULL;
+        firmware_load_addr = RESET_VECTOR;
+    } else if (!machine->firmware || !strcmp(machine->firmware, "default")) {
+        if (machine->kernel_filename) {
+            firmware_name = RISCV64_BIOS_BIN;
+            firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
+        } else {
+            firmware_name = BIOS_FILENAME;
+            firmware_load_addr = RESET_VECTOR;
+        }
     } else {
-        firmware_name = BIOS_FILENAME;
+        firmware_name = machine->firmware;
         firmware_load_addr = RESET_VECTOR;
     }
 
-    /* Load the firmware */
-    firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
-                                                     &firmware_load_addr, NULL);
+    /* Load the firmware if necessary */
+    if (firmware_name) {
+        const char *filename = riscv_find_firmware(firmware_name, NULL);
+        firmware_end_addr = riscv_load_firmware(filename, &firmware_load_addr,
+                                                NULL);
+    } else {
+        firmware_end_addr = firmware_load_addr;
+    }
 
     riscv_boot_info_init(&boot_info, &s->soc.u_cpus);
     if (machine->kernel_filename) {
@@ -635,8 +651,15 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
             fdt_load_addr = 0;
         }
 
+        hwaddr start_addr;
+        if (firmware_name) {
+            start_addr = firmware_load_addr;
+        } else {
+            start_addr = kernel_entry;
+        }
+
         /* Load the reset vector */
-        riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr,
+        riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, start_addr,
                                   memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
                                   memmap[MICROCHIP_PFSOC_ENVM_DATA].size,
                                   kernel_entry, fdt_load_addr);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] Improve Microchip Polarfire SoC customization
  2025-02-20 22:29   ` Philippe Mathieu-Daudé
@ 2025-02-21  0:13     ` Sebastian Huber
  0 siblings, 0 replies; 18+ messages in thread
From: Sebastian Huber @ 2025-02-21  0:13 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Conor Dooley, qemu-devel, Conor Dooley, Bin Meng,
	alistair francis, qemu-riscv

----- Am 20. Feb 2025 um 23:29 schrieb Philippe Mathieu-Daudé philmd@linaro.org:

> Hi Conor,
> 
> On 20/2/25 19:30, Conor Dooley wrote:
>> +cc qemu-riscv, Alistar.
>> 
>> On Fri, Feb 14, 2025 at 07:24:37AM +0100, Sebastian Huber wrote:
>>> Booting the microchip-icicle-kit machine using the latest PolarFire SoC
>>> Hart Software Services (HSS) no longer works since Qemu lacks support
>>> for several registers (clocks, DRAM controller). Also reading from the
>>> SDCard does not work currently.
>> 
>> On that note, I think the inaccurate docs about polarfire should be
>> removed. There's a wiki page here with dead links, or links to things
>> that do not work anymore:
>> https://wiki.qemu.org/Documentation/Platforms/RISCV#Microchip_PolarFire_SoC_Icicle_Kit
>> I think the whole section should be removed, find it kinda odd that
>> there's a polarfire section but not for any other board. Either way,
>> it's talking about something that just does not work, the current HSS
>> and Yocto don't boot.
>> 
>> There's also a docs page here:
>> https://www.qemu.org/docs/master/system/riscv/microchip-icicle-kit.html
>> that has a copy of the table your patch 4 modifies, that probably should
>> be updated to match your changes.
>> 
>> In a similar vein to the wiki, it talks about the HSS and booting a
>> yocto wic image. I think those should be deleted since they don't work.
>> 
>> Alistar/Other RISC-V folks, what do you think? Bin wrote the port but
>> seems to be AFK and I don't have the capacity to fix any of that stuff
>> on top of what I already do in my spare time - do you agree that
>> deleting the now inaccurate docs makes sense?
>> 
>>> In order to allow tests runs for real-time kernels such as RTEMS and
>>> Zephyr, improve the boot customization. This patch set enables a direct
>>> run of kernel executables, for example:
>>>
>>> qemu-system-riscv64 -no-reboot -nographic \
>>>    -serial null -serial mon:stdio \
>>>    -smp 2 \
>>>    -bios none \
>>>    -machine microchip-icicle-kit,clint-timebase-frequency=10000000 \
>>>    -kernel rtos.elf
>> 
>> The series breaks my usage:
>> qemu//build/qemu-system-riscv64 -M microchip-icicle-kit \
>>          -m 3G -smp 5 \
>>          -kernel vmlinux.bin \
>>          -dtb riscvpc.dtb \
>>          -initrd initramfs.cpio.gz \
>>          -display none -serial null \
>>          -serial mon:stdio \
>>          -D qemu.log -d unimp
>> opensbi-riscv64-generic-fw_dynamic.bin: No such file or directory
>> qemu-system-riscv64: could not load firmware
>> 'opensbi-riscv64-generic-fw_dynamic.bin'
>> make: *** [Makefile:305: qemu-icicle] Error 1
>> 
>> Figure it is likely to be your patch 4? The file does exist, so probably
>> some sort of path-to-it issues?
> 
> Maybe missing the -L option?
> 
>   -L path         set the directory for the BIOS, VGA BIOS and keymaps

It was an error in patch 4/5. I sent a v2 version of it.

You have to find the firmware, before you can load it.

-- 
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] Improve Microchip Polarfire SoC customization
  2025-02-20 18:30 ` [PATCH 0/5] Improve Microchip Polarfire SoC customization Conor Dooley
  2025-02-20 22:29   ` Philippe Mathieu-Daudé
@ 2025-02-24  5:14   ` Alistair Francis
  2025-02-24 21:28     ` Conor Dooley
  1 sibling, 1 reply; 18+ messages in thread
From: Alistair Francis @ 2025-02-24  5:14 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Sebastian Huber, qemu-devel, Conor Dooley, Bin Meng,
	alistair.francis, qemu-riscv

On Fri, Feb 21, 2025 at 4:31 AM Conor Dooley <conor@kernel.org> wrote:
>
> +cc qemu-riscv, Alistar.
>
> On Fri, Feb 14, 2025 at 07:24:37AM +0100, Sebastian Huber wrote:
> > Booting the microchip-icicle-kit machine using the latest PolarFire SoC
> > Hart Software Services (HSS) no longer works since Qemu lacks support
> > for several registers (clocks, DRAM controller). Also reading from the
> > SDCard does not work currently.
>
> On that note, I think the inaccurate docs about polarfire should be
> removed. There's a wiki page here with dead links, or links to things
> that do not work anymore:
> https://wiki.qemu.org/Documentation/Platforms/RISCV#Microchip_PolarFire_SoC_Icicle_Kit
> I think the whole section should be removed, find it kinda odd that
> there's a polarfire section but not for any other board. Either way,
> it's talking about something that just does not work, the current HSS
> and Yocto don't boot.

The wiki is independent of the QEMU code base, so you can modify it. I
agree that we should remove outdated/wrong information as it's very
confusing to users.

>
> There's also a docs page here:
> https://www.qemu.org/docs/master/system/riscv/microchip-icicle-kit.html

That is generated from the QEMU codebase
`docs/system/riscv/microchip-icicle-kit.rst` and should be updated,
ideally with this patch set

> that has a copy of the table your patch 4 modifies, that probably should
> be updated to match your changes.
>
> In a similar vein to the wiki, it talks about the HSS and booting a
> yocto wic image. I think those should be deleted since they don't work.

Agreed

Alistair

>
> Alistar/Other RISC-V folks, what do you think? Bin wrote the port but
> seems to be AFK and I don't have the capacity to fix any of that stuff
> on top of what I already do in my spare time - do you agree that
> deleting the now inaccurate docs makes sense?
>
> > In order to allow tests runs for real-time kernels such as RTEMS and
> > Zephyr, improve the boot customization. This patch set enables a direct
> > run of kernel executables, for example:
> >
> > qemu-system-riscv64 -no-reboot -nographic \
> >   -serial null -serial mon:stdio \
> >   -smp 2 \
> >   -bios none \
> >   -machine microchip-icicle-kit,clint-timebase-frequency=10000000 \
> >   -kernel rtos.elf
>
> The series breaks my usage:
> qemu//build/qemu-system-riscv64 -M microchip-icicle-kit \
>         -m 3G -smp 5 \
>         -kernel vmlinux.bin \
>         -dtb riscvpc.dtb \
>         -initrd initramfs.cpio.gz \
>         -display none -serial null \
>         -serial mon:stdio \
>         -D qemu.log -d unimp
> opensbi-riscv64-generic-fw_dynamic.bin: No such file or directory
> qemu-system-riscv64: could not load firmware 'opensbi-riscv64-generic-fw_dynamic.bin'
> make: *** [Makefile:305: qemu-icicle] Error 1
>
> Figure it is likely to be your patch 4? The file does exist, so probably
> some sort of path-to-it issues?
>
> Cheers,
> Conor.
>
> >
> > Sebastian Huber (5):
> >   hw/misc: Add MPFS system reset support
> >   hw/riscv: More flexible FDT placement for MPFS
> >   hw/riscv: Make FDT optional for MPFS
> >   hw/riscv: Allow direct start of kernel for MPFS
> >   hw/riscv: Configurable MPFS CLINT timebase freq
> >
> >  hw/misc/mchp_pfsoc_sysreg.c        |   7 ++
> >  hw/riscv/microchip_pfsoc.c         | 147 +++++++++++++++++++++--------
> >  include/hw/riscv/microchip_pfsoc.h |   1 +
> >  3 files changed, 115 insertions(+), 40 deletions(-)
> >
> > --
> > 2.43.0
> >
> >


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] hw/riscv: More flexible FDT placement for MPFS
  2025-02-14  6:24 ` [PATCH 2/5] hw/riscv: More flexible FDT placement for MPFS Sebastian Huber
@ 2025-02-24  5:15   ` Alistair Francis
  0 siblings, 0 replies; 18+ messages in thread
From: Alistair Francis @ 2025-02-24  5:15 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: qemu-devel, Conor Dooley, Bin Meng

On Fri, Feb 14, 2025 at 4:26 PM Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
>
> If the kernel entry is in the high DRAM area, place the FDT into this
> area.
>
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>

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

Alistair

> ---
>  hw/riscv/microchip_pfsoc.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index ec7e2e4226..2ddc3464bb 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -626,8 +626,15 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>          kernel_entry = boot_info.image_low_addr;
>
>          /* 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,
> +        hwaddr kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> +        hwaddr kernel_ram_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
> +
> +        if (kernel_entry - kernel_ram_base >= kernel_ram_size) {
> +            kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
> +            kernel_ram_size = mem_high_size;
> +        }
> +
> +        fdt_load_addr = riscv_compute_fdt_addr(kernel_ram_base, kernel_ram_size,
>                                                 machine, &boot_info);
>          riscv_load_fdt(fdt_load_addr, machine->fdt);
>
> --
> 2.43.0
>
>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] hw/riscv: Make FDT optional for MPFS
  2025-02-14  6:24 ` [PATCH 3/5] hw/riscv: Make FDT optional " Sebastian Huber
@ 2025-02-24  5:22   ` Alistair Francis
  2025-02-24  5:43     ` Sebastian Huber
  0 siblings, 1 reply; 18+ messages in thread
From: Alistair Francis @ 2025-02-24  5:22 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: qemu-devel, Conor Dooley, Bin Meng

On Fri, Feb 14, 2025 at 4:27 PM Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
>
> Real-time kernels such as RTEMS or Zephyr may use a static device tree
> built into the kernel image.  Do not require to use the -dtb option if
> -kernel is used for the microchip-icicle-kit machine.

That's a fair point, but it might also confuse people who expect QEMU
to generate a DTB if they don't specify one.

This at least needs a documentation update, but a warning log might
also be warranted if no `-dtb` is supplied

Alistair

>
> Signed-off-by: Sebastian Huber <sebastian.huber@embedded-brains.de>
> ---
>  hw/riscv/microchip_pfsoc.c | 53 ++++++++++++++++++--------------------
>  1 file changed, 25 insertions(+), 28 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 2ddc3464bb..f9e256df52 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -516,7 +516,6 @@ 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;
> -    bool kernel_as_payload = false;
>      target_ulong firmware_end_addr, kernel_start_addr;
>      uint64_t kernel_entry;
>      uint64_t fdt_load_addr;
> @@ -589,25 +588,12 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>       *
>       * This ensures backwards compatibility with how we used to expose -bios
>       * to users but allows them to run through direct kernel booting as well.
> -     *
> -     * When -kernel is used for direct boot, -dtb must be present to provide
> -     * a valid device tree for the board, as we don't generate device tree.
>       */
>
> -    if (machine->kernel_filename && machine->dtb) {
> -        int fdt_size;
> -        machine->fdt = load_device_tree(machine->dtb, &fdt_size);
> -        if (!machine->fdt) {
> -            error_report("load_device_tree() failed");
> -            exit(1);
> -        }
> -
> +    if (machine->kernel_filename) {
>          firmware_name = RISCV64_BIOS_BIN;
>          firmware_load_addr = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> -        kernel_as_payload = true;
> -    }
> -
> -    if (!kernel_as_payload) {
> +    } else {
>          firmware_name = BIOS_FILENAME;
>          firmware_load_addr = RESET_VECTOR;
>      }
> @@ -617,7 +603,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>                                                       &firmware_load_addr, NULL);
>
>      riscv_boot_info_init(&boot_info, &s->soc.u_cpus);
> -    if (kernel_as_payload) {
> +    if (machine->kernel_filename) {
>          kernel_start_addr = riscv_calc_kernel_start_addr(&boot_info,
>                                                           firmware_end_addr);
>
> @@ -625,19 +611,30 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>                            true, NULL);
>          kernel_entry = boot_info.image_low_addr;
>
> -        /* Compute the fdt load address in dram */
> -        hwaddr kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> -        hwaddr kernel_ram_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
> -
> -        if (kernel_entry - kernel_ram_base >= kernel_ram_size) {
> -            kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
> -            kernel_ram_size = mem_high_size;
> +        if (machine->dtb) {
> +            int fdt_size;
> +            machine->fdt = load_device_tree(machine->dtb, &fdt_size);
> +            if (!machine->fdt) {
> +                error_report("load_device_tree() failed");
> +                exit(1);
> +            }
> +
> +            /* Compute the FDT load address in DRAM */
> +            hwaddr kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_LO].base;
> +            hwaddr kernel_ram_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
> +
> +            if (kernel_entry - kernel_ram_base >= kernel_ram_size) {
> +                kernel_ram_base = memmap[MICROCHIP_PFSOC_DRAM_HI].base;
> +                kernel_ram_size = mem_high_size;
> +            }
> +
> +            fdt_load_addr = riscv_compute_fdt_addr(kernel_ram_base, kernel_ram_size,
> +                                                   machine, &boot_info);
> +            riscv_load_fdt(fdt_load_addr, machine->fdt);
> +        } else {
> +            fdt_load_addr = 0;
>          }
>
> -        fdt_load_addr = riscv_compute_fdt_addr(kernel_ram_base, kernel_ram_size,
> -                                               machine, &boot_info);
> -        riscv_load_fdt(fdt_load_addr, machine->fdt);
> -
>          /* Load the reset vector */
>          riscv_setup_rom_reset_vec(machine, &s->soc.u_cpus, firmware_load_addr,
>                                    memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
> --
> 2.43.0
>
>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] hw/riscv: Make FDT optional for MPFS
  2025-02-24  5:22   ` Alistair Francis
@ 2025-02-24  5:43     ` Sebastian Huber
  2025-02-24  6:27       ` Alistair Francis
  0 siblings, 1 reply; 18+ messages in thread
From: Sebastian Huber @ 2025-02-24  5:43 UTC (permalink / raw)
  To: Alistair Francis; +Cc: qemu-devel, Conor Dooley, Bin Meng

----- Am 24. Feb 2025 um 6:22 schrieb Alistair Francis alistair23@gmail.com:

> On Fri, Feb 14, 2025 at 4:27 PM Sebastian Huber
> <sebastian.huber@embedded-brains.de> wrote:
>>
>> Real-time kernels such as RTEMS or Zephyr may use a static device tree
>> built into the kernel image.  Do not require to use the -dtb option if
>> -kernel is used for the microchip-icicle-kit machine.
> 
> That's a fair point, but it might also confuse people who expect QEMU
> to generate a DTB if they don't specify one.
> 
> This at least needs a documentation update, but a warning log might
> also be warranted if no `-dtb` is supplied

I will add a documentation update to:

docs/system/riscv/microchip-icicle-kit.rst

Should I change this patch to require "-dtb none" to select no DTB at all?

-- 
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] hw/riscv: Make FDT optional for MPFS
  2025-02-24  5:43     ` Sebastian Huber
@ 2025-02-24  6:27       ` Alistair Francis
  0 siblings, 0 replies; 18+ messages in thread
From: Alistair Francis @ 2025-02-24  6:27 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: qemu-devel, Conor Dooley, Bin Meng

On Mon, Feb 24, 2025 at 3:43 PM Sebastian Huber
<sebastian.huber@embedded-brains.de> wrote:
>
> ----- Am 24. Feb 2025 um 6:22 schrieb Alistair Francis alistair23@gmail.com:
>
> > On Fri, Feb 14, 2025 at 4:27 PM Sebastian Huber
> > <sebastian.huber@embedded-brains.de> wrote:
> >>
> >> Real-time kernels such as RTEMS or Zephyr may use a static device tree
> >> built into the kernel image.  Do not require to use the -dtb option if
> >> -kernel is used for the microchip-icicle-kit machine.
> >
> > That's a fair point, but it might also confuse people who expect QEMU
> > to generate a DTB if they don't specify one.
> >
> > This at least needs a documentation update, but a warning log might
> > also be warranted if no `-dtb` is supplied
>
> I will add a documentation update to:
>
> docs/system/riscv/microchip-icicle-kit.rst
>
> Should I change this patch to require "-dtb none" to select no DTB at all?

I don't think so, generally not specifying something means that it
won't be loaded (-bios is just odd for historical reasons).

Maybe just a log warning like "The QEMU Icicle machine does not
generate a device tree, so no device tree is being provided to the
guest".

It'll be a bit annoying for people who don't want to run Linux (like
your case), but all other Linux class RISC-V boards provide a DTB, so
it's a little confusing

Alistair

>
> --
> embedded brains GmbH & Co. KG
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.huber@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax:   +49-89-18 94 741 - 08
>
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] Improve Microchip Polarfire SoC customization
  2025-02-24  5:14   ` Alistair Francis
@ 2025-02-24 21:28     ` Conor Dooley
  2025-02-26  2:46       ` Alistair Francis
  0 siblings, 1 reply; 18+ messages in thread
From: Conor Dooley @ 2025-02-24 21:28 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Sebastian Huber, qemu-devel, Conor Dooley, Bin Meng,
	alistair.francis, qemu-riscv

[-- Attachment #1: Type: text/plain, Size: 1889 bytes --]

On Mon, Feb 24, 2025 at 03:14:00PM +1000, Alistair Francis wrote:
> On Fri, Feb 21, 2025 at 4:31 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > +cc qemu-riscv, Alistar.
> >
> > On Fri, Feb 14, 2025 at 07:24:37AM +0100, Sebastian Huber wrote:
> > > Booting the microchip-icicle-kit machine using the latest PolarFire SoC
> > > Hart Software Services (HSS) no longer works since Qemu lacks support
> > > for several registers (clocks, DRAM controller). Also reading from the
> > > SDCard does not work currently.
> >
> > On that note, I think the inaccurate docs about polarfire should be
> > removed. There's a wiki page here with dead links, or links to things
> > that do not work anymore:
> > https://wiki.qemu.org/Documentation/Platforms/RISCV#Microchip_PolarFire_SoC_Icicle_Kit
> > I think the whole section should be removed, find it kinda odd that
> > there's a polarfire section but not for any other board. Either way,
> > it's talking about something that just does not work, the current HSS
> > and Yocto don't boot.
> 
> The wiki is independent of the QEMU code base, so you can modify it. I
> agree that we should remove outdated/wrong information as it's very
> confusing to users.

I don't have an account to make the modification, any clue who has one
and can hook me up with one?

> > There's also a docs page here:
> > https://www.qemu.org/docs/master/system/riscv/microchip-icicle-kit.html
> 
> That is generated from the QEMU codebase
> `docs/system/riscv/microchip-icicle-kit.rst` and should be updated,
> ideally with this patch set

Ye, I think so too.

> > that has a copy of the table your patch 4 modifies, that probably should
> > be updated to match your changes.
> >
> > In a similar vein to the wiki, it talks about the HSS and booting a
> > yocto wic image. I think those should be deleted since they don't work.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/5] Improve Microchip Polarfire SoC customization
  2025-02-24 21:28     ` Conor Dooley
@ 2025-02-26  2:46       ` Alistair Francis
  0 siblings, 0 replies; 18+ messages in thread
From: Alistair Francis @ 2025-02-26  2:46 UTC (permalink / raw)
  To: Conor Dooley, Philippe Mathieu-Daudé, Peter Maydell
  Cc: Sebastian Huber, qemu-devel, Conor Dooley, Bin Meng,
	alistair.francis, qemu-riscv

On Tue, Feb 25, 2025 at 7:28 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Feb 24, 2025 at 03:14:00PM +1000, Alistair Francis wrote:
> > On Fri, Feb 21, 2025 at 4:31 AM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > +cc qemu-riscv, Alistar.
> > >
> > > On Fri, Feb 14, 2025 at 07:24:37AM +0100, Sebastian Huber wrote:
> > > > Booting the microchip-icicle-kit machine using the latest PolarFire SoC
> > > > Hart Software Services (HSS) no longer works since Qemu lacks support
> > > > for several registers (clocks, DRAM controller). Also reading from the
> > > > SDCard does not work currently.
> > >
> > > On that note, I think the inaccurate docs about polarfire should be
> > > removed. There's a wiki page here with dead links, or links to things
> > > that do not work anymore:
> > > https://wiki.qemu.org/Documentation/Platforms/RISCV#Microchip_PolarFire_SoC_Icicle_Kit
> > > I think the whole section should be removed, find it kinda odd that
> > > there's a polarfire section but not for any other board. Either way,
> > > it's talking about something that just does not work, the current HSS
> > > and Yocto don't boot.
> >
> > The wiki is independent of the QEMU code base, so you can modify it. I
> > agree that we should remove outdated/wrong information as it's very
> > confusing to users.
>
> I don't have an account to make the modification, any clue who has one
> and can hook me up with one?

@Peter Maydell  @Philippe Mathieu-Daudé can you create wiki accounts?

I don't seem to have permissions to create new accounts

Alistair


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-02-26  2:47 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-14  6:24 [PATCH 0/5] Improve Microchip Polarfire SoC customization Sebastian Huber
2025-02-14  6:24 ` [PATCH 1/5] hw/misc: Add MPFS system reset support Sebastian Huber
2025-02-14  6:24 ` [PATCH 2/5] hw/riscv: More flexible FDT placement for MPFS Sebastian Huber
2025-02-24  5:15   ` Alistair Francis
2025-02-14  6:24 ` [PATCH 3/5] hw/riscv: Make FDT optional " Sebastian Huber
2025-02-24  5:22   ` Alistair Francis
2025-02-24  5:43     ` Sebastian Huber
2025-02-24  6:27       ` Alistair Francis
2025-02-14  6:24 ` [PATCH 4/5] hw/riscv: Allow direct start of kernel " Sebastian Huber
2025-02-21  0:11   ` [PATCH v2] " Sebastian Huber
2025-02-14  6:24 ` [PATCH 5/5] hw/riscv: Configurable MPFS CLINT timebase freq Sebastian Huber
2025-02-16 14:06   ` Philippe Mathieu-Daudé
2025-02-20 18:30 ` [PATCH 0/5] Improve Microchip Polarfire SoC customization Conor Dooley
2025-02-20 22:29   ` Philippe Mathieu-Daudé
2025-02-21  0:13     ` Sebastian Huber
2025-02-24  5:14   ` Alistair Francis
2025-02-24 21:28     ` Conor Dooley
2025-02-26  2:46       ` Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).