RE: [PATCH 5/5] powerpc: using reset hcall when kvm,has-reset

Bhushan Bharat-R65777
2013-07-15T15:05:54Z

> -----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 <bharat.bhushan@freescale.com>
> > ---
> > 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.

>
> > + 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?

-Bharat

>
>
> Alex
>
> > + BUG();
> > +}
> > +
> > static int __init epapr_paravirt_init(void) {
> > struct device_node *hyper_node;
> > @@ -58,6 +68,8 @@ static int __init epapr_paravirt_init(void)
> > if (of_get_property(hyper_node, "has-idle", NULL))
> > ppc_md.power_save = epapr_ev_idle;
> > #endif
> > + if (of_get_property(hyper_node, "kvm,has-reset", NULL))
> > + ppc_md.restart = epapr_hypercall_reset;
> >
> > epapr_paravirt_enabled = true;
> >
> > --
> > 1.7.0.4
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> > the body of a message to majordomo@vger.kernel.org More majordomo info
> > at http://vger.kernel.org/majordomo-info.html
>