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

[Avi Kivity <avi@redhat.com>]

On 12/25/2011 08:39 PM, Jan Kiszka wrote:
> On 2011-12-25 14:03, Avi Kivity wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Unlike all of the other cpuid bits, the TSC deadline timer bit is set
> > unconditionally, regardless of what userspace wants.
> > 
> > This is broken in several ways:
> >  - if userspace doesn't use KVM_CREATE_IRQCHIP, and doesn't emulate the TSC
> >    deadline timer feature, a guest that uses the feature will break
> >  - live migration to older host kernels that don't support the TSC deadline
> >    timer will cause the feature to be pulled from under the guest's feet;
> >    breaking it
> >  - guests that are broken wrt the feature will fail.
> > 
> > Fix by not enabling the feature automatically; instead report it to userspace.
> > 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.
> > 
> > Fixes the Illumos guest kernel, which uses the TSC deadline timer feature.
> > 
> > [avi: add the KVM_CAP + documentation]
> > 
> > Reported-by: Alexey Zaytsev <alexey.zaytsev@gmail.com>
> > Signed-off-by: Avi Kivity <avi@redhat.com>
> > ---
> > 
> > As we're running out of time and everyone's checking their socks instead of
> > inboxes I've added the missing parts myself.  Jan, if you accidentally see
> > this, please review and add your signoff.
>
> I'm sorry for not holding my promise, was distracted the past days.
> Patch looks good to me, just some minor phrasing corrections below.

Not a problem at all, I can guess you had much better things to do.

>
> Signed-off-by; Jan Kiszka <jan.kiszka@siemens.com>
>
> > 
> >  Documentation/virtual/kvm/api.txt |    9 +++++++++
> >  arch/x86/kvm/cpuid.c              |   16 ++++++----------
> >  arch/x86/kvm/x86.c                |    3 +++
> >  include/linux/kvm.h               |    1 +
> >  4 files changed, 19 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> > index 5b03eee..da1f8fd 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -1100,6 +1100,15 @@ emulate them efficiently. The fields in each entry are defined as follows:
> >     eax, ebx, ecx, edx: the values returned by the cpuid instruction for
> >           this function/index combination
> >  
> > +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
>                       ^^^                               ^^^
>                       and                               you
>

Thanks, added your signoff and adjusted this.

-- 
error compiling committee.c: too many arguments to function