[PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Avi Kivity]

From: Jan Kiszka <jan.kiszka@siemens.com>

Unlike all of the other cpuid bits, the TSC deadline timer bit is set
unconditionally, regardless of what userspace wants.

This is broken in several ways:
 - if userspace doesn't use KVM_CREATE_IRQCHIP, and doesn't emulate the TSC
   deadline timer feature, a guest that uses the feature will break
 - live migration to older host kernels that don't support the TSC deadline
   timer will cause the feature to be pulled from under the guest's feet;
   breaking it
 - guests that are broken wrt the feature will fail.

Fix by not enabling the feature automatically; instead report it to userspace.
Because the feature depends on KVM_CREATE_IRQCHIP, which we cannot guarantee
will be called, we expose it via a KVM_CAP_TSC_DEADLINE_TIMER and not
KVM_GET_SUPPORTED_CPUID.

Fixes the Illumos guest kernel, which uses the TSC deadline timer feature.

[avi: add the KVM_CAP + documentation]

Reported-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
Signed-off-by: Avi Kivity <avi@redhat.com>
---

As we're running out of time and everyone's checking their socks instead of
inboxes I've added the missing parts myself.  Jan, if you accidentally see
this, please review and add your signoff.

 Documentation/virtual/kvm/api.txt |    9 +++++++++
 arch/x86/kvm/cpuid.c              |   16 ++++++----------
 arch/x86/kvm/x86.c                |    3 +++
 include/linux/kvm.h               |    1 +
 4 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 5b03eee..da1f8fd 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1100,6 +1100,15 @@ emulate them efficiently. The fields in each entry are defined as follows:
    eax, ebx, ecx, edx: the values returned by the cpuid instruction for
          this function/index combination
 
+The TSC deadline timer feature (CPUID leaf 1, ecx[24]) is always returned
+as false, since the feature depends on KVM_CREATE_IRQCHIP for local APIC
+support.  Instead it is reported via
+
+  ioctl(KVM_CHECK_EXTENSION, KVM_CAP_TSC_DEADLINE_TIMER)
+
+if that returns true you use KVM_CREATE_IRQCHIP, or if emulate the
+feature in userspace, then you can enable the feature for KVM_SET_CPUID2.
+
 4.47 KVM_PPC_GET_PVINFO
 
 Capability: KVM_CAP_PPC_GET_PVINFO
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 230f713..89b02bf 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -27,7 +27,6 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;
 	struct kvm_lapic *apic = vcpu->arch.apic;
-	u32 timer_mode_mask;
 
 	best = kvm_find_cpuid_entry(vcpu, 1, 0);
 	if (!best)
@@ -40,15 +39,12 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
 			best->ecx |= bit(X86_FEATURE_OSXSAVE);
 	}
 
-	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
-		best->function == 0x1) {
-		best->ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);
-		timer_mode_mask = 3 << 17;
-	} else
-		timer_mode_mask = 1 << 17;
-
-	if (apic)
-		apic->lapic_timer.timer_mode_mask = timer_mode_mask;
+	if (apic) {
+		if (best->ecx & bit(X86_FEATURE_TSC_DEADLINE_TIMER))
+			apic->lapic_timer.timer_mode_mask = 3 << 17;
+		else
+			apic->lapic_timer.timer_mode_mask = 1 << 17;
+	}
 
 	kvm_pmu_cpuid_update(vcpu);
 }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index df23dff..1171def 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2089,6 +2089,9 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_TSC_CONTROL:
 		r = kvm_has_tsc_control;
 		break;
+	case KVM_CAP_TSC_DEADLINE_TIMER:
+		r = boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER);
+		break;
 	default:
 		r = 0;
 		break;
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index c3892fc..68e67e5 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -557,6 +557,7 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_MAX_VCPUS 66       /* returns max vcpus per vm */
 #define KVM_CAP_PPC_PAPR 68
 #define KVM_CAP_S390_GMAP 71
