On Tue, Mar 20, 2012 at 12:53:57PM +0000, Liu, Jinsong wrote: > Rik van Riel wrote: > > On 03/09/2012 01:27 PM, Liu, Jinsong wrote: > > > >> As for 'tsc deadline' feature exposing, my patch (as attached) just > >> obey qemu general cpuid exposing method, and also satisfied your > >> target I think. > > > > One question. > > > > Why is TSC_DEADLINE not exposed in the cpuid allowed feature > > bits in do_cpuid_ent() in arch/x86/kvm/x86.c ? > > > > /* cpuid 1.ecx */ > > const u32 kvm_supported_word4_x86_features = > > F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ | > > 0 /* DS-CPL, VMX, SMX, EST */ | > > 0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /* > > Reserved */ | > > F(FMA) | F(CX16) | 0 /* xTPR Update, PDCM */ | > > 0 /* Reserved, DCA */ | F(XMM4_1) | > > F(XMM4_2) | F(X2APIC) | F(MOVBE) | F(POPCNT) | > > 0 /* Reserved*/ | F(AES) | F(XSAVE) | 0 /* OSXSAVE > > */ | F(AVX) | > > F(F16C) | F(RDRAND); > > > > Would it make sense to expose F(TSC_DEADLINE) above? > > > > Or is there something truly special about tsc deadline > > that means it should be different from everything else? > > 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. We have many other features that depend on proper support from userspace otherwise they wouldn't work, but are listed on GET_SUPPORTED_CPUID, don't we? Why is TSC-deadline special? GET_SUPPORTED_CPUID just means "KVM supports it as long as userspace supports it too and enables it", it doesn't mean "CPUID bit that will be enabled by default"[1]. > Refer changeset 4d25a066b69fb749a39d0d4c610689dd765a0b0e. That changeset was necessary because the kernel was doing this on update_cpu if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL && best->function == 0x1) { best->ecx |= bit(X86_FEATURE_TSC_DEADLINE_TIMER); And that was really wrong, because it enabled the bit unconditionally. But I don't understand why KVM_CAP_TSC_DEADLINE_TIMER was created if we already have KVM_GET_SUPPORTED_CPUID to tell userspace which bits are supported by KVM. [1] From Documentation/virtual/kvm/api.txt: "KVM_GET_SUPPORTED_CPUID [...] This ioctl returns x86 cpuid features which are supported by both the hardware and kvm. Userspace can use the information returned by this ioctl to construct cpuid information (for KVM_SET_CPUID2) that is consistent with hardware, kernel, and userspace capabilities, and with ^^^^^^^^^^^^^^^^^^^^^^^^^^ user requirements (for example, the user may wish to constrain cpuid to emulate older hardware, or for feature consistency across a cluster)." -- Eduardo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html