Multi Architecture System Emulation

The usages of this define are pure TCG and there is no architecture specific variation of the value. Localise it to the TCG engine to remove another architecture agnostic piece from cpu-defs.h.

This follows on from a28177820a868eafda8fab007561cc19f41941f4 where temp_buf was moved out of the CPU_COMMON obsoleting the need for the super early definition.

Cc Richard Henderson <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

---
 include/exec/cpu-defs.h | 1 -
 tcg/tcg.h               | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

Remove un-needed usages of CPU_GET_ENV by converting the APIs to use CPUState pointers and retrieving the env_ptr as minimally needed.

FIXME: apply target-foo change pattern to all archs.

 #ifdef TARGET_I386
     X86CPU *x86_cpu = X86_CPU(cpu);

+ CPUArchState *env = (CPUArchState *)cpu->env_ptr;
#endif

#ifdef TARGET_I386

+            env = cpu->env_ptr;
             x86_cpu = X86_CPU(cpu);

#endif

Quoted 4 lines · Peter Crosthwaite

Remove un-needed usages of CPU_GET_ENV by converting the APIs to use CPUState pointers and retrieving the env_ptr as minimally needed.

FIXME: apply target-foo change pattern to all archs.

Yes, please.

Quoted 4 lines

 #ifdef TARGET_I386
     X86CPU *x86_cpu = X86_CPU(cpu);

+ CPUArchState *env = (CPUArchState *)cpu->env_ptr;
#endif

  CPUArchState *env = &x86_cpu.env;

Quoted 4 lines

#ifdef TARGET_I386

+            env = cpu->env_ptr;
             x86_cpu = X86_CPU(cpu);

#endif

Likewise.

Or, indeed, it might be worth propagating away "env" entirely, so you don't have to reload it here at the end of the setjmp.

Otherwise this starter patch looks good.

r~

#########################################################

 # cpu emulator library

Makefile.target +7 -3

-obj-y = exec.o translate-all.o cpu-exec.o
-obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
+obj-y += exec.o
+arch-obj-$(call lnot,$(TARGET_MULTI)) += translate-all.o
+arch-obj-$(call lnot,$(TARGET_MULTI)) += cpu-exec.o
+arch-obj-$(call lnot,$(TARGET_MULTI)) += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
 obj-$(CONFIG_TCG_INTERPRETER) += tci.o
 obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o
 obj-y += fpu/softfloat.o
-obj-y += target-$(TARGET_BASE_ARCH)/
+obj-$(CONFIG_ARCH_SINGLE) += target-$(TARGET_BASE_ARCH)/
+arch-obj-$(CONFIG_ARCH_MULTI) += target-$(TARGET_BASE_ARCH)/
+obj-$(CONFIG_ARCH_MULTI) += target-$(TARGET_BASE_ARCH)/hw/
 obj-y += disas.o
 obj-$(call notempty,$(TARGET_XML_FILES)) += gdbstub-xml.o
 obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o

Quoted 22 lines · Peter Crosthwaite

#########################################################

 # cpu emulator library

Makefile.target +7 -3

-obj-y = exec.o translate-all.o cpu-exec.o
-obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
+obj-y += exec.o
+arch-obj-$(call lnot,$(TARGET_MULTI)) += translate-all.o
+arch-obj-$(call lnot,$(TARGET_MULTI)) += cpu-exec.o
+arch-obj-$(call lnot,$(TARGET_MULTI)) += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
 obj-$(CONFIG_TCG_INTERPRETER) += tci.o
 obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o
 obj-y += fpu/softfloat.o
-obj-y += target-$(TARGET_BASE_ARCH)/
+obj-$(CONFIG_ARCH_SINGLE) += target-$(TARGET_BASE_ARCH)/
+arch-obj-$(CONFIG_ARCH_MULTI) += target-$(TARGET_BASE_ARCH)/
+obj-$(CONFIG_ARCH_MULTI) += target-$(TARGET_BASE_ARCH)/hw/
 obj-y += disas.o
 obj-$(call notempty,$(TARGET_XML_FILES)) += gdbstub-xml.o
 obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o

@@ -132,7 +136,8 @@ obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o
 obj-y += qtest.o bootdevice.o
 obj-y += hw/
 obj-$(CONFIG_KVM) += kvm-all.o

Makefile.target obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o numa.o · line 143

Quoted 6 lines

Makefile.target +2 -1

-obj-y += memory.o savevm.o cputlb.o
+obj-y += memory.o savevm.o
+arch-obj-$(call lnot,$(TARGET_MULTI)) += cputlb.o
 obj-y += memory_mapping.o
 obj-y += dump.o
 LIBS := $(libs_softmmu) $(LIBS)

How does translate-all.o get built for CONFIG_ARCH_SINGLE?

Is arch-obj-n added to obj-y somewhere I'm missing? Such an inclusion would seem to tidy some of the lines above...

r~

Quoted 2 lines

Is arch-obj-n added to obj-y somewhere I'm missing? Such an inclusion would seem to tidy some of the lines above...

No it's not. I didn't even know that was ok.

The goal is to split the functions such that cpu-exec is CPU specific content, while cpus.c is generic code only. The function interface to cpu-exec needs to be virtualised to prepare support for multi-arch and moving these definitions out saves bloating the QOM interface. So move these definitions out of cpu-exec to the architecture independent cpus.c.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 cpu-exec.c | 49 -------------------------------------------------
 cpus.c     | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 49 deletions(-)

cpu-exec.c +0 -10

index 0266609..dbea47c 100644
@@ -25,7 +25,6 @@
 #include "sysemu/qtest.h"
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
-#include "exec/memory-internal.h"
 #include "qemu/rcu.h"
 
 /* -icount align implementation. */

@@ -127,52 +126,6 @@ static void init_delay_params(SyncClocks *sc, const CPUState *cpu)
 }
 #endif /* CONFIG USER ONLY */
 
-void cpu_loop_exit(CPUState *cpu)
-{
-    cpu->current_tb = NULL;
-    siglongjmp(cpu->jmp_env, 1);
-}
-
-/* exit the current TB from a signal handler. The host registers are
-   restored in a state compatible with the CPU emulator
- */

cpu-exec.c +0 -29

