On 04/04/2017 18:04, Christoffer Dall wrote:
>> For pause, only the requester should do the clearing.
This suggests that maybe this should not be a request. The request
would be just the need to act on a GIC command, exactly as before this patch.
What I don't understand is:
>> With this patch, while the vcpu will still initially enter
>> the guest, it will exit immediately due to the IPI sent by the vcpu
>> kick issued after making the vcpu request.
Isn't this also true of KVM_REQ_VCPU_EXIT that was used before?
So this:
+ vcpu->arch.power_off || kvm_request_pending(vcpu)) {
+ WRITE_ONCE(vcpu->mode, OUTSIDE_GUEST_MODE);
is the crux of the fix, you can keep using vcpu->arch.pause.
By the way, vcpu->arch.power_off can go away from this "if" too because
KVM_RUN and KVM_SET_MP_STATE are mutually exclusive through the vcpu mutex.
The earlier check is enough:
if (vcpu->arch.power_off || vcpu->arch.pause)
vcpu_sleep(vcpu);
>> + /*
>> + * Indicate we're in guest mode now, before doing a final
>> + * check for pending vcpu requests. The general barrier
>> + * pairs with the one in kvm_arch_vcpu_should_kick().
>> + * Please see the comment there for more details.
>> + */
>> + WRITE_ONCE(vcpu->mode, IN_GUEST_MODE);
>> + smp_mb();
>
> There are two changes here:
>
> there's a change from a normal write to a WRITE_ONCE and there's also a
> change to that adds a memory barrier. I feel like I'd like to know if
> these are tied together or two separate cleanups. I also wonder if we
> could split out more general changes from the pause thing to have a
> better log of why we changed the run loop?
You probably should just use smp_store_mb here.
Paolo
