All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
@ 2010-12-14  3:27 Wei, Gang
  2010-12-14  7:35 ` Keir Fraser
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Wei, Gang @ 2010-12-14  3:27 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com; +Cc: Keir Fraser, Wei, Gang

[-- Attachment #1: Type: text/plain, Size: 8488 bytes --]

vtdt: Modify vlapic code to add vtdt support

Accesses to MSR_IA32_TSC_DEADLINE are trapped, with value stored in a new field vlapic->hw.tdt_msr. vlapic->pt is reused in one shot mode for vtdt to trigger expire events.

For details, please refer to the Intel Architectures Software Developer's Manual 3A, 10.5.4.1 TSC-Deadline Mode.

Signed-off-by: Wei Gang <gang.wei@intel.com>

diff -r d042ca4c6b68 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Thu Dec 09 22:33:02 2010 +0800
+++ b/xen/arch/x86/hvm/hvm.c	Thu Dec 09 22:33:06 2010 +0800
@@ -2205,6 +2205,10 @@ int hvm_msr_read_intercept(unsigned int 
         *msr_content = vcpu_vlapic(v)->hw.apic_base_msr;
         break;
 
+    case MSR_IA32_TSC_DEADLINE:
+        *msr_content = vlapic_tdt_msr_get(vcpu_vlapic(v));
+        break;
+
     case MSR_IA32_CR_PAT:
         *msr_content = v->arch.hvm_vcpu.pat_cr;
         break;
@@ -2310,6 +2314,10 @@ int hvm_msr_write_intercept(unsigned int
 
     case MSR_IA32_APICBASE:
         vlapic_msr_set(vcpu_vlapic(v), msr_content);
+        break;
+
+    case MSR_IA32_TSC_DEADLINE:
+        vlapic_tdt_msr_set(vcpu_vlapic(v), msr_content);
         break;
 
     case MSR_IA32_CR_PAT:
diff -r d042ca4c6b68 xen/arch/x86/hvm/vlapic.c
--- a/xen/arch/x86/hvm/vlapic.c	Thu Dec 09 22:33:02 2010 +0800
+++ b/xen/arch/x86/hvm/vlapic.c	Thu Dec 09 22:33:06 2010 +0800
@@ -56,7 +56,7 @@ static unsigned int vlapic_lvt_mask[VLAP
 static unsigned int vlapic_lvt_mask[VLAPIC_LVT_NUM] =
 {
      /* LVTT */
-     LVT_MASK | APIC_LVT_TIMER_PERIODIC,
+     LVT_MASK | APIC_TIMER_MODE_MASK,
      /* LVTTHMR */
      LVT_MASK | APIC_MODE_MASK,
      /* LVTPC */
@@ -79,7 +79,16 @@ static unsigned int vlapic_lvt_mask[VLAP
     (vlapic_get_reg(vlapic, lvt_type) & APIC_MODE_MASK)
 
 #define vlapic_lvtt_period(vlapic)                              \
-    (vlapic_get_reg(vlapic, APIC_LVTT) & APIC_LVT_TIMER_PERIODIC)
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_PERIODIC)
+
+#define vlapic_lvtt_oneshot(vlapic)                             \
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_ONESHOT)
+
+#define vlapic_lvtt_tdt(vlapic)                                 \
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_TSC_DEADLINE)
 
 
 /*
@@ -464,9 +473,20 @@ static void vlapic_read_aligned(
         break;
 
     case APIC_TMCCT: /* Timer CCR */
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+        {
+            *result = 0;
+            break;
+        }
         *result = vlapic_get_tmcct(vlapic);
         break;
 
+    case APIC_TMICT: /* Timer ICR */
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+        {
+            *result = 0;
+            break;
+        }
     default:
         *result = vlapic_get_reg(vlapic, offset);
         break;
@@ -533,6 +553,12 @@ static void vlapic_pt_cb(struct vcpu *v,
     *(s_time_t *)data = hvm_get_guest_time(v);
 }
 
+static void vlapic_tdt_pt_cb(struct vcpu *v, void *data)
+{
+    *(s_time_t *)data = hvm_get_guest_time(v);
+    vcpu_vlapic(v)->hw.tdt_msr = 0;
+}
+
 static int vlapic_write(struct vcpu *v, unsigned long address,
                         unsigned long len, unsigned long val)
 {
@@ -643,7 +669,11 @@ static int vlapic_write(struct vcpu *v, 
         break;
 
     case APIC_LVTT:         /* LVT Timer Reg */
+        destroy_periodic_time(&vlapic->pt);
         vlapic->pt.irq = val & APIC_VECTOR_MASK;
+        vlapic_set_reg(vlapic, APIC_TMICT, 0);
+        vlapic_set_reg(vlapic, APIC_TMCCT, 0);
+        vlapic->hw.tdt_msr = 0;
     case APIC_LVTTHMR:      /* LVT Thermal Monitor */
     case APIC_LVTPC:        /* LVT Performance Counter */
     case APIC_LVT0:         /* LVT LINT0 Reg */
@@ -666,6 +696,9 @@ static int vlapic_write(struct vcpu *v, 
     {
         uint64_t period;
 
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+            break;
+
         vlapic_set_reg(vlapic, APIC_TMICT, val);
         if ( val == 0 )
         {
@@ -746,6 +779,73 @@ void vlapic_msr_set(struct vlapic *vlapi
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                 "apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
+}
+
+uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
+{
+    if ( !vlapic_lvtt_tdt(vlapic) )
+        return 0;
+
+    return vlapic->hw.tdt_msr;
+}
+
+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 */
+    if ( !vlapic_lvtt_tdt(vlapic) )
+    {
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "ignore tsc deadline msr write");
+        return;
+    }
+    
+    /* 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);
+
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", delta);
+
+        vlapic->hw.tdt_msr = value;
+        /* .... reprogram tdt timer */
+        create_periodic_time(v, &vlapic->pt, delta, 0,
+                             vlapic->pt.irq, vlapic_tdt_pt_cb,
+                             &vlapic->timer_last_update);
+        vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
+    }
+    else
+    {
+        vlapic->hw.tdt_msr = 0;
+
+        /* trigger a timer event if needed */
+        if ( value > 0 )
+        {
+            create_periodic_time(v, &vlapic->pt, 0, 0,
+                                 vlapic->pt.irq, vlapic_tdt_pt_cb,
+                                 &vlapic->timer_last_update);
+            vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
+        }
+        else
+        {
+            /* .... stop tdt timer */
+            destroy_periodic_time(&vlapic->pt);
+        }
+
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "value[0x%016"PRIx64"]", 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);
 }
 
 static int __vlapic_accept_pic_intr(struct vcpu *v)
@@ -860,6 +960,17 @@ void vlapic_reset(struct vlapic *vlapic)
     destroy_periodic_time(&vlapic->pt);
 }
 
+static void lapic_tdt_rearm(struct vlapic *s)
+{
+    uint64_t tdt_msr = vlapic_tdt_msr_get(s);
+
+    s->pt.irq = vlapic_get_reg(s, APIC_LVTT) & APIC_VECTOR_MASK;
+    if ( tdt_msr == 0)
+       return;
+
+    vlapic_tdt_msr_set(s, tdt_msr);
+}
+
 /* rearm the actimer if needed, after a HVM restore */
 static void lapic_rearm(struct vlapic *s)
 {
@@ -953,7 +1064,10 @@ static int lapic_load_regs(struct domain
         return -EINVAL;
 
     vlapic_adjust_i8259_target(d);
-    lapic_rearm(s);
+    if ( vlapic_lvtt_tdt(s) )
+        lapic_tdt_rearm(s);
+    else
+        lapic_rearm(s);
     return 0;
 }
 
diff -r d042ca4c6b68 xen/include/asm-x86/hvm/vlapic.h
--- a/xen/include/asm-x86/hvm/vlapic.h	Thu Dec 09 22:33:02 2010 +0800
+++ b/xen/include/asm-x86/hvm/vlapic.h	Thu Dec 09 22:33:06 2010 +0800
@@ -90,6 +90,8 @@ void vlapic_reset(struct vlapic *vlapic)
 void vlapic_reset(struct vlapic *vlapic);
 
 void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
+void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
+uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
 
 int vlapic_accept_pic_intr(struct vcpu *v);
 
diff -r d042ca4c6b68 xen/include/public/arch-x86/hvm/save.h
--- a/xen/include/public/arch-x86/hvm/save.h	Thu Dec 09 22:33:02 2010 +0800
+++ b/xen/include/public/arch-x86/hvm/save.h	Thu Dec 09 22:33:06 2010 +0800
@@ -265,6 +265,7 @@ struct hvm_hw_lapic {
     uint64_t             apic_base_msr;
     uint32_t             disabled; /* VLAPIC_xx_DISABLED */
     uint32_t             timer_divisor;
+    uint64_t             tdt_msr;
 };
 
 DECLARE_HVM_SAVE_TYPE(LAPIC, 5, struct hvm_hw_lapic);

[-- Attachment #2: 4-vtdt.patch --]
[-- Type: application/octet-stream, Size: 8243 bytes --]

vtdt: Modify vlapic code to add vtdt support.

Accesses to MSR_IA32_TSC_DEADLINE are trapped, with value stored in a new field vlapic->hw.tdt_msr. vlapic->pt is reused in one shot mode for vtdt to trigger expire events.

For details, please refer to the Intel Architectures Software Developer's Manual 3A, 10.5.4.1 TSC-Deadline Mode.

Signed-off-by: Wei Gang <gang.wei@intel.com>

diff -r d042ca4c6b68 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Thu Dec 09 22:33:02 2010 +0800
+++ b/xen/arch/x86/hvm/hvm.c	Thu Dec 09 22:33:06 2010 +0800
@@ -2205,6 +2205,10 @@ int hvm_msr_read_intercept(unsigned int 
         *msr_content = vcpu_vlapic(v)->hw.apic_base_msr;
         break;
 
+    case MSR_IA32_TSC_DEADLINE:
+        *msr_content = vlapic_tdt_msr_get(vcpu_vlapic(v));
+        break;
+
     case MSR_IA32_CR_PAT:
         *msr_content = v->arch.hvm_vcpu.pat_cr;
         break;
@@ -2310,6 +2314,10 @@ int hvm_msr_write_intercept(unsigned int
 
     case MSR_IA32_APICBASE:
         vlapic_msr_set(vcpu_vlapic(v), msr_content);
+        break;
+
+    case MSR_IA32_TSC_DEADLINE:
+        vlapic_tdt_msr_set(vcpu_vlapic(v), msr_content);
         break;
 
     case MSR_IA32_CR_PAT:
diff -r d042ca4c6b68 xen/arch/x86/hvm/vlapic.c
--- a/xen/arch/x86/hvm/vlapic.c	Thu Dec 09 22:33:02 2010 +0800
+++ b/xen/arch/x86/hvm/vlapic.c	Thu Dec 09 22:33:06 2010 +0800
@@ -56,7 +56,7 @@ static unsigned int vlapic_lvt_mask[VLAP
 static unsigned int vlapic_lvt_mask[VLAPIC_LVT_NUM] =
 {
      /* LVTT */
-     LVT_MASK | APIC_LVT_TIMER_PERIODIC,
+     LVT_MASK | APIC_TIMER_MODE_MASK,
      /* LVTTHMR */
      LVT_MASK | APIC_MODE_MASK,
      /* LVTPC */
@@ -79,7 +79,16 @@ static unsigned int vlapic_lvt_mask[VLAP
     (vlapic_get_reg(vlapic, lvt_type) & APIC_MODE_MASK)
 
 #define vlapic_lvtt_period(vlapic)                              \
-    (vlapic_get_reg(vlapic, APIC_LVTT) & APIC_LVT_TIMER_PERIODIC)
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_PERIODIC)
+
+#define vlapic_lvtt_oneshot(vlapic)                             \
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_ONESHOT)
+
+#define vlapic_lvtt_tdt(vlapic)                                 \
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_TSC_DEADLINE)
 
 
 /*
@@ -464,9 +473,20 @@ static void vlapic_read_aligned(
         break;
 
     case APIC_TMCCT: /* Timer CCR */
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+        {
+            *result = 0;
+            break;
+        }
         *result = vlapic_get_tmcct(vlapic);
         break;
 
+    case APIC_TMICT: /* Timer ICR */
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+        {
+            *result = 0;
+            break;
+        }
     default:
         *result = vlapic_get_reg(vlapic, offset);
         break;
@@ -533,6 +553,12 @@ static void vlapic_pt_cb(struct vcpu *v,
     *(s_time_t *)data = hvm_get_guest_time(v);
 }
 
+static void vlapic_tdt_pt_cb(struct vcpu *v, void *data)
+{
+    *(s_time_t *)data = hvm_get_guest_time(v);
+    vcpu_vlapic(v)->hw.tdt_msr = 0;
+}
+
 static int vlapic_write(struct vcpu *v, unsigned long address,
                         unsigned long len, unsigned long val)
 {
@@ -643,7 +669,11 @@ static int vlapic_write(struct vcpu *v, 
         break;
 
     case APIC_LVTT:         /* LVT Timer Reg */
+        destroy_periodic_time(&vlapic->pt);
         vlapic->pt.irq = val & APIC_VECTOR_MASK;
+        vlapic_set_reg(vlapic, APIC_TMICT, 0);
+        vlapic_set_reg(vlapic, APIC_TMCCT, 0);
+        vlapic->hw.tdt_msr = 0;
     case APIC_LVTTHMR:      /* LVT Thermal Monitor */
     case APIC_LVTPC:        /* LVT Performance Counter */
     case APIC_LVT0:         /* LVT LINT0 Reg */
@@ -666,6 +696,9 @@ static int vlapic_write(struct vcpu *v, 
     {
         uint64_t period;
 
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+            break;
+
         vlapic_set_reg(vlapic, APIC_TMICT, val);
         if ( val == 0 )
         {
@@ -746,6 +779,73 @@ void vlapic_msr_set(struct vlapic *vlapi
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                 "apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
+}
+
+uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
+{
+    if ( !vlapic_lvtt_tdt(vlapic) )
+        return 0;
+
+    return vlapic->hw.tdt_msr;
+}
+
+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 */
+    if ( !vlapic_lvtt_tdt(vlapic) )
+    {
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "ignore tsc deadline msr write");
+        return;
+    }
+    
+    /* 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);
+
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", delta);
+
+        vlapic->hw.tdt_msr = value;
+        /* .... reprogram tdt timer */
+        create_periodic_time(v, &vlapic->pt, delta, 0,
+                             vlapic->pt.irq, vlapic_tdt_pt_cb,
+                             &vlapic->timer_last_update);
+        vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
+    }
+    else
+    {
+        vlapic->hw.tdt_msr = 0;
+
+        /* trigger a timer event if needed */
+        if ( value > 0 )
+        {
+            create_periodic_time(v, &vlapic->pt, 0, 0,
+                                 vlapic->pt.irq, vlapic_tdt_pt_cb,
+                                 &vlapic->timer_last_update);
+            vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
+        }
+        else
+        {
+            /* .... stop tdt timer */
+            destroy_periodic_time(&vlapic->pt);
+        }
+
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "value[0x%016"PRIx64"]", 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);
 }
 
 static int __vlapic_accept_pic_intr(struct vcpu *v)
@@ -860,6 +960,17 @@ void vlapic_reset(struct vlapic *vlapic)
     destroy_periodic_time(&vlapic->pt);
 }
 
+static void lapic_tdt_rearm(struct vlapic *s)
+{
+    uint64_t tdt_msr = vlapic_tdt_msr_get(s);
+
+    s->pt.irq = vlapic_get_reg(s, APIC_LVTT) & APIC_VECTOR_MASK;
+    if ( tdt_msr == 0)
+       return;
+
+    vlapic_tdt_msr_set(s, tdt_msr);
+}
+
 /* rearm the actimer if needed, after a HVM restore */
 static void lapic_rearm(struct vlapic *s)
 {
@@ -953,7 +1064,10 @@ static int lapic_load_regs(struct domain
         return -EINVAL;
 
     vlapic_adjust_i8259_target(d);
-    lapic_rearm(s);
+    if ( vlapic_lvtt_tdt(s) )
+        lapic_tdt_rearm(s);
+    else
+        lapic_rearm(s);
     return 0;
 }
 
diff -r d042ca4c6b68 xen/include/asm-x86/hvm/vlapic.h
--- a/xen/include/asm-x86/hvm/vlapic.h	Thu Dec 09 22:33:02 2010 +0800
+++ b/xen/include/asm-x86/hvm/vlapic.h	Thu Dec 09 22:33:06 2010 +0800
@@ -90,6 +90,8 @@ void vlapic_reset(struct vlapic *vlapic)
 void vlapic_reset(struct vlapic *vlapic);
 
 void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
+void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
+uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
 
 int vlapic_accept_pic_intr(struct vcpu *v);
 
diff -r d042ca4c6b68 xen/include/public/arch-x86/hvm/save.h
--- a/xen/include/public/arch-x86/hvm/save.h	Thu Dec 09 22:33:02 2010 +0800
+++ b/xen/include/public/arch-x86/hvm/save.h	Thu Dec 09 22:33:06 2010 +0800
@@ -265,6 +265,7 @@ struct hvm_hw_lapic {
     uint64_t             apic_base_msr;
     uint32_t             disabled; /* VLAPIC_xx_DISABLED */
     uint32_t             timer_divisor;
+    uint64_t             tdt_msr;
 };
 
 DECLARE_HVM_SAVE_TYPE(LAPIC, 5, struct hvm_hw_lapic);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  3:27 [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support Wei, Gang
@ 2010-12-14  7:35 ` Keir Fraser
  2010-12-14  8:22   ` Wei, Gang
  2010-12-14  9:21 ` Keir Fraser
  2010-12-14 10:29 ` Wei, Gang
  2 siblings, 1 reply; 13+ messages in thread
From: Keir Fraser @ 2010-12-14  7:35 UTC (permalink / raw)
  To: Wei, Gang, xen-devel@lists.xensource.com; +Cc: Tim Deegan

On 14/12/2010 03:27, "Wei, Gang" <gang.wei@intel.com> wrote:

> diff -r d042ca4c6b68 xen/include/public/arch-x86/hvm/save.h
> --- a/xen/include/public/arch-x86/hvm/save.h Thu Dec 09 22:33:02 2010 +0800
> +++ b/xen/include/public/arch-x86/hvm/save.h Thu Dec 09 22:33:06 2010 +0800
> @@ -265,6 +265,7 @@ struct hvm_hw_lapic {
>      uint64_t             apic_base_msr;
>      uint32_t             disabled; /* VLAPIC_xx_DISABLED */
>      uint32_t             timer_divisor;
> +    uint64_t             tdt_msr;
>  };

Is this backward compatible with old HVM save images?

We ought to support extending the size of existing HVM save structures, but
I'm not sure that we do. Tim?

 -- Keir

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  7:35 ` Keir Fraser
@ 2010-12-14  8:22   ` Wei, Gang
  2010-12-14  8:48     ` Keir Fraser
  0 siblings, 1 reply; 13+ messages in thread
