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

[Roman Bolshakov]

On Tue, Jun 30, 2020 at 06:04:23PM +0200, Paolo Bonzini wrote:
> On 30/06/20 17:50, Roman Bolshakov wrote:
> > On Tue, Jun 30, 2020 at 02:33:42PM +0200, Paolo Bonzini wrote:
> >> Can a signal interrupt hv_vcpu_run?  If so you actually don't need
> >> hv_vcpu_interrupt at all.
> > 
> > Existing signal masking and SIG_IPI didn't work IIRC when I tried to add
> > a primitive version of gdbstub support.
> 
> You can try pthread_kill followed by hv_vcpu_interrupt if it doesn't.
> The signal would be delivered after return to userspace.
> 

I looked at the signal setup for HVF again. I was wrong with regards to
SIG_IPI. It isn't delivered to vCPU because the signal is masked, this
fixes it:

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index d81f569aed..7bf05bca21 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -479,6 +479,7 @@ int hvf_init_vcpu(CPUState *cpu)

     pthread_sigmask(SIG_BLOCK, NULL, &set);
     sigdelset(&set, SIG_IPI);
+    pthread_sigmask(SIG_SETMASK, &set, NULL);

     init_emu();
     init_decoder();

But the signal is delivered only after vmxexit, perhaps a sequence of
pthread_kill() and hv_vcpu_interrupt() is really needed.

So, there are two race windows on kernel-to-user border in v2: just
before checking the deadline and vmenter and just after vmxexit and
re-arm of preemption timer, that's two places where kicks could be lost.
The approach you proposed seems to address them.

Thanks,
Roman

> >> You can also require the preemption time, all
> >> processor that support HVF have it, but never set it by default.  The
> >> deadline can be left at 0 all the time; instead, you toggle the bit in
> >> the pin-based controls.  In the signal handler you do:
> >>
> >>    if (atomic_xchg(&env->hvf_in_guest, false)) {
> >>            wvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS,
> >>                  rvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS)
> >>                    | VMCS_PIN_BASED_CTLS_VMX_PREEMPT_TIMER);
> >>    }
> >>
> >> In the main loop you do:
> >>
> >>    atomic_set(&env->hvf_guest_mode, true);
> >>    smp_mb();
> >>    hv_vcpu_run(...);
> >>    atomic_set(&env->hvf_guest_mode, false);
> >>
> >> and in the preemption timer vmexit handler:
> >>    
> >>            wvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS,
> >>                  rvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS)
> >>                    & ~VMCS_PIN_BASED_CTLS_VMX_PREEMPT_TIMER);
> >>
> >