qemu-ppc
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: I think I found definition of PIR on 970MP

From: BALATON Zoltan
Subject: Re: I think I found definition of PIR on 970MP
Date: Sun, 16 Mar 2025 14:59:12 +0100 (CET) 
On Wed, 12 Mar 2025, BALATON Zoltan wrote:

On Wed, 12 Mar 2025, Howard Spoelstra wrote:

On Wed, Mar 12, 2025 at 1:51 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:

On Wed, 12 Mar 2025, BALATON Zoltan wrote:

On Wed, 12 Mar 2025, Howard Spoelstra wrote:

On Tue, Mar 11, 2025 at 11:10 PM BALATON Zoltan <balaton@eik.bme.hu>

wrote:

On Wed, 12 Mar 2025, Andrew Randrianasulu wrote:

---quote from  user manual ---

2.1.1.4 Processor ID Register (PIR)
The Processor Identification Register (PIR) is a 32-bit register that
holds a processor identification tag. In the
970MP processing unit, this tag is in the three least-significant bits
(29:31). The least-significant bit of the
processor identification tag (PID) is hardwired to ‘0’ for PU0 and to
‘1’ for PU1. This tag is used to tag bus
transactions and to differentiate processors in multiprocessor
systems. The PIR is a read-only register. The
format of the register is as follows:

0:28—Reserved (read as zeros)
29:31 PID3-bit processor ID value (least-significant bit hardwired to
differentiate PU0 and PU1)


During power-on reset, PID is set to a unique value for each processor
in a multi processor system.

=====

7.2.2.4 Processor ID (PROCID[0:1])–Input
The 2-bit processor ID is used to assign unique IDs to the two 970MP
processing units in a system that can
have up to eight processors. The PROCID signals are sampled during
power-on reset, and the 2-bit value is
placed in the second and third lowest-order bits of the Processor ID
Register (PIR) of each processing unit.
The lowest-order PIR bit is hardwired to a '0' for PU0 and to '1' for

PU1.

Timing: These signals should be permanently tied to VDD or GND, as
appropriate for the required ID value.

=== quote end ===


This suggests that at least on G5 it's strapped in hardware by wiring

the

chip pins so each CPU has a different ID. The G4 could write it from
software but maybe it doesn't do that and sets it by hardware pins as
well. In any case, OpenBIOS does not write it but would set the reg
property based on it so probably it should be set in QEMU when creating
the CPUs. Then the part of the OpenBIOS patch that comments out the

call

to set reg property based on pir and the part that sets the reg

property

later can be dropped and thus the patch simplified (need less changes

to

OpenBIOS).

To set PIR in QEMU look at what other machines do. By searching for

PIR in

hw/ppc I've found:

hw/ppc/spapr_cpu_core.c::spapr_realize_vcpu()
...
     env->spr_cb[SPR_PIR].default_value = cs->cpu_index;

hw/ppc/e500.c::ppce500_init()
for (i = 0; i < smp_cpus; i++) {
...
         env->spr_cb[SPR_BOOKE_PIR].default_value = cs->cpu_index = i;

The SPR number is different on e500 but the code is more similar to

mac99

so in mac99 I think we should do what e500 does but use SPR_PIR

instead of

SPR_BOOKE_PIR. What I'm not sure about if the cpu_reset in the kick
function would undo this but since it sets the default_value maybe that
means it would reset to this value so it should work. There's a -d
cpu_reset option to dump registers on cpu_reset but I think that shows
values before reset. You can try updating the patches accordingly (add

a

line to QEMU as above and undo part of the openbios patch that should

no

longer be needed) and test if that works any better. Even if it does

not

fix anything it would be cleaner so one step towards upstreaming. I

don't

plan to do anything with this, one of you should be able to do this as

an

excercise.



You mean something like this in mac_newworld.c around line 300 after
/* Check this */
openpic_irqs[i].irq[OPENPIC_OUTPUT_RESET] =
qdev_get_gpio_in(dev, PPC6xx_INPUT_HRESET);

----> env->spr_cb[SPR_PIR].default_value = env->core_index = i;

After that it looks like the pir no longer needs to be set in openbios.

But

I think I see PIR 0 for CPU 1 and PIR 1 for CPU 0, which seems wrong.

The -d cpu_reset shows both cpus reset to the same values at boot:


Do you see PIR in these dumps? Maybe we need to add that too. Andrew

checked

the PIR = printed by OpenBIOS but that showed 0 for both CPUs. I don't

know

why, you should try to read the QEMU source to see where it's set and

where

the default_value you set gets copied in the actual register value. I'd
expect that to happen at cpu_reset but I don't know. The relevant code

should

be either in qemu/target/ppc or qemu/hw/ppc.


Yes, at end of qemu/target/ppc/cpu_init.c::ppc_cpu_reset_hold() there's a
loop:
     for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
         ppc_spr_t *spr = &env->spr_cb[i];

         if (!spr->name) {
             continue;
         }
         env->spr[i] = spr->default_value;
     }
It would skip regs with no name set which is probably those that were not
registered. It's registered in register_604_sprs, register_74xx_sprs,
register_book3s_ids_sprs so should be there but only printed for BookE in
ppc_cpu_dump_state. To see it in below listing you would need to add
something like:

