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 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).