-{
-    AddressSpaceDispatch *d;
-
-    if (qemu_in_vcpu_thread()) {
-        /* Do not let the guest prolong the critical section as much as it
-         * as it desires.
-         *
-         * Currently, this is prevented by the I/O thread's periodinc kicking
-         * of the VCPU thread (iothread_requesting_mutex, qemu_cpu_kick_thread)
-         * but this will go away once TCG's execution moves out of the global
-         * mutex.
-         *
-         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
-         * only protects cpu->as->dispatch.  Since we reload it below, we can
-         * split the critical section.
-         */
-        rcu_read_unlock();
-        rcu_read_lock();
-    }
-
-    /* The CPU and TLB are protected by the iothread lock.  */
-    d = atomic_rcu_read(&cpu->as->dispatch);
-    cpu->memory_dispatch = d;
-    tlb_flush(cpu, 1);
-}
-#endif
-
 /* Execute a TB, and fix up the CPU state afterwards if necessary */
 static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, uint8_t *tb_ptr)
 {

@@ -344,8 +297,6 @@ static void cpu_handle_debug_exception(CPUState *cpu)
 
 /* main execution loop */
 
-volatile sig_atomic_t exit_request;
-
 int cpu_exec(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);

cpus.c +49 -0

index c8a2911..2dc4a9a 100644
@@ -28,6 +28,7 @@
 #include "monitor/monitor.h"
 #include "qapi/qmp/qerror.h"
 #include "sysemu/sysemu.h"
+#include "exec/memory-internal.h"
 #include "exec/gdbstub.h"
 #include "sysemu/dma.h"
 #include "sysemu/kvm.h"

@@ -64,6 +65,8 @@
 
 #endif /* CONFIG_LINUX */
 
+volatile sig_atomic_t exit_request;
+
 static CPUState *next_cpu;
 int64_t max_delay;
 int64_t max_advance;

@@ -1394,6 +1397,52 @@ static void tcg_exec_all(void)
     exit_request = 0;
 }
 
+/* exit the current TB from a signal handler. The host registers are
+   restored in a state compatible with the CPU emulator
+ */
+#if defined(CONFIG_SOFTMMU)
+void cpu_resume_from_signal(CPUState *cpu, void *puc)
+{
+    /* XXX: restore cpu registers saved in host registers */
+
+    cpu->exception_index = -1;
+    siglongjmp(cpu->jmp_env, 1);
+}
+
+void cpu_reload_memory_map(CPUState *cpu)
+{
+    AddressSpaceDispatch *d;
+
+    if (qemu_in_vcpu_thread()) {
+        /* Do not let the guest prolong the critical section as much as it
+         * as it desires.
+         *
+         * Currently, this is prevented by the I/O thread's periodinc kicking
+         * of the VCPU thread (iothread_requesting_mutex, qemu_cpu_kick_thread)
+         * but this will go away once TCG's execution moves out of the global
+         * mutex.
+         *
+         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
+         * only protects cpu->as->dispatch.  Since we reload it below, we can
+         * split the critical section.
+         */
+        rcu_read_unlock();
+        rcu_read_lock();
+    }
+
+    /* The CPU and TLB are protected by the iothread lock.  */
+    d = atomic_rcu_read(&cpu->as->dispatch);
+    cpu->memory_dispatch = d;
+    tlb_flush(cpu, 1);
+}
+#endif
+
+void cpu_loop_exit(CPUState *cpu)
+{
+    cpu->current_tb = NULL;
+    siglongjmp(cpu->jmp_env, 1);
+}
+
 void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
 {
     /* XXX: implement xxx_cpu_list for targets that still miss it */

Quoted 44 lines · Peter Crosthwaite

The goal is to split the functions such that cpu-exec is CPU specific content, while cpus.c is generic code only. The function interface to cpu-exec needs to be virtualised to prepare support for multi-arch and moving these definitions out saves bloating the QOM interface. So move these definitions out of cpu-exec to the architecture independent cpus.c.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 cpu-exec.c | 49 -------------------------------------------------
 cpus.c     | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 49 deletions(-)

cpu-exec.c +0 -16

index 0266609..dbea47c 100644
@@ -25,7 +25,6 @@
 #include "sysemu/qtest.h"
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
-#include "exec/memory-internal.h"
 #include "qemu/rcu.h"
 
 /* -icount align implementation. */

@@ -127,52 +126,6 @@ static void init_delay_params(SyncClocks *sc, const CPUState *cpu)
 }
 #endif /* CONFIG USER ONLY */
 
-void cpu_loop_exit(CPUState *cpu)
-{
-    cpu->current_tb = NULL;
-    siglongjmp(cpu->jmp_env, 1);
-}
-
-/* exit the current TB from a signal handler. The host registers are
-   restored in a state compatible with the CPU emulator
- */
-#if defined(CONFIG_SOFTMMU)
-void cpu_resume_from_signal(CPUState *cpu, void *puc)
-{
-    /* XXX: restore cpu registers saved in host registers */
-
-    cpu->exception_index = -1;

cpu-exec.c static void init_delay_params(SyncClocks *sc, const CPUState *cpu) · line 148

Quoted 4 lines

cpu-exec.c +0 -4

-    siglongjmp(cpu->jmp_env, 1);
-}
-
-void cpu_reload_memory_map(CPUState *cpu)

This doesn't really belong in cpus.c... I don't have a better suggestion though, so perhaps just add a FIXME in case one day there is a cpu-exec-common.c.

Paolo

Quoted 114 lines

cpu-exec.c +0 -29

-{
-    AddressSpaceDispatch *d;
-
-    if (qemu_in_vcpu_thread()) {
-        /* Do not let the guest prolong the critical section as much as it
-         * as it desires.
-         *
-         * Currently, this is prevented by the I/O thread's periodinc kicking
-         * of the VCPU thread (iothread_requesting_mutex, qemu_cpu_kick_thread)
-         * but this will go away once TCG's execution moves out of the global
-         * mutex.
-         *
-         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
-         * only protects cpu->as->dispatch.  Since we reload it below, we can
-         * split the critical section.
-         */
-        rcu_read_unlock();
-        rcu_read_lock();
-    }
-
-    /* The CPU and TLB are protected by the iothread lock.  */
-    d = atomic_rcu_read(&cpu->as->dispatch);
-    cpu->memory_dispatch = d;
-    tlb_flush(cpu, 1);
-}
-#endif
-
 /* Execute a TB, and fix up the CPU state afterwards if necessary */
 static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, uint8_t *tb_ptr)
 {

@@ -344,8 +297,6 @@ static void cpu_handle_debug_exception(CPUState *cpu)
 
 /* main execution loop */
 
-volatile sig_atomic_t exit_request;
-
 int cpu_exec(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);

cpus.c +49 -0

index c8a2911..2dc4a9a 100644
@@ -28,6 +28,7 @@
 #include "monitor/monitor.h"
 #include "qapi/qmp/qerror.h"
 #include "sysemu/sysemu.h"
+#include "exec/memory-internal.h"
 #include "exec/gdbstub.h"
 #include "sysemu/dma.h"
 #include "sysemu/kvm.h"

@@ -64,6 +65,8 @@
 
 #endif /* CONFIG_LINUX */
 
+volatile sig_atomic_t exit_request;
+
 static CPUState *next_cpu;
 int64_t max_delay;
 int64_t max_advance;

@@ -1394,6 +1397,52 @@ static void tcg_exec_all(void)
     exit_request = 0;
 }
 