+#define KVM_CAP_TSC_DEADLINE_TIMER 72
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
-- 
1.7.7.1

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Jan Kiszka]

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

On 2011-12-25 14:03, Avi Kivity wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Unlike all of the other cpuid bits, the TSC deadline timer bit is set
> unconditionally, regardless of what userspace wants.
> 
> This is broken in several ways:
>  - if userspace doesn't use KVM_CREATE_IRQCHIP, and doesn't emulate the TSC
>    deadline timer feature, a guest that uses the feature will break
>  - live migration to older host kernels that don't support the TSC deadline
>    timer will cause the feature to be pulled from under the guest's feet;
>    breaking it
>  - guests that are broken wrt the feature will fail.
> 
> Fix by not enabling the feature automatically; instead report it to userspace.
> Because the feature depends on KVM_CREATE_IRQCHIP, which we cannot guarantee
> will be called, we expose it via a KVM_CAP_TSC_DEADLINE_TIMER and not
> KVM_GET_SUPPORTED_CPUID.
> 
> Fixes the Illumos guest kernel, which uses the TSC deadline timer feature.
> 
> [avi: add the KVM_CAP + documentation]
> 
> Reported-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
> 
> As we're running out of time and everyone's checking their socks instead of
> inboxes I've added the missing parts myself.  Jan, if you accidentally see
> this, please review and add your signoff.

I'm sorry for not holding my promise, was distracted the past days.
Patch looks good to me, just some minor phrasing corrections below.

Signed-off-by; Jan Kiszka <jan.kiszka@siemens.com>

> 
>  Documentation/virtual/kvm/api.txt |    9 +++++++++
>  arch/x86/kvm/cpuid.c              |   16 ++++++----------
>  arch/x86/kvm/x86.c                |    3 +++
>  include/linux/kvm.h               |    1 +
>  4 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 5b03eee..da1f8fd 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1100,6 +1100,15 @@ emulate them efficiently. The fields in each entry are defined as follows:
>     eax, ebx, ecx, edx: the values returned by the cpuid instruction for
>           this function/index combination
>  
> +The TSC deadline timer feature (CPUID leaf 1, ecx[24]) is always returned
> +as false, since the feature depends on KVM_CREATE_IRQCHIP for local APIC
> +support.  Instead it is reported via
> +
> +  ioctl(KVM_CHECK_EXTENSION, KVM_CAP_TSC_DEADLINE_TIMER)
> +
> +if that returns true you use KVM_CREATE_IRQCHIP, or if emulate the
                      ^^^                               ^^^
                      and                               you
> +feature in userspace, then you can enable the feature for KVM_SET_CPUID2.
> +
>  4.47 KVM_PPC_GET_PVINFO
>  
>  Capability: KVM_CAP_PPC_GET_PVINFO
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 230f713..89b02bf 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -27,7 +27,6 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_cpuid_entry2 *best;
>  	struct kvm_lapic *apic = vcpu->arch.apic;
> -	u32 timer_mode_mask;
>  
>  	best = kvm_find_cpuid_entry(vcpu, 1, 0);
>  	if (!best)
> @@ -40,15 +39,12 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
>  			best->ecx |= bit(X86_FEATURE_OSXSAVE);
>  	}
>  
> -	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> -		best->function == 0x1) {
> -		best->ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);
> -		timer_mode_mask = 3 << 17;
> -	} else
> -		timer_mode_mask = 1 << 17;
> -
> -	if (apic)
> -		apic->lapic_timer.timer_mode_mask = timer_mode_mask;
> +	if (apic) {
> +		if (best->ecx & bit(X86_FEATURE_TSC_DEADLINE_TIMER))
> +			apic->lapic_timer.timer_mode_mask = 3 << 17;
> +		else
> +			apic->lapic_timer.timer_mode_mask = 1 << 17;
> +	}
>  
>  	kvm_pmu_cpuid_update(vcpu);
>  }
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index df23dff..1171def 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2089,6 +2089,9 @@ int kvm_dev_ioctl_check_extension(long ext)
>  	case KVM_CAP_TSC_CONTROL:
>  		r = kvm_has_tsc_control;
>  		break;
> +	case KVM_CAP_TSC_DEADLINE_TIMER:
> +		r = boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER);
> +		break;
>  	default:
>  		r = 0;
>  		break;
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index c3892fc..68e67e5 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -557,6 +557,7 @@ struct kvm_ppc_pvinfo {
>  #define KVM_CAP_MAX_VCPUS 66       /* returns max vcpus per vm */
>  #define KVM_CAP_PPC_PAPR 68
>  #define KVM_CAP_S390_GMAP 71
> +#define KVM_CAP_TSC_DEADLINE_TIMER 72
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  

Thanks,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Sasha Levin]

