Re: [PATCH v4 3/4] Introduce the NVMM impl

[Maxime Villard]

Le 10/03/2020 à 11:58, Paolo Bonzini a écrit :
> On 10/03/20 07:45, Maxime Villard wrote:
>>> It reproduces the existing logic found in whpx-all.c, and if there is
>>>
>>
>> It's buggy there too and it has to be fixed in the hypervisor so it
>> can't be done at the same time I'm both. KVM does it right by having
>> a flag ("immediate_exit") that is set by the signal handler and
>> checked by the hypervisor.
> 
> For what it's worth, WHPX's whpx_kick_vcpu invokes a "cancel entry" API
> that is probably similar to what KVM does, so there's nothing to do there.
> 
> Paolo

Having had some time to look at the actual code today, I'm wondering why
WHPX uses whpx_vcpu_kick(), and why I even use qemu_cpu_kick_self() in
NVMM.

whpx_vcpu_kick() cancels the RunVirtualProcessor on the given VCPU. But
that VCPU is not executing anyway, since we're the calling thread, and if
we are in userland then we cannot be in the guest. It seems to me the code
should just break the loop directly instead of doing a self-kick, given
that it is known that the VCPU is not executing.

Maybe, whpx_vcpu_kick() causes a WHvRunVpExitReasonCanceled in the
WHvRunVirtualProcessor() call that follows, which in turn causes "ret=1"
to leave the loop. That is, maybe the next WHvRunVirtualProcessor() acks
the cancellation and leaves without doing anything, even if the
cancellation was received when this function wasn't executing. So there is
no bad effect, given that we still end up leaving the loop, which is the
desired functional behavior.

Looking at NVMM now, it seems to me there is the same thing. We do a
self-kick but we're the calling thread and know the VCPU isn't executing.
As a result of the self-kick the IPI handler sets
        qcpu->stop = true;
And in the next iteration of the loop, we break because this bool is set.
Again, we took a difficult path, but this is the desired final behavior.

In both WHPX and NVMM, it seems that we are doing unnecessary self-kicks,
and could just break the loop right away.

What am I getting wrong? The thread logic in Qemu is not really easy to
grasp..

Thanks,
Maxime