| Message ID | 20210915181049.27597-6-agraf@csgraf.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hvf: Implement Apple Silicon Support | expand |
On 9/15/21 8:10 PM, Alexander Graf wrote: > From: Peter Collingbourne <pcc@google.com> > > Sleep on WFI until the VTIMER is due but allow ourselves to be woken > up on IPI. > > In this implementation IPI is blocked on the CPU thread at startup and > pselect() is used to atomically unblock the signal and begin sleeping. > The signal is sent unconditionally so there's no need to worry about > races between actually sleeping and the "we think we're sleeping" > state. It may lead to an extra wakeup but that's better than missing > it entirely. > > Signed-off-by: Peter Collingbourne <pcc@google.com> > [agraf: Remove unused 'set' variable, always advance PC on WFX trap, > support vm stop / continue operations and cntv offsets] > Signed-off-by: Alexander Graf <agraf@csgraf.de> > Acked-by: Roman Bolshakov <r.bolshakov@yadro.com> > Reviewed-by: Sergio Lopez <slp@redhat.com> > > --- > diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c > index 8fe008dab5..49f265cc08 100644 > --- a/target/arm/hvf/hvf.c > +++ b/target/arm/hvf/hvf.c > @@ -2,6 +2,7 @@ > * QEMU Hypervisor.framework support for Apple Silicon > > * Copyright 2020 Alexander Graf <agraf@csgraf.de> > + * Copyright 2020 Google LLC > * > * This work is licensed under the terms of the GNU GPL, version 2 or later. > * See the COPYING file in the top-level directory. > @@ -490,6 +491,7 @@ int hvf_arch_init_vcpu(CPUState *cpu) > > void hvf_kick_vcpu_thread(CPUState *cpu) > { > + cpus_kick_thread(cpu); Doesn't this belong to the previous patch? > hv_vcpus_exit(&cpu->hvf->fd, 1); > } > +static void hvf_wfi(CPUState *cpu) > +{ > + ARMCPU *arm_cpu = ARM_CPU(cpu); > + hv_return_t r; > + uint64_t ctl; > + uint64_t cval; > + int64_t ticks_to_sleep; > + uint64_t seconds; > + uint64_t nanos; > + > + if (cpu->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_FIQ)) { > + /* Interrupt pending, no need to wait */ > + return; > + } > + > + r = hv_vcpu_get_sys_reg(cpu->hvf->fd, HV_SYS_REG_CNTV_CTL_EL0, &ctl); > + assert_hvf_ok(r); > + > + if (!(ctl & 1) || (ctl & 2)) { > + /* Timer disabled or masked, just wait for an IPI. */ > + hvf_wait_for_ipi(cpu, NULL); > + return; > + } > + > + r = hv_vcpu_get_sys_reg(cpu->hvf->fd, HV_SYS_REG_CNTV_CVAL_EL0, &cval); > + assert_hvf_ok(r); > + > + ticks_to_sleep = cval - hvf_vtimer_val(); > + if (ticks_to_sleep < 0) { > + return; > + } > + > + nanos = ticks_to_sleep * gt_cntfrq_period_ns(arm_cpu); > + seconds = nanos / NANOSECONDS_PER_SECOND; muldiv64()? > + nanos -= (seconds * NANOSECONDS_PER_SECOND); > + > + /* > + * Don't sleep for less than the time a context switch would take, > + * so that we can satisfy fast timer requests on the same CPU. > + * Measurements on M1 show the sweet spot to be ~2ms. > + */ > + if (!seconds && nanos < (2 * SCALE_MS)) { > + return; > + } > + > + struct timespec ts = { seconds, nanos }; QEMU style still declares variables at top of function/block. > + hvf_wait_for_ipi(cpu, &ts); > +}
On Wed, 15 Sept 2021 at 19:10, Alexander Graf <agraf@csgraf.de> wrote: > > From: Peter Collingbourne <pcc@google.com> > > Sleep on WFI until the VTIMER is due but allow ourselves to be woken > up on IPI. > > In this implementation IPI is blocked on the CPU thread at startup and > pselect() is used to atomically unblock the signal and begin sleeping. > The signal is sent unconditionally so there's no need to worry about > races between actually sleeping and the "we think we're sleeping" > state. It may lead to an extra wakeup but that's better than missing > it entirely. > > Signed-off-by: Peter Collingbourne <pcc@google.com> > [agraf: Remove unused 'set' variable, always advance PC on WFX trap, > support vm stop / continue operations and cntv offsets] > Signed-off-by: Alexander Graf <agraf@csgraf.de> > Acked-by: Roman Bolshakov <r.bolshakov@yadro.com> > Reviewed-by: Sergio Lopez <slp@redhat.com> > > --- Other than the points Philippe raises, Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 16.09.21 06:49, Philippe Mathieu-Daudé wrote: > On 9/15/21 8:10 PM, Alexander Graf wrote: >> From: Peter Collingbourne <pcc@google.com> >> >> Sleep on WFI until the VTIMER is due but allow ourselves to be woken >> up on IPI. >> >> In this implementation IPI is blocked on the CPU thread at startup and >> pselect() is used to atomically unblock the signal and begin sleeping. >> The signal is sent unconditionally so there's no need to worry about >> races between actually sleeping and the "we think we're sleeping" >> state. It may lead to an extra wakeup but that's better than missing >> it entirely. >> >> Signed-off-by: Peter Collingbourne <pcc@google.com> >> [agraf: Remove unused 'set' variable, always advance PC on WFX trap, >> support vm stop / continue operations and cntv offsets] >> Signed-off-by: Alexander Graf <agraf@csgraf.de> >> Acked-by: Roman Bolshakov <r.bolshakov@yadro.com> >> Reviewed-by: Sergio Lopez <slp@redhat.com> >> >> --- >> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c >> index 8fe008dab5..49f265cc08 100644 >> --- a/target/arm/hvf/hvf.c >> +++ b/target/arm/hvf/hvf.c >> @@ -2,6 +2,7 @@ >> * QEMU Hypervisor.framework support for Apple Silicon >> >> * Copyright 2020 Alexander Graf <agraf@csgraf.de> >> + * Copyright 2020 Google LLC >> * >> * This work is licensed under the terms of the GNU GPL, version 2 or later. >> * See the COPYING file in the top-level directory. >> @@ -490,6 +491,7 @@ int hvf_arch_init_vcpu(CPUState *cpu) >> >> void hvf_kick_vcpu_thread(CPUState *cpu) >> { >> + cpus_kick_thread(cpu); > Doesn't this belong to the previous patch? Until this patch, we're never running outside guest context on the vCPU thread, so hv_vcpus_exit() is enough to kick us out :). Thanks a lot for the review! Alex
diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c index 4f75927a8e..93976f4ece 100644 --- a/accel/hvf/hvf-accel-ops.c +++ b/accel/hvf/hvf-accel-ops.c @@ -370,15 +370,14 @@ static int hvf_init_vcpu(CPUState *cpu) cpu->hvf = g_malloc0(sizeof(*cpu->hvf)); /* init cpu signals */ - sigset_t set; struct sigaction sigact; memset(&sigact, 0, sizeof(sigact)); sigact.sa_handler = dummy_signal; sigaction(SIG_IPI, &sigact, NULL); - pthread_sigmask(SIG_BLOCK, NULL, &set); - sigdelset(&set, SIG_IPI); + pthread_sigmask(SIG_BLOCK, NULL, &cpu->hvf->unblock_ipi_mask); + sigdelset(&cpu->hvf->unblock_ipi_mask, SIG_IPI); #ifdef __aarch64__ r = hv_vcpu_create(&cpu->hvf->fd, (hv_vcpu_exit_t **)&cpu->hvf->exit, NULL); diff --git a/include/sysemu/hvf_int.h b/include/sysemu/hvf_int.h index 7c245c7b11..6545f7cd61 100644 --- a/include/sysemu/hvf_int.h +++ b/include/sysemu/hvf_int.h @@ -52,6 +52,7 @@ struct hvf_vcpu_state { uint64_t fd; void *exit; bool vtimer_masked; + sigset_t unblock_ipi_mask; }; void assert_hvf_ok(hv_return_t ret); diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index 8fe008dab5..49f265cc08 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -2,6 +2,7 @@ * QEMU Hypervisor.framework support for Apple Silicon * Copyright 2020 Alexander Graf <agraf@csgraf.de> + * Copyright 2020 Google LLC * * This work is licensed under the terms of the GNU GPL, version 2 or later. * See the COPYING file in the top-level directory. @@ -490,6 +491,7 @@ int hvf_arch_init_vcpu(CPUState *cpu) void hvf_kick_vcpu_thread(CPUState *cpu) { + cpus_kick_thread(cpu); hv_vcpus_exit(&cpu->hvf->fd, 1); } @@ -608,6 +610,77 @@ static uint64_t hvf_vtimer_val_raw(void) return mach_absolute_time() - hvf_state->vtimer_offset; } +static uint64_t hvf_vtimer_val(void) +{ + if (!runstate_is_running()) { + /* VM is paused, the vtimer value is in vtimer.vtimer_val */ + return vtimer.vtimer_val; + } + + return hvf_vtimer_val_raw(); +} + +static void hvf_wait_for_ipi(CPUState *cpu, struct timespec *ts) +{ + /* + * Use pselect to sleep so that other threads can IPI us while we're + * sleeping. + */ + qatomic_mb_set(&cpu->thread_kicked, false); + qemu_mutex_unlock_iothread(); + pselect(0, 0, 0, 0, ts, &cpu->hvf->unblock_ipi_mask); + qemu_mutex_lock_iothread(); +} + +static void hvf_wfi(CPUState *cpu) +{ + ARMCPU *arm_cpu = ARM_CPU(cpu); + hv_return_t r; + uint64_t ctl; + uint64_t cval; + int64_t ticks_to_sleep; + uint64_t seconds; + uint64_t nanos; + + if (cpu->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_FIQ)) { + /* Interrupt pending, no need to wait */ + return; + } + + r = hv_vcpu_get_sys_reg(cpu->hvf->fd, HV_SYS_REG_CNTV_CTL_EL0, &ctl); + assert_hvf_ok(r); + + if (!(ctl & 1) || (ctl & 2)) { + /* Timer disabled or masked, just wait for an IPI. */ + hvf_wait_for_ipi(cpu, NULL); + return; + } + + r = hv_vcpu_get_sys_reg(cpu->hvf->fd, HV_SYS_REG_CNTV_CVAL_EL0, &cval); + assert_hvf_ok(r); + + ticks_to_sleep = cval - hvf_vtimer_val(); + if (ticks_to_sleep < 0) { + return; + } + + nanos = ticks_to_sleep * gt_cntfrq_period_ns(arm_cpu); + seconds = nanos / NANOSECONDS_PER_SECOND; + nanos -= (seconds * NANOSECONDS_PER_SECOND); + + /* + * Don't sleep for less than the time a context switch would take, + * so that we can satisfy fast timer requests on the same CPU. + * Measurements on M1 show the sweet spot to be ~2ms. + */ + if (!seconds && nanos < (2 * SCALE_MS)) { + return; + } + + struct timespec ts = { seconds, nanos }; + hvf_wait_for_ipi(cpu, &ts); +} + static void hvf_sync_vtimer(CPUState *cpu) { ARMCPU *arm_cpu = ARM_CPU(cpu); @@ -728,6 +801,9 @@ int hvf_vcpu_exec(CPUState *cpu) } case EC_WFX_TRAP: advance_pc = true; + if (!(syndrome & WFX_IS_WFE)) { + hvf_wfi(cpu); + } break; case EC_AA64_HVC: cpu_synchronize_state(cpu);