| Message ID | 1365172636-28628-3-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
Reviewed-by: liguang <lig.fnst@cn.fujitsu.com> 在 2013-04-05五的 16:36 +0200，Igor Mammedov写道： > When APIC is hotplugged during CPU hotplug, device_set_realized() > calls device_reset() on it. And if QEMU runs in KVM mode, following > call chain will fail: > apic_reset_common() > -> kvm_apic_vapic_base_update() > -> kvm_vcpu_ioctl(cpu->kvm_fd,...) > due to cpu->kvm_fd not being initialized yet. > > cpu->kvm_fd is initialized during qemu_init_vcpu() call but x86_cpu_apic_init() > can't be moved after it because kvm_init_vcpu() -> kvm_arch_reset_vcpu() > relies on APIC to determine if CPU is BSP for setting initial env->mp_state. > > So split APIC device creation from its initialization and realize APIC > after CPU is created, when it's safe to call APIC's reset method. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v2: > * s/x86_cpu_apic_init()/x86_cpu_apic_realize()/ > --- > target-i386/cpu.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 8b348d0..41f0f47 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -2050,9 +2050,8 @@ static void mce_init(X86CPU *cpu) > } > > #ifndef CONFIG_USER_ONLY > -static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) > +static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) > { > - static int apic_mapped; > CPUX86State *env = &cpu->env; > APICCommonState *apic; > const char *apic_type = "apic"; > @@ -2075,6 +2074,16 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) > /* TODO: convert to link<> */ > apic = APIC_COMMON(env->apic_state); > apic->cpu = cpu; > +} > + > +static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) > +{ > + CPUX86State *env = &cpu->env; > + static int apic_mapped; > + > + if (env->apic_state == NULL) { > + return; > + } > > if (qdev_init(env->apic_state)) { > error_setg(errp, "APIC device '%s' could not be initialized", > @@ -2092,6 +2101,10 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) > apic_mapped = 1; > } > } > +#else > +static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) > +{ > +} > #endif > > static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > @@ -2142,7 +2155,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > qemu_register_reset(x86_cpu_machine_reset_cb, cpu); > > if (cpu->env.cpuid_features & CPUID_APIC || smp_cpus > 1) { > - x86_cpu_apic_init(cpu, &local_err); > + x86_cpu_apic_create(cpu, &local_err); > if (local_err != NULL) { > goto out; > } > @@ -2151,6 +2164,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > > mce_init(cpu); > qemu_init_vcpu(&cpu->env); > + > + x86_cpu_apic_realize(cpu, &local_err); > + if (local_err != NULL) { > + goto out; > + } > cpu_reset(CPU(cpu)); > > xcc->parent_realize(dev, &local_err);
On Fri, Apr 05, 2013 at 04:36:54PM +0200, Igor Mammedov wrote: > When APIC is hotplugged during CPU hotplug, device_set_realized() > calls device_reset() on it. And if QEMU runs in KVM mode, following > call chain will fail: > apic_reset_common() > -> kvm_apic_vapic_base_update() > -> kvm_vcpu_ioctl(cpu->kvm_fd,...) > due to cpu->kvm_fd not being initialized yet. > > cpu->kvm_fd is initialized during qemu_init_vcpu() call but x86_cpu_apic_init() > can't be moved after it because kvm_init_vcpu() -> kvm_arch_reset_vcpu() > relies on APIC to determine if CPU is BSP for setting initial env->mp_state. > > So split APIC device creation from its initialization and realize APIC > after CPU is created, when it's safe to call APIC's reset method. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > --- > v2: > * s/x86_cpu_apic_init()/x86_cpu_apic_realize()/ > --- > target-i386/cpu.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 8b348d0..41f0f47 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -2050,9 +2050,8 @@ static void mce_init(X86CPU *cpu) > } > > #ifndef CONFIG_USER_ONLY > -static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) > +static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) > { > - static int apic_mapped; > CPUX86State *env = &cpu->env; > APICCommonState *apic; > const char *apic_type = "apic"; > @@ -2075,6 +2074,16 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) > /* TODO: convert to link<> */ > apic = APIC_COMMON(env->apic_state); > apic->cpu = cpu; > +} > + > +static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) > +{ > + CPUX86State *env = &cpu->env; > + static int apic_mapped; > + > + if (env->apic_state == NULL) { > + return; > + } > > if (qdev_init(env->apic_state)) { > error_setg(errp, "APIC device '%s' could not be initialized", > @@ -2092,6 +2101,10 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) > apic_mapped = 1; > } > } > +#else > +static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) > +{ > +} > #endif > > static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > @@ -2142,7 +2155,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > qemu_register_reset(x86_cpu_machine_reset_cb, cpu); > > if (cpu->env.cpuid_features & CPUID_APIC || smp_cpus > 1) { > - x86_cpu_apic_init(cpu, &local_err); > + x86_cpu_apic_create(cpu, &local_err); > if (local_err != NULL) { > goto out; > } > @@ -2151,6 +2164,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > > mce_init(cpu); > qemu_init_vcpu(&cpu->env); > + > + x86_cpu_apic_realize(cpu, &local_err); > + if (local_err != NULL) { > + goto out; > + } > cpu_reset(CPU(cpu)); > > xcc->parent_realize(dev, &local_err); > -- > 1.8.1.4 > >
Am 05.04.2013 16:36, schrieb Igor Mammedov: > When APIC is hotplugged during CPU hotplug, device_set_realized() > calls device_reset() on it. And if QEMU runs in KVM mode, following > call chain will fail: > apic_reset_common() > -> kvm_apic_vapic_base_update() > -> kvm_vcpu_ioctl(cpu->kvm_fd,...) > due to cpu->kvm_fd not being initialized yet. > > cpu->kvm_fd is initialized during qemu_init_vcpu() call but x86_cpu_apic_init() > can't be moved after it because kvm_init_vcpu() -> kvm_arch_reset_vcpu() > relies on APIC to determine if CPU is BSP for setting initial env->mp_state. > > So split APIC device creation from its initialization and realize APIC > after CPU is created, when it's safe to call APIC's reset method. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v2: > * s/x86_cpu_apic_init()/x86_cpu_apic_realize()/ Thanks, applied to qom-cpu (dropping "call" from commit message to squeeze it into 76 chars): https://github.com/afaerber/qemu-cpu/commits/qom-cpu Andreas
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8b348d0..41f0f47 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2050,9 +2050,8 @@ static void mce_init(X86CPU *cpu) } #ifndef CONFIG_USER_ONLY -static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) +static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) { - static int apic_mapped; CPUX86State *env = &cpu->env; APICCommonState *apic; const char *apic_type = "apic"; @@ -2075,6 +2074,16 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) /* TODO: convert to link<> */ apic = APIC_COMMON(env->apic_state); apic->cpu = cpu; +} + +static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) +{ + CPUX86State *env = &cpu->env; + static int apic_mapped; + + if (env->apic_state == NULL) { + return; + } if (qdev_init(env->apic_state)) { error_setg(errp, "APIC device '%s' could not be initialized", @@ -2092,6 +2101,10 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) apic_mapped = 1; } } +#else +static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp) +{ +} #endif static void x86_cpu_realizefn(DeviceState *dev, Error **errp) @@ -2142,7 +2155,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) qemu_register_reset(x86_cpu_machine_reset_cb, cpu); if (cpu->env.cpuid_features & CPUID_APIC || smp_cpus > 1) { - x86_cpu_apic_init(cpu, &local_err); + x86_cpu_apic_create(cpu, &local_err); if (local_err != NULL) { goto out; } @@ -2151,6 +2164,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) mce_init(cpu); qemu_init_vcpu(&cpu->env); + + x86_cpu_apic_realize(cpu, &local_err); + if (local_err != NULL) { + goto out; + } cpu_reset(CPU(cpu)); xcc->parent_realize(dev, &local_err);
When APIC is hotplugged during CPU hotplug, device_set_realized() calls device_reset() on it. And if QEMU runs in KVM mode, following call chain will fail: apic_reset_common() -> kvm_apic_vapic_base_update() -> kvm_vcpu_ioctl(cpu->kvm_fd,...) due to cpu->kvm_fd not being initialized yet. cpu->kvm_fd is initialized during qemu_init_vcpu() call but x86_cpu_apic_init() can't be moved after it because kvm_init_vcpu() -> kvm_arch_reset_vcpu() relies on APIC to determine if CPU is BSP for setting initial env->mp_state. So split APIC device creation from its initialization and realize APIC after CPU is created, when it's safe to call APIC's reset method. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- v2: * s/x86_cpu_apic_init()/x86_cpu_apic_realize()/ --- target-i386/cpu.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)