[PATCH u-boot-marvell 00/29] kwboot higher baudrate

Hello Stefan and others,
this series adds support for booting Marvell platforms via UART (those bootable with kwboot) at higher baudrates.
Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than 115200 Bd.
The user can direct kwboot to use higher baudrate via the -B option. (BTW this option was there before but did not work with the -b option.)
Only the payload part of the KWB image is uploaded at this higher baudrate. The header part is still uploaded at 115200 Bd, since the code that changes baudrate is injected into header as a binary extension. (The payload part makes up majority of the binary, though. On Turris Omnia the payload currently makes ~87%.)
The series also contains various other fixes, refactors and improvements of the code, upon which the main change is done.
Marek & Pali
Marek Behún (13): tools: kwbimage: Fix printf format warning tools: kwboot: Fix buffer overflow in kwboot_terminal() tools: kwboot: Make the quit sequence buffer const tools: kwboot: Refactor and fix writing buffer tools: kwboot: Fix comparison of integers with different size tools: kwboot: Use a function to check whether received byte is a Xmodem reply tools: kwboot: Print new line after SPL output tools: kwboot: Allow greater timeout when executing header code tools: kwbimage: Simplify iteration over version 1 optional headers tools: kwbimage: Refactor image_version() tools: kwbimage: Refactor kwbimage header size determination doc/kwboot.1: Update man page MAINTAINERS: Add entry for kwbimage / kwboot tools
Pali Rohár (16): tools: kwboot: Print version information header tools: kwboot: Fix kwboot_xm_sendblock() function when kwboot_tty_recv() fails tools: kwboot: Fix return type of kwboot_xm_makeblock() function tools: kwboot: Fix printing progress tools: kwboot: Print newline on error when progress was not completed tools: kwboot: Split sending image into header and data stages tools: kwboot: Allow non-xmodem text output from BootROM only in a specific case tools: kwboot: Properly finish xmodem transfer tools: kwboot: Always call kwboot_img_patch_hdr() tools: kwboot: Don't patch image header if signed tools: kwboot: Patch source address in image header tools: kwboot: Patch destination address to DDR area for SPI image tools: kwboot: Round up header size to 128 B when patching tools: kwboot: Support higher baudrates when booting via UART tools: kwboot: Allow any baudrate on Linux tools: kwboot: Add Pali and Marek as authors
MAINTAINERS | 10 + doc/kwboot.1 | 58 ++- tools/kwbimage.c | 127 ++--- tools/kwbimage.h | 93 +++- tools/kwboot.c | 1103 +++++++++++++++++++++++++++++++++++------ tools/termios_linux.h | 170 +++++++ 6 files changed, 1289 insertions(+), 272 deletions(-) create mode 100644 tools/termios_linux.h

On 32-bit ARM the compiler complains: tools/kwbimage.c:547: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘unsigned int’
Fix this by using %zu instead of %lu format specifier.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwbimage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index aa865cc443..ed11c835a5 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -544,7 +544,7 @@ static int kwb_export_pubkey(RSA *key, struct pubkey_der_v1 *dst, FILE *hashf, }
if (4 + size_seq > sizeof(dst->key)) { - fprintf(stderr, "export pk failed: seq too large (%d, %lu)\n", + fprintf(stderr, "export pk failed: seq too large (%d, %zu)\n", 4 + size_seq, sizeof(dst->key)); fprintf(stderr, errmsg, keyname); return -ENOBUFS;

The `in` variable is set to -1 in kwboot_terminal() if stdin is not a tty. In this case we should not look whether -1 is set in fd_set, for it can lead to a buffer overflow, which can be reproduced with echo "xyz" | ./tools/kwboot -t /dev/ttyUSB0
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 7feeaa45a2..e6e99849a7 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -552,7 +552,7 @@ kwboot_terminal(int tty) break; }
- if (FD_ISSET(in, &rfds)) { + if (in >= 0 && FD_ISSET(in, &rfds)) { rc = kwboot_term_pipe(in, tty, quit, &s); if (rc) break;

This buffer is never written to. Make it const.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index e6e99849a7..f18f6d2134 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -460,7 +460,7 @@ can: }
static int -kwboot_term_pipe(int in, int out, char *quit, int *s) +kwboot_term_pipe(int in, int out, const char *quit, int *s) { ssize_t nin, nout; char _buf[128], *buf = _buf; @@ -504,7 +504,7 @@ static int kwboot_terminal(int tty) { int rc, in, s; - char *quit = "\34c"; + const char *quit = "\34c"; struct termios otio, tio;
rc = -1;

There are 3 instances in kwboot.c where we need to write() a given buffer whole (iteratively writing until all data are written), and 2 of those instances are wrong, for they do not increment the buffer pointer.
Refactor the code into a new function kwboot_write() where it is fixed.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 55 ++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index f18f6d2134..22cdd137ad 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -72,6 +72,23 @@ static int msg_req_delay = KWBOOT_MSG_REQ_DELAY; static int msg_rsp_timeo = KWBOOT_MSG_RSP_TIMEO; static int blk_rsp_timeo = KWBOOT_BLK_RSP_TIMEO;
+static ssize_t +kwboot_write(int fd, const char *buf, size_t len) +{ + size_t tot = 0; + + while (tot < len) { + ssize_t wr = write(fd, buf + tot, len - tot); + + if (wr < 0) + return -1; + + tot += wr; + } + + return tot; +} + static void kwboot_printv(const char *fmt, ...) { @@ -191,26 +208,13 @@ out: static int kwboot_tty_send(int fd, const void *buf, size_t len) { - int rc; - ssize_t n; - if (!buf) return 0;
- rc = -1; - - do { - n = write(fd, buf, len); - if (n < 0) - goto out; - - buf = (char *)buf + n; - len -= n; - } while (len > 0); + if (kwboot_write(fd, buf, len) < 0) + return -1;
- rc = tcdrain(fd); -out: - return rc; + return tcdrain(fd); }
static int @@ -462,7 +466,7 @@ can: static int kwboot_term_pipe(int in, int out, const char *quit, int *s) { - ssize_t nin, nout; + ssize_t nin; char _buf[128], *buf = _buf;
nin = read(in, buf, sizeof(_buf)); @@ -480,22 +484,15 @@ kwboot_term_pipe(int in, int out, const char *quit, int *s) buf++; nin--; } else { - while (*s > 0) { - nout = write(out, quit, *s); - if (nout <= 0) - return -1; - (*s) -= nout; - } + if (kwboot_write(out, quit, *s) < 0) + return -1; + *s = 0; } } }
- while (nin > 0) { - nout = write(out, buf, nin); - if (nout <= 0) - return -1; - nin -= nout; - } + if (kwboot_write(out, buf, nin) < 0) + return -1;
return 0; }

From: Pali Rohár pali@kernel.org
Print kwboot's (U-Boot's) version when printing usage.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 22cdd137ad..454339db14 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -11,6 +11,7 @@
#include "kwbimage.h" #include "mkimage.h" +#include "version.h"
#include <stdlib.h> #include <stdio.h> @@ -681,6 +682,7 @@ out: static void kwboot_usage(FILE *stream, char *progname) { + fprintf(stream, "kwboot version %s\n", PLAIN_VERSION); fprintf(stream, "Usage: %s [OPTIONS] [-b <image> | -D <image> ] [-B <baud> ] <TTY>\n", progname);

From: Pali Rohár pali@kernel.org
When kwboot_tty_recv() fails or times out, it does not set the `c` variable to NAK. The variable is then compared, while it holds either an undefined value or a value from previous iteration. Set `c` to NAK so that the other side will try to resend current block, and remove the now unnecessary break.
In other failure cases return immediately.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 454339db14..b9a402ca91 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -380,12 +380,15 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block) do { rc = kwboot_tty_send(fd, block, sizeof(*block)); if (rc) - break; + return rc;
do { rc = kwboot_tty_recv(fd, &c, 1, blk_rsp_timeo); - if (rc) - break; + if (rc) { + if (errno != ETIMEDOUT) + return rc; + c = NAK; + }
if (c != ACK && c != NAK && c != CAN) printf("%c", c);

From: Pali Rohár pali@kernel.org
Function kwboot_xm_makeblock() always returns length of xmodem block. It is always non-negative and calculated from variable with size_t type. Set return type of this function to size_t and remove dead code which checks for negative value.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index b9a402ca91..88353d19c0 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -347,7 +347,7 @@ kwboot_debugmsg(int tty, void *msg) return rc; }
-static int +static size_t kwboot_xm_makeblock(struct kwboot_block *block, const void *data, size_t size, int pnum) { @@ -441,9 +441,6 @@ kwboot_xmodem(int tty, const void *_data, size_t size) n = kwboot_xm_makeblock(&block, data + N, size - N, pnum++); - if (n < 0) - goto can; - if (!n) break;

The compiler complains that we are comparing int with size_t when compiled with -W.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 88353d19c0..3d9f73e697 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -352,8 +352,7 @@ kwboot_xm_makeblock(struct kwboot_block *block, const void *data, size_t size, int pnum) { const size_t blksz = sizeof(block->data); - size_t n; - int i; + size_t i, n;
block->soh = SOH; block->pnum = pnum;

From: Pali Rohár pali@kernel.org
Ensure that `pos` is still in range up to the `width` so printing 100% works also for bigger images. After printing 100% progress reset it to zero, so that next progressbar can be started.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 3d9f73e697..eb4b3fe230 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -140,12 +140,14 @@ __progress(int pct, char c) fputc(c, stdout);
nl = "]\n"; - pos++; + pos = (pos + 1) % width;
if (pct == 100) { - while (pos++ < width) + while (pos && pos++ < width) fputc(' ', stdout); fputs(nl, stdout); + nl = ""; + pos = 0; }
fflush(stdout); @@ -162,6 +164,9 @@ kwboot_progress(int _pct, char c)
if (kwboot_verbose) __progress(pct, c); + + if (pct == 100) + pct = 0; }
static int

From: Pali Rohár pali@kernel.org
When progress was not completed, current terminal position is in progress bar. So print newline before printing error message to make error message more readable.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/kwboot.c b/tools/kwboot.c index eb4b3fe230..0e533e3698 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -459,6 +459,7 @@ kwboot_xmodem(int tty, const void *_data, size_t size) rc = kwboot_tty_send_char(tty, EOT);
out: + kwboot_printv("\n"); return rc;
can:

From: Pali Rohár pali@kernel.org
This change is required to implement other features in kwboot.
Split sending header and data parts of the image into two stages.
Signed-off-by: Pali Rohár pali@kernel.org [ refactored ] Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwbimage.h | 8 +++-- tools/kwboot.c | 84 +++++++++++++++++++++++++++++++----------------- 2 files changed, 61 insertions(+), 31 deletions(-)
diff --git a/tools/kwbimage.h b/tools/kwbimage.h index e063e3e41e..074bdfbe46 100644 --- a/tools/kwbimage.h +++ b/tools/kwbimage.h @@ -195,6 +195,10 @@ struct register_set_hdr_v1 { #define OPT_HDR_V1_BINARY_TYPE 0x2 #define OPT_HDR_V1_REGISTER_TYPE 0x3
+#define KWBHEADER_V0_SIZE(hdr) \ + (((hdr)->ext & 0x1) ? sizeof(struct kwb_header) : \ + sizeof(struct main_hdr_v0)) + #define KWBHEADER_V1_SIZE(hdr) \ (((hdr)->headersz_msb << 16) | le16_to_cpu((hdr)->headersz_lsb))
@@ -225,9 +229,9 @@ void init_kwb_image_type (void); * header, byte 8 was reserved, and always set to 0. In the v1 header, * byte 8 has been changed to a proper field, set to 1. */ -static inline unsigned int image_version(void *header) +static inline unsigned int image_version(const void *header) { - unsigned char *ptr = header; + const unsigned char *ptr = header; return ptr[8]; }
diff --git a/tools/kwboot.c b/tools/kwboot.c index 0e533e3698..7f231c0823 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -57,11 +57,13 @@ static unsigned char kwboot_msg_debug[] = { #define NAK 21 /* target block negative ack */ #define CAN 24 /* target/sender transfer cancellation */
+#define KWBOOT_XM_BLKSZ 128 /* xmodem block size */ + struct kwboot_block { uint8_t soh; uint8_t pnum; uint8_t _pnum; - uint8_t data[128]; + uint8_t data[KWBOOT_XM_BLKSZ]; uint8_t csum; } __packed;
@@ -356,16 +358,15 @@ static size_t kwboot_xm_makeblock(struct kwboot_block *block, const void *data, size_t size, int pnum) { - const size_t blksz = sizeof(block->data); size_t i, n;
block->soh = SOH; block->pnum = pnum; block->_pnum = ~block->pnum;
- n = size < blksz ? size : blksz; + n = size < KWBOOT_XM_BLKSZ ? size : KWBOOT_XM_BLKSZ; memcpy(&block->data[0], data, n); - memset(&block->data[n], 0, blksz - n); + memset(&block->data[n], 0, KWBOOT_XM_BLKSZ - n);
block->csum = 0; for (i = 0; i < n; i++) @@ -425,48 +426,73 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block) }
static int -kwboot_xmodem(int tty, const void *_data, size_t size) +kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data, + size_t size) { - const uint8_t *data = _data; - int rc, pnum, N, err; - - pnum = 1; - N = 0; + size_t sent, left; + int rc;
- kwboot_printv("Sending boot image...\n"); + kwboot_printv("Sending boot image %s (%zu bytes)...\n", + header ? "header" : "data", size);
- sleep(2); /* flush isn't effective without it */ - tcflush(tty, TCIOFLUSH); + left = size; + sent = 0;
- do { + while (sent < size) { struct kwboot_block block; - int n; + size_t blksz;
- n = kwboot_xm_makeblock(&block, - data + N, size - N, - pnum++); - if (!n) - break; + blksz = kwboot_xm_makeblock(&block, data, left, (*pnum)++); + data += blksz;
rc = kwboot_xm_sendblock(tty, &block); if (rc) goto out;
- N += n; - kwboot_progress(N * 100 / size, '.'); - } while (1); + sent += blksz; + left -= blksz; + + kwboot_progress(sent * 100 / size, '.'); + }
- rc = kwboot_tty_send_char(tty, EOT); + kwboot_printv("Done\n");
+ return 0; out: kwboot_printv("\n"); return rc; +}
-can: - err = errno; - kwboot_tty_send_char(tty, CAN); - errno = err; - goto out; +static int +kwboot_xmodem(int tty, const void *_img, size_t size) +{ + const uint8_t *img = _img; + int rc, pnum; + size_t hdrsz; + + if (image_version(img) == 0) + hdrsz = KWBHEADER_V0_SIZE((struct main_hdr_v0 *)img); + else + hdrsz = KWBHEADER_V1_SIZE((struct main_hdr_v1 *)img); + + kwboot_printv("Waiting 2s and flushing tty\n"); + sleep(2); /* flush isn't effective without it */ + tcflush(tty, TCIOFLUSH); + + pnum = 1; + + rc = kwboot_xmodem_one(tty, &pnum, 1, img, hdrsz); + if (rc) + return rc; + + img += hdrsz; + size -= hdrsz; + + rc = kwboot_xmodem_one(tty, &pnum, 0, img, size); + if (rc) + return rc; + + return kwboot_tty_send_char(tty, EOT); }
static int

This is a non-functional change that should make the code more readable.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 7f231c0823..2e5684b91c 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -375,6 +375,12 @@ kwboot_xm_makeblock(struct kwboot_block *block, const void *data, return n; }
+static int +_is_xm_reply(char c) +{ + return c == ACK || c == NAK || c == CAN; +} + static int kwboot_xm_sendblock(int fd, struct kwboot_block *block) { @@ -395,10 +401,10 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block) c = NAK; }
- if (c != ACK && c != NAK && c != CAN) + if (!_is_xm_reply(c)) printf("%c", c);
- } while (c != ACK && c != NAK && c != CAN); + } while (!_is_xm_reply(c));
if (c != ACK) kwboot_progress(-1, '+');

