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

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

On 10/21/2024 11:19 AM, Peter Xu wrote:
> On Thu, Oct 17, 2024 at 08:14:05AM -0700, Steve Sistare wrote:
>> 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);
> 
> This is the probe/preinit iterator, might be good to keep it simple to only
> make the decision of choosing one accel, then move this line over to
> configure_accelerators() at the end.

It's actually simpler to leave it here.  Hoisting object_set_accelerator_compat_props
would require extra code to derive the ac parameter.

- Steve

>>       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",
>> -- 
>> 1.8.3.1
>>
>