* [Qemu-devel] [PATCH v2] cpu: Correct cpu-hotplug failure
@ 2013-07-25 9:49 Chen Fan
2013-07-25 10:08 ` Igor Mammedov
0 siblings, 1 reply; 3+ messages in thread
From: Chen Fan @ 2013-07-25 9:49 UTC (permalink / raw)
To: qemu-devel; +Cc: Andreas Färber
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,
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.
Signed-off-by: Chen Fan <chen.fan.fnst@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;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] cpu: Correct cpu-hotplug failure
2013-07-25 9:49 [Qemu-devel] [PATCH v2] cpu: Correct cpu-hotplug failure Chen Fan
@ 2013-07-25 10:08 ` Igor Mammedov
2013-07-26 3:01 ` chenfan
0 siblings, 1 reply; 3+ messages in thread
From: Igor Mammedov @ 2013-07-25 10:08 UTC (permalink / raw)
To: Chen Fan; +Cc: qemu-devel, Andreas Färber
On Thu, 25 Jul 2013 17:49:11 +0800
Chen Fan <chen.fan.fnst@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.
>
> Signed-off-by: Chen Fan <chen.fan.fnst@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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] cpu: Correct cpu-hotplug failure
2013-07-25 10:08 ` Igor Mammedov
@ 2013-07-26 3:01 ` chenfan
0 siblings, 0 replies; 3+ messages in thread
From: chenfan @ 2013-07-26 3:01 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, Andreas Färber
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.fnst@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.fnst@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;
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-07-26 3:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-25 9:49 [Qemu-devel] [PATCH v2] cpu: Correct cpu-hotplug failure Chen Fan
2013-07-25 10:08 ` Igor Mammedov
2013-07-26 3:01 ` chenfan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).