More core code ENV_GET_CPU removals

PATCH Applied Peter Crosthwaite 4 participants 18 messages Jun 24, 2015 – Jul 2, 2015

lore

Share & tools

lore

Peter Crosthwaite

author

2015-06-24 02:31 UTC #0 lore raw

v4 cover More core code ENV_GET_CPU removals

Hi All,

I'm moving towards the goal of having no core code usages of ENV_GET_CPU. This has two advantages:

1: It means we are closer to common-obj'ing core code like exec.c, cpus.c and friends.
2: Multi arch is easier if ENV_GET_CPU() stays arch specific. It means I don't need those patches where I reorder the env within the arch specific CPUState. This allows continuing placement of arch specifics before the env in the CPU container (which has TCG perf advantages).

Due to point 1, I'm sending this ahead as I think it has standalone value, rather than send as part of multi-arch.

Regards,
Peter

changed since v3:
Fix C99 declaration.
Drop P8

changed since v2:
Fix CPU_GET_ENV typo
Prepend Bharata's patches

changed since v1 (AF, Eduardo review):
Rebase of Bharata's patches
Add () to fn's in commit messages
Remove uses of ENV_GET_CPU() in target code
Include and complete cpu-exec patch (formerly in multi-arch RFC v2)

Bharata B Rao (3):
cpus: Add Error argument to cpu_exec_init()
cpus: Convert cpu_index into a bitmap
ppc: Move cpu_exec_init() call to realize function

Peter Crosthwaite (4):
translate-all: Change tb_flush() env argument to cpu
gdbserver: _fork: Change fn to accept cpu instead of env
cpus: Change tcg_cpu_exec() arg to cpu, not env
cpus: Change exec_init() arg to cpu, not env

 bsd-user/main.c             |  2 +-
 cpus.c                      |  7 +++--
 dtc                         |  2 +-
 exec.c                      | 63 ++++++++++++++++++++++++++++++++++++++-------
 gdbstub.c                   |  9 +++----
 include/exec/exec-all.h     |  4 +--
 include/exec/gdbstub.h      |  2 +-
 include/qom/cpu.h           |  1 +
 linux-user/main.c           |  2 +-
 linux-user/signal.c         |  2 +-
 qom/cpu.c                   |  7 +++++
 target-alpha/cpu.c          |  2 +-
 target-alpha/sys_helper.c   |  2 +-
 target-arm/cpu.c            |  2 +-
 target-cris/cpu.c           |  2 +-
 target-i386/cpu.c           |  2 +-
 target-i386/translate.c     |  2 +-
 target-lm32/cpu.c           |  2 +-
 target-m68k/cpu.c           |  2 +-
 target-microblaze/cpu.c     |  2 +-
 target-mips/cpu.c           |  2 +-
 target-moxie/cpu.c          |  2 +-
 target-openrisc/cpu.c       |  2 +-
 target-ppc/translate_init.c |  9 +++++--
 target-s390x/cpu.c          |  2 +-
 target-sh4/cpu.c            |  2 +-
 target-sparc/cpu.c          |  2 +-
 target-tricore/cpu.c        |  2 +-
 target-unicore32/cpu.c      |  2 +-
 target-xtensa/cpu.c         |  2 +-
 translate-all.c             |  6 ++---
 31 files changed, 101 insertions(+), 51 deletions(-)

Signature · 1 line

1.9.1

Peter Crosthwaite

author

2015-06-24 02:31 UTC #1 lore raw

PATCH v4 1/7 cpus: Add Error argument to cpu_exec_init()

From: Bharata B Rao <[email protected]>

Add an Error argument to cpu_exec_init() to let users collect the error. This is in preparation to change the CPU enumeration logic in cpu_exec_init(). With the new enumeration logic, cpu_exec_init() can fail if cpu_index values corresponding to max_cpus have already been handed out.

Since all current callers of cpu_exec_init() are from instance_init, use error_abort Error arugment to abort in case of an error.

Signed-off-by Bharata B Rao <[email protected]>

Reviewed-by Eduardo Habkost <[email protected]>

Reviewed-by Igor Mammedov <[email protected]>

Reviewed-by David Gibson <[email protected]>

Reviewed-by Peter Crosthwaite <[email protected]>

Acked-by Paolo Bonzini <[email protected]>

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

---
 exec.c                      | 2 +-
 include/exec/exec-all.h     | 2 +-
 target-alpha/cpu.c          | 2 +-
 target-arm/cpu.c            | 2 +-
 target-cris/cpu.c           | 2 +-
 target-i386/cpu.c           | 2 +-
 target-lm32/cpu.c           | 2 +-
 target-m68k/cpu.c           | 2 +-
 target-microblaze/cpu.c     | 2 +-
 target-mips/cpu.c           | 2 +-
 target-moxie/cpu.c          | 2 +-
 target-openrisc/cpu.c       | 2 +-
 target-ppc/translate_init.c | 2 +-
 target-s390x/cpu.c          | 2 +-
 target-sh4/cpu.c            | 2 +-
 target-sparc/cpu.c          | 2 +-
 target-tricore/cpu.c        | 2 +-
 target-unicore32/cpu.c      | 2 +-
 target-xtensa/cpu.c         | 2 +-
 19 files changed, 19 insertions(+), 19 deletions(-)

exec.c +1 -1

index f7883d2..a822f30 100644
@@ -513,7 +513,7 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as)
 }
 #endif
 
-void cpu_exec_init(CPUArchState *env)
+void cpu_exec_init(CPUArchState *env, Error **errp)
 {
     CPUState *cpu = ENV_GET_CPU(env);
     CPUClass *cc = CPU_GET_CLASS(cpu);

include/exec/exec-all.h +1 -1

index 2573e8c..a68c2e3 100644
@@ -88,7 +88,7 @@ void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 TranslationBlock *tb_gen_code(CPUState *cpu,
                               target_ulong pc, target_ulong cs_base, int flags,
                               int cflags);
-void cpu_exec_init(CPUArchState *env);
+void cpu_exec_init(CPUArchState *env, Error **errp);
 void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
 
 #if !defined(CONFIG_USER_ONLY)

target-alpha/cpu.c +1 -1

index a98b7d8..e865ba7 100644
@@ -257,7 +257,7 @@ static void alpha_cpu_initfn(Object *obj)
     CPUAlphaState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
     tlb_flush(cs, 1);
 
     alpha_translate_init();

target-arm/cpu.c +1 -1

index 80669a6..1c40cc0 100644
@@ -392,7 +392,7 @@ static void arm_cpu_initfn(Object *obj)
     uint32_t Aff1, Aff0;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(&cpu->env);
+    cpu_exec_init(&cpu->env, &error_abort);
     cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
                                          g_free, g_free);

