From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41396) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuQR3-0008N0-Uj for qemu-devel@nongnu.org; Mon, 18 May 2015 15:10:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YuQR0-0006iH-Jp for qemu-devel@nongnu.org; Mon, 18 May 2015 15:10:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33393) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuQR0-0006hk-7E for qemu-devel@nongnu.org; Mon, 18 May 2015 15:10:42 -0400 Date: Mon, 18 May 2015 16:09:49 -0300 From: Eduardo Habkost Message-ID: <20150518190949.GE17796@thinpad.lan.raisama.net> References: <1431408986-18323-1-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1431408986-18323-1-git-send-email-bharata@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH v2 0/3] Bitmap based CPU enumeration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: zhugh.fnst@cn.fujitsu.com, agraf@suse.de, qemu-devel@nongnu.org, imammedo@redhat.com, afaerber@suse.de, david@gibson.dropbear.id.au 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