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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2129 bytes --]

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

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";
>  }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]