Re: [RFC PATCH] softmmu: Fix async_run_on_cpu() use in tcg_commit_cpu()

[Richard Henderson]

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.

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


r~