qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest


From: Eduardo Habkost
Subject: Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest
Date: Tue, 20 Mar 2012 10:33:15 -0300
User-agent: Mutt/1.5.21 (2010-09-15)

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]