All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@web.de>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [CFT PATCH 03/12] inline cpu_halted into sole caller
Date: Tue, 08 Feb 2011 21:24:46 +0100	[thread overview]
Message-ID: <4D51A68E.8070501@web.de> (raw)
In-Reply-To: <1297185509-20996-4-git-send-email-pbonzini@redhat.com>

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

  reply	other threads:[~2011-02-08 20:24 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-08 17:18 [Qemu-devel] [CFT PATCH 00/12] Tricky parts of my iothread-for-win32 stuff Paolo Bonzini
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 01/12] io-thread: make sure to initialize qemu_work_cond and qemu_cpu_cond Paolo Bonzini
2011-02-14 19:21   ` Anthony Liguori
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 02/12] cris, microblaze: use cpu_has_work Paolo Bonzini
2011-02-08 19:42   ` Edgar E. Iglesias
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 03/12] inline cpu_halted into sole caller Paolo Bonzini
2011-02-08 20:24   ` Jan Kiszka [this message]
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 04/12] change qemu_thread_equal API to always compare with current thread Paolo Bonzini
2011-02-08 20:25   ` [Qemu-devel] " Jan Kiszka
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 05/12] always qemu_cpu_kick after unhalting a cpu Paolo Bonzini
2011-02-08 20:25   ` [Qemu-devel] " Jan Kiszka
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 06/12] exit round-robin vcpu loop if cpu->stopped is true Paolo Bonzini
2011-02-08 20:24   ` [Qemu-devel] " Jan Kiszka
2011-02-09  7:24     ` Paolo Bonzini
2011-02-09  7:24     ` Paolo Bonzini
2011-02-09  8:40       ` Jan Kiszka
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 07/12] always signal pause_cond after stopping a VCPU Paolo Bonzini
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 08/12] do not use timedwait on qemu_halt_cond Paolo Bonzini
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 09/12] do not use timedwait on qemu_system_cond Paolo Bonzini
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 10/12] do not use timedwait on qemu_pause_cond Paolo Bonzini
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 11/12] do not use timedwait on qemu_cpu_cond Paolo Bonzini
2011-02-08 17:18 ` [Qemu-devel] [CFT PATCH 12/12] iothread stops the vcpu thread via IPI Paolo Bonzini
2011-02-08 19:31 ` [Qemu-devel] [CFT PATCH 00/12] Tricky parts of my iothread-for-win32 stuff Aurelien Jarno
2011-02-08 20:38 ` [Qemu-devel] " Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D51A68E.8070501@web.de \
    --to=jan.kiszka@web.de \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.