On Sun, 2011-12-25 at 15:03 +0200, Avi Kivity wrote:
> +The TSC deadline timer feature (CPUID leaf 1, ecx[24]) is always returned
> +as false, since the feature depends on KVM_CREATE_IRQCHIP for local APIC
> +support.  Instead it is reported via
> +
> +  ioctl(KVM_CHECK_EXTENSION, KVM_CAP_TSC_DEADLINE_TIMER)
> +
> +if that returns true you use KVM_CREATE_IRQCHIP, or if emulate the
> +feature in userspace, then you can enable the feature for KVM_SET_CPUID2.

Thats a bit strange, it's going to be a somewhat ugly special case in
userspace code. It also means we can't simply proxy through CPUID from
kernel to the guest and only disable things we don't support, we now
have to enable them as well.

> +	if (apic) {
> +		if (best->ecx & bit(X86_FEATURE_TSC_DEADLINE_TIMER))
> +			apic->lapic_timer.timer_mode_mask = 3 << 17;
> +		else
> +			apic->lapic_timer.timer_mode_mask = 1 << 17;
> +	}

Can we change these to be:

if(...)
	apic->lapic_timer.timer_mode_mask = APIC_LVT_TIMER_PERIODIC | APIC_LVT_TIMER_TSCDEADLINE;
else
	apic->lapic_timer.timer_mode_mask = APIC_LVT_TIMER_PERIODIC;

-- 

Sasha.

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Sasha Levin]

On Sun, 2011-12-25 at 21:00 +0200, Sasha Levin wrote:
> On Sun, 2011-12-25 at 15:03 +0200, Avi Kivity wrote:
> > +	if (apic) {
> > +		if (best->ecx & bit(X86_FEATURE_TSC_DEADLINE_TIMER))
> > +			apic->lapic_timer.timer_mode_mask = 3 << 17;
> > +		else
> > +			apic->lapic_timer.timer_mode_mask = 1 << 17;
> > +	}
> 
> Can we change these to be:
> 
> if(...)
> 	apic->lapic_timer.timer_mode_mask = APIC_LVT_TIMER_PERIODIC | APIC_LVT_TIMER_TSCDEADLINE;
> else
> 	apic->lapic_timer.timer_mode_mask = APIC_LVT_TIMER_PERIODIC;

Actually,

apic->lapic_timer.timer_mode_mask = APIC_LVT_TIMER_PERIODIC;
if(...)
	apic->lapic_timer.timer_mode_mask |= APIC_LVT_TIMER_TSCDEADLINE;

-- 

Sasha.

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Jan Kiszka]

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

On 2011-12-25 20:00, Sasha Levin wrote:
> On Sun, 2011-12-25 at 15:03 +0200, Avi Kivity wrote:
>> +The TSC deadline timer feature (CPUID leaf 1, ecx[24]) is always returned
>> +as false, since the feature depends on KVM_CREATE_IRQCHIP for local APIC
>> +support.  Instead it is reported via
>> +
>> +  ioctl(KVM_CHECK_EXTENSION, KVM_CAP_TSC_DEADLINE_TIMER)
>> +
>> +if that returns true you use KVM_CREATE_IRQCHIP, or if emulate the
>> +feature in userspace, then you can enable the feature for KVM_SET_CPUID2.
> 
> Thats a bit strange, it's going to be a somewhat ugly special case in
> userspace code.

Just add something like

if (kvm_has_cap_tscdeadline())
	set_cpuid_feature_bit(vcpu, tcs_deadline)

to the in-kernel apic setup code in userland.

> It also means we can't simply proxy through CPUID from
> kernel to the guest and only disable things we don't support, we now
> have to enable them as well.

That's unavoidable as you can't tell for sure if TSC_DEADLINE can be
supported at all times the user may invoke KVM_GET_SUPPORTED_CPUID -
that depends on the dynamic VM configuration.

Just proxying the bits through like kvmtool may do right now is a very
special case KVM can't support due to existing user space code and the
needs of migration.

> 
>> +	if (apic) {
>> +		if (best->ecx & bit(X86_FEATURE_TSC_DEADLINE_TIMER))
>> +			apic->lapic_timer.timer_mode_mask = 3 << 17;
>> +		else
>> +			apic->lapic_timer.timer_mode_mask = 1 << 17;
>> +	}
> 
> Can we change these to be:
> 
> if(...)
> 	apic->lapic_timer.timer_mode_mask = APIC_LVT_TIMER_PERIODIC | APIC_LVT_TIMER_TSCDEADLINE;
> else
> 	apic->lapic_timer.timer_mode_mask = APIC_LVT_TIMER_PERIODIC;
> 

Yep, using symbols is better. As the patch needs some polishing
regarding the doc wording, I think this could be cleaned up as well.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

RE: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Liu, Jinsong]

Avi Kivity wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Unlike all of the other cpuid bits, the TSC deadline timer bit is set
> unconditionally, regardless of what userspace wants.
> 
> This is broken in several ways:
>  - if userspace doesn't use KVM_CREATE_IRQCHIP, and doesn't emulate
>    the TSC deadline timer feature, a guest that uses the feature will
>  break - live migration to older host kernels that don't support the
>    TSC deadline timer will cause the feature to be pulled from under
>    the guest's feet; breaking it
>  - guests that are broken wrt the feature will fail.
> 
> Fix by not enabling the feature automatically; instead report it to
> userspace. 
> Because the feature depends on KVM_CREATE_IRQCHIP, which we cannot
> guarantee 
> will be called, we expose it via a KVM_CAP_TSC_DEADLINE_TIMER and not
> KVM_GET_SUPPORTED_CPUID.
> 
> Fixes the Illumos guest kernel, which uses the TSC deadline timer
> feature. 
> 
> [avi: add the KVM_CAP + documentation]
> 
> Reported-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
> 
> As we're running out of time and everyone's checking their socks
> instead of 
> inboxes I've added the missing parts myself.  Jan, if you
> accidentally see 
> this, please review and add your signoff.
> 
>  Documentation/virtual/kvm/api.txt |    9 +++++++++
>  arch/x86/kvm/cpuid.c              |   16 ++++++----------
>  arch/x86/kvm/x86.c                |    3 +++
>  include/linux/kvm.h               |    1 +
>  4 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt
> b/Documentation/virtual/kvm/api.txt 
> index 5b03eee..da1f8fd 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1100,6 +1100,15 @@ emulate them efficiently. The fields in each
>     entry are defined as follows: eax, ebx, ecx, edx: the values
>           returned by the cpuid instruction for this function/index
> combination 
> 
> +The TSC deadline timer feature (CPUID leaf 1, ecx[24]) is always
> returned +as false, since the feature depends on KVM_CREATE_IRQCHIP
> for local APIC +support.  Instead it is reported via
> +
> +  ioctl(KVM_CHECK_EXTENSION, KVM_CAP_TSC_DEADLINE_TIMER)
> +
> +if that returns true you use KVM_CREATE_IRQCHIP, or if emulate the
> +feature in userspace, then you can enable the feature for
> KVM_SET_CPUID2. +
>  4.47 KVM_PPC_GET_PVINFO
> 
>  Capability: KVM_CAP_PPC_GET_PVINFO
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 230f713..89b02bf 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -27,7 +27,6 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_cpuid_entry2 *best;
>  	struct kvm_lapic *apic = vcpu->arch.apic;
> -	u32 timer_mode_mask;
> 
>  	best = kvm_find_cpuid_entry(vcpu, 1, 0);
>  	if (!best)
> @@ -40,15 +39,12 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
>  			best->ecx |= bit(X86_FEATURE_OSXSAVE);
>  	}
> 
> -	if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
> -		best->function == 0x1) {
> -		best->ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER);
> -		timer_mode_mask = 3 << 17;
> -	} else
> -		timer_mode_mask = 1 << 17;
> -
> -	if (apic)
> -		apic->lapic_timer.timer_mode_mask = timer_mode_mask;
> +	if (apic) {
> +		if (best->ecx & bit(X86_FEATURE_TSC_DEADLINE_TIMER))
> +			apic->lapic_timer.timer_mode_mask = 3 << 17;
> +		else
> +			apic->lapic_timer.timer_mode_mask = 1 << 17;
> +	}
> 
>  	kvm_pmu_cpuid_update(vcpu);
>  }
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index df23dff..1171def 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2089,6 +2089,9 @@ int kvm_dev_ioctl_check_extension(long ext)
>  	case KVM_CAP_TSC_CONTROL:
>  		r = kvm_has_tsc_control;
>  		break;
> +	case KVM_CAP_TSC_DEADLINE_TIMER:
> +		r = boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER);
> +		break;

