On Wed, 5 Feb 2025 at 22:02, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
> +Igor > > On 9/12/24 21:36, p...@philjordan.eu wrote: > > From: Phil Dennis-Jordan <p...@philjordan.eu> > > > > Some VM state required for fully configuring vCPUs is only available > > after all devices have been through their init phase. This extra > > function, called just before each vCPU makes its first VM entry, > > allows us to perform such architecture-specific initialisation. > > > > Signed-off-by: Phil Dennis-Jordan <p...@philjordan.eu> > > --- > > accel/hvf/hvf-accel-ops.c | 5 +++++ > > include/sysemu/hvf_int.h | 1 + > > target/arm/hvf/hvf.c | 4 ++++ > > target/i386/hvf/hvf.c | 4 ++++ > > 4 files changed, 14 insertions(+) > > > > diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c > > index d60874d3e6..c17a9a10de 100644 > > --- a/accel/hvf/hvf-accel-ops.c > > +++ b/accel/hvf/hvf-accel-ops.c > > @@ -442,6 +442,11 @@ static void *hvf_cpu_thread_fn(void *arg) > > cpu_thread_signal_created(cpu); > > qemu_guest_random_seed_thread_part2(cpu->random_seed); > > > > + if (!cpu_can_run(cpu)) { > > + qemu_wait_io_event(cpu); > > + } > > + hvf_vcpu_before_first_run(cpu); > > Could this be fixed by the cpu_list_add() split? > https://lore.kernel.org/qemu-devel/20250128142152.9889-1-phi...@linaro.org/ > > You mean by implementing a wire() handler for HVF CPU classes? Possibly - I'll need to apply those patches locally and trace in what context those wire methods would run. HVF wants most vCPU-specific functions to be run on the thread owning the vCPU, so if wire() runs on the main QEMU event handling thread (or anything other than the vCPU's own thread), it won't work for patches 2 & 7 from this series which actually do stuff in these before_first_run() handlers. I notice that Igor's v2 of the cpu_list_add patch set no longer includes the wire()/unwire() handlers… https://patchew.org/QEMU/20250207162048.1890669-1-imamm...@redhat.com/ Another option might be to use async_run_on_cpu() for such early on-vCPU-thread initialisation, but I figured that option would perhaps be a little to indirect to readers of the code and difficult to reason about. > do { > > if (cpu_can_run(cpu)) { > > r = hvf_vcpu_exec(cpu); >