From nobody Wed Nov 5 02:12:20 2025 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; dkim=fail; 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 1500961754541292.33020059841; Mon, 24 Jul 2017 22:49:14 -0700 (PDT) Received: from localhost ([::1]:58575 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dZsiX-0003Ct-0V for importer@patchew.org; Tue, 25 Jul 2017 01:49:13 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51557) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dZser-0000Yt-2z for qemu-devel@nongnu.org; Tue, 25 Jul 2017 01:45:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dZseo-0006Fe-7u for qemu-devel@nongnu.org; Tue, 25 Jul 2017 01:45:25 -0400 Received: from ozlabs.org ([103.22.144.67]:59081) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dZsen-0006Em-SF; Tue, 25 Jul 2017 01:45:22 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3xGnJb3NMNz9s3w; Tue, 25 Jul 2017 15:45:19 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1500961519; bh=aZwYGWHMugbyyb/PLrF+3HZr+EF3vThhYFf51scDKqs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Txh0QSUH6d1A7VIaD/YtUf2pPF4+th250Pl/6jKwiU4ofUeWyaz76WvHyJeQiJLE6 +X0b+duZiEmepC3B2BPLWT8jVD7H20NhqcyUQ1QA3DKF4CT48nEiL//OLMVjIKyMLR SKK8/LHXav25eju2aPsXuCa5nVYzsN2yI6qvu34g= From: David Gibson To: peter.maydell@linaro.org Date: Tue, 25 Jul 2017 15:45:09 +1000 Message-Id: <20170725054510.15177-4-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.13.3 In-Reply-To: <20170725054510.15177-1-david@gibson.dropbear.id.au> References: <20170725054510.15177-1-david@gibson.dropbear.id.au> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 103.22.144.67 Subject: [Qemu-devel] [PULL 3/4] spapr: Fix QEMU abort during memory unplug 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: lvivier@redhat.com, thuth@redhat.com, qemu-devel@nongnu.org, aik@ozlabs.ru, agraf@suse.de, groug@kaod.org, qemu-ppc@nongnu.org, bharata@linux.vnet.ibm.com, David Gibson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZohoMail: RDKM_2 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Bharata B Rao Commit 0cffce56 (hw/ppc/spapr.c: adding pending_dimm_unplugs to sPAPRMachineState) introduced a new way to track pending LMBs of DIMM device that is marked for removal. Since this commit we can hit the assert in spapr_pending_dimm_unplugs_add() in the following situation: - DIMM device removal fails as the guest doesn't allow the removal. - Subsequent attempt to remove the same DIMM would hit the assert as the corresponding sPAPRDIMMState is still part of the pending_dimm_unplugs list. Fix this by removing the assert and conditionally adding the sPAPRDIMMState to pending_dimm_unplugs list only when it is not already present. Fixes: 0cffce56ae3501c5783d779f97993ce478acf856 Signed-off-by: Bharata B Rao [dwg: Tweaked to avoid returning NULL when spapr_pending_dimm_unplugs_add() does find an existing entry] Reviewed-by: Daniel Barboza Signed-off-by: David Gibson --- hw/ppc/spapr.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 1cb09e7c64..2a3e53d5d5 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2850,11 +2850,26 @@ static sPAPRDIMMState *spapr_pending_dimm_unplugs_f= ind(sPAPRMachineState *s, return dimm_state; } =20 -static void spapr_pending_dimm_unplugs_add(sPAPRMachineState *spapr, - sPAPRDIMMState *dimm_state) +static sPAPRDIMMState *spapr_pending_dimm_unplugs_add(sPAPRMachineState *s= papr, + uint32_t nr_lmbs, + PCDIMMDevice *dimm) { - g_assert(!spapr_pending_dimm_unplugs_find(spapr, dimm_state->dimm)); - QTAILQ_INSERT_HEAD(&spapr->pending_dimm_unplugs, dimm_state, next); + sPAPRDIMMState *ds =3D NULL; + + /* + * If this request is for a DIMM whose removal had failed earlier + * (due to guest's refusal to remove the LMBs), we would have this + * dimm already in the pending_dimm_unplugs list. In that + * case don't add again. + */ + ds =3D spapr_pending_dimm_unplugs_find(spapr, dimm); + if (!ds) { + ds =3D g_malloc0(sizeof(sPAPRDIMMState)); + ds->nr_lmbs =3D nr_lmbs; + ds->dimm =3D dimm; + QTAILQ_INSERT_HEAD(&spapr->pending_dimm_unplugs, ds, next); + } + return ds; } =20 static void spapr_pending_dimm_unplugs_remove(sPAPRMachineState *spapr, @@ -2875,7 +2890,6 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_sta= te(sPAPRMachineState *ms, uint32_t avail_lmbs =3D 0; uint64_t addr_start, addr; int i; - sPAPRDIMMState *ds; =20 addr_start =3D object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, &error_abort); @@ -2891,11 +2905,7 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_st= ate(sPAPRMachineState *ms, addr +=3D SPAPR_MEMORY_BLOCK_SIZE; } =20 - ds =3D g_malloc0(sizeof(sPAPRDIMMState)); - ds->nr_lmbs =3D avail_lmbs; - ds->dimm =3D dimm; - spapr_pending_dimm_unplugs_add(ms, ds); - return ds; + return spapr_pending_dimm_unplugs_add(ms, avail_lmbs, dimm); } =20 /* Callback to be called during DRC release. */ @@ -2911,6 +2921,7 @@ void spapr_lmb_release(DeviceState *dev) * during the unplug process. In this case recover it. */ if (ds =3D=3D NULL) { ds =3D spapr_recover_pending_dimm_state(spapr, PC_DIMM(dev)); + g_assert(ds); /* The DRC being examined by the caller at least must be counted */ g_assert(ds->nr_lmbs); } @@ -2942,18 +2953,13 @@ static void spapr_memory_unplug_request(HotplugHand= ler *hotplug_dev, uint64_t addr_start, addr; int i; sPAPRDRConnector *drc; - sPAPRDIMMState *ds; - addr_start =3D object_property_get_uint(OBJECT(dimm), PC_DIMM_ADDR_PRO= P, &local_err); if (local_err) { goto out; } =20 - ds =3D g_malloc0(sizeof(sPAPRDIMMState)); - ds->nr_lmbs =3D nr_lmbs; - ds->dimm =3D dimm; - spapr_pending_dimm_unplugs_add(spapr, ds); + spapr_pending_dimm_unplugs_add(spapr, nr_lmbs, dimm); =20 addr =3D addr_start; for (i =3D 0; i < nr_lmbs; i++) { --=20 2.13.3