2017-04-05 20:29+0200, Paolo Bonzini:
> On 05/04/2017 19:45, Christoffer Dall wrote:
>>>> But the problem is that kvm_make_all_cpus_request() only sends IPIs to
>>>> CPUs where the mode was different from OUTSIDE_GUEST_MODE, so there it's
>>>> about !OUTSIDE_GUEST_MODE rather than !IN_GUEST_MODE, so there's some
>>>> subtlety here which I feel like it's dangerous to paper over.
>>> Right, that needs fixing in the code.
>> Really? I thought Paolo said that this is the intended behavior and
>> semantics; non-urgent requests that should just be serviced before the
>> next guest entry.
>
> Indeed, that's right...
>
>>> guest_mode is just an optimization that allows us to skip sending the
>>> IPI when the VCPU is known to handle the request as soon as possible.
>>>
>>> IN_GUEST_MODE: we must force VM exit or the request could never be
>>> handled
>>> EXITING_GUEST_MODE: another request already forces the VM exit and
>>> we're just waiting for the VCPU to notice our request
>>> OUTSIDE_GUEST_MODE: KVM is going to notice our request without any
>>> intervention
>>> READING_SHADOW_PAGE_TABLES: same as OUTSIDE_GUEST_MODE -- rename to
>>> unwieldly OUTSIDE_GUEST_MODE_READING_SHADOW_PAGE_TABLES?
>> Again, I thought Paolo was arguing that EXITING_GUEST_MODE makes the
>> whole thing work because you check that after checking requests?
>
> ... but apparently I was wrong here, see my email from this morning.
Ok, I'll prepare a patch that uses kvm_arch_vcpu_should_kick() in
kvm_make_all_cpus_request().
>>> The kick is needed only in IN_GUEST_MODE and a wake up is needed in case
>>> where the guest is halted OUTSIDE_GUEST_MODE ...
>>> Hm, maybe we should add a halt state too?
>> Wouldn't that be swait_active(&vcpu->wq) ? You could add a wrapper
>> though.
>
> Yes. I think the wrapper is probably unnecessary.
The state would only make sense if it were different -- there is a time
when the VCPU is halted, but swait_active(&vcpu->wq) returns false.
Probably doesn't have any good use now, though.
>> What I think you need is a way to distinguish the semantics of calling
>> kvm_make_all_cpus_request(), perhaps by adding a 'bool wake_up'
>> parameter.
>
> That would be fine.
I think the wakeup is more tied to the request type than to the place
calling it, so we could say which requests are "lazy" and don't do
wakeup and have kvm_make_all_cpus_request() without an extra argument.
In the end, I would like if kvm_make_all_cpus_request() was an optimized
variant of
kvm_for_each_vcpu(i, vcpu, kvm) {
kvm_make_request(vcpu, request);
kvm_vcpu_kick(vcpu);
}
