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

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

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.

- Steve