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>
---
hw/m68k/q800.c | 13 ++++++++-----
include/hw/m68k/q800.h | 2 ++
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 3730b30dd1..c34b2548ca 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,10 @@ 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,
+ M68K_CPU_TYPE_NAME("m68040"));
+ object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
+ qemu_register_reset(main_cpu_reset, &m->cpu);
/* RAM */
memory_region_add_subregion(get_system_memory(), 0, machine->ram);
@@ -430,7 +432,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 +608,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;
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index 76ea6560b2..0f54f1c2cf 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -29,6 +29,8 @@
struct Q800MachineState {
MachineState parent_obj;
+
+ M68kCPU cpu;
};
#define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
--
2.30.2On 31/5/23 14:53, 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>
> ---
> hw/m68k/q800.c | 13 ++++++++-----
> include/hw/m68k/q800.h | 2 ++
> 2 files changed, 10 insertions(+), 5 deletions(-)
> @@ -407,8 +407,10 @@ 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,
> + M68K_CPU_TYPE_NAME("m68040"));
Shouldn't we keep using machine->cpu_type?
If the m68040 is the single CPU usable, we should set
MachineClass::valid_cpu_types[] in q800_machine_class_init().
> + object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
> + qemu_register_reset(main_cpu_reset, &m->cpu);
On 31/05/2023 18:43, Philippe Mathieu-Daudé wrote:
> On 31/5/23 14:53, 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>
>> ---
>> hw/m68k/q800.c | 13 ++++++++-----
>> include/hw/m68k/q800.h | 2 ++
>> 2 files changed, 10 insertions(+), 5 deletions(-)
>
>
>> @@ -407,8 +407,10 @@ 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,
>> + M68K_CPU_TYPE_NAME("m68040"));
>
> Shouldn't we keep using machine->cpu_type?
>
> If the m68040 is the single CPU usable, we should set
> MachineClass::valid_cpu_types[] in q800_machine_class_init().
>
>> + object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
>> + qemu_register_reset(main_cpu_reset, &m->cpu);
Yes I can do that: I don't think it makes any difference to the q800 machine here
because the MacOS toolbox ROM doesn't appear to boot with anything other than a 68040
CPU, but it could be useful to make this explicit.
ATB,
Mark.
Le 31/05/2023 à 19:43, Philippe Mathieu-Daudé a écrit :
> On 31/5/23 14:53, 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>
>> ---
>> hw/m68k/q800.c | 13 ++++++++-----
>> include/hw/m68k/q800.h | 2 ++
>> 2 files changed, 10 insertions(+), 5 deletions(-)
>
>
>> @@ -407,8 +407,10 @@ 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,
>> + M68K_CPU_TYPE_NAME("m68040"));
>
> Shouldn't we keep using machine->cpu_type?
>
> If the m68040 is the single CPU usable, we should set
> MachineClass::valid_cpu_types[] in q800_machine_class_init().
>
You're right. Quadra 800 only exists with 68040 processor.
Originally I didn't want to stick to the processor and memory limit to be able to experiment, but as
the work of Mark makes the machine converging to the real hardware specs, I think we can force to
use only 68040. The "virt" machine is a better vehicule to experiment now.
Thanks,
Laurent
On 31/5/23 14:53, 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>
> ---
> hw/m68k/q800.c | 13 ++++++++-----
> include/hw/m68k/q800.h | 2 ++
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index 3730b30dd1..c34b2548ca 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,10 @@ 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,
> + M68K_CPU_TYPE_NAME("m68040"));
> + object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
CPUs are QDev-based, shouldn't we use qdev_realize()?
> + qemu_register_reset(main_cpu_reset, &m->cpu);
>
> /* RAM */
> memory_region_add_subregion(get_system_memory(), 0, machine->ram);
> @@ -430,7 +432,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 +608,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;
> diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
> index 76ea6560b2..0f54f1c2cf 100644
> --- a/include/hw/m68k/q800.h
> +++ b/include/hw/m68k/q800.h
> @@ -29,6 +29,8 @@
>
> struct Q800MachineState {
> MachineState parent_obj;
> +
> + M68kCPU cpu;
Declared in "target/m68k/cpu-qom.h" (missing #include).
> };
>
> #define TYPE_Q800_MACHINE MACHINE_TYPE_NAME("q800")
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> On 31/5/23 14:53, 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>
>> ---
>> hw/m68k/q800.c | 13 ++++++++-----
>> include/hw/m68k/q800.h | 2 ++
>> 2 files changed, 10 insertions(+), 5 deletions(-)
>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>> index 3730b30dd1..c34b2548ca 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,10 @@ 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,
>> + M68K_CPU_TYPE_NAME("m68040"));
>> + object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
>
> CPUs are QDev-based, shouldn't we use qdev_realize()?
Yes, we should.
[...]
On 31/05/2023 16:00, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
>> On 31/5/23 14:53, 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>
>>> ---
>>> hw/m68k/q800.c | 13 ++++++++-----
>>> include/hw/m68k/q800.h | 2 ++
>>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>>> index 3730b30dd1..c34b2548ca 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,10 @@ 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,
>>> + M68K_CPU_TYPE_NAME("m68040"));
>>> + object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
>>
>> CPUs are QDev-based, shouldn't we use qdev_realize()?
>
> Yes, we should.
>
> [...]
Interesting. I remember thinking that CPUs were different, so I'm fairly sure I
borrowed this from some similar code in hw/arm :)
Shouldn't the above be directly equivalent to qdev_realize(dev, NULL, &error_fatal)
given that the CPU doesn't connect to a bus?
ATB,
Mark.
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> On 31/05/2023 16:00, Markus Armbruster wrote:
>
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> On 31/5/23 14:53, 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>
>>>> ---
>>>> hw/m68k/q800.c | 13 ++++++++-----
>>>> include/hw/m68k/q800.h | 2 ++
>>>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>>>> index 3730b30dd1..c34b2548ca 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,10 @@ 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,
>>>> + M68K_CPU_TYPE_NAME("m68040"));
>>>> + object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
>>>
>>> CPUs are QDev-based, shouldn't we use qdev_realize()?
>>
>> Yes, we should.
>> [...]
>
> Interesting. I remember thinking that CPUs were different, so I'm fairly sure I borrowed this from some similar code in hw/arm :)
>
> Shouldn't the above be directly equivalent to qdev_realize(dev, NULL, &error_fatal) given that the CPU doesn't connect to a bus?
It's been a while since I worked on this...
Commit ce189ab230b (qdev: Convert bus-less devices to qdev_realize()
with Coccinelle) looks like you're right.
On 01/06/2023 10:00, Markus Armbruster wrote:
> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
>
>> On 31/05/2023 16:00, Markus Armbruster wrote:
>>
>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>
>>>> On 31/5/23 14:53, 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>
>>>>> ---
>>>>> hw/m68k/q800.c | 13 ++++++++-----
>>>>> include/hw/m68k/q800.h | 2 ++
>>>>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>>>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>>>>> index 3730b30dd1..c34b2548ca 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,10 @@ 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,
>>>>> + M68K_CPU_TYPE_NAME("m68040"));
>>>>> + object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
>>>>
>>>> CPUs are QDev-based, shouldn't we use qdev_realize()?
>>>
>>> Yes, we should.
>>> [...]
>>
>> Interesting. I remember thinking that CPUs were different, so I'm fairly sure I borrowed this from some similar code in hw/arm :)
>>
>> Shouldn't the above be directly equivalent to qdev_realize(dev, NULL, &error_fatal) given that the CPU doesn't connect to a bus?
>
> It's been a while since I worked on this...
>
> Commit ce189ab230b (qdev: Convert bus-less devices to qdev_realize()
> with Coccinelle) looks like you're right.
Thanks for the confirmation! Given that this matches existing code that doesn't use
cpu_create(), I'm inclined to keep this as-is to avoid creating another pattern for
instantiating CPUs.
ATB,
Mark.
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> On 01/06/2023 10:00, Markus Armbruster wrote:
>
>> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
>>
>>> On 31/05/2023 16:00, Markus Armbruster wrote:
>>>
>>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>>
>>>>> On 31/5/23 14:53, 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>
>>>>>> ---
>>>>>> hw/m68k/q800.c | 13 ++++++++-----
>>>>>> include/hw/m68k/q800.h | 2 ++
>>>>>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>>>>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>>>>>> index 3730b30dd1..c34b2548ca 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,10 @@ 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,
>>>>>> + M68K_CPU_TYPE_NAME("m68040"));
>>>>>> + object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
>>>>>
>>>>> CPUs are QDev-based, shouldn't we use qdev_realize()?
>>>>
>>>> Yes, we should.
>>>> [...]
>>>
>>> Interesting. I remember thinking that CPUs were different, so I'm fairly sure I borrowed this from some similar code in hw/arm :)
>>>
>>> Shouldn't the above be directly equivalent to qdev_realize(dev, NULL, &error_fatal) given that the CPU doesn't connect to a bus?
>>
>> It's been a while since I worked on this...
>>
>> Commit ce189ab230b (qdev: Convert bus-less devices to qdev_realize()
>> with Coccinelle) looks like you're right.
>
> Thanks for the confirmation! Given that this matches existing code that doesn't use cpu_create(), I'm inclined to keep this as-is to avoid creating another pattern for instantiating CPUs.
Wherever you *can* use qdev_realize(), you should. The less we access
property "realized" outside qdev core, the better.
I few accesses have crept in since I converted the tree to
qdev_realize() & friends. Another conversion pass would be in order.
On 13/06/2023 12:50, Markus Armbruster wrote:
> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
>
>> On 01/06/2023 10:00, Markus Armbruster wrote:
>>
>>> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
>>>
>>>> On 31/05/2023 16:00, Markus Armbruster wrote:
>>>>
>>>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>>>
>>>>>> On 31/5/23 14:53, 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>
>>>>>>> ---
>>>>>>> hw/m68k/q800.c | 13 ++++++++-----
>>>>>>> include/hw/m68k/q800.h | 2 ++
>>>>>>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>>>>>> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
>>>>>>> index 3730b30dd1..c34b2548ca 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,10 @@ 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,
>>>>>>> + M68K_CPU_TYPE_NAME("m68040"));
>>>>>>> + object_property_set_bool(OBJECT(&m->cpu), "realized", true, &error_fatal);
>>>>>>
>>>>>> CPUs are QDev-based, shouldn't we use qdev_realize()?
>>>>>
>>>>> Yes, we should.
>>>>> [...]
>>>>
>>>> Interesting. I remember thinking that CPUs were different, so I'm fairly sure I borrowed this from some similar code in hw/arm :)
>>>>
>>>> Shouldn't the above be directly equivalent to qdev_realize(dev, NULL, &error_fatal) given that the CPU doesn't connect to a bus?
>>>
>>> It's been a while since I worked on this...
>>>
>>> Commit ce189ab230b (qdev: Convert bus-less devices to qdev_realize()
>>> with Coccinelle) looks like you're right.
>>
>> Thanks for the confirmation! Given that this matches existing code that doesn't use cpu_create(), I'm inclined to keep this as-is to avoid creating another pattern for instantiating CPUs.
>
> Wherever you *can* use qdev_realize(), you should. The less we access
> property "realized" outside qdev core, the better.
No worries, in that case I will switch it to use qdev_realize() in v4.
> I few accesses have crept in since I converted the tree to
> qdev_realize() & friends. Another conversion pass would be in order.
ATB,
Mark.
© 2016 - 2026 Red Hat, Inc.