Re: [PATCH v10 7/9] s390x/cpu topology: add max_threads machine class attribute

2022-10-27 Thread Pierre Morel




On 10/27/22 12:00, Cédric Le Goater wrote:

Hello Pierre,

On 10/12/22 18:21, Pierre Morel wrote:

The S390 CPU topology accepts the smp.threads argument while
in reality it does not effectively allow multthreading.

Let's keep this behavior for machines older than 7.3 and
refuse to use threads in newer machines until multithreading
is really proposed to the guest by the machine.


This change is unrelated to the rest of the series and we could merge it
for 7.2. We still have time for it.


OK, then I send it on its own

Regards,
Pierre

...

--
Pierre Morel
IBM Lab Boeblingen



Re: [PATCH v10 7/9] s390x/cpu topology: add max_threads machine class attribute

2022-10-27 Thread Cédric Le Goater

Hello Pierre,

On 10/12/22 18:21, Pierre Morel wrote:

The S390 CPU topology accepts the smp.threads argument while
in reality it does not effectively allow multthreading.

Let's keep this behavior for machines older than 7.3 and
refuse to use threads in newer machines until multithreading
is really proposed to the guest by the machine.


This change is unrelated to the rest of the series and we could merge it
for 7.2. We still have time for it.

Thanks,

C.

 

Signed-off-by: Pierre Morel 
---
  include/hw/s390x/s390-virtio-ccw.h |  1 +
  hw/s390x/s390-virtio-ccw.c | 10 ++
  2 files changed, 11 insertions(+)

diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index 6c4b4645fc..319dfac1bb 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -48,6 +48,7 @@ struct S390CcwMachineClass {
  bool css_migration_enabled;
  bool hpage_1m_allowed;
  bool topology_allowed;
+int max_threads;
  };
  
  /* runtime-instrumentation allowed by the machine */

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3a13fad4df..d6ce31d168 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -85,8 +85,15 @@ out:
  static void s390_init_cpus(MachineState *machine)
  {
  MachineClass *mc = MACHINE_GET_CLASS(machine);
+S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
  int i;
  
+if (machine->smp.threads > s390mc->max_threads) {

+error_report("S390 does not support more than %d threads.",
+ s390mc->max_threads);
+exit(1);
+}
+
  /* initialize possible_cpus */
  mc->possible_cpu_arch_ids(machine);
  
@@ -617,6 +624,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)

  s390mc->css_migration_enabled = true;
  s390mc->hpage_1m_allowed = true;
  s390mc->topology_allowed = true;
+s390mc->max_threads = 1;
  mc->init = ccw_init;
  mc->reset = s390_machine_reset;
  mc->block_default_type = IF_VIRTIO;
@@ -887,12 +895,14 @@ 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", },
+{ TYPE_S390_CPU_TOPOLOGY, "max_threads", "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;
+s390mc->max_threads = S390_MAX_CPUS;
  }
  DEFINE_CCW_MACHINE(7_2, "7.2", false);
  





Re: [PATCH v10 7/9] s390x/cpu topology: add max_threads machine class attribute

2022-10-26 Thread Pierre Morel




On 10/18/22 19:36, Cédric Le Goater wrote:

On 10/12/22 18:21, Pierre Morel wrote:

The S390 CPU topology accepts the smp.threads argument while
in reality it does not effectively allow multthreading.

Let's keep this behavior for machines older than 7.3 and
refuse to use threads in newer machines until multithreading
is really proposed to the guest by the machine.

Signed-off-by: Pierre Morel 
---
  include/hw/s390x/s390-virtio-ccw.h |  1 +
  hw/s390x/s390-virtio-ccw.c | 10 ++
  2 files changed, 11 insertions(+)

diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h

index 6c4b4645fc..319dfac1bb 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -48,6 +48,7 @@ struct S390CcwMachineClass {
  bool css_migration_enabled;
  bool hpage_1m_allowed;
  bool topology_allowed;
+    int max_threads;
  };
  /* runtime-instrumentation allowed by the machine */
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3a13fad4df..d6ce31d168 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -85,8 +85,15 @@ out:
  static void s390_init_cpus(MachineState *machine)
  {
  MachineClass *mc = MACHINE_GET_CLASS(machine);
+    S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
  int i;
+    if (machine->smp.threads > s390mc->max_threads) {
+    error_report("S390 does not support more than %d threads.",
+ s390mc->max_threads);
+    exit(1);
+    }
+
  /* initialize possible_cpus */
  mc->possible_cpu_arch_ids(machine);
@@ -617,6 +624,7 @@ static void ccw_machine_class_init(ObjectClass 
*oc, void *data)

  s390mc->css_migration_enabled = true;
  s390mc->hpage_1m_allowed = true;
  s390mc->topology_allowed = true;
+    s390mc->max_threads = 1;
  mc->init = ccw_init;
  mc->reset = s390_machine_reset;
  mc->block_default_type = IF_VIRTIO;
@@ -887,12 +895,14 @@ 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", },
+    { TYPE_S390_CPU_TOPOLOGY, "max_threads", "off", },


I don't understand this change.


hum, this was a try to understand how GlobalProperty_compat works and I 
forgot to remove it.

I must say I did not understand exactly how it works.




C.



  };
  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;