target-cris/cpu.c +1 -1

index 16cfba9..bb8e7ea 100644
@@ -170,7 +170,7 @@ static void cris_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     env->pregs[PR_VR] = ccc->vr;

target-i386/cpu.c +1 -1

index 36b07f9..433f38b 100644
@@ -3007,7 +3007,7 @@ static void x86_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     object_property_add(obj, "family", "int",
                         x86_cpuid_version_get_family,

target-lm32/cpu.c +1 -1

index f8081f5..da4fde1 100644
@@ -151,7 +151,7 @@ static void lm32_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     env->flags = 0;

target-m68k/cpu.c +1 -1

index 4cfb725..ae3d765 100644
@@ -168,7 +168,7 @@ static void m68k_cpu_initfn(Object *obj)
     static bool inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     if (tcg_enabled() && !inited) {
         inited = true;

target-microblaze/cpu.c +1 -1

index c592bf7..967ea01 100644
@@ -190,7 +190,7 @@ static void mb_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     set_float_rounding_mode(float_round_nearest_even, &env->fp_status);

target-mips/cpu.c +1 -1

index 958c999..1fd9f22 100644
@@ -115,7 +115,7 @@ static void mips_cpu_initfn(Object *obj)
     CPUMIPSState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     if (tcg_enabled()) {
         mips_tcg_init();

target-moxie/cpu.c +1 -1

index 47b617f..415c65a 100644
@@ -66,7 +66,7 @@ static void moxie_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(&cpu->env);
+    cpu_exec_init(&cpu->env, &error_abort);
 
     if (tcg_enabled() && !inited) {
         inited = 1;

target-openrisc/cpu.c +1 -1

index 39bedc1..cd6c657 100644
@@ -92,7 +92,7 @@ static void openrisc_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(&cpu->env);
+    cpu_exec_init(&cpu->env, &error_abort);
 
 #ifndef CONFIG_USER_ONLY
     cpu_openrisc_mmu_init(cpu);

target-ppc/translate_init.c +1 -1

index d74f4f0..52d95ce 100644
@@ -9633,7 +9633,7 @@ static void ppc_cpu_initfn(Object *obj)
     CPUPPCState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
     cpu->cpu_dt_id = cs->cpu_index;
 
     env->msr_mask = pcc->msr_mask;

target-s390x/cpu.c +1 -1

index 69bac35..ced5592 100644
@@ -212,7 +212,7 @@ static void s390_cpu_initfn(Object *obj)
 #endif
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 #if !defined(CONFIG_USER_ONLY)
     qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
     qemu_get_timedate(&tm, 0);

target-sh4/cpu.c +1 -1

index cccb14f..2c2060b 100644
@@ -248,7 +248,7 @@ static void superh_cpu_initfn(Object *obj)
     CPUSH4State *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     env->movcal_backup_tail = &(env->movcal_backup);

target-sparc/cpu.c +1 -1

index a952097..4d1da7c 100644
@@ -802,7 +802,7 @@ static void sparc_cpu_initfn(Object *obj)
     CPUSPARCState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     if (tcg_enabled()) {
         gen_intermediate_code_init(env);

target-tricore/cpu.c +1 -1

index b3e5512..6fa60b1 100644
@@ -92,7 +92,7 @@ static void tricore_cpu_initfn(Object *obj)
     CPUTriCoreState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     if (tcg_enabled()) {
         tricore_tcg_init();

target-unicore32/cpu.c +1 -1

index 5b32987..eda039c 100644
@@ -111,7 +111,7 @@ static void uc32_cpu_initfn(Object *obj)
     static bool inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
 #ifdef CONFIG_USER_ONLY
     env->uncached_asr = ASR_MODE_USER;

target-xtensa/cpu.c +1 -1

index 2b75678..8d69d23 100644
@@ -114,7 +114,7 @@ static void xtensa_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     env->config = xcc->config;
-    cpu_exec_init(env);
+    cpu_exec_init(env, &error_abort);
 
     if (tcg_enabled() && !tcg_inited) {
         tcg_inited = true;

Signature · 1 line

1.9.1

Peter Crosthwaite

author

2015-06-24 02:31 UTC #3 lore raw

PATCH v4 2/7 cpus: Convert cpu_index into a bitmap

In reply to Peter Crosthwaite

From: Bharata B Rao <[email protected]>

Currently CPUState.cpu_index is monotonically increasing and a newly created CPU always gets the next higher index. The next available index is calculated by counting the existing number of CPUs. This is fine as long as we only add CPUs, but there are architectures which are starting to support CPU removal too. For an architecture like PowerPC which derives its CPU identifier (device tree ID) from cpu_index, the existing logic of generating cpu_index values causes problems.

With the currently proposed method of handling vCPU removal by parking the vCPU fd in QEMU
(Ref: http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg02604.html),
generating cpu_index this way will not work for PowerPC.

This patch changes the way cpu_index is handed out by maintaining a bit map of the CPUs that tracks both addition and removal of CPUs.

The CPU bitmap allocation logic is part of cpu_exec_init() which is called by instance_init routines of various CPU targets. Newly added cpu_exec_exit() API handles the deallocation part and this routine is called from generic CPU::instance_finalize().

Note: This new CPU enumeration is for !CONFIG_USER_ONLY only. CONFIG_USER_ONLY continues to have the old enumeration logic.

Signed-off-by Bharata B Rao <[email protected]>

Reviewed-by Eduardo Habkost <[email protected]>

Reviewed-by Igor Mammedov <[email protected]>

Reviewed-by David Gibson <[email protected]>

Reviewed-by Peter Crosthwaite <[email protected]>

Acked-by Paolo Bonzini <[email protected]>

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

---
 exec.c            | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 include/qom/cpu.h |  1 +
 qom/cpu.c         |  7 +++++++
 3 files changed, 58 insertions(+), 5 deletions(-)

exec.c +50 -5

index a822f30..9a9f734 100644
@@ -513,21 +513,66 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as)
 }
 #endif
 
+#ifndef CONFIG_USER_ONLY
+static DECLARE_BITMAP(cpu_index_map, MAX_CPUMASK_BITS);
+
+static int cpu_get_free_index(Error **errp)
+{
+    int cpu = find_first_zero_bit(cpu_index_map, max_cpus);
+
+    if (cpu >= max_cpus) {
+        error_setg(errp, "Trying to use more CPUs than allowed max of %d\n",
+                   max_cpus);
+        return -1;
+    }
+
+    bitmap_set(cpu_index_map, cpu, 1);
+    return cpu;
+}
+
+void cpu_exec_exit(CPUState *cpu)
+{
+    if (cpu->cpu_index == -1) {
+        /* cpu_index was never allocated by this @cpu or was already freed. */
+        return;
+    }
+
+    bitmap_clear(cpu_index_map, cpu->cpu_index, 1);
+    cpu->cpu_index = -1;
+}
+#else
+
+static int cpu_get_free_index(Error **errp)
+{
+    CPUState *some_cpu;
+    int cpu_index = 0;
+
+    CPU_FOREACH(some_cpu) {
+        cpu_index++;
+    }
+    return cpu_index;
+}
+
+void cpu_exec_exit(CPUState *cpu)
+{
+}
+#endif
+
 void cpu_exec_init(CPUArchState *env, Error **errp)
 {
     CPUState *cpu = ENV_GET_CPU(env);
     CPUClass *cc = CPU_GET_CLASS(cpu);
-    CPUState *some_cpu;
     int cpu_index;
+    Error *local_err = NULL;
 
 #if defined(CONFIG_USER_ONLY)
     cpu_list_lock();
 #endif
-    cpu_index = 0;
-    CPU_FOREACH(some_cpu) {
-        cpu_index++;
+    cpu_index = cpu->cpu_index = cpu_get_free_index(&local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
     }
-    cpu->cpu_index = cpu_index;
     cpu->numa_node = 0;
     QTAILQ_INIT(&cpu->breakpoints);
     QTAILQ_INIT(&cpu->watchpoints);

include/qom/cpu.h +1 -0

index 39f0f19..7db310e 100644
@@ -672,6 +672,7 @@ void cpu_watchpoint_remove_all(CPUState *cpu, int mask);
 
 void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
+void cpu_exec_exit(CPUState *cpu);
 
 #ifdef CONFIG_SOFTMMU
 extern const struct VMStateDescription vmstate_cpu_common;

qom/cpu.c +7 -0

index 108bfa2..061a0c3 100644
@@ -312,9 +312,15 @@ static void cpu_common_initfn(Object *obj)
     CPUState *cpu = CPU(obj);
     CPUClass *cc = CPU_GET_CLASS(obj);
 
+    cpu->cpu_index = -1;
     cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
 }
 
+static void cpu_common_finalize(Object *obj)
+{
+    cpu_exec_exit(CPU(obj));
+}
+
 static int64_t cpu_common_get_arch_id(CPUState *cpu)
 {
     return cpu->cpu_index;

@@ -356,6 +362,7 @@ static const TypeInfo cpu_type_info = {
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(CPUState),
     .instance_init = cpu_common_initfn,
+    .instance_finalize = cpu_common_finalize,
     .abstract = true,
     .class_size = sizeof(CPUClass),
     .class_init = cpu_class_init,

Signature · 1 line

1.9.1

Peter Crosthwaite

author

2015-06-24 02:31 UTC #6 lore raw

PATCH v4 3/7 ppc: Move cpu_exec_init() call to realize function

In reply to Peter Crosthwaite

From: Bharata B Rao <[email protected]>

Move cpu_exec_init() call from instance_init to realize. This allows any failures from cpu_exec_init() to be handled appropriately. Also add corresponding cpu_exec_exit() call from unrealize.

cpu_dt_id assignment from instance_init is no longer needed since correct assignment for cpu_dt_id is already present in realizefn.

Signed-off-by Bharata B Rao <[email protected]>

Reviewed-by David Gibson <[email protected]>

Reviewed-by Peter Crosthwaite <[email protected]>

Acked-by Paolo Bonzini <[email protected]>

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

---
 target-ppc/translate_init.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

target-ppc/translate_init.c +7 -2

index 52d95ce..2b72f2d 100644
@@ -8928,6 +8928,11 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
+    cpu_exec_init(&cpu->env, &local_err);
+    if (local_err != NULL) {
+        error_propagate(errp, local_err);
+        return;
+    }
     cpu->cpu_dt_id = (cs->cpu_index / smp_threads) * max_smt
         + (cs->cpu_index % smp_threads);
 #endif

@@ -9141,6 +9146,8 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
     opc_handler_t **table;
     int i, j;
 
+    cpu_exec_exit(CPU(dev));
+
     for (i = 0; i < PPC_CPU_OPCODES_LEN; i++) {
         if (env->opcodes[i] == &invalid_handler) {
             continue;

@@ -9633,8 +9640,6 @@ static void ppc_cpu_initfn(Object *obj)
     CPUPPCState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
-    cpu->cpu_dt_id = cs->cpu_index;
 
     env->msr_mask = pcc->msr_mask;
     env->mmu_model = pcc->mmu_model;

Signature · 1 line

1.9.1

Peter Crosthwaite

author

2015-06-24 02:31 UTC #7 lore raw

PATCH v4 4/7 translate-all: Change tb_flush() env argument to cpu

In reply to Peter Crosthwaite

All of the core-code usages of this API have the cpu pointer handy so pass it in. There are only 3 architecture specific usages (2 of which are commented out) which can just use ENV_GET_CPU() locally to get the cpu pointer. The reduces core code usage of the CPU env, which brings us closer to common-obj'ing these core files.

Cc Riku Voipio <[email protected]>

Cc Eduardo Habkost <[email protected]>

Cc Paolo Bonzini <[email protected]>

Reviewed-by Eduardo Habkost <[email protected]>

Acked-by Eduardo Habkost <[email protected]>

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

---
changed since v1:
remove target-code uses of ENV_GET_CPU
Add () to fns and macros in commit message

---
 dtc                       | 2 +-
 exec.c                    | 3 +--
 gdbstub.c                 | 6 ++----
 include/exec/exec-all.h   | 2 +-
 linux-user/signal.c       | 2 +-
 target-alpha/sys_helper.c | 2 +-
 target-i386/translate.c   | 2 +-
 translate-all.c           | 6 ++----
 8 files changed, 10 insertions(+), 15 deletions(-)

dtc +1 -1

index 65cc4d2..bc895d6 160000
@@ -1 +1 @@
-Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
+Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde

exec.c +1 -2

index 9a9f734..24b2f87 100644
@@ -802,8 +802,7 @@ void cpu_single_step(CPUState *cpu, int enabled)
         } else {
             /* must flush all the translated code to avoid inconsistencies */
             /* XXX: only flush what is necessary */
-            CPUArchState *env = cpu->env_ptr;
-            tb_flush(env);
+            tb_flush(cpu);
         }
     }
 }

gdbstub.c +2 -4

index cea2a84..0fa8dd8 100644
@@ -1226,7 +1226,6 @@ void gdb_set_stop_cpu(CPUState *cpu)
 static void gdb_vm_state_change(void *opaque, int running, RunState state)
 {
     GDBState *s = gdbserver_state;
-    CPUArchState *env = s->c_cpu->env_ptr;
     CPUState *cpu = s->c_cpu;
     char buf[256];
     const char *type;

@@ -1261,7 +1260,7 @@ static void gdb_vm_state_change(void *opaque, int running, RunState state)
             cpu->watchpoint_hit = NULL;
             goto send_packet;
         }
-        tb_flush(env);
+        tb_flush(cpu);
         ret = GDB_SIGNAL_TRAP;
         break;
     case RUN_STATE_PAUSED:

@@ -1490,7 +1489,6 @@ gdb_queuesig (void)
 int
 gdb_handlesig(CPUState *cpu, int sig)
 {
-    CPUArchState *env = cpu->env_ptr;
     GDBState *s;
     char buf[256];
     int n;

@@ -1502,7 +1500,7 @@ gdb_handlesig(CPUState *cpu, int sig)
 
     /* disable single step if it was enabled */
     cpu_single_step(cpu, 0);
-    tb_flush(env);
+    tb_flush(cpu);
 
     if (sig != 0) {
         snprintf(buf, sizeof(buf), "S%02x", target_signal_to_gdb(sig));

include/exec/exec-all.h +1 -1

index a68c2e3..cffd21e 100644
@@ -216,7 +216,7 @@ static inline unsigned int tb_phys_hash_func(tb_page_addr_t pc)
 }
 
 void tb_free(TranslationBlock *tb);
-void tb_flush(CPUArchState *env);
+void tb_flush(CPUState *cpu);
 void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr);
 
 #if defined(USE_DIRECT_JUMP)

linux-user/signal.c +1 -1

index 1166f2f..9d4cef4 100644
@@ -2348,7 +2348,7 @@ static void setup_frame(int sig, struct target_sigaction *ka,
 
 		/* Flush instruction space. */
 		//flush_sig_insns(current->mm, (unsigned long) &(sf->insns[0]));
-                //		tb_flush(env);
+                //		tb_flush(CPU(sparc_env_get_cpu(env)));
 	}
         unlock_user(sf, sf_addr, sizeof(struct target_signal_frame));
 	return;

target-alpha/sys_helper.c +1 -1

index ae2e174..1c59e10 100644
@@ -74,7 +74,7 @@ void helper_tbis(CPUAlphaState *env, uint64_t p)
 
 void helper_tb_flush(CPUAlphaState *env)
 {
-    tb_flush(env);
+    tb_flush(CPU(alpha_env_get_cpu(env)));
 }
 
 void helper_halt(uint64_t restart)

target-i386/translate.c +1 -1

index 7a1bdee..82e2245 100644
@@ -6925,7 +6925,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
         gen_debug(s, pc_start - s->cs_base);
 #else
         /* start debug */
-        tb_flush(env);
+        tb_flush(CPU(x86_env_get_cpu(env)));
         qemu_set_log(CPU_LOG_INT | CPU_LOG_TB_IN_ASM);
 #endif
         break;

translate-all.c +2 -4

index b6b0e1c..61416b1 100644
@@ -769,10 +769,8 @@ static void page_flush_tb(void)
 
 /* flush all the translation blocks */
 /* XXX: tb_flush is currently not thread safe */
-void tb_flush(CPUArchState *env1)
+void tb_flush(CPUState *cpu)
 {
-    CPUState *cpu = ENV_GET_CPU(env1);
-
 #if defined(DEBUG_FLUSH)
     printf("qemu: flush code_size=%ld nb_tbs=%d avg_tb_size=%ld\n",
            (unsigned long)(tcg_ctx.code_gen_ptr - tcg_ctx.code_gen_buffer),

@@ -1011,7 +1009,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
     tb = tb_alloc(pc);
     if (!tb) {
         /* flush must be done */
-        tb_flush(env);
+        tb_flush(cpu);
         /* cannot fail at this point */
         tb = tb_alloc(pc);
         /* Don't forget to invalidate previous TB info.  */

Signature · 1 line

1.9.1

Peter Crosthwaite

author

2015-06-24 02:31 UTC #12 lore raw

PATCH v4 5/7 gdbserver: _fork: Change fn to accept cpu instead of env

In reply to Peter Crosthwaite

All callsites to this function navigate the cpu->env_ptr only for the function to take the env ptr back to the original cpu ptr. Change the function to just pass in the CPU pointer instead. Removes a core code usage of ENV_GET_CPU (in gdbstub.c).

Cc Riku Voipio <[email protected]>

Reviewed-by Andreas Färber <[email protected]>

Reviewed-by Eduardo Habkost <[email protected]>

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

---
 bsd-user/main.c        | 2 +-
 gdbstub.c              | 3 +--
 include/exec/gdbstub.h | 2 +-
 linux-user/main.c      | 2 +-
 4 files changed, 4 insertions(+), 5 deletions(-)

bsd-user/main.c +1 -1

index ba0b998..45a1436 100644
@@ -92,7 +92,7 @@ void fork_start(void)
 void fork_end(int child)
 {
     if (child) {
-        gdbserver_fork((CPUArchState *)thread_cpu->env_ptr);
+        gdbserver_fork(thread_cpu);
     }
 }

gdbstub.c +1 -2

index 0fa8dd8..aa5ba51 100644
@@ -1629,9 +1629,8 @@ int gdbserver_start(int port)
 }
 
 /* Disable gdb stub for child processes.  */
-void gdbserver_fork(CPUArchState *env)
+void gdbserver_fork(CPUState *cpu)
 {
-    CPUState *cpu = ENV_GET_CPU(env);
     GDBState *s = gdbserver_state;
 
     if (gdbserver_fd < 0 || s->fd < 0) {

include/exec/gdbstub.h +1 -1

index a608a26..05f57c2 100644
@@ -22,7 +22,7 @@ void gdb_exit(CPUArchState *, int);
 int gdb_queuesig (void);
 int gdb_handlesig(CPUState *, int);
 void gdb_signalled(CPUArchState *, int);
-void gdbserver_fork(CPUArchState *);
+void gdbserver_fork(CPUState *);
 #endif
 /* Get or set a register.  Returns the size of the register.  */
 typedef int (*gdb_reg_cb)(CPUArchState *env, uint8_t *buf, int reg);

linux-user/main.c +1 -1

index c855bcc..12b18d0 100644
@@ -130,7 +130,7 @@ void fork_end(int child)
         pthread_cond_init(&exclusive_cond, NULL);
         pthread_cond_init(&exclusive_resume, NULL);
         pthread_mutex_init(&tcg_ctx.tb_ctx.tb_lock, NULL);
-        gdbserver_fork((CPUArchState *)thread_cpu->env_ptr);
+        gdbserver_fork(thread_cpu);
     } else {
         pthread_mutex_unlock(&exclusive_lock);
         pthread_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock);

Signature · 1 line

1.9.1

Peter Crosthwaite

author

2015-06-24 02:31 UTC #13 lore raw

PATCH v4 6/7 cpus: Change tcg_cpu_exec() arg to cpu, not env

In reply to Peter Crosthwaite

The sole caller of this function navigates the cpu->env_ptr only for this function to take it back the cpu pointer straight away. Pass in cpu pointer instead and grab the env pointer locally in the function. Removes a core code usage of ENV_GET_CPU().

Reviewed-by Andreas Färber <[email protected]>

Reviewed-by Eduardo Habkost <[email protected]>

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

---
Changed since v3:
Add () to tcg_cpu_exec in commit subject
Changed since v1:
Add () to ENV_GET_CPU() in commit message

---
 cpus.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

cpus.c +3 -4

index 4f0e54d..abf6f06 100644
@@ -1344,9 +1344,9 @@ int vm_stop_force_state(RunState state)
     }
 }
 
-static int tcg_cpu_exec(CPUArchState *env)
+static int tcg_cpu_exec(CPUState *cpu)
 {
-    CPUState *cpu = ENV_GET_CPU(env);
+    CPUArchState *env = cpu->env_ptr;
     int ret;
 #ifdef CONFIG_PROFILER
     int64_t ti;

@@ -1408,13 +1408,12 @@ static void tcg_exec_all(void)
     }
     for (; next_cpu != NULL && !exit_request; next_cpu = CPU_NEXT(next_cpu)) {
         CPUState *cpu = next_cpu;
-        CPUArchState *env = cpu->env_ptr;
 
         qemu_clock_enable(QEMU_CLOCK_VIRTUAL,
                           (cpu->singlestep_enabled & SSTEP_NOTIMER) == 0);
 
         if (cpu_can_run(cpu)) {
-            r = tcg_cpu_exec(env);
+            r = tcg_cpu_exec(cpu);
             if (r == EXCP_DEBUG) {
                 cpu_handle_guest_debug(cpu);
                 break;

Signature · 1 line

1.9.1

Peter Crosthwaite

author

2015-06-24 02:31 UTC #14 lore raw

PATCH v4 7/7 cpus: Change exec_init() arg to cpu, not env

In reply to Peter Crosthwaite

The callers (most of them in target-foo/cpu.c) to this function all have the cpu pointer handy. Just pass it to avoid an ENV_GET_CPU() from core code (in exec.c).

Cc Paolo Bonzini <[email protected]>

Cc Richard Henderson <[email protected]>

Cc Peter Maydell <[email protected]>

Cc "Edgar E. Iglesias" <[email protected]>

Cc "Andreas Färber" <[email protected]>

Cc Eduardo Habkost <[email protected]>

Cc Michael Walle <[email protected]>

Cc Aurelien Jarno <[email protected]>

Cc Leon Alrae <[email protected]>

Cc Anthony Green <[email protected]>

Cc Jia Liu <[email protected]>

Cc Alexander Graf <[email protected]>

Cc Blue Swirl <[email protected]>

Cc Mark Cave-Ayland <[email protected]>

Cc Bastian Koppelmann <[email protected]>

Cc Guan Xuetao <[email protected]>

Cc Max Filippov <[email protected]>

Reviewed-by Andreas Färber <[email protected]>

Reviewed-by Aurelien Jarno <[email protected]>

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

---
changed since v3:
inline cpu->env_ptr to get rid of C99 declaration (eduardo review). changed since v1:
Add () to function in commit message.
Rebase on Bharata's Error ** patch

---
 exec.c                      | 5 ++---
 include/exec/exec-all.h     | 2 +-
 target-alpha/cpu.c          | 2 +-
 target-arm/cpu.c            | 2 +-
 target-cris/cpu.c           | 2 +-
 target-i386/cpu.c           | 2 +-
 target-lm32/cpu.c           | 2 +-
 target-m68k/cpu.c           | 2 +-
 target-microblaze/cpu.c     | 2 +-
 target-mips/cpu.c           | 2 +-
 target-moxie/cpu.c          | 2 +-
 target-openrisc/cpu.c       | 2 +-
 target-ppc/translate_init.c | 2 +-
 target-s390x/cpu.c          | 2 +-
 target-sh4/cpu.c            | 2 +-
 target-sparc/cpu.c          | 2 +-
 target-tricore/cpu.c        | 2 +-
 target-unicore32/cpu.c      | 2 +-
 target-xtensa/cpu.c         | 2 +-
 19 files changed, 20 insertions(+), 21 deletions(-)

exec.c +2 -3

index 24b2f87..e7f3f02 100644
@@ -558,9 +558,8 @@ void cpu_exec_exit(CPUState *cpu)
 }
 #endif
 
-void cpu_exec_init(CPUArchState *env, Error **errp)
+void cpu_exec_init(CPUState *cpu, Error **errp)
 {
-    CPUState *cpu = ENV_GET_CPU(env);
     CPUClass *cc = CPU_GET_CLASS(cpu);
     int cpu_index;
     Error *local_err = NULL;

@@ -590,7 +589,7 @@ void cpu_exec_init(CPUArchState *env, Error **errp)
     }
 #if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
     register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
-                    cpu_save, cpu_load, env);
+                    cpu_save, cpu_load, cpu->env_ptr);
     assert(cc->vmsd == NULL);
     assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
 #endif

include/exec/exec-all.h +1 -1

index cffd21e..4bf13e0 100644
@@ -88,7 +88,7 @@ void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 TranslationBlock *tb_gen_code(CPUState *cpu,
                               target_ulong pc, target_ulong cs_base, int flags,
                               int cflags);
-void cpu_exec_init(CPUArchState *env, Error **errp);
+void cpu_exec_init(CPUState *env, Error **errp);
 void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
 
 #if !defined(CONFIG_USER_ONLY)

target-alpha/cpu.c +1 -1

index e865ba7..421d7e5 100644
@@ -257,7 +257,7 @@ static void alpha_cpu_initfn(Object *obj)
     CPUAlphaState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
     tlb_flush(cs, 1);
 
     alpha_translate_init();

target-arm/cpu.c +1 -1

index 1c40cc0..5a8cdb5 100644
@@ -392,7 +392,7 @@ static void arm_cpu_initfn(Object *obj)
     uint32_t Aff1, Aff0;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(&cpu->env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
     cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
                                          g_free, g_free);

target-cris/cpu.c +1 -1

index bb8e7ea..0db209b 100644
@@ -170,7 +170,7 @@ static void cris_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     env->pregs[PR_VR] = ccc->vr;

target-i386/cpu.c +1 -1

index 433f38b..a35b045 100644
@@ -3007,7 +3007,7 @@ static void x86_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     object_property_add(obj, "family", "int",
                         x86_cpuid_version_get_family,

target-lm32/cpu.c +1 -1

index da4fde1..c2b77c6 100644
@@ -151,7 +151,7 @@ static void lm32_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     env->flags = 0;

target-m68k/cpu.c +1 -1

index ae3d765..4f246da 100644
@@ -168,7 +168,7 @@ static void m68k_cpu_initfn(Object *obj)
     static bool inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     if (tcg_enabled() && !inited) {
         inited = true;

target-microblaze/cpu.c +1 -1

index 967ea01..128d032 100644
@@ -190,7 +190,7 @@ static void mb_cpu_initfn(Object *obj)
     static bool tcg_initialized;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     set_float_rounding_mode(float_round_nearest_even, &env->fp_status);

target-mips/cpu.c +1 -1

index 1fd9f22..4027d0f 100644
@@ -115,7 +115,7 @@ static void mips_cpu_initfn(Object *obj)
     CPUMIPSState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     if (tcg_enabled()) {
         mips_tcg_init();

target-moxie/cpu.c +1 -1

index 415c65a..6b035aa 100644
@@ -66,7 +66,7 @@ static void moxie_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(&cpu->env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     if (tcg_enabled() && !inited) {
         inited = 1;

target-openrisc/cpu.c +1 -1

index cd6c657..d97f3c0 100644
@@ -92,7 +92,7 @@ static void openrisc_cpu_initfn(Object *obj)
     static int inited;
 
     cs->env_ptr = &cpu->env;
-    cpu_exec_init(&cpu->env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
 #ifndef CONFIG_USER_ONLY
     cpu_openrisc_mmu_init(cpu);

target-ppc/translate_init.c +1 -1

index 2b72f2d..b95ac68 100644
@@ -8928,7 +8928,7 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp)
         return;
     }
 
-    cpu_exec_init(&cpu->env, &local_err);
+    cpu_exec_init(cs, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
         return;

target-s390x/cpu.c +1 -1

index ced5592..c3e21b4 100644
@@ -212,7 +212,7 @@ static void s390_cpu_initfn(Object *obj)
 #endif
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 #if !defined(CONFIG_USER_ONLY)
     qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
     qemu_get_timedate(&tm, 0);

target-sh4/cpu.c +1 -1

index 2c2060b..5c65ab4 100644
@@ -248,7 +248,7 @@ static void superh_cpu_initfn(Object *obj)
     CPUSH4State *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     env->movcal_backup_tail = &(env->movcal_backup);

target-sparc/cpu.c +1 -1

index 4d1da7c..9528e3a 100644
@@ -802,7 +802,7 @@ static void sparc_cpu_initfn(Object *obj)
     CPUSPARCState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     if (tcg_enabled()) {
         gen_intermediate_code_init(env);

target-tricore/cpu.c +1 -1

index 6fa60b1..2029ef6 100644
@@ -92,7 +92,7 @@ static void tricore_cpu_initfn(Object *obj)
     CPUTriCoreState *env = &cpu->env;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     if (tcg_enabled()) {
         tricore_tcg_init();

target-unicore32/cpu.c +1 -1

index eda039c..fc451a1 100644
@@ -111,7 +111,7 @@ static void uc32_cpu_initfn(Object *obj)
     static bool inited;
 
     cs->env_ptr = env;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
 #ifdef CONFIG_USER_ONLY
     env->uncached_asr = ASR_MODE_USER;

target-xtensa/cpu.c +1 -1

index 8d69d23..da8129d 100644
@@ -114,7 +114,7 @@ static void xtensa_cpu_initfn(Object *obj)
 
     cs->env_ptr = env;
     env->config = xcc->config;
-    cpu_exec_init(env, &error_abort);
+    cpu_exec_init(cs, &error_abort);
 
     if (tcg_enabled() && !tcg_inited) {
         tcg_inited = true;

Signature · 1 line

1.9.1

Andreas Färber

Applied

2015-06-24 13:37 UTC #2 lore raw

Re: [Qemu-devel] [PATCH qom v4 1/7] cpus: Add Error argument to cpu_exec_init()

In reply to Peter Crosthwaite

s/cpus/cpu/

Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:

Quoted 10 lines

From: Bharata B Rao <[email protected]>
Add an Error argument to cpu_exec_init() to let users collect the error. This is in preparation to change the CPU enumeration logic in cpu_exec_init(). With the new enumeration logic, cpu_exec_init() can fail if cpu_index values corresponding to max_cpus have already been handed out.
Since all current callers of cpu_exec_init() are from instance_init, use error_abort Error arugment to abort in case of an error.

argument

Quoted 21 lines

Signed-off-by Bharata B Rao <[email protected]>
Reviewed-by Eduardo Habkost <[email protected]>
Reviewed-by Igor Mammedov <[email protected]>
Reviewed-by David Gibson <[email protected]>
Reviewed-by Peter Crosthwaite <[email protected]>
Acked-by Paolo Bonzini <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>
---
 exec.c                      | 2 +-
 include/exec/exec-all.h     | 2 +-
 target-alpha/cpu.c          | 2 +-
 target-arm/cpu.c            | 2 +-
 target-cris/cpu.c           | 2 +-
 target-i386/cpu.c           | 2 +-
 target-lm32/cpu.c           | 2 +-
 target-m68k/cpu.c           | 2 +-
 target-microblaze/cpu.c     | 2 +-
 target-mips/cpu.c           | 2 +-
 target-moxie/cpu.c          | 2 +-
 target-openrisc/cpu.c       | 2 +-

Quoted 8 lines

 target-ppc/translate_init.c | 2 +-
 target-s390x/cpu.c          | 2 +-
 target-sh4/cpu.c            | 2 +-
 target-sparc/cpu.c          | 2 +-
 target-tricore/cpu.c        | 2 +-
 target-unicore32/cpu.c      | 2 +-
 target-xtensa/cpu.c         | 2 +-
 19 files changed, 19 insertions(+), 19 deletions(-)

Applied to qom-cpu with the above changes.

Thanks,
Andreas

Signature · 3 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Andreas Färber

2015-06-24 13:53 UTC #4 lore raw

Re: [Qemu-devel] [PATCH qom v4 2/7] cpus: Convert cpu_index into a bitmap

In reply to Peter Crosthwaite

Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:

Quoted 33 lines

From: Bharata B Rao <[email protected]>
Currently CPUState.cpu_index is monotonically increasing and a newly created CPU always gets the next higher index. The next available index is calculated by counting the existing number of CPUs. This is fine as long as we only add CPUs, but there are architectures which are starting to support CPU removal too. For an architecture like PowerPC which derives its CPU identifier (device tree ID) from cpu_index, the existing logic of generating cpu_index values causes problems.
With the currently proposed method of handling vCPU removal by parking the vCPU fd in QEMU
(Ref: http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg02604.html),
generating cpu_index this way will not work for PowerPC.
This patch changes the way cpu_index is handed out by maintaining a bit map of the CPUs that tracks both addition and removal of CPUs.
The CPU bitmap allocation logic is part of cpu_exec_init() which is called by instance_init routines of various CPU targets. Newly added cpu_exec_exit() API handles the deallocation part and this routine is called from generic CPU::instance_finalize().
Note: This new CPU enumeration is for !CONFIG_USER_ONLY only. CONFIG_USER_ONLY continues to have the old enumeration logic.
Signed-off-by Bharata B Rao <[email protected]>
Reviewed-by Eduardo Habkost <[email protected]>
Reviewed-by Igor Mammedov <[email protected]>
Reviewed-by David Gibson <[email protected]>
Reviewed-by Peter Crosthwaite <[email protected]>
Acked-by Paolo Bonzini <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>

Quoted 5 lines

---
 exec.c            | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 include/qom/cpu.h |  1 +
 qom/cpu.c         |  7 +++++++
 3 files changed, 58 insertions(+), 5 deletions(-)

Doesn't apply on my tree, trying manually...

Andreas

Signature · 3 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Andreas Färber

2015-06-24 15:31 UTC #8 lore raw

Re: [Qemu-devel] [PATCH qom v4 4/7] translate-all: Change tb_flush() env argument to cpu

In reply to Peter Crosthwaite

Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:

dtc line 1

Quoted 7 lines

dtc +1 -1

index 65cc4d2..bc895d6 160000
@@ -1 +1 @@
-Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
+Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde

This looks accidental.

Andreas

Signature · 3 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Andreas Färber

Applied

2015-06-24 15:39 UTC #5 lore raw

Re: [Qemu-devel] [PATCH qom v4 2/7] cpus: Convert cpu_index into a bitmap

In reply to Andreas Färber

s/cpus/cpu/

Am 24.06.2015 um 15:53 schrieb Andreas Färber:

Quoted 4 lines

Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:
Quoted 3 lines
From: Bharata B Rao <[email protected]>
Currently CPUState.cpu_index is monotonically increasing and a newly

CPUState::cpu_index

Quoted 12 lines

Quoted 12 lines
created CPU always gets the next higher index. The next available index is calculated by counting the existing number of CPUs. This is fine as long as we only add CPUs, but there are architectures which are starting to support CPU removal too. For an architecture like PowerPC which derives its CPU identifier (device tree ID) from cpu_index, the existing logic of generating cpu_index values causes problems.
With the currently proposed method of handling vCPU removal by parking the vCPU fd in QEMU
(Ref: http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg02604.html),
generating cpu_index this way will not work for PowerPC.

Quoted 7 lines

Quoted 7 lines
This patch changes the way cpu_index is handed out by maintaining a bit map of the CPUs that tracks both addition and removal of CPUs.
The CPU bitmap allocation logic is part of cpu_exec_init() which is called by instance_init routines of various CPU targets. Newly added cpu_exec_exit() API handles the deallocation part and this routine is called from generic CPU::instance_finalize().

(Here it's in TypeInfo:: actually.)

Quoted 17 lines

Quoted 11 lines
Note: This new CPU enumeration is for !CONFIG_USER_ONLY only. CONFIG_USER_ONLY continues to have the old enumeration logic.
Signed-off-by Bharata B Rao <[email protected]>
Reviewed-by Eduardo Habkost <[email protected]>
Reviewed-by Igor Mammedov <[email protected]>
Reviewed-by David Gibson <[email protected]>
Reviewed-by Peter Crosthwaite <[email protected]>
Acked-by Paolo Bonzini <[email protected]>
Signed-off-by Peter Crosthwaite <[email protected]>
Quoted 5 lines
---
 exec.c            | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
 include/qom/cpu.h |  1 +
 qom/cpu.c         |  7 +++++++
 3 files changed, 58 insertions(+), 5 deletions(-)

Quoted 1 line

Doesn't apply on my tree, trying manually...

Caused by exec.c -> qom/cpu.c code movements from Eduardo in my queue.

Applied to qom-cpu.

Thanks,
Andreas

Signature · 3 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Peter Crosthwaite

2015-06-24 17:06 UTC #9 lore raw

Re: [Qemu-devel] [PATCH qom v4 4/7] translate-all: Change tb_flush() env argument to cpu

In reply to Andreas Färber

Quoted 11 lines · Andreas Färber

Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:
dtc line 1
Quoted 7 lines
dtc +1 -1
index 65cc4d2..bc895d6 160000
@@ -1 +1 @@
-Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
+Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde
This looks accidental.

Doh :( It is. Do you need a respin? My brain has gotten too used to filtering out submodule changes when inspecting diffs.

Regards,
Peter

Quoted 7 lines

Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)

Andreas Färber

2015-06-24 17:14 UTC #16 lore raw

In reply to Peter Crosthwaite

Hi,

Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:

Quoted 12 lines

Hi All,
I'm moving towards the goal of having no core code usages of ENV_GET_CPU. This has two advantages:
1: It means we are closer to common-obj'ing core code like exec.c, cpus.c and friends.
2: Multi arch is easier if ENV_GET_CPU() stays arch specific. It means I don't need those patches where I reorder the env within the arch specific CPUState. This allows continuing placement of arch specifics before the env in the CPU container (which has TCG perf advantages).

Quoted 2 lines

Due to point 1, I'm sending this ahead as I think it has standalone value, rather than send as part of multi-arch.

Thanks for rebasing and combining this so handily, rebased onto qom-cpu with the dtc change stripped and a few massages to the messages:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

I considered modifying the ppc patch to have a local CPUState variable for the future, but left it as is for now.

Most changes seemed mechanical. Alex gave a go-ahead; a few CCs haven't replied yet, please speak up within the next few days if we should wait (in particular Paolo and Riku).

I still need to test on BSD - if someone has a Tested-by there that would be helpful.

Regards,
Andreas

Quoted 44 lines

Bharata B Rao (3):
cpus: Add Error argument to cpu_exec_init()
cpus: Convert cpu_index into a bitmap
ppc: Move cpu_exec_init() call to realize function

 bsd-user/main.c             |  2 +-
 cpus.c                      |  7 +++--
 dtc                         |  2 +-
 exec.c                      | 63 ++++++++++++++++++++++++++++++++++++++-------
 gdbstub.c                   |  9 +++----
 include/exec/exec-all.h     |  4 +--
 include/exec/gdbstub.h      |  2 +-
 include/qom/cpu.h           |  1 +
 linux-user/main.c           |  2 +-
 linux-user/signal.c         |  2 +-
 qom/cpu.c                   |  7 +++++
 target-alpha/cpu.c          |  2 +-
 target-alpha/sys_helper.c   |  2 +-
 target-arm/cpu.c            |  2 +-
 target-cris/cpu.c           |  2 +-
 target-i386/cpu.c           |  2 +-
 target-i386/translate.c     |  2 +-
 target-lm32/cpu.c           |  2 +-
 target-m68k/cpu.c           |  2 +-
 target-microblaze/cpu.c     |  2 +-
 target-mips/cpu.c           |  2 +-
 target-moxie/cpu.c          |  2 +-
 target-openrisc/cpu.c       |  2 +-
 target-ppc/translate_init.c |  9 +++++--
 target-s390x/cpu.c          |  2 +-
 target-sh4/cpu.c            |  2 +-
 target-sparc/cpu.c          |  2 +-
 target-tricore/cpu.c        |  2 +-
 target-unicore32/cpu.c      |  2 +-
 target-xtensa/cpu.c         |  2 +-
 translate-all.c             |  6 ++---
 31 files changed, 101 insertions(+), 51 deletions(-)

Signature · 3 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Paolo Bonzini

2015-06-24 17:16 UTC #17 lore raw

Quoted 3 lines · Andreas Färber

Most changes seemed mechanical. Alex gave a go-ahead; a few CCs haven't replied yet, please speak up within the next few days if we should wait (in particular Paolo and Riku).

Everything fine by me.

Paolo

Andreas Färber

2015-06-24 17:23 UTC #10 lore raw

Re: [Qemu-devel] [PATCH qom v4 4/7] translate-all: Change tb_flush() env argument to cpu

In reply to Peter Crosthwaite

Am 24.06.2015 um 19:06 schrieb Peter Crosthwaite:

Quoted 13 lines · Andreas Färber

Quoted 11 lines
Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:
dtc line 1
Quoted 7 lines
dtc +1 -1
index 65cc4d2..bc895d6 160000
@@ -1 +1 @@
-Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
+Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde
This looks accidental.

Quoted 1 line

Doh :( It is. Do you need a respin?

No, thanks. Each respin requires me to re-review, so I just deleted the patch lines from the email before applying.

Regards,
Andreas

Signature · 3 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Peter Crosthwaite

2015-06-24 17:29 UTC #11 lore raw

Re: [Qemu-devel] [PATCH qom v4 4/7] translate-all: Change tb_flush() env argument to cpu

Quoted 14 lines · Andreas Färber

Am 24.06.2015 um 19:06 schrieb Peter Crosthwaite:
Quoted 13 lines · Andreas Färber
Quoted 11 lines
Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:
dtc line 1
Quoted 7 lines
dtc +1 -1
index 65cc4d2..bc895d6 160000
@@ -1 +1 @@
-Subproject commit 65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf
+Subproject commit bc895d6d09695d05ceb8b52486ffe861d6cfbdde
This looks accidental.

Quoted 5 lines

Quoted 1 line
Doh :( It is. Do you need a respin?
No, thanks. Each respin requires me to re-review, so I just deleted the patch lines from the email before applying.

Thanks. Appreciated.

Regards,
Peter

Quoted 8 lines

Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)

Andreas Färber

2015-07-02 15:55 UTC #15 lore raw

Re: [Qemu-devel] [PATCH qom v4 7/7] cpus: Change exec_init() arg to cpu, not env

In reply to Peter Crosthwaite

Am 24.06.2015 um 04:31 schrieb Peter Crosthwaite:

include/exec/exec-all.h void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr); · line 94

Quoted 13 lines

include/exec/exec-all.h +1 -1

index cffd21e..4bf13e0 100644
@@ -88,7 +88,7 @@ void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 TranslationBlock *tb_gen_code(CPUState *cpu,
                               target_ulong pc, target_ulong cs_base, int flags,
                               int cflags);
-void cpu_exec_init(CPUArchState *env, Error **errp);
+void cpu_exec_init(CPUState *env, Error **errp);
 void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
 
 #if !defined(CONFIG_USER_ONLY)

Nit: Fixing s/env/cpu/

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

index 4bf13e0..a9c1f88 100644
@@ -88,7 +88,7 @@ void QEMU_NORETURN cpu_io_recompile(CPUState *cpu,

uintptr_t retaddr);
TranslationBlock *tb_gen_code(CPUState *cpu,
target_ulong pc, target_ulong cs_base,
int flags,

                               int cflags);

-void cpu_exec_init(CPUState *env, Error **errp);
+void cpu_exec_init(CPUState *cpu, Error **errp);
void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);

#if !defined(CONFIG_USER_ONLY)

Regards,
Andreas

Signature · 3 lines

SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)