Skip to content

Commit 4cd4ef9

Browse files
committed
[opentitanlib,qemu] Add additional SPI console TX sync for QEMU
This is only enabled if the QEMU interface is configured for the SPI console - any other interface will retain the existing behaviour and avoid the additional poling from the host side. For QEMU specifically, we introduce a host-side poll to see the TX ready pin go low after reading out the header (but before reading out the payload). This ensures we don't run into synchronization issues due to emulation for very small SPI transfers in which the entirity of the header & payload are read out by the host before the device has a chance to run enough guest SW to signal that the GPIO ready pin should go low. In such a scenario, the device would look to host SW as if it is ready to send more data, even though in reality the guest SW has barely progressed enough to register that the host has started reading, causing the two devices to get out of sync. Signed-off-by: Alex Jones <[email protected]>
1 parent ff79735 commit 4cd4ef9

File tree

19 files changed

+99
-12
lines changed

19 files changed

+99
-12
lines changed

sw/host/opentitanlib/src/console/spi.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub struct SpiConsoleDevice<'a> {
2121
next_read_address: Cell<u32>,
2222
device_tx_ready_pin: Option<&'a Rc<dyn GpioPin>>,
2323
ignore_frame_num: bool,
24+
wait_tx_ready_deassert: bool,
2425
}
2526

2627
impl<'a> SpiConsoleDevice<'a> {
@@ -36,10 +37,16 @@ impl<'a> SpiConsoleDevice<'a> {
3637
spi: &'a dyn Target,
3738
device_tx_ready_pin: Option<&'a Rc<dyn GpioPin>>,
3839
ignore_frame_num: bool,
40+
interface: Option<&str>,
3941
) -> Result<Self> {
4042
let flash = SpiFlash {
4143
..Default::default()
4244
};
45+
// QEMU is currently the only interface that requires additional
46+
// synchronization on the TX-ready indicator pin to avoid timing
47+
// & synchronization issues for small transactions, due to its
48+
// nature as an emulated interface.
49+
let wait_tx_ready_deassert = interface == Some("qemu");
4350
Ok(Self {
4451
spi,
4552
flash,
@@ -48,6 +55,7 @@ impl<'a> SpiConsoleDevice<'a> {
4855
next_read_address: Cell::new(0),
4956
device_tx_ready_pin,
5057
ignore_frame_num,
58+
wait_tx_ready_deassert,
5159
})
5260
}
5361

@@ -75,6 +83,19 @@ impl<'a> SpiConsoleDevice<'a> {
7583
let mut header = vec![0u8; SpiConsoleDevice::SPI_FRAME_HEADER_SIZE];
7684
self.read_data(read_address, &mut header)?;
7785

86+
// When using small (<= 4 byte) SPI transactions on emulation targets,
87+
// sometimes the entire transaction can occur before the logic low
88+
// transmission of the TX-indicator GPIO is issued, causing the host
89+
// to erroneously see the device as ready and start another transaction
90+
// whilst the device is still processing the previous. For such targets,
91+
// add additional synchronization by waiting for the TX-indicator pin
92+
// to go low after reading the header.
93+
if let Some(ready_pin) = self.get_tx_ready_pin()? {
94+
if self.wait_tx_ready_deassert {
95+
while ready_pin.read()? {}
96+
}
97+
}
98+
7899
let magic_number: u32 = u32::from_le_bytes(header[0..4].try_into().unwrap());
79100
let frame_number: u32 = u32::from_le_bytes(header[4..8].try_into().unwrap());
80101
let data_len_bytes: usize = u32::from_le_bytes(header[8..12].try_into().unwrap()) as usize;

sw/host/provisioning/cp/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ fn main() -> Result<()> {
5454
&*spi,
5555
Some(device_console_tx_ready_pin),
5656
/*ignore_frame_num=*/ false,
57+
Some(opts.init.backend_opts.interface.as_str()),
5758
)?;
5859

5960
let provisioning_data = ManufCpProvisioningData {

sw/host/provisioning/ft/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ fn main() -> Result<()> {
133133
&*spi,
134134
Some(device_console_tx_ready_pin),
135135
/*ignore_frame_num=*/ false,
136+
Some(opts.init.backend_opts.interface.as_str()),
136137
)?;
137138
InitializeTest::print_result("load_bitstream", opts.init.load_bitstream.init(&transport))?;
138139

sw/host/tests/chip/ottf_console_with_gpio_tx_indicator/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ fn spi_device_console_test(opts: &Opts, transport: &TransportWrapper) -> Result<
4646
&*spi,
4747
Some(device_console_tx_ready_pin),
4848
/*ignore_frame_num=*/ false,
49+
Some(opts.init.backend_opts.interface.as_str()),
4950
)?;
5051

5152
// Load the ELF binary and get the expect data.

sw/host/tests/chip/spi_device_ottf_console/src/main.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,12 @@ fn spi_device_console_test(opts: &Opts, transport: &TransportWrapper) -> Result<
4848
let mut stdout = std::io::stdout();
4949
let spi = transport.spi(&opts.console_spi)?;
5050

51-
let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?;
51+
let spi_console_device = SpiConsoleDevice::new(
52+
&*spi,
53+
None,
54+
/*ignore_frame_num=*/ false,
55+
Some(opts.init.backend_opts.interface.as_str()),
56+
)?;
5257
let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?;
5358

5459
/* Load the ELF binary and get the expect data.*/

sw/host/tests/chip/spi_device_ujson_console_test/src/main.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,12 @@ fn main() -> Result<()> {
109109

110110
let transport = opts.init.init_target()?;
111111
let spi = transport.spi(&opts.console_spi)?;
112-
let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?;
112+
let spi_console_device = SpiConsoleDevice::new(
113+
&*spi,
114+
None,
115+
/*ignore_frame_num=*/ false,
116+
Some(opts.init.backend_opts.interface.as_str()),
117+
)?;
113118
let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?;
114119

115120
execute_test!(test_perso_blob_strcut, &opts, &spi_console_device);

sw/host/tests/crypto/aes_gcm_nist_kat/src/main.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,12 @@ fn run_aes_gcm_testcase(
137137

138138
fn test_aes_gcm(opts: &Opts, transport: &TransportWrapper) -> Result<()> {
139139
let spi = transport.spi("BOOTSTRAP")?;
140-
let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?;
140+
let spi_console_device = SpiConsoleDevice::new(
141+
&*spi,
142+
None,
143+
/*ignore_frame_num=*/ false,
144+
Some(opts.init.backend_opts.interface.as_str()),
145+
)?;
141146
let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?;
142147

143148
let mut test_counter = 0u32;

sw/host/tests/crypto/aes_nist_kat/src/main.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,12 @@ fn run_aes_testcase(
130130

131131
fn test_aes(opts: &Opts, transport: &TransportWrapper) -> Result<()> {
132132
let spi = transport.spi("BOOTSTRAP")?;
133-
let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?;
133+
let spi_console_device = SpiConsoleDevice::new(
134+
&*spi,
135+
None,
136+
/*ignore_frame_num=*/ false,
137+
Some(opts.init.backend_opts.interface.as_str()),
138+
)?;
134139
let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?;
135140

136141
let mut test_counter = 0u32;

sw/host/tests/crypto/drbg_kat/src/main.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,12 @@ fn run_drbg_testcase(
128128

129129
fn test_drbg(opts: &Opts, transport: &TransportWrapper) -> Result<()> {
130130
let spi = transport.spi("BOOTSTRAP")?;
131-
let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?;
131+
let spi_console_device = SpiConsoleDevice::new(
132+
&*spi,
133+
None,
134+
/*ignore_frame_num=*/ false,
135+
Some(opts.init.backend_opts.interface.as_str()),
136+
)?;
132137
let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?;
133138

134139
let mut test_counter = 0u32;

sw/host/tests/crypto/ecdh_kat/src/main.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,12 @@ fn run_ecdh_testcase(
220220

221221
fn test_ecdh(opts: &Opts, transport: &TransportWrapper) -> Result<()> {
222222
let spi = transport.spi("BOOTSTRAP")?;
223-
let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?;
223+
let spi_console_device = SpiConsoleDevice::new(
224+
&*spi,
225+
None,
226+
/*ignore_frame_num=*/ false,
227+
Some(opts.init.backend_opts.interface.as_str()),
228+
)?;
224229
let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?;
225230

226231
let mut test_counter = 0u32;

0 commit comments

Comments
 (0)