S390 CPU topology is only allowed for s390-virtio-ccw-7.3 and
newer S390 machines.
We keep the possibility to disable the topology on these newer
machines with the property topology-disable.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
include/hw/boards.h | 3 ++
include/hw/s390x/cpu-topology.h | 18 +++++++++-
include/hw/s390x/s390-virtio-ccw.h | 2 ++
hw/core/machine.c | 5 +++
hw/s390x/s390-virtio-ccw.c | 53 +++++++++++++++++++++++++++++-
util/qemu-config.c | 4 +++
qemu-options.hx | 6 +++-
7 files changed, 88 insertions(+), 3 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 311ed17e18..67147c47bf 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -379,6 +379,9 @@ struct MachineState {
} \
type_init(machine_initfn##_register_types)
+extern GlobalProperty hw_compat_7_2[];
+extern const size_t hw_compat_7_2_len;
+
extern GlobalProperty hw_compat_7_1[];
extern const size_t hw_compat_7_1_len;
diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h
index 35a8a981ec..747c9ab4c6 100644
--- a/include/hw/s390x/cpu-topology.h
+++ b/include/hw/s390x/cpu-topology.h
@@ -12,6 +12,8 @@
#include "hw/qdev-core.h"
#include "qom/object.h"
+#include "cpu.h"
+#include "hw/s390x/s390-virtio-ccw.h"
#define S390_TOPOLOGY_POLARITY_H 0x00
@@ -43,7 +45,21 @@ void s390_topology_new_cpu(int core_id);
static inline bool s390_has_topology(void)
{
- return false;
+ static S390CcwMachineState *ccw;
+ Object *obj;
+
+ if (ccw) {
+ return !ccw->topology_disable;
+ }
+
+ /* we have to bail out for the "none" machine */
+ obj = object_dynamic_cast(qdev_get_machine(),
+ TYPE_S390_CCW_MACHINE);
+ if (!obj) {
+ return false;
+ }
+ ccw = S390_CCW_MACHINE(obj);
+ return !ccw->topology_disable;
}
#endif
diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index 9e7a0d75bc..6c4b4645fc 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -28,6 +28,7 @@ struct S390CcwMachineState {
bool dea_key_wrap;
bool pv;
bool zpcii_disable;
+ bool topology_disable;
uint8_t loadparm[8];
};
@@ -46,6 +47,7 @@ struct S390CcwMachineClass {
bool cpu_model_allowed;
bool css_migration_enabled;
bool hpage_1m_allowed;
+ bool topology_allowed;
};
/* runtime-instrumentation allowed by the machine */
diff --git a/hw/core/machine.c b/hw/core/machine.c
index aa520e74a8..93c497655e 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -40,6 +40,11 @@
#include "hw/virtio/virtio-pci.h"
#include "qom/object_interfaces.h"
+GlobalProperty hw_compat_7_2[] = {
+ { "s390-topology", "topology-disable", "true" },
+};
+const size_t hw_compat_7_2_len = G_N_ELEMENTS(hw_compat_7_2);
+
GlobalProperty hw_compat_7_1[] = {};
const size_t hw_compat_7_1_len = G_N_ELEMENTS(hw_compat_7_1);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 362378454a..3a13fad4df 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -616,6 +616,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
s390mc->cpu_model_allowed = true;
s390mc->css_migration_enabled = true;
s390mc->hpage_1m_allowed = true;
+ s390mc->topology_allowed = true;
mc->init = ccw_init;
mc->reset = s390_machine_reset;
mc->block_default_type = IF_VIRTIO;
@@ -726,6 +727,27 @@ bool hpage_1m_allowed(void)
return get_machine_class()->hpage_1m_allowed;
}
+static inline bool machine_get_topology_disable(Object *obj, Error **errp)
+{
+ S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
+
+ return ms->topology_disable;
+}
+
+static inline void machine_set_topology_disable(Object *obj, bool value,
+ Error **errp)
+{
+ S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
+
+ if (!get_machine_class()->topology_allowed) {
+ error_setg(errp, "Property topology-disable not available on machine %s",
+ get_machine_class()->parent_class.name);
+ return;
+ }
+
+ ms->topology_disable = value;
+}
+
static char *machine_get_loadparm(Object *obj, Error **errp)
{
S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
@@ -784,6 +806,13 @@ static inline void s390_machine_initfn(Object *obj)
object_property_set_description(obj, "zpcii-disable",
"disable zPCI interpretation facilties");
object_property_set_bool(obj, "zpcii-disable", false, NULL);
+
+ object_property_add_bool(obj, "topology-disable",
+ machine_get_topology_disable,
+ machine_set_topology_disable);
+ object_property_set_description(obj, "topology-disable",
+ "disable CPU topology");
+ object_property_set_bool(obj, "topology-disable", false, NULL);
}
static const TypeInfo ccw_machine_info = {
@@ -836,14 +865,36 @@ bool css_migration_enabled(void)
} \
type_init(ccw_machine_register_##suffix)
+static void ccw_machine_7_3_instance_options(MachineState *machine)
+{
+}
+
+static void ccw_machine_7_3_class_options(MachineClass *mc)
+{
+}
+DEFINE_CCW_MACHINE(7_3, "7.3", true);
+
static void ccw_machine_7_2_instance_options(MachineState *machine)
{
+ S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
+
+ ccw_machine_7_3_instance_options(machine);
+ ms->topology_disable = true;
}
static void ccw_machine_7_2_class_options(MachineClass *mc)
{
+ S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
+ static GlobalProperty compat[] = {
+ { TYPE_S390_CPU_TOPOLOGY, "topology-allowed", "off", },
+ };
+
+ ccw_machine_7_3_class_options(mc);
+ compat_props_add(mc->compat_props, hw_compat_7_2, hw_compat_7_2_len);
+ compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
+ s390mc->topology_allowed = false;
}
-DEFINE_CCW_MACHINE(7_2, "7.2", true);
+DEFINE_CCW_MACHINE(7_2, "7.2", false);
static void ccw_machine_7_1_instance_options(MachineState *machine)
{
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 5325f6bf80..c19e8bc8f3 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -240,6 +240,10 @@ static QemuOptsList machine_opts = {
.name = "zpcii-disable",
.type = QEMU_OPT_BOOL,
.help = "disable zPCI interpretation facilities",
+ },{
+ .name = "topology-disable",
+ .type = QEMU_OPT_BOOL,
+ .help = "disable CPU topology",
},
{ /* End of list */ }
}
diff --git a/qemu-options.hx b/qemu-options.hx
index 95b998a13b..c804b0f899 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -38,7 +38,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
" hmat=on|off controls ACPI HMAT support (default=off)\n"
" memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n"
" cxl-fmw.0.targets.0=firsttarget,cxl-fmw.0.targets.1=secondtarget,cxl-fmw.0.size=size[,cxl-fmw.0.interleave-granularity=granularity]\n"
- " zpcii-disable=on|off disables zPCI interpretation facilities (default=off)\n",
+ " zpcii-disable=on|off disables zPCI interpretation facilities (default=off)\n"
+ " topology-disable=on|off disables CPU topology (default=off)\n",
QEMU_ARCH_ALL)
SRST
``-machine [type=]name[,prop=value[,...]]``
@@ -163,6 +164,9 @@ SRST
Disables zPCI interpretation facilties on s390-ccw hosts.
This feature can be used to disable hardware virtual assists
related to zPCI devices. The default is off.
+
+ ``topology-disable=on|off``
+ Disables CPU topology on for S390 machines starting with s390-ccw-virtio-7.3.
ERST
DEF("M", HAS_ARG, QEMU_OPTION_M,
--
2.31.1On 10/12/22 18:21, Pierre Morel wrote: > S390 CPU topology is only allowed for s390-virtio-ccw-7.3 and > newer S390 machines. > We keep the possibility to disable the topology on these newer > machines with the property topology-disable. Isn't 'topology' enough for the property ? I don't think the '-disable' prefix adds much to the meaning. C.
On 10/18/22 19:51, Cédric Le Goater wrote: > On 10/12/22 18:21, Pierre Morel wrote: >> S390 CPU topology is only allowed for s390-virtio-ccw-7.3 and >> newer S390 machines. >> We keep the possibility to disable the topology on these newer >> machines with the property topology-disable. > > Isn't 'topology' enough for the property ? I don't think the > '-disable' prefix adds much to the meaning. > > C. > > Agreed. Pierre -- Pierre Morel IBM Lab Boeblingen
On 10/12/22 18:21, Pierre Morel wrote:
> S390 CPU topology is only allowed for s390-virtio-ccw-7.3 and
> newer S390 machines.
> We keep the possibility to disable the topology on these newer
> machines with the property topology-disable.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> include/hw/boards.h | 3 ++
> include/hw/s390x/cpu-topology.h | 18 +++++++++-
> include/hw/s390x/s390-virtio-ccw.h | 2 ++
> hw/core/machine.c | 5 +++
> hw/s390x/s390-virtio-ccw.c | 53 +++++++++++++++++++++++++++++-
> util/qemu-config.c | 4 +++
> qemu-options.hx | 6 +++-
> 7 files changed, 88 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 311ed17e18..67147c47bf 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -379,6 +379,9 @@ struct MachineState {
> } \
> type_init(machine_initfn##_register_types)
>
> +extern GlobalProperty hw_compat_7_2[];
> +extern const size_t hw_compat_7_2_len;
QEMU 7.2 is not out yet.
> +
> extern GlobalProperty hw_compat_7_1[];
> extern const size_t hw_compat_7_1_len;
>
> diff --git a/include/hw/s390x/cpu-topology.h b/include/hw/s390x/cpu-topology.h
> index 35a8a981ec..747c9ab4c6 100644
> --- a/include/hw/s390x/cpu-topology.h
> +++ b/include/hw/s390x/cpu-topology.h
> @@ -12,6 +12,8 @@
>
> #include "hw/qdev-core.h"
> #include "qom/object.h"
> +#include "cpu.h"
> +#include "hw/s390x/s390-virtio-ccw.h"
>
> #define S390_TOPOLOGY_POLARITY_H 0x00
>
> @@ -43,7 +45,21 @@ void s390_topology_new_cpu(int core_id);
>
> static inline bool s390_has_topology(void)
> {
> - return false;
> + static S390CcwMachineState *ccw;
hmm, s390_has_topology is a static inline. It would be preferable to
change its definition to extern.
> + Object *obj;
> +
> + if (ccw) {
> + return !ccw->topology_disable;
> + }
> +
> + /* we have to bail out for the "none" machine */
> + obj = object_dynamic_cast(qdev_get_machine(),
> + TYPE_S390_CCW_MACHINE);
> + if (!obj) {
> + return false;
> + }
> + ccw = S390_CCW_MACHINE(obj);
> + return !ccw->topology_disable;
> }
>
> #endif
> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> index 9e7a0d75bc..6c4b4645fc 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -28,6 +28,7 @@ struct S390CcwMachineState {
> bool dea_key_wrap;
> bool pv;
> bool zpcii_disable;
> + bool topology_disable;
> uint8_t loadparm[8];
> };
>
> @@ -46,6 +47,7 @@ struct S390CcwMachineClass {
> bool cpu_model_allowed;
> bool css_migration_enabled;
> bool hpage_1m_allowed;
> + bool topology_allowed;
'topology_disable' in the state and 'topology_allowed' in the class.
This is confusing :/
you should add 'topology_allowed' in its own patch and maybe call
it 'topology_capable' ? it is a QEMU capability AIUI
> };
>
> /* runtime-instrumentation allowed by the machine */
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index aa520e74a8..93c497655e 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -40,6 +40,11 @@
> #include "hw/virtio/virtio-pci.h"
> #include "qom/object_interfaces.h"
>
> +GlobalProperty hw_compat_7_2[] = {
> + { "s390-topology", "topology-disable", "true" },
May be use TYPE_S390_CPU_TOPOLOGY instead.
But again, this should only apply to 7.1 machines and below. 7.2 is
not out yet.
> +};
> +const size_t hw_compat_7_2_len = G_N_ELEMENTS(hw_compat_7_2);
> +
> GlobalProperty hw_compat_7_1[] = {};
> const size_t hw_compat_7_1_len = G_N_ELEMENTS(hw_compat_7_1);
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 362378454a..3a13fad4df 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -616,6 +616,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)
> s390mc->cpu_model_allowed = true;
> s390mc->css_migration_enabled = true;
> s390mc->hpage_1m_allowed = true;
> + s390mc->topology_allowed = true;
> mc->init = ccw_init;
> mc->reset = s390_machine_reset;
> mc->block_default_type = IF_VIRTIO;
> @@ -726,6 +727,27 @@ bool hpage_1m_allowed(void)
> return get_machine_class()->hpage_1m_allowed;
> }
>
> +static inline bool machine_get_topology_disable(Object *obj, Error **errp)
> +{
> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> +
> + return ms->topology_disable;
> +}
> +
> +static inline void machine_set_topology_disable(Object *obj, bool value,
> + Error **errp)
> +{
> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> +
> + if (!get_machine_class()->topology_allowed) {
> + error_setg(errp, "Property topology-disable not available on machine %s",
> + get_machine_class()->parent_class.name);
OK. I get it now. May be we should consider adding the capability concept
David introduced in the pseries machine. Please take a look. That's not
for this patchset though. It would be too much work.
> + return;
> + }
> +
> + ms->topology_disable = value;
> +}
> +
> static char *machine_get_loadparm(Object *obj, Error **errp)
> {
> S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> @@ -784,6 +806,13 @@ static inline void s390_machine_initfn(Object *obj)
> object_property_set_description(obj, "zpcii-disable",
> "disable zPCI interpretation facilties");
> object_property_set_bool(obj, "zpcii-disable", false, NULL);
> +
> + object_property_add_bool(obj, "topology-disable",
> + machine_get_topology_disable,
> + machine_set_topology_disable);
> + object_property_set_description(obj, "topology-disable",
> + "disable CPU topology");
> + object_property_set_bool(obj, "topology-disable", false, NULL);
All the properties should be added in the machine class_init routine.
There is a preliminary cleanup patch required to move them all :/
> }
>
> static const TypeInfo ccw_machine_info = {
> @@ -836,14 +865,36 @@ bool css_migration_enabled(void)
> } \
> type_init(ccw_machine_register_##suffix)
>
> +static void ccw_machine_7_3_instance_options(MachineState *machine)
> +{
> +}
> +
> +static void ccw_machine_7_3_class_options(MachineClass *mc)
> +{
> +}
> +DEFINE_CCW_MACHINE(7_3, "7.3", true);
That's too early.
> +
> static void ccw_machine_7_2_instance_options(MachineState *machine)
> {
> + S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
> +
> + ccw_machine_7_3_instance_options(machine);
> + ms->topology_disable = true;
> }
>
> static void ccw_machine_7_2_class_options(MachineClass *mc)
> {
> + S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
> + static GlobalProperty compat[] = {
> + { TYPE_S390_CPU_TOPOLOGY, "topology-allowed", "off", },
hmm, "topology-allowed" is not a TYPE_S390_CPU_TOPOLOGY property.
> + };
> +
> + ccw_machine_7_3_class_options(mc);
> + compat_props_add(mc->compat_props, hw_compat_7_2, hw_compat_7_2_len);
> + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
> + s390mc->topology_allowed = false;
> }
> -DEFINE_CCW_MACHINE(7_2, "7.2", true);
> +DEFINE_CCW_MACHINE(7_2, "7.2", false);
>
> static void ccw_machine_7_1_instance_options(MachineState *machine)
> {
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index 5325f6bf80..c19e8bc8f3 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -240,6 +240,10 @@ static QemuOptsList machine_opts = {
> .name = "zpcii-disable",
> .type = QEMU_OPT_BOOL,
> .help = "disable zPCI interpretation facilities",
> + },{
> + .name = "topology-disable",
> + .type = QEMU_OPT_BOOL,
> + .help = "disable CPU topology",
> },
> { /* End of list */ }
> }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 95b998a13b..c804b0f899 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -38,7 +38,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
> " hmat=on|off controls ACPI HMAT support (default=off)\n"
> " memory-backend='backend-id' specifies explicitly provided backend for main RAM (default=none)\n"
> " cxl-fmw.0.targets.0=firsttarget,cxl-fmw.0.targets.1=secondtarget,cxl-fmw.0.size=size[,cxl-fmw.0.interleave-granularity=granularity]\n"
> - " zpcii-disable=on|off disables zPCI interpretation facilities (default=off)\n",
> + " zpcii-disable=on|off disables zPCI interpretation facilities (default=off)\n"
> + " topology-disable=on|off disables CPU topology (default=off)\n",
> QEMU_ARCH_ALL)
> SRST
> ``-machine [type=]name[,prop=value[,...]]``
> @@ -163,6 +164,9 @@ SRST
> Disables zPCI interpretation facilties on s390-ccw hosts.
> This feature can be used to disable hardware virtual assists
> related to zPCI devices. The default is off.
> +
> + ``topology-disable=on|off``
> + Disables CPU topology on for S390 machines starting with s390-ccw-virtio-7.3.
> ERST
>
> DEF("M", HAS_ARG, QEMU_OPTION_M,
On 10/18/22 19:34, Cédric Le Goater wrote:
> On 10/12/22 18:21, Pierre Morel wrote:
>> S390 CPU topology is only allowed for s390-virtio-ccw-7.3 and
>> newer S390 machines.
>> We keep the possibility to disable the topology on these newer
>> machines with the property topology-disable.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> include/hw/boards.h | 3 ++
>> include/hw/s390x/cpu-topology.h | 18 +++++++++-
>> include/hw/s390x/s390-virtio-ccw.h | 2 ++
>> hw/core/machine.c | 5 +++
>> hw/s390x/s390-virtio-ccw.c | 53 +++++++++++++++++++++++++++++-
>> util/qemu-config.c | 4 +++
>> qemu-options.hx | 6 +++-
>> 7 files changed, 88 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 311ed17e18..67147c47bf 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -379,6 +379,9 @@ struct MachineState {
>> } \
>> type_init(machine_initfn##_register_types)
>> +extern GlobalProperty hw_compat_7_2[];
>> +extern const size_t hw_compat_7_2_len;
>
> QEMU 7.2 is not out yet.
OK, thanks, I will use 7.2 instead of 7.3 as the new machine.
We see later if it goes to 8.0
>
>> +
>> extern GlobalProperty hw_compat_7_1[];
>> extern const size_t hw_compat_7_1_len;
>> diff --git a/include/hw/s390x/cpu-topology.h
>> b/include/hw/s390x/cpu-topology.h
>> index 35a8a981ec..747c9ab4c6 100644
>> --- a/include/hw/s390x/cpu-topology.h
>> +++ b/include/hw/s390x/cpu-topology.h
>> @@ -12,6 +12,8 @@
>> #include "hw/qdev-core.h"
>> #include "qom/object.h"
>> +#include "cpu.h"
>> +#include "hw/s390x/s390-virtio-ccw.h"
>> #define S390_TOPOLOGY_POLARITY_H 0x00
>> @@ -43,7 +45,21 @@ void s390_topology_new_cpu(int core_id);
>> static inline bool s390_has_topology(void)
>> {
>> - return false;
>> + static S390CcwMachineState *ccw;
>
> hmm, s390_has_topology is a static inline. It would be preferable to
> change its definition to extern.
OK
>
>> + Object *obj;
>> +
>> + if (ccw) {
>> + return !ccw->topology_disable;
>> + }
>> +
>> + /* we have to bail out for the "none" machine */
>> + obj = object_dynamic_cast(qdev_get_machine(),
>> + TYPE_S390_CCW_MACHINE);
>> + if (!obj) {
>> + return false;
>> + }
>> + ccw = S390_CCW_MACHINE(obj);
>> + return !ccw->topology_disable;
>> }
>> #endif
>> diff --git a/include/hw/s390x/s390-virtio-ccw.h
>> b/include/hw/s390x/s390-virtio-ccw.h
>> index 9e7a0d75bc..6c4b4645fc 100644
>> --- a/include/hw/s390x/s390-virtio-ccw.h
>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>> @@ -28,6 +28,7 @@ struct S390CcwMachineState {
>> bool dea_key_wrap;
>> bool pv;
>> bool zpcii_disable;
>> + bool topology_disable;
>> uint8_t loadparm[8];
>> };
>> @@ -46,6 +47,7 @@ struct S390CcwMachineClass {
>> bool cpu_model_allowed;
>> bool css_migration_enabled;
>> bool hpage_1m_allowed;
>> + bool topology_allowed;
>
> 'topology_disable' in the state and 'topology_allowed' in the class.
> This is confusing :/
>
> you should add 'topology_allowed' in its own patch and maybe call
> it 'topology_capable' ? it is a QEMU capability AIUI
yes, OK, I separate the two.
. topology_capable to know if the QEMU version is capable of handling
topology
- topology_disable for migration if KVM is not able to handle the
topology on one of the machines involved.
>
>> };
>> /* runtime-instrumentation allowed by the machine */
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index aa520e74a8..93c497655e 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -40,6 +40,11 @@
>> #include "hw/virtio/virtio-pci.h"
>> #include "qom/object_interfaces.h"
>> +GlobalProperty hw_compat_7_2[] = {
>> + { "s390-topology", "topology-disable", "true" },
>
> May be use TYPE_S390_CPU_TOPOLOGY instead.
>
> But again, this should only apply to 7.1 machines and below. 7.2 is
> not out yet.
OK
>
>
>> +};
>> +const size_t hw_compat_7_2_len = G_N_ELEMENTS(hw_compat_7_2);
>> +
>> GlobalProperty hw_compat_7_1[] = {};
>> const size_t hw_compat_7_1_len = G_N_ELEMENTS(hw_compat_7_1);
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 362378454a..3a13fad4df 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -616,6 +616,7 @@ static void ccw_machine_class_init(ObjectClass
>> *oc, void *data)
>> s390mc->cpu_model_allowed = true;
>> s390mc->css_migration_enabled = true;
>> s390mc->hpage_1m_allowed = true;
>> + s390mc->topology_allowed = true;
>> mc->init = ccw_init;
>> mc->reset = s390_machine_reset;
>> mc->block_default_type = IF_VIRTIO;
>> @@ -726,6 +727,27 @@ bool hpage_1m_allowed(void)
>> return get_machine_class()->hpage_1m_allowed;
>> }
>> +static inline bool machine_get_topology_disable(Object *obj, Error
>> **errp)
>> +{
>> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>> +
>> + return ms->topology_disable;
>> +}
>> +
>> +static inline void machine_set_topology_disable(Object *obj, bool value,
>> + Error **errp)
>> +{
>> + S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>> +
>> + if (!get_machine_class()->topology_allowed) {
>> + error_setg(errp, "Property topology-disable not available on
>> machine %s",
>> + get_machine_class()->parent_class.name);
>
> OK. I get it now. May be we should consider adding the capability concept
> David introduced in the pseries machine. Please take a look. That's not
> for this patchset though. It would be too much work.
Yes, interesting, would be something to do in the near future.
>
>> + return;
>> + }
>> +
>> + ms->topology_disable = value;
>> +}
>> +
>> static char *machine_get_loadparm(Object *obj, Error **errp)
>> {
>> S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>> @@ -784,6 +806,13 @@ static inline void s390_machine_initfn(Object *obj)
>> object_property_set_description(obj, "zpcii-disable",
>> "disable zPCI interpretation facilties");
>> object_property_set_bool(obj, "zpcii-disable", false, NULL);
>> +
>> + object_property_add_bool(obj, "topology-disable",
>> + machine_get_topology_disable,
>> + machine_set_topology_disable);
>> + object_property_set_description(obj, "topology-disable",
>> + "disable CPU topology");
>> + object_property_set_bool(obj, "topology-disable", false, NULL);
>
> All the properties should be added in the machine class_init routine.
> There is a preliminary cleanup patch required to move them all :/
OK, I will move it to the class_init
>> }
>> static const TypeInfo ccw_machine_info = {
>> @@ -836,14 +865,36 @@ bool css_migration_enabled(void)
>>
>> } \
>> type_init(ccw_machine_register_##suffix)
>> +static void ccw_machine_7_3_instance_options(MachineState *machine)
>> +{
>> +}
>> +
>> +static void ccw_machine_7_3_class_options(MachineClass *mc)
>> +{
>> +}
>> +DEFINE_CCW_MACHINE(7_3, "7.3", true);
>
> That's too early.
>
>> +
>> static void ccw_machine_7_2_instance_options(MachineState *machine)
>> {
>> + S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
>> +
>> + ccw_machine_7_3_instance_options(machine);
>> + ms->topology_disable = true;
>> }
>> static void ccw_machine_7_2_class_options(MachineClass *mc)
>> {
>> + S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
>> + static GlobalProperty compat[] = {
>> + { TYPE_S390_CPU_TOPOLOGY, "topology-allowed", "off", },
>
> hmm, "topology-allowed" is not a TYPE_S390_CPU_TOPOLOGY property.
I do not understand what I did there and why.
I guess I better do nothing there.
Thanks for the comments,
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
On Tue, Oct 18 2022, Cédric Le Goater <clg@kaod.org> wrote:
> On 10/12/22 18:21, Pierre Morel wrote:
>> S390 CPU topology is only allowed for s390-virtio-ccw-7.3 and
>> newer S390 machines.
>> We keep the possibility to disable the topology on these newer
>> machines with the property topology-disable.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> include/hw/boards.h | 3 ++
>> include/hw/s390x/cpu-topology.h | 18 +++++++++-
>> include/hw/s390x/s390-virtio-ccw.h | 2 ++
>> hw/core/machine.c | 5 +++
>> hw/s390x/s390-virtio-ccw.c | 53 +++++++++++++++++++++++++++++-
>> util/qemu-config.c | 4 +++
>> qemu-options.hx | 6 +++-
>> 7 files changed, 88 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 311ed17e18..67147c47bf 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -379,6 +379,9 @@ struct MachineState {
>> } \
>> type_init(machine_initfn##_register_types)
>>
>> +extern GlobalProperty hw_compat_7_2[];
>> +extern const size_t hw_compat_7_2_len;
>
> QEMU 7.2 is not out yet.
Yes, and the introduction of the new compat machines needs to go into a
separate patch. I'm usually preparing that patch while QEMU is in
freeze, but feel free to cook up a patch earlier if you need it.
(...)
>> +static void ccw_machine_7_3_instance_options(MachineState *machine)
>> +{
>> +}
>> +
>> +static void ccw_machine_7_3_class_options(MachineClass *mc)
>> +{
>> +}
>> +DEFINE_CCW_MACHINE(7_3, "7.3", true);
>
> That's too early.
Also, the next QEMU version will be 8.0, not 7.3 :)
On 10/19/22 11:03, Cornelia Huck wrote:
> On Tue, Oct 18 2022, Cédric Le Goater <clg@kaod.org> wrote:
>
>> On 10/12/22 18:21, Pierre Morel wrote:
>>> S390 CPU topology is only allowed for s390-virtio-ccw-7.3 and
>>> newer S390 machines.
>>> We keep the possibility to disable the topology on these newer
>>> machines with the property topology-disable.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>> include/hw/boards.h | 3 ++
>>> include/hw/s390x/cpu-topology.h | 18 +++++++++-
>>> include/hw/s390x/s390-virtio-ccw.h | 2 ++
>>> hw/core/machine.c | 5 +++
>>> hw/s390x/s390-virtio-ccw.c | 53 +++++++++++++++++++++++++++++-
>>> util/qemu-config.c | 4 +++
>>> qemu-options.hx | 6 +++-
>>> 7 files changed, 88 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>>> index 311ed17e18..67147c47bf 100644
>>> --- a/include/hw/boards.h
>>> +++ b/include/hw/boards.h
>>> @@ -379,6 +379,9 @@ struct MachineState {
>>> } \
>>> type_init(machine_initfn##_register_types)
>>>
>>> +extern GlobalProperty hw_compat_7_2[];
>>> +extern const size_t hw_compat_7_2_len;
>>
>> QEMU 7.2 is not out yet.
>
> Yes, and the introduction of the new compat machines needs to go into a
> separate patch. I'm usually preparing that patch while QEMU is in
> freeze, but feel free to cook up a patch earlier if you need it.
OK, Thanks, I understand I put it in a separate file so it can be
adapted at the moment the series will need to be merged.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
© 2016 - 2026 Red Hat, Inc.