+/* exit the current TB from a signal handler. The host registers are
+   restored in a state compatible with the CPU emulator
+ */
+#if defined(CONFIG_SOFTMMU)
+void cpu_resume_from_signal(CPUState *cpu, void *puc)
+{
+    /* XXX: restore cpu registers saved in host registers */
+
+    cpu->exception_index = -1;
+    siglongjmp(cpu->jmp_env, 1);
+}
+
+void cpu_reload_memory_map(CPUState *cpu)
+{
+    AddressSpaceDispatch *d;
+
+    if (qemu_in_vcpu_thread()) {
+        /* Do not let the guest prolong the critical section as much as it
+         * as it desires.
+         *
+         * Currently, this is prevented by the I/O thread's periodinc kicking
+         * of the VCPU thread (iothread_requesting_mutex, qemu_cpu_kick_thread)
+         * but this will go away once TCG's execution moves out of the global
+         * mutex.
+         *
+         * This pair matches cpu_exec's rcu_read_lock()/rcu_read_unlock(), which
+         * only protects cpu->as->dispatch.  Since we reload it below, we can
+         * split the critical section.
+         */
+        rcu_read_unlock();
+        rcu_read_lock();
+    }
+
+    /* The CPU and TLB are protected by the iothread lock.  */
+    d = atomic_rcu_read(&cpu->as->dispatch);
+    cpu->memory_dispatch = d;
+    tlb_flush(cpu, 1);
+}
+#endif
+
+void cpu_loop_exit(CPUState *cpu)
+{
+    cpu->current_tb = NULL;
+    siglongjmp(cpu->jmp_env, 1);
+}
+
 void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
 {
     /* XXX: implement xxx_cpu_list for targets that still miss it */

The goal is to split the functions such that cpu-exec is CPU specific content, while cpus.c is generic code only. The function interface to cpu-exec needs to be virtualised to prepare support for multi-arch and moving these definitions out saves bloating the QOM interface. So move these definitions out of cpu-exec to the architecture independent cpus.c.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 cpu-exec.c | 49 -------------------------------------------------
 cpus.c     | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 49 deletions(-)

Create a global list of tcg_exec_init functions that is populated at startup. Multiple translation engines can register an init function and all will be called on the master call to tcg_exec_init.

Introduce a new module, translate-common. This is a common-obj for translation functionality such as this.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 Makefile.objs         |  1 +
 include/qemu-common.h |  1 +
 translate-all.c       |  7 ++++++-
 translate-common.c    | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 translate-common.c

Create a global list of tcg_exec_init functions that is populated at startup. Multiple translation engines can register an init function and all will be called on the master call to tcg_exec_init.

Introduce a new module, translate-common. This is a common-obj for translation functionality such as this.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 Makefile.objs         |  1 +
 include/qemu-common.h |  1 +
 translate-all.c       |  7 ++++++-
 translate-common.c    | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 translate-common.c

Makefile.objs +0 -0

index 4881d2c..294016e 100644

Makefile.objs +0 -0

 common-obj-$(CONFIG_SLIRP) += slirp/
 

include/qemu-common.h +1 -0

index 6b373ff..88fbcfa 100644
@@ -263,6 +263,7 @@ typedef struct PCIHostDeviceAddress {
     unsigned int function;
 } PCIHostDeviceAddress;
 
+void tcg_exec_init_add(void (*fn)(unsigned long));
 void tcg_exec_init(unsigned long tb_size);
 bool tcg_enabled(void);
 

translate-all.c +6 -1

index 62042af..b2edfb4 100644
@@ -677,7 +677,7 @@ static inline void code_gen_alloc(size_t tb_size)
 /* Must be called before using the QEMU cpus. 'tb_size' is the size
    (in bytes) allocated to the translation buffer. Zero means default
    size. */
-void tcg_exec_init(unsigned long tb_size)
+static void do_tcg_exec_init(unsigned long tb_size)
 {
     cpu_gen_init();
     code_gen_alloc(tb_size);

@@ -691,6 +691,11 @@ void tcg_exec_init(unsigned long tb_size)
 #endif
 }
 
+static __attribute__((constructor)) void register_tcg_exec_init(void)
+{
+    tcg_exec_init_add(do_tcg_exec_init);
+}
+
 bool tcg_enabled(void)
 {
     return tcg_ctx.code_gen_buffer != NULL;

translate-common.c new file +50 -1

new file mode 100644
index 0000000..563ae5a
@@ -0,0 +1,50 @@
+/*
+ *  Host code generation common components
+ *
+ *  Copyright (c) 2015 Peter Crosthwaite <[email protected]>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu-common.h"
+
+typedef struct TCGExecInitFn {
+    void (*do_tcg_exec_init)(unsigned long tb_size);
+    QLIST_ENTRY(TCGExecInitFn) list;
+} TCGExecInitFn;
+
+static QLIST_HEAD(, TCGExecInitFn) tcg_exec_init_list;
+
+void tcg_exec_init_add(void (*fn)(unsigned long))
+{
+    static bool inited;
+    TCGExecInitFn *lelem = g_malloc0(sizeof *lelem);
+
+    if (!inited) {
+        inited = true;
+        QLIST_INIT(&tcg_exec_init_list);
+    }
+
+    lelem->do_tcg_exec_init = fn;
+    QLIST_INSERT_HEAD(&tcg_exec_init_list, lelem, list);
+}
+
+void tcg_exec_init(unsigned long tb_size)
+{
+    TCGExecInitFn *t;
+
+    QLIST_FOREACH(t, &tcg_exec_init_list, list) {
+        t->do_tcg_exec_init(tb_size);
+    }
+}
--

1.9.1

Quoted 16 lines · Peter Crosthwaite

Create a global list of tcg_exec_init functions that is populated at startup. Multiple translation engines can register an init function and all will be called on the master call to tcg_exec_init.

Introduce a new module, translate-common. This is a common-obj for translation functionality such as this.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 Makefile.objs         |  1 +
 include/qemu-common.h |  1 +
 translate-all.c       |  7 ++++++-
 translate-common.c    | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 translate-common.c

Makefile.objs common-obj-y += dma-helpers.o · line 71

Quoted 10 lines

Makefile.objs +1 -0

index 4881d2c..294016e 100644
@@ -67,6 +67,7 @@ common-obj-y += dma-helpers.o
 common-obj-y += vl.o
 vl.o-cflags := $(GPROF_CFLAGS) $(SDL_CFLAGS)
 common-obj-y += tpm.o
+common-obj-y += translate-common.o
 

The functions added to this module needed to accessible from linux-user so this has to be an obj-y.

In system mode emulation (at least) this definition has no architecture specific dependencies. Move it to common code such that common code can use it (primarily for defining function prototypes).

Signed-off-by Peter Crosthwaite <[email protected]>

Quoted 4 lines · Peter Crosthwaite

In system mode emulation (at least) this definition has no architecture specific dependencies. Move it to common code such that common code can use it (primarily for defining function prototypes).

Quoted 1 line

Signed-off-by Peter Crosthwaite <[email protected]>

I suspect you can instead make a header that is included by arch-obj files, and move a lot of stuff there from include/exec/exec-all.h (for example all the prototypes that use tb_page_addr_t).

Paolo

Quoted 3 lines

I suspect you can instead make a header that is included by arch-obj files, and move a lot of stuff there from include/exec/exec-all.h (for example all the prototypes that use tb_page_addr_t).

So the problem was I needed this from cpu-qom which is a common-obj which is why I went for super-global on this one.

This function has no architecture specific dependencies and should be callable from core code. Move it to qom/cpu.h.

Signed-off-by Peter Crosthwaite <[email protected]>

---

From: Paolo Bonzini [mailto:[email protected]] On Behalf Of Paolo Bonzini

include/qom/cpu.h void cpu_watchpoint_remove_all(CPUState *cpu, int mask); · line 699

Quoted 5 lines · Peter Crosthwaite

include/qom/cpu.h +5 -0

+    /* If not executing code then assume we are ok.  */
+    if (cpu->current_tb == NULL) {
+        return true;
+    }
+    return cpu->can_do_io != 0;

For what it's worth, I think the "if" here is dead. Pavel?

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

include/qom/cpu.h void cpu_watchpoint_remove_all(CPUState *cpu, int mask); · line 699

Quoted 5 lines

include/qom/cpu.h +5 -0

+    /* If not executing code then assume we are ok.  */
+    if (cpu->current_tb == NULL) {
+        return true;
+    }
+    return cpu->can_do_io != 0;

For what it's worth, I think the "if" here is dead. Pavel?

cpu_can_do_io can be called from cpus.c and translate-all.c In both cases these calls could be made outside the generated code.

From: Paolo Bonzini [mailto:[email protected]]

Quoted 6 lines · Pavel Dovgaluk

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

Quoted 5 lines

include/qom/cpu.h +5 -0

+    /* If not executing code then assume we are ok.  */
+    if (cpu->current_tb == NULL) {
+        return true;
+    }
+    return cpu->can_do_io != 0;

Quoted 3 lines

Quoted 1 line

Quoted 1 line

For what it's worth, I think the "if" here is dead. Pavel?

cpu_can_do_io can be called from cpus.c and translate-all.c In both cases these calls could be made outside the generated code.

Yes, but doesn't your commit 626cf8f (icount: set can_do_io outside TB execution, 2014-12-08) cause cpu->can_do_io == 0 to imply cpu->current_tb != NULL?

Move this function to common code. It has no arch specific dependencies. Prepares support for multi-arch where the translate-all interface needs to be virtualised. One less thing to virtualise.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 translate-all.c    | 30 ------------------------------
 translate-common.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 30 deletions(-)

These exception indicies are generic and don't have any reliance on the per-arch cpu.h defs. Move them to cpu-all.h so they can be used by core code that does not have access to cpu-defs.h.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 include/exec/cpu-all.h  | 6 ++++++
 include/exec/cpu-defs.h | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

This is currently provided by cpu-defs and is a target specific definition. However, to prepare for multi-arch only the bare minimum content from cpu-defs.h should be exported to core code. And this is all we need. So split it to a new header that the target_multi cpu.h can include to save on having to include the ill-defined cpu-defs.h.

Allow multiple inclusion for multi-arch where multiple cpu.h's need to be included and target_long will vary for each.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 include/exec/cpu-defs.h    | 23 +-------------------
 include/exec/target-long.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 22 deletions(-)
 create mode 100644 include/exec/target-long.h

Quoted 9 lines · Peter Crosthwaite

This is currently provided by cpu-defs and is a target specific definition. However, to prepare for multi-arch only the bare minimum content from cpu-defs.h should be exported to core code. And this is all we need. So split it to a new header that the target_multi cpu.h can include to save on having to include the ill-defined cpu-defs.h.

Allow multiple inclusion for multi-arch where multiple cpu.h's need to be included and target_long will vary for each.

Quoted 6 lines

Signed-off-by Peter Crosthwaite <[email protected]>

---
 include/exec/cpu-defs.h    | 23 +-------------------
 include/exec/target-long.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 22 deletions(-)
 create mode 100644 include/exec/target-long.h

Multiple inclusion with a typedef? How's that supposed to work?

He later #defines target_{,u}long to e.g. arm_target_{,u}long.

From: "Peter Maydell" <[email protected]>
To: "Paolo Bonzini" <[email protected]>
Cc: "Richard Henderson" <[email protected]>, "Peter Crosthwaite" <[email protected]>, "QEMU Developers" <[email protected]>, "Edgar E. Iglesias" <[email protected]>, "Peter Crosthwaite" <[email protected]>, "Andreas Färber" <[email protected]>
Sent: Monday, June 1, 2015 10:25:03 PM
Subject: Re: [RFC v2 10/34] include/exec: Split target_long def to new header

Quoted 1 line · Paolo Bonzini

He later #defines target_{,u}long to e.g. arm_target_{,u}long.

Yikes.

Quoted 17 lines · Richard Henderson

Quoted 15 lines · Peter Crosthwaite

This is currently provided by cpu-defs and is a target specific definition. However, to prepare for multi-arch only the bare minimum content from cpu-defs.h should be exported to core code. And this is all we need. So split it to a new header that the target_multi cpu.h can include to save on having to include the ill-defined cpu-defs.h.

Allow multiple inclusion for multi-arch where multiple cpu.h's need to be included and target_long will vary for each.

Signed-off-by Peter Crosthwaite <[email protected]>

---
  include/exec/cpu-defs.h    | 23 +-------------------
  include/exec/target-long.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 53 insertions(+), 22 deletions(-)

create mode 100644 include/exec/target-long.h

Quoted 1 line

Multiple inclusion with a typedef? How's that supposed to work?

He later #defines target_{,u}long to e.g. arm_target_{,u}long.

Quoted 2 lines

He later #defines target_{,u}long to e.g. arm_target_{,u}long.

Ok, here's where I'm not liking things. It shouldn't be a typedef in some places and a define others. From this description, it sounds like it ought to always be a define.

Quoted 28 lines · Paolo Bonzini

Quoted 22 lines · Richard Henderson

Quoted 10 lines · Peter Crosthwaite

This is currently provided by cpu-defs and is a target specific definition. However, to prepare for multi-arch only the bare minimum content from cpu-defs.h should be exported to core code. And this is all we need. So split it to a new header that the target_multi cpu.h can include to save on having to include the ill-defined cpu-defs.h.

Allow multiple inclusion for multi-arch where multiple cpu.h's need to be included and target_long will vary for each.

Quoted 7 lines

Signed-off-by Peter Crosthwaite <[email protected]>

---
  include/exec/cpu-defs.h    | 23 +-------------------
  include/exec/target-long.h | 52
++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 53 insertions(+), 22 deletions(-)

create mode 100644 include/exec/target-long.h

Multiple inclusion with a typedef? How's that supposed to work?

Quoted 1 line

He later #defines target_{,u}long to e.g. arm_target_{,u}long.

Ok, here's where I'm not liking things. It shouldn't be a typedef in some places and a define others. From this description, it sounds like it ought to always be a define.

r~

The #define-always change does make for a cleaner end result but I stayed away from it purely because I was thinking typedefs are better for type-definitions. But if we are open to the change of the #define based implementation I am all for it as the target-foo/cpu.h change pattern in minimised.

We still have a similar problems with cpu-defs.h/CPUTLBEntry though. I have to think harder about how that can be done, but one solution is to conditionally change the tlb_table defs in CPU_COMMON to be just a dummy uint8_t[] in MULTI_ARCH case. This is ok, as the struct fields are only accessible by arch-obj-y which will get the full-service definition via non TARGET_MULTI_ARCH arch-obj-y compile. The work is half done for us, as CPUTLBTable already has a uint8_t padding system in place.

CPUIOTLBEntry can be moved to another header as it has no arch specific deps.

All in all, we can do this with 0 #define foo arm_foo in arch cpu.h's, with these edits.

Move the architecture agnostic function prototypes for exec.c out of cputlb.h to exec-all.h. This allows hiding of the arch specific cputlb.h from exec.c which should be getting close to having no architecture specifics. Prepares support for multi-arch, which will have a minimal cpu.h that services exec.c but not cputlb.h.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 exec.c                  |  1 -
 include/exec/cputlb.h   | 14 --------------
 include/exec/exec-all.h | 15 +++++++++++++++
 3 files changed, 15 insertions(+), 15 deletions(-)

exec.c +0 -1

index 0cb54f7..71ec173 100644
@@ -48,7 +48,6 @@
 #endif
 #include "exec/cpu-all.h"
 #include "qemu/rcu_queue.h"
-#include "exec/cputlb.h"
 #include "translate-all.h"
 
 #include "exec/memory-internal.h"

include/exec/cputlb.h +0 -14

index e0da9d7..9167ff6 100644
@@ -30,19 +30,5 @@ void cpu_tlb_reset_dirty_all(ram_addr_t start1, ram_addr_t length);
 void tlb_set_dirty(CPUArchState *env, target_ulong vaddr);
 extern int tlb_flush_count;
 
-/* exec.c */
-void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr);
-
-MemoryRegionSection *
-address_space_translate_for_iotlb(CPUState *cpu, hwaddr addr, hwaddr *xlat,
-                                  hwaddr *plen);
-hwaddr memory_region_section_get_iotlb(CPUState *cpu,
-                                       MemoryRegionSection *section,
-                                       target_ulong vaddr,
-                                       hwaddr paddr, hwaddr xlat,
-                                       int prot,
-                                       target_ulong *address);
-bool memory_region_is_unassigned(MemoryRegion *mr);
-
 #endif
 #endif

include/exec/exec-all.h +15 -0

index 12630ea..d52885e 100644
@@ -356,6 +356,21 @@ static inline tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong
 #else
 /* cputlb.c */
 tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr);
