From f65590cb084b7ac1a4deb64ff697caaf4a4e0407 Mon Sep 17 00:00:00 2001 From: "Jes B. Klinke" Date: Tue, 19 Nov 2024 14:26:01 -0800 Subject: [PATCH 1/3] [opentitantool] Enhance SPI and I2C traits Add a `set_pins()` method to the I2C trait. This allows instructing the debugger to set up its pinmux, if it has such support, in the same way as the existing method on the SPI trait. Also, add a new pin to both SPI and I2C called `gsc_ready`. This one will not be used by normal I2C or SPI communication, but the debugger can be instructed to wait for a falling edge on that pin, before continuing the transaction, typically the sequence given to `run_transcation()` would look somewhat like `[Write(...), GscReady, Read(...)]` which would have the effect that the debugger would send a number of bytes, and then wait for falling edge before resuming clocking to retrieve the response. This is useful when communicating with Google Security Chips. Finally, the SPI trait is enhanced with a `TpmPoll` instruction, which is to be used like `[Write(...), TpmPoll, Read/Write(...)]` when communicating with standards-compliant TPM devices. It has the effect that the debugger will first send the request (must be 4 bytes), then it will repeatedly poll by reading a single byte at a time, until the device responds with 0x01, before the debugger reads or writes the main data part. Signed-off-by: Jes B. Klinke Change-Id: Ied56bac014a6d5bfe501a6bd3f34c7b21b69265d --- sw/host/opentitanlib/src/app/i2c.rs | 65 ++++++++++++++++--- sw/host/opentitanlib/src/app/mod.rs | 1 + sw/host/opentitanlib/src/app/spi.rs | 17 ++++- sw/host/opentitanlib/src/io/i2c.rs | 18 +++++ sw/host/opentitanlib/src/io/spi.rs | 16 ++++- sw/host/opentitanlib/src/proxy/handler.rs | 30 +++++++++ sw/host/opentitanlib/src/proxy/protocol.rs | 17 +++++ .../src/transport/chip_whisperer/spi.rs | 13 +++- .../src/transport/dediprog/spi.rs | 4 ++ .../src/transport/hyperdebug/i2c.rs | 1 + .../src/transport/hyperdebug/spi.rs | 13 +++- .../opentitanlib/src/transport/proxy/i2c.rs | 32 ++++++++- .../opentitanlib/src/transport/proxy/spi.rs | 13 ++++ .../src/transport/ti50emulator/i2c.rs | 1 + .../src/transport/ti50emulator/spi.rs | 5 ++ .../src/transport/ultradebug/spi.rs | 8 ++- 16 files changed, 235 insertions(+), 19 deletions(-) diff --git a/sw/host/opentitanlib/src/app/i2c.rs b/sw/host/opentitanlib/src/app/i2c.rs index 7b52ecc24460d..fb42f90092543 100644 --- a/sw/host/opentitanlib/src/app/i2c.rs +++ b/sw/host/opentitanlib/src/app/i2c.rs @@ -2,11 +2,12 @@ // Licensed under the Apache License, Version 2.0, see LICENSE for details. // SPDX-License-Identifier: Apache-2.0 +use crate::io::gpio::GpioPin; use crate::io::i2c::{self, Bus, DeviceStatus, Mode}; use crate::transport::Transport; use anyhow::Result; -use std::cell::Cell; +use std::cell::{Cell, RefCell}; use std::rc::Rc; use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Duration; @@ -45,8 +46,17 @@ pub struct LogicalI2cWrapper { physical_wrapper: Rc, /// Unique ID of this `LogicalI2cWrapper`. uid: usize, - default_addr: Cell>, - max_speed: Cell>, + /// I2C port settings applying to this named logical I2C port. + inner: RefCell, +} + +/// I2C port settings applying to this named logical I2C port. +struct Inner { + default_addr: Option, + max_speed: Option, + serial_clock: Option>, + serial_data: Option>, + gsc_ready: Option>, } impl LogicalI2cWrapper { @@ -59,8 +69,13 @@ impl LogicalI2cWrapper { Ok(Self { physical_wrapper, uid: COUNTER.fetch_add(1, Ordering::Relaxed), - default_addr: Cell::new(conf.default_addr), - max_speed: Cell::new(conf.bits_per_sec), + inner: RefCell::new(Inner { + default_addr: conf.default_addr, + max_speed: conf.bits_per_sec, + serial_clock: None, + serial_data: None, + gsc_ready: None, + }), }) } @@ -70,16 +85,28 @@ impl LogicalI2cWrapper { // settings. return Ok(()); } - if let Some(addr) = self.default_addr.get() { + let inner = self.inner.borrow(); + if let Some(addr) = inner.default_addr { self.physical_wrapper .underlying_target .set_default_address(addr)?; } - if let Some(speed) = self.max_speed.get() { + if let Some(speed) = inner.max_speed { self.physical_wrapper .underlying_target .set_max_speed(speed)?; } + match ( + inner.serial_clock.as_ref(), + inner.serial_data.as_ref(), + inner.gsc_ready.as_ref(), + ) { + (None, None, None) => (), + (scl, sda, rdy) => self + .physical_wrapper + .underlying_target + .set_pins(scl, sda, rdy)?, + } self.physical_wrapper.last_used_by_uid.set(Some(self.uid)); Ok(()) } @@ -94,12 +121,32 @@ impl Bus for LogicalI2cWrapper { self.physical_wrapper.underlying_target.get_max_speed() } fn set_max_speed(&self, max_speed: u32) -> Result<()> { - self.max_speed.set(Some(max_speed)); + self.inner.borrow_mut().max_speed = Some(max_speed); + Ok(()) + } + + fn set_pins( + &self, + serial_clock: Option<&Rc>, + serial_data: Option<&Rc>, + gsc_ready: Option<&Rc>, + ) -> Result<()> { + log::error!("LogicalI2cWrapper::set_pins()"); + let mut inner = self.inner.borrow_mut(); + if serial_clock.is_some() { + inner.serial_clock = serial_clock.map(Rc::clone); + } + if serial_data.is_some() { + inner.serial_data = serial_data.map(Rc::clone); + } + if gsc_ready.is_some() { + inner.gsc_ready = gsc_ready.map(Rc::clone); + } Ok(()) } fn set_default_address(&self, addr: u8) -> Result<()> { - self.default_addr.set(Some(addr)); + self.inner.borrow_mut().default_addr = Some(addr); Ok(()) } diff --git a/sw/host/opentitanlib/src/app/mod.rs b/sw/host/opentitanlib/src/app/mod.rs index d6516fde9e997..98aad81015ca1 100644 --- a/sw/host/opentitanlib/src/app/mod.rs +++ b/sw/host/opentitanlib/src/app/mod.rs @@ -180,6 +180,7 @@ pub struct SpiConfiguration { pub host_out_device_in: Option, pub host_in_device_out: Option, pub chip_select: Option, + pub gsc_ready: Option, pub bits_per_word: Option, pub bits_per_sec: Option, } diff --git a/sw/host/opentitanlib/src/app/spi.rs b/sw/host/opentitanlib/src/app/spi.rs index 6887df696fe41..df5659a9cad33 100644 --- a/sw/host/opentitanlib/src/app/spi.rs +++ b/sw/host/opentitanlib/src/app/spi.rs @@ -60,6 +60,7 @@ struct Inner { host_out_device_in: Option>, host_in_device_out: Option>, chip_select: Option>, + gsc_ready: Option>, } impl LogicalSpiWrapper { @@ -81,6 +82,7 @@ impl LogicalSpiWrapper { host_out_device_in: Self::lookup_pin(transport, &conf.host_out_device_in)?, host_in_device_out: Self::lookup_pin(transport, &conf.host_in_device_out)?, chip_select: Self::lookup_pin(transport, &conf.chip_select)?, + gsc_ready: Self::lookup_pin(transport, &conf.gsc_ready)?, }), }) } @@ -123,12 +125,13 @@ impl LogicalSpiWrapper { inner.host_out_device_in.as_ref(), inner.host_in_device_out.as_ref(), inner.chip_select.as_ref(), + inner.gsc_ready.as_ref(), ) { - (None, None, None, None) => (), - (clock, hodi, hido, cs) => self + (None, None, None, None, None) => (), + (clock, hodi, hido, cs, rdy) => self .physical_wrapper .underlying_target - .set_pins(clock, hodi, hido, cs)?, + .set_pins(clock, hodi, hido, cs, rdy)?, } self.physical_wrapper.last_used_by_uid.set(Some(self.uid)); Ok(()) @@ -166,12 +169,17 @@ impl Target for LogicalSpiWrapper { .supports_bidirectional_transfer() } + fn supports_tpm_poll(&self) -> Result { + self.physical_wrapper.underlying_target.supports_tpm_poll() + } + fn set_pins( &self, serial_clock: Option<&Rc>, host_out_device_in: Option<&Rc>, host_in_device_out: Option<&Rc>, chip_select: Option<&Rc>, + gsc_ready: Option<&Rc>, ) -> Result<()> { let mut inner = self.inner.borrow_mut(); if serial_clock.is_some() { @@ -186,6 +194,9 @@ impl Target for LogicalSpiWrapper { if chip_select.is_some() { inner.chip_select = chip_select.map(Rc::clone); } + if gsc_ready.is_some() { + inner.gsc_ready = gsc_ready.map(Rc::clone); + } Ok(()) } diff --git a/sw/host/opentitanlib/src/io/i2c.rs b/sw/host/opentitanlib/src/io/i2c.rs index 7fac99f942b42..3443564080755 100644 --- a/sw/host/opentitanlib/src/io/i2c.rs +++ b/sw/host/opentitanlib/src/io/i2c.rs @@ -9,6 +9,7 @@ use std::rc::Rc; use std::time::Duration; use thiserror::Error; +use super::gpio; use crate::app::TransportWrapper; use crate::impl_serializable_error; use crate::transport::TransportError; @@ -63,6 +64,8 @@ pub enum I2cError { MissingAddress, #[error("I2C port not in device mode")] NotInDeviceMode, + #[error("Given pin does not support requested I2C function")] + InvalidPin, #[error("Generic error {0}")] Generic(String), } @@ -72,6 +75,8 @@ impl_serializable_error!(I2cError); pub enum Transfer<'rd, 'wr> { Read(&'rd mut [u8]), Write(&'wr [u8]), + // Wait for pulse on non-standard Google signal + GscReady, } /// Status of I2C read operations (data from device to host). @@ -143,6 +148,19 @@ pub trait Bus { /// 1_000_000. fn set_max_speed(&self, max_speed: u32) -> Result<()>; + /// Sets which pins should be used for I2C communication. `None` value means use the same pin + /// as previously, or the implementation default if never before specified. This call is not + /// supported by most backend transports, and ones that do support it may still have + /// restrictions on which set of pins can be used in which roles. + fn set_pins( + &self, + _serial_clock: Option<&Rc>, + _serial_data: Option<&Rc>, + _gsc_ready: Option<&Rc>, + ) -> Result<()> { + Err(I2cError::InvalidPin.into()) + } + /// Sets the "default" device address, used in cases when not overriden by parameter to /// `run_transaction()`. fn set_default_address(&self, addr: u8) -> Result<()>; diff --git a/sw/host/opentitanlib/src/io/spi.rs b/sw/host/opentitanlib/src/io/spi.rs index a640bfd77caf6..3dbb5c22c8d0e 100644 --- a/sw/host/opentitanlib/src/io/spi.rs +++ b/sw/host/opentitanlib/src/io/spi.rs @@ -45,7 +45,13 @@ impl SpiParams { ) -> Result> { let spi = transport.spi(self.bus.as_deref().unwrap_or(default_instance))?; if let Some(ref cs) = self.chip_select { - spi.set_pins(None, None, None, Some(&transport.gpio_pin(cs.as_str())?))?; + spi.set_pins( + None, + None, + None, + Some(&transport.gpio_pin(cs.as_str())?), + None, + )?; } if let Some(speed) = self.speed { spi.set_max_speed(speed)?; @@ -156,6 +162,10 @@ pub enum Transfer<'rd, 'wr> { Write(&'wr [u8]), // Check supports_bidirectional_transfer before using this. Both(&'wr [u8], &'rd mut [u8]), + // Repeatedly poll until device response with 0x01 + TpmPoll, + // Wait for pulse on non-standard Google signal + GscReady, } /// A trait which represents a SPI Target. @@ -178,6 +188,9 @@ pub trait Target { /// Indicates whether `Transfer::Both()` is supported. fn supports_bidirectional_transfer(&self) -> Result; + /// Indicates whether `Transfer::TpmPoll` is supported. + fn supports_tpm_poll(&self) -> Result; + /// Sets which pins should be used for SPI communication. `None` value means use the same pin /// as previously, or the implementation default if never before specified. This call is not /// supported by most backend transports, and ones that do support it may still have @@ -188,6 +201,7 @@ pub trait Target { _host_out_device_in: Option<&Rc>, _host_in_device_out: Option<&Rc>, _chip_select: Option<&Rc>, + _gsc_ready: Option<&Rc>, ) -> Result<()> { Err(SpiError::InvalidPin.into()) } diff --git a/sw/host/opentitanlib/src/proxy/handler.rs b/sw/host/opentitanlib/src/proxy/handler.rs index cebe18f20aa3a..eb75a4224348e 100644 --- a/sw/host/opentitanlib/src/proxy/handler.rs +++ b/sw/host/opentitanlib/src/proxy/handler.rs @@ -334,17 +334,23 @@ impl<'a> TransportCommandHandler<'a> { has_support, })) } + SpiRequest::SupportsTpmPoll => { + let has_support = instance.supports_tpm_poll()?; + Ok(Response::Spi(SpiResponse::SupportsTpmPoll { has_support })) + } SpiRequest::SetPins { serial_clock, host_out_device_in, host_in_device_out, chip_select, + gsc_ready, } => { instance.set_pins( self.optional_pin(serial_clock)?.as_ref(), self.optional_pin(host_out_device_in)?.as_ref(), self.optional_pin(host_in_device_out)?.as_ref(), self.optional_pin(chip_select)?.as_ref(), + self.optional_pin(gsc_ready)?.as_ref(), )?; Ok(Response::Spi(SpiResponse::SetPins)) } @@ -378,6 +384,8 @@ impl<'a> TransportCommandHandler<'a> { SpiTransferRequest::Both { data } => SpiTransferResponse::Both { data: vec![0; data.len()], }, + SpiTransferRequest::TpmPoll => SpiTransferResponse::TpmPoll, + SpiTransferRequest::GscReady => SpiTransferResponse::GscReady, }) .collect(); // Now carefully craft a proper parameter to the @@ -400,6 +408,12 @@ impl<'a> TransportCommandHandler<'a> { SpiTransferRequest::Both { data: wdata }, SpiTransferResponse::Both { data }, ) => spi::Transfer::Both(wdata, data), + (SpiTransferRequest::TpmPoll, SpiTransferResponse::TpmPoll) => { + spi::Transfer::TpmPoll + } + (SpiTransferRequest::GscReady, SpiTransferResponse::GscReady) => { + spi::Transfer::GscReady + } _ => { // This can only happen if the logic in this method is // flawed. (Never due to network input.) @@ -452,6 +466,18 @@ impl<'a> TransportCommandHandler<'a> { instance.set_max_speed(*value)?; Ok(Response::I2c(I2cResponse::SetMaxSpeed)) } + I2cRequest::SetPins { + serial_clock, + serial_data, + gsc_ready, + } => { + instance.set_pins( + self.optional_pin(serial_clock)?.as_ref(), + self.optional_pin(serial_data)?.as_ref(), + self.optional_pin(gsc_ready)?.as_ref(), + )?; + Ok(Response::I2c(I2cResponse::SetPins)) + } I2cRequest::RunTransaction { address, transaction: reqs, @@ -464,6 +490,7 @@ impl<'a> TransportCommandHandler<'a> { data: vec![0; *len as usize], }, I2cTransferRequest::Write { .. } => I2cTransferResponse::Write, + I2cTransferRequest::GscReady => I2cTransferResponse::GscReady, }) .collect(); // Now carefully craft a proper parameter to the @@ -482,6 +509,9 @@ impl<'a> TransportCommandHandler<'a> { I2cTransferRequest::Write { data }, I2cTransferResponse::Write, ) => i2c::Transfer::Write(data), + (I2cTransferRequest::GscReady, I2cTransferResponse::GscReady) => { + i2c::Transfer::GscReady + } _ => { // This can only happen if the logic in this method is // flawed. (Never due to network input.) diff --git a/sw/host/opentitanlib/src/proxy/protocol.rs b/sw/host/opentitanlib/src/proxy/protocol.rs index 483a4e107253e..64affbc243fc1 100644 --- a/sw/host/opentitanlib/src/proxy/protocol.rs +++ b/sw/host/opentitanlib/src/proxy/protocol.rs @@ -210,6 +210,8 @@ pub enum SpiTransferRequest { Read { len: u32 }, Write { data: Vec }, Both { data: Vec }, + TpmPoll, + GscReady, } #[derive(Serialize, Deserialize)] @@ -217,6 +219,8 @@ pub enum SpiTransferResponse { Read { data: Vec }, Write, Both { data: Vec }, + TpmPoll, + GscReady, } #[derive(Serialize, Deserialize)] @@ -234,11 +238,13 @@ pub enum SpiRequest { value: u32, }, SupportsBidirectionalTransfer, + SupportsTpmPoll, SetPins { serial_clock: Option, host_out_device_in: Option, host_in_device_out: Option, chip_select: Option, + gsc_ready: Option, }, GetMaxTransferCount, GetMaxTransferSizes, @@ -270,6 +276,9 @@ pub enum SpiResponse { SupportsBidirectionalTransfer { has_support: bool, }, + SupportsTpmPoll { + has_support: bool, + }, SetPins, GetMaxTransferCount { number: usize, @@ -292,12 +301,14 @@ pub enum SpiResponse { pub enum I2cTransferRequest { Read { len: u32 }, Write { data: Vec }, + GscReady, } #[derive(Serialize, Deserialize)] pub enum I2cTransferResponse { Read { data: Vec }, Write, + GscReady, } #[derive(Serialize, Deserialize)] @@ -310,6 +321,11 @@ pub enum I2cRequest { SetMaxSpeed { value: u32, }, + SetPins { + serial_clock: Option, + serial_data: Option, + gsc_ready: Option, + }, RunTransaction { address: Option, transaction: Vec, @@ -331,6 +347,7 @@ pub enum I2cResponse { speed: u32, }, SetMaxSpeed, + SetPins, RunTransaction { transaction: Vec, }, diff --git a/sw/host/opentitanlib/src/transport/chip_whisperer/spi.rs b/sw/host/opentitanlib/src/transport/chip_whisperer/spi.rs index e021a61fcefdb..195558fcd617c 100644 --- a/sw/host/opentitanlib/src/transport/chip_whisperer/spi.rs +++ b/sw/host/opentitanlib/src/transport/chip_whisperer/spi.rs @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0, see LICENSE for details. // SPDX-License-Identifier: Apache-2.0 -use anyhow::Result; +use anyhow::{bail, Result}; use std::cell::RefCell; use std::rc::Rc; @@ -35,6 +35,7 @@ impl Spi { Transfer::Read(buf) => usb.spi1_read(buf)?, Transfer::Write(buf) => usb.spi1_write(buf)?, Transfer::Both(wbuf, rbuf) => usb.spi1_both(wbuf, rbuf)?, + _ => bail!(TransportError::UnsupportedOperation), } } Ok(()) @@ -79,20 +80,26 @@ impl Target for Spi { Ok(true) } + fn supports_tpm_poll(&self) -> Result { + Ok(false) + } + fn set_pins( &self, serial_clock: Option<&Rc>, host_out_device_in: Option<&Rc>, host_in_device_out: Option<&Rc>, chip_select: Option<&Rc>, + gsc_ready: Option<&Rc>, ) -> Result<()> { match ( serial_clock, host_out_device_in, host_in_device_out, chip_select, + gsc_ready, ) { - (Some(clk), Some(sdo), Some(sdi), Some(cs)) => { + (Some(clk), Some(sdo), Some(sdi), Some(cs), None) => { let usb = self.device.borrow(); usb.spi1_enable(false)?; usb.spi1_setpins( @@ -104,7 +111,7 @@ impl Target for Spi { usb.spi1_enable(true)?; Ok(()) } - (None, None, None, None) => Ok(()), + (None, None, None, None, None) => Ok(()), _ => { // Explicitly choosing pins for some of the SPI signals, while leaving others at // their default, is not supported. diff --git a/sw/host/opentitanlib/src/transport/dediprog/spi.rs b/sw/host/opentitanlib/src/transport/dediprog/spi.rs index 1a714c900a570..175e61f1cb5dc 100644 --- a/sw/host/opentitanlib/src/transport/dediprog/spi.rs +++ b/sw/host/opentitanlib/src/transport/dediprog/spi.rs @@ -349,6 +349,10 @@ impl Target for DediprogSpi { Ok(false) } + fn supports_tpm_poll(&self) -> Result { + Ok(false) + } + fn get_max_transfer_count(&self) -> Result { // Arbitrary value: number of `Transfers` that can be in a single transaction. Ok(42) diff --git a/sw/host/opentitanlib/src/transport/hyperdebug/i2c.rs b/sw/host/opentitanlib/src/transport/hyperdebug/i2c.rs index 725c20588d3c5..8fb155604d764 100644 --- a/sw/host/opentitanlib/src/transport/hyperdebug/i2c.rs +++ b/sw/host/opentitanlib/src/transport/hyperdebug/i2c.rs @@ -380,6 +380,7 @@ impl Bus for HyperdebugI2cBus { transaction = &mut transaction[1..]; } [] => (), + _ => bail!(TransportError::UnsupportedOperation), } } Ok(()) diff --git a/sw/host/opentitanlib/src/transport/hyperdebug/spi.rs b/sw/host/opentitanlib/src/transport/hyperdebug/spi.rs index 53fdabe8f523b..69684342f63d3 100644 --- a/sw/host/opentitanlib/src/transport/hyperdebug/spi.rs +++ b/sw/host/opentitanlib/src/transport/hyperdebug/spi.rs @@ -684,14 +684,23 @@ impl Target for HyperdebugSpiTarget { Ok((self.feature_bitmap & FEATURE_BIT_FULL_DUPLEX) != 0) } + fn supports_tpm_poll(&self) -> Result { + Ok(false) + } + fn set_pins( &self, serial_clock: Option<&Rc>, host_out_device_in: Option<&Rc>, host_in_device_out: Option<&Rc>, chip_select: Option<&Rc>, + gsc_ready: Option<&Rc>, ) -> Result<()> { - if serial_clock.is_some() || host_out_device_in.is_some() || host_in_device_out.is_some() { + if serial_clock.is_some() + || host_out_device_in.is_some() + || host_in_device_out.is_some() + || gsc_ready.is_some() + { bail!(SpiError::InvalidPin); } if let Some(pin) = chip_select { @@ -824,6 +833,8 @@ impl Target for HyperdebugSpiTarget { self.transmit(wbuf, FULL_DUPLEX)?; self.receive(rbuf)?; } + [Transfer::TpmPoll, ..] => bail!(TransportError::UnsupportedOperation), + [Transfer::GscReady, ..] => bail!(TransportError::UnsupportedOperation), [] => (), } idx += 1; diff --git a/sw/host/opentitanlib/src/transport/proxy/i2c.rs b/sw/host/opentitanlib/src/transport/proxy/i2c.rs index 1175fdb51dcb6..faa7898513073 100644 --- a/sw/host/opentitanlib/src/transport/proxy/i2c.rs +++ b/sw/host/opentitanlib/src/transport/proxy/i2c.rs @@ -8,7 +8,8 @@ use std::rc::Rc; use std::time::Duration; use super::ProxyError; -use crate::io::i2c::{Bus, DeviceStatus, Mode, Transfer}; +use crate::io::gpio; +use crate::io::i2c::{Bus, DeviceStatus, I2cError, Mode, Transfer}; use crate::proxy::protocol::{ I2cRequest, I2cResponse, I2cTransferRequest, I2cTransferResponse, Request, Response, }; @@ -42,6 +43,17 @@ impl ProxyI2c { } } +fn i2c_pin_name(pin: Option<&Rc>) -> Result> { + if let Some(pin) = pin { + let Some(name) = pin.get_internal_pin_name() else { + bail!(I2cError::InvalidPin) + }; + Ok(Some(name.to_string())) + } else { + Ok(None) + } +} + impl Bus for ProxyI2c { fn set_mode(&self, mode: Mode) -> Result<()> { match mode { @@ -71,6 +83,22 @@ impl Bus for ProxyI2c { } } + fn set_pins( + &self, + serial_clock: Option<&Rc>, + serial_data: Option<&Rc>, + gsc_ready: Option<&Rc>, + ) -> Result<()> { + match self.execute_command(I2cRequest::SetPins { + serial_clock: i2c_pin_name(serial_clock)?, + serial_data: i2c_pin_name(serial_data)?, + gsc_ready: i2c_pin_name(gsc_ready)?, + })? { + I2cResponse::SetPins => Ok(()), + _ => bail!(ProxyError::UnexpectedReply()), + } + } + fn set_default_address(&self, addr: u8) -> Result<()> { self.default_address.set(Some(addr)); Ok(()) @@ -87,6 +115,7 @@ impl Bus for ProxyI2c { Transfer::Write(wbuf) => req.push(I2cTransferRequest::Write { data: wbuf.to_vec(), }), + Transfer::GscReady => req.push(I2cTransferRequest::GscReady), } } match self.execute_command(I2cRequest::RunTransaction { @@ -104,6 +133,7 @@ impl Bus for ProxyI2c { rbuf.clone_from_slice(data); } (I2cTransferResponse::Write, Transfer::Write(_)) => (), + (I2cTransferResponse::GscReady, Transfer::GscReady) => (), _ => bail!(ProxyError::UnexpectedReply()), } } diff --git a/sw/host/opentitanlib/src/transport/proxy/spi.rs b/sw/host/opentitanlib/src/transport/proxy/spi.rs index 17cb826b8f8ea..c92f667253ee6 100644 --- a/sw/host/opentitanlib/src/transport/proxy/spi.rs +++ b/sw/host/opentitanlib/src/transport/proxy/spi.rs @@ -101,18 +101,27 @@ impl Target for ProxySpi { } } + fn supports_tpm_poll(&self) -> Result { + match self.execute_command(SpiRequest::SupportsTpmPoll)? { + SpiResponse::SupportsTpmPoll { has_support } => Ok(has_support), + _ => bail!(ProxyError::UnexpectedReply()), + } + } + fn set_pins( &self, serial_clock: Option<&Rc>, host_out_device_in: Option<&Rc>, host_in_device_out: Option<&Rc>, chip_select: Option<&Rc>, + gsc_ready: Option<&Rc>, ) -> Result<()> { match self.execute_command(SpiRequest::SetPins { serial_clock: spi_pin_name(serial_clock)?, host_out_device_in: spi_pin_name(host_out_device_in)?, host_in_device_out: spi_pin_name(host_in_device_out)?, chip_select: spi_pin_name(chip_select)?, + gsc_ready: spi_pin_name(gsc_ready)?, })? { SpiResponse::SetPins => Ok(()), _ => bail!(ProxyError::UnexpectedReply()), @@ -166,6 +175,8 @@ impl Target for ProxySpi { data: wbuf.to_vec(), }) } + Transfer::TpmPoll => req.push(SpiTransferRequest::TpmPoll), + Transfer::GscReady => req.push(SpiTransferRequest::GscReady), } } match self.execute_command(SpiRequest::RunTransaction { transaction: req })? { @@ -181,6 +192,8 @@ impl Target for ProxySpi { rbuf.clone_from_slice(data); } (SpiTransferResponse::Write, Transfer::Write(_)) => (), + (SpiTransferResponse::TpmPoll, Transfer::TpmPoll) => (), + (SpiTransferResponse::GscReady, Transfer::GscReady) => (), _ => bail!(ProxyError::UnexpectedReply()), } } diff --git a/sw/host/opentitanlib/src/transport/ti50emulator/i2c.rs b/sw/host/opentitanlib/src/transport/ti50emulator/i2c.rs index 85c72fd6e4fd9..c75529e516aeb 100644 --- a/sw/host/opentitanlib/src/transport/ti50emulator/i2c.rs +++ b/sw/host/opentitanlib/src/transport/ti50emulator/i2c.rs @@ -261,6 +261,7 @@ impl Bus for Ti50I2cBus { Transfer::Read(rd) => { self.read(addr, rd)?; } + _ => bail!(TransportError::UnsupportedOperation), } } Ok(()) diff --git a/sw/host/opentitanlib/src/transport/ti50emulator/spi.rs b/sw/host/opentitanlib/src/transport/ti50emulator/spi.rs index 51563c628bbd5..1fe6e0b9682f6 100644 --- a/sw/host/opentitanlib/src/transport/ti50emulator/spi.rs +++ b/sw/host/opentitanlib/src/transport/ti50emulator/spi.rs @@ -47,6 +47,11 @@ impl Target for Ti50Spi { Err(TransportError::UnsupportedOperation.into()) } + /// Indicates whether `Transfer::TpmPoll` is supported. + fn supports_tpm_poll(&self) -> Result { + Err(TransportError::UnsupportedOperation.into()) + } + /// Returns the maximum number of transfers allowed in a single transaction. fn get_max_transfer_count(&self) -> Result { Err(TransportError::UnsupportedOperation.into()) diff --git a/sw/host/opentitanlib/src/transport/ultradebug/spi.rs b/sw/host/opentitanlib/src/transport/ultradebug/spi.rs index e30dbf7197f04..f953b77aea0ec 100644 --- a/sw/host/opentitanlib/src/transport/ultradebug/spi.rs +++ b/sw/host/opentitanlib/src/transport/ultradebug/spi.rs @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0, see LICENSE for details. // SPDX-License-Identifier: Apache-2.0 -use anyhow::{Context, Result}; +use anyhow::{bail, Context, Result}; use std::cell::RefCell; use std::rc::Rc; @@ -12,6 +12,7 @@ use crate::io::spi::{ }; use crate::transport::ultradebug::mpsse; use crate::transport::ultradebug::Ultradebug; +use crate::transport::TransportError; struct Inner { mode: TransferMode, @@ -101,6 +102,10 @@ impl Target for UltradebugSpi { Ok(true) } + fn supports_tpm_poll(&self) -> Result { + Ok(false) + } + fn get_max_transfer_count(&self) -> Result { // Arbitrary value: number of `Transfers` that can be in a single transaction. Ok(42) @@ -164,6 +169,7 @@ impl Target for UltradebugSpi { }, rbuf, ), + _ => bail!(TransportError::UnsupportedOperation), }); } if cs_not_already_asserted { From 9fcfe3e85fc102dc89b962fd705857836667ad1e Mon Sep 17 00:00:00 2001 From: "Jes B. Klinke" Date: Tue, 19 Nov 2024 14:13:38 -0800 Subject: [PATCH 2/3] [opentitantool] HyperDebug support for advanced TPM primitives Add support in the HyperDebug transport driver for `TpmPoll` and `GscReady`. Signed-off-by: Jes B. Klinke Change-Id: I154229794bcb88cc7845329c03621eec5c4da4a5 --- .../src/transport/hyperdebug/i2c.rs | 82 ++++++++++-- .../src/transport/hyperdebug/mod.rs | 18 ++- .../src/transport/hyperdebug/spi.rs | 121 +++++++++++++++++- 3 files changed, 203 insertions(+), 18 deletions(-) diff --git a/sw/host/opentitanlib/src/transport/hyperdebug/i2c.rs b/sw/host/opentitanlib/src/transport/hyperdebug/i2c.rs index 8fb155604d764..20c9260bc99f2 100644 --- a/sw/host/opentitanlib/src/transport/hyperdebug/i2c.rs +++ b/sw/host/opentitanlib/src/transport/hyperdebug/i2c.rs @@ -9,6 +9,7 @@ use std::rc::Rc; use std::time::Duration; use zerocopy::{AsBytes, FromBytes, FromZeroes}; +use crate::io::gpio::GpioPin; use crate::io::i2c::{self, Bus, DeviceStatus, DeviceTransfer, I2cError, ReadStatus, Transfer}; use crate::transport::hyperdebug::{BulkInterface, Inner}; use crate::transport::{TransportError, TransportInterfaceType}; @@ -59,7 +60,7 @@ struct CmdTransferLong { write_count: u8, read_count: u8, read_count1: u8, - reserved: u8, + flags: u8, data: [u8; USB_MAX_SIZE - 6], } @@ -175,7 +176,13 @@ impl HyperdebugI2cBus { } /// Transmit data for a single I2C operation, using one or more USB packets. - fn transmit_then_receive(&self, addr: u8, wbuf: &[u8], rbuf: &mut [u8]) -> Result<()> { + fn transmit_then_receive( + &self, + addr: u8, + wbuf: &[u8], + rbuf: &mut [u8], + gsc_ready: bool, + ) -> Result<()> { ensure!( rbuf.len() < self.max_read_size, I2cError::InvalidDataLength(rbuf.len()) @@ -185,7 +192,9 @@ impl HyperdebugI2cBus { I2cError::InvalidDataLength(wbuf.len()) ); let encapsulation_header_size = if self.cmsis_encapsulation { 1 } else { 0 }; - let mut index = if rbuf.len() < 128 { + let mut index = if false + /*rbuf.len() < 128*/ + { // Short format header let mut req = CmdTransferShort { encapsulation_header: Self::CMSIS_DAP_CUSTOM_COMMAND_I2C, @@ -197,6 +206,10 @@ impl HyperdebugI2cBus { }; let databytes = cmp::min(USB_MAX_SIZE - 4 - encapsulation_header_size, wbuf.len()); req.data[..databytes].clone_from_slice(&wbuf[..databytes]); + log::error!( + "I2C wrote {:?}", + &req.as_bytes()[1 - encapsulation_header_size..1 + 4 + databytes] + ); self.usb_write_bulk(&req.as_bytes()[1 - encapsulation_header_size..1 + 4 + databytes])?; databytes } else { @@ -206,13 +219,17 @@ impl HyperdebugI2cBus { port: self.bus_idx | (((wbuf.len() & 0x0F00) >> 4) as u8), addr, write_count: (wbuf.len() & 0x00FF) as u8, - read_count: (rbuf.len() & 0x007F) as u8, + read_count: (rbuf.len() & 0x007F | 0x0080) as u8, read_count1: (rbuf.len() >> 7) as u8, - reserved: 0, + flags: if gsc_ready { 0x80 } else { 0x00 }, data: [0; USB_MAX_SIZE - 6], }; let databytes = cmp::min(USB_MAX_SIZE - 6 - encapsulation_header_size, wbuf.len()); req.data[..databytes].clone_from_slice(&wbuf[..databytes]); + log::error!( + "I2C wrote {:?}", + &req.as_bytes()[1 - encapsulation_header_size..1 + 6 + databytes] + ); self.usb_write_bulk(&req.as_bytes()[1 - encapsulation_header_size..1 + 6 + databytes])?; databytes }; @@ -220,6 +237,7 @@ impl HyperdebugI2cBus { // Transmit any more data without further header. while index < wbuf.len() { let databytes = cmp::min(USB_MAX_SIZE, wbuf.len() - index); + log::error!("I2C wrote {:?}", &wbuf[index..index + databytes]); self.usb_write_bulk(&wbuf[index..index + databytes])?; index += databytes; } @@ -230,6 +248,10 @@ impl HyperdebugI2cBus { let read_count = self.usb_read_bulk( &mut resp.as_bytes_mut()[1 - encapsulation_header_size + bytecount..][..64], )?; + log::error!( + "I2C read {:?}", + &resp.as_bytes_mut()[1 - encapsulation_header_size + bytecount..][..read_count] + ); ensure!( read_count > 0, TransportError::CommunicationError("Truncated I2C response".to_string()) @@ -336,6 +358,25 @@ impl Bus for HyperdebugI2cBus { .cmd_no_output(&format!("i2c set speed {} {}", &self.bus_idx, max_speed)) } + fn set_pins( + &self, + serial_clock: Option<&Rc>, + serial_data: Option<&Rc>, + gsc_ready: Option<&Rc>, + ) -> Result<()> { + if serial_clock.is_some() || serial_data.is_some() { + bail!(I2cError::InvalidPin); + } + if let Some(pin) = gsc_ready { + self.inner.cmd_no_output(&format!( + "i2c set ready {} {}", + &self.bus_idx, + pin.get_internal_pin_name().ok_or(I2cError::InvalidPin)? + ))?; + } + Ok(()) + } + fn set_default_address(&self, addr: u8) -> Result<()> { self.default_addr.set(Some(addr)); Ok(()) @@ -347,6 +388,22 @@ impl Bus for HyperdebugI2cBus { .ok_or(I2cError::MissingAddress)?; while !transaction.is_empty() { match transaction { + [Transfer::Write(wbuf), Transfer::GscReady, Transfer::Read(rbuf), ..] => { + // Hyperdebug can do I2C write followed by I2C read as a single USB + // request/reply. Take advantage of that by detecting pairs of + // Transfer::Write followed by Transfer::Read. + ensure!( + wbuf.len() <= self.max_write_size, + I2cError::InvalidDataLength(wbuf.len()) + ); + ensure!( + rbuf.len() <= self.max_read_size, + I2cError::InvalidDataLength(rbuf.len()) + ); + self.transmit_then_receive(addr, wbuf, rbuf, true)?; + // Skip three steps ahead, as three items were processed. + transaction = &mut transaction[3..]; + } [Transfer::Write(wbuf), Transfer::Read(rbuf), ..] => { // Hyperdebug can do I2C write followed by I2C read as a single USB // request/reply. Take advantage of that by detecting pairs of @@ -359,7 +416,16 @@ impl Bus for HyperdebugI2cBus { rbuf.len() <= self.max_read_size, I2cError::InvalidDataLength(rbuf.len()) ); - self.transmit_then_receive(addr, wbuf, rbuf)?; + self.transmit_then_receive(addr, wbuf, rbuf, false)?; + // Skip two steps ahead, as two items were processed. + transaction = &mut transaction[2..]; + } + [Transfer::Write(wbuf), Transfer::GscReady, ..] => { + ensure!( + wbuf.len() <= self.max_write_size, + I2cError::InvalidDataLength(wbuf.len()) + ); + self.transmit_then_receive(addr, wbuf, &mut [], true)?; // Skip two steps ahead, as two items were processed. transaction = &mut transaction[2..]; } @@ -368,7 +434,7 @@ impl Bus for HyperdebugI2cBus { wbuf.len() <= self.max_write_size, I2cError::InvalidDataLength(wbuf.len()) ); - self.transmit_then_receive(addr, wbuf, &mut [])?; + self.transmit_then_receive(addr, wbuf, &mut [], false)?; transaction = &mut transaction[1..]; } [Transfer::Read(rbuf), ..] => { @@ -376,7 +442,7 @@ impl Bus for HyperdebugI2cBus { rbuf.len() <= self.max_read_size, I2cError::InvalidDataLength(rbuf.len()) ); - self.transmit_then_receive(addr, &[], rbuf)?; + self.transmit_then_receive(addr, &[], rbuf, false)?; transaction = &mut transaction[1..]; } [] => (), diff --git a/sw/host/opentitanlib/src/transport/hyperdebug/mod.rs b/sw/host/opentitanlib/src/transport/hyperdebug/mod.rs index 0a455f6ea95da..49e9b43035aff 100644 --- a/sw/host/opentitanlib/src/transport/hyperdebug/mod.rs +++ b/sw/host/opentitanlib/src/transport/hyperdebug/mod.rs @@ -138,6 +138,7 @@ impl Hyperdebug { const GOOGLE_CAP_GPIO_MONITORING: u16 = 0x0004; const GOOGLE_CAP_GPIO_BITBANGING: u16 = 0x0008; const GOOGLE_CAP_UART_QUEUE_CLEAR: u16 = 0x0010; + const GOOGLE_CAP_TPM_POLL: u16 = 0x0020; /// Establish connection with a particular HyperDebug. pub fn open( @@ -442,6 +443,7 @@ pub struct CachedIo { pub struct Conn { console_port: RefCell, + first_use: Cell, } // The way that the HyperDebug allows callers to request optimization for a sequence of operations @@ -467,6 +469,7 @@ impl Inner { flock_serial(&port, port_name)?; let conn = Rc::new(Conn { console_port: RefCell::new(port), + first_use: Cell::new(true), }); // Return a (strong) reference to the newly opened connection, while keeping a weak // reference to the same in this `Inner` object. The result is that if the caller keeps @@ -560,10 +563,16 @@ impl Conn { fn execute_command(&self, cmd: &str, mut callback: impl FnMut(&str)) -> Result<()> { let port: &mut TTYPort = &mut self.console_port.borrow_mut(); - // Send Ctrl-C, followed by the command, then newline. This will discard any previous - // partial input, before executing our command. - port.write(format!("\x03{}\n", cmd).as_bytes()) - .context("writing to HyperDebug console")?; + if self.first_use.get() { + // Send Ctrl-C, followed by the command, then newline. This will discard any previous + // partial input, before executing our command. + port.write(format!("\x03{}\n", cmd).as_bytes()) + .context("writing to HyperDebug console")?; + self.first_use.set(false); + } else { + port.write(format!("{}\n", cmd).as_bytes()) + .context("writing to HyperDebug console")?; + } // Now process response from HyperDebug. First we expect to see the echo of the command // we just "typed". Then zero, one or more lines of useful output, which we want to pass @@ -656,6 +665,7 @@ impl Transport for Hyperdebug { &self.spi_interface, enable_cmd, idx, + self.get_cmsis_google_capabilities()? & Self::GOOGLE_CAP_TPM_POLL != 0, )?); self.cached_io_interfaces .spis diff --git a/sw/host/opentitanlib/src/transport/hyperdebug/spi.rs b/sw/host/opentitanlib/src/transport/hyperdebug/spi.rs index 69684342f63d3..66324cd53021e 100644 --- a/sw/host/opentitanlib/src/transport/hyperdebug/spi.rs +++ b/sw/host/opentitanlib/src/transport/hyperdebug/spi.rs @@ -24,6 +24,7 @@ pub struct HyperdebugSpiTarget { target_enable_cmd: u8, target_idx: u8, feature_bitmap: u16, + supports_tpm_poll: bool, max_sizes: MaxSizes, cs_asserted_count: Cell, } @@ -73,6 +74,8 @@ const EEPROM_FLAGS_WIDTH_4WIRE: u32 = 0x00000100; const EEPROM_FLAGS_WIDTH_8WIRE: u32 = 0x00000180; const EEPROM_FLAGS_DTR: u32 = 0x00000200; const EEPROM_FLAGS_DUMMY_CYCLES_POS: u8 = 10; +const EEPROM_FLAGS_GSC_READY: u32 = 0x04000000; +const EEPROM_FLAGS_TPM: u32 = 0x08000000; const EEPROM_FLAGS_WRITE_ENABLE: u32 = 0x10000000; const EEPROM_FLAGS_POLL_BUSY: u32 = 0x20000000; const EEPROM_FLAGS_DOUBLE_BUFFER: u32 = 0x40000000; @@ -251,6 +254,7 @@ impl HyperdebugSpiTarget { spi_interface: &BulkInterface, enable_cmd: u8, idx: u8, + supports_tpm_poll: bool, ) -> Result { let mut usb_handle = inner.usb_device.borrow_mut(); @@ -293,6 +297,7 @@ impl HyperdebugSpiTarget { target_enable_cmd: enable_cmd, target_idx: idx, feature_bitmap: resp.feature_bitmap, + supports_tpm_poll, max_sizes: MaxSizes { read: resp.max_read_chunk as usize, write: resp.max_write_chunk as usize, @@ -337,6 +342,48 @@ impl HyperdebugSpiTarget { Ok(()) } + /// Preform TPM transactions, that is, send four bytes of header/address, then repeatedly poll + /// for ready statys from the device, before sending/receiving the data bytes. Optionally + /// wait for falling edge on "GSC ready" pin, at appropriate time during tracsation. + fn tpm_transmit(&self, wbuf: &[u8], rbuf_len: usize, await_gsc_ready: bool) -> Result<()> { + const TPM_HEADER_SIZE: usize = 4; + let mut req = CmdEepromTransferStart::new(); + if rbuf_len == 0 { + req.flags |= EEPROM_FLAGS_WRITE; + req.count = (wbuf.len() - TPM_HEADER_SIZE) as u16; + } else { + req.count = rbuf_len as u16; + ensure!( + wbuf.len() == TPM_HEADER_SIZE, + SpiError::InvalidDataLength(wbuf.len()) + ); + } + + req.flags |= (TPM_HEADER_SIZE as u32) << EEPROM_FLAGS_ADDR_LEN_POS; + req.flags |= EEPROM_FLAGS_TPM; + if await_gsc_ready { + req.flags |= EEPROM_FLAGS_GSC_READY; + } + + let data_start_offset = 0; + // Optional write data bytes + let databytes = std::cmp::min(USB_MAX_SIZE - 8 - data_start_offset, wbuf.len()); + req.data[data_start_offset..data_start_offset + databytes] + .clone_from_slice(&wbuf[0..databytes]); + self.usb_write_bulk(&req.as_bytes()[0..8 + data_start_offset + databytes])?; + let mut index = databytes; + + while index < wbuf.len() { + let mut req = CmdTransferContinue::new(); + req.data_index = index as u16; + let databytes = std::cmp::min(USB_MAX_SIZE - 4, wbuf.len() - index); + req.data[0..databytes].clone_from_slice(&wbuf[index..index + databytes]); + self.usb_write_bulk(&req.as_bytes()[0..4 + databytes])?; + index += databytes; + } + Ok(()) + } + /// Receive data for a single SPI operation, using one or more USB packets. fn receive(&self, rbuf: &mut [u8]) -> Result<()> { let mut resp = RspTransferStart::new(); @@ -685,7 +732,7 @@ impl Target for HyperdebugSpiTarget { } fn supports_tpm_poll(&self) -> Result { - Ok(false) + Ok(self.supports_tpm_poll) } fn set_pins( @@ -696,11 +743,7 @@ impl Target for HyperdebugSpiTarget { chip_select: Option<&Rc>, gsc_ready: Option<&Rc>, ) -> Result<()> { - if serial_clock.is_some() - || host_out_device_in.is_some() - || host_in_device_out.is_some() - || gsc_ready.is_some() - { + if serial_clock.is_some() || host_out_device_in.is_some() || host_in_device_out.is_some() { bail!(SpiError::InvalidPin); } if let Some(pin) = chip_select { @@ -710,6 +753,13 @@ impl Target for HyperdebugSpiTarget { pin.get_internal_pin_name().ok_or(SpiError::InvalidPin)? ))?; } + if let Some(pin) = gsc_ready { + self.inner.cmd_no_output(&format!( + "spi set ready {} {}", + &self.target_idx, + pin.get_internal_pin_name().ok_or(SpiError::InvalidPin)? + ))?; + } Ok(()) } @@ -747,6 +797,36 @@ impl Target for HyperdebugSpiTarget { self.receive(rbuf)?; return Ok(()); } + [Transfer::Write(wbuf), Transfer::GscReady, Transfer::TpmPoll, Transfer::Read(rbuf)] => { + // Hyperdebug can do SPI TPM transaction as a single USB + // request/reply. + ensure!( + wbuf.len() <= self.max_sizes.write, + SpiError::InvalidDataLength(wbuf.len()) + ); + ensure!( + rbuf.len() <= self.max_sizes.read, + SpiError::InvalidDataLength(rbuf.len()) + ); + self.tpm_transmit(wbuf, rbuf.len(), true)?; + self.receive(rbuf)?; + return Ok(()); + } + [Transfer::Write(wbuf), Transfer::TpmPoll, Transfer::Read(rbuf)] => { + // Hyperdebug can do SPI TPM transaction as a single USB + // request/reply. + ensure!( + wbuf.len() <= self.max_sizes.write, + SpiError::InvalidDataLength(wbuf.len()) + ); + ensure!( + rbuf.len() <= self.max_sizes.read, + SpiError::InvalidDataLength(rbuf.len()) + ); + self.tpm_transmit(wbuf, rbuf.len(), false)?; + self.receive(rbuf)?; + return Ok(()); + } [Transfer::Write(wbuf)] => { ensure!( wbuf.len() <= self.max_sizes.write, @@ -766,6 +846,35 @@ impl Target for HyperdebugSpiTarget { return Ok(()); } } + [Transfer::Write(wbuf1), Transfer::TpmPoll, Transfer::Write(wbuf2), Transfer::GscReady] => + { + // Hyperdebug can do SPI TPM transaction as a single USB + // request/reply. + ensure!( + wbuf1.len() + wbuf2.len() <= self.max_sizes.write, + SpiError::InvalidDataLength(wbuf1.len() + wbuf2.len()) + ); + let mut combined_buf = vec![0u8; wbuf1.len() + wbuf2.len()]; + combined_buf[..wbuf1.len()].clone_from_slice(wbuf1); + combined_buf[wbuf1.len()..].clone_from_slice(wbuf2); + self.tpm_transmit(&combined_buf, 0, true)?; + self.receive(&mut [])?; + return Ok(()); + } + [Transfer::Write(wbuf1), Transfer::TpmPoll, Transfer::Write(wbuf2)] => { + // Hyperdebug can do SPI TPM transaction as a single USB + // request/reply. + ensure!( + wbuf1.len() + wbuf2.len() <= self.max_sizes.write, + SpiError::InvalidDataLength(wbuf1.len() + wbuf2.len()) + ); + let mut combined_buf = vec![0u8; wbuf1.len() + wbuf2.len()]; + combined_buf[..wbuf1.len()].clone_from_slice(wbuf1); + combined_buf[wbuf1.len()..].clone_from_slice(wbuf2); + self.tpm_transmit(&combined_buf, 0, false)?; + self.receive(&mut [])?; + return Ok(()); + } [Transfer::Read(rbuf)] => { ensure!( rbuf.len() <= self.max_sizes.read, From 2d71bed2dc71496d5f6265efd4a0cac7f6ea4374 Mon Sep 17 00:00:00 2001 From: "Jes B. Klinke" Date: Tue, 19 Nov 2024 13:15:01 -0800 Subject: [PATCH 3/3] [opentitantool] Make TPM driver take advantage of new primitives Adapt TPM driver layer to use advanced transport features, if available. This change speeds up TPM communication between 2x and 4x when HyperDebug is used to communicate with GSC chips. (This CL also removes unused code which tried to deal with SPI TPM transactions on debuggers which do not support bidirectional SPI transfers.) Signed-off-by: Jes B. Klinke Change-Id: I16c56d367478c1394825b1be56a3622452af207a --- sw/host/opentitanlib/src/tpm/driver.rs | 158 ++++++++--------------- sw/host/opentitantool/src/command/i2c.rs | 14 +- sw/host/opentitantool/src/command/spi.rs | 14 +- sw/host/tpm2_test_server/src/main.rs | 18 ++- 4 files changed, 73 insertions(+), 131 deletions(-) diff --git a/sw/host/opentitanlib/src/tpm/driver.rs b/sw/host/opentitanlib/src/tpm/driver.rs index 6c9f8ffc708b5..0ea33f8b5ff7b 100644 --- a/sw/host/opentitanlib/src/tpm/driver.rs +++ b/sw/host/opentitanlib/src/tpm/driver.rs @@ -190,43 +190,15 @@ pub trait Driver { } } -type GpioPinAndMonitoring = (Rc, Rc); - -fn wait_for_gsc_ready(gsc_ready_pin: &Option) -> Result<()> { - let Some((gsc_ready_pin, monitoring)) = gsc_ready_pin else { - return Ok(()); - }; - let start_time = Instant::now(); - while !monitoring - .monitoring_read(&[gsc_ready_pin.borrow()], true)? - .events - .into_iter() - .any(|e| e.edge == gpio::Edge::Falling) - { - if Instant::now().duration_since(start_time) > TIMEOUT { - bail!(TpmError::Timeout) - } - } - Ok(()) -} - /// Implementation of the low level interface via standard SPI protocol. pub struct SpiDriver { spi: Rc, - gsc_ready_pin: Option<(Rc, Rc)>, + use_gsc_ready: bool, } impl SpiDriver { - pub fn new( - spi: Rc, - gsc_ready_pin: Option<(Rc, Rc)>, - ) -> Result { - if let Some((gsc_ready_pin, monitoring)) = &gsc_ready_pin { - // Set up monitoring of edges on the GSC ready pin. This will be more efficient than - // starting/stopping the monitoring on each TPM operation. - monitoring.monitoring_start(&[gsc_ready_pin.borrow()])?; - } - Ok(Self { spi, gsc_ready_pin }) + pub fn new(spi: Rc, use_gsc_ready: bool) -> Result { + Ok(Self { spi, use_gsc_ready }) } /// Numerical TPM register address as used in SPI protocol. @@ -311,55 +283,46 @@ const TIMEOUT: Duration = Duration::from_millis(500); impl Driver for SpiDriver { fn read_register(&self, register: Register, data: &mut [u8]) -> Result<()> { - if !self.spi.supports_bidirectional_transfer()? { - // SPI transport does not support bidirectional transfer. Assume that the TPM will - // send 0x01 on the byte immediately following the fourth and final request byte. - let req = self.compose_header(register, data.len(), true); - let mut buffer = vec![0u8; data.len() + 1]; + if !self.spi.supports_tpm_poll()? { + // Fallback on polling TPM status from this Rust code. + return self.do_read_register(register, data); + } + let req = self.compose_header(register, data.len(), true /* is_read */); + if self.use_gsc_ready { self.spi.run_transaction(&mut [ spi::Transfer::Write(&req), - spi::Transfer::Read(&mut buffer), - ])?; - ensure!(buffer[0] & 1 != 0, "TPM did not respond as expected",); - data.clone_from_slice(&buffer[1..]); - return Ok(()); - } - let result = self.do_read_register(register, data); - if result.is_ok() { - wait_for_gsc_ready(&self.gsc_ready_pin)?; + spi::Transfer::GscReady, + spi::Transfer::TpmPoll, + spi::Transfer::Read(data), + ]) + } else { + self.spi.run_transaction(&mut [ + spi::Transfer::Write(&req), + spi::Transfer::TpmPoll, + spi::Transfer::Read(data), + ]) } - result } fn write_register(&self, register: Register, data: &[u8]) -> Result<()> { - if !self.spi.supports_bidirectional_transfer()? { - /* - * SPI transport does not support bidirectional transfer. Assume that the TPM will - * send 0x01 on the byte immediately following the fourth and final request byte. - */ - let req = self.compose_header(register, data.len(), true); - let mut buffer = [0u8; 1]; + if !self.spi.supports_tpm_poll()? { + // Fallback on polling TPM status from this Rust code. + return self.do_write_register(register, data); + } + let req = self.compose_header(register, data.len(), false /* is_read */); + if self.use_gsc_ready { self.spi.run_transaction(&mut [ spi::Transfer::Write(&req), - spi::Transfer::Read(&mut buffer), + spi::Transfer::TpmPoll, spi::Transfer::Write(data), - ])?; - ensure!(buffer[0] & 1 != 0, "TPM did not respond as expected",); - return Ok(()); - } - let result = self.do_write_register(register, data); - if result.is_ok() { - wait_for_gsc_ready(&self.gsc_ready_pin)?; - } - result - } -} - -impl Drop for SpiDriver { - fn drop(&mut self) { - if let Some((gsc_ready_pin, monitoring)) = &self.gsc_ready_pin { - // Stop monitoring of the gsc_ready pin, by reading one final time. - let _ = monitoring.monitoring_read(&[gsc_ready_pin.borrow()], false); + spi::Transfer::GscReady, + ]) + } else { + self.spi.run_transaction(&mut [ + spi::Transfer::Write(&req), + spi::Transfer::TpmPoll, + spi::Transfer::Write(data), + ]) } } } @@ -367,20 +330,12 @@ impl Drop for SpiDriver { /// Implementation of the low level interface via Google I2C protocol. pub struct I2cDriver { i2c: Rc, - gsc_ready_pin: Option<(Rc, Rc)>, + use_gsc_ready: bool, } impl I2cDriver { - pub fn new( - i2c: Rc, - gsc_ready_pin: Option<(Rc, Rc)>, - ) -> Result { - if let Some((gsc_ready_pin, monitoring)) = &gsc_ready_pin { - // Set up monitoring of edges on the GSC ready pin. This will be more efficient than - // starting/stopping the monitoring on each TPM operation. - monitoring.monitoring_start(&[gsc_ready_pin.borrow()])?; - } - Ok(Self { i2c, gsc_ready_pin }) + pub fn new(i2c: Rc, use_gsc_ready: bool) -> Result { + Ok(Self { i2c, use_gsc_ready }) } /// Numerical TPM register address as used in Google I2C protocol. @@ -395,7 +350,7 @@ impl I2cDriver { } fn try_read_register(&self, register: Register, data: &mut [u8]) -> Result<()> { - if self.gsc_ready_pin.is_none() { + if !self.use_gsc_ready { // Do two I2C transfers in one call, for lowest latency. self.i2c.run_transaction( None, /* default addr */ @@ -405,16 +360,13 @@ impl I2cDriver { ], ) } else { - // Since we need to check for the GSC ready signal in between, we have to do one I2C - // transfer at a time, and tolerate the latency of multiple roundtrip. - self.i2c.run_transaction( - None, /* default addr */ - &mut [i2c::Transfer::Write(&[Self::addr(register).unwrap()])], - )?; - wait_for_gsc_ready(&self.gsc_ready_pin)?; self.i2c.run_transaction( None, /* default addr */ - &mut [i2c::Transfer::Read(data)], + &mut [ + i2c::Transfer::Write(&[Self::addr(register).unwrap()]), + i2c::Transfer::GscReady, + i2c::Transfer::Read(data), + ], ) } } @@ -456,20 +408,16 @@ impl Driver for I2cDriver { fn write_register(&self, register: Register, data: &[u8]) -> Result<()> { let mut buffer = vec![Self::addr(register).unwrap()]; buffer.extend_from_slice(data); - self.i2c.run_transaction( - None, /* default addr */ - &mut [i2c::Transfer::Write(&buffer)], - )?; - wait_for_gsc_ready(&self.gsc_ready_pin)?; - Ok(()) - } -} - -impl Drop for I2cDriver { - fn drop(&mut self) { - if let Some((gsc_ready_pin, monitoring)) = &self.gsc_ready_pin { - // Stop monitoring of the gsc_ready pin, by reading one final time. - let _ = monitoring.monitoring_read(&[gsc_ready_pin.borrow()], false); + if !self.use_gsc_ready { + self.i2c.run_transaction( + None, /* default addr */ + &mut [i2c::Transfer::Write(&buffer)], + ) + } else { + self.i2c.run_transaction( + None, /* default addr */ + &mut [i2c::Transfer::Write(&buffer), i2c::Transfer::GscReady], + ) } } } diff --git a/sw/host/opentitantool/src/command/i2c.rs b/sw/host/opentitantool/src/command/i2c.rs index f735ef4fd9625..8ab2ae3b4d2a0 100644 --- a/sw/host/opentitantool/src/command/i2c.rs +++ b/sw/host/opentitantool/src/command/i2c.rs @@ -274,14 +274,12 @@ impl CommandDispatch for I2cTpm { transport: &TransportWrapper, ) -> Result>> { let context = context.downcast_ref::().unwrap(); - let ready_pin = match &self.gsc_ready { - Some(pin) => Some((transport.gpio_pin(pin)?, transport.gpio_monitoring()?)), - None => None, - }; - let tpm_driver: Box = Box::new(tpm::I2cDriver::new( - context.params.create(transport, "TPM")?, - ready_pin, - )?); + let i2c = context.params.create(transport, "TPM")?; + if let Some(pin) = &self.gsc_ready { + i2c.set_pins(None, None, Some(&transport.gpio_pin(pin)?))?; + } + let tpm_driver: Box = + Box::new(tpm::I2cDriver::new(i2c, self.gsc_ready.is_some())?); self.command.run(&tpm_driver, transport) } } diff --git a/sw/host/opentitantool/src/command/spi.rs b/sw/host/opentitantool/src/command/spi.rs index 68111f429e63a..6ec13f4466407 100644 --- a/sw/host/opentitantool/src/command/spi.rs +++ b/sw/host/opentitantool/src/command/spi.rs @@ -294,14 +294,12 @@ impl CommandDispatch for SpiTpm { transport: &TransportWrapper, ) -> Result>> { let context = context.downcast_ref::().unwrap(); - let ready_pin = match &self.gsc_ready { - Some(pin) => Some((transport.gpio_pin(pin)?, transport.gpio_monitoring()?)), - None => None, - }; - let tpm_driver: Box = Box::new(tpm::SpiDriver::new( - context.params.create(transport, "TPM")?, - ready_pin, - )?); + let spi = context.params.create(transport, "TPM")?; + if let Some(pin) = &self.gsc_ready { + spi.set_pins(None, None, None, None, Some(&transport.gpio_pin(pin)?))?; + } + let tpm_driver: Box = + Box::new(tpm::SpiDriver::new(spi, self.gsc_ready.is_some())?); self.command.run(&tpm_driver, transport) } } diff --git a/sw/host/tpm2_test_server/src/main.rs b/sw/host/tpm2_test_server/src/main.rs index 2dd4f53f531cb..c2e8c616a4a07 100644 --- a/sw/host/tpm2_test_server/src/main.rs +++ b/sw/host/tpm2_test_server/src/main.rs @@ -84,19 +84,17 @@ pub fn main() -> anyhow::Result<()> { let bus: Box = match options.bus { TpmBus::Spi { params, gsc_ready } => { let spi = params.create(&transport, "TPM")?; - let ready_pin = match &gsc_ready { - Some(pin) => Some((transport.gpio_pin(pin)?, transport.gpio_monitoring()?)), - None => None, - }; - Box::new(SpiDriver::new(spi, ready_pin)?) + if let Some(pin) = &gsc_ready { + spi.set_pins(None, None, None, None, Some(&transport.gpio_pin(pin)?))?; + } + Box::new(SpiDriver::new(spi, gsc_ready.is_some())?) } TpmBus::I2C { params, gsc_ready } => { let i2c = params.create(&transport, "TPM")?; - let ready_pin = match &gsc_ready { - Some(pin) => Some((transport.gpio_pin(pin)?, transport.gpio_monitoring()?)), - None => None, - }; - Box::new(I2cDriver::new(i2c, ready_pin)?) + if let Some(pin) = &gsc_ready { + i2c.set_pins(None, None, Some(&transport.gpio_pin(pin)?))?; + } + Box::new(I2cDriver::new(i2c, gsc_ready.is_some())?) } }; bus.init()?;