[Qemu-devel] [PATCH uq/master v2 0/2] correctly reset the CPU on INIT interrupts

[Qemu-devel] [PATCH uq/master v2 0/2] correctly reset the CPU on INIT interrupts

[Paolo Bonzini]

These patches finally implement INIT entirely in userspace.  The problem
here was that the CPU was being reset after kvm_arch_reset_vcpu is called.
This made it harder to hook into the reset process and put APs into
KVM_MP_STATE_INIT_RECEIVED state (instead of KVM_MP_STATE_UNINITIALIZED
which is the state after a system reset).

In this series, patch 1 removes the kvm_arch_reset_vcpu from the generic
code, and moves it into each architecture's CPU reset callback (half of
our supported architectures do not need the callback anyway).

With this in place, patch 2 can add a similar x86-specific callback that
is used after an INIT reset.  Apart from this callback, the code for
INITs is shared entirely between the userspace irqchip and in-kernel
irqchip cases.

Paolo Bonzini (2):
  kvm: remove generic kvm_arch_reset_vcpu callback
  kvm: forward INIT signals coming from the chipset

 include/sysemu/kvm.h   |  2 --
 kvm-all.c              | 11 -----------
 target-arm/kvm.c       |  4 ----
 target-i386/cpu.c      |  5 +++++
 target-i386/helper.c   |  4 ++++
 target-i386/kvm.c      | 37 ++++++++++++++++++++++++++-----------
 target-i386/kvm_i386.h |  2 ++
 target-ppc/kvm.c       |  4 ----
 target-s390x/cpu.c     |  4 ++++
 target-s390x/cpu.h     |  1 +
 10 files changed, 42 insertions(+), 32 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH uq/master v2 1/2] kvm: reset state from the CPU's reset method

[Paolo Bonzini]

Now that we have a CPU object with a reset method, it is better to
keep the KVM reset close to the CPU reset.  Using qemu_register_reset
as we do now keeps them far apart.

As a side effect, a CPU reset (cpu_reset) will reset the KVM state too.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/kvm.h   |  2 --
 kvm-all.c              | 11 -----------
 target-arm/kvm.c       |  4 ----
 target-i386/cpu.c      |  5 +++++
 target-i386/kvm_i386.h |  1 +
 target-ppc/kvm.c       |  4 ----
 target-s390x/cpu.c     |  4 ++++
 target-s390x/cpu.h     |  1 +
 8 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index f2d97b5..50072c5 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -199,8 +199,6 @@ int kvm_arch_init_vcpu(CPUState *cpu);
 /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
 unsigned long kvm_arch_vcpu_id(CPUState *cpu);
 
-void kvm_arch_reset_vcpu(CPUState *cpu);
-
 int kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
 int kvm_arch_on_sigbus(int code, void *addr);
 
diff --git a/kvm-all.c b/kvm-all.c
index 9b433d3..57616ef 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -207,13 +207,6 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
     return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
 }
 
-static void kvm_reset_vcpu(void *opaque)
-{
-    CPUState *cpu = opaque;
-
-    kvm_arch_reset_vcpu(cpu);
-}
-
 int kvm_init_vcpu(CPUState *cpu)
 {
     KVMState *s = kvm_state;
@@ -253,10 +246,6 @@ int kvm_init_vcpu(CPUState *cpu)
     }
 
     ret = kvm_arch_init_vcpu(cpu);
-    if (ret == 0) {
-        qemu_register_reset(kvm_reset_vcpu, cpu);
-        kvm_arch_reset_vcpu(cpu);
-    }
 err:
     return ret;
 }
diff --git a/target-arm/kvm.c b/target-arm/kvm.c
index 82e2e08..841b85f 100644
--- a/target-arm/kvm.c
+++ b/target-arm/kvm.c
@@ -430,10 +430,6 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
     return 0;
 }
 
-void kvm_arch_reset_vcpu(CPUState *cs)
-{
-}
-
 bool kvm_arch_stop_on_emulation_error(CPUState *cs)
 {
     return true;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a0640db..a5746cd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -24,6 +24,7 @@
 #include "cpu.h"
 #include "sysemu/kvm.h"
 #include "sysemu/cpus.h"
+#include "kvm_i386.h"
 #include "topology.h"
 
 #include "qemu/option.h"
@@ -2015,6 +2016,10 @@ static void x86_cpu_reset(CPUState *s)
     }
 
     s->halted = !cpu_is_bsp(cpu);
