On 04/05/2017 14:06, Andrew Jones wrote:
>>> +VCPU threads may need to consider requests before and/or after calling
>>> +functions that may put them to sleep, e.g. kvm_vcpu_block(). Whether they
>>> +do or not, and, if they do, which requests need consideration, is
>>> +architecture dependent. kvm_vcpu_block() calls kvm_arch_vcpu_runnable()
>>> +to check if it should awaken. One reason to do so is to provide
>>> +architectures a function where requests may be checked if necessary.
>> What did you have in mind here?
> I was trying to point out vcpu request concerns with respect to sleeping
> vcpus, but to stay as general as possible. I can't really think of
> anything else to say here, other than to give some hypothetical example.
> For a while I was thinking I might check requests (kvm_request_pending())
> from the kvm_arch_vcpu_runnable() call for ARM, but then changed my mind
> on that - leaving it only checking the pause and power_off booleans.
> Anyway, I don't think the above paragraph is "wrong", but if it's
> confusing then I can change / remove it as people like. Just let me know
> how you'd like it changed :-)
I think the x86 scheme, where you only process requests once you have
decided you'll get IN_GUEST_MODE, is a good one.
That is, they _may_ check some requests in kvm_arch_vcpu_runnable but
not process them.
For ARM this would be:
if (vcpu->arch.power_off || vcpu->arch.pause) {
vcpu_sleep(vcpu);
ret = 0;
} else {
ret = vcpu_enter_guest(vcpu);
}
where vcpu_enter_guest is basically the "while (ret > 0)" loop in
kvm_arch_vcpu_ioctl_run:
/*
* Check conditions before entering the guest
*/
cond_resched();
update_vttbr(vcpu->kvm);
preempt_disable();
...
if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
vcpu->arch.power_off || vcpu->arch.pause) {
local_irq_enable();
kvm_pmu_sync_hwstate(vcpu);
kvm_timer_sync_hwstate(vcpu);
kvm_vgic_sync_hwstate(vcpu);
preempt_enable();
return ret;
}
...
preempt_enable();
return handle_exit(vcpu, run, ret);
In your case, you don't need to check any request in
kvm_arch_vcpu_runnable, I think. This split would also solve my review
doubt from "Re: [PATCH v3 05/10] KVM: arm/arm64: don't clear exit
request from caller".
Paolo
