From 816e3bfd3ac1a347d2208d82234776fc9c0b0789 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Thu, 31 Oct 2024 14:32:13 +0100 Subject: [PATCH 1/3] tdx-tdcall: don't use heap allocated buffers for TD reports There's little reason to use a heap-allocated buffer for requesting TD reports putting it behind a global lock. Furthermore the TDX module doesn't require the additional data to be inside the TD report buffer. Instead, put the buffer on the stack and directly pass a pointer to the additional data. Note that with and without this patch, the memory passed to the TDX module must be identity-mapped. Signed-off-by: Tom Dohrmann --- tdx-tdcall/src/lib.rs | 1 - tdx-tdcall/src/tdreport.rs | 93 +++----------------------------------- 2 files changed, 6 insertions(+), 88 deletions(-) diff --git a/tdx-tdcall/src/lib.rs b/tdx-tdcall/src/lib.rs index 31ab3ac4..a4b572e6 100644 --- a/tdx-tdcall/src/lib.rs +++ b/tdx-tdcall/src/lib.rs @@ -21,7 +21,6 @@ #![no_std] -extern crate alloc; use core::ffi::c_void; #[cfg(feature = "use_tdx_emulation")] diff --git a/tdx-tdcall/src/tdreport.rs b/tdx-tdcall/src/tdreport.rs index c1448315..47bf6c60 100644 --- a/tdx-tdcall/src/tdreport.rs +++ b/tdx-tdcall/src/tdreport.rs @@ -2,22 +2,16 @@ // // SPDX-License-Identifier: BSD-2-Clause-Patent -use core::alloc::Layout; use core::fmt; use core::mem::{size_of, zeroed}; use core::ptr::{slice_from_raw_parts, slice_from_raw_parts_mut}; -use lazy_static::lazy_static; use scroll::{Pread, Pwrite}; -use spin::Mutex; use crate::{td_call, TdCallError, TdcallArgs, TDCALL_TDREPORT, TDVMCALL_STATUS_SUCCESS}; pub const TD_REPORT_SIZE: usize = 0x400; pub const TD_REPORT_ADDITIONAL_DATA_SIZE: usize = 64; -// The buffer to td_report() must be aligned to TD_REPORT_SIZE. -const TD_REPORT_BUFF_SIZE: usize = TD_REPORT_SIZE + TD_REPORT_ADDITIONAL_DATA_SIZE; - #[repr(C)] #[derive(Debug, Pread, Pwrite, Clone, Copy)] pub struct ReportType { @@ -189,62 +183,8 @@ impl Default for TdxReport { } } -/// Struct to fulfill the alignment requirement of buffer for td-report tdcall. -struct TdxReportBuf { - start: u64, -} - -impl TdxReportBuf { - fn new() -> Self { - // Safety: align is a power of two - let start = unsafe { - alloc::alloc::alloc_zeroed(Layout::from_size_align_unchecked( - TD_REPORT_BUFF_SIZE, - TD_REPORT_SIZE, - )) - } as u64; - Self { start } - } - - fn report_buf_start(&mut self) -> u64 { - self.start - } - - fn report_buf(&self) -> &[u8] { - unsafe { core::slice::from_raw_parts(self.start as *const u8, TD_REPORT_SIZE) } - } - - fn additional_buf_mut(&mut self) -> &mut [u8] { - unsafe { - core::slice::from_raw_parts_mut( - (self.start + TD_REPORT_SIZE as u64) as *mut u8, - TD_REPORT_ADDITIONAL_DATA_SIZE, - ) - } - } - - fn to_owned(&self) -> TdxReport { - let mut report: TdxReport = TdxReport::default(); - report.as_bytes_mut().copy_from_slice(self.report_buf()); - report - } -} - -impl Drop for TdxReportBuf { - fn drop(&mut self) { - unsafe { - // Safety: align is a power of two - alloc::alloc::dealloc( - self.start as *mut u8, - Layout::from_size_align_unchecked(TD_REPORT_BUFF_SIZE, TD_REPORT_SIZE), - ) - } - } -} - -lazy_static! { - static ref TD_REPORT: Mutex = Mutex::new(TdxReportBuf::new()); -} +#[repr(C, align(1024))] +struct TdxReportBuf(TdxReport); /// Create a TDREPORT_STRUCT structure that contains the measurements/configuration /// information of the guest TD, measurements/configuration information of the Intel @@ -254,15 +194,12 @@ lazy_static! { pub fn tdcall_report( additional_data: &[u8; TD_REPORT_ADDITIONAL_DATA_SIZE], ) -> Result { - let mut buff = TD_REPORT.lock(); - - let addr = buff.report_buf_start(); - buff.additional_buf_mut().copy_from_slice(additional_data); + let mut buf = TdxReportBuf(TdxReport::default()); let mut args = TdcallArgs { rax: TDCALL_TDREPORT, - rcx: addr, - rdx: addr.checked_add(TD_REPORT_SIZE as u64).unwrap(), + rcx: &mut buf as *mut _ as u64, + rdx: additional_data.as_ptr() as u64, ..Default::default() }; @@ -271,7 +208,7 @@ pub fn tdcall_report( return Err(args.r10.into()); } - Ok(buff.to_owned()) + Ok(buf.0) } #[cfg(test)] @@ -282,22 +219,4 @@ mod tests { fn test_tdx_report_size() { assert_eq!(size_of::(), 0x400); } - - #[test] - fn test_tdx_report_buf() { - let mut buf = TdxReportBuf::new(); - assert_eq!(buf.report_buf_start() & 0x3ff, 0); - - let additional = buf.additional_buf_mut().as_ptr() as u64; - assert_eq!(buf.report_buf_start() + 0x400, additional); - - assert_eq!(TD_REPORT.lock().report_buf_start() & 0x3ff, 0); - } - - #[test] - fn test_to_owned() { - let buff = TdxReportBuf::new(); - - buff.to_owned(); - } } From 820a44d98a4e34b843340935c210df4258b493fd Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Thu, 31 Oct 2024 14:42:19 +0100 Subject: [PATCH 2/3] tdx-tdcall: fix comment Signed-off-by: Tom Dohrmann --- tdx-tdcall/src/tdreport.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tdx-tdcall/src/tdreport.rs b/tdx-tdcall/src/tdreport.rs index 47bf6c60..d2abea10 100644 --- a/tdx-tdcall/src/tdreport.rs +++ b/tdx-tdcall/src/tdreport.rs @@ -190,7 +190,7 @@ struct TdxReportBuf(TdxReport); /// information of the guest TD, measurements/configuration information of the Intel /// TDX module and a REPORTMACSTRUCT /// -/// Details can be found in TDX GHCI spec section 'TDG.MR.REPORT' +/// Details can be found in TDX module ABI spec section 'TDG.MR.REPORT' pub fn tdcall_report( additional_data: &[u8; TD_REPORT_ADDITIONAL_DATA_SIZE], ) -> Result { From 8a975373d0d2c144404aa3197838eec74ce7ff20 Mon Sep 17 00:00:00 2001 From: Jiaqi Gao Date: Wed, 11 Dec 2024 01:50:46 -0500 Subject: [PATCH 3/3] tdx-tdcall: align the additional data of report Add a wrapper struct to hold the additional data to meet the 64B alignment. Signed-off-by: Jiaqi Gao --- tdx-tdcall/src/tdreport.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tdx-tdcall/src/tdreport.rs b/tdx-tdcall/src/tdreport.rs index d2abea10..87470eab 100644 --- a/tdx-tdcall/src/tdreport.rs +++ b/tdx-tdcall/src/tdreport.rs @@ -186,6 +186,9 @@ impl Default for TdxReport { #[repr(C, align(1024))] struct TdxReportBuf(TdxReport); +#[repr(C, align(64))] +struct AdditionalDataBuf([u8; TD_REPORT_ADDITIONAL_DATA_SIZE]); + /// Create a TDREPORT_STRUCT structure that contains the measurements/configuration /// information of the guest TD, measurements/configuration information of the Intel /// TDX module and a REPORTMACSTRUCT @@ -194,12 +197,13 @@ struct TdxReportBuf(TdxReport); pub fn tdcall_report( additional_data: &[u8; TD_REPORT_ADDITIONAL_DATA_SIZE], ) -> Result { - let mut buf = TdxReportBuf(TdxReport::default()); + let mut report_buf = TdxReportBuf(TdxReport::default()); + let additional_data_buf = AdditionalDataBuf(*additional_data); let mut args = TdcallArgs { rax: TDCALL_TDREPORT, - rcx: &mut buf as *mut _ as u64, - rdx: additional_data.as_ptr() as u64, + rcx: &mut report_buf as *mut _ as u64, + rdx: &additional_data_buf as *const _ as u64, ..Default::default() }; @@ -208,7 +212,7 @@ pub fn tdcall_report( return Err(args.r10.into()); } - Ok(buf.0) + Ok(report_buf.0) } #[cfg(test)]