+
+    if (kvm_enabled()) {
+        kvm_arch_reset_vcpu(s);
+    }
 #endif
 }
 
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 4392ab4..3accc2d 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -14,6 +14,7 @@
 #include "sysemu/kvm.h"
 
 bool kvm_allows_irq0_override(void);
+void kvm_arch_reset_vcpu(CPUState *cs);
 
 int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
                           uint32_t flags, uint32_t *dev_id);
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index e663ff0..0adea12 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -424,10 +424,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
     return ret;
 }
 
-void kvm_arch_reset_vcpu(CPUState *cpu)
-{
-}
-
 static void kvm_sw_tlb_put(PowerPCCPU *cpu)
 {
     CPUPPCState *env = &cpu->env;
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 23fe51f..6321384 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -84,6 +84,10 @@ static void s390_cpu_reset(CPUState *s)
      * after incrementing the cpu counter */
 #if !defined(CONFIG_USER_ONLY)
     s->halted = 1;
+
+    if (kvm_enabled()) {
+        kvm_arch_reset_vcpu(s);
+    }
 #endif
     tlb_flush(env, 1);
 }
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index e351005..fc84159 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -352,6 +352,7 @@ void s390x_cpu_timer(void *opaque);
 int s390_virtio_hypercall(CPUS390XState *env);
 
 #ifdef CONFIG_KVM
+void kvm_arch_reset_vcpu(CPUState *cs);
 void kvm_s390_interrupt(S390CPU *cpu, int type, uint32_t code);
 void kvm_s390_virtio_irq(S390CPU *cpu, int config_change, uint64_t token);
 void kvm_s390_interrupt_internal(S390CPU *cpu, int type, uint32_t parm,
-- 
1.8.1.4

[Qemu-devel] [PATCH uq/master v2 2/2] kvm: forward INIT signals coming from the chipset

[Paolo Bonzini]

When an INIT comes in, we can do the entire reset process in userspace.
However, we have to be careful and move APs into KVM_MP_STATE_INIT_RECEIVED,
so that the in-kernel APIC will listen to startup IPIs.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/helper.c   |  4 ++++
 target-i386/kvm.c      | 37 ++++++++++++++++++++++++++-----------
 target-i386/kvm_i386.h |  1 +
 3 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 9449a0c..bbc5adf 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -19,6 +19,7 @@
 
 #include "cpu.h"
 #include "sysemu/kvm.h"
+#include "kvm_i386.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
@@ -1290,6 +1291,9 @@ void do_cpu_init(X86CPU *cpu)
     cpu_reset(cs);
     cs->interrupt_request = sipi;
     env->pat = pat;
+    if (kvm_enabled()) {
+        kvm_arch_do_init_vcpu(cs);
+    }
     apic_init_reset(env->apic_state);
 }
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index df30fa6..42a4571 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -30,6 +30,8 @@
 #include "qemu/config-file.h"
 #include "hw/pc.h"
 #include "hw/apic.h"
+#include "hw/apic_internal.h"
+#include "hw/apic-msidef.h"
 #include "exec/ioport.h"
 #include "hyperv.h"
 #include "hw/pci/pci.h"
@@ -676,6 +678,17 @@ void kvm_arch_reset_vcpu(CPUState *cs)
     }
 }
 
+void kvm_arch_do_init_vcpu(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+
+    /* APs go straight into wait-for-SIPI state after INIT# is asserted.  */
+    if (env->mp_state == KVM_MP_STATE_UNINITIALIZED) {
+        env->mp_state = KVM_MP_STATE_INIT_RECEIVED;
+    }
+}
+
 static int kvm_get_supported_msrs(KVMState *s)
 {
     static int kvm_supported_msrs;
@@ -1773,14 +1786,15 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run *run)
         }
     }
 
