Skip to content

Commit

Permalink
[opentitanlib] add DFT disabling strap config and workaround
Browse files Browse the repository at this point in the history
DFT straps were put on the same pins as the console UART (IOC3 and 4).
This causes JTAG reliability issues in test unlocked states if we do
not pull the straps low when they should be sampled. This provides a
software workaround.

Signed-off-by: Tim Trippel <[email protected]>
  • Loading branch information
timothytrippel committed Dec 20, 2023
1 parent 64da4f1 commit af73135
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 6 deletions.
1 change: 1 addition & 0 deletions sw/device/silicon_creator/manuf/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ cc_library(
"//sw/device/lib/dif:flash_ctrl",
"//sw/device/lib/dif:pinmux",
"//sw/device/lib/dif:uart",
"//sw/device/lib/runtime:hart",
"//sw/device/lib/runtime:log",
"//sw/device/lib/runtime:print",
],
Expand Down
10 changes: 10 additions & 0 deletions sw/device/silicon_creator/manuf/lib/ast_program.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "sw/device/lib/dif/dif_flash_ctrl.h"
#include "sw/device/lib/dif/dif_pinmux.h"
#include "sw/device/lib/dif/dif_uart.h"
#include "sw/device/lib/runtime/hart.h"
#include "sw/device/lib/runtime/log.h"
#include "sw/device/lib/runtime/print.h"
#include "sw/device/lib/testing/flash_ctrl_testutils.h"
Expand Down Expand Up @@ -88,6 +89,14 @@ status_t ast_program_init(bool verbose) {
}

