Patchwork [1/2] cpu-exec: rid cs_base of TranslationBlock

login
register
mail settings
Submitter liguang
Date April 24, 2013, 1:48 a.m.
Message ID <1366768096-2846-1-git-send-email-lig.fnst@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/239028/
State New
Headers show

Comments

liguang - April 24, 2013, 1:48 a.m.
cs_base is only meaningful for target-i386/sparc,
so, get rid of cs_base for other target

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 cpu-exec.c              |   26 ++++++++++++++++++--------
 exec.c                  |    6 +++---
 hw/i386/kvmvapic.c      |    6 ++----
 include/exec/exec-all.h |    5 +++--
 target-i386/cpu.h       |    6 ++----
 translate-all.c         |   24 ++++++++++++------------
 6 files changed, 40 insertions(+), 33 deletions(-)
Paolo Bonzini - April 24, 2013, 6:36 a.m.
Il 24/04/2013 03:48, liguang ha scritto:
> cs_base is only meaningful for target-i386/sparc,
> so, get rid of cs_base for other target

This is really ugly, we're trying to get less target-dependent code
outside target-*, not more.

Also, please limit the number of people that you CC.

Paolo

> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  cpu-exec.c              |   26 ++++++++++++++++++--------
>  exec.c                  |    6 +++---
>  hw/i386/kvmvapic.c      |    6 ++----
>  include/exec/exec-all.h |    5 +++--
>  target-i386/cpu.h       |    6 ++----
>  translate-all.c         |   24 ++++++++++++------------
>  6 files changed, 40 insertions(+), 33 deletions(-)
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index 31c089d..f3c1d1c 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -84,7 +84,7 @@ static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
>      if (max_cycles > CF_COUNT_MASK)
>          max_cycles = CF_COUNT_MASK;
>  
> -    tb = tb_gen_code(env, orig_tb->pc, orig_tb->cs_base, orig_tb->flags,
> +    tb = tb_gen_code(env, orig_tb->pc, orig_tb->flags,
>                       max_cycles);
>      cpu->current_tb = tb;
>      /* execute the generated code */
> @@ -96,7 +96,6 @@ static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
>  
>  static TranslationBlock *tb_find_slow(CPUArchState *env,
>                                        target_ulong pc,
> -                                      target_ulong cs_base,
>                                        uint64_t flags)
>  {
>      TranslationBlock *tb, **ptb1;
> @@ -117,7 +116,12 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
>              goto not_found;
>          if (tb->pc == pc &&
>              tb->page_addr[0] == phys_page1 &&
> -            tb->cs_base == cs_base &&
> +#if defined(TARGET_I386)
> +            tb->cs_base == env->segs[R_CS].base &&
> +#endif
> +#if defined(TARGET_SPARC)
> +            tb->cs_base == env->npc &&
> +#endif
>              tb->flags == flags) {
>              /* check next page if needed */
>              if (tb->page_addr[1] != -1) {
> @@ -136,7 +140,7 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
>      }
>   not_found:
>     /* if no translated code available, then translate it now */
> -    tb = tb_gen_code(env, pc, cs_base, flags, 0);
> +    tb = tb_gen_code(env, pc, flags, 0);
>  
>   found:
>      /* Move the last found TB to the head of the list */
> @@ -153,17 +157,23 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
>  static inline TranslationBlock *tb_find_fast(CPUArchState *env)
>  {
>      TranslationBlock *tb;
> -    target_ulong cs_base, pc;
> +    target_ulong pc;
>      int flags;
>  
>      /* we record a subset of the CPU state. It will
>         always be the same before a given translated block
>         is executed. */
> -    cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
> +    cpu_get_tb_cpu_state(env, &pc, &flags);
>      tb = env->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
> -    if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
> +    if (unlikely(!tb || tb->pc != pc ||
> +#if defined(TARGET_I386)
> +                 tb->cs_base != env->segs[R_CS].base ||
> +#endif
> +#if defined(TARGET_SPARC)
> +                 tb->cs_base != env->npc ||
> +#endif
>                   tb->flags != flags)) {
> -        tb = tb_find_slow(env, pc, cs_base, flags);
> +        tb = tb_find_slow(env, pc, flags);
>      }
>      return tb;
>  }
> diff --git a/exec.c b/exec.c
> index fa1e0c3..a14db2c 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1471,7 +1471,7 @@ static const MemoryRegionOps notdirty_mem_ops = {
>  static void check_watchpoint(int offset, int len_mask, int flags)
>  {
>      CPUArchState *env = cpu_single_env;
> -    target_ulong pc, cs_base;
> +    target_ulong pc;
>      target_ulong vaddr;
>      CPUWatchpoint *wp;
>      int cpu_flags;
> @@ -1495,8 +1495,8 @@ static void check_watchpoint(int offset, int len_mask, int flags)
>                      env->exception_index = EXCP_DEBUG;
>                      cpu_loop_exit(env);
>                  } else {
> -                    cpu_get_tb_cpu_state(env, &pc, &cs_base, &cpu_flags);
> -                    tb_gen_code(env, pc, cs_base, cpu_flags, 1);
> +                    cpu_get_tb_cpu_state(env, &pc, &cpu_flags);
> +                    tb_gen_code(env, pc, cpu_flags, 1);
>                      cpu_resume_from_signal(env, NULL);
>                  }
>              }
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index ed9b448..8b4260e 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -388,7 +388,6 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
>      uint8_t opcode[2];
>      uint32_t imm32;
>      target_ulong current_pc = 0;
> -    target_ulong current_cs_base = 0;
>      int current_flags = 0;
>  
>      if (smp_cpus == 1) {
> @@ -399,8 +398,7 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
>  
>      if (!kvm_enabled()) {
>          cpu_restore_state(env, env->mem_io_pc);
> -        cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
> -                             &current_flags);
> +        cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
>      }
>  
>      pause_all_vcpus();
> @@ -440,7 +438,7 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
>  
>      if (!kvm_enabled()) {
>          cs->current_tb = NULL;
> -        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
> +        tb_gen_code(env, current_pc, current_flags, 1);
>          cpu_resume_from_signal(env, NULL);
>      }
>  }
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index e856191..560a7d1 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -85,8 +85,7 @@ bool cpu_restore_state(CPUArchState *env, uintptr_t searched_pc);
>  void QEMU_NORETURN cpu_resume_from_signal(CPUArchState *env1, void *puc);
>  void QEMU_NORETURN cpu_io_recompile(CPUArchState *env, uintptr_t retaddr);
>  TranslationBlock *tb_gen_code(CPUArchState *env, 
> -                              target_ulong pc, target_ulong cs_base, int flags,
> -                              int cflags);
> +                              target_ulong pc, int flags, int cflags);
>  void cpu_exec_init(CPUArchState *env);
>  void QEMU_NORETURN cpu_loop_exit(CPUArchState *env1);
>  int page_unprotect(target_ulong address, uintptr_t pc, void *puc);
> @@ -135,7 +134,9 @@ static inline void tlb_flush(CPUArchState *env, int flush_global)
>  
>  struct TranslationBlock {
>      target_ulong pc;   /* simulated PC corresponding to this block (EIP + CS base) */
> +#if defined(TARGET_I386) || defined(TARGET_SPARC)
>      target_ulong cs_base; /* CS base for this block */
> +#endif
>      uint64_t flags; /* flags defining in which context the code was generated */
>      uint16_t size;      /* size of target code for this block (1 <=
>                             size <= TARGET_PAGE_SIZE) */
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 2b4e319..3ce1b2e 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -1188,11 +1188,9 @@ static inline void cpu_pc_from_tb(CPUX86State *env, TranslationBlock *tb)
>      env->eip = tb->pc - tb->cs_base;
>  }
>  
> -static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc,
> -                                        target_ulong *cs_base, int *flags)
> +static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc, int *flags)
>  {
> -    *cs_base = env->segs[R_CS].base;
> -    *pc = *cs_base + env->eip;
> +    *pc = env->segs[R_CS].base + env->eip;
>      *flags = env->hflags |
>          (env->eflags & (IOPL_MASK | TF_MASK | RF_MASK | VM_MASK | AC_MASK));
>  }
> diff --git a/translate-all.c b/translate-all.c
> index a98c646..f9efbbd 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -931,8 +931,7 @@ static void build_page_bitmap(PageDesc *p)
>  }
>  
>  TranslationBlock *tb_gen_code(CPUArchState *env,
> -                              target_ulong pc, target_ulong cs_base,
> -                              int flags, int cflags)
> +                              target_ulong pc, int flags, int cflags)
>  {
>      TranslationBlock *tb;
>      uint8_t *tc_ptr;
> @@ -952,7 +951,12 @@ TranslationBlock *tb_gen_code(CPUArchState *env,
>      }
>      tc_ptr = tcg_ctx.code_gen_ptr;
>      tb->tc_ptr = tc_ptr;
> -    tb->cs_base = cs_base;
> +#if defined(TARGET_I386)
> +    tb->cs_base = env->segs[R_CS].base;
> +#endif
> +#if defined(TARGET_SPARC)
> +    tb->cs_base = env->npc;
> +#endif
>      tb->flags = flags;
>      tb->cflags = cflags;
>      cpu_gen_code(env, tb, &code_gen_size);
> @@ -1007,7 +1011,6 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
>      TranslationBlock *current_tb = NULL;
>      int current_tb_modified = 0;
>      target_ulong current_pc = 0;
> -    target_ulong current_cs_base = 0;
>      int current_flags = 0;
>  #endif /* TARGET_HAS_PRECISE_SMC */
>  
> @@ -1063,8 +1066,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
>  
>                  current_tb_modified = 1;
>                  cpu_restore_state_from_tb(current_tb, env, env->mem_io_pc);
> -                cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
> -                                     &current_flags);
> +                cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
>              }
>  #endif /* TARGET_HAS_PRECISE_SMC */
>              /* we need to do that to handle the case where a signal
> @@ -1099,7 +1101,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
>             modifying the memory. It will ensure that it cannot modify
>             itself */
>          cpu->current_tb = NULL;
> -        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
> +        tb_gen_code(env, current_pc, current_flags, 1);
>          cpu_resume_from_signal(env, NULL);
>      }
>  #endif
> @@ -1149,7 +1151,6 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
>      CPUState *cpu = NULL;
>      int current_tb_modified = 0;
>      target_ulong current_pc = 0;
> -    target_ulong current_cs_base = 0;
>      int current_flags = 0;
>  #endif
>  
> @@ -1181,8 +1182,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
>  
>              current_tb_modified = 1;
>              cpu_restore_state_from_tb(current_tb, env, pc);
> -            cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
> -                                 &current_flags);
> +            cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
>          }
>  #endif /* TARGET_HAS_PRECISE_SMC */
>          tb_phys_invalidate(tb, addr);
> @@ -1195,7 +1195,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
>             modifying the memory. It will ensure that it cannot modify
>             itself */
>          cpu->current_tb = NULL;
> -        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
> +        tb_gen_code(env, current_pc, current_flags, 1);
>          cpu_resume_from_signal(env, puc);
>      }
>  #endif
> @@ -1463,7 +1463,7 @@ void cpu_io_recompile(CPUArchState *env, uintptr_t retaddr)
>      tb_phys_invalidate(tb, -1);
>      /* FIXME: In theory this could raise an exception.  In practice
>         we have already translated the block once so it's probably ok.  */
> -    tb_gen_code(env, pc, cs_base, flags, cflags);
> +    tb_gen_code(env, pc, flags, cflags);
>      /* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not
>         the first in the TB) then we end up generating a whole new TB and
>         repeating the fault, which is horribly inefficient.
>
Aurelien Jarno - April 24, 2013, 7:11 a.m.
On Wed, Apr 24, 2013 at 08:36:54AM +0200, Paolo Bonzini wrote:
> Il 24/04/2013 03:48, liguang ha scritto:
> > cs_base is only meaningful for target-i386/sparc,
> > so, get rid of cs_base for other target
> 
> This is really ugly, we're trying to get less target-dependent code
> outside target-*, not more.

Fully agreed. It also breaks the interface between the target and
cpu-exec.c by assuming tb->cs_base will always be env->segs[R_CS].base.

The only cleanup that can be done here is to rename cs_base into flags2
to make it less target dependent, and the code in cpu-exec.c should just
guarantee to choose tbs which match both flags and flags2 without
actually caring about the meaning of the values.

Even that way, this is the kind of cleanup touching a lot of code
without real benefit, except maybe for sparc which currently "abuse"
cs_base.

> Also, please limit the number of people that you CC.
> 
> Paolo
> 
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > ---
> >  cpu-exec.c              |   26 ++++++++++++++++++--------
> >  exec.c                  |    6 +++---
> >  hw/i386/kvmvapic.c      |    6 ++----
> >  include/exec/exec-all.h |    5 +++--
> >  target-i386/cpu.h       |    6 ++----
> >  translate-all.c         |   24 ++++++++++++------------
> >  6 files changed, 40 insertions(+), 33 deletions(-)
> > 
> > diff --git a/cpu-exec.c b/cpu-exec.c
> > index 31c089d..f3c1d1c 100644
> > --- a/cpu-exec.c
> > +++ b/cpu-exec.c
> > @@ -84,7 +84,7 @@ static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
> >      if (max_cycles > CF_COUNT_MASK)
> >          max_cycles = CF_COUNT_MASK;
> >  
> > -    tb = tb_gen_code(env, orig_tb->pc, orig_tb->cs_base, orig_tb->flags,
> > +    tb = tb_gen_code(env, orig_tb->pc, orig_tb->flags,
> >                       max_cycles);
> >      cpu->current_tb = tb;
> >      /* execute the generated code */
> > @@ -96,7 +96,6 @@ static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
> >  
> >  static TranslationBlock *tb_find_slow(CPUArchState *env,
> >                                        target_ulong pc,
> > -                                      target_ulong cs_base,
> >                                        uint64_t flags)
> >  {
> >      TranslationBlock *tb, **ptb1;
> > @@ -117,7 +116,12 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
> >              goto not_found;
> >          if (tb->pc == pc &&
> >              tb->page_addr[0] == phys_page1 &&
> > -            tb->cs_base == cs_base &&
> > +#if defined(TARGET_I386)
> > +            tb->cs_base == env->segs[R_CS].base &&
> > +#endif
> > +#if defined(TARGET_SPARC)
> > +            tb->cs_base == env->npc &&
> > +#endif
> >              tb->flags == flags) {
> >              /* check next page if needed */
> >              if (tb->page_addr[1] != -1) {
> > @@ -136,7 +140,7 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
> >      }
> >   not_found:
> >     /* if no translated code available, then translate it now */
> > -    tb = tb_gen_code(env, pc, cs_base, flags, 0);
> > +    tb = tb_gen_code(env, pc, flags, 0);
> >  
> >   found:
> >      /* Move the last found TB to the head of the list */
> > @@ -153,17 +157,23 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
> >  static inline TranslationBlock *tb_find_fast(CPUArchState *env)
> >  {
> >      TranslationBlock *tb;
> > -    target_ulong cs_base, pc;
> > +    target_ulong pc;
> >      int flags;
> >  
> >      /* we record a subset of the CPU state. It will
> >         always be the same before a given translated block
> >         is executed. */
> > -    cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
> > +    cpu_get_tb_cpu_state(env, &pc, &flags);
> >      tb = env->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
> > -    if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
> > +    if (unlikely(!tb || tb->pc != pc ||
> > +#if defined(TARGET_I386)
> > +                 tb->cs_base != env->segs[R_CS].base ||
> > +#endif
> > +#if defined(TARGET_SPARC)
> > +                 tb->cs_base != env->npc ||
> > +#endif
> >                   tb->flags != flags)) {
> > -        tb = tb_find_slow(env, pc, cs_base, flags);
> > +        tb = tb_find_slow(env, pc, flags);
> >      }
> >      return tb;
> >  }
> > diff --git a/exec.c b/exec.c
> > index fa1e0c3..a14db2c 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1471,7 +1471,7 @@ static const MemoryRegionOps notdirty_mem_ops = {
> >  static void check_watchpoint(int offset, int len_mask, int flags)
> >  {
> >      CPUArchState *env = cpu_single_env;
> > -    target_ulong pc, cs_base;
> > +    target_ulong pc;
> >      target_ulong vaddr;
> >      CPUWatchpoint *wp;
> >      int cpu_flags;
> > @@ -1495,8 +1495,8 @@ static void check_watchpoint(int offset, int len_mask, int flags)
> >                      env->exception_index = EXCP_DEBUG;
> >                      cpu_loop_exit(env);
> >                  } else {
> > -                    cpu_get_tb_cpu_state(env, &pc, &cs_base, &cpu_flags);
> > -                    tb_gen_code(env, pc, cs_base, cpu_flags, 1);
> > +                    cpu_get_tb_cpu_state(env, &pc, &cpu_flags);
> > +                    tb_gen_code(env, pc, cpu_flags, 1);
> >                      cpu_resume_from_signal(env, NULL);
> >                  }
> >              }
> > diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> > index ed9b448..8b4260e 100644
> > --- a/hw/i386/kvmvapic.c
> > +++ b/hw/i386/kvmvapic.c
> > @@ -388,7 +388,6 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
> >      uint8_t opcode[2];
> >      uint32_t imm32;
> >      target_ulong current_pc = 0;
> > -    target_ulong current_cs_base = 0;
> >      int current_flags = 0;
> >  
> >      if (smp_cpus == 1) {
> > @@ -399,8 +398,7 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
> >  
> >      if (!kvm_enabled()) {
> >          cpu_restore_state(env, env->mem_io_pc);
> > -        cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
> > -                             &current_flags);
> > +        cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
> >      }
> >  
> >      pause_all_vcpus();
> > @@ -440,7 +438,7 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
> >  
> >      if (!kvm_enabled()) {
> >          cs->current_tb = NULL;
> > -        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
> > +        tb_gen_code(env, current_pc, current_flags, 1);
> >          cpu_resume_from_signal(env, NULL);
> >      }
> >  }
> > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> > index e856191..560a7d1 100644
> > --- a/include/exec/exec-all.h
> > +++ b/include/exec/exec-all.h
> > @@ -85,8 +85,7 @@ bool cpu_restore_state(CPUArchState *env, uintptr_t searched_pc);
> >  void QEMU_NORETURN cpu_resume_from_signal(CPUArchState *env1, void *puc);
> >  void QEMU_NORETURN cpu_io_recompile(CPUArchState *env, uintptr_t retaddr);
> >  TranslationBlock *tb_gen_code(CPUArchState *env, 
> > -                              target_ulong pc, target_ulong cs_base, int flags,
> > -                              int cflags);
> > +                              target_ulong pc, int flags, int cflags);
> >  void cpu_exec_init(CPUArchState *env);
> >  void QEMU_NORETURN cpu_loop_exit(CPUArchState *env1);
> >  int page_unprotect(target_ulong address, uintptr_t pc, void *puc);
> > @@ -135,7 +134,9 @@ static inline void tlb_flush(CPUArchState *env, int flush_global)
> >  
> >  struct TranslationBlock {
> >      target_ulong pc;   /* simulated PC corresponding to this block (EIP + CS base) */
> > +#if defined(TARGET_I386) || defined(TARGET_SPARC)
> >      target_ulong cs_base; /* CS base for this block */
> > +#endif
> >      uint64_t flags; /* flags defining in which context the code was generated */
> >      uint16_t size;      /* size of target code for this block (1 <=
> >                             size <= TARGET_PAGE_SIZE) */
> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > index 2b4e319..3ce1b2e 100644
> > --- a/target-i386/cpu.h
> > +++ b/target-i386/cpu.h
> > @@ -1188,11 +1188,9 @@ static inline void cpu_pc_from_tb(CPUX86State *env, TranslationBlock *tb)
> >      env->eip = tb->pc - tb->cs_base;
> >  }
> >  
> > -static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc,
> > -                                        target_ulong *cs_base, int *flags)
> > +static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc, int *flags)
> >  {
> > -    *cs_base = env->segs[R_CS].base;
> > -    *pc = *cs_base + env->eip;
> > +    *pc = env->segs[R_CS].base + env->eip;
> >      *flags = env->hflags |
> >          (env->eflags & (IOPL_MASK | TF_MASK | RF_MASK | VM_MASK | AC_MASK));
> >  }
> > diff --git a/translate-all.c b/translate-all.c
> > index a98c646..f9efbbd 100644
> > --- a/translate-all.c
> > +++ b/translate-all.c
> > @@ -931,8 +931,7 @@ static void build_page_bitmap(PageDesc *p)
> >  }
> >  
> >  TranslationBlock *tb_gen_code(CPUArchState *env,
> > -                              target_ulong pc, target_ulong cs_base,
> > -                              int flags, int cflags)
> > +                              target_ulong pc, int flags, int cflags)
> >  {
> >      TranslationBlock *tb;
> >      uint8_t *tc_ptr;
> > @@ -952,7 +951,12 @@ TranslationBlock *tb_gen_code(CPUArchState *env,
> >      }
> >      tc_ptr = tcg_ctx.code_gen_ptr;
> >      tb->tc_ptr = tc_ptr;
> > -    tb->cs_base = cs_base;
> > +#if defined(TARGET_I386)
> > +    tb->cs_base = env->segs[R_CS].base;
> > +#endif
> > +#if defined(TARGET_SPARC)
> > +    tb->cs_base = env->npc;
> > +#endif
> >      tb->flags = flags;
> >      tb->cflags = cflags;
> >      cpu_gen_code(env, tb, &code_gen_size);
> > @@ -1007,7 +1011,6 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
> >      TranslationBlock *current_tb = NULL;
> >      int current_tb_modified = 0;
> >      target_ulong current_pc = 0;
> > -    target_ulong current_cs_base = 0;
> >      int current_flags = 0;
> >  #endif /* TARGET_HAS_PRECISE_SMC */
> >  
> > @@ -1063,8 +1066,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
> >  
> >                  current_tb_modified = 1;
> >                  cpu_restore_state_from_tb(current_tb, env, env->mem_io_pc);
> > -                cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
> > -                                     &current_flags);
> > +                cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
> >              }
> >  #endif /* TARGET_HAS_PRECISE_SMC */
> >              /* we need to do that to handle the case where a signal
> > @@ -1099,7 +1101,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
> >             modifying the memory. It will ensure that it cannot modify
> >             itself */
> >          cpu->current_tb = NULL;
> > -        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
> > +        tb_gen_code(env, current_pc, current_flags, 1);
> >          cpu_resume_from_signal(env, NULL);
> >      }
> >  #endif
> > @@ -1149,7 +1151,6 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
> >      CPUState *cpu = NULL;
> >      int current_tb_modified = 0;
> >      target_ulong current_pc = 0;
> > -    target_ulong current_cs_base = 0;
> >      int current_flags = 0;
> >  #endif
> >  
> > @@ -1181,8 +1182,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
> >  
> >              current_tb_modified = 1;
> >              cpu_restore_state_from_tb(current_tb, env, pc);
> > -            cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
> > -                                 &current_flags);
> > +            cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
> >          }
> >  #endif /* TARGET_HAS_PRECISE_SMC */
> >          tb_phys_invalidate(tb, addr);
> > @@ -1195,7 +1195,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
> >             modifying the memory. It will ensure that it cannot modify
> >             itself */
> >          cpu->current_tb = NULL;
> > -        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
> > +        tb_gen_code(env, current_pc, current_flags, 1);
> >          cpu_resume_from_signal(env, puc);
> >      }
> >  #endif
> > @@ -1463,7 +1463,7 @@ void cpu_io_recompile(CPUArchState *env, uintptr_t retaddr)
> >      tb_phys_invalidate(tb, -1);
> >      /* FIXME: In theory this could raise an exception.  In practice
> >         we have already translated the block once so it's probably ok.  */
> > -    tb_gen_code(env, pc, cs_base, flags, cflags);
> > +    tb_gen_code(env, pc, flags, cflags);
> >      /* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not
> >         the first in the TB) then we end up generating a whole new TB and
> >         repeating the fault, which is horribly inefficient.
> > 
> 
> 
>
liguang - April 24, 2013, 7:25 a.m.
在 2013-04-24三的 09:11 +0200,Aurelien Jarno写道:
> On Wed, Apr 24, 2013 at 08:36:54AM +0200, Paolo Bonzini wrote:
> > Il 24/04/2013 03:48, liguang ha scritto:
> > > cs_base is only meaningful for target-i386/sparc,
> > > so, get rid of cs_base for other target
> > 
> > This is really ugly, we're trying to get less target-dependent code
> > outside target-*, not more.

I think it's easy to be arch independent by just
call a generic function instead of "#if defined(*)",
and archs can implement their own specific in this function.


> 
> Fully agreed. It also breaks the interface between the target and
> cpu-exec.c by assuming tb->cs_base will always be env->segs[R_CS].base.
> 

I'm not going to assume that (maybe it's the fact),
I did some random tests, seems break nothing.

> The only cleanup that can be done here is to rename cs_base into flags2
> to make it less target dependent, and the code in cpu-exec.c should just
> guarantee to choose tbs which match both flags and flags2 without
> actually caring about the meaning of the values.
> 
> Even that way, this is the kind of cleanup touching a lot of code
> without real benefit, except maybe for sparc which currently "abuse"
> cs_base.
> 
> > Also, please limit the number of people that you CC.
> > 
> > Paolo
> > 
> > > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > > ---
> > >  cpu-exec.c              |   26 ++++++++++++++++++--------
> > >  exec.c                  |    6 +++---
> > >  hw/i386/kvmvapic.c      |    6 ++----
> > >  include/exec/exec-all.h |    5 +++--
> > >  target-i386/cpu.h       |    6 ++----
> > >  translate-all.c         |   24 ++++++++++++------------
> > >  6 files changed, 40 insertions(+), 33 deletions(-)
> > > 
> > > diff --git a/cpu-exec.c b/cpu-exec.c
> > > index 31c089d..f3c1d1c 100644
> > > --- a/cpu-exec.c
> > > +++ b/cpu-exec.c
> > > @@ -84,7 +84,7 @@ static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
> > >      if (max_cycles > CF_COUNT_MASK)
> > >          max_cycles = CF_COUNT_MASK;
> > >  
> > > -    tb = tb_gen_code(env, orig_tb->pc, orig_tb->cs_base, orig_tb->flags,
> > > +    tb = tb_gen_code(env, orig_tb->pc, orig_tb->flags,
> > >                       max_cycles);
> > >      cpu->current_tb = tb;
> > >      /* execute the generated code */
> > > @@ -96,7 +96,6 @@ static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
> > >  
> > >  static TranslationBlock *tb_find_slow(CPUArchState *env,
> > >                                        target_ulong pc,
> > > -                                      target_ulong cs_base,
> > >                                        uint64_t flags)
> > >  {
> > >      TranslationBlock *tb, **ptb1;
> > > @@ -117,7 +116,12 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
> > >              goto not_found;
> > >          if (tb->pc == pc &&
> > >              tb->page_addr[0] == phys_page1 &&
> > > -            tb->cs_base == cs_base &&
> > > +#if defined(TARGET_I386)
> > > +            tb->cs_base == env->segs[R_CS].base &&
> > > +#endif
> > > +#if defined(TARGET_SPARC)
> > > +            tb->cs_base == env->npc &&
> > > +#endif
> > >              tb->flags == flags) {
> > >              /* check next page if needed */
> > >              if (tb->page_addr[1] != -1) {
> > > @@ -136,7 +140,7 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
> > >      }
> > >   not_found:
> > >     /* if no translated code available, then translate it now */
> > > -    tb = tb_gen_code(env, pc, cs_base, flags, 0);
> > > +    tb = tb_gen_code(env, pc, flags, 0);
> > >  
> > >   found:
> > >      /* Move the last found TB to the head of the list */
> > > @@ -153,17 +157,23 @@ static TranslationBlock *tb_find_slow(CPUArchState *env,
> > >  static inline TranslationBlock *tb_find_fast(CPUArchState *env)
> > >  {
> > >      TranslationBlock *tb;
> > > -    target_ulong cs_base, pc;
> > > +    target_ulong pc;
> > >      int flags;
> > >  
> > >      /* we record a subset of the CPU state. It will
> > >         always be the same before a given translated block
> > >         is executed. */
> > > -    cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
> > > +    cpu_get_tb_cpu_state(env, &pc, &flags);
> > >      tb = env->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
> > > -    if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
> > > +    if (unlikely(!tb || tb->pc != pc ||
> > > +#if defined(TARGET_I386)
> > > +                 tb->cs_base != env->segs[R_CS].base ||
> > > +#endif
> > > +#if defined(TARGET_SPARC)
> > > +                 tb->cs_base != env->npc ||
> > > +#endif
> > >                   tb->flags != flags)) {
> > > -        tb = tb_find_slow(env, pc, cs_base, flags);
> > > +        tb = tb_find_slow(env, pc, flags);
> > >      }
> > >      return tb;
> > >  }
> > > diff --git a/exec.c b/exec.c
> > > index fa1e0c3..a14db2c 100644
> > > --- a/exec.c
> > > +++ b/exec.c
> > > @@ -1471,7 +1471,7 @@ static const MemoryRegionOps notdirty_mem_ops = {
> > >  static void check_watchpoint(int offset, int len_mask, int flags)
> > >  {
> > >      CPUArchState *env = cpu_single_env;
> > > -    target_ulong pc, cs_base;
> > > +    target_ulong pc;
> > >      target_ulong vaddr;
> > >      CPUWatchpoint *wp;
> > >      int cpu_flags;
> > > @@ -1495,8 +1495,8 @@ static void check_watchpoint(int offset, int len_mask, int flags)
> > >                      env->exception_index = EXCP_DEBUG;
> > >                      cpu_loop_exit(env);
> > >                  } else {
> > > -                    cpu_get_tb_cpu_state(env, &pc, &cs_base, &cpu_flags);
> > > -                    tb_gen_code(env, pc, cs_base, cpu_flags, 1);
> > > +                    cpu_get_tb_cpu_state(env, &pc, &cpu_flags);
> > > +                    tb_gen_code(env, pc, cpu_flags, 1);
> > >                      cpu_resume_from_signal(env, NULL);
> > >                  }
> > >              }
> > > diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> > > index ed9b448..8b4260e 100644
> > > --- a/hw/i386/kvmvapic.c
> > > +++ b/hw/i386/kvmvapic.c
> > > @@ -388,7 +388,6 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
> > >      uint8_t opcode[2];
> > >      uint32_t imm32;
> > >      target_ulong current_pc = 0;
> > > -    target_ulong current_cs_base = 0;
> > >      int current_flags = 0;
> > >  
> > >      if (smp_cpus == 1) {
> > > @@ -399,8 +398,7 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
> > >  
> > >      if (!kvm_enabled()) {
> > >          cpu_restore_state(env, env->mem_io_pc);
> > > -        cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
> > > -                             &current_flags);
> > > +        cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
> > >      }
> > >  
> > >      pause_all_vcpus();
> > > @@ -440,7 +438,7 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
> > >  
> > >      if (!kvm_enabled()) {
> > >          cs->current_tb = NULL;
> > > -        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
> > > +        tb_gen_code(env, current_pc, current_flags, 1);
> > >          cpu_resume_from_signal(env, NULL);
> > >      }
> > >  }
> > > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> > > index e856191..560a7d1 100644
> > > --- a/include/exec/exec-all.h
> > > +++ b/include/exec/exec-all.h
> > > @@ -85,8 +85,7 @@ bool cpu_restore_state(CPUArchState *env, uintptr_t searched_pc);
> > >  void QEMU_NORETURN cpu_resume_from_signal(CPUArchState *env1, void *puc);
> > >  void QEMU_NORETURN cpu_io_recompile(CPUArchState *env, uintptr_t retaddr);
> > >  TranslationBlock *tb_gen_code(CPUArchState *env, 
> > > -                              target_ulong pc, target_ulong cs_base, int flags,
> > > -                              int cflags);
> > > +                              target_ulong pc, int flags, int cflags);
> > >  void cpu_exec_init(CPUArchState *env);
> > >  void QEMU_NORETURN cpu_loop_exit(CPUArchState *env1);
> > >  int page_unprotect(target_ulong address, uintptr_t pc, void *puc);
> > > @@ -135,7 +134,9 @@ static inline void tlb_flush(CPUArchState *env, int flush_global)
> > >  
> > >  struct TranslationBlock {
> > >      target_ulong pc;   /* simulated PC corresponding to this block (EIP + CS base) */
> > > +#if defined(TARGET_I386) || defined(TARGET_SPARC)
> > >      target_ulong cs_base; /* CS base for this block */
> > > +#endif
> > >      uint64_t flags; /* flags defining in which context the code was generated */
> > >      uint16_t size;      /* size of target code for this block (1 <=
> > >                             size <= TARGET_PAGE_SIZE) */
> > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> > > index 2b4e319..3ce1b2e 100644
> > > --- a/target-i386/cpu.h
> > > +++ b/target-i386/cpu.h
> > > @@ -1188,11 +1188,9 @@ static inline void cpu_pc_from_tb(CPUX86State *env, TranslationBlock *tb)
> > >      env->eip = tb->pc - tb->cs_base;
> > >  }
> > >  
> > > -static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc,
> > > -                                        target_ulong *cs_base, int *flags)
> > > +static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc, int *flags)
> > >  {
> > > -    *cs_base = env->segs[R_CS].base;
> > > -    *pc = *cs_base + env->eip;
> > > +    *pc = env->segs[R_CS].base + env->eip;
> > >      *flags = env->hflags |
> > >          (env->eflags & (IOPL_MASK | TF_MASK | RF_MASK | VM_MASK | AC_MASK));
> > >  }
> > > diff --git a/translate-all.c b/translate-all.c
> > > index a98c646..f9efbbd 100644
> > > --- a/translate-all.c
> > > +++ b/translate-all.c
> > > @@ -931,8 +931,7 @@ static void build_page_bitmap(PageDesc *p)
> > >  }
> > >  
> > >  TranslationBlock *tb_gen_code(CPUArchState *env,
> > > -                              target_ulong pc, target_ulong cs_base,
> > > -                              int flags, int cflags)
> > > +                              target_ulong pc, int flags, int cflags)
> > >  {
> > >      TranslationBlock *tb;
> > >      uint8_t *tc_ptr;
> > > @@ -952,7 +951,12 @@ TranslationBlock *tb_gen_code(CPUArchState *env,
> > >      }
> > >      tc_ptr = tcg_ctx.code_gen_ptr;
> > >      tb->tc_ptr = tc_ptr;
> > > -    tb->cs_base = cs_base;
> > > +#if defined(TARGET_I386)
> > > +    tb->cs_base = env->segs[R_CS].base;
> > > +#endif
> > > +#if defined(TARGET_SPARC)
> > > +    tb->cs_base = env->npc;
> > > +#endif
> > >      tb->flags = flags;
> > >      tb->cflags = cflags;
> > >      cpu_gen_code(env, tb, &code_gen_size);
> > > @@ -1007,7 +1011,6 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
> > >      TranslationBlock *current_tb = NULL;
> > >      int current_tb_modified = 0;
> > >      target_ulong current_pc = 0;
> > > -    target_ulong current_cs_base = 0;
> > >      int current_flags = 0;
> > >  #endif /* TARGET_HAS_PRECISE_SMC */
> > >  
> > > @@ -1063,8 +1066,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
> > >  
> > >                  current_tb_modified = 1;
> > >                  cpu_restore_state_from_tb(current_tb, env, env->mem_io_pc);
> > > -                cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
> > > -                                     &current_flags);
> > > +                cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
> > >              }
> > >  #endif /* TARGET_HAS_PRECISE_SMC */
> > >              /* we need to do that to handle the case where a signal
> > > @@ -1099,7 +1101,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
> > >             modifying the memory. It will ensure that it cannot modify
> > >             itself */
> > >          cpu->current_tb = NULL;
> > > -        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
> > > +        tb_gen_code(env, current_pc, current_flags, 1);
> > >          cpu_resume_from_signal(env, NULL);
> > >      }
> > >  #endif
> > > @@ -1149,7 +1151,6 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
> > >      CPUState *cpu = NULL;
> > >      int current_tb_modified = 0;
> > >      target_ulong current_pc = 0;
> > > -    target_ulong current_cs_base = 0;
> > >      int current_flags = 0;
> > >  #endif
> > >  
> > > @@ -1181,8 +1182,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
> > >  
> > >              current_tb_modified = 1;
> > >              cpu_restore_state_from_tb(current_tb, env, pc);
> > > -            cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
> > > -                                 &current_flags);
> > > +            cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
> > >          }
> > >  #endif /* TARGET_HAS_PRECISE_SMC */
> > >          tb_phys_invalidate(tb, addr);
> > > @@ -1195,7 +1195,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
> > >             modifying the memory. It will ensure that it cannot modify
> > >             itself */
> > >          cpu->current_tb = NULL;
> > > -        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
> > > +        tb_gen_code(env, current_pc, current_flags, 1);
> > >          cpu_resume_from_signal(env, puc);
> > >      }
> > >  #endif
> > > @@ -1463,7 +1463,7 @@ void cpu_io_recompile(CPUArchState *env, uintptr_t retaddr)
> > >      tb_phys_invalidate(tb, -1);
> > >      /* FIXME: In theory this could raise an exception.  In practice
> > >         we have already translated the block once so it's probably ok.  */
> > > -    tb_gen_code(env, pc, cs_base, flags, cflags);
> > > +    tb_gen_code(env, pc, flags, cflags);
> > >      /* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not
> > >         the first in the TB) then we end up generating a whole new TB and
> > >         repeating the fault, which is horribly inefficient.
> > > 
> > 
> > 
> > 
>
Peter Maydell - April 24, 2013, 7:33 a.m.
On 24 April 2013 08:25, li guang <lig.fnst@cn.fujitsu.com> wrote:
> 在 2013-04-24三的 09:11 +0200,Aurelien Jarno写道:
>> On Wed, Apr 24, 2013 at 08:36:54AM +0200, Paolo Bonzini wrote:
>> > Il 24/04/2013 03:48, liguang ha scritto:
>> > > cs_base is only meaningful for target-i386/sparc,
>> > > so, get rid of cs_base for other target
>> >
>> > This is really ugly, we're trying to get less target-dependent code
>> > outside target-*, not more.
>
> I think it's easy to be arch independent by just
> call a generic function

We already have that, this is exactly what the target
cpu_get_tb_cpu_state() function is for! It abstracts
away the target's specific use of these fields, so the
common code can treat it as an opaque blob of state.

> I'm not going to assume that (maybe it's the fact),
> I did some random tests, seems break nothing.

You have absolutely broken things here -- if your random
tests didn't identify what then your testing process was
just not solid enough to find the corner cases.

-- PMM

Patch

diff --git a/cpu-exec.c b/cpu-exec.c
index 31c089d..f3c1d1c 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -84,7 +84,7 @@  static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
     if (max_cycles > CF_COUNT_MASK)
         max_cycles = CF_COUNT_MASK;
 
-    tb = tb_gen_code(env, orig_tb->pc, orig_tb->cs_base, orig_tb->flags,
+    tb = tb_gen_code(env, orig_tb->pc, orig_tb->flags,
                      max_cycles);
     cpu->current_tb = tb;
     /* execute the generated code */
