On 7/9/23 18:28, Richard Henderson wrote:
On 9/7/23 09:14, Philippe Mathieu-Daudé wrote:
CPUState::halt_cond is an accelerator specific pointer, used
in particular by TCG (which tcg_commit() is about).
The pointer is set by the AccelOpsClass::create_vcpu_thread()
handler.
AccelOpsClass::create_vcpu_thread() is called by the generic
qemu_init_vcpu(), which expect the accelerator handler to
eventually call cpu_thread_signal_created() which is protected
with a QemuCond. It is safer to check the vCPU is created with
this field rather than the 'halt_cond' pointer set in
create_vcpu_thread() before the vCPU thread is initialized.

This avoids calling tcg_commit() until all CPUs are realized.

Here we can see for a machine with N CPUs, tcg_commit()
is called N times before the 'machine_creation_done' event:

   (lldb) settings set -- target.run-args  "-M" "virt" "-smp" "512" "-display" "none"    (lldb) breakpoint set --name qemu_machine_creation_done --one-shot true
   (lldb) breakpoint set --name tcg_commit_cpu --auto-continue true
   (lldb) run
   Process 84089 launched: 'qemu-system-aarch64' (arm64)
   Process 84089 stopped
   * thread #1, queue = 'com.apple.main-thread', stop reason = one-shot breakpoint 2
   (lldb) breakpoint list --brief
   Current breakpoints:
   2: name = 'tcg_commit_cpu', locations = 2, resolved = 2, hit count = 512 Options: enabled auto-continue               ^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^


Of course the function is called 512 times: you asked for 512 cpus, and each has its own address space which needs initializing.

The AS are still initialized at the same time, but we defer the listener
callback until the vCPU is ready (what was expected first IIUC).

If you skip the call before cpu->created, when exactly are you going to do it?

With this patch tcg_commit_cpu() is only called by vCPU threads, in
their processing loop. i.e: comparing backtraces, now the first hit
is:
(lldb) bt
* thread #514, stop reason = breakpoint 4.2
* frame #0: 0x1005d9d48 qemu-system-aarch64`tcg_commit_cpu(cpu=0x173358000, data=...) at physmem.c:2493:63 frame #1: 0x10000d684 qemu-system-aarch64`process_queued_cpu_work(cpu=0x173358000) at cpus-common.c:360:13 frame #2: 0x100297390 qemu-system-aarch64`qemu_wait_io_event [inlined] qemu_wait_io_event_common(cpu=<unavailable>) at cpus.c:412:5 [artificial] frame #3: 0x100623b98 qemu-system-aarch64`mttcg_cpu_thread_fn(arg=0x173358000) at tcg-accel-ops-mttcg.c:123:9 frame #4: 0x10079f15c qemu-system-aarch64`qemu_thread_start(args=<unavailable>) at qemu-thread-posix.c:541:9
    frame #5: 0x18880ffa8 libsystem_pthread.dylib`_pthread_start + 148


Reply via email to