status_t ast_program_config(bool verbose) {
// Add delay to synchronize DFT strap sampling with HyperDebug UART console
// swapping (since DFT straps use same UART pins as OpenTitan console).
// Note: this should match the delay time in
// `sw/host/opentitanlib/src/app/mod.rs:reset_target()`.
if (kDeviceType == kDeviceSilicon) {
busy_spin_micros(/*usec=*/5 * 1000); // Wait 5ms.
}

TRY(ast_program_init(verbose));

// Read AST calibration values from flash.
Expand All @@ -97,6 +106,7 @@ status_t ast_program_config(bool verbose) {
(kFlashInfoFieldAstCalibrationData.page * device_info.bytes_per_page) +
kFlashInfoFieldAstCalibrationData.byte_offset;
uint32_t ast_data[kFlashInfoAstCalibrationDataSizeIn32BitWords];
TRY(flash_ctrl_testutils_wait_for_init(&flash_state));
TRY(flash_ctrl_testutils_read(&flash_state, byte_address,
kFlashInfoFieldAstCalibrationData.partition,
ast_data, kDifFlashCtrlPartitionTypeInfo,
Expand Down
27 changes: 27 additions & 0 deletions sw/host/opentitanlib/src/app/config/hyperdebug_chipwhisperer.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@
"pull_mode": "PullUp",
"alias_of": "CN10_29"
},
{
"name": "DFT_STRAP0",
"mode": "Alternate",
"alias_of": "IOC3"
},
{
"name": "DFT_STRAP1",
"mode": "Alternate",
"alias_of": "IOC4"
},
{
"name": "IOA0",
"alias_of": "CN8_6"
Expand Down Expand Up @@ -237,5 +247,22 @@
"name": "console",
"alias_of": "UART2"
}
],
"strappings": [
{
"name": "PRERESET_DFT_DISABLE",
"pins": [
{
"name": "DFT_STRAP0",
"mode": "PushPull",
"level": false
},
{
"name": "DFT_STRAP1",
"mode": "PushPull",
"level": false
}
]
}
]
}
26 changes: 21 additions & 5 deletions sw/host/opentitanlib/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ pub struct I2cConfiguration {

pub struct TransportWrapperBuilder {
interface: String,
disable_dft_on_reset: bool,
openocd_adapter_config: Option<PathBuf>,
provides_list: Vec<(String, String)>,
requires_list: Vec<(String, String)>,
Expand All @@ -211,6 +212,7 @@ pub struct TransportWrapperBuilder {
// transport will have been computed from a number ConfigurationFiles.
pub struct TransportWrapper {
transport: Rc<dyn Transport>,
disable_dft_on_reset: Cell<bool>,
openocd_adapter_config: Option<PathBuf>,
provides_map: HashMap<String, String>,
pin_map: HashMap<String, String>,
Expand All @@ -227,9 +229,10 @@ pub struct TransportWrapper {
}

impl TransportWrapperBuilder {
pub fn new(interface: String) -> Self {
pub fn new(interface: String, disable_dft_on_reset: bool) -> Self {
Self {
interface,
disable_dft_on_reset,
openocd_adapter_config: None,
provides_list: Vec::new(),
requires_list: Vec::new(),
Expand Down Expand Up @@ -623,6 +626,7 @@ impl TransportWrapperBuilder {
let i2c_conf_map = Self::consolidate_i2c_conf_map(&self.i2c_conf_map)?;
let mut transport_wrapper = TransportWrapper {
transport: Rc::from(transport),
disable_dft_on_reset: Cell::new(self.disable_dft_on_reset),
openocd_adapter_config: self.openocd_adapter_config,
provides_map,
pin_map: self.pin_alias_map,
Expand Down Expand Up @@ -666,6 +670,11 @@ impl TransportWrapperBuilder {
}

impl TransportWrapper {
pub fn ignore_dft_straps_on_reset(&self) -> Result<()> {
self.disable_dft_on_reset.set(false);
Ok(())
}

/// Returns a `Capabilities` object to check the capabilities of this
/// transport object.
pub fn capabilities(&self) -> Result<crate::transport::Capabilities> {
Expand Down Expand Up @@ -923,17 +932,24 @@ impl TransportWrapper {
}

pub fn reset_target(&self, reset_delay: Duration, clear_uart_rx: bool) -> Result<()> {
let reset_strapping = self.pin_strapping("RESET")?;
log::info!("Asserting the reset signal");
reset_strapping.apply()?;
if self.disable_dft_on_reset.get() {
self.pin_strapping("PRERESET_DFT_DISABLE")?.apply()?;
}
self.pin_strapping("RESET")?.apply()?;
std::thread::sleep(reset_delay);
if clear_uart_rx {
log::info!("Clearing the UART RX buffer");
self.uart("console")?.clear_rx_buffer()?;
}
log::info!("Deasserting the reset signal");
reset_strapping.remove()?;
std::thread::sleep(reset_delay);
self.pin_strapping("RESET")?.remove()?;
if self.disable_dft_on_reset.get() {
std::thread::sleep(Duration::from_millis(5));
// We remove the DFT strapping after waiting some time, as the DFT straps should have been
// sampled by then and we can resume our desired pin configuration.
self.pin_strapping("PRERESET_DFT_DISABLE")?.remove()?;
}
Ok(())
}
}
Expand Down
9 changes: 8 additions & 1 deletion sw/host/opentitanlib/src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ pub struct BackendOpts {
#[arg(long, default_value = "")]
pub interface: String,

/// Whether to disable DFT with a strapping config during reset. Only required in TestUnlocked*
/// LC states activate the JTAG RV TAP. This is required since the DFT straps share the console
/// UART pins, that hyperdebug tries to pull high. In mission mode states this should be set to
/// false, as DFT straps are not sampled here.
#[arg(long)]
pub disable_dft_on_reset: bool,

/// USB Vendor ID of the interface.
#[arg(long, value_parser = u16::from_str)]
pub usb_vid: Option<u16>,
Expand Down Expand Up @@ -73,7 +80,7 @@ pub enum Error {
/// Creates the requested backend interface according to [`BackendOpts`].
pub fn create(args: &BackendOpts) -> Result<TransportWrapper> {
let interface = args.interface.as_str();
let mut env = TransportWrapperBuilder::new(interface.to_string());
let mut env = TransportWrapperBuilder::new(interface.to_string(), args.disable_dft_on_reset);

for conf_file in &args.conf {
process_config_file(&mut env, conf_file.as_ref())?
Expand Down
2 changes: 2 additions & 0 deletions sw/host/provisioning/orchestrator/ot_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def cp_provision(self, require_confirmation=True):
"sram_cp_provision_fpga_cw310_rom_with_fake_keys.elf")
else:
platform_flags = """--interface=hyper310 \
--disable-dft-on-reset \
--openocd=third_party/openocd/build_openocd/bin/openocd \
--openocd-adapter-config=external/openocd/tcl/interface/cmsis-dap.cfg"""
elf = elf_path.format("sram_cp_provision_silicon_creator.elf")
Expand Down Expand Up @@ -135,6 +136,7 @@ def ft_provision(self, ecc_priv_keyfile, require_confirmation=True):

platform_bazel_flags = "--//signing:token=//signing/tokens:nitrokey"
platform_harness_flags = """--interface=hyper310 \
--disable-dft-on-reset \
--openocd=third_party/openocd/build_openocd/bin/openocd \
--openocd-adapter-config=external/openocd/tcl/interface/cmsis-dap.cfg"""

Expand Down
3 changes: 3 additions & 0 deletions sw/host/tests/manuf/provisioning/ft/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ fn main() -> Result<()> {
}
};

// Once we are in a mission mode, we no longer need to provide a DFT strapping sequence on
// every reset, as DFT is no longer enabled in mission modes.
transport.ignore_dft_straps_on_reset()?;
run_ft_personalize(
&transport,
&opts.init,
Expand Down

0 comments on commit af73135

Please sign in to comment.