Re: [Qemu-devel] [PATCH for-1.2] cpus: Register reset callback centrally

[Andreas Färber]

Am 01.08.2012 23:13, schrieb Andreas Färber:
> Despite repeated protest commit 65dee38052597b6285eb208125369f01b29ba6c1
> (target-i386: move cpu_reset and reset callback to cpu.c) moved
> registration of a reset callback from hw/pc.c to target-i386/cpu.c
> while all other CPU reset handlers were still registered from machines.
> 
> Instead, improve the situation by registering the callback in
> qemu_init_vcpu(). Now new machines or CPUs no longer need to register
> their own callbacks, and we can revert the code in target-i386/cpu.c.
> 
> Signed-off-by: Andreas Färber <address@hidden>
> Cc: Anthony Liguori <address@hidden>
> Cc: Igor Mammedov <address@hidden>
> ---
>  cpus.c            |    8 ++++++++
>  target-i386/cpu.c |   11 -----------
>  2 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 756e624..f717a61 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1020,6 +1020,13 @@ static void qemu_dummy_start_vcpu(CPUArchState *env)
>      }
>  }
>  
> +static void cpu_machine_reset(void *opaque)
> +{
> +    CPUState *cpu = opaque;
> +
> +    cpu_reset(cpu);
> +}
> +
>  void qemu_init_vcpu(void *_env)
>  {
>      CPUArchState *env = _env;
> @@ -1027,6 +1034,7 @@ void qemu_init_vcpu(void *_env)
>      env->nr_cores = smp_cores;
>      env->nr_threads = smp_threads;
>      env->stopped = 1;
> +    qemu_register_reset(cpu_machine_reset, ENV_GET_CPU(env));
>      if (kvm_enabled()) {
>          qemu_kvm_start_vcpu(env);
>      } else if (tcg_enabled()) {
[snip]

Note that this is safe as long as qemu_init_vcpu() happens inside
cpu_foo_init(), the current convention. I believe Igor's APIC/BSP patch
that just got committed replaced an earlier attempt to change that.
Further reset handlers would then be registered after this one and IIUC
executed in registration order so that worst case cpu_reset() would be
executed multiple times if not dropped from machines' reset callbacks.

What is missing here though is unregistration of the new reset callback
for future hot unplug. But waiting for comments before I drill some
qemu_finalize_vcpu() API callable from qom/cpu.c.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg