[Skiboot] [PATCH 09/51] libflash/ipmi-hiomap: Improve BMC error state behaviour

[Andrew Jeffery]

impi_hiomap_handle_events()'s implementation contained an early-exit
that was only valid for handling ackable events, but the state provided
in the BMC's notification contains non-ackable state as well. Remove the
early exit to ensure we don't attempt to proceed with requests that we
know will fail which occur after an initial request has cleared the
ackable bits (but failed to restore the window state).

Include a test case to capture the desired behaviour.

Cc: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
---
 libflash/ipmi-hiomap.c           | 11 +----------
 libflash/ipmi-hiomap.h           |  1 -
 libflash/test/test-ipmi-hiomap.c | 33 ++++++++++++++++++++++++++++++++
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/libflash/ipmi-hiomap.c b/libflash/ipmi-hiomap.c
index ac47a1005ef8..3d6197541cd6 100644
--- a/libflash/ipmi-hiomap.c
+++ b/libflash/ipmi-hiomap.c
@@ -447,7 +447,6 @@ static void hiomap_event(uint8_t events, void *context)
 
 	lock(&ctx->lock);
 	ctx->bmc_state = events | (ctx->bmc_state & HIOMAP_E_ACK_MASK);
-	ctx->update = true;
 	unlock(&ctx->lock);
 }
 
@@ -562,20 +561,12 @@ static bool ipmi_hiomap_restore_window(struct ipmi_hiomap *ctx)
 static int ipmi_hiomap_handle_events(struct ipmi_hiomap *ctx)
 {
 	uint8_t status;
-	bool update;
 
 	lock(&ctx->lock);
 	status = ctx->bmc_state;
-	update = ctx->update;
-	if (update) {
-		ctx->bmc_state &= ~HIOMAP_E_ACK_MASK;
-		ctx->update = false;
-	}
+	ctx->bmc_state &= ~HIOMAP_E_ACK_MASK;
 	unlock(&ctx->lock);
 
-	if (!update)
-		return 0;
-
 	if (!(status & HIOMAP_E_DAEMON_READY)) {
 		prerror("Daemon not ready\n");
 		return FLASH_ERR_DEVICE_GONE;
diff --git a/libflash/ipmi-hiomap.h b/libflash/ipmi-hiomap.h
index f60f725d2ef0..edd4ee0a364f 100644
--- a/libflash/ipmi-hiomap.h
+++ b/libflash/ipmi-hiomap.h
@@ -48,7 +48,6 @@ struct ipmi_hiomap {
 	 * three variables are protected by lock to avoid conflict.
 	 */
 	struct lock lock;
-	bool update;
 	uint8_t bmc_state;
 	enum lpc_window_state window_state;
 };
diff --git a/libflash/test/test-ipmi-hiomap.c b/libflash/test/test-ipmi-hiomap.c
index 4e1232f22ed0..08c9b32f816a 100644
--- a/libflash/test/test-ipmi-hiomap.c
+++ b/libflash/test/test-ipmi-hiomap.c
@@ -1,5 +1,6 @@
 #include <assert.h>
 #include <ccan/container_of/container_of.h>
+#include <libflash/blocklevel.h>
 #include <lock.h>
 #include <lpc.h>
 #include <hiomap.h>
@@ -12,6 +13,9 @@
 #include "../ipmi-hiomap.h"
 #include "../errors.h"
 
+/* Stub for blocklevel debug macros */
+bool libflash_debug;
+
 const struct bmc_sw_config bmc_sw_hiomap = {
 	.ipmi_oem_hiomap_cmd         = IPMI_CODE(0x3a, 0x5a),
 };
@@ -638,6 +642,34 @@ static void test_hiomap_protocol_reset_recovery(void)
 	free(buf);
 }
 
+static const struct scenario_event
+scenario_hiomap_protocol_persistent_error[] = {
+	{ .type = scenario_event_p, .p = &hiomap_ack_call, },
+	{ .type = scenario_event_p, .p = &hiomap_get_info_call, },
+	{ .type = scenario_event_p, .p = &hiomap_get_flash_info_call, },
+	{ .type = scenario_sel, .s = { .bmc_state = HIOMAP_E_PROTOCOL_RESET } },
+	SCENARIO_SENTINEL,
+};
+
+static void test_hiomap_protocol_persistent_error(void)
+{
+	struct blocklevel_device *bl;
+	struct ipmi_hiomap *ctx;
+	char buf;
+	int rc;
+
+	scenario_enter(scenario_hiomap_protocol_persistent_error);
+	assert(!ipmi_hiomap_init(&bl));
+	ctx = container_of(bl, struct ipmi_hiomap, bl);
+	assert(ctx->bmc_state == HIOMAP_E_PROTOCOL_RESET);
+	rc = bl->read(bl, 0, &buf, sizeof(buf));
+	assert(rc == FLASH_ERR_DEVICE_GONE);
+	rc = bl->read(bl, 0, &buf, sizeof(buf));
+	assert(rc == FLASH_ERR_DEVICE_GONE);
+	ipmi_hiomap_exit(bl);
+	scenario_exit();
+}
+
 struct test_case {
 	const char *name;
 	void (*fn)(void);
@@ -653,6 +685,7 @@ struct test_case test_cases[] = {
 	TEST_CASE(test_hiomap_event_daemon_lost_flash_control),
 	TEST_CASE(test_hiomap_event_daemon_regained_flash_control_dirty),
 	TEST_CASE(test_hiomap_protocol_reset_recovery),
+	TEST_CASE(test_hiomap_protocol_persistent_error),
 	{ NULL, NULL },
 };
 
-- 
2.19.1