-    if (!kvm_irqchip_in_kernel()) {
-        /* Force the VCPU out of its inner loop to process any INIT requests
-         * or pending TPR access reports. */
-        if (cpu->interrupt_request &
-            (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
-            cpu->exit_request = 1;
-        }
+    /* Force the VCPU out of its inner loop to process any INIT requests
+     * or (for userspace APIC, but it is cheap to combine the checks here)
+     * pending TPR access reports.
+     */
+    if (cpu->interrupt_request & (CPU_INTERRUPT_INIT | CPU_INTERRUPT_TPR)) {
+        cpu->exit_request = 1;
+    }
 
+    if (!kvm_irqchip_in_kernel()) {
         /* Try to inject an interrupt if the guest can accept it */
         if (run->ready_for_interrupt_injection &&
             (cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
@@ -1860,6 +1874,11 @@ int kvm_arch_process_async_events(CPUState *cs)
         }
     }
 
+    if (cs->interrupt_request & CPU_INTERRUPT_INIT) {
+        kvm_cpu_synchronize_state(env);
+        do_cpu_init(cpu);
+    }
+
     if (kvm_irqchip_in_kernel()) {
         return 0;
     }
@@ -1873,10 +1892,6 @@ int kvm_arch_process_async_events(CPUState *cs)
         (cs->interrupt_request & CPU_INTERRUPT_NMI)) {
         cs->halted = 0;
     }
-    if (cs->interrupt_request & CPU_INTERRUPT_INIT) {
-        kvm_cpu_synchronize_state(env);
-        do_cpu_init(cpu);
-    }
     if (cs->interrupt_request & CPU_INTERRUPT_SIPI) {
         kvm_cpu_synchronize_state(env);
         do_cpu_sipi(cpu);
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 3accc2d..ce38ee6 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -15,6 +15,7 @@
 
 bool kvm_allows_irq0_override(void);
 void kvm_arch_reset_vcpu(CPUState *cs);
+void kvm_arch_do_init_vcpu(CPUState *cs);
 
 int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
                           uint32_t flags, uint32_t *dev_id);
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH uq/master v2 1/2] kvm: reset state from the CPU's reset method

[Gleb Natapov]

On Fri, Mar 22, 2013 at 09:37:16PM +0100, Paolo Bonzini wrote:
> Now that we have a CPU object with a reset method, it is better to
> keep the KVM reset close to the CPU reset.  Using qemu_register_reset
> as we do now keeps them far apart.
> 
> As a side effect, a CPU reset (cpu_reset) will reset the KVM state too.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/sysemu/kvm.h   |  2 --
>  kvm-all.c              | 11 -----------
>  target-arm/kvm.c       |  4 ----
>  target-i386/cpu.c      |  5 +++++
>  target-i386/kvm_i386.h |  1 +
>  target-ppc/kvm.c       |  4 ----
>  target-s390x/cpu.c     |  4 ++++
>  target-s390x/cpu.h     |  1 +
>  8 files changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index f2d97b5..50072c5 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -199,8 +199,6 @@ int kvm_arch_init_vcpu(CPUState *cpu);
>  /* Returns VCPU ID to be used on KVM_CREATE_VCPU ioctl() */
>  unsigned long kvm_arch_vcpu_id(CPUState *cpu);
>  
> -void kvm_arch_reset_vcpu(CPUState *cpu);
> -
>  int kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>  int kvm_arch_on_sigbus(int code, void *addr);
>  
> diff --git a/kvm-all.c b/kvm-all.c
> index 9b433d3..57616ef 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -207,13 +207,6 @@ static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
>      return kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
>  }
>  
> -static void kvm_reset_vcpu(void *opaque)
> -{
> -    CPUState *cpu = opaque;
> -
> -    kvm_arch_reset_vcpu(cpu);
> -}
> -
>  int kvm_init_vcpu(CPUState *cpu)
>  {
>      KVMState *s = kvm_state;
> @@ -253,10 +246,6 @@ int kvm_init_vcpu(CPUState *cpu)
>      }
>  
>      ret = kvm_arch_init_vcpu(cpu);
> -    if (ret == 0) {
> -        qemu_register_reset(kvm_reset_vcpu, cpu);
> -        kvm_arch_reset_vcpu(cpu);
> -    }
>  err:
>      return ret;
>  }
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 82e2e08..841b85f 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -430,10 +430,6 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
>      return 0;
>  }
>  
> -void kvm_arch_reset_vcpu(CPUState *cs)
> -{
> -}
> -
>  bool kvm_arch_stop_on_emulation_error(CPUState *cs)
>  {
>      return true;
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index a0640db..a5746cd 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -24,6 +24,7 @@
>  #include "cpu.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/cpus.h"
> +#include "kvm_i386.h"
>  #include "topology.h"
>  
>  #include "qemu/option.h"
> @@ -2015,6 +2016,10 @@ static void x86_cpu_reset(CPUState *s)
>      }
>  
>      s->halted = !cpu_is_bsp(cpu);
> +
> +    if (kvm_enabled()) {
> +        kvm_arch_reset_vcpu(s);
> +    }
>  #endif
>  }
>  
> diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
> index 4392ab4..3accc2d 100644
> --- a/target-i386/kvm_i386.h
> +++ b/target-i386/kvm_i386.h
> @@ -14,6 +14,7 @@
>  #include "sysemu/kvm.h"
>  
>  bool kvm_allows_irq0_override(void);
> +void kvm_arch_reset_vcpu(CPUState *cs);
>  
>  int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
>                            uint32_t flags, uint32_t *dev_id);
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index e663ff0..0adea12 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -424,10 +424,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      return ret;
>  }
>  
> -void kvm_arch_reset_vcpu(CPUState *cpu)
> -{
> -}
> -
>  static void kvm_sw_tlb_put(PowerPCCPU *cpu)
>  {
>      CPUPPCState *env = &cpu->env;
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 23fe51f..6321384 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -84,6 +84,10 @@ static void s390_cpu_reset(CPUState *s)
>       * after incrementing the cpu counter */
>  #if !defined(CONFIG_USER_ONLY)
>      s->halted = 1;
> +
> +    if (kvm_enabled()) {
> +        kvm_arch_reset_vcpu(s);
Does this compile with kvm support disabled?

> +    }
>  #endif
>      tlb_flush(env, 1);
>  }
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index e351005..fc84159 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -352,6 +352,7 @@ void s390x_cpu_timer(void *opaque);
>  int s390_virtio_hypercall(CPUS390XState *env);
>  
>  #ifdef CONFIG_KVM
> +void kvm_arch_reset_vcpu(CPUState *cs);
>  void kvm_s390_interrupt(S390CPU *cpu, int type, uint32_t code);
>  void kvm_s390_virtio_irq(S390CPU *cpu, int config_change, uint64_t token);
>  void kvm_s390_interrupt_internal(S390CPU *cpu, int type, uint32_t parm,

--
			Gleb.

Re: [Qemu-devel] [PATCH uq/master v2 1/2] kvm: reset state from the CPU's reset method

[Gleb Natapov]

On Tue, Apr 02, 2013 at 04:29:32PM +0300, Gleb Natapov wrote:
> >  static void kvm_sw_tlb_put(PowerPCCPU *cpu)
> >  {
> >      CPUPPCState *env = &cpu->env;
> > diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> > index 23fe51f..6321384 100644
> > --- a/target-s390x/cpu.c
> > +++ b/target-s390x/cpu.c
> > @@ -84,6 +84,10 @@ static void s390_cpu_reset(CPUState *s)
> >       * after incrementing the cpu counter */
> >  #if !defined(CONFIG_USER_ONLY)
> >      s->halted = 1;
> > +
> > +    if (kvm_enabled()) {
> > +        kvm_arch_reset_vcpu(s);
> Does this compile with kvm support disabled?
> 
Well, it does not:
  CC    s390x-softmmu/target-s390x/cpu.o
/users/gleb/work/qemu/target-s390x/cpu.c: In function 's390_cpu_reset':
/users/gleb/work/qemu/target-s390x/cpu.c:89:9: error: implicit
declaration of function 'kvm_arch_reset_vcpu'
[-Werror=implicit-function-declaration]
/users/gleb/work/qemu/target-s390x/cpu.c:89:9: error: nested extern
declaration of 'kvm_arch_reset_vcpu' [-Werror=nested-externs]
cc1: all warnings being treated as errors

I wonder if it is portable between compilers to rely on code in if(0){} to
be dropped in all levels of optimizations.

--
			Gleb.

Re: [Qemu-devel] [PATCH uq/master v2 1/2] kvm: reset state from the CPU's reset method

[Paolo Bonzini]

Il 08/04/2013 14:19, Gleb Natapov ha scritto:
>> > Does this compile with kvm support disabled?

Oops, sorry, I thought I had replied to this email (with "hmm, let me
check").

> Well, it does not:
>   CC    s390x-softmmu/target-s390x/cpu.o
> /users/gleb/work/qemu/target-s390x/cpu.c: In function 's390_cpu_reset':
> /users/gleb/work/qemu/target-s390x/cpu.c:89:9: error: implicit
> declaration of function 'kvm_arch_reset_vcpu'
> [-Werror=implicit-function-declaration]
> /users/gleb/work/qemu/target-s390x/cpu.c:89:9: error: nested extern
> declaration of 'kvm_arch_reset_vcpu' [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> 
> I wonder if it is portable between compilers to rely on code in if(0){} to
> be dropped in all levels of optimizations.

It generally is okay to assume it (I think early GCC 3.x releases had no
-O0 dead-code optimization, but it was a long time ago).  However:

* in QEMU only some files have kvm_enabled() as 0 when KVM is disabled.
 Files that are shared among multiple targets have it defined to
kvm_allowed.  This is not the problem here.

* you still need to define the prototypes for anything you call, of course.

Paolo

Re: [Qemu-devel] [PATCH uq/master v2 1/2] kvm: reset state from the CPU's reset method

[Andreas Färber]

Am 08.04.2013 14:19, schrieb Gleb Natapov:
> On Tue, Apr 02, 2013 at 04:29:32PM +0300, Gleb Natapov wrote:
>>>  static void kvm_sw_tlb_put(PowerPCCPU *cpu)
>>>  {
>>>      CPUPPCState *env = &cpu->env;
>>> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
>>> index 23fe51f..6321384 100644
>>> --- a/target-s390x/cpu.c
>>> +++ b/target-s390x/cpu.c
>>> @@ -84,6 +84,10 @@ static void s390_cpu_reset(CPUState *s)
>>>       * after incrementing the cpu counter */
>>>  #if !defined(CONFIG_USER_ONLY)
>>>      s->halted = 1;
>>> +
>>> +    if (kvm_enabled()) {
>>> +        kvm_arch_reset_vcpu(s);
>> Does this compile with kvm support disabled?
>>
> Well, it does not:
>   CC    s390x-softmmu/target-s390x/cpu.o
> /users/gleb/work/qemu/target-s390x/cpu.c: In function 's390_cpu_reset':
> /users/gleb/work/qemu/target-s390x/cpu.c:89:9: error: implicit
> declaration of function 'kvm_arch_reset_vcpu'
> [-Werror=implicit-function-declaration]
> /users/gleb/work/qemu/target-s390x/cpu.c:89:9: error: nested extern
> declaration of 'kvm_arch_reset_vcpu' [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> 
> I wonder if it is portable between compilers to rely on code in if(0){} to
> be dropped in all levels of optimizations.

No, we had a previous case where --enable-debug broke if (kvm_enabled())
{...} but regular builds worked.

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 uq/master v2 1/2] kvm: reset state from the CPU's reset method

[Gleb Natapov]

On Mon, Apr 08, 2013 at 04:36:47PM +0200, Andreas Färber wrote:
> Am 08.04.2013 14:19, schrieb Gleb Natapov:
> > On Tue, Apr 02, 2013 at 04:29:32PM +0300, Gleb Natapov wrote:
> >>>  static void kvm_sw_tlb_put(PowerPCCPU *cpu)
> >>>  {
> >>>      CPUPPCState *env = &cpu->env;
> >>> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> >>> index 23fe51f..6321384 100644
> >>> --- a/target-s390x/cpu.c
> >>> +++ b/target-s390x/cpu.c
> >>> @@ -84,6 +84,10 @@ static void s390_cpu_reset(CPUState *s)
> >>>       * after incrementing the cpu counter */
> >>>  #if !defined(CONFIG_USER_ONLY)
> >>>      s->halted = 1;
> >>> +
> >>> +    if (kvm_enabled()) {
> >>> +        kvm_arch_reset_vcpu(s);
> >> Does this compile with kvm support disabled?
> >>
> > Well, it does not:
> >   CC    s390x-softmmu/target-s390x/cpu.o
> > /users/gleb/work/qemu/target-s390x/cpu.c: In function 's390_cpu_reset':
> > /users/gleb/work/qemu/target-s390x/cpu.c:89:9: error: implicit
> > declaration of function 'kvm_arch_reset_vcpu'
> > [-Werror=implicit-function-declaration]
> > /users/gleb/work/qemu/target-s390x/cpu.c:89:9: error: nested extern
> > declaration of 'kvm_arch_reset_vcpu' [-Werror=nested-externs]
> > cc1: all warnings being treated as errors
> > 
> > I wonder if it is portable between compilers to rely on code in if(0){} to
> > be dropped in all levels of optimizations.
> 
> No, we had a previous case where --enable-debug broke if (kvm_enabled())
> {...} but regular builds worked.
> 
Can you recall what compiler was it with? 4.7.2 works with -O0.

--
			Gleb.