[PATCH] use guest value of version field in kvmclock

[PATCH] use guest value of version field in kvmclock

[Glauber Costa]

Avi noticed that we have to use guest's value for the version field,
instead of keeping track of it ourselves. If we don't do that,
the following situation can arise:

 vcpu->arch.hv_clock.version is initialized to zero.
 Guest reads version (result: 2)
 Guest starts reading data
 Live migration; vcpu->arch.hv_clock.version is zeroed
 Steal time update; vcpu->arch.hv_clock.version += 2; write to guest
 Guest continues reading data
 Guest reads version (result: 2)

Signed-off-by: Glauber Costa <glommer@redhat.com>
CC: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/x86.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c39ab4a..7fdc84a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1151,15 +1151,18 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
 	vcpu->last_guest_tsc = tsc_timestamp;
 	vcpu->hv_clock.flags = 0;
 
+	shared_kaddr = kmap_atomic(vcpu->time_page, KM_USER0);
+
 	/*
 	 * The interface expects us to write an even number signaling that the
 	 * update is finished. Since the guest won't see the intermediate
-	 * state, we just increase by 2 at the end.
+	 * state, we just increase by 2 at the end. We use the guest's value as
+	 * a basis to make it migration-safe.
 	 */
+	memcpy(&vcpu->hv_clock, shared_kaddr + vcpu->time_offset,
+	       sizeof(vcpu->hv_clock));
 	vcpu->hv_clock.version += 2;
 
-	shared_kaddr = kmap_atomic(vcpu->time_page, KM_USER0);
-
 	memcpy(shared_kaddr + vcpu->time_offset, &vcpu->hv_clock,
 	       sizeof(vcpu->hv_clock));
 
-- 
1.7.2.3

Re: [PATCH] use guest value of version field in kvmclock

[Glauber Costa]

On Wed, Feb 2, 2011 at 10:51 AM, Glauber Costa <glommer@redhat.com> wrote:
> Avi noticed that we have to use guest's value for the version field,
> instead of keeping track of it ourselves. If we don't do that,
> the following situation can arise:
>
>  vcpu->arch.hv_clock.version is initialized to zero.
>  Guest reads version (result: 2)
>  Guest starts reading data
>  Live migration; vcpu->arch.hv_clock.version is zeroed
>  Steal time update; vcpu->arch.hv_clock.version += 2; write to guest
>  Guest continues reading data
>  Guest reads version (result: 2)
Please ignore it. I did test this patch but appearently used the wrong
module, and it tricked me
this version is obviously wrong.

>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> CC: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/x86.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index c39ab4a..7fdc84a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1151,15 +1151,18 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
>        vcpu->last_guest_tsc = tsc_timestamp;
>        vcpu->hv_clock.flags = 0;
>
> +       shared_kaddr = kmap_atomic(vcpu->time_page, KM_USER0);
> +
>        /*
>         * The interface expects us to write an even number signaling that the
>         * update is finished. Since the guest won't see the intermediate
> -        * state, we just increase by 2 at the end.
> +        * state, we just increase by 2 at the end. We use the guest's value as
> +        * a basis to make it migration-safe.
>         */
> +       memcpy(&vcpu->hv_clock, shared_kaddr + vcpu->time_offset,
> +              sizeof(vcpu->hv_clock));
>        vcpu->hv_clock.version += 2;
>
> -       shared_kaddr = kmap_atomic(vcpu->time_page, KM_USER0);
> -
>        memcpy(shared_kaddr + vcpu->time_offset, &vcpu->hv_clock,
>               sizeof(vcpu->hv_clock));
>
> --
> 1.7.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
Sent from my Atari.