Re: [PATCH] kvm: x86: Fix kvm clock versioning.

2014-11-06 Thread Paolo Bonzini


On 04/11/2014 01:57, Owen Hofmann wrote:
> kvm updates the version number for the guest paravirt clock structure by
> incrementing the version of its private copy. It does not read the guest
> version, so will write version = 2 in the first update for every new VM,
> including after restoring a saved state. If guest state is saved during
> reading the clock, it could read and accept struct fields and guest TSC
> from two different updates. This changes the code to increment the guest
> version and write it back.
> 
> Signed-off-by: Owen Hofmann 
> ---
>  arch/x86/kvm/x86.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0033df3..c25c9d8 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1637,16 +1637,16 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
>   vcpu->hv_clock.system_time = kernel_ns + v->kvm->arch.kvmclock_offset;
>   vcpu->last_guest_tsc = tsc_timestamp;
>  
> + if (unlikely(kvm_read_guest_cached(v->kvm, &vcpu->pv_time,
> + &guest_hv_clock, sizeof(guest_hv_clock
> + return 0;
> +
>   /*
>* 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.
>*/
> - vcpu->hv_clock.version += 2;
> -
> - if (unlikely(kvm_read_guest_cached(v->kvm, &vcpu->pv_time,
> - &guest_hv_clock, sizeof(guest_hv_clock
> - return 0;
> + vcpu->hv_clock.version = guest_hv_clock.version + 2;
>  
>   /* retain PVCLOCK_GUEST_STOPPED if set in guest copy */
>   pvclock_flags = (guest_hv_clock.flags & PVCLOCK_GUEST_STOPPED);
> 

Applied, thanks.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] kvm: x86: Fix kvm clock versioning.

2014-11-04 Thread Marcelo Tosatti
On Mon, Nov 03, 2014 at 04:57:18PM -0800, Owen Hofmann wrote:
> kvm updates the version number for the guest paravirt clock structure by
> incrementing the version of its private copy. It does not read the guest
> version, so will write version = 2 in the first update for every new VM,
> including after restoring a saved state. If guest state is saved during
> reading the clock, it could read and accept struct fields and guest TSC
> from two different updates. This changes the code to increment the guest
> version and write it back.
> 
> Signed-off-by: Owen Hofmann 
> ---
>  arch/x86/kvm/x86.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0033df3..c25c9d8 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1637,16 +1637,16 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
>   vcpu->hv_clock.system_time = kernel_ns + v->kvm->arch.kvmclock_offset;
>   vcpu->last_guest_tsc = tsc_timestamp;
>  
> + if (unlikely(kvm_read_guest_cached(v->kvm, &vcpu->pv_time,
> + &guest_hv_clock, sizeof(guest_hv_clock
> + return 0;
> +
>   /*
>* 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.
>*/
> - vcpu->hv_clock.version += 2;
> -
> - if (unlikely(kvm_read_guest_cached(v->kvm, &vcpu->pv_time,
> - &guest_hv_clock, sizeof(guest_hv_clock
> - return 0;
> + vcpu->hv_clock.version = guest_hv_clock.version + 2;
>  
>   /* retain PVCLOCK_GUEST_STOPPED if set in guest copy */
>   pvclock_flags = (guest_hv_clock.flags & PVCLOCK_GUEST_STOPPED);
> -- 
> 2.1.0.rc2.206.gedb03e5

Reviewed-by: Marcelo Tosatti 

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] kvm: x86: Fix kvm clock versioning.

2014-11-03 Thread Owen Hofmann
kvm updates the version number for the guest paravirt clock structure by
incrementing the version of its private copy. It does not read the guest
version, so will write version = 2 in the first update for every new VM,
including after restoring a saved state. If guest state is saved during
reading the clock, it could read and accept struct fields and guest TSC
from two different updates. This changes the code to increment the guest
version and write it back.

Signed-off-by: Owen Hofmann 
---
 arch/x86/kvm/x86.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0033df3..c25c9d8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1637,16 +1637,16 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
vcpu->hv_clock.system_time = kernel_ns + v->kvm->arch.kvmclock_offset;
vcpu->last_guest_tsc = tsc_timestamp;
 
+   if (unlikely(kvm_read_guest_cached(v->kvm, &vcpu->pv_time,
+   &guest_hv_clock, sizeof(guest_hv_clock
+   return 0;
+
/*
 * 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.
 */
-   vcpu->hv_clock.version += 2;
-
-   if (unlikely(kvm_read_guest_cached(v->kvm, &vcpu->pv_time,
-   &guest_hv_clock, sizeof(guest_hv_clock
-   return 0;
+   vcpu->hv_clock.version = guest_hv_clock.version + 2;
 
/* retain PVCLOCK_GUEST_STOPPED if set in guest copy */
pvclock_flags = (guest_hv_clock.flags & PVCLOCK_GUEST_STOPPED);
-- 
2.1.0.rc2.206.gedb03e5

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html