On Thu, 14 May 2020 17:27:53 +0100 Alex Bennée <alex.ben...@linaro.org> wrote:
> a > Alex Bennée <alex.ben...@linaro.org> writes: > > > Basing the cpu_index on the number of currently allocated vCPUs fails > > when vCPUs aren't removed in a LIFO manner. This is especially true > > when we are allocating a cpu_index for each guest thread in > > linux-user where there is no ordering constraint on their allocation > > and de-allocation. > > > > [I've dropped the assert which is there to guard against out-of-order > > removal as this should probably be caught higher up the stack. Maybe > > we could just ifdef CONFIG_SOFTTMU it?] for machines where we care about cross version migration (arm/virt,s390,x86,spapr), we do manual cpu_index assignment on keep control on its stability So orderining probably shouldn't matter for other softmmu boards, but what I'd watch for is arrays within devices where cpu_index is used as index (ex: would be apic emulation (but its not affected by this patch since x86 control cpu_index assignment)) > > > > Signed-off-by: Alex Bennée <alex.ben...@linaro.org> > > Cc: Nikolay Igotti <igo...@gmail.com> > > Cc: Paolo Bonzini <pbonz...@redhat.com> > > Cc: Igor Mammedov <imamm...@redhat.com> > > Cc: Eduardo Habkost <ehabk...@redhat.com> > > --- > > cpus-common.c | 9 ++++----- > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/cpus-common.c b/cpus-common.c > > index 55d5df89237..5a7d2f6132b 100644 > > --- a/cpus-common.c > > +++ b/cpus-common.c > > @@ -61,13 +61,14 @@ static bool cpu_index_auto_assigned; > > static int cpu_get_free_index(void) > > { > > CPUState *some_cpu; > > - int cpu_index = 0; > > + int max_cpu_index = 0; > > > > cpu_index_auto_assigned = true; > > CPU_FOREACH(some_cpu) { > > - cpu_index++; > > + max_cpu_index = MAX(some_cpu->cpu_index, max_cpu_index); > > } > > - return cpu_index; > > + max_cpu_index++; > > + return max_cpu_index; > > } > > OK some ending up with cpu_index = 1 threw off devices that would do > qemu_get_cpu(0) so I've tweaked the algorithm to: > > static int cpu_get_free_index(void) > { > CPUState *some_cpu; > int max_cpu_index = 0; > > cpu_index_auto_assigned = true; > CPU_FOREACH(some_cpu) { > if (some_cpu->cpu_index >= max_cpu_index) { > max_cpu_index = some_cpu->cpu_index + 1; > } > } > return max_cpu_index; > } > > > > > void cpu_list_add(CPUState *cpu) > > @@ -90,8 +91,6 @@ void cpu_list_remove(CPUState *cpu) > > return; > > } > > > > - assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus))); > > - > > QTAILQ_REMOVE_RCU(&cpus, cpu, node); > > cpu->cpu_index = UNASSIGNED_CPU_INDEX; > > } > >