+
+/* exec.c */
+void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr);
+
+MemoryRegionSection *
+address_space_translate_for_iotlb(CPUState *cpu, hwaddr addr, hwaddr *xlat,
+                                  hwaddr *plen);
+hwaddr memory_region_section_get_iotlb(CPUState *cpu,
+                                       MemoryRegionSection *section,
+                                       target_ulong vaddr,
+                                       hwaddr paddr, hwaddr xlat,
+                                       int prot,
+                                       target_ulong *address);
+bool memory_region_is_unassigned(MemoryRegion *mr);
+
 #endif
 
 /* vl.c */

Quoted 6 lines · Peter Crosthwaite

Move the architecture agnostic function prototypes for exec.c out of cputlb.h to exec-all.h. This allows hiding of the arch specific cputlb.h from exec.c which should be getting close to having no architecture specifics. Prepares support for multi-arch, which will have a minimal cpu.h that services exec.c but not cputlb.h.

Quoted 1 line

Signed-off-by Peter Crosthwaite <[email protected]>

Since you are at it, tlb_protect_code/tlb_unprotect_code can also be moved to this new header (and to translate-common.c).

Paolo

Quoted 69 lines

---
 exec.c                  |  1 -
 include/exec/cputlb.h   | 14 --------------
 include/exec/exec-all.h | 15 +++++++++++++++
 3 files changed, 15 insertions(+), 15 deletions(-)

