On Mon, 10 Oct 2016 13:58:25 -0300
Eduardo Habkost <ehabkost@xxxxxxxxxx> wrote:
> On Mon, Oct 10, 2016 at 02:25:32PM +0200, Igor Mammedov wrote:
> > On Fri, 7 Oct 2016 17:29:01 -0300
> > Eduardo Habkost <ehabkost@xxxxxxxxxx> 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 <ehabkost@xxxxxxxxxx>
> > > ---
> > > 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 <pbonzini@xxxxxxxxxx>
> > >
> > > 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
>
> Oops, leftover from v5. Thanks for spotting that! Fixup below.
>
> Signed-off-by: Eduardo Habkost <ehabkost@xxxxxxxxxx>
Reviewed-by: Igor Mammedov <imammedo@xxxxxxxxxx>
> ---
> target-i386/cpu.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 23cc19b..4294746 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -3103,8 +3103,6 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> goto out;
> }
>
> - x86_cpu_filter_features(cpu);
> -
> if (x86_cpu_filter_features(cpu) &&
> (cpu->check_cpuid || cpu->enforce_cpuid)) {
> x86_cpu_report_filtered_features(cpu);
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list