astlpc: Handle loss of bmc-ready state
Up until now the host implementation assumed the BMC was always present.
This may not be the case, so implement the facility to manage the Tx
queue state across BMC reboots.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I6e4c5270bc2e2bca10fed102e93cec13f763667e
diff --git a/astlpc.c b/astlpc.c
index 4092ed9..d32211e 100644
--- a/astlpc.c
+++ b/astlpc.c
@@ -173,11 +173,42 @@
return 0;
}
+static int mctp_astlpc_kcs_set_status(struct mctp_binding_astlpc *astlpc,
+ uint8_t status)
+{
+ uint8_t data;
+ int rc;
+
+ /* Since we're setting the status register, we want the other endpoint
+ * to be interrupted. However, some hardware may only raise a host-side
+ * interrupt on an ODR event.
+ * So, write a dummy value of 0xff to ODR, which will ensure that an
+ * interrupt is triggered, and can be ignored by the host.
+ */
+ data = 0xff;
+ status |= KCS_STATUS_OBF;
+
+ rc = astlpc->ops.kcs_write(astlpc->ops_data, MCTP_ASTLPC_KCS_REG_STATUS,
+ status);
+ if (rc) {
+ astlpc_prwarn(astlpc, "KCS status write failed");
+ return -1;
+ }
+
+ rc = astlpc->ops.kcs_write(astlpc->ops_data, MCTP_ASTLPC_KCS_REG_DATA,
+ data);
+ if (rc) {
+ astlpc_prwarn(astlpc, "KCS dummy data write failed");
+ return -1;
+ }
+
+ return 0;
+}
+
static int mctp_astlpc_init_bmc(struct mctp_binding_astlpc *astlpc)
{
struct mctp_lpcmap_hdr hdr = { 0 };
uint8_t status;
- int rc;
/* Flip the buffers as the names are defined in terms of the host */
astlpc->layout.rx.offset = tx_offset;
@@ -202,14 +233,7 @@
/* set status indicating that the BMC is now active */
status = KCS_STATUS_BMC_READY | KCS_STATUS_OBF;
- /* XXX: Should we be calling mctp_astlpc_kcs_set_status() instead? */
- rc = astlpc->ops.kcs_write(astlpc->ops_data, MCTP_ASTLPC_KCS_REG_STATUS,
- status);
- if (rc) {
- astlpc_prwarn(astlpc, "KCS write failed");
- }
-
- return rc;
+ return mctp_astlpc_kcs_set_status(astlpc, status);
}
static int mctp_binding_astlpc_start_bmc(struct mctp_binding *b)
@@ -299,38 +323,6 @@
return __mctp_astlpc_kcs_ready(astlpc, status, true);
}
-static int mctp_astlpc_kcs_set_status(struct mctp_binding_astlpc *astlpc,
- uint8_t status)
-{
- uint8_t data;
- int rc;
-
- /* Since we're setting the status register, we want the other endpoint
- * to be interrupted. However, some hardware may only raise a host-side
- * interrupt on an ODR event.
- * So, write a dummy value of 0xff to ODR, which will ensure that an
- * interrupt is triggered, and can be ignored by the host.
- */
- data = 0xff;
- status |= KCS_STATUS_OBF;
-
- rc = astlpc->ops.kcs_write(astlpc->ops_data, MCTP_ASTLPC_KCS_REG_STATUS,
- status);
- if (rc) {
- astlpc_prwarn(astlpc, "KCS status write failed");
- return -1;
- }
-
- rc = astlpc->ops.kcs_write(astlpc->ops_data, MCTP_ASTLPC_KCS_REG_DATA,
- data);
- if (rc) {
- astlpc_prwarn(astlpc, "KCS dummy data write failed");
- return -1;
- }
-
- return 0;
-}
-
static int mctp_astlpc_kcs_send(struct mctp_binding_astlpc *astlpc,
uint8_t data)
{
@@ -454,9 +446,16 @@
updated = astlpc->kcs_status ^ status;
+ astlpc_prdebug(astlpc, "%s: status: 0x%x, update: 0x%x", __func__,
+ status, updated);
+
if (updated & KCS_STATUS_BMC_READY) {
- if (!(status & KCS_STATUS_BMC_READY))
+ if (status & KCS_STATUS_BMC_READY) {
+ astlpc->kcs_status = status;
+ return astlpc->binding.start(&astlpc->binding);
+ } else {
mctp_binding_set_tx_enabled(&astlpc->binding, false);
+ }
}
if (updated & KCS_STATUS_CHANNEL_ACTIVE)
@@ -506,14 +505,23 @@
break;
case 0xff:
/* No responsibilities for the BMC on 0xff */
- if (astlpc->mode == MCTP_BINDING_ASTLPC_MODE_BMC)
- return 0;
-
- return mctp_astlpc_update_channel(astlpc, status);
+ if (astlpc->mode == MCTP_BINDING_ASTLPC_MODE_HOST) {
+ rc = mctp_astlpc_update_channel(astlpc, status);
+ if (rc < 0)
+ return rc;
+ }
+ break;
default:
astlpc_prwarn(astlpc, "unknown message 0x%x", data);
}
- return 0;
+
+ /* Handle silent loss of bmc-ready */
+ if (astlpc->mode == MCTP_BINDING_ASTLPC_MODE_HOST) {
+ if (!(status & KCS_STATUS_BMC_READY && data == 0xff))
+ return mctp_astlpc_update_channel(astlpc, status);
+ }
+
+ return rc;
}
/* allocate and basic initialisation */
@@ -595,6 +603,9 @@
void mctp_astlpc_destroy(struct mctp_binding_astlpc *astlpc)
{
+ /* Clear channel-active and bmc-ready */
+ if (astlpc->mode == MCTP_BINDING_ASTLPC_MODE_BMC)
+ mctp_astlpc_kcs_set_status(astlpc, KCS_STATUS_OBF);
__mctp_free(astlpc);
}
diff --git a/tests/test_astlpc.c b/tests/test_astlpc.c
index 7a2b9d9..faea13c 100644
--- a/tests/test_astlpc.c
+++ b/tests/test_astlpc.c
@@ -199,6 +199,7 @@
/* BMC initialisation */
endpoint_init(&ctx->bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &ctx->kcs,
ctx->lpc_mem);
+ assert(ctx->kcs[KCS_REG_STATUS] & KCS_STATUS_BMC_READY);
/* Host initialisation */
endpoint_init(&ctx->host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &ctx->kcs,
@@ -482,6 +483,56 @@
network_destroy(&ctx);
}
+static void astlpc_test_host_tx_bmc_gone(void)
+{
+ struct astlpc_test ctx = { 0 };
+ uint8_t unwritten[MCTP_BTU];
+ uint8_t msg[MCTP_BTU];
+ int rc;
+
+ /* Test harness initialisation */
+
+ network_init(&ctx);
+
+ memset(&msg[0], 0x5a, sizeof(msg));
+ memset(&unwritten[0], 0, sizeof(unwritten));
+
+ ctx.msg = &msg[0];
+ ctx.count = 0;
+
+ /* Clear bmc-ready */
+ endpoint_destroy(&ctx.bmc);
+
+ /* Host detects that the BMC is disabled */
+ mctp_astlpc_poll(ctx.host.astlpc);
+
+ /* Host attempts to send the single-packet message, but is prevented */
+ rc = mctp_message_tx(ctx.host.mctp, 8, msg, sizeof(msg));
+ assert(rc == 0);
+ assert(!(ctx.kcs[MCTP_ASTLPC_KCS_REG_STATUS] & KCS_STATUS_OBF));
+ astlpc_assert_tx_packet(&ctx.host, &unwritten[0], MCTP_BTU);
+
+ /* BMC comes back */
+ endpoint_init(&ctx.bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &ctx.kcs,
+ ctx.lpc_mem);
+ mctp_set_rx_all(ctx.bmc.mctp, rx_message, &ctx);
+
+ /* Host triggers channel init */
+ mctp_astlpc_poll(ctx.host.astlpc);
+
+ /* BMC handles channel init */
+ mctp_astlpc_poll(ctx.bmc.astlpc);
+
+ /* Host completes channel init, flushing the Tx queue */
+ mctp_astlpc_poll(ctx.host.astlpc);
+
+ /* BMC receives the single-packet message */
+ mctp_astlpc_poll(ctx.bmc.astlpc);
+ assert(ctx.count == 1);
+
+ network_destroy(&ctx);
+}
+
/* clang-format off */
#define TEST_CASE(test) { #test, test }
static const struct {
@@ -493,7 +544,8 @@
TEST_CASE(astlpc_test_simple_message_bmc_to_host),
TEST_CASE(astlpc_test_simple_message_host_to_bmc),
TEST_CASE(astlpc_test_packetised_message_bmc_to_host),
- TEST_CASE(astlpc_test_simple_indirect_message_bmc_to_host)
+ TEST_CASE(astlpc_test_simple_indirect_message_bmc_to_host),
+ TEST_CASE(astlpc_test_host_tx_bmc_gone),
};
/* clang-format on */