[Qemu-devel] [PATCH qom-cpu for-1.6] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState"

[Qemu-devel] [PATCH qom-cpu for-1.6] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState"

[Andreas Färber]

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

Re: [Qemu-devel] [PATCH qom-cpu for-1.6] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState"

[Igor Mammedov]

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

Re: [Qemu-devel] [PATCH qom-cpu for-1.6] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState"

[Andreas Färber]

Am 28.07.2013 08:13, schrieb Igor Mammedov:
> 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

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

I used the diff of the offending commit to determine placement of
qemu_init_vcpu() and only avoided duplicate CPU() casts, so other
targets should be fine as well.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH qom-cpu for-1.6] cpu: Partially revert "cpu: Change qemu_init_vcpu() argument to CPUState"

[Jia Liu]

On Sat, Jul 27, 2013 at 10:01 AM, 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>
> ---
>  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);
>  }


target-openrisc Tested-by: Jia Liu <proljc@gmail.com>


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