Wei, Gang
2010-Dec-14 03:27 UTC
[Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2010-Dec-14 07:35 UTC
Re: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei, Gang
2010-Dec-14 08:22 UTC
RE: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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?_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Dec-14 08:48 UTC
Re: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei, Gang
2010-Dec-14 09:13 UTC
RE: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Dec-14 09:21 UTC
Re: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei, Gang
2010-Dec-14 09:30 UTC
RE: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Dec-14 09:56 UTC
Re: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Dec-14 09:59 UTC
Re: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Dec-14 10:01 UTC
Re: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei, Gang
2010-Dec-14 10:13 UTC
RE: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2010-Dec-14 10:18 UTC
Re: [Xen-devel] [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei, Gang
2010-Dec-14 10:29 UTC
[Xen-devel] RE: [PATCH 3/5] vtdt: Modify vlapic code to add vtdt support
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);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel