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

[Jan Kiszka <jan.kiszka@web.de>]

[-- 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 --]