From 2d71bed2dc71496d5f6265efd4a0cac7f6ea4374 Mon Sep 17 00:00:00 2001 From: "Jes B. Klinke" Date: Tue, 19 Nov 2024 13:15:01 -0800 Subject: [PATCH] [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 6c9f8ffc708b5f..0ea33f8b5ff7bc 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 f735ef4fd9625c..8ab2ae3b4d2a05 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 68111f429e63a4..6ec13f4466407c 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 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()?;