diff mbox

[RFC,6/8] cpu: Add per-cpu address space

Message ID 1385133359-13770-7-git-send-email-edgar.iglesias@gmail.com
State New
Headers show

Commit Message

Edgar E. Iglesias Nov. 22, 2013, 3:15 p.m. UTC
From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.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(-)

Comments

Peter Maydell Nov. 22, 2013, 3:50 p.m. UTC | #1
On 22 November 2013 15:15,  <edgar.iglesias@gmail.com> wrote:
> @@ -176,6 +176,9 @@ typedef struct CPUWatchpoint {
>      sigjmp_buf jmp_env;                                                 \
>      int exception_index;                                                \
>                                                                          \
> +    /* Per CPU address-space.  */                                       \
> +    AddressSpace *as;                                                   \
> +                                                                        \

Does this really have to live in the env struct rather than
CPUState ?

thanks
-- PMM
Edgar E. Iglesias Nov. 22, 2013, 4:02 p.m. UTC | #2
Hi, no I actually had it in cpustate first but had to do env-get-cpu all
over so i moved it to env. Iiuc env-get-cpu involves a dyn typecheck. I
havent meassured the perf impact though.

Sorry for phone email...

Cheers

---
Sent from my phone
On Nov 22, 2013 4:51 PM, "Peter Maydell" <peter.maydell@linaro.org> wrote:

> On 22 November 2013 15:15,  <edgar.iglesias@gmail.com> wrote:
> > @@ -176,6 +176,9 @@ typedef struct CPUWatchpoint {
> >      sigjmp_buf jmp_env;
> \
> >      int exception_index;
>  \
> >
>  \
> > +    /* Per CPU address-space.  */
> \
> > +    AddressSpace *as;
> \
> > +
>  \
>
> Does this really have to live in the env struct rather than
> CPUState ?
>
> thanks
> -- PMM
>
Andreas Färber Nov. 23, 2013, 6 p.m. UTC | #3
Am 22.11.2013 17:02, schrieb Edgar E. Iglesias:
> Hi, no I actually had it in cpustate first but had to do env-get-cpu all
> over so i moved it to env. Iiuc env-get-cpu involves a dyn typecheck.

No, it doesn't any more, it's just a pointer offset.

Andreas

> I havent meassured the perf impact though.
> 
> Sorry for phone email...
> 
> Cheers
> 
> ---
> Sent from my phone
> 
> On Nov 22, 2013 4:51 PM, "Peter Maydell" <peter.maydell@linaro.org
> <mailto:peter.maydell@linaro.org>> wrote:
> 
>     On 22 November 2013 15:15,  <edgar.iglesias@gmail.com
>     <mailto:edgar.iglesias@gmail.com>> wrote:
>     > @@ -176,6 +176,9 @@ typedef struct CPUWatchpoint {
>     >      sigjmp_buf jmp_env;                                          
>           \
>     >      int exception_index;                                        
>            \
>     >                                                                  
>            \
>     > +    /* Per CPU address-space.  */                                
>           \
>     > +    AddressSpace *as;                                            
>           \
>     > +                                                                
>            \
> 
>     Does this really have to live in the env struct rather than
>     CPUState ?
> 
>     thanks
>     -- PMM
>
Edgar E. Iglesias Nov. 23, 2013, 9:56 p.m. UTC | #4
On Sat, Nov 23, 2013 at 07:00:51PM +0100, Andreas Färber wrote:
> Am 22.11.2013 17:02, schrieb Edgar E. Iglesias:
> > Hi, no I actually had it in cpustate first but had to do env-get-cpu all
> > over so i moved it to env. Iiuc env-get-cpu involves a dyn typecheck.
> 
> No, it doesn't any more, it's just a pointer offset.
> 

Hi Andreas,

Looking at todays master, for example for i386:

target-i386/cpu-qom.h:
#define ENV_GET_CPU(e) CPU(x86_env_get_cpu(e))

include/qom/cpu.h:
#define CPU(obj) OBJECT_CHECK(CPUState, (obj), TYPE_CPU)

include/qom/object.h:
#define OBJECT_CHECK(type, obj, name) \
    ((type *)object_dynamic_cast_assert(OBJECT(obj), (name), \
                                        __FILE__, __LINE__, __func__))


Maybe we should remove the CPU() around xx_env_get_cpu(e)?

I'm happy to move the cpu address space into CPUState, but right 
now I'm afraid ENV_GET_CPU will slow down some of these hot paths.

Cheers,
Edgar
diff mbox

Patch

diff --git a/cputlb.c b/cputlb.c
index 0399172..a2264a3 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -254,7 +254,7 @@  void tlb_set_page(CPUArchState *env, target_ulong vaddr,
     }
 
     sz = size;
-    section = address_space_translate_for_iotlb(&address_space_memory, paddr,
+    section = address_space_translate_for_iotlb(env->as, paddr,
                                                 &xlat, &sz);
     assert(sz >= TARGET_PAGE_SIZE);
 
@@ -327,7 +327,7 @@  tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr)
         cpu_ldub_code(env1, addr);
     }
     pd = env1->iotlb[mmu_idx][page_index] & ~TARGET_PAGE_MASK;
-    mr = iotlb_to_region(&address_space_memory, pd);
+    mr = iotlb_to_region(env1->as, pd);
     if (memory_region_is_unassigned(mr)) {
         CPUState *cpu = ENV_GET_CPU(env1);
         CPUClass *cc = CPU_GET_CLASS(cpu);
diff --git a/exec.c b/exec.c
index 0162eb3..acbd2e6 100644
--- a/exec.c
+++ b/exec.c
@@ -129,6 +129,7 @@  static PhysPageMap next_map;
 
 static void io_mem_init(void);
 static void memory_map_init(void);
+static void tcg_commit(MemoryListener *listener);
 
 static MemoryRegion io_mem_watch;
 #endif
@@ -361,6 +362,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);
@@ -380,6 +400,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);
@@ -409,9 +430,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));
     }
 }