kvm tsc deadline timer is pure software emulated, not depend on host physically.

Thanks,
Jinsong

>  	default:
>  		r = 0;
>  		break;
> diff --git a/include/linux/kvm.h b/include/linux/kvm.h
> index c3892fc..68e67e5 100644
> --- a/include/linux/kvm.h
> +++ b/include/linux/kvm.h
> @@ -557,6 +557,7 @@ struct kvm_ppc_pvinfo {
>  #define KVM_CAP_MAX_VCPUS 66       /* returns max vcpus per vm */
>  #define KVM_CAP_PPC_PAPR 68
>  #define KVM_CAP_S390_GMAP 71
> +#define KVM_CAP_TSC_DEADLINE_TIMER 72
> 
>  #ifdef KVM_CAP_IRQ_ROUTING

RE: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Liu, Jinsong]

Sasha Levin wrote:
> On Sun, 2011-12-25 at 21:00 +0200, Sasha Levin wrote:
>> On Sun, 2011-12-25 at 15:03 +0200, Avi Kivity wrote:
>>> +	if (apic) {
>>> +		if (best->ecx & bit(X86_FEATURE_TSC_DEADLINE_TIMER))
>>> +			apic->lapic_timer.timer_mode_mask = 3 << 17;
>>> +		else
>>> +			apic->lapic_timer.timer_mode_mask = 1 << 17;
>>> +	}
>> 
>> Can we change these to be:
>> 
>> if(...)
>> 	apic->lapic_timer.timer_mode_mask = APIC_LVT_TIMER_PERIODIC |
>> 	APIC_LVT_TIMER_TSCDEADLINE; else apic->lapic_timer.timer_mode_mask
>> = APIC_LVT_TIMER_PERIODIC; 
> 
> Actually,
> 
> apic->lapic_timer.timer_mode_mask = APIC_LVT_TIMER_PERIODIC;
> if(...)
> 	apic->lapic_timer.timer_mode_mask |= APIC_LVT_TIMER_TSCDEADLINE;

Is it good semantically? APIC_LVT_TIMER_PERIODIC and APIC_LVT_TIMER_TSCDEADLINE is timer mode, where timer_mode_mask is timer mode mask.

Thanks,
Jinsong

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Avi Kivity]

