Re: [RFC V1 04/14] accel: set accelerator and machine props earlier

[Steven Sistare <steven.sistare@oracle.com>]

On 10/18/2024 11:32 AM, Steven Sistare wrote:
> On 10/18/2024 11:08 AM, Fabiano Rosas wrote:
>> Steve Sistare <steven.sistare@oracle.com> writes:
>>
>>> Make all global and compat properties available before the first objects
>>> are created.  Set accelerator compatibility properties in
>>> configure_accelerators, when the accelerator is chosen, and call
>>> configure_accelerators earlier.  Set machine options earlier.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> ---
>>>   accel/accel-system.c |  2 --
>>>   system/vl.c          | 34 ++++++++++++++++++----------------
>>>   2 files changed, 18 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/accel/accel-system.c b/accel/accel-system.c
>>> index f6c947d..c8aeae4 100644
>>> --- a/accel/accel-system.c
>>> +++ b/accel/accel-system.c
>>> @@ -41,8 +41,6 @@ int accel_init_machine(AccelState *accel, MachineState *ms)
>>>           ms->accelerator = NULL;
>>>           *(acc->allowed) = false;
>>>           object_unref(OBJECT(accel));
>>> -    } else {
>>> -        object_set_accelerator_compat_props(acc->compat_props);
>>>       }
>>>       return ret;
>>>   }
>>> diff --git a/system/vl.c b/system/vl.c
>>> index b94a6b9..bca2292 100644
>>> --- a/system/vl.c
>>> +++ b/system/vl.c
>>> @@ -2346,6 +2346,7 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>>>           goto bad;
>>>       }
>>> +    object_set_accelerator_compat_props(ac->compat_props);
>>>       acs->accel = accel;
>>>       return 1;
>>> @@ -3728,29 +3729,14 @@ void qemu_init(int argc, char **argv)
>>>       parse_memory_options();
>>>       qemu_create_machine(machine_opts_dict);
>>> -
>>> -    suspend_mux_open();
>>> -
>>> -    qemu_disable_default_devices();
>>> -    qemu_setup_display();
>>> -    qemu_create_default_devices();
>>> -    qemu_create_early_backends();
>>> -
>>>       qemu_apply_legacy_machine_options(machine_opts_dict);
>>>       qemu_apply_machine_options(machine_opts_dict);
>>>       qobject_unref(machine_opts_dict);
>>> -    phase_advance(PHASE_MACHINE_CREATED);
>>> -    /*
>>> -     * Note: uses machine properties such as kernel-irqchip, must run
>>> -     * after qemu_apply_machine_options.
>>> -     */
>>>       accel = configure_accelerators(argv[0]);
>>> -    create_accelerator(accel);
>>> -    phase_advance(PHASE_ACCEL_CREATED);
>>>       /*
>>> -     * Beware, QOM objects created before this point miss global and
>>> +     * QOM objects created after this point see all global and
>>>        * compat properties.
>>>        *
>>>        * Global properties get set up by qdev_prop_register_global(),
>>> @@ -3765,6 +3751,22 @@ void qemu_init(int argc, char **argv)
>>>        * called from do_configure_accelerator().
>>>        */
>>> +    suspend_mux_open();
>>> +
>>> +    qemu_disable_default_devices();
>>> +    qemu_setup_display();
>>> +    qemu_create_default_devices();
>>> +    qemu_create_early_backends();
>>> +
>>> +    phase_advance(PHASE_MACHINE_CREATED);
>>> +
>>> +    /*
>>> +     * Note: uses machine properties such as kernel-irqchip, must run
>>> +     * after qemu_apply_machine_options.
>>> +     */
>>> +    create_accelerator(accel);
>>> +    phase_advance(PHASE_ACCEL_CREATED);
>>> +
>>>       machine_class = MACHINE_GET_CLASS(current_machine);
>>>       if (!qtest_enabled() && machine_class->deprecation_reason) {
>>>           warn_report("Machine type '%s' is deprecated: %s",
>>
>> Hi Steve,
>>
>> after this commit:
>>
>> $ QTEST_QEMU_BINARY=./qemu-system-aarch64 ./tests/qtest/xlnx-can-test
>> # random seed: R02Saf9b44f2d88dd417052905692ee79981
>> 1..5
>> # Start of aarch64 tests
>> # Start of net tests
>> # Start of can tests
>> # starting QEMU: exec ./qemu-system-aarch64 -qtest unix:/tmp/qtest-2396.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-2396.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -machine xlnx-zcu102 -object can-bus,id=canbus -machine canbus0=canbus -machine canbus1=canbus -accel qtest
>> qemu-system-aarch64: Device 'canbus' not found
>>
>> I tried briefly to figure out what the issue is, but I don't really
>> understand the dependencies involved. Hope you can tell us.
> 
> Thanks! I forgot to define the preinit method for the qtest accelerator in patch 1.
> I'll verify that fixes the problem and send you a one-off patch if you want to continue
> testing.

Actually that is not a problem.  qtest qtest_init_accel does nothing, so preinit will do
nothing, so it is OK to not define preinit.

Still looking.

- Steve