[ot_transport] add fpga_ops and make bitstream methods part of it

nbdd0121 · nbdd0121 · commit 1c9ce6b31a9b · 2025-11-28T01:01:24.000Z
This eliminates the special `dispatch` mechanism for FPGA programming.

Signed-off-by: Gary Guo &lt;gary.guo@lowrisc.org&gt;
diff --git a/sw/host/opentitanlib/src/app/mod.rs b/sw/host/opentitanlib/src/app/mod.rs
@@ -19,8 +19,8 @@ use crate::io::jtag::{JtagChain, JtagParams};
 use crate::io::spi::{Target, TransferMode};
 use crate::io::uart::Uart;
 use crate::transport::{
-    Capability, ProgressIndicator, ProxyOps, Transport, TransportError, TransportInterfaceType,
-    ioexpander,
+    Capability, FpgaOps, ProgressIndicator, ProxyOps, Transport, TransportError,
+    TransportInterfaceType, ioexpander,
 };
 
 use anyhow::{Result, bail, ensure};
@@ -998,6 +998,11 @@ impl TransportWrapper {
         self.transport.emulator()
     }
 
+    /// Methods available only on FPGA implementations.
+    pub fn fpga_ops(&self) -> Result<&dyn FpgaOps> {
+        self.transport.fpga_ops()
+    }
+
     /// Methods available only on Proxy implementation.
     pub fn proxy_ops(&self) -> Result<&dyn ProxyOps> {
         self.transport.proxy_ops()
diff --git a/sw/host/opentitanlib/src/test_utils/init.rs b/sw/host/opentitanlib/src/test_utils/init.rs
@@ -144,7 +144,10 @@ impl InitializeTest {
         let _uart = self.bootstrap.options.uart_params.create(&transport)?;
 
         // Load a bitstream.
-        Self::print_result("load_bitstream", self.load_bitstream.init(&transport))?;
+        Self::print_result(
+            "load_bitstream",
+            self.load_bitstream.init(&transport).map(|_| None),
+        )?;
 
         // Bootstrap an rv32 test program.
         Self::print_result("bootstrap", self.bootstrap.init(&transport))?;
diff --git a/sw/host/opentitanlib/src/test_utils/load_bitstream.rs b/sw/host/opentitanlib/src/test_utils/load_bitstream.rs
@@ -8,7 +8,7 @@ use std::path::{Path, PathBuf};
 use std::time::Duration;
 
 use crate::app::{StagedProgressBar, TransportWrapper};
-use crate::transport::common::fpga::{ClearBitstream, FpgaProgram};
+use crate::transport::common::fpga::FpgaProgram;
 
 /// Load a bitstream into the FPGA.
 #[derive(Debug, Args)]
@@ -31,28 +31,21 @@ pub struct LoadBitstream {
 }
 
 impl LoadBitstream {
-    pub fn init(
-        &self,
-        transport: &TransportWrapper,
-    ) -> Result<Option<Box<dyn erased_serde::Serialize>>> {
+    pub fn init(&self, transport: &TransportWrapper) -> Result<()> {
         // Clear out existing bitstream, if requested.
         if self.clear_bitstream {
             log::info!("Clearing bitstream.");
-            transport.dispatch(&ClearBitstream)?;
+            transport.fpga_ops()?.clear_bitstream()?;
         }
         // Load the specified bitstream, if provided.
         if let Some(bitstream) = &self.bitstream {
-            self.load(transport, bitstream)
-        } else {
-            Ok(None)
+            self.load(transport, bitstream)?;
         }
+
+        Ok(())
     }
 
-    pub fn load(
-        &self,
-        transport: &TransportWrapper,
-        file: &Path,
-    ) -> Result<Option<Box<dyn erased_serde::Serialize>>> {
+    pub fn load(&self, transport: &TransportWrapper, file: &Path) -> Result<()> {
         log::info!("Loading bitstream: {:?}", file);
         let payload = std::fs::read(file)?;
         let progress = StagedProgressBar::new();
@@ -64,8 +57,11 @@ impl LoadBitstream {
         };
 
         if operation.should_skip(transport)? {
-            return Ok(None);
+            return Ok(());
         }
-        transport.dispatch(&operation)
+
+        transport
+            .fpga_ops()?
+            .load_bitstream(&operation.bitstream, &*operation.progress)
     }
 }
diff --git a/sw/host/opentitanlib/src/transport/common/fpga.rs b/sw/host/opentitanlib/src/transport/common/fpga.rs
@@ -65,6 +65,3 @@ impl FpgaProgram {
         Ok(false)
     }
 }
-
-/// Command for Transport::dispatch().
-pub struct ClearBitstream;
diff --git a/sw/host/opentitanlib/src/transport/errors.rs b/sw/host/opentitanlib/src/transport/errors.rs
@@ -83,6 +83,7 @@ pub enum TransportInterfaceType {
     I2c,
     Jtag,
     Emulator,
+    FpgaOps,
     ProxyOps,
     GpioMonitoring,
     GpioBitbanging,
diff --git a/sw/host/opentitanlib/src/transport/mod.rs b/sw/host/opentitanlib/src/transport/mod.rs
@@ -138,6 +138,11 @@ pub trait Transport {
         Err(TransportError::InvalidInterface(TransportInterfaceType::Emulator).into())
     }
 
+    /// Methods available only on FPGA implementations.
+    fn fpga_ops(&self) -> Result<&dyn FpgaOps> {
+        Err(TransportError::InvalidInterface(TransportInterfaceType::FpgaOps).into())
+    }
+
     /// Methods available only on Proxy implementation.
     fn proxy_ops(&self) -> Result<&dyn ProxyOps> {
         Err(TransportError::InvalidInterface(TransportInterfaceType::ProxyOps).into())
@@ -162,6 +167,13 @@ pub trait Transport {
     }
 }
 
+/// Methods available only on FPGA transports.
+pub trait FpgaOps {
+    fn load_bitstream(&self, bitstream: &[u8], progress: &dyn ProgressIndicator) -> Result<()>;
+
+    fn clear_bitstream(&self) -> Result<()>;
+}
+
 /// Methods available only on the Proxy implementation of the Transport trait.
 pub trait ProxyOps {
     /// Returns a string->string map containing user-defined aspects "provided" by the testbed
diff --git a/sw/host/opentitantool/src/command/clear_bitstream.rs b/sw/host/opentitantool/src/command/clear_bitstream.rs
@@ -8,7 +8,6 @@ use std::any::Any;
 
 use opentitanlib::app::TransportWrapper;
 use opentitanlib::app::command::CommandDispatch;
-use opentitanlib::transport::common::fpga;
 
 /// Clear the bitstream of the FPGA
 #[derive(Debug, Args)]
@@ -20,6 +19,7 @@ impl CommandDispatch for ClearBitstream {
         _context: &dyn Any,
         transport: &TransportWrapper,
     ) -> Result<Option<Box<dyn erased_serde::Serialize>>> {
-        transport.dispatch(&fpga::ClearBitstream)
+        transport.fpga_ops()?.clear_bitstream()?;
+        Ok(None)
     }
 }
diff --git a/sw/host/opentitantool/src/command/load_bitstream.rs b/sw/host/opentitantool/src/command/load_bitstream.rs
@@ -5,13 +5,11 @@
 use anyhow::Result;
 use clap::Args;
 use std::any::Any;
-use std::fs;
 use std::path::PathBuf;
 use std::time::Duration;
 
+use opentitanlib::app::TransportWrapper;
 use opentitanlib::app::command::CommandDispatch;
-use opentitanlib::app::{StagedProgressBar, TransportWrapper};
-use opentitanlib::transport::common::fpga::FpgaProgram;
 
 /// Load a bitstream into the FPGA.
 #[derive(Debug, Args)]
@@ -33,19 +31,14 @@ impl CommandDispatch for LoadBitstream {
         _context: &dyn Any,
         transport: &TransportWrapper,
     ) -> Result<Option<Box<dyn erased_serde::Serialize>>> {
-        log::info!("Loading bitstream: {:?}", self.filename);
-        let bitstream = fs::read(&self.filename)?;
-        let progress = StagedProgressBar::new();
-        let operation = FpgaProgram {
-            bitstream,
+        opentitanlib::test_utils::load_bitstream::LoadBitstream {
+            clear_bitstream: false,
+            bitstream: Some(self.filename.clone()),
             rom_reset_pulse: self.rom_reset_pulse,
             rom_timeout: self.rom_timeout,
-            progress: Box::new(progress),
-        };
-
-        if operation.should_skip(transport)? {
-            return Ok(None);
         }
-        transport.dispatch(&operation)
+        .init(transport)?;
+
+        Ok(None)
     }
 }
diff --git a/sw/host/ot_transports/chipwhisperer/src/lib.rs b/sw/host/ot_transports/chipwhisperer/src/lib.rs
@@ -18,9 +18,9 @@ use opentitanlib::io::spi::Target;
 use opentitanlib::io::uart::flow::SoftwareFlowControl;
 use opentitanlib::io::uart::serial::SerialPortUart;
 use opentitanlib::io::uart::{Uart, UartError};
-use opentitanlib::transport::common::fpga::{ClearBitstream, FpgaProgram};
 use opentitanlib::transport::{
-    Capabilities, Capability, Transport, TransportError, TransportInterfaceType,
+    Capabilities, Capability, FpgaOps, ProgressIndicator, Transport, TransportError,
+    TransportInterfaceType,
 };
 use opentitanlib::util::fs::builtin_file;
 use opentitanlib::util::parse_int::ParseInt;
@@ -100,22 +100,6 @@ impl<B: Board> ChipWhisperer<B> {
             )?))
         }
     }
