[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] target-i386: move CPU object creation to cp
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] target-i386: move CPU object creation to cpu.c |
Date: |
Fri, 14 Dec 2012 10:42:52 -0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Dec 14, 2012 at 11:26:55AM +0100, Igor Mammedov wrote:
> On Wed, 12 Dec 2012 16:16:22 -0200
> Eduardo Habkost <address@hidden> 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 <address@hidden>
> > ---
> > 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'
Thanks. I'll change it.
>
> > + */
> > +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.
I believe we want to call cpu_x86_parse_featurestr() _after_ the CPU
object is created, don't we? Because one day the feature string is going
be used to set properties in the CPU object.
But we can move object_new() after g_strsplit() (because evantually the
CPU object creation is going to use the first part of cpu_model to
lookup the class name).
>
> > + 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
--
Eduardo