From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Subject: Re: [PATCH v4 3/8] KVM Guest: Add VCPU running/pre-empted state for guest
Date: Fri, 24 Aug 2012 11:09:39 +0530
Message-ID: <87vcg8q25g.fsf@linux.vnet.ibm.com>
References: <20120821112346.3512.99814.stgit@abhimanyu.in.ibm.com> <20120821112630.3512.5109.stgit@abhimanyu> <20120823093643.GC32710@amt.cnet>
Mime-Version: 1.0
Content-Type: text/plain
Cc: avi@redhat.com, raghukt@linux.vnet.ibm.com, alex.shi@intel.com,
	kvm@vger.kernel.org, stefano.stabellini@eu.citrix.com,
	peterz@infradead.org, hpa@zytor.com, vsrivatsa@gmail.com,
	mingo@elte.hu
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from e28smtp08.in.ibm.com ([122.248.162.8]:38841 "EHLO
	e28smtp08.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751436Ab2HXFk0 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 24 Aug 2012 01:40:26 -0400
Received: from /spool/local
	by e28smtp08.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted
	for <kvm@vger.kernel.org> from <nikunj@linux.vnet.ibm.com>;
	Fri, 24 Aug 2012 11:10:23 +0530
Received: from d28av01.in.ibm.com (d28av01.in.ibm.com [9.184.220.63])
	by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7O5eLFc2163096
	for <kvm@vger.kernel.org>; Fri, 24 Aug 2012 11:10:21 +0530
Received: from d28av01.in.ibm.com (loopback [127.0.0.1])
	by d28av01.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7OB9pGb022943
	for <kvm@vger.kernel.org>; Fri, 24 Aug 2012 16:39:55 +0530
In-Reply-To: <20120823093643.GC32710@amt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, 23 Aug 2012 06:36:43 -0300, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> On Tue, Aug 21, 2012 at 04:56:35PM +0530, Nikunj A. Dadhania wrote:
> >  
> > +void kvm_disable_vcpu_state(void)
> > +{
> > +	if (!has_vcpu_state)
> > +		return;
> > +
> > +	wrmsr(MSR_KVM_VCPU_STATE, 0, 0);
> 
> wrmsrl (to be consistent).
>
Sure, will change
 
> > +}
> > +
> >  #ifdef CONFIG_SMP
> >  static void __init kvm_smp_prepare_boot_cpu(void)
> >  {
> > @@ -410,6 +440,7 @@ static void __cpuinit kvm_guest_cpu_online(void *dummy)
> >  
> >  static void kvm_guest_cpu_offline(void *dummy)
> >  {
> > +	kvm_disable_vcpu_state();
> 
> Should disable MSR at kvm_pv_guest_cpu_reboot.
> 
Sure, can you explain the difference for my understanding?

> >  	kvm_disable_steal_time();
> >  	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
> >  		wrmsrl(MSR_KVM_PV_EOI_EN, 0);
> > @@ -469,6 +500,11 @@ void __init kvm_guest_init(void)
> >  	if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
> >  		apic_set_eoi_write(kvm_guest_apic_eoi_write);
> >  
> > +#ifdef CONFIG_PARAVIRT_TLB_FLUSH
> > +	if (kvm_para_has_feature(KVM_FEATURE_VCPU_STATE))
> > +		has_vcpu_state = 1;
> > +#endif
> 
> Why only this hunk guarded by CONFIG_PARAVIRT_TLB_FLUSH and not
> the rest of the code?
> 
The guest should have been compiled with CONFIG_PARAVIRT_TLB_FLUSH, as
the config also brings in HAVE_RCU_TABLE_FREE code into picture. We
should not enable this code without HAVE_RCU_TABLE_FREE.

Did not want to spray this across all the code, as the compiler will
take care of throwing out the kvm_tlb_flush_others

> Is there a switch to enable/disable this feature on the kernel
> command line? 
>
No, havent added it. 

> Grep for early_param in kvm.c.
> 
Let me know if that is required.

Regards
Nikunj