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:
signature.asc
Description: OpenPGP digital signature
