From nobody Sun Apr 28 03:06:58 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.zoho.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 1498036900804718.3156628175349; Wed, 21 Jun 2017 02:21:40 -0700 (PDT) Received: from localhost ([::1]:52687 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbpT-00053g-Ks for importer@patchew.org; Wed, 21 Jun 2017 05:21:39 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40374) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbmw-0003DC-IT for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dNbmt-0001y7-Bj for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:02 -0400 Received: from ozlabs.org ([103.22.144.67]:50433) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dNbms-0001xY-VC; Wed, 21 Jun 2017 05:18:59 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3wszfm1gYQz9s78; Wed, 21 Jun 2017 19:18:55 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1498036736; bh=bq7NI//KfW4rSOY1IaRL+Jjk0H8oLn9UoRTgTiDkd/c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KbgwLHDpvp8UtUEIfUbsoMd+pILld1WInhoFpbtyd996RyqeaxOaGh7mPsyV7VHWA ekqAr6CjG/zYaIP5gx/YRlBf6Bl+XGQITWqh21FltSwCSTSTMNrcK/BXFjy+HIGVXA So7Kw0TZb6+ZrHebbLM4uH/f1H+z7jyFbq7c6SmY= From: David Gibson To: mdroth@linux.vnet.ibm.com Date: Wed, 21 Jun 2017 17:18:44 +0800 Message-Id: <20170621091848.28256-2-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.9.4 In-Reply-To: <20170621091848.28256-1-david@gibson.dropbear.id.au> References: <20170621091848.28256-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] [PATCH 1/5] spapr: Remove 'awaiting_allocation' DRC flag 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, sursingh@redhat.com, qemu-devel@nongnu.org, 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" The awaiting_allocation flag in the DRC was introduced by aab9913 "spapr_drc: Prevent detach racing against attach for CPU DR", allegedly to prevent a guest crash on racing attach and detach. Except.. information from the BZ actually suggests a qemu crash, not a guest crash. And there shouldn't be a problem here anyway: if the guest has already moved the DRC away from UNUSABLE state, the detach would already be deferred, and if it hadn't it should be safe to detach it (the guest should fail gracefully when it attempts to change the allocation state). I think this was probably just a bandaid for some other problem in the state management. So, remove awaiting_allocation and associated code. Signed-off-by: David Gibson Tested-by: Daniel Barboza --- hw/ppc/spapr_drc.c | 23 +++-------------------- include/hw/ppc/spapr_drc.h | 1 - 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index f34355d..59e19f8 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -170,19 +170,13 @@ static uint32_t drc_set_usable(sPAPRDRConnector *drc) if (!drc->dev) { return RTAS_OUT_NO_SUCH_INDICATOR; } - if (drc->awaiting_release && drc->awaiting_allocation) { - /* kernel is acknowledging a previous hotplug event - * while we are already removing it. - * it's safe to ignore awaiting_allocation here since we know the - * situation is predicated on the guest either already having done - * so (boot-time hotplug), or never being able to acquire in the - * first place (hotplug followed by immediate unplug). - */ + if (drc->awaiting_release) { + /* Don't allow the guest to move a device away from UNUSABLE + * state when we want to unplug it */ return RTAS_OUT_NO_SUCH_INDICATOR; } =20 drc->allocation_state =3D SPAPR_DR_ALLOCATION_STATE_USABLE; - drc->awaiting_allocation =3D false; =20 return RTAS_OUT_SUCCESS; } @@ -357,10 +351,6 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceSta= te *d, void *fdt, drc->fdt =3D fdt; drc->fdt_start_offset =3D fdt_start_offset; =20 - if (spapr_drc_type(drc) !=3D SPAPR_DR_CONNECTOR_TYPE_PCI) { - drc->awaiting_allocation =3D true; - } - object_property_add_link(OBJECT(drc), "device", object_get_typename(OBJECT(drc->dev)), (Object **)(&drc->dev), @@ -398,12 +388,6 @@ void spapr_drc_detach(sPAPRDRConnector *drc, DeviceSta= te *d, Error **errp) return; } =20 - if (drc->awaiting_allocation) { - drc->awaiting_release =3D true; - trace_spapr_drc_awaiting_allocation(spapr_drc_index(drc)); - return; - } - spapr_drc_release(drc); } =20 @@ -490,7 +474,6 @@ static const VMStateDescription vmstate_spapr_drc =3D { VMSTATE_UINT32(dr_indicator, sPAPRDRConnector), VMSTATE_BOOL(configured, sPAPRDRConnector), VMSTATE_BOOL(awaiting_release, sPAPRDRConnector), - VMSTATE_BOOL(awaiting_allocation, sPAPRDRConnector), VMSTATE_END_OF_LIST() } }; diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index d15e9eb..42c3722 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -199,7 +199,6 @@ typedef struct sPAPRDRConnector { sPAPRConfigureConnectorState *ccs; =20 bool awaiting_release; - bool awaiting_allocation; =20 /* device pointer, via link property */ DeviceState *dev; --=20 2.9.4 From nobody Sun Apr 28 03:06:58 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.zoho.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 1498036875258585.8501399045415; Wed, 21 Jun 2017 02:21:15 -0700 (PDT) Received: from localhost ([::1]:52684 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbp3-0004iC-SQ for importer@patchew.org; Wed, 21 Jun 2017 05:21:13 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40373) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbmw-0003DB-IT for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dNbmt-0001yC-Bp for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:02 -0400 Received: from ozlabs.org ([103.22.144.67]:47677) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dNbms-0001xX-V1; Wed, 21 Jun 2017 05:18:59 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3wszfm0MFTz9s2s; Wed, 21 Jun 2017 19:18:56 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1498036736; bh=KdxH+bdjuv3/GQWhQjVaLjcfP6ISn8sF/QKpfa+Szzc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Z8rzmONdsRWUX7IitRq11/g29MwfT36A6KWP2MpvXJVCMcVIM5b2HWfSB8QraUuU5 jC+F3Y5OpTxQugAz0eCMW7APmOs2WIrNVZ7gZeP4++arh4nTFT02ppGFjU4y1UMK29 JNTG/1Aj+6Oxl9XiouL6Mhu1P1vXav5tLKL+ptUw= From: David Gibson To: mdroth@linux.vnet.ibm.com Date: Wed, 21 Jun 2017 17:18:45 +0800 Message-Id: <20170621091848.28256-3-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.9.4 In-Reply-To: <20170621091848.28256-1-david@gibson.dropbear.id.au> References: <20170621091848.28256-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] [PATCH 2/5] spapr: Refactor spapr_drc_detach() 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, sursingh@redhat.com, qemu-devel@nongnu.org, 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" This function has two unused parameters - remove them. It also sets awaiting_release on all paths, except one. On that path setting it is harmless, since it will be immediately cleared by spapr_drc_release(). So factor it out of the if statements. Signed-off-by: David Gibson Tested-by: Daniel Barboza --- hw/ppc/spapr.c | 9 ++------- hw/ppc/spapr_drc.c | 12 ++++++------ hw/ppc/spapr_pci.c | 7 +------ include/hw/ppc/spapr_drc.h | 2 +- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 5aa3760..8b5ab3a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2827,7 +2827,7 @@ static void spapr_memory_unplug_request(HotplugHandle= r *hotplug_dev, addr / SPAPR_MEMORY_BLOCK_SIZE); g_assert(drc); =20 - spapr_drc_detach(drc, dev, errp); + spapr_drc_detach(drc); addr +=3D SPAPR_MEMORY_BLOCK_SIZE; } =20 @@ -2902,7 +2902,6 @@ void spapr_core_unplug_request(HotplugHandler *hotplu= g_dev, DeviceState *dev, { int index; sPAPRDRConnector *drc; - Error *local_err =3D NULL; CPUCore *cc =3D CPU_CORE(dev); int smt =3D kvmppc_smt_threads(); =20 @@ -2919,11 +2918,7 @@ void spapr_core_unplug_request(HotplugHandler *hotpl= ug_dev, DeviceState *dev, drc =3D spapr_drc_by_id(TYPE_SPAPR_DRC_CPU, index * smt); g_assert(drc); =20 - spapr_drc_detach(drc, dev, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } + spapr_drc_detach(drc); =20 spapr_hotplug_req_remove_by_index(drc); } diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 59e19f8..dae1f79 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -70,7 +70,7 @@ static uint32_t drc_isolate_physical(sPAPRDRConnector *dr= c) uint32_t drc_index =3D spapr_drc_index(drc); if (drc->configured) { trace_spapr_drc_set_isolation_state_finalizing(drc_index); - spapr_drc_detach(drc, DEVICE(drc->dev), NULL); + spapr_drc_detach(drc); } else { trace_spapr_drc_set_isolation_state_deferring(drc_index); } @@ -134,7 +134,7 @@ static uint32_t drc_isolate_logical(sPAPRDRConnector *d= rc) uint32_t drc_index =3D spapr_drc_index(drc); if (drc->configured) { trace_spapr_drc_set_isolation_state_finalizing(drc_index); - spapr_drc_detach(drc, DEVICE(drc->dev), NULL); + spapr_drc_detach(drc); } else { trace_spapr_drc_set_isolation_state_deferring(drc_index); } @@ -187,7 +187,7 @@ static uint32_t drc_set_unusable(sPAPRDRConnector *drc) if (drc->awaiting_release) { uint32_t drc_index =3D spapr_drc_index(drc); trace_spapr_drc_set_allocation_state_finalizing(drc_index); - spapr_drc_detach(drc, DEVICE(drc->dev), NULL); + spapr_drc_detach(drc); } =20 return RTAS_OUT_SUCCESS; @@ -371,20 +371,20 @@ static void spapr_drc_release(sPAPRDRConnector *drc) drc->dev =3D NULL; } =20 -void spapr_drc_detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp) +void spapr_drc_detach(sPAPRDRConnector *drc) { trace_spapr_drc_detach(spapr_drc_index(drc)); =20 + drc->awaiting_release =3D true; + if (drc->isolation_state !=3D SPAPR_DR_ISOLATION_STATE_ISOLATED) { trace_spapr_drc_awaiting_isolated(spapr_drc_index(drc)); - drc->awaiting_release =3D true; return; } =20 if (spapr_drc_type(drc) !=3D SPAPR_DR_CONNECTOR_TYPE_PCI && drc->allocation_state !=3D SPAPR_DR_ALLOCATION_STATE_UNUSABLE) { trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc)); - drc->awaiting_release =3D true; return; } =20 diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index bda8938..af925c0 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1476,7 +1476,6 @@ static void spapr_pci_unplug_request(HotplugHandler *= plug_handler, PCIDevice *pdev =3D PCI_DEVICE(plugged_dev); sPAPRDRConnectorClass *drck; sPAPRDRConnector *drc =3D spapr_phb_get_pci_drc(phb, pdev); - Error *local_err =3D NULL; =20 if (!phb->dr_enabled) { error_setg(errp, QERR_BUS_NO_HOTPLUG, @@ -1514,11 +1513,7 @@ static void spapr_pci_unplug_request(HotplugHandler = *plug_handler, } } =20 - spapr_drc_detach(drc, DEVICE(pdev), &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } + spapr_drc_detach(drc); =20 /* if this isn't func 0, defer unplug event. otherwise signal remo= val * for all present functions diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 42c3722..ab64235 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -234,6 +234,6 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Ob= ject *owner, =20 void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState *d, void *fdt, int fdt_start_offset, Error **errp); -void spapr_drc_detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp); +void spapr_drc_detach(sPAPRDRConnector *drc); =20 #endif /* HW_SPAPR_DRC_H */ --=20 2.9.4 From nobody Sun Apr 28 03:06:58 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.zoho.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 1498037005793652.8293600173671; Wed, 21 Jun 2017 02:23:25 -0700 (PDT) Received: from localhost ([::1]:52695 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbrA-0006XT-Gl for importer@patchew.org; Wed, 21 Jun 2017 05:23:24 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40376) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbmw-0003DF-JW for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dNbmt-0001yQ-Im for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:02 -0400 Received: from ozlabs.org ([103.22.144.67]:47723) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dNbms-0001xb-V6; Wed, 21 Jun 2017 05:18:59 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3wszfm2fKwz9s74; Wed, 21 Jun 2017 19:18:56 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1498036736; bh=E04Kk4Q5Pxj2+CPpR+4pvPrLao0zaHk5KCrYTetuc4I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LD4lMMrr+4OrumAdcq4Jl+9VdPtv1zKf7s/KoR0zkXs5O4vxM0kGhh7emv9xDKM3i hVtJ3KWcA9rPnKPKPlwK+aMd/SUJGDQFeCf6RVjj8bxYempOfG5ieQIjqzjGJA5eiB bIGEyBzxuYgf9/+t4Ez3j0PfJN50wfXVPcI48cPM= From: David Gibson To: mdroth@linux.vnet.ibm.com Date: Wed, 21 Jun 2017 17:18:46 +0800 Message-Id: <20170621091848.28256-4-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.9.4 In-Reply-To: <20170621091848.28256-1-david@gibson.dropbear.id.au> References: <20170621091848.28256-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] [PATCH 3/5] spapr: Cleanups relating to DRC awaiting_release field 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, sursingh@redhat.com, qemu-devel@nongnu.org, 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" 'awaiting_release' indicates that the host has requested an unplug of the device attached to the DRC, but the guest has not (yet) put the device into a state where it is safe to complete removal. 1. Rename it to 'unplug_requested' which to me at least is clearer 2. Remove the ->release_pending() method used to check this from outside spapr_drc.c. The method only plausibly has one implementation, so use a plain function (spapr_drc_unplug_requested()) instead. 3. Remove it from the migration stream. Attempting to migrate mid-unplug is broken not just for spapr - in general management has no good way to determine if the device should be present on the destination or not. So, until that's fixed, there's no point adding extra things to the stream. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Tested-by: Daniel Barboza --- hw/ppc/spapr_drc.c | 26 +++++++++----------------- hw/ppc/spapr_pci.c | 6 ++---- include/hw/ppc/spapr_drc.h | 11 ++++++----- 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index dae1f79..7fa9595 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -66,7 +66,7 @@ static uint32_t drc_isolate_physical(sPAPRDRConnector *dr= c) * configured state, as suggested by the state diagram from PAPR+ * 2.7, 13.4 */ - if (drc->awaiting_release) { + if (drc->unplug_requested) { uint32_t drc_index =3D spapr_drc_index(drc); if (drc->configured) { trace_spapr_drc_set_isolation_state_finalizing(drc_index); @@ -116,7 +116,7 @@ static uint32_t drc_isolate_logical(sPAPRDRConnector *d= rc) * actually being unplugged, fail the isolation request here. */ if (spapr_drc_type(drc) =3D=3D SPAPR_DR_CONNECTOR_TYPE_LMB - && !drc->awaiting_release) { + && !drc->unplug_requested) { return RTAS_OUT_HW_ERROR; } =20 @@ -130,7 +130,7 @@ static uint32_t drc_isolate_logical(sPAPRDRConnector *d= rc) * configured state, as suggested by the state diagram from PAPR+ * 2.7, 13.4 */ - if (drc->awaiting_release) { + if (drc->unplug_requested) { uint32_t drc_index =3D spapr_drc_index(drc); if (drc->configured) { trace_spapr_drc_set_isolation_state_finalizing(drc_index); @@ -170,7 +170,7 @@ static uint32_t drc_set_usable(sPAPRDRConnector *drc) if (!drc->dev) { return RTAS_OUT_NO_SUCH_INDICATOR; } - if (drc->awaiting_release) { + if (drc->unplug_requested) { /* Don't allow the guest to move a device away from UNUSABLE * state when we want to unplug it */ return RTAS_OUT_NO_SUCH_INDICATOR; @@ -184,7 +184,7 @@ static uint32_t drc_set_usable(sPAPRDRConnector *drc) static uint32_t drc_set_unusable(sPAPRDRConnector *drc) { drc->allocation_state =3D SPAPR_DR_ALLOCATION_STATE_UNUSABLE; - if (drc->awaiting_release) { + if (drc->unplug_requested) { uint32_t drc_index =3D spapr_drc_index(drc); trace_spapr_drc_set_allocation_state_finalizing(drc_index); spapr_drc_detach(drc); @@ -363,7 +363,7 @@ static void spapr_drc_release(sPAPRDRConnector *drc) =20 drck->release(drc->dev); =20 - drc->awaiting_release =3D false; + drc->unplug_requested =3D false; g_free(drc->fdt); drc->fdt =3D NULL; drc->fdt_start_offset =3D 0; @@ -375,7 +375,7 @@ void spapr_drc_detach(sPAPRDRConnector *drc) { trace_spapr_drc_detach(spapr_drc_index(drc)); =20 - drc->awaiting_release =3D true; + drc->unplug_requested =3D true; =20 if (drc->isolation_state !=3D SPAPR_DR_ISOLATION_STATE_ISOLATED) { trace_spapr_drc_awaiting_isolated(spapr_drc_index(drc)); @@ -391,11 +391,6 @@ void spapr_drc_detach(sPAPRDRConnector *drc) spapr_drc_release(drc); } =20 -static bool release_pending(sPAPRDRConnector *drc) -{ - return drc->awaiting_release; -} - static void drc_reset(void *opaque) { sPAPRDRConnector *drc =3D SPAPR_DR_CONNECTOR(opaque); @@ -408,7 +403,7 @@ static void drc_reset(void *opaque) /* immediately upon reset we can safely assume DRCs whose devices * are pending removal can be safely removed. */ - if (drc->awaiting_release) { + if (drc->unplug_requested) { spapr_drc_release(drc); } =20 @@ -453,7 +448,7 @@ static bool spapr_drc_needed(void *opaque) case SPAPR_DR_CONNECTOR_TYPE_LMB: rc =3D !((drc->isolation_state =3D=3D SPAPR_DR_ISOLATION_STATE_UNI= SOLATED) && (drc->allocation_state =3D=3D SPAPR_DR_ALLOCATION_STATE_USA= BLE) && - drc->configured && !drc->awaiting_release); + drc->configured); break; case SPAPR_DR_CONNECTOR_TYPE_PHB: case SPAPR_DR_CONNECTOR_TYPE_VIO: @@ -473,7 +468,6 @@ static const VMStateDescription vmstate_spapr_drc =3D { VMSTATE_UINT32(allocation_state, sPAPRDRConnector), VMSTATE_UINT32(dr_indicator, sPAPRDRConnector), VMSTATE_BOOL(configured, sPAPRDRConnector), - VMSTATE_BOOL(awaiting_release, sPAPRDRConnector), VMSTATE_END_OF_LIST() } }; @@ -564,11 +558,9 @@ static void spapr_dr_connector_instance_init(Object *o= bj) static void spapr_dr_connector_class_init(ObjectClass *k, void *data) { DeviceClass *dk =3D DEVICE_CLASS(k); - sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_CLASS(k); =20 dk->realize =3D realize; dk->unrealize =3D unrealize; - drck->release_pending =3D release_pending; /* * Reason: it crashes FIXME find and document the real reason */ diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index af925c0..3dfb77d 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1474,7 +1474,6 @@ static void spapr_pci_unplug_request(HotplugHandler *= plug_handler, { sPAPRPHBState *phb =3D SPAPR_PCI_HOST_BRIDGE(DEVICE(plug_handler)); PCIDevice *pdev =3D PCI_DEVICE(plugged_dev); - sPAPRDRConnectorClass *drck; sPAPRDRConnector *drc =3D spapr_phb_get_pci_drc(phb, pdev); =20 if (!phb->dr_enabled) { @@ -1486,8 +1485,7 @@ static void spapr_pci_unplug_request(HotplugHandler *= plug_handler, g_assert(drc); g_assert(drc->dev =3D=3D plugged_dev); =20 - drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); - if (!drck->release_pending(drc)) { + if (!spapr_drc_unplug_requested(drc)) { PCIBus *bus =3D PCI_BUS(qdev_get_parent_bus(DEVICE(pdev))); uint32_t slotnr =3D PCI_SLOT(pdev->devfn); sPAPRDRConnector *func_drc; @@ -1503,7 +1501,7 @@ static void spapr_pci_unplug_request(HotplugHandler *= plug_handler, func_drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(func_drc); state =3D func_drck->dr_entity_sense(func_drc); if (state =3D=3D SPAPR_DR_ENTITY_SENSE_PRESENT - && !func_drck->release_pending(func_drc)) { + && !spapr_drc_unplug_requested(func_drc)) { error_setg(errp, "PCI: slot %d, function %d still present. " "Must unplug all non-0 functions first.", diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index ab64235..7846cca 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -198,10 +198,9 @@ typedef struct sPAPRDRConnector { bool configured; sPAPRConfigureConnectorState *ccs; =20 - bool awaiting_release; - /* device pointer, via link property */ DeviceState *dev; + bool unplug_requested; } sPAPRDRConnector; =20 typedef struct sPAPRDRConnectorClass { @@ -217,9 +216,6 @@ typedef struct sPAPRDRConnectorClass { uint32_t (*isolate)(sPAPRDRConnector *drc); uint32_t (*unisolate)(sPAPRDRConnector *drc); void (*release)(DeviceState *dev); - - /* QEMU interfaces for managing hotplug operations */ - bool (*release_pending)(sPAPRDRConnector *drc); } sPAPRDRConnectorClass; =20 uint32_t spapr_drc_index(sPAPRDRConnector *drc); @@ -236,4 +232,9 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceStat= e *d, void *fdt, int fdt_start_offset, Error **errp); void spapr_drc_detach(sPAPRDRConnector *drc); =20 +static inline bool spapr_drc_unplug_requested(sPAPRDRConnector *drc) +{ + return drc->unplug_requested; +} + #endif /* HW_SPAPR_DRC_H */ --=20 2.9.4 From nobody Sun Apr 28 03:06:58 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.zoho.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 1498037111292940.8500207569305; Wed, 21 Jun 2017 02:25:11 -0700 (PDT) Received: from localhost ([::1]:52706 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbsr-0000Sd-EJ for importer@patchew.org; Wed, 21 Jun 2017 05:25:09 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40375) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbmw-0003DE-JC for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dNbmt-0001yV-JH for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:02 -0400 Received: from ozlabs.org ([103.22.144.67]:43189) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dNbms-0001xc-VI; Wed, 21 Jun 2017 05:18:59 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3wszfm3YFNz9s7B; Wed, 21 Jun 2017 19:18:56 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1498036736; bh=ssqwyjC8xntHmXFQPC/mU7M/dyu70nz3YA3ttRumB+8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=demva7tGVmqCRjgj5udrLqAzb3rpMC2MQmLeEbBg0uPYMgsvJVaKI0HL0yCqqz4lX IVkxExS2ntbmA9P/vTaQruUrvsdE49aNWrOB3h17GHIe3Hhf73LkVyGNgKI88JYrKZ QEmZvsO8RIELT7Jnzn2mGun9/4EZLzmWLyR3O4ww= From: David Gibson To: mdroth@linux.vnet.ibm.com Date: Wed, 21 Jun 2017 17:18:47 +0800 Message-Id: <20170621091848.28256-5-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.9.4 In-Reply-To: <20170621091848.28256-1-david@gibson.dropbear.id.au> References: <20170621091848.28256-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] [PATCH 4/5] spapr: Consolidate DRC state variables 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, sursingh@redhat.com, qemu-devel@nongnu.org, 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" Each DRC has three fields describing its state: isolation_state, allocation_state and configured. At first this seems like a reasonable representation, since its based directly on the PAPR defined isolation-state and allocation-state indicators. However: * Only a few combinations of the two fields' values are permitted * allocation_state isn't used at all for physical DRCs * The indicators are write only so they don't really have a well defined current value independent of each other This replaces these variables with a single state variable, whose names and numbers are based on the diagram in LoPAPR section 13.4. Along with this we add code to check the current state on various operations and make sure the requested transition is permitted. Strictly speaking, this makes guest visible changes to behaviour (since we probably allowed some transitions we shouldn't have before). However, a hypothetical guest broken by that wasn't PAPR compliant, and probably wouldn't have worked under PowerVM. Signed-off-by: David Gibson Tested-by: Daniel Barboza --- hw/ppc/spapr_drc.c | 224 ++++++++++++++++++++++++-----------------= ---- hw/ppc/trace-events | 3 +- include/hw/ppc/spapr_drc.h | 25 ++++- 3 files changed, 144 insertions(+), 108 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 7fa9595..54c3757 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -48,6 +48,17 @@ uint32_t spapr_drc_index(sPAPRDRConnector *drc) =20 static uint32_t drc_isolate_physical(sPAPRDRConnector *drc) { + switch (drc->state) { + case SPAPR_DRC_STATE_PHYSICAL_POWERON: + return RTAS_OUT_SUCCESS; /* Nothing to do */ + case SPAPR_DRC_STATE_PHYSICAL_CONFIGURED: + break; /* see below */ + case SPAPR_DRC_STATE_PHYSICAL_UNISOLATE: + return RTAS_OUT_PARAM_ERROR; /* not allowed */ + default: + g_assert_not_reached(); + } + /* if the guest is configuring a device attached to this DRC, we * should reset the configuration state at this point since it may * no longer be reliable (guest released device and needs to start @@ -56,32 +67,29 @@ static uint32_t drc_isolate_physical(sPAPRDRConnector *= drc) g_free(drc->ccs); drc->ccs =3D NULL; =20 - drc->isolation_state =3D SPAPR_DR_ISOLATION_STATE_ISOLATED; + drc->state =3D SPAPR_DRC_STATE_PHYSICAL_POWERON; =20 - /* if we're awaiting release, but still in an unconfigured state, - * it's likely the guest is still in the process of configuring - * the device and is transitioning the devices to an ISOLATED - * state as a part of that process. so we only complete the - * removal when this transition happens for a device in a - * configured state, as suggested by the state diagram from PAPR+ - * 2.7, 13.4 - */ if (drc->unplug_requested) { uint32_t drc_index =3D spapr_drc_index(drc); - if (drc->configured) { - trace_spapr_drc_set_isolation_state_finalizing(drc_index); - spapr_drc_detach(drc); - } else { - trace_spapr_drc_set_isolation_state_deferring(drc_index); - } + trace_spapr_drc_set_isolation_state_finalizing(drc_index); + spapr_drc_detach(drc); } - drc->configured =3D false; =20 return RTAS_OUT_SUCCESS; } =20 static uint32_t drc_unisolate_physical(sPAPRDRConnector *drc) { + switch (drc->state) { + case SPAPR_DRC_STATE_PHYSICAL_UNISOLATE: + case SPAPR_DRC_STATE_PHYSICAL_CONFIGURED: + return RTAS_OUT_SUCCESS; /* Nothing to do */ + case SPAPR_DRC_STATE_PHYSICAL_POWERON: + break; /* see below */ + default: + g_assert_not_reached(); + } + /* cannot unisolate a non-existent resource, and, or resources * which are in an 'UNUSABLE' allocation state. (PAPR 2.7, * 13.5.3.5) @@ -90,13 +98,25 @@ static uint32_t drc_unisolate_physical(sPAPRDRConnector= *drc) return RTAS_OUT_NO_SUCH_INDICATOR; } =20 - drc->isolation_state =3D SPAPR_DR_ISOLATION_STATE_UNISOLATED; + drc->state =3D SPAPR_DRC_STATE_PHYSICAL_UNISOLATE; =20 return RTAS_OUT_SUCCESS; } =20 static uint32_t drc_isolate_logical(sPAPRDRConnector *drc) { + switch (drc->state) { + case SPAPR_DRC_STATE_LOGICAL_AVAILABLE: + case SPAPR_DRC_STATE_LOGICAL_UNUSABLE: + return RTAS_OUT_SUCCESS; /* Nothing to do */ + case SPAPR_DRC_STATE_LOGICAL_CONFIGURED: + break; /* see below */ + case SPAPR_DRC_STATE_LOGICAL_UNISOLATE: + return RTAS_OUT_PARAM_ERROR; /* not allowed */ + default: + g_assert_not_reached(); + } + /* if the guest is configuring a device attached to this DRC, we * should reset the configuration state at this point since it may * no longer be reliable (guest released device and needs to start @@ -120,7 +140,7 @@ static uint32_t drc_isolate_logical(sPAPRDRConnector *d= rc) return RTAS_OUT_HW_ERROR; } =20 - drc->isolation_state =3D SPAPR_DR_ISOLATION_STATE_ISOLATED; + drc->state =3D SPAPR_DRC_STATE_LOGICAL_AVAILABLE; =20 /* if we're awaiting release, but still in an unconfigured state, * it's likely the guest is still in the process of configuring @@ -132,36 +152,46 @@ static uint32_t drc_isolate_logical(sPAPRDRConnector = *drc) */ if (drc->unplug_requested) { uint32_t drc_index =3D spapr_drc_index(drc); - if (drc->configured) { - trace_spapr_drc_set_isolation_state_finalizing(drc_index); - spapr_drc_detach(drc); - } else { - trace_spapr_drc_set_isolation_state_deferring(drc_index); - } + trace_spapr_drc_set_isolation_state_finalizing(drc_index); + spapr_drc_detach(drc); } - drc->configured =3D false; - return RTAS_OUT_SUCCESS; } =20 static uint32_t drc_unisolate_logical(sPAPRDRConnector *drc) { - /* cannot unisolate a non-existent resource, and, or resources - * which are in an 'UNUSABLE' allocation state. (PAPR 2.7, - * 13.5.3.5) - */ - if (!drc->dev || - drc->allocation_state =3D=3D SPAPR_DR_ALLOCATION_STATE_UNUSABLE) { - return RTAS_OUT_NO_SUCH_INDICATOR; + switch (drc->state) { + case SPAPR_DRC_STATE_LOGICAL_UNISOLATE: + case SPAPR_DRC_STATE_LOGICAL_CONFIGURED: + return RTAS_OUT_SUCCESS; /* Nothing to do */ + case SPAPR_DRC_STATE_LOGICAL_AVAILABLE: + break; /* see below */ + case SPAPR_DRC_STATE_LOGICAL_UNUSABLE: + return RTAS_OUT_NO_SUCH_INDICATOR; /* not allowed */ + default: + g_assert_not_reached(); } =20 - drc->isolation_state =3D SPAPR_DR_ISOLATION_STATE_UNISOLATED; + /* Move to AVAILABLE state should have ensured device was present */ + g_assert(drc->dev); =20 + drc->state =3D SPAPR_DRC_STATE_LOGICAL_UNISOLATE; return RTAS_OUT_SUCCESS; } =20 static uint32_t drc_set_usable(sPAPRDRConnector *drc) { + switch (drc->state) { + case SPAPR_DRC_STATE_LOGICAL_AVAILABLE: + case SPAPR_DRC_STATE_LOGICAL_UNISOLATE: + case SPAPR_DRC_STATE_LOGICAL_CONFIGURED: + return RTAS_OUT_SUCCESS; /* Nothing to do */ + case SPAPR_DRC_STATE_LOGICAL_UNUSABLE: + break; /* see below */ + default: + g_assert_not_reached(); + } + /* if there's no resource/device associated with the DRC, there's * no way for us to put it in an allocation state consistent with * being 'USABLE'. PAPR 2.7, 13.5.3.4 documents that this should @@ -176,14 +206,26 @@ static uint32_t drc_set_usable(sPAPRDRConnector *drc) return RTAS_OUT_NO_SUCH_INDICATOR; } =20 - drc->allocation_state =3D SPAPR_DR_ALLOCATION_STATE_USABLE; + drc->state =3D SPAPR_DRC_STATE_LOGICAL_AVAILABLE; =20 return RTAS_OUT_SUCCESS; } =20 static uint32_t drc_set_unusable(sPAPRDRConnector *drc) { - drc->allocation_state =3D SPAPR_DR_ALLOCATION_STATE_UNUSABLE; + switch (drc->state) { + case SPAPR_DRC_STATE_LOGICAL_UNUSABLE: + return RTAS_OUT_SUCCESS; /* Nothing to do */ + case SPAPR_DRC_STATE_LOGICAL_AVAILABLE: + break; /* see below */ + case SPAPR_DRC_STATE_LOGICAL_UNISOLATE: + case SPAPR_DRC_STATE_LOGICAL_CONFIGURED: + return RTAS_OUT_NO_SUCH_INDICATOR; /* not allowed */ + default: + g_assert_not_reached(); + } + + drc->state =3D SPAPR_DRC_STATE_LOGICAL_UNUSABLE; if (drc->unplug_requested) { uint32_t drc_index =3D spapr_drc_index(drc); trace_spapr_drc_set_allocation_state_finalizing(drc_index); @@ -241,11 +283,16 @@ static sPAPRDREntitySense physical_entity_sense(sPAPR= DRConnector *drc) =20 static sPAPRDREntitySense logical_entity_sense(sPAPRDRConnector *drc) { - if (drc->dev - && (drc->allocation_state !=3D SPAPR_DR_ALLOCATION_STATE_UNUSABLE)= ) { - return SPAPR_DR_ENTITY_SENSE_PRESENT; - } else { + switch (drc->state) { + case SPAPR_DRC_STATE_LOGICAL_UNUSABLE: return SPAPR_DR_ENTITY_SENSE_UNUSABLE; + case SPAPR_DRC_STATE_LOGICAL_AVAILABLE: + case SPAPR_DRC_STATE_LOGICAL_UNISOLATE: + case SPAPR_DRC_STATE_LOGICAL_CONFIGURED: + g_assert(drc->dev); + return SPAPR_DR_ENTITY_SENSE_PRESENT; + default: + g_assert_not_reached(); } } =20 @@ -338,13 +385,12 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceSt= ate *d, void *fdt, { trace_spapr_drc_attach(spapr_drc_index(drc)); =20 - if (drc->isolation_state !=3D SPAPR_DR_ISOLATION_STATE_ISOLATED) { + if (drc->dev) { error_setg(errp, "an attached device is still awaiting release"); return; } - if (spapr_drc_type(drc) =3D=3D SPAPR_DR_CONNECTOR_TYPE_PCI) { - g_assert(drc->allocation_state =3D=3D SPAPR_DR_ALLOCATION_STATE_US= ABLE); - } + g_assert((drc->state =3D=3D SPAPR_DRC_STATE_LOGICAL_UNUSABLE) + || (drc->state =3D=3D SPAPR_DRC_STATE_PHYSICAL_POWERON)); g_assert(fdt); =20 drc->dev =3D d; @@ -373,18 +419,16 @@ static void spapr_drc_release(sPAPRDRConnector *drc) =20 void spapr_drc_detach(sPAPRDRConnector *drc) { + sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); + trace_spapr_drc_detach(spapr_drc_index(drc)); =20 - drc->unplug_requested =3D true; + g_assert(drc->dev); =20 - if (drc->isolation_state !=3D SPAPR_DR_ISOLATION_STATE_ISOLATED) { - trace_spapr_drc_awaiting_isolated(spapr_drc_index(drc)); - return; - } + drc->unplug_requested =3D true; =20 - if (spapr_drc_type(drc) !=3D SPAPR_DR_CONNECTOR_TYPE_PCI && - drc->allocation_state !=3D SPAPR_DR_ALLOCATION_STATE_UNUSABLE) { - trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc)); + if (drc->state !=3D drck->empty_state) { + trace_spapr_drc_awaiting_quiesce(spapr_drc_index(drc)); return; } =20 @@ -394,6 +438,7 @@ void spapr_drc_detach(sPAPRDRConnector *drc) static void drc_reset(void *opaque) { sPAPRDRConnector *drc =3D SPAPR_DR_CONNECTOR(opaque); + sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); =20 trace_spapr_drc_reset(spapr_drc_index(drc)); =20 @@ -410,19 +455,10 @@ static void drc_reset(void *opaque) drc->awaiting_allocation =3D false; =20 if (drc->dev) { - /* A device present at reset is coldplugged */ - drc->isolation_state =3D SPAPR_DR_ISOLATION_STATE_UNISOLATED; - if (spapr_drc_type(drc) !=3D SPAPR_DR_CONNECTOR_TYPE_PCI) { - drc->allocation_state =3D SPAPR_DR_ALLOCATION_STATE_USABLE; - } - drc->dr_indicator =3D SPAPR_DR_INDICATOR_ACTIVE; + /* A device present at reset is ready to go, same as coldplugged */ + drc->state =3D drck->ready_state; } else { - /* Otherwise device is absent, but might be hotplugged */ - drc->isolation_state =3D SPAPR_DR_ISOLATION_STATE_ISOLATED; - if (spapr_drc_type(drc) !=3D SPAPR_DR_CONNECTOR_TYPE_PCI) { - drc->allocation_state =3D SPAPR_DR_ALLOCATION_STATE_UNUSABLE; - } - drc->dr_indicator =3D SPAPR_DR_INDICATOR_INACTIVE; + drc->state =3D drck->empty_state; } } =20 @@ -430,7 +466,6 @@ static bool spapr_drc_needed(void *opaque) { sPAPRDRConnector *drc =3D (sPAPRDRConnector *)opaque; sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); - bool rc =3D false; sPAPRDREntitySense value =3D drck->dr_entity_sense(drc); =20 /* If no dev is plugged in there is no need to migrate the DRC state */ @@ -439,23 +474,10 @@ static bool spapr_drc_needed(void *opaque) } =20 /* - * If there is dev plugged in, we need to migrate the DRC state when - * it is different from cold-plugged state - */ - switch (spapr_drc_type(drc)) { - case SPAPR_DR_CONNECTOR_TYPE_PCI: - case SPAPR_DR_CONNECTOR_TYPE_CPU: - case SPAPR_DR_CONNECTOR_TYPE_LMB: - rc =3D !((drc->isolation_state =3D=3D SPAPR_DR_ISOLATION_STATE_UNI= SOLATED) && - (drc->allocation_state =3D=3D SPAPR_DR_ALLOCATION_STATE_USA= BLE) && - drc->configured); - break; - case SPAPR_DR_CONNECTOR_TYPE_PHB: - case SPAPR_DR_CONNECTOR_TYPE_VIO: - default: - g_assert_not_reached(); - } - return rc; + * We need to migrate the state if it's not equal to the expected + * long-term state, which is the same as the coldplugged initial + * state */ + return (drc->state !=3D drck->ready_state); } =20 static const VMStateDescription vmstate_spapr_drc =3D { @@ -464,10 +486,8 @@ static const VMStateDescription vmstate_spapr_drc =3D { .minimum_version_id =3D 1, .needed =3D spapr_drc_needed, .fields =3D (VMStateField []) { - VMSTATE_UINT32(isolation_state, sPAPRDRConnector), - VMSTATE_UINT32(allocation_state, sPAPRDRConnector), + VMSTATE_UINT32(state, sPAPRDRConnector), VMSTATE_UINT32(dr_indicator, sPAPRDRConnector), - VMSTATE_BOOL(configured, sPAPRDRConnector), VMSTATE_END_OF_LIST() } }; @@ -536,23 +556,20 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owne= r, const char *type, object_property_set_bool(OBJECT(drc), true, "realized", NULL); g_free(prop_name); =20 - /* PCI slot always start in a USABLE state, and stay there */ - if (spapr_drc_type(drc) =3D=3D SPAPR_DR_CONNECTOR_TYPE_PCI) { - drc->allocation_state =3D SPAPR_DR_ALLOCATION_STATE_USABLE; - } - return drc; } =20 static void spapr_dr_connector_instance_init(Object *obj) { sPAPRDRConnector *drc =3D SPAPR_DR_CONNECTOR(obj); + sPAPRDRConnectorClass *drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); =20 object_property_add_uint32_ptr(obj, "id", &drc->id, NULL); object_property_add(obj, "index", "uint32", prop_get_index, NULL, NULL, NULL, NULL); object_property_add(obj, "fdt", "struct", prop_get_fdt, NULL, NULL, NULL, NULL); + drc->state =3D drck->empty_state; } =20 static void spapr_dr_connector_class_init(ObjectClass *k, void *data) @@ -574,6 +591,8 @@ static void spapr_drc_physical_class_init(ObjectClass *= k, void *data) drck->dr_entity_sense =3D physical_entity_sense; drck->isolate =3D drc_isolate_physical; drck->unisolate =3D drc_unisolate_physical; + drck->ready_state =3D SPAPR_DRC_STATE_PHYSICAL_CONFIGURED; + drck->empty_state =3D SPAPR_DRC_STATE_PHYSICAL_POWERON; } =20 static void spapr_drc_logical_class_init(ObjectClass *k, void *data) @@ -583,6 +602,8 @@ static void spapr_drc_logical_class_init(ObjectClass *k= , void *data) drck->dr_entity_sense =3D logical_entity_sense; drck->isolate =3D drc_isolate_logical; drck->unisolate =3D drc_unisolate_logical; + drck->ready_state =3D SPAPR_DRC_STATE_LOGICAL_CONFIGURED; + drck->empty_state =3D SPAPR_DRC_STATE_LOGICAL_UNUSABLE; } =20 static void spapr_drc_cpu_class_init(ObjectClass *k, void *data) @@ -986,6 +1007,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *c= pu, uint64_t wa_offset; uint32_t drc_index; sPAPRDRConnector *drc; + sPAPRDRConnectorClass *drck; sPAPRConfigureConnectorState *ccs; sPAPRDRCCResponse resp =3D SPAPR_DR_CC_RESPONSE_CONTINUE; int rc; @@ -1005,12 +1027,17 @@ static void rtas_ibm_configure_connector(PowerPCCPU= *cpu, goto out; } =20 - if (!drc->fdt) { - trace_spapr_rtas_ibm_configure_connector_missing_fdt(drc_index); + if ((drc->state !=3D SPAPR_DRC_STATE_LOGICAL_UNISOLATE) + && (drc->state !=3D SPAPR_DRC_STATE_PHYSICAL_UNISOLATE)) { + /* Need to unisolate the device before configuring */ rc =3D SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE; goto out; } =20 + g_assert(drc->fdt); + + drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); + ccs =3D drc->ccs; if (!ccs) { ccs =3D g_new0(sPAPRConfigureConnectorState, 1); @@ -1040,18 +1067,11 @@ static void rtas_ibm_configure_connector(PowerPCCPU= *cpu, case FDT_END_NODE: ccs->fdt_depth--; if (ccs->fdt_depth =3D=3D 0) { - sPAPRDRIsolationState state =3D drc->isolation_state; uint32_t drc_index =3D spapr_drc_index(drc); - /* done sending the device tree, don't need to track - * the state anymore - */ + + /* done sending the device tree, move to configured state = */ trace_spapr_drc_set_configured(drc_index); - if (state =3D=3D SPAPR_DR_ISOLATION_STATE_UNISOLATED) { - drc->configured =3D true; - } else { - /* guest should be not configuring an isolated device = */ - trace_spapr_drc_set_configured_skipping(drc_index); - } + drc->state =3D drck->ready_state; g_free(ccs); drc->ccs =3D NULL; ccs =3D NULL; diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events index 3e8e3cf..8e79f7e 100644 --- a/hw/ppc/trace-events +++ b/hw/ppc/trace-events @@ -46,8 +46,7 @@ spapr_drc_set_configured(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_set_configured_skipping(uint32_t index) "drc: 0x%"PRIx32", isola= ted device" spapr_drc_attach(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_detach(uint32_t index) "drc: 0x%"PRIx32 -spapr_drc_awaiting_isolated(uint32_t index) "drc: 0x%"PRIx32 -spapr_drc_awaiting_unusable(uint32_t index) "drc: 0x%"PRIx32 +spapr_drc_awaiting_quiesce(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_awaiting_allocation(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_reset(uint32_t index) "drc: 0x%"PRIx32 spapr_drc_realize(uint32_t index) "drc: 0x%"PRIx32 diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 7846cca..c229722 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -172,6 +172,24 @@ typedef enum { SPAPR_DR_CC_RESPONSE_NOT_CONFIGURABLE =3D -9003, } sPAPRDRCCResponse; =20 +typedef enum { + /* + * Values come from Fig. 12 in LoPAPR section 13.4 + * + * These are exposed in the migration stream, so don't change + * them. + */ + SPAPR_DRC_STATE_INVALID =3D 0, + SPAPR_DRC_STATE_LOGICAL_UNUSABLE =3D 1, + SPAPR_DRC_STATE_LOGICAL_AVAILABLE =3D 2, + SPAPR_DRC_STATE_LOGICAL_UNISOLATE =3D 3, + SPAPR_DRC_STATE_LOGICAL_CONFIGURED =3D 4, + SPAPR_DRC_STATE_PHYSICAL_AVAILABLE =3D 5, + SPAPR_DRC_STATE_PHYSICAL_POWERON =3D 6, + SPAPR_DRC_STATE_PHYSICAL_UNISOLATE =3D 7, + SPAPR_DRC_STATE_PHYSICAL_CONFIGURED =3D 8, +} sPAPRDRCState; + /* rtas-configure-connector state */ typedef struct sPAPRConfigureConnectorState { int fdt_offset; @@ -188,14 +206,11 @@ typedef struct sPAPRDRConnector { /* DR-indicator */ uint32_t dr_indicator; =20 - /* sensor/indicator states */ - uint32_t isolation_state; - uint32_t allocation_state; + uint32_t state; =20 /* configure-connector state */ void *fdt; int fdt_start_offset; - bool configured; sPAPRConfigureConnectorState *ccs; =20 /* device pointer, via link property */ @@ -206,6 +221,8 @@ typedef struct sPAPRDRConnector { typedef struct sPAPRDRConnectorClass { /*< private >*/ DeviceClass parent; + sPAPRDRCState empty_state; + sPAPRDRCState ready_state; =20 /*< public >*/ sPAPRDRConnectorTypeShift typeshift; --=20 2.9.4 From nobody Sun Apr 28 03:06:58 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.zoho.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 1498037054404654.0859135183877; Wed, 21 Jun 2017 02:24:14 -0700 (PDT) Received: from localhost ([::1]:52699 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbrw-0007ao-7y for importer@patchew.org; Wed, 21 Jun 2017 05:24:12 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40397) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dNbmy-0003EW-1R for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dNbmw-0001zt-OP for qemu-devel@nongnu.org; Wed, 21 Jun 2017 05:19:04 -0400 Received: from ozlabs.org ([103.22.144.67]:35591) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dNbmv-0001yc-Iv; Wed, 21 Jun 2017 05:19:02 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3wszfm4857z9s7F; Wed, 21 Jun 2017 19:18:56 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1498036736; bh=v9wMK3CqdewwmhnCsFhWKSh50RuMnLG4iIyGahdYo+c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jakvy8hfrfXBFvAXyDFBqn5ruKMBH4ozw+9BuWnDoeBf1VBWsZVqVFcdt4+wR6Sb5 ei5kmbk0hk98Y2cBJQUC8l2vGOm0O28saNM0dPRRBXcMX4iuD2w+XOyw9+aMKjG1fy Hf0kqSc1KkWm3WamrhNGeBQh6NENh6h6LJTmfxbA= From: David Gibson To: mdroth@linux.vnet.ibm.com Date: Wed, 21 Jun 2017 17:18:48 +0800 Message-Id: <20170621091848.28256-6-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.9.4 In-Reply-To: <20170621091848.28256-1-david@gibson.dropbear.id.au> References: <20170621091848.28256-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] [PATCH 5/5] spapr: Remove sPAPRConfigureConnectorState sub-structure 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, sursingh@redhat.com, qemu-devel@nongnu.org, 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" Most of the time, the state of a DRC object is contained in the single 'state' variable. However, during the transition from UNISOLATE to CONFIGURED state requires multiple calls to the ibm,configure-connector RTAS call to retrieve the device tree for the attached device. We need some extra state to keep track of where we're up to in delivering the device tree information to the guest. Currently that extra state is in a sPAPRConfigureConnectorState substructure which is only allocated when we're in the middle of the configure connector process. That sounds like a good idea, but the extra state is only two integers - on many platforms that will take up the same room as the (maybe NULL) ccs pointer even before malloc() overhead. Plus it's another object whose lifetime we need to manage. In short, it's not worth it. So, fold the sPAPRConfigureConnectorState substructure directly into the DRC object. Previously the structure was allocated lazily when the configure-connector call discovers it's not there. Now, we need to initialize the subfields pre-emptively, as soon as we enter UNISOLATE state. Although it's not strictly necessary (the field values should only ever be consulted when in UNISOLATE state), we try to keep them at -1 when in other states, as a debugging aid. Signed-off-by: David Gibson Tested-by: Daniel Barboza --- hw/ppc/spapr_drc.c | 56 +++++++++++++++---------------------------= ---- include/hw/ppc/spapr_drc.h | 16 +++++-------- 2 files changed, 24 insertions(+), 48 deletions(-) diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c index 54c3757..4251031 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -59,14 +59,6 @@ static uint32_t drc_isolate_physical(sPAPRDRConnector *d= rc) g_assert_not_reached(); } =20 - /* if the guest is configuring a device attached to this DRC, we - * should reset the configuration state at this point since it may - * no longer be reliable (guest released device and needs to start - * over, or unplug occurred so the FDT is no longer valid) - */ - g_free(drc->ccs); - drc->ccs =3D NULL; - drc->state =3D SPAPR_DRC_STATE_PHYSICAL_POWERON; =20 if (drc->unplug_requested) { @@ -99,6 +91,8 @@ static uint32_t drc_unisolate_physical(sPAPRDRConnector *= drc) } =20 drc->state =3D SPAPR_DRC_STATE_PHYSICAL_UNISOLATE; + drc->ccs_offset =3D drc->fdt_start_offset; + drc->ccs_depth =3D 0; =20 return RTAS_OUT_SUCCESS; } @@ -117,14 +111,6 @@ static uint32_t drc_isolate_logical(sPAPRDRConnector *= drc) g_assert_not_reached(); } =20 - /* if the guest is configuring a device attached to this DRC, we - * should reset the configuration state at this point since it may - * no longer be reliable (guest released device and needs to start - * over, or unplug occurred so the FDT is no longer valid) - */ - g_free(drc->ccs); - drc->ccs =3D NULL; - /* * Fail any requests to ISOLATE the LMB DRC if this LMB doesn't * belong to a DIMM device that is marked for removal. @@ -176,6 +162,9 @@ static uint32_t drc_unisolate_logical(sPAPRDRConnector = *drc) g_assert(drc->dev); =20 drc->state =3D SPAPR_DRC_STATE_LOGICAL_UNISOLATE; + drc->ccs_offset =3D drc->fdt_start_offset; + drc->ccs_depth =3D 0; + return RTAS_OUT_SUCCESS; } =20 @@ -442,9 +431,6 @@ static void drc_reset(void *opaque) =20 trace_spapr_drc_reset(spapr_drc_index(drc)); =20 - g_free(drc->ccs); - drc->ccs =3D NULL; - /* immediately upon reset we can safely assume DRCs whose devices * are pending removal can be safely removed. */ @@ -460,6 +446,9 @@ static void drc_reset(void *opaque) } else { drc->state =3D drck->empty_state; } + + drc->ccs_offset =3D -1; + drc->ccs_depth =3D -1; } =20 static bool spapr_drc_needed(void *opaque) @@ -1008,7 +997,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *c= pu, uint32_t drc_index; sPAPRDRConnector *drc; sPAPRDRConnectorClass *drck; - sPAPRConfigureConnectorState *ccs; sPAPRDRCCResponse resp =3D SPAPR_DR_CC_RESPONSE_CONTINUE; int rc; =20 @@ -1038,25 +1026,18 @@ static void rtas_ibm_configure_connector(PowerPCCPU= *cpu, =20 drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); =20 - ccs =3D drc->ccs; - if (!ccs) { - ccs =3D g_new0(sPAPRConfigureConnectorState, 1); - ccs->fdt_offset =3D drc->fdt_start_offset; - drc->ccs =3D ccs; - } - do { uint32_t tag; const char *name; const struct fdt_property *prop; int fdt_offset_next, prop_len; =20 - tag =3D fdt_next_tag(drc->fdt, ccs->fdt_offset, &fdt_offset_next); + tag =3D fdt_next_tag(drc->fdt, drc->ccs_offset, &fdt_offset_next); =20 switch (tag) { case FDT_BEGIN_NODE: - ccs->fdt_depth++; - name =3D fdt_get_name(drc->fdt, ccs->fdt_offset, NULL); + drc->ccs_depth++; + name =3D fdt_get_name(drc->fdt, drc->ccs_offset, NULL); =20 /* provide the name of the next OF node */ wa_offset =3D CC_VAL_DATA_OFFSET; @@ -1065,23 +1046,22 @@ static void rtas_ibm_configure_connector(PowerPCCPU= *cpu, resp =3D SPAPR_DR_CC_RESPONSE_NEXT_CHILD; break; case FDT_END_NODE: - ccs->fdt_depth--; - if (ccs->fdt_depth =3D=3D 0) { + drc->ccs_depth--; + if (drc->ccs_depth =3D=3D 0) { uint32_t drc_index =3D spapr_drc_index(drc); =20 /* done sending the device tree, move to configured state = */ trace_spapr_drc_set_configured(drc_index); drc->state =3D drck->ready_state; - g_free(ccs); - drc->ccs =3D NULL; - ccs =3D NULL; + drc->ccs_offset =3D -1; + drc->ccs_depth =3D -1; resp =3D SPAPR_DR_CC_RESPONSE_SUCCESS; } else { resp =3D SPAPR_DR_CC_RESPONSE_PREV_PARENT; } break; case FDT_PROP: - prop =3D fdt_get_property_by_offset(drc->fdt, ccs->fdt_offset, + prop =3D fdt_get_property_by_offset(drc->fdt, drc->ccs_offset, &prop_len); name =3D fdt_string(drc->fdt, fdt32_to_cpu(prop->nameoff)); =20 @@ -1106,8 +1086,8 @@ static void rtas_ibm_configure_connector(PowerPCCPU *= cpu, /* keep seeking for an actionable tag */ break; } - if (ccs) { - ccs->fdt_offset =3D fdt_offset_next; + if (drc->ccs_offset >=3D 0) { + drc->ccs_offset =3D fdt_offset_next; } } while (resp =3D=3D SPAPR_DR_CC_RESPONSE_CONTINUE); =20 diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index c229722..4c54864 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -190,12 +190,6 @@ typedef enum { SPAPR_DRC_STATE_PHYSICAL_CONFIGURED =3D 8, } sPAPRDRCState; =20 -/* rtas-configure-connector state */ -typedef struct sPAPRConfigureConnectorState { - int fdt_offset; - int fdt_depth; -} sPAPRConfigureConnectorState; - typedef struct sPAPRDRConnector { /*< private >*/ DeviceState parent; @@ -208,14 +202,16 @@ typedef struct sPAPRDRConnector { =20 uint32_t state; =20 - /* configure-connector state */ - void *fdt; - int fdt_start_offset; - sPAPRConfigureConnectorState *ccs; + /* RTAS ibm,configure-connector state */ + /* (only valid in UNISOLATE state) */ + int ccs_offset; + int ccs_depth; =20 /* device pointer, via link property */ DeviceState *dev; bool unplug_requested; + void *fdt; + int fdt_start_offset; } sPAPRDRConnector; =20 typedef struct sPAPRDRConnectorClass { --=20 2.9.4