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

2013-04-08 Thread 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:
  CCs390x-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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-08 Thread 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:
   CCs390x-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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-08 Thread 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:
   CCs390x-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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-08 Thread 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:
CCs390x-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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-04-02 Thread 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 pbonz...@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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-03-22 Thread 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 pbonz...@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


--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html