1. Patchew
  2. QEMU
  3. [PULL 0/2] ppc-for-10.0-3 queue
  4. View patch

[PULL 1/2] target/ppc: Big-core scratch register fix

Nicholas Piggin posted 2 patches 8 months, 1 week ago
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>
[PULL 1/2] target/ppc: Big-core scratch register fix
Posted by Nicholas Piggin 8 months, 1 week ago 
The per-core SCRATCH0-7 registers are shared between big cores, which
was missed in the big-core implementation. It is difficult to model
well with the big-core == 2xPnvCore scheme we moved to, this fix
uses the even PnvCore to store the scrach data.

Also remove a stray log message that came in with the same patch that
introduced patch.

Fixes: c26504afd5f5c ("ppc/pnv: Add a big-core mode that joins two regular cores")
Cc: qemu-stable@nongnu.org
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/misc_helper.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index 2d9512c116..46ae454afd 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -332,6 +332,10 @@ target_ulong helper_load_sprd(CPUPPCState *env)
     PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
     target_ulong sprc = env->spr[SPR_POWER_SPRC];
 
+    if (pc->big_core) {
+        pc = pnv_chip_find_core(pc->chip, CPU_CORE(pc)->core_id & ~0x1);
+    }
+
     switch (sprc & 0x3e0) {
     case 0: /* SCRATCH0-3 */
     case 1: /* SCRATCH4-7 */
@@ -368,6 +372,10 @@ void helper_store_sprd(CPUPPCState *env, target_ulong val)
     PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
     int nr;
 
+    if (pc->big_core) {
+        pc = pnv_chip_find_core(pc->chip, CPU_CORE(pc)->core_id & ~0x1);
+    }
+
     switch (sprc & 0x3e0) {
     case 0: /* SCRATCH0-3 */
     case 1: /* SCRATCH4-7 */
@@ -378,7 +386,6 @@ void helper_store_sprd(CPUPPCState *env, target_ulong val)
          * information. Could also dump these upon checkstop.
          */
         nr = (sprc >> 3) & 0x7;
-        qemu_log("SPRD write 0x" TARGET_FMT_lx " to SCRATCH%d\n", val, nr);
         pc->scratch[nr] = val;
         break;
     default:
-- 
2.47.1
Re: [PULL 1/2] target/ppc: Big-core scratch register fix
Posted by Thomas Huth 7 months, 3 weeks ago 
On 08/04/2025 14.45, Nicholas Piggin wrote:
> The per-core SCRATCH0-7 registers are shared between big cores, which
> was missed in the big-core implementation. It is difficult to model
> well with the big-core == 2xPnvCore scheme we moved to, this fix
> uses the even PnvCore to store the scrach data.
> 
> Also remove a stray log message that came in with the same patch that
> introduced patch.
> 
> Fixes: c26504afd5f5c ("ppc/pnv: Add a big-core mode that joins two regular cores")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   target/ppc/misc_helper.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index 2d9512c116..46ae454afd 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -332,6 +332,10 @@ target_ulong helper_load_sprd(CPUPPCState *env)
>       PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
>       target_ulong sprc = env->spr[SPR_POWER_SPRC];
>   
> +    if (pc->big_core) {
> +        pc = pnv_chip_find_core(pc->chip, CPU_CORE(pc)->core_id & ~0x1);
> +    }
> +
>       switch (sprc & 0x3e0) {
>       case 0: /* SCRATCH0-3 */
>       case 1: /* SCRATCH4-7 */
> @@ -368,6 +372,10 @@ void helper_store_sprd(CPUPPCState *env, target_ulong val)
>       PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
>       int nr;
>   
> +    if (pc->big_core) {
> +        pc = pnv_chip_find_core(pc->chip, CPU_CORE(pc)->core_id & ~0x1);
> +    }
> +

  Hi Nicholas,

this patch breaks compilation when QEMU has been configured with 
"--without-default-devices" :

FAILED: qemu-system-ppc64
cc -m64 @qemu-system-ppc64.rsp
/usr/bin/ld: libqemu-ppc64-softmmu.a.p/target_ppc_misc_helper.c.o: in 
function `helper_load_sprd':
.../qemu/target/ppc/misc_helper.c:336:(.text+0xcab): undefined reference to 
`pnv_chip_find_core'
/usr/bin/ld: libqemu-ppc64-softmmu.a.p/target_ppc_misc_helper.c.o: in 
function `helper_store_sprd':
.../qemu/target/ppc/misc_helper.c:376:(.text+0xda3): undefined reference to 
`pnv_chip_find_core'
collect2: error: ld returned 1 exit status

Could you please have a look?

  Thanks,
   Thomas
Re: [PULL 1/2] target/ppc: Big-core scratch register fix
Posted by Nicholas Piggin 7 months, 2 weeks ago 
On Thu Apr 24, 2025 at 6:25 PM AEST, Thomas Huth wrote:
> On 08/04/2025 14.45, Nicholas Piggin wrote:
>> The per-core SCRATCH0-7 registers are shared between big cores, which
>> was missed in the big-core implementation. It is difficult to model
>> well with the big-core == 2xPnvCore scheme we moved to, this fix
>> uses the even PnvCore to store the scrach data.
>> 
>> Also remove a stray log message that came in with the same patch that
>> introduced patch.
>> 
>> Fixes: c26504afd5f5c ("ppc/pnv: Add a big-core mode that joins two regular cores")
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>   target/ppc/misc_helper.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>> 
>> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
>> index 2d9512c116..46ae454afd 100644
>> --- a/target/ppc/misc_helper.c
>> +++ b/target/ppc/misc_helper.c
>> @@ -332,6 +332,10 @@ target_ulong helper_load_sprd(CPUPPCState *env)
>>       PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
>>       target_ulong sprc = env->spr[SPR_POWER_SPRC];
>>   
>> +    if (pc->big_core) {
>> +        pc = pnv_chip_find_core(pc->chip, CPU_CORE(pc)->core_id & ~0x1);
>> +    }
>> +
>>       switch (sprc & 0x3e0) {
>>       case 0: /* SCRATCH0-3 */
>>       case 1: /* SCRATCH4-7 */
>> @@ -368,6 +372,10 @@ void helper_store_sprd(CPUPPCState *env, target_ulong val)
>>       PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
>>       int nr;
>>   
>> +    if (pc->big_core) {
>> +        pc = pnv_chip_find_core(pc->chip, CPU_CORE(pc)->core_id & ~0x1);
>> +    }
>> +
>
>   Hi Nicholas,
>
> this patch breaks compilation when QEMU has been configured with 
> "--without-default-devices" :
>
> FAILED: qemu-system-ppc64
> cc -m64 @qemu-system-ppc64.rsp
> /usr/bin/ld: libqemu-ppc64-softmmu.a.p/target_ppc_misc_helper.c.o: in 
> function `helper_load_sprd':
> .../qemu/target/ppc/misc_helper.c:336:(.text+0xcab): undefined reference to 
> `pnv_chip_find_core'
> /usr/bin/ld: libqemu-ppc64-softmmu.a.p/target_ppc_misc_helper.c.o: in 
> function `helper_store_sprd':
> .../qemu/target/ppc/misc_helper.c:376:(.text+0xda3): undefined reference to 
> `pnv_chip_find_core'
> collect2: error: ld returned 1 exit status
>
> Could you please have a look?

Thanks for the report, I have a hopefully simple fix just going through
CI now... Do you know if there's any reason to exclude a bunch of
targets in the build-without-defaults CI test? I wonder if we could just
enable all, it shouldn't add too much time to build test.

Thanks,
Nick
Re: [PULL 1/2] target/ppc: Big-core scratch register fix
Posted by Thomas Huth 7 months, 2 weeks ago 
On 30/04/2025 02.00, Nicholas Piggin wrote:
> On Thu Apr 24, 2025 at 6:25 PM AEST, Thomas Huth wrote:
>> On 08/04/2025 14.45, Nicholas Piggin wrote:
>>> The per-core SCRATCH0-7 registers are shared between big cores, which
>>> was missed in the big-core implementation. It is difficult to model
>>> well with the big-core == 2xPnvCore scheme we moved to, this fix
>>> uses the even PnvCore to store the scrach data.
>>>
>>> Also remove a stray log message that came in with the same patch that
>>> introduced patch.
>>>
>>> Fixes: c26504afd5f5c ("ppc/pnv: Add a big-core mode that joins two regular cores")
>>> Cc: qemu-stable@nongnu.org
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>>    target/ppc/misc_helper.c | 9 ++++++++-
>>>    1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
>>> index 2d9512c116..46ae454afd 100644
>>> --- a/target/ppc/misc_helper.c
>>> +++ b/target/ppc/misc_helper.c
>>> @@ -332,6 +332,10 @@ target_ulong helper_load_sprd(CPUPPCState *env)
>>>        PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
>>>        target_ulong sprc = env->spr[SPR_POWER_SPRC];
>>>    
>>> +    if (pc->big_core) {
>>> +        pc = pnv_chip_find_core(pc->chip, CPU_CORE(pc)->core_id & ~0x1);
>>> +    }
>>> +
>>>        switch (sprc & 0x3e0) {
>>>        case 0: /* SCRATCH0-3 */
>>>        case 1: /* SCRATCH4-7 */
>>> @@ -368,6 +372,10 @@ void helper_store_sprd(CPUPPCState *env, target_ulong val)
>>>        PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
>>>        int nr;
>>>    
>>> +    if (pc->big_core) {
>>> +        pc = pnv_chip_find_core(pc->chip, CPU_CORE(pc)->core_id & ~0x1);
>>> +    }
>>> +
>>
>>    Hi Nicholas,
>>
>> this patch breaks compilation when QEMU has been configured with
>> "--without-default-devices" :
>>
>> FAILED: qemu-system-ppc64
>> cc -m64 @qemu-system-ppc64.rsp
>> /usr/bin/ld: libqemu-ppc64-softmmu.a.p/target_ppc_misc_helper.c.o: in
>> function `helper_load_sprd':
>> .../qemu/target/ppc/misc_helper.c:336:(.text+0xcab): undefined reference to
>> `pnv_chip_find_core'
>> /usr/bin/ld: libqemu-ppc64-softmmu.a.p/target_ppc_misc_helper.c.o: in
>> function `helper_store_sprd':
>> .../qemu/target/ppc/misc_helper.c:376:(.text+0xda3): undefined reference to
>> `pnv_chip_find_core'
>> collect2: error: ld returned 1 exit status
>>
>> Could you please have a look?
> 
> Thanks for the report, I have a hopefully simple fix just going through
> CI now... Do you know if there's any reason to exclude a bunch of
> targets in the build-without-defaults CI test? I wonder if we could just
> enable all, it shouldn't add too much time to build test.

I think that setting has been added back then when we still built all 
machines with --without-default-devices and only disabled the optional 
devices. Then Paolo once cleaned this up (see commit bf616ce47be6802bbe7d 
for example), so that all boards now get disabled by default, too. Since 
that point in time, the runtime of the job is likely much decreased. So yes, 
I think we could nowadays add more targets to that job without risking to 
hit the timeout again. Could you maybe suggest a patch?

  Thanks,
   Thomas

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.