Google
  Web www.spinics.net

RE: [PATCH v9 2/4] KVM: PPC: epapr: Add idle hcall support for host

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



> -----Original Message-----
> From: Alexander Graf [mailto:agraf@xxxxxxx]
> Sent: Wednesday, March 07, 2012 5:39 PM
> To: Wood Scott-B07421
> Cc: Yoder Stuart-B08248; kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx
> 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-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[KVM Development]     [KVM ARM]     [KVM ia64]     [Linux USB Devel]     [Linux Video]     [Linux Audio Users]     [Photo]     [Video Projectors]     [PDAs]     [Free Online Dating]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Big List of Linux Books]

  Powered by Linux