| Message ID | 20220125171313.14498-7-kabel@kernel.org |
|---|---|
| State | Accepted |
| Commit | 5875ad48e2c5119c629da3751c49ad5e3f0c96b0 |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | Another set of kwboot improvements | expand |
On 1/25/22 18:13, Marek Behún wrote: > From: Pali Rohár <pali@kernel.org> > > Unfortunately during some stages of xmodem transfer, A385 BootROM is not > able to handle repeated xmodem packets. So if an error occurs during that > stage, stop the transfer and return failure. > > Signed-off-by: Pali Rohár <pali@kernel.org> > Signed-off-by: Marek Behún <marek.behun@nic.cz> Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > tools/kwboot.c | 28 +++++++++++++++++++++++++--- > 1 file changed, 25 insertions(+), 3 deletions(-) > > diff --git a/tools/kwboot.c b/tools/kwboot.c > index a619a6c3c1..dfac8aed7b 100644 > --- a/tools/kwboot.c > +++ b/tools/kwboot.c > @@ -946,7 +946,7 @@ kwboot_xm_recv_reply(int fd, char *c, int stop_on_non_xm, > > static int > kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, > - int *done_print, int baudrate) > + int *done_print, int baudrate, int allow_retries) > { > int non_xm_print, baud_changed; > int rc, err, retries; > @@ -977,7 +977,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, > > if (!allow_non_xm && c != ACK) > kwboot_progress(-1, '+'); > - } while (c == NAK && retries++ < 16); > + } while (c == NAK && allow_retries && retries++ < 16); > > if (non_xm_print) > kwboot_printv("\n"); > @@ -1044,8 +1044,30 @@ kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data, > > last_block = (left <= blksz); > > + /* > + * Handling of repeated xmodem packets is completely broken in > + * Armada 385 BootROM - it completely ignores xmodem packet > + * numbers, they are only used for checksum verification. > + * BootROM can handle a retry of the xmodem packet only during > + * the transmission of kwbimage header and only if BootROM > + * itself sent NAK response to previous attempt (it does it on > + * checksum failure). During the transmission of kwbimage data > + * part, BootROM always expects next xmodem packet, even if it > + * sent NAK to previous attempt - there is absolutely no way to > + * repair incorrectly transmitted xmodem packet during kwbimage > + * data part upload. Also, if kwboot receives non-ACK/NAK > + * response (meaning that original BootROM response was damaged > + * on UART) there is no way to detect if BootROM accepted xmodem > + * packet or not and no way to check if kwboot could repeat the > + * packet or not. > + * > + * Stop transfer and return failure if kwboot receives unknown > + * reply if non-xmodem reply is not allowed (for all xmodem > + * packets except the last header packet) or when non-ACK reply > + * is received during data part transfer. > + */ > rc = kwboot_xm_sendblock(tty, &block, header && last_block, > - &done_print, baudrate); > + &done_print, baudrate, header); > if (rc) > goto out; > Viele Grüße, Stefan Roese
diff --git a/tools/kwboot.c b/tools/kwboot.c index a619a6c3c1..dfac8aed7b 100644 --- a/tools/kwboot.c +++ b/tools/kwboot.c @@ -946,7 +946,7 @@ kwboot_xm_recv_reply(int fd, char *c, int stop_on_non_xm, static int kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, - int *done_print, int baudrate) + int *done_print, int baudrate, int allow_retries) { int non_xm_print, baud_changed; int rc, err, retries; @@ -977,7 +977,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int allow_non_xm, if (!allow_non_xm && c != ACK) kwboot_progress(-1, '+'); - } while (c == NAK && retries++ < 16); + } while (c == NAK && allow_retries && retries++ < 16); if (non_xm_print) kwboot_printv("\n"); @@ -1044,8 +1044,30 @@ kwboot_xmodem_one(int tty, int *pnum, int header, const uint8_t *data, last_block = (left <= blksz); + /* + * Handling of repeated xmodem packets is completely broken in + * Armada 385 BootROM - it completely ignores xmodem packet + * numbers, they are only used for checksum verification. + * BootROM can handle a retry of the xmodem packet only during + * the transmission of kwbimage header and only if BootROM + * itself sent NAK response to previous attempt (it does it on + * checksum failure). During the transmission of kwbimage data + * part, BootROM always expects next xmodem packet, even if it + * sent NAK to previous attempt - there is absolutely no way to + * repair incorrectly transmitted xmodem packet during kwbimage + * data part upload. Also, if kwboot receives non-ACK/NAK + * response (meaning that original BootROM response was damaged + * on UART) there is no way to detect if BootROM accepted xmodem + * packet or not and no way to check if kwboot could repeat the + * packet or not. + * + * Stop transfer and return failure if kwboot receives unknown + * reply if non-xmodem reply is not allowed (for all xmodem + * packets except the last header packet) or when non-ACK reply + * is received during data part transfer. + */ rc = kwboot_xm_sendblock(tty, &block, header && last_block, - &done_print, baudrate); + &done_print, baudrate, header); if (rc) goto out;