KVM: x86: disable kvmclock on non constant TSC hosts

KVM: x86: disable kvmclock on non constant TSC hosts

[Marcelo Tosatti]

Currently, this code path is posing us big troubles,
and we won't have a decent patch in time. So, temporarily
disable it.

There's a module parameter for the adventurous who want to force
it.

Signed-off-by: Glauber Costa <glommer@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 arch/x86/kvm/x86.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e96edda..6286f08 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -979,6 +979,9 @@ out:
 	return r;
 }
 
+static int force_kvmclock = 0;
+module_param(force_kvmclock, bool, 0644);
+
 int kvm_dev_ioctl_check_extension(long ext)
 {
 	int r;
@@ -989,7 +992,6 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
 	case KVM_CAP_SET_TSS_ADDR:
 	case KVM_CAP_EXT_CPUID:
-	case KVM_CAP_CLOCKSOURCE:
 	case KVM_CAP_PIT:
 	case KVM_CAP_NOP_IO_DELAY:
 	case KVM_CAP_MP_STATE:
@@ -1015,6 +1017,9 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_IOMMU:
 		r = iommu_found();
 		break;
+	case KVM_CAP_CLOCKSOURCE:
+		r = force_kvmclock || boot_cpu_has(X86_FEATURE_CONSTANT_TSC);
+		break;
 	default:
 		r = 0;
 		break;

KVM: x86: disable kvmclock on non constant TSC hosts

[Marcelo Tosatti]

This is better.

Currently, this code path is posing us big troubles,
and we won't have a decent patch in time. So, temporarily
disable it.
 
Signed-off-by: Glauber Costa <glommer@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/x86.c
===================================================================
--- kvm.orig/arch/x86/kvm/x86.c
+++ kvm/arch/x86/kvm/x86.c
@@ -989,7 +989,6 @@ int kvm_dev_ioctl_check_extension(long e
 	case KVM_CAP_MMU_SHADOW_CACHE_CONTROL:
 	case KVM_CAP_SET_TSS_ADDR:
 	case KVM_CAP_EXT_CPUID:
-	case KVM_CAP_CLOCKSOURCE:
 	case KVM_CAP_PIT:
 	case KVM_CAP_NOP_IO_DELAY:
 	case KVM_CAP_MP_STATE:
@@ -1015,6 +1014,9 @@ int kvm_dev_ioctl_check_extension(long e
 	case KVM_CAP_IOMMU:
 		r = iommu_found();
 		break;
+	case KVM_CAP_CLOCKSOURCE:
+		r = boot_cpu_has(X86_FEATURE_CONSTANT_TSC);
+		break;
 	default:
 		r = 0;
 		break;

Re: KVM: x86: disable kvmclock on non constant TSC hosts

[Avi Kivity]

Marcelo Tosatti wrote:
> This is better.
>
> Currently, this code path is posing us big troubles,
> and we won't have a decent patch in time. So, temporarily
> disable it.
>  
>   

Applied to master and kvm-updates/2.6.29, thanks.

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

Re: KVM: x86: disable kvmclock on non constant TSC hosts

[Mark McLoughlin]

On Wed, 2009-02-11 at 14:01 +0200, Avi Kivity wrote:
> Marcelo Tosatti wrote:
> > This is better.
> >
> > Currently, this code path is posing us big troubles,
> > and we won't have a decent patch in time. So, temporarily
> > disable it.
> >  
> >   
> 
> Applied to master and kvm-updates/2.6.29, thanks.

Should we add:

Cc: stable@kernel.org

Cheers,
Mark.

Re: KVM: x86: disable kvmclock on non constant TSC hosts

[Marcelo Tosatti]

On Wed, Feb 11, 2009 at 08:08:23PM +0000, Mark McLoughlin wrote:
> On Wed, 2009-02-11 at 14:01 +0200, Avi Kivity wrote:
> > Marcelo Tosatti wrote:
> > > This is better.
> > >
> > > Currently, this code path is posing us big troubles,
> > > and we won't have a decent patch in time. So, temporarily
> > > disable it.
> > >  
> > >   
> > 
> > Applied to master and kvm-updates/2.6.29, thanks.
> 
> Should we add:
> 
> Cc: stable@kernel.org

I suppose Avi prefers the master->stable route so it can get 
autotested?

But some sort of flag in the commit, like "Stable:Y" would
help. Could setup a robot to monitor kvm-commits and maintain
kvm-updates/2.6.2current (the queue to Linus), with human intervention
when necessary.

Then autotest can run continuously on that.

Re: KVM: x86: disable kvmclock on non constant TSC hosts

[Avi Kivity]

Marcelo Tosatti wrote:
> On Wed, Feb 11, 2009 at 08:08:23PM +0000, Mark McLoughlin wrote:
>   
>> On Wed, 2009-02-11 at 14:01 +0200, Avi Kivity wrote:
>>     
>>> Marcelo Tosatti wrote:
>>>       
>>>> This is better.
>>>>
>>>> Currently, this code path is posing us big troubles,
>>>> and we won't have a decent patch in time. So, temporarily
>>>> disable it.
>>>>  
>>>>   
>>>>         
>>> Applied to master and kvm-updates/2.6.29, thanks.
>>>       
>> Should we add:
>>
>> Cc: stable@kernel.org
>>     
>
> I suppose Avi prefers the master->stable route so it can get 
> autotested?
>
> But some sort of flag in the commit, like "Stable:Y" would
> help. Could setup a robot to monitor kvm-commits and maintain
> kvm-updates/2.6.2current (the queue to Linus), with human intervention
> when necessary.
>
> Then autotest can run continuously on that.
>   

Does 'Cc: stable' mean -stable picks it up automatically?  That doesn't 
seem right.


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

Re: KVM: x86: disable kvmclock on non constant TSC hosts

[Mark McLoughlin]

On Thu, 2009-02-12 at 12:02 +0200, Avi Kivity wrote:
> Marcelo Tosatti wrote:
> > On Wed, Feb 11, 2009 at 08:08:23PM +0000, Mark McLoughlin wrote:
> >   
> >> On Wed, 2009-02-11 at 14:01 +0200, Avi Kivity wrote:
> >>     
> >>> Marcelo Tosatti wrote:
> >>>       
> >>>> This is better.
> >>>>
> >>>> Currently, this code path is posing us big troubles,
> >>>> and we won't have a decent patch in time. So, temporarily
> >>>> disable it.
> >>>>  
> >>>>   
> >>>>         
> >>> Applied to master and kvm-updates/2.6.29, thanks.
> >>>       
> >> Should we add:
> >>
> >> Cc: stable@kernel.org
> >>     
> >
> > I suppose Avi prefers the master->stable route so it can get 
> > autotested?
> >
> > But some sort of flag in the commit, like "Stable:Y" would
> > help. Could setup a robot to monitor kvm-commits and maintain
> > kvm-updates/2.6.2current (the queue to Linus), with human intervention
> > when necessary.
> >
> > Then autotest can run continuously on that.

I'm not really following Marcelo here ...

> Does 'Cc: stable' mean -stable picks it up automatically?  That doesn't 
> seem right.

Yeah, I think if you CC: stable it gets pulled into the stable queue by
scripts once linus merges it.

Cheers,
Mark.

Re: KVM: x86: disable kvmclock on non constant TSC hosts

[Chris Wright]

* Avi Kivity (avi@redhat.com) wrote:
> Does 'Cc: stable' mean -stable picks it up automatically?  That doesn't  
> seem right.

Yes.  If you add "Cc: stable@kernel.org" to the commit message, it will
get routed to stable when it's committed upstream (still gets human
touch, it's not automatically committed to stable).

thanks,
-chris

Re: KVM: x86: disable kvmclock on non constant TSC hosts

[Avi Kivity]

Chris Wright wrote:
> * Avi Kivity (avi@redhat.com) wrote:
>   
>> Does 'Cc: stable' mean -stable picks it up automatically?  That doesn't  
>> seem right.
>>     
>
> Yes.  If you add "Cc: stable@kernel.org" to the commit message, it will
> get routed to stable when it's committed upstream (still gets human
> touch, it's not automatically committed to stable).
>   

What about porting the patch?  And testing it?

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

Re: KVM: x86: disable kvmclock on non constant TSC hosts

[Chris Wright]

* Avi Kivity (avi@redhat.com) wrote:
> Chris Wright wrote:
>> * Avi Kivity (avi@redhat.com) wrote:
>>   
>>> Does 'Cc: stable' mean -stable picks it up automatically?  That 
>>> doesn't  seem right.
>>
>> Yes.  If you add "Cc: stable@kernel.org" to the commit message, it will
>> get routed to stable when it's committed upstream (still gets human
>> touch, it's not automatically committed to stable).
>
> What about porting the patch?  And testing it?

Porting is done by stable maintainers if it's trivial, push back to
developer/subsytem maintainer if it's not.  Testing depends again, in
most cases the upstream testing is still valid.  If not..."please
verify"

Re: KVM: x86: disable kvmclock on non constant TSC hosts

[Avi Kivity]

Chris Wright wrote:
> Porting is done by stable maintainers if it's trivial, push back to
> developer/subsytem maintainer if it's not.  Testing depends again, in
> most cases the upstream testing is still valid.  If not..."please
> verify"
>   

Warm, fuzzy feeling.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.