Skip to content

Commit 5350817

Browse files
[rescue,test] add new rescue error handling tests
This introduces new end-to-end tests for rescue error handling: - Adds a usbdfu_in_transaction_cancel test to verify the device's behavior when a USB DFU transaction is cancelled during an upload. - Adds a rescue_image_too_big test for Xmodem to check the handling of oversized firmware images during rescue, ensuring the device correctly cancels the operation and remains functional. Signed-off-by: Anthony Chen <[email protected]>
1 parent 382e702 commit 5350817

File tree

4 files changed

+93
-3
lines changed

4 files changed

+93
-3
lines changed

sw/device/silicon_creator/rom_ext/e2e/rescue/BUILD

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,7 @@ opentitan_test(
782782
"//sw/device/silicon_creator/rom_ext:rom_ext_dice_x509_slot_a": "romext",
783783
":boot_test_slot_a": "firmware",
784784
},
785+
timeout = "long",
785786
test_cmd = """
786787
--clear-bitstream
787788
--bootstrap={firmware}
@@ -878,6 +879,29 @@ opentitan_test(
878879
),
879880
)
880881

882+
opentitan_test(
883+
name = "usbdfu_in_transaction_cancel",
884+
exec_env = {
885+
# Unsupported on cw310: USB control request with a minimal
886+
# timeout does not cancel transactions on this platform.
887+
"//hw/top_earlgrey:fpga_cw340_rom_ext": None,
888+
},
889+
fpga = fpga_params(
890+
assemble = "{romext}@{rom_ext_slot_a} {firmware}@{owner_slot_a}",
891+
binaries = {
892+
"//sw/device/silicon_creator/rom_ext:rom_ext_usbdfu": "romext",
893+
":boot_test_slot_a": "firmware",
894+
},
895+
params = "-p usb-dfu -t strap -v 3",
896+
test_cmd = """
897+
--clear-bitstream
898+
--bootstrap={firmware}
899+
rescue {params} --action=usb-dfu-in-transaction-cancel
900+
""",
901+
test_harness = "//sw/host/tests/rescue:dfu_rescue_error_handling",
902+
),
903+
)
904+
881905
# Check that when we are not configured to rescue on watchdog timeout that
882906
# a watchdog timeout event simply boots the firmware normally.
883907
opentitan_test(

sw/host/opentitanlib/src/rescue/usbdfu.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ impl UsbDfu {
3333
}
3434
}
3535

36-
fn device(&self) -> Ref<'_, UsbBackend> {
36+
pub fn device(&self) -> Ref<'_, UsbBackend> {
3737
let device = self.usb.borrow();
3838
Ref::map(device, |d| d.as_ref().expect("device handle"))
3939
}

sw/host/tests/rescue/dfu_rescue_error_handling.rs

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use std::time::Duration;
1111
use opentitanlib::app::TransportWrapper;
1212
use opentitanlib::io::eeprom::{AddressMode, MODE_111, Transaction};
1313
use opentitanlib::io::spi::SpiParams;
14-
use opentitanlib::rescue::dfu::{DfuOperations, DfuRequestType};
14+
use opentitanlib::rescue::dfu::{DfuOperations, DfuRequest, DfuRequestType};
1515
use opentitanlib::rescue::{EntryMode, Rescue, RescueMode, RescueParams, SpiDfu, UsbDfu};
1616
use opentitanlib::spiflash::SpiFlash;
1717
use opentitanlib::test_utils::init::InitializeTest;
@@ -50,6 +50,7 @@ pub enum DfuRescueTestActions {
5050
InvalidSpiDfuRequests,
5151
InvalidSpiFlashTransaction,
5252
UsbDfuOutChunkTooBig,
53+
UsbDfuInTransactionCancel,
5354
}
5455

5556
const SET_INTERFACE: u8 = 0x0b;
@@ -327,6 +328,40 @@ fn usb_dfu_out_chunk_too_big(params: &RescueParams, transport: &TransportWrapper
327328
Ok(())
328329
}
329330

331+
fn usb_dfu_in_transaction_cancel(
332+
params: &RescueParams,
333+
transport: &TransportWrapper,
334+
) -> Result<()> {
335+
let rescue = UsbDfu::new(params.clone());
336+
rescue.enter(transport, EntryMode::Reset)?;
337+
rescue.set_mode(RescueMode::DeviceId)?;
338+
let usb = rescue.device();
339+
let mut data = vec![0u8; 2048];
340+
// Issue a USB control request with minimal timeout to cancel the transaction.
341+
let result = usb.handle().read_control(
342+
DfuRequestType::In.into(),
343+
DfuRequest::UpLoad.into(),
344+
0,
345+
rescue.get_interface() as u16,
346+
&mut data,
347+
Duration::from_millis(1),
348+
);
349+
350+
if result.is_ok() {
351+
return Err(anyhow!("Invalid read control should fail"));
352+
}
353+
354+
rescue.reboot()?;
355+
356+
let uart = transport.uart("console")?;
357+
UartConsole::wait_for(&*uart, r"Finished", Duration::from_secs(5))?;
358+
359+
#[cfg(feature = "ot_coverage_enabled")]
360+
UartConsole::wait_for_coverage(&*uart, Duration::from_secs(5))?;
361+
362+
Ok(())
363+
}
364+
330365
fn main() -> Result<()> {
331366
let opts = Opts::parse();
332367
opts.init.init_logging();
@@ -346,6 +381,9 @@ fn main() -> Result<()> {
346381
DfuRescueTestActions::UsbDfuOutChunkTooBig => {
347382
usb_dfu_out_chunk_too_big(&rescue.params, &transport)?
348383
}
384+
DfuRescueTestActions::UsbDfuInTransactionCancel => {
385+
usb_dfu_in_transaction_cancel(&rescue.params, &transport)?
386+
}
349387
},
350388
}
351389

sw/host/tests/rescue/xmodem_rescue_error_handling.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@
33
// SPDX-License-Identifier: Apache-2.0
44

55
#![allow(clippy::bool_assert_comparison)]
6-
use anyhow::Result;
6+
use anyhow::{anyhow, Result};
77
use clap::Parser;
88

99
use std::rc::Rc;
1010
use std::time::Duration;
1111

1212
use opentitanlib::app::TransportWrapper;
13+
use opentitanlib::chip::boot_svc::BootSlot;
1314
use opentitanlib::io::uart::Uart;
1415
use opentitanlib::rescue::xmodem::XmodemError;
1516
use opentitanlib::rescue::{EntryMode, Rescue, RescueMode, RescueSerial};
@@ -343,6 +344,32 @@ fn recv_finish_nak(
343344
Ok(())
344345
}
345346

347+
fn rescue_image_too_big(
348+
transport: &TransportWrapper,
349+
uart: &dyn Uart,
350+
rescue: &RescueSerial,
351+
) -> Result<()> {
352+
rescue.enter(transport, EntryMode::Reset)?;
353+
let image_too_big = [0u8; 1026*1024];
354+
match rescue.update_firmware(BootSlot::SlotB, &image_too_big) {
355+
Ok(_) => {
356+
return Err(anyhow!("Expects cancel during firmware rescue, but got OK."));
357+
}
358+
Err(e) => {
359+
if e.to_string().contains("Cancelled") {
360+
log::info!("Operation cancelled by device as expected");
361+
} else {
362+
return Err(e);
363+
}
364+
}
365+
};
366+
// Check for kErrorRescueImageTooBig.
367+
UartConsole::wait_for(uart, r"BFV:02525309", Duration::from_secs(5))?;
368+
// Ensure we can still boot into Owner SW.
369+
UartConsole::wait_for(uart, r"Finished", Duration::from_secs(5))?;
370+
Ok(())
371+
}
372+
346373
fn main() -> Result<()> {
347374
let opts = Opts::parse();
348375
opts.init.init_logging();
@@ -361,5 +388,6 @@ fn main() -> Result<()> {
361388
recv_data_cancel(&transport, &*uart, &rescue)?;
362389
recv_data_nak(&transport, &*uart, &rescue)?;
363390
recv_finish_nak(&transport, &*uart, &rescue)?;
391+
rescue_image_too_big(&transport, &*uart, &rescue)?;
364392
Ok(())
365393
}

0 commit comments

Comments
 (0)