From: Wei, Gang @ 2010-12-14  8:22 UTC (permalink / raw)
  To: Keir Fraser, xen-devel@lists.xensource.com; +Cc: Tim Deegan, Wei, Gang

Keir Fraser wrote on 2010-12-14:
> On 14/12/2010 03:27, "Wei, Gang" <gang.wei@intel.com> wrote:
> 
>> diff -r d042ca4c6b68 xen/include/public/arch-x86/hvm/save.h
>> --- a/xen/include/public/arch-x86/hvm/save.h Thu Dec 09 22:33:02
>> 2010
>> +0800
>> +++ b/xen/include/public/arch-x86/hvm/save.h Thu Dec 09 22:33:06
>> +++ 2010
>> +++ +0800
>> @@ -265,6 +265,7 @@ struct hvm_hw_lapic {
>>      uint64_t             apic_base_msr;
>>      uint32_t             disabled; /* VLAPIC_xx_DISABLED */
>>      uint32_t             timer_divisor;
>> +    uint64_t             tdt_msr;
>>  };
> 
> Is this backward compatible with old HVM save images?

I am not sure about this. If it isn't, would you accept to simply add another data trunk for TDT msr?

Jimmy

> 
> We ought to support extending the size of existing HVM save
> structures, but I'm not sure that we do. Tim?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  8:22   ` Wei, Gang
@ 2010-12-14  8:48     ` Keir Fraser
  2010-12-14  9:13       ` Wei, Gang
  0 siblings, 1 reply; 13+ messages in thread
