Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v1 3/5] spapr: Implement CPUClass::get_migration_id() for PowerPC CPUs

[Mark Cave-Ayland]

On 07/07/16 02:00, David Gibson wrote:

> On Wed, Jul 06, 2016 at 02:29:19PM +0530, Bharata B Rao wrote:
>> cpu_index is used as migration_id by default. For machine type versions
>> that set use-migration-id property, cpu_dt_it is returned.
>>
>> Signed-off-by: Bharata B Rao <address@hidden>
> 
> This seems ceonceptually wrong.  The logic to determine the stable ID
> is still coming from the CPU code.  But kind of the point here is that
> in most cases the stable IDs really belong to the machine type, not
> the CPU.
> 
> Would it make more sense to just have migration-id be a settable
> property, which the machine type needs to set between init and
> realize?  (or leave as default == cpu_index).
> 
> That way we could have machine version dependent differences without
> this awkward use_migration_id flag (after all, if it's not always used
> as the id for migration, 'migration_id' is kind of a bad name...).
> 
>> ---
>>  target-ppc/translate_init.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index efd6b88..9ca2f5e 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -10359,6 +10359,17 @@ static gchar *ppc_gdb_arch_name(CPUState *cs)
>>  #endif
>>  }
>>  
>> +static int ppc_cpu_get_migration_id(CPUState *cs)
>> +{
>> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +
>> +    if (cs->use_migration_id) {
>> +        return (int) cpu->cpu_dt_id;
>> +    } else {
>> +        return cs->cpu_index;
>> +    }
>> +}
>> +
>>  static void ppc_cpu_class_init(ObjectClass *oc, void *data)
>>  {
>>      PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
>> @@ -10412,6 +10423,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
>> *data)
>>  #ifndef CONFIG_USER_ONLY
>>      cc->virtio_is_big_endian = ppc_cpu_is_big_endian;
>>  #endif
>> +    cc->get_migration_id = ppc_cpu_get_migration_id;
>>  
>>      dc->fw_name = "PowerPC,UNKNOWN";
>>  }
> 

Could this be another potential user for the PPCMachineClass solution I
was looking at for timebase migration in
https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg01208.html? It
seems like a similar problem where machine-level state is needed for
migration purposes.


ATB,

Mark.