Re: [Qemu-devel] [PATCH v2 0/3] Bitmap based CPU enumeration

[Eduardo Habkost]

On Tue, May 12, 2015 at 11:06:23AM +0530, Bharata B Rao wrote:
> This patch changes the way cpu_index is handed out to newly created
> CPUs by tracking the allocted CPUs in a bitmap. More information and
> the need for this patch is described in patch 2/3 of this series. These
> generic changes are needed to support CPU hot plug/unplug on PowerPC.

What about the existing vmstate and savevm calls on cpu_exec_init()?
Won't QEMU crash if you destroy the CPU without unregistering the
vmstate and savevm handlers?

> 
> An open question is about handling of holes correctly in the allocated
> bitmap to support VM migration after CPU unplug. This was briefly discussed
> here:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg00560.html
> 
> Should cpu_exec_init() API support specifying of a particular cpu_index
> in addition to returning the next available cpu_index by default ? I know
> that QEMU cmdline semantics for CPU device add/delele haven't been defined
> yet, but should we now make provision in cpu_exec_init() to allocate the
> required cpu_index ?

I don't believe we need it, and instead we should make cpu_index
irrelevant. cpu_index is just an arbitrary ID assigned to the CPU, and
any interface that depends on it for something needs to use clearer and
more meaningful parameters as input (such as socket/core/thread
information), instead of cpu_index.

For example, on X86 the APIC ID depends on the CPU socket/core/thread
IDs, and today we just use the cpu_index to calculate it. In the future,
we need to let users choose (directly or indirectly) the specific
socket/core/thread IDs for the CPU, so the APIC ID can be calculated
without requiring cpu_index.

Probably the same applies to cpu_dt_id on PPC: you need to be able to
calculate cpu_dt_id without cpu_index (using core and thread IDs as
input, I guess?).


> 
> Changes in v2
> -------------
> Following changes to address Eduardo's comments:
> 
> - Call cpu_exec_init() from generic CPU::instance_finalize() instead
>   of touching all archs.
> - Initialize cpu_index to -1 from generic CPU::instance_init().
> - Callers of cpu_exec_init() now pass error_abort argument instead of NULL.
> - Consolidate the older CPU enumeration code for CONFIG_USER_ONLY under
>   cpu_get_free_index().
> 
> v1: https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg01385.html
> v0: https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg02950.html
> 
> Bharata B Rao (3):
>   cpus: Add Error argument to cpu_exec_init()
>   cpus: Convert cpu_index into a bitmap
>   ppc: Move cpu_exec_init() call to realize function
> 
>  exec.c                      | 57 
> ++++++++++++++++++++++++++++++++++++++++-----
>  include/exec/exec-all.h     |  2 +-
>  include/qom/cpu.h           |  1 +
>  qom/cpu.c                   |  7 ++++++
>  target-alpha/cpu.c          |  2 +-
>  target-arm/cpu.c            |  2 +-
>  target-cris/cpu.c           |  2 +-
>  target-i386/cpu.c           |  2 +-
>  target-lm32/cpu.c           |  2 +-
>  target-m68k/cpu.c           |  2 +-
>  target-microblaze/cpu.c     |  2 +-
>  target-mips/cpu.c           |  2 +-
>  target-moxie/cpu.c          |  2 +-
>  target-openrisc/cpu.c       |  2 +-
>  target-ppc/translate_init.c |  9 +++++--
>  target-s390x/cpu.c          |  2 +-
>  target-sh4/cpu.c            |  2 +-
>  target-sparc/cpu.c          |  2 +-
>  target-tricore/cpu.c        |  2 +-
>  target-unicore32/cpu.c      |  2 +-
>  target-xtensa/cpu.c         |  2 +-
>  21 files changed, 83 insertions(+), 25 deletions(-)
> 
> -- 
> 2.1.0
> 

-- 
Eduardo