[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 05/23] q800: move CPU object into Q800MachineState
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 05/23] q800: move CPU object into Q800MachineState |
Date: |
Tue, 13 Jun 2023 13:50:48 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
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.