Re: [Qemu-devel] [Qemu-ppc] Determining interest in PPC e500spin, yield, and openpic patches

[Thomas Huth <thuth@redhat.com>]

On 15.06.2016 22:12, alarson@ddci.com wrote:
[...]
> The following patch has a fix for that, and also raises a separate
> issue that I'd be happy to resolve after getting some guidance.
[...]
@@ -104,6 +108,16 @@
 
     cpu_synchronize_state(cpu);
     stl_p(&curspin->pir, env->spr[SPR_PIR]);
+/* The stl_p() above seems wrong to me.  First of all, it seems more appropriate
+ * in a guest ROM/BOOT code than in qemu emulation.  However, SPR_PIR is never
+ * initialized, so the effect of the stl_p() is to overwrite the curspin->pir
+ * with 0. It makes more sense to load the SPR_PIR with the curspin->pir, which
+ * is what the following does.
+ *    env->spr[SPR_PIR]=ldl_p(&curspin->pir);
+ * Alternately SPR_PIR could be initialized from SPR_BOOKE_PIR which is properly
+ * initialized, so this could also work:
+ *    env->spr[SPR_PIR] = env->spr[SPR_BOOKE_PIR]
+*/
     env->nip = ldq_p(&curspin->addr) & (map_size - 1);
     env->gpr[3] = ldq_p(&curspin->r3);
     env->gpr[4] = 0;

I'm not very familiar with the e500 code, but as far as I understand the
ppce500_spin.c code, it provides the spin table facility from ePAPR for the
guests that is normally provided by the boot firmware instead. Some more
information why this has been done can be found in the original commit
message here:
  http://git.qemu.org/?p=qemu.git;a=commitdiff;h=5c145dacacad04f751c

So it's right to set up curspin->pir here (not the other way round), but
I think SPR_PIR was just a typo and should be SPR_BOOKE_PIR instead,
since the PIR register for BookE CPUs has the number 286 and not 1023.
So does it work for you if you simply replace the line with:

    stl_p(&curspin->pir, env->spr[SPR_BOOKE_PIR]);

?

 Thomas