diff mbox series

[v4,05/24] q800: move CPU object into Q800MachineState

Message ID 20230621085353.113233-6-mark.cave-ayland@ilande.co.uk
State New
Headers show
Series q800: add support for booting MacOS Classic - part 1 | expand

Commit Message

Mark Cave-Ayland June 21, 2023, 8:53 a.m. UTC 
Also change the instantiation of the CPU to use object_initialize_child()
followed by a separate realisation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/m68k/q800.c         | 18 +++++++++++++-----
 include/hw/m68k/q800.h |  3 +++
 2 files changed, 16 insertions(+), 5 deletions(-)

Comments

BALATON Zoltan June 21, 2023, 11:56 a.m. UTC | #1 
On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> Also change the instantiation of the CPU to use object_initialize_child()
> followed by a separate realisation.

Also seems to restrict valid CPU types but not mentioned in commit 
message. Should this patch be split up?

Regards,
BALATON Zoltan

> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/m68k/q800.c         | 18 +++++++++++++-----
> include/hw/m68k/q800.h |  3 +++
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 062a3c6c76..2b651de3c1 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -364,7 +364,7 @@ static uint8_t fake_mac_rom[] = {
>
> static void q800_machine_init(MachineState *machine)
> {
> -    M68kCPU *cpu = NULL;
> +    Q800MachineState *m = Q800_MACHINE(machine);
>     int linux_boot;
>     int32_t kernel_size;
>     uint64_t elf_entry;
> @@ -407,8 +407,9 @@ static void q800_machine_init(MachineState *machine)
>     }
>
>     /* init CPUs */
> -    cpu = M68K_CPU(cpu_create(machine->cpu_type));
> -    qemu_register_reset(main_cpu_reset, cpu);
> +    object_initialize_child(OBJECT(machine), "cpu", &m->cpu, machine->cpu_type);
> +    qdev_realize(DEVICE(&m->cpu), NULL, &error_fatal);
> +    qemu_register_reset(main_cpu_reset, &m->cpu);
>
>     /* RAM */
>     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
> @@ -430,7 +431,8 @@ static void q800_machine_init(MachineState *machine)
>
>     /* IRQ Glue */
>     glue = qdev_new(TYPE_GLUE);
> -    object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
> +    object_property_set_link(OBJECT(glue), "cpu", OBJECT(&m->cpu),
> +                             &error_abort);
>     sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
>
>     /* VIA 1 */
> @@ -605,7 +607,7 @@ static void q800_machine_init(MachineState *machine)
>
>     macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
>
> -    cs = CPU(cpu);
> +    cs = CPU(&m->cpu);
>     if (linux_boot) {
>         uint64_t high;
>         void *param_blob, *param_ptr, *param_rng_seed;
> @@ -735,6 +737,11 @@ static GlobalProperty hw_compat_q800[] = {
> };
> static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
>
> +static const char *q800_machine_valid_cpu_types[] = {
> +    M68K_CPU_TYPE_NAME("m68040"),
> +    NULL
> +};
> +
> static void q800_machine_class_init(ObjectClass *oc, void *data)
> {
>     MachineClass *mc = MACHINE_CLASS(oc);
> @@ -742,6 +749,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
>     mc->desc = "Macintosh Quadra 800";
>     mc->init = q800_machine_init;
>     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
> +    mc->valid_cpu_types = q800_machine_valid_cpu_types;
>     mc->max_cpus = 1;
>     mc->block_default_type = IF_SCSI;
>     mc->default_ram_id = "m68k_mac.ram";
> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
> index f3bc17aa1b..4cb1a51dfe 100644
> --- a/include/hw/m68k/q800.h
> +++ b/include/hw/m68k/q800.h
> @@ -25,6 +25,7 @@
>
> #include "hw/boards.h"
> #include "qom/object.h"
> +#include "target/m68k/cpu-qom.h"
>
> /*
>  * The main Q800 machine
> @@ -32,6 +33,8 @@
>
> struct Q800MachineState {
>     MachineState parent_obj;
> +
> +    M68kCPU cpu;
> };
>
> #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
>
BALATON Zoltan June 21, 2023, 12:27 p.m. UTC | #2 
On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
> Also change the instantiation of the CPU to use object_initialize_child()
> followed by a separate realisation.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/m68k/q800.c         | 18 +++++++++++++-----
> include/hw/m68k/q800.h |  3 +++
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 062a3c6c76..2b651de3c1 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -364,7 +364,7 @@ static uint8_t fake_mac_rom[] = {
>
> static void q800_machine_init(MachineState *machine)
> {
> -    M68kCPU *cpu = NULL;
> +    Q800MachineState *m = Q800_MACHINE(machine);
>     int linux_boot;
>     int32_t kernel_size;
>     uint64_t elf_entry;
> @@ -407,8 +407,9 @@ static void q800_machine_init(MachineState *machine)
>     }
>
>     /* init CPUs */
> -    cpu = M68K_CPU(cpu_create(machine->cpu_type));
> -    qemu_register_reset(main_cpu_reset, cpu);
> +    object_initialize_child(OBJECT(machine), "cpu", &m->cpu, machine->cpu_type);

As OBJECT(machine) will be used frequently maybe you can make this more 
readable by adding a local var for that. Also renaming sysbus local to sbd 
may improve readability somewhat.

Regards,
BALATON Zoltan

> +    qdev_realize(DEVICE(&m->cpu), NULL, &error_fatal);
> +    qemu_register_reset(main_cpu_reset, &m->cpu);
>
>     /* RAM */
>     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
> @@ -430,7 +431,8 @@ static void q800_machine_init(MachineState *machine)
>
>     /* IRQ Glue */
>     glue = qdev_new(TYPE_GLUE);
> -    object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
> +    object_property_set_link(OBJECT(glue), "cpu", OBJECT(&m->cpu),
> +                             &error_abort);
>     sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
>
>     /* VIA 1 */
> @@ -605,7 +607,7 @@ static void q800_machine_init(MachineState *machine)
>
>     macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
>
> -    cs = CPU(cpu);
> +    cs = CPU(&m->cpu);
>     if (linux_boot) {
>         uint64_t high;
>         void *param_blob, *param_ptr, *param_rng_seed;
> @@ -735,6 +737,11 @@ static GlobalProperty hw_compat_q800[] = {
> };
> static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
>
> +static const char *q800_machine_valid_cpu_types[] = {
> +    M68K_CPU_TYPE_NAME("m68040"),
> +    NULL
> +};
> +
> static void q800_machine_class_init(ObjectClass *oc, void *data)
> {
>     MachineClass *mc = MACHINE_CLASS(oc);
> @@ -742,6 +749,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
>     mc->desc = "Macintosh Quadra 800";
>     mc->init = q800_machine_init;
>     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
> +    mc->valid_cpu_types = q800_machine_valid_cpu_types;
>     mc->max_cpus = 1;
>     mc->block_default_type = IF_SCSI;
>     mc->default_ram_id = "m68k_mac.ram";
> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
> index f3bc17aa1b..4cb1a51dfe 100644
> --- a/include/hw/m68k/q800.h
> +++ b/include/hw/m68k/q800.h
> @@ -25,6 +25,7 @@
>
> #include "hw/boards.h"
> #include "qom/object.h"
> +#include "target/m68k/cpu-qom.h"
>
> /*
>  * The main Q800 machine
> @@ -32,6 +33,8 @@
>
> struct Q800MachineState {
>     MachineState parent_obj;
> +
> +    M68kCPU cpu;
> };
>
> #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
>
Mark Cave-Ayland June 21, 2023, 2:28 p.m. UTC | #3 
On 21/06/2023 12:56, BALATON Zoltan wrote:

> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>> Also change the instantiation of the CPU to use object_initialize_child()
>> followed by a separate realisation.
> 
> Also seems to restrict valid CPU types but not mentioned in commit message. Should 
> this patch be split up?

Hmmm good point. Laurent, would you like me to split this into a separate patch or 
would updating the commit message be sufficient?

>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/m68k/q800.c         | 18 +++++++++++++-----
>> include/hw/m68k/q800.h |  3 +++
>> 2 files changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>> index 062a3c6c76..2b651de3c1 100644
>> --- a/hw/m68k/q800.c
>> +++ b/hw/m68k/q800.c
>> @@ -364,7 +364,7 @@ static uint8_t fake_mac_rom[] = {
>>
>> static void q800_machine_init(MachineState *machine)
>> {
>> -    M68kCPU *cpu = NULL;
>> +    Q800MachineState *m = Q800_MACHINE(machine);
>>     int linux_boot;
>>     int32_t kernel_size;
>>     uint64_t elf_entry;
>> @@ -407,8 +407,9 @@ static void q800_machine_init(MachineState *machine)
>>     }
>>
>>     /* init CPUs */
>> -    cpu = M68K_CPU(cpu_create(machine->cpu_type));
>> -    qemu_register_reset(main_cpu_reset, cpu);
>> +    object_initialize_child(OBJECT(machine), "cpu", &m->cpu, machine->cpu_type);
>> +    qdev_realize(DEVICE(&m->cpu), NULL, &error_fatal);
>> +    qemu_register_reset(main_cpu_reset, &m->cpu);
>>
>>     /* RAM */
>>     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
>> @@ -430,7 +431,8 @@ static void q800_machine_init(MachineState *machine)
>>
>>     /* IRQ Glue */
>>     glue = qdev_new(TYPE_GLUE);
>> -    object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
>> +    object_property_set_link(OBJECT(glue), "cpu", OBJECT(&m->cpu),
>> +                             &error_abort);
>>     sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
>>
>>     /* VIA 1 */
>> @@ -605,7 +607,7 @@ static void q800_machine_init(MachineState *machine)
>>
>>     macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
>>
>> -    cs = CPU(cpu);
>> +    cs = CPU(&m->cpu);
>>     if (linux_boot) {
>>         uint64_t high;
>>         void *param_blob, *param_ptr, *param_rng_seed;
>> @@ -735,6 +737,11 @@ static GlobalProperty hw_compat_q800[] = {
>> };
>> static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
>>
>> +static const char *q800_machine_valid_cpu_types[] = {
>> +    M68K_CPU_TYPE_NAME("m68040"),
>> +    NULL
>> +};
>> +
>> static void q800_machine_class_init(ObjectClass *oc, void *data)
>> {
>>     MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -742,6 +749,7 @@ static void q800_machine_class_init(ObjectClass *oc, void *data)
>>     mc->desc = "Macintosh Quadra 800";
>>     mc->init = q800_machine_init;
>>     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
>> +    mc->valid_cpu_types = q800_machine_valid_cpu_types;
>>     mc->max_cpus = 1;
>>     mc->block_default_type = IF_SCSI;
>>     mc->default_ram_id = "m68k_mac.ram";
>> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
>> index f3bc17aa1b..4cb1a51dfe 100644
>> --- a/include/hw/m68k/q800.h
>> +++ b/include/hw/m68k/q800.h
>> @@ -25,6 +25,7 @@
>>
>> #include "hw/boards.h"
>> #include "qom/object.h"
>> +#include "target/m68k/cpu-qom.h"
>>
>> /*
>>  * The main Q800 machine
>> @@ -32,6 +33,8 @@
>>
>> struct Q800MachineState {
>>     MachineState parent_obj;
>> +
>> +    M68kCPU cpu;
>> };
>>
>> #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")

ATB,

Mark.
Laurent Vivier June 21, 2023, 5:06 p.m. UTC | #4 
Le 21/06/2023 à 16:28, Mark Cave-Ayland a écrit :
> On 21/06/2023 12:56, BALATON Zoltan wrote:
> 
>> On Wed, 21 Jun 2023, Mark Cave-Ayland wrote:
>>> Also change the instantiation of the CPU to use object_initialize_child()
>>> followed by a separate realisation.
>>
>> Also seems to restrict valid CPU types but not mentioned in commit message. Should this patch be 
>> split up?
> 
> Hmmm good point. Laurent, would you like me to split this into a separate patch or would updating 
> the commit message be sufficient?

I'm going to merge the series. Could you send me the update of the commit message?

Thanks,
Laurent
diff mbox series

Patch

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 062a3c6c76..2b651de3c1 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -364,7 +364,7 @@  static uint8_t fake_mac_rom[] = {
 
 static void q800_machine_init(MachineState *machine)
 {
-    M68kCPU *cpu = NULL;
+    Q800MachineState *m = Q800_MACHINE(machine);
     int linux_boot;
     int32_t kernel_size;
     uint64_t elf_entry;
@@ -407,8 +407,9 @@  static void q800_machine_init(MachineState *machine)
     }
 
     /* init CPUs */
-    cpu = M68K_CPU(cpu_create(machine->cpu_type));
-    qemu_register_reset(main_cpu_reset, cpu);
+    object_initialize_child(OBJECT(machine), "cpu", &m->cpu, machine->cpu_type);
+    qdev_realize(DEVICE(&m->cpu), NULL, &error_fatal);
+    qemu_register_reset(main_cpu_reset, &m->cpu);
 
     /* RAM */
     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
@@ -430,7 +431,8 @@  static void q800_machine_init(MachineState *machine)
 
     /* IRQ Glue */
     glue = qdev_new(TYPE_GLUE);
-    object_property_set_link(OBJECT(glue), "cpu", OBJECT(cpu), &error_abort);
+    object_property_set_link(OBJECT(glue), "cpu", OBJECT(&m->cpu),
+                             &error_abort);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(glue), &error_fatal);
 
     /* VIA 1 */
@@ -605,7 +607,7 @@  static void q800_machine_init(MachineState *machine)
 
     macfb_mode = (NUBUS_MACFB(dev)->macfb).mode;
 
-    cs = CPU(cpu);
+    cs = CPU(&m->cpu);
     if (linux_boot) {
         uint64_t high;
         void *param_blob, *param_ptr, *param_rng_seed;
@@ -735,6 +737,11 @@  static GlobalProperty hw_compat_q800[] = {
 };
 static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
 
+static const char *q800_machine_valid_cpu_types[] = {
+    M68K_CPU_TYPE_NAME("m68040"),
+    NULL
+};
+
 static void q800_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -742,6 +749,7 @@  static void q800_machine_class_init(ObjectClass *oc, void *data)
     mc->desc = "Macintosh Quadra 800";
     mc->init = q800_machine_init;
     mc->default_cpu_type = M68K_CPU_TYPE_NAME("m68040");
+    mc->valid_cpu_types = q800_machine_valid_cpu_types;
     mc->max_cpus = 1;
     mc->block_default_type = IF_SCSI;
     mc->default_ram_id = "m68k_mac.ram";
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index f3bc17aa1b..4cb1a51dfe 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -25,6 +25,7 @@ 
 
 #include "hw/boards.h"
 #include "qom/object.h"
+#include "target/m68k/cpu-qom.h"
 
 /*
  * The main Q800 machine
@@ -32,6 +33,8 @@ 
 
 struct Q800MachineState {
     MachineState parent_obj;
+
+    M68kCPU cpu;
 };
 
 #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")