Re: [PATCH 4/8] i386: hvf: Implement CPU kick

[Roman Bolshakov <r.bolshakov@yadro.com>]

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);