@@ -96,7 +96,6 @@  static void cpu_exec_nocache(CPUArchState *env, int max_cycles,
 
 static TranslationBlock *tb_find_slow(CPUArchState *env,
                                       target_ulong pc,
-                                      target_ulong cs_base,
                                       uint64_t flags)
 {
     TranslationBlock *tb, **ptb1;
@@ -117,7 +116,12 @@  static TranslationBlock *tb_find_slow(CPUArchState *env,
             goto not_found;
         if (tb->pc == pc &&
             tb->page_addr[0] == phys_page1 &&
-            tb->cs_base == cs_base &&
+#if defined(TARGET_I386)
+            tb->cs_base == env->segs[R_CS].base &&
+#endif
+#if defined(TARGET_SPARC)
+            tb->cs_base == env->npc &&
+#endif
             tb->flags == flags) {
             /* check next page if needed */
             if (tb->page_addr[1] != -1) {
@@ -136,7 +140,7 @@  static TranslationBlock *tb_find_slow(CPUArchState *env,
     }
  not_found:
    /* if no translated code available, then translate it now */
-    tb = tb_gen_code(env, pc, cs_base, flags, 0);
+    tb = tb_gen_code(env, pc, flags, 0);
 
  found:
     /* Move the last found TB to the head of the list */
@@ -153,17 +157,23 @@  static TranslationBlock *tb_find_slow(CPUArchState *env,
 static inline TranslationBlock *tb_find_fast(CPUArchState *env)
 {
     TranslationBlock *tb;
-    target_ulong cs_base, pc;
+    target_ulong pc;
     int flags;
 
     /* we record a subset of the CPU state. It will
        always be the same before a given translated block
        is executed. */
-    cpu_get_tb_cpu_state(env, &pc, &cs_base, &flags);
+    cpu_get_tb_cpu_state(env, &pc, &flags);
     tb = env->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
-    if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
+    if (unlikely(!tb || tb->pc != pc ||
+#if defined(TARGET_I386)
+                 tb->cs_base != env->segs[R_CS].base ||
+#endif
+#if defined(TARGET_SPARC)
+                 tb->cs_base != env->npc ||
+#endif
                  tb->flags != flags)) {
-        tb = tb_find_slow(env, pc, cs_base, flags);
+        tb = tb_find_slow(env, pc, flags);
     }
     return tb;
 }
diff --git a/exec.c b/exec.c
index fa1e0c3..a14db2c 100644
--- a/exec.c
+++ b/exec.c
@@ -1471,7 +1471,7 @@  static const MemoryRegionOps notdirty_mem_ops = {
 static void check_watchpoint(int offset, int len_mask, int flags)
 {
     CPUArchState *env = cpu_single_env;
-    target_ulong pc, cs_base;
+    target_ulong pc;
     target_ulong vaddr;
     CPUWatchpoint *wp;
     int cpu_flags;
@@ -1495,8 +1495,8 @@  static void check_watchpoint(int offset, int len_mask, int flags)
                     env->exception_index = EXCP_DEBUG;
                     cpu_loop_exit(env);
                 } else {
-                    cpu_get_tb_cpu_state(env, &pc, &cs_base, &cpu_flags);
-                    tb_gen_code(env, pc, cs_base, cpu_flags, 1);
+                    cpu_get_tb_cpu_state(env, &pc, &cpu_flags);
+                    tb_gen_code(env, pc, cpu_flags, 1);
                     cpu_resume_from_signal(env, NULL);
                 }
             }
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index ed9b448..8b4260e 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -388,7 +388,6 @@  static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
     uint8_t opcode[2];
     uint32_t imm32;
     target_ulong current_pc = 0;
-    target_ulong current_cs_base = 0;
     int current_flags = 0;
 
     if (smp_cpus == 1) {
@@ -399,8 +398,7 @@  static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
 
     if (!kvm_enabled()) {
         cpu_restore_state(env, env->mem_io_pc);
-        cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
-                             &current_flags);
+        cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
     }
 
     pause_all_vcpus();
@@ -440,7 +438,7 @@  static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
 
     if (!kvm_enabled()) {
         cs->current_tb = NULL;
-        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+        tb_gen_code(env, current_pc, current_flags, 1);
         cpu_resume_from_signal(env, NULL);
     }
 }
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index e856191..560a7d1 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -85,8 +85,7 @@  bool cpu_restore_state(CPUArchState *env, uintptr_t searched_pc);
 void QEMU_NORETURN cpu_resume_from_signal(CPUArchState *env1, void *puc);
 void QEMU_NORETURN cpu_io_recompile(CPUArchState *env, uintptr_t retaddr);
 TranslationBlock *tb_gen_code(CPUArchState *env, 
-                              target_ulong pc, target_ulong cs_base, int flags,
-                              int cflags);
+                              target_ulong pc, int flags, int cflags);
 void cpu_exec_init(CPUArchState *env);
 void QEMU_NORETURN cpu_loop_exit(CPUArchState *env1);
 int page_unprotect(target_ulong address, uintptr_t pc, void *puc);
@@ -135,7 +134,9 @@  static inline void tlb_flush(CPUArchState *env, int flush_global)
 
 struct TranslationBlock {
     target_ulong pc;   /* simulated PC corresponding to this block (EIP + CS base) */
+#if defined(TARGET_I386) || defined(TARGET_SPARC)
     target_ulong cs_base; /* CS base for this block */
+#endif
     uint64_t flags; /* flags defining in which context the code was generated */
     uint16_t size;      /* size of target code for this block (1 <=
                            size <= TARGET_PAGE_SIZE) */
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 2b4e319..3ce1b2e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1188,11 +1188,9 @@  static inline void cpu_pc_from_tb(CPUX86State *env, TranslationBlock *tb)
     env->eip = tb->pc - tb->cs_base;
 }
 
-static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc,
-                                        target_ulong *cs_base, int *flags)
+static inline void cpu_get_tb_cpu_state(CPUX86State *env, target_ulong *pc, int *flags)
 {
-    *cs_base = env->segs[R_CS].base;
-    *pc = *cs_base + env->eip;
+    *pc = env->segs[R_CS].base + env->eip;
     *flags = env->hflags |
         (env->eflags & (IOPL_MASK | TF_MASK | RF_MASK | VM_MASK | AC_MASK));
 }
diff --git a/translate-all.c b/translate-all.c
index a98c646..f9efbbd 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -931,8 +931,7 @@  static void build_page_bitmap(PageDesc *p)
 }
 
 TranslationBlock *tb_gen_code(CPUArchState *env,
-                              target_ulong pc, target_ulong cs_base,
-                              int flags, int cflags)
+                              target_ulong pc, int flags, int cflags)
 {
     TranslationBlock *tb;
     uint8_t *tc_ptr;
@@ -952,7 +951,12 @@  TranslationBlock *tb_gen_code(CPUArchState *env,
     }
     tc_ptr = tcg_ctx.code_gen_ptr;
     tb->tc_ptr = tc_ptr;
-    tb->cs_base = cs_base;
+#if defined(TARGET_I386)
+    tb->cs_base = env->segs[R_CS].base;
+#endif
+#if defined(TARGET_SPARC)
+    tb->cs_base = env->npc;
+#endif
     tb->flags = flags;
     tb->cflags = cflags;
     cpu_gen_code(env, tb, &code_gen_size);
@@ -1007,7 +1011,6 @@  void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
     TranslationBlock *current_tb = NULL;
     int current_tb_modified = 0;
     target_ulong current_pc = 0;
-    target_ulong current_cs_base = 0;
     int current_flags = 0;
 #endif /* TARGET_HAS_PRECISE_SMC */
 
@@ -1063,8 +1066,7 @@  void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
 
                 current_tb_modified = 1;
                 cpu_restore_state_from_tb(current_tb, env, env->mem_io_pc);
-                cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
-                                     &current_flags);
+                cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
             }
 #endif /* TARGET_HAS_PRECISE_SMC */
             /* we need to do that to handle the case where a signal
@@ -1099,7 +1101,7 @@  void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
            modifying the memory. It will ensure that it cannot modify
            itself */
         cpu->current_tb = NULL;
