Re: [PATCH v2] target/ppc: Deprecate Power8E and Power8NVL

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Aditya,

On 29/3/25 15:26, Aditya Gupta wrote:
> Power8E and Power8NVL variants are not of much use in QEMU now, and not
> being maintained either.
> 
> Newer skiboot might not be able to boot Power8NVL since skiboot v7.0
> 
> Deprecate the 8E and 8NVL variants.
> 
> After deprecation, QEMU will print a warning like below when the
> CPU/Chips are used:
> 
>      $ ./build/qemu-system-ppc64 -M powernv8 --cpu power8nvl -nographic
>      qemu-system-ppc64: warning: CPU model power8nvl_v1.0-powerpc64-cpu is deprecated -- CPU is unmaintained.
>      ...
>      $ ./build/qemu-system-ppc64 -M powernv8 --cpu power8e -nographic
>      qemu-system-ppc64: warning: CPU model power8e_v2.1-powerpc64-cpu is deprecated -- CPU is unmaintained.
>      ...
>      $ ./build/qemu-system-ppc64 -M pseries --cpu power8e -nographic
>      qemu-system-ppc64: warning: CPU model power8e_v2.1-powerpc64-cpu is deprecated -- CPU is unmaintained.
>      ...
> 
> Also, print '(deprecated)' for deprecated CPUs in 'qemu-system-ppc64
> --cpu ?':
> 
>      $ ./build/qemu-system-ppc64 --cpu help
>        ...
>        power8e_v2.1     PVR 004b0201 (deprecated)
>        power8e          (alias for power8e_v2.1)
>        power8nvl_v1.0   PVR 004c0100 (deprecated)
>        power8nvl        (alias for power8nvl_v1.0)
>        power8_v2.0      PVR 004d0200
>        power8           (alias for power8_v2.0)
>        power9_v2.0      PVR 004e1200
>        power9_v2.2      PVR 004e1202
>        ...
> 
> Suggested-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> 
> ---
> Cover Letter
> ============
> 
> Power8E and Power8NVL are not maintained, and not useful to qemu, and
> upstream skiboot also has removed support till Power8 DD1.
> Power8NVL CPU doesn't boot since skiboot v7.0, or following skiboot commit
> to be exact:
> 
>      commit c5424f683ee3 ("Remove support for POWER8 DD1")
> 
> No direct way to deprecate the pnv chips, a field like deprecation_note
> could be added, but felt not needed as the chip will only get used if
> the user requests corresponding 8E / 8NVL CPU, which will print
> deprecation warning.
> 
> Also, no separate pnv machine for 8E and 8NVL, user has to pass --cpu,
> which will throw the deprecation warning. So just deprecating CPUs should
> be enough.
> 
> Changelog
> =========
> v2:
>    + add mention to docs/about/deprecated.rst
>    + add '(deprecated)' in output of qemu-system-ppc64 --cpu help
> ---
> ---
>   docs/about/deprecated.rst |  9 +++++++++
>   target/ppc/cpu-models.c   | 17 ++++++++++++++---
>   target/ppc/cpu_init.c     |  7 ++++++-
>   3 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index e2b4f077d453..d6b39db2aaca 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -275,6 +275,15 @@ embedded 405 for power management (OCC) and other internal tasks, it
>   is theoretically possible to use QEMU to model them. Let's keep the
>   CPU implementation for a while before removing all support.
>   
> +Power8E and Power8NVL CPUs and corresponding Pnv chips (since 10.0)
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +The Power8E and Power8NVL variants of Power8 are not really useful anymore
> +in qemu, and are old and unmaintained now.
> +
> +The CPUs as well as corresponding Power8NVL and Power8E PnvChips will also
> +be considered deprecated.
> +
>   System emulator machines
>   ------------------------
>   
> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> index ece348178188..6f2062e2c484 100644
> --- a/target/ppc/cpu-models.c
> +++ b/target/ppc/cpu-models.c
> @@ -32,17 +32,22 @@
>   /* PowerPC CPU definitions                                                 */
>   #define POWERPC_DEF_PREFIX(pvr, svr, type)                                  \
>       glue(glue(glue(glue(pvr, _), svr), _), type)
> -#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type)                    \
> +#define POWERPC_DEF_SVR_DEPR(_name, _desc, _pvr, _svr, _type, _is_deprecated) \
>       static void                                                             \
>       glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_class_init)            \
>       (ObjectClass *oc, void *data)                                           \
>       {                                                                       \
>           DeviceClass *dc = DEVICE_CLASS(oc);                                 \
> +        CPUClass *cc    = CPU_CLASS(oc);                                    \
>           PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);                       \
>                                                                               \
>           pcc->pvr          = _pvr;                                           \
>           pcc->svr          = _svr;                                           \
>           dc->desc          = _desc;                                          \
> +                                                                            \
> +        if (_is_deprecated) {                                               \
> +            cc->deprecation_note = "CPU is unmaintained.";                  \

The CPUs deprecated in this patch are indeed unmaintained, but maybe
future deprecated ones will have a different reason.

Maybe use s/_is_deprecated/_deprecation_note/, so passing a NULL
deprecation note is OK?

           cc->deprecation_note = _deprecation_note; \

> +        }                                                                   \
>       }                                                                       \
>                                                                               \
>       static const TypeInfo                                                   \
> @@ -63,6 +68,12 @@
>       type_init(                                                              \
>           glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_register_types))
>   
> +#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type)                    \
> +    POWERPC_DEF_SVR_DEPR(_name, _desc, _pvr, _svr, _type, false)

        POWERPC_DEF_SVR_DEPR(..., NULL)

