From fe3d46805de22d900ff8fe2312406f848d7774ed Mon Sep 17 00:00:00 2001 From: "Jes B. Klinke" Date: Tue, 19 Nov 2024 13:15:01 -0800 Subject: [PATCH] WIP: TPM optimizations Change-Id: I16c56d367478c1394825b1be56a3622452af207a --- sw/host/opentitanlib/src/tpm/driver.rs | 147 +++++++++++------------ sw/host/opentitantool/src/command/i2c.rs | 12 +- sw/host/opentitantool/src/command/spi.rs | 12 +- sw/host/tpm2_test_server/src/main.rs | 18 ++- 4 files changed, 91 insertions(+), 98 deletions(-) diff --git a/sw/host/opentitanlib/src/tpm/driver.rs b/sw/host/opentitanlib/src/tpm/driver.rs index 6c9f8ffc708b5f..d8907f1ef1ea4d 100644 --- a/sw/host/opentitanlib/src/tpm/driver.rs +++ b/sw/host/opentitanlib/src/tpm/driver.rs @@ -16,6 +16,7 @@ use crate::io::i2c; use crate::io::spi; use crate::tpm::access::TpmAccess; use crate::tpm::status::TpmStatus; +use crate::transport::TransportError; /// Tpm registers, can be specified in command line arguments. #[allow(non_camel_case_types)] @@ -213,20 +214,20 @@ fn wait_for_gsc_ready(gsc_ready_pin: &Option) -> Result<() /// 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)>, + use_gsc_ready: bool, ) -> Result { - if let Some((gsc_ready_pin, monitoring)) = &gsc_ready_pin { + /*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 }) + }*/ + Ok(Self { spi, use_gsc_ready }) } /// Numerical TPM register address as used in SPI protocol. @@ -311,76 +312,76 @@ 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()? { + return self.do_read_register(register, data); + } + let req = self.compose_header(register, data.len(), true /* is_read */); + let res = 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), + ]) + }; + if res.is_err() { + bail!(TpmError::Timeout); } - result + Ok(()) } 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()? { + return self.do_write_register(register, data); + } + let req = self.compose_header(register, data.len(), false /* is_read */); + match 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), + ]) + } { + Ok(()) => (), + Err(error) => { + if let Some(fisk) = error.downcast_ref::() { + if let TransportError::CommunicationError(_) = fisk { + // Check that the actual error is "timeout" + bail!(TpmError::Timeout); + } + } + bail!(error); + } } + Ok(()) } } /// 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)>, + use_gsc_ready: bool, ) -> 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 }) + Ok(Self { i2c, use_gsc_ready }) } /// Numerical TPM register address as used in Google I2C protocol. @@ -395,7 +396,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 +406,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 +454,17 @@ 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], + )?; } + Ok(()) } } diff --git a/sw/host/opentitantool/src/command/i2c.rs b/sw/host/opentitantool/src/command/i2c.rs index f735ef4fd9625c..ef3292120506c2 100644 --- a/sw/host/opentitantool/src/command/i2c.rs +++ b/sw/host/opentitantool/src/command/i2c.rs @@ -274,13 +274,13 @@ 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 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( - context.params.create(transport, "TPM")?, - ready_pin, + 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 68111f429e63a4..182acb56b4fa81 100644 --- a/sw/host/opentitantool/src/command/spi.rs +++ b/sw/host/opentitantool/src/command/spi.rs @@ -294,13 +294,13 @@ 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 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( - context.params.create(transport, "TPM")?, - ready_pin, + 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 2dd4f53f531cb6..c2e8c616a4a077 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()?;