[PATCH] x86: fix delta calculation in TSC deadline timer emulation

[PATCH] x86: fix delta calculation in TSC deadline timer emulation

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

In the virtual LAPIC, correct the delta calculation when emulating the
TSC deadline timer.

Without this fix, XenServer (which is based on Xen 4.1) does not work
when running as an HVM guest.  dom0 fails to boot because its timer
interrupts are very delayed (by several minutes in some cases).

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
---
A 4.1.x candidate?
---
 xen/arch/x86/hvm/vlapic.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 8401756..1aa2810 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -913,9 +913,8 @@ void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value)
     guest_time = hvm_get_guest_time(v);
     if ( value > guest_tsc )
     {
-        uint64_t delta = value - v->arch.hvm_vcpu.cache_tsc_offset;
-        delta = gtsc_to_gtime(v->domain, delta);
-        delta = max_t(s64, delta - guest_time, 0);
+        uint64_t delta = gtsc_to_gtime(v->domain, value - guest_tsc);
+        delta = max_t(s64, delta, 0);
 
         HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", delta);
 
-- 
1.7.2.5

Re: [PATCH] x86: fix delta calculation in TSC deadline timer emulation

[Jan Beulich]

>>> On 10.04.12 at 19:08, David Vrabel <david.vrabel@citrix.com> wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> In the virtual LAPIC, correct the delta calculation when emulating the
> TSC deadline timer.
> 
> Without this fix, XenServer (which is based on Xen 4.1) does not work
> when running as an HVM guest.  dom0 fails to boot because its timer
> interrupts are very delayed (by several minutes in some cases).

Looks generally fine to me, but I'd like you to clean up a little more:
guest_time is now not needed here anymore, and really should also
not matter in this function at all (we're only dealing with TSC values).
Hence I'd like to ask that the variable, its initialization, and its sole
remaining use (passed to the HVM_DBG_LOG() at the end of the
function) be deleted as part of the patch. If you agree, feel free
to add my ack when you resend.

Jan

> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> ---
> A 4.1.x candidate?
> ---
>  xen/arch/x86/hvm/vlapic.c |    5 ++---
>  1 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> index 8401756..1aa2810 100644
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -913,9 +913,8 @@ void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t 
> value)
>      guest_time = hvm_get_guest_time(v);
>      if ( value > guest_tsc )
>      {
> -        uint64_t delta = value - v->arch.hvm_vcpu.cache_tsc_offset;
> -        delta = gtsc_to_gtime(v->domain, delta);
> -        delta = max_t(s64, delta - guest_time, 0);
> +        uint64_t delta = gtsc_to_gtime(v->domain, value - guest_tsc);
> +        delta = max_t(s64, delta, 0);
>  
>          HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", 
> delta);
>  
> -- 
> 1.7.2.5

[PATCHv2] x86: fix delta calculation in TSC deadline timer emulation

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

In the virtual LAPIC, correct the delta calculation when emulating the
TSC deadline timer.

Without this fix, XenServer (which is based on Xen 4.1) does not work
when running as an HVM guest.  dom0 fails to boot because its timer
interrupts are very delayed (by several minutes in some cases).

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
---
A 4.1.x candidate?

Changes since v1:
- remove unused guest_time variable
---
 xen/arch/x86/hvm/vlapic.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 8401756..abdb556 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -898,7 +898,6 @@ uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
 void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value)
 {
     uint64_t guest_tsc;
-    uint64_t guest_time;
     struct vcpu *v = vlapic_vcpu(vlapic);
 
     /* may need to exclude some other conditions like vlapic->hw.disabled */
@@ -910,12 +909,10 @@ void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value)
     
     /* new_value = 0, >0 && <= now, > now */
     guest_tsc = hvm_get_guest_tsc(v);
-    guest_time = hvm_get_guest_time(v);
     if ( value > guest_tsc )
     {
-        uint64_t delta = value - v->arch.hvm_vcpu.cache_tsc_offset;
-        delta = gtsc_to_gtime(v->domain, delta);
-        delta = max_t(s64, delta - guest_time, 0);
+        uint64_t delta = gtsc_to_gtime(v->domain, value - guest_tsc);
+        delta = max_t(s64, delta, 0);
 
         HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", delta);
 
@@ -949,9 +946,8 @@ void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value)
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER,
                 "tdt_msr[0x%016"PRIx64"],"
-                " gtsc[0x%016"PRIx64"],"
-                " gtime[0x%016"PRIx64"]",
-                vlapic->hw.tdt_msr, guest_tsc, guest_time);
+                " gtsc[0x%016"PRIx64"]",
+                vlapic->hw.tdt_msr, guest_tsc);
 }
 
 static int __vlapic_accept_pic_intr(struct vcpu *v)
-- 
1.7.2.5

Re: [PATCHv2] x86: fix delta calculation in TSC deadline timer emulation

[Jan Beulich]

>>> On 11.04.12 at 12:31, David Vrabel <david.vrabel@citrix.com> wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> In the virtual LAPIC, correct the delta calculation when emulating the
> TSC deadline timer.
> 
> Without this fix, XenServer (which is based on Xen 4.1) does not work
> when running as an HVM guest.  dom0 fails to boot because its timer
> interrupts are very delayed (by several minutes in some cases).
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

> ---
> A 4.1.x candidate?

Presumably.

> Changes since v1:
> - remove unused guest_time variable
> ---
>  xen/arch/x86/hvm/vlapic.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> index 8401756..abdb556 100644
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -898,7 +898,6 @@ uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
>  void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value)
>  {
>      uint64_t guest_tsc;
> -    uint64_t guest_time;
>      struct vcpu *v = vlapic_vcpu(vlapic);
>  
>      /* may need to exclude some other conditions like vlapic->hw.disabled */
> @@ -910,12 +909,10 @@ void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t 
> value)
>      
>      /* new_value = 0, >0 && <= now, > now */
>      guest_tsc = hvm_get_guest_tsc(v);
> -    guest_time = hvm_get_guest_time(v);
>      if ( value > guest_tsc )
>      {
> -        uint64_t delta = value - v->arch.hvm_vcpu.cache_tsc_offset;
> -        delta = gtsc_to_gtime(v->domain, delta);
> -        delta = max_t(s64, delta - guest_time, 0);
> +        uint64_t delta = gtsc_to_gtime(v->domain, value - guest_tsc);
> +        delta = max_t(s64, delta, 0);
>  
>          HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", 
> delta);
>  
> @@ -949,9 +946,8 @@ void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t 
> value)
>  
>      HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER,
>                  "tdt_msr[0x%016"PRIx64"],"
> -                " gtsc[0x%016"PRIx64"],"
> -                " gtime[0x%016"PRIx64"]",
> -                vlapic->hw.tdt_msr, guest_tsc, guest_time);
> +                " gtsc[0x%016"PRIx64"]",
> +                vlapic->hw.tdt_msr, guest_tsc);
>  }
>  
>  static int __vlapic_accept_pic_intr(struct vcpu *v)
> -- 
> 1.7.2.5