From: Pali Rohár pali@kernel.org
When sending image header / image data, BootROM does not send any non-xmodem text output. We should therefore interpret unknown bytes in the xmodem protocol as errors and resend current packet. This should improve the transfer in case there are errors on the UART line.
Text output from BootROM may only happen after whole image header is sent and before ACK for the last packet of image header is received. In this case BootROM may execute code from the image, which may interact with UART (U-Boot SPL, for example, prints stuff on UART).
Print received non-xmodem output from BootROM only in this case.
Signed-off-by: Pali Rohár pali@kernel.org [ refactored & simplified ] Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 2e5684b91c..2829b74a2e 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -382,17 +382,26 @@ _is_xm_reply(char c) }
static int -kwboot_xm_sendblock(int fd, struct kwboot_block *block) +kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, + int *done_print) { int rc, retries; char c;
+ *done_print = 0; + retries = 16; do { rc = kwboot_tty_send(fd, block, sizeof(*block)); if (rc) return rc;
+ if (allow_non_xm && !*done_print) { + kwboot_progress(100, '.'); + kwboot_printv("Done\n"); + *done_print = 1; + } + do { rc = kwboot_tty_recv(fd, &c, 1, blk_rsp_timeo); if (rc) { @@ -401,14 +410,14 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block) c = NAK; }
- if (!_is_xm_reply(c)) - printf("%c", c); - + if (!_is_xm_reply(c) && allow_non_xm) { + putchar(c); + fflush(stdout); + } } while (!_is_xm_reply(c));
- if (c != ACK) + if (!allow_non_xm && c != ACK) kwboot_progress(-1, '+'); - } while (c == NAK && retries-- > 0);
rc = -1; @@ -435,6 +444,7 @@ static int kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data, size_t size) { + int done_print = 0; size_t sent, left; int rc;
@@ -446,22 +456,28 @@ kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data,
while (sent < size) { struct kwboot_block block; + int last_block; size_t blksz;
blksz = kwboot_xm_makeblock(&block, data, left, (*pnum)++); data += blksz;
- rc = kwboot_xm_sendblock(tty, &block); + last_block = (left <= blksz); + + rc = kwboot_xm_sendblock(tty, &block, header && last_block, + &done_print); if (rc) goto out;
sent += blksz; left -= blksz;
- kwboot_progress(sent * 100 / size, '.'); + if (!done_print) + kwboot_progress(sent * 100 / size, '.'); }
- kwboot_printv("Done\n"); + if (!done_print) + kwboot_printv("Done\n");
return 0; out:

There is no separation between output from the code from binary header (U-Boot SPL in most cases) and subsequent kwboot output.
Print '\n' to make distinguishing these two easier.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 2829b74a2e..2e16db83fa 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -385,6 +385,7 @@ static int kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, int *done_print) { + int non_xm_print = 0; int rc, retries; char c;
@@ -413,6 +414,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, if (!_is_xm_reply(c) && allow_non_xm) { putchar(c); fflush(stdout); + non_xm_print = 1; } } while (!_is_xm_reply(c));
@@ -420,6 +422,9 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, kwboot_progress(-1, '+'); } while (c == NAK && retries-- > 0);
+ if (non_xm_print) + kwboot_printv("\n"); + rc = -1;
switch (c) {

When executing header code (which contains U-Boot SPL in most cases), wait 10s after every non-xmodem character received (i.e. printed by U-Boot SPL) before timing out.
Sometimes DDR training, which runs in SPL, may be slow.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 2e16db83fa..0e60cc8a70 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -24,6 +24,7 @@ #include <unistd.h> #include <stdint.h> #include <termios.h> +#include <time.h> #include <sys/mman.h> #include <sys/stat.h>
@@ -68,6 +69,7 @@ struct kwboot_block { } __packed;
#define KWBOOT_BLK_RSP_TIMEO 1000 /* ms */ +#define KWBOOT_HDR_RSP_TIMEO 10000 /* ms */
static int kwboot_verbose;
@@ -381,12 +383,35 @@ _is_xm_reply(char c) return c == ACK || c == NAK || c == CAN; }
+static uint64_t +_now(void) +{ + struct timespec ts; + + if (clock_gettime(CLOCK_MONOTONIC, &ts)) { + static int err_print; + + if (!err_print) { + perror("clock_gettime() does not work"); + err_print = 1; + } + + /* this will just make the timeout not work */ + return -1ULL; + } + + return ts.tv_sec * 1000 + (ts.tv_nsec + 500000) / 1000000; +} + static int kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, int *done_print) { + int recv_timeo = allow_non_xm ? KWBOOT_HDR_RSP_TIMEO : blk_rsp_timeo; int non_xm_print = 0; + uint64_t timeout = 0; int rc, retries; + int err; char c;
*done_print = 0; @@ -404,14 +429,22 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, }
do { - rc = kwboot_tty_recv(fd, &c, 1, blk_rsp_timeo); + err = 0; + rc = kwboot_tty_recv(fd, &c, 1, recv_timeo); if (rc) { - if (errno != ETIMEDOUT) + if (errno != ETIMEDOUT) { return rc; - c = NAK; + } else if (timeout && timeout < _now()) { + errno = ETIMEDOUT; + return -1; + } else { + err = errno; + c = NAK; + } }
if (!_is_xm_reply(c) && allow_non_xm) { + timeout = _now() + recv_timeo; putchar(c); fflush(stdout); non_xm_print = 1; @@ -425,6 +458,11 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, if (non_xm_print) kwboot_printv("\n");
+ if (err) { + errno = err; + return -1; + } + rc = -1;
switch (c) {

From: Pali Rohár pali@kernel.org
After kwboot sends EOT, BootROM sends back ACK. Add code for handling this and retry sending EOT on error.
Signed-off-by: Pali Rohár pali@kernel.org [ refactored ] Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 65 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 17 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 0e60cc8a70..6674a253ff 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -383,6 +383,28 @@ _is_xm_reply(char c) return c == ACK || c == NAK || c == CAN; }
+static int +_xm_reply_to_error(int c) { + int rc = -1; + + switch (c) { + case ACK: + rc = 0; + break; + case NAK: + errno = EBADMSG; + break; + case CAN: + errno = ECANCELED; + break; + default: + errno = EPROTO; + break; + } + + return rc; +} + static uint64_t _now(void) { @@ -463,24 +485,32 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, return -1; }
- rc = -1; + return _xm_reply_to_error(c); +}
- switch (c) { - case ACK: - rc = 0; - break; - case NAK: - errno = EBADMSG; - break; - case CAN: - errno = ECANCELED; - break; - default: - errno = EPROTO; - break; - } +static int +kwboot_xm_finish(int tty) +{ + int rc, retries; + char c;
- return rc; + retries = 16; + do { + rc = kwboot_tty_send_char(tty, EOT); + if (rc) + return rc; + + do { + rc = kwboot_tty_recv(tty, &c, 1, blk_rsp_timeo); + if (rc) { + if (errno != ETIMEDOUT) + return rc; + c = NAK; + } + } while (!_is_xm_reply(c) && retries-- > 0); + } while (c == NAK && retries-- > 0); + + return _xm_reply_to_error(c); }
static int @@ -557,7 +587,8 @@ kwboot_xmodem(int tty, const void *_img, size_t size) if (rc) return rc;
- return kwboot_tty_send_char(tty, EOT); + kwboot_printv("Finishing transfer\n"); + return kwboot_xm_finish(tty); }
static int

