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

Alexander Graf
2013-07-15T15:09:33Z

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

We should only patch it if kvm_para_available(). That should guard us against everything.

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

Alex