From fe9b285ea93f3340949251e620ebc8be3dfc2236 Mon Sep 17 00:00:00 2001 From: Alex Jones Date: Tue, 18 Nov 2025 12:57:42 +0000 Subject: [PATCH 1/2] [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 --- sw/host/opentitanlib/src/console/spi.rs | 21 +++++++++++++++++++ sw/host/provisioning/cp/src/main.rs | 1 + sw/host/provisioning/ft/src/main.rs | 1 + .../src/main.rs | 1 + .../chip/spi_device_ottf_console/src/main.rs | 7 ++++++- .../spi_device_ujson_console_test/src/main.rs | 7 ++++++- .../tests/crypto/aes_gcm_nist_kat/src/main.rs | 7 ++++++- sw/host/tests/crypto/aes_nist_kat/src/main.rs | 7 ++++++- sw/host/tests/crypto/drbg_kat/src/main.rs | 7 ++++++- sw/host/tests/crypto/ecdh_kat/src/main.rs | 7 ++++++- sw/host/tests/crypto/ecdsa_kat/src/main.rs | 7 ++++++- sw/host/tests/crypto/hash_kat/src/main.rs | 7 ++++++- sw/host/tests/crypto/hmac_kat/src/main.rs | 7 ++++++- sw/host/tests/crypto/kmac_kat/src/main.rs | 7 ++++++- sw/host/tests/crypto/rsa_kat/src/main.rs | 7 ++++++- .../tests/crypto/sphincsplus_kat/src/main.rs | 7 ++++++- .../manuf/cp_provision_functest/src/main.rs | 1 + .../manuf/manuf_ujson_msg_padding/src/main.rs | 1 + .../manuf/manuf_ujson_msg_size/src/main.rs | 1 + 19 files changed, 99 insertions(+), 12 deletions(-) diff --git a/sw/host/opentitanlib/src/console/spi.rs b/sw/host/opentitanlib/src/console/spi.rs index 2fbbf3f4996f4..d083ec4c90543 100644 --- a/sw/host/opentitanlib/src/console/spi.rs +++ b/sw/host/opentitanlib/src/console/spi.rs @@ -21,6 +21,7 @@ pub struct SpiConsoleDevice<'a> { next_read_address: Cell, device_tx_ready_pin: Option<&'a Rc>, ignore_frame_num: bool, + wait_tx_ready_deassert: bool, } impl<'a> SpiConsoleDevice<'a> { @@ -36,10 +37,16 @@ impl<'a> SpiConsoleDevice<'a> { spi: &'a dyn Target, device_tx_ready_pin: Option<&'a Rc>, ignore_frame_num: bool, + interface: Option<&str>, ) -> Result { let flash = SpiFlash { ..Default::default() }; + // QEMU is currently the only interface that requires additional + // synchronization on the TX-ready indicator pin to avoid timing + // & synchronization issues for small transactions, due to its + // nature as an emulated interface. + let wait_tx_ready_deassert = interface == Some("qemu"); Ok(Self { spi, flash, @@ -48,6 +55,7 @@ impl<'a> SpiConsoleDevice<'a> { next_read_address: Cell::new(0), device_tx_ready_pin, ignore_frame_num, + wait_tx_ready_deassert, }) } @@ -75,6 +83,19 @@ impl<'a> SpiConsoleDevice<'a> { let mut header = vec![0u8; SpiConsoleDevice::SPI_FRAME_HEADER_SIZE]; self.read_data(read_address, &mut header)?; + // When using small (<= 4 byte) SPI transactions on emulation targets, + // sometimes the entire transaction can occur before the logic low + // transmission of the TX-indicator GPIO is issued, causing the host + // to erroneously see the device as ready and start another transaction + // whilst the device is still processing the previous. For such targets, + // add additional synchronization by waiting for the TX-indicator pin + // to go low after reading the header. + if let Some(ready_pin) = self.get_tx_ready_pin()? { + if self.wait_tx_ready_deassert { + while ready_pin.read()? {} + } + } + let magic_number: u32 = u32::from_le_bytes(header[0..4].try_into().unwrap()); let frame_number: u32 = u32::from_le_bytes(header[4..8].try_into().unwrap()); let data_len_bytes: usize = u32::from_le_bytes(header[8..12].try_into().unwrap()) as usize; diff --git a/sw/host/provisioning/cp/src/main.rs b/sw/host/provisioning/cp/src/main.rs index c588211929c67..a4c663d2264e8 100644 --- a/sw/host/provisioning/cp/src/main.rs +++ b/sw/host/provisioning/cp/src/main.rs @@ -54,6 +54,7 @@ fn main() -> Result<()> { &*spi, Some(device_console_tx_ready_pin), /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), )?; let provisioning_data = ManufCpProvisioningData { diff --git a/sw/host/provisioning/ft/src/main.rs b/sw/host/provisioning/ft/src/main.rs index 69eb1d722a663..95bedd8ff4186 100644 --- a/sw/host/provisioning/ft/src/main.rs +++ b/sw/host/provisioning/ft/src/main.rs @@ -133,6 +133,7 @@ fn main() -> Result<()> { &*spi, Some(device_console_tx_ready_pin), /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), )?; InitializeTest::print_result("load_bitstream", opts.init.load_bitstream.init(&transport))?; diff --git a/sw/host/tests/chip/ottf_console_with_gpio_tx_indicator/src/main.rs b/sw/host/tests/chip/ottf_console_with_gpio_tx_indicator/src/main.rs index 889baeb63dbff..fa50df4bb9ad7 100644 --- a/sw/host/tests/chip/ottf_console_with_gpio_tx_indicator/src/main.rs +++ b/sw/host/tests/chip/ottf_console_with_gpio_tx_indicator/src/main.rs @@ -46,6 +46,7 @@ fn spi_device_console_test(opts: &Opts, transport: &TransportWrapper) -> Result< &*spi, Some(device_console_tx_ready_pin), /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), )?; // Load the ELF binary and get the expect data. diff --git a/sw/host/tests/chip/spi_device_ottf_console/src/main.rs b/sw/host/tests/chip/spi_device_ottf_console/src/main.rs index e8a9911f239e1..6d30daa284a71 100644 --- a/sw/host/tests/chip/spi_device_ottf_console/src/main.rs +++ b/sw/host/tests/chip/spi_device_ottf_console/src/main.rs @@ -48,7 +48,12 @@ fn spi_device_console_test(opts: &Opts, transport: &TransportWrapper) -> Result< let mut stdout = std::io::stdout(); let spi = transport.spi(&opts.console_spi)?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; /* Load the ELF binary and get the expect data.*/ diff --git a/sw/host/tests/chip/spi_device_ujson_console_test/src/main.rs b/sw/host/tests/chip/spi_device_ujson_console_test/src/main.rs index fe46ff307af92..719bd9450babe 100644 --- a/sw/host/tests/chip/spi_device_ujson_console_test/src/main.rs +++ b/sw/host/tests/chip/spi_device_ujson_console_test/src/main.rs @@ -109,7 +109,12 @@ fn main() -> Result<()> { let transport = opts.init.init_target()?; let spi = transport.spi(&opts.console_spi)?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; execute_test!(test_perso_blob_strcut, &opts, &spi_console_device); diff --git a/sw/host/tests/crypto/aes_gcm_nist_kat/src/main.rs b/sw/host/tests/crypto/aes_gcm_nist_kat/src/main.rs index 225914dd25741..d31087a0fa5c5 100644 --- a/sw/host/tests/crypto/aes_gcm_nist_kat/src/main.rs +++ b/sw/host/tests/crypto/aes_gcm_nist_kat/src/main.rs @@ -137,7 +137,12 @@ fn run_aes_gcm_testcase( fn test_aes_gcm(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let mut test_counter = 0u32; diff --git a/sw/host/tests/crypto/aes_nist_kat/src/main.rs b/sw/host/tests/crypto/aes_nist_kat/src/main.rs index 7384a7c8b6aa3..84a32222fa007 100644 --- a/sw/host/tests/crypto/aes_nist_kat/src/main.rs +++ b/sw/host/tests/crypto/aes_nist_kat/src/main.rs @@ -130,7 +130,12 @@ fn run_aes_testcase( fn test_aes(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let mut test_counter = 0u32; diff --git a/sw/host/tests/crypto/drbg_kat/src/main.rs b/sw/host/tests/crypto/drbg_kat/src/main.rs index 6cfab96f8ec55..9e6dd84e4d035 100644 --- a/sw/host/tests/crypto/drbg_kat/src/main.rs +++ b/sw/host/tests/crypto/drbg_kat/src/main.rs @@ -128,7 +128,12 @@ fn run_drbg_testcase( fn test_drbg(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let mut test_counter = 0u32; diff --git a/sw/host/tests/crypto/ecdh_kat/src/main.rs b/sw/host/tests/crypto/ecdh_kat/src/main.rs index e69e572565284..cac3240120861 100644 --- a/sw/host/tests/crypto/ecdh_kat/src/main.rs +++ b/sw/host/tests/crypto/ecdh_kat/src/main.rs @@ -220,7 +220,12 @@ fn run_ecdh_testcase( fn test_ecdh(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let mut test_counter = 0u32; diff --git a/sw/host/tests/crypto/ecdsa_kat/src/main.rs b/sw/host/tests/crypto/ecdsa_kat/src/main.rs index 0757b81656825..9eb298ab19474 100644 --- a/sw/host/tests/crypto/ecdsa_kat/src/main.rs +++ b/sw/host/tests/crypto/ecdsa_kat/src/main.rs @@ -512,7 +512,12 @@ fn run_ecdsa_testcase( fn test_ecdsa(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let mut test_counter = 0u32; diff --git a/sw/host/tests/crypto/hash_kat/src/main.rs b/sw/host/tests/crypto/hash_kat/src/main.rs index 39ac4d952499d..4c8f6ed1303d3 100644 --- a/sw/host/tests/crypto/hash_kat/src/main.rs +++ b/sw/host/tests/crypto/hash_kat/src/main.rs @@ -158,7 +158,12 @@ fn run_hash_testcase( fn test_hash(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let mut test_counter = 0u32; diff --git a/sw/host/tests/crypto/hmac_kat/src/main.rs b/sw/host/tests/crypto/hmac_kat/src/main.rs index c90926f5ac232..02f31bbdc2055 100644 --- a/sw/host/tests/crypto/hmac_kat/src/main.rs +++ b/sw/host/tests/crypto/hmac_kat/src/main.rs @@ -126,7 +126,12 @@ fn run_hmac_testcase( fn test_hmac(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let mut test_counter = 0u32; diff --git a/sw/host/tests/crypto/kmac_kat/src/main.rs b/sw/host/tests/crypto/kmac_kat/src/main.rs index 29441b54af37a..04f1bcb82718e 100644 --- a/sw/host/tests/crypto/kmac_kat/src/main.rs +++ b/sw/host/tests/crypto/kmac_kat/src/main.rs @@ -141,7 +141,12 @@ fn run_kmac_testcase( fn test_kmac(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let mut test_counter = 0u32; diff --git a/sw/host/tests/crypto/rsa_kat/src/main.rs b/sw/host/tests/crypto/rsa_kat/src/main.rs index f2d1a74a3be38..73e82f4fa17b9 100644 --- a/sw/host/tests/crypto/rsa_kat/src/main.rs +++ b/sw/host/tests/crypto/rsa_kat/src/main.rs @@ -236,7 +236,12 @@ fn run_rsa_testcase( fn test_rsa(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let test_vector_files = &opts.rsa_json; diff --git a/sw/host/tests/crypto/sphincsplus_kat/src/main.rs b/sw/host/tests/crypto/sphincsplus_kat/src/main.rs index 577260f956e30..ebc50030a0d51 100644 --- a/sw/host/tests/crypto/sphincsplus_kat/src/main.rs +++ b/sw/host/tests/crypto/sphincsplus_kat/src/main.rs @@ -127,7 +127,12 @@ fn run_sphincsplus_testcase( fn test_sphincsplus(opts: &Opts, transport: &TransportWrapper) -> Result<()> { let spi = transport.spi("BOOTSTRAP")?; - let spi_console_device = SpiConsoleDevice::new(&*spi, None, /*ignore_frame_num=*/ false)?; + let spi_console_device = SpiConsoleDevice::new( + &*spi, + None, + /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), + )?; let _ = UartConsole::wait_for(&spi_console_device, r"Running [^\r\n]*", opts.timeout)?; let mut test_counter = 0u32; diff --git a/sw/host/tests/manuf/cp_provision_functest/src/main.rs b/sw/host/tests/manuf/cp_provision_functest/src/main.rs index e146e041305a5..9bfbcc77c5b81 100644 --- a/sw/host/tests/manuf/cp_provision_functest/src/main.rs +++ b/sw/host/tests/manuf/cp_provision_functest/src/main.rs @@ -232,6 +232,7 @@ fn main() -> Result<()> { &*spi, Some(device_console_tx_ready_pin), /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), )?; // Transition from RAW to TEST_UNLOCKED0. diff --git a/sw/host/tests/manuf/manuf_ujson_msg_padding/src/main.rs b/sw/host/tests/manuf/manuf_ujson_msg_padding/src/main.rs index c1a06760b978e..59ead6ada7021 100644 --- a/sw/host/tests/manuf/manuf_ujson_msg_padding/src/main.rs +++ b/sw/host/tests/manuf/manuf_ujson_msg_padding/src/main.rs @@ -47,6 +47,7 @@ fn main() -> Result<()> { &*spi, Some(device_console_tx_ready_pin), /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), )?; // Receive the payloads from the device. diff --git a/sw/host/tests/manuf/manuf_ujson_msg_size/src/main.rs b/sw/host/tests/manuf/manuf_ujson_msg_size/src/main.rs index 08c7d9baeca14..e0ea1c1750843 100644 --- a/sw/host/tests/manuf/manuf_ujson_msg_size/src/main.rs +++ b/sw/host/tests/manuf/manuf_ujson_msg_size/src/main.rs @@ -46,6 +46,7 @@ fn main() -> Result<()> { &*spi, Some(device_console_tx_ready_pin), /*ignore_frame_num=*/ false, + Some(opts.init.backend_opts.interface.as_str()), )?; // Receive the payloads from the device. From f46795585a719723237108e352da3e447147347d Mon Sep 17 00:00:00 2001 From: Alex Jones Date: Tue, 18 Nov 2025 13:09:12 +0000 Subject: [PATCH 2/2] [bazel,qemu] Enable SPI console with TX indicator test for QEMU Enables the `ottf_console_with_gpio_tx_indicator` test target for the QEMU execution environment, as this test exercises functionality that is required to be supported for provisioning flows. As QEMU does not currently implement the relevant Pinmux connections between OpenTitan and GPIO, a minor extension is made to the test to allow users to optionally specify which GPIO pin should be used as the ready-indicator, rather than hardcoding to IOA5. This is then overriden for the QEMU execution environment only. Signed-off-by: Alex Jones --- sw/device/tests/BUILD | 9 +++++++++ .../chip/ottf_console_with_gpio_tx_indicator/src/main.rs | 7 ++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/sw/device/tests/BUILD b/sw/device/tests/BUILD index 75d6a15cb61ab..7c93594b2e3bf 100644 --- a/sw/device/tests/BUILD +++ b/sw/device/tests/BUILD @@ -4381,6 +4381,7 @@ opentitan_test( exec_env = { "//hw/top_earlgrey:fpga_hyper310_rom_with_fake_keys": None, "//hw/top_earlgrey:fpga_cw340_rom_with_fake_keys": None, + "//hw/top_earlgrey:sim_qemu_rom_with_fake_keys": None, }, fpga = fpga_params( # We run this as part of the manuf test suite as we want to run it in @@ -4396,6 +4397,14 @@ opentitan_test( """, test_harness = "//sw/host/tests/chip/ottf_console_with_gpio_tx_indicator", ), + qemu = qemu_params( + test_cmd = """ + --bootstrap="{firmware}" + "{firmware:elf}" + --console-tx-indicator-pin=0 + """, + test_harness = "//sw/host/tests/chip/ottf_console_with_gpio_tx_indicator", + ), deps = [ "//hw/top_earlgrey/sw/autogen:top_earlgrey", "//sw/device/lib/runtime:hart", diff --git a/sw/host/tests/chip/ottf_console_with_gpio_tx_indicator/src/main.rs b/sw/host/tests/chip/ottf_console_with_gpio_tx_indicator/src/main.rs index fa50df4bb9ad7..71fdffb605b47 100644 --- a/sw/host/tests/chip/ottf_console_with_gpio_tx_indicator/src/main.rs +++ b/sw/host/tests/chip/ottf_console_with_gpio_tx_indicator/src/main.rs @@ -34,12 +34,17 @@ struct Opts { /// Path to the firmware's ELF file, for querying symbol addresses. #[arg(value_name = "FIRMWARE_ELF")] firmware_elf: PathBuf, + + /// Name of the SPI interface to connect to the OTTF console. + #[arg(long, default_value = "IOA5")] + console_tx_indicator_pin: String, } fn spi_device_console_test(opts: &Opts, transport: &TransportWrapper) -> Result<()> { // Setup the SPI console with the GPIO TX indicator pin. let spi = transport.spi(&opts.console_spi)?; - let device_console_tx_ready_pin = &transport.gpio_pin("IOA5")?; + let device_console_tx_ready_pin = + &transport.gpio_pin(opts.console_tx_indicator_pin.as_str())?; device_console_tx_ready_pin.set_mode(PinMode::Input)?; device_console_tx_ready_pin.set_pull_mode(PullMode::None)?; let spi_console_device = SpiConsoleDevice::new(