From bacf3f770106ed473a3b8f9f6909f16cbe1b0a68 Mon Sep 17 00:00:00 2001
From: Alexandre Courbot <acourbot@chromium.org>
Date: Thu, 17 Nov 2022 15:51:40 +0900
Subject: [PATCH] crosvm: use structure to specify memory options

We currently have many different memory-related options. This does not
play well with the idea of having them in a configuration file, where
they would ideally be grouped under a single "mem" section.

Turn the `mem` option into a struct, which currently only has one member
specifying the desired RAM size. This makes the struct compatible with
the existing argument (since the first argument of a struct does not
need to be explicitly mentioned), which allowing us to extend the struct
with more options in the future.

This allows us to enable the `mem` option in configuration files.

BUG=b:255223604
TEST=cargo test parse_mem_opts

Change-Id: I24c324002e4e399e5be492bbc9ddcac7fd2b349e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4033213
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
---
 src/crosvm/cmdline.rs | 11 +++++++----
 src/crosvm/config.rs  | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/src/crosvm/cmdline.rs b/src/crosvm/cmdline.rs
index 516699a35a..c4a25d63d8 100644
--- a/src/crosvm/cmdline.rs
+++ b/src/crosvm/cmdline.rs
@@ -98,6 +98,7 @@ use crate::crosvm::config::GidMap;
 #[cfg(feature = "direct")]
 use crate::crosvm::config::HostPcieRootPortParameters;
 use crate::crosvm::config::HypervisorKind;
+use crate::crosvm::config::MemOptions;
 use crate::crosvm::config::TouchDeviceOption;
 use crate::crosvm::config::VhostUserFsOption;
 use crate::crosvm::config::VhostUserOption;
@@ -1200,10 +1201,11 @@ pub struct RunCommand {
     pub mac_address: Option<net_util::MacAddress>,
 
     #[argh(option, short = 'm', arg_name = "N")]
-    #[serde(skip)] // TODO(b/255223604)
     #[merge(strategy = overwrite_option)]
-    /// amount of guest memory in MiB. (default: 256)
-    pub mem: Option<u64>,
+    /// memory parameters.
+    /// Possible key values:
+    ///     size=NUM - amount of guest memory in MiB. (default: 256)
+    pub mem: Option<MemOptions>,
 
     #[argh(option, from_str_fn(parse_mmio_address_range))]
     #[serde(skip)] // TODO(b/255223604)
@@ -2102,7 +2104,8 @@ impl TryFrom<RunCommand> for super::config::Config {
 
         cfg.delay_rt = cmd.delay_rt;
 
-        cfg.memory = cmd.mem;
+        let mem = cmd.mem.unwrap_or_default();
+        cfg.memory = mem.size;
 
         #[cfg(target_arch = "aarch64")]
         {
diff --git a/src/crosvm/config.rs b/src/crosvm/config.rs
index 2643780738..24af74f60e 100644
--- a/src/crosvm/config.rs
+++ b/src/crosvm/config.rs
@@ -101,6 +101,14 @@ pub struct CpuOptions {
     pub num_cores: Option<usize>,
 }
 
+#[derive(Debug, Default, Deserialize, FromKeyValues)]
+#[serde(deny_unknown_fields, rename_all = "kebab-case")]
+pub struct MemOptions {
+    /// Amount of guest memory in MiB.
+    #[serde(default)]
+    pub size: Option<u64>,
+}
+
 #[derive(Serialize, Deserialize)]
 pub struct VhostUserOption {
     pub socket: PathBuf,
@@ -1781,6 +1789,18 @@ mod tests {
         );
     }
 
+    #[test]
+    fn parse_mem_opts() {
+        let res: MemOptions = from_key_values("").unwrap();
+        assert_eq!(res.size, None);
+
+        let res: MemOptions = from_key_values("1024").unwrap();
+        assert_eq!(res.size, Some(1024));
+
+        let res: MemOptions = from_key_values("size=0x4000").unwrap();
+        assert_eq!(res.size, Some(16384));
+    }
+
     #[cfg(feature = "audio_cras")]
     #[test]
     fn parse_ac97_vaild() {