| Message ID | 8dd683d6589f7bd52bd7250df87a547cf1932186.1423821709.git.zhugh.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Feb 13, 2015 at 06:25:28PM +0800, Zhu Guihua wrote: > From: Gu Zheng <guz.fnst@cn.fujitsu.com> > > Move cpu vmstate register from cpu_exec_init into cpu_common_realizefn, > and use cc->get_arch_id as the instance id that suggested by Igor to > fix the migration issue. If you are implementing something new, please do that in a separate patch, either before or after moving the code. Makes it easier to review and easier to revert in case something goes wrong. See two additional issues below: > > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > --- > exec.c | 25 ++++++++++++++++++------- > include/qom/cpu.h | 2 ++ > qom/cpu.c | 4 ++++ > 3 files changed, 24 insertions(+), 7 deletions(-) > > diff --git a/exec.c b/exec.c > index 6dff7bc..8361591 100644 > --- a/exec.c > +++ b/exec.c > @@ -513,10 +513,26 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as) > } > #endif > > +void cpu_vmstate_register(CPUState *cpu) > +{ > + CPUClass *cc = CPU_GET_CLASS(cpu); > + int cpu_index = cc->get_arch_id(cpu) + max_cpus; Breaks linux-user build: LINK x86_64-linux-user/qemu-x86_64 exec.o: In function `cpu_vmstate_register': /home/ehabkost/rh/proj/virt/qemu/exec.c:533: undefined reference to `max_cpus' collect2: error: ld returned 1 exit status Makefile:182: recipe for target 'qemu-x86_64' failed make[1]: *** [qemu-x86_64] Error 1 Makefile:169: recipe for target 'subdir-x86_64-linux-user' failed make: *** [subdir-x86_64-linux-user] Error 2 > + int compat_index = cc->get_compat_arch_id(cpu); > + > + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > + vmstate_register_with_alias_id(NULL, cpu_index, &vmstate_cpu_common, > + cpu, compat_index, 3); > + } > + > + if (cc->vmsd != NULL) { > + vmstate_register_with_alias_id(NULL, cpu_index, cc->vmsd, > + cpu, compat_index, 3); > + } > +} > + > void cpu_exec_init(CPUArchState *env) > { > CPUState *cpu = ENV_GET_CPU(env); > - CPUClass *cc = CPU_GET_CLASS(cpu); > CPUState *some_cpu; > int cpu_index; > > @@ -539,18 +555,13 @@ void cpu_exec_init(CPUArchState *env) > #if defined(CONFIG_USER_ONLY) > cpu_list_unlock(); > #endif > - if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > - vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu); > - } > #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY) > + CPUClass *cc = CPU_GET_CLASS(cpu); > register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION, > cpu_save, cpu_load, env); > assert(cc->vmsd == NULL); > assert(qdev_get_vmsd(DEVICE(cpu)) == NULL); > #endif > - if (cc->vmsd != NULL) { > - vmstate_register(NULL, cpu_index, cc->vmsd, cpu); > - } > } > > #if defined(CONFIG_USER_ONLY) > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 2e68dd2..d0a50e2 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -565,6 +565,8 @@ void cpu_interrupt(CPUState *cpu, int mask); > > #endif /* USER_ONLY */ > > +void cpu_vmstate_register(CPUState *cpu); > + > #ifdef CONFIG_SOFTMMU > static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr, > bool is_write, bool is_exec, > diff --git a/qom/cpu.c b/qom/cpu.c > index 83d7766..8e37045 100644 > --- a/qom/cpu.c > +++ b/qom/cpu.c > @@ -302,6 +302,10 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp) > { > CPUState *cpu = CPU(dev); > > +#if !defined(CONFIG_USER_ONLY) > + cpu_vmstate_register(cpu); > +#endif CONFIG_USER_ONLY is never set on qom/cpu.c because it is target-independent code. Good news is that we already have vmstate stubs, so you shouldn't need any CONFIG_USER_ONLY ifdefs around the vmstate code (but it looks like we will need a max_cpus stub). > + > if (dev->hotplugged) { > cpu_synchronize_post_init(cpu); > cpu_resume(cpu); > -- > 1.9.3 > >
On Fri, 13 Feb 2015 18:25:28 +0800 Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote: > From: Gu Zheng <guz.fnst@cn.fujitsu.com> > > Move cpu vmstate register from cpu_exec_init into cpu_common_realizefn, > and use cc->get_arch_id as the instance id that suggested by Igor to > fix the migration issue. > > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> > Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> > --- > exec.c | 25 ++++++++++++++++++------- > include/qom/cpu.h | 2 ++ > qom/cpu.c | 4 ++++ > 3 files changed, 24 insertions(+), 7 deletions(-) > > diff --git a/exec.c b/exec.c > index 6dff7bc..8361591 100644 > --- a/exec.c > +++ b/exec.c > @@ -513,10 +513,26 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as) > } > #endif > > +void cpu_vmstate_register(CPUState *cpu) > +{ > + CPUClass *cc = CPU_GET_CLASS(cpu); > + int cpu_index = cc->get_arch_id(cpu) + max_cpus; Why do you add max_cpus here? > + int compat_index = cc->get_compat_arch_id(cpu); > + > + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > + vmstate_register_with_alias_id(NULL, cpu_index, &vmstate_cpu_common, > + cpu, compat_index, 3); > + } > + > + if (cc->vmsd != NULL) { > + vmstate_register_with_alias_id(NULL, cpu_index, cc->vmsd, > + cpu, compat_index, 3); > + } > +} > + > void cpu_exec_init(CPUArchState *env) > { > CPUState *cpu = ENV_GET_CPU(env); > - CPUClass *cc = CPU_GET_CLASS(cpu); > CPUState *some_cpu; > int cpu_index; > > @@ -539,18 +555,13 @@ void cpu_exec_init(CPUArchState *env) > #if defined(CONFIG_USER_ONLY) > cpu_list_unlock(); > #endif > - if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > - vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu); > - } > #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY) > + CPUClass *cc = CPU_GET_CLASS(cpu); > register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION, > cpu_save, cpu_load, env); > assert(cc->vmsd == NULL); > assert(qdev_get_vmsd(DEVICE(cpu)) == NULL); > #endif > - if (cc->vmsd != NULL) { > - vmstate_register(NULL, cpu_index, cc->vmsd, cpu); > - } > } > > #if defined(CONFIG_USER_ONLY) > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 2e68dd2..d0a50e2 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -565,6 +565,8 @@ void cpu_interrupt(CPUState *cpu, int mask); > > #endif /* USER_ONLY */ > > +void cpu_vmstate_register(CPUState *cpu); > + > #ifdef CONFIG_SOFTMMU > static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr, > bool is_write, bool is_exec, > diff --git a/qom/cpu.c b/qom/cpu.c > index 83d7766..8e37045 100644 > --- a/qom/cpu.c > +++ b/qom/cpu.c > @@ -302,6 +302,10 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp) > { > CPUState *cpu = CPU(dev); > > +#if !defined(CONFIG_USER_ONLY) > + cpu_vmstate_register(cpu); > +#endif > + > if (dev->hotplugged) { > cpu_synchronize_post_init(cpu); > cpu_resume(cpu);
On 03/06/2015 04:53 PM, Igor Mammedov wrote: > On Fri, 13 Feb 2015 18:25:28 +0800 > Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote: > >> From: Gu Zheng <guz.fnst@cn.fujitsu.com> >> >> Move cpu vmstate register from cpu_exec_init into cpu_common_realizefn, >> and use cc->get_arch_id as the instance id that suggested by Igor to >> fix the migration issue. >> >> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> >> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com> >> --- >> exec.c | 25 ++++++++++++++++++------- >> include/qom/cpu.h | 2 ++ >> qom/cpu.c | 4 ++++ >> 3 files changed, 24 insertions(+), 7 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index 6dff7bc..8361591 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -513,10 +513,26 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as) >> } >> #endif >> >> +void cpu_vmstate_register(CPUState *cpu) >> +{ >> + CPUClass *cc = CPU_GET_CLASS(cpu); >> + int cpu_index = cc->get_arch_id(cpu) + max_cpus; > Why do you add max_cpus here? Adding max_cpus is to avoid cpu_index equals comat_index which would cause migration fails, this reason is that find_se() always compares instance_id with se->instance_id or se->alias_id when do migration. Thanks, Chen > >> + int compat_index = cc->get_compat_arch_id(cpu); >> + >> + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { >> + vmstate_register_with_alias_id(NULL, cpu_index, &vmstate_cpu_common, >> + cpu, compat_index, 3); >> + } >> + >> + if (cc->vmsd != NULL) { >> + vmstate_register_with_alias_id(NULL, cpu_index, cc->vmsd, >> + cpu, compat_index, 3); >> + } >> +} >> + >> void cpu_exec_init(CPUArchState *env) >> { >> CPUState *cpu = ENV_GET_CPU(env); >> - CPUClass *cc = CPU_GET_CLASS(cpu); >> CPUState *some_cpu; >> int cpu_index; >> >> @@ -539,18 +555,13 @@ void cpu_exec_init(CPUArchState *env) >> #if defined(CONFIG_USER_ONLY) >> cpu_list_unlock(); >> #endif >> - if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { >> - vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu); >> - } >> #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY) >> + CPUClass *cc = CPU_GET_CLASS(cpu); >> register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION, >> cpu_save, cpu_load, env); >> assert(cc->vmsd == NULL); >> assert(qdev_get_vmsd(DEVICE(cpu)) == NULL); >> #endif >> - if (cc->vmsd != NULL) { >> - vmstate_register(NULL, cpu_index, cc->vmsd, cpu); >> - } >> } >> >> #if defined(CONFIG_USER_ONLY) >> diff --git a/include/qom/cpu.h b/include/qom/cpu.h >> index 2e68dd2..d0a50e2 100644 >> --- a/include/qom/cpu.h >> +++ b/include/qom/cpu.h >> @@ -565,6 +565,8 @@ void cpu_interrupt(CPUState *cpu, int mask); >> >> #endif /* USER_ONLY */ >> >> +void cpu_vmstate_register(CPUState *cpu); >> + >> #ifdef CONFIG_SOFTMMU >> static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr, >> bool is_write, bool is_exec, >> diff --git a/qom/cpu.c b/qom/cpu.c >> index 83d7766..8e37045 100644 >> --- a/qom/cpu.c >> +++ b/qom/cpu.c >> @@ -302,6 +302,10 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp) >> { >> CPUState *cpu = CPU(dev); >> >> +#if !defined(CONFIG_USER_ONLY) >> + cpu_vmstate_register(cpu); >> +#endif >> + >> if (dev->hotplugged) { >> cpu_synchronize_post_init(cpu); >> cpu_resume(cpu); > . >
diff --git a/exec.c b/exec.c index 6dff7bc..8361591 100644 --- a/exec.c +++ b/exec.c @@ -513,10 +513,26 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as) } #endif +void cpu_vmstate_register(CPUState *cpu) +{ + CPUClass *cc = CPU_GET_CLASS(cpu); + int cpu_index = cc->get_arch_id(cpu) + max_cpus; + int compat_index = cc->get_compat_arch_id(cpu); + + if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { + vmstate_register_with_alias_id(NULL, cpu_index, &vmstate_cpu_common, + cpu, compat_index, 3); + } + + if (cc->vmsd != NULL) { + vmstate_register_with_alias_id(NULL, cpu_index, cc->vmsd, + cpu, compat_index, 3); + } +} + void cpu_exec_init(CPUArchState *env) { CPUState *cpu = ENV_GET_CPU(env); - CPUClass *cc = CPU_GET_CLASS(cpu); CPUState *some_cpu; int cpu_index; @@ -539,18 +555,13 @@ void cpu_exec_init(CPUArchState *env) #if defined(CONFIG_USER_ONLY) cpu_list_unlock(); #endif - if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { - vmstate_register(NULL, cpu_index, &vmstate_cpu_common, cpu); - } #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY) + CPUClass *cc = CPU_GET_CLASS(cpu); register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION, cpu_save, cpu_load, env); assert(cc->vmsd == NULL); assert(qdev_get_vmsd(DEVICE(cpu)) == NULL); #endif - if (cc->vmsd != NULL) { - vmstate_register(NULL, cpu_index, cc->vmsd, cpu); - } } #if defined(CONFIG_USER_ONLY) diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 2e68dd2..d0a50e2 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -565,6 +565,8 @@ void cpu_interrupt(CPUState *cpu, int mask); #endif /* USER_ONLY */ +void cpu_vmstate_register(CPUState *cpu); + #ifdef CONFIG_SOFTMMU static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr, bool is_write, bool is_exec, diff --git a/qom/cpu.c b/qom/cpu.c index 83d7766..8e37045 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -302,6 +302,10 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp) { CPUState *cpu = CPU(dev); +#if !defined(CONFIG_USER_ONLY) + cpu_vmstate_register(cpu); +#endif + if (dev->hotplugged) { cpu_synchronize_post_init(cpu); cpu_resume(cpu);