On 9/12/24 21:36, phil@philjordan.eu wrote:
> From: Phil Dennis-Jordan <phil@philjordan.eu>
>
> This seems to be entirely superfluous and is costly enough to show up in
So the pthread_kill(cpu->thread, SIG_IPI) is entirely superfluous?
Yes, that is my conclusion after looking at the code and testing without the signal sending. (I am only talking about HVF/x86 here!)
By my understanding, the HVF vCPU thread is always in one of 3 states:
- running hv_vcpu_run_until (hv_vcpu_interrupt() forces the transition out of this, even in edge cases)
- waiting in qemu_wait_io_event In the main hvf_cpu_thread_fn loop (signalling the condition variable will bring it out of this)
- actively processing a VM exit, I/O request, etc (whatever it’s doing can’t be interrupted, but it will make progress and check its job queue on completion)
So I can’t see any purpose the signal is supposed to be serving. I mean, maybe I’ve missed something important - always happy to be corrected and learn something new. :-)
I’ll still need to investigate if the arm64 version is also doing this unnecessarily and whether we can remove the signal handler entirely.
> profiling. hv_vcpu_interrupt() has been demonstrated to very reliably
> cause VM exits - even if the target vCPU isn't even running, it will
> immediately exit on entry.
>
> Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu>
> ---
> target/i386/hvf/hvf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 3b6ee79fb2..936c31dbdd 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -214,7 +214,7 @@ static inline bool apic_bus_freq_is_known(CPUX86State *env)
>
> void hvf_kick_vcpu_thread(CPUState *cpu)
> {
> - cpus_kick_thread(cpu);
> + cpu->thread_kicked = true;
> hv_vcpu_interrupt(&cpu->accel->fd, 1);
> }
>