Re: [PATCH 1/2] KVM: emulate lapic tsc deadline timer for guest
On Thu, Sep 15, 2011 at 02:22:58PM +0800, Liu, Jinsong wrote: > Marcelo Tosatti wrote: > >> diff --git a/arch/x86/include/asm/apicdef.h > >> b/arch/x86/include/asm/apicdef.h > >> index 34595d5..3925d80 100644 > >> --- a/arch/x86/include/asm/apicdef.h > >> +++ b/arch/x86/include/asm/apicdef.h > >> @@ -100,7 +100,9 @@ > >> #define APIC_TIMER_BASE_CLKIN 0x0 > >> #define APIC_TIMER_BASE_TMBASE 0x1 > >> #define APIC_TIMER_BASE_DIV 0x2 > >> +#define APIC_LVT_TIMER_ONESHOT (0 << 17) > >> #define APIC_LVT_TIMER_PERIODIC (1 << 17) > >> +#define APIC_LVT_TIMER_TSCDEADLINE (2 << 17) > >> #define APIC_LVT_MASKED (1 << 16) > >> #define APIC_LVT_LEVEL_TRIGGER (1 << 15) > >> #define APIC_LVT_REMOTE_IRR (1 << 14) > > > > Please have a separate, introductory patch for definitions that are > > not KVM specific. > > > > OK, will present a separate patch. BTW, will the separate patch still be send > to kvm@vger.kernel.org? Yes. > > >> +++ b/arch/x86/include/asm/kvm_host.h > >> @@ -671,6 +671,8 @@ u8 kvm_get_guest_memory_type(struct kvm_vcpu > >> *vcpu, gfn_t gfn); > >> > >> extern bool tdp_enabled; > >> > >> +extern u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu); > >> + > > > > No need for extern. > > > > Any special concern, or, for coding style? a little curious :) It is not necessary. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] KVM: emulate lapic tsc deadline timer for guest
On Thu, Sep 15, 2011 at 04:17:20PM +0800, Liu, Jinsong wrote:
> Marcelo Tosatti wrote:
> >> + } else if (apic_lvtt_tscdeadline(apic)) {
> >> + /* lapic timer in tsc deadline mode */
> >> + u64 guest_tsc, guest_tsc_delta, ns = 0;
> >> + struct kvm_vcpu *vcpu = apic->vcpu;
> >> + unsigned long this_tsc_khz = vcpu_tsc_khz(vcpu); +
> >> unsigned long
> >> flags; +
> >> + if (unlikely(!apic->lapic_timer.tscdeadline || !this_tsc_khz))
> >> + return; +
> >> + local_irq_save(flags);
> >> +
> >> + now = apic->lapic_timer.timer.base->get_time();
> >> + kvm_get_msr(vcpu, MSR_IA32_TSC, &guest_tsc);
> >
> > Use kvm_x86_ops->read_l1_tsc(vcpu) instead of direct MSR read
> > (to avoid reading L2 guest TSC in case of nested virt).
> >
> >> + guest_tsc_delta = apic->lapic_timer.tscdeadline - guest_tsc;
> >
> > if (guest_tsc <= tscdeadline), the timer should start immediately.
> >
>
> Yes, under such case the timer does start immediately, with ns = 0
No, guest_tsc_delta is unsigned, so the "< 0" comparation fails.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] KVM: emulate lapic tsc deadline timer for guest
Marcelo Tosatti wrote:
>> +} else if (apic_lvtt_tscdeadline(apic)) {
>> +/* lapic timer in tsc deadline mode */
>> +u64 guest_tsc, guest_tsc_delta, ns = 0;
>> +struct kvm_vcpu *vcpu = apic->vcpu;
>> +unsigned long this_tsc_khz = vcpu_tsc_khz(vcpu); +
>> unsigned long
>> flags; +
>> +if (unlikely(!apic->lapic_timer.tscdeadline || !this_tsc_khz))
>> +return; +
>> +local_irq_save(flags);
>> +
>> +now = apic->lapic_timer.timer.base->get_time();
>> +kvm_get_msr(vcpu, MSR_IA32_TSC, &guest_tsc);
>
> Use kvm_x86_ops->read_l1_tsc(vcpu) instead of direct MSR read
> (to avoid reading L2 guest TSC in case of nested virt).
>
>> +guest_tsc_delta = apic->lapic_timer.tscdeadline - guest_tsc;
>
> if (guest_tsc <= tscdeadline), the timer should start immediately.
>
Yes, under such case the timer does start immediately, with ns = 0
Thanks,
Jinsong--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] KVM: emulate lapic tsc deadline timer for guest
Marcelo Tosatti wrote:
>> diff --git a/arch/x86/include/asm/apicdef.h
>> b/arch/x86/include/asm/apicdef.h
>> index 34595d5..3925d80 100644
>> --- a/arch/x86/include/asm/apicdef.h
>> +++ b/arch/x86/include/asm/apicdef.h
>> @@ -100,7 +100,9 @@
>> #define APIC_TIMER_BASE_CLKIN 0x0
>> #define APIC_TIMER_BASE_TMBASE 0x1
>> #define APIC_TIMER_BASE_DIV 0x2
>> +#define APIC_LVT_TIMER_ONESHOT (0 << 17)
>> #define APIC_LVT_TIMER_PERIODIC (1 << 17)
>> +#define APIC_LVT_TIMER_TSCDEADLINE (2 << 17)
>> #define APIC_LVT_MASKED (1 << 16)
>> #define APIC_LVT_LEVEL_TRIGGER (1 << 15)
>> #define APIC_LVT_REMOTE_IRR (1 << 14)
>
> Please have a separate, introductory patch for definitions that are
> not KVM specific.
>
OK, will present a separate patch. BTW, will the separate patch still be send
to kvm@vger.kernel.org?
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -671,6 +671,8 @@ u8 kvm_get_guest_memory_type(struct kvm_vcpu
>> *vcpu, gfn_t gfn);
>>
>> extern bool tdp_enabled;
>>
>> +extern u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu);
>> +
>
> No need for extern.
>
Any special concern, or, for coding style? a little curious :)
>> +} else if (apic_lvtt_tscdeadline(apic)) {
>> +/* lapic timer in tsc deadline mode */
>> +u64 guest_tsc, guest_tsc_delta, ns = 0;
>> +struct kvm_vcpu *vcpu = apic->vcpu;
>> +unsigned long this_tsc_khz = vcpu_tsc_khz(vcpu); +
>> unsigned long
>> flags; +
>> +if (unlikely(!apic->lapic_timer.tscdeadline || !this_tsc_khz))
>> +return; +
>> +local_irq_save(flags);
>> +
>> +now = apic->lapic_timer.timer.base->get_time();
>> +kvm_get_msr(vcpu, MSR_IA32_TSC, &guest_tsc);
>
> Use kvm_x86_ops->read_l1_tsc(vcpu) instead of direct MSR read
> (to avoid reading L2 guest TSC in case of nested virt).
>
Fine. I use some old version kvm (Jul 22), and didn't notice Nadav's patch
checked in Aug 2 with read_l1_tsc hook.
Thanks for tell me.
>> +guest_tsc_delta = apic->lapic_timer.tscdeadline - guest_tsc;
>
> if (guest_tsc <= tscdeadline), the timer should start immediately.
>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 6cb353c..a73c059 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -610,6 +610,16 @@ static void update_cpuid(struct kvm_vcpu *vcpu)
>> if (kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE))
>> best->ecx |= bit(X86_FEATURE_OSXSAVE);
>> }
>> +
>> +/*
>> + * When cpu has tsc deadline timer capacibility, use bit 17/18
>> + * as timer mode mask. Otherwise only use bit 17. + */
>> +if (cpu_has_tsc_deadline_timer && best->function == 0x1) {
>> +best->ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);
>> +vcpu->arch.apic->lapic_timer.timer_mode_mask = (3 << 17); +
>> } else
>> +vcpu->arch.apic->lapic_timer.timer_mode_mask = (1 << 17);
>> }
>
> The deadline timer is entirely emulated, whether the host CPU supports
> it or not is irrelevant.
>
> Why was this changed from previous submissions?
Hmm, will explain in next email.
Thanks,
Jinsong
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] KVM: emulate lapic tsc deadline timer for guest
On Tue, Sep 13, 2011 at 10:36:51PM +0800, Liu, Jinsong wrote:
> >From 7b12021e1d1b79797b49e41cc0a7be05a6180d9a Mon Sep 17 00:00:00 2001
> From: Liu, Jinsong
> Date: Tue, 13 Sep 2011 21:52:54 +0800
> Subject: [PATCH] KVM: emulate lapic tsc deadline timer for guest
>
> This patch emulate lapic tsc deadline timer for guest:
> Enumerate tsc deadline timer capability by CPUID;
> Enable tsc deadline timer mode by lapic MMIO;
> Start tsc deadline timer by WRMSR;
>
> Signed-off-by: Liu, Jinsong
> ---
> arch/x86/include/asm/apicdef.h|2 +
> arch/x86/include/asm/cpufeature.h |3 +
> arch/x86/include/asm/kvm_host.h |2 +
> arch/x86/include/asm/msr-index.h |2 +
> arch/x86/kvm/kvm_timer.h |2 +
> arch/x86/kvm/lapic.c | 122
> ++---
> arch/x86/kvm/lapic.h |3 +
> arch/x86/kvm/x86.c| 20 ++-
> 8 files changed, 132 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
> index 34595d5..3925d80 100644
> --- a/arch/x86/include/asm/apicdef.h
> +++ b/arch/x86/include/asm/apicdef.h
> @@ -100,7 +100,9 @@
> #define APIC_TIMER_BASE_CLKIN 0x0
> #define APIC_TIMER_BASE_TMBASE 0x1
> #define APIC_TIMER_BASE_DIV 0x2
> +#define APIC_LVT_TIMER_ONESHOT (0 << 17)
> #define APIC_LVT_TIMER_PERIODIC (1 << 17)
> +#define APIC_LVT_TIMER_TSCDEADLINE (2 << 17)
> #define APIC_LVT_MASKED (1 << 16)
> #define APIC_LVT_LEVEL_TRIGGER (1 << 15)
> #define APIC_LVT_REMOTE_IRR (1 << 14)
Please have a separate, introductory patch for definitions that are not
KVM specific.
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -671,6 +671,8 @@ u8 kvm_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t
> gfn);
>
> extern bool tdp_enabled;
>
> +extern u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu);
> +
No need for extern.
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 2b2255b..925d4b9 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -135,9 +135,23 @@ static inline int apic_lvt_vector(struct kvm_lapic
> *apic, int lvt_type)
> return apic_get_reg(apic, lvt_type) & APIC_VECTOR_MASK;
> }
>
> +static inline int apic_lvtt_oneshot(struct kvm_lapic *apic)
> +{
> + return ((apic_get_reg(apic, APIC_LVTT) &
> + apic->lapic_timer.timer_mode_mask) == APIC_LVT_TIMER_ONESHOT);
> +}
> +
> static inline int apic_lvtt_period(struct kvm_lapic *apic)
> {
> - return apic_get_reg(apic, APIC_LVTT) & APIC_LVT_TIMER_PERIODIC;
> + return ((apic_get_reg(apic, APIC_LVTT) &
> + apic->lapic_timer.timer_mode_mask) == APIC_LVT_TIMER_PERIODIC);
> +}
> +
> +static inline int apic_lvtt_tscdeadline(struct kvm_lapic *apic)
> +{
> + return ((apic_get_reg(apic, APIC_LVTT) &
> + apic->lapic_timer.timer_mode_mask) ==
> + APIC_LVT_TIMER_TSCDEADLINE);
> }
>
> static inline int apic_lvt_nmi_mode(u32 lvt_val)
> @@ -166,7 +180,7 @@ static inline int apic_x2apic_mode(struct kvm_lapic *apic)
> }
>
> static unsigned int apic_lvt_mask[APIC_LVT_NUM] = {
> - LVT_MASK | APIC_LVT_TIMER_PERIODIC, /* LVTT */
> + LVT_MASK , /* part LVTT mask, timer mode mask added at runtime */
> LVT_MASK | APIC_MODE_MASK, /* LVTTHMR */
> LVT_MASK | APIC_MODE_MASK, /* LVTPC */
> LINT_MASK, LINT_MASK, /* LVT0-1 */
> @@ -570,6 +584,9 @@ static u32 __apic_read(struct kvm_lapic *apic, unsigned
> int offset)
> break;
>
> case APIC_TMCCT:/* Timer CCR */
> + if (apic_lvtt_tscdeadline(apic))
> + return 0;
> +
> val = apic_get_tmcct(apic);
> break;
>
> @@ -664,29 +681,32 @@ static void update_divide_count(struct kvm_lapic *apic)
>
> static void start_apic_timer(struct kvm_lapic *apic)
> {
> - ktime_t now = apic->lapic_timer.timer.base->get_time();
> -
> - apic->lapic_timer.period = (u64)apic_get_reg(apic, APIC_TMICT) *
> - APIC_BUS_CYCLE_NS * apic->divide_count;
> + ktime_t now;
> atomic_set(&apic->lapic_timer.pending, 0);
>
> - if (!apic->lapic_timer.period)
> - return;
> - /*
> - * Do not allow the guest to program periodic timers with small
> - * interval, since the hrtimers are not throttled by the host
> - * scheduler.
> - */
> - if (apic_lvtt_period(apic)) {
> - if (apic->lapic_timer.period < NSEC_PER_MSEC/2)
> - apic->lapic_timer.period = NSEC_PER_MSEC/2;
> - }
> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) {
> + /* lapic timer in oneshot or peroidic mode */
> + now = apic->lapic_timer.timer.base->get_time();
> +
[PATCH 1/2] KVM: emulate lapic tsc deadline timer for guest
>From 7b12021e1d1b79797b49e41cc0a7be05a6180d9a Mon Sep 17 00:00:00 2001
From: Liu, Jinsong
Date: Tue, 13 Sep 2011 21:52:54 +0800
Subject: [PATCH] KVM: emulate lapic tsc deadline timer for guest
This patch emulate lapic tsc deadline timer for guest:
Enumerate tsc deadline timer capability by CPUID;
Enable tsc deadline timer mode by lapic MMIO;
Start tsc deadline timer by WRMSR;
Signed-off-by: Liu, Jinsong
---
arch/x86/include/asm/apicdef.h|2 +
arch/x86/include/asm/cpufeature.h |3 +
arch/x86/include/asm/kvm_host.h |2 +
arch/x86/include/asm/msr-index.h |2 +
arch/x86/kvm/kvm_timer.h |2 +
arch/x86/kvm/lapic.c | 122 ++---
arch/x86/kvm/lapic.h |3 +
arch/x86/kvm/x86.c| 20 ++-
8 files changed, 132 insertions(+), 24 deletions(-)
diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
index 34595d5..3925d80 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -100,7 +100,9 @@
#defineAPIC_TIMER_BASE_CLKIN 0x0
#defineAPIC_TIMER_BASE_TMBASE 0x1
#defineAPIC_TIMER_BASE_DIV 0x2
+#defineAPIC_LVT_TIMER_ONESHOT (0 << 17)
#defineAPIC_LVT_TIMER_PERIODIC (1 << 17)
+#defineAPIC_LVT_TIMER_TSCDEADLINE (2 << 17)
#defineAPIC_LVT_MASKED (1 << 16)
#defineAPIC_LVT_LEVEL_TRIGGER (1 << 15)
#defineAPIC_LVT_REMOTE_IRR (1 << 14)
diff --git a/arch/x86/include/asm/cpufeature.h
b/arch/x86/include/asm/cpufeature.h
index 4258aac..8a26e48 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -120,6 +120,7 @@
#define X86_FEATURE_X2APIC (4*32+21) /* x2APIC */
#define X86_FEATURE_MOVBE (4*32+22) /* MOVBE instruction */
#define X86_FEATURE_POPCNT (4*32+23) /* POPCNT instruction */
+#define X86_FEATURE_TSC_DEADLINE_TIMER(4*32+24) /* Tsc deadline timer */
#define X86_FEATURE_AES(4*32+25) /* AES instructions */
#define X86_FEATURE_XSAVE (4*32+26) /* XSAVE/XRSTOR/XSETBV/XGETBV */
#define X86_FEATURE_OSXSAVE(4*32+27) /* "" XSAVE enabled in the OS */
@@ -284,6 +285,8 @@ extern const char * const x86_power_flags[32];
#define cpu_has_xmm4_1 boot_cpu_has(X86_FEATURE_XMM4_1)
#define cpu_has_xmm4_2 boot_cpu_has(X86_FEATURE_XMM4_2)
#define cpu_has_x2apic boot_cpu_has(X86_FEATURE_X2APIC)
+#define cpu_has_tsc_deadline_timer \
+ boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER)
#define cpu_has_xsave boot_cpu_has(X86_FEATURE_XSAVE)
#define cpu_has_hypervisor boot_cpu_has(X86_FEATURE_HYPERVISOR)
#define cpu_has_pclmulqdq boot_cpu_has(X86_FEATURE_PCLMULQDQ)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 307e3cf..2ce6529 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -671,6 +671,8 @@ u8 kvm_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t
gfn);
extern bool tdp_enabled;
+extern u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu);
+
/* control of guest tsc rate supported? */
extern bool kvm_has_tsc_control;
/* minimum supported tsc_khz for guests */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index d52609a..a6962d9 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -229,6 +229,8 @@
#define MSR_IA32_APICBASE_ENABLE (1<<11)
#define MSR_IA32_APICBASE_BASE (0xf<<12)
+#define MSR_IA32_TSCDEADLINE 0x06e0
+
#define MSR_IA32_UCODE_WRITE 0x0079
#define MSR_IA32_UCODE_REV 0x008b
diff --git a/arch/x86/kvm/kvm_timer.h b/arch/x86/kvm/kvm_timer.h
index 64bc6ea..497dbaa 100644
--- a/arch/x86/kvm/kvm_timer.h
+++ b/arch/x86/kvm/kvm_timer.h
@@ -2,6 +2,8 @@
struct kvm_timer {
struct hrtimer timer;
s64 period; /* unit: ns */
+ u32 timer_mode_mask;
+ u64 tscdeadline;
atomic_t pending; /* accumulated triggered timers
*/
bool reinject;
struct kvm_timer_ops *t_ops;
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 2b2255b..925d4b9 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -135,9 +135,23 @@ static inline int apic_lvt_vector(struct kvm_lapic *apic,
int lvt_type)
return apic_get_reg(apic, lvt_type) & APIC_VECTOR_MASK;
}
+static inline int apic_lvtt_oneshot(struct kvm_lapic *apic)
+{
+ return ((apic_get_reg(apic, APIC_LVTT) &
+ apic->lapic_timer.timer_mode_mask) == APIC_LVT_TIMER_ONESHOT);
+}
+
static inline int apic_lvtt_period(struct kvm_lapic *apic)
{
- return apic_get_reg(apic, APIC_LVTT) & APIC_LVT_TIMER_PERIODIC;
+
