From patchwork Mon Jul 17 06:56:11 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gibson X-Patchwork-Id: 789253 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3x9vPm3Zy9z9sRg for ; Mon, 17 Jul 2017 17:02:52 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.b="DFoRGmL3"; dkim-atps=neutral Received: from localhost ([::1]:48313 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dX03O-0007ey-CY for incoming@patchwork.ozlabs.org; Mon, 17 Jul 2017 03:02:50 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40744) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dWzxP-0002gD-Rh for qemu-devel@nongnu.org; Mon, 17 Jul 2017 02:56:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dWzxM-0001d5-JX for qemu-devel@nongnu.org; Mon, 17 Jul 2017 02:56:39 -0400 Received: from ozlabs.org ([2401:3900:2:1::2]:49637) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dWzxL-0001a0-S5; Mon, 17 Jul 2017 02:56:36 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3x9vGN1nz0z9t3P; Mon, 17 Jul 2017 16:56:26 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1500274588; bh=KcHApYgFuqdruyXL0AOtblFcfBfS8OKL12yVLOIn6wU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DFoRGmL3eU/tPKSdWSmjf5LWGI5ydKPNH6T2SVbochwXyyN/CY/4b/MtrqzEGD7+e zcCZ8BSkokPBNc0M71sBe5Ard5jH4+IRYIO23b8dj+LHtdcxsB2sQQyBcXenEUSZWk afDzeOJkvX2Usp2hq8PUflBRGYRtK/8c0qji+O58= From: David Gibson To: peter.maydell@linaro.org Date: Mon, 17 Jul 2017 16:56:11 +1000 Message-Id: <20170717065621.4688-12-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.13.3 In-Reply-To: <20170717065621.4688-1-david@gibson.dropbear.id.au> References: <20170717065621.4688-1-david@gibson.dropbear.id.au> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2401:3900:2:1::2 Subject: [Qemu-devel] [PULL 11/21] 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: qemu-devel@nongnu.org, aik@ozlabs.ru, mdroth@linux.vnet.ibm.com, groug@kaod.org, qemu-ppc@nongnu.org, sjitindarsingh@gmail.com, David Gibson Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" 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 Reviewed-by: Daniel Barboza 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 37d7ae7930..11b9fa534b 100644 --- a/hw/ppc/spapr_drc.c +++ b/hw/ppc/spapr_drc.c @@ -59,14 +59,6 @@ static uint32_t drc_isolate_physical(sPAPRDRConnector *drc) 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 - * over, or unplug occurred so the FDT is no longer valid) - */ - g_free(drc->ccs); - drc->ccs = NULL; - drc->state = SPAPR_DRC_STATE_PHYSICAL_POWERON; if (drc->unplug_requested) { @@ -99,6 +91,8 @@ static uint32_t drc_unisolate_physical(sPAPRDRConnector *drc) } drc->state = SPAPR_DRC_STATE_PHYSICAL_UNISOLATE; + drc->ccs_offset = drc->fdt_start_offset; + drc->ccs_depth = 0; return RTAS_OUT_SUCCESS; } @@ -117,14 +111,6 @@ static uint32_t drc_isolate_logical(sPAPRDRConnector *drc) 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 - * over, or unplug occurred so the FDT is no longer valid) - */ - g_free(drc->ccs); - drc->ccs = 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); drc->state = SPAPR_DRC_STATE_LOGICAL_UNISOLATE; + drc->ccs_offset = drc->fdt_start_offset; + drc->ccs_depth = 0; + return RTAS_OUT_SUCCESS; } @@ -441,9 +430,6 @@ void spapr_drc_reset(sPAPRDRConnector *drc) trace_spapr_drc_reset(spapr_drc_index(drc)); - g_free(drc->ccs); - drc->ccs = NULL; - /* immediately upon reset we can safely assume DRCs whose devices * are pending removal can be safely removed. */ @@ -457,6 +443,9 @@ void spapr_drc_reset(sPAPRDRConnector *drc) } else { drc->state = drck->empty_state; } + + drc->ccs_offset = -1; + drc->ccs_depth = -1; } static void drc_reset(void *opaque) @@ -1005,7 +994,6 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, uint32_t drc_index; sPAPRDRConnector *drc; sPAPRDRConnectorClass *drck; - sPAPRConfigureConnectorState *ccs; sPAPRDRCCResponse resp = SPAPR_DR_CC_RESPONSE_CONTINUE; int rc; @@ -1035,25 +1023,18 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc); - ccs = drc->ccs; - if (!ccs) { - ccs = g_new0(sPAPRConfigureConnectorState, 1); - ccs->fdt_offset = drc->fdt_start_offset; - drc->ccs = ccs; - } - do { uint32_t tag; const char *name; const struct fdt_property *prop; int fdt_offset_next, prop_len; - tag = fdt_next_tag(drc->fdt, ccs->fdt_offset, &fdt_offset_next); + tag = fdt_next_tag(drc->fdt, drc->ccs_offset, &fdt_offset_next); switch (tag) { case FDT_BEGIN_NODE: - ccs->fdt_depth++; - name = fdt_get_name(drc->fdt, ccs->fdt_offset, NULL); + drc->ccs_depth++; + name = fdt_get_name(drc->fdt, drc->ccs_offset, NULL); /* provide the name of the next OF node */ wa_offset = CC_VAL_DATA_OFFSET; @@ -1062,23 +1043,22 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD; break; case FDT_END_NODE: - ccs->fdt_depth--; - if (ccs->fdt_depth == 0) { + drc->ccs_depth--; + if (drc->ccs_depth == 0) { uint32_t drc_index = spapr_drc_index(drc); /* done sending the device tree, move to configured state */ trace_spapr_drc_set_configured(drc_index); drc->state = drck->ready_state; - g_free(ccs); - drc->ccs = NULL; - ccs = NULL; + drc->ccs_offset = -1; + drc->ccs_depth = -1; resp = SPAPR_DR_CC_RESPONSE_SUCCESS; } else { resp = SPAPR_DR_CC_RESPONSE_PREV_PARENT; } break; case FDT_PROP: - prop = fdt_get_property_by_offset(drc->fdt, ccs->fdt_offset, + prop = fdt_get_property_by_offset(drc->fdt, drc->ccs_offset, &prop_len); name = fdt_string(drc->fdt, fdt32_to_cpu(prop->nameoff)); @@ -1103,8 +1083,8 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu, /* keep seeking for an actionable tag */ break; } - if (ccs) { - ccs->fdt_offset = fdt_offset_next; + if (drc->ccs_offset >= 0) { + drc->ccs_offset = fdt_offset_next; } } while (resp == SPAPR_DR_CC_RESPONSE_CONTINUE); diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h index 4ceaaf0eff..9d4fd41d22 100644 --- a/include/hw/ppc/spapr_drc.h +++ b/include/hw/ppc/spapr_drc.h @@ -191,12 +191,6 @@ typedef enum { SPAPR_DRC_STATE_PHYSICAL_CONFIGURED = 8, } sPAPRDRCState; -/* rtas-configure-connector state */ -typedef struct sPAPRConfigureConnectorState { - int fdt_offset; - int fdt_depth; -} sPAPRConfigureConnectorState; - typedef struct sPAPRDRConnector { /*< private >*/ DeviceState parent; @@ -209,14 +203,16 @@ typedef struct sPAPRDRConnector { uint32_t state; - /* 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; /* device pointer, via link property */ DeviceState *dev; bool unplug_requested; + void *fdt; + int fdt_start_offset; } sPAPRDRConnector; typedef struct sPAPRDRConnectorClass {