From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52069) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VRTt0-0002BB-Hr for qemu-devel@nongnu.org; Wed, 02 Oct 2013 17:23:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VRTsr-0005vI-JK for qemu-devel@nongnu.org; Wed, 02 Oct 2013 17:23:10 -0400 Received: from e9.ny.us.ibm.com ([32.97.182.139]:42001) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VRTsr-0005vC-Fa for qemu-devel@nongnu.org; Wed, 02 Oct 2013 17:23:01 -0400 Received: from /spool/local by e9.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 2 Oct 2013 17:23:00 -0400 Received: from b01cxnp23032.gho.pok.ibm.com (b01cxnp23032.gho.pok.ibm.com [9.57.198.27]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 7DD90C90041 for ; Wed, 2 Oct 2013 17:22:57 -0400 (EDT) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by b01cxnp23032.gho.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r92LMvEU2359728 for ; Wed, 2 Oct 2013 21:22:57 GMT Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r92LMvbu004908 for ; Wed, 2 Oct 2013 18:22:57 -0300 Message-ID: <524C8EB0.7080702@linux.vnet.ibm.com> Date: Wed, 02 Oct 2013 17:22:56 -0400 From: "Jason J. Herne" MIME-Version: 1.0 References: <1375366359-11553-1-git-send-email-jjherne@us.ibm.com> <1375366359-11553-7-git-send-email-jjherne@us.ibm.com> <52287903.7040006@suse.de> <52332E3C.3040502@linux.vnet.ibm.com> In-Reply-To: <52332E3C.3040502@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 6/8] [PATCH RFC v3] s390-qemu: cpu hotplug - s390 cpu init improvements for hotplug Reply-To: jjherne@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: jjherne@linux.vnet.ibm.com Cc: ehabkost@redhat.com, qemu-devel@nongnu.org, agraf@suse.de, borntraeger@de.ibm.com, jfrei@linux.vnet.ibm.com, imammedo@redhat.com, "Jason J. Herne" , =?ISO-8859-15?Q?Andreas_F=E4rber?= On 09/13/2013 11:24 AM, Jason J. Herne wrote: > On 09/05/2013 08:28 AM, Andreas Färber wrote: >> Am 01.08.2013 16:12, schrieb Jason J. Herne: >>> From: "Jason J. Herne" >>> >>> s390_new_cpu is created to encapsulate the creation of a new QOM >>> S390CPU >>> object given a cpuid and a model string. >>> >>> All actual cpu initialization code is moved from boot time >>> specific functions to >>> s390_cpu_initfn (qom init routine) or to s390_new_cpu. This is >>> done to allow us >>> to use the same basic code path for a cpu created at boot time >>> and one created >>> during a hotplug operation. >> >> Intentionally indented? >> >>> >>> Signed-off-by: Jason J. Herne >>> --- >>> hw/s390x/s390-virtio.c | 25 ++++++++++++------------- >>> target-s390x/cpu.c | 4 ++-- >>> target-s390x/cpu.h | 1 + >>> target-s390x/helper.c | 12 ++++++++++++ >>> 4 files changed, 27 insertions(+), 15 deletions(-) >>> >>> diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c >>> index 5ad9cf3..103f32e 100644 >>> --- a/hw/s390x/s390-virtio.c >>> +++ b/hw/s390x/s390-virtio.c > ... >>> @@ -197,19 +202,13 @@ void s390_init_cpus(const char *cpu_model) >>> cpu_model = "host"; >>> } >>> >>> - ipi_states = g_malloc(sizeof(S390CPU *) * smp_cpus); >>> - >>> - for (i = 0; i < smp_cpus; i++) { >>> - S390CPU *cpu; >>> - CPUState *cs; >>> + ipi_states = g_malloc(sizeof(S390CPU *) * max_cpus); >>> >>> - cpu = cpu_s390x_init(cpu_model); >>> - cs = CPU(cpu); >>> - >>> - ipi_states[i] = cpu; >>> - cs->halted = 1; >>> - cpu->env.exception_index = EXCP_HLT; >>> - cpu->env.storage_keys = s390_get_storage_keys(); >>> + for (i = 0; i < max_cpus; i++) { >>> + ipi_states[i] = NULL; >> >> Using g_malloc0() above would hopefully be more efficient and would >> allow to leave the loop untouched for easier review. >> > > I don't follow. I'm completely changing this loop. I do not believe we > can obtain the same functionality implemented here while not touching > the loop. > >>> + if (i < smp_cpus) { >>> + s390_new_cpu(cpu_model, i); >>> + } >>> } >>> } >>> >>> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c >>> index 6be6c08..c90a91c 100644 >>> --- a/target-s390x/cpu.c >>> +++ b/target-s390x/cpu.c >>> @@ -116,7 +116,6 @@ static void s390_cpu_initfn(Object *obj) >>> S390CPU *cpu = S390_CPU(obj); >>> CPUS390XState *env = &cpu->env; >>> static bool inited; >>> - static int cpu_num = 0; >>> #if !defined(CONFIG_USER_ONLY) >>> struct tm tm; >>> #endif >>> @@ -135,8 +134,9 @@ static void s390_cpu_initfn(Object *obj) >>> * cpu counter in s390_cpu_reset to a negative number at >>> * initial ipl */ >>> cs->halted = 1; >>> + cpu->env.exception_index = EXCP_HLT; >>> + env->storage_keys = s390_get_storage_keys(); >> >> 4/8? >> > > Are you asking if this belongs in patch #4? if so, I would say no. It > does deal with storage keys, yes. But we're not changing storage key > semantics here (as we are in patch 4), we're just moving where the > storage key ptr gets set. This is in support of re-organizing how cpus > are initialized as per patch title/description. > Ping? :) -- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)