astlpc: Implement version negotiation
Binding version negotiation was previously left as a todo. With the
upcoming efforts to introduce MTU negotiation we need to repurpose some
of the fields in the control structure (in a backwards-compatible way),
so make sure we can first negotiate the protocol version before
proceeding to change the semantics of the fields.
Change-Id: Ibd2283987b8b60b3ab7ada667e2b507b4ac09034
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/astlpc.c b/astlpc.c
index 6abb123..97ea0d2 100644
--- a/astlpc.c
+++ b/astlpc.c
@@ -55,6 +55,7 @@
struct mctp_astlpc_layout layout;
uint8_t mode;
+ uint16_t version;
/* direct ops data */
struct mctp_binding_astlpc_ops ops;
@@ -88,6 +89,7 @@
/* clang-format off */
#define ASTLPC_MCTP_MAGIC 0x4d435450
+#define ASTLPC_VER_BAD 0
#define ASTLPC_VER_MIN 1
#define ASTLPC_VER_CUR 1
@@ -250,10 +252,44 @@
return mctp_astlpc_init_bmc(astlpc);
}
+static bool mctp_astlpc_validate_version(uint16_t bmc_ver_min,
+ uint16_t bmc_ver_cur,
+ uint16_t host_ver_min,
+ uint16_t host_ver_cur)
+{
+ if (!(bmc_ver_min && bmc_ver_cur && host_ver_min && host_ver_cur)) {
+ mctp_prerr("Invalid version present in [%" PRIu16 ", %" PRIu16
+ "], [%" PRIu16 ", %" PRIu16 "]",
+ bmc_ver_min, bmc_ver_cur, host_ver_min,
+ host_ver_cur);
+ return false;
+ } else if (bmc_ver_min > bmc_ver_cur) {
+ mctp_prerr("Invalid bmc version range [%" PRIu16 ", %" PRIu16
+ "]",
+ bmc_ver_min, bmc_ver_cur);
+ return false;
+ } else if (host_ver_min > host_ver_cur) {
+ mctp_prerr("Invalid host version range [%" PRIu16 ", %" PRIu16
+ "]",
+ host_ver_min, host_ver_cur);
+ return false;
+ } else if ((host_ver_cur < bmc_ver_min) ||
+ (host_ver_min > bmc_ver_cur)) {
+ mctp_prerr(
+ "Unable to satisfy version negotiation with ranges [%" PRIu16
+ ", %" PRIu16 "] and [%" PRIu16 ", %" PRIu16 "]",
+ bmc_ver_min, bmc_ver_cur, host_ver_min, host_ver_cur);
+ return false;
+ }
+
+ return true;
+}
+
static int mctp_astlpc_init_host(struct mctp_binding_astlpc *astlpc)
{
const uint16_t ver_min_be = htobe16(ASTLPC_VER_MIN);
const uint16_t ver_cur_be = htobe16(ASTLPC_VER_CUR);
+ uint16_t bmc_ver_min, bmc_ver_cur;
struct mctp_lpcmap_hdr hdr;
uint8_t status;
int rc;
@@ -276,6 +312,15 @@
astlpc->layout.tx.offset = be32toh(hdr.tx_offset);
astlpc->layout.tx.size = be32toh(hdr.tx_size);
+ bmc_ver_min = be16toh(hdr.bmc_ver_min);
+ bmc_ver_cur = be16toh(hdr.bmc_ver_cur);
+
+ if (!mctp_astlpc_validate_version(bmc_ver_min, bmc_ver_cur,
+ ASTLPC_VER_MIN, ASTLPC_VER_CUR)) {
+ astlpc_prerr(astlpc, "Cannot negotiate with invalid versions");
+ return -EINVAL;
+ }
+
mctp_astlpc_lpc_write(astlpc, &ver_min_be,
offsetof(struct mctp_lpcmap_hdr, host_ver_min),
sizeof(ver_min_be));
@@ -385,19 +430,56 @@
return 0;
}
+static uint16_t mctp_astlpc_negotiate_version(uint16_t bmc_ver_min,
+ uint16_t bmc_ver_cur,
+ uint16_t host_ver_min,
+ uint16_t host_ver_cur)
+{
+ if (!mctp_astlpc_validate_version(bmc_ver_min, bmc_ver_cur,
+ host_ver_min, host_ver_cur))
+ return ASTLPC_VER_BAD;
+
+ if (bmc_ver_cur < host_ver_cur)
+ return bmc_ver_cur;
+
+ return host_ver_cur;
+}
+
static void mctp_astlpc_init_channel(struct mctp_binding_astlpc *astlpc)
{
- /* todo: actual version negotiation */
- uint16_t negotiated = htobe16(1);
- mctp_astlpc_lpc_write(astlpc, &negotiated,
+ uint16_t negotiated, negotiated_be;
+ struct mctp_lpcmap_hdr hdr;
+ uint8_t status;
+
+ mctp_astlpc_lpc_read(astlpc, &hdr, 0, sizeof(hdr));
+
+ /* Version negotiation */
+ negotiated =
+ mctp_astlpc_negotiate_version(ASTLPC_VER_MIN, ASTLPC_VER_CUR,
+ be16toh(hdr.host_ver_min),
+ be16toh(hdr.host_ver_cur));
+
+ /* Populate the negotiated version */
+ astlpc->version = negotiated;
+ negotiated_be = htobe16(negotiated);
+ mctp_astlpc_lpc_write(astlpc, &negotiated_be,
offsetof(struct mctp_lpcmap_hdr, negotiated_ver),
- sizeof(negotiated));
+ sizeof(negotiated_be));
- mctp_astlpc_kcs_set_status(astlpc, KCS_STATUS_BMC_READY |
- KCS_STATUS_CHANNEL_ACTIVE |
- KCS_STATUS_OBF);
+ /* Finalise the configuration */
+ status = KCS_STATUS_BMC_READY | KCS_STATUS_OBF;
+ if (negotiated > 0) {
+ astlpc_prinfo(astlpc, "Negotiated binding version %" PRIu16,
+ negotiated);
+ status |= KCS_STATUS_CHANNEL_ACTIVE;
+ } else {
+ astlpc_prerr(astlpc, "Failed to initialise channel\n");
+ }
- mctp_binding_set_tx_enabled(&astlpc->binding, true);
+ mctp_astlpc_kcs_set_status(astlpc, status);
+
+ mctp_binding_set_tx_enabled(&astlpc->binding,
+ status & KCS_STATUS_CHANNEL_ACTIVE);
}
static void mctp_astlpc_rx_start(struct mctp_binding_astlpc *astlpc)
@@ -439,6 +521,32 @@
mctp_binding_set_tx_enabled(&astlpc->binding, true);
}
+static int mctp_astlpc_finalise_channel(struct mctp_binding_astlpc *astlpc)
+{
+ uint16_t negotiated;
+ int rc;
+
+ rc = mctp_astlpc_lpc_read(astlpc, &negotiated,
+ offsetof(struct mctp_lpcmap_hdr,
+ negotiated_ver),
+ sizeof(negotiated));
+ if (rc < 0)
+ return rc;
+
+ negotiated = be16toh(negotiated);
+
+ if (negotiated == ASTLPC_VER_BAD || negotiated < ASTLPC_VER_MIN ||
+ negotiated > ASTLPC_VER_CUR) {
+ astlpc_prerr(astlpc, "Failed to negotiate version, got: %u\n",
+ negotiated);
+ return -EINVAL;
+ }
+
+ astlpc->version = negotiated;
+
+ return 0;
+}
+
static int mctp_astlpc_update_channel(struct mctp_binding_astlpc *astlpc,
uint8_t status)
{
@@ -461,9 +569,14 @@
}
}
- if (updated & KCS_STATUS_CHANNEL_ACTIVE)
- mctp_binding_set_tx_enabled(&astlpc->binding,
- status & KCS_STATUS_CHANNEL_ACTIVE);
+ if (astlpc->version == 0 || updated & KCS_STATUS_CHANNEL_ACTIVE) {
+ bool enable;
+
+ rc = mctp_astlpc_finalise_channel(astlpc);
+ enable = (status & KCS_STATUS_CHANNEL_ACTIVE) && rc == 0;
+
+ mctp_binding_set_tx_enabled(&astlpc->binding, enable);
+ }
astlpc->kcs_status = status;
diff --git a/tests/test_astlpc.c b/tests/test_astlpc.c
index d1a7ab4..332e949 100644
--- a/tests/test_astlpc.c
+++ b/tests/test_astlpc.c
@@ -163,8 +163,8 @@
.lpc_write = mctp_astlpc_mmio_lpc_write,
};
-static void endpoint_init(struct astlpc_endpoint *ep, mctp_eid_t eid,
- uint8_t mode, uint8_t (*kcs)[2], void *lpc_mem)
+static int endpoint_init(struct astlpc_endpoint *ep, mctp_eid_t eid,
+ uint8_t mode, uint8_t (*kcs)[2], void *lpc_mem)
{
/*
* Configure the direction of the KCS interface so we know whether to
@@ -182,7 +182,7 @@
ep->astlpc = mctp_astlpc_init(mode, MCTP_BTU, lpc_mem,
&astlpc_direct_mmio_ops, &ep->mmio);
- mctp_register_bus(ep->mctp, &ep->astlpc->binding, eid);
+ return mctp_register_bus(ep->mctp, &ep->astlpc->binding, eid);
}
static void endpoint_destroy(struct astlpc_endpoint *ep)
@@ -193,17 +193,21 @@
static void network_init(struct astlpc_test *ctx)
{
+ int rc;
+
ctx->lpc_mem = calloc(1, 1 * 1024 * 1024);
assert(ctx->lpc_mem);
/* BMC initialisation */
- endpoint_init(&ctx->bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &ctx->kcs,
- ctx->lpc_mem);
+ rc = endpoint_init(&ctx->bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC,
+ &ctx->kcs, ctx->lpc_mem);
+ assert(!rc);
assert(ctx->kcs[MCTP_ASTLPC_KCS_REG_STATUS] & KCS_STATUS_BMC_READY);
/* Host initialisation */
- endpoint_init(&ctx->host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &ctx->kcs,
- ctx->lpc_mem);
+ rc = endpoint_init(&ctx->host, 9, MCTP_BINDING_ASTLPC_MODE_HOST,
+ &ctx->kcs, ctx->lpc_mem);
+ assert(!rc);
/* BMC processes host channel init request, alerts host */
mctp_astlpc_poll(ctx->bmc.astlpc);
@@ -379,24 +383,141 @@
mctp_destroy(mctp);
}
-static void astlpc_test_simple_init(void)
+static void astlpc_test_bad_version(void)
+{
+ assert(0 ==
+ mctp_astlpc_negotiate_version(ASTLPC_VER_BAD, ASTLPC_VER_CUR,
+ ASTLPC_VER_MIN, ASTLPC_VER_CUR));
+ assert(0 ==
+ mctp_astlpc_negotiate_version(ASTLPC_VER_MIN, ASTLPC_VER_BAD,
+ ASTLPC_VER_MIN, ASTLPC_VER_CUR));
+ assert(0 ==
+ mctp_astlpc_negotiate_version(ASTLPC_VER_MIN, ASTLPC_VER_CUR,
+ ASTLPC_VER_BAD, ASTLPC_VER_CUR));
+ assert(0 ==
+ mctp_astlpc_negotiate_version(ASTLPC_VER_MIN, ASTLPC_VER_CUR,
+ ASTLPC_VER_MIN, ASTLPC_VER_BAD));
+ assert(0 == mctp_astlpc_negotiate_version(
+ ASTLPC_VER_CUR + 1, ASTLPC_VER_CUR, ASTLPC_VER_MIN,
+ ASTLPC_VER_CUR + 1));
+ assert(0 == mctp_astlpc_negotiate_version(
+ ASTLPC_VER_MIN, ASTLPC_VER_CUR + 1,
+ ASTLPC_VER_CUR + 1, ASTLPC_VER_CUR));
+}
+
+static void astlpc_test_incompatible_versions(void)
+{
+ assert(0 == mctp_astlpc_negotiate_version(
+ ASTLPC_VER_CUR, ASTLPC_VER_CUR, ASTLPC_VER_CUR + 1,
+ ASTLPC_VER_CUR + 1));
+ assert(0 == mctp_astlpc_negotiate_version(
+ ASTLPC_VER_CUR + 1, ASTLPC_VER_CUR + 1,
+ ASTLPC_VER_CUR, ASTLPC_VER_CUR));
+}
+
+static void astlpc_test_choose_bmc_ver_cur(void)
+{
+ assert(2 == mctp_astlpc_negotiate_version(1, 2, 2, 3));
+}
+
+static void astlpc_test_choose_host_ver_cur(void)
+{
+ assert(2 == mctp_astlpc_negotiate_version(2, 3, 1, 2));
+}
+
+static void astlpc_test_version_host_fails_negotiation(void)
{
struct astlpc_endpoint bmc, host;
+ struct mctp_lpcmap_hdr *hdr;
uint8_t kcs[2] = { 0 };
void *lpc_mem;
+ int rc;
/* Test harness initialisation */
lpc_mem = calloc(1, 1 * 1024 * 1024);
assert(lpc_mem);
/* BMC initialisation */
- endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs, lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
+ lpc_mem);
+ assert(!rc);
+
+ /* Now the BMC is initialised, break its version announcement */
+ hdr = lpc_mem;
+ hdr->bmc_ver_cur = ASTLPC_VER_BAD;
+
+ /* Host initialisation */
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs,
+ lpc_mem);
+ assert(rc < 0);
+
+ endpoint_destroy(&bmc);
+ endpoint_destroy(&host);
+ free(lpc_mem);
+}
+
+static void astlpc_test_version_bmc_fails_negotiation(void)
+{
+ struct astlpc_endpoint bmc, host;
+ struct mctp_lpcmap_hdr *hdr;
+ uint8_t kcs[2] = { 0 };
+ void *lpc_mem;
+ int rc;
+
+ /* Test harness initialisation */
+ lpc_mem = calloc(1, 1 * 1024 * 1024);
+ assert(lpc_mem);
+
+ /* BMC initialisation */
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
+ lpc_mem);
+ assert(!rc);
+
+ /* Host initialisation */
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs,
+ lpc_mem);
+ assert(!rc);
+
+ /* Now the host is initialised, break its version announcement */
+ hdr = lpc_mem;
+ hdr->host_ver_cur = ASTLPC_VER_BAD;
+
+ /* Poll the BMC to detect the broken host version */
+ mctp_astlpc_poll(bmc.astlpc);
+ assert(!(kcs[MCTP_ASTLPC_KCS_REG_STATUS] & KCS_STATUS_CHANNEL_ACTIVE));
+
+ /* Poll the host so it detects failed negotiation */
+ rc = mctp_astlpc_poll(host.astlpc);
+ assert(rc < 0);
+
+ endpoint_destroy(&bmc);
+ endpoint_destroy(&host);
+ free(lpc_mem);
+}
+
+static void astlpc_test_simple_init(void)
+{
+ struct astlpc_endpoint bmc, host;
+ uint8_t kcs[2] = { 0 };
+ void *lpc_mem;
+ int rc;
+
+ /* Test harness initialisation */
+ lpc_mem = calloc(1, 1 * 1024 * 1024);
+ assert(lpc_mem);
+
+ /* BMC initialisation */
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
+ lpc_mem);
+ assert(!rc);
/* Verify the BMC binding was initialised */
assert(kcs[MCTP_ASTLPC_KCS_REG_STATUS] & KCS_STATUS_BMC_READY);
/* Host initialisation */
- endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs, lpc_mem);
+ rc = endpoint_init(&host, 9, MCTP_BINDING_ASTLPC_MODE_HOST, &kcs,
+ lpc_mem);
+ assert(!rc);
/* Host sends channel init command */
assert(kcs[MCTP_ASTLPC_KCS_REG_STATUS] & KCS_STATUS_IBF);
@@ -514,8 +635,9 @@
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);
+ rc = endpoint_init(&ctx.bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &ctx.kcs,
+ ctx.lpc_mem);
+ assert(!rc);
mctp_set_rx_all(ctx.bmc.mctp, rx_message, &ctx);
/* Host triggers channel init */
@@ -546,7 +668,9 @@
assert(lpc_mem);
/* BMC initialisation */
- endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs, lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
+ lpc_mem);
+ assert(!rc);
/* Check for a command despite none present */
rc = mctp_astlpc_poll(bmc.astlpc);
@@ -570,7 +694,9 @@
assert(lpc_mem);
/* BMC initialisation */
- endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs, lpc_mem);
+ rc = endpoint_init(&bmc, 8, MCTP_BINDING_ASTLPC_MODE_BMC, &kcs,
+ lpc_mem);
+ assert(!rc);
/* 0x5a isn't legal in v1 or v2 */
kcs[MCTP_ASTLPC_KCS_REG_DATA] = 0x5a;
@@ -593,6 +719,12 @@
void (*test)(void);
} astlpc_tests[] = {
TEST_CASE(astlpc_test_simple_init),
+ TEST_CASE(astlpc_test_bad_version),
+ TEST_CASE(astlpc_test_incompatible_versions),
+ TEST_CASE(astlpc_test_choose_bmc_ver_cur),
+ TEST_CASE(astlpc_test_choose_host_ver_cur),
+ TEST_CASE(astlpc_test_version_host_fails_negotiation),
+ TEST_CASE(astlpc_test_version_bmc_fails_negotiation),
TEST_CASE(astlpc_test_host_before_bmc),
TEST_CASE(astlpc_test_simple_message_bmc_to_host),
TEST_CASE(astlpc_test_simple_message_host_to_bmc),