[Qemu-devel] [CFT PATCH 00/12] Tricky parts of my iothread-for-win32 stuff

[Qemu-devel] [CFT PATCH 00/12] Tricky parts of my iothread-for-win32 stuff

[Paolo Bonzini]

Ok, so let's start the dance...

These are the generic parts of the iothread-for-win32 work I made
last year, rebased to today's qemu (qemu-kvm.git/uq/master merged
with qemu.git/master, because it conflicts with some patches from Jan).
I never tested them too much, and that's one of the reasons why I never
submitted the stuff.  Perhaps with some help they can make it this time.

Testing I made now is still a bit limited, but the patches can boot
RHEL and Fedora under both Windows iothread and Linux iothread.  FWIW,
under Wine I need iothread to boot Linux.

The main idea is to replace timedwaits with waits.  First because the
timedwaits were such only out of laziness; second because timedwait
is a pain to implement for Win32 where we have to manually implement
condvars.

Anthony Liguori (1):
  io-thread: make sure to initialize qemu_work_cond and qemu_cpu_cond

Paolo Bonzini (11):
  cris, microblaze: use cpu_has_work
  inline cpu_halted into sole caller
  change qemu_thread_equal API to always compare with current thread
  always qemu_cpu_kick after unhalting a cpu
  exit round-robin vcpu loop if cpu->stopped is true
  always signal pause_cond after stopping a VCPU
  do not use timedwait on qemu_halt_cond
  do not use timedwait on qemu_system_cond
  do not use timedwait on qemu_pause_cond
  do not use timedwait on qemu_cpu_cond
  iothread stops the vcpu thread via IPI

 cpu-defs.h               |    1 +
 cpu-exec.c               |    9 +++++++--
 cpus.c                   |   38 +++++++++++++++++++-------------------
 hw/apic.c                |    1 +
 hw/ppc.c                 |    2 ++
 hw/sun4m.c               |   11 +++++++++--
 hw/sun4u.c               |    4 ++--
 qemu-thread.c            |    4 ++--
 qemu-thread.h            |    2 +-
 target-alpha/exec.h      |   11 -----------
 target-arm/exec.h        |   13 -------------
 target-cris/exec.h       |   13 -------------
 target-i386/exec.h       |   12 ------------
 target-m68k/exec.h       |   10 ----------
 target-microblaze/exec.h |   13 -------------
 target-mips/exec.h       |   11 -----------
 target-ppc/exec.h        |   11 -----------
 target-s390x/exec.h      |   12 ------------
 target-s390x/kvm.c       |    1 +
 target-sh4/cpu.h         |    1 -
 target-sh4/exec.h        |   11 -----------
 target-sparc/exec.h      |   10 ----------
 22 files changed, 45 insertions(+), 156 deletions(-)

-- 
1.7.3.5

[Qemu-devel] [CFT PATCH 01/12] io-thread: make sure to initialize qemu_work_cond and qemu_cpu_cond

[Paolo Bonzini]

From: Anthony Liguori <aliguori@us.ibm.com>

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 cpus.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index 6a85dc8..474be16 100644
--- a/cpus.c
+++ b/cpus.c
@@ -667,8 +667,10 @@ int qemu_init_main_loop(void)
     if (ret)
         return ret;
 
-    qemu_cond_init(&qemu_pause_cond);
+    qemu_cond_init(&qemu_cpu_cond);
     qemu_cond_init(&qemu_system_cond);
+    qemu_cond_init(&qemu_pause_cond);
+    qemu_cond_init(&qemu_work_cond);
     qemu_mutex_init(&qemu_fair_mutex);
     qemu_mutex_init(&qemu_global_mutex);
     qemu_mutex_lock(&qemu_global_mutex);
-- 
1.7.3.5

Re: [Qemu-devel] [CFT PATCH 01/12] io-thread: make sure to initialize qemu_work_cond and qemu_cpu_cond

[Anthony Liguori]

On 02/08/2011 11:18 AM, Paolo Bonzini wrote:
> From: Anthony Liguori<aliguori@us.ibm.com>
>
> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>    

Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>   cpus.c |    4 +++-
>   1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index 6a85dc8..474be16 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -667,8 +667,10 @@ int qemu_init_main_loop(void)
>       if (ret)
>           return ret;
>
> -    qemu_cond_init(&qemu_pause_cond);
> +    qemu_cond_init(&qemu_cpu_cond);
>       qemu_cond_init(&qemu_system_cond);
> +    qemu_cond_init(&qemu_pause_cond);
> +    qemu_cond_init(&qemu_work_cond);
>       qemu_mutex_init(&qemu_fair_mutex);
>       qemu_mutex_init(&qemu_global_mutex);
>       qemu_mutex_lock(&qemu_global_mutex);
>    

[Qemu-devel] [CFT PATCH 02/12] cris, microblaze: use cpu_has_work

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-cris/exec.h       |    4 +---
 target-microblaze/exec.h |    4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/target-cris/exec.h b/target-cris/exec.h
