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

[Bharata B Rao]

On Tue, Jul 05, 2016 at 09:15:51AM +0200, Igor Mammedov wrote:
> 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.
> > 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 ?
> > + */
> ARM potentially could have get_arch_id() == MPIDR (return 64bit int)
> that would get truncated in this case.
> 
> I wonder if we could add "int CPUState::migration_id" and let machine
> code set it to what it uses for stable cpu id and then cpu_realize
> could call vmstate_register with it.

Hmm how would that help ? instance_id argument to vmstate_register() is
being treated as 32 bit integer as pointed by Greg earlier.

If we want to use 64 bit arch_id/migration_id as instance_id arg in
vmstate_register, shouldn't we be changing instance_id to 64 bit ?

Regards,
Bharata.