On 15.07.2013, at 20:21, Scott Wood wrote: > On 07/15/2013 10:16:41 AM, Bhushan Bharat-R65777 wrote: >> > -----Original Message----- >> > From: Alexander Graf [mailto:agraf@xxxxxxx] >> > Sent: Monday, July 15, 2013 8:40 PM >> > To: Bhushan Bharat-R65777 >> > Cc: kvm@xxxxxxxxxxxxxxx; kvm-ppc@xxxxxxxxxxxxxxx; 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@xxxxxxx] >> > >> Sent: Monday, July 15, 2013 5:20 PM >> > >> To: Bhushan Bharat-R65777 >> > >> Cc: kvm@xxxxxxxxxxxxxxx; kvm-ppc@xxxxxxxxxxxxxxx; 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 <bharat.bhushan@xxxxxxxxxxxxx> >> > >>> --- >> > >>> 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 <asm/cacheflush.h> >> > >>> #include <asm/code-patching.h> >> > >>> #include <asm/machdep.h> >> > >>> +#include <asm/kvm_para.h> >> > >>> +#include <asm/kvm_host.h> >> > >> >> > >> 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. Ah, figured that part was obvious :). > >> > >>> + 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. If we also check for kvm_para_available() I agree. Otherwise QEMU might advertise the reset hcall, but the guest kernel may not implement KVM hypercalls. In that case the device tree check will succeed, but the actual hypercall will not. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