From: Keir Fraser @ 2010-12-14  8:48 UTC (permalink / raw)
  To: Wei, Gang, xen-devel@lists.xensource.com; +Cc: Tim Deegan

On 14/12/2010 08:22, "Wei, Gang" <gang.wei@intel.com> wrote:

>>> @@ -265,6 +265,7 @@ struct hvm_hw_lapic {
>>>      uint64_t             apic_base_msr;
>>>      uint32_t             disabled; /* VLAPIC_xx_DISABLED */
>>>      uint32_t             timer_divisor;
>>> +    uint64_t             tdt_msr;
>>>  };
>> 
>> Is this backward compatible with old HVM save images?
> 
> I am not sure about this. If it isn't, would you accept to simply add another
> data trunk for TDT msr?

That's Tim's call. I would personally prefer for Xen to accept truncated
chunks, and extend them with sensible 'old save image' defaults, such as
all-zeroes. That would be a generic solution to this case which will be
reusable in future, and avoid needlessly creating extra chunk types just for
backward compatibility reasons.

 -- Keir

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  8:48     ` Keir Fraser
@ 2010-12-14  9:13       ` Wei, Gang
  2010-12-14  9:59         ` Keir Fraser
  0 siblings, 1 reply; 13+ messages in thread
From: Wei, Gang @ 2010-12-14  9:13 UTC (permalink / raw)
  To: Keir Fraser, xen-devel@lists.xensource.com; +Cc: Tim Deegan, Wei, Gang

Keir Fraser wrote on 2010-12-14:
>>>> @@ -265,6 +265,7 @@ struct hvm_hw_lapic {
>>>>      uint64_t             apic_base_msr;
>>>>      uint32_t             disabled; /* VLAPIC_xx_DISABLED */
>>>>      uint32_t             timer_divisor;
>>>> +    uint64_t             tdt_msr;
>>>>  };
>>> 
>>> Is this backward compatible with old HVM save images?
>> 
>> I am not sure about this. If it isn't, would you accept to simply
>> add another data trunk for TDT msr?
> 
> That's Tim's call. I would personally prefer for Xen to accept
> truncated chunks, and extend them with sensible 'old save image' defaults, such as all-zeroes.
> That would be a generic solution to this case which will be reusable
> in future, and avoid needlessly creating extra chunk types just for
> backward compatibility reasons.

Ok. Let's wait for Tim to answer the call.

I just found some code in xen/hvm/save.h: _hvm_check_entry()
    if ( type != d->typecode || len != d->length )
    {
        gdprintk(XENLOG_WARNING, 
                 "HVM restore mismatch: expected type %u length %u, "
                 "saw type %u length %u\n", type, len, d->typecode, d->length);
        return -1;
    }

So I am assuming it would not be backward compatible with old HVM save images.

Jimmy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  3:27 [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support Wei, Gang
  2010-12-14  7:35 ` Keir Fraser
