Message ID | 1424986952-18579-5-git-send-email-ehabkost@redhat.com |
---|---|
State | New |
Headers | show |
Am 26.02.2015 um 22:42 schrieb Eduardo Habkost: > Instead of setting APIC ID automatically when creating a X86CPU, require > the property to be set before realizing the object (which all callers of > cpu_x86_create() already do). > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target-i386/cpu-qom.h | 2 +- > target-i386/cpu.c | 7 ++++++- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h > index 4a6f48a..31a0c1e 100644 > --- a/target-i386/cpu-qom.h > +++ b/target-i386/cpu-qom.h > @@ -93,7 +93,7 @@ typedef struct X86CPU { > bool expose_kvm; > bool migratable; > bool host_features; > - uint32_t apic_id; > + int64_t apic_id; > > /* if true the CPUID code directly forward host cache leaves to the guest */ > bool cache_info_passthrough; > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index c4a96b6..9819c47 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -2786,6 +2786,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > Error *local_err = NULL; > static bool ht_warned; > > + if (cpu->apic_id < 0) { Since you were setting it immediately before setting realized=true, #ifdef CONFIG_USER_ONLY cpu->apic_id = cs->cpu_index; #else > + error_setg(errp, "apic-id property was not initialized properly"); > + return; #endif would keep any x86-specific logic out of cpu_init(). Regards, Andreas > + } > + > if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) { > env->cpuid_level = 7; > } > @@ -2929,7 +2934,7 @@ static void x86_cpu_initfn(Object *obj) > NULL, NULL, (void *)cpu->filtered_features, NULL); > > cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY; > - cpu->apic_id = x86_cpu_apic_id_from_index(cs->cpu_index); > + cpu->apic_id = -1; > > x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort); >
On Mon, Mar 02, 2015 at 06:50:44PM +0100, Andreas Färber wrote: > Am 26.02.2015 um 22:42 schrieb Eduardo Habkost: > > Instead of setting APIC ID automatically when creating a X86CPU, require > > the property to be set before realizing the object (which all callers of > > cpu_x86_create() already do). > > > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > target-i386/cpu-qom.h | 2 +- > > target-i386/cpu.c | 7 ++++++- > > 2 files changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h > > index 4a6f48a..31a0c1e 100644 > > --- a/target-i386/cpu-qom.h > > +++ b/target-i386/cpu-qom.h > > @@ -93,7 +93,7 @@ typedef struct X86CPU { > > bool expose_kvm; > > bool migratable; > > bool host_features; > > - uint32_t apic_id; > > + int64_t apic_id; > > > > /* if true the CPUID code directly forward host cache leaves to the guest */ > > bool cache_info_passthrough; > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index c4a96b6..9819c47 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -2786,6 +2786,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > > Error *local_err = NULL; > > static bool ht_warned; > > > > + if (cpu->apic_id < 0) { > > Since you were setting it immediately before setting realized=true, > > #ifdef CONFIG_USER_ONLY > cpu->apic_id = cs->cpu_index; > #else > > > + error_setg(errp, "apic-id property was not initialized properly"); > > + return; > > #endif > > would keep any x86-specific logic out of cpu_init(). My initial intention was to make a x86-specific cpu_init() to avoid #ifdefs in the code, but considering that it would be interesting to finally start using cpu_generic_init(), it would make sense to have the #ifdef on realize. Anyway, the good news is that we probably don't even need to set apic_id = cpu_index on *-user, making the #ifdef unnecessary. See my replies to Peter at: [PULL 08/11] linux-user: Check for cpu_init() errors
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index 4a6f48a..31a0c1e 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -93,7 +93,7 @@ typedef struct X86CPU { bool expose_kvm; bool migratable; bool host_features; - uint32_t apic_id; + int64_t apic_id; /* if true the CPUID code directly forward host cache leaves to the guest */ bool cache_info_passthrough; diff --git a/target-i386/cpu.c b/target-i386/cpu.c index c4a96b6..9819c47 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2786,6 +2786,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) Error *local_err = NULL; static bool ht_warned; + if (cpu->apic_id < 0) { + error_setg(errp, "apic-id property was not initialized properly"); + return; + } + if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) { env->cpuid_level = 7; } @@ -2929,7 +2934,7 @@ static void x86_cpu_initfn(Object *obj) NULL, NULL, (void *)cpu->filtered_features, NULL); cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY; - cpu->apic_id = x86_cpu_apic_id_from_index(cs->cpu_index); + cpu->apic_id = -1; x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);