Re: [PATCH 10/13] powerpc/64s: idle simplify KVM idle on POWER9

[Gautham R Shenoy <ego@linux.vnet.ibm.com>]

On Tue, Aug 08, 2017 at 10:42:57PM +1000, Nicholas Piggin wrote:
> On Tue, 8 Aug 2017 16:06:43 +0530
> Gautham R Shenoy <ego@linux.vnet.ibm.com> wrote:
> 
> > Hi Nicholas,
> > 
> > On Sun, Aug 06, 2017 at 03:02:38AM +1000, Nicholas Piggin wrote:
> > > POWER9 CPUs have independent MMU contexts per thread so KVM
> > > does not have to bring sibling threads into real-mode when
> > > switching MMU mode to guest. This can simplify POWER9 sleep/wake
> > > paths and avoids hwsyncs.
> > > @@ -2858,11 +2883,13 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
> > > 
> > >  	/* Let secondaries go back to the offline loop */
> > >  	for (i = 0; i < controlled_threads; ++i) {
> > > -		kvmppc_release_hwthread(pcpu + i);
> > >  		if (sip && sip->napped[i])
> > >  			kvmppc_ipi_thread(pcpu + i);
> > >  		cpumask_clear_cpu(pcpu + i, &vc->kvm->arch.cpu_in_guest);
> > >  	}  
> > 
> > We are sending an IPI to the thread that has exited the guest and is
> > currently napping. The IPI wakes it up so that it can executes
> > offline loop. But we haven't released the hwthread yet, which means
> > that hwthread_req for this thread is still set.
> > 
> > The thread wakes up from nap, executes the pnv_powersave_wakeup code
> > where it can enter kvm_start_guest. Is this a legitimate race or am I
> > missing something?
> 
> Oh I think it's just a silly mistake in my patch, good catch.

Ah,np!

> Would moving this loop below the one below solve it? I wasn't
> completely happy with uglifying these loops by making the
> primary release different than secondary... maybe I will just
> move the difference into kvmppc_release_hwthread and which is
> less intrusive to callers.

I think moving it to kvmppc_release_hwthread is a good idea.

> 
> Thanks,
> Nick
> 

--
Thanks and Regards
gautham.