refactor(vmm): Better error messages for CPUID normalization

Show which combination of subleaf, register and bit(s) failed to set,
remove unused error variant and reorder error variants in alphabetical
order.

Signed-off-by: Takahiro Itazuri <[email protected]>

Author: zulinx86

src/vmm/src/cpu_config/x86_64/cpuid/amd/normalize.rs

@@ -48,33 +48,35 @@ pub enum PassthroughCacheTopologyError {
 pub enum FeatureEntryError {
     /// Missing leaf 0x80000008.
     MissingLeaf0x80000008,
-    /// Failed to set `nt` (number of physical threads) due to overflow.
-    NumberOfPhysicalThreadsOverflow,
-    /// Failed to set `nt` (number of physical threads).
+    /// Failed to set number of physical threads (CPUID.80000008H:ECX[7:0]): {0}
     NumberOfPhysicalThreads(CheckedAssignError),
+    /// Failed to set number of physical threads (CPUID.80000008H:ECX[7:0]) due to overflow.
+    NumberOfPhysicalThreadsOverflow,
 }
 
 /// Error type for setting leaf 0x8000001d section of [`super::AmdCpuid::normalize`].
 #[derive(Debug, thiserror::Error, displaydoc::Display, Eq, PartialEq)]
 pub enum ExtendedCacheTopologyError {
     /// Missing leaf 0x8000001d.
     MissingLeaf0x8000001d,
-    /// Failed to set `num_sharing_cache` due to overflow.
-    NumSharingCacheOverflow,
-    /// Failed to set `num_sharing_cache`: {0}
-    NumSharingCache(CheckedAssignError),
+    #[rustfmt::skip]
+    /// Failed to set number of logical processors sharing cache(CPUID.(EAX=8000001DH,ECX={0}):EAX[25:14]): {1}
+    NumSharingCache(u32, CheckedAssignError),
+    #[rustfmt::skip]
+    /// Failed to set number of logical processors sharing cache (CPUID.(EAX=8000001DH,ECX={0}):EAX[25:14]) due to overflow.
+    NumSharingCacheOverflow(u32),
 }
 
 /// Error type for setting leaf 0x8000001e section of [`super::AmdCpuid::normalize`].
 #[derive(Debug, thiserror::Error, displaydoc::Display, Eq, PartialEq)]
 pub enum ExtendedApicIdError {
+    /// Failed to set compute unit ID (CPUID.8000001EH:EBX[7:0]): {0}
+    ComputeUnitId(CheckedAssignError),
+    /// Failed to set extended APIC ID (CPUID.8000001EH:EAX[31:0]): {0}
+    ExtendedApicId(CheckedAssignError),
     /// Missing leaf 0x8000001e.
     MissingLeaf0x8000001e,
-    /// Failed to set `extended_apic_id`: {0}
-    ExtendedApicId(CheckedAssignError),
-    /// Failed to set `compute_unit_id`: {0}
-    ComputeUnitId(CheckedAssignError),
-    /// Failed to set `threads_per_compute_unit`: {0}
+    /// Failed to set threads per core unit (CPUID:8000001EH:EBX[15:8]): {0}
     ThreadPerComputeUnit(CheckedAssignError),
 }
 
@@ -282,16 +284,16 @@ impl super::AmdCpuid {
                         // SAFETY: We know `cpus_per_core > 0` therefore this is always safe.
                         let sub = u32::from(cpus_per_core.checked_sub(1).unwrap());
                         set_range(&mut subleaf.result.eax, 14..26, sub)
-                            .map_err(ExtendedCacheTopologyError::NumSharingCache)?;
+                            .map_err(|err| ExtendedCacheTopologyError::NumSharingCache(i, err))?;
                     }
                     // L3 Cache
                     // The L3 cache is shared among all the logical threads
                     3 => {
                         let sub = cpu_count
                             .checked_sub(1)
-                            .ok_or(ExtendedCacheTopologyError::NumSharingCacheOverflow)?;
+                            .ok_or(ExtendedCacheTopologyError::NumSharingCacheOverflow(i))?;
                         set_range(&mut subleaf.result.eax, 14..26, u32::from(sub))
-                            .map_err(ExtendedCacheTopologyError::NumSharingCache)?;
+                            .map_err(|err| ExtendedCacheTopologyError::NumSharingCache(i, err))?;
                     }
                     _ => (),
                 }

src/vmm/src/cpu_config/x86_64/cpuid/intel/normalize.rs

