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

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 10665 bytes --]

On Wed, Aug 20, 2025 at 05:55:17PM +0530, 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
>     ...

QEMU v10.1.0-rc4 is scheduled to become the final release (QEMU v10.1.0)
on Tuesday, August 26th. My view is that there is no need to delay the
release for this issue, because it affects few users and
--without-default-devices implies they are already customizing the
build. They can use a workaround the issue by enabling POWERNV or wait
for QEMU v10.1.1 (-stable tree) for the fix.

I have added it as a known issue to the QEMU 10.1 changelog:
https://wiki.qemu.org/ChangeLog/10.1#Known_issues

If anyone disagrees, please let me know.

Thanks,
Stefan

> 
> 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>
> ---
> 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);
>      }
>  }
>  
> -- 
> 2.50.1
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]