index 93ce768..34c0132 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -37,9 +37,7 @@ static inline int cpu_halted(CPUState *env) {
 	if (!env->halted)
 		return 0;
 
-	/* IRQ, NMI and GURU execeptions wakes us up.  */
-	if (env->interrupt_request
-	    & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
+	if (cpu_has_work(env)) {
 		env->halted = 0;
 		return 0;
 	}
diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
index 87b2494..ab19828 100644
--- a/target-microblaze/exec.h
+++ b/target-microblaze/exec.h
@@ -36,9 +36,7 @@ static inline int cpu_halted(CPUState *env) {
 	if (!env->halted)
 		return 0;
 
-	/* IRQ, NMI and GURU execeptions wakes us up.  */
-	if (env->interrupt_request
-	    & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
+	if (cpu_has_work(env)) {
 		env->halted = 0;
 		return 0;
 	}
-- 
1.7.3.5

Re: [Qemu-devel] [CFT PATCH 02/12] cris, microblaze: use cpu_has_work

[Edgar E. Iglesias]

On Tue, Feb 08, 2011 at 06:18:19PM +0100, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks, I've applied this one.

Cheers



> ---
>  target-cris/exec.h       |    4 +---
>  target-microblaze/exec.h |    4 +---
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/target-cris/exec.h b/target-cris/exec.h
> index 93ce768..34c0132 100644
> --- a/target-cris/exec.h
> +++ b/target-cris/exec.h
> @@ -37,9 +37,7 @@ static inline int cpu_halted(CPUState *env) {
>  	if (!env->halted)
>  		return 0;
>  
> -	/* IRQ, NMI and GURU execeptions wakes us up.  */
> -	if (env->interrupt_request
> -	    & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
> +	if (cpu_has_work(env)) {
>  		env->halted = 0;
>  		return 0;
>  	}
> diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
> index 87b2494..ab19828 100644
> --- a/target-microblaze/exec.h
> +++ b/target-microblaze/exec.h
> @@ -36,9 +36,7 @@ static inline int cpu_halted(CPUState *env) {
>  	if (!env->halted)
>  		return 0;
>  
> -	/* IRQ, NMI and GURU execeptions wakes us up.  */
> -	if (env->interrupt_request
> -	    & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
> +	if (cpu_has_work(env)) {
>  		env->halted = 0;
>  		return 0;
>  	}
> -- 
> 1.7.3.5
> 
> 
> 

[Qemu-devel] [CFT PATCH 03/12] inline cpu_halted into sole caller

[Paolo Bonzini]

All implementations are now the same except SH, which can fit in
the default implementation easily.  The newly added flag will not make
much sense on non-SH platforms, but I left it anyway.  Alternatively you
could #ifdef it out on non-SH.

This reduces the number of places that have to be audited for patch 5
("always qemu_cpu_kick after unhalting a cpu").

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-defs.h               |    1 +
 cpu-exec.c               |    9 +++++++--
 target-alpha/exec.h      |   11 -----------
 target-arm/exec.h        |   13 -------------
 target-cris/exec.h       |   11 -----------
 target-i386/exec.h       |   12 ------------
 target-m68k/exec.h       |   10 ----------
 target-microblaze/exec.h |   11 -----------
 target-mips/exec.h       |   11 -----------
 target-ppc/exec.h        |   11 -----------
 target-s390x/exec.h      |   12 ------------
 target-sh4/cpu.h         |    1 -
 target-sh4/exec.h        |   11 -----------
 target-sparc/exec.h      |   10 ----------
 14 files changed, 8 insertions(+), 126 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index db809ed..e4dee97 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -159,6 +159,7 @@ typedef struct CPUWatchpoint {
     target_ulong mem_io_vaddr; /* target virtual addr at which the      \
                                      memory was accessed */             \
     uint32_t halted; /* Nonzero if the CPU is in suspend state */       \
+    uint32_t intr_at_halt; /* Nonzero if an irq woke CPU from halted state */ \
     uint32_t interrupt_request;                                         \
     volatile sig_atomic_t exit_request;                                 \
     CPU_COMMON_TLB                                                      \
diff --git a/cpu-exec.c b/cpu-exec.c
index 8c9fb8b..3d6ff35 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -230,8 +230,13 @@ int cpu_exec(CPUState *env1)
     uint8_t *tc_ptr;
     unsigned long next_tb;
 
-    if (cpu_halted(env1) == EXCP_HALTED)
-        return EXCP_HALTED;
+    if (env1->halted) {
+        if (!cpu_has_work(env1))
+            return EXCP_HALTED;
+
+        env1->halted = 0;
+        env1->intr_at_halt = 1;
+    }
 
     cpu_single_env = env1;
 
diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index a8a38d2..6ae96d1 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -42,17 +42,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & CPU_INTERRUPT_HARD);
 }
 
-static inline int cpu_halted(CPUState *env)
-{
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->pc = tb->pc;
diff --git a/target-arm/exec.h b/target-arm/exec.h
index e4c35a3..44e1b55 100644
--- a/target-arm/exec.h
+++ b/target-arm/exec.h
@@ -32,19 +32,6 @@ static inline int cpu_has_work(CPUState *env)
             (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB));
 }
 
-static inline int cpu_halted(CPUState *env) {
-    if (!env->halted)
-        return 0;
-    /* An interrupt wakes the CPU even if the I and F CPSR bits are
-       set.  We use EXITTB to silently wake CPU without causing an
-       actual interrupt.  */
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 #if !defined(CONFIG_USER_ONLY)
 #include "softmmu_exec.h"
 #endif
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 34c0132..2d5d297 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -33,17 +33,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
 }
 
-static inline int cpu_halted(CPUState *env) {
-	if (!env->halted)
-		return 0;
-
-	if (cpu_has_work(env)) {
-		env->halted = 0;
-		return 0;
-	}
-	return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->pc = tb->pc;
diff --git a/target-i386/exec.h b/target-i386/exec.h
index fc8945b..3e7386e 100644
--- a/target-i386/exec.h
+++ b/target-i386/exec.h
@@ -304,18 +304,6 @@ static inline int cpu_has_work(CPUState *env)
     return work;
 }
 
-static inline int cpu_halted(CPUState *env) {
-    /* handle exit of HALTED state */
-    if (!env->halted)
-        return 0;
-    /* disable halt condition */
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 /* load efer and update the corresponding hflags. XXX: do consistency
    checks with cpuid bits ? */
 static inline void cpu_load_efer(CPUState *env, uint64_t val)
diff --git a/target-m68k/exec.h b/target-m68k/exec.h
index f31e06e..91daa6b 100644
--- a/target-m68k/exec.h
+++ b/target-m68k/exec.h
@@ -33,16 +33,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & (CPU_INTERRUPT_HARD));
 }
 
-static inline int cpu_halted(CPUState *env) {
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->pc = tb->pc;
diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
index ab19828..1efff30 100644
--- a/target-microblaze/exec.h
+++ b/target-microblaze/exec.h
@@ -32,17 +32,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
 }
 
-static inline int cpu_halted(CPUState *env) {
-	if (!env->halted)
-		return 0;
-
-	if (cpu_has_work(env)) {
-		env->halted = 0;
-		return 0;
-	}
-	return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->sregs[SR_PC] = tb->pc;
diff --git a/target-mips/exec.h b/target-mips/exec.h
index 1273654..b3c5a13 100644
--- a/target-mips/exec.h
+++ b/target-mips/exec.h
@@ -36,17 +36,6 @@ static inline int cpu_has_work(CPUState *env)
     return has_work;
 }
 
-static inline int cpu_halted(CPUState *env)
-{
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void compute_hflags(CPUState *env)
 {
     env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
diff --git a/target-ppc/exec.h b/target-ppc/exec.h
index 4688ef5..f87847a 100644
--- a/target-ppc/exec.h
+++ b/target-ppc/exec.h
@@ -38,17 +38,6 @@ static inline int cpu_has_work(CPUState *env)
 }
 
 
-static inline int cpu_halted(CPUState *env)
-{
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->nip = tb->pc;
diff --git a/target-s390x/exec.h b/target-s390x/exec.h
index bf3f264..f7893f3 100644
--- a/target-s390x/exec.h
+++ b/target-s390x/exec.h
@@ -34,18 +34,6 @@ static inline int cpu_has_work(CPUState *env)
     return env->interrupt_request & CPU_INTERRUPT_HARD; // guess
 }
 
-static inline int cpu_halted(CPUState *env)
-{
-    if (!env->halted) {
-       return 0;
-    }
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock* tb)
 {
     env->psw.addr = tb->pc;
diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index 789d188..7188c4d 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -184,7 +184,6 @@ typedef struct CPUSH4State {
     uint32_t cvr;		/* Cache Version Register */
 
     void *intc_handle;
-    int intr_at_halt;		/* SR_BL ignored during sleep */
     memory_content *movcal_backup;
     memory_content **movcal_backup_tail;
 } CPUSH4State;
diff --git a/target-sh4/exec.h b/target-sh4/exec.h
index 2999c02..9f1c1f6 100644
--- a/target-sh4/exec.h
+++ b/target-sh4/exec.h
@@ -32,17 +32,6 @@ static inline int cpu_has_work(CPUState *env)
     return (env->interrupt_request & CPU_INTERRUPT_HARD);
 }
 
-static inline int cpu_halted(CPUState *env) {
-    if (!env->halted)
-        return 0;
-    if (cpu_has_work(env)) {
-        env->halted = 0;
-        env->intr_at_halt = 1;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 #ifndef CONFIG_USER_ONLY
 #include "softmmu_exec.h"
 #endif
diff --git a/target-sparc/exec.h b/target-sparc/exec.h
index f811571..f5c221e 100644
--- a/target-sparc/exec.h
+++ b/target-sparc/exec.h
@@ -22,16 +22,6 @@ static inline int cpu_has_work(CPUState *env1)
 }
 
 
-static inline int cpu_halted(CPUState *env1) {
-    if (!env1->halted)
-        return 0;
-    if (cpu_has_work(env1)) {
-        env1->halted = 0;
-        return 0;
-    }
-    return EXCP_HALTED;
-}
-
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->pc = tb->pc;
-- 
1.7.3.5

[Qemu-devel] Re: [CFT PATCH 03/12] inline cpu_halted into sole caller

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 10097 bytes --]

On 2011-02-08 18:18, Paolo Bonzini wrote:
> All implementations are now the same except SH, which can fit in
> the default implementation easily.  The newly added flag will not make
> much sense on non-SH platforms, but I left it anyway.  Alternatively you
> could #ifdef it out on non-SH.

I think we can live with that additional variable setting. Just add a
comment why it's there.

> 
> This reduces the number of places that have to be audited for patch 5
> ("always qemu_cpu_kick after unhalting a cpu").
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpu-defs.h               |    1 +
>  cpu-exec.c               |    9 +++++++--
>  target-alpha/exec.h      |   11 -----------
>  target-arm/exec.h        |   13 -------------
>  target-cris/exec.h       |   11 -----------
>  target-i386/exec.h       |   12 ------------
>  target-m68k/exec.h       |   10 ----------
>  target-microblaze/exec.h |   11 -----------
>  target-mips/exec.h       |   11 -----------
>  target-ppc/exec.h        |   11 -----------
>  target-s390x/exec.h      |   12 ------------
>  target-sh4/cpu.h         |    1 -
>  target-sh4/exec.h        |   11 -----------
>  target-sparc/exec.h      |   10 ----------
>  14 files changed, 8 insertions(+), 126 deletions(-)
> 
> diff --git a/cpu-defs.h b/cpu-defs.h
> index db809ed..e4dee97 100644
> --- a/cpu-defs.h
> +++ b/cpu-defs.h
> @@ -159,6 +159,7 @@ typedef struct CPUWatchpoint {
>      target_ulong mem_io_vaddr; /* target virtual addr at which the      \
>                                       memory was accessed */             \
>      uint32_t halted; /* Nonzero if the CPU is in suspend state */       \
> +    uint32_t intr_at_halt; /* Nonzero if an irq woke CPU from halted state */ \
>      uint32_t interrupt_request;                                         \
>      volatile sig_atomic_t exit_request;                                 \
>      CPU_COMMON_TLB                                                      \
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 8c9fb8b..3d6ff35 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -230,8 +230,13 @@ int cpu_exec(CPUState *env1)
>      uint8_t *tc_ptr;
>      unsigned long next_tb;
>  
> -    if (cpu_halted(env1) == EXCP_HALTED)
> -        return EXCP_HALTED;
> +    if (env1->halted) {
> +        if (!cpu_has_work(env1))
> +            return EXCP_HALTED;
> +
> +        env1->halted = 0;
> +        env1->intr_at_halt = 1;
> +    }
>  
>      cpu_single_env = env1;
>  
> diff --git a/target-alpha/exec.h b/target-alpha/exec.h
> index a8a38d2..6ae96d1 100644
> --- a/target-alpha/exec.h
> +++ b/target-alpha/exec.h
> @@ -42,17 +42,6 @@ static inline int cpu_has_work(CPUState *env)
>      return (env->interrupt_request & CPU_INTERRUPT_HARD);
>  }
>  
> -static inline int cpu_halted(CPUState *env)
> -{
> -    if (!env->halted)
> -        return 0;
> -    if (cpu_has_work(env)) {
> -        env->halted = 0;
> -        return 0;
> -    }
> -    return EXCP_HALTED;
> -}
> -
>  static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
>  {
>      env->pc = tb->pc;
> diff --git a/target-arm/exec.h b/target-arm/exec.h
> index e4c35a3..44e1b55 100644
> --- a/target-arm/exec.h
> +++ b/target-arm/exec.h
> @@ -32,19 +32,6 @@ static inline int cpu_has_work(CPUState *env)
>              (CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB));
>  }
>  
> -static inline int cpu_halted(CPUState *env) {
> -    if (!env->halted)
> -        return 0;
> -    /* An interrupt wakes the CPU even if the I and F CPSR bits are
> -       set.  We use EXITTB to silently wake CPU without causing an
> -       actual interrupt.  */
> -    if (cpu_has_work(env)) {
> -        env->halted = 0;
> -        return 0;
> -    }
> -    return EXCP_HALTED;
> -}
> -
>  #if !defined(CONFIG_USER_ONLY)
>  #include "softmmu_exec.h"
>  #endif
> diff --git a/target-cris/exec.h b/target-cris/exec.h
> index 34c0132..2d5d297 100644
> --- a/target-cris/exec.h
> +++ b/target-cris/exec.h
> @@ -33,17 +33,6 @@ static inline int cpu_has_work(CPUState *env)
>      return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
>  }
>  
> -static inline int cpu_halted(CPUState *env) {
> -	if (!env->halted)
> -		return 0;
> -
> -	if (cpu_has_work(env)) {
> -		env->halted = 0;
> -		return 0;
> -	}
> -	return EXCP_HALTED;
> -}
> -
>  static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
>  {
>      env->pc = tb->pc;
> diff --git a/target-i386/exec.h b/target-i386/exec.h
> index fc8945b..3e7386e 100644
> --- a/target-i386/exec.h
> +++ b/target-i386/exec.h
> @@ -304,18 +304,6 @@ static inline int cpu_has_work(CPUState *env)
>      return work;
>  }
>  
> -static inline int cpu_halted(CPUState *env) {
> -    /* handle exit of HALTED state */
> -    if (!env->halted)
> -        return 0;
> -    /* disable halt condition */
> -    if (cpu_has_work(env)) {
> -        env->halted = 0;
> -        return 0;
> -    }
> -    return EXCP_HALTED;
> -}
> -
>  /* load efer and update the corresponding hflags. XXX: do consistency
>     checks with cpuid bits ? */
>  static inline void cpu_load_efer(CPUState *env, uint64_t val)
> diff --git a/target-m68k/exec.h b/target-m68k/exec.h
> index f31e06e..91daa6b 100644
> --- a/target-m68k/exec.h
> +++ b/target-m68k/exec.h
> @@ -33,16 +33,6 @@ static inline int cpu_has_work(CPUState *env)
>      return (env->interrupt_request & (CPU_INTERRUPT_HARD));
>  }
>  
> -static inline int cpu_halted(CPUState *env) {
> -    if (!env->halted)
> -        return 0;
> -    if (cpu_has_work(env)) {
> -        env->halted = 0;
> -        return 0;
> -    }
> -    return EXCP_HALTED;
> -}
> -
>  static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
>  {
>      env->pc = tb->pc;
> diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
> index ab19828..1efff30 100644
> --- a/target-microblaze/exec.h
> +++ b/target-microblaze/exec.h
> @@ -32,17 +32,6 @@ static inline int cpu_has_work(CPUState *env)
>      return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
>  }
>  
> -static inline int cpu_halted(CPUState *env) {
> -	if (!env->halted)
> -		return 0;
> -
> -	if (cpu_has_work(env)) {
> -		env->halted = 0;
> -		return 0;
> -	}
> -	return EXCP_HALTED;
> -}
> -
>  static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
>  {
>      env->sregs[SR_PC] = tb->pc;
> diff --git a/target-mips/exec.h b/target-mips/exec.h
> index 1273654..b3c5a13 100644
> --- a/target-mips/exec.h
> +++ b/target-mips/exec.h
> @@ -36,17 +36,6 @@ static inline int cpu_has_work(CPUState *env)
>      return has_work;
>  }
>  
> -static inline int cpu_halted(CPUState *env)
> -{
> -    if (!env->halted)
> -        return 0;
> -    if (cpu_has_work(env)) {
> -        env->halted = 0;
> -        return 0;
> -    }
> -    return EXCP_HALTED;
> -}
> -
>  static inline void compute_hflags(CPUState *env)
>  {
>      env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
> diff --git a/target-ppc/exec.h b/target-ppc/exec.h
> index 4688ef5..f87847a 100644
> --- a/target-ppc/exec.h
> +++ b/target-ppc/exec.h
> @@ -38,17 +38,6 @@ static inline int cpu_has_work(CPUState *env)
>  }
>  
>  
> -static inline int cpu_halted(CPUState *env)
> -{
> -    if (!env->halted)
> -        return 0;
> -    if (cpu_has_work(env)) {
> -        env->halted = 0;
> -        return 0;
> -    }
> -    return EXCP_HALTED;
> -}
> -
>  static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
>  {
>      env->nip = tb->pc;
> diff --git a/target-s390x/exec.h b/target-s390x/exec.h
> index bf3f264..f7893f3 100644
> --- a/target-s390x/exec.h
> +++ b/target-s390x/exec.h
> @@ -34,18 +34,6 @@ static inline int cpu_has_work(CPUState *env)
>      return env->interrupt_request & CPU_INTERRUPT_HARD; // guess
>  }
>  
> -static inline int cpu_halted(CPUState *env)
> -{
> -    if (!env->halted) {
> -       return 0;
> -    }
> -    if (cpu_has_work(env)) {
> -        env->halted = 0;
> -        return 0;
> -    }
> -    return EXCP_HALTED;
> -}
> -
>  static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock* tb)
>  {
>      env->psw.addr = tb->pc;
> diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
> index 789d188..7188c4d 100644
> --- a/target-sh4/cpu.h
> +++ b/target-sh4/cpu.h
> @@ -184,7 +184,6 @@ typedef struct CPUSH4State {
>      uint32_t cvr;		/* Cache Version Register */
>  
>      void *intc_handle;
> -    int intr_at_halt;		/* SR_BL ignored during sleep */
>      memory_content *movcal_backup;
>      memory_content **movcal_backup_tail;
>  } CPUSH4State;
> diff --git a/target-sh4/exec.h b/target-sh4/exec.h
> index 2999c02..9f1c1f6 100644
> --- a/target-sh4/exec.h
> +++ b/target-sh4/exec.h
> @@ -32,17 +32,6 @@ static inline int cpu_has_work(CPUState *env)
>      return (env->interrupt_request & CPU_INTERRUPT_HARD);
>  }
>  
> -static inline int cpu_halted(CPUState *env) {
> -    if (!env->halted)
> -        return 0;
> -    if (cpu_has_work(env)) {
> -        env->halted = 0;
> -        env->intr_at_halt = 1;
> -        return 0;
> -    }
> -    return EXCP_HALTED;
> -}
> -
>  #ifndef CONFIG_USER_ONLY
>  #include "softmmu_exec.h"
>  #endif
> diff --git a/target-sparc/exec.h b/target-sparc/exec.h
> index f811571..f5c221e 100644
> --- a/target-sparc/exec.h
> +++ b/target-sparc/exec.h
> @@ -22,16 +22,6 @@ static inline int cpu_has_work(CPUState *env1)
>  }
>  
>  
> -static inline int cpu_halted(CPUState *env1) {
> -    if (!env1->halted)
> -        return 0;
> -    if (cpu_has_work(env1)) {
> -        env1->halted = 0;
> -        return 0;
> -    }
> -    return EXCP_HALTED;
> -}
> -
>  static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
>  {
>      env->pc = tb->pc;

Nice cleanup.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

[Qemu-devel] [CFT PATCH 04/12] change qemu_thread_equal API to always compare with current thread

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c        |   10 ++--------
 qemu-thread.c |    4 ++--
 qemu-thread.h |    2 +-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/cpus.c b/cpus.c
index 474be16..c1d0ceb 100644
--- a/cpus.c
+++ b/cpus.c
@@ -857,11 +857,8 @@ void qemu_cpu_kick_self(void)
 int qemu_cpu_self(void *_env)
 {
     CPUState *env = _env;
-    QemuThread this;
 
-    qemu_thread_self(&this);
-
-    return qemu_thread_equal(&this, env->thread);
+    return qemu_thread_equal(env->thread);
 }
 
 void qemu_mutex_lock_iothread(void)
@@ -990,10 +987,7 @@ void cpu_stop_current(void)
 
 void vm_stop(int reason)
 {
-    QemuThread me;
-    qemu_thread_self(&me);
-
-    if (!qemu_thread_equal(&me, &io_thread)) {
+    if (!qemu_thread_equal(&io_thread)) {
         qemu_system_vmstop_request(reason);
         /*
          * FIXME: should not return to device code in case
diff --git a/qemu-thread.c b/qemu-thread.c
index fbc78fe..7f5c0b6 100644
--- a/qemu-thread.c
+++ b/qemu-thread.c
@@ -181,9 +181,9 @@ void qemu_thread_self(QemuThread *thread)
     thread->thread = pthread_self();
 }
 
-int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2)
+int qemu_thread_equal(QemuThread *thread)
 {
-   return pthread_equal(thread1->thread, thread2->thread);
+   return pthread_equal(pthread_self(), thread->thread);
 }
 
 void qemu_thread_exit(void *retval)
diff --git a/qemu-thread.h b/qemu-thread.h
index 19bb30c..fa4bfb9 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -38,7 +38,7 @@ void qemu_thread_create(QemuThread *thread,
                        void *arg);
 void qemu_thread_signal(QemuThread *thread, int sig);
 void qemu_thread_self(QemuThread *thread);
-int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
+int qemu_thread_equal(QemuThread *thread);
 void qemu_thread_exit(void *retval);
 
 #endif
-- 
1.7.3.5

[Qemu-devel] Re: [CFT PATCH 04/12] change qemu_thread_equal API to always compare with current thread

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2209 bytes --]

On 2011-02-08 18:18, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpus.c        |   10 ++--------
>  qemu-thread.c |    4 ++--
>  qemu-thread.h |    2 +-
>  3 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 474be16..c1d0ceb 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -857,11 +857,8 @@ void qemu_cpu_kick_self(void)
>  int qemu_cpu_self(void *_env)
>  {
>      CPUState *env = _env;
> -    QemuThread this;
>  
> -    qemu_thread_self(&this);
> -
> -    return qemu_thread_equal(&this, env->thread);
> +    return qemu_thread_equal(env->thread);
>  }
>  
>  void qemu_mutex_lock_iothread(void)
> @@ -990,10 +987,7 @@ void cpu_stop_current(void)
>  
>  void vm_stop(int reason)
>  {
> -    QemuThread me;
> -    qemu_thread_self(&me);
> -
> -    if (!qemu_thread_equal(&me, &io_thread)) {
> +    if (!qemu_thread_equal(&io_thread)) {
>          qemu_system_vmstop_request(reason);
>          /*
>           * FIXME: should not return to device code in case
> diff --git a/qemu-thread.c b/qemu-thread.c
> index fbc78fe..7f5c0b6 100644
> --- a/qemu-thread.c
> +++ b/qemu-thread.c
> @@ -181,9 +181,9 @@ void qemu_thread_self(QemuThread *thread)
>      thread->thread = pthread_self();
>  }
>  
> -int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2)
> +int qemu_thread_equal(QemuThread *thread)
>  {
> -   return pthread_equal(thread1->thread, thread2->thread);
> +   return pthread_equal(pthread_self(), thread->thread);
>  }
>  
>  void qemu_thread_exit(void *retval)
> diff --git a/qemu-thread.h b/qemu-thread.h
> index 19bb30c..fa4bfb9 100644
> --- a/qemu-thread.h
> +++ b/qemu-thread.h
> @@ -38,7 +38,7 @@ void qemu_thread_create(QemuThread *thread,
>                         void *arg);
>  void qemu_thread_signal(QemuThread *thread, int sig);
>  void qemu_thread_self(QemuThread *thread);
> -int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
> +int qemu_thread_equal(QemuThread *thread);
>  void qemu_thread_exit(void *retval);
>  
>  #endif

I would call this "qemu_thread_is_current" or so to clarify what is
tested here.

JAn


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

[Qemu-devel] [CFT PATCH 05/12] always qemu_cpu_kick after unhalting a cpu

[Paolo Bonzini]

This ensures env->halt_cond is broadcast, and the loop in
qemu_tcg_wait_io_event and qemu_kvm_wait_io_event is exited
naturally rather than through a timeout.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/apic.c          |    1 +
 hw/pc.c            |    1 +
 hw/ppc.c           |    2 ++
 hw/sun4m.c         |   11 +++++++++--
 hw/sun4u.c         |    4 ++--
 target-s390x/kvm.c |    1 +
 6 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 2f8376a..97e3b2b 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -527,6 +527,7 @@ void apic_sipi(DeviceState *d)
         return;
     cpu_x86_load_seg_cache_sipi(s->cpu_env, s->sipi_vector);
     s->wait_for_sipi = 0;
+    qemu_cpu_kick(s->cpu_env);
 }
 
 static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
diff --git a/hw/pc.c b/hw/pc.c
index 4dfdc0b..fc1aa27 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -907,6 +907,7 @@ static void pc_cpu_reset(void *opaque)
 
     cpu_reset(env);
     env->halted = !cpu_is_bsp(env);
+    qemu_cpu_kick(env);
 }
 
 static CPUState *pc_new_cpu(const char *cpu_model)
diff --git a/hw/ppc.c b/hw/ppc.c
index 968aec1..de02d33 100644
--- a/hw/ppc.c
+++ b/hw/ppc.c
@@ -208,6 +208,7 @@ static void ppc970_set_irq (void *opaque, int pin, int level)
             } else {
                 LOG_IRQ("%s: restart the CPU\n", __func__);
                 env->halted = 0;
+                qemu_cpu_kick(env);
             }
             break;
         case PPC970_INPUT_HRESET:
@@ -300,6 +301,7 @@ static void ppc40x_set_irq (void *opaque, int pin, int level)
             } else {
                 LOG_IRQ("%s: restart the CPU\n", __func__);
                 env->halted = 0;
+                qemu_cpu_kick(env);
             }
             break;
         case PPC40x_INPUT_DEBUG:
diff --git a/hw/sun4m.c b/hw/sun4m.c
index 30e8a21..47d88be 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -253,15 +253,21 @@ void cpu_check_irqs(CPUState *env)
     }
 }
 
+static void cpu_kick_irq(CPUState *env)
+{
+    env->halted = 0;
+    cpu_check_irqs(env);
+    qemu_cpu_kick(env);
+}
+
 static void cpu_set_irq(void *opaque, int irq, int level)
 {
     CPUState *env = opaque;
 
     if (level) {
         trace_sun4m_cpu_set_irq_raise(irq);
-        env->halted = 0;
         env->pil_in |= 1 << irq;
-        cpu_check_irqs(env);
+        cpu_kick_irq(env);
     } else {
         trace_sun4m_cpu_set_irq_lower(irq);
         env->pil_in &= ~(1 << irq);
@@ -279,6 +285,7 @@ static void main_cpu_reset(void *opaque)
 
     cpu_reset(env);
     env->halted = 0;
+    qemu_cpu_kick(env);
 }
 
 static void secondary_cpu_reset(void *opaque)
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 90b1ce2..d282324 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -298,6 +298,7 @@ static void cpu_kick_irq(CPUState *env)
 {
     env->halted = 0;
     cpu_check_irqs(env);
+    qemu_cpu_kick(env);
 }
 
 static void cpu_set_irq(void *opaque, int irq, int level)
@@ -306,9 +307,8 @@ static void cpu_set_irq(void *opaque, int irq, int level)
 
     if (level) {
         CPUIRQ_DPRINTF("Raise CPU IRQ %d\n", irq);
-        env->halted = 0;
         env->pil_in |= 1 << irq;
-        cpu_check_irqs(env);
+        cpu_kick_irq(env);
     } else {
         CPUIRQ_DPRINTF("Lower CPU IRQ %d\n", irq);
         env->pil_in &= ~(1 << irq);
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 1702c46..fc90b8d 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -196,6 +196,7 @@ static void kvm_s390_interrupt_internal(CPUState *env, int type, uint32_t parm,
 
     env->halted = 0;
     env->exception_index = -1;
+    qemu_cpu_kick (env);
 
     kvmint.type = type;
     kvmint.parm = parm;
-- 
1.7.3.5

[Qemu-devel] Re: [CFT PATCH 05/12] always qemu_cpu_kick after unhalting a cpu

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 4253 bytes --]

On 2011-02-08 18:18, Paolo Bonzini wrote:
> This ensures env->halt_cond is broadcast, and the loop in
> qemu_tcg_wait_io_event and qemu_kvm_wait_io_event is exited
> naturally rather than through a timeout.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/apic.c          |    1 +
>  hw/pc.c            |    1 +
>  hw/ppc.c           |    2 ++
>  hw/sun4m.c         |   11 +++++++++--
>  hw/sun4u.c         |    4 ++--
>  target-s390x/kvm.c |    1 +
>  6 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/apic.c b/hw/apic.c
> index 2f8376a..97e3b2b 100644
> --- a/hw/apic.c
> +++ b/hw/apic.c
> @@ -527,6 +527,7 @@ void apic_sipi(DeviceState *d)
>          return;
>      cpu_x86_load_seg_cache_sipi(s->cpu_env, s->sipi_vector);
>      s->wait_for_sipi = 0;
> +    qemu_cpu_kick(s->cpu_env);
>  }
>  
>  static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
> diff --git a/hw/pc.c b/hw/pc.c
> index 4dfdc0b..fc1aa27 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -907,6 +907,7 @@ static void pc_cpu_reset(void *opaque)
>  
>      cpu_reset(env);
>      env->halted = !cpu_is_bsp(env);
> +    qemu_cpu_kick(env);

That should be unneeded, reset is performed with all vcpus stopped.

>  }
>  
>  static CPUState *pc_new_cpu(const char *cpu_model)
> diff --git a/hw/ppc.c b/hw/ppc.c
> index 968aec1..de02d33 100644
> --- a/hw/ppc.c
> +++ b/hw/ppc.c
> @@ -208,6 +208,7 @@ static void ppc970_set_irq (void *opaque, int pin, int level)
>              } else {
>                  LOG_IRQ("%s: restart the CPU\n", __func__);
>                  env->halted = 0;
> +                qemu_cpu_kick(env);
>              }
>              break;
>          case PPC970_INPUT_HRESET:
> @@ -300,6 +301,7 @@ static void ppc40x_set_irq (void *opaque, int pin, int level)
>              } else {
>                  LOG_IRQ("%s: restart the CPU\n", __func__);
>                  env->halted = 0;
> +                qemu_cpu_kick(env);
>              }
>              break;
>          case PPC40x_INPUT_DEBUG:
> diff --git a/hw/sun4m.c b/hw/sun4m.c
> index 30e8a21..47d88be 100644
> --- a/hw/sun4m.c
> +++ b/hw/sun4m.c
> @@ -253,15 +253,21 @@ void cpu_check_irqs(CPUState *env)
>      }
>  }
>  
> +static void cpu_kick_irq(CPUState *env)
> +{
> +    env->halted = 0;
> +    cpu_check_irqs(env);
> +    qemu_cpu_kick(env);
> +}
> +
>  static void cpu_set_irq(void *opaque, int irq, int level)
>  {
>      CPUState *env = opaque;
>  
>      if (level) {
>          trace_sun4m_cpu_set_irq_raise(irq);
> -        env->halted = 0;
>          env->pil_in |= 1 << irq;
> -        cpu_check_irqs(env);
> +        cpu_kick_irq(env);
>      } else {
>          trace_sun4m_cpu_set_irq_lower(irq);
>          env->pil_in &= ~(1 << irq);
> @@ -279,6 +285,7 @@ static void main_cpu_reset(void *opaque)
>  
>      cpu_reset(env);
>      env->halted = 0;
> +    qemu_cpu_kick(env);

Same here.

>  }
>  
>  static void secondary_cpu_reset(void *opaque)
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index 90b1ce2..d282324 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -298,6 +298,7 @@ static void cpu_kick_irq(CPUState *env)
>  {
>      env->halted = 0;
>      cpu_check_irqs(env);
> +    qemu_cpu_kick(env);
>  }
>  
>  static void cpu_set_irq(void *opaque, int irq, int level)
> @@ -306,9 +307,8 @@ static void cpu_set_irq(void *opaque, int irq, int level)
>  
>      if (level) {
>          CPUIRQ_DPRINTF("Raise CPU IRQ %d\n", irq);
> -        env->halted = 0;
>          env->pil_in |= 1 << irq;
> -        cpu_check_irqs(env);
> +        cpu_kick_irq(env);
>      } else {
>          CPUIRQ_DPRINTF("Lower CPU IRQ %d\n", irq);
>          env->pil_in &= ~(1 << irq);
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 1702c46..fc90b8d 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -196,6 +196,7 @@ static void kvm_s390_interrupt_internal(CPUState *env, int type, uint32_t parm,
>  
>      env->halted = 0;
>      env->exception_index = -1;
> +    qemu_cpu_kick (env);
>  
>      kvmint.type = type;
>      kvmint.parm = parm;

Makes sense otherwise.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

[Qemu-devel] [CFT PATCH 06/12] exit round-robin vcpu loop if cpu->stopped is true

[Paolo Bonzini]

Sometimes vcpus are stopped directly without going through ->stop = 1.
Exit the VCPU execution loop in this case as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index c1d0ceb..5b13961 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1061,7 +1061,7 @@ bool cpu_exec_all(void)
             if (r == EXCP_DEBUG) {
                 break;
             }
-        } else if (env->stop) {
+        } else if (env->stop || env->stopped) {
             break;
         }
     }
-- 
1.7.3.5

[Qemu-devel] Re: [CFT PATCH 06/12] exit round-robin vcpu loop if cpu->stopped is true

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 795 bytes --]

On 2011-02-08 18:18, Paolo Bonzini wrote:
> Sometimes vcpus are stopped directly without going through ->stop = 1.
> Exit the VCPU execution loop in this case as well.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  cpus.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index c1d0ceb..5b13961 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1061,7 +1061,7 @@ bool cpu_exec_all(void)
>              if (r == EXCP_DEBUG) {
>                  break;
>              }
> -        } else if (env->stop) {
> +        } else if (env->stop || env->stopped) {
>              break;
>          }
>      }

Hmm, does this path actually trigger? If yes, does it happen to obsolete
the global exit_request hack?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

[Qemu-devel] Re: [CFT PATCH 06/12] exit round-robin vcpu loop if cpu->stopped is true

[Paolo Bonzini]

On 02/08/2011 09:24 PM, Jan Kiszka wrote:
> Hmm, does this path actually trigger? If yes, does it happen to obsolete
> the global exit_request hack?

No idea, I wanted to make the initial work as mechanical as possible. 
By inspection, cpu_stop_current is doing

          cpu_single_env->stopped = 1;
          cpu_exit(cpu_single_env);

without setting ->stop.  (I'm changing that in patch 7, but I'm setting 
->stop to 0, not 1).

Paolo

[Qemu-devel] Re: [CFT PATCH 06/12] exit round-robin vcpu loop if cpu->stopped is true

[Paolo Bonzini]

On 02/08/2011 09:24 PM, Jan Kiszka wrote:
> Hmm, does this path actually trigger? If yes, does it happen to obsolete
> the global exit_request hack?

No idea, I wanted to make the initial work as mechanical as possible. 
By inspection, cpu_stop_current is doing

          cpu_single_env->stopped = 1;
          cpu_exit(cpu_single_env);

without setting ->stop.  (I'm changing that in patch 7, but I'm setting 
->stop to 0, not 1).

Paolo

[Qemu-devel] Re: [CFT PATCH 06/12] exit round-robin vcpu loop if cpu->stopped is true

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

On 2011-02-09 08:24, Paolo Bonzini wrote:
> On 02/08/2011 09:24 PM, Jan Kiszka wrote:
>> Hmm, does this path actually trigger? If yes, does it happen to obsolete
>> the global exit_request hack?
> 
> No idea, I wanted to make the initial work as mechanical as possible. By
> inspection, cpu_stop_current is doing
> 
>          cpu_single_env->stopped = 1;
>          cpu_exit(cpu_single_env);
> 
> without setting ->stop.  (I'm changing that in patch 7, but I'm setting
> ->stop to 0, not 1).

Checked my own patches again: :) exit_request is not obsoleted this way,
at least as long as we have !CONFIG_IOTHREAD hanging around.

Also, I don't see any compelling reason now why that test should be
unneeded. Rather, this looks like a sleeping race between asynchronous
and synchronous vcpu stop, probably papered over by polling the cond
variables so far.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

[Qemu-devel] [CFT PATCH 07/12] always signal pause_cond after stopping a VCPU

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/cpus.c b/cpus.c
index 5b13961..ee2a6cc 100644
--- a/cpus.c
+++ b/cpus.c
@@ -980,8 +980,10 @@ static void qemu_system_vmstop_request(int reason)
 void cpu_stop_current(void)
 {
     if (cpu_single_env) {
+        cpu_single_env->stop = 0;
         cpu_single_env->stopped = 1;
         cpu_exit(cpu_single_env);
+        qemu_cond_signal(&qemu_pause_cond);
     }
 }
 
-- 
1.7.3.5

[Qemu-devel] [CFT PATCH 08/12] do not use timedwait on qemu_halt_cond

[Paolo Bonzini]

The following conditions can cause cpu_has_work(env) to become true:

- env->queued_work_first: run_on_cpu is already kicking the VCPU

- env->stop = 1: pause_all_vcpus is already kicking the VCPU

- env->stopped = 0: resume_all_vcpus is already kicking the VCPU

- vm_running = 1: vm_start is calling resume_all_vcpus

- env->halted = 0: see previous patch

- qemu_cpu_has_work(env): when it becomes true, board code should set
  env->halted = 0 too.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index ee2a6cc..be3d82d 100644
--- a/cpus.c
+++ b/cpus.c
@@ -746,7 +746,7 @@ static void qemu_tcg_wait_io_event(void)
     CPUState *env;
 
     while (!any_cpu_has_work())
-        qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000);
+        qemu_cond_wait(tcg_halt_cond, &qemu_global_mutex);
 
     qemu_mutex_unlock(&qemu_global_mutex);
 
@@ -768,7 +768,7 @@ static void qemu_tcg_wait_io_event(void)
 static void qemu_kvm_wait_io_event(CPUState *env)
 {
     while (!cpu_has_work(env))
-        qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
+        qemu_cond_wait(env->halt_cond, &qemu_global_mutex);
 
     qemu_kvm_eat_signals(env);
     qemu_wait_io_event_common(env);
-- 
1.7.3.5

[Qemu-devel] [CFT PATCH 09/12] do not use timedwait on qemu_system_cond

[Paolo Bonzini]

qemu_main_loop_start is the only place where qemu_system_ready is set
to 1, and it signals qemu_system_cond.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index be3d82d..3d12c50 100644
--- a/cpus.c
+++ b/cpus.c
@@ -798,7 +798,7 @@ static void *kvm_cpu_thread_fn(void *arg)
 
     /* and wait for machine initialization */
     while (!qemu_system_ready)
-        qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+        qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex);
 
     while (1) {
         if (cpu_can_run(env))
@@ -824,7 +824,7 @@ static void *tcg_cpu_thread_fn(void *arg)
 
     /* and wait for machine initialization */
     while (!qemu_system_ready)
-        qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+        qemu_cond_wait(&qemu_system_cond, &qemu_global_mutex);
 
     while (1) {
         cpu_exec_all();
-- 
1.7.3.5

[Qemu-devel] [CFT PATCH 10/12] do not use timedwait on qemu_pause_cond

[Paolo Bonzini]

all_vcpus_paused can start returning true after penv->stopped changes
from 0 to 1.  When this is done, qemu_pause_cond is always signaled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index 3d12c50..97ba166 100644
--- a/cpus.c
+++ b/cpus.c
@@ -904,7 +904,7 @@ void pause_all_vcpus(void)
     }
 
     while (!all_vcpus_paused()) {
-        qemu_cond_timedwait(&qemu_pause_cond, &qemu_global_mutex, 100);
+        qemu_cond_wait(&qemu_pause_cond, &qemu_global_mutex);
         penv = first_cpu;
         while (penv) {
             qemu_cpu_kick(penv);
-- 
1.7.3.5

[Qemu-devel] [CFT PATCH 11/12] do not use timedwait on qemu_cpu_cond

[Paolo Bonzini]

Whenever env->created becomes true, qemu_cpu_cond is signaled by
{kvm,tcg}_cpu_thread_fn.

I change qemu_cond_signal to qemu_cond_broadcast because right now
there is only one listened but, if there were more, there would be
no reason to wake up only one.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cpus.c b/cpus.c
index 97ba166..67fd672 100644
--- a/cpus.c
+++ b/cpus.c
@@ -794,7 +794,7 @@ static void *kvm_cpu_thread_fn(void *arg)
 
     /* signal CPU creation */
     env->created = 1;
-    qemu_cond_signal(&qemu_cpu_cond);
+    qemu_cond_broadcast(&qemu_cpu_cond);
 
     /* and wait for machine initialization */
     while (!qemu_system_ready)
@@ -820,7 +820,7 @@ static void *tcg_cpu_thread_fn(void *arg)
     qemu_mutex_lock(&qemu_global_mutex);
     for (env = first_cpu; env != NULL; env = env->next_cpu)
         env->created = 1;
-    qemu_cond_signal(&qemu_cpu_cond);
+    qemu_cond_broadcast(&qemu_cpu_cond);
 
     /* and wait for machine initialization */
     while (!qemu_system_ready)
@@ -935,7 +935,7 @@ static void tcg_init_vcpu(void *_env)
         qemu_cond_init(env->halt_cond);
         qemu_thread_create(env->thread, tcg_cpu_thread_fn, env);
         while (env->created == 0)
-            qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
+            qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
         tcg_cpu_thread = env->thread;
         tcg_halt_cond = env->halt_cond;
     } else {
@@ -951,7 +951,7 @@ static void kvm_start_vcpu(CPUState *env)
     qemu_cond_init(env->halt_cond);
     qemu_thread_create(env->thread, kvm_cpu_thread_fn, env);
     while (env->created == 0)
-        qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
+        qemu_cond_wait(&qemu_cpu_cond, &qemu_global_mutex);
 }
 
 void qemu_init_vcpu(void *_env)
-- 
1.7.3.5

[Qemu-devel] [CFT PATCH 12/12] iothread stops the vcpu thread via IPI

[Paolo Bonzini]

This "if" has no reason to exist in the iothread world, and it breaks
Wine because of the slowness of the thread primitives there.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpus.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/cpus.c b/cpus.c
index 67fd672..3d74ad7 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1053,8 +1053,10 @@ bool cpu_exec_all(void)
         qemu_clock_enable(vm_clock,
                           (env->singlestep_enabled & SSTEP_NOTIMER) == 0);
 
+#ifndef CONFIG_IOTHREAD
         if (qemu_alarm_pending())
             break;
+#endif
         if (cpu_can_run(env)) {
             r = qemu_cpu_exec(env);
             if (kvm_enabled()) {
-- 
1.7.3.5

Re: [Qemu-devel] [CFT PATCH 00/12] Tricky parts of my iothread-for-win32 stuff

[Aurelien Jarno]

On Tue, Feb 08, 2011 at 06:18:17PM +0100, Paolo Bonzini wrote:
> Ok, so let's start the dance...
> 
> These are the generic parts of the iothread-for-win32 work I made
> last year, rebased to today's qemu (qemu-kvm.git/uq/master merged
> with qemu.git/master, because it conflicts with some patches from Jan).
> I never tested them too much, and that's one of the reasons why I never
> submitted the stuff.  Perhaps with some help they can make it this time.
> 
> Testing I made now is still a bit limited, but the patches can boot
> RHEL and Fedora under both Windows iothread and Linux iothread.  FWIW,
> under Wine I need iothread to boot Linux.
> 
> The main idea is to replace timedwaits with waits.  First because the
> timedwaits were such only out of laziness; second because timedwait
> is a pain to implement for Win32 where we have to manually implement
> condvars.

Thanks a lot for those patches, if we don't have anybody picking up the
work with this basis, we can definitely consider Windows support dead.

> Anthony Liguori (1):
>   io-thread: make sure to initialize qemu_work_cond and qemu_cpu_cond
> 
> Paolo Bonzini (11):
>   cris, microblaze: use cpu_has_work
>   inline cpu_halted into sole caller
>   change qemu_thread_equal API to always compare with current thread
>   always qemu_cpu_kick after unhalting a cpu
>   exit round-robin vcpu loop if cpu->stopped is true
>   always signal pause_cond after stopping a VCPU
>   do not use timedwait on qemu_halt_cond
>   do not use timedwait on qemu_system_cond
>   do not use timedwait on qemu_pause_cond
>   do not use timedwait on qemu_cpu_cond
>   iothread stops the vcpu thread via IPI
> 
>  cpu-defs.h               |    1 +
>  cpu-exec.c               |    9 +++++++--
>  cpus.c                   |   38 +++++++++++++++++++-------------------
>  hw/apic.c                |    1 +
>  hw/ppc.c                 |    2 ++
>  hw/sun4m.c               |   11 +++++++++--
>  hw/sun4u.c               |    4 ++--
>  qemu-thread.c            |    4 ++--
>  qemu-thread.h            |    2 +-
>  target-alpha/exec.h      |   11 -----------
>  target-arm/exec.h        |   13 -------------
>  target-cris/exec.h       |   13 -------------
>  target-i386/exec.h       |   12 ------------
>  target-m68k/exec.h       |   10 ----------
>  target-microblaze/exec.h |   13 -------------
>  target-mips/exec.h       |   11 -----------
>  target-ppc/exec.h        |   11 -----------
>  target-s390x/exec.h      |   12 ------------
>  target-s390x/kvm.c       |    1 +
>  target-sh4/cpu.h         |    1 -
>  target-sh4/exec.h        |   11 -----------
>  target-sparc/exec.h      |   10 ----------
>  22 files changed, 45 insertions(+), 156 deletions(-)
> 
> -- 
> 1.7.3.5
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] Re: [CFT PATCH 00/12] Tricky parts of my iothread-for-win32 stuff

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]

On 2011-02-08 18:18, Paolo Bonzini wrote:
> Ok, so let's start the dance...
> 
> These are the generic parts of the iothread-for-win32 work I made
> last year, rebased to today's qemu (qemu-kvm.git/uq/master merged
> with qemu.git/master, because it conflicts with some patches from Jan).

More breakage to come :). But that can be sorted out.

> I never tested them too much, and that's one of the reasons why I never
> submitted the stuff.  Perhaps with some help they can make it this time.
> 
> Testing I made now is still a bit limited, but the patches can boot
> RHEL and Fedora under both Windows iothread and Linux iothread.  FWIW,
> under Wine I need iothread to boot Linux.
> 
> The main idea is to replace timedwaits with waits.  First because the
> timedwaits were such only out of laziness; second because timedwait
> is a pain to implement for Win32 where we have to manually implement
> condvars.

I bet that timed polling was just to work around early fragility of the
iothread code /wrt TCG. qemu-kvm does not poll either (but uses an
"interesting" mixture of !IOTHREAD and private code instead...). The
conversion makes sense, though I haven't looked at every detail.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]