Re: [Qemu-devel] [PATCH 1/2] target-i386: move CPU object creation to cpu.c

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 12 Dec 2012 16:16:22 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> As we will need to create the CPU object after splitting the CPU model
> string (because we're going to use different subclasses for each CPU
> model), move the CPU object creation to cpu_x86_register(), and at the
> same time rename cpu_x86_register() to cpu_x86_create().
> 
> This will also simplify the CPU creation code to a trivial
> cpu_x86_create()+cpu_x86_realize() sequence. This will be useful for
> code that have to set additional properties before cpu_x86_realize() is
> called (e.g. the PC CPU initialization code, that needs to set APIC IDs
> depending on the CPU cores/threads topology).
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c    | 16 +++++++++++++---
>  target-i386/cpu.h    |  2 +-
>  target-i386/helper.c |  9 ++-------
>  3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3b9bbfe..b242bf1 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1539,13 +1539,22 @@ static void filter_features_for_kvm(X86CPU *cpu)
>  }
>  #endif
>  
> -int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
> +/* Create and initialize a X86CPU object, based on the full CPU model string
> + * (that may include "+feature,-feature,feature=xxx" feature strings)
feature format of cpu_model string misses just 'feature'

> + */
> +X86CPU *cpu_x86_create(const char *cpu_model)
>  {
> +    X86CPU *cpu;
> +    CPUX86State *env;
>      x86_def_t def1, *def = &def1;
>      Error *error = NULL;
>      char *name, *features;
>      gchar **model_pieces;
>  
> +    cpu = X86_CPU(object_new(TYPE_X86_CPU));
Could we put this after cpu_x86_parse_featurestr(), it's really not needed
before it now and eventually we would like to move it there anyway.

> +    env = &cpu->env;
> +    env->cpu_model_str = cpu_model;
> +
>      memset(def, 0, sizeof(*def));
>  
>      model_pieces = g_strsplit(cpu_model, ",", 2);
> @@ -1578,10 +1587,11 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
>      }
>  
>      g_strfreev(model_pieces);
> -    return 0;
> +    return cpu;
>  error:
> +    object_delete(OBJECT(cpu));
>      g_strfreev(model_pieces);
> -    return -1;
> +    return NULL;
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 386c4f6..3ebaae9 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -980,7 +980,7 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
>  void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>                     uint32_t *eax, uint32_t *ebx,
>                     uint32_t *ecx, uint32_t *edx);
> -int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
> +X86CPU *cpu_x86_create(const char *cpu_model);
>  void cpu_clear_apic_feature(CPUX86State *env);
>  void host_cpuid(uint32_t function, uint32_t count,
>                  uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index bf206cf..23af4a8 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1243,15 +1243,10 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
>  X86CPU *cpu_x86_init(const char *cpu_model)
>  {
>      X86CPU *cpu;
> -    CPUX86State *env;
>      Error *error = NULL;
>  
> -    cpu = X86_CPU(object_new(TYPE_X86_CPU));
> -    env = &cpu->env;
> -    env->cpu_model_str = cpu_model;
> -
> -    if (cpu_x86_register(cpu, cpu_model) < 0) {
> -        object_delete(OBJECT(cpu));
> +    cpu = cpu_x86_create(cpu_model);
> +    if (!cpu) {
>          return NULL;
>      }
>  
> -- 
> 1.7.11.7
> 


-- 
Regards,
  Igor