| Message ID | 20220621204643.371397-4-richard.henderson@linaro.org |
|---|---|
| State | Accepted |
| Commit | a82fd5a4ec24d923ff1e6da128c0fd4a74079d99 |
| Headers | show |
| Series | [PULL,1/9] tcg/ppc: implement rem[u]_i{32,64} with mod[su][wd] | expand |
On Tue, 21 Jun 2022 at 21:53, Richard Henderson <richard.henderson@linaro.org> wrote: > > From: Philippe Mathieu-Daudé <f4bug@amsat.org> > > Move TCG cflags initialization to thread handler. > Remove the duplicated assert checks. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Message-Id: <20220323171751.78612-6-philippe.mathieu.daude@gmail.com> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Hi; I've just noticed that this commit seems to break icount mode when there's more than one vCPU. Specifically: > diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c > index 1a72149f0e..cc8adc2380 100644 > --- a/accel/tcg/tcg-accel-ops-rr.c > +++ b/accel/tcg/tcg-accel-ops-rr.c > @@ -152,7 +152,9 @@ static void *rr_cpu_thread_fn(void *arg) > Notifier force_rcu; > CPUState *cpu = arg; > > - assert(tcg_enabled()); > + g_assert(tcg_enabled()); > + tcg_cpu_init_cflags(cpu, false); > + In icount mode we round-robin on the same CPU thread, so the rr_cpu_thread_fn() gets called only once, and we set the TCG cflags on the first CPU here, but not on any others. > rcu_register_thread(); > force_rcu.notify = rr_force_rcu; > rcu_add_force_rcu_notifier(&force_rcu); > @@ -275,9 +277,6 @@ void rr_start_vcpu_thread(CPUState *cpu) > static QemuCond *single_tcg_halt_cond; > static QemuThread *single_tcg_cpu_thread; > > - g_assert(tcg_enabled()); > - tcg_cpu_init_cflags(cpu, false); > - This code gets called for each vCPU, whether we are going to give it its own thread or not, so when we did this check in the old location we would call tcg_cpu_init_cflags() on every vCPU. > if (!single_tcg_cpu_thread) { > cpu->thread = g_new0(QemuThread, 1); > cpu->halt_cond = g_new0(QemuCond, 1); So now only one vCPU gets the CF_USE_ICOUNT thread set, and the guest kernel hangs shortly after it brings up the secondary CPU. Reverting commit a82fd5a4ec24d923ff1e fixes the problem. thanks -- PMM
diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c index d50239e0e2..ba997f6cfe 100644 --- a/accel/tcg/tcg-accel-ops-mttcg.c +++ b/accel/tcg/tcg-accel-ops-mttcg.c @@ -70,6 +70,8 @@ static void *mttcg_cpu_thread_fn(void *arg) assert(tcg_enabled()); g_assert(!icount_enabled()); + tcg_cpu_init_cflags(cpu, current_machine->smp.max_cpus > 1); + rcu_register_thread(); force_rcu.notifier.notify = mttcg_force_rcu; force_rcu.cpu = cpu; @@ -139,9 +141,6 @@ void mttcg_start_vcpu_thread(CPUState *cpu) { char thread_name[VCPU_THREAD_NAME_SIZE]; - g_assert(tcg_enabled()); - tcg_cpu_init_cflags(cpu, current_machine->smp.max_cpus > 1); - cpu->thread = g_new0(QemuThread, 1); cpu->halt_cond = g_malloc0(sizeof(QemuCond)); qemu_cond_init(cpu->halt_cond); diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c index 1a72149f0e..cc8adc2380 100644 --- a/accel/tcg/tcg-accel-ops-rr.c +++ b/accel/tcg/tcg-accel-ops-rr.c @@ -152,7 +152,9 @@ static void *rr_cpu_thread_fn(void *arg) Notifier force_rcu; CPUState *cpu = arg; - assert(tcg_enabled()); + g_assert(tcg_enabled()); + tcg_cpu_init_cflags(cpu, false); + rcu_register_thread(); force_rcu.notify = rr_force_rcu; rcu_add_force_rcu_notifier(&force_rcu); @@ -275,9 +277,6 @@ void rr_start_vcpu_thread(CPUState *cpu) static QemuCond *single_tcg_halt_cond; static QemuThread *single_tcg_cpu_thread; - g_assert(tcg_enabled()); - tcg_cpu_init_cflags(cpu, false); - if (!single_tcg_cpu_thread) { cpu->thread = g_new0(QemuThread, 1); cpu->halt_cond = g_new0(QemuCond, 1);