From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 5/5] powerpc: using reset hcall when kvm,has-reset Date: Mon, 15 Jul 2013 13:21:43 -0500 Message-ID: <1373912503.8183.311@snotra> References: <1373886679-19581-1-git-send-email-Bharat.Bhushan@freescale.com> <1373886679-19581-6-git-send-email-Bharat.Bhushan@freescale.com> <7A2C8450-46FF-4160-B045-F2A3F8F3B8B7@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D070D27C7@039-SN2MPN1-013.039d.mgd> <9F4ADF63-D183-4C39-9959-087AE64592DA@suse.de> <6A3DF150A5B70D4F9B66A25E3F7C888D070D2883@039-SN2MPN1-013.039d.mgd.msft.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Alexander Graf , "kvm@vger.kernel.org" , "kvm-ppc@vger.kernel.org" , Wood Scott-B07421 , Yoder Stuart-B08248 To: Bhushan Bharat-R65777 Return-path: In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D070D2883@039-SN2MPN1-013.039d.mgd.msft.net> (from R65777@freescale.com on Mon Jul 15 10:16:41 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 07/15/2013 10:16:41 AM, Bhushan Bharat-R65777 wrote: > > > > -----Original Message----- > > From: Alexander Graf [mailto:agraf@suse.de] > > Sent: Monday, July 15, 2013 8:40 PM > > To: Bhushan Bharat-R65777 > > Cc: kvm@vger.kernel.org; kvm-ppc@vger.kernel.org; Wood > Scott-B07421; Yoder > > Stuart-B08248 > > Subject: Re: [PATCH 5/5] powerpc: using reset hcall when > kvm,has-reset > > > > > > On 15.07.2013, at 17:05, Bhushan Bharat-R65777 wrote: > > > > > > > > > > >> -----Original Message----- > > >> From: Alexander Graf [mailto:agraf@suse.de] > > >> Sent: Monday, July 15, 2013 5:20 PM > > >> To: Bhushan Bharat-R65777 > > >> Cc: kvm@vger.kernel.org; kvm-ppc@vger.kernel.org; Wood > Scott-B07421; > > >> Yoder Stuart-B08248; Bhushan Bharat-R65777 > > >> Subject: Re: [PATCH 5/5] powerpc: using reset hcall when > > >> kvm,has-reset > > >> > > >> > > >> On 15.07.2013, at 13:11, Bharat Bhushan wrote: > > >> > > >>> Detect the availability of the reset hcalls by looking at > > >>> kvm,has-reset property on the /hypervisor node in the device > tree > > >>> passed to the VM and patches the reset mechanism to use reset > hcall. > > >>> > > >>> This patch uses the reser hcall when kvm,has-reset is there in > > >> > > >> Your patch description is pretty broken :). > > >> > > >>> > > >>> Signed-off-by: Bharat Bhushan > > >>> --- > > >>> arch/powerpc/kernel/epapr_paravirt.c | 12 ++++++++++++ > > >>> 1 files changed, 12 insertions(+), 0 deletions(-) > > >>> > > >>> diff --git a/arch/powerpc/kernel/epapr_paravirt.c > > >>> b/arch/powerpc/kernel/epapr_paravirt.c > > >>> index d44a571..651d701 100644 > > >>> --- a/arch/powerpc/kernel/epapr_paravirt.c > > >>> +++ b/arch/powerpc/kernel/epapr_paravirt.c > > >>> @@ -22,6 +22,8 @@ > > >>> #include > > >>> #include > > >>> #include > > >>> +#include > > >>> +#include > > >> > > >> Why would we need kvm_host.h? This is guest code. > > >> > > >>> > > >>> #if !defined(CONFIG_64BIT) || defined(CONFIG_PPC_BOOK3E_64) > extern > > >>> void epapr_ev_idle(void); @@ -30,6 +32,14 @@ extern u32 > > >>> epapr_ev_idle_start[]; > > >>> > > >>> bool epapr_paravirt_enabled; > > >>> > > >>> +void epapr_hypercall_reset(char *cmd) { > > >>> + long ret; > > >>> + ret = kvm_hypercall0(KVM_HC_VM_RESET); > > >> > > >> Is this available without CONFIG_KVM_GUEST? kvm_hypercall() > simply > > >> returns "unimplemented" for everything when that config option > is not set. > > > > > > We are here because we patched the ppc_md.restart to point to new > handler. > > > So I think we should patch the ppc_md.restart only if > CONFIG_KVM_GUEST is > > true. > > > > We should only patch it if kvm_para_available(). That should guard > us against > > everything. It also should depend on whether the reset hcall is advertised in the device tree. > > >>> + printk("error: system reset returned with error %ld\n", > ret); > > >> > > >> So we should fall back to the normal reset handler here. > > > > > > Do you mean return normally from here, no BUG() etc? > > > > If we guard the patching against everything, we can treat a broken > hcall as BUG. > > However, if we don't we want to fall back to the normal guts based > reset. > > Will let Scott comment on this? > > But ppc_md.restart can point to only one handler and during paravirt > patching we changed this to new handler. So we cannot jump back to > guts type handler I don't think it's worth implementing a fall-back scheme -- if KVM advertises that the reset hcall exists, then it had better exist. BTW, this is not part of ePAPR so it should not be in epapr_paravirt.c. It should go in kvm.c. -Scott