From: Pali Rohár pali@kernel.org
The kwboot_img_patch_hdr() function already decides if header patching is needed. Always call this function and deprecate the unneeded command line option `-p`.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 6674a253ff..031cea7550 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -692,9 +692,9 @@ out: }
static void * -kwboot_mmap_image(const char *path, size_t *size, int prot) +kwboot_mmap_image(const char *path, size_t *size) { - int rc, fd, flags; + int rc, fd; struct stat st; void *img;
@@ -709,9 +709,7 @@ kwboot_mmap_image(const char *path, size_t *size, int prot) if (rc) goto out;
- flags = (prot & PROT_WRITE) ? MAP_PRIVATE : MAP_SHARED; - - img = mmap(NULL, st.st_size, prot, flags, fd, 0); + img = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); if (img == MAP_FAILED) { img = NULL; goto out; @@ -816,7 +814,6 @@ kwboot_usage(FILE *stream, char *progname) fprintf(stream, "\n"); fprintf(stream, " -b <image>: boot <image> with preamble (Kirkwood, Armada 370/XP)\n"); - fprintf(stream, " -p: patch <image> to type 0x69 (uart boot)\n"); fprintf(stream, " -D <image>: boot <image> without preamble (Dove)\n"); fprintf(stream, " -d: enter debug mode\n"); @@ -836,7 +833,7 @@ int main(int argc, char **argv) { const char *ttypath, *imgpath; - int rv, rc, tty, term, prot, patch; + int rv, rc, tty, term; void *bootmsg; void *debugmsg; void *img; @@ -850,7 +847,6 @@ main(int argc, char **argv) imgpath = NULL; img = NULL; term = 0; - patch = 0; size = 0; speed = B115200;
@@ -877,7 +873,7 @@ main(int argc, char **argv) break;
case 'p': - patch = 1; + /* nop, for backward compatibility */ break;
case 't': @@ -917,9 +913,6 @@ main(int argc, char **argv) if (!bootmsg && !term && !debugmsg) goto usage;
- if (patch && !imgpath) - goto usage; - if (argc - optind < 1) goto usage;
@@ -932,16 +925,12 @@ main(int argc, char **argv) }
if (imgpath) { - prot = PROT_READ | (patch ? PROT_WRITE : 0); - - img = kwboot_mmap_image(imgpath, &size, prot); + img = kwboot_mmap_image(imgpath, &size); if (!img) { perror(imgpath); goto out; } - }
- if (patch) { rc = kwboot_img_patch_hdr(img, size); if (rc) { fprintf(stderr, "%s: Invalid image.\n", imgpath);

From: Pali Rohár pali@kernel.org
It is not possible to modify image with secure header due to cryptographic signature.
Signed-off-by: Pali Rohár pali@kernel.org [ refactored ] Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 031cea7550..c23357f5bc 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -739,6 +739,18 @@ kwboot_img_csum8(void *_data, size_t size) return csum; }
+static int +kwboot_img_is_secure(void *img) +{ + struct opt_hdr_v1 *ohdr; + + for_each_opt_hdr_v1 (ohdr, img) + if (ohdr->headertype == OPT_HDR_V1_SECURE_TYPE) + return 1; + + return 0; +} + static int kwboot_img_patch_hdr(void *img, size_t size) { @@ -747,7 +759,9 @@ kwboot_img_patch_hdr(void *img, size_t size) uint8_t csum; size_t hdrsz = sizeof(*hdr); int image_ver; + int is_secure;
+ is_secure = 0; rc = -1; hdr = img;
@@ -779,9 +793,16 @@ kwboot_img_patch_hdr(void *img, size_t size) goto out; }
- if (hdr->blockid == IBR_HDR_UART_ID) { - rc = 0; - goto out; + is_secure = kwboot_img_is_secure(img); + + if (hdr->blockid != IBR_HDR_UART_ID) { + if (is_secure) { + fprintf(stderr, + "Image has secure header with signature for non-UART booting\n"); + errno = EINVAL; + goto out; + } + kwboot_printv("Patching image boot signature to UART\n"); }
hdr->blockid = IBR_HDR_UART_ID;

From: Pali Rohár pali@kernel.org
Some image types have source address in non-bytes unit; for example for SATA images, it is in 512 B units.
We need to multiply by unit size when patching image type to UART.
Signed-off-by: Pali Rohár pali@kernel.org [ refactored ] Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index c23357f5bc..9cc7936959 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -756,6 +756,7 @@ kwboot_img_patch_hdr(void *img, size_t size) { int rc; struct main_hdr_v1 *hdr; + uint32_t srcaddr; uint8_t csum; size_t hdrsz = sizeof(*hdr); int image_ver; @@ -805,8 +806,6 @@ kwboot_img_patch_hdr(void *img, size_t size) kwboot_printv("Patching image boot signature to UART\n"); }
- hdr->blockid = IBR_HDR_UART_ID; - if (image_ver == 0) { struct main_hdr_v0 *hdr_v0 = img;
@@ -818,6 +817,29 @@ kwboot_img_patch_hdr(void *img, size_t size) : sizeof(*hdr_v0); }
+ srcaddr = le32_to_cpu(hdr->srcaddr); + + switch (hdr->blockid) { + case IBR_HDR_SATA_ID: + if (srcaddr < 1) { + errno = EINVAL; + goto out; + } + hdr->srcaddr = cpu_to_le32((srcaddr - 1) * 512); + break; + + case IBR_HDR_SDIO_ID: + hdr->srcaddr = cpu_to_le32(srcaddr * 512); + break; + + case IBR_HDR_PEX_ID: + if (hdr->srcaddr == 0xFFFFFFFF) + hdr->srcaddr = cpu_to_le32(hdrsz); + break; + } + + hdr->blockid = IBR_HDR_UART_ID; + hdr->checksum = kwboot_img_csum8(hdr, hdrsz) - csum;
rc = 0;

From: Pali Rohár pali@kernel.org
SPI/NOR kwbimage may have destination address set to 0xFFFFFFFF, which means that the image is not downloaded to DDR but rather it is executed directly from SPI/NOR. In this case execution address is set to SPI/NOR area.
When patching image to UART type, change destination and execution addresses from SPI/NOR XIP area to DDR area 0x00800000 (which is default for A38x).
Signed-off-by: Pali Rohár pali@kernel.org [ refactored ] Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 9cc7936959..7c6ea2f75d 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -836,6 +836,13 @@ kwboot_img_patch_hdr(void *img, size_t size) if (hdr->srcaddr == 0xFFFFFFFF) hdr->srcaddr = cpu_to_le32(hdrsz); break; + + case IBR_HDR_SPI_ID: + if (hdr->destaddr == 0xFFFFFFFF) { + kwboot_printv("Patching destination and execution addresses from SPI/NOR XIP area to DDR area 0x00800000\n"); + hdr->destaddr = cpu_to_le32(0x00800000); + hdr->execaddr = cpu_to_le32(0x00800000); + } }
hdr->blockid = IBR_HDR_UART_ID;

Rename this function to kwbimage_version() and don't cast argument if not needed.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwbimage.c | 8 ++++---- tools/kwbimage.h | 4 ++-- tools/kwboot.c | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index 6678ef15bd..e98ef455de 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -284,7 +284,7 @@ static uint8_t image_checksum8(void *start, uint32_t len)
size_t kwbimage_header_size(unsigned char *ptr) { - if (image_version((void *)ptr) == 0) + if (kwbimage_version((void *)ptr) == 0) return sizeof(struct main_hdr_v0); else return KWBHEADER_V1_SIZE((struct main_hdr_v1 *)ptr); @@ -1624,7 +1624,7 @@ static void kwbimage_print_header(const void *ptr)
printf("Image Type: MVEBU Boot from %s Image\n", image_boot_mode_name(mhdr->blockid)); - printf("Image version:%d\n", image_version((void *)ptr)); + printf("Image version:%d\n", kwbimage_version(ptr));
for_each_opt_hdr_v1 (ohdr, mhdr) { if (ohdr->headertype == OPT_HDR_V1_BINARY_TYPE) { @@ -1661,7 +1661,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, return -FDT_ERR_BADSTRUCTURE;
/* Only version 0 extended header has checksum */ - if (image_version((void *)ptr) == 0) { + if (kwbimage_version(ptr) == 0) { struct main_hdr_v0 *mhdr = (struct main_hdr_v0 *)ptr;
if (mhdr->ext & 0x1) { @@ -1678,7 +1678,7 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, if (checksum != ext_hdr->checksum) return -FDT_ERR_BADSTRUCTURE; } - } else if (image_version((void *)ptr) == 1) { + } else if (kwbimage_version(ptr) == 1) { struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr; const uint8_t *mhdr_end; struct opt_hdr_v1 *ohdr; diff --git a/tools/kwbimage.h b/tools/kwbimage.h index 9a949e03c0..738034beb1 100644 --- a/tools/kwbimage.h +++ b/tools/kwbimage.h @@ -229,7 +229,7 @@ void init_kwb_image_type (void); * header, byte 8 was reserved, and always set to 0. In the v1 header, * byte 8 has been changed to a proper field, set to 1. */ -static inline unsigned int image_version(const void *header) +static inline unsigned int kwbimage_version(const void *header) { const unsigned char *ptr = header; return ptr[8]; @@ -258,7 +258,7 @@ static inline int opt_hdr_v1_valid_size(const struct opt_hdr_v1 *ohdr, static inline struct opt_hdr_v1 *opt_hdr_v1_first(void *img) { struct main_hdr_v1 *mhdr;
- if (image_version(img) != 1) + if (kwbimage_version(img) != 1) return NULL;
mhdr = img; diff --git a/tools/kwboot.c b/tools/kwboot.c index 7c6ea2f75d..f3afe2d383 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -565,7 +565,7 @@ kwboot_xmodem(int tty, const void *_img, size_t size) int rc, pnum; size_t hdrsz;
- if (image_version(img) == 0) + if (kwbimage_version(img) == 0) hdrsz = KWBHEADER_V0_SIZE((struct main_hdr_v0 *)img); else hdrsz = KWBHEADER_V1_SIZE((struct main_hdr_v1 *)img); @@ -771,7 +771,7 @@ kwboot_img_patch_hdr(void *img, size_t size) goto out; }
- image_ver = image_version(img); + image_ver = kwbimage_version(img); if (image_ver != 0 && image_ver != 1) { fprintf(stderr, "Invalid image header version\n"); errno = EINVAL;

Add functions kwbheader_size() and kwbheader_size_for_csum().
Refactor code determining header size to use these functions.
Refactor header checksum determining function.
Remove stuff that is not needed anymore.
This simplifies the code a little and fixes one instance of validating header size meant for checksum instead of whole header size.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwbimage.c | 29 +++++++---------------------- tools/kwbimage.h | 35 +++++++++++++++++++++++------------ tools/kwboot.c | 26 +++++++++++--------------- 3 files changed, 41 insertions(+), 49 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index e98ef455de..d3a52df736 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -282,14 +282,6 @@ static uint8_t image_checksum8(void *start, uint32_t len) return csum; }
-size_t kwbimage_header_size(unsigned char *ptr) -{ - if (kwbimage_version((void *)ptr) == 0) - return sizeof(struct main_hdr_v0); - else - return KWBHEADER_V1_SIZE((struct main_hdr_v1 *)ptr); -} - /* * Verify checksum over a complete header that includes the checksum field. * Return 1 when OK, otherwise 0. @@ -300,7 +292,7 @@ static int main_hdr_checksum_ok(void *hdr) struct main_hdr_v0 *main_hdr = (struct main_hdr_v0 *)hdr; uint8_t checksum;
- checksum = image_checksum8(hdr, kwbimage_header_size(hdr)); + checksum = image_checksum8(hdr, kwbheader_size_for_csum(hdr)); /* Calculated checksum includes the header checksum field. Compensate * for that. */ @@ -1651,8 +1643,8 @@ static int kwbimage_check_image_types(uint8_t type) static int kwbimage_verify_header(unsigned char *ptr, int image_size, struct image_tool_params *params) { - uint8_t checksum; - size_t header_size = kwbimage_header_size(ptr); + size_t header_size = kwbheader_size(ptr); + uint8_t csum;
if (header_size > image_size) return -FDT_ERR_BADSTRUCTURE; @@ -1665,17 +1657,10 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, struct main_hdr_v0 *mhdr = (struct main_hdr_v0 *)ptr;
if (mhdr->ext & 0x1) { - struct ext_hdr_v0 *ext_hdr; - - if (header_size + sizeof(*ext_hdr) > image_size) - return -FDT_ERR_BADSTRUCTURE; + struct ext_hdr_v0 *ext_hdr = (void *)(mhdr + 1);
- ext_hdr = (struct ext_hdr_v0 *) - (ptr + sizeof(struct main_hdr_v0)); - checksum = image_checksum8(ext_hdr, - sizeof(struct ext_hdr_v0) - - sizeof(uint8_t)); - if (checksum != ext_hdr->checksum) + csum = image_checksum8(ext_hdr, sizeof(*ext_hdr) - 1); + if (csum != ext_hdr->checksum) return -FDT_ERR_BADSTRUCTURE; } } else if (kwbimage_version(ptr) == 1) { @@ -1834,7 +1819,7 @@ static int kwbimage_generate(struct image_tool_params *params, static int kwbimage_extract_subimage(void *ptr, struct image_tool_params *params) { struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr; - size_t header_size = kwbimage_header_size(ptr); + size_t header_size = kwbheader_size(ptr); struct opt_hdr_v1 *ohdr; int idx = params->pflag; int cur_idx = 0; diff --git a/tools/kwbimage.h b/tools/kwbimage.h index 738034beb1..56a748d4cf 100644 --- a/tools/kwbimage.h +++ b/tools/kwbimage.h @@ -69,11 +69,6 @@ struct ext_hdr_v0 { uint8_t checksum; } __packed;
-struct kwb_header { - struct main_hdr_v0 kwb_hdr; - struct ext_hdr_v0 kwb_exthdr; -} __packed; - /* Structure of the main header, version 1 (Armada 370/38x/XP) */ struct main_hdr_v1 { uint8_t blockid; /* 0x0 */ @@ -195,13 +190,6 @@ struct register_set_hdr_v1 { #define OPT_HDR_V1_BINARY_TYPE 0x2 #define OPT_HDR_V1_REGISTER_TYPE 0x3
-#define KWBHEADER_V0_SIZE(hdr) \ - (((hdr)->ext & 0x1) ? sizeof(struct kwb_header) : \ - sizeof(struct main_hdr_v0)) - -#define KWBHEADER_V1_SIZE(hdr) \ - (((hdr)->headersz_msb << 16) | le16_to_cpu((hdr)->headersz_lsb)) - enum kwbimage_cmd { CMD_INVALID, CMD_BOOT_FROM, @@ -235,6 +223,29 @@ static inline unsigned int kwbimage_version(const void *header) return ptr[8]; }
+static inline size_t kwbheader_size(const void *header) +{ + if (kwbimage_version(header) == 0) { + const struct main_hdr_v0 *hdr = header; + + return sizeof(*hdr) + + (hdr->ext & 0x1) ? sizeof(struct ext_hdr_v0) : 0; + } else { + const struct main_hdr_v1 *hdr = header; + + return (hdr->headersz_msb << 16) | + le16_to_cpu(hdr->headersz_lsb); + } +} + +static inline size_t kwbheader_size_for_csum(const void *header) +{ + if (kwbimage_version(header) == 0) + return sizeof(struct main_hdr_v0); + else + return kwbheader_size(header); +} + static inline uint32_t opt_hdr_v1_size(const struct opt_hdr_v1 *ohdr) { return (ohdr->headersz_msb << 16) | le16_to_cpu(ohdr->headersz_lsb); diff --git a/tools/kwboot.c b/tools/kwboot.c index f3afe2d383..1481735346 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -565,10 +565,7 @@ kwboot_xmodem(int tty, const void *_img, size_t size) int rc, pnum; size_t hdrsz;
- if (kwbimage_version(img) == 0) - hdrsz = KWBHEADER_V0_SIZE((struct main_hdr_v0 *)img); - else - hdrsz = KWBHEADER_V1_SIZE((struct main_hdr_v1 *)img); + hdrsz = kwbheader_size(img);
kwboot_printv("Waiting 2s and flushing tty\n"); sleep(2); /* flush isn't effective without it */ @@ -729,9 +726,13 @@ out: }
static uint8_t -kwboot_img_csum8(void *_data, size_t size) +kwboot_hdr_csum8(const void *hdr) { - uint8_t *data = _data, csum; + const uint8_t *data = hdr; + uint8_t csum; + size_t size; + + size = kwbheader_size_for_csum(hdr);
for (csum = 0; size-- > 0; data++) csum += *data; @@ -778,17 +779,14 @@ kwboot_img_patch_hdr(void *img, size_t size) goto out; }
- if (image_ver == 0) - hdrsz = sizeof(*hdr); - else - hdrsz = KWBHEADER_V1_SIZE(hdr); + hdrsz = kwbheader_size(hdr);
if (size < hdrsz) { errno = EINVAL; goto out; }
- csum = kwboot_img_csum8(hdr, hdrsz) - hdr->checksum; + csum = kwboot_hdr_csum8(hdr) - hdr->checksum; if (csum != hdr->checksum) { errno = EINVAL; goto out; @@ -812,9 +810,7 @@ kwboot_img_patch_hdr(void *img, size_t size) hdr_v0->nandeccmode = IBR_HDR_ECC_DISABLED; hdr_v0->nandpagesize = 0;
- hdr_v0->srcaddr = hdr_v0->ext - ? sizeof(struct kwb_header) - : sizeof(*hdr_v0); + hdr_v0->srcaddr = cpu_to_le32(hdrsz); }
srcaddr = le32_to_cpu(hdr->srcaddr); @@ -847,7 +843,7 @@ kwboot_img_patch_hdr(void *img, size_t size)
hdr->blockid = IBR_HDR_UART_ID;
- hdr->checksum = kwboot_img_csum8(hdr, hdrsz) - csum; + hdr->checksum = kwboot_hdr_csum8(hdr) - csum;
rc = 0; out:

From: Pali Rohár pali@kernel.org
The beginning of image data must be sent in a separate xmodem block; the block must not contain end of header with the beginning of data.
Therefore we need to ensure that the image header size is a multiple of xmodem block size (which is 128 B).
Read the file into a malloc()ed buffer of enough size instead of mmap()ing it. (If we are going to move the data, most of the pages will be dirty anyway.) Then move the payload if header size needs to be increased.
Signed-off-by: Pali Rohár pali@kernel.org [ refactored ] Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 91 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 79 insertions(+), 12 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 1481735346..6d7d812416 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -25,7 +25,6 @@ #include <stdint.h> #include <termios.h> #include <time.h> -#include <sys/mman.h> #include <sys/stat.h>
/* @@ -689,11 +688,12 @@ out: }
static void * -kwboot_mmap_image(const char *path, size_t *size) +kwboot_read_image(const char *path, size_t *size, size_t reserve) { int rc, fd; struct stat st; void *img; + off_t tot;
rc = -1; img = NULL; @@ -706,17 +706,30 @@ kwboot_mmap_image(const char *path, size_t *size) if (rc) goto out;
- img = mmap(NULL, st.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); - if (img == MAP_FAILED) { - img = NULL; + img = malloc(st.st_size + reserve); + if (!img) goto out; + + tot = 0; + while (tot < st.st_size) { + ssize_t rd = read(fd, img + tot, st.st_size - tot); + + if (rd < 0) + goto out; + + tot += rd; + + if (!rd && tot < st.st_size) { + errno = EIO; + goto out; + } }
rc = 0; *size = st.st_size; out: if (rc && img) { - munmap(img, st.st_size); + free(img); img = NULL; } if (fd >= 0) @@ -752,8 +765,41 @@ kwboot_img_is_secure(void *img) return 0; }
+static void +kwboot_img_grow_hdr(void *img, size_t *size, size_t grow) +{ + uint32_t hdrsz, datasz, srcaddr; + struct main_hdr_v1 *hdr = img; + uint8_t *data; + + srcaddr = le32_to_cpu(hdr->srcaddr); + + hdrsz = kwbheader_size(hdr); + data = (uint8_t *)img + srcaddr; + datasz = *size - srcaddr; + + /* only move data if there is not enough space */ + if (hdrsz + grow > srcaddr) { + size_t need = hdrsz + grow - srcaddr; + + /* move data by enough bytes */ + memmove(data + need, data, datasz); + + hdr->srcaddr = cpu_to_le32(srcaddr + need); + *size += need; + } + + if (kwbimage_version(img) == 1) { + struct main_hdr_v1 *hdr = img; + + hdrsz += grow; + hdr->headersz_msb = hdrsz >> 16; + hdr->headersz_lsb = cpu_to_le16(hdrsz & 0xffff); + } +} + static int -kwboot_img_patch_hdr(void *img, size_t size) +kwboot_img_patch_hdr(void *img, size_t *size) { int rc; struct main_hdr_v1 *hdr; @@ -767,7 +813,7 @@ kwboot_img_patch_hdr(void *img, size_t size) rc = -1; hdr = img;
- if (size < hdrsz) { + if (*size < hdrsz) { errno = EINVAL; goto out; } @@ -781,7 +827,7 @@ kwboot_img_patch_hdr(void *img, size_t size)
hdrsz = kwbheader_size(hdr);
- if (size < hdrsz) { + if (*size < hdrsz) { errno = EINVAL; goto out; } @@ -841,10 +887,31 @@ kwboot_img_patch_hdr(void *img, size_t size) } }
+ if (hdrsz > le32_to_cpu(hdr->srcaddr) || + *size < le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize)) { + errno = EINVAL; + goto out; + } + hdr->blockid = IBR_HDR_UART_ID;
+ if (hdrsz % KWBOOT_XM_BLKSZ) { + size_t offset = (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % + KWBOOT_XM_BLKSZ; + + if (is_secure) { + fprintf(stderr, "Cannot align image with secure header\n"); + errno = EINVAL; + goto out; + } + + kwboot_printv("Aligning image header to Xmodem block size\n"); + kwboot_img_grow_hdr(img, size, offset); + } + hdr->checksum = kwboot_hdr_csum8(hdr) - csum;
+ *size = le32_to_cpu(hdr->srcaddr) + le32_to_cpu(hdr->blocksize); rc = 0; out: return rc; @@ -971,13 +1038,13 @@ main(int argc, char **argv) }
if (imgpath) { - img = kwboot_mmap_image(imgpath, &size); + img = kwboot_read_image(imgpath, &size, KWBOOT_XM_BLKSZ); if (!img) { perror(imgpath); goto out; }
- rc = kwboot_img_patch_hdr(img, size); + rc = kwboot_img_patch_hdr(img, &size); if (rc) { fprintf(stderr, "%s: Invalid image.\n", imgpath); goto out; @@ -1020,7 +1087,7 @@ out: close(tty);
if (img) - munmap(img, size); + free(img);
return rv;

From: Pali Rohár pali@kernel.org
Add support for uploading the boot image (the data part only) at higher baudrate than the standard one.
The kwboot utility already has -B option, but choosing other baudrate than the standard one (115200 Bd) can only work for debug mode, not for booting the device. The BootROM for kwboot supported platforms (Orion, Kirkwood, Dove, Discovery, AXP, A37x, A38x, A39x) cannot change the baudrate when uploading boot image via the Xmodem protocol, nor can it be configured via strapping pins.
So instead we add this support by injecting baudrate changing code into the kwbimage v1 header as a new optional binary extension. This code is executed by BootROM after it receives the whole header. The code sends the magic string "$baudratechange\0" just before changing the baudrate to let kwboot know that it should also change it. This is because the injected code is run as the last binary extension, and we do not want to loose possible output from other possible binary extensions that came before it (in most cases this is U-Boot SPL).
We also inject the code before the payload (the data part of the image), to change the baudrate back to the standard value, in case the payload does not reset UART.
This change improves boot time via UART significantly (depending on the chosen baudrate), which is very useful when debugging.
Signed-off-by: Pali Rohár pali@kernel.org [ major refactor ] Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 617 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 589 insertions(+), 28 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c index 6d7d812416..dbe59833eb 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -70,6 +70,186 @@ struct kwboot_block { #define KWBOOT_BLK_RSP_TIMEO 1000 /* ms */ #define KWBOOT_HDR_RSP_TIMEO 10000 /* ms */
+/* ARM code making baud rate changing function return to _start. */ +static unsigned char kwboot_pre_baud_code[] = { + 0xf8, 0xe0, 0x8f, 0xe2, /* adr lr, _start */ +}; + +/* ARM code for binary header injection to change baudrate */ +static unsigned char kwboot_baud_code[] = { + /* ; #define UART_BASE 0xd0012000 */ + /* ; #define THR 0x00 */ + /* ; #define DLL 0x00 */ + /* ; #define DLH 0x04 */ + /* ; #define LCR 0x0c */ + /* ; #define DLAB 0x80 */ + /* ; #define LSR 0x14 */ + /* ; #define THRE 0x20 */ + /* ; #define TEMT 0x40 */ + /* ; #define DIV_ROUND(a, b) ((a + b/2) / b) */ + /* ; */ + /* ; u32 set_baudrate(u32 old_b, u32 new_b) { */ + /* ; const u8 *str = "$baudratechange"; */ + /* ; u8 c; */ + /* ; do { */ + /* ; c = *str++; */ + /* ; writel(UART_BASE + THR, c); */ + /* ; } while (c); */ + /* ; while */ + /* ; (!(readl(UART_BASE + LSR) & TEMT)); */ + /* ; u32 lcr = readl(UART_BASE + LCR); */ + /* ; writel(UART_BASE + LCR, lcr | DLAB); */ + /* ; u8 old_dll = readl(UART_BASE + DLL); */ + /* ; u8 old_dlh = readl(UART_BASE + DLH); */ + /* ; u16 old_dl = old_dll | (old_dlh << 8); */ + /* ; u32 clk = old_b * old_dl; */ + /* ; u16 new_dl = DIV_ROUND(clk, new_b); */ + /* ; u8 new_dll = new_dl & 0xff; */ + /* ; u8 new_dlh = (new_dl >> 8) & 0xff; */ + /* ; writel(UART_BASE + DLL, new_dll); */ + /* ; writel(UART_BASE + DLH, new_dlh); */ + /* ; writel(UART_BASE + LCR, lcr & ~DLAB); */ + /* ; msleep(1); */ + /* ; return 0; */ + /* ; } */ + + 0xfe, 0x5f, 0x2d, 0xe9, /* push { r1 - r12, lr } */ + + /* ; r0 = UART_BASE */ + 0x02, 0x0a, 0xa0, 0xe3, /* mov r0, #0x2000 */ + 0x01, 0x00, 0x4d, 0xe3, /* movt r0, #0xd001 */ + + /* ; r2 = address of preamble string */ + 0xd0, 0x20, 0x8f, 0xe2, /* adr r2, preamble */ + + /* ; Send preamble string over UART */ + /* .Lloop_preamble: */ + /* */ + /* ; Wait until Transmitter Holding is Empty */ + /* .Lloop_thre: */ + /* ; r1 = UART_BASE[LSR] & THRE */ + 0x14, 0x10, 0x90, 0xe5, /* ldr r1, [r0, #0x14] */ + 0x20, 0x00, 0x11, 0xe3, /* tst r1, #0x20 */ + 0xfc, 0xff, 0xff, 0x0a, /* beq .Lloop_thre */ + + /* ; Put character into Transmitter FIFO */ + /* ; r1 = *r2++ */ + 0x01, 0x10, 0xd2, 0xe4, /* ldrb r1, [r2], #1 */ + /* ; UART_BASE[THR] = r1 */ + 0x00, 0x10, 0x80, 0xe5, /* str r1, [r0, #0x0] */ + + /* ; Loop until end of preamble string */ + 0x00, 0x00, 0x51, 0xe3, /* cmp r1, #0 */ + 0xf8, 0xff, 0xff, 0x1a, /* bne .Lloop_preamble */ + + /* ; Wait until Transmitter FIFO is Empty */ + /* .Lloop_txempty: */ + /* ; r1 = UART_BASE[LSR] & TEMT */ + 0x14, 0x10, 0x90, 0xe5, /* ldr r1, [r0, #0x14] */ + 0x40, 0x00, 0x11, 0xe3, /* tst r1, #0x40 */ + 0xfc, 0xff, 0xff, 0x0a, /* beq .Lloop_txempty */ + + /* ; Set Divisor Latch Access Bit */ + /* ; UART_BASE[LCR] |= DLAB */ + 0x0c, 0x10, 0x90, 0xe5, /* ldr r1, [r0, #0x0c] */ + 0x80, 0x10, 0x81, 0xe3, /* orr r1, r1, #0x80 */ + 0x0c, 0x10, 0x80, 0xe5, /* str r1, [r0, #0x0c] */ + + /* ; Read current Divisor Latch */ + /* ; r1 = UART_BASE[DLH]<<8 | UART_BASE[DLL] */ + 0x00, 0x10, 0x90, 0xe5, /* ldr r1, [r0, #0x00] */ + 0xff, 0x10, 0x01, 0xe2, /* and r1, r1, #0xff */ + 0x01, 0x20, 0xa0, 0xe1, /* mov r2, r1 */ + 0x04, 0x10, 0x90, 0xe5, /* ldr r1, [r0, #0x04] */ + 0xff, 0x10, 0x01, 0xe2, /* and r1, r1, #0xff */ + 0x41, 0x14, 0xa0, 0xe1, /* asr r1, r1, #8 */ + 0x02, 0x10, 0x81, 0xe1, /* orr r1, r1, r2 */ + + /* ; Read old baudrate value */ + /* ; r2 = old_baudrate */ + 0x8c, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */ + + /* ; Calculate base clock */ + /* ; r1 = r2 * r1 */ + 0x92, 0x01, 0x01, 0xe0, /* mul r1, r2, r1 */ + + /* ; Read new baudrate value */ + /* ; r2 = baudrate */ + 0x88, 0x20, 0x9f, 0xe5, /* ldr r2, baudrate */ + + /* ; Calculate new Divisor Latch */ + /* ; r1 = DIV_ROUND(r1, r2) = */ + /* ; = (r1 + r2/2) / r2 */ + 0xa2, 0x10, 0x81, 0xe0, /* add r1, r1, r2, lsr #1 */ + 0x02, 0x40, 0xa0, 0xe1, /* mov r4, r2 */ + 0xa1, 0x00, 0x54, 0xe1, /* cmp r4, r1, lsr #1 */ + /* .Lloop_div1: */ + 0x84, 0x40, 0xa0, 0x91, /* movls r4, r4, lsl #1 */ + 0xa1, 0x00, 0x54, 0xe1, /* cmp r4, r1, lsr #1 */ + 0xfc, 0xff, 0xff, 0x9a, /* bls .Lloop_div1 */ + 0x00, 0x30, 0xa0, 0xe3, /* mov r3, #0 */ + /* .Lloop_div2: */ + 0x04, 0x00, 0x51, 0xe1, /* cmp r1, r4 */ + 0x04, 0x10, 0x41, 0x20, /* subhs r1, r1, r4 */ + 0x03, 0x30, 0xa3, 0xe0, /* adc r3, r3, r3 */ + 0xa4, 0x40, 0xa0, 0xe1, /* mov r4, r4, lsr #1 */ + 0x02, 0x00, 0x54, 0xe1, /* cmp r4, r2 */ + 0xf9, 0xff, 0xff, 0x2a, /* bhs .Lloop_div2 */ + 0x03, 0x10, 0xa0, 0xe1, /* mov r1, r3 */ + + /* ; Set new Divisor Latch Low */ + /* ; UART_BASE[DLL] = r1 & 0xff */ + 0x01, 0x20, 0xa0, 0xe1, /* mov r2, r1 */ + 0xff, 0x20, 0x02, 0xe2, /* and r2, r2, #0xff */ + 0x00, 0x20, 0x80, 0xe5, /* str r2, [r0, #0x00] */ + + /* ; Set new Divisor Latch High */ + /* ; UART_BASE[DLH] = r1>>8 & 0xff */ + 0x41, 0x24, 0xa0, 0xe1, /* asr r2, r1, #8 */ + 0xff, 0x20, 0x02, 0xe2, /* and r2, r2, #0xff */ + 0x04, 0x20, 0x80, 0xe5, /* str r2, [r0, #0x04] */ + + /* ; Clear Divisor Latch Access Bit */ + /* ; UART_BASE[LCR] &= ~DLAB */ + 0x0c, 0x10, 0x90, 0xe5, /* ldr r1, [r0, #0x0c] */ + 0x80, 0x10, 0xc1, 0xe3, /* bic r1, r1, #0x80 */ + 0x0c, 0x10, 0x80, 0xe5, /* str r1, [r0, #0x0c] */ + + /* ; Sleep 1ms ~~ 600000 cycles at 1200 MHz */ + /* ; r1 = 600000 */ + 0x9f, 0x1d, 0xa0, 0xe3, /* mov r1, #0x27c0 */ + 0x09, 0x10, 0x40, 0xe3, /* movt r1, #0x0009 */ + /* .Lloop_sleep: */ + 0x01, 0x10, 0x41, 0xe2, /* sub r1, r1, #1 */ + 0x00, 0x00, 0x51, 0xe3, /* cmp r1, #0 */ + 0xfc, 0xff, 0xff, 0x1a, /* bne .Lloop_sleep */ + + /* ; Return 0 - no error */ + 0x00, 0x00, 0xa0, 0xe3, /* mov r0, #0 */ + 0xfe, 0x9f, 0xbd, 0xe8, /* pop { r1 - r12, pc } */ + + /* ; Preamble string */ + /* preamble: */ + 0x24, 0x62, 0x61, 0x75, /* .asciz "$baudratechange" */ + 0x64, 0x72, 0x61, 0x74, + 0x65, 0x63, 0x68, 0x61, + 0x6e, 0x67, 0x65, 0x00, + + /* ; Placeholder for old baudrate value */ + /* old_baudrate: */ + 0x00, 0x00, 0x00, 0x00, /* .word 0 */ + + /* ; Placeholder for new baudrate value */ + /* new_baudrate: */ + 0x00, 0x00, 0x00, 0x00, /* .word 0 */ + /* _start: */ +}; + +#define KWBOOT_BAUDRATE_BIN_HEADER_SZ (sizeof(kwboot_baud_code) + \ + sizeof(struct opt_hdr_v1) + 8) + +static const char kwb_baud_magic[16] = "$baudratechange"; + static int kwboot_verbose;
static int msg_req_delay = KWBOOT_MSG_REQ_DELAY; @@ -233,26 +413,184 @@ kwboot_tty_send_char(int fd, unsigned char c) }
static speed_t -kwboot_tty_speed(int baudrate) +kwboot_tty_baudrate_to_speed(int baudrate) { switch (baudrate) { +#ifdef B4000000 + case 4000000: + return B4000000; +#endif +#ifdef B3500000 + case 3500000: + return B3500000; +#endif +#ifdef B3000000 + case 3000000: + return B3000000; +#endif +#ifdef B2500000 + case 2500000: + return B2500000; +#endif +#ifdef B2000000 + case 2000000: + return B2000000; +#endif +#ifdef B1500000 + case 1500000: + return B1500000; +#endif +#ifdef B1152000 + case 1152000: + return B1152000; +#endif +#ifdef B1000000 + case 1000000: + return B1000000; +#endif +#ifdef B921600 + case 921600: + return B921600; +#endif +#ifdef B614400 + case 614400: + return B614400; +#endif +#ifdef B576000 + case 576000: + return B576000; +#endif +#ifdef B500000 + case 500000: + return B500000; +#endif +#ifdef B460800 + case 460800: + return B460800; +#endif +#ifdef B307200 + case 307200: + return B307200; +#endif +#ifdef B230400 + case 230400: + return B230400; +#endif +#ifdef B153600 + case 153600: + return B153600; +#endif +#ifdef B115200 case 115200: return B115200; +#endif +#ifdef B76800 + case 76800: + return B76800; +#endif +#ifdef B57600 case 57600: return B57600; +#endif +#ifdef B38400 case 38400: return B38400; +#endif +#ifdef B19200 case 19200: return B19200; +#endif +#ifdef B9600 case 9600: return B9600; +#endif +#ifdef B4800 + case 4800: + return B4800; +#endif +#ifdef B2400 + case 2400: + return B2400; +#endif +#ifdef B1800 + case 1800: + return B1800; +#endif +#ifdef B1200 + case 1200: + return B1200; +#endif +#ifdef B600 + case 600: + return B600; +#endif +#ifdef B300 + case 300: + return B300; +#endif +#ifdef B200 + case 200: + return B200; +#endif +#ifdef B150 + case 150: + return B150; +#endif +#ifdef B134 + case 134: + return B134; +#endif +#ifdef B110 + case 110: + return B110; +#endif +#ifdef B75 + case 75: + return B75; +#endif +#ifdef B50 + case 50: + return B50; +#endif + default: + return B0; } +} + +static int +kwboot_tty_change_baudrate(int fd, int baudrate) +{ + struct termios tio; + speed_t speed; + int rc; + + rc = tcgetattr(fd, &tio); + if (rc) + return rc;
- return -1; + speed = kwboot_tty_baudrate_to_speed(baudrate); + if (speed == B0) { + errno = EINVAL; + return -1; + } + + rc = cfsetospeed(&tio, speed); + if (rc) + return rc; + + rc = cfsetispeed(&tio, speed); + if (rc) + return rc; + + rc = tcsetattr(fd, TCSANOW, &tio); + if (rc) + return rc; + + return 0; }
static int -kwboot_open_tty(const char *path, speed_t speed) +kwboot_open_tty(const char *path, int baudrate) { int rc, fd; struct termios tio; @@ -271,13 +609,14 @@ kwboot_open_tty(const char *path, speed_t speed) tio.c_cc[VMIN] = 1; tio.c_cc[VTIME] = 10;
- cfsetospeed(&tio, speed); - cfsetispeed(&tio, speed); - rc = tcsetattr(fd, TCSANOW, &tio); if (rc) goto out;
+ rc = kwboot_tty_change_baudrate(fd, baudrate); + if (rc) + goto out; + rc = fd; out: if (rc < 0) { @@ -424,13 +763,40 @@ _now(void) return ts.tv_sec * 1000 + (ts.tv_nsec + 500000) / 1000000; }
+static int +kwboot_baud_magic_handle(int fd, char c, int baudrate) +{ + static size_t rcv_len; + + if (rcv_len < sizeof(kwb_baud_magic)) { + /* try to recognize whole magic word */ + if (c == kwb_baud_magic[rcv_len]) { + rcv_len++; + } else { + printf("%.*s%c", (int)rcv_len, kwb_baud_magic, c); + fflush(stdout); + rcv_len = 0; + } + } + + if (rcv_len == sizeof(kwb_baud_magic)) { + /* magic word received */ + kwboot_printv("\nChanging baudrate to %d Bd\n", baudrate); + + return kwboot_tty_change_baudrate(fd, baudrate) ? : 1; + } else { + return 0; + } +} + static int kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, - int *done_print) + int *done_print, int baudrate) { int recv_timeo = allow_non_xm ? KWBOOT_HDR_RSP_TIMEO : blk_rsp_timeo; int non_xm_print = 0; uint64_t timeout = 0; + int baud_changed = 0; int rc, retries; int err; char c; @@ -454,21 +820,41 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, rc = kwboot_tty_recv(fd, &c, 1, recv_timeo); if (rc) { if (errno != ETIMEDOUT) { - return rc; + goto can; } else if (timeout && timeout < _now()) { errno = ETIMEDOUT; - return -1; + goto can; } else { err = errno; c = NAK; } }
+ /* + * After whole header is sent, but before ACK of the last + * block is received, we receive possible UART output + * from header, which we want to print, unless it is the + * magic string informing us about baudrate change (as + * was requested via -B option). So we need to search for + * this magic string, and when found, not print it, but + * instead change the baudrate. + */ if (!_is_xm_reply(c) && allow_non_xm) { timeout = _now() + recv_timeo; - putchar(c); - fflush(stdout); - non_xm_print = 1; + if (baudrate && !baud_changed) { + rc = kwboot_baud_magic_handle(fd, c, + baudrate); + if (rc == 1) + baud_changed = 1; + else if (!rc) + non_xm_print = 1; + else + goto can; + } else if (!baudrate || !baud_changed) { + putchar(c); + fflush(stdout); + non_xm_print = 1; + } } } while (!_is_xm_reply(c));
@@ -484,7 +870,20 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, return -1; }
+ if (allow_non_xm && baudrate && !baud_changed) { + fprintf(stderr, "Baudrate was not changed\n"); + rc = -1; + errno = EPROTO; + goto can; + } + return _xm_reply_to_error(c); +can: + err = errno; + kwboot_tty_send_char(fd, CAN); + kwboot_printv("\n"); + errno = err; + return rc; }
static int @@ -514,7 +913,7 @@ kwboot_xm_finish(int tty)
static int kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data, - size_t size) + size_t size, int baudrate) { int done_print = 0; size_t sent, left; @@ -537,7 +936,7 @@ kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data, last_block = (left <= blksz);
rc = kwboot_xm_sendblock(tty, &block, header && last_block, - &done_print); + &done_print, baudrate); if (rc) goto out;
@@ -558,7 +957,7 @@ out: }
static int -kwboot_xmodem(int tty, const void *_img, size_t size) +kwboot_xmodem(int tty, const void *_img, size_t size, int baudrate) { const uint8_t *img = _img; int rc, pnum; @@ -572,19 +971,42 @@ kwboot_xmodem(int tty, const void *_img, size_t size)
pnum = 1;
- rc = kwboot_xmodem_one(tty, &pnum, 1, img, hdrsz); + rc = kwboot_xmodem_one(tty, &pnum, 1, img, hdrsz, baudrate); if (rc) return rc;
img += hdrsz; size -= hdrsz;
- rc = kwboot_xmodem_one(tty, &pnum, 0, img, size); + rc = kwboot_xmodem_one(tty, &pnum, 0, img, size, 0); if (rc) return rc;
kwboot_printv("Finishing transfer\n"); - return kwboot_xm_finish(tty); + rc = kwboot_xm_finish(tty); + if (rc) + return rc; + + if (baudrate) { + char buf[sizeof(kwb_baud_magic)]; + + /* Wait 1s for baudrate change magic */ + rc = kwboot_tty_recv(tty, buf, sizeof(buf), 1000); + if (rc) + return rc; + + if (memcmp(buf, kwb_baud_magic, sizeof(buf))) { + errno = EPROTO; + return -1; + } + + kwboot_printv("\nChanging baudrate back to 115200 Bd\n\n"); + rc = kwboot_tty_change_baudrate(tty, 115200); + if (rc) + return rc; + } + + return 0; }
static int @@ -765,6 +1187,37 @@ kwboot_img_is_secure(void *img) return 0; }
+static void * +kwboot_img_grow_data_left(void *img, size_t *size, size_t grow) +{ + uint32_t hdrsz, datasz, srcaddr; + struct main_hdr_v1 *hdr = img; + uint8_t *data; + + srcaddr = le32_to_cpu(hdr->srcaddr); + + hdrsz = kwbheader_size(hdr); + data = (uint8_t *)img + srcaddr; + datasz = *size - srcaddr; + + /* only move data if there is not enough space */ + if (hdrsz + grow > srcaddr) { + size_t need = hdrsz + grow - srcaddr; + + /* move data by enough bytes */ + memmove(data + need, data, datasz); + *size += need; + srcaddr += need; + } + + srcaddr -= grow; + hdr->srcaddr = cpu_to_le32(srcaddr); + hdr->destaddr = cpu_to_le32(le32_to_cpu(hdr->destaddr) - grow); + hdr->execaddr = cpu_to_le32(le32_to_cpu(hdr->execaddr) - grow); + + return (uint8_t *)img + srcaddr; +} + static void kwboot_img_grow_hdr(void *img, size_t *size, size_t grow) { @@ -798,8 +1251,62 @@ kwboot_img_grow_hdr(void *img, size_t *size, size_t grow) } }
+static void * +kwboot_add_bin_ohdr_v1(void *img, size_t *size, uint32_t binsz) +{ + struct main_hdr_v1 *hdr = img; + struct opt_hdr_v1 *ohdr; + uint32_t ohdrsz; + + ohdrsz = binsz + 8 + sizeof(*ohdr); + kwboot_img_grow_hdr(img, size, ohdrsz); + + if (hdr->ext & 0x1) { + for_each_opt_hdr_v1 (ohdr, img) + if (opt_hdr_v1_next(ohdr) == NULL) + break; + + *opt_hdr_v1_ext(ohdr) |= 1; + ohdr = opt_hdr_v1_next(ohdr); + } else { + hdr->ext |= 1; + ohdr = (void *)(hdr + 1); + } + + ohdr->headertype = OPT_HDR_V1_BINARY_TYPE; + ohdr->headersz_msb = ohdrsz >> 16; + ohdr->headersz_lsb = cpu_to_le16(ohdrsz & 0xffff); + + memset(&ohdr->data[0], 0, ohdrsz - sizeof(*ohdr)); + + return &ohdr->data[4]; +} + +static void +_copy_baudrate_change_code(void *dst, int pre, int old_baud, int new_baud) +{ + size_t codesz = sizeof(kwboot_baud_code); + uint8_t *code = dst; + + if (pre) { + size_t presz = sizeof(kwboot_pre_baud_code); + + /* + * We need to prepend code that loads lr register with value + * just after the baudrate change code so that when baud rate + * changing function returns, it returns to original payload. + */ + memcpy(code, kwboot_pre_baud_code, presz); + code += presz; + } + + memcpy(code, kwboot_baud_code, codesz - 8); + *(uint32_t *)(code + codesz - 8) = cpu_to_le32(old_baud); + *(uint32_t *)(code + codesz - 4) = cpu_to_le32(new_baud); +} + static int -kwboot_img_patch_hdr(void *img, size_t *size) +kwboot_img_patch(void *img, size_t *size, int baudrate) { int rc; struct main_hdr_v1 *hdr; @@ -895,6 +1402,51 @@ kwboot_img_patch_hdr(void *img, size_t *size)
hdr->blockid = IBR_HDR_UART_ID;
+ if (baudrate) { + uint32_t codesz = sizeof(kwboot_baud_code); + void *code; + + if (image_ver == 0) { + fprintf(stderr, + "Cannot inject code for changing baudrate into v0 image header\n"); + errno = EINVAL; + goto out; + } + + if (is_secure) { + fprintf(stderr, + "Cannot inject code for changing baudrate into image with secure header\n"); + errno = EINVAL; + goto out; + } + + /* + * First inject code that changes the baudrate from the default + * value of 115200 Bd to requested value. This code is inserted + * as a new opt hdr, so it is executed by BootROM after the + * header part is received. + */ + kwboot_printv("Injecting binary header code for changing baudrate to %d Bd\n", + baudrate); + + code = kwboot_add_bin_ohdr_v1(img, size, codesz); + _copy_baudrate_change_code(code, 0, 115200, baudrate); + + /* + * Now inject code that changes the baudrate back to 115200 Bd. + * This code is prepended to the data part of the image, so it + * is executed before U-Boot proper. + */ + kwboot_printv("Injecting code for changing baudrate back\n"); + + codesz += sizeof(kwboot_pre_baud_code); + code = kwboot_img_grow_data_left(img, size, codesz); + _copy_baudrate_change_code(code, 1, baudrate, 115200); + + /* recompute header size */ + hdrsz = kwbheader_size(hdr); + } + if (hdrsz % KWBOOT_XM_BLKSZ) { size_t offset = (KWBOOT_XM_BLKSZ - hdrsz % KWBOOT_XM_BLKSZ) % KWBOOT_XM_BLKSZ; @@ -951,7 +1503,8 @@ main(int argc, char **argv) void *debugmsg; void *img; size_t size; - speed_t speed; + size_t after_img_rsv; + int baudrate;
rv = 1; tty = -1; @@ -961,7 +1514,8 @@ main(int argc, char **argv) img = NULL; term = 0; size = 0; - speed = B115200; + after_img_rsv = KWBOOT_XM_BLKSZ; + baudrate = 115200;
kwboot_verbose = isatty(STDOUT_FILENO);
@@ -1011,9 +1565,7 @@ main(int argc, char **argv) break;
case 'B': - speed = kwboot_tty_speed(atoi(optarg)); - if (speed == -1) - goto usage; + baudrate = atoi(optarg); break;
case 'h': @@ -1031,20 +1583,29 @@ main(int argc, char **argv)
ttypath = argv[optind++];
- tty = kwboot_open_tty(ttypath, speed); + tty = kwboot_open_tty(ttypath, imgpath ? 115200 : baudrate); if (tty < 0) { perror(ttypath); goto out; }
+ if (baudrate == 115200) + /* do not change baudrate during Xmodem to the same value */ + baudrate = 0; + else + /* ensure we have enough space for baudrate change code */ + after_img_rsv += KWBOOT_BAUDRATE_BIN_HEADER_SZ + + sizeof(kwboot_pre_baud_code) + + sizeof(kwboot_baud_code); + if (imgpath) { - img = kwboot_read_image(imgpath, &size, KWBOOT_XM_BLKSZ); + img = kwboot_read_image(imgpath, &size, after_img_rsv); if (!img) { perror(imgpath); goto out; }
- rc = kwboot_img_patch_hdr(img, &size); + rc = kwboot_img_patch(img, &size, baudrate); if (rc) { fprintf(stderr, "%s: Invalid image.\n", imgpath); goto out; @@ -1066,7 +1627,7 @@ main(int argc, char **argv) }
if (img) { - rc = kwboot_xmodem(tty, img, size); + rc = kwboot_xmodem(tty, img, size, baudrate); if (rc) { perror("xmodem"); goto out;

On Wed, 25 Aug 2021 15:46:30 +0200 Marek Behún marek.behun@nic.cz wrote:
@@ -765,6 +1187,37 @@ kwboot_img_is_secure(void *img) return 0; }
+static void * +kwboot_img_grow_data_left(void *img, size_t *size, size_t grow) +{
- uint32_t hdrsz, datasz, srcaddr;
- struct main_hdr_v1 *hdr = img;
- uint8_t *data;
- srcaddr = le32_to_cpu(hdr->srcaddr);
- hdrsz = kwbheader_size(hdr);
- data = (uint8_t *)img + srcaddr;
- datasz = *size - srcaddr;
- /* only move data if there is not enough space */
- if (hdrsz + grow > srcaddr) {
size_t need = hdrsz + grow - srcaddr;
/* move data by enough bytes */
memmove(data + need, data, datasz);
*size += need;
srcaddr += need;
- }
- srcaddr -= grow;
- hdr->srcaddr = cpu_to_le32(srcaddr);
- hdr->destaddr = cpu_to_le32(le32_to_cpu(hdr->destaddr) - grow);
- hdr->execaddr = cpu_to_le32(le32_to_cpu(hdr->execaddr) - grow);
Sorry, we need to do one more thing here: hdr->blocksize = cpu_to_le32(le32_to_cpu(hdr->blocksize) + grow);
I will fix this in v2, unless Stefan is willing to fix this himself.
Marek

OK, there were yet some other issues with execaddr when higheer baudrate is used. I will send v2 in a few days, to wait for some more reactions for v1.
Marek

From: Pali Rohár pali@kernel.org
The A38x platform supports more baudrates than just those defined by the Bn constants, and some of them are higher than the highest Bn baudrate (the highest is 4 MBd while A38x support 5.15 MBd).
On Linux, add support for arbitrary baudrates. (Since there is no standard POSIX API to specify arbitrary baudrate for a tty device, this change is Linux-specific.)
We need to use raw TCGETS2/TCSETS2 or TCGETS/TCSETS ioctls with the BOTHER flag in struct termios2/termios, defined in Linux headers <asm/ioctls.h> and <asm/termbits.h>. Since these headers conflict with glibc's header file <termios.h>, it is not possible to use libc's termios functions and we need to reimplement them via ioctl() calls.
Note that the Bnnn constants from <termios.h> need not be compatible with Bnnn constants from <asm/termbits.h>.
Signed-off-by: Pali Rohár pali@kernel.org [ termios macros rewritten to static inline functions (for type control) and moved to tools/termios.h ] Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 16 +++- tools/termios_linux.h | 170 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 tools/termios_linux.h
diff --git a/tools/kwboot.c b/tools/kwboot.c index dbe59833eb..f60cfd130e 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -23,10 +23,15 @@ #include <errno.h> #include <unistd.h> #include <stdint.h> -#include <termios.h> #include <time.h> #include <sys/stat.h>
+#ifdef __linux__ +#include "termios_linux.h" +#else +#include <termios.h> +#endif + /* * Marvell BootROM UART Sensing */ @@ -553,7 +558,11 @@ kwboot_tty_baudrate_to_speed(int baudrate) return B50; #endif default: +#ifdef BOTHER + return BOTHER; +#else return B0; +#endif } }
@@ -574,6 +583,11 @@ kwboot_tty_change_baudrate(int fd, int baudrate) return -1; }
+#ifdef BOTHER + if (speed == BOTHER) + tio.c_ospeed = tio.c_ispeed = baudrate; +#endif + rc = cfsetospeed(&tio, speed); if (rc) return rc; diff --git a/tools/termios_linux.h b/tools/termios_linux.h new file mode 100644 index 0000000000..1c4223afa7 --- /dev/null +++ b/tools/termios_linux.h @@ -0,0 +1,170 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * termios fuctions to support arbitrary baudrates (on Linux) + * + * Copyright (c) 2021 Pali Rohár pali@kernel.org + * Copyright (c) 2021 Marek Behún marek.behun@nic.cz + */ + +#ifndef _TERMIOS_LINUX_H_ +#define _TERMIOS_LINUX_H_ + +/* + * We need to use raw TCGETS2/TCSETS2 or TCGETS/TCSETS ioctls with the BOTHER + * flag in struct termios2/termios, defined in Linux headers <asm/ioctls.h> and + * <asm/termbits.h>. Since these headers conflict with glibc's header file + * <termios.h>, it is not possible to use libc's termios functions and we need + * to reimplement them via ioctl() calls. + * + * An arbitrary baudrate is supported when the macro BOTHER is defined. The + * baudrate value itself is then stored into the c_ospeed and c_ispeed members. + * If ioctls TCGETS2/TCSETS2 are defined and supported then these fields are + * present in struct termios2, otherwise these fields are present in struct + * termios. + * + * Note that the Bnnn constants from <termios.h> need not be compatible with Bnnn + * constants from <asm/termbits.h>. + */ + +#include <errno.h> +#include <sys/ioctl.h> +#include <sys/types.h> +#include <asm/ioctls.h> +#include <asm/termbits.h> + +#if defined(BOTHER) && defined(TCGETS2) +#define termios termios2 +#endif + +static inline int tcgetattr(int fd, struct termios *t) +{ +#if defined(BOTHER) && defined(TCGETS2) + return ioctl(fd, TCGETS2, t); +#else + return ioctl(fd, TCGETS, t); +#endif +} + +static inline int tcsetattr(int fd, int a, const struct termios *t) +{ + int cmd; + + switch (a) { +#if defined(BOTHER) && defined(TCGETS2) + case TCSANOW: + cmd = TCSETS2; + break; + case TCSADRAIN: + cmd = TCSETSW2; + break; + case TCSAFLUSH: + cmd = TCSETSF2; + break; +#else + case TCSANOW: + cmd = TCSETS; + break; + case TCSADRAIN: + cmd = TCSETSW; + break; + case TCSAFLUSH: + cmd = TCSETSF; + break; +#endif + default: + errno = EINVAL; + return -1; + } + + return ioctl(fd, cmd, t); +} + +static inline int tcdrain(int fd) +{ + return ioctl(fd, TCSBRK, 1); +} + +static inline int tcflush(int fd, int q) +{ + return ioctl(fd, TCFLSH, q); +} + +static inline int tcsendbreak(int fd, int d) +{ + return ioctl(fd, TCSBRK, d); +} + +static inline int tcflow(int fd, int a) +{ + return ioctl(fd, TCXONC, a); +} + +static inline pid_t tcgetsid(int fd) +{ + pid_t sid; + + if (ioctl(fd, TIOCGSID, &sid) < 0) + return (pid_t)-1; + + return sid; +} + +static inline speed_t cfgetospeed(const struct termios *t) +{ + return t->c_cflag & CBAUD; +} + +static inline int cfsetospeed(struct termios *t, speed_t s) +{ + t->c_cflag &= ~CBAUD; + t->c_cflag |= s; + + return 0; +} + +#ifdef IBSHIFT +static inline speed_t cfgetispeed(const struct termios *t) +{ + return (t->c_cflag >> IBSHIFT) & CBAUD; +} + +static inline int cfsetispeed(struct termios *t, speed_t s) +{ + t->c_cflag &= ~(CBAUD << IBSHIFT); + t->c_cflag |= s << IBSHIFT; + + return 0; +} +#else /* !IBSHIFT */ +static inline speed_t cfgetispeed(const struct termios *t) +{ + return cfgetospeed(t); +} + +static inline int cfsetispeed(struct termios *t, speed_t s) +{ + return cfsetospeed(t, s); +} +#endif /* !IBSHIFT */ + +static inline int cfsetspeed(struct termios *t, speed_t s) +{ + cfsetospeed(t, s); +#ifdef IBSHIFT + cfsetispeed(t, s); +#endif + + return 0; +} + +static void cfmakeraw(struct termios *t) +{ + t->c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP | INLCR | IGNCR | + ICRNL | IXON); + t->c_oflag &= ~OPOST; + t->c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN); + t->c_cflag &= ~(CSIZE | PARENB); + t->c_cflag |= CS8; +} + +#endif /* _TERMIOS_LINUX_H_ */

From: Pali Rohár pali@kernel.org
Add Pali and Marek as another authors of the kwboot utility.
Signed-off-by: Pali Rohár pali@kernel.org Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwboot.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/tools/kwboot.c b/tools/kwboot.c index f60cfd130e..b183e4b35b 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -3,6 +3,8 @@ * supports Kirkwood, Dove, Armada 370, Armada XP * * (c) 2012 Daniel Stodden daniel.stodden@gmail.com + * (c) 2021 Pali Rohár pali@kernel.org + * (c) 2021 Marek Behún marek.behun@nic.cz * * References: marvell.com, "88F6180, 88F6190, 88F6192, and 88F6281 * Integrated Controller: Functional Specifications" December 2,

Update man page for the kwboot utility.
Signed-off-by: Marek Behún marek.behun@nic.cz --- doc/kwboot.1 | 58 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 20 deletions(-)
diff --git a/doc/kwboot.1 b/doc/kwboot.1 index 1e9ca268f7..cf113b8b27 100644 --- a/doc/kwboot.1 +++ b/doc/kwboot.1 @@ -1,21 +1,22 @@ -.TH KWBOOT 1 "2012-05-19" +.TH KWBOOT 1 "2021-08-25"
.SH NAME -kwboot - Boot Marvell Kirkwood SoCs over a serial link. +kwboot - Boot Marvell Kirkwood (and others 32-bit) SoCs over a serial link. .SH SYNOPSIS .B kwboot .RB [ "-b \fIimage\fP" ] -.RB [ "-p" ] .RB [ "-t" ] .RB [ "-B \fIbaudrate\fP" ] .RB \fITTY\fP .SH "DESCRIPTION"
-The \fBmkimage\fP program boots boards based on Marvell's Kirkwood -platform over their integrated UART. Boot image files will typically +The \fBkwboot\fP program boots boards based on Marvell's 32-bit +platforms including Orion, Kirkwood, Dove, Discovery, AXP, A37x, A38x +and A39x over their integrated UART. Boot image files will typically contain a second stage boot loader, such as U-Boot. The image file must conform to Marvell's BootROM firmware image format -(\fIkwbimage\fP), created using a tool such as \fBmkimage\fP. +(\fIkwbimage v0\fP or \fIv1\fP), created using a tool such as +\fBmkimage\fP.
Following power-up or a system reset, system BootROM code polls the UART for a brief period of time, sensing a handshake message which @@ -36,25 +37,23 @@ by the second-stage loader. Handshake; then upload file \fIimage\fP over \fITTY\fP.
Note that for the encapsulated boot code to be executed, \fIimage\fP -must be of type "UART boot" (0x69). Boot images of different types, -such as backup images of vendor firmware downloaded from flash memory -(type 0x8B), will not work (or not as expected). See \fB-p\fP for a -workaround. +must be of type "UART boot" (0x69). The \fBkwboot\fP program changes +this type automatically, unless the \fIimage\fP is signed, in which +case it cannot be changed.
This mode writes handshake status and upload progress indication to -stdout. +stdout. It is possible that \fIimage\fP contains an optional binary +code in it's header which may also print some output via UART (for +example U-Boot SPL does this). In such a case, this output is also +written to stdout after the header is sent.
.TP .BI "-p" -In combination with \fB-b\fP, patches the header in \fIimage\fP prior -to upload, to "UART boot" type. +Obsolete. Does nothing.
-This option attempts on-the-fly conversion of some none-UART image -types, such as images which were originally formatted to be stored in -flash memory. - -Conversion is performed in memory. The contents of \fIimage\fP will -not be altered. +In the past, when this option was used, the program patched the header +in the image prior upload, to "UART boot" type. This is now done by +default.
.TP .BI "-t" @@ -69,7 +68,22 @@ after receiving 'ctrl-\' followed by 'c' from console input.
.TP .BI "-B \fIbaudrate\fP" -Adjust the baud rate on \fITTY\fP. Default rate is 115200. +If used in combination with \fB-b\fP, inject into the image header +code that changes baud rate to \fIbaudrate\fP after uploading image +header, and code that changes the baud rate back to the default +(115200 Bd) before executing payload, and also adjust the baud rate +on \fITTY\fP correspondingly. This can make the upload significantly +faster. + +If used in combination with \fB-t\fP, adjust the baud rate to +\fIbaudrate\fP on \fITTY\fP before starting terminal. + +If both \fB-b\fP and \fB-t\fP are used, the baud rate is changed +back to 115200 after the upload. + +Tested values for \fIbaudrate\fP for Armada 38x include: 115200, +230400, 460800, 500000, 576000, 921600, 1000000, 1152000, 1500000, +2000000, 2500000, 3125000, 4000000 and 5150000.
.SH "SEE ALSO" .PP @@ -82,3 +96,7 @@ Daniel Stodden daniel.stodden@gmail.com Luka Perkov luka@openwrt.org .br David Purdy david.c.purdy@gmail.com +.br +Pali Rohár pali@kernel.org +.br +Marek Behún marek.behun@nic.cz

On Thu, Aug 26, 2021 at 1:46 AM Marek Behún marek.behun@nic.cz wrote:
Update man page for the kwboot utility.
Signed-off-by: Marek Behún marek.behun@nic.cz
doc/kwboot.1 | 58 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 20 deletions(-)
diff --git a/doc/kwboot.1 b/doc/kwboot.1 index 1e9ca268f7..cf113b8b27 100644 --- a/doc/kwboot.1 +++ b/doc/kwboot.1 @@ -1,21 +1,22 @@ -.TH KWBOOT 1 "2012-05-19" +.TH KWBOOT 1 "2021-08-25"
.SH NAME -kwboot - Boot Marvell Kirkwood SoCs over a serial link. +kwboot - Boot Marvell Kirkwood (and others 32-bit) SoCs over a serial link. .SH SYNOPSIS .B kwboot .RB [ "-b \fIimage\fP" ] -.RB [ "-p" ] .RB [ "-t" ]
I notice there's no mention of -d might be good to add it while we're here.
.RB [ "-B \fIbaudrate\fP" ] .RB \fITTY\fP .SH "DESCRIPTION"
-The \fBmkimage\fP program boots boards based on Marvell's Kirkwood -platform over their integrated UART. Boot image files will typically +The \fBkwboot\fP program boots boards based on Marvell's 32-bit +platforms including Orion, Kirkwood, Dove, Discovery, AXP, A37x, A38x +and A39x over their integrated UART. Boot image files will typically contain a second stage boot loader, such as U-Boot. The image file must conform to Marvell's BootROM firmware image format -(\fIkwbimage\fP), created using a tool such as \fBmkimage\fP. +(\fIkwbimage v0\fP or \fIv1\fP), created using a tool such as +\fBmkimage\fP.
Following power-up or a system reset, system BootROM code polls the UART for a brief period of time, sensing a handshake message which @@ -36,25 +37,23 @@ by the second-stage loader. Handshake; then upload file \fIimage\fP over \fITTY\fP.
Note that for the encapsulated boot code to be executed, \fIimage\fP -must be of type "UART boot" (0x69). Boot images of different types, -such as backup images of vendor firmware downloaded from flash memory -(type 0x8B), will not work (or not as expected). See \fB-p\fP for a -workaround. +must be of type "UART boot" (0x69). The \fBkwboot\fP program changes +this type automatically, unless the \fIimage\fP is signed, in which +case it cannot be changed.
This mode writes handshake status and upload progress indication to -stdout. +stdout. It is possible that \fIimage\fP contains an optional binary +code in it's header which may also print some output via UART (for +example U-Boot SPL does this). In such a case, this output is also +written to stdout after the header is sent.
.TP .BI "-p" -In combination with \fB-b\fP, patches the header in \fIimage\fP prior -to upload, to "UART boot" type. +Obsolete. Does nothing.
-This option attempts on-the-fly conversion of some none-UART image -types, such as images which were originally formatted to be stored in -flash memory.
-Conversion is performed in memory. The contents of \fIimage\fP will -not be altered. +In the past, when this option was used, the program patched the header +in the image prior upload, to "UART boot" type. This is now done by +default.
.TP .BI "-t" @@ -69,7 +68,22 @@ after receiving 'ctrl-\' followed by 'c' from console input.
.TP .BI "-B \fIbaudrate\fP" -Adjust the baud rate on \fITTY\fP. Default rate is 115200. +If used in combination with \fB-b\fP, inject into the image header +code that changes baud rate to \fIbaudrate\fP after uploading image +header, and code that changes the baud rate back to the default +(115200 Bd) before executing payload, and also adjust the baud rate +on \fITTY\fP correspondingly. This can make the upload significantly +faster.
+If used in combination with \fB-t\fP, adjust the baud rate to +\fIbaudrate\fP on \fITTY\fP before starting terminal.
+If both \fB-b\fP and \fB-t\fP are used, the baud rate is changed +back to 115200 after the upload.
+Tested values for \fIbaudrate\fP for Armada 38x include: 115200, +230400, 460800, 500000, 576000, 921600, 1000000, 1152000, 1500000, +2000000, 2500000, 3125000, 4000000 and 5150000.
.SH "SEE ALSO" .PP @@ -82,3 +96,7 @@ Daniel Stodden daniel.stodden@gmail.com Luka Perkov luka@openwrt.org .br David Purdy david.c.purdy@gmail.com +.br +Pali Rohár pali@kernel.org +.br
+Marek Behún marek.behun@nic.cz
2.31.1

On Friday 27 August 2021 13:37:54 Chris Packham wrote:
On Thu, Aug 26, 2021 at 1:46 AM Marek Behún marek.behun@nic.cz wrote:
Update man page for the kwboot utility.
Signed-off-by: Marek Behún marek.behun@nic.cz
doc/kwboot.1 | 58 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 20 deletions(-)
diff --git a/doc/kwboot.1 b/doc/kwboot.1 index 1e9ca268f7..cf113b8b27 100644 --- a/doc/kwboot.1 +++ b/doc/kwboot.1 @@ -1,21 +1,22 @@ -.TH KWBOOT 1 "2012-05-19" +.TH KWBOOT 1 "2021-08-25"
.SH NAME -kwboot - Boot Marvell Kirkwood SoCs over a serial link. +kwboot - Boot Marvell Kirkwood (and others 32-bit) SoCs over a serial link. .SH SYNOPSIS .B kwboot .RB [ "-b \fIimage\fP" ] -.RB [ "-p" ] .RB [ "-t" ]
I notice there's no mention of -d might be good to add it while we're here.
Option -d is broken and does not work. So not mentioning it here is a good idea. Maybe I could look at it and try to fix in future...
.RB [ "-B \fIbaudrate\fP" ] .RB \fITTY\fP .SH "DESCRIPTION"
-The \fBmkimage\fP program boots boards based on Marvell's Kirkwood -platform over their integrated UART. Boot image files will typically +The \fBkwboot\fP program boots boards based on Marvell's 32-bit +platforms including Orion, Kirkwood, Dove, Discovery, AXP, A37x, A38x +and A39x over their integrated UART. Boot image files will typically contain a second stage boot loader, such as U-Boot. The image file must conform to Marvell's BootROM firmware image format -(\fIkwbimage\fP), created using a tool such as \fBmkimage\fP. +(\fIkwbimage v0\fP or \fIv1\fP), created using a tool such as +\fBmkimage\fP.
Following power-up or a system reset, system BootROM code polls the UART for a brief period of time, sensing a handshake message which @@ -36,25 +37,23 @@ by the second-stage loader. Handshake; then upload file \fIimage\fP over \fITTY\fP.
Note that for the encapsulated boot code to be executed, \fIimage\fP -must be of type "UART boot" (0x69). Boot images of different types, -such as backup images of vendor firmware downloaded from flash memory -(type 0x8B), will not work (or not as expected). See \fB-p\fP for a -workaround. +must be of type "UART boot" (0x69). The \fBkwboot\fP program changes +this type automatically, unless the \fIimage\fP is signed, in which +case it cannot be changed.
This mode writes handshake status and upload progress indication to -stdout. +stdout. It is possible that \fIimage\fP contains an optional binary +code in it's header which may also print some output via UART (for +example U-Boot SPL does this). In such a case, this output is also +written to stdout after the header is sent.
.TP .BI "-p" -In combination with \fB-b\fP, patches the header in \fIimage\fP prior -to upload, to "UART boot" type. +Obsolete. Does nothing.
-This option attempts on-the-fly conversion of some none-UART image -types, such as images which were originally formatted to be stored in -flash memory.
-Conversion is performed in memory. The contents of \fIimage\fP will -not be altered. +In the past, when this option was used, the program patched the header +in the image prior upload, to "UART boot" type. This is now done by +default.
.TP .BI "-t" @@ -69,7 +68,22 @@ after receiving 'ctrl-\' followed by 'c' from console input.
.TP .BI "-B \fIbaudrate\fP" -Adjust the baud rate on \fITTY\fP. Default rate is 115200. +If used in combination with \fB-b\fP, inject into the image header +code that changes baud rate to \fIbaudrate\fP after uploading image +header, and code that changes the baud rate back to the default +(115200 Bd) before executing payload, and also adjust the baud rate +on \fITTY\fP correspondingly. This can make the upload significantly +faster.
+If used in combination with \fB-t\fP, adjust the baud rate to +\fIbaudrate\fP on \fITTY\fP before starting terminal.
+If both \fB-b\fP and \fB-t\fP are used, the baud rate is changed +back to 115200 after the upload.
+Tested values for \fIbaudrate\fP for Armada 38x include: 115200, +230400, 460800, 500000, 576000, 921600, 1000000, 1152000, 1500000, +2000000, 2500000, 3125000, 4000000 and 5150000.
.SH "SEE ALSO" .PP @@ -82,3 +96,7 @@ Daniel Stodden daniel.stodden@gmail.com Luka Perkov luka@openwrt.org .br David Purdy david.c.purdy@gmail.com +.br +Pali Rohár pali@kernel.org +.br
+Marek Behún marek.behun@nic.cz
2.31.1

On Friday 27 August 2021 10:13:01 Pali Rohár wrote:
On Friday 27 August 2021 13:37:54 Chris Packham wrote:
On Thu, Aug 26, 2021 at 1:46 AM Marek Behún marek.behun@nic.cz wrote:
Update man page for the kwboot utility.
Signed-off-by: Marek Behún marek.behun@nic.cz
doc/kwboot.1 | 58 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 20 deletions(-)
diff --git a/doc/kwboot.1 b/doc/kwboot.1 index 1e9ca268f7..cf113b8b27 100644 --- a/doc/kwboot.1 +++ b/doc/kwboot.1 @@ -1,21 +1,22 @@ -.TH KWBOOT 1 "2012-05-19" +.TH KWBOOT 1 "2021-08-25"
.SH NAME -kwboot - Boot Marvell Kirkwood SoCs over a serial link. +kwboot - Boot Marvell Kirkwood (and others 32-bit) SoCs over a serial link. .SH SYNOPSIS .B kwboot .RB [ "-b \fIimage\fP" ] -.RB [ "-p" ] .RB [ "-t" ]
I notice there's no mention of -d might be good to add it while we're here.
Option -d is broken and does not work. So not mentioning it here is a good idea. Maybe I could look at it and try to fix in future...
Anyway, it looks like that A385 bootrom has bugs in debug console and so -d is on this SoC unusable. When I enter into debug mode, boorom somehow does not send output from (debug) commands over UART. But it sends ECHO over UART...
.RB [ "-B \fIbaudrate\fP" ] .RB \fITTY\fP .SH "DESCRIPTION"
-The \fBmkimage\fP program boots boards based on Marvell's Kirkwood -platform over their integrated UART. Boot image files will typically +The \fBkwboot\fP program boots boards based on Marvell's 32-bit +platforms including Orion, Kirkwood, Dove, Discovery, AXP, A37x, A38x +and A39x over their integrated UART. Boot image files will typically contain a second stage boot loader, such as U-Boot. The image file must conform to Marvell's BootROM firmware image format -(\fIkwbimage\fP), created using a tool such as \fBmkimage\fP. +(\fIkwbimage v0\fP or \fIv1\fP), created using a tool such as +\fBmkimage\fP.
Following power-up or a system reset, system BootROM code polls the UART for a brief period of time, sensing a handshake message which @@ -36,25 +37,23 @@ by the second-stage loader. Handshake; then upload file \fIimage\fP over \fITTY\fP.
Note that for the encapsulated boot code to be executed, \fIimage\fP -must be of type "UART boot" (0x69). Boot images of different types, -such as backup images of vendor firmware downloaded from flash memory -(type 0x8B), will not work (or not as expected). See \fB-p\fP for a -workaround. +must be of type "UART boot" (0x69). The \fBkwboot\fP program changes +this type automatically, unless the \fIimage\fP is signed, in which +case it cannot be changed.
This mode writes handshake status and upload progress indication to -stdout. +stdout. It is possible that \fIimage\fP contains an optional binary +code in it's header which may also print some output via UART (for +example U-Boot SPL does this). In such a case, this output is also +written to stdout after the header is sent.
.TP .BI "-p" -In combination with \fB-b\fP, patches the header in \fIimage\fP prior -to upload, to "UART boot" type. +Obsolete. Does nothing.
-This option attempts on-the-fly conversion of some none-UART image -types, such as images which were originally formatted to be stored in -flash memory.
-Conversion is performed in memory. The contents of \fIimage\fP will -not be altered. +In the past, when this option was used, the program patched the header +in the image prior upload, to "UART boot" type. This is now done by +default.
.TP .BI "-t" @@ -69,7 +68,22 @@ after receiving 'ctrl-\' followed by 'c' from console input.
.TP .BI "-B \fIbaudrate\fP" -Adjust the baud rate on \fITTY\fP. Default rate is 115200. +If used in combination with \fB-b\fP, inject into the image header +code that changes baud rate to \fIbaudrate\fP after uploading image +header, and code that changes the baud rate back to the default +(115200 Bd) before executing payload, and also adjust the baud rate +on \fITTY\fP correspondingly. This can make the upload significantly +faster.
+If used in combination with \fB-t\fP, adjust the baud rate to +\fIbaudrate\fP on \fITTY\fP before starting terminal.
+If both \fB-b\fP and \fB-t\fP are used, the baud rate is changed +back to 115200 after the upload.
+Tested values for \fIbaudrate\fP for Armada 38x include: 115200, +230400, 460800, 500000, 576000, 921600, 1000000, 1152000, 1500000, +2000000, 2500000, 3125000, 4000000 and 5150000.
.SH "SEE ALSO" .PP @@ -82,3 +96,7 @@ Daniel Stodden daniel.stodden@gmail.com Luka Perkov luka@openwrt.org .br David Purdy david.c.purdy@gmail.com +.br +Pali Rohár pali@kernel.org +.br
+Marek Behún marek.behun@nic.cz
2.31.1

On Fri, 27 Aug 2021 10:36:52 +0200 Pali Rohár pali@kernel.org wrote:
Anyway, it looks like that A385 bootrom has bugs in debug console and so -d is on this SoC unusable. When I enter into debug mode, boorom somehow does not send output from (debug) commands over UART. But it sends ECHO over UART...
It's possible that debug UART is the other UART... I seem to remember something like that from documentation, but am not entirely sure.
Marek

On Friday 27 August 2021 15:35:55 Marek Behún wrote:
On Fri, 27 Aug 2021 10:36:52 +0200 Pali Rohár pali@kernel.org wrote:
Anyway, it looks like that A385 bootrom has bugs in debug console and so -d is on this SoC unusable. When I enter into debug mode, boorom somehow does not send output from (debug) commands over UART. But it sends ECHO over UART...
It's possible that debug UART is the other UART... I seem to remember something like that from documentation, but am not entirely sure.
Marek
I can send commands in debug mode and I checked that bootrom really executes them. But it does not send any output back. So bootrom's "stdin" is working fine but bootrom's "stdout" for bootrom commands is broken.
I have not found any details about it.

Add entry for these tools with Marek, Pali and Stefan as maintainers.
Signed-off-by: Marek Behún marek.behun@nic.cz --- MAINTAINERS | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS index 776ff703b9..d4b21dd660 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -772,6 +772,16 @@ S: Maintained T: git https://source.denx.de/u-boot/custodians/u-boot-i2c.git F: drivers/i2c/
+KWBIMAGE / KWBOOT TOOLS +M: Pali Rohár pali@kernel.org +M: Marek Behún marek.behun@nic.cz +M: Stefan Roese sr@denx.de +S: Maintained +T: git https://source.denx.de/u-boot/custodians/u-boot-marvell.git +F: doc/README.kwbimage +F: doc/kwboot.1 +F: tools/kwb* + LOGGING M: Simon Glass sjg@chromium.org S: Maintained

Create macro for_each_opt_hdr_v1 and functions opt_hdr_v1_size(), opt_hdr_v1_valid_size(), opt_hdr_v1_ext(), opt_hdr_v1_first() and opt_hdr_v1_next() to simplify iteration over version 1 optional headers.
This prevents ugly code repetition and makes it nicer to read.
Signed-off-by: Marek Behún marek.behun@nic.cz --- tools/kwbimage.c | 90 +++++++++++++----------------------------------- tools/kwbimage.h | 58 +++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 67 deletions(-)
diff --git a/tools/kwbimage.c b/tools/kwbimage.c index ed11c835a5..6678ef15bd 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1620,34 +1620,20 @@ static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd, static void kwbimage_print_header(const void *ptr) { struct main_hdr_v0 *mhdr = (struct main_hdr_v0 *)ptr; + struct opt_hdr_v1 *ohdr;
printf("Image Type: MVEBU Boot from %s Image\n", image_boot_mode_name(mhdr->blockid)); printf("Image version:%d\n", image_version((void *)ptr)); - if (image_version((void *)ptr) == 1) { - struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr;
- if (mhdr->ext & 0x1) { - struct opt_hdr_v1 *ohdr = (struct opt_hdr_v1 *) - ((uint8_t *)ptr + - sizeof(*mhdr)); - - while (1) { - uint32_t ohdr_size; - - ohdr_size = (ohdr->headersz_msb << 16) | - le16_to_cpu(ohdr->headersz_lsb); - if (ohdr->headertype == OPT_HDR_V1_BINARY_TYPE) { - printf("BIN Hdr Size: "); - genimg_print_size(ohdr_size - 12 - 4 * ohdr->data[0]); - } - if (!(*((uint8_t *)ohdr + ohdr_size - 4) & 0x1)) - break; - ohdr = (struct opt_hdr_v1 *)((uint8_t *)ohdr + - ohdr_size); - } + for_each_opt_hdr_v1 (ohdr, mhdr) { + if (ohdr->headertype == OPT_HDR_V1_BINARY_TYPE) { + printf("BIN Hdr Size: "); + genimg_print_size(opt_hdr_v1_size(ohdr) - 12 - + 4 * ohdr->data[0]); } } + printf("Data Size: "); genimg_print_size(mhdr->blocksize - sizeof(uint32_t)); printf("Load Address: %08x\n", mhdr->destaddr); @@ -1694,33 +1680,15 @@ static int kwbimage_verify_header(unsigned char *ptr, int image_size, } } else if (image_version((void *)ptr) == 1) { struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr; + const uint8_t *mhdr_end; + struct opt_hdr_v1 *ohdr; uint32_t offset; uint32_t size;
- if (mhdr->ext & 0x1) { - uint32_t ohdr_size; - struct opt_hdr_v1 *ohdr = (struct opt_hdr_v1 *) - (ptr + sizeof(*mhdr)); - - while (1) { - if ((uint8_t *)ohdr + sizeof(*ohdr) > - (uint8_t *)mhdr + header_size) - return -FDT_ERR_BADSTRUCTURE; - - ohdr_size = (ohdr->headersz_msb << 16) | - le16_to_cpu(ohdr->headersz_lsb); - - if (ohdr_size < 8 || - (uint8_t *)ohdr + ohdr_size > - (uint8_t *)mhdr + header_size) - return -FDT_ERR_BADSTRUCTURE; - - if (!(*((uint8_t *)ohdr + ohdr_size - 4) & 0x1)) - break; - ohdr = (struct opt_hdr_v1 *)((uint8_t *)ohdr + - ohdr_size); - } - } + mhdr_end = (uint8_t *)mhdr + header_size; + for_each_opt_hdr_v1 (ohdr, ptr) + if (!opt_hdr_v1_valid_size(ohdr, mhdr_end)) + return -FDT_ERR_BADSTRUCTURE;
offset = le32_to_cpu(mhdr->srcaddr);
@@ -1867,36 +1835,24 @@ static int kwbimage_extract_subimage(void *ptr, struct image_tool_params *params { struct main_hdr_v1 *mhdr = (struct main_hdr_v1 *)ptr; size_t header_size = kwbimage_header_size(ptr); + struct opt_hdr_v1 *ohdr; int idx = params->pflag; int cur_idx = 0; uint32_t offset; ulong image; ulong size;
- if (image_version((void *)ptr) == 1 && (mhdr->ext & 0x1)) { - struct opt_hdr_v1 *ohdr = (struct opt_hdr_v1 *) - ((uint8_t *)ptr + - sizeof(*mhdr)); + for_each_opt_hdr_v1 (ohdr, ptr) { + if (ohdr->headertype != OPT_HDR_V1_BINARY_TYPE) + continue;
- while (1) { - uint32_t ohdr_size = (ohdr->headersz_msb << 16) | - le16_to_cpu(ohdr->headersz_lsb); - - if (ohdr->headertype == OPT_HDR_V1_BINARY_TYPE) { - if (idx == cur_idx) { - image = (ulong)&ohdr->data[4 + - 4 * ohdr->data[0]]; - size = ohdr_size - 12 - - 4 * ohdr->data[0]; - goto extract; - } - ++cur_idx; - } - if (!(*((uint8_t *)ohdr + ohdr_size - 4) & 0x1)) - break; - ohdr = (struct opt_hdr_v1 *)((uint8_t *)ohdr + - ohdr_size); + if (idx == cur_idx) { + image = (ulong)&ohdr->data[4 + 4 * ohdr->data[0]]; + size = opt_hdr_v1_size(ohdr) - 12 - 4 * ohdr->data[0]; + goto extract; } + + ++cur_idx; }
if (idx != cur_idx) { diff --git a/tools/kwbimage.h b/tools/kwbimage.h index 074bdfbe46..9a949e03c0 100644 --- a/tools/kwbimage.h +++ b/tools/kwbimage.h @@ -235,4 +235,62 @@ static inline unsigned int image_version(const void *header) return ptr[8]; }
+static inline uint32_t opt_hdr_v1_size(const struct opt_hdr_v1 *ohdr) +{ + return (ohdr->headersz_msb << 16) | le16_to_cpu(ohdr->headersz_lsb); +} + +static inline int opt_hdr_v1_valid_size(const struct opt_hdr_v1 *ohdr, + const void *mhdr_end) +{ + uint32_t ohdr_size; + + if ((void *)(ohdr + 1) > mhdr_end) + return 0; + + ohdr_size = opt_hdr_v1_size(ohdr); + if (ohdr_size < 8 || (void *)((uint8_t *)ohdr + ohdr_size) > mhdr_end) + return 0; + + return 1; +} + +static inline struct opt_hdr_v1 *opt_hdr_v1_first(void *img) { + struct main_hdr_v1 *mhdr; + + if (image_version(img) != 1) + return NULL; + + mhdr = img; + if (mhdr->ext & 0x1) + return (struct opt_hdr_v1 *)(mhdr + 1); + else + return NULL; +} + +static inline uint8_t *opt_hdr_v1_ext(struct opt_hdr_v1 *cur) +{ + uint32_t size = opt_hdr_v1_size(cur); + + return (uint8_t *)cur + size - 4; +} + +static inline struct opt_hdr_v1 *_opt_hdr_v1_next(struct opt_hdr_v1 *cur) +{ + return (struct opt_hdr_v1 *)((uint8_t *)cur + opt_hdr_v1_size(cur)); +} + +static inline struct opt_hdr_v1 *opt_hdr_v1_next(struct opt_hdr_v1 *cur) +{ + if (*opt_hdr_v1_ext(cur) & 0x1) + return _opt_hdr_v1_next(cur); + else + return NULL; +} + +#define for_each_opt_hdr_v1(ohdr, img) \ + for ((ohdr) = opt_hdr_v1_first((img)); \ + (ohdr) != NULL; \ + (ohdr) = opt_hdr_v1_next((ohdr))) + #endif /* _KWBIMAGE_H_ */

On Thu, Aug 26, 2021 at 1:46 AM Marek Behún marek.behun@nic.cz wrote:
Hello Stefan and others,
this series adds support for booting Marvell platforms via UART (those bootable with kwboot) at higher baudrates.
Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than 115200 Bd.
The user can direct kwboot to use higher baudrate via the -B option. (BTW this option was there before but did not work with the -b option.)
Only the payload part of the KWB image is uploaded at this higher baudrate. The header part is still uploaded at 115200 Bd, since the code that changes baudrate is injected into header as a binary extension. (The payload part makes up majority of the binary, though. On Turris Omnia the payload currently makes ~87%.)
The series also contains various other fixes, refactors and improvements of the code, upon which the main change is done.
Marek & Pali
What tree/sha is this series based on. I've tried to apply them out of patchwork and it fails at "tools: kwbimage: Simplify iteration over version 1 optional headers"
Marek Behún (13): tools: kwbimage: Fix printf format warning tools: kwboot: Fix buffer overflow in kwboot_terminal() tools: kwboot: Make the quit sequence buffer const tools: kwboot: Refactor and fix writing buffer tools: kwboot: Fix comparison of integers with different size tools: kwboot: Use a function to check whether received byte is a Xmodem reply tools: kwboot: Print new line after SPL output tools: kwboot: Allow greater timeout when executing header code tools: kwbimage: Simplify iteration over version 1 optional headers tools: kwbimage: Refactor image_version() tools: kwbimage: Refactor kwbimage header size determination doc/kwboot.1: Update man page MAINTAINERS: Add entry for kwbimage / kwboot tools
Pali Rohár (16): tools: kwboot: Print version information header tools: kwboot: Fix kwboot_xm_sendblock() function when kwboot_tty_recv() fails tools: kwboot: Fix return type of kwboot_xm_makeblock() function tools: kwboot: Fix printing progress tools: kwboot: Print newline on error when progress was not completed tools: kwboot: Split sending image into header and data stages tools: kwboot: Allow non-xmodem text output from BootROM only in a specific case tools: kwboot: Properly finish xmodem transfer tools: kwboot: Always call kwboot_img_patch_hdr() tools: kwboot: Don't patch image header if signed tools: kwboot: Patch source address in image header tools: kwboot: Patch destination address to DDR area for SPI image tools: kwboot: Round up header size to 128 B when patching tools: kwboot: Support higher baudrates when booting via UART tools: kwboot: Allow any baudrate on Linux tools: kwboot: Add Pali and Marek as authors
MAINTAINERS | 10 + doc/kwboot.1 | 58 ++- tools/kwbimage.c | 127 ++--- tools/kwbimage.h | 93 +++- tools/kwboot.c | 1103 +++++++++++++++++++++++++++++++++++------ tools/termios_linux.h | 170 +++++++ 6 files changed, 1289 insertions(+), 272 deletions(-) create mode 100644 tools/termios_linux.h
-- 2.31.1

On Fri, Aug 27, 2021 at 1:16 PM Chris Packham judge.packham@gmail.com wrote:
On Thu, Aug 26, 2021 at 1:46 AM Marek Behún marek.behun@nic.cz wrote:
Hello Stefan and others,
this series adds support for booting Marvell platforms via UART (those bootable with kwboot) at higher baudrates.
Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than 115200 Bd.
The user can direct kwboot to use higher baudrate via the -B option. (BTW this option was there before but did not work with the -b option.)
Only the payload part of the KWB image is uploaded at this higher baudrate. The header part is still uploaded at 115200 Bd, since the code that changes baudrate is injected into header as a binary extension. (The payload part makes up majority of the binary, though. On Turris Omnia the payload currently makes ~87%.)
The series also contains various other fixes, refactors and improvements of the code, upon which the main change is done.
Marek & Pali
What tree/sha is this series based on. I've tried to apply them out of patchwork and it fails at "tools: kwbimage: Simplify iteration over version 1 optional headers"
Ah it needs http://patchwork.ozlabs.org/project/uboot/list/?series=257577
Marek Behún (13): tools: kwbimage: Fix printf format warning tools: kwboot: Fix buffer overflow in kwboot_terminal() tools: kwboot: Make the quit sequence buffer const tools: kwboot: Refactor and fix writing buffer tools: kwboot: Fix comparison of integers with different size tools: kwboot: Use a function to check whether received byte is a Xmodem reply tools: kwboot: Print new line after SPL output tools: kwboot: Allow greater timeout when executing header code tools: kwbimage: Simplify iteration over version 1 optional headers tools: kwbimage: Refactor image_version() tools: kwbimage: Refactor kwbimage header size determination doc/kwboot.1: Update man page MAINTAINERS: Add entry for kwbimage / kwboot tools
Pali Rohár (16): tools: kwboot: Print version information header tools: kwboot: Fix kwboot_xm_sendblock() function when kwboot_tty_recv() fails tools: kwboot: Fix return type of kwboot_xm_makeblock() function tools: kwboot: Fix printing progress tools: kwboot: Print newline on error when progress was not completed tools: kwboot: Split sending image into header and data stages tools: kwboot: Allow non-xmodem text output from BootROM only in a specific case tools: kwboot: Properly finish xmodem transfer tools: kwboot: Always call kwboot_img_patch_hdr() tools: kwboot: Don't patch image header if signed tools: kwboot: Patch source address in image header tools: kwboot: Patch destination address to DDR area for SPI image tools: kwboot: Round up header size to 128 B when patching tools: kwboot: Support higher baudrates when booting via UART tools: kwboot: Allow any baudrate on Linux tools: kwboot: Add Pali and Marek as authors
MAINTAINERS | 10 + doc/kwboot.1 | 58 ++- tools/kwbimage.c | 127 ++--- tools/kwbimage.h | 93 +++- tools/kwboot.c | 1103 +++++++++++++++++++++++++++++++++++------ tools/termios_linux.h | 170 +++++++ 6 files changed, 1289 insertions(+), 272 deletions(-) create mode 100644 tools/termios_linux.h
-- 2.31.1

On Fri, 27 Aug 2021 13:16:25 +1200 Chris Packham judge.packham@gmail.com wrote:
On Thu, Aug 26, 2021 at 1:46 AM Marek Behún marek.behun@nic.cz wrote:
Hello Stefan and others,
this series adds support for booting Marvell platforms via UART (those bootable with kwboot) at higher baudrates.
Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than 115200 Bd.
The user can direct kwboot to use higher baudrate via the -B option. (BTW this option was there before but did not work with the -b option.)
Only the payload part of the KWB image is uploaded at this higher baudrate. The header part is still uploaded at 115200 Bd, since the code that changes baudrate is injected into header as a binary extension. (The payload part makes up majority of the binary, though. On Turris Omnia the payload currently makes ~87%.)
The series also contains various other fixes, refactors and improvements of the code, upon which the main change is done.
Marek & Pali
What tree/sha is this series based on. I've tried to apply them out of patchwork and it fails at "tools: kwbimage: Simplify iteration over version 1 optional headers"
Applies to master and to u-boot-marvell, but you need the 6 additional patches for kwbimage/kwboot that are on patchwork but not yet applied to u-boot-marvell: https://patchwork.ozlabs.org/project/uboot/list/?series=257577 https://patchwork.ozlabs.org/project/uboot/patch/20210817050320.11983-1-xypr... https://patchwork.ozlabs.org/project/uboot/patch/20210817051158.13283-1-xypr...
Also some more fixes were yet needed, which I will sent in v2.
To make it simpler for you I pushed v2 into https://gitlab.nic.cz/turris/turris-omnia-uboot branch kwboot-baudrate-improvements
Marek

On Fri, Aug 27, 2021 at 1:39 PM Marek Behún marek.behun@nic.cz wrote:
On Fri, 27 Aug 2021 13:16:25 +1200 Chris Packham judge.packham@gmail.com wrote:
On Thu, Aug 26, 2021 at 1:46 AM Marek Behún marek.behun@nic.cz wrote:
Hello Stefan and others,
this series adds support for booting Marvell platforms via UART (those bootable with kwboot) at higher baudrates.
Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than 115200 Bd.
The user can direct kwboot to use higher baudrate via the -B option. (BTW this option was there before but did not work with the -b option.)
Only the payload part of the KWB image is uploaded at this higher baudrate. The header part is still uploaded at 115200 Bd, since the code that changes baudrate is injected into header as a binary extension. (The payload part makes up majority of the binary, though. On Turris Omnia the payload currently makes ~87%.)
The series also contains various other fixes, refactors and improvements of the code, upon which the main change is done.
Marek & Pali
What tree/sha is this series based on. I've tried to apply them out of patchwork and it fails at "tools: kwbimage: Simplify iteration over version 1 optional headers"
Applies to master and to u-boot-marvell, but you need the 6 additional patches for kwbimage/kwboot that are on patchwork but not yet applied to u-boot-marvell: https://patchwork.ozlabs.org/project/uboot/list/?series=257577 https://patchwork.ozlabs.org/project/uboot/patch/20210817050320.11983-1-xypr... https://patchwork.ozlabs.org/project/uboot/patch/20210817051158.13283-1-xypr...
Also some more fixes were yet needed, which I will sent in v2.
To make it simpler for you I pushed v2 into https://gitlab.nic.cz/turris/turris-omnia-uboot branch kwboot-baudrate-improvements
Thanks. I took it for a spin on the x530. In terms of regression testing the default behaviour is good.
The higher speed settings weren't so good. I started with 3125000 and that doesn't get onto the 2nd part of the download, same for 1152000 and 4000000 (I stopped trying higher speeds at that point). Using 230400 and 460800 it does make it through the 2nd part of the download but when I go back on to the console it appears to be unresponsive.
All of this could be down to the serial hardware in my system (the x530 uses a real RS232 interface not a TTL) and I have had problems with the MosChip USB-Serial adapter in my test PC in the past. I wouldn't reject this series based on me not being able to get it working, the important thing for me is the default behaviour at the standard baudrate which is good.
One usability thing I'd like to see is retaining support for -t (I use that quite a lot when recovering a system). Ideally we'd still be able to drop into the console at 115200 once the download is complete.
Marek

On Friday 27 August 2021 14:45:07 Chris Packham wrote:
On Fri, Aug 27, 2021 at 1:39 PM Marek Behún marek.behun@nic.cz wrote:
On Fri, 27 Aug 2021 13:16:25 +1200 Chris Packham judge.packham@gmail.com wrote:
On Thu, Aug 26, 2021 at 1:46 AM Marek Behún marek.behun@nic.cz wrote:
Hello Stefan and others,
this series adds support for booting Marvell platforms via UART (those bootable with kwboot) at higher baudrates.
Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than 115200 Bd.
The user can direct kwboot to use higher baudrate via the -B option. (BTW this option was there before but did not work with the -b option.)
Only the payload part of the KWB image is uploaded at this higher baudrate. The header part is still uploaded at 115200 Bd, since the code that changes baudrate is injected into header as a binary extension. (The payload part makes up majority of the binary, though. On Turris Omnia the payload currently makes ~87%.)
The series also contains various other fixes, refactors and improvements of the code, upon which the main change is done.
Marek & Pali
What tree/sha is this series based on. I've tried to apply them out of patchwork and it fails at "tools: kwbimage: Simplify iteration over version 1 optional headers"
Applies to master and to u-boot-marvell, but you need the 6 additional patches for kwbimage/kwboot that are on patchwork but not yet applied to u-boot-marvell: https://patchwork.ozlabs.org/project/uboot/list/?series=257577 https://patchwork.ozlabs.org/project/uboot/patch/20210817050320.11983-1-xypr... https://patchwork.ozlabs.org/project/uboot/patch/20210817051158.13283-1-xypr...
Also some more fixes were yet needed, which I will sent in v2.
To make it simpler for you I pushed v2 into https://gitlab.nic.cz/turris/turris-omnia-uboot branch kwboot-baudrate-improvements
Thanks. I took it for a spin on the x530. In terms of regression testing the default behaviour is good.
The higher speed settings weren't so good. I started with 3125000 and that doesn't get onto the 2nd part of the download, same for 1152000 and 4000000 (I stopped trying higher speeds at that point). Using 230400 and 460800 it does make it through the 2nd part of the download but when I go back on to the console it appears to be unresponsive.
All of this could be down to the serial hardware in my system (the x530 uses a real RS232 interface not a TTL) and I have had problems with the MosChip USB-Serial adapter in my test PC in the past. I wouldn't reject this series based on me not being able to get it working, the important thing for me is the default behaviour at the standard baudrate which is good.
You need to choose speed which is supported by both armada board and your computer. Armada boards support speeds calculated by formula: speed = TCLK / ( 16 * d ) where d is 16-bit integer number. So for A385 board with 250 MHz TCLK you can set speeds from above formula +/- 30%. Tested were: 300, 600, 1200, 1800, 2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400, 460800, 500000, 576000, 921600, 1000000, 1152000, 1500000, 2000000, 2500000, 3125000, 4000000, 5150000. If you have a board with different TCLK you obviously need to use different baudrates.
But it is possible that your computer does not support these speeds. E.g. more chinese USB TTL adapters have base clock 3 MHz and so they could not support 3150000 or 2500000 speeds.
One usability thing I'd like to see is retaining support for -t (I use that quite a lot when recovering a system). Ideally we'd still be able to drop into the console at 115200 once the download is complete.
-t is fully supported, I'm using -t with -B 5150000 and it is working fine. After successful download, speed on both A38x and host computer is changed back to the value 115200.

On Fri, 27 Aug 2021, 8:32 PM Pali Rohár, pali@kernel.org wrote:
On Friday 27 August 2021 14:45:07 Chris Packham wrote:
On Fri, Aug 27, 2021 at 1:39 PM Marek Behún marek.behun@nic.cz wrote:
On Fri, 27 Aug 2021 13:16:25 +1200 Chris Packham judge.packham@gmail.com wrote:
On Thu, Aug 26, 2021 at 1:46 AM Marek Behún marek.behun@nic.cz
wrote:
Hello Stefan and others,
this series adds support for booting Marvell platforms via UART
(those
bootable with kwboot) at higher baudrates.
Tested on Turris Omnia up to 5.15 MBd, which is 44x faster than 115200 Bd.
The user can direct kwboot to use higher baudrate via the -B
option.
(BTW this option was there before but did not work with the -b
option.)
Only the payload part of the KWB image is uploaded at this higher baudrate. The header part is still uploaded at 115200 Bd, since
the code
that changes baudrate is injected into header as a binary
extension.
(The payload part makes up majority of the binary, though. On
Turris
Omnia the payload currently makes ~87%.)
The series also contains various other fixes, refactors and
improvements
of the code, upon which the main change is done.
Marek & Pali
What tree/sha is this series based on. I've tried to apply them out
of
patchwork and it fails at "tools: kwbimage: Simplify iteration over version 1 optional headers"
Applies to master and to u-boot-marvell, but you need the 6 additional patches for kwbimage/kwboot that are on patchwork but not yet applied to u-boot-marvell: https://patchwork.ozlabs.org/project/uboot/list/?series=257577
https://patchwork.ozlabs.org/project/uboot/patch/20210817050320.11983-1-xypr...
https://patchwork.ozlabs.org/project/uboot/patch/20210817051158.13283-1-xypr...
Also some more fixes were yet needed, which I will sent in v2.
To make it simpler for you I pushed v2 into https://gitlab.nic.cz/turris/turris-omnia-uboot branch kwboot-baudrate-improvements
Thanks. I took it for a spin on the x530. In terms of regression testing the default behaviour is good.
The higher speed settings weren't so good. I started with 3125000 and that doesn't get onto the 2nd part of the download, same for 1152000 and 4000000 (I stopped trying higher speeds at that point). Using 230400 and 460800 it does make it through the 2nd part of the download but when I go back on to the console it appears to be unresponsive.
All of this could be down to the serial hardware in my system (the x530 uses a real RS232 interface not a TTL) and I have had problems with the MosChip USB-Serial adapter in my test PC in the past. I wouldn't reject this series based on me not being able to get it working, the important thing for me is the default behaviour at the standard baudrate which is good.
You need to choose speed which is supported by both armada board and your computer. Armada boards support speeds calculated by formula: speed = TCLK / ( 16 * d ) where d is 16-bit integer number. So for A385 board with 250 MHz TCLK you can set speeds from above formula +/- 30%. Tested were: 300, 600, 1200, 1800, 2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400, 460800, 500000, 576000, 921600, 1000000, 1152000, 1500000, 2000000, 2500000, 3125000, 4000000, 5150000. If you have a board with different TCLK you obviously need to use different baudrates.
But it is possible that your computer does not support these speeds. E.g. more chinese USB TTL adapters have base clock 3 MHz and so they could not support 3150000 or 2500000 speeds.
One usability thing I'd like to see is retaining support for -t (I use that quite a lot when recovering a system). Ideally we'd still be able to drop into the console at 115200 once the download is complete.
-t is fully supported, I'm using -t with -B 5150000 and it is working fine. After successful download, speed on both A38x and host computer is changed back to the value 115200.
Ah I misread the man page update

On Fri, 27 Aug 2021 14:45:07 +1200 Chris Packham judge.packham@gmail.com wrote:
The higher speed settings weren't so good. I started with 3125000 and that doesn't get onto the 2nd part of the download, same for 1152000 and 4000000 (I stopped trying higher speeds at that point). Using 230400 and 460800 it does make it through the 2nd part of the download but when I go back on to the console it appears to be unresponsive.
Did you try v2 or v1? If you tried v1 with old kwb images (created before the previous kwbimage patch series), this is expected. v2 fixes this issue.
Marek

On Sat, 28 Aug 2021, 12:31 AM Marek Behún, marek.behun@nic.cz wrote:
On Fri, 27 Aug 2021 14:45:07 +1200 Chris Packham judge.packham@gmail.com wrote:
The higher speed settings weren't so good. I started with 3125000 and that doesn't get onto the 2nd part of the download, same for 1152000 and 4000000 (I stopped trying higher speeds at that point). Using 230400 and 460800 it does make it through the 2nd part of the download but when I go back on to the console it appears to be unresponsive.
Did you try v2 or v1? If you tried v1 with old kwb images (created before the previous kwbimage patch series), this is expected. v2 fixes this issue.
I pulled the from the repo/branch you posted and built fresh images.
participants (4)
-
Chris Packham
-
Marek Behún
-
Pali Rohár
-
Pavol Rohár