-
-    pub fn load_bitstream(&self, fpga_program: &FpgaProgram) -> Result<()> {
-        // Program the FPGA bitstream.
-        log::info!("Programming the FPGA bitstream.");
-        let usb = self.device.borrow();
-        usb.spi1_enable(false)?;
-        usb.fpga_program(&fpga_program.bitstream, fpga_program.progress.as_ref())?;
-        Ok(())
-    }
-
-    pub fn clear_bitstream(&self) -> Result<()> {
-        let usb = self.device.borrow();
-        usb.spi1_enable(false)?;
-        usb.clear_bitstream()?;
-        Ok(())
-    }
 }
 
 impl<B: Board + 'static> Transport for ChipWhisperer<B> {
@@ -162,11 +146,12 @@ impl<B: Board + 'static> Transport for ChipWhisperer<B> {
         Ok(inner.spi.as_ref().unwrap().clone())
     }
 
+    fn fpga_ops(&self) -> Result<&dyn FpgaOps> {
+        Ok(self)
+    }
+
     fn dispatch(&self, action: &dyn Any) -> Result<Option<Box<dyn erased_serde::Serialize>>> {
-        if let Some(fpga_program) = action.downcast_ref::<FpgaProgram>() {
-            self.load_bitstream(fpga_program)?;
-            Ok(None)
-        } else if action.downcast_ref::<ResetSam3x>().is_some() {
+        if action.downcast_ref::<ResetSam3x>().is_some() {
             self.device.borrow().reset_sam3x()?;
             Ok(None)
         } else if action.downcast_ref::<SetPll>().is_some() {
@@ -180,9 +165,6 @@ impl<B: Board + 'static> Transport for ChipWhisperer<B> {
             usb.pll_out_enable(2, false)?;
             usb.pll_write_defaults()?;
             Ok(None)
-        } else if action.downcast_ref::<ClearBitstream>().is_some() {
-            self.clear_bitstream()?;
-            Ok(None)
         } else if action.downcast_ref::<GetSam3xFwVersion>().is_some() {
             let usb = self.device.borrow();
             Ok(Some(Box::new(usb.get_firmware_version()?)))
@@ -192,6 +174,23 @@ impl<B: Board + 'static> Transport for ChipWhisperer<B> {
     }
 }
 
+impl<B: Board> FpgaOps for ChipWhisperer<B> {
+    fn load_bitstream(&self, bitstream: &[u8], progress: &dyn ProgressIndicator) -> Result<()> {
+        // Program the FPGA bitstream.
+        log::info!("Programming the FPGA bitstream.");
+        let usb = self.device.borrow();
+        usb.spi1_enable(false)?;
+        usb.fpga_program(bitstream, progress)?;
+        Ok(())
+    }
+
+    fn clear_bitstream(&self) -> Result<()> {
+        let usb = self.device.borrow();
+        usb.spi1_enable(false)?;
+        usb.clear_bitstream()?;
+        Ok(())
+    }
+}
 /// Command for Transport::dispatch().
 pub struct SetPll {}
 
diff --git a/sw/host/ot_transports/hyperdebug/src/lib.rs b/sw/host/ot_transports/hyperdebug/src/lib.rs
@@ -26,10 +26,9 @@ use opentitanlib::io::jtag::{JtagChain, JtagParams};
 use opentitanlib::io::spi::Target;
 use opentitanlib::io::uart::Uart;
 use opentitanlib::io::uart::serial::flock_serial;
-use opentitanlib::transport::common::fpga::{ClearBitstream, FpgaProgram};
 use opentitanlib::transport::{
-    Capabilities, Capability, SetJtagPins, Transport, TransportError, TransportInterfaceType,
-    UpdateFirmware,
+    Capabilities, Capability, FpgaOps, ProgressIndicator, SetJtagPins, Transport, TransportError,
+    TransportInterfaceType, UpdateFirmware,
 };
 use opentitanlib::util::fs::builtin_file;
 use opentitanlib::util::usb::UsbBackend;
@@ -83,7 +82,7 @@ pub trait Flavor {
     }
     fn get_default_usb_vid() -> u16;
     fn get_default_usb_pid() -> u16;
-    fn load_bitstream(_fpga_program: &FpgaProgram) -> Result<()> {
+    fn load_bitstream(_bitstream: &[u8], _progress: &dyn ProgressIndicator) -> Result<()> {
         Err(TransportError::UnsupportedOperation.into())
     }
     fn clear_bitstream() -> Result<()> {
@@ -817,6 +816,10 @@ impl<T: Flavor> Transport for Hyperdebug<T> {
         )?))
     }
 