> +#define POWERPC_DEPRECATED_CPU(_name, _pvr, _type, _desc)                   \
> +    POWERPC_DEF_SVR_DEPR(_name, _desc, _pvr, POWERPC_SVR_NONE, _type, true)
> +
>   #define POWERPC_DEF(_name, _pvr, _type, _desc)                              \
>       POWERPC_DEF_SVR(_name, _desc, _pvr, POWERPC_SVR_NONE, _type)
>   
> @@ -722,11 +733,11 @@
>                   "POWER7 v2.3")
>       POWERPC_DEF("power7p_v2.1",  CPU_POWERPC_POWER7P_v21,            POWER7,
>                   "POWER7+ v2.1")
> -    POWERPC_DEF("power8e_v2.1",  CPU_POWERPC_POWER8E_v21,            POWER8,
> +    POWERPC_DEPRECATED_CPU("power8e_v2.1",  CPU_POWERPC_POWER8E_v21, POWER8,
>                   "POWER8E v2.1")

        POWERPC_DEF_SVR_DEPR("power8e_v2.1", CPU_POWERPC_POWER8E_v21,
                             POWER8, "POWER8E v2.1",
                             "CPU is unmaintained")

>       POWERPC_DEF("power8_v2.0",   CPU_POWERPC_POWER8_v20,             POWER8,
>                   "POWER8 v2.0")
> -    POWERPC_DEF("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10,         POWER8,
> +    POWERPC_DEPRECATED_CPU("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10, POWER8,
>                   "POWER8NVL v1.0")

        POWERPC_DEF_SVR_DEPR("power8nvl_v1.0", CPU_POWERPC_POWER8NVL_v10,
                             POWER8, "POWER8NVL v1.0",
                             "CPU is unmaintained")

>       POWERPC_DEF("power9_v2.0",   CPU_POWERPC_POWER9_DD20,            POWER9,
>                   "POWER9 v2.0")
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 7decc09aec8f..fade53f7e2d6 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -7143,6 +7143,7 @@ static void ppc_cpu_list_entry(gpointer data, gpointer user_data)
>   {
>       ObjectClass *oc = data;
>       PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> +    CPUClass *cc = CPU_CLASS(oc);
>       DeviceClass *family = DEVICE_CLASS(ppc_cpu_get_family_class(pcc));
>       const char *typename = object_class_get_name(oc);
>       char *name;
> @@ -7153,7 +7154,11 @@ static void ppc_cpu_list_entry(gpointer data, gpointer user_data)
>       }
>   
>       name = cpu_model_from_type(typename);
> -    qemu_printf("  %-16s PVR %08x\n", name, pcc->pvr);
> +    if (cc->deprecation_note) {
> +        qemu_printf("  %-16s PVR %08x (deprecated)\n", name, pcc->pvr);
> +    } else {
> +        qemu_printf("  %-16s PVR %08x\n", name, pcc->pvr);
> +    }
>       for (i = 0; ppc_cpu_aliases[i].alias != NULL; i++) {
>           PowerPCCPUAlias *alias = &ppc_cpu_aliases[i];
>           ObjectClass *alias_oc = ppc_cpu_class_by_name(alias->model);