From patchwork Fri Feb 15 06:56:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Jeffery X-Patchwork-Id: 1042618 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44141P6DYyz9s4Z for ; Fri, 15 Feb 2019 18:01:29 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=aj.id.au Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=aj.id.au header.i=@aj.id.au header.b="ro142FDS"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b="Jn1hiLTr"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 44141P4gngzDqVp for ; Fri, 15 Feb 2019 18:01:29 +1100 (AEDT) X-Original-To: skiboot@lists.ozlabs.org Delivered-To: skiboot@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=aj.id.au (client-ip=66.111.4.26; helo=out2-smtp.messagingengine.com; envelope-from=andrew@aj.id.au; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=aj.id.au Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=aj.id.au header.i=@aj.id.au header.b="ro142FDS"; dkim=pass (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b="Jn1hiLTr"; dkim-atps=neutral Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com [66.111.4.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4413xH5GmpzDqVM for ; Fri, 15 Feb 2019 17:57:55 +1100 (AEDT) Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 8555221F24; Fri, 15 Feb 2019 01:57:53 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Fri, 15 Feb 2019 01:57:53 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=aj.id.au; h=from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; s=fm2; bh=c5ccKeUmSrepM XQegQVXTC1ayMt7hGCXIoW+9SyAsPg=; b=ro142FDSzbZLGH4SbCU+S5RrQN1hm vDNCOzpQN9dRr7Kv8rNU21ypL2TAZJVPvEni0T6+ml4v+cToNfWnXtqE1Es+qBoC Qe3zTNUFUNmL7HN2NS6bYMpn5hvE+4RtVBgE1mJUx1nHDhpmFeQubYcpUrnnppSy myJ9lM1bz60xGzghbkCUBk4TZcLb8qKjq07PHbAvRPd9A0RYXOkvn1orX29YbIch X6dB6aKih5/J/ompXWmQO4noyge/DPEwdirgyafMqiIxO2UeI+2reEUCqvVZym8O FCPHp/S9Al3W+2xoEmqbVqtgAFS3whEgQTLoIXedK4jKEpQi6RXk6xu8w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:date:from :in-reply-to:message-id:mime-version:references:subject:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; bh=c5ccKeUmSrepMXQegQVXTC1ayMt7hGCXIoW+9SyAsPg=; b=Jn1hiLTr VPR71Jh8ysZ4b4/a86IM9W9b/GMxtaPR7gvnC39jkLUh4Z8eyCm/gE/nDTasrzAM onjl3Bb84v8sG91iO9I3c7xFquHvj9lvKCZWBFJA8oDzIbuX5uaWTsYSS27zWIPu yYwL8bi741toq3MOdz8bM3sij74V9RjYgrwBwe0pb/LuApFyG2l1kn3pN1XNN28U h60oUes7kQGEZdZWGgwMZxTn62UMhJPXEjBLsg6VAYPsKIOJ3uaYFJcF6GZarbD3 UKrZDhnSMDs3J8EowHiHyDmv6np8zlUriourT23Pf4K2vc6BCuXO9gGoh+1V8fAB 1g5Momo36q/Fug== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedtledruddtiedgleekucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfquhhtnecuuegrihhlohhuthemucef tddtnecunecujfgurhephffvufffkffojghfggfgsedtkeertdertddtnecuhfhrohhmpe etnhgurhgvficulfgvfhhfvghrhicuoegrnhgurhgvfiesrghjrdhiugdrrghuqeenucfk phepudegrddvrdekledrjeehnecurfgrrhgrmhepmhgrihhlfhhrohhmpegrnhgurhgvfi esrghjrdhiugdrrghunecuvehluhhsthgvrhfuihiivgepie X-ME-Proxy: Received: from localhost.localdomain (unknown [14.2.89.75]) by mail.messagingengine.com (Postfix) with ESMTPA id 08E85E4438; Fri, 15 Feb 2019 01:57:50 -0500 (EST) From: Andrew Jeffery To: skiboot@lists.ozlabs.org Date: Fri, 15 Feb 2019 17:26:27 +1030 Message-Id: <20190215065708.6086-11-andrew@aj.id.au> X-Mailer: git-send-email 2.19.1 In-Reply-To: <20190215065708.6086-1-andrew@aj.id.au> References: <20190215065708.6086-1-andrew@aj.id.au> MIME-Version: 1.0 Subject: [Skiboot] [PATCH 10/51] libflash/ipmi-hiomap: Overhaul error handling X-BeenThere: skiboot@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list for skiboot development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrew Jeffery Errors-To: skiboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Skiboot" The aim is to improve the robustness with respect to absence of the BMC-side daemon. The current error handling roughly mirrors what was done for the mailbox implementation, but there's room for improvement. Errors are split into two classes, those that affect the transport state and those that affect the window validity. From here, we push the transport state error checks right to the bottom of the stack, to ensure the link is known to be in a good state before any message is sent. Window validity tests remain as they were in the hiomap_window_move() and ipmi_hiomap_read() functions. Validity tests are not necessary in the write and erase paths as we will receive an error response from the BMC when performing a dirty or flush on an invalid window. Recovery also remains as it was, done on entry to the blocklevel callbacks. If an error state is encountered in the middle of an operation no attempt is made to recover it on the spot, instead the error is returned up the stack and the caller can choose how it wishes to respond. Signed-off-by: Andrew Jeffery --- libflash/ipmi-hiomap.c | 308 ++++++++++++++++++++++++++--------------- 1 file changed, 198 insertions(+), 110 deletions(-) diff --git a/libflash/ipmi-hiomap.c b/libflash/ipmi-hiomap.c index 3d6197541cd6..32bd557b7c8b 100644 --- a/libflash/ipmi-hiomap.c +++ b/libflash/ipmi-hiomap.c @@ -49,30 +49,59 @@ static inline uint16_t bytes_to_blocks(struct ipmi_hiomap *ctx, uint32_t bytes) return bytes >> ctx->block_size_shift; } -/* Is the current window able perform the complete operation */ -static bool hiomap_window_valid(struct ipmi_hiomap *ctx, uint64_t pos, +/* Call under ctx->lock */ +static int hiomap_protocol_ready(struct ipmi_hiomap *ctx) +{ + if (!(ctx->bmc_state & HIOMAP_E_DAEMON_READY)) + return FLASH_ERR_DEVICE_GONE; + if (ctx->bmc_state & HIOMAP_E_FLASH_LOST) + return FLASH_ERR_AGAIN; + + return 0; +} + +static int hiomap_queue_msg_sync(struct ipmi_hiomap *ctx, struct ipmi_msg *msg) +{ + int rc; + + /* + * There's an unavoidable TOCTOU race here with the BMC sending an + * event saying it's no-longer available right after we test but before + * we call into the IPMI stack to send the message. + * hiomap_queue_msg_sync() exists to capture the race in a single + * location. + */ + lock(&ctx->lock); + rc = hiomap_protocol_ready(ctx); + unlock(&ctx->lock); + if (rc) + return rc; + + ipmi_queue_msg_sync(msg); + + return 0; +} + +/* Call under ctx->lock */ +static int hiomap_window_valid(struct ipmi_hiomap *ctx, uint64_t pos, uint64_t len) { - enum lpc_window_state window_state; - uint8_t bmc_state; + if (ctx->bmc_state & HIOMAP_E_FLASH_LOST) + return FLASH_ERR_AGAIN; + if (ctx->bmc_state & HIOMAP_E_PROTOCOL_RESET) + return FLASH_ERR_AGAIN; + if (ctx->bmc_state & HIOMAP_E_WINDOW_RESET) + return FLASH_ERR_AGAIN; + if (ctx->window_state == closed_window) + return FLASH_ERR_PARM_ERROR; + if (pos < ctx->current.cur_pos) + return FLASH_ERR_PARM_ERROR; + if ((pos + len) > (ctx->current.cur_pos + ctx->current.size)) + return FLASH_ERR_PARM_ERROR; - lock(&ctx->lock); - bmc_state = ctx->bmc_state; - window_state = ctx->window_state; - unlock(&ctx->lock); - - if (bmc_state & HIOMAP_E_FLASH_LOST) - return false; - if (window_state == closed_window) - return false; - if (pos < ctx->current.cur_pos) /* start */ - return false; - if ((pos + len) > (ctx->current.cur_pos + ctx->current.size)) /* end */ - return false; - return true; + return 0; } - static void ipmi_hiomap_cmd_cb(struct ipmi_msg *msg) { struct ipmi_hiomap_result *res = msg->user_data; @@ -155,6 +184,7 @@ static void ipmi_hiomap_cmd_cb(struct ipmi_msg *msg) } parms = (struct hiomap_v2_create_window *)&msg->data[2]; + ctx->current.lpc_addr = blocks_to_bytes(ctx, le16_to_cpu(parms->lpc_addr)); ctx->current.size = @@ -189,13 +219,23 @@ static void ipmi_hiomap_cmd_cb(struct ipmi_msg *msg) ipmi_free_msg(msg); } -static bool hiomap_get_info(struct ipmi_hiomap *ctx) +static void hiomap_init(struct ipmi_hiomap *ctx) +{ + /* + * Speculatively mark the daemon as available so we attempt to perform + * the handshake without immediately bailing out. + */ + lock(&ctx->lock); + ctx->bmc_state = HIOMAP_E_DAEMON_READY; + unlock(&ctx->lock); +} + +static int hiomap_get_info(struct ipmi_hiomap *ctx) { RESULT_INIT(res, ctx); unsigned char req[3]; struct ipmi_msg *msg; - - ctx->bmc_state = 0; + int rc; /* Negotiate protocol version 2 */ req[0] = HIOMAP_C_GET_INFO; @@ -205,43 +245,46 @@ static bool hiomap_get_info(struct ipmi_hiomap *ctx) msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, bmc_platform->sw->ipmi_oem_hiomap_cmd, ipmi_hiomap_cmd_cb, &res, req, sizeof(req), 6); - ipmi_queue_msg_sync(msg); + + rc = hiomap_queue_msg_sync(ctx, msg); + if (rc) + return rc; if (res.cc != IPMI_CC_NO_ERROR) { prerror("%s failed: %d\n", __func__, res.cc); - return false; + return FLASH_ERR_PARM_ERROR; /* XXX: Find something better? */ } - lock(&ctx->lock); - ctx->bmc_state |= HIOMAP_E_DAEMON_READY; - unlock(&ctx->lock); - - return true; + return 0; } -static bool hiomap_get_flash_info(struct ipmi_hiomap *ctx) +static int hiomap_get_flash_info(struct ipmi_hiomap *ctx) { RESULT_INIT(res, ctx); unsigned char req[2]; struct ipmi_msg *msg; + int rc; req[0] = HIOMAP_C_GET_FLASH_INFO; req[1] = ++ctx->seq; msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, bmc_platform->sw->ipmi_oem_hiomap_cmd, ipmi_hiomap_cmd_cb, &res, req, sizeof(req), 2 + 2 + 2); - ipmi_queue_msg_sync(msg); + + rc = hiomap_queue_msg_sync(ctx, msg); + if (rc) + return rc; if (res.cc != IPMI_CC_NO_ERROR) { prerror("%s failed: %d\n", __func__, res.cc); - return false; + return FLASH_ERR_PARM_ERROR; /* XXX: Find something better? */ } - return true; + return 0; } -static bool hiomap_window_move(struct ipmi_hiomap *ctx, uint8_t command, - uint64_t pos, uint64_t len, uint64_t *size) +static int hiomap_window_move(struct ipmi_hiomap *ctx, uint8_t command, + uint64_t pos, uint64_t len, uint64_t *size) { enum lpc_window_state want_state; struct hiomap_v2_range *range; @@ -250,15 +293,25 @@ static bool hiomap_window_move(struct ipmi_hiomap *ctx, uint8_t command, struct ipmi_msg *msg; bool valid_state; bool is_read; + int rc; is_read = (command == HIOMAP_C_CREATE_READ_WINDOW); want_state = is_read ? read_window : write_window; + + lock(&ctx->lock); + valid_state = want_state == ctx->window_state; - if (valid_state && hiomap_window_valid(ctx, pos, len)) { + rc = hiomap_window_valid(ctx, pos, len); + if (valid_state && !rc) { + unlock(&ctx->lock); *size = len; - return true; + return 0; } + ctx->window_state = closed_window; + + unlock(&ctx->lock); + req[0] = command; req[1] = ++ctx->seq; @@ -266,21 +319,21 @@ static bool hiomap_window_move(struct ipmi_hiomap *ctx, uint8_t command, range->offset = cpu_to_le16(bytes_to_blocks(ctx, pos)); range->size = cpu_to_le16(bytes_to_blocks(ctx, len)); - lock(&ctx->lock); - ctx->window_state = closed_window; - unlock(&ctx->lock); - msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, bmc_platform->sw->ipmi_oem_hiomap_cmd, ipmi_hiomap_cmd_cb, &res, req, sizeof(req), 2 + 2 + 2 + 2); - ipmi_queue_msg_sync(msg); + + rc = hiomap_queue_msg_sync(ctx, msg); + if (rc) + return rc; if (res.cc != IPMI_CC_NO_ERROR) { prlog(PR_INFO, "%s failed: %d\n", __func__, res.cc); - return false; + return FLASH_ERR_PARM_ERROR; /* XXX: Find something better? */ } + lock(&ctx->lock); *size = len; /* Is length past the end of the window? */ if ((pos + len) > (ctx->current.cur_pos + ctx->current.size)) @@ -288,19 +341,22 @@ static bool hiomap_window_move(struct ipmi_hiomap *ctx, uint8_t command, *size = (ctx->current.cur_pos + ctx->current.size) - pos; if (len != 0 && *size == 0) { + unlock(&ctx->lock); prerror("Invalid window properties: len: %"PRIu64", size: %"PRIu64"\n", len, *size); - return false; + return FLASH_ERR_PARM_ERROR; } prlog(PR_DEBUG, "Opened %s window from 0x%x for %u bytes at 0x%x\n", (command == HIOMAP_C_CREATE_READ_WINDOW) ? "read" : "write", ctx->current.cur_pos, ctx->current.size, ctx->current.lpc_addr); - return true; + unlock(&ctx->lock); + + return 0; } -static bool hiomap_mark_dirty(struct ipmi_hiomap *ctx, uint64_t offset, +static int hiomap_mark_dirty(struct ipmi_hiomap *ctx, uint64_t offset, uint64_t size) { struct hiomap_v2_range *range; @@ -309,13 +365,14 @@ static bool hiomap_mark_dirty(struct ipmi_hiomap *ctx, uint64_t offset, unsigned char req[6]; struct ipmi_msg *msg; uint32_t pos; + int rc; lock(&ctx->lock); state = ctx->window_state; unlock(&ctx->lock); if (state != write_window) - return false; + return FLASH_ERR_PARM_ERROR; req[0] = HIOMAP_C_MARK_DIRTY; req[1] = ++ctx->seq; @@ -328,32 +385,36 @@ static bool hiomap_mark_dirty(struct ipmi_hiomap *ctx, uint64_t offset, msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, bmc_platform->sw->ipmi_oem_hiomap_cmd, ipmi_hiomap_cmd_cb, &res, req, sizeof(req), 2); - ipmi_queue_msg_sync(msg); + + rc = hiomap_queue_msg_sync(ctx, msg); + if (rc) + return rc; if (res.cc != IPMI_CC_NO_ERROR) { prerror("%s failed: %d\n", __func__, res.cc); - return false; + return FLASH_ERR_PARM_ERROR; } prlog(PR_DEBUG, "Marked flash dirty at 0x%" PRIx64 " for %" PRIu64 "\n", offset, size); - return true; + return 0; } -static bool hiomap_flush(struct ipmi_hiomap *ctx) +static int hiomap_flush(struct ipmi_hiomap *ctx) { enum lpc_window_state state; RESULT_INIT(res, ctx); unsigned char req[2]; struct ipmi_msg *msg; + int rc; lock(&ctx->lock); state = ctx->window_state; unlock(&ctx->lock); if (state != write_window) - return false; + return FLASH_ERR_PARM_ERROR; req[0] = HIOMAP_C_FLUSH; req[1] = ++ctx->seq; @@ -361,23 +422,27 @@ static bool hiomap_flush(struct ipmi_hiomap *ctx) msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, bmc_platform->sw->ipmi_oem_hiomap_cmd, ipmi_hiomap_cmd_cb, &res, req, sizeof(req), 2); - ipmi_queue_msg_sync(msg); + + rc = hiomap_queue_msg_sync(ctx, msg); + if (rc) + return rc; if (res.cc != IPMI_CC_NO_ERROR) { prerror("%s failed: %d\n", __func__, res.cc); - return false; + return FLASH_ERR_PARM_ERROR; } prlog(PR_DEBUG, "Flushed writes\n"); - return true; + return 0; } -static bool hiomap_ack(struct ipmi_hiomap *ctx, uint8_t ack) +static int hiomap_ack(struct ipmi_hiomap *ctx, uint8_t ack) { RESULT_INIT(res, ctx); unsigned char req[3]; struct ipmi_msg *msg; + int rc; req[0] = HIOMAP_C_ACK; req[1] = ++ctx->seq; @@ -386,19 +451,22 @@ static bool hiomap_ack(struct ipmi_hiomap *ctx, uint8_t ack) msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, bmc_platform->sw->ipmi_oem_hiomap_cmd, ipmi_hiomap_cmd_cb, &res, req, sizeof(req), 2); - ipmi_queue_msg_sync(msg); + + rc = hiomap_queue_msg_sync(ctx, msg); + if (rc) + return rc; if (res.cc != IPMI_CC_NO_ERROR) { prlog(PR_DEBUG, "%s failed: %d\n", __func__, res.cc); - return false; + return FLASH_ERR_PARM_ERROR; } prlog(PR_DEBUG, "Acked events: 0x%x\n", ack); - return true; + return 0; } -static bool hiomap_erase(struct ipmi_hiomap *ctx, uint64_t offset, +static int hiomap_erase(struct ipmi_hiomap *ctx, uint64_t offset, uint64_t size) { struct hiomap_v2_range *range; @@ -407,13 +475,14 @@ static bool hiomap_erase(struct ipmi_hiomap *ctx, uint64_t offset, unsigned char req[6]; struct ipmi_msg *msg; uint32_t pos; + int rc; lock(&ctx->lock); state = ctx->window_state; unlock(&ctx->lock); if (state != write_window) - return false; + return FLASH_ERR_PARM_ERROR; req[0] = HIOMAP_C_ERASE; req[1] = ++ctx->seq; @@ -426,17 +495,19 @@ static bool hiomap_erase(struct ipmi_hiomap *ctx, uint64_t offset, msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, bmc_platform->sw->ipmi_oem_hiomap_cmd, ipmi_hiomap_cmd_cb, &res, req, sizeof(req), 2); - ipmi_queue_msg_sync(msg); + rc = hiomap_queue_msg_sync(ctx, msg); + if (rc) + return rc; if (res.cc != IPMI_CC_NO_ERROR) { prerror("%s failed: %d\n", __func__, res.cc); - return false; + return FLASH_ERR_PARM_ERROR; } prlog(PR_DEBUG, "Erased flash at 0x%" PRIx64 " for %" PRIu64 "\n", offset, size); - return true; + return 0; } static void hiomap_event(uint8_t events, void *context) @@ -535,7 +606,7 @@ static int lpc_window_write(struct ipmi_hiomap *ctx, uint32_t pos, } /* Try to restore the window state */ -static bool ipmi_hiomap_restore_window(struct ipmi_hiomap *ctx) +static int ipmi_hiomap_restore_window(struct ipmi_hiomap *ctx) { uint64_t size; uint8_t cmd; @@ -543,7 +614,7 @@ static bool ipmi_hiomap_restore_window(struct ipmi_hiomap *ctx) lock(&ctx->lock); if (ctx->window_state == closed_window) { unlock(&ctx->lock); - return true; + return 0; } cmd = ctx->window_state == read_window ? @@ -561,6 +632,7 @@ static bool ipmi_hiomap_restore_window(struct ipmi_hiomap *ctx) static int ipmi_hiomap_handle_events(struct ipmi_hiomap *ctx) { uint8_t status; + int rc; lock(&ctx->lock); status = ctx->bmc_state; @@ -574,10 +646,11 @@ static int ipmi_hiomap_handle_events(struct ipmi_hiomap *ctx) if (status & HIOMAP_E_ACK_MASK) { /* ACK is unversioned, can send it if the daemon is ready */ - if (!hiomap_ack(ctx, status & HIOMAP_E_ACK_MASK)) { + rc = hiomap_ack(ctx, status & HIOMAP_E_ACK_MASK); + if (rc) { prlog(PR_DEBUG, "Failed to ack events: 0x%x\n", status & HIOMAP_E_ACK_MASK); - return FLASH_ERR_AGAIN; + return rc; } } @@ -589,19 +662,22 @@ static int ipmi_hiomap_handle_events(struct ipmi_hiomap *ctx) if (status & HIOMAP_E_PROTOCOL_RESET) { prlog(PR_INFO, "Protocol was reset\n"); - if (!hiomap_get_info(ctx)) { + rc = hiomap_get_info(ctx); + if (rc) { prerror("Failure to renegotiate after protocol reset\n"); - return FLASH_ERR_DEVICE_GONE; + return rc; } - if (!hiomap_get_flash_info(ctx)) { + rc = hiomap_get_flash_info(ctx); + if (rc) { prerror("Failure to fetch flash info after protocol reset\n"); - return FLASH_ERR_DEVICE_GONE; + return rc; } - if (!ipmi_hiomap_restore_window(ctx)) { + rc = ipmi_hiomap_restore_window(ctx); + if (rc) { prerror("Failure to restore window state after protocol reset\n"); - return FLASH_ERR_DEVICE_GONE; + return rc; } prlog(PR_INFO, "Restored window state after protocol reset\n"); @@ -611,9 +687,10 @@ static int ipmi_hiomap_handle_events(struct ipmi_hiomap *ctx) if (status & HIOMAP_E_WINDOW_RESET) { prlog(PR_INFO, "Window was reset\n"); - if (!ipmi_hiomap_restore_window(ctx)) { + rc = ipmi_hiomap_restore_window(ctx); + if (rc) { prerror("Failed to restore previous window parameters after protocol reset\n"); - return FLASH_ERR_DEVICE_GONE; + return rc; } prlog(PR_INFO, "Restored window state after window reset\n"); @@ -643,9 +720,10 @@ static int ipmi_hiomap_read(struct blocklevel_device *bl, uint64_t pos, len); while (len > 0) { /* Move window and get a new size to read */ - if (!hiomap_window_move(ctx, HIOMAP_C_CREATE_READ_WINDOW, pos, - len, &size)) - return FLASH_ERR_PARM_ERROR; + rc = hiomap_window_move(ctx, HIOMAP_C_CREATE_READ_WINDOW, pos, + len, &size); + if (rc) + return rc; /* Perform the read for this window */ rc = lpc_window_read(ctx, pos, buf, size); @@ -653,8 +731,11 @@ static int ipmi_hiomap_read(struct blocklevel_device *bl, uint64_t pos, return rc; /* Check we can trust what we read */ - if (!hiomap_window_valid(ctx, pos, size)) - return FLASH_ERR_AGAIN; + lock(&ctx->lock); + rc = hiomap_window_valid(ctx, pos, size); + unlock(&ctx->lock); + if (rc) + return rc; len -= size; pos += size; @@ -685,17 +766,19 @@ static int ipmi_hiomap_write(struct blocklevel_device *bl, uint64_t pos, len); while (len > 0) { /* Move window and get a new size to read */ - if (!hiomap_window_move(ctx, HIOMAP_C_CREATE_WRITE_WINDOW, pos, - len, &size)) - return FLASH_ERR_PARM_ERROR; + rc = hiomap_window_move(ctx, HIOMAP_C_CREATE_WRITE_WINDOW, pos, + len, &size); + if (rc) + return rc; /* Perform the write for this window */ rc = lpc_window_write(ctx, pos, buf, size); if (rc) return rc; - if (!hiomap_mark_dirty(ctx, pos, size)) - return FLASH_ERR_PARM_ERROR; + rc = hiomap_mark_dirty(ctx, pos, size); + if (rc) + return rc; /* * The BMC *should* flush if the window is implicitly closed, @@ -703,8 +786,9 @@ static int ipmi_hiomap_write(struct blocklevel_device *bl, uint64_t pos, * * XXX: Removing this could improve performance */ - if (!hiomap_flush(ctx)) - return FLASH_ERR_PARM_ERROR; + rc = hiomap_flush(ctx); + if (rc) + return rc; len -= size; pos += size; @@ -735,20 +819,22 @@ static int ipmi_hiomap_erase(struct blocklevel_device *bl, uint64_t pos, uint64_t size; /* Move window and get a new size to erase */ - if (!hiomap_window_move(ctx, HIOMAP_C_CREATE_WRITE_WINDOW, pos, - len, &size)) - return FLASH_ERR_PARM_ERROR; + rc = hiomap_window_move(ctx, HIOMAP_C_CREATE_WRITE_WINDOW, pos, + len, &size); + if (rc) + return rc; - if (!hiomap_erase(ctx, pos, size)) - return FLASH_ERR_PARM_ERROR; + rc = hiomap_erase(ctx, pos, size); + if (rc) + return rc; /* * Flush directly, don't mark that region dirty otherwise it * isn't clear if a write happened there or not */ - - if (!hiomap_flush(ctx)) - return FLASH_ERR_PARM_ERROR; + rc = hiomap_flush(ctx); + if (rc) + return rc; len -= size; pos += size; @@ -770,9 +856,9 @@ static int ipmi_hiomap_get_flash_info(struct blocklevel_device *bl, if (rc) return rc; - if (!hiomap_get_flash_info(ctx)) { - return FLASH_ERR_DEVICE_GONE; - } + rc = hiomap_get_flash_info(ctx); + if (rc) + return rc; ctx->bl.erase_mask = ctx->erase_granule - 1; @@ -811,29 +897,31 @@ int ipmi_hiomap_init(struct blocklevel_device **bl) ctx->bl.erase = &ipmi_hiomap_erase; ctx->bl.get_info = &ipmi_hiomap_get_flash_info; - rc = ipmi_sel_register(CMD_OP_HIOMAP_EVENT, hiomap_event, ctx); - if (rc < 0) - goto err; + hiomap_init(ctx); /* Ack all pending ack-able events to avoid spurious failures */ - if (!hiomap_ack(ctx, HIOMAP_E_ACK_MASK)) { + rc = hiomap_ack(ctx, HIOMAP_E_ACK_MASK); + if (rc) { prlog(PR_DEBUG, "Failed to ack events: 0x%x\n", HIOMAP_E_ACK_MASK); - rc = FLASH_ERR_AGAIN; goto err; } + rc = ipmi_sel_register(CMD_OP_HIOMAP_EVENT, hiomap_event, ctx); + if (rc < 0) + goto err; + /* Negotiate protocol behaviour */ - if (!hiomap_get_info(ctx)) { - prerror("Failed to get hiomap parameters\n"); - rc = FLASH_ERR_DEVICE_GONE; + rc = hiomap_get_info(ctx); + if (rc) { + prerror("Failed to get hiomap parameters: %d\n", rc); goto err; } /* Grab the flash parameters */ - if (!hiomap_get_flash_info(ctx)) { - prerror("Failed to get flash parameters\n"); - rc = FLASH_ERR_DEVICE_GONE; + rc = hiomap_get_flash_info(ctx); + if (rc) { + prerror("Failed to get flash parameters: %d\n", rc); goto err; }