+    fn fpga_ops(&self) -> Result<&dyn FpgaOps> {
+        Ok(self)
+    }
+
     fn dispatch(&self, action: &dyn Any) -> Result<Option<Box<dyn erased_serde::Serialize>>> {
         if let Some(update_firmware_action) = action.downcast_ref::<UpdateFirmware>() {
             let usb_vid = self.inner.usb_device.borrow().get_vendor_id();
@@ -856,10 +859,6 @@ impl<T: Flavor> Transport for Hyperdebug<T> {
                 }
                 _ => Err(TransportError::UnsupportedOperation.into()),
             }
-        } else if let Some(fpga_program) = action.downcast_ref::<FpgaProgram>() {
-            T::load_bitstream(fpga_program).map(|_| None)
-        } else if action.downcast_ref::<ClearBitstream>().is_some() {
-            T::clear_bitstream().map(|_| None)
         } else {
             Err(TransportError::UnsupportedOperation.into())
         }
@@ -893,6 +892,16 @@ impl<T: Flavor> Transport for Hyperdebug<T> {
     }
 }
 
+impl<T: Flavor> FpgaOps for Hyperdebug<T> {
+    fn load_bitstream(&self, bitstream: &[u8], progress: &dyn ProgressIndicator) -> Result<()> {
+        T::load_bitstream(bitstream, progress)
+    }
+
+    fn clear_bitstream(&self) -> Result<()> {
+        T::clear_bitstream()
+    }
+}
+
 /// A `StandardFlavor` is a plain Hyperdebug board.
 pub struct StandardFlavor;
 
