Skip to content

Commit 6a274cb

Browse files
committed
host_sp_comms: add low-level "debug" interface
When debugging recent IPCC problems, it would have been very useful to have a low-level "debug" interface for sending traffic between host and SP so that we could probe the issues in question on a bench machine. I hacked something for this by overloading an IPCC transaction that wasn't used on the machine I wanted to test on, which was enormously impactful, as it exposed a quartz problem in the FPGA that Nathanael was able to fix quickly. But it was obviously a hack, and moreoever, since it was tied to IPCC (and IPCC was considered suspect at the time), using it felt like a bit of a gamble. Having a dedicated debugging interface that is below the level of IPCC, and not complected with COBS encoding, Hubpack, and so on, would be generally useful. This is the Hubris side of that: I've implemented a parallel protocol for supporting three simple debug messages: 1. A discard message: this simply throws away what is sent to it, with the side effect of emitting a message into a ring buffer. 2. An echo message: this echos the data sent to it back to the distant end, and produces a ring buffer entry. 3. A "chargen" message: this causes $n$ bytes from a repeating pattern of printable ASCII characters to be back to the host, limited by the number requested and size of the output buffer. Since the IPCC header and magic number are fixed, debug messages are easily distinguishable from normal IPCC messages by looking for a prefix string in a received "frame" from the host: the two can never collide.
1 parent 7ecfd02 commit 6a274cb

File tree

4 files changed

+146
-5
lines changed

4 files changed

+146
-5
lines changed