+    s390mc->max_threads = S390_MAX_CPUS;
  }
  DEFINE_CCW_MACHINE(7_2, "7.2", false);




--
Pierre Morel
IBM Lab Boeblingen



Re: [PATCH v10 7/9] s390x/cpu topology: add max_threads machine class attribute

2022-10-18 Thread Cédric Le Goater

On 10/12/22 18:21, Pierre Morel wrote:

The S390 CPU topology accepts the smp.threads argument while
in reality it does not effectively allow multthreading.

Let's keep this behavior for machines older than 7.3 and
refuse to use threads in newer machines until multithreading
is really proposed to the guest by the machine.

Signed-off-by: Pierre Morel 
---
  include/hw/s390x/s390-virtio-ccw.h |  1 +
  hw/s390x/s390-virtio-ccw.c | 10 ++
  2 files changed, 11 insertions(+)

diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index 6c4b4645fc..319dfac1bb 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -48,6 +48,7 @@ struct S390CcwMachineClass {
  bool css_migration_enabled;
  bool hpage_1m_allowed;
  bool topology_allowed;
+int max_threads;
  };
  
  /* runtime-instrumentation allowed by the machine */

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3a13fad4df..d6ce31d168 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -85,8 +85,15 @@ out:
  static void s390_init_cpus(MachineState *machine)
  {
  MachineClass *mc = MACHINE_GET_CLASS(machine);
+S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
  int i;
  
+if (machine->smp.threads > s390mc->max_threads) {

+error_report("S390 does not support more than %d threads.",
+ s390mc->max_threads);
+exit(1);
+}
+
  /* initialize possible_cpus */
  mc->possible_cpu_arch_ids(machine);
  
@@ -617,6 +624,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data)

  s390mc->css_migration_enabled = true;
  s390mc->hpage_1m_allowed = true;
  s390mc->topology_allowed = true;
+s390mc->max_threads = 1;
  mc->init = ccw_init;
  mc->reset = s390_machine_reset;
  mc->block_default_type = IF_VIRTIO;
@@ -887,12 +895,14 @@ 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", },
+{ TYPE_S390_CPU_TOPOLOGY, "max_threads", "off", },


I don't understand this change.


C.



  };
  
  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;
+s390mc->max_threads = S390_MAX_CPUS;
  }
  DEFINE_CCW_MACHINE(7_2, "7.2", false);
  





[PATCH v10 7/9] s390x/cpu topology: add max_threads machine class attribute

2022-10-12 Thread Pierre Morel
The S390 CPU topology accepts the smp.threads argument while
in reality it does not effectively allow multthreading.

Let's keep this behavior for machines older than 7.3 and
refuse to use threads in newer machines until multithreading
is really proposed to the guest by the machine.

Signed-off-by: Pierre Morel 
---
 include/hw/s390x/s390-virtio-ccw.h |  1 +
 hw/s390x/s390-virtio-ccw.c | 10 ++
 2 files changed, 11 insertions(+)

diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index 6c4b4645fc..319dfac1bb 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -48,6 +48,7 @@ struct S390CcwMachineClass {
 bool css_migration_enabled;
 bool hpage_1m_allowed;
 bool topology_allowed;
+int max_threads;
 };
 
 /* runtime-instrumentation allowed by the machine */
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3a13fad4df..d6ce31d168 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -85,8 +85,15 @@ out:
 static void s390_init_cpus(MachineState *machine)
 {
 MachineClass *mc = MACHINE_GET_CLASS(machine);
+S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
 int i;
 
+if (machine->smp.threads > s390mc->max_threads) {
+error_report("S390 does not support more than %d threads.",
+ s390mc->max_threads);
+exit(1);
+}
+
 /* initialize possible_cpus */
 mc->possible_cpu_arch_ids(machine);
 
@@ -617,6 +624,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
*data)
 s390mc->css_migration_enabled = true;
 s390mc->hpage_1m_allowed = true;
 s390mc->topology_allowed = true;
+s390mc->max_threads = 1;
 mc->init = ccw_init;
 mc->reset = s390_machine_reset;
 mc->block_default_type = IF_VIRTIO;
@@ -887,12 +895,14 @@ 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", },
+{ TYPE_S390_CPU_TOPOLOGY, "max_threads", "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;
+s390mc->max_threads = S390_MAX_CPUS;
 }
 DEFINE_CCW_MACHINE(7_2, "7.2", false);
 
-- 
2.31.1