On 09.01.2012, at 17:04, Yoder Stuart-B08248 wrote:

> 
> 
>> -----Original Message-----
>> From: kvm-ppc-ow...@vger.kernel.org [mailto:kvm-ppc-ow...@vger.kernel.org] 
>> On Behalf Of
>> Alexander Graf
>> Sent: Monday, January 09, 2012 8:15 AM
>> To: Liu Yu-B13201
>> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; 
>> ga...@kernel.crashing.org; Wood Scott-
>> B07421; Tabi Timur-B04825
>> Subject: Re: [PATCH v2 2/3] KVM: PPC: epapr: Add idle hcall support for host
>> 
>> 
>> On 05.01.2012, at 10:07, Liu Yu wrote:
>> 
>>> And add a new flag definition in kvm_ppc_pvinfo to indicate whether
>>> host support EV_IDLE hcall.
>>> 
>>> Signed-off-by: Liu Yu <yu....@freescale.com>
>>> ---
>>> v2:
>>> 1. instead of adding new field in kvm_ppc_pvinfo, use flags.
>>> 2. expose hcall definitions to userspace
>>> 
>>> arch/powerpc/include/asm/kvm_para.h |   14 ++++++++++++--
>>> arch/powerpc/kvm/powerpc.c          |    8 ++++++++
>>> include/linux/kvm.h                 |    2 ++
>>> 3 files changed, 22 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/arch/powerpc/include/asm/kvm_para.h
>>> b/arch/powerpc/include/asm/kvm_para.h
>>> index 50533f9..e8632b6 100644
>>> --- a/arch/powerpc/include/asm/kvm_para.h
>>> +++ b/arch/powerpc/include/asm/kvm_para.h
>>> @@ -41,9 +41,19 @@ struct kvm_vcpu_arch_shared { };
>>> 
>>> #define KVM_SC_MAGIC_R0             0x4b564d21 /* "KVM!" */
>>> -#define HC_VENDOR_KVM              (42 << 16)
>>> +
>>> +#include <asm/epapr_hcalls.h>
>>> +
>>> +/* ePAPR Hypercall Vendor ID */
>>> +#define HC_VENDOR_EPAPR            (EV_EPAPR_VENDOR_ID << 16)
>>> +#define HC_VENDOR_KVM              (EV_KVM_VENDOR_ID << 16)
>>> +
>>> +/* ePAPR Hypercall Token */
>>> +#define HC_EV_IDLE         EV_IDLE
>>> +
>>> +/* ePAPR Hypercall Return Codes */
>>> #define HC_EV_SUCCESS               0
>>> -#define HC_EV_UNIMPLEMENTED        12
>>> +#define HC_EV_UNIMPLEMENTED        EV_UNIMPLEMENTED
>>> 
>>> #define KVM_FEATURE_MAGIC_PAGE      1
>>> 
>>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>>> index c33f6a7..1242ee1 100644
>>> --- a/arch/powerpc/kvm/powerpc.c
>>> +++ b/arch/powerpc/kvm/powerpc.c
>>> @@ -81,6 +81,10 @@ int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
>>> 
>>>             /* Second return value is in r4 */
>>>             break;
>>> +   case HC_VENDOR_EPAPR | HC_EV_IDLE:
>>> +           r = HC_EV_SUCCESS;
>>> +           kvm_vcpu_block(vcpu);
>> 
>> Hrm. This will return on signal. So if the guest sends an idle hcall, then 
>> user space gets a
>> random signal, we'll continue executing the guest CPU, getting us out of 
>> idle even though the
>> guest didn't expect it, since the guest really wants to get an interrupt 
>> after the idle hcall.
> 
> Is that a problem though?   Won't it be just like a spurious interrupt where
> the guest would wake up, see that there is nothing to do, and then
> go idle again?   What is your concern here?

We would have changed state by enabling interrupts in the asm code. Also, it's 
not how hardware would work when setting MSR_WE, which if I understand Scott 
correctly, is the purpose of this hypercall.

> 
>> Maybe we should just set the MSR_WE bit, so we'll get back into wait state 
>> on next vcpu_run?
>> But then again that's guest visible, so I suppose a separate flag would be 
>> better.
> 
> Put a separate flag where?

In the vcpu struct. Just a bit somewhere to check in 
kvmppc_core_prepare_to_enter() in parallel to MSR_WE.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to