app/cosmo/base.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ features = ["stm32h753", "usart6", "baud_rate_3M", "hardware_flow_control", "vla
256256
uses = ["usart6", "dbgmcu"]
257257
interrupts = {"usart6.irq" = "usart-irq"}
258258
priority = 9
259-
max-sizes = {flash = 69120, ram = 65536}
259+
max-sizes = {flash = 69792, ram = 65536}
260260
stacksize = 5400
261261
start = true
262262
task-slots = ["sys", { cpu_seq = "cosmo_seq" }, "hf", "control_plane_agent", "net", "packrat", "i2c_driver", { spi_driver = "spi2_driver" }, "sprot", "auxflash"]

app/gimlet/base.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ features = ["stm32h753", "uart7", "baud_rate_3M", "hardware_flow_control", "vlan
242242
uses = ["uart7", "dbgmcu"]
243243
interrupts = {"uart7.irq" = "usart-irq"}
244244
priority = 8
245-
max-sizes = {flash = 67680, ram = 65536}
245+
max-sizes = {flash = 68608, ram = 65536}
246246
stacksize = 5376
247247
start = true
248248
task-slots = ["sys", { cpu_seq = "gimlet_seq" }, "hf", "control_plane_agent", "net", "packrat", "i2c_driver", { spi_driver = "spi2_driver" }, "sprot"]

task/host-sp-comms/src/main.rs

Lines changed: 110 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,25 @@ const MAX_HOST_FAIL_MESSAGE_LEN: usize = 4096;
104104
// of that for us.
105105
const NUM_HOST_MAC_ADDRESSES: u16 = 3;
106106

107+
// The same IO path can be used for both IPCC, and a lower-level debugging
108+
// interface, for testing the IO path itself. We differentiate between the two
109+
// by detecting specific header types, and parsing the corresponding messages
110+
// into a typed enum: debug message handling is then short-circuited.
111+
enum DebugCmd<'a> {
112+
Discard,
113+
Echo(&'a [u8]),
114+
CharGen(u16),
115+
}
116+
107117
#[derive(Debug, Clone, Copy, PartialEq, counters::Count)]
108118
enum Trace {
109119
#[count(skip)]
110120
None,
111121
UartRxOverrun,
112122
ParseError(#[count(children)] DecodeFailureReason),
123+
DebugDiscard,
124+
DebugEcho(u64),
125+
DebugCharGen(u16),
113126
SetState {
114127
now: u64,
115128
#[count(children)]
@@ -786,6 +799,56 @@ impl ServerImpl {
786799
}
787800
}
788801

802+
// Process a request message from the host.
803+
fn process_message(
804+
&mut self,
805+
reset_tx_buf: bool,
806+
) -> Result<(), DecodeFailureReason> {
807+
// Debug messages have a distinct header that separates them from normal
808+
// IPCC messages.
809+
if is_debug_message(self.rx_buf) {
810+
self.process_debug_message(reset_tx_buf)
811+
} else {
812+
self.process_ipcc_message(reset_tx_buf)
813+
}
814+
}
815+
816+
// Process a framed debug packet
817+
fn process_debug_message(
818+
&mut self,
819+
reset_tx_buf: bool,
820+
) -> Result<(), DecodeFailureReason> {
821+
match parse_debug_message(self.rx_buf) {
822+
Ok(cmd) => {
823+
if reset_tx_buf {
824+
self.tx_buf.reset();
825+
}
826+
match cmd {
827+
DebugCmd::Discard => ringbuf_entry!(Trace::DebugDiscard),
828+
DebugCmd::Echo(data) => {
829+
ringbuf_entry!(Trace::DebugEcho(data.len() as u64));
830+
let _ = self.tx_buf.try_copy_raw_data(data);
831+
}
832+
DebugCmd::CharGen(count) => {
833+
ringbuf_entry!(Trace::DebugCharGen(count));
834+
// const CHARGEN: &'static [u8] =
835+
// b"!\"#$%&'()*+,-./0123456789:;\
836+
// <=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]\
837+
// ^_abcdefghijklmnopqrstuvwxyz{|}~`";
838+
let mut it = (b'!'..=b'~').cycle().take(count.into());
839+
let _ = self.tx_buf.try_fill(&mut it);
840+
}
841+
}
842+
Ok(())
843+
}
844+
Err(err) => {
845+
ringbuf_entry!(Trace::ParseError(err));
846+
self.rx_buf.clear();
847+
Err(err)
848+
}
849+
}
850+
}
851+
789852
// Process the framed packet sitting in `self.rx_buf`. If it warrants a
790853
// response, we configure `self.tx_buf` appropriate: either populating it
791854
// with a response if we can come up with that response immediately, or
@@ -800,10 +863,11 @@ impl ServerImpl {
800863
//
801864
// This method always (i.e., on success or failure) clears `rx_buf` before
802865
// returning to prepare for the next packet.
803-
fn process_message(
866+
fn process_ipcc_message(
804867
&mut self,
805868
reset_tx_buf: bool,
806869
) -> Result<(), DecodeFailureReason> {
870+
// If the message was Not a debug message, then parse it normally.
807871
let (header, request, data) = match parse_received_message(self.rx_buf)
808872
{
809873
Ok((header, request, data)) => (header, request, data),
@@ -1663,8 +1727,9 @@ impl NotificationHandler for ServerImpl {
16631727
}
16641728
}
16651729

1666-
// This is conceptually a method on `ServerImpl`, but it takes a reference to
1667-
// `rx_buf` instead of `self` to avoid borrow checker issues.
1730+
// Parse a received message. This is conceptually a method on `ServerImpl`,
1731+
// but it takes a reference to `rx_buf` instead of `self` to avoid borrow
1732+
// checker issues.
16681733
fn parse_received_message(
16691734
rx_buf: &mut [u8],
16701735
) -> Result<(Header, HostToSp, &[u8]), DecodeFailureReason> {
@@ -1690,6 +1755,48 @@ fn parse_received_message(
16901755
Ok((header, request, data))
16911756
}
16921757

1758+
// Debug messages have a distinct header that separates them from IPCC
1759+
// messages. The first 5 bytes are the ASCII characters, "DEBUG". The
1760+
// 6th and 7th bytes encode the command number, as 0 padded ASCII hexadecimal
1761+
// string (using upper case for the non-numeric hexadigits). The commands
1762+
// are:
1763+
//
1764+
// "07" (0x07 == 7 dec) Echo
1765+
// "09" (0x09 == 9 dec) Discard
1766+
// "13" (0x13 == 19 dec) CharGen
1767+
//
1768+
// The command values correspond to the IANA-assigned port numbers for the
1769+
// corresponding UDP and TCP/IP services.
1770+
//
1771+
// This is conceptually a method on `ServerImpl`, but it takes references to
1772+
// several of its fields instead of `self` to avoid borrow checker issues.
1773+
fn is_debug_message(msg: &[u8]) -> bool {
1774+
if msg.len() < 7 {
1775+
return false;
1776+
}
1777+
msg[0..5] == *b"DEBUG"
1778+
}
1779+
1780+
// This is conceptually a method on `ServerImpl`, but it takes references to
1781+
// several of its fields instead of `self` to avoid borrow checker issues.
1782+
fn parse_debug_message(
1783+
msg: &[u8],
1784+
) -> Result<DebugCmd<'_>, DecodeFailureReason> {
1785+
assert!(is_debug_message(msg));
1786+
match &msg[5..7] {
1787+
b"07" => Ok(DebugCmd::Echo(&msg[7..])),
1788+
b"09" => Ok(DebugCmd::Discard),
1789+
b"13" if msg.len() == 11 => {
1790+
let nstr = str::from_utf8(&msg[7..11])
1791+
.map_err(|_| DecodeFailureReason::Deserialize)?;
1792+
let n = u16::from_str_radix(nstr, 16)
1793+
.map_err(|_| DecodeFailureReason::Deserialize)?;
1794+
Ok(DebugCmd::CharGen(n))
1795+
}
1796+
_ => Err(DecodeFailureReason::Deserialize),
1797+
}
1798+
}
1799+
16931800
// This is conceptually a method on `ServerImpl`, but it takes references to
16941801
// several of its fields instead of `self` to avoid borrow checker issues.
16951802
fn handle_reboot_waiting_in_a2_timer(

task/host-sp-comms/src/tx_buf.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,40 @@ impl TxBuf {
293293
let n = corncobs::encode_buf(&self.msg[..msg_len], &mut self.pkt[1..]);
294294
self.state = State::ToSend(0..n + 1);
295295
}
296+
297+
// Copies a "raw" slice of bytes into the output packet buffer.
298+
pub(crate) fn try_copy_raw_data(&mut self, bs: &[u8]) -> Result<usize, ()> {
299+
let n = usize::min(self.pkt.len() - 2, bs.len());
300+
if bs[..n].contains(&0) {
301+
return Err(());
302+
}
303+
let end = n + 1;
304+
self.pkt[0] = 0;
305+
self.pkt[1..end].copy_from_slice(&bs[..n]);
306+
self.pkt[end] = 0;
307+
self.state = State::ToSend(0..end + 1);
308+
Ok(n)
309+
}
310+
311+
pub(crate) fn try_fill(
312+
&mut self,
313+
it: &mut impl Iterator<Item = u8>,
314+
) -> Result<(), ()> {
315+
let max = self.pkt.len() - 2;
316+
let mut idx = 0;
317+
self.pkt[idx] = 0;
318+
idx += 1;
319+
for b in it.take(max) {
320+
if b == 0 {
321+
return Err(());
322+
}
323+
self.pkt[idx] = b;
324+
idx += 1;
325+
}
326+
self.pkt[idx] = 0;
327+
self.state = State::ToSend(0..idx + 1);
328+
Ok(())
329+
}
296330
}
297331

298332
enum State {

0 commit comments

Comments
 (0)