From 56d2ea9b78f01cc437b6ae427ba050d5acb15605 Mon Sep 17 00:00:00 2001
From: Archana Shinde <archana.m.shinde@intel.com>
Date: Thu, 11 May 2023 00:40:10 -0700
Subject: [PATCH 1/3] kata-ctl: Refactor kernel module check

Adding vhost and vhost-net to the kernel modules. These do not require
any kernel module parameters to be checked. Currently, kernel params is
a required field. Make this as optional. Could make this as <Option>,
but making this a slice instead, as a module could have multiple kernel
params. Refactor the function that checks are for kernel modules into
two with one specifically checking if the module is loaded and other
checking for module parameters.

Refactor some of the tests to take into account these changes.

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
---
 src/tools/kata-ctl/src/arch/x86_64/mod.rs | 67 ++++++++++++++----
 src/tools/kata-ctl/src/check.rs           | 85 ++++++++++++-----------
 src/tools/kata-ctl/src/types.rs           |  2 +-
 3 files changed, 96 insertions(+), 58 deletions(-)

diff --git a/src/tools/kata-ctl/src/arch/x86_64/mod.rs b/src/tools/kata-ctl/src/arch/x86_64/mod.rs
index 53779104da..457f87d9f2 100644
--- a/src/tools/kata-ctl/src/arch/x86_64/mod.rs
+++ b/src/tools/kata-ctl/src/arch/x86_64/mod.rs
@@ -59,17 +59,29 @@ mod arch_specific {
     static MODULE_LIST: &[KernelModule] = &[
         KernelModule {
             name: "kvm",
-            parameter: KernelParam {
+            params: &[KernelParam {
                 name: "kvmclock_periodic_sync",
                 value: KernelParamType::Simple("Y"),
-            },
+            }],
         },
         KernelModule {
             name: "kvm_intel",
-            parameter: KernelParam {
+            params: &[KernelParam {
                 name: "unrestricted_guest",
                 value: KernelParamType::Predicate(unrestricted_guest_param_check),
-            },
+            }],
+        },
+        KernelModule {
+            name: "vhost",
+            params: &[],
+        },
+        KernelModule {
+            name: "vhost_net",
+            params: &[],
+        },
+        KernelModule {
+            name: "vhost_vsock",
+            params: &[],
         },
     ];
 
@@ -253,6 +265,38 @@ mod arch_specific {
         }
     }
 
