On Tue, Jan 15, 2013 at 07:43:40PM -0500, Christoffer Dall wrote:
> Hi KVM guys,
>
> I've had a bit of challenges figuring out the exact functinality and
> synchronization of vcpu->requests and friends. In lack of a better
> method, I wrote some comments as a patch.
>
> I think this code really deserves some explaining, as it is really hard
> to understand otherwise. Unfortunately, I wasn't able to write down
> concise and exact comments, but I hope someone else feels up to the
> challenge.
>
> Let me know if I just got this completely wrong and upside down.
>
> Thanks,
> Christoffer
> ---
> include/linux/kvm_host.h | 4 ++++
> virt/kvm/kvm_main.c | 29 +++++++++++++++++++++++++++--
> 2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index cbe0d68..25deef8 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -252,6 +252,10 @@ struct kvm_vcpu {
> struct kvm_vcpu_arch arch;
> };
>
> +/*
> + * XXX: Could we explain what we're trying to achieve? Is this an
> + * optimization as to not send multiple IPIs?
> + */
> static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu)
> {
> return cmpxchg(&vcpu->mode, IN_GUEST_MODE, EXITING_GUEST_MODE);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index e45c20c..ccc292d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -165,7 +165,18 @@ static void ack_flush(void *_completed)
> {
> }
>
> -static bool make_all_cpus_request(struct kvm *kvm, unsigned int req)
> +/**
> + * make_all_cpus_request - place request on vcpus
> + * @kvm: KVM Struct
> + * @req: Request to make on the VCPU
> + *
> + * This function places a request on a VCPU and ensures that the VCPU request
> + * is handled before returning from the function, if the VCPU is in guest
> + * mode (or exiting, or reading shadow page tables?).
> + *
> + * Returns true if at least of the vcpus were sent an IPI and responded to
> it,
> + */
> +static bool make_all_vcpus_request(struct kvm *kvm, unsigned int req)
> {
> int i, cpu, me;
> cpumask_var_t cpus;
> @@ -179,9 +190,19 @@ static bool make_all_cpus_request(struct kvm *kvm,
> unsigned int req)
> kvm_make_request(req, vcpu);
> cpu = vcpu->cpu;
>
> + /*
> + * Is the following really true? Can we have an example of the
> + * race that would otherwise happen? Doesn't the make_request
> + * pair against the IPI and interrupt disabling, not the mode?
> + */
> /* Set ->requests bit before we read ->mode */
> smp_mb();
cpu0 cpu1
make_all_reqs
mode = vcpu->mode = OUTSIDE_GUEST_MODE
vcpu->mode = IN_GUEST_MODE
if (vcpu->requests)
set_bit(KVM_REQ_y, vcpu)
no kick, its outside guest mode
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
