From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34598) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjSU0-00044o-DE for qemu-devel@nongnu.org; Fri, 14 Dec 2012 05:27:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TjSTt-00070u-Cu for qemu-devel@nongnu.org; Fri, 14 Dec 2012 05:27:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:24488) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjSTt-00070j-53 for qemu-devel@nongnu.org; Fri, 14 Dec 2012 05:27:01 -0500 Date: Fri, 14 Dec 2012 11:26:55 +0100 From: Igor Mammedov Message-ID: <20121214112655.797afc33@thinkpad.mammed.net> In-Reply-To: <1355336183-18858-2-git-send-email-ehabkost@redhat.com> References: <1355336183-18858-1-git-send-email-ehabkost@redhat.com> <1355336183-18858-2-git-send-email-ehabkost@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] target-i386: move CPU object creation to cpu.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Andreas =?UTF-8?B?RsOkcmJlcg==?= On Wed, 12 Dec 2012 16:16:22 -0200 Eduardo Habkost 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 > --- > 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