boot: serial: end serial recovery on inactivity

[TheArkadiuszGrzelka]

What is broken today

Serial recovery has no way out once it is entered. boot_serial_enter()
calls boot_serial_start(), which sets bs_entry and never clears it, and
the every-boot wait (CONFIG_BOOT_SERIAL_WAIT_FOR_DFU) latches the same flag
on the first MCUmgr command, so the read loop in boot_serial_read_console()
runs until the device is reset by hand.

That is fine when a human is standing next to the board. It is not fine for a
field device whose only recovery transport is the same wire the application
talks on: an aborted upload strands a unit that has a perfectly good image to
fall back on, and no watchdog can rescue it, because MCUBOOT_WATCHDOG_FEED()
runs inside that very loop. A hung recovery is fed exactly like a busy one.

Why a plain timeout is the wrong shape

CONFIG_BOOT_SERIAL_WAIT_FOR_DFU already has a timeout, but it is a
pre-transfer window: the first MCUmgr command latches bs_entry and the wait
becomes permanent. Bounding the whole session instead would cut a slow transfer
in half - a full image over a 9600 baud RS485 link takes minutes.

The discriminating signal is data, not elapsed time. A slow transfer keeps
delivering MCUmgr commands; an aborted one goes silent.

The change

boot_serial_start_inactivity() counts down like boot_serial_check_start(),
but every received command restarts the countdown instead of latching
bs_entry, and it reports whether any command arrived at all. A transfer in
progress extends the wait for as long as it keeps making progress, however slow
the link; only silence ends recovery.

Exposed as CONFIG_BOOT_SERIAL_INACTIVITY_TIMEOUT (int, ms, default 0),
offered under both entrance methods that have an installed image to resume:
BOOT_SERIAL_BOOT_MODE and BOOT_SERIAL_WAIT_FOR_DFU.

Nothing existing is removed or re-gated:

• At the default of 0 nothing new is compiled in, and the image comes out the
  same size as one built without the patch (see the table below).
• CONFIG_BOOT_SERIAL_NO_APPLICATION stays unbounded on purpose - with no
  bootable image there is nothing to resume, so ending recovery there would be
  strictly worse. The GPIO and pin-reset entrances stay unbounded too.

Why it exits by resetting the SoC

The image was selected by boot_go() before recovery was entered, so returning
into the boot path after an upload would jump to the image that upload has just
replaced. Nothing in the bootloader unwinds the state a finished MCUmgr session
leaves behind either. Returning was tried first and locks the CPU up on a
Cortex-M0+ (STM32C071): a variant that returns instead of resetting ends in
lockup once a single MCUmgr command has been received before the silence, with
PC 0xFFFFFFFE, S_LOCKUP set in DHCSR, and MSP loaded from the image header
magic 0x96F3B83D instead of a vector table. The same image with no command
received boots the application normally. A reset costs one boot, starts from a state the
bootloader already knows how to handle, and is the same exit MCUmgr's own reset
command takes.

Under BOOT_SERIAL_WAIT_FOR_DFU the reset is taken only if at least one
command was received. A window that expires with no command at all is not a
session, and boots exactly as it does today.

A non-zero timeout therefore requires CONFIG_REBOOT, expressed as a Kconfig
dependency.

boot_serial_enter() takes the timeout as an argument rather than being
duplicated, so the status callback, the indication LED, the log line and the
boot_console_init() error check stay in one place for every entrance method.

Verification

Size, on pic32cm_gc00_cpro/pic32cm5112gc00100 (Cortex-M23), WAIT_FOR_DFU
entrance, this branch against its merge base:

• BOOT_SERIAL_INACTIVITY_TIMEOUT=0: the MCUboot binary is byte for byte
  identical to one built without this patch, and boot_serial_start_inactivity
  is not linked in.
• BOOT_SERIAL_INACTIVITY_TIMEOUT=30000: 41000 B against 40836 B, so 164 B.

A build with REBOOT=n was checked too: the symbol falls back to 0 and the
image matches the timeout-unset one. No new warnings in any of the builds.

On hardware: STM32C071 (Cortex-M0+), MCUmgr and Modbus sharing one RS485 line
at 9600 baud, BOOT_SERIAL_INACTIVITY_TIMEOUT=30000, and for the last two runs
BOOT_SERIAL_WAIT_FOR_DFU=y with BOOT_SERIAL_WAIT_FOR_DFU_TIMEOUT=5000. Time
is measured from the last byte the host sent to the first answer from the
application.

Run	Entrance	Measured	What it shows
Idle abort	boot mode	165 s	recovery ends on its own, running the image it already had
Full upload	boot mode	35027/35027 B in 77.9 s	a transfer far longer than the timeout is not cut short
Empty window	WAIT_FOR_DFU	22.6 s	an empty window is not a session, plain return into the boot path
One command, then silence	WAIT_FOR_DFU	184.8 s	a session takes the reset exit

The 165 s against a 30 s setting is not this patch. boot_serial_read_console()
charges the timeout for f->read() only, while the two k_uptime_get_32()
calls that do the measuring fall outside the measured window, and on a core
without hardware divide they dominate: sampling the PC over SWD through a whole
window put 72% of the samples in __udivmoddi4, __udivsi3 and
sys_clock_tick_get, against 4.8% in console_read. The same effect measures
3.2x on the Cortex-M23 board above. It is pre-existing and reported as #2841;
the countdown stays proportional, and every run in the table is consistent with
one factor.

