Skip to content

Commit

Permalink
[opentitantool] Multi-byte read()/write() in serial console
Browse files Browse the repository at this point in the history
The serial console library suffers from the need not to consume any
additional bytes, if one of the regular expressions matches, for which
we have found no better solution than to read a single byte at a time.

At Google, we often use `opentitantool console` and pipe the output into
other test scripts (which may do regular expressions).  In such cases,
the opentitanlib is not performing any regular expression matching, but
it still reading a single byte at a time, and writing each byte using a
separate `write()` operating system call.  This is highly inefficient,
when the data goes through anonymous pipes, and is often further
processed just one or a few bytes at a time by other processes.

This CL modifies the logic in `uart/console.rs` to recognize the case of
there not being any regular expressions to match, and in that case read
up to 1024 bytes at a time from the transport USB device.

I plan on combining this with below enhancements to HyperDebug firmware,
to make it preferably compose USB packets with multiple characters
captured on its UARTs.

https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/6020315

Change-Id: I4958a891c842a82d20de1d323a69d5510d3f812e
Signed-off-by: Jes B. Klinke <[email protected]>
  • Loading branch information
jesultra committed Nov 14, 2024
1 parent 9da36ca commit 72642c0
Showing 1 changed file with 20 additions and 3 deletions.
23 changes: 20 additions & 3 deletions sw/host/opentitanlib/src/uart/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,14 @@ impl UartConsole {
Token(TOKEN_COUNTER.fetch_add(1, Ordering::Relaxed))
}

/// Returns `true` if any regular expressions are used to match the streamed output. If so,
/// then this struct will keep a window of the most recent output, and take care not to read
/// any more characters from the underlying stream should one of the regular expressions
/// match.
fn uses_regex(&self) -> bool {
self.exit_success.is_some() || self.exit_failure.is_some()
}

// Maintain a buffer for the exit regexes to match against.
fn append_buffer(&mut self, data: &[u8]) {
self.buffer.push_str(&String::from_utf8_lossy(data));
Expand All @@ -199,8 +207,15 @@ impl UartConsole {
where
T: ConsoleDevice + ?Sized,
{
let mut buf = [0u8; 1];
let len = device.console_read(&mut buf, timeout)?;
let mut buf = [0u8; 1024];
let effective_buf = if self.uses_regex() {
// Read one byte at a time when matching, to avoid the risk of consuming output past a
// match.
&mut buf[..1]
} else {
&mut buf
};
let len = device.console_read(effective_buf, timeout)?;
if len == 0 {
return Ok(false);
}
Expand All @@ -223,7 +238,9 @@ impl UartConsole {
self.logfile
.as_mut()
.map_or(Ok(()), |f| f.write_all(&buf[..len]))?;
self.append_buffer(&buf[..len]);
if self.uses_regex() {
self.append_buffer(&buf[..len]);
}
Ok(true)
}

Expand Down

0 comments on commit 72642c0

Please sign in to comment.