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

[Jan Kiszka]

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 <address@hidden>
> ---
>  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

signature.asc
Description: OpenPGP digital signature