I added  qemu_fprintf(f, "  PIR " TARGET_FMT_lx "\n",

                    env->spr[SPR_PIR]);
at line 7630

Result:  PIR 0000000000000000 for both CPUS
Yes, so is this dump function called before or after the part that sets the PIR register? If you can't find it from the source you can run with gdb --args qemu-system-ppc ... then set a breakpoint on ppc_cpu_dump_state and do a backtrace to see where it was called from then you know where to look. But since OpenBIOS also prints PIR=0 it's probably not set. Then try to find why the default value is not copied in the loop above in qemu/target/ppc/cpu_init.c::ppc_cpu_reset_hold(). You could also set a breakpoint there and see if it's called before the dump or on startup before OpenBIOS is started. It should be run due to cpu_reset called from the reset function we register for the CPUs but I don't know, you have to check it. If we have code that should be run and do what we want but it's not happening try to find why it does not reach that code.
I've tried to figure this out. In addition to fix ppc_cpu_dump_state to print PIR I've added debug printf to print env->spr[SPR_PIR] both before and after in ppc_core99_reset and set the default value when CPU is created like this (based on previous patch): 

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 57d83f989c..7de62b1610 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -124,7 +124,9 @@ static void ppc_core99_reset(void *opaque)
 {
     PowerPCCPU *cpu = opaque;

+fprintf(stderr, "%s: %p PIR:" TARGET_FMT_lx "\n", __func__, opaque, 
cpu->env.spr[SPR_PIR]);
     cpu_reset(CPU(cpu));
+fprintf(stderr, "%s: %p PIR:" TARGET_FMT_lx "\n", __func__, opaque, 
cpu->env.spr[SPR_PIR]);
     /* 970 CPUs want to get their initial IP as part of their boot protocol */
     cpu->env.nip = PROM_BASE + 0x100;
 }
@@ -174,13 +176,16 @@ static void ppc_core99_init(MachineState *machine)
     cpus = g_new0(PowerPCCPU *, machine->smp.cpus);
     for (i = 0; i < machine->smp.cpus; i++) {
         cpus[i] = POWERPC_CPU(cpu_create(machine->cpu_type));
-fprintf(stderr, "cpus[%d] = %p %p\n", i, (void *)cpus[i], (void 
*)&cpus[i]->env);
+        env = &cpus[i]->env;
+fprintf(stderr, "cpus[%d] = %p %p\n", i, (void *)cpus[i], (void *)env);
         /* Secondary CPUs start halted */
         object_property_set_bool(OBJECT(cpus[i]), "start-powered-off", i != 0,
                                  &error_abort);
         /* Set time-base frequency to 100 Mhz */
         cpu_ppc_tb_init(&cpus[i]->env, TBFREQ);
         qemu_register_reset(ppc_core99_reset, cpus[i]);
+        env->spr_cb[SPR_PIR].default_value = i;
+        CPU(cpus[i])->cpu_index = i;
     }
     env = &cpus[0]->env;


Then found that with -d cpu_reset I get:

cpus[0] = 0x5569922b1230 0x5569922b3df0
cpus[1] = 0x556992326ab0 0x556992329670
ppc_core99_reset: 0x5569922b1230 PIR:00000000
CPU Reset (CPU 0)
NIP 00000000   LR 00000000 CTR 00000000 XER 00000000 PIR 00000000 CPU#0
[...]
ppc_core99_reset: 0x5569922b1230 PIR:00000000
ppc_core99_reset: 0x556992326ab0 PIR:00000000
CPU Reset (CPU 1)
NIP 00000000   LR 00000000 CTR 00000000 XER 00000000 PIR 00000000 CPU#1
[...]
ppc_core99_reset: 0x556992326ab0 PIR:00000001
so the state dump is done *before* reset and cpu_reset sets PIR correctly. However in OpenBIOS still see: 

Pir = 0
CPU type PowerPC,G4
Pir = 0
CPU type PowerPC,G4
Then I started to think why... So only the first CPU is started, the others are created powered off. Therefore OpenBIOS runs only on the first CPU so reading PIR there will always get 0 even if called multiple times in a loop for different CPUs. While the loop handles additional CPUs it can't read their PIR as it's still running on first CPU. So this code in OpenBIOS does not match how PowerMac works at least on QEMU so we should instead pass the CPU number to CPU init functions and set reg and other values accordingly, we can't use mfspr to get values of a CPU that we're not running on (and is not even running at this point as it's still powered off). So we need a way to pass down the CPU number instead. The argument of cpu_g4_init is struct cpudef so maybe we need to add an index field to that struct and set it to the CPU number in the loop and use that instead of the inline asm mfspr in cpu_add_pir_property to add reg and state properties. I can make an openbios patch for that but I don't mind if one of you do it. I can send the patch to add PIR dump in QEMU but we're in freeze now so it may get lost by the time development reopens and it does not seem to be that useful now. 

Regards,
BALATON Zoltan
reply via email to
[Prev in Thread] Current Thread [Next in Thread]