Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH v0 2/5] cpu: Optionally use arch_id instead of cpu_index in cpu vmstate_register()

[Igor Mammedov]

On Tue,  5 Jul 2016 10:12:49 +0530
Bharata B Rao <address@hidden> wrote:

> Introduce CPUState.prefer_arch_id_over_cpu_index and
> MachineClass.prefer_arch_id_over_cpu_index that allow target
> machines to optionally switch to using arch_id instead of cpu_index
> as instance_id in vmstate_register(). This will help allow successful
> migration in cases where holes are introduced in cpu_index range
> after CPU hot removals.
> 
> Whether to use arch_id or cpu_index is based on machine type version
> and hence added MachineClass.prefer_arch_id_over_cpu_index. However
> the enforcement is via and during CPU creation and hence added
> CPUState.prefer_arch_id_over_cpu_index. So it becomes a two step
> process for the target to enable the use of arch_id:
> 
> 1. Set MachineClass.prefer_arch_id_over_cpu_index.
you could reuse compat mechanism like x86 instead of adding doing #1
see PC_COMPAT_2_6 and how it's used

> 2. Ensure CPUState.prefer_arch_id_over_cpu_index is set for all CPUs
>    based on 1. above.
> 
> Suggested-by: Igor Mammedov <address@hidden>
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  exec.c              | 10 ++++++++--
>  include/hw/boards.h |  1 +
>  include/qom/cpu.h   |  4 ++++
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 8ce8e90..7cc1d06 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -616,15 +616,21 @@ static void cpu_release_index(CPUState *cpu)
>      bitmap_clear(cpu_index_map, cpu->cpu_index, 1);
>  }
>  
> +/*
> + * TODO: cpu_index and instance_id are of type int
> while .get_arch_id()is
> + * of type int64_t. What is the consequence of changing instance_id
> to int64_t ?
> + */
>  static void cpu_vmstate_register(CPUState *cpu)
>  {
>      CPUClass *cc = CPU_GET_CLASS(cpu);
> +    int instance_id = cpu->prefer_arch_id_over_cpu_index ?
> +                      cc->get_arch_id(cpu) : cpu->cpu_index;
>  
>      if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> -        vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common,
> cpu);
> +        vmstate_register(NULL, instance_id, &vmstate_cpu_common,
> cpu); }
>      if (cc->vmsd != NULL) {
> -        vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
> +        vmstate_register(NULL, instance_id, cc->vmsd, cpu);
>      }
>  }
>  
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3ed6155..decabba 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -123,6 +123,7 @@ struct MachineClass {
>      ram_addr_t default_ram_size;
>      bool option_rom_has_mr;
>      bool rom_file_has_mr;
> +    bool prefer_arch_id_over_cpu_index;
>  
>      HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>                                             DeviceState *dev);
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 32f3af3..1f1706e 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -273,6 +273,9 @@ struct qemu_work_item {
>   * @kvm_fd: vCPU file descriptor for KVM.
>   * @work_mutex: Lock to prevent multiple access to queued_work_*.
>   * @queued_work_first: First asynchronous work pending.
> + * @prefer_arch_id_over_cpu_index: Set to enforce the use of
> + *         CPUClass.get_arch_id() over cpu_index during vmstate
> registration
> + *         and any other uses by target machine where arch_id is
> preferred. *
>   * State of one CPU core or thread.
>   */
> @@ -360,6 +363,7 @@ struct CPUState {
>         (absolute value) offset as small as possible.  This reduces
> code size, especially for hosts without large memory offsets.  */
>      uint32_t tcg_exit_req;
> +    bool prefer_arch_id_over_cpu_index;
this property applies to all cpu, so shouldn't it be class property?

>  };
>  
>  QTAILQ_HEAD(CPUTailQ, CPUState);