On Mon, Dec 16, 2013 at 01:11:47PM +0100, Andreas Färber wrote:
> Am 16.12.2013 09:05, schrieb edgar.igles...@gmail.com:
> > From: "Edgar E. Iglesias" <edgar.igles...@xilinx.com>
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.igles...@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.
Hi,
This was discussed when I posted the RFC. My first try had this member in
CPUState but some of the hot paths for mmio accesses needed to do
ENV_GET_CPU() to get hold of the CS and AS. ENV_GET_CPU does a runtime type
check that would slow things down. That's the reason I moved it to env.
I'm not against moving the field back if it doesnt cost too much. Maybe we
should consider removing the CPU() around ENV_GET_CPU()?
See:
http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02889.html
Cheers,
Edgar
