[PATCH 01/11] hvf: Add facility for initialisation code prior to first vCPU run

phil@philjordan.eu posted 11 patches 5 months ago
[PATCH 01/11] hvf: Add facility for initialisation code prior to first vCPU run
Posted by phil@philjordan.eu 5 months ago
From: Phil Dennis-Jordan <phil@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 <phil@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);
+
     do {
         if (cpu_can_run(cpu)) {
             r = hvf_vcpu_exec(cpu);
diff --git a/include/sysemu/hvf_int.h b/include/sysemu/hvf_int.h
index 42ae18433f..2775bd82d7 100644
--- a/include/sysemu/hvf_int.h
+++ b/include/sysemu/hvf_int.h
@@ -67,6 +67,7 @@ const char *hvf_return_string(hv_return_t ret);
 int hvf_arch_init(void);
 hv_return_t hvf_arch_vm_create(MachineState *ms, uint32_t pa_range);
 int hvf_arch_init_vcpu(CPUState *cpu);
+void hvf_vcpu_before_first_run(CPUState *cpu);
 void hvf_arch_vcpu_destroy(CPUState *cpu);
 int hvf_vcpu_exec(CPUState *);
 hvf_slot *hvf_find_overlap_slot(uint64_t, uint64_t);
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index ca7ea92774..0b334c268e 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1061,6 +1061,10 @@ int hvf_arch_init_vcpu(CPUState *cpu)
     return 0;
 }
 
+void hvf_vcpu_before_first_run(CPUState *cpu)
+{
+}
+
 void hvf_kick_vcpu_thread(CPUState *cpu)
 {
     cpus_kick_thread(cpu);
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index c5d025d557..3b6ee79fb2 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -338,6 +338,10 @@ int hvf_arch_init_vcpu(CPUState *cpu)
     return 0;
 }
 
+void hvf_vcpu_before_first_run(CPUState *cpu)
+{
+}
+
 static void hvf_store_events(CPUState *cpu, uint32_t ins_len, uint64_t idtvec_info)
 {
     X86CPU *x86_cpu = X86_CPU(cpu);
-- 
2.39.3 (Apple Git-146)
Re: [PATCH 01/11] hvf: Add facility for initialisation code prior to first vCPU run
Posted by Philippe Mathieu-Daudé 3 months, 1 week ago
+Igor

On 9/12/24 21:36, phil@philjordan.eu wrote:
> From: Phil Dennis-Jordan <phil@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 <phil@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-philmd@linaro.org/

>       do {
>           if (cpu_can_run(cpu)) {
>               r = hvf_vcpu_exec(cpu);
Re: [PATCH 01/11] hvf: Add facility for initialisation code prior to first vCPU run
Posted by Phil Dennis-Jordan 3 months ago
On Wed, 5 Feb 2025 at 22:02, Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> +Igor
>
> On 9/12/24 21:36, phil@philjordan.eu wrote:
> > From: Phil Dennis-Jordan <phil@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 <phil@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-philmd@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-imammedo@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);
>
Re: [PATCH 01/11] hvf: Add facility for initialisation code prior to first vCPU run
Posted by Alexander Graf 5 months ago
On 09.12.24 21:36, phil@philjordan.eu wrote:
> From: Phil Dennis-Jordan <phil@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 <phil@philjordan.eu>


Reviewed-by: Alexander Graf <agraf@csgraf.de>

Alex