On 09.01.2012, at 17:04, Yoder Stuart-B08248 wrote: > > >> -----Original Message----- >> From: kvm-ppc-owner@xxxxxxxxxxxxxxx [mailto:kvm-ppc-owner@xxxxxxxxxxxxxxx] On Behalf Of >> Alexander Graf >> Sent: Monday, January 09, 2012 8:15 AM >> To: Liu Yu-B13201 >> Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; galak@xxxxxxxxxxxxxxxxxxx; 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.liu@xxxxxxxxxxxxx> >>> --- >>> 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" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html