* [PATCH 1/6] hw/hppa: Use the machine default boot order at reset
2025-10-09 14:31 [PATCH 0/6] hw/hppa: Clarify machine variables and move them out of CPUArchState Philippe Mathieu-Daudé
@ 2025-10-09 14:31 ` Philippe Mathieu-Daudé
2025-10-09 17:40 ` Richard Henderson
2025-10-09 14:31 ` [PATCH 2/6] hw/hppa: Convert type_init() -> DEFINE_TYPES() Philippe Mathieu-Daudé
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 14:31 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Anton Johansson, Helge Deller,
Philippe Mathieu-Daudé
Since new machines might be added with different boot order
than 'c', take the machine default one at reset.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/hppa/machine.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index dacedc5409c..1cce5c47b91 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -640,6 +640,7 @@ static void machine_HP_C3700_init(MachineState *machine)
static void hppa_machine_reset(MachineState *ms, ResetType type)
{
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
unsigned int smp_cpus = ms->smp.cpus;
int i;
@@ -671,7 +672,7 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
cpu[0]->env.kernel_entry = 0;
cpu[0]->env.initrd_base = 0;
cpu[0]->env.initrd_end = 0;
- cpu[0]->env.cmdline_or_bootorder = 'c';
+ cpu[0]->env.cmdline_or_bootorder = mc->default_boot_order[0];
}
static void hppa_nmi(NMIState *n, int cpu_index, Error **errp)
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/6] hw/hppa: Use the machine default boot order at reset
2025-10-09 14:31 ` [PATCH 1/6] hw/hppa: Use the machine default boot order at reset Philippe Mathieu-Daudé
@ 2025-10-09 17:40 ` Richard Henderson
0 siblings, 0 replies; 12+ messages in thread
From: Richard Henderson @ 2025-10-09 17:40 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Anton Johansson, Helge Deller
On 10/9/25 07:31, Philippe Mathieu-Daudé wrote:
> Since new machines might be added with different boot order
> than 'c', take the machine default one at reset.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/hppa/machine.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index dacedc5409c..1cce5c47b91 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -640,6 +640,7 @@ static void machine_HP_C3700_init(MachineState *machine)
>
> static void hppa_machine_reset(MachineState *ms, ResetType type)
> {
> + MachineClass *mc = MACHINE_GET_CLASS(ms);
> unsigned int smp_cpus = ms->smp.cpus;
> int i;
>
> @@ -671,7 +672,7 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
> cpu[0]->env.kernel_entry = 0;
> cpu[0]->env.initrd_base = 0;
> cpu[0]->env.initrd_end = 0;
> - cpu[0]->env.cmdline_or_bootorder = 'c';
> + cpu[0]->env.cmdline_or_bootorder = mc->default_boot_order[0];
> }
>
> static void hppa_nmi(NMIState *n, int cpu_index, Error **errp)
Is this perhaps trying to reset boot order to 'c' for reboot?
r~
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/6] hw/hppa: Convert type_init() -> DEFINE_TYPES()
2025-10-09 14:31 [PATCH 0/6] hw/hppa: Clarify machine variables and move them out of CPUArchState Philippe Mathieu-Daudé
2025-10-09 14:31 ` [PATCH 1/6] hw/hppa: Use the machine default boot order at reset Philippe Mathieu-Daudé
@ 2025-10-09 14:31 ` Philippe Mathieu-Daudé
2025-10-09 17:07 ` Richard Henderson
2025-10-09 14:31 ` [PATCH 3/6] hw/hppa: Factor QOM HPPA_COMMON_MACHINE out Philippe Mathieu-Daudé
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 14:31 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Anton Johansson, Helge Deller,
Philippe Mathieu-Daudé
Prefer DEFINE_TYPES() macro over type_init() to register
multiple QOM types.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/hppa/machine.c | 42 ++++++++++++++++++------------------------
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 1cce5c47b91..bbee70022f1 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -710,16 +710,6 @@ static void HP_B160L_machine_init_class_init(ObjectClass *oc, const void *data)
nc->nmi_monitor_handler = hppa_nmi;
}
-static const TypeInfo HP_B160L_machine_init_typeinfo = {
- .name = MACHINE_TYPE_NAME("B160L"),
- .parent = TYPE_MACHINE,
- .class_init = HP_B160L_machine_init_class_init,
- .interfaces = (const InterfaceInfo[]) {
- { TYPE_NMI },
- { }
- },
-};
-
static void HP_C3700_machine_init_class_init(ObjectClass *oc, const void *data)
{
static const char * const valid_cpu_types[] = {
@@ -746,20 +736,24 @@ static void HP_C3700_machine_init_class_init(ObjectClass *oc, const void *data)
nc->nmi_monitor_handler = hppa_nmi;
}
-static const TypeInfo HP_C3700_machine_init_typeinfo = {
- .name = MACHINE_TYPE_NAME("C3700"),
- .parent = TYPE_MACHINE,
- .class_init = HP_C3700_machine_init_class_init,
- .interfaces = (const InterfaceInfo[]) {
- { TYPE_NMI },
- { }
+static const TypeInfo hppa_machine_types[] = {
+ {
+ .name = MACHINE_TYPE_NAME("B160L"),
+ .parent = TYPE_MACHINE,
+ .class_init = HP_B160L_machine_init_class_init,
+ .interfaces = (const InterfaceInfo[]) {
+ { TYPE_NMI },
+ { }
+ },
+ }, {
+ .name = MACHINE_TYPE_NAME("C3700"),
+ .parent = TYPE_MACHINE,
+ .class_init = HP_C3700_machine_init_class_init,
+ .interfaces = (const InterfaceInfo[]) {
+ { TYPE_NMI },
+ { }
+ },
},
};
-static void hppa_machine_init_register_types(void)
-{
- type_register_static(&HP_B160L_machine_init_typeinfo);
- type_register_static(&HP_C3700_machine_init_typeinfo);
-}
-
-type_init(hppa_machine_init_register_types)
+DEFINE_TYPES(hppa_machine_types)
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/6] hw/hppa: Factor QOM HPPA_COMMON_MACHINE out
2025-10-09 14:31 [PATCH 0/6] hw/hppa: Clarify machine variables and move them out of CPUArchState Philippe Mathieu-Daudé
2025-10-09 14:31 ` [PATCH 1/6] hw/hppa: Use the machine default boot order at reset Philippe Mathieu-Daudé
2025-10-09 14:31 ` [PATCH 2/6] hw/hppa: Convert type_init() -> DEFINE_TYPES() Philippe Mathieu-Daudé
@ 2025-10-09 14:31 ` Philippe Mathieu-Daudé
2025-10-09 17:09 ` Richard Henderson
2025-10-09 14:31 ` [PATCH 4/6] hw/hppa: Introduce HppaMachineState::boot_info::firmware structure Philippe Mathieu-Daudé
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 14:31 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Anton Johansson, Helge Deller,
Philippe Mathieu-Daudé
B160L and C3700 share a lot of common code. Factor it out
as an abstract HPPA_COMMON_MACHINE QOM parent.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/hppa/machine.c | 61 +++++++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index bbee70022f1..a2996ef7682 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -36,6 +36,13 @@
#include "net/net.h"
#include "qemu/log.h"
+#define TYPE_HPPA_COMMON_MACHINE MACHINE_TYPE_NAME("hppa-common")
+OBJECT_DECLARE_SIMPLE_TYPE(HppaMachineState, HPPA_COMMON_MACHINE)
+
+struct HppaMachineState {
+ MachineState parent_obj;
+};
+
#define MIN_SEABIOS_HPPA_VERSION 12 /* require at least this fw version */
#define HPA_POWER_BUTTON (FIRMWARE_END - 0x10)
@@ -684,6 +691,22 @@ static void hppa_nmi(NMIState *n, int cpu_index, Error **errp)
}
}
+static void hppa_machine_common_class_init(ObjectClass *oc, const void *data)
+{
+ MachineClass *mc = MACHINE_CLASS(oc);
+ NMIClass *nc = NMI_CLASS(oc);
+
+ mc->reset = hppa_machine_reset;
+ mc->block_default_type = IF_SCSI;
+ mc->default_cpus = 1;
+ mc->max_cpus = HPPA_MAX_CPUS;
+ mc->default_boot_order = "cd";
+ mc->default_ram_id = "ram";
+ mc->default_nic = "tulip";
+
+ nc->nmi_monitor_handler = hppa_nmi;
+}
+
static void HP_B160L_machine_init_class_init(ObjectClass *oc, const void *data)
{
static const char * const valid_cpu_types[] = {
@@ -691,23 +714,13 @@ static void HP_B160L_machine_init_class_init(ObjectClass *oc, const void *data)
NULL
};
MachineClass *mc = MACHINE_CLASS(oc);
- NMIClass *nc = NMI_CLASS(oc);
mc->desc = "HP B160L workstation";
mc->default_cpu_type = TYPE_HPPA_CPU;
mc->valid_cpu_types = valid_cpu_types;
mc->init = machine_HP_B160L_init;
- mc->reset = hppa_machine_reset;
- mc->block_default_type = IF_SCSI;
- mc->max_cpus = HPPA_MAX_CPUS;
- mc->default_cpus = 1;
mc->is_default = true;
mc->default_ram_size = 512 * MiB;
- mc->default_boot_order = "cd";
- mc->default_ram_id = "ram";
- mc->default_nic = "tulip";
-
- nc->nmi_monitor_handler = hppa_nmi;
}
static void HP_C3700_machine_init_class_init(ObjectClass *oc, const void *data)
@@ -717,42 +730,34 @@ static void HP_C3700_machine_init_class_init(ObjectClass *oc, const void *data)
NULL
};
MachineClass *mc = MACHINE_CLASS(oc);
- NMIClass *nc = NMI_CLASS(oc);
mc->desc = "HP C3700 workstation";
mc->default_cpu_type = TYPE_HPPA64_CPU;
mc->valid_cpu_types = valid_cpu_types;
mc->init = machine_HP_C3700_init;
- mc->reset = hppa_machine_reset;
- mc->block_default_type = IF_SCSI;
mc->max_cpus = HPPA_MAX_CPUS;
- mc->default_cpus = 1;
- mc->is_default = false;
mc->default_ram_size = 1024 * MiB;
- mc->default_boot_order = "cd";
- mc->default_ram_id = "ram";
- mc->default_nic = "tulip";
-
- nc->nmi_monitor_handler = hppa_nmi;
}
static const TypeInfo hppa_machine_types[] = {
{
- .name = MACHINE_TYPE_NAME("B160L"),
- .parent = TYPE_MACHINE,
- .class_init = HP_B160L_machine_init_class_init,
+ .name = TYPE_HPPA_COMMON_MACHINE,
+ .parent = TYPE_MACHINE,
+ .instance_size = sizeof(HppaMachineState),
+ .class_init = hppa_machine_common_class_init,
+ .abstract = true,
.interfaces = (const InterfaceInfo[]) {
{ TYPE_NMI },
{ }
},
+ }, {
+ .name = MACHINE_TYPE_NAME("B160L"),
+ .parent = TYPE_HPPA_COMMON_MACHINE,
+ .class_init = HP_B160L_machine_init_class_init,
}, {
.name = MACHINE_TYPE_NAME("C3700"),
- .parent = TYPE_MACHINE,
+ .parent = TYPE_HPPA_COMMON_MACHINE,
.class_init = HP_C3700_machine_init_class_init,
- .interfaces = (const InterfaceInfo[]) {
- { TYPE_NMI },
- { }
- },
},
};
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/6] hw/hppa: Introduce HppaMachineState::boot_info::firmware structure
2025-10-09 14:31 [PATCH 0/6] hw/hppa: Clarify machine variables and move them out of CPUArchState Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2025-10-09 14:31 ` [PATCH 3/6] hw/hppa: Factor QOM HPPA_COMMON_MACHINE out Philippe Mathieu-Daudé
@ 2025-10-09 14:31 ` Philippe Mathieu-Daudé
2025-10-09 17:20 ` Richard Henderson
2025-10-09 14:31 ` [PATCH 5/6] hw/hppa: Introduce HppaMachineState::boot_info::kernel structure Philippe Mathieu-Daudé
2025-10-09 14:31 ` [PATCH 6/6] hw/hppa: Move kernel addresses to HppaMachineState::boot_info structure Philippe Mathieu-Daudé
5 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 14:31 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Anton Johansson, Helge Deller,
Philippe Mathieu-Daudé
Current code uses CPUHPPAState::@kernel_entry to hold either:
- kernel entry virtual address
- firmware interactive mode
and CPUHPPAState::@cmdline_or_bootorder to hold either:
- kernel &cmdline physical address
- firmware boot order
Besides, these variables don't belong to CPUHPPAState, they
depend on how the machine is started, and only apply to the
first CPU.
Introduce firmware specific fields in HppaMachineState, using
their correct type. Introduce the @is_kernel field, to allow
distinguishing between firmware or kernel mode.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/hppa/machine.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index a2996ef7682..138cd97efd9 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -41,6 +41,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(HppaMachineState, HPPA_COMMON_MACHINE)
struct HppaMachineState {
MachineState parent_obj;
+
+ struct {
+ bool is_kernel; /* kernel:1 firmware:0 */
+ union {
+ struct {
+ char bootorder;
+ bool interactive_mode;
+ } firmware;
+ };
+ } boot_info;
};
#define MIN_SEABIOS_HPPA_VERSION 12 /* require at least this fw version */
@@ -356,6 +366,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
const char *initrd_filename = machine->initrd_filename;
const char *firmware = machine->firmware;
MachineClass *mc = MACHINE_GET_CLASS(machine);
+ HppaMachineState *hms = HPPA_COMMON_MACHINE(machine);
DeviceState *dev;
PCIDevice *pci_dev;
char *firmware_filename;
@@ -481,6 +492,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
"-0x%08" PRIx64 ", entry at 0x%08" PRIx64
", size %" PRIu64 " kB\n",
kernel_low, kernel_high, kernel_entry, size / KiB);
+ hms->boot_info.is_kernel = true;
if (kernel_cmdline) {
cpu[0]->env.cmdline_or_bootorder = 0x4000;
@@ -519,13 +531,15 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
}
}
- if (!kernel_entry) {
+ if (!hms->boot_info.is_kernel) {
/* When booting via firmware, tell firmware if we want interactive
- * mode (kernel_entry=1), and to boot from CD (cmdline_or_bootorder='d')
- * or hard disc (cmdline_or_bootorder='c').
+ * mode (interactive_mode=1), and to boot from CD (bootorder='d')
+ * or hard disc (bootorder='c').
*/
- kernel_entry = machine->boot_config.has_menu ? machine->boot_config.menu : 0;
- cpu[0]->env.cmdline_or_bootorder = machine->boot_config.order[0];
+ hms->boot_info.firmware.interactive_mode = machine->boot_config.has_menu
+ ? machine->boot_config.menu
+ : 0;
+ hms->boot_info.firmware.bootorder = machine->boot_config.order[0];
}
/* Keep initial kernel_entry for first boot */
@@ -648,6 +662,7 @@ static void machine_HP_C3700_init(MachineState *machine)
static void hppa_machine_reset(MachineState *ms, ResetType type)
{
MachineClass *mc = MACHINE_GET_CLASS(ms);
+ HppaMachineState *hms = HPPA_COMMON_MACHINE(ms);
unsigned int smp_cpus = ms->smp.cpus;
int i;
@@ -668,8 +683,12 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
}
cpu[0]->env.gr[26] = ms->ram_size;
- cpu[0]->env.gr[25] = cpu[0]->env.kernel_entry;
- cpu[0]->env.gr[24] = cpu[0]->env.cmdline_or_bootorder;
+ cpu[0]->env.gr[25] = hms->boot_info.is_kernel
+ ? cpu[0]->env.kernel_entry
+ : hms->boot_info.firmware.interactive_mode;
+ cpu[0]->env.gr[24] = hms->boot_info.is_kernel
+ ? cpu[0]->env.cmdline_or_bootorder
+ : hms->boot_info.firmware.bootorder;
cpu[0]->env.gr[23] = cpu[0]->env.initrd_base;
cpu[0]->env.gr[22] = cpu[0]->env.initrd_end;
cpu[0]->env.gr[21] = smp_cpus;
@@ -679,7 +698,8 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
cpu[0]->env.kernel_entry = 0;
cpu[0]->env.initrd_base = 0;
cpu[0]->env.initrd_end = 0;
- cpu[0]->env.cmdline_or_bootorder = mc->default_boot_order[0];
+ cpu[0]->env.cmdline_or_bootorder = 0;
+ hms->boot_info.firmware.bootorder = mc->default_boot_order[0];
}
static void hppa_nmi(NMIState *n, int cpu_index, Error **errp)
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 4/6] hw/hppa: Introduce HppaMachineState::boot_info::firmware structure
2025-10-09 14:31 ` [PATCH 4/6] hw/hppa: Introduce HppaMachineState::boot_info::firmware structure Philippe Mathieu-Daudé
@ 2025-10-09 17:20 ` Richard Henderson
2025-10-09 17:25 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 12+ messages in thread
From: Richard Henderson @ 2025-10-09 17:20 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Anton Johansson, Helge Deller
On 10/9/25 07:31, Philippe Mathieu-Daudé wrote:
> Current code uses CPUHPPAState::@kernel_entry to hold either:
> - kernel entry virtual address
> - firmware interactive mode
> and CPUHPPAState::@cmdline_or_bootorder to hold either:
> - kernel &cmdline physical address
> - firmware boot order
>
> Besides, these variables don't belong to CPUHPPAState, they
> depend on how the machine is started, and only apply to the
> first CPU.
>
> Introduce firmware specific fields in HppaMachineState, using
> their correct type. Introduce the @is_kernel field, to allow
> distinguishing between firmware or kernel mode.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/hppa/machine.c | 36 ++++++++++++++++++++++++++++--------
> 1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index a2996ef7682..138cd97efd9 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -41,6 +41,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(HppaMachineState, HPPA_COMMON_MACHINE)
>
> struct HppaMachineState {
> MachineState parent_obj;
> +
> + struct {
> + bool is_kernel; /* kernel:1 firmware:0 */
> + union {
> + struct {
> + char bootorder;
> + bool interactive_mode;
> + } firmware;
> + };
> + } boot_info;
I think this is more complicated than necessary.
> @@ -481,6 +492,7 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
> "-0x%08" PRIx64 ", entry at 0x%08" PRIx64
> ", size %" PRIu64 " kB\n",
> kernel_low, kernel_high, kernel_entry, size / KiB);
> + hms->boot_info.is_kernel = true;
>
> if (kernel_cmdline) {
> cpu[0]->env.cmdline_or_bootorder = 0x4000;
> @@ -519,13 +531,15 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
> }
> }
>
> - if (!kernel_entry) {
> + if (!hms->boot_info.is_kernel) {
> /* When booting via firmware, tell firmware if we want interactive
> - * mode (kernel_entry=1), and to boot from CD (cmdline_or_bootorder='d')
> - * or hard disc (cmdline_or_bootorder='c').
> + * mode (interactive_mode=1), and to boot from CD (bootorder='d')
> + * or hard disc (bootorder='c').
> */
> - kernel_entry = machine->boot_config.has_menu ? machine->boot_config.menu : 0;
> - cpu[0]->env.cmdline_or_bootorder = machine->boot_config.order[0];
> + hms->boot_info.firmware.interactive_mode = machine->boot_config.has_menu
> + ? machine->boot_config.menu
> + : 0;
> + hms->boot_info.firmware.bootorder = machine->boot_config.order[0];
...
> @@ -668,8 +683,12 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
> }
>
> cpu[0]->env.gr[26] = ms->ram_size;
> - cpu[0]->env.gr[25] = cpu[0]->env.kernel_entry;
> - cpu[0]->env.gr[24] = cpu[0]->env.cmdline_or_bootorder;
> + cpu[0]->env.gr[25] = hms->boot_info.is_kernel
> + ? cpu[0]->env.kernel_entry
> + : hms->boot_info.firmware.interactive_mode;
> + cpu[0]->env.gr[24] = hms->boot_info.is_kernel
> + ? cpu[0]->env.cmdline_or_bootorder
> + : hms->boot_info.firmware.bootorder;
I think you'd be better served by placing
uint64_t boot_gr24;
uint64_t boot_gr25;
in HppaMachineState and set them in machine_HP_common_init_tail.
> @@ -679,7 +698,8 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
> cpu[0]->env.kernel_entry = 0;
> cpu[0]->env.initrd_base = 0;
> cpu[0]->env.initrd_end = 0;
> - cpu[0]->env.cmdline_or_bootorder = mc->default_boot_order[0];
> + cpu[0]->env.cmdline_or_bootorder = 0;
> + hms->boot_info.firmware.bootorder = mc->default_boot_order[0];
Setting hms in reset seems wrong.
r~
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 4/6] hw/hppa: Introduce HppaMachineState::boot_info::firmware structure
2025-10-09 17:20 ` Richard Henderson
@ 2025-10-09 17:25 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 17:25 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Anton Johansson, Helge Deller
On 9/10/25 19:20, Richard Henderson wrote:
> On 10/9/25 07:31, Philippe Mathieu-Daudé wrote:
>> Current code uses CPUHPPAState::@kernel_entry to hold either:
>> - kernel entry virtual address
>> - firmware interactive mode
>> and CPUHPPAState::@cmdline_or_bootorder to hold either:
>> - kernel &cmdline physical address
>> - firmware boot order
>>
>> Besides, these variables don't belong to CPUHPPAState, they
>> depend on how the machine is started, and only apply to the
>> first CPU.
>>
>> Introduce firmware specific fields in HppaMachineState, using
>> their correct type. Introduce the @is_kernel field, to allow
>> distinguishing between firmware or kernel mode.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/hppa/machine.c | 36 ++++++++++++++++++++++++++++--------
>> 1 file changed, 28 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
>> index a2996ef7682..138cd97efd9 100644
>> --- a/hw/hppa/machine.c
>> +++ b/hw/hppa/machine.c
>> @@ -41,6 +41,16 @@ OBJECT_DECLARE_SIMPLE_TYPE(HppaMachineState,
>> HPPA_COMMON_MACHINE)
>> struct HppaMachineState {
>> MachineState parent_obj;
>> +
>> + struct {
>> + bool is_kernel; /* kernel:1 firmware:0 */
>> + union {
>> + struct {
>> + char bootorder;
>> + bool interactive_mode;
>> + } firmware;
>> + };
>> + } boot_info;
>
> I think this is more complicated than necessary.
Certainly...
>> @@ -481,6 +492,7 @@ static void
>> machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
>> "-0x%08" PRIx64 ", entry at 0x%08" PRIx64
>> ", size %" PRIu64 " kB\n",
>> kernel_low, kernel_high, kernel_entry, size /
>> KiB);
>> + hms->boot_info.is_kernel = true;
>> if (kernel_cmdline) {
>> cpu[0]->env.cmdline_or_bootorder = 0x4000;
>> @@ -519,13 +531,15 @@ static void
>> machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
>> }
>> }
>> - if (!kernel_entry) {
>> + if (!hms->boot_info.is_kernel) {
>> /* When booting via firmware, tell firmware if we want
>> interactive
>> - * mode (kernel_entry=1), and to boot from CD
>> (cmdline_or_bootorder='d')
>> - * or hard disc (cmdline_or_bootorder='c').
>> + * mode (interactive_mode=1), and to boot from CD
>> (bootorder='d')
>> + * or hard disc (bootorder='c').
>> */
>> - kernel_entry = machine->boot_config.has_menu ? machine-
>> >boot_config.menu : 0;
>> - cpu[0]->env.cmdline_or_bootorder = machine-
>> >boot_config.order[0];
>> + hms->boot_info.firmware.interactive_mode = machine-
>> >boot_config.has_menu
>> + ? machine-
>> >boot_config.menu
>> + : 0;
>> + hms->boot_info.firmware.bootorder = machine-
>> >boot_config.order[0];
> ...
>> @@ -668,8 +683,12 @@ static void hppa_machine_reset(MachineState *ms,
>> ResetType type)
>> }
>> cpu[0]->env.gr[26] = ms->ram_size;
>> - cpu[0]->env.gr[25] = cpu[0]->env.kernel_entry;
>> - cpu[0]->env.gr[24] = cpu[0]->env.cmdline_or_bootorder;
>> + cpu[0]->env.gr[25] = hms->boot_info.is_kernel
>> + ? cpu[0]->env.kernel_entry
>> + : hms->boot_info.firmware.interactive_mode;
>> + cpu[0]->env.gr[24] = hms->boot_info.is_kernel
>> + ? cpu[0]->env.cmdline_or_bootorder
>> + : hms->boot_info.firmware.bootorder;
>
>
> I think you'd be better served by placing
>
> uint64_t boot_gr24;
> uint64_t boot_gr25;
Clever :)
> in HppaMachineState and set them in machine_HP_common_init_tail.
>
>> @@ -679,7 +698,8 @@ static void hppa_machine_reset(MachineState *ms,
>> ResetType type)
>> cpu[0]->env.kernel_entry = 0;
>> cpu[0]->env.initrd_base = 0;
>> cpu[0]->env.initrd_end = 0;
>> - cpu[0]->env.cmdline_or_bootorder = mc->default_boot_order[0];
>> + cpu[0]->env.cmdline_or_bootorder = 0;
>> + hms->boot_info.firmware.bootorder = mc->default_boot_order[0];
>
> Setting hms in reset seems wrong.
Yes, a pre-existing issue. I couldn't figure that piece of code (and
comment) myself.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/6] hw/hppa: Introduce HppaMachineState::boot_info::kernel structure
2025-10-09 14:31 [PATCH 0/6] hw/hppa: Clarify machine variables and move them out of CPUArchState Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2025-10-09 14:31 ` [PATCH 4/6] hw/hppa: Introduce HppaMachineState::boot_info::firmware structure Philippe Mathieu-Daudé
@ 2025-10-09 14:31 ` Philippe Mathieu-Daudé
2025-10-09 14:31 ` [PATCH 6/6] hw/hppa: Move kernel addresses to HppaMachineState::boot_info structure Philippe Mathieu-Daudé
5 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 14:31 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Anton Johansson, Helge Deller,
Philippe Mathieu-Daudé
CPUHPPAState::@cmdline_or_bootorder is now only used in kernel
mode. Move it to HppaMachineState::kernel, and use the 'hwaddr'
type.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
target/hppa/cpu.h | 1 -
hw/hppa/machine.c | 11 +++++++----
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index e14f238827b..0aa06f6ebec 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -273,7 +273,6 @@ typedef struct CPUArchState {
bool is_pa20;
target_ulong kernel_entry; /* Linux kernel was loaded here */
- target_ulong cmdline_or_bootorder;
target_ulong initrd_base, initrd_end;
} CPUHPPAState;
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 138cd97efd9..c315d13ea01 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -49,6 +49,9 @@ struct HppaMachineState {
char bootorder;
bool interactive_mode;
} firmware;
+ struct {
+ hwaddr cmdline_paddr;
+ } kernel;
};
} boot_info;
};
@@ -495,8 +498,8 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
hms->boot_info.is_kernel = true;
if (kernel_cmdline) {
- cpu[0]->env.cmdline_or_bootorder = 0x4000;
- pstrcpy_targphys("cmdline", cpu[0]->env.cmdline_or_bootorder,
+ hms->boot_info.kernel.cmdline_paddr = 0x4000;
+ pstrcpy_targphys("cmdline", hms->boot_info.kernel.cmdline_paddr,
TARGET_PAGE_SIZE, kernel_cmdline);
}
@@ -687,7 +690,7 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
? cpu[0]->env.kernel_entry
: hms->boot_info.firmware.interactive_mode;
cpu[0]->env.gr[24] = hms->boot_info.is_kernel
- ? cpu[0]->env.cmdline_or_bootorder
+ ? hms->boot_info.kernel.cmdline_paddr
: hms->boot_info.firmware.bootorder;
cpu[0]->env.gr[23] = cpu[0]->env.initrd_base;
cpu[0]->env.gr[22] = cpu[0]->env.initrd_end;
@@ -698,7 +701,7 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
cpu[0]->env.kernel_entry = 0;
cpu[0]->env.initrd_base = 0;
cpu[0]->env.initrd_end = 0;
- cpu[0]->env.cmdline_or_bootorder = 0;
+ hms->boot_info.kernel.cmdline_paddr = 0;
hms->boot_info.firmware.bootorder = mc->default_boot_order[0];
}
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/6] hw/hppa: Move kernel addresses to HppaMachineState::boot_info structure
2025-10-09 14:31 [PATCH 0/6] hw/hppa: Clarify machine variables and move them out of CPUArchState Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2025-10-09 14:31 ` [PATCH 5/6] hw/hppa: Introduce HppaMachineState::boot_info::kernel structure Philippe Mathieu-Daudé
@ 2025-10-09 14:31 ` Philippe Mathieu-Daudé
5 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-09 14:31 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Anton Johansson, Helge Deller,
Philippe Mathieu-Daudé
Restrict kernel* related variables scope, move fields that don't
belong to CPUHPPAState to HppaMachineState::boot_info::kernel.
Replace target_ulong -> vaddr type for virtual addresses.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
target/hppa/cpu.h | 3 ---
hw/hppa/machine.c | 31 +++++++++++++++++--------------
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 0aa06f6ebec..49d0243f677 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -271,9 +271,6 @@ typedef struct CPUArchState {
struct {} end_reset_fields;
bool is_pa20;
-
- target_ulong kernel_entry; /* Linux kernel was loaded here */
- target_ulong initrd_base, initrd_end;
} CPUHPPAState;
/**
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index c315d13ea01..a50efac375d 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -50,6 +50,8 @@ struct HppaMachineState {
bool interactive_mode;
} firmware;
struct {
+ vaddr entry; /* Linux kernel was loaded here */
+ vaddr initrd_base, initrd_end;
hwaddr cmdline_paddr;
} kernel;
};
@@ -365,8 +367,6 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
TranslateFn *translate)
{
const char *kernel_filename = machine->kernel_filename;
- const char *kernel_cmdline = machine->kernel_cmdline;
- const char *initrd_filename = machine->initrd_filename;
const char *firmware = machine->firmware;
MachineClass *mc = MACHINE_GET_CLASS(machine);
HppaMachineState *hms = HPPA_COMMON_MACHINE(machine);
@@ -375,7 +375,6 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
char *firmware_filename;
uint64_t firmware_low, firmware_high;
long size;
- uint64_t kernel_entry = 0, kernel_low, kernel_high;
MemoryRegion *addr_space = get_system_memory();
MemoryRegion *rom_region;
SysBusDevice *s;
@@ -481,6 +480,11 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
/* Load kernel */
if (kernel_filename) {
+ uint64_t kernel_entry;
+ uint64_t kernel_low, kernel_high;
+ const char *kernel_cmdline = machine->kernel_cmdline;
+ const char *initrd_filename = machine->initrd_filename;
+
size = load_elf(kernel_filename, NULL, linux_kernel_virt_to_phys,
NULL, &kernel_entry, &kernel_low, &kernel_high, NULL,
ELFDATA2MSB, EM_PARISC, 0, 0);
@@ -496,6 +500,8 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
", size %" PRIu64 " kB\n",
kernel_low, kernel_high, kernel_entry, size / KiB);
hms->boot_info.is_kernel = true;
+ /* Keep initial kernel_entry for first boot */
+ hms->boot_info.kernel.entry = kernel_entry;
if (kernel_cmdline) {
hms->boot_info.kernel.cmdline_paddr = 0x4000;
@@ -529,8 +535,8 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
}
load_image_targphys(initrd_filename, initrd_base, initrd_size);
- cpu[0]->env.initrd_base = initrd_base;
- cpu[0]->env.initrd_end = initrd_base + initrd_size;
+ hms->boot_info.kernel.initrd_base = initrd_base;
+ hms->boot_info.kernel.initrd_end = initrd_base + initrd_size;
}
}
@@ -544,9 +550,6 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
: 0;
hms->boot_info.firmware.bootorder = machine->boot_config.order[0];
}
-
- /* Keep initial kernel_entry for first boot */
- cpu[0]->env.kernel_entry = kernel_entry;
}
/*
@@ -687,20 +690,20 @@ static void hppa_machine_reset(MachineState *ms, ResetType type)
cpu[0]->env.gr[26] = ms->ram_size;
cpu[0]->env.gr[25] = hms->boot_info.is_kernel
- ? cpu[0]->env.kernel_entry
+ ? hms->boot_info.kernel.entry
: hms->boot_info.firmware.interactive_mode;
cpu[0]->env.gr[24] = hms->boot_info.is_kernel
? hms->boot_info.kernel.cmdline_paddr
: hms->boot_info.firmware.bootorder;
- cpu[0]->env.gr[23] = cpu[0]->env.initrd_base;
- cpu[0]->env.gr[22] = cpu[0]->env.initrd_end;
+ cpu[0]->env.gr[23] = hms->boot_info.kernel.initrd_base;
+ cpu[0]->env.gr[22] = hms->boot_info.kernel.initrd_end;
cpu[0]->env.gr[21] = smp_cpus;
cpu[0]->env.gr[19] = FW_CFG_IO_BASE;
/* reset static fields to avoid starting Linux kernel & initrd on reboot */
- cpu[0]->env.kernel_entry = 0;
- cpu[0]->env.initrd_base = 0;
- cpu[0]->env.initrd_end = 0;
+ hms->boot_info.kernel.entry = 0;
+ hms->boot_info.kernel.initrd_base = 0;
+ hms->boot_info.kernel.initrd_end = 0;
hms->boot_info.kernel.cmdline_paddr = 0;
hms->boot_info.firmware.bootorder = mc->default_boot_order[0];
}
--
2.51.0
^ permalink raw reply related [flat|nested] 12+ messages in thread