exec.c +0 -1

index 0cb54f7..71ec173 100644
@@ -48,7 +48,6 @@
 #endif
 #include "exec/cpu-all.h"
 #include "qemu/rcu_queue.h"
-#include "exec/cputlb.h"
 #include "translate-all.h"
 
 #include "exec/memory-internal.h"

include/exec/cputlb.h +0 -14

index e0da9d7..9167ff6 100644
@@ -30,19 +30,5 @@ void cpu_tlb_reset_dirty_all(ram_addr_t start1, ram_addr_t length);
 void tlb_set_dirty(CPUArchState *env, target_ulong vaddr);
 extern int tlb_flush_count;
 
-/* exec.c */
-void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr);
-
-MemoryRegionSection *
-address_space_translate_for_iotlb(CPUState *cpu, hwaddr addr, hwaddr *xlat,
-                                  hwaddr *plen);
-hwaddr memory_region_section_get_iotlb(CPUState *cpu,
-                                       MemoryRegionSection *section,
-                                       target_ulong vaddr,
-                                       hwaddr paddr, hwaddr xlat,
-                                       int prot,
-                                       target_ulong *address);
-bool memory_region_is_unassigned(MemoryRegion *mr);
-
 #endif
 #endif

include/exec/exec-all.h +15 -0

