From: "Liu, Jinsong" <jinsong.liu@intel.com> To: Eduardo Habkost <ehabkost@redhat.com> Cc: Rik van Riel <riel@redhat.com>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, Jan Kiszka <jan.kiszka@siemens.com>, Avi Kivity <avi@redhat.com> Subject: RE: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest Date: Fri, 23 Mar 2012 03:49:27 +0000 [thread overview] Message-ID: <DE8DF0795D48FD4CA783C40EC8292335101001@SHSMSX101.ccr.corp.intel.com> (raw) In-Reply-To: <20120320133314.GV9375@otherpad.lan.raisama.net> Eduardo Habkost wrote: > 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. > Yes, exactly. That's why we need this patch. > > [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)." The fixbug patch is implemented by Jan and Avi, I reply per my understanding. I think for tsc deadline timer feature, KVM_CAP_TSC_DEADLINE_TIMER is slightly better than KVM_GET_SUPPORTED_CPUID. If use KVM_GET_SUPPORTED_CPUID, it means tsc deadline features bind to host cpuid, while it fact it could be pure software emulated by kvm (though currently it implemented as bound to hareware). For the sake of extension, it choose KVM_CAP_TSC_DEADLINE_TIMER. Thanks, Jinsong
WARNING: multiple messages have this Message-ID (diff)
From: "Liu, Jinsong" <jinsong.liu@intel.com> To: Eduardo Habkost <ehabkost@redhat.com> Cc: Jan Kiszka <jan.kiszka@siemens.com>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, Avi Kivity <avi@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest Date: Fri, 23 Mar 2012 03:49:27 +0000 [thread overview] Message-ID: <DE8DF0795D48FD4CA783C40EC8292335101001@SHSMSX101.ccr.corp.intel.com> (raw) In-Reply-To: <20120320133314.GV9375@otherpad.lan.raisama.net> Eduardo Habkost wrote: > 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. > Yes, exactly. That's why we need this patch. > > [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)." The fixbug patch is implemented by Jan and Avi, I reply per my understanding. I think for tsc deadline timer feature, KVM_CAP_TSC_DEADLINE_TIMER is slightly better than KVM_GET_SUPPORTED_CPUID. If use KVM_GET_SUPPORTED_CPUID, it means tsc deadline features bind to host cpuid, while it fact it could be pure software emulated by kvm (though currently it implemented as bound to hareware). For the sake of extension, it choose KVM_CAP_TSC_DEADLINE_TIMER. Thanks, Jinsong
next prev parent reply other threads:[~2012-03-23 3:49 UTC|newest] Thread overview: 100+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-12-28 21:55 [PATCH 2/2] Expose tsc deadline timer cpuid to guest Liu, Jinsong 2011-12-28 21:55 ` [Qemu-devel] " Liu, Jinsong 2012-01-04 16:48 ` Jan Kiszka 2012-01-04 16:48 ` [Qemu-devel] " Jan Kiszka 2012-01-05 20:07 ` Liu, Jinsong 2012-01-05 20:07 ` [Qemu-devel] " Liu, Jinsong 2012-01-05 20:34 ` Jan Kiszka 2012-01-05 20:34 ` [Qemu-devel] " Jan Kiszka 2012-01-07 18:23 ` Liu, Jinsong 2012-01-07 18:23 ` [Qemu-devel] " Liu, Jinsong 2012-01-08 21:24 ` Jan Kiszka 2012-01-08 21:24 ` [Qemu-devel] " Jan Kiszka 2012-02-27 16:05 ` Liu, Jinsong 2012-02-27 16:05 ` [Qemu-devel] " Liu, Jinsong 2012-02-27 17:18 ` Jan Kiszka 2012-02-27 17:18 ` [Qemu-devel] " Jan Kiszka 2012-02-28 10:30 ` Liu, Jinsong 2012-02-28 10:30 ` [Qemu-devel] " Liu, Jinsong 2012-03-06 7:49 ` Liu, Jinsong 2012-03-06 7:49 ` Liu, Jinsong 2012-03-06 10:14 ` Jan Kiszka 2012-03-06 10:14 ` Jan Kiszka 2012-03-09 18:27 ` Liu, Jinsong 2012-03-09 18:27 ` [Qemu-devel] " Liu, Jinsong 2012-03-09 18:56 ` Jan Kiszka 2012-03-09 18:56 ` Jan Kiszka 2012-03-09 19:09 ` Liu, Jinsong 2012-03-09 19:09 ` Liu, Jinsong 2012-03-09 20:52 ` Jan Kiszka 2012-03-09 20:52 ` Jan Kiszka 2012-03-10 1:07 ` Andreas Färber 2012-03-10 1:07 ` Andreas Färber 2012-03-11 18:54 ` Liu, Jinsong 2012-03-11 18:54 ` Liu, Jinsong 2012-03-12 17:21 ` Eduardo Habkost 2012-03-25 8:51 ` Liu, Jinsong 2012-03-25 8:51 ` [Qemu-devel] " Liu, Jinsong 2012-03-09 19:29 ` Liu, Jinsong 2012-03-09 19:29 ` [Qemu-devel] " Liu, Jinsong 2012-03-19 22:35 ` Rik van Riel 2012-03-20 12:53 ` Liu, Jinsong 2012-03-20 13:33 ` Eduardo Habkost 2012-03-20 13:33 ` Eduardo Habkost 2012-03-23 3:49 ` Liu, Jinsong [this message] 2012-03-23 3:49 ` Liu, Jinsong 2012-03-23 13:46 ` Eduardo Habkost 2012-03-23 13:46 ` Eduardo Habkost 2012-03-23 14:17 ` Liu, Jinsong 2012-03-23 14:17 ` Liu, Jinsong 2012-04-19 20:03 ` Eduardo Habkost 2012-04-20 10:12 ` Jan Kiszka 2012-04-20 15:00 ` Eduardo Habkost 2012-04-20 15:19 ` [Qemu-devel] " Jan Kiszka 2012-04-20 15:36 ` Eduardo Habkost 2012-04-21 7:23 ` Jan Kiszka 2012-04-23 14:48 ` Eduardo Habkost 2012-04-23 16:31 ` Jan Kiszka 2012-04-23 20:02 ` Eduardo Habkost 2012-04-24 16:06 ` Jan Kiszka 2012-04-24 17:19 ` [Qemu-devel] " Eduardo Habkost 2012-05-07 18:21 ` Semantics of "-cpu host" (was Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest) Eduardo Habkost 2012-05-07 18:21 ` [Qemu-devel] Semantics of "-cpu host" (was " Eduardo Habkost 2012-05-08 0:58 ` Alexander Graf 2012-05-08 0:58 ` [Qemu-devel] " Alexander Graf 2012-05-08 20:14 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Eduardo Habkost 2012-05-08 20:14 ` [Qemu-devel] Semantics of "-cpu host" (was " Eduardo Habkost 2012-05-08 22:07 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf 2012-05-08 22:07 ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf 2012-05-09 8:14 ` Gleb Natapov 2012-05-09 8:14 ` [Qemu-devel] " Gleb Natapov 2012-05-09 8:42 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf 2012-05-09 8:42 ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf 2012-05-09 8:51 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Gleb Natapov 2012-05-09 8:51 ` [Qemu-devel] Semantics of "-cpu host" (was " Gleb Natapov 2012-05-09 9:05 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf 2012-05-09 9:05 ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf 2012-05-09 9:38 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Gleb Natapov 2012-05-09 9:38 ` [Qemu-devel] Semantics of "-cpu host" (was " Gleb Natapov 2012-05-09 9:54 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf 2012-05-09 9:54 ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf 2012-05-09 19:38 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Eduardo Habkost 2012-05-09 19:38 ` [Qemu-devel] Semantics of "-cpu host" (was " Eduardo Habkost 2012-05-09 20:30 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf 2012-05-09 20:30 ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf 2012-05-10 12:53 ` Gleb Natapov 2012-05-10 12:53 ` [Qemu-devel] " Gleb Natapov 2012-05-10 13:21 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Alexander Graf 2012-05-10 13:21 ` [Qemu-devel] Semantics of "-cpu host" (was " Alexander Graf 2012-05-10 13:39 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Gleb Natapov 2012-05-10 13:39 ` [Qemu-devel] Semantics of "-cpu host" (was " Gleb Natapov 2012-05-10 14:12 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Eduardo Habkost 2012-05-10 14:12 ` [Qemu-devel] Semantics of "-cpu host" (was " Eduardo Habkost 2012-05-09 7:16 ` Semantics of "-cpu host" (was Re: [Qemu-devel] " Andre Przywara 2012-05-09 7:16 ` [Qemu-devel] Semantics of "-cpu host" (was " Andre Przywara 2012-06-14 19:02 ` [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest Liu, Jinsong 2012-06-14 19:02 ` Liu, Jinsong 2012-06-14 19:12 ` Eduardo Habkost 2012-06-14 19:12 ` Eduardo Habkost 2012-06-14 19:18 ` Liu, Jinsong 2012-06-14 19:18 ` [Qemu-devel] " Liu, Jinsong
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=DE8DF0795D48FD4CA783C40EC8292335101001@SHSMSX101.ccr.corp.intel.com \ --to=jinsong.liu@intel.com \ --cc=avi@redhat.com \ --cc=ehabkost@redhat.com \ --cc=jan.kiszka@siemens.com \ --cc=kvm@vger.kernel.org \ --cc=qemu-devel@nongnu.org \ --cc=riel@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.