Re: [PATCH] fix: Fix build error with CONFIG_POWERNV disabled

["Cédric Le Goater" <clg@redhat.com>]

On 8/20/25 14:25, Aditya Gupta wrote:
> Currently when CONFIG_POWERNV is not enabled, the build fails, such as
> with --without-default-devices:
> 
>      $ ./configure --without-default-devices
>      $ make
> 
>      [281/283] Linking target qemu-system-ppc64
>      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':
>      .../target/ppc/misc_helper.c:335:(.text+0xcdc): 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':
>      .../target/ppc/misc_helper.c:375:(.text+0xdf4): undefined reference to `pnv_chip_find_core'
>      collect2: error: ld returned 1 exit status
>      ...
> 
> This is since target/ppc/misc_helper.c references PowerNV specific
> 'pnv_chip_find_core' call.
> 
> Split the PowerNV specific SPRD code out of the generic PowerPC code, by
> moving the SPRD code to pnv.c
> 
> Fixes: 9808ce6d5cb ("target/ppc: Big-core scratch register fix")
> Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reported-by: Thomas Huth <thuth@redhat.com>
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>


Acked-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
> Note that while moving the code, the 'target_ulong' type for sprc has been
> modified to 'uint64_t'.
> 
> Based on the discussion happened on [1].
> Requires patch 1 and patch 2 of [1] to be applied, to fix the build.
> 
> [1]: https://lore.kernel.org/qemu-devel/20250526112346.48744-1-philmd@linaro.org/
> ---
> ---
>   hw/ppc/pnv.c             | 86 ++++++++++++++++++++++++++++++++++++++++
>   target/ppc/cpu.h         |  4 ++
>   target/ppc/misc_helper.c | 59 +++------------------------
>   3 files changed, 96 insertions(+), 53 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index d84c9067edb3..9c74f46091a7 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -21,6 +21,7 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/datadir.h"
> +#include "qemu/log.h"
>   #include "qemu/units.h"
>   #include "qemu/cutils.h"
>   #include "qapi/error.h"
> @@ -1794,12 +1795,83 @@ static void pnv_chip_power9_pec_realize(PnvChip *chip, Error **errp)
>       }
>   }
>   
> +static uint64_t pnv_handle_sprd_load(CPUPPCState *env)
> +{
> +    PowerPCCPU *cpu = env_archcpu(env);
> +    PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
> +    uint64_t 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 */
> +        return pc->scratch[(sprc >> 3) & 0x7];
> +
> +    case 0x1e0: /* core thread state */
> +        if (env->excp_model == POWERPC_EXCP_POWER9) {
> +            /*
> +             * Only implement for POWER9 because skiboot uses it to check
> +             * big-core mode. Other bits are unimplemented so we would
> +             * prefer to get unimplemented message on POWER10 if it were
> +             * used anywhere.
> +             */
> +            if (pc->big_core) {
> +                return PPC_BIT(63);
> +            } else {
> +                return 0;
> +            }
> +        }
> +        /* fallthru */
> +
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "mfSPRD: Unimplemented SPRC:0x"
> +                                  TARGET_FMT_lx"\n", sprc);
> +        break;
> +    }
> +    return 0;
> +}
> +
> +static void pnv_handle_sprd_store(CPUPPCState *env, uint64_t val)
> +{
> +    PowerPCCPU *cpu = env_archcpu(env);
> +    uint64_t sprc = env->spr[SPR_POWER_SPRC];
> +    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 */
> +        /*
> +         * Log stores to SCRATCH, because some firmware uses these for
> +         * debugging and logging, but they would normally be read by the BMC,
> +         * which is not implemented in QEMU yet. This gives a way to get at the
> +         * information. Could also dump these upon checkstop.
> +         */
> +        nr = (sprc >> 3) & 0x7;
> +        pc->scratch[nr] = val;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "mtSPRD: Unimplemented SPRC:0x"
> +                                  TARGET_FMT_lx"\n", sprc);
> +        break;
> +    }
> +}
> +
>   static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>   {
>       PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
>       Pnv9Chip *chip9 = PNV9_CHIP(dev);
>       PnvChip *chip = PNV_CHIP(dev);
>       Pnv9Psi *psi9 = &chip9->psi;
> +    PowerPCCPU *cpu;
> +    PowerPCCPUClass *cpu_class;
>       Error *local_err = NULL;
>       int i;
>   
> @@ -1827,6 +1899,12 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>           return;
>       }
>   
> +    /* Set handlers for Special registers, such as SPRD */
> +    cpu = chip->cores[0]->threads[0];
> +    cpu_class = POWERPC_CPU_GET_CLASS(cpu);
> +    cpu_class->load_sprd = pnv_handle_sprd_load;
> +    cpu_class->store_sprd = pnv_handle_sprd_store;
> +
>       /* XIVE interrupt controller (POWER9) */
>       object_property_set_int(OBJECT(&chip9->xive), "ic-bar",
>                               PNV9_XIVE_IC_BASE(chip), &error_fatal);
> @@ -2078,6 +2156,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>       PnvChipClass *pcc = PNV_CHIP_GET_CLASS(dev);
>       PnvChip *chip = PNV_CHIP(dev);
>       Pnv10Chip *chip10 = PNV10_CHIP(dev);
> +    PowerPCCPU *cpu;
> +    PowerPCCPUClass *cpu_class;
>       Error *local_err = NULL;
>       int i;
>   
> @@ -2105,6 +2185,12 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>           return;
>       }
>   
> +    /* Set handlers for Special registers, such as SPRD */
> +    cpu = chip->cores[0]->threads[0];
> +    cpu_class = POWERPC_CPU_GET_CLASS(cpu);
> +    cpu_class->load_sprd = pnv_handle_sprd_load;
> +    cpu_class->store_sprd = pnv_handle_sprd_store;
> +
>       /* XIVE2 interrupt controller (POWER10) */
>       object_property_set_int(OBJECT(&chip10->xive), "ic-bar",
>                               PNV10_XIVE2_IC_BASE(chip), &error_fatal);
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 6b90543811f0..0e26e4343de7 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1522,6 +1522,10 @@ struct PowerPCCPUClass {
>       void (*init_proc)(CPUPPCState *env);
>       int  (*check_pow)(CPUPPCState *env);
>       int  (*check_attn)(CPUPPCState *env);
> +
> +    /* Handlers to be set by the machine initialising the chips */
> +    uint64_t (*load_sprd)(CPUPPCState *env);
> +    void (*store_sprd)(CPUPPCState *env, uint64_t val);
>   };
>   
>   static inline bool ppc_cpu_core_single_threaded(CPUState *cs)
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index e7d94625185c..0e625cbb704d 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -328,69 +328,22 @@ target_ulong helper_load_sprd(CPUPPCState *env)
>        * accessed by powernv machines.
>        */
>       PowerPCCPU *cpu = env_archcpu(env);
> -    PnvCore *pc = pnv_cpu_state(cpu)->pnv_core;
> -    target_ulong sprc = env->spr[SPR_POWER_SPRC];
> +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>   
> -    if (pc->big_core) {
> -        pc = pnv_chip_find_core(pc->chip, CPU_CORE(pc)->core_id & ~0x1);
> +    if (pcc->load_sprd) {
> +        return pcc->load_sprd(env);
>       }
>   
> -    switch (sprc & 0x3e0) {
> -    case 0: /* SCRATCH0-3 */
> -    case 1: /* SCRATCH4-7 */
> -        return pc->scratch[(sprc >> 3) & 0x7];
> -
> -    case 0x1e0: /* core thread state */
> -        if (env->excp_model == POWERPC_EXCP_POWER9) {
> -            /*
> -             * Only implement for POWER9 because skiboot uses it to check
> -             * big-core mode. Other bits are unimplemented so we would
> -             * prefer to get unimplemented message on POWER10 if it were
> -             * used anywhere.
> -             */
> -            if (pc->big_core) {
> -                return PPC_BIT(63);
> -            } else {
> -                return 0;
> -            }
> -        }
> -        /* fallthru */
> -
> -    default:
> -        qemu_log_mask(LOG_UNIMP, "mfSPRD: Unimplemented SPRC:0x"
> -                                  TARGET_FMT_lx"\n", sprc);
> -        break;
> -    }
>       return 0;
>   }
>   
>   void helper_store_sprd(CPUPPCState *env, target_ulong val)
>   {
> -    target_ulong sprc = env->spr[SPR_POWER_SPRC];
>       PowerPCCPU *cpu = env_archcpu(env);
> -    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);
> -    }
> +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>   
> -    switch (sprc & 0x3e0) {
> -    case 0: /* SCRATCH0-3 */
> -    case 1: /* SCRATCH4-7 */
> -        /*
> -         * Log stores to SCRATCH, because some firmware uses these for
> -         * debugging and logging, but they would normally be read by the BMC,
> -         * which is not implemented in QEMU yet. This gives a way to get at the
> -         * information. Could also dump these upon checkstop.
> -         */
> -        nr = (sprc >> 3) & 0x7;
> -        pc->scratch[nr] = val;
> -        break;
> -    default:
> -        qemu_log_mask(LOG_UNIMP, "mtSPRD: Unimplemented SPRC:0x"
> -                                  TARGET_FMT_lx"\n", sprc);
> -        break;
> +    if (pcc->store_sprd) {
> +        return pcc->store_sprd(env, val);
>       }
>   }
>