index 12630ea..d52885e 100644
@@ -356,6 +356,21 @@ static inline tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong
 #else
 /* cputlb.c */
 tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr);
+
+/* exec.c */
+void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr);
+
+MemoryRegionSection *
+address_space_translate_for_iotlb(CPUState *cpu, hwaddr addr, hwaddr *xlat,
+                                  hwaddr *plen);
+hwaddr memory_region_section_get_iotlb(CPUState *cpu,
+                                       MemoryRegionSection *section,
+                                       target_ulong vaddr,
+                                       hwaddr paddr, hwaddr xlat,
+                                       int prot,
+                                       target_ulong *address);
+bool memory_region_is_unassigned(MemoryRegion *mr);
+
 #endif
 
 /* vl.c */

Quoted 11 lines · Paolo Bonzini

Quoted 7 lines · Peter Crosthwaite

Move the architecture agnostic function prototypes for exec.c out of cputlb.h to exec-all.h. This allows hiding of the arch specific cputlb.h from exec.c which should be getting close to having no architecture specifics. Prepares support for multi-arch, which will have a minimal cpu.h that services exec.c but not cputlb.h.

Signed-off-by Peter Crosthwaite <[email protected]>

Quoted 2 lines

Since you are at it, tlb_protect_code/tlb_unprotect_code can also be moved to this new header (and to translate-common.c).

Just a follow up patch? There is RB from Richard so would like to keep it self contained. I can put this extra on the end of the "header" series which I will split off from this series as the next non-RFC component.

Move the architecture agnostic function prototypes for exec.c out of cputlb.h to exec-all.h. This allows hiding of the arch specific cputlb.h from exec.c which should be getting close to having no architecture specifics. Prepares support for multi-arch, which will have a minimal cpu.h that services exec.c but not cputlb.h.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 exec.c                  |  1 -
 include/exec/cputlb.h   | 14 --------------
 include/exec/exec-all.h | 15 +++++++++++++++
 3 files changed, 15 insertions(+), 15 deletions(-)

exec.c +0 -0

 #include "translate-all.h"
 
 #include "exec/memory-internal.h"

This is one of very few things in exec-all with a genuine CPU architecture dependency. Move these hashing helpers to a new header to trim exec-all.h down to a near architecture-agnostic header.

The defs are only used by cpu-exec and translate-all which are both arch-obj's so the new tb-hash.h has no core code usage.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 cpu-exec.c              |  2 ++
 include/exec/exec-all.h | 20 --------------------
 include/exec/tb-hash.h  | 43 +++++++++++++++++++++++++++++++++++++++++++
 target-multi/helper.h   |  1 +
 translate-all.c         |  1 +
 5 files changed, 47 insertions(+), 20 deletions(-)
 create mode 100644 include/exec/tb-hash.h
 create mode 100644 target-multi/helper.h

cpu-exec.c +2 -0

index dbea47c..f255ea9 100644
@@ -27,6 +27,8 @@
 #include "exec/address-spaces.h"
 #include "qemu/rcu.h"
 
+#include "exec/tb-hash.h"
+
 /* -icount align implementation. */
 
 typedef struct SyncClocks {

include/exec/exec-all.h +0 -20

index d52885e..745cb4a 100644
@@ -195,26 +195,6 @@ struct TBContext {
     int tb_invalidated_flag;
 };
 
-static inline unsigned int tb_jmp_cache_hash_page(target_ulong pc)
-{
-    target_ulong tmp;
-    tmp = pc ^ (pc >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS));
-    return (tmp >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS)) & TB_JMP_PAGE_MASK;
-}
-
-static inline unsigned int tb_jmp_cache_hash_func(target_ulong pc)
-{
-    target_ulong tmp;
-    tmp = pc ^ (pc >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS));
-    return (((tmp >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS)) & TB_JMP_PAGE_MASK)
-	    | (tmp & TB_JMP_ADDR_MASK));
-}
-
-static inline unsigned int tb_phys_hash_func(tb_page_addr_t pc)
-{
-    return (pc >> 2) & (CODE_GEN_PHYS_HASH_SIZE - 1);
-}
-
 void tb_free(TranslationBlock *tb);
 void tb_flush(CPUState *cpu);
 void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);

include/exec/tb-hash.h new file +43 -0

new file mode 100644
index 0000000..e0bd786
@@ -0,0 +1,43 @@
+/*
+ * internal execution defines for qemu
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef EXEC_TB_HASH
+#define EXEC_TB_HASH
+
+static inline unsigned int tb_jmp_cache_hash_page(target_ulong pc)
+{
+    target_ulong tmp;
+    tmp = pc ^ (pc >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS));
+    return (tmp >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS)) & TB_JMP_PAGE_MASK;
+}
+
+static inline unsigned int tb_jmp_cache_hash_func(target_ulong pc)
+{
+    target_ulong tmp;
+    tmp = pc ^ (pc >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS));
+    return (((tmp >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS)) & TB_JMP_PAGE_MASK)
+           | (tmp & TB_JMP_ADDR_MASK));
+}
+
+static inline unsigned int tb_phys_hash_func(tb_page_addr_t pc)
+{
+    return (pc >> 2) & (CODE_GEN_PHYS_HASH_SIZE - 1);
+}
+
+#endif

target-multi/helper.h new file +1 -0

new file mode 100644
index 0000000..6b9ee59
@@ -0,0 +1 @@
+/* Multi arch has no helpers, but core code expects this file anyway */

translate-all.c +1 -0

index 7d27c5d..bf0d689 100644
@@ -58,6 +58,7 @@
 #endif
 
 #include "exec/cputlb.h"
+#include "exec/tb-hash.h"
 #include "translate-all.h"
 #include "qemu/bitmap.h"
 #include "qemu/timer.h"

This is one of very few things in exec-all with a genuine CPU architecture dependency. Move these hashing helpers to a new header to trim exec-all.h down to a near architecture-agnostic header.

The defs are only used by cpu-exec and translate-all which are both arch-obj's so the new tb-hash.h has no core code usage.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 cpu-exec.c              |  2 ++
 include/exec/exec-all.h | 20 --------------------
 include/exec/tb-hash.h  | 43 +++++++++++++++++++++++++++++++++++++++++++
 target-multi/helper.h   |  1 +
 translate-all.c         |  1 +
 5 files changed, 47 insertions(+), 20 deletions(-)
 create mode 100644 include/exec/tb-hash.h
 create mode 100644 target-multi/helper.h

This is one of very few things in exec-all with a genuine CPU architecture dependency. Move these hashing helpers to a new header to trim exec-all.h down to a near architecture-agnostic header.

The defs are only used by cpu-exec and translate-all which are both arch-obj's so the new tb-hash.h has no core code usage.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 cpu-exec.c              |  2 ++
 include/exec/exec-all.h | 20 --------------------
 include/exec/tb-hash.h  | 43 +++++++++++++++++++++++++++++++++++++++++++
 target-multi/helper.h   |  1 +

 translate-all.c         |  1 +
 5 files changed, 47 insertions(+), 20 deletions(-)
 create mode 100644 include/exec/tb-hash.h
 create mode 100644 target-multi/helper.h

cpu-exec.c +2 -0

index dbea47c..f255ea9 100644
@@ -27,6 +27,8 @@
 #include "exec/address-spaces.h"
 #include "qemu/rcu.h"
 
+#include "exec/tb-hash.h"
+
 /* -icount align implementation. */
 
 typedef struct SyncClocks {

include/exec/exec-all.h +0 -20

index d52885e..745cb4a 100644
@@ -195,26 +195,6 @@ struct TBContext {
     int tb_invalidated_flag;
 };
 
-static inline unsigned int tb_jmp_cache_hash_page(target_ulong pc)
-{
-    target_ulong tmp;
-    tmp = pc ^ (pc >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS));
-    return (tmp >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS)) & TB_JMP_PAGE_MASK;
-}
-
-static inline unsigned int tb_jmp_cache_hash_func(target_ulong pc)
-{
-    target_ulong tmp;
-    tmp = pc ^ (pc >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS));
-    return (((tmp >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS)) & TB_JMP_PAGE_MASK)
-	    | (tmp & TB_JMP_ADDR_MASK));
-}
-
-static inline unsigned int tb_phys_hash_func(tb_page_addr_t pc)
-{
-    return (pc >> 2) & (CODE_GEN_PHYS_HASH_SIZE - 1);
-}
-
 void tb_free(TranslationBlock *tb);
 void tb_flush(CPUState *cpu);
 void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);

include/exec/tb-hash.h new file +43 -0

new file mode 100644
index 0000000..e0bd786
@@ -0,0 +1,43 @@
+/*
+ * internal execution defines for qemu
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef EXEC_TB_HASH
+#define EXEC_TB_HASH
+
+static inline unsigned int tb_jmp_cache_hash_page(target_ulong pc)
+{
+    target_ulong tmp;
+    tmp = pc ^ (pc >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS));
+    return (tmp >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS)) & TB_JMP_PAGE_MASK;
+}
+
+static inline unsigned int tb_jmp_cache_hash_func(target_ulong pc)
+{
+    target_ulong tmp;
+    tmp = pc ^ (pc >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS));
+    return (((tmp >> (TARGET_PAGE_BITS - TB_JMP_PAGE_BITS)) & TB_JMP_PAGE_MASK)
+           | (tmp & TB_JMP_ADDR_MASK));
+}
+
+static inline unsigned int tb_phys_hash_func(tb_page_addr_t pc)
+{
+    return (pc >> 2) & (CODE_GEN_PHYS_HASH_SIZE - 1);
+}
+
+#endif

target-multi/helper.h new file +1 -0

new file mode 100644
index 0000000..6b9ee59
@@ -0,0 +1 @@
+/* Multi arch has no helpers, but core code expects this file anyway */

translate-all.c +1 -0

index 7d27c5d..bf0d689 100644
@@ -58,6 +58,7 @@
 #endif
 
 #include "exec/cputlb.h"
+#include "exec/tb-hash.h"
 #include "translate-all.h"
 #include "qemu/bitmap.h"
 #include "qemu/timer.h"

These are not Architecture specific in any way so move them out of cpu-defs.h. tb-hash.h is an appropriate place as a leading user and their strong relationship to TB hashing and caching.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 include/exec/cpu-defs.h | 8 --------
 include/exec/tb-hash.h  | 8 ++++++++
 2 files changed, 8 insertions(+), 8 deletions(-)

Allow subsequent inclusion of cpu-defs.h. This allows including multiple cpu.h's and each getting the right set of definitions for its env structure definition. All define symbols are undeffed and redeffed to the new values.

CPUTLBEntry and CPUIOTLBEntry need to be renamed via #define by the caller to avoid namespace collisions.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 include/exec/cpu-defs.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

include/exec/cpu-defs.h +11 -3

index a1c418f..6b59e58 100644
@@ -16,8 +16,10 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
-#ifndef CPU_DEFS_H
-#define CPU_DEFS_H
+
+/* No multiple include guard intended. Multi-arch setups may require multiple
+ * cpu.h's included which means this can be and should be reached twice.
+ */
 
 #ifndef NEED_CPU_H
 #error cpu.h included from common code

@@ -34,12 +36,19 @@
 
 #include "exec/target-long.h"
 
+#undef CPU_COMMON
+#undef CPU_COMMON_TLB
+
 #if !defined(CONFIG_USER_ONLY)
+#undef CPU_TLB_BITS
+#undef CPU_TLB_SIZE
+#undef CPU_VTLB_SIZE
 #define CPU_TLB_BITS 8
 #define CPU_TLB_SIZE (1 << CPU_TLB_BITS)
 /* use a fully associative victim tlb of 8 entries */
 #define CPU_VTLB_SIZE 8
 
+#undef CPU_TLB_ENTRY_BITS
 #if HOST_LONG_BITS == 32 && TARGET_LONG_BITS == 32
 #define CPU_TLB_ENTRY_BITS 4
 #else