@ 2010-12-14  9:21 ` Keir Fraser
  2010-12-14  9:30   ` Wei, Gang
  2010-12-14 10:29 ` Wei, Gang
  2 siblings, 1 reply; 13+ messages in thread
From: Keir Fraser @ 2010-12-14  9:21 UTC (permalink / raw)
  To: Wei, Gang, xen-devel@lists.xensource.com

On 14/12/2010 03:27, "Wei, Gang" <gang.wei@intel.com> wrote:

> @@ -643,7 +669,11 @@ static int vlapic_write(struct vcpu *v,
>          break;
>  
>      case APIC_LVTT:         /* LVT Timer Reg */
> +        destroy_periodic_time(&vlapic->pt);
>          vlapic->pt.irq = val & APIC_VECTOR_MASK;
> +        vlapic_set_reg(vlapic, APIC_TMICT, 0);
> +        vlapic_set_reg(vlapic, APIC_TMCCT, 0);
> +        vlapic->hw.tdt_msr = 0;

Writing any value to LVTT zaps TMICT,TMCCT,MSR_TDT? That seems pretty
unlikely to me! This obviously has effects on behaviour outside TDT
emulation as it affects TMICT/TMCCT emulation. Looks dangerous, as well as
wrong.

Also I now notice that this patch is not against tip of xen-unstable, as
these changes should be to new function vlapic_reg_write().

 -- Keir

>      case APIC_LVTTHMR:      /* LVT Thermal Monitor */
>      case APIC_LVTPC:        /* LVT Performance Counter */
>      case APIC_LVT0:         /* LVT LINT0 Reg */
> @@ -666,6 +696,9 @@ static int vlapic_write(struct vcpu *v,
>      {
>          uint64_t period;
>  
> +        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
> +            break;
> +
>          vlapic_set_reg(vlapic, APIC_TMICT, val);
>          if ( val == 0 )
>          {
> @@ -746,6 +779,73 @@ void vlapic_msr_set(struct vlapic *vlapi
>  
>      HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
>                  "apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
> +}
> +
> +uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
> +{
> +    if ( !vlapic_lvtt_tdt(vlapic) )
> +        return 0;
> +
> +    return vlapic->hw.tdt_msr;
> +}
> +
> +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 */
> +    if ( !vlapic_lvtt_tdt(vlapic) )
> +    {
> +        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "ignore tsc deadline msr write");
> +        return;
> +    }
> +    
> +    /* 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);
> +
> +        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", delta);
> +
> +        vlapic->hw.tdt_msr = value;
> +        /* .... reprogram tdt timer */
> +        create_periodic_time(v, &vlapic->pt, delta, 0,
> +                             vlapic->pt.irq, vlapic_tdt_pt_cb,
> +                             &vlapic->timer_last_update);
> +        vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
> +    }
> +    else
> +    {
> +        vlapic->hw.tdt_msr = 0;
> +
> +        /* trigger a timer event if needed */
> +        if ( value > 0 )
> +        {
> +            create_periodic_time(v, &vlapic->pt, 0, 0,
> +                                 vlapic->pt.irq, vlapic_tdt_pt_cb,
> +                                 &vlapic->timer_last_update);
> +            vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
> +        }
> +        else
> +        {
> +            /* .... stop tdt timer */
> +            destroy_periodic_time(&vlapic->pt);
> +        }
> +
> +        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "value[0x%016"PRIx64"]", 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);
>  }
>  
>  static int __vlapic_accept_pic_intr(struct vcpu *v)
> @@ -860,6 +960,17 @@ void vlapic_reset(struct vlapic *vlapic)
>      destroy_periodic_time(&vlapic->pt);
>  }
>  
> +static void lapic_tdt_rearm(struct vlapic *s)
> +{
> +    uint64_t tdt_msr = vlapic_tdt_msr_get(s);
> +
> +    s->pt.irq = vlapic_get_reg(s, APIC_LVTT) & APIC_VECTOR_MASK;
> +    if ( tdt_msr == 0)
> +       return;
> +
> +    vlapic_tdt_msr_set(s, tdt_msr);
> +}
> +
>  /* rearm the actimer if needed, after a HVM restore */
>  static void lapic_rearm(struct vlapic *s)
>  {
> @@ -953,7 +1064,10 @@ static int lapic_load_regs(struct domain
>          return -EINVAL;
>  
>      vlapic_adjust_i8259_target(d);
> -    lapic_rearm(s);
> +    if ( vlapic_lvtt_tdt(s) )
> +        lapic_tdt_rearm(s);
> +    else
> +        lapic_rearm(s);
>      return 0;
>  }
>  
> diff -r d042ca4c6b68 xen/include/asm-x86/hvm/vlapic.h
> --- a/xen/include/asm-x86/hvm/vlapic.h Thu Dec 09 22:33:02 2010 +0800
> +++ b/xen/include/asm-x86/hvm/vlapic.h Thu Dec 09 22:33:06 2010 +0800
> @@ -90,6 +90,8 @@ void vlapic_reset(struct vlapic *vlapic)
>  void vlapic_reset(struct vlapic *vlapic);
>  
>  void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
> +void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
> +uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
>  
>  int vlapic_accept_pic_intr(struct vcpu *v);
>  
> diff -r d042ca4c6b68 xen/include/public/arch-x86/hvm/save.h
> --- a/xen/include/public/arch-x86/hvm/save.h Thu Dec 09 22:33:02 2010 +0800
> +++ b/xen/include/public/arch-x86/hvm/save.h Thu Dec 09 22:33:06 2010 +0800
> @@ -265,6 +265,7 @@ struct hvm_hw_lapic {
>      uint64_t             apic_base_msr;
>      uint32_t             disabled; /* VLAPIC_xx_DISABLED */
>      uint32_t             timer_divisor;
> +    uint64_t             tdt_msr;
>  };
>  
>  DECLARE_HVM_SAVE_TYPE(LAPIC, 5, struct hvm_hw_lapic);
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  9:21 ` Keir Fraser
@ 2010-12-14  9:30   ` Wei, Gang
  2010-12-14  9:56     ` Keir Fraser
  0 siblings, 1 reply; 13+ messages in thread
From: Wei, Gang @ 2010-12-14  9:30 UTC (permalink / raw)
  To: Keir Fraser, xen-devel@lists.xensource.com; +Cc: Wei, Gang

