From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41461) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKVF0-0005fk-8h for qemu-devel@nongnu.org; Fri, 13 Sep 2013 11:25:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VKVEr-0001HC-6r for qemu-devel@nongnu.org; Fri, 13 Sep 2013 11:25:02 -0400 Received: from e39.co.us.ibm.com ([32.97.110.160]:52647) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKVEr-0001H2-0S for qemu-devel@nongnu.org; Fri, 13 Sep 2013 11:24:53 -0400 Received: from /spool/local by e39.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 13 Sep 2013 09:24:52 -0600 Received: from d03relay04.boulder.ibm.com (d03relay04.boulder.ibm.com [9.17.195.106]) by d03dlp01.boulder.ibm.com (Postfix) with ESMTP id 9736A1FF001E for ; Fri, 13 Sep 2013 09:24:46 -0600 (MDT) Received: from d03av01.boulder.ibm.com (d03av01.boulder.ibm.com [9.17.195.167]) by d03relay04.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r8DFOnjv308932 for ; Fri, 13 Sep 2013 09:24:49 -0600 Received: from d03av01.boulder.ibm.com (loopback [127.0.0.1]) by d03av01.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r8DFOmaI008490 for ; Fri, 13 Sep 2013 09:24:49 -0600 Message-ID: <52332E3C.3040502@linux.vnet.ibm.com> Date: Fri, 13 Sep 2013 11:24:44 -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> In-Reply-To: <52287903.7040006@suse.de> 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: =?ISO-8859-15?Q?Andreas_F=E4rber?= 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" 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. -- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)