@@ -1717,10 +1739,6 @@  static MemoryListener core_memory_listener = {
     .priority = 1,
 };
 
-static MemoryListener tcg_memory_listener = {
-    .commit = tcg_commit,
-};
-
 void address_space_init_dispatch(AddressSpace *as)
 {
     as->dispatch = NULL;
@@ -1755,9 +1773,6 @@  static void memory_map_init(void)
     address_space_init(&address_space_io, system_io, "I/O");
 
     memory_listener_register(&core_memory_listener, &address_space_memory);
-    if (tcg_enabled()) {
-        memory_listener_register(&tcg_memory_listener, &address_space_memory);
-    }
 }
 
 MemoryRegion *get_system_memory(void)
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;                                                   \
+                                                                        \
     /* user data */                                                     \
     void *opaque;                                                       \
 
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 6129365..61770ee 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -95,6 +95,7 @@  void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
 void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end,
                               int is_cpu_write_access);
 #if !defined(CONFIG_USER_ONLY)
+void cpu_address_space_init(CPUState *cpu, AddressSpace *as);
 /* cputlb.c */
 void tlb_flush_page(CPUArchState *env, target_ulong addr);
 void tlb_flush(CPUArchState *env, int flush_global);
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index 69d856a..1dacb4d 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -119,7 +119,7 @@  static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
                                               uintptr_t retaddr)
 {
     uint64_t val;
-    MemoryRegion *mr = iotlb_to_region(&address_space_memory, physaddr);
+    MemoryRegion *mr = iotlb_to_region(env->as, physaddr);
 
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
     env->mem_io_pc = retaddr;
@@ -325,7 +325,7 @@  static inline void glue(io_write, SUFFIX)(CPUArchState *env,
                                           target_ulong addr,
                                           uintptr_t retaddr)
 {
-    MemoryRegion *mr = iotlb_to_region(&address_space_memory, physaddr);
+    MemoryRegion *mr = iotlb_to_region(env->as, physaddr);
 
     physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
     if (mr != &io_mem_rom && mr != &io_mem_notdirty && !can_do_io(env)) {
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;