• Known limitation, unchanged by this patch: in a configuration with no
  staging slot, an upload aborted mid-transfer has already overwritten the
  primary slot. The timeout fires and the device resets, but with no valid
  image it lands in BOOT_SERIAL_NO_APPLICATION and waits there for a retry.
  That is the intended behaviour; no timeout value can conjure back an image
  that was overwritten.

[nordicjm]

the new parts should be gated that when it's disabled, there are no code changes. Also fix issues in whole PR, they have been highlighted only once

[TheArkadiuszGrzelka]

All four points are addressed, and the gating request too: everything the option adds is behind MCUBOOT_SERIAL_BOOT_MODE_INACTIVITY_TIMEOUT, so at the default of 0 the read loop, the command handler and boot_serial_start() / boot_serial_check_start() compile exactly as they do today - no code changes when it is disabled. The file-scope bs_timeout_ms is gone; the countdown is back to the local timeout_in_ms, with a bs_rearm flag only under that guard.

[TheArkadiuszGrzelka]

Re-measured everything on hardware against this branch, not against the v2.4.0
backport the description quotes. Board: STM32C071 (Cortex-M0+), MCUmgr and
Modbus sharing one RS485 line at 9600 baud, BOOT_SERIAL_INACTIVITY_TIMEOUT=30000,
and for the last two runs BOOT_SERIAL_WAIT_FOR_DFU=y with
BOOT_SERIAL_WAIT_FOR_DFU_TIMEOUT=5000. Time is measured from the last byte the
host sent to the first answer from the application.

Run	Entrance	Measured	What it shows
Idle abort	boot mode	165 s	recovery ends on its own, same image as before
Full upload	boot mode	35027/35027 B in 77.9 s	a transfer far longer than the timeout is not cut short
Empty window	WAIT_FOR_DFU	22.6 s	an empty window is not a session, plain return into the boot path
One command, then silence	WAIT_FOR_DFU	184.8 s	a session takes the reset exit

Correction to the description

The 33.1 s in the description does not reproduce against this branch: the same
scenario now takes 165 s with a 30000 ms timeout. The countdown itself is fine,
the time accounting in boot_serial_read_console() is not:

MCUBOOT_WATCHDOG_FEED();
uint32_t start = k_uptime_get_32();
rc = f->read(...);
elapsed_in_ms = (k_uptime_get_32() - start);
timeout_in_ms -= elapsed_in_ms;

Only f->read() is charged to the timeout. The two k_uptime_get_32() calls
that do the measuring are outside the measured window, and on a core without
hardware divide they are not cheap: sampling the PC over SWD through a whole
recovery window put 72% of the samples in __udivmoddi4, __udivsi3 and
sys_clock_tick_get, against 4.8% in console_read. That gives the 5.5x here
and 3.2x on a Cortex-M23 board I checked the same way. The run-4 figure follows
from it exactly: 165 s for the 30 s timeout plus about 20 s for the 5 s window
on the next boot is 185 s, measured 184.8 s.

This is pre-existing and independent of this patch, which only makes the number
worth trusting for the first time. I will report it separately rather than fold
it in here.

Why it resets instead of returning

I built a variant with sys_reboot() replaced by a plain return, and ran it
twice on the same image:

• No command received, then silence: the application starts normally, PC in the
  application at +164 s and still there 130 s later.
• Exactly one MCUmgr command, then silence: the core is in lockup at the same
  point. PC 0xFFFFFFFE, DHCSR 0x01080001 with S_LOCKUP set, IPSR 3, and
  MSP 0x96F3B818, which is the image header magic 0x96F3B83D. The stack
  pointer came from the image header instead of a vector table.

The single MCUmgr command is the only difference between the two runs.

The option when it is off

Checked on a second SoC, a PIC32CM5112GC00100 (Cortex-M23), WAIT_FOR_DFU
entrance: with BOOT_SERIAL_INACTIVITY_TIMEOUT=0 the MCUboot binary is byte for
byte identical to one built without this patch, and boot_serial_start_inactivity
is not linked in. Enabled, it costs 164 B (40836 to 41000). I am replacing the
nrf52840dk table in the description with this, since it is a measurement I can
reproduce on request.

Kconfig

That is in the current diff: BOOT_SERIAL_BOOT_MODE is a menuconfig now, and
BOOT_SERIAL_INACTIVITY_TIMEOUT is offered under both entrances, with the help
text only on the boot-mode one.

[nordicjm]

tried setting BOOT_SERIAL_WAIT_FOR_DFU_TIMEOUT to 2500 and BOOT_SERIAL_INACTIVITY_TIMEOUT to 3000, and it enters, I send a command, I get a response, I wait and it boots the application, however it seems to wait 9 seconds instead of 3, wondering if a #2842 fix is needed?

[TheArkadiuszGrzelka]

Yes, that is exactly what #2842 fixes. boot_serial_read_console() charged only the time spent inside f->read() against the timeout, so the watchdog feed, the decode and dispatch of a received line and the two uptime reads per iteration all went uncharged, and the overshoot grows on slower cores where reading the uptime is a software division. Measured on an STM32C071 at 48 MHz, a 5000 ms window ran 22.6 s before the fix and 6.0 s after. With 2500 and 3000 your run should land near 3 s once it is applied.

#2842 has just been updated to your style comment and repushed on current main, so testing the two together would confirm it from your side as well.