From nobody Sat May 4 22:04:07 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1507883863327335.70310923170223; Fri, 13 Oct 2017 01:37:43 -0700 (PDT) Received: from localhost ([::1]:48987 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e2vTP-0006Dn-Gb for importer@patchew.org; Fri, 13 Oct 2017 04:37:39 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58362) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e2vRM-00051c-Um for qemu-devel@nongnu.org; Fri, 13 Oct 2017 04:35:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e2vRI-0003PT-E8 for qemu-devel@nongnu.org; Fri, 13 Oct 2017 04:35:32 -0400 Received: from 12.mo5.mail-out.ovh.net ([46.105.39.65]:33834) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1e2vRI-0003Or-5F for qemu-devel@nongnu.org; Fri, 13 Oct 2017 04:35:28 -0400 Received: from player773.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo5.mail-out.ovh.net (Postfix) with ESMTP id 5805313FDA5 for ; Fri, 13 Oct 2017 10:35:26 +0200 (CEST) Received: from [192.168.0.243] (gar31-1-82-66-74-139.fbx.proxad.net [82.66.74.139]) (Authenticated sender: groug@kaod.org) by player773.ha.ovh.net (Postfix) with ESMTPA id C8F6D600074; Fri, 13 Oct 2017 10:35:19 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Fri, 13 Oct 2017 10:35:19 +0200 Message-ID: <150788371922.25736.3447778326403865231.stgit@bahia> In-Reply-To: <150788370618.25736.8030708425923435364.stgit@bahia> References: <150788370618.25736.8030708425923435364.stgit@bahia> User-Agent: StGit/0.17.1-46-g6855-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Ovh-Tracer-Id: 8849010321608645058 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedttddrtdeigddtjecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfqggfjpdevjffgvefmvefgnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 46.105.39.65 Subject: [Qemu-devel] [PATCH 1/2] spapr_cpu_core: instantiate CPUs separately X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Cornelia Huck , "Dr. David Alan Gilbert" , Markus Armbruster , qemu-ppc@nongnu.org, Igor Mammedov , David Gibson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 The current code assumes that only the CPU core object holds a reference on each individual CPU object, and happily frees their allocated memory when the core is unrealized. This is dangerous as some other code can legitimely keep a pointer to a CPU if it calls object_ref(), but it would end up with a dangling pointer. Let's allocate all CPUs with object_new() and let QOM frees them when their reference count reaches zero. Signed-off-by: Greg Kurz --- hw/ppc/spapr.c | 10 +++------- hw/ppc/spapr_cpu_core.c | 29 +++++++++-------------------- include/hw/ppc/spapr_cpu_core.h | 2 +- 3 files changed, 13 insertions(+), 28 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index fd9813bde82f..ba391c6ed5de 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3153,12 +3153,10 @@ void spapr_core_release(DeviceState *dev) =20 if (smc->pre_2_10_has_unused_icps) { sPAPRCPUCore *sc =3D SPAPR_CPU_CORE(OBJECT(dev)); - sPAPRCPUCoreClass *scc =3D SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); - size_t size =3D object_type_get_instance_size(scc->cpu_type); int i; =20 for (i =3D 0; i < cc->nr_threads; i++) { - CPUState *cs =3D CPU(sc->threads + i * size); + CPUState *cs =3D CPU(sc->threads[i]); =20 pre_2_10_vmstate_register_dummy_icp(cs->cpu_index); } @@ -3204,7 +3202,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_d= ev, DeviceState *dev, sPAPRMachineClass *smc =3D SPAPR_MACHINE_CLASS(mc); sPAPRCPUCore *core =3D SPAPR_CPU_CORE(OBJECT(dev)); CPUCore *cc =3D CPU_CORE(dev); - CPUState *cs =3D CPU(core->threads); + CPUState *cs =3D CPU(core->threads[0]); sPAPRDRConnector *drc; Error *local_err =3D NULL; int smt =3D kvmppc_smt_threads(); @@ -3249,13 +3247,11 @@ static void spapr_core_plug(HotplugHandler *hotplug= _dev, DeviceState *dev, core_slot->cpu =3D OBJECT(dev); =20 if (smc->pre_2_10_has_unused_icps) { - sPAPRCPUCoreClass *scc =3D SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); - size_t size =3D object_type_get_instance_size(scc->cpu_type); int i; =20 for (i =3D 0; i < cc->nr_threads; i++) { sPAPRCPUCore *sc =3D SPAPR_CPU_CORE(dev); - void *obj =3D sc->threads + i * size; + void *obj =3D sc->threads[i]; =20 cs =3D CPU(obj); pre_2_10_vmstate_unregister_dummy_icp(cs->cpu_index); diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 3a4c17401226..47c408b52ca1 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -79,21 +79,18 @@ const char *spapr_get_cpu_core_type(const char *cpu_typ= e) static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp) { sPAPRCPUCore *sc =3D SPAPR_CPU_CORE(OBJECT(dev)); - sPAPRCPUCoreClass *scc =3D SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); - size_t size =3D object_type_get_instance_size(scc->cpu_type); CPUCore *cc =3D CPU_CORE(dev); int i; =20 for (i =3D 0; i < cc->nr_threads; i++) { - void *obj =3D sc->threads + i * size; - DeviceState *dev =3D DEVICE(obj); + DeviceState *dev =3D DEVICE(sc->threads[i]); CPUState *cs =3D CPU(dev); PowerPCCPU *cpu =3D POWERPC_CPU(cs); =20 spapr_cpu_destroy(cpu); object_unparent(cpu->intc); cpu_remove_sync(cs); - object_unparent(obj); + object_unparent(sc->threads[i]); } g_free(sc->threads); } @@ -146,9 +143,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Er= ror **errp) sPAPRCPUCore *sc =3D SPAPR_CPU_CORE(OBJECT(dev)); sPAPRCPUCoreClass *scc =3D SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); CPUCore *cc =3D CPU_CORE(OBJECT(dev)); - size_t size; Error *local_err =3D NULL; - void *obj; int i, j; =20 if (!spapr) { @@ -156,17 +151,14 @@ static void spapr_cpu_core_realize(DeviceState *dev, = Error **errp) return; } =20 - size =3D object_type_get_instance_size(scc->cpu_type); - sc->threads =3D g_malloc0(size * cc->nr_threads); + sc->threads =3D g_new(void *, cc->nr_threads); for (i =3D 0; i < cc->nr_threads; i++) { char id[32]; CPUState *cs; PowerPCCPU *cpu; =20 - obj =3D sc->threads + i * size; - - object_initialize(obj, size, scc->cpu_type); - cs =3D CPU(obj); + sc->threads[i] =3D object_new(scc->cpu_type); + cs =3D CPU(sc->threads[i]); cpu =3D POWERPC_CPU(cs); cs->cpu_index =3D cc->core_id + i; cpu->vcpu_id =3D (cc->core_id * spapr->vsmt / smp_threads) + i; @@ -184,17 +176,15 @@ static void spapr_cpu_core_realize(DeviceState *dev, = Error **errp) cpu->node_id =3D sc->node_id; =20 snprintf(id, sizeof(id), "thread[%d]", i); - object_property_add_child(OBJECT(sc), id, obj, &local_err); + object_property_add_child(OBJECT(sc), id, sc->threads[i], &local_e= rr); if (local_err) { goto err; } - object_unref(obj); + object_unref(sc->threads[i]); } =20 for (j =3D 0; j < cc->nr_threads; j++) { - obj =3D sc->threads + j * size; - - spapr_cpu_core_realize_child(obj, spapr, &local_err); + spapr_cpu_core_realize_child(sc->threads[j], spapr, &local_err); if (local_err) { goto err; } @@ -203,8 +193,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Er= ror **errp) =20 err: while (--i >=3D 0) { - obj =3D sc->threads + i * size; - object_unparent(obj); + object_unparent(sc->threads[i]); } g_free(sc->threads); error_propagate(errp, local_err); diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_cor= e.h index f2d48d6a6786..f5b943a6682c 100644 --- a/include/hw/ppc/spapr_cpu_core.h +++ b/include/hw/ppc/spapr_cpu_core.h @@ -28,7 +28,7 @@ typedef struct sPAPRCPUCore { CPUCore parent_obj; =20 /*< public >*/ - void *threads; + void **threads; int node_id; } sPAPRCPUCore; =20 From nobody Sat May 4 22:04:07 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1507883887752187.57143429874668; Fri, 13 Oct 2017 01:38:07 -0700 (PDT) Received: from localhost ([::1]:48989 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e2vTr-0006aR-1B for importer@patchew.org; Fri, 13 Oct 2017 04:38:07 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58457) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e2vRY-000588-Oo for qemu-devel@nongnu.org; Fri, 13 Oct 2017 04:35:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e2vRT-0003Ua-PU for qemu-devel@nongnu.org; Fri, 13 Oct 2017 04:35:44 -0400 Received: from 6.mo5.mail-out.ovh.net ([178.32.119.138]:56336) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1e2vRT-0003Tn-K4 for qemu-devel@nongnu.org; Fri, 13 Oct 2017 04:35:39 -0400 Received: from player773.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo5.mail-out.ovh.net (Postfix) with ESMTP id 26863140016 for ; Fri, 13 Oct 2017 10:35:38 +0200 (CEST) Received: from [192.168.0.243] (gar31-1-82-66-74-139.fbx.proxad.net [82.66.74.139]) (Authenticated sender: groug@kaod.org) by player773.ha.ovh.net (Postfix) with ESMTPA id BA7BC600092; Fri, 13 Oct 2017 10:35:31 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Fri, 13 Oct 2017 10:35:31 +0200 Message-ID: <150788373123.25736.7359515819699182906.stgit@bahia> In-Reply-To: <150788370618.25736.8030708425923435364.stgit@bahia> References: <150788370618.25736.8030708425923435364.stgit@bahia> User-Agent: StGit/0.17.1-46-g6855-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Ovh-Tracer-Id: 8852388017625143746 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedttddrtdeigddtjecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfqggfjpdevjffgvefmvefgnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 178.32.119.138 Subject: [Qemu-devel] [PATCH 2/2] monitor: add proper reference counting of the current CPU X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Cornelia Huck , "Dr. David Alan Gilbert" , Markus Armbruster , qemu-ppc@nongnu.org, Igor Mammedov , David Gibson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 If a CPU selected with the "cpu" command is hot-unplugged then "info cpus" causes QEMU to exit: (qemu) device_del cpu1 (qemu) info cpus qemu:qemu_cpu_kick_thread: No such process This happens because "cpu" stores the pointer to the selected CPU into the monitor structure. When the CPU is hot-unplugged, we end up with a dangling pointer. The "info cpus" command then does: hmp_info_cpus() monitor_get_cpu_index() mon_get_cpu() cpu_synchronize_state() <--- called with dangling pointer This could cause a QEMU crash as well. This patch switches the monitor to use object_ref() to ensure the CPU object doesn't vanish unexpectedly. The reference is dropped either when "cpu" is used to switch to another CPU, or when the selected CPU is unrealized and cpu_list_remove() sets its cpu_index back to UNASSIGNED_CPU_INDEX. Signed-off-by: Greg Kurz --- monitor.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/monitor.c b/monitor.c index fe0d1bdbb461..1c0b9a2c3ad3 100644 --- a/monitor.c +++ b/monitor.c @@ -579,6 +579,9 @@ static void monitor_data_init(Monitor *mon) =20 static void monitor_data_destroy(Monitor *mon) { + if (mon->mon_cpu) { + object_unref((Object *) mon->mon_cpu); + } qemu_chr_fe_deinit(&mon->chr, false); if (monitor_is_qmp(mon)) { json_message_parser_destroy(&mon->qmp.parser); @@ -1047,12 +1050,21 @@ int monitor_set_cpu(int cpu_index) if (cpu =3D=3D NULL) { return -1; } + if (cur_mon->mon_cpu) { + object_unref((Object *) cur_mon->mon_cpu); + } cur_mon->mon_cpu =3D cpu; + object_ref((Object *) cpu); return 0; } =20 CPUState *mon_get_cpu(void) { + if (cur_mon->mon_cpu && + cur_mon->mon_cpu->cpu_index =3D=3D UNASSIGNED_CPU_INDEX) { + object_unref((Object *) cur_mon->mon_cpu); + cur_mon->mon_cpu =3D NULL; + } if (!cur_mon->mon_cpu) { if (!first_cpu) { return NULL;