+    fn check_kernel_params(kernel_module: &KernelModule) -> Result<()> {
+        const MODULES_PATH: &str = "/sys/module";
+
+        for param in kernel_module.params {
+            let module_param_path = format!(
+                "{}/{}/parameters/{}",
+                MODULES_PATH, kernel_module.name, param.name
+            );
+
+            // Here the currently loaded kernel parameter value
+            // is retrieved and returned on success
+            let param_value_host = std::fs::read_to_string(module_param_path)
+                .map(|val| val.replace('\n', ""))
+                .map_err(|_err| {
+                    anyhow!(
+                        "'{:}' kernel module parameter `{:}` not found.",
+                        kernel_module.name,
+                        param.name
+                    )
+                })?;
+
+            check_kernel_param(
+                kernel_module.name,
+                param.name,
+                &param_value_host,
+                param.value.clone(),
+            )
+            .map_err(|e| anyhow!(e.to_string()))?;
+        }
+        Ok(())
+    }
+
     fn check_kernel_param(
         module: &str,
         param_name: &str,
@@ -282,19 +326,12 @@ mod arch_specific {
         info!(sl!(), "check kernel modules for: x86_64");
 
         for module in MODULE_LIST {
-            let module_loaded =
-                check::check_kernel_module_loaded(module.name, module.parameter.name);
+            let module_loaded = check::check_kernel_module_loaded(module);
 
             match module_loaded {
-                Ok(param_value_host) => {
-                    let parameter_check = check_kernel_param(
-                        module.name,
-                        module.parameter.name,
-                        &param_value_host,
-                        module.parameter.value.clone(),
-                    );
-
-                    match parameter_check {
+                Ok(_) => {
+                    let check = check_kernel_params(module);
+                    match check {
                         Ok(_v) => info!(sl!(), "{} Ok", module.name),
                         Err(e) => return Err(e),
                     }
diff --git a/src/tools/kata-ctl/src/check.rs b/src/tools/kata-ctl/src/check.rs
index d531425c2f..f8d39dbde8 100644
--- a/src/tools/kata-ctl/src/check.rs
+++ b/src/tools/kata-ctl/src/check.rs
@@ -5,6 +5,7 @@
 
 // Contains checks that are not architecture-specific
 
+use crate::types::KernelModule;
 use anyhow::{anyhow, Result};
 use nix::fcntl::{open, OFlag};
 use nix::sys::stat::Mode;
@@ -324,17 +325,16 @@ pub fn check_official_releases() -> Result<()> {
 }
 
 #[cfg(any(target_arch = "x86_64"))]
-pub fn check_kernel_module_loaded(module: &str, parameter: &str) -> Result<String, String> {
+pub fn check_kernel_module_loaded(kernel_module: &KernelModule) -> Result<(), String> {
     const MODPROBE_PARAMETERS_DRY_RUN: &str = "--dry-run";
     const MODPROBE_PARAMETERS_FIRST_TIME: &str = "--first-time";
-    const MODULES_PATH: &str = "/sys/module";
 
     let status_modinfo_success;
 
     // Partial check w/ modinfo
     // verifies that the module exists
     match Command::new(MODINFO_PATH)
-        .arg(module)
+        .arg(kernel_module.name)
         .stdout(Stdio::piped())
         .output()
     {
@@ -361,7 +361,7 @@ pub fn check_kernel_module_loaded(module: &str, parameter: &str) -> Result<Strin
     match Command::new(MODPROBE_PATH)
         .arg(MODPROBE_PARAMETERS_DRY_RUN)
         .arg(MODPROBE_PARAMETERS_FIRST_TIME)
-        .arg(module)
+        .arg(kernel_module.name)
         .stdout(Stdio::piped())
         .output()
     {
@@ -371,8 +371,8 @@ pub fn check_kernel_module_loaded(module: &str, parameter: &str) -> Result<Strin
 
             if status_modprobe_success && status_modinfo_success {
                 // This condition is true in the case that the module exist, but is not already loaded
-                let msg = format!("The kernel module `{:}` exist but is not already loaded. Try reloading it using 'modprobe {:}=Y'",
-                        module, module
+                let msg = format!("The kernel module `{:}` exist but is not already loaded. Try reloading it using 'modprobe {:}'",
+                kernel_module.name, kernel_module.name
                     );
                 return Err(msg);
             }
@@ -386,27 +386,14 @@ pub fn check_kernel_module_loaded(module: &str, parameter: &str) -> Result<Strin
             return Err(msg);
         }
     }
-
-    let module_path = format!("{}/{}/parameters/{}", MODULES_PATH, module, parameter);
-
-    // Here the currently loaded kernel parameter value
-    // is retrieved and returned on success
-    match read_file_contents(&module_path) {
-        Ok(result) => Ok(result.replace('\n', "")),
-        Err(_e) => {
-            let msg = format!(
-                "'{:}' kernel module parameter `{:}` not found.",
-                module, parameter
-            );
-            Err(msg)
-        }
-    }
+    Ok(())
 }
 
 #[cfg(any(target_arch = "s390x", target_arch = "x86_64"))]
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::types::{KernelModule, KernelParam, KernelParamType};
     use semver::Version;
     use slog::warn;
     use std::fs;
@@ -612,12 +599,13 @@ mod tests {
     #[test]
     fn check_module_loaded() {
         #[allow(dead_code)]
-        #[derive(Debug)]
+
         struct TestData<'a> {
             module_name: &'a str,
             param_name: &'a str,
+            kernel_module: &'a KernelModule<'a>,
             param_value: &'a str,
-            result: Result<String>,
+            result: Result<()>,
         }
 
         let tests = &[
@@ -625,45 +613,58 @@ mod tests {
             TestData {
                 module_name: "",
                 param_name: "",
+                kernel_module: &KernelModule {
+                    name: "",
+                    params: &[KernelParam {
+                        name: "",
+                        value: KernelParamType::Simple("Y"),
+                    }],
+                },
                 param_value: "",
                 result: Err(anyhow!("modinfo: ERROR: Module {} not found.", "")),
             },
-            TestData {
-                module_name: "kvm",
-                param_name: "",
-                param_value: "",
-                result: Err(anyhow!(
-                    "'{:}' kernel module parameter `{:}` not found.",
-                    "kvm",
-                    ""
-                )),
-            },
             // Success scenarios
+            TestData {
+                module_name: "kvm",
+                param_name: "",
+                kernel_module: &KernelModule {
+                    name: "kvm",
+                    params: &[KernelParam {
+                        name: "nonexistantparam",
+                        value: KernelParamType::Simple("Y"),
+                    }],
+                },
+                param_value: "",
+                result: Ok(()),
+            },
             TestData {
                 module_name: "kvm",
                 param_name: "kvmclock_periodic_sync",
+                kernel_module: &KernelModule {
+                    name: "kvm",
+                    params: &[KernelParam {
+                        name: "kvmclock_periodic_sync",
+                        value: KernelParamType::Simple("Y"),
+                    }],
+                },
                 param_value: "Y",
-                result: Ok("Y".to_string()),
+                result: Ok(()),
             },
         ];
 
         for (i, d) in tests.iter().enumerate() {
-            let msg = format!("test[{}]: {:?}", i, d);
-            let result = check_kernel_module_loaded(d.module_name, d.param_name);
+            let msg = format!("test[{}]", i);
+            let result = check_kernel_module_loaded(d.kernel_module);
             let msg = format!("{}, result: {:?}", msg, result);
 
             if d.result.is_ok() {
-                assert_eq!(
-                    result.as_ref().unwrap(),
-                    d.result.as_ref().unwrap(),
-                    "{}",
-                    msg
-                );
+                assert_eq!(result, Ok(()));
                 continue;
             }
 
             let expected_error = format!("{}", &d.result.as_ref().unwrap_err());
             let actual_error = result.unwrap_err().to_string();
+            println!("testing for {}", d.module_name);
             assert!(actual_error == expected_error, "{}", msg);
         }
     }
diff --git a/src/tools/kata-ctl/src/types.rs b/src/tools/kata-ctl/src/types.rs
index fcafeb435e..072b1b0c46 100644
--- a/src/tools/kata-ctl/src/types.rs
+++ b/src/tools/kata-ctl/src/types.rs
@@ -69,5 +69,5 @@ pub struct KernelParam<'a> {
 #[allow(dead_code)]
 pub struct KernelModule<'a> {
     pub name: &'a str,
-    pub parameter: KernelParam<'a>,
+    pub params: &'a [KernelParam<'a>],
 }

From 1b406b9d0c3cccfbcfcf0bfcdff19a24b632e029 Mon Sep 17 00:00:00 2001
From: Archana Shinde <archana.m.shinde@intel.com>
Date: Thu, 11 May 2023 00:47:01 -0700
Subject: [PATCH 2/3] kata-ctl:Implement functionality to check host is capable
 of running VM

Implement functionality to add to the env output if the host is capable
of running a VM.

Fixes: #6727

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
---
 src/tools/kata-ctl/src/arch/aarch64/mod.rs     |  5 +++++
 src/tools/kata-ctl/src/arch/powerpc64le/mod.rs |  5 +++++
 src/tools/kata-ctl/src/arch/s390x/mod.rs       | 15 +++++++++++++++
 src/tools/kata-ctl/src/arch/x86_64/mod.rs      | 17 +++++++++++++++++
 src/tools/kata-ctl/src/check.rs                |  3 +++
 src/tools/kata-ctl/src/ops/env_ops.rs          |  9 +++++++--
 6 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/src/tools/kata-ctl/src/arch/aarch64/mod.rs b/src/tools/kata-ctl/src/arch/aarch64/mod.rs
index 0f4012a368..41d28f8db1 100644
--- a/src/tools/kata-ctl/src/arch/aarch64/mod.rs
+++ b/src/tools/kata-ctl/src/arch/aarch64/mod.rs
@@ -80,6 +80,11 @@ mod arch_specific {
         Some(CHECK_LIST)
     }
 
+    pub fn host_is_vmcontainer_capable() -> Result<bool> {
+        // TODO: Not implemented
+        Ok(true)
+    }
+
     #[allow(dead_code)]
     // Guest protection is not supported on ARM64.
     pub fn available_guest_protection() -> Result<check::GuestProtection, check::ProtectionError> {
diff --git a/src/tools/kata-ctl/src/arch/powerpc64le/mod.rs b/src/tools/kata-ctl/src/arch/powerpc64le/mod.rs
index fc849c6318..436d5a4d23 100644
--- a/src/tools/kata-ctl/src/arch/powerpc64le/mod.rs
+++ b/src/tools/kata-ctl/src/arch/powerpc64le/mod.rs
@@ -33,6 +33,11 @@ mod arch_specific {
         // to the goloang implementation of function getCPUDetails()
     }
 
+    pub fn host_is_vmcontainer_capable() -> Result<bool> {
+        // TODO: Not implemented
+        Ok(true)
+    }
+
     pub fn available_guest_protection() -> Result<check::GuestProtection, check::ProtectionError> {
         if !Uid::effective().is_root() {
             return Err(check::ProtectionError::NoPerms);
diff --git a/src/tools/kata-ctl/src/arch/s390x/mod.rs b/src/tools/kata-ctl/src/arch/s390x/mod.rs
index d3efe6f3c6..991d676d1c 100644
--- a/src/tools/kata-ctl/src/arch/s390x/mod.rs
+++ b/src/tools/kata-ctl/src/arch/s390x/mod.rs
@@ -78,6 +78,21 @@ mod arch_specific {
         Some(CHECK_LIST)
     }
 
+    pub fn host_is_vmcontainer_capable() -> Result<bool> {
+        let mut count = 0;
+        if check_cpu().is_err() {
+            count += 1;
+        };
+
+        // TODO: Add additional checks for kernel modules
+
+        if count == 0 {
+            return Ok(true);
+        };
+
+        Err(anyhow!("System is not capable of running a VM"))
+    }
+
     #[allow(dead_code)]
     fn retrieve_cpu_facilities() -> Result<HashMap<i32, bool>> {
         let f = std::fs::File::open(check::PROC_CPUINFO)?;
diff --git a/src/tools/kata-ctl/src/arch/x86_64/mod.rs b/src/tools/kata-ctl/src/arch/x86_64/mod.rs
index 457f87d9f2..fb2d0a35ec 100644
--- a/src/tools/kata-ctl/src/arch/x86_64/mod.rs
+++ b/src/tools/kata-ctl/src/arch/x86_64/mod.rs
@@ -343,6 +343,23 @@ mod arch_specific {
         }
         Ok(())
     }
+
+    pub fn host_is_vmcontainer_capable() -> Result<bool> {
+        let mut count = 0;
+        if check_cpu("check_cpu").is_err() {
+            count += 1;
+        };
+
+        if check_kernel_modules("check_modules").is_err() {
+            count += 1;
+        };
+
+        if count == 0 {
+            return Ok(true);
+        };
+
+        Err(anyhow!("System is not capable of running a VM"))
+    }
 }
 
 #[cfg(target_arch = "x86_64")]
diff --git a/src/tools/kata-ctl/src/check.rs b/src/tools/kata-ctl/src/check.rs
index f8d39dbde8..78e30a93a9 100644
--- a/src/tools/kata-ctl/src/check.rs
+++ b/src/tools/kata-ctl/src/check.rs
@@ -5,7 +5,9 @@
 
 // Contains checks that are not architecture-specific
 
+#[cfg(any(target_arch = "x86_64"))]
 use crate::types::KernelModule;
+
 use anyhow::{anyhow, Result};
 use nix::fcntl::{open, OFlag};
 use nix::sys::stat::Mode;
@@ -393,6 +395,7 @@ pub fn check_kernel_module_loaded(kernel_module: &KernelModule) -> Result<(), St
 #[cfg(test)]
 mod tests {
     use super::*;
+    #[cfg(any(target_arch = "x86_64"))]
     use crate::types::{KernelModule, KernelParam, KernelParamType};
     use semver::Version;
     use slog::warn;
diff --git a/src/tools/kata-ctl/src/ops/env_ops.rs b/src/tools/kata-ctl/src/ops/env_ops.rs
index 05602e479c..d687f96172 100644
--- a/src/tools/kata-ctl/src/ops/env_ops.rs
+++ b/src/tools/kata-ctl/src/ops/env_ops.rs
@@ -255,6 +255,12 @@ fn get_host_info() -> Result<HostInfo> {
 
     let guest_protection = guest_protection.to_string();
 
+    let mut vm_container_capable = true;
+
+    if arch_specific::host_is_vmcontainer_capable().is_err() {
+        vm_container_capable = false;
+    }
+
     let support_vsocks = utils::supports_vsocks(utils::VHOST_VSOCK_DEVICE)?;
 
     Ok(HostInfo {
@@ -264,8 +270,7 @@ fn get_host_info() -> Result<HostInfo> {
         cpu: host_cpu,
         memory: memory_info,
         available_guest_protection: guest_protection,
-        // TODO: See https://github.com/kata-containers/kata-containers/issues/6727
-        vm_container_capable: true,
+        vm_container_capable,
         support_vsocks,
     })
 }

From 610f7986e4c5cf62197006121ed9e6cf357b5a15 Mon Sep 17 00:00:00 2001
From: Archana Shinde <archana.m.shinde@intel.com>
Date: Wed, 21 Jun 2023 07:30:35 -0700
Subject: [PATCH 3/3] check: Relax the unrestricted_guest check when running in
 a VM

When running on a VM, the kernel parameter "unrestricted_guest" for
kernel module "kvm_intel" is not required. So, return success when running
on a VM without checking value of this kernel parameter.

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
---
 src/tools/kata-ctl/src/arch/x86_64/mod.rs | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/tools/kata-ctl/src/arch/x86_64/mod.rs b/src/tools/kata-ctl/src/arch/x86_64/mod.rs
index fb2d0a35ec..67fd4ddc89 100644
--- a/src/tools/kata-ctl/src/arch/x86_64/mod.rs
+++ b/src/tools/kata-ctl/src/arch/x86_64/mod.rs
@@ -238,13 +238,9 @@ mod arch_specific {
 
         let running_on_vmm_alt = running_on_vmm()?;
 
+        // Kernel param "unrestricted_guest" is not required when running under a hypervisor
         if running_on_vmm_alt {
-            let msg = format!("You are running in a VM, where the kernel module '{}' parameter '{:}' has a value '{:}'. This causes conflict when running kata.",
-                module,
-                param_name,
-                param_value_host
-            );
-            return Err(anyhow!(msg));
+            return Ok(());
         }
 
         if param_value_host == expected_param_value.to_string() {