| Message ID | 1331161979-12759-3-git-send-email-stuart.yoder@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On 08.03.2012, at 00:12, Stuart Yoder wrote: > From: Liu Yu-B13201 <Yu.Liu@freescale.com> > > 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@freescale.com> > [stuart.yoder@freescale.com: fixes for conditions allowing idle] > Signed-off-by: Stuart Yoder <stuart.yoder@freescale.com> > --- > -v9: pad hcall instruction array with nops > > arch/powerpc/include/asm/Kbuild | 1 + > arch/powerpc/include/asm/kvm_para.h | 14 ++++++++++++-- > arch/powerpc/kvm/booke.c | 6 ++++++ > arch/powerpc/kvm/powerpc.c | 19 ++++++++++++++++--- > include/linux/kvm.h | 2 ++ > 5 files changed, 37 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild > index d51df17..b68cb1c 100644 > --- a/arch/powerpc/include/asm/Kbuild > +++ b/arch/powerpc/include/asm/Kbuild > @@ -34,3 +34,4 @@ header-y += termios.h > header-y += types.h > header-y += ucontext.h > header-y += unistd.h > +header-y += epapr_hcalls.h > diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h > index b3b4667..5617c8c 100644 > --- a/arch/powerpc/include/asm/kvm_para.h > +++ b/arch/powerpc/include/asm/kvm_para.h > @@ -90,9 +90,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 Shouldn't all this rather be in an epapr header file? > > #define KVM_FEATURE_MAGIC_PAGE 1 > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index f0ef0be..9fde645 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -593,6 +593,12 @@ static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu) > unsigned int priority; > > if (vcpu->requests) { > + /* kvm_vcpu_block() sets KVM_REQ_UNHALT, but it is > + * not cleared elsewhere as on x86. Clear it here > + * for now, otherwise we never go idle. > + */ > + clear_bit(KVM_REQ_UNHALT, &vcpu->requests); Shouldn't the same thing hit us on non-booke as well? Also, it sounds unrelated to me and probably shouldn't be in this patch. > + > if (kvm_check_request(KVM_REQ_PENDING_TIMER, vcpu)) { > smp_mb(); > update_timer_ints(vcpu); > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index ee489f4..2595916 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -48,8 +48,7 @@ static unsigned int perfmon_refcount; > > int kvm_arch_vcpu_runnable(struct kvm_vcpu *v) > { > - bool ret = !(v->arch.shared->msr & MSR_WE) || > - !!(v->arch.pending_exceptions) || > + bool ret = !!(v->arch.pending_exceptions) || > v->requests; Huh? Alex -- 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
On 03/07/2012 05:27 PM, Alexander Graf wrote: > On 08.03.2012, at 00:12, Stuart Yoder wrote: >> >> if (vcpu->requests) { >> + /* kvm_vcpu_block() sets KVM_REQ_UNHALT, but it is >> + * not cleared elsewhere as on x86. Clear it here >> + * for now, otherwise we never go idle. >> + */ >> + clear_bit(KVM_REQ_UNHALT, &vcpu->requests); > > Shouldn't the same thing hit us on non-booke as well? Also, it sounds unrelated to me and probably shouldn't be in this patch. Until recently we didn't check for requests in kvm_arch_vcpu_runnable(). And yes, book3s will need this too. >> if (kvm_check_request(KVM_REQ_PENDING_TIMER, vcpu)) { >> smp_mb(); >> update_timer_ints(vcpu); >> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c >> index ee489f4..2595916 100644 >> --- a/arch/powerpc/kvm/powerpc.c >> +++ b/arch/powerpc/kvm/powerpc.c >> @@ -48,8 +48,7 @@ static unsigned int perfmon_refcount; >> >> int kvm_arch_vcpu_runnable(struct kvm_vcpu *v) >> { >> - bool ret = !(v->arch.shared->msr & MSR_WE) || >> - !!(v->arch.pending_exceptions) || >> + bool ret = !!(v->arch.pending_exceptions) || >> v->requests; > > Huh? MSR_WE is not going to get set if the idle hcall is used, so this check was preventing us from blocking. The check isn't needed anyway, as nothing can actually change MSR_WE while we're in kvm_vcpu_block(), which is the only user of kvm_arch_vcpu_runnable(), and the MSR_WE path won't call kvm_vcpu_block() if MSR_WE isn't set. -Scott -- 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
On 08.03.2012, at 00:37, Scott Wood wrote: > On 03/07/2012 05:27 PM, Alexander Graf wrote: >> On 08.03.2012, at 00:12, Stuart Yoder wrote: >>> >>> if (vcpu->requests) { >>> + /* kvm_vcpu_block() sets KVM_REQ_UNHALT, but it is >>> + * not cleared elsewhere as on x86. Clear it here >>> + * for now, otherwise we never go idle. >>> + */ >>> + clear_bit(KVM_REQ_UNHALT, &vcpu->requests); >> >> Shouldn't the same thing hit us on non-booke as well? Also, it sounds unrelated to me and probably shouldn't be in this patch. > > Until recently we didn't check for requests in kvm_arch_vcpu_runnable(). > > And yes, book3s will need this too. > >>> if (kvm_check_request(KVM_REQ_PENDING_TIMER, vcpu)) { >>> smp_mb(); >>> update_timer_ints(vcpu); >>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c >>> index ee489f4..2595916 100644 >>> --- a/arch/powerpc/kvm/powerpc.c >>> +++ b/arch/powerpc/kvm/powerpc.c >>> @@ -48,8 +48,7 @@ static unsigned int perfmon_refcount; >>> >>> int kvm_arch_vcpu_runnable(struct kvm_vcpu *v) >>> { >>> - bool ret = !(v->arch.shared->msr & MSR_WE) || >>> - !!(v->arch.pending_exceptions) || >>> + bool ret = !!(v->arch.pending_exceptions) || >>> v->requests; >> >> Huh? > > MSR_WE is not going to get set if the idle hcall is used, so this check > was preventing us from blocking. > > The check isn't needed anyway, as nothing can actually change MSR_WE > while we're in kvm_vcpu_block(), which is the only user of > kvm_arch_vcpu_runnable(), and the MSR_WE path won't call > kvm_vcpu_block() if MSR_WE isn't set. Ah, this is only removing the MSR_WE check. Ok. Alex -- 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
On Wed, Mar 7, 2012 at 5:27 PM, Alexander Graf <agraf@suse.de> wrote: >> #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 > > Shouldn't all this rather be in an epapr header file? Think I see a cleaner way of handling this, will address in next respin. Stuart -- 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
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild index d51df17..b68cb1c 100644 --- a/arch/powerpc/include/asm/Kbuild +++ b/arch/powerpc/include/asm/Kbuild @@ -34,3 +34,4 @@ header-y += termios.h header-y += types.h header-y += ucontext.h header-y += unistd.h +header-y += epapr_hcalls.h diff --git a/arch/powerpc/include/asm/kvm_para.h b/arch/powerpc/include/asm/kvm_para.h index b3b4667..5617c8c 100644 --- a/arch/powerpc/include/asm/kvm_para.h +++ b/arch/powerpc/include/asm/kvm_para.h @@ -90,9 +90,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/booke.c b/arch/powerpc/kvm/booke.c index f0ef0be..9fde645 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -593,6 +593,12 @@ static void kvmppc_core_check_exceptions(struct kvm_vcpu *vcpu) unsigned int priority; if (vcpu->requests) { + /* kvm_vcpu_block() sets KVM_REQ_UNHALT, but it is + * not cleared elsewhere as on x86. Clear it here + * for now, otherwise we never go idle. + */ + clear_bit(KVM_REQ_UNHALT, &vcpu->requests); + if (kvm_check_request(KVM_REQ_PENDING_TIMER, vcpu)) { smp_mb(); update_timer_ints(vcpu); diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index ee489f4..2595916 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -48,8 +48,7 @@ static unsigned int perfmon_refcount; int kvm_arch_vcpu_runnable(struct kvm_vcpu *v) { - bool ret = !(v->arch.shared->msr & MSR_WE) || - !!(v->arch.pending_exceptions) || + bool ret = !!(v->arch.pending_exceptions) || v->requests; #ifdef CONFIG_BOOKE @@ -100,6 +99,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); + break; default: r = HC_EV_UNIMPLEMENTED; break; @@ -888,9 +891,16 @@ out: static int kvm_vm_ioctl_get_pvinfo(struct kvm_ppc_pvinfo *pvinfo) { + u32 inst_nop = 0x60000000; +#ifdef CONFIG_KVM_BOOKE_HV + u32 inst_sc1 = 0x44000022; + pvinfo->hcall[0] = inst_sc1; + pvinfo->hcall[1] = inst_nop; + pvinfo->hcall[2] = inst_nop; + pvinfo->hcall[3] = inst_nop; +#else u32 inst_lis = 0x3c000000; u32 inst_ori = 0x60000000; - u32 inst_nop = 0x60000000; u32 inst_sc = 0x44000002; u32 inst_imm_mask = 0xffff; @@ -907,6 +917,9 @@ static int kvm_vm_ioctl_get_pvinfo(struct kvm_ppc_pvinfo *pvinfo) pvinfo->hcall[1] = inst_ori | (KVM_SC_MAGIC_R0 & inst_imm_mask); pvinfo->hcall[2] = inst_sc; pvinfo->hcall[3] = inst_nop; +#endif + + pvinfo->flags = KVM_PPC_PVINFO_FLAGS_EV_IDLE; return 0; } diff --git a/include/linux/kvm.h b/include/linux/kvm.h index 28ced78..32b8f54 100644 --- a/include/linux/kvm.h +++ b/include/linux/kvm.h @@ -438,6 +438,8 @@ struct kvm_ppc_pvinfo { __u8 pad[108]; }; +#define KVM_PPC_PVINFO_FLAGS_EV_IDLE (1<<0) + #define KVMIO 0xAE /*