From 35450f9bfe22d6db449874a553ddc29f55f45e85 Mon Sep 17 00:00:00 2001
From: Joshua Job <jjob@scalecomputing.com>
Date: Thu, 2 May 2024 17:33:31 -0700
Subject: [PATCH] MSR: Fixed several issues related to inclusion

Some platforms and feature lists can end up with partial inclusions.
---
 src/main.rs | 15 +++++++------
 src/msr.rs  | 65 ++++++++++++++++++++++++++++-------------------------
 2 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index 0e17f9c..cf5d732 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -40,6 +40,7 @@ struct Disp {
     #[cfg(all(target_os = "linux", feature = "kvm"))]
     #[structopt(long)]
     skip_kvm: bool,
+    #[cfg(feature = "use_msr")]
     #[structopt(long)]
     skip_msr: bool,
 }
@@ -78,9 +79,9 @@ impl Command for Disp {
 
             #[cfg(feature = "use_msr")]
             if !self.skip_msr {
-                #[cfg(all(target_os = "linux"))]
+                #[cfg(target_os = "linux")]
                 {
-                    match msr::LinuxMsrStore::new() {
+                    match msr::linux::LinuxMsrStore::new() {
                         Ok(linux_store) => {
                             println!("MSRS:");
                             for msr in &config.msrs {
@@ -174,7 +175,7 @@ impl Command for Facts {
                     let msr = {
                         #[cfg(feature = "use_msr")]
                         {
-                            Box::new(msr::LinuxMsrStore::new()?) as Box<dyn MsrStore>
+                            Box::new(msr::linux::LinuxMsrStore::new()?) as Box<dyn MsrStore>
                         }
                         #[cfg(not(feature = "use_msr"))]
                         {
@@ -188,12 +189,12 @@ impl Command for Facts {
             {
                 (
                     CpuidType::func(),
-                    Box::new(msr::LinuxMsrStore::new()?) as Box<dyn MsrStore>,
+                    Box::new(msr::linux::LinuxMsrStore::new()?) as Box<dyn MsrStore>,
                 )
             }
-            #[cfg(all(
-                not(feature = "kvm"),
-                any(not(target_os = "linux"), not(feature = "use_msr"))
+            #[cfg(any(
+                not(target_os = "linux"),
+                all(not(feature = "kvm"), not(feature = "use_msr"))
             ))]
             {
                 (
diff --git a/src/msr.rs b/src/msr.rs
index c5fe973..278d923 100644
--- a/src/msr.rs
+++ b/src/msr.rs
@@ -4,7 +4,6 @@
 use super::bitfield::{self, Facter};
 use super::facts::{self, GenericFact};
 use serde::{Deserialize, Serialize};
-use std::fs;
 use std::vec::Vec;
 use std::{convert, error, fmt, io};
 
@@ -54,40 +53,44 @@ impl MsrStore for EmptyMSR {
 }
 
 #[cfg(all(target_os = "linux", feature = "use_msr"))]
-pub struct LinuxMsrStore {
-    msr_device: fs::File,
-}
+pub mod linux {
+    use super::*;
+    use std::fs;
+    use std::io;
 
-#[cfg(all(target_os = "linux", feature = "use_msr"))]
-impl LinuxMsrStore {
-    pub fn new() -> Result<LinuxMsrStore> {
-        Ok(LinuxMsrStore {
-            msr_device: fs::OpenOptions::new()
-                .read(true)
-                .open("/dev/cpu/0/msr")
-                .map_err(|e| match e.kind() {
-                    io::ErrorKind::NotFound => Error::NotAvailible,
-                    io::ErrorKind::PermissionDenied => Error::NotAvailible,
-                    _ => Error::IOError(e),
-                })?,
-        })
+    pub struct LinuxMsrStore {
+        msr_device: fs::File,
     }
-}
 
-#[cfg(all(target_os = "linux", feature = "use_msr"))]
-impl MsrStore for LinuxMsrStore {
-    fn is_empty(&self) -> bool {
-        false
+    impl LinuxMsrStore {
+        pub fn new() -> Result<LinuxMsrStore> {
+            Ok(LinuxMsrStore {
+                msr_device: fs::OpenOptions::new()
+                    .read(true)
+                    .open("/dev/cpu/0/msr")
+                    .map_err(|e| match e.kind() {
+                        io::ErrorKind::NotFound => Error::NotAvailible,
+                        io::ErrorKind::PermissionDenied => Error::NotAvailible,
+                        _ => Error::IOError(e),
+                    })?,
+            })
+        }
     }
-    fn get_value<'a>(&self, desc: &'a MSRDesc) -> std::result::Result<MSRValue<'a>, Error> {
-        use std::os::unix::fs::FileExt;
-        let mut msr_bytes = [u8::MIN; 8];
-        self.msr_device
-            .read_at(&mut msr_bytes, desc.address.into())?;
-        Ok(MSRValue {
-            desc,
-            value: u64::from_le_bytes(msr_bytes),
-        })
+
+    impl MsrStore for LinuxMsrStore {
+        fn is_empty(&self) -> bool {
+            false
+        }
+        fn get_value<'a>(&self, desc: &'a MSRDesc) -> std::result::Result<MSRValue<'a>, Error> {
+            use std::os::unix::fs::FileExt;
+            let mut msr_bytes = [u8::MIN; 8];
+            self.msr_device
+                .read_at(&mut msr_bytes, desc.address.into())?;
+            Ok(MSRValue {
+                desc,
+                value: u64::from_le_bytes(msr_bytes),
+            })
+        }
     }
 }