From: Igor Mammedov <imammedo@redhat.com>
To: "Andreas Färber" <afaerber@suse.de>
Cc: Peter Maydell <peter.maydell@linaro.org>,
Jia Liu <proljc@gmail.com>, Anthony Green <green@moxielogic.com>,
qemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>,
Blue Swirl <blauwirbel@gmail.com>,
Max Filippov <jcmvbkbc@gmail.com>,
Michael Walle <michael@walle.cc>,
"open list:PowerPC" <qemu-ppc@nongnu.org>,
Paul Brook <paul@codesourcery.com>,
chen.fan.fnst@cn.fujitsu.com,
"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
Guan Xuetao <gxt@mprc.pku.edu.cn>,
Aurelien Jarno <aurelien@aurel32.net>,
Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH qom-cpu for-1.6] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState"
Date: Sun, 28 Jul 2013 08:13:09 +0200 [thread overview]
Message-ID: <20130728081309.3f904480@thinkpad> (raw)
In-Reply-To: <1374890510-5479-1-git-send-email-afaerber@suse.de>
On Sat, 27 Jul 2013 04:01:50 +0200
Andreas Färber <afaerber@suse.de> wrote:
> Commit c643bed99 moved qemu_init_vcpu() calls to common CPUState code.
> This causes x86 cpu-add to fail with "KVM: setting VAPIC address failed".
>
> The reason for the failure is that CPUClass::kvm_fd is not yet
> initialized in the following call graph:
> ->x86_cpu_realizefn
> ->x86_cpu_apic_realize
> ->qdev_init
> ->device_set_realized
> ->device_reset (hotplugged == 1)
> ->apic_reset_common
> ->vapic_base_update
> ->kvm_apic_vapic_base_update
> This causes attempted KVM vCPU ioctls to fail.
>
> By contrast, in the non-hotplug case the APIC is reset much later, when
> the vCPU is already initialized.
>
> As a quick and safe solution, move the qemu_init_vcpu() call back into
> the targets' realize functions.
>
> Reported-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
ACK for target-i386
> ---
> qom/cpu.c | 2 --
> target-alpha/cpu.c | 3 +++
> target-arm/cpu.c | 4 +++-
> target-cris/cpu.c | 5 +++--
> target-i386/cpu.c | 4 +++-
> target-lm32/cpu.c | 6 ++++--
> target-m68k/cpu.c | 4 +++-
> target-microblaze/cpu.c | 5 +++--
> target-mips/cpu.c | 5 +++--
> target-moxie/cpu.c | 5 +++--
> target-openrisc/cpu.c | 5 +++--
> target-ppc/translate_init.c | 2 ++
> target-s390x/cpu.c | 5 +++--
> target-sh4/cpu.c | 5 +++--
> target-sparc/cpu.c | 2 ++
> target-unicore32/cpu.c | 2 ++
> target-xtensa/cpu.c | 2 ++
> 17 files changed, 45 insertions(+), 21 deletions(-)
>
> diff --git a/qom/cpu.c b/qom/cpu.c
> index dbc9fb6..aa95108 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -228,8 +228,6 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp)
> {
> CPUState *cpu = CPU(dev);
>
> - qemu_init_vcpu(cpu);
> -
> if (dev->hotplugged) {
> cpu_synchronize_post_init(cpu);
> notifier_list_notify(&cpu_added_notifiers, dev);
> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
> index 64c70bc..cfad2ea 100644
> --- a/target-alpha/cpu.c
> +++ b/target-alpha/cpu.c
> @@ -33,8 +33,11 @@ static void alpha_cpu_set_pc(CPUState *cs, vaddr value)
>
> static void alpha_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> + CPUState *cs = CPU(dev);
> AlphaCPUClass *acc = ALPHA_CPU_GET_CLASS(dev);
>
> + qemu_init_vcpu(cs);
> +
> acc->parent_realize(dev, errp);
> }
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 87d35c6..5a7566b 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -159,6 +159,7 @@ static void arm_cpu_finalizefn(Object *obj)
>
> static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> + CPUState *cs = CPU(dev);
> ARMCPU *cpu = ARM_CPU(dev);
> ARMCPUClass *acc = ARM_CPU_GET_CLASS(dev);
> CPUARMState *env = &cpu->env;
> @@ -214,7 +215,8 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>
> init_cpreg_list(cpu);
>
> - cpu_reset(CPU(cpu));
> + cpu_reset(cs);
> + qemu_init_vcpu(cs);
>
> acc->parent_realize(dev, errp);
> }
> diff --git a/target-cris/cpu.c b/target-cris/cpu.c
> index 45f2d6b..44301a4 100644
> --- a/target-cris/cpu.c
> +++ b/target-cris/cpu.c
> @@ -137,10 +137,11 @@ void cris_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>
> static void cris_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> - CRISCPU *cpu = CRIS_CPU(dev);
> + CPUState *cs = CPU(dev);
> CRISCPUClass *ccc = CRIS_CPU_GET_CLASS(dev);
>
> - cpu_reset(CPU(cpu));
> + cpu_reset(cs);
> + qemu_init_vcpu(cs);
>
> ccc->parent_realize(dev, errp);
> }
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 2b59b7d..df2fb1b 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2333,6 +2333,7 @@ static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)
>
> static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> + CPUState *cs = CPU(dev);
> X86CPU *cpu = X86_CPU(dev);
> X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
> CPUX86State *env = &cpu->env;
> @@ -2387,12 +2388,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> #endif
>
> mce_init(cpu);
> + qemu_init_vcpu(cs);
>
> x86_cpu_apic_realize(cpu, &local_err);
> if (local_err != NULL) {
> goto out;
> }
> - cpu_reset(CPU(cpu));
> + cpu_reset(cs);
>
> xcc->parent_realize(dev, &local_err);
> out:
> diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
> index 962d553..869878c 100644
> --- a/target-lm32/cpu.c
> +++ b/target-lm32/cpu.c
> @@ -46,10 +46,12 @@ static void lm32_cpu_reset(CPUState *s)
>
> static void lm32_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> - LM32CPU *cpu = LM32_CPU(dev);
> + CPUState *cs = CPU(dev);
> LM32CPUClass *lcc = LM32_CPU_GET_CLASS(dev);
>
> - cpu_reset(CPU(cpu));
> + cpu_reset(cs);
> +
> + qemu_init_vcpu(cs);
>
> lcc->parent_realize(dev, errp);
> }
> diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
> index c0bcb0d..008d8db 100644
> --- a/target-m68k/cpu.c
> +++ b/target-m68k/cpu.c
> @@ -143,12 +143,14 @@ static const M68kCPUInfo m68k_cpus[] = {
>
> static void m68k_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> + CPUState *cs = CPU(dev);
> M68kCPU *cpu = M68K_CPU(dev);
> M68kCPUClass *mcc = M68K_CPU_GET_CLASS(dev);
>
> m68k_cpu_init_gdb(cpu);
>
> - cpu_reset(CPU(cpu));
> + cpu_reset(cs);
> + qemu_init_vcpu(cs);
>
> mcc->parent_realize(dev, errp);
> }
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index c75d1bd..0ef9aa4 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -90,10 +90,11 @@ static void mb_cpu_reset(CPUState *s)
>
> static void mb_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> - MicroBlazeCPU *cpu = MICROBLAZE_CPU(dev);
> + CPUState *cs = CPU(dev);
> MicroBlazeCPUClass *mcc = MICROBLAZE_CPU_GET_CLASS(dev);
>
> - cpu_reset(CPU(cpu));
> + cpu_reset(cs);
> + qemu_init_vcpu(cs);
>
> mcc->parent_realize(dev, errp);
> }
> diff --git a/target-mips/cpu.c b/target-mips/cpu.c
> index f81f9e9..9dd47e8 100644
> --- a/target-mips/cpu.c
> +++ b/target-mips/cpu.c
> @@ -62,10 +62,11 @@ static void mips_cpu_reset(CPUState *s)
>
> static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> - MIPSCPU *cpu = MIPS_CPU(dev);
> + CPUState *cs = CPU(dev);
> MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(dev);
>
> - cpu_reset(CPU(cpu));
> + cpu_reset(cs);
> + qemu_init_vcpu(cs);
>
> mcc->parent_realize(dev, errp);
> }
> diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
> index 6550be5..d97a091 100644
> --- a/target-moxie/cpu.c
> +++ b/target-moxie/cpu.c
> @@ -45,10 +45,11 @@ static void moxie_cpu_reset(CPUState *s)
>
> static void moxie_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> - MoxieCPU *cpu = MOXIE_CPU(dev);
> + CPUState *cs = CPU(dev);
> MoxieCPUClass *mcc = MOXIE_CPU_GET_CLASS(dev);
>
> - cpu_reset(CPU(cpu));
> + qemu_init_vcpu(cs);
> + cpu_reset(cs);
>
> mcc->parent_realize(dev, errp);
> }
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> index aa269fb..075f00a 100644
> --- a/target-openrisc/cpu.c
> +++ b/target-openrisc/cpu.c
> @@ -66,10 +66,11 @@ static inline void set_feature(OpenRISCCPU *cpu, int feature)
>
> static void openrisc_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> - OpenRISCCPU *cpu = OPENRISC_CPU(dev);
> + CPUState *cs = CPU(dev);
> OpenRISCCPUClass *occ = OPENRISC_CPU_GET_CLASS(dev);
>
> - cpu_reset(CPU(cpu));
> + qemu_init_vcpu(cs);
> + cpu_reset(cs);
>
> occ->parent_realize(dev, errp);
> }
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 8215946..3c81798 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7861,6 +7861,8 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
> 34, "power-spe.xml", 0);
> }
>
> + qemu_init_vcpu(cs);
> +
> pcc->parent_realize(dev, errp);
>
> #if defined(PPC_DUMP_CPU)
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 1d16da3..9b82495 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -101,10 +101,11 @@ static void s390_cpu_machine_reset_cb(void *opaque)
>
> static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> - S390CPU *cpu = S390_CPU(dev);
> + CPUState *cs = CPU(dev);
> S390CPUClass *scc = S390_CPU_GET_CLASS(dev);
>
> - cpu_reset(CPU(cpu));
> + qemu_init_vcpu(cs);
> + cpu_reset(cs);
>
> scc->parent_realize(dev, errp);
> }
> diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
> index bda3c51..34b2b57 100644
> --- a/target-sh4/cpu.c
> +++ b/target-sh4/cpu.c
> @@ -240,10 +240,11 @@ static const TypeInfo sh7785_type_info = {
>
> static void superh_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> - SuperHCPU *cpu = SUPERH_CPU(dev);
> + CPUState *cs = CPU(dev);
> SuperHCPUClass *scc = SUPERH_CPU_GET_CLASS(dev);
>
> - cpu_reset(CPU(cpu));
> + cpu_reset(cs);
> + qemu_init_vcpu(cs);
>
> scc->parent_realize(dev, errp);
> }
> diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
> index c7b4a90..47ce60d 100644
> --- a/target-sparc/cpu.c
> +++ b/target-sparc/cpu.c
> @@ -743,6 +743,8 @@ static void sparc_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> SPARCCPUClass *scc = SPARC_CPU_GET_CLASS(dev);
>
> + qemu_init_vcpu(CPU(dev));
> +
> scc->parent_realize(dev, errp);
> }
>
> diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
> index 46813e5..3f78208 100644
> --- a/target-unicore32/cpu.c
> +++ b/target-unicore32/cpu.c
> @@ -92,6 +92,8 @@ static void uc32_cpu_realizefn(DeviceState *dev, Error **errp)
> {
> UniCore32CPUClass *ucc = UNICORE32_CPU_GET_CLASS(dev);
>
> + qemu_init_vcpu(CPU(dev));
> +
> ucc->parent_realize(dev, errp);
> }
>
> diff --git a/target-xtensa/cpu.c b/target-xtensa/cpu.c
> index e966aa0..c19d17a 100644
> --- a/target-xtensa/cpu.c
> +++ b/target-xtensa/cpu.c
> @@ -90,6 +90,8 @@ static void xtensa_cpu_realizefn(DeviceState *dev, Error **errp)
>
> cs->gdb_num_regs = xcc->config->gdb_regmap.num_regs;
>
> + qemu_init_vcpu(cs);
> +
> xcc->parent_realize(dev, errp);
> }
>
> --
> 1.8.1.4
>
>
--
Regards,
Igor
next prev parent reply other threads:[~2013-07-28 6:13 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-27 2:01 [Qemu-devel] [PATCH qom-cpu for-1.6] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState" Andreas Färber
2013-07-28 6:13 ` Igor Mammedov [this message]
2013-07-28 11:20 ` Andreas Färber
2013-07-28 12:31 ` Jia Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130728081309.3f904480@thinkpad \
--to=imammedo@redhat.com \
--cc=afaerber@suse.de \
--cc=agraf@suse.de \
--cc=aurelien@aurel32.net \
--cc=blauwirbel@gmail.com \
--cc=chen.fan.fnst@cn.fujitsu.com \
--cc=edgar.iglesias@gmail.com \
--cc=green@moxielogic.com \
--cc=gxt@mprc.pku.edu.cn \
--cc=jcmvbkbc@gmail.com \
--cc=michael@walle.cc \
--cc=paul@codesourcery.com \
--cc=peter.maydell@linaro.org \
--cc=proljc@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=rth@twiddle.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.