From cefb6ca9c1a7cc26da2c004ab35569cb974fa627 Mon Sep 17 00:00:00 2001 From: Michael Cardell Widerkrantz Date: Fri, 24 Mar 2023 11:47:30 +0100 Subject: [PATCH] fw: Change max frame size to 128 bytes --- doc/framing_protocol/framing_protocol.md | 16 +++---- doc/system_description/software.md | 20 ++++---- hw/application_fpga/fw/tk1/main.c | 8 ++-- hw/application_fpga/fw/tk1/proto.c | 61 ++++++++++++++---------- hw/application_fpga/fw/tk1/proto.h | 4 +- 5 files changed, 60 insertions(+), 49 deletions(-) diff --git a/doc/framing_protocol/framing_protocol.md b/doc/framing_protocol/framing_protocol.md index 21145a9..df6203f 100644 --- a/doc/framing_protocol/framing_protocol.md +++ b/doc/framing_protocol/framing_protocol.md @@ -125,12 +125,12 @@ The bits in the command header byte should be interpreted as: 0. 1 byte 1. 4 bytes 2. 32 bytes -3. 512 bytes +3. 128 bytes Note that the number of bytes indicated by the command data length field does **not** include the command header byte. This means that a complete -command frame, with a header indicating a data length of 512 bytes, is -512+1 bytes in length. +command frame, with a header indicating a data length of 128 bytes, is +128+1 bytes in length. Note that the host sets the frame ID tag. The ID tag in a given command MUST be preserved in the corresponding response to the command. @@ -148,7 +148,7 @@ Some examples to clarify endpoints and commands: data. The single byte could indicate action such as reading from the TRNG or resetting the application_fpga. -* 0x13: A command to the FW in the application_fpga with 512 bytes of +* 0x13: A command to the FW in the application_fpga with 128 bytes of data. The data could for example be parts of an application binary to be loaded into the program memory. @@ -179,13 +179,13 @@ The bits in the response header byte should be interpreted as: 0. 1 byte 1. 4 bytes 2. 32 bytes -3. 512 bytes +3. 128 bytes Note that the number of bytes indicated by the response data length field does **not** include the response header byte. This means that a complete -response frame, with a header indicating a data length of 512 bytes, is -512+1 bytes in length. +response frame, with a header indicating a data length of 128 bytes, is +128+1 bytes in length. Note that the ID in a response MUST be the same ID as was present in the header of the command being responded to. @@ -205,7 +205,7 @@ less available for the "payload" of the response. responds with a single byte of data. * 0x1b: A successful command to the application running in the - application_fpga. The response contains 512 bytes of data, for example + application_fpga. The response contains 128 bytes of data, for example an EdDSA Ed25519 signature. diff --git a/doc/system_description/software.md b/doc/system_description/software.md index a311303..16e5530 100644 --- a/doc/system_description/software.md +++ b/doc/system_description/software.md @@ -308,10 +308,10 @@ Response to `FW_CMD_LOAD_APP` | *name* | *size (bytes)* | *comment* | |--------|----------------|---------------------| -| data | 511 | Raw binary app data | +| data | 127 | Raw binary app data | -Load 511 bytes of raw app binary into device RAM. Should be sent -consecutively over the complete raw binary. (512 == largest frame +Load 127 bytes of raw app binary into device RAM. Should be sent +consecutively over the complete raw binary. (128 == largest frame length minus the command byte). #### `FW_RSP_LOAD_APP_DATA` (0x06) @@ -377,9 +377,9 @@ host <- ``` host -> - u8 CMD[1 + 512]; + u8 CMD[1 + 128]; - CMD[0].len = 512 // command frame format + CMD[0].len = 128 // command frame format CMD[1] = 0x03 // FW_CMD_LOAD_APP CMD[2..6] = APP_SIZE @@ -398,11 +398,11 @@ host <- RSP[3..] = 0 -repeat ceil(APP_SIZE / 511) times: +repeat ceil(APP_SIZE / 127) times: host -> - u8 CMD[1 + 512]; + u8 CMD[1 + 128]; - CMD[0].len = 512 // command frame format + CMD[0].len = 128 // command frame format CMD[1] = 0x05 // FW_CMD_LOAD_APP_DATA CMD[2..] = APP_DATA (511 bytes of app data, pad with zeros) @@ -422,9 +422,9 @@ Except response from last chunk of app data which is: ``` host <- - u8 RSP[1 + 512] + u8 RSP[1 + 128] - RSP[0].len = 512 // command frame format + RSP[0].len = 128 // command frame format RSP[1] = 0x07 // FW_RSP_LOAD_APP_DATA_READY RSP[2] = STATUS diff --git a/hw/application_fpga/fw/tk1/main.c b/hw/application_fpga/fw/tk1/main.c index 6ca9798..ebd1d6f 100644 --- a/hw/application_fpga/fw/tk1/main.c +++ b/hw/application_fpga/fw/tk1/main.c @@ -188,7 +188,7 @@ static enum state initial_commands(const struct frame_header *hdr, uint32_t local_app_size; htif_puts("cmd: load-app(size, uss)\n"); - if (hdr->len != 512) { + if (hdr->len != 128) { // Bad length state = FW_STATE_FAIL; break; @@ -253,14 +253,14 @@ static enum state loading_commands(const struct frame_header *hdr, switch (cmd[0]) { case FW_CMD_LOAD_APP_DATA: htif_puts("cmd: load-app-data\n"); - if (hdr->len != 512) { + if (hdr->len != 128) { // Bad length state = FW_STATE_FAIL; break; } - if (ctx->left > (512 - 1)) { - nbytes = 512 - 1; + if (ctx->left > (128 - 1)) { + nbytes = 128 - 1; } else { nbytes = ctx->left; } diff --git a/hw/application_fpga/fw/tk1/proto.c b/hw/application_fpga/fw/tk1/proto.c index 13c1910..a78c539 100644 --- a/hw/application_fpga/fw/tk1/proto.c +++ b/hw/application_fpga/fw/tk1/proto.c @@ -5,6 +5,7 @@ #include "proto.h" #include "../tk1_mem.h" +#include "assert.h" #include "led.h" #include "lib.h" #include "state.h" @@ -22,6 +23,7 @@ static uint8_t genhdr(uint8_t id, uint8_t endpoint, uint8_t status, static int parseframe(uint8_t b, struct frame_header *hdr); static void write(uint8_t *buf, size_t nbytes); static int read(uint8_t *buf, size_t bufsize, size_t nbytes); +static int bytelen(enum cmdlen cmdlen); static uint8_t genhdr(uint8_t id, uint8_t endpoint, uint8_t status, enum cmdlen len) @@ -71,25 +73,7 @@ static int parseframe(uint8_t b, struct frame_header *hdr) hdr->id = (b & 0x60) >> 5; hdr->endpoint = (b & 0x18) >> 3; - - // Length - switch (b & 0x3) { - case LEN_1: - hdr->len = 1; - break; - case LEN_4: - hdr->len = 4; - break; - case LEN_32: - hdr->len = 32; - break; - case LEN_512: - hdr->len = 512; - break; - default: - // Unknown length - return -1; - } + hdr->len = bytelen(b & 0x3); return 0; } @@ -104,27 +88,22 @@ void fwreply(struct frame_header hdr, enum fwcmd rspcode, uint8_t *buf) switch (rspcode) { case FW_RSP_NAME_VERSION: len = LEN_32; - nbytes = 32; break; case FW_RSP_LOAD_APP: len = LEN_4; - nbytes = 4; break; case FW_RSP_LOAD_APP_DATA: len = LEN_4; - nbytes = 4; break; case FW_RSP_LOAD_APP_DATA_READY: - len = LEN_512; - nbytes = 512; + len = LEN_128; break; case FW_RSP_GET_UDI: len = LEN_32; - nbytes = 32; break; default: @@ -134,6 +113,8 @@ void fwreply(struct frame_header hdr, enum fwcmd rspcode, uint8_t *buf) return; } + nbytes = bytelen(len); + // Frame Protocol Header writebyte(genhdr(hdr.id, hdr.endpoint, 0x0, len)); @@ -182,3 +163,33 @@ static int read(uint8_t *buf, size_t bufsize, size_t nbytes) return 0; } + +// bytelen returns the number of bytes a cmdlen takes +static int bytelen(enum cmdlen cmdlen) +{ + int len; + + switch (cmdlen) { + case LEN_1: + len = 1; + break; + + case LEN_4: + len = 4; + break; + + case LEN_32: + len = 32; + break; + + case LEN_128: + len = 128; + break; + + default: + // Shouldn't happen + assert(1 == 2); + } + + return len; +} diff --git a/hw/application_fpga/fw/tk1/proto.h b/hw/application_fpga/fw/tk1/proto.h index d99f0e5..105d5ad 100644 --- a/hw/application_fpga/fw/tk1/proto.h +++ b/hw/application_fpga/fw/tk1/proto.h @@ -19,10 +19,10 @@ enum cmdlen { LEN_1, LEN_4, LEN_32, - LEN_512 + LEN_128 }; -#define CMDLEN_MAXBYTES 512 +#define CMDLEN_MAXBYTES 128 // clang-format off enum fwcmd {