@@ -34,14 +34,14 @@ pub enum NormalizeCpuidError {
 #[allow(clippy::enum_variant_names)]
 #[derive(Debug, thiserror::Error, displaydoc::Display, Eq, PartialEq)]
 pub enum DeterministicCacheError {
-    /// Failed to set `Maximum number of addressable IDs for logical processors sharing this cache` due to underflow in cpu count.
-    MaxCpusPerCoreUnderflow,
-    /// Failed to set `Maximum number of addressable IDs for logical processors sharing this cache`: {0}.
-    MaxCpusPerCore(CheckedAssignError),
-    /// Failed to set `Maximum number of addressable IDs for processor cores in the physical package` due to underflow in cores.
-    MaxCorePerPackageUnderflow,
-    /// Failed to set `Maximum number of addressable IDs for processor cores in the physical package`: {0}.
+    /// Failed to set max addressable core ID in physical package (CPUID.04H:EAX[31:26]): {0}.
     MaxCorePerPackage(CheckedAssignError),
+    /// Failed to set max addressable core ID in physical package (CPUID.04H:EAX[31:26]) due to underflow in cores.
+    MaxCorePerPackageUnderflow,
+    /// Failed to set max addressable processor ID sharing cache (CPUID.04H:EAX[25:14]): {0}.
+    MaxCpusPerCore(CheckedAssignError),
+    /// Failed to set max addressable processor ID sharing cache (CPUID.04H:EAX[25:14]) due to underflow in cpu count.
+    MaxCpusPerCoreUnderflow,
 }
 
 /// We always use this brand string.

src/vmm/src/cpu_config/x86_64/cpuid/normalize.rs

@@ -61,16 +61,14 @@ pub enum GetMaxCpusPerPackageError {
 #[rustfmt::skip]
 #[derive(Debug, thiserror::Error, displaydoc::Display, Eq, PartialEq)]
 pub enum ExtendedTopologyError {
-    /// Failed to set `Number of bits to shift right on x2APIC ID to get a unique topology ID of the next level type`: {0}
-    ApicId(CheckedAssignError),
-    /// Failed to set `Number of logical processors at this level type`: {0}
-    LogicalProcessors(CheckedAssignError),
-    /// Failed to set `Domain Type`: {0}
-    DomainType(CheckedAssignError),
-    /// Failed to set `Input ECX`: {0}
-    InputEcx(CheckedAssignError),
-    /// Failed to set all leaves, as more than `u32::MAX` sub-leaves are present: {0}
-    Overflow(<u32 as TryFrom<usize>>::Error),
+    /// Failed to set domain type (CPUID.(EAX=0xB,ECX={0}):ECX[15:8]): {1}
+    DomainType(u32, CheckedAssignError),
+    /// Failed to set input ECX (CPUID.(EAX=0xB,ECX={0}):ECX[7:0]): {1}
+    InputEcx(u32, CheckedAssignError),
+    /// Failed to set number of logical processors (CPUID.(EAX=0xB,ECX={0}):EBX[15:0]): {1}
+    NumLogicalProcs(u32, CheckedAssignError),
+    /// Failed to set right-shift bits (CPUID.(EAX=0xB,ECX={0}):EAX[4:0]): {1}
+    RightShiftBits(u32, CheckedAssignError),
     /// Unexpected subleaf: {0}
     UnexpectedSubleaf(u32)
 }
@@ -348,18 +346,18 @@ impl super::Cpuid {
                         // To get the next level APIC ID, shift right with at most 1 because we have
                         // maximum 2 logical procerssors per core that can be represented by 1 bit.
                         set_range(&mut subleaf.result.eax, 0..5, u32::from(cpu_bits))
-                            .map_err(ExtendedTopologyError::ApicId)?;
+                            .map_err(|err| ExtendedTopologyError::RightShiftBits(index, err))?;
 
                         // When cpu_count == 1 or HT is disabled, there is 1 logical core at this
                         // domain; otherwise there are 2
                         set_range(&mut subleaf.result.ebx, 0..16, u32::from(cpus_per_core))
-                            .map_err(ExtendedTopologyError::LogicalProcessors)?;
+                            .map_err(|err| ExtendedTopologyError::NumLogicalProcs(index, err))?;
 
                         // Skip setting 0 to ECX[7:0] since it's already reset to 0.
 
                         // Set the domain type identification value for logical processor,
                         set_range(&mut subleaf.result.ecx, 8..16, 1)
-                            .map_err(ExtendedTopologyError::DomainType)?;
+                            .map_err(|err| ExtendedTopologyError::DomainType(index, err))?;
                     }
                     // Core domain
                     1 => {
@@ -373,17 +371,17 @@ impl super::Cpuid {
                             0..5,
                             MAX_SUPPORTED_VCPUS.next_power_of_two().ilog2(),
                         )
-                        .map_err(ExtendedTopologyError::ApicId)?;
+                        .map_err(|err| ExtendedTopologyError::RightShiftBits(index, err))?;
                         set_range(&mut subleaf.result.ebx, 0..16, u32::from(cpu_count))
-                            .map_err(ExtendedTopologyError::LogicalProcessors)?;
+                            .map_err(|err| ExtendedTopologyError::NumLogicalProcs(index, err))?;
 
                         // Setting the input ECX value (i.e. `index`)
                         set_range(&mut subleaf.result.ecx, 0..8, index)
-                            .map_err(ExtendedTopologyError::InputEcx)?;
+                            .map_err(|err| ExtendedTopologyError::InputEcx(index, err))?;
 
                         // Set the domain type identification value for core.
                         set_range(&mut subleaf.result.ecx, 8..16, 2)
-                            .map_err(ExtendedTopologyError::DomainType)?;
+                            .map_err(|err| ExtendedTopologyError::DomainType(index, err))?;
                     }
                     _ => {
                         // KVM no longer returns any subleaf numbers greater than 0. The patch was