Skip to content

Commit f4afa2f

Browse files
Fix: Update Commit structures to accurately reflect TCBs
When we updated the TCB structure, we overlooked modifying the commit-related structures to account for differences in TCB layout across CPU generations. This commit ensures that raw TCB bytes are correctly constructed for the FFI layer, and that conversions from the user-level structure to the FFI format use the appropriate TCB structure based on the processor generation. The API snp_set_config function remains the same, it will tell the CPU generation using a CPUID call. Signed-off-by: DGonzalezVillal <Diego.GonzalezVillalobos@amd.com>
1 parent f7f59c6 commit f4afa2f

4 files changed

Lines changed: 167 additions & 19 deletions

File tree

src/firmware/host/mod.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use std::convert::TryInto;
4242
use super::linux::host::types::SnpCommit;
4343

4444
#[cfg(all(target_os = "linux", feature = "snp"))]
45-
use super::linux::host::types::SnpPlatformStatus as FFISnpPlatformStatus;
45+
use super::linux::host::types::{SnpPlatformStatus as FFISnpPlatformStatus, SnpSetConfig};
4646

4747
/// The CPU-unique identifier for the platform.
4848
#[derive(Clone, Debug, PartialEq, Eq)]
@@ -267,8 +267,16 @@ impl Firmware {
267267
/// let status: bool = firmware.snp_set_config(configuration).unwrap();
268268
/// ```
269269
#[cfg(feature = "snp")]
270-
pub fn snp_set_config(&mut self, new_config: Config) -> Result<(), UserApiError> {
271-
let mut binding = new_config.try_into()?;
270+
pub fn snp_set_config(
271+
&mut self,
272+
new_config: Config,
273+
generation: Option<Generation>,
274+
) -> Result<(), UserApiError> {
275+
let mut binding: SnpSetConfig = match generation {
276+
Some(gen) => (new_config, gen).try_into()?,
277+
None => new_config.try_into()?,
278+
};
279+
272280
let mut cmd_buf = Command::from_mut(&mut binding);
273281

274282
SNP_SET_CONFIG

src/firmware/host/types/snp.rs

Lines changed: 141 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -377,26 +377,76 @@ impl Config {
377377
}
378378

379379
#[cfg(feature = "snp")]
380+
///TryFrom to FFI Config when manually passing in the CPU generation
381+
impl TryFrom<(Config, Generation)> for FFI::types::SnpSetConfig {
382+
type Error = std::io::Error;
383+
384+
fn try_from(args: (Config, Generation)) -> Result<Self, Self::Error> {
385+
let mut snp_config: SnpSetConfig = Default::default();
386+
let (value, generation) = args;
387+
388+
let mut buffer = Vec::new();
389+
write_tcb(&mut buffer, &value.reported_tcb, &generation)?;
390+
snp_config.reported_tcb = buffer.try_into().map_err(|_| {
391+
std::io::Error::new(
392+
std::io::ErrorKind::InvalidData,
393+
"Failed to convert TCB into bytes",
394+
)
395+
})?;
396+
snp_config.mask_id = value.mask_id;
397+
398+
Ok(snp_config)
399+
}
400+
}
401+
402+
#[cfg(feature = "snp")]
403+
/// TryFrom to FFI Config type when CPU Generation is unknown
380404
impl TryFrom<Config> for FFI::types::SnpSetConfig {
381-
type Error = uuid::Error;
405+
type Error = std::io::Error;
382406

383407
fn try_from(value: Config) -> Result<Self, Self::Error> {
384408
let mut snp_config: SnpSetConfig = Default::default();
385-
386-
snp_config.reported_tcb = value.reported_tcb;
409+
let generation = Generation::identify_host_generation()?;
410+
411+
let mut buffer = Vec::new();
412+
write_tcb(&mut buffer, &value.reported_tcb, &generation)?;
413+
snp_config.reported_tcb = buffer.try_into().map_err(|_| {
414+
std::io::Error::new(
415+
std::io::ErrorKind::InvalidData,
416+
"Failed to convert TCB into bytes",
417+
)
418+
})?;
387419
snp_config.mask_id = value.mask_id;
388420

389421
Ok(snp_config)
390422
}
391423
}
392424

393425
#[cfg(feature = "snp")]
426+
/// TryFrom from FFI Config type when CPU Generation is manually passed in
427+
impl TryFrom<(FFI::types::SnpSetConfig, Generation)> for Config {
428+
type Error = std::io::Error;
429+
430+
fn try_from(value: (FFI::types::SnpSetConfig, Generation)) -> Result<Self, Self::Error> {
431+
let reported_tcb = parse_tcb(&mut value.0.reported_tcb.as_slice(), &value.1)?;
432+
Ok(Self {
433+
reported_tcb,
434+
mask_id: value.0.mask_id,
435+
..Default::default()
436+
})
437+
}
438+
}
439+
440+
#[cfg(feature = "snp")]
441+
/// TryFrom from FFI Config type when CPU Generation is unknown
394442
impl TryFrom<FFI::types::SnpSetConfig> for Config {
395-
type Error = uuid::Error;
443+
type Error = std::io::Error;
396444

397445
fn try_from(value: FFI::types::SnpSetConfig) -> Result<Self, Self::Error> {
446+
let generation = Generation::identify_host_generation()?;
447+
let reported_tcb = parse_tcb(&mut value.reported_tcb.as_slice(), &generation)?;
398448
Ok(Self {
399-
reported_tcb: value.reported_tcb,
449+
reported_tcb,
400450
mask_id: value.mask_id,
401451
..Default::default()
402452
})
@@ -938,8 +988,8 @@ mod tests {
938988
assert_eq!(config_mask, mask);
939989

940990
// Test conversion to FFI type
941-
let snp_config: SnpSetConfig = config.try_into().unwrap();
942-
assert_eq!(snp_config.reported_tcb, tcb);
991+
let snp_config: SnpSetConfig = (config, Generation::Milan).try_into().unwrap();
992+
assert_eq!(snp_config.reported_tcb, tcb.to_legacy_bytes());
943993
let snp_config_mask = snp_config.mask_id;
944994

945995
assert_eq!(snp_config_mask, mask);
@@ -1031,12 +1081,12 @@ mod tests {
10311081
let mask = MaskId(0x3);
10321082
let config = Config::new(tcb, mask);
10331083

1034-
let ffi_config: SnpSetConfig = config.try_into().unwrap();
1035-
assert_eq!(ffi_config.reported_tcb, tcb);
1084+
let ffi_config: SnpSetConfig = (config, Generation::Milan).try_into().unwrap();
1085+
assert_eq!(ffi_config.reported_tcb, tcb.to_legacy_bytes());
10361086
let ffi_config_mask = ffi_config.mask_id;
10371087
assert_eq!(ffi_config_mask, mask);
10381088

1039-
let converted_config: Config = ffi_config.try_into().unwrap();
1089+
let converted_config: Config = (ffi_config, Generation::Milan).try_into().unwrap();
10401090
assert_eq!(converted_config.reported_tcb, tcb);
10411091
let converted_config_mask = converted_config.mask_id;
10421092
assert_eq!(converted_config_mask, mask);
@@ -1077,7 +1127,7 @@ mod tests {
10771127
let mask = MaskId(u32::MAX);
10781128
let config = Config::new(tcb, mask);
10791129

1080-
let ffi_result: Result<SnpSetConfig, _> = config.try_into();
1130+
let ffi_result: Result<SnpSetConfig, _> = (config, Generation::Milan).try_into();
10811131
assert!(ffi_result.is_ok());
10821132

10831133
let default_config = Config::default();
@@ -1086,6 +1136,86 @@ mod tests {
10861136
assert_eq!(default_config_mask_id, Default::default());
10871137
}
10881138

1139+
#[test]
1140+
#[cfg(feature = "snp")]
1141+
fn test_config_edge_cases() {
1142+
// Test with maximum values
1143+
let tcb = TcbVersion::new(Some(255), 255, 255, 255, 255);
1144+
let mask_id = MaskId(u32::MAX);
1145+
let config = Config::new(tcb, mask_id);
1146+
1147+
// Convert to SnpSetConfig
1148+
let result: Result<SnpSetConfig, _> = (config, Generation::Turin).try_into();
1149+
assert!(result.is_ok());
1150+
let snp_config = result.unwrap();
1151+
1152+
// Convert back to Config
1153+
let result: Result<Config, _> = (snp_config, Generation::Turin).try_into();
1154+
assert!(result.is_ok());
1155+
let round_trip = result.unwrap();
1156+
1157+
assert_eq!(round_trip.reported_tcb, tcb);
1158+
let round_trip_mask_id = round_trip.mask_id;
1159+
assert_eq!(round_trip_mask_id, mask_id);
1160+
1161+
// Test with minimum values
1162+
let tcb = TcbVersion::new(Some(0), 0, 0, 0, 0);
1163+
let mask_id = MaskId(0);
1164+
let config = Config::new(tcb, mask_id);
1165+
1166+
// Convert to SnpSetConfig
1167+
let result: Result<SnpSetConfig, _> = (config, Generation::Turin).try_into();
1168+
assert!(result.is_ok());
1169+
let snp_config = result.unwrap();
1170+
1171+
// Convert back to Config
1172+
let result: Result<Config, _> = (snp_config, Generation::Turin).try_into();
1173+
assert!(result.is_ok());
1174+
let round_trip = result.unwrap();
1175+
1176+
assert_eq!(round_trip.reported_tcb, tcb);
1177+
let round_trip_mask_id = round_trip.mask_id;
1178+
assert_eq!(round_trip_mask_id, mask_id);
1179+
}
1180+
1181+
#[test]
1182+
#[cfg(feature = "snp")]
1183+
fn test_different_generation_conversions() {
1184+
let tcb = TcbVersion::new(Some(1), 2, 3, 4, 5);
1185+
let mask_id = MaskId(0x3);
1186+
let config = Config::new(tcb, mask_id);
1187+
1188+
// Test all generations
1189+
let generations = [Generation::Milan, Generation::Genoa, Generation::Turin];
1190+
1191+
for generation in generations {
1192+
// Convert to SnpSetConfig
1193+
let snp_config: Result<SnpSetConfig, _> = (config, generation).try_into();
1194+
assert!(snp_config.is_ok());
1195+
let snp_config = snp_config.unwrap();
1196+
1197+
// Convert back to Config
1198+
let round_trip: Result<Config, _> = (snp_config, generation).try_into();
1199+
assert!(round_trip.is_ok());
1200+
let round_trip = round_trip.unwrap();
1201+
1202+
// For non-Turin generations, FMC will be lost in the conversion
1203+
match generation {
1204+
Generation::Turin => assert_eq!(round_trip.reported_tcb, tcb),
1205+
_ => {
1206+
// FMC field is not preserved for legacy generations
1207+
assert_eq!(round_trip.reported_tcb.bootloader, tcb.bootloader);
1208+
assert_eq!(round_trip.reported_tcb.tee, tcb.tee);
1209+
assert_eq!(round_trip.reported_tcb.snp, tcb.snp);
1210+
assert_eq!(round_trip.reported_tcb.microcode, tcb.microcode);
1211+
assert_eq!(round_trip.reported_tcb.fmc, None); // FMC lost in legacy format
1212+
}
1213+
}
1214+
let round_trip_mask_id = round_trip.mask_id;
1215+
assert_eq!(round_trip_mask_id, mask_id);
1216+
}
1217+
}
1218+
10891219
#[test]
10901220
fn test_version_comparisons() {
10911221
let v1 = TcbVersion::new(None, 1, 2, 3, 4);

src/firmware/linux/host/types/snp.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,8 +229,8 @@ pub struct SnpCommit {
229229
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
230230
#[repr(C, packed)]
231231
pub struct SnpSetConfig {
232-
/// The TCB_VERSION to report in guest attestation reports.
233-
pub reported_tcb: UAPI::TcbVersion,
232+
/// The bytes corresponding to the TCB_VERSION to report in guest attestation reports.
233+
pub reported_tcb: [u8; 8],
234234

235235
/// mask_id [0] : whether chip id is present in attestation reports or not
236236
/// mask_id [1]: whether attestation reports are signed or not

tests/api.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,10 @@ mod sev {
119119
#[cfg(all(feature = "snp", target_os = "linux"))]
120120
mod snp {
121121
use serial_test::serial;
122-
use sev::firmware::host::{Config, Firmware, MaskId, SnpPlatformStatus, TcbVersion};
122+
use sev::{
123+
firmware::host::{Config, Firmware, MaskId, SnpPlatformStatus, TcbVersion},
124+
Generation,
125+
};
123126

124127
#[cfg_attr(not(host), ignore)]
125128
#[test]
@@ -178,7 +181,10 @@ mod snp {
178181
#[serial]
179182
fn set_config() {
180183
let mut fw: Firmware = Firmware::open().unwrap();
181-
fw.snp_set_config(Config::default()).unwrap();
184+
185+
let generation = Generation::identify_host_generation().unwrap();
186+
187+
fw.snp_set_config(Config::default(), generation).unwrap();
182188
}
183189

184190
#[cfg_attr(not(all(host, feature = "dangerous_hw_tests")), ignore)]
@@ -187,7 +193,11 @@ mod snp {
187193
fn test_host_fw_error() {
188194
let mut fw: Firmware = Firmware::open().unwrap();
189195
let invalid_config = Config::new(TcbVersion::new(None, 100, 100, 100, 100), MaskId(31));
190-
let fw_error = fw.snp_set_config(invalid_config).unwrap_err().to_string();
196+
let generation = Generation::identify_host_generation().unwrap();
197+
let fw_error = fw
198+
.snp_set_config(invalid_config, generation)
199+
.unwrap_err()
200+
.to_string();
191201
assert_eq!(fw_error, "Firmware Error Encountered: Known SEV FW Error: Status Code: 0x16: Given parameter is invalid.")
192202
}
193203
}

0 commit comments

Comments
 (0)