Re: [PATCH] target/s390x: Add a CONFIG switch to disable legacy CPUs

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

Hi Thomas,

On 13/6/24 19:07, Thomas Huth wrote:
> Old CPU models are not officially supported anymore by IBM, and for
> downstream builds of QEMU, we would like to be able to disable these
> CPUs in the build. Thus add a CONFIG switch that can be used to
> disable these CPUs (and old machine types that use them by default).
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   If you're interested, the PDF that can be downloaded from
>   https://www.ibm.com/support/pages/ibm-mainframe-life-cycle-history
>   shows the supported CPUs in a nice diagram

I'd add this link ...

>   hw/s390x/s390-virtio-ccw.c | 9 +++++++++
>   target/s390x/cpu_models.c  | 3 +++
>   target/s390x/Kconfig       | 5 +++++
>   3 files changed, 17 insertions(+)
> 
> diff --git a/target/s390x/Kconfig b/target/s390x/Kconfig
> index d886be48b4..8a95f2bc3f 100644
> --- a/target/s390x/Kconfig
> +++ b/target/s390x/Kconfig
> @@ -2,3 +2,8 @@ config S390X
>       bool
>       select PCI
>       select S390_FLIC
> +
> +config S390X_LEGACY_CPUS
> +    bool
> +    default y
> +    depends on S390X
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index efb508cd2e..ffae95dcb3 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -22,6 +22,7 @@
>   #include "qemu/module.h"
>   #include "qemu/hw-version.h"
>   #include "qemu/qemu-print.h"
> +#include CONFIG_DEVICES
>   #ifndef CONFIG_USER_ONLY
>   #include "sysemu/sysemu.h"
>   #include "target/s390x/kvm/pv.h"
> @@ -47,6 +48,7 @@
>    * generation 15 one base feature and one optional feature have been deprecated.
>    */
>   static S390CPUDef s390_cpu_defs[] = {
> +#ifdef CONFIG_S390X_LEGACY_CPUS

... here :)

>       CPUDEF_INIT(0x2064, 7, 1, 38, 0x00000000U, "z900", "IBM zSeries 900 GA1"),
>       CPUDEF_INIT(0x2064, 7, 2, 38, 0x00000000U, "z900.2", "IBM zSeries 900 GA2"),
>       CPUDEF_INIT(0x2064, 7, 3, 38, 0x00000000U, "z900.3", "IBM zSeries 900 GA3"),
> @@ -78,6 +80,7 @@ static S390CPUDef s390_cpu_defs[] = {
>       CPUDEF_INIT(0x2964, 13, 1, 47, 0x08000000U, "z13", "IBM z13 GA1"),
>       CPUDEF_INIT(0x2964, 13, 2, 47, 0x08000000U, "z13.2", "IBM z13 GA2"),
>       CPUDEF_INIT(0x2965, 13, 2, 47, 0x08000000U, "z13s", "IBM z13s GA1"),
> +#endif
>       CPUDEF_INIT(0x3906, 14, 1, 47, 0x08000000U, "z14", "IBM z14 GA1"),
>       CPUDEF_INIT(0x3906, 14, 2, 47, 0x08000000U, "z14.2", "IBM z14 GA2"),
>       CPUDEF_INIT(0x3907, 14, 1, 47, 0x08000000U, "z14ZR1", "IBM z14 Model ZR1 GA1"),
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 3d0bc3e7f2..7529d2fba8 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -47,6 +47,7 @@
>   #include "migration/blocker.h"
>   #include "qapi/visitor.h"
>   #include "hw/s390x/cpu-topology.h"
> +#include CONFIG_DEVICES
>   
>   static Error *pv_mig_blocker;
>   
> @@ -603,6 +604,8 @@ static void s390_nmi(NMIState *n, int cpu_index, Error **errp)
>       s390_cpu_restart(S390_CPU(cs));
>   }
>   
> +#ifdef CONFIG_S390X_LEGACY_CPUS
> +
>   static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
>   {
>       /* same logic as in sclp.c */
> @@ -623,6 +626,8 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
>       return newsz;
>   }
>   
> +#endif
> +
>   static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp)
>   {
>       S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> @@ -989,6 +994,8 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
>   }
>   DEFINE_CCW_MACHINE(6_1, "6.1", false);
>   
> +#ifdef CONFIG_S390X_LEGACY_CPUS
> +
>   static void ccw_machine_6_0_instance_options(MachineState *machine)
>   {
>       static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V6_0 };

Should we deprecate machines up to v6.0?

> @@ -1272,6 +1279,8 @@ static void ccw_machine_2_4_class_options(MachineClass *mc)
>   }
>   DEFINE_CCW_MACHINE(2_4, "2.4", false);
>   
> +#endif
> +
>   static void ccw_machine_register_types(void)
>   {
>       type_register_static(&ccw_machine_info);