Re: [PATCH v3 02/10] KVM: Add documentation for VCPU requests

[Paolo Bonzini <pbonzini@redhat.com>]

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