Keir Fraser wrote on 2010-12-14:
> On 14/12/2010 03:27, "Wei, Gang" <gang.wei@intel.com> wrote:
> 
>> @@ -643,7 +669,11 @@ static int vlapic_write(struct vcpu *v,
>>          break;
>>      case APIC_LVTT:         /* LVT Timer Reg */
>> +        destroy_periodic_time(&vlapic->pt);
>>          vlapic->pt.irq = val & APIC_VECTOR_MASK;
>> +        vlapic_set_reg(vlapic, APIC_TMICT, 0);
>> +        vlapic_set_reg(vlapic, APIC_TMCCT, 0);
>> +        vlapic->hw.tdt_msr = 0;
> 
> Writing any value to LVTT zaps TMICT,TMCCT,MSR_TDT? That seems pretty
> unlikely to me! This obviously has effects on behaviour outside TDT
> emulation as it affects TMICT/TMCCT emulation. Looks dangerous, as well as wrong.

It should be better to do the zaps only while the mode bits of LVTT changes.

> 
> Also I now notice that this patch is not against tip of xen-unstable,
> as these changes should be to new function vlapic_reg_write().

Sorry for that. I had rebased it to the tip, but happened to send out the old one. I will resend this patch.

Jimmy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  9:30   ` Wei, Gang
@ 2010-12-14  9:56     ` Keir Fraser
  2010-12-14 10:13       ` Wei, Gang
  0 siblings, 1 reply; 13+ messages in thread
From: Keir Fraser @ 2010-12-14  9:56 UTC (permalink / raw)
  To: Wei, Gang, xen-devel@lists.xensource.com

On 14/12/2010 09:30, "Wei, Gang" <gang.wei@intel.com> wrote:

>> Writing any value to LVTT zaps TMICT,TMCCT,MSR_TDT? That seems pretty
>> unlikely to me! This obviously has effects on behaviour outside TDT
>> emulation as it affects TMICT/TMCCT emulation. Looks dangerous, as well as
>> wrong.
> 
> It should be better to do the zaps only while the mode bits of LVTT changes.

What is real hardware behaviour here? I don't see any zappings documented.

 -- Keir

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  9:13       ` Wei, Gang
@ 2010-12-14  9:59         ` Keir Fraser
  2010-12-14 10:01           ` Keir Fraser
  0 siblings, 1 reply; 13+ messages in thread
From: Keir Fraser @ 2010-12-14  9:59 UTC (permalink / raw)
  To: Wei, Gang, xen-devel@lists.xensource.com; +Cc: Tim Deegan

On 14/12/2010 09:13, "Wei, Gang" <gang.wei@intel.com> wrote:

> Keir Fraser wrote on 2010-12-14:
>>>>> @@ -265,6 +265,7 @@ struct hvm_hw_lapic {
>>>>>      uint64_t             apic_base_msr;
>>>>>      uint32_t             disabled; /* VLAPIC_xx_DISABLED */
>>>>>      uint32_t             timer_divisor;
>>>>> +    uint64_t             tdt_msr;
>>>>>  };
>>>> 
>>>> Is this backward compatible with old HVM save images?
>>> 
>>> I am not sure about this. If it isn't, would you accept to simply
>>> add another data trunk for TDT msr?
>> 
>> That's Tim's call. I would personally prefer for Xen to accept
>> truncated chunks, and extend them with sensible 'old save image' defaults,
>> such as all-zeroes.
>> That would be a generic solution to this case which will be reusable
>> in future, and avoid needlessly creating extra chunk types just for
>> backward compatibility reasons.
> 
> Ok. Let's wait for Tim to answer the call.

Well I propose the attached patch, and then you can use
hvm_load_entry_zeroextend() in vlapic.c. I would split this patch into two
pieces if I apply it (code movement first, then the zeroextend logic).

It needs an Ack from Tim however.

 -- Keir

> I just found some code in xen/hvm/save.h: _hvm_check_entry()
>     if ( type != d->typecode || len != d->length )
>     {
>         gdprintk(XENLOG_WARNING,
>                  "HVM restore mismatch: expected type %u length %u, "
>                  "saw type %u length %u\n", type, len, d->typecode,
> d->length);
>         return -1;
>     }
> 
> So I am assuming it would not be backward compatible with old HVM save images.
> 
> Jimmy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  9:59         ` Keir Fraser
@ 2010-12-14 10:01           ` Keir Fraser
  2010-12-14 10:18             ` Tim Deegan
  0 siblings, 1 reply; 13+ messages in thread
From: Keir Fraser @ 2010-12-14 10:01 UTC (permalink / raw)
  To: Keir Fraser, Wei, Gang, xen-devel@lists.xensource.com; +Cc: Tim Deegan

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On 14/12/2010 09:59, "Keir Fraser" <keir@xen.org> wrote:

>>> That's Tim's call. I would personally prefer for Xen to accept
>>> truncated chunks, and extend them with sensible 'old save image' defaults,
>>> such as all-zeroes.
>>> That would be a generic solution to this case which will be reusable
>>> in future, and avoid needlessly creating extra chunk types just for
>>> backward compatibility reasons.
>> 
>> Ok. Let's wait for Tim to answer the call.
> 
> Well I propose the attached patch, and then you can use
> hvm_load_entry_zeroextend() in vlapic.c. I would split this patch into two
> pieces if I apply it (code movement first, then the zeroextend logic).
> 
> It needs an Ack from Tim however.

Forgot to attach the patch. Attached this time.

 -- Keir


[-- Attachment #2: 00-hvm-restore-zeroextend --]
[-- Type: application/octet-stream, Size: 8054 bytes --]

diff -r ab785e37499c xen/arch/x86/hvm/hpet.c
--- a/xen/arch/x86/hvm/hpet.c	Thu Dec 09 14:50:56 2010 +0100
+++ b/xen/arch/x86/hvm/hpet.c	Tue Dec 14 09:51:09 2010 +0000
@@ -546,7 +546,7 @@
     spin_lock(&hp->lock);
 
     /* Reload the HPET registers */
-    if ( _hvm_check_entry(h, HVM_SAVE_CODE(HPET), HVM_SAVE_LENGTH(HPET)) )
+    if ( _hvm_check_entry(h, HVM_SAVE_CODE(HPET), HVM_SAVE_LENGTH(HPET), 1) )
     {
         spin_unlock(&hp->lock);
         return -EINVAL;
diff -r ab785e37499c xen/common/hvm/save.c
--- a/xen/common/hvm/save.c	Thu Dec 09 14:50:56 2010 +0100
+++ b/xen/common/hvm/save.c	Tue Dec 14 09:51:09 2010 +0000
@@ -244,6 +244,69 @@
     /* Not reached */
 }
 
+int _hvm_init_entry(struct hvm_domain_context *h,
+                    uint16_t tc, uint16_t inst, uint32_t len)
+{
+    struct hvm_save_descriptor *d 
+        = (struct hvm_save_descriptor *)&h->data[h->cur];
+    if ( h->size - h->cur < len + sizeof (*d) )
+    {
+        gdprintk(XENLOG_WARNING,
+                 "HVM save: no room for %"PRIu32" + %u bytes "
+                 "for typecode %"PRIu16"\n",
+                 len, (unsigned) sizeof (*d), tc);
+        return -1;
+    }
+    d->typecode = tc;
+    d->instance = inst;
+    d->length = len;
+    h->cur += sizeof (*d);
+    return 0;
+}
+
+void _hvm_write_entry(struct hvm_domain_context *h,
+                      void *src, uint32_t src_len)
+{
+    memcpy(&h->data[h->cur], src, src_len);
+    h->cur += src_len;
+}
+
+int _hvm_check_entry(struct hvm_domain_context *h, 
+                     uint16_t type, uint32_t len, bool_t strict_length)
+{
+    struct hvm_save_descriptor *d 
+        = (struct hvm_save_descriptor *)&h->data[h->cur];
+    if ( len + sizeof (*d) > h->size - h->cur)
+    {
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore: not enough data left to read %u bytes "
+                 "for type %u\n", len, type);
+        return -1;
+    }    
+    if ( type != d->typecode || (len < d->length) ||
+         (strict_length && (len != d->length)) )
+    {
+        gdprintk(XENLOG_WARNING, 
+                 "HVM restore mismatch: expected type %u length %u, "
+                 "saw type %u length %u\n", type, len, d->typecode, d->length);
+        return -1;
+    }
+    h->cur += sizeof (*d);
+    return 0;
+}
+
+void _hvm_read_entry(struct hvm_domain_context *h,
+                     void *dest, uint32_t dest_len)
+{
+    struct hvm_save_descriptor *d 
+        = (struct hvm_save_descriptor *)&h->data[h->cur - sizeof(*d)];
+    BUG_ON(d->length > dest_len);
+    memcpy(dest, &h->data[h->cur], d->length);
+    if ( d->length < dest_len )
+        memset((char *)dest + d->length, 0, dest_len - d->length);
+    h->cur += d->length;
+}
+
 /*
  * Local variables:
  * mode: C
diff -r ab785e37499c xen/include/xen/hvm/save.h
--- a/xen/include/xen/hvm/save.h	Thu Dec 09 14:50:56 2010 +0100
+++ b/xen/include/xen/hvm/save.h	Tue Dec 14 09:51:09 2010 +0000
@@ -30,77 +30,50 @@
 } hvm_domain_context_t;
 
 /* Marshalling an entry: check space and fill in the header */
-static inline int _hvm_init_entry(struct hvm_domain_context *h,
-                                  uint16_t tc, uint16_t inst, uint32_t len)
-{
-    struct hvm_save_descriptor *d 
-        = (struct hvm_save_descriptor *)&h->data[h->cur];
-    if ( h->size - h->cur < len + sizeof (*d) )
-    {
-        gdprintk(XENLOG_WARNING,
-                 "HVM save: no room for %"PRIu32" + %u bytes "
-                 "for typecode %"PRIu16"\n",
-                 len, (unsigned) sizeof (*d), tc);
-        return -1;
-    }
-    d->typecode = tc;
-    d->instance = inst;
-    d->length = len;
-    h->cur += sizeof (*d);
-    return 0;
-}
+int _hvm_init_entry(struct hvm_domain_context *h,
+                    uint16_t tc, uint16_t inst, uint32_t len);
 
 /* Marshalling: copy the contents in a type-safe way */
-#define _hvm_write_entry(_x, _h, _src) do {                     \
-    *(HVM_SAVE_TYPE(_x) *)(&(_h)->data[(_h)->cur]) = *(_src);   \
-    (_h)->cur += HVM_SAVE_LENGTH(_x);                           \
-} while (0)
+void _hvm_write_entry(struct hvm_domain_context *h,
+                      void *src, uint32_t src_len);
 
 /* Marshalling: init and copy; evaluates to zero on success */
-#define hvm_save_entry(_x, _inst, _h, _src) ({          \
-    int r;                                              \
-    r = _hvm_init_entry((_h), HVM_SAVE_CODE(_x),        \
-                        (_inst), HVM_SAVE_LENGTH(_x));  \
-    if ( r == 0 )                                       \
-        _hvm_write_entry(_x, (_h), (_src));             \
+#define hvm_save_entry(_x, _inst, _h, _src) ({                  \
+    int r;                                                      \
+    r = _hvm_init_entry((_h), HVM_SAVE_CODE(_x),                \
+                        (_inst), HVM_SAVE_LENGTH(_x));          \
+    if ( r == 0 )                                               \
+        _hvm_write_entry((_h), (_src), HVM_SAVE_LENGTH(_x));    \
     r; })
 
 /* Unmarshalling: test an entry's size and typecode and record the instance */
-static inline int _hvm_check_entry(struct hvm_domain_context *h, 
-                                   uint16_t type, uint32_t len)
-{
-    struct hvm_save_descriptor *d 
-        = (struct hvm_save_descriptor *)&h->data[h->cur];
-    if ( len + sizeof (*d) > h->size - h->cur)
-    {
-        gdprintk(XENLOG_WARNING, 
-                 "HVM restore: not enough data left to read %u bytes "
-                 "for type %u\n", len, type);
-        return -1;
-    }    
-    if ( type != d->typecode || len != d->length )
-    {
-        gdprintk(XENLOG_WARNING, 
-                 "HVM restore mismatch: expected type %u length %u, "
-                 "saw type %u length %u\n", type, len, d->typecode, d->length);
-        return -1;
-    }
-    h->cur += sizeof (*d);
-    return 0;
-}
+int _hvm_check_entry(struct hvm_domain_context *h, 
+                     uint16_t type, uint32_t len, bool_t strict_length);
 
 /* Unmarshalling: copy the contents in a type-safe way */
-#define _hvm_read_entry(_x, _h, _dst) do {                      \
-    *(_dst) = *(HVM_SAVE_TYPE(_x) *) (&(_h)->data[(_h)->cur]);  \
-    (_h)->cur += HVM_SAVE_LENGTH(_x);                           \
-} while (0)
+void _hvm_read_entry(struct hvm_domain_context *h,
+                     void *dest, uint32_t dest_len);
 
-/* Unmarshalling: check, then copy. Evaluates to zero on success. */
-#define hvm_load_entry(_x, _h, _dst) ({                                 \
-    int r;                                                              \
-    r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), HVM_SAVE_LENGTH(_x)); \
-    if ( r == 0 )                                                       \
-        _hvm_read_entry(_x, (_h), (_dst));                              \
+/*
+ * Unmarshalling: check, then copy. Evaluates to zero on success. This load
+ * function requires the save entry to be the same size as the dest structure.
+ */
+#define hvm_load_entry(_x, _h, _dst) ({                                    \
+    int r;                                                                 \
+    r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), HVM_SAVE_LENGTH(_x), 1); \
+    if ( r == 0 )                                                          \
+        _hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH(_x));                \
+    r; })
+
+/*
+ * Unmarshalling: check, then copy. Evaluates to zero on success. This load
+ * function will zero-extend a short save entry to fill th edest structure.
+ */
+#define hvm_load_entry_zeroextend(_x, _h, _dst) ({                         \
+    int r;                                                                 \
+    r = _hvm_check_entry((_h), HVM_SAVE_CODE(_x), HVM_SAVE_LENGTH(_x), 0); \
+    if ( r == 0 )                                                          \
+        _hvm_read_entry((_h), (_dst), HVM_SAVE_LENGTH(_x));                \
     r; })
 
 /* Unmarshalling: what is the instance ID of the next entry? */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  9:56     ` Keir Fraser
@ 2010-12-14 10:13       ` Wei, Gang
  0 siblings, 0 replies; 13+ messages in thread
From: Wei, Gang @ 2010-12-14 10:13 UTC (permalink / raw)
  To: Keir Fraser, xen-devel@lists.xensource.com; +Cc: Wei, Gang

Keir Fraser wrote on 2010-12-14:
>>> Writing any value to LVTT zaps TMICT,TMCCT,MSR_TDT? That seems
>>> pretty unlikely to me! This obviously has effects on behaviour
>>> outside TDT emulation as it affects TMICT/TMCCT emulation. Looks
>>> dangerous, as well as wrong.
>> 
>> It should be better to do the zaps only while the mode bits of LVTT changes.
> 
> What is real hardware behaviour here? I don't see any zappings documented.

SDM said "A write to the LVT Timer Register that changes the timer mode
disarms the local APIC timer".

Jimmy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14 10:01           ` Keir Fraser
@ 2010-12-14 10:18             ` Tim Deegan
  0 siblings, 0 replies; 13+ messages in thread
From: Tim Deegan @ 2010-12-14 10:18 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel@lists.xensource.com, Wei, Gang

At 10:01 +0000 on 14 Dec (1292320899), Keir Fraser wrote:
> On 14/12/2010 09:59, "Keir Fraser" <keir@xen.org> wrote:
> 
> >>> That's Tim's call. I would personally prefer for Xen to accept
> >>> truncated chunks, and extend them with sensible 'old save image' defaults,
> >>> such as all-zeroes.
> >>> That would be a generic solution to this case which will be reusable
> >>> in future, and avoid needlessly creating extra chunk types just for
> >>> backward compatibility reasons.
> >> 
> >> Ok. Let's wait for Tim to answer the call.
> > 
> > Well I propose the attached patch, and then you can use
> > hvm_load_entry_zeroextend() in vlapic.c. I would split this patch into two
> > pieces if I apply it (code movement first, then the zeroextend logic).
> > 
> > It needs an Ack from Tim however.

Looks good. 

Acked-by: Tim Deegan <Tim.Deegan@citrix.com>

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
  2010-12-14  3:27 [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support Wei, Gang
  2010-12-14  7:35 ` Keir Fraser
  2010-12-14  9:21 ` Keir Fraser
@ 2010-12-14 10:29 ` Wei, Gang
  2 siblings, 0 replies; 13+ messages in thread
From: Wei, Gang @ 2010-12-14 10:29 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com; +Cc: Wei, Gang, Keir Fraser, Tim Deegan

[-- Attachment #1: Type: text/plain, Size: 8890 bytes --]

Resend.

vtdt: Modify vlapic code to add vtdt support

Accesses to MSR_IA32_TSC_DEADLINE are trapped, with value stored in a new field vlapic->hw.tdt_msr. vlapic->pt is reused in one shot mode for vtdt to trigger expire events.

For details, please refer to the Intel Architectures Software Developer's Manual 3A, 10.5.4.1 TSC-Deadline Mode.

Signed-off-by: Wei Gang <gang.wei@intel.com>

diff -r 41b85041d655 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Wed Dec 15 23:31:41 2010 +0800
+++ b/xen/arch/x86/hvm/hvm.c	Wed Dec 15 23:31:44 2010 +0800
@@ -2235,6 +2235,10 @@ int hvm_msr_read_intercept(unsigned int 
             goto gp_fault;
         break;
 
+    case MSR_IA32_TSC_DEADLINE:
+        *msr_content = vlapic_tdt_msr_get(vcpu_vlapic(v));
+        break;
+
     case MSR_IA32_CR_PAT:
         *msr_content = v->arch.hvm_vcpu.pat_cr;
         break;
@@ -2340,6 +2344,10 @@ int hvm_msr_write_intercept(unsigned int
 
     case MSR_IA32_APICBASE:
         vlapic_msr_set(vcpu_vlapic(v), msr_content);
+        break;
+
+    case MSR_IA32_TSC_DEADLINE:
+        vlapic_tdt_msr_set(vcpu_vlapic(v), msr_content);
         break;
 
     case MSR_IA32_APICBASE_MSR ... MSR_IA32_APICBASE_MSR + 0x3ff:
diff -r 41b85041d655 xen/arch/x86/hvm/vlapic.c
--- a/xen/arch/x86/hvm/vlapic.c	Wed Dec 15 23:31:41 2010 +0800
+++ b/xen/arch/x86/hvm/vlapic.c	Thu Dec 16 00:16:13 2010 +0800
@@ -56,7 +56,7 @@ static unsigned int vlapic_lvt_mask[VLAP
 static unsigned int vlapic_lvt_mask[VLAPIC_LVT_NUM] =
 {
      /* LVTT */
-     LVT_MASK | APIC_LVT_TIMER_PERIODIC,
+     LVT_MASK | APIC_TIMER_MODE_MASK,
      /* LVTTHMR */
      LVT_MASK | APIC_MODE_MASK,
      /* LVTPC */
@@ -79,7 +79,16 @@ static unsigned int vlapic_lvt_mask[VLAP
     (vlapic_get_reg(vlapic, lvt_type) & APIC_MODE_MASK)
 
 #define vlapic_lvtt_period(vlapic)                              \
-    (vlapic_get_reg(vlapic, APIC_LVTT) & APIC_LVT_TIMER_PERIODIC)
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_PERIODIC)
+
+#define vlapic_lvtt_oneshot(vlapic)                             \
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_ONESHOT)
+
+#define vlapic_lvtt_tdt(vlapic)                                 \
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_TSC_DEADLINE)
 
 
 /*
@@ -481,9 +490,20 @@ static void vlapic_read_aligned(
         break;
 
     case APIC_TMCCT: /* Timer CCR */
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+        {
+            *result = 0;
+            break;
+        }
         *result = vlapic_get_tmcct(vlapic);
         break;
 
+    case APIC_TMICT: /* Timer ICR */
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+        {
+            *result = 0;
+            break;
+        }
     default:
         *result = vlapic_get_reg(vlapic, offset);
         break;
@@ -566,6 +586,12 @@ static void vlapic_pt_cb(struct vcpu *v,
     *(s_time_t *)data = hvm_get_guest_time(v);
 }
 
+static void vlapic_tdt_pt_cb(struct vcpu *v, void *data)
+{
+    *(s_time_t *)data = hvm_get_guest_time(v);
+    vcpu_vlapic(v)->hw.tdt_msr = 0;
+}
+
 static int vlapic_reg_write(struct vcpu *v,
                             unsigned int offset, unsigned long val)
 {
@@ -657,6 +683,14 @@ static int vlapic_reg_write(struct vcpu 
         break;
 
     case APIC_LVTT:         /* LVT Timer Reg */
+        if ( (vlapic_get_reg(vlapic, offset) & APIC_TIMER_MODE_MASK) !=
+             (val & APIC_TIMER_MODE_MASK) )
+        {
+            destroy_periodic_time(&vlapic->pt);
+            vlapic_set_reg(vlapic, APIC_TMICT, 0);
+            vlapic_set_reg(vlapic, APIC_TMCCT, 0);
+            vlapic->hw.tdt_msr = 0;
+        }
         vlapic->pt.irq = val & APIC_VECTOR_MASK;
     case APIC_LVTTHMR:      /* LVT Thermal Monitor */
     case APIC_LVTPC:        /* LVT Performance Counter */
@@ -680,6 +714,9 @@ static int vlapic_reg_write(struct vcpu 
     {
         uint64_t period;
 
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+            break;
+
         vlapic_set_reg(vlapic, APIC_TMICT, val);
         if ( val == 0 )
         {
@@ -842,6 +879,73 @@ void vlapic_msr_set(struct vlapic *vlapi
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                 "apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
+}
+
+uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
+{
+    if ( !vlapic_lvtt_tdt(vlapic) )
+        return 0;
+
+    return vlapic->hw.tdt_msr;
+}
+
+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 */
+    if ( !vlapic_lvtt_tdt(vlapic) )
+    {
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "ignore tsc deadline msr write");
+        return;
+    }
+    
+    /* 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);
+
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", delta);
+
+        vlapic->hw.tdt_msr = value;
+        /* .... reprogram tdt timer */
+        create_periodic_time(v, &vlapic->pt, delta, 0,
+                             vlapic->pt.irq, vlapic_tdt_pt_cb,
+                             &vlapic->timer_last_update);
+        vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
+    }
+    else
+    {
+        vlapic->hw.tdt_msr = 0;
+
+        /* trigger a timer event if needed */
+        if ( value > 0 )
+        {
+            create_periodic_time(v, &vlapic->pt, 0, 0,
+                                 vlapic->pt.irq, vlapic_tdt_pt_cb,
+                                 &vlapic->timer_last_update);
+            vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
+        }
+        else
+        {
+            /* .... stop tdt timer */
+            destroy_periodic_time(&vlapic->pt);
+        }
+
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "value[0x%016"PRIx64"]", 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);
 }
 
 static int __vlapic_accept_pic_intr(struct vcpu *v)
@@ -956,6 +1060,17 @@ void vlapic_reset(struct vlapic *vlapic)
     destroy_periodic_time(&vlapic->pt);
 }
 
+static void lapic_tdt_rearm(struct vlapic *s)
+{
+    uint64_t tdt_msr = vlapic_tdt_msr_get(s);
+
+    s->pt.irq = vlapic_get_reg(s, APIC_LVTT) & APIC_VECTOR_MASK;
+    if ( tdt_msr == 0)
+       return;
+
+    vlapic_tdt_msr_set(s, tdt_msr);
+}
+
 /* rearm the actimer if needed, after a HVM restore */
 static void lapic_rearm(struct vlapic *s)
 {
@@ -1023,7 +1138,7 @@ static int lapic_load_hidden(struct doma
     }
     s = vcpu_vlapic(v);
     
-    if ( hvm_load_entry(LAPIC, h, &s->hw) != 0 ) 
+    if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) != 0 ) 
         return -EINVAL;
 
     vmx_vlapic_msr_changed(v);
@@ -1050,7 +1165,10 @@ static int lapic_load_regs(struct domain
         return -EINVAL;
 
     vlapic_adjust_i8259_target(d);
-    lapic_rearm(s);
+    if ( vlapic_lvtt_tdt(s) )
+        lapic_tdt_rearm(s);
+    else
+        lapic_rearm(s);
     return 0;
 }
 
diff -r 41b85041d655 xen/include/asm-x86/hvm/vlapic.h
--- a/xen/include/asm-x86/hvm/vlapic.h	Wed Dec 15 23:31:41 2010 +0800
+++ b/xen/include/asm-x86/hvm/vlapic.h	Wed Dec 15 23:31:44 2010 +0800
@@ -92,6 +92,8 @@ void vlapic_reset(struct vlapic *vlapic)
 void vlapic_reset(struct vlapic *vlapic);
 
 void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
+void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
+uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
 
 int vlapic_accept_pic_intr(struct vcpu *v);
 
diff -r 41b85041d655 xen/include/public/arch-x86/hvm/save.h
--- a/xen/include/public/arch-x86/hvm/save.h	Wed Dec 15 23:31:41 2010 +0800
+++ b/xen/include/public/arch-x86/hvm/save.h	Wed Dec 15 23:31:44 2010 +0800
@@ -265,6 +265,7 @@ struct hvm_hw_lapic {
     uint64_t             apic_base_msr;
     uint32_t             disabled; /* VLAPIC_xx_DISABLED */
     uint32_t             timer_divisor;
+    uint64_t             tdt_msr;
 };
 
 DECLARE_HVM_SAVE_TYPE(LAPIC, 5, struct hvm_hw_lapic);

[-- Attachment #2: 4-vtdt.patch --]
[-- Type: application/octet-stream, Size: 8621 bytes --]

vtdt: Modify vlapic code to add vtdt support

Accesses to MSR_IA32_TSC_DEADLINE are trapped, with value stored in a new field vlapic->hw.tdt_msr. vlapic->pt is reused in one shot mode for vtdt to trigger expire events.

For details, please refer to the Intel Architectures Software Developer's Manual 3A, 10.5.4.1 TSC-Deadline Mode.

Signed-off-by: Wei Gang <gang.wei@intel.com>

diff -r 41b85041d655 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Wed Dec 15 23:31:41 2010 +0800
+++ b/xen/arch/x86/hvm/hvm.c	Wed Dec 15 23:31:44 2010 +0800
@@ -2235,6 +2235,10 @@ int hvm_msr_read_intercept(unsigned int 
             goto gp_fault;
         break;
 
+    case MSR_IA32_TSC_DEADLINE:
+        *msr_content = vlapic_tdt_msr_get(vcpu_vlapic(v));
+        break;
+
     case MSR_IA32_CR_PAT:
         *msr_content = v->arch.hvm_vcpu.pat_cr;
         break;
@@ -2340,6 +2344,10 @@ int hvm_msr_write_intercept(unsigned int
 
     case MSR_IA32_APICBASE:
         vlapic_msr_set(vcpu_vlapic(v), msr_content);
+        break;
+
+    case MSR_IA32_TSC_DEADLINE:
+        vlapic_tdt_msr_set(vcpu_vlapic(v), msr_content);
         break;
 
     case MSR_IA32_APICBASE_MSR ... MSR_IA32_APICBASE_MSR + 0x3ff:
diff -r 41b85041d655 xen/arch/x86/hvm/vlapic.c
--- a/xen/arch/x86/hvm/vlapic.c	Wed Dec 15 23:31:41 2010 +0800
+++ b/xen/arch/x86/hvm/vlapic.c	Thu Dec 16 00:16:13 2010 +0800
@@ -56,7 +56,7 @@ static unsigned int vlapic_lvt_mask[VLAP
 static unsigned int vlapic_lvt_mask[VLAPIC_LVT_NUM] =
 {
      /* LVTT */
-     LVT_MASK | APIC_LVT_TIMER_PERIODIC,
+     LVT_MASK | APIC_TIMER_MODE_MASK,
      /* LVTTHMR */
      LVT_MASK | APIC_MODE_MASK,
      /* LVTPC */
@@ -79,7 +79,16 @@ static unsigned int vlapic_lvt_mask[VLAP
     (vlapic_get_reg(vlapic, lvt_type) & APIC_MODE_MASK)
 
 #define vlapic_lvtt_period(vlapic)                              \
-    (vlapic_get_reg(vlapic, APIC_LVTT) & APIC_LVT_TIMER_PERIODIC)
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_PERIODIC)
+
+#define vlapic_lvtt_oneshot(vlapic)                             \
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_ONESHOT)
+
+#define vlapic_lvtt_tdt(vlapic)                                 \
+    ((vlapic_get_reg(vlapic, APIC_LVTT) & APIC_TIMER_MODE_MASK) \
+     == APIC_TIMER_MODE_TSC_DEADLINE)
 
 
 /*
@@ -481,9 +490,20 @@ static void vlapic_read_aligned(
         break;
 
     case APIC_TMCCT: /* Timer CCR */
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+        {
+            *result = 0;
+            break;
+        }
         *result = vlapic_get_tmcct(vlapic);
         break;
 
+    case APIC_TMICT: /* Timer ICR */
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+        {
+            *result = 0;
+            break;
+        }
     default:
         *result = vlapic_get_reg(vlapic, offset);
         break;
@@ -566,6 +586,12 @@ static void vlapic_pt_cb(struct vcpu *v,
     *(s_time_t *)data = hvm_get_guest_time(v);
 }
 
+static void vlapic_tdt_pt_cb(struct vcpu *v, void *data)
+{
+    *(s_time_t *)data = hvm_get_guest_time(v);
+    vcpu_vlapic(v)->hw.tdt_msr = 0;
+}
+
 static int vlapic_reg_write(struct vcpu *v,
                             unsigned int offset, unsigned long val)
 {
@@ -657,6 +683,14 @@ static int vlapic_reg_write(struct vcpu 
         break;
 
     case APIC_LVTT:         /* LVT Timer Reg */
+        if ( (vlapic_get_reg(vlapic, offset) & APIC_TIMER_MODE_MASK) !=
+             (val & APIC_TIMER_MODE_MASK) )
+        {
+            destroy_periodic_time(&vlapic->pt);
+            vlapic_set_reg(vlapic, APIC_TMICT, 0);
+            vlapic_set_reg(vlapic, APIC_TMCCT, 0);
+            vlapic->hw.tdt_msr = 0;
+        }
         vlapic->pt.irq = val & APIC_VECTOR_MASK;
     case APIC_LVTTHMR:      /* LVT Thermal Monitor */
     case APIC_LVTPC:        /* LVT Performance Counter */
@@ -680,6 +714,9 @@ static int vlapic_reg_write(struct vcpu 
     {
         uint64_t period;
 
+        if ( !vlapic_lvtt_oneshot(vlapic) && !vlapic_lvtt_period(vlapic) )
+            break;
+
         vlapic_set_reg(vlapic, APIC_TMICT, val);
         if ( val == 0 )
         {
@@ -842,6 +879,73 @@ void vlapic_msr_set(struct vlapic *vlapi
 
     HVM_DBG_LOG(DBG_LEVEL_VLAPIC,
                 "apic base msr is 0x%016"PRIx64, vlapic->hw.apic_base_msr);
+}
+
+uint64_t  vlapic_tdt_msr_get(struct vlapic *vlapic)
+{
+    if ( !vlapic_lvtt_tdt(vlapic) )
+        return 0;
+
+    return vlapic->hw.tdt_msr;
+}
+
+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 */
+    if ( !vlapic_lvtt_tdt(vlapic) )
+    {
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "ignore tsc deadline msr write");
+        return;
+    }
+    
+    /* 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);
+
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "delta[0x%016"PRIx64"]", delta);
+
+        vlapic->hw.tdt_msr = value;
+        /* .... reprogram tdt timer */
+        create_periodic_time(v, &vlapic->pt, delta, 0,
+                             vlapic->pt.irq, vlapic_tdt_pt_cb,
+                             &vlapic->timer_last_update);
+        vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
+    }
+    else
+    {
+        vlapic->hw.tdt_msr = 0;
+
+        /* trigger a timer event if needed */
+        if ( value > 0 )
+        {
+            create_periodic_time(v, &vlapic->pt, 0, 0,
+                                 vlapic->pt.irq, vlapic_tdt_pt_cb,
+                                 &vlapic->timer_last_update);
+            vlapic->timer_last_update = vlapic->pt.last_plt_gtime;
+        }
+        else
+        {
+            /* .... stop tdt timer */
+            destroy_periodic_time(&vlapic->pt);
+        }
+
+        HVM_DBG_LOG(DBG_LEVEL_VLAPIC_TIMER, "value[0x%016"PRIx64"]", 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);
 }
 
 static int __vlapic_accept_pic_intr(struct vcpu *v)
@@ -956,6 +1060,17 @@ void vlapic_reset(struct vlapic *vlapic)
     destroy_periodic_time(&vlapic->pt);
 }
 
+static void lapic_tdt_rearm(struct vlapic *s)
+{
+    uint64_t tdt_msr = vlapic_tdt_msr_get(s);
+
+    s->pt.irq = vlapic_get_reg(s, APIC_LVTT) & APIC_VECTOR_MASK;
+    if ( tdt_msr == 0)
+       return;
+
+    vlapic_tdt_msr_set(s, tdt_msr);
+}
+
 /* rearm the actimer if needed, after a HVM restore */
 static void lapic_rearm(struct vlapic *s)
 {
@@ -1023,7 +1138,7 @@ static int lapic_load_hidden(struct doma
     }
     s = vcpu_vlapic(v);
     
-    if ( hvm_load_entry(LAPIC, h, &s->hw) != 0 ) 
+    if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) != 0 ) 
         return -EINVAL;
 
     vmx_vlapic_msr_changed(v);
@@ -1050,7 +1165,10 @@ static int lapic_load_regs(struct domain
         return -EINVAL;
 
     vlapic_adjust_i8259_target(d);
-    lapic_rearm(s);
+    if ( vlapic_lvtt_tdt(s) )
+        lapic_tdt_rearm(s);
+    else
+        lapic_rearm(s);
     return 0;
 }
 
diff -r 41b85041d655 xen/include/asm-x86/hvm/vlapic.h
--- a/xen/include/asm-x86/hvm/vlapic.h	Wed Dec 15 23:31:41 2010 +0800
+++ b/xen/include/asm-x86/hvm/vlapic.h	Wed Dec 15 23:31:44 2010 +0800
@@ -92,6 +92,8 @@ void vlapic_reset(struct vlapic *vlapic)
 void vlapic_reset(struct vlapic *vlapic);
 
 void vlapic_msr_set(struct vlapic *vlapic, uint64_t value);
+void vlapic_tdt_msr_set(struct vlapic *vlapic, uint64_t value);
+uint64_t vlapic_tdt_msr_get(struct vlapic *vlapic);
 
 int vlapic_accept_pic_intr(struct vcpu *v);
 
diff -r 41b85041d655 xen/include/public/arch-x86/hvm/save.h
--- a/xen/include/public/arch-x86/hvm/save.h	Wed Dec 15 23:31:41 2010 +0800
+++ b/xen/include/public/arch-x86/hvm/save.h	Wed Dec 15 23:31:44 2010 +0800
@@ -265,6 +265,7 @@ struct hvm_hw_lapic {
     uint64_t             apic_base_msr;
     uint32_t             disabled; /* VLAPIC_xx_DISABLED */
     uint32_t             timer_divisor;
+    uint64_t             tdt_msr;
 };
 
 DECLARE_HVM_SAVE_TYPE(LAPIC, 5, struct hvm_hw_lapic);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2010-12-14 10:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-14  3:27 [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support Wei, Gang
2010-12-14  7:35 ` Keir Fraser
2010-12-14  8:22   ` Wei, Gang
2010-12-14  8:48     ` Keir Fraser
2010-12-14  9:13       ` Wei, Gang
2010-12-14  9:59         ` Keir Fraser
2010-12-14 10:01           ` Keir Fraser
2010-12-14 10:18             ` Tim Deegan
2010-12-14  9:21 ` Keir Fraser
2010-12-14  9:30   ` Wei, Gang
2010-12-14  9:56     ` Keir Fraser
2010-12-14 10:13       ` Wei, Gang
2010-12-14 10:29 ` Wei, Gang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.