mbox: Clarify sequence number constraints
And implement the specified behaviour.
Change-Id: I268d5896aa8dda3875cd79f4ff18929c8e3aea49
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/Documentation/mbox_protocol.md b/Documentation/mbox_protocol.md
index 8a32511..a8be0f0 100644
--- a/Documentation/mbox_protocol.md
+++ b/Documentation/mbox_protocol.md
@@ -153,15 +153,14 @@
cases for a back channel for the BMC to pass new information to the
host which will be discussed later.
-To initiate a request the host must set a command code (see
-Commands) into mailbox data register 0. It is also the hosts
-responsibility to generate a unique sequence number into mailbox
-register 1. After this any command specific data should be written
-(see Layout). The host must then generate an interrupt to the BMC by
-using bit 0 of its control register and wait for an interrupt on the
-response register. Generating an interrupt automatically sets bit 7 of the
-corresponding control register. This bit can be used to poll for
-messages.
+To initiate a request the host must set a command code (see Commands) into
+mailbox data register 0, and generate a sequence number (see Sequence Numbers)
+to write to mailbox register data 1. After these two values, any
+command-specific data should be written (see Layout). The host must then
+generate an interrupt to the BMC by using bit 0 of its control register and
+wait for an interrupt on the response register. Generating an interrupt
+automatically sets bit 7 of the corresponding control register. This bit can be
+used to poll for messages.
On receiving an interrupt (or polling on bit 7 of its Control
Register) the BMC should read the message from the general registers
@@ -312,13 +311,6 @@
MARK_WRITE_ERASED 0x0a (V2)
```
-### Sequence
-
-The host must ensure a unique sequence number at the start of a
-command/response pair. The BMC must ensure the responses to
-a particular message contain the same sequence number that was in the
-command request from the host.
-
### Responses
```
@@ -329,8 +321,28 @@
TIMEOUT 5
BUSY 6 (V2)
WINDOW_ERROR 7 (V2)
+SEQ_ERROR 8 (V2)
```
+### Sequence Numbers
+
+Sequence numbers are included in messages for correlation of commands and
+responses. V1 and V2 of the protocol permit either zero or one commands to be
+in progress (yet to receive a response).
+
+For generality, the host must generate a sequence number that is unique with
+respect to the previous command (one that has received a response) and any
+in-progress commands. Sequence numbers meeting this requirement are considered
+valid. The BMC's response to a command must contain the same sequence number
+issued by the host as found in the relevant command.
+
+Sequence numbers may be reused in accordance with the constraints outlined
+above, however it is not an error if the BMC receives a `GET_MBOX_INFO` with an
+invalid sequence number. For all other cases, the BMC must respond with
+`SEQ_ERROR` if the constraints are violated. If the host receives a `SEQ_ERROR`
+response it must consider any in-progress commands to have failed. The host may
+retry the affected command(s) after generating a suitable sequence number.
+
#### Description:
SUCCESS - Command completed successfully
diff --git a/mbox.h b/mbox.h
index d9a8d03..087975d 100644
--- a/mbox.h
+++ b/mbox.h
@@ -54,6 +54,7 @@
#define MBOX_R_TIMEOUT 0x05
#define MBOX_R_BUSY 0x06
#define MBOX_R_WINDOW_ERROR 0x07
+#define MBOX_R_SEQ_ERROR 0x08
/* Argument Flags */
#define FLAGS_NONE 0x00
@@ -137,6 +138,7 @@
sd_bus *bus;
bool terminate;
uint8_t bmc_events;
+ uint8_t prev_seq;
/* Window State */
/* The window list struct containing all current "windows" */
diff --git a/mboxd_msg.c b/mboxd_msg.c
index 9f8aba1..a5bcf62 100644
--- a/mboxd_msg.c
+++ b/mboxd_msg.c
@@ -650,19 +650,27 @@
}
/*
- * check_cmd_valid() - Check if the given command is a valid mbox command code
+ * check_req_valid() - Check if the given request is a valid mbox request
* @context: The mbox context pointer
- * @cmd: The command code
+ * @cmd: The request registers
*
- * Return: 0 if command is valid otherwise negative error code
+ * Return: 0 if request is valid otherwise negative error code
*/
-static int check_cmd_valid(struct mbox_context *context, int cmd)
+static int check_req_valid(struct mbox_context *context, union mbox_regs *req)
{
- if (cmd <= 0 || cmd > NUM_MBOX_CMDS) {
+ uint8_t cmd = req->msg.command;
+ uint8_t seq = req->msg.seq;
+
+ if (cmd > NUM_MBOX_CMDS) {
MSG_ERR("Unknown mbox command: %d\n", cmd);
return -MBOX_R_PARAM_ERROR;
}
+ if (seq == context->prev_seq && cmd != MBOX_C_GET_MBOX_INFO) {
+ MSG_ERR("Invalid sequence number: %d\n", seq);
+ return -MBOX_R_SEQ_ERROR;
+ }
+
if (context->state & STATE_SUSPENDED) {
if (cmd != MBOX_C_GET_MBOX_INFO && cmd != MBOX_C_ACK) {
MSG_ERR("Cannot use that cmd while suspended: %d\n",
@@ -714,7 +722,7 @@
int rc = 0, len;
MSG_OUT("Got data in with command %d\n", req->msg.command);
- rc = check_cmd_valid(context, req->msg.command);
+ rc = check_req_valid(context, req);
if (rc < 0) {
resp.response = -rc;
} else {
@@ -727,6 +735,8 @@
}
}
+ context->prev_seq = req->msg.seq;
+
MSG_OUT("Writing response to MBOX regs: %d\n", resp.response);
len = write(context->fds[MBOX_FD].fd, &resp, sizeof(resp));
if (len < sizeof(resp)) {