@@ -100,4 +109,3 @@ typedef struct CPUIOTLBEntry {
     /* soft mmu support */                                              \
     CPU_COMMON_TLB                                                      \
 
-#endif

Quoted 7 lines · Peter Crosthwaite

Allow subsequent inclusion of cpu-defs.h. This allows including multiple cpu.h's and each getting the right set of definitions for its env structure definition. All define symbols are undeffed and redeffed to the new values.

CPUTLBEntry and CPUIOTLBEntry need to be renamed via #define by the caller to avoid namespace collisions.

Please document these things (#undeffing of preprocessor symbols and what needs to be renamed) in the header.

Also the same is true for include/exec/target-long.h---where additionally I am not sure about how you'd deal with a redefined typedef. Some compilers flag it as an error even if the source type is the same.

Not dealing with this problem as of this RFC, but comments on how to solve it welcome. The approach adopted by this series is to implement multi-arch as multiple translators, so a single global TCG profiler is now ill-defined.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 monitor.c | 4 ++++
 1 file changed, 4 insertions(+)

monitor.c +4 -0

index 1a17cf3..f3ee785 100644
@@ -1036,13 +1036,17 @@ static void hmp_info_registers(Monitor *mon, const QDict *qdict)
 
 static void hmp_info_jit(Monitor *mon, const QDict *qdict)
 {
+#if 0
     dump_exec_info((FILE *)mon, monitor_fprintf);
+#endif
     dump_drift_info((FILE *)mon, monitor_fprintf);
 }
 
 static void hmp_info_opcount(Monitor *mon, const QDict *qdict)
 {
+#if 0
     dump_opcount_info((FILE *)mon, monitor_fprintf);
+#endif
 }
 
 static void hmp_info_history(Monitor *mon, const QDict *qdict)

Also for example tcg/tcg-opc.h should only depend on TCG_TARGET_REG_BITS, i.e. not on the arch. Hence tcg_op_defs should also be arch-independent.

Quoted 4 lines · Peter Crosthwaite

Not dealing with this problem as of this RFC, but comments on how to solve it welcome. The approach adopted by this series is to implement multi-arch as multiple translators, so a single global TCG profiler is now ill-defined.

Probably some kind of tcg-common.c? The counts can be added together for all CPUs, and hence moved out of tcg_ctx.

Also for example tcg/tcg-opc.h should only depend on TCG_TARGET_REG_BITS, i.e. not on the arch. Hence tcg_op_defs should also be arch-independent.

Paolo

Quoted 28 lines

Signed-off-by Peter Crosthwaite <[email protected]>

---
 monitor.c | 4 ++++
 1 file changed, 4 insertions(+)

monitor.c +4 -0

index 1a17cf3..f3ee785 100644
@@ -1036,13 +1036,17 @@ static void hmp_info_registers(Monitor *mon, const QDict *qdict)
 
 static void hmp_info_jit(Monitor *mon, const QDict *qdict)
 {
+#if 0
     dump_exec_info((FILE *)mon, monitor_fprintf);
+#endif
     dump_drift_info((FILE *)mon, monitor_fprintf);
 }
 
 static void hmp_info_opcount(Monitor *mon, const QDict *qdict)
 {
+#if 0
     dump_opcount_info((FILE *)mon, monitor_fprintf);
+#endif
 }
 
 static void hmp_info_history(Monitor *mon, const QDict *qdict)

Not dealing with this issue as of this RFC. Comments welcome on how to handle it. It is probably a case that any form of CPU selection is ill defined in multi-arch. So just a case of #ifndef TARGET_MULTI or something like that?

Signed-off-by Peter Crosthwaite <[email protected]>

---
 cpus.c | 2 ++
 1 file changed, 2 insertions(+)

cpus.c +2 -0

index 2dc4a9a..441a026 100644
@@ -1446,9 +1446,11 @@ void cpu_loop_exit(CPUState *cpu)
 void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
 {
     /* XXX: implement xxx_cpu_list for targets that still miss it */
+#if 0
 #if defined(cpu_list)
     cpu_list(f, cpu_fprintf);
 #endif
+#endif
 }
 
 CpuInfoList *qmp_query_cpus(Error **errp)

This is almost certainly wrong but it should work for my test case where TARGET_PAGE_SIZE is cross-arch consistent. This is needed to get vfio/pci.c to compile due to it's use of HOST_PAGE_ALIGN.

Cc Alex Williamson <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

---
 translate-all.c    | 4 ----
 translate-common.c | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

Quoted 10 lines · Peter Crosthwaite

This is almost certainly wrong but it should work for my test case where TARGET_PAGE_SIZE is cross-arch consistent. This is needed to get vfio/pci.c to compile due to it's use of HOST_PAGE_ALIGN.

Cc Alex Williamson <[email protected]>

Signed-off-by Peter Crosthwaite <[email protected]>

---
 translate-all.c    | 4 ----
 translate-common.c | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

Actually, this isn't a hack at all, since here we're talking about the HOST page size. Which is of course completely independent of the TARGET page sizes.

r~

This won't work in multi-arch where this service is not provided to core code. The architecture specific uses of this in monitor.c should have their functionality migrated out of monitor.c to arch specific virtualised hooks. For the moment, just get rid of it to enable testing of multi-arch on platforms that don't need this.

Signed-off-by Peter Crosthwaite <[email protected]>

---
 monitor.c | 1 -
 1 file changed, 1 deletion(-)

monitor.c +0 -0

index f3ee785..ab6024a 100644

monitor.c +0 -1

 #include "qmp-commands.h"
 #include "hmp.h"
 #include "qemu/thread.h"
--

1.9.1

It does not work in multi-arch as it requires the CPU specific TARGET_VIRT_ADDR_SPACE_BITS global define. Just use the generic version that does no masking. If this masking is still needed (??) then the variable can be virtualised in the CPUState.

Signed-off-by Peter Crosthwaite <[email protected]>

disas.c generic_print_address (bfd_vma addr, struct disassemble_info *info) · line 82

Quoted 8 lines · Peter Crosthwaite

disas.c +0 -8

-/* Print address in hex, truncated to the width of a target virtual address. */
-static void
-generic_print_target_address(bfd_vma addr, struct disassemble_info *info)
-{
-    uint64_t mask = ~0ULL >> (64 - TARGET_VIRT_ADDR_SPACE_BITS);
-    generic_print_address(addr & mask, info);
-}
-

IMO, we could just drop the masking.

I don't know of any target which doesn't at least sanity check the high bits of an address before looking up the virtual address in the page tables.

So again, I don't think this is a hack at all, just cleaning up cruft.

r~

Not sure as to whether virtualise or not here, just commenting out for the moment as watchpoints are nor critical to this RFC.

Signed-off-by Peter Crosthwaite <[email protected]>

Quoted 4 lines · Peter Crosthwaite

Not sure as to whether virtualise or not here, just commenting out for the moment as watchpoints are nor critical to this RFC.

Signed-off-by Peter Crosthwaite <[email protected]>

Yes, it should be virtualized.

Paolo