Re: [PATCH v2 4/9] KVM: arm/arm64: replace vcpu->arch.pause with a vcpu request

[Paolo Bonzini <pbonzini@redhat.com>]

On 06/04/2017 22:14, Christoffer Dall wrote:
>> So it seems like there are no races after all in KVM/ARM code
>
> No races after Drew's fix has been applied to set vcpu->mode =
> IN_GUEST_MODE, before checking the pause flag, correct?  (I think that's
> what the spin model below is modeling).

Yes.  All of them include the vcpu->mode = IN_GUEST_MODE assignment and
(implicitly because spin is sequentially consistent) the memory barrier.

>> After this experiment, I think I like Drew's KVM_REQ_PAUSE more than I did
>> yesterday.  However, yet another alternative is to leave pause/power_off as
>> they are, while taking some inspiration from his patch to do some cleanups:
>>
>> 1) change the "if"
>>
>>                 if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
>>                         vcpu->arch.power_off || vcpu->arch.pause) {
>>
>> to test kvm_requests_pending instead of pause/power_off
>>
>> 2) clear KVM_REQ_VCPU_EXIT before the other "if":
>>
>>                 if (vcpu->arch.power_off || vcpu->arch.pause)
>>                         vcpu_sleep(vcpu);
>
> I like using requests as only requests from one thread to the VCPU
> thread, and not to maintain specific state about a VCPU.
> 
> The benefit of Drew's approach is that since these pieces of state are
> boolean, you can have just a single check in the critical path in the
> run loop instead of having to access multiple fields.

I think you'd still need two checks for KVM_REQ_PAUSE/KVM_REQ_POWEROFF:
one to check whether to sleep, and one to check whether to abort the
vmentry.

Pause and power_off could be merged into a single bitmask if necessary, too.

Paolo