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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux