Ok, I'll review the patch to see what you've implemented. Thanks! Paolo
On 30/06/20 12:12, Roman Bolshakov wrote: > On Mon, Jun 29, 2020 at 04:18:46PM +0200, Paolo Bonzini wrote: >> On 29/06/20 16:04, Roman Bolshakov wrote: >>> My approach is based >>> hv_vcpu_run() and should hopefully work almost anywhere where >>> Hypervisor.framework is available because Hypervisor framework exposes >>> timer value >>> (https://developer.apple.com/documentation/hypervisor/vmcs_guest_vmx_timer_value) >>> since macOS 10.10.3+. >> >> There are a few other constants for which it would be unwise to write >> from userspace, so that's not a big consolation. :) >> > > Hi Paolo, > > So, I've tried Big Sur Beta and it has exactly the same performance > issue with hv_vcpu_run_until() while hv_vcpu_run() works as good as it > worked on 10.15.5. I've submitted FB7827341 to Apple wrt the issue. > >>> I can also test how hv_vcpu_run_until() performs with HV_DEADLINE_FOREVER >>> on the Beta. And if the performance issues with VMX-preemption timer and >>> hv_vcpu_run_until() are fixed there. >> >> Thanks! The main thing to test on Big Sur would be: 1) whether the >> preemption timer bit in the pin controls "sticks" to 0 after setting it > > It does not. If it's set, it stays there. > >> 2) whether the bit reads back as zero after >> hv_vcpu_run_until(HV_DEADLINE_FOREVER). >> > > Likewise, it's not cleared if set. > > As far as I understand, hv_vcpu_run_until(HV_DEADLINE_FOREVER) works > like hv_vcpu_run() without VMX-preemption timer. Otherwise > hv_vcpu_run_until() implicitly sets VMX-preemption timer Pin-based > control and sets the timer value. > > Thanks, > Roman > > Here's the patch over v2 that adds support of hv_vcpu_run_until() on Big Sur: > diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c > index 317304aa1d..ad202f7358 100644 > --- a/target/i386/hvf/hvf.c > +++ b/target/i386/hvf/hvf.c > @@ -72,8 +72,12 @@ > #include "sysemu/accel.h" > #include "target/i386/cpu.h" > > +#if defined(__MAC_10_16) && __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_10_16 > +#define HVF_MAX_DEADLINE HV_DEADLINE_FOREVER > +#else > /* Maximum value of VMX-preemption timer */ > #define HVF_MAX_DEADLINE UINT32_MAX > +#endif > > HVFState *hvf_state; > > @@ -693,6 +697,7 @@ int hvf_vcpu_exec(CPUState *cpu) > CPUX86State *env = &x86_cpu->env; > int ret = 0; > uint64_t rip = 0; > + hv_return_t r; > > if (hvf_process_events(cpu)) { > return EXCP_HLT; > @@ -718,10 +723,22 @@ int hvf_vcpu_exec(CPUState *cpu) > /* Use VMX-preemption timer trick only if available */ > if (rvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS) & > VMCS_PIN_BASED_CTLS_VMX_PREEMPT_TIMER) { > +#if defined(__MAC_10_16) && __MAC_OS_X_VERSION_MIN_REQUIRED >= __MAC_10_16 > + r = hv_vcpu_run_until(cpu->hvf_fd, > + atomic_read(&env->hvf_deadline)); > + } else { > + /* > + * Equivalent to behaviour of hv_vcpu_run() with VMX-preemption > + * timer disabled, prone to kick loss. > + */ > + r = hv_vcpu_run_until(cpu->hvf_fd, HVF_MAX_DEADLINE); > + } > +#else > wvmcs(cpu->hvf_fd, VMCS_PREEMPTION_TIMER_VALUE, > atomic_read(&env->hvf_deadline)); > } > - hv_return_t r = hv_vcpu_run(cpu->hvf_fd); > + r = hv_vcpu_run(cpu->hvf_fd); > +#endif > atomic_set(&env->hvf_deadline, HVF_MAX_DEADLINE); > assert_hvf_ok(r); > >
