> -----Original Message-----
> From: Alexander Graf [mailto:[email protected]]
> Sent: Wednesday, March 07, 2012 5:39 PM
> To: Wood Scott-B07421
> Cc: Yoder Stuart-B08248; [email protected]; [email protected]
> Subject: Re: [PATCH v9 2/4] KVM: PPC: epapr: Add idle hcall support for host
>
>
> 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.
Where should this go? Something like this?
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -283,6 +283,8 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
/* Tell the guest about our interrupt status */
kvmppc_update_int_pending(vcpu, *pending, old_pending);
+ clear_bit(KVM_REQ_UNHALT, &vcpu->requests);
+
return 0;
}
> >>> 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.
I'll add an additional comment to the patch description.
Stuart
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