On 12/26/2011 10:42 AM, Liu, Jinsong wrote:
> >  }
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index df23dff..1171def 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2089,6 +2089,9 @@ int kvm_dev_ioctl_check_extension(long ext)
> >  	case KVM_CAP_TSC_CONTROL:
> >  		r = kvm_has_tsc_control;
> >  		break;
> > +	case KVM_CAP_TSC_DEADLINE_TIMER:
> > +		r = boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER);
> > +		break;
>
> kvm tsc deadline timer is pure software emulated, not depend on host physically.

Yeah, I want to reconsider it in the 3.3 cycle.  I'm not so hot on
exposing non hardware features anymore, at least we want better
reporting to userspace.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Alexey Zaytsev]

On Mon, Dec 26, 2011 at 12:35, Avi Kivity <avi@redhat.com> wrote:
> On 12/26/2011 10:42 AM, Liu, Jinsong wrote:
>> >  }
>> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> > index df23dff..1171def 100644
>> > --- a/arch/x86/kvm/x86.c
>> > +++ b/arch/x86/kvm/x86.c
>> > @@ -2089,6 +2089,9 @@ int kvm_dev_ioctl_check_extension(long ext)
>> >     case KVM_CAP_TSC_CONTROL:
>> >             r = kvm_has_tsc_control;
>> >             break;
>> > +   case KVM_CAP_TSC_DEADLINE_TIMER:
>> > +           r = boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER);
>> > +           break;
>>
>> kvm tsc deadline timer is pure software emulated, not depend on host physically.
>
> Yeah, I want to reconsider it in the 3.3 cycle.  I'm not so hot on
> exposing non hardware features anymore, at least we want better
> reporting to userspace.
>

Just as a reminder, could you please revert the problematic commit
(a3e06bbe) for 3.2?

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Avi Kivity]

On 12/26/2011 12:43 PM, Alexey Zaytsev wrote:
> On Mon, Dec 26, 2011 at 12:35, Avi Kivity <avi@redhat.com> wrote:
> > On 12/26/2011 10:42 AM, Liu, Jinsong wrote:
> >> >  }
> >> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >> > index df23dff..1171def 100644
> >> > --- a/arch/x86/kvm/x86.c
> >> > +++ b/arch/x86/kvm/x86.c
> >> > @@ -2089,6 +2089,9 @@ int kvm_dev_ioctl_check_extension(long ext)
> >> >     case KVM_CAP_TSC_CONTROL:
> >> >             r = kvm_has_tsc_control;
> >> >             break;
> >> > +   case KVM_CAP_TSC_DEADLINE_TIMER:
> >> > +           r = boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER);
> >> > +           break;
> >>
> >> kvm tsc deadline timer is pure software emulated, not depend on host physically.
> >
> > Yeah, I want to reconsider it in the 3.3 cycle.  I'm not so hot on
> > exposing non hardware features anymore, at least we want better
> > reporting to userspace.
> >
>
> Just as a reminder, could you please revert the problematic commit
> (a3e06bbe) for 3.2?

I plan to post this as a fix.

If  you can, please test it:

  git://git.kernel.org/pub/scm/virt/kvm/kvm.git kvm-updates/3.2

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Avi Kivity]

On 12/25/2011 08:39 PM, Jan Kiszka wrote:
> On 2011-12-25 14:03, Avi Kivity wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Unlike all of the other cpuid bits, the TSC deadline timer bit is set
> > unconditionally, regardless of what userspace wants.
> > 
> > This is broken in several ways:
> >  - if userspace doesn't use KVM_CREATE_IRQCHIP, and doesn't emulate the TSC
> >    deadline timer feature, a guest that uses the feature will break
> >  - live migration to older host kernels that don't support the TSC deadline
> >    timer will cause the feature to be pulled from under the guest's feet;
> >    breaking it
> >  - guests that are broken wrt the feature will fail.
> > 
> > Fix by not enabling the feature automatically; instead report it to userspace.
> > Because the feature depends on KVM_CREATE_IRQCHIP, which we cannot guarantee
> > will be called, we expose it via a KVM_CAP_TSC_DEADLINE_TIMER and not
> > KVM_GET_SUPPORTED_CPUID.
> > 
> > Fixes the Illumos guest kernel, which uses the TSC deadline timer feature.
> > 
> > [avi: add the KVM_CAP + documentation]
> > 
> > Reported-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
> > Signed-off-by: Avi Kivity <avi@redhat.com>
> > ---
> > 
> > As we're running out of time and everyone's checking their socks instead of
> > inboxes I've added the missing parts myself.  Jan, if you accidentally see
> > this, please review and add your signoff.
>
> I'm sorry for not holding my promise, was distracted the past days.
> Patch looks good to me, just some minor phrasing corrections below.

