uart_buffering: extend CONSOLE_READ command for easy log concatenationstabilize-7134.B

I'm planning to add functionality to the kernel which periodically
dumps the EC console log and concatenates it so we can use it in bug
reports and feedback. However, stitching together logs collected
using the existing SNAPSHOT/READ commands is difficult. This adds a
new version of READ which acts mostly the same but will only give you
the difference between the two most recent snapshots when you pass
the corresponding argument.

BRANCH=ToT
BUG=chromium:492721
TEST=On samus with kernel interface, cat
  /sys/kernel/debug/cros_ec/console_log
  and verify that the most recent bit of the log is printed,
  use spammy commands like 'accelinfo on' and make sure nothing breaks,
  check that 'ectool console' behavior has not changed

Change-Id: Ib8216caa917715820c3e265400f0db2125e8808b
Signed-off-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/273581
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>

2 files changed, 72 insertions, 9 deletions

diff --git a/common/uart_buffering.c b/common/uart_buffering.c
index 42a925ae67..8ff8f1cc12 100644
--- a/common/uart_buffering.c
+++ b/common/uart_buffering.c
@@ -47,6 +47,8 @@ static volatile int rx_buf_head;
 static volatile int rx_buf_tail;
 static int tx_snapshot_head;
 static int tx_snapshot_tail;
+static int tx_last_snapshot_head;
+static int tx_next_snapshot_head;
 static int uart_suspended;
 
 /**
@@ -60,7 +62,7 @@ static int uart_suspended;
  */
 static int __tx_char(void *context, int c)
 {
-	int tx_buf_next;
+	int tx_buf_next, tx_buf_new_tail;
 
 	/* Do newline to CRLF translation */
 	if (c == '\n' && __tx_char(NULL, '\r'))
@@ -70,6 +72,21 @@ static int __tx_char(void *context, int c)
 	if (tx_buf_next == tx_buf_tail)
 		return 1;
 
+	/*
+	 * If we do a READ_RECENT, the buffer may have wrapped around, and
+	 * we'll drop most of the logs in this case. Make sure the place
+	 * we read from in that case is always ahead of the new tx_buf_head.
+	 *
+	 * We also want to make sure that the next time we snapshot and want
+	 * to READ_RECENT, we don't start reading from a stale tail.
+	 */
+	tx_buf_new_tail = TX_BUF_NEXT(tx_buf_next);
+	if (tx_buf_next == tx_last_snapshot_head &&
+	    tx_last_snapshot_head != tx_snapshot_head)
+		tx_last_snapshot_head = tx_buf_new_tail;
+	if (tx_buf_next == tx_next_snapshot_head)
+		tx_next_snapshot_head = tx_buf_new_tail;
+
 	tx_buf[tx_buf_head] = c;
 	tx_buf_head = tx_buf_next;
 	return 0;
@@ -345,6 +362,9 @@ static int host_command_console_snapshot(struct host_cmd_handler_args *args)
 	/* Assume the whole circular buffer is full */
 	tx_snapshot_head = tx_buf_head;
 	tx_snapshot_tail = TX_BUF_NEXT(tx_snapshot_head);
+	/* Set up pointer for just the new part of the buffer */
+	tx_last_snapshot_head = tx_next_snapshot_head;
+	tx_next_snapshot_head = tx_buf_head;
 
 	/*
 	 * Immediately skip any unused bytes.  This doesn't always work,
@@ -368,28 +388,33 @@ DECLARE_HOST_COMMAND(EC_CMD_CONSOLE_SNAPSHOT,
 		     host_command_console_snapshot,
 		     EC_VER_MASK(0));
 
-static int host_command_console_read(struct host_cmd_handler_args *args)
+/*
+ * Common code for host_command_console_read and
+ * host_command_console_read_recent.
+ */
+static int console_read_helper(struct host_cmd_handler_args *args,
+			       int *tail)
 {
 	char *dest = (char *)args->response;
 
 	/* If no snapshot data, return empty response */
-	if (tx_snapshot_head == tx_snapshot_tail)
+	if (tx_snapshot_head == *tail)
 		return EC_RES_SUCCESS;
 
 	/* Copy data to response */
-	while (tx_snapshot_tail != tx_snapshot_head &&
+	while (*tail != tx_snapshot_head &&
 	       args->response_size < args->response_max - 1) {
 
 		/*
 		 * Copy only non-zero bytes, so that we don't copy unused
 		 * bytes if the buffer hasn't completely rolled at boot.
 		 */
-		if (tx_buf[tx_snapshot_tail]) {
-			*(dest++) = tx_buf[tx_snapshot_tail];
+		if (tx_buf[*tail]) {
+			*(dest++) = tx_buf[*tail];
 			args->response_size++;
 		}
 
-		tx_snapshot_tail = TX_BUF_NEXT(tx_snapshot_tail);
+		*tail = TX_BUF_NEXT(*tail);
 	}
 
 	/* Null-terminate */
@@ -398,6 +423,29 @@ static int host_command_console_read(struct host_cmd_handler_args *args)
 
 	return EC_RES_SUCCESS;
 }
+
+static int host_command_console_read(struct host_cmd_handler_args *args)
+{
+	const struct ec_params_console_read_v1 *p;
+
+	if (args->version == 0) {
+		/*
+		 * Prior versions of this command only support reading from
+		 * an entire snapshot, not just the output since the last
+		 * snapshot.
+		 */
+		return console_read_helper(args, &tx_snapshot_tail);
+	} else if (args->version == 1) {
+		/* Check the params to figure out where to start reading. */
+		p = args->params;
+		if (p->subcmd == CONSOLE_READ_NEXT)
+			return console_read_helper(args, &tx_snapshot_tail);
+		else if (p->subcmd == CONSOLE_READ_RECENT)
+			return console_read_helper(args,
+						   &tx_last_snapshot_head);
+	}
+	return EC_RES_INVALID_PARAM;
+}
 DECLARE_HOST_COMMAND(EC_CMD_CONSOLE_READ,
 		     host_command_console_read,
-		     EC_VER_MASK(0));
+		     EC_VER_MASK(0) | EC_VER_MASK(1));
diff --git a/include/ec_commands.h b/include/ec_commands.h
index c1b5637af6..fbae48cb49 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -2409,13 +2409,28 @@ struct ec_params_charge_control {
 #define EC_CMD_CONSOLE_SNAPSHOT 0x97
 
 /*
- * Read next chunk of data from saved snapshot.
+ * Read data from the saved snapshot. If the subcmd parameter is
+ * CONSOLE_READ_NEXT, this will return data starting from the beginning of
+ * the latest snapshot. If it is CONSOLE_READ_RECENT, it will start from the
+ * end of the previous snapshot.
+ *
+ * The params are only looked at in version >= 1 of this command. Prior
+ * versions will just default to CONSOLE_READ_NEXT behavior.
  *
  * Response is null-terminated string.  Empty string, if there is no more
  * remaining output.
  */
 #define EC_CMD_CONSOLE_READ 0x98
 
+enum ec_console_read_subcmd {
+	CONSOLE_READ_NEXT = 0,
+	CONSOLE_READ_RECENT
+};
+
+struct ec_params_console_read_v1 {
+	uint8_t subcmd; /* enum ec_console_read_subcmd */
+} __packed;
+
 /*****************************************************************************/
 
 /*