-        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+        tb_gen_code(env, current_pc, current_flags, 1);
         cpu_resume_from_signal(env, NULL);
     }
 #endif
@@ -1149,7 +1151,6 @@  static void tb_invalidate_phys_page(tb_page_addr_t addr,
     CPUState *cpu = NULL;
     int current_tb_modified = 0;
     target_ulong current_pc = 0;
-    target_ulong current_cs_base = 0;
     int current_flags = 0;
 #endif
 
@@ -1181,8 +1182,7 @@  static void tb_invalidate_phys_page(tb_page_addr_t addr,
 
             current_tb_modified = 1;
             cpu_restore_state_from_tb(current_tb, env, pc);
-            cpu_get_tb_cpu_state(env, &current_pc, &current_cs_base,
-                                 &current_flags);
+            cpu_get_tb_cpu_state(env, &current_pc, &current_flags);
         }
 #endif /* TARGET_HAS_PRECISE_SMC */
         tb_phys_invalidate(tb, addr);
@@ -1195,7 +1195,7 @@  static void tb_invalidate_phys_page(tb_page_addr_t addr,
            modifying the memory. It will ensure that it cannot modify
            itself */
         cpu->current_tb = NULL;
-        tb_gen_code(env, current_pc, current_cs_base, current_flags, 1);
+        tb_gen_code(env, current_pc, current_flags, 1);
         cpu_resume_from_signal(env, puc);
     }
 #endif
@@ -1463,7 +1463,7 @@  void cpu_io_recompile(CPUArchState *env, uintptr_t retaddr)
     tb_phys_invalidate(tb, -1);
     /* FIXME: In theory this could raise an exception.  In practice
        we have already translated the block once so it's probably ok.  */
-    tb_gen_code(env, pc, cs_base, flags, cflags);
+    tb_gen_code(env, pc, flags, cflags);
     /* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not
        the first in the TB) then we end up generating a whole new TB and
        repeating the fault, which is horribly inefficient.