From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41052) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btZeH-0007zJ-0Y for qemu-devel@nongnu.org; Mon, 10 Oct 2016 08:25:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btZeD-00015j-RZ for qemu-devel@nongnu.org; Mon, 10 Oct 2016 08:25:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34716) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btZeD-00015L-EW for qemu-devel@nongnu.org; Mon, 10 Oct 2016 08:25:37 -0400 Date: Mon, 10 Oct 2016 14:25:32 +0200 From: Igor Mammedov Message-ID: <20161010142532.6ee864b8@nial.brq.redhat.com> In-Reply-To: <1475872142-3986-3-git-send-email-ehabkost@redhat.com> References: <1475872142-3986-1-git-send-email-ehabkost@redhat.com> <1475872142-3986-3-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 v6 2/3] target-i386: x86_cpu_load_features() function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, dahi@linux.vnet.ibm.com, Paolo Bonzini , Jiri Denemark , Markus Armbruster , Richard Henderson , libvir-list@redhat.com On Fri, 7 Oct 2016 17:29:01 -0300 Eduardo Habkost wrote: > When probing for CPU model information, we need to reuse the code > that initializes CPUID fields, but not the remaining side-effects > of x86_cpu_realizefn(). Move that code to a separate function > that can be reused later. > > Signed-off-by: Eduardo Habkost > --- > Changes v5 -> v6: > * Move x86_cpu_filter_features() outside x86_cpu_load_features(), > as the CPU model querying API won't run > x86_cpu_filter_features() on most cases > > Changes v4 -> v5: > * Fix typo on x86_cpu_load_features() comment > Reported-by: Paolo Bonzini > > Changes series v3 -> v4: > * New patch added to series > --- > target-i386/cpu.c | 67 +++++++++++++++++++++++++++++++++++-------------------- > 1 file changed, 43 insertions(+), 24 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 1e8127b..23cc19b 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -2993,34 +2993,13 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) > env->features[FEAT_XSAVE_COMP_HI] = mask >> 32; > } > > -#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ > - (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ > - (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) > -#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ > - (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ > - (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) > -static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > +/* Load CPUID data based on configured features */ > +static void x86_cpu_load_features(X86CPU *cpu, Error **errp) > { > - CPUState *cs = CPU(dev); > - X86CPU *cpu = X86_CPU(dev); > - X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); > CPUX86State *env = &cpu->env; > - Error *local_err = NULL; > - static bool ht_warned; > FeatureWord w; > GList *l; > - > - if (xcc->kvm_required && !kvm_enabled()) { > - char *name = x86_cpu_class_get_model_name(xcc); > - error_setg(&local_err, "CPU model '%s' requires KVM", name); > - g_free(name); > - goto out; > - } > - > - if (cpu->apic_id == UNASSIGNED_APIC_ID) { > - error_setg(errp, "apic-id property was not initialized properly"); > - return; > - } > + Error *local_err = NULL; > > /*TODO: cpu->host_features incorrectly overwrites features > * set using "feat=on|off". Once we fix this, we can convert > @@ -3086,6 +3065,46 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > env->cpuid_xlevel2 = env->cpuid_min_xlevel2; > } > > +out: > + if (local_err != NULL) { > + error_propagate(errp, local_err); > + } > +} > + > +#define IS_INTEL_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_INTEL_1 && \ > + (env)->cpuid_vendor2 == CPUID_VENDOR_INTEL_2 && \ > + (env)->cpuid_vendor3 == CPUID_VENDOR_INTEL_3) > +#define IS_AMD_CPU(env) ((env)->cpuid_vendor1 == CPUID_VENDOR_AMD_1 && \ > + (env)->cpuid_vendor2 == CPUID_VENDOR_AMD_2 && \ > + (env)->cpuid_vendor3 == CPUID_VENDOR_AMD_3) > +static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > +{ > + CPUState *cs = CPU(dev); > + X86CPU *cpu = X86_CPU(dev); > + X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); > + CPUX86State *env = &cpu->env; > + Error *local_err = NULL; > + static bool ht_warned; > + > + if (xcc->kvm_required && !kvm_enabled()) { > + char *name = x86_cpu_class_get_model_name(xcc); > + error_setg(&local_err, "CPU model '%s' requires KVM", name); > + g_free(name); > + goto out; > + } > + > + if (cpu->apic_id == UNASSIGNED_APIC_ID) { > + error_setg(errp, "apic-id property was not initialized properly"); > + return; > + } > + > + x86_cpu_load_features(cpu, &local_err); > + if (local_err) { > + goto out; > + } > + > + x86_cpu_filter_features(cpu); that makes 2 invocations of ^^ inside realize, see followup line vvvv [...] > if (x86_cpu_filter_features(cpu) && > (cpu->check_cpuid || cpu->enforce_cpuid)) { > x86_cpu_report_filtered_features(cpu);