From 03862314eba9e453c8c89ccb036cca9afc55ddbc Mon Sep 17 00:00:00 2001
From: Wei Liu <liuwe@microsoft.com>
Date: Wed, 17 Nov 2021 13:24:13 +0000
Subject: [PATCH] arch: x86_64: add safety comments for `impl ByteValued`

Group impl clauses together to avoid repetition.

Signed-off-by: Wei Liu <liuwe@microsoft.com>
---
 arch/src/x86_64/mod.rs     | 10 ++++------
 arch/src/x86_64/mptable.rs |  2 +-
 arch/src/x86_64/smbios.rs  |  6 +++---
 arch/src/x86_64/tdx/mod.rs | 13 +++++++------
 4 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/arch/src/x86_64/mod.rs b/arch/src/x86_64/mod.rs
index 8ceed06c5..ce39829cb 100644
--- a/arch/src/x86_64/mod.rs
+++ b/arch/src/x86_64/mod.rs
@@ -118,17 +118,15 @@ impl SgxEpcRegion {
 #[derive(Copy, Clone, Default)]
 struct StartInfoWrapper(hvm_start_info);
 
-// It is safe to initialize StartInfoWrapper which is a wrapper over `hvm_start_info` (a series of ints).
-unsafe impl ByteValued for StartInfoWrapper {}
-
 #[derive(Copy, Clone, Default)]
 struct MemmapTableEntryWrapper(hvm_memmap_table_entry);
 
-unsafe impl ByteValued for MemmapTableEntryWrapper {}
-
 #[derive(Copy, Clone, Default)]
 struct ModlistEntryWrapper(hvm_modlist_entry);
 
+// SAFETY: These data structures only contain a series of integers
+unsafe impl ByteValued for StartInfoWrapper {}
+unsafe impl ByteValued for MemmapTableEntryWrapper {}
 unsafe impl ByteValued for ModlistEntryWrapper {}
 
 // This is a workaround to the Rust enforcement specifying that any implementation of a foreign
@@ -139,7 +137,7 @@ unsafe impl ByteValued for ModlistEntryWrapper {}
 #[derive(Copy, Clone, Default)]
 struct BootParamsWrapper(boot_params);
 
-// It is safe to initialize BootParamsWrap which is a wrapper over `boot_params` (a series of ints).
+// SAFETY: BootParamsWrap is a wrapper over `boot_params` (a series of ints).
 unsafe impl ByteValued for BootParamsWrapper {}
 
 #[derive(Debug)]
diff --git a/arch/src/x86_64/mptable.rs b/arch/src/x86_64/mptable.rs
index fe6383da5..c7903e232 100644
--- a/arch/src/x86_64/mptable.rs
+++ b/arch/src/x86_64/mptable.rs
@@ -35,7 +35,7 @@ struct MpcLintsrcWrapper(mpspec::mpc_lintsrc);
 #[derive(Copy, Clone, Default)]
 struct MpfIntelWrapper(mpspec::mpf_intel);
 
-// These `mpspec` wrapper types are only data, reading them from data is a safe initialization.
+// SAFETY: These `mpspec` wrapper types are only data, reading them from data is a safe initialization.
 unsafe impl ByteValued for MpcBusWrapper {}
 unsafe impl ByteValued for MpcCpuWrapper {}
 unsafe impl ByteValued for MpcIntsrcWrapper {}
diff --git a/arch/src/x86_64/smbios.rs b/arch/src/x86_64/smbios.rs
index 4ad8af07c..2edc49481 100644
--- a/arch/src/x86_64/smbios.rs
+++ b/arch/src/x86_64/smbios.rs
@@ -82,7 +82,6 @@ pub struct Smbios30Entrypoint {
     pub max_size: u32,
     pub physptr: u64,
 }
-unsafe impl ByteValued for Smbios30Entrypoint {}
 
 impl Clone for Smbios30Entrypoint {
     fn clone(&self) -> Self {
@@ -112,8 +111,6 @@ impl Clone for SmbiosBiosInfo {
     }
 }
 
-unsafe impl ByteValued for SmbiosBiosInfo {}
-
 #[repr(packed)]
 #[derive(Default, Copy)]
 pub struct SmbiosSysInfo {
@@ -136,6 +133,9 @@ impl Clone for SmbiosSysInfo {
     }
 }
 
+// SAFETY: These data structures only contain a series of integers
+unsafe impl ByteValued for Smbios30Entrypoint {}
+unsafe impl ByteValued for SmbiosBiosInfo {}
 unsafe impl ByteValued for SmbiosSysInfo {}
 
 fn write_and_incr<T: ByteValued>(
diff --git a/arch/src/x86_64/tdx/mod.rs b/arch/src/x86_64/tdx/mod.rs
index ee6f0b243..1e0831e00 100644
--- a/arch/src/x86_64/tdx/mod.rs
+++ b/arch/src/x86_64/tdx/mod.rs
@@ -69,8 +69,6 @@ pub struct TdVmmDataRegion {
     pub region_type: TdVmmDataRegionType,
 }
 
-unsafe impl ByteValued for TdVmmDataRegion {}
-
 #[repr(u16)]
 #[derive(Clone, Copy, Debug)]
 pub enum TdVmmDataRegionType {
@@ -192,7 +190,6 @@ struct HobHeader {
     length: u16,
     reserved: u32,
 }
-unsafe impl ByteValued for HobHeader {}
 
 #[repr(C)]
 #[derive(Copy, Clone, Default, Debug)]
@@ -206,7 +203,6 @@ struct HobHandoffInfoTable {
     efi_free_memory_bottom: u64,
     efi_end_of_hob_list: u64,
 }
-unsafe impl ByteValued for HobHandoffInfoTable {}
 
 #[repr(C)]
 #[derive(Copy, Clone, Default, Debug)]
@@ -227,7 +223,6 @@ struct HobResourceDescriptor {
     physical_start: u64,
     resource_length: u64,
 }
-unsafe impl ByteValued for HobResourceDescriptor {}
 
 #[repr(C)]
 #[derive(Copy, Clone, Default, Debug)]
@@ -235,7 +230,6 @@ struct HobGuidType {
     header: HobHeader,
     name: EfiGuid,
 }
-unsafe impl ByteValued for HobGuidType {}
 
 #[repr(C)]
 #[derive(Copy, Clone, Default, Debug)]
@@ -243,6 +237,13 @@ struct TdVmmData {
     guid_type: HobGuidType,
     region: TdVmmDataRegion,
 }
+
+// SAFETY: These data structures only contain a series of integers
+unsafe impl ByteValued for TdVmmDataRegion {}
+unsafe impl ByteValued for HobHeader {}
+unsafe impl ByteValued for HobHandoffInfoTable {}
+unsafe impl ByteValued for HobResourceDescriptor {}
+unsafe impl ByteValued for HobGuidType {}
 unsafe impl ByteValued for TdVmmData {}
 
 pub struct TdHob {