diff --git a/chip/g/upgrade_fw.c b/chip/g/upgrade_fw.c index c8ff5b80d5..ccb69dfb57 100644 --- a/chip/g/upgrade_fw.c +++ b/chip/g/upgrade_fw.c @@ -13,6 +13,7 @@ #include "memory.h" #include "uart.h" +#include "upgrade_fw.h" #include "cryptoc/sha.h" #define CPRINTF(format, args...) cprintf(CC_EXTENSION, format, ## args) @@ -28,20 +29,6 @@ enum return_value { UPGRADE_GEN_ERROR = 6, }; -/* - * The payload of the upgrade command. (Integer values in network byte order). - * - * block digest: the first four bytes of the sha1 digest of the rest of the - * structure. - * block_base: address where this block needs to be written to. - * block_body: variable size data to written at address 'block_base'. - */ -struct upgrade_command { - uint32_t block_digest; - uint32_t block_base; - uint8_t block_body[0]; -} __packed; - /* * This array defines two possibe sections available for the firmare update. * The section whcih does not map the current execting code is picked as the @@ -91,6 +78,7 @@ void fw_upgrade_command_handler(void *body, size_t *response_size) { struct upgrade_command *cmd_body = body; + void *upgrade_data; uint8_t *rv = body; uint8_t sha1_digest[SHA_DIGEST_SIZE]; size_t body_size; @@ -102,12 +90,12 @@ void fw_upgrade_command_handler(void *body, */ *response_size = sizeof(*rv); - if (cmd_size < offsetof(struct upgrade_command, block_body)) { + if (cmd_size < sizeof(struct upgrade_command)) { CPRINTF("%s:%d\n", __func__, __LINE__); *rv = UPGRADE_GEN_ERROR; return; } - body_size = cmd_size - offsetof(struct upgrade_command, block_body); + body_size = cmd_size - sizeof(struct upgrade_command); if (!cmd_body->block_base && !body_size) { /* @@ -171,8 +159,9 @@ void fw_upgrade_command_handler(void *body, CPRINTF("%s: programming at address 0x%x\n", __func__, block_offset + CONFIG_PROGRAM_MEMORY_BASE); - if (flash_physical_write(block_offset, body_size, - cmd_body->block_body) != EC_SUCCESS) { + upgrade_data = cmd_body + 1; + if (flash_physical_write(block_offset, body_size, upgrade_data) + != EC_SUCCESS) { *rv = UPGRADE_WRITE_FAILURE; CPRINTF("%s:%d upgrade write error\n", __func__, __LINE__); @@ -180,7 +169,7 @@ void fw_upgrade_command_handler(void *body, } /* Werify that data was written properly. */ - if (memcmp(cmd_body->block_body, (void *) + if (memcmp(upgrade_data, (void *) (block_offset + CONFIG_PROGRAM_MEMORY_BASE), body_size)) { *rv = UPGRADE_VERIFY_ERROR; diff --git a/chip/g/upgrade_fw.h b/chip/g/upgrade_fw.h index ce9debd5fb..f7fcf0de8a 100644 --- a/chip/g/upgrade_fw.h +++ b/chip/g/upgrade_fw.h @@ -8,6 +8,58 @@ #include +#define UPGRADE_PROTOCOL_VERSION 2 + +/* This is the format of the header the flash update function expects. */ +struct upgrade_command { + uint32_t block_digest; /* first 4 bytes of sha1 of the rest of the + * block. + */ + uint32_t block_base; /* Offset of this block into the flash SPI. */ + /* The actual payload goes here. */ +} __packed; + +/* + * This is the format of the header the host uses when sending update PDUs + * over USB. The PDUs are 1K bytes in size, they are fragmented into USB units + * of 64 bytes each and reassembled on the receive side before being passed to + * the flash update function. + * + * The flash update function receives the PDU body starting at the cmd field + * below, and puts its reply into the beginning of the same buffer. + */ +struct update_pdu_header { + uint32_t block_size; /* Total size of the block, including this + * field. + */ + struct upgrade_command cmd; +}; + +/* + * Response to the message initiating the update sequence. + * + * When responding to the very first packet of the upgrade sequence, the + * original USB update implementation was responding with a four byte value, + * just as to any other block of the transfer sequence. + * + * It became clear that there is a need to be able to enhance the upgrade + * protocol, while stayng backwards compatible. + * + * All newer protocol versions (satring with version 2) respond to the very + * first packet with an 8 byte or larger response, where the first 4 bytes are + * a version specific data, and the second 4 bytes - the protocol version + * number. + * + * This way the host receiving of a four byte value in response to the first + * packet is considered an indication of the target running the 'legacy' + * protocol, version 1. Receiving of an 8 byte or longer response would + * communicates the protocol version in the second 4 bytes. + */ +struct first_response_pdu { + uint32_t return_value; + uint32_t protocol_version; +}; + /* TODO: Handle this in upgrade_fw.c, not usb_upgrade.c */ #define UPGRADE_DONE 0xB007AB1E diff --git a/chip/g/usb_upgrade.c b/chip/g/usb_upgrade.c index 45cf06e87c..e41a9dac30 100644 --- a/chip/g/usb_upgrade.c +++ b/chip/g/usb_upgrade.c @@ -67,25 +67,6 @@ enum rx_state { rx_awaiting_reset /* Waiting for reset confirmation. */ }; -/* This is the format of the header the programmer expects. */ -struct upgrade_command { - uint32_t block_digest; /* first 4 bytes of sha1 of the rest of the - block. */ - uint32_t block_base; /* Offset of this block into the flash SPI. */ -}; - -/* This is the format of the header the host uses. */ -struct update_pdu_header { - uint32_t block_size; /* Total size of the block, including this - field. */ - union { - struct upgrade_command cmd; - uint32_t resp; /* The programmer puts response to the same - buffer where the command was. */ - }; - /* The actual payload goes here. */ -}; - enum rx_state rx_state_ = rx_idle; static uint8_t *block_buffer; static uint32_t block_size; @@ -131,8 +112,6 @@ static int valid_transfer_start(struct consumer const *consumer, size_t count, */ static uint64_t prev_activity_timestamp; -#define UPGRADE_PROTOCOL_VERSION 2 - /* Called to deal with data from the host */ static void upgrade_out_handler(struct consumer const *consumer, size_t count) { @@ -160,28 +139,7 @@ static void upgrade_out_handler(struct consumer const *consumer, size_t count) } if (rx_state_ == rx_idle) { - /* - * When responding to the very first packet of the upgrade - * sequence, the original implementation was responding with a - * four byte value, just as to any other block of the transfer - * sequence. - * - * It became clear that there is a need to be able to enhance - * the upgrade protocol, while stayng backwards compatible. To - * achieve that we respond to the very first packet with an 8 - * byte value, the first 4 bytes the same as before, the - * second 4 bytes - the protocol version number. - * - * This way if on the host side receiving of a four byte value - * in response to the first packet is an indication of the - * 'legacy' protocol, version 0. Receiving of an 8 byte - * response would communicate the protocol version in the - * second 4 bytes. - */ - struct { - uint32_t value; - uint32_t version; - } startup_resp; + struct first_response_pdu *startup_resp; if (!valid_transfer_start(consumer, count, &updu)) return; @@ -193,20 +151,32 @@ static void upgrade_out_handler(struct consumer const *consumer, size_t count) cmd), &resp_size); + /* + * The handler reuses receive buffer to return the result + * value. + */ + startup_resp = (struct first_response_pdu *)&updu.cmd; if (resp_size == 4) { - /* Already in network order. */ - startup_resp.value = updu.resp; + /* + * The handler is happy, returned a 4 byte base + * offset, it is in startup_resp->return_value now in + * the proper byte order. + */ rx_state_ = rx_outside_block; } else { - /* This must be a single byte error code. */ - startup_resp.value = htobe32(*((uint8_t *)&updu.resp)); + /* + * This must be a single byte error code, convert it + * into a 4 byte network order representation. + */ + startup_resp->return_value = htobe32 + (*((uint8_t *)&startup_resp->return_value)); } - - startup_resp.version = htobe32(UPGRADE_PROTOCOL_VERSION); + startup_resp->protocol_version = + htobe32(UPGRADE_PROTOCOL_VERSION); /* Let the host know what upgrader had to say. */ - QUEUE_ADD_UNITS(&upgrade_to_usb, &startup_resp, - sizeof(startup_resp)); + QUEUE_ADD_UNITS(&upgrade_to_usb, startup_resp, + sizeof(*startup_resp)); return; } diff --git a/extra/usb_updater/usb_updater.c b/extra/usb_updater/usb_updater.c index bbb276652e..952f7db3c6 100644 --- a/extra/usb_updater/usb_updater.c +++ b/extra/usb_updater/usb_updater.c @@ -403,11 +403,6 @@ static void usb_findit(uint16_t vid, uint16_t pid, struct usb_endpoint *uep) printf("READY\n-------\n"); } -struct upgrade_command { - uint32_t block_digest; - uint32_t block_base; -}; - struct update_pdu { uint32_t block_size; /* Total block size, include this field's size. */ struct upgrade_command cmd; @@ -415,26 +410,6 @@ struct update_pdu { }; #define FLASH_BASE 0x40000 -/* - * When responding to the very first packet of the upgrade sequence, the - * original implementation was responding with a four byte value, just as to - * any other block of the transfer sequence. - * - * It became clear that there is a need to be able to enhance the upgrade - * protocol, while stayng backwards compatible. To achieve that a new startup - * response option was introduced, 8 bytes in size. The first 4 bytes the same - * as before, the second 4 bytes - the protocol version number. - * - * So, receiving of a four byte value in response to the startup packet is an - * indication of the 'legacy' protocol, version 0. Receiving of an 8 byte - * value communicates the protocol version in the second 4 bytes. - */ - -struct startup_resp { - uint32_t value; - uint32_t version; -}; - static int transfer_block(struct usb_endpoint *uep, struct update_pdu *updu, uint8_t *transfer_data_ptr, size_t payload_size) { @@ -535,7 +510,7 @@ static void transfer_section(struct transfer_endpoint *tep, exit(1); } } else { - struct startup_resp resp; + struct first_response_pdu resp; size_t rxed_size; rxed_size = sizeof(resp); @@ -553,7 +528,7 @@ static void transfer_section(struct transfer_endpoint *tep, if ((rxed_size != 1) || *((uint8_t *)&resp)) { fprintf(stderr, "got response of size %zd, value %#x\n", - rxed_size, resp.value); + rxed_size, resp.return_value); exit(1); } @@ -571,7 +546,7 @@ static void transfer_and_reboot(struct transfer_endpoint *tep, uint32_t out; uint32_t reply; struct update_pdu updu; - struct startup_resp first_resp; + struct first_response_pdu first_resp; size_t rxed_size; struct usb_endpoint *uep = &tep->uep; @@ -596,11 +571,11 @@ static void transfer_and_reboot(struct transfer_endpoint *tep, if (rxed_size == sizeof(uint32_t)) protocol_version = 0; else - protocol_version = be32toh(first_resp.version); + protocol_version = be32toh(first_resp.protocol_version); printf("Target running protocol version %d\n", protocol_version); - reply = be32toh(first_resp.value); + reply = be32toh(first_resp.return_value); if (reply < 256) { fprintf(stderr, "Target reports error %d\n", reply);