Re: [PATCH v9 07/10] s390x/cpu_topology: CPU topology migration

[Pierre Morel <pmorel@linux.ibm.com>]

On 9/8/22 20:04, Janis Schoetterl-Glausch wrote:
> On Fri, 2022-09-02 at 09:55 +0200, Pierre Morel wrote:
>> The migration can only take place if both source and destination
>> of the migration both use or both do not use the CPU topology
>> facility.
>>
>> We indicate a change in topology during migration postload for the
>> case the topology changed between source and destination.
> 
> You always set the report bit after migration, right?
> In the last series you actually migrated the bit.
> Why the change? With the code you have actually migrating the bit isn't
> hard.

As for the moment the vCPU do not migrate from real CPU I thought based 
on the remark of Nico that there is no need to set the bit after a 
migration.

>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   hw/s390x/cpu-topology.c         | 79 +++++++++++++++++++++++++++++++++
>>   include/hw/s390x/cpu-topology.h |  1 +
>>   target/s390x/cpu-sysemu.c       |  8 ++++
>>   target/s390x/cpu.h              |  1 +
>>   4 files changed, 89 insertions(+)
>>
>> diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
>> index 6098d6ea1f..b6bf839e40 100644
>> --- a/hw/s390x/cpu-topology.c
>> +++ b/hw/s390x/cpu-topology.c
>> @@ -19,6 +19,7 @@
>>   #include "target/s390x/cpu.h"
>>   #include "hw/s390x/s390-virtio-ccw.h"
>>   #include "hw/s390x/cpu-topology.h"
>> +#include "migration/vmstate.h"
>>   
>>   S390Topology *s390_get_topology(void)
>>   {
>> @@ -132,6 +133,83 @@ static void s390_topology_reset(DeviceState *dev)
>>       s390_cpu_topology_reset();
>>   }
>>   
>> +/**
>> + * cpu_topology_postload
>> + * @opaque: a pointer to the S390Topology
>> + * @version_id: version identifier
>> + *
>> + * We check that the topology is used or is not used
>> + * on both side identically.
>> + *
>> + * If the topology is in use we set the Modified Topology Change Report
>> + * on the destination host.
>> + */
>> +static int cpu_topology_postload(void *opaque, int version_id)
>> +{
>> +    S390Topology *topo = opaque;
>> +    int ret;
>> +
>> +    if (topo->topology_needed != s390_has_feat(S390_FEAT_CONFIGURATION_TOPOLOGY)) {
> 
> Does this function even run if topology_needed is false?
> In that case there is no data saved, so no reason to load it either.
> If so you can only check that both the source and the destination have
> the feature enabled. You would need to always send the topology VMSD in
> order to check that the feature is disabled.
> 
> Does qemu allow you to attempt to migrate to a host with another cpu
> model?
> If it disallowes that you wouldn't need to do any checks, right?

hum yes I must rework this

Thanks,
Pierre


-- 
Pierre Morel
IBM Lab Boeblingen