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

On Tue, 8 Aug 2017 16:06:43 +0530
Gautham R Shenoy [off-list ref] wrote:

Hi Nicholas,

On Sun, Aug 06, 2017 at 03:02:38AM +1000, Nicholas Piggin wrote:

quoted

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.

quoted

@@ -444,6 +439,7 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
 	beq	1f
 	b	kvm_start_guest
 1:
+END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)  

This would be 7 nops on power9. Should we move this to a different
function and do a bl to that?

Yes that's a good idea.

quoted

+static void kvmppc_release_hwthread_secondary(int cpu)
+{
+	struct paca_struct *tpaca;
+
+	if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+		WARN_ON(1);
+		return;
+	}
+
+	tpaca = &paca[cpu];
+	tpaca->kvm_hstate.hwthread_req = 0;
+	kvmppc_release_hwthread(cpu);
+}
+
+  

Extra blank line not needed.

Sure.

quoted

@@ -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.
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.

Thanks,
Nick