Re: [Qemu-devel] [PATCH v1 08/22] cpu: Add per-cpu address space

["Andreas Färber" <afaerber@suse.de>]

Am 16.12.2013 09:05, schrieb edgar.iglesias@gmail.com:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
> 
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> ---
>  cputlb.c                        |    4 ++--
>  exec.c                          |   31 +++++++++++++++++++++++--------
>  include/exec/cpu-defs.h         |    3 +++
>  include/exec/exec-all.h         |    1 +
>  include/exec/softmmu_template.h |    4 ++--
>  include/qom/cpu.h               |    2 ++
>  6 files changed, 33 insertions(+), 12 deletions(-)
[...]
> diff --git a/exec.c b/exec.c
> index 803bbde..edb6a43 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -136,6 +136,7 @@ typedef struct subpage_t {
>  
>  static void io_mem_init(void);
>  static void memory_map_init(void);
> +static void tcg_commit(MemoryListener *listener);
>  
>  static MemoryRegion io_mem_watch;
>  #endif
> @@ -434,6 +435,25 @@ CPUState *qemu_get_cpu(int index)
>      return NULL;
>  }
>  
> +#if !defined(CONFIG_USER_ONLY)
> +void cpu_address_space_init(CPUState *cpu, AddressSpace *as)
> +{
> +    CPUArchState *env = cpu->env_ptr;
> +
> +    if (tcg_enabled()) {
> +        if (cpu->tcg_as_listener) {
> +            memory_listener_unregister(cpu->tcg_as_listener);
> +        } else {
> +            cpu->tcg_as_listener = g_new0(MemoryListener, 1);
> +        }
> +        cpu->tcg_as_listener->commit = tcg_commit;
> +        memory_listener_register(cpu->tcg_as_listener, as);
> +    }
> +
> +    env->as = as;
> +}
> +#endif
> +
>  void cpu_exec_init(CPUArchState *env)
>  {
>      CPUState *cpu = ENV_GET_CPU(env);
> @@ -453,6 +473,7 @@ void cpu_exec_init(CPUArchState *env)
>      QTAILQ_INIT(&env->breakpoints);
>      QTAILQ_INIT(&env->watchpoints);
>  #ifndef CONFIG_USER_ONLY
> +    cpu_address_space_init(cpu, &address_space_memory);
>      cpu->thread_id = qemu_get_thread_id();
>  #endif
>      QTAILQ_INSERT_TAIL(&cpus, cpu, node);
> @@ -482,9 +503,10 @@ static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
>  #else
>  static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
>  {
> +    CPUArchState *env = cpu->env_ptr;
>      hwaddr phys = cpu_get_phys_page_debug(cpu, pc);
>      if (phys != -1) {
> -        tb_invalidate_phys_addr(&address_space_memory,
> +        tb_invalidate_phys_addr(env->as,
>                                  phys | (pc & ~TARGET_PAGE_MASK));
>      }
>  }
[...]
> diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
> index 01cd8c7..406b36c 100644
> --- a/include/exec/cpu-defs.h
> +++ b/include/exec/cpu-defs.h
> @@ -176,6 +176,9 @@ typedef struct CPUWatchpoint {
>      sigjmp_buf jmp_env;                                                 \
>      int exception_index;                                                \
>                                                                          \
> +    /* Per CPU address-space.  */                                       \
> +    AddressSpace *as;                                                   \

Why are you adding this field here rather than in CPUState alongside the
other field? This being a pointer I can't imagine problems for
non-softmmu, and I had posted patches a while ago to move the
surrounding fields there. Having it in CPUState would avoid some of the
env_ptr accesses above, which were supposed to be an interim solution only.

Regards,
Andreas

> +                                                                        \
>      /* user data */                                                     \
>      void *opaque;                                                       \
>  
[...]
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 7739e00..c1febae 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -186,6 +186,8 @@ struct CPUState {
>      uint32_t interrupt_request;
>      int singlestep_enabled;
>  
> +    MemoryListener *tcg_as_listener;
> +
>      void *env_ptr; /* CPUArchState */
>      struct TranslationBlock *current_tb;
>      struct GDBRegisterState *gdb_regs;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg