On Thu, 2013-07-25 at 12:08 +0200, Igor Mammedov wrote: > On Thu, 25 Jul 2013 17:49:11 +0800 > Chen Fan <chen.fan.f...@cn.fujitsu.com> wrote: > > > v1-v2: Change cpu_apic_realize to post_vcpu_init. > > > > When useing x86_64-softmmu --enable-kvm boot qemu, cpu-add command fails to > > add a vcpu, > ^^^ using > > there show (KVM: setting VAPIC address failed). > > > > The reason is that we use an uninitialized cpu->kvm-fd to ioctl. > > so we move realizing apic to the back of qemu_init_vcpu. > Could you describe here why patch fixes problem and how it's different > from non hotplug case. So anyone touching this code in future > could know what dependencies to check. > Ok, I will resend this patch. Thanks
> > > > Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com> > > --- > > include/qom/cpu.h | 2 ++ > > qom/cpu.c | 13 +++++++++++++ > > target-i386/cpu.c | 10 ++++------ > > 3 files changed, 19 insertions(+), 6 deletions(-) > > > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > > index daf1835..4b16385 100644 > > --- a/include/qom/cpu.h > > +++ b/include/qom/cpu.h > > @@ -80,6 +80,7 @@ struct TranslationBlock; > > * @synchronize_from_tb: Callback for synchronizing state from a TCG > > * #TranslationBlock. > > * @get_phys_page_debug: Callback for obtaining a physical address. > > + * @post_vcpu_init: Callback for doing some extra initialization. > > * @vmsd: State description for migration. > > * > > * Represents a CPU family or model. > > @@ -108,6 +109,7 @@ typedef struct CPUClass { > > void (*set_pc)(CPUState *cpu, vaddr value); > > void (*synchronize_from_tb)(CPUState *cpu, struct TranslationBlock > > *tb); > > hwaddr (*get_phys_page_debug)(CPUState *cpu, vaddr addr); > > + void (*post_vcpu_init)(CPUState *cpu, Error **errp); > > > > const struct VMStateDescription *vmsd; > > int (*write_elf64_note)(WriteCoreDumpFunction f, CPUState *cpu, > > diff --git a/qom/cpu.c b/qom/cpu.c > > index 5c45ab5..28f63b7 100644 > > --- a/qom/cpu.c > > +++ b/qom/cpu.c > > @@ -213,12 +213,25 @@ static ObjectClass *cpu_common_class_by_name(const > > char *cpu_model) > > return NULL; > > } > > > > +static void post_vcpu_init(CPUState *cpu, Error **errp) > > +{ > > + CPUClass *cc = CPU_GET_CLASS(cpu); > > + if (cc->post_vcpu_init != NULL) { > > + (*cc->post_vcpu_init)(cpu, errp); > > + } > > +} > > + > > static void cpu_common_realizefn(DeviceState *dev, Error **errp) > > { > > CPUState *cpu = CPU(dev); > > > > qemu_init_vcpu(cpu); > > > > + post_vcpu_init(cpu, errp); > > + if (error_is_set(errp)) { > > + return; > > + } > > + > > if (dev->hotplugged) { > > cpu_synchronize_post_init(cpu); > > notifier_list_notify(&cpu_added_notifiers, dev); > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index cd350cb..d51ab8b 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -2311,8 +2311,9 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error > > **errp) > > apic->cpu = cpu; > > } > > > > -static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) > > +static void x86_cpu_apic_realize(CPUState *s, Error **errp) > > { > > + X86CPU *cpu = X86_CPU(s); > > CPUX86State *env = &cpu->env; > > > > if (env->apic_state == NULL) { > > @@ -2326,7 +2327,7 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error > > **errp) > > } > > } > > #else > > -static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) > > +static void x86_cpu_apic_realize(CPUState *s, Error **errp) > > { > > } > > #endif > > @@ -2388,10 +2389,6 @@ static void x86_cpu_realizefn(DeviceState *dev, > > Error **errp) > > > > mce_init(cpu); > > > > - x86_cpu_apic_realize(cpu, &local_err); > > - if (local_err != NULL) { > > - goto out; > > - } > > cpu_reset(CPU(cpu)); > > > > xcc->parent_realize(dev, &local_err); > > @@ -2540,6 +2537,7 @@ static void x86_cpu_common_class_init(ObjectClass > > *oc, void *data) > > cc->synchronize_from_tb = x86_cpu_synchronize_from_tb; > > cc->get_arch_id = x86_cpu_get_arch_id; > > cc->get_paging_enabled = x86_cpu_get_paging_enabled; > > + cc->post_vcpu_init = x86_cpu_apic_realize; > > #ifndef CONFIG_USER_ONLY > > cc->get_memory_mapping = x86_cpu_get_memory_mapping; > > cc->get_phys_page_debug = x86_cpu_get_phys_page_debug; > >