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

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/17/24 17:14, 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);
>       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);

This makes it impossible to refer to a backend in a machine option from 
the command line.  While this is not done for now, it is not a 
limitation I'd like to introduce.

Could qemu_apply_machine_options() be moved after 
configure_accelerators(), and phase_advance(PHASE_MACHINE_CREATED) with it?

Paolo


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