cr50: pass params to vendor commands as struct

This makes it easier to add params or flags for vendor commands
without changing all of the command handlers.  It also reduces code
size by 56 bytes.

For now, existing command handlers continue to use
DECLARE_VENDOR_COMMAND().  Added DECLARE_VENDOR_COMMAND_P() for
handlers which take the params struct directly.  The CCD command will
be the first user of that, since it will have different rules for
'open' based on where the command comes from.

No change to existing command behavior.

BUG=b:79983505
BRANCH=cr50
TEST=gsctool -I still works

Change-Id: I7ed288a9c45e381162e246b50ae88cf76e67490d
Signed-off-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1069538
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

4 files changed, 125 insertions, 124 deletions

diff --git a/chip/g/usb_upgrade.c b/chip/g/usb_upgrade.c
index 3c3982af05..d6fdf80488 100644
--- a/chip/g/usb_upgrade.c
+++ b/chip/g/usb_upgrade.c
@@ -107,39 +107,17 @@ static int valid_transfer_start(struct consumer const *consumer, size_t count,
 	return 1;
 }
 
-static void usb_extension_route_command(uint16_t command_code,
-					uint8_t *buffer,
-					size_t in_size,
-					size_t *out_size)
-{
-	uint32_t rv;
-	size_t buf_size;
-
-	/*
-	 * The return code normally put into the TPM response header
-	 * is not present in the USB response. Vendor command return
-	 * code is guaranteed to fit in a byte. Let's keep space for
-	 * it in the front of the buffer.
-	 */
-	buf_size = *out_size - 1;
-	rv = extension_route_command(command_code, buffer, in_size, &buf_size,
-				     VENDOR_CMD_FROM_USB);
-	/*
-	 * Copy actual response, if any, one byte up, to free room for
-	 * the return code.
-	 */
-	if (buf_size)
-		memmove(buffer + 1, buffer, buf_size);
-	*out_size = buf_size + 1;
-
-	buffer[0] = rv;  /* We care about LSB only. */
-}
-
 static int try_vendor_command(struct consumer const *consumer, size_t count)
 {
 	struct update_frame_header ufh;
 	struct update_frame_header *cmd_buffer;
-	int rv = 0;
+	uint16_t *subcommand;
+	size_t request_size;
+	/*
+	 * Should be enough for any vendor command/response. We'll generate an
+	 * error if it is not.
+	 */
+	uint8_t subcommand_body[32];
 
 	if (count < sizeof(ufh))
 		return 0;	/* Too short to be a valid vendor command. */
@@ -169,47 +147,58 @@ static int try_vendor_command(struct consumer const *consumer, size_t count)
 	queue_peek_units(consumer->queue, cmd_buffer, 0, count);
 
 	/* Looks like this is a vendor command, let's verify it. */
-	if (usb_pdu_valid(&cmd_buffer->cmd,
+	if (!usb_pdu_valid(&cmd_buffer->cmd,
 			  count - offsetof(struct update_frame_header, cmd))) {
-		uint16_t *subcommand;
-		size_t response_size;
-		size_t request_size;
+		/* Didn't verify */
+		shared_mem_release(cmd_buffer);
+		return 0;
+	}
+
+	/* Looks good; remove from the queue and process it. */
+	queue_advance_head(consumer->queue, count);
+
+	subcommand = (uint16_t *)(cmd_buffer + 1);
+	request_size = count - sizeof(struct update_frame_header) -
+		       sizeof(*subcommand);
+
+	if (request_size > sizeof(subcommand_body)) {
+		const uint8_t err = VENDOR_RC_REQUEST_TOO_BIG;
+
+		CPRINTS("%s: payload too big (%d)", __func__, request_size);
+		QUEUE_ADD_UNITS(&upgrade_to_usb, &err, 1);
+	} else {
+		uint32_t rv;
+		struct vendor_cmd_params p = {
+			.code = be16toh(*subcommand),
+			.buffer = subcommand_body,
+			.in_size = request_size,
+			/*
+			 * The return code normally put into the TPM response
+			 * header is not present in the USB response. Vendor
+			 * command return code is guaranteed to fit in a
+			 * byte. Let's keep space for it in the front of the
+			 * buffer.
+			 */
+			.out_size = sizeof(subcommand_body) - 1,
+			.flags = VENDOR_CMD_FROM_USB
+		};
+		memcpy(subcommand_body, subcommand + 1, request_size);
+		rv = extension_route_command(&p);
 		/*
-		 * Should be enough for any vendor command/response. We'll
-		 * generate an error if it is not.
+		 * Copy actual response, if any, one byte up, to free room for
+		 * the return code.
 		 */
-		uint8_t subcommand_body[32];
-
-		/* looks good, let's process it. */
-		rv = 1;
-
-		/* Now remove if from the queue. */
-		queue_advance_head(consumer->queue, count);
-
-		subcommand = (uint16_t *)(cmd_buffer + 1);
-		request_size = count - sizeof(struct update_frame_header) -
-			sizeof(*subcommand);
-
-		if (request_size > sizeof(subcommand_body)) {
-			CPRINTS("%s: vendor command payload too big (%d)",
-				__func__, request_size);
-			subcommand_body[0] = VENDOR_RC_REQUEST_TOO_BIG;
-			response_size = 1;
-		} else {
-			memcpy(subcommand_body, subcommand + 1, request_size);
-			response_size = sizeof(subcommand_body);
-			usb_extension_route_command(be16toh(*subcommand),
-						    subcommand_body,
-						    request_size,
-						    &response_size);
-		}
+		if (p.out_size)
+			memmove(subcommand_body + 1, subcommand_body,
+				p.out_size);
+		subcommand_body[0] = rv;  /* We care about LSB only. */
 
-		QUEUE_ADD_UNITS(&upgrade_to_usb,
-				subcommand_body, response_size);
+		QUEUE_ADD_UNITS(&upgrade_to_usb, subcommand_body,
+				p.out_size + 1);
 	}
-	shared_mem_release(cmd_buffer);
 
-	return rv;
+	shared_mem_release(cmd_buffer);
+	return 1;
 }
 
 /*
diff --git a/common/extension.c b/common/extension.c
index d75bbbe4f3..fecb61ec9e 100644
--- a/common/extension.c
+++ b/common/extension.c
@@ -11,11 +11,7 @@
 
 #define CPRINTS(format, args...) cprints(CC_EXTENSION, format, ## args)
 
-uint32_t extension_route_command(uint16_t command_code,
-				 void *buffer,
-				 size_t in_size,
-				 size_t *out_size,
-				 uint32_t flags)
+uint32_t extension_route_command(struct vendor_cmd_params *p)
 {
 	struct extension_command *cmd_p;
 	struct extension_command *end_p;
@@ -27,8 +23,8 @@ uint32_t extension_route_command(uint16_t command_code,
 #endif
 
 	/* Filter commands from USB */
-	if (flags & VENDOR_CMD_FROM_USB) {
-		switch (command_code) {
+	if (p->flags & VENDOR_CMD_FROM_USB) {
+		switch (p->code) {
 #ifdef CR50_DEV
 		case VENDOR_CC_IMMEDIATE_RESET:
 		case VENDOR_CC_INVALIDATE_INACTIVE_RW:
@@ -54,7 +50,7 @@ uint32_t extension_route_command(uint16_t command_code,
 	 * Cr50 firmware.
 	 */
 	if (board_id_is_mismatched()) {
-		switch (command_code) {
+		switch (p->code) {
 		case EXTENSION_FW_UPGRADE:
 		case VENDOR_CC_REPORT_TPM_STATE:
 		case VENDOR_CC_TURN_UPDATE_ON:
@@ -71,15 +67,14 @@ uint32_t extension_route_command(uint16_t command_code,
 	cmd_p = (struct extension_command *)&__extension_cmds;
 	end_p = (struct extension_command *)&__extension_cmds_end;
 	while (cmd_p != end_p) {
-		if (cmd_p->command_code == command_code)
-			return cmd_p->handler(command_code, buffer,
-					      in_size, out_size);
+		if (cmd_p->command_code == p->code)
+			return cmd_p->handler(p);
 		cmd_p++;
 	}
 
 ignore_cmd:
 	/* Command not found or not allowed */
-	CPRINTS("%s: ignore %d: %s", __func__, command_code, why_ignore);
-	*out_size = 0;
+	CPRINTS("%s: ignore %d: %s", __func__, p->code, why_ignore);
+	p->out_size = 0;
 	return VENDOR_RC_NO_SUCH_COMMAND;
 }
diff --git a/common/tpm_registers.c b/common/tpm_registers.c
index a8a3546109..215905efa9 100644
--- a/common/tpm_registers.c
+++ b/common/tpm_registers.c
@@ -643,21 +643,21 @@ static void call_extension_command(struct tpm_cmd_header *tpmh,
 
 	/* Verify there is room for at least the extension command header. */
 	if (command_size >= sizeof(struct tpm_cmd_header)) {
-		uint16_t subcommand_code;
-
-		/* The header takes room in the buffer. */
-		*total_size -= sizeof(struct tpm_cmd_header);
-
-		subcommand_code = be16toh(tpmh->subcommand_code);
-		rc = extension_route_command(subcommand_code,
-					     tpmh + 1,
-					     command_size -
-					     sizeof(struct tpm_cmd_header),
-					     total_size,
-					     flags);
+		struct vendor_cmd_params p = {
+			.code = be16toh(tpmh->subcommand_code),
+			/* The header takes room in the buffer. */
+			.buffer = tpmh + 1,
+			.in_size = command_size - sizeof(struct tpm_cmd_header),
+			.out_size = *total_size - sizeof(struct tpm_cmd_header),
+			.flags = flags
+		};
+
+		rc = extension_route_command(&p);
+
 		/* Add the header size back. */
-		*total_size += sizeof(struct tpm_cmd_header);
+		*total_size = p.out_size + sizeof(struct tpm_cmd_header);
 		tpmh->size = htobe32(*total_size);
+
 		/* Flag errors from commands as vendor-specific */
 		if (rc)
 			rc |= VENDOR_RC_ERR;
diff --git a/include/extension.h b/include/extension.h
index bec6bf3ffd..7ef43bc2a3 100644
--- a/include/extension.h
+++ b/include/extension.h
@@ -22,19 +22,31 @@ enum vendor_cmd_flags {
 	VENDOR_CMD_FROM_USB = (1 << 0),
 };
 
-/*
- * Type of function handling extension commands.
- *
- * @param buffer        As input points to the input data to be processed, as
- *                      output stores data, processing result.
- * @param command_size  Number of bytes of input data
- * @param response_size On input - max size of the buffer, on output - actual
- *                      number of data returned by the handler.
- */
-typedef enum vendor_cmd_rc (*extension_handler)(enum vendor_cmd_cc code,
-						void *buffer,
-						size_t command_size,
-						size_t *response_size);
+/* Parameters for vendor commands */
+struct vendor_cmd_params {
+	/* Command code */
+	enum vendor_cmd_cc code;
+
+	/* On input, data to be processed.  On output, response data. */
+	void *buffer;
+
+	/* Number of bytes of input data */
+	size_t in_size;
+
+	/*
+	 * On input, size of output buffer.  On output, actual response size.
+	 * Both in bytes.  A single response byte usually indicates an error
+	 * and contains the error code.
+	 */
+	size_t out_size;
+
+	/* Flags; zero or more of enum vendor_cmd_flags */
+	uint32_t flags;
+};
+
+/* Type of function handling extension commands. */
+typedef enum vendor_cmd_rc
+		(*extension_handler)(struct vendor_cmd_params *params);
 
 /**
  * Find handler for an extension command.
@@ -42,20 +54,11 @@ typedef enum vendor_cmd_rc (*extension_handler)(enum vendor_cmd_cc code,
  * Use the interface specific function call in order to check the policies for
  * handling the commands on that interface.
  *
- * @param command_code Code associated with a extension command handler.
- * @param buffer       Data to be processd by the handler, the same space
- *                     is used for data returned by the handler.
- * @param in_size      Size of the input data.
- * @param out_size     On input: max size of the buffer.  On output: actual
- *                     number of bytes returned by the handler; a single byte
- *                     usually indicates an error and contains the error code.
- * @param flags        Zero or more flags from vendor_cmd_flags.
+ * @param p		Parameters for the command
+ * @return The return code from processing the command.
  */
-uint32_t extension_route_command(uint16_t command_code,
-				 void *buffer,
-				 size_t in_size,
-				 size_t *out_size,
-				 uint32_t flags);
+uint32_t extension_route_command(struct vendor_cmd_params *p);
+
 
 /* Pointer table */
 struct extension_command {
@@ -64,20 +67,34 @@ struct extension_command {
 } __packed;
 
 #define DECLARE_EXTENSION_COMMAND(code, func)				\
-	static enum vendor_cmd_rc func##_wrap(enum vendor_cmd_cc code,	\
-				       void *cmd_body,			\
-				       size_t cmd_size,			\
-				       size_t *response_size) {		\
-		func(cmd_body, cmd_size, response_size);		\
+	static enum vendor_cmd_rc					\
+	func##_wrap(struct vendor_cmd_params *params)			\
+	{								\
+		func(params->buffer, params->in_size,			\
+		     &params->out_size);				\
 		return 0;						\
 	}								\
 	const struct extension_command __keep __extension_cmd_##code	\
 	__attribute__((section(".rodata.extensioncmds")))		\
 		= {.command_code = code, .handler = func##_wrap }
 
-#define DECLARE_VENDOR_COMMAND(code, func)				\
-	const struct extension_command __keep __vendor_cmd_##code	\
+/* Vendor command which takes params directly */
+#define DECLARE_VENDOR_COMMAND(cmd_code, func)				\
+	static enum vendor_cmd_rc					\
+	func##_wrap(struct vendor_cmd_params *params)			\
+	{								\
+		func(params->code, params->buffer, params->in_size,	\
+		     &params->out_size);				\
+		return 0;						\
+	}								\
+	const struct extension_command __keep __vendor_cmd_##cmd_code	\
+	__attribute__((section(".rodata.extensioncmds")))		\
+		= {.command_code = cmd_code, .handler = func##_wrap}
+
+/* Vendor command which takes params as struct */
+#define DECLARE_VENDOR_COMMAND_P(cmd_code, func)			\
+	const struct extension_command __keep __vendor_cmd_##cmd_code	\
 	__attribute__((section(".rodata.extensioncmds")))		\
-		= {.command_code = code, .handler = func}
+		= {.command_code = cmd_code, .handler = func}
 
 #endif  /* __EC_INCLUDE_EXTENSION_H */