Not a problem at all, I can guess you had much better things to do.

>
> Signed-off-by; Jan Kiszka <jan.kiszka@siemens.com>
>
> > 
> >  Documentation/virtual/kvm/api.txt |    9 +++++++++
> >  arch/x86/kvm/cpuid.c              |   16 ++++++----------
> >  arch/x86/kvm/x86.c                |    3 +++
> >  include/linux/kvm.h               |    1 +
> >  4 files changed, 19 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> > index 5b03eee..da1f8fd 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -1100,6 +1100,15 @@ emulate them efficiently. The fields in each entry are defined as follows:
> >     eax, ebx, ecx, edx: the values returned by the cpuid instruction for
> >           this function/index combination
> >  
> > +The TSC deadline timer feature (CPUID leaf 1, ecx[24]) is always returned
> > +as false, since the feature depends on KVM_CREATE_IRQCHIP for local APIC
> > +support.  Instead it is reported via
> > +
> > +  ioctl(KVM_CHECK_EXTENSION, KVM_CAP_TSC_DEADLINE_TIMER)
> > +
> > +if that returns true you use KVM_CREATE_IRQCHIP, or if emulate the
>                       ^^^                               ^^^
>                       and                               you
>

Thanks, added your signoff and adjusted this.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Alexey Zaytsev]

On Mon, Dec 26, 2011 at 12:45, Avi Kivity <avi@redhat.com> wrote:
> On 12/26/2011 12:43 PM, Alexey Zaytsev wrote:
>> On Mon, Dec 26, 2011 at 12:35, Avi Kivity <avi@redhat.com> wrote:
>> > On 12/26/2011 10:42 AM, Liu, Jinsong wrote:
>> >> >  }
>> >> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> >> > index df23dff..1171def 100644
>> >> > --- a/arch/x86/kvm/x86.c
>> >> > +++ b/arch/x86/kvm/x86.c
>> >> > @@ -2089,6 +2089,9 @@ int kvm_dev_ioctl_check_extension(long ext)
>> >> >     case KVM_CAP_TSC_CONTROL:
>> >> >             r = kvm_has_tsc_control;
>> >> >             break;
>> >> > +   case KVM_CAP_TSC_DEADLINE_TIMER:
>> >> > +           r = boot_cpu_has(X86_FEATURE_TSC_DEADLINE_TIMER);
>> >> > +           break;
>> >>
>> >> kvm tsc deadline timer is pure software emulated, not depend on host physically.
>> >
>> > Yeah, I want to reconsider it in the 3.3 cycle.  I'm not so hot on
>> > exposing non hardware features anymore, at least we want better
>> > reporting to userspace.
>> >
>>
>> Just as a reminder, could you please revert the problematic commit
>> (a3e06bbe) for 3.2?
>
> I plan to post this as a fix.
>
> If  you can, please test it:
>
>  git://git.kernel.org/pub/scm/virt/kvm/kvm.git kvm-updates/3.2
>

Works for me, thank you.

Re: [PATCH] KVM: Don't automatically expose the TSC deadline timer in cpuid

[Avi Kivity]

On 12/26/2011 01:25 PM, Alexey Zaytsev wrote:
> >
> > If  you can, please test it:
> >
> >  git://git.kernel.org/pub/scm/virt/kvm/kvm.git kvm-updates/3.2
> >
>
> Works for me, thank you.

Thanks, added your Tested-by:.

-- 
error compiling committee.c: too many arguments to function