| Message ID | 20241209203629.74436-4-phil@philjordan.eu (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hvf and APIC fixes, improvements, and optimisations | expand |
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? > 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); > } >
On Mon 9. Dec 2024 at 22:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > 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); > > } > > > >
On 10.12.2024 04:22, Philippe Mathieu-Daudé wrote: > 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? > >> 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); >> } > SIG_IPI is macOS crutch handled in XNU kernel that was essential until Phil submitted proper kick support with hv_vcpu_interrupt().
On Tue 10. Dec 2024 at 10:21, Roman Bolshakov <rbolshakov@ddn.com> wrote: > On 10.12.2024 04:22, Philippe Mathieu-Daudé wrote: > > 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? > > > >> 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); > >> } > > > SIG_IPI is macOS crutch handled in XNU kernel that was essential until > Phil submitted proper kick support with hv_vcpu_interrupt(). > > Ah yes, perhaps it allowed exit from hv_vcpu_run(). hv_vcpu_run_until() definitely does not exit early upon receiving SIG_IPI (USR1).
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); }