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

Pierre Morel
2022-10-26T09:04:58Z

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 <pmorel@linux.ibm.com>
>> ---
>>   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