@@ -980,11 +989,11 @@ impl<B: Board> Flavor for ChipWhispererFlavor<B> {
     fn get_default_usb_pid() -> u16 {
         StandardFlavor::get_default_usb_pid()
     }
-    fn load_bitstream(fpga_program: &FpgaProgram) -> Result<()> {
+    fn load_bitstream(bitstream: &[u8], progress: &dyn ProgressIndicator) -> Result<()> {
         // Try to establish a connection to the native Chip Whisperer interface
         // which we will use for bitstream loading.
         let board = ChipWhisperer::<B>::new(None, None, None, &[])?;
-        board.load_bitstream(fpga_program)?;
+        board.load_bitstream(bitstream, progress)?;
         Ok(())
     }
     fn clear_bitstream() -> Result<()> {
diff --git a/sw/host/provisioning/ft/src/main.rs b/sw/host/provisioning/ft/src/main.rs
@@ -135,7 +135,10 @@ fn main() -> Result<()> {
     transport.apply_default_configuration(None)?;
     let spi = transport.spi(&opts.console_spi)?;
     let spi_console_device = SpiConsoleDevice::new(&*spi, None)?;
-    InitializeTest::print_result("load_bitstream", opts.init.load_bitstream.init(&transport))?;
+    InitializeTest::print_result(
+        "load_bitstream",
+        opts.init.load_bitstream.init(&transport).map(|_| None),
+    )?;
 
     // Parse and format LC tokens.
     let _test_unlock_token =
diff --git a/sw/host/tests/manuf/manuf_cp_device_info_flash_wr/src/main.rs b/sw/host/tests/manuf/manuf_cp_device_info_flash_wr/src/main.rs
@@ -138,7 +138,10 @@ fn main() -> Result<()> {
     // want to perform bootstrap yet.
     let transport = backend::create(&opts.init.backend_opts)?;
     transport.apply_default_configuration(None)?;
-    InitializeTest::print_result("load_bitstream", opts.init.load_bitstream.init(&transport))?;
+    InitializeTest::print_result(
+        "load_bitstream",
+        opts.init.load_bitstream.init(&transport).map(|_| None),
+    )?;
 
     execute_test!(manuf_cp_device_info_flash_wr, &opts, &transport);
 

Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,3 @@ impl FpgaProgram {`
`65`	`65`	`Ok(false)`
`66`	`66`	`}`
`67`	`67`	`}`
`68`		`-`
`69`		`-/// Command for Transport::dispatch().`
`70`		`-pub struct ClearBitstream;`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,6 @@ use std::any::Any;`
`8`	`8`
`9`	`9`	`use opentitanlib::app::TransportWrapper;`
`10`	`10`	`use opentitanlib::app::command::CommandDispatch;`
`11`		`-use opentitanlib::transport::common::fpga;`
`12`	`11`
`13`	`12`	`/// Clear the bitstream of the FPGA`
`14`	`13`	`#[derive(Debug, Args)]`
`@@ -20,6 +19,7 @@ impl CommandDispatch for ClearBitstream {`
`20`	`19`	`_context: &dyn Any,`
`21`	`20`	`transport: &TransportWrapper,`
`22`	`21`	`) -> Result<Option<Box<dyn erased_serde::Serialize>>> {`
`23`		`- transport.dispatch(&fpga::ClearBitstream)`
	`22`	`+ transport.fpga_ops()?.clear_bitstream()?;`
	`23`	`+ Ok(None)`
`24`	`24`	`}`
`25`	`25`	`}`