api: forbid ht_enabled=true on ARM

Signed-off-by: alindima <[email protected]>

Author: alindima

CHANGELOG.md

@@ -57,6 +57,7 @@
   interfaces. Specifying interfaces that allow forwarding requests to MMDS is done
   by adding the network interface's ID to the `network_interfaces` field of PUT
   `/mmds/config` request's body.
+- Configuring `ht_enabled: true` on aarch64 via the API is forbidden.
 
 ### Fixed
 

src/api_server/src/parsed_request.rs

@@ -789,7 +789,7 @@ pub(crate) mod tests {
         let body = "{ \
             \"vcpu_count\": 0, \
             \"mem_size_mib\": 0, \
-            \"ht_enabled\": true \
+            \"ht_enabled\": false \
         }";
         sender
             .write_all(http_request("PUT", "/machine-config", Some(&body)).as_bytes())
@@ -1010,7 +1010,7 @@ pub(crate) mod tests {
         let body = "{ \
             \"vcpu_count\": 0, \
             \"mem_size_mib\": 0, \
-            \"ht_enabled\": true \
+            \"ht_enabled\": false \
         }";
         sender
             .write_all(http_request("PATCH", "/machine-config", Some(&body)).as_bytes())
@@ -1021,7 +1021,7 @@ pub(crate) mod tests {
         let body = "{ \
             \"vcpu_count\": 0, \
             \"mem_size_mib\": 0, \
-            \"ht_enabled\": true, \
+            \"ht_enabled\": false, \
             \"cpu_template\": \"C3\" \
         }";
         sender

src/api_server/src/request/machine_configuration.rs

@@ -19,6 +19,7 @@ pub(crate) fn parse_put_machine_config(body: &Body) -> Result<ParsedRequest, Err
         Error::SerdeJson(e)
     })?;
 
+    #[cfg(target_arch = "aarch64")]
     check_unsupported_fields(&vm_config)?;
 
     if vm_config.vcpu_count.is_none()
@@ -43,6 +44,7 @@ pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, E
         Error::SerdeJson(e)
     })?;
 
+    #[cfg(target_arch = "aarch64")]
     check_unsupported_fields(&vm_config)?;
 
     if vm_config.vcpu_count.is_none()
@@ -57,17 +59,24 @@ pub(crate) fn parse_patch_machine_config(body: &Body) -> Result<ParsedRequest, E
     )))
 }
 
-fn check_unsupported_fields(_vm_config: &VmConfig) -> Result<(), Error> {
-    #[cfg(target_arch = "aarch64")]
-    {
-        if _vm_config.cpu_template.is_some() {
-            // cpu_template is not supported on aarch64
-            return Err(Error::Generic(
-                StatusCode::BadRequest,
-                "CPU templates are not supported on aarch64".to_string(),
-            ));
-        }
+#[cfg(target_arch = "aarch64")]
+fn check_unsupported_fields(vm_config: &VmConfig) -> Result<(), Error> {
+    if vm_config.cpu_template.is_some() {
+        // cpu_template is not supported on aarch64
+        return Err(Error::Generic(
+            StatusCode::BadRequest,
+            "CPU templates are not supported on aarch64".to_string(),
+        ));
+    }
+
+    if let Some(true) = vm_config.ht_enabled {
+        // ht_enabled: true is not supported on aarch64
+        return Err(Error::Generic(
+            StatusCode::BadRequest,
+            "Enabling HyperThreading is not supported on aarch64".to_string(),
+        ));
     }
+
     Ok(())
 }
 
@@ -91,13 +100,13 @@ mod tests {
         // 2. Test case for mandatory fields.
         let body = r#"{
                 "mem_size_mib": 1024,
-                "ht_enabled": true
+                "ht_enabled": false
               }"#;
         assert!(parse_put_machine_config(&Body::new(body)).is_err());
 
         let body = r#"{
                 "vcpu_count": 8,
-                "ht_enabled": true
+                "ht_enabled": false
                 }"#;
         assert!(parse_put_machine_config(&Body::new(body)).is_err());
 
@@ -111,13 +120,13 @@ mod tests {
         let body = r#"{
                 "vcpu_count": 8,
                 "mem_size_mib": 1024,
-                "ht_enabled": true,
+                "ht_enabled": false,
                 "track_dirty_pages": true
               }"#;
         let expected_config = VmConfig {
             vcpu_count: Some(8),
             mem_size_mib: Some(1024),
-            ht_enabled: Some(true),
+            ht_enabled: Some(false),
             cpu_template: None,
             track_dirty_pages: true,
         };
@@ -131,7 +140,7 @@ mod tests {
         let body = r#"{
                 "vcpu_count": 8,
                 "mem_size_mib": 1024,
-                "ht_enabled": true,
+                "ht_enabled": false,
                 "cpu_template": "T2",
                 "track_dirty_pages": true
               }"#;
@@ -142,7 +151,7 @@ mod tests {
             let expected_config = VmConfig {
                 vcpu_count: Some(8),
                 mem_size_mib: Some(1024),
-                ht_enabled: Some(true),
+                ht_enabled: Some(false),
                 cpu_template: Some(CpuFeaturesTemplate::T2),
                 track_dirty_pages: true,
             };
@@ -157,6 +166,35 @@ mod tests {
         {
             assert!(parse_put_machine_config(&Body::new(body)).is_err());
         }
+
+        // 5. Test that setting `ht_enabled: true` is successful on x86_64 while on aarch64, it is not.
+        let body = r#"{
+            "vcpu_count": 8,
+            "mem_size_mib": 1024,
+            "ht_enabled": true,
+            "track_dirty_pages": true
+          }"#;
+
+        #[cfg(target_arch = "x86_64")]
+        {
+            let expected_config = VmConfig {
+                vcpu_count: Some(8),
+                mem_size_mib: Some(1024),
+                ht_enabled: Some(true),
+                cpu_template: None,
+                track_dirty_pages: true,
+            };
+
+            match vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()) {
+                VmmAction::SetVmConfiguration(config) => assert_eq!(config, expected_config),
+                _ => panic!("Test failed."),
+            }
+        }
+
+        #[cfg(target_arch = "aarch64")]
+        {
+            assert!(parse_put_machine_config(&Body::new(body)).is_err());
+        }
     }
 
     #[test]

src/api_server/swagger/firecracker.yaml

@@ -774,6 +774,7 @@ definitions:
     description:
       The CPU Template defines a set of flags to be disabled from the microvm so that
       the features exposed to the guest are the same as in the selected instance type.
+      Works only on Intel.
     enum:
       - C3
       - T2
@@ -935,7 +936,7 @@ definitions:
         $ref: "#/definitions/CpuTemplate"
       ht_enabled:
         type: boolean
-        description: Flag for enabling/disabling Hyperthreading
+        description: Flag for enabling/disabling Hyperthreading. x86-only.
       mem_size_mib:
         type: integer
         description: Memory size of VM

tests/integration_tests/functional/test_api.py

@@ -181,7 +181,7 @@ def test_api_put_update_pre_boot(test_microvm_with_api):
     # The machine configuration has a default value, so all PUTs are updates.
     microvm_config_json = {
         'vcpu_count': 4,
-        'ht_enabled': True,
+        'ht_enabled': not platform.machine() == 'aarch64',
         'mem_size_mib': 256,
         'track_dirty_pages': True
     }
@@ -393,6 +393,7 @@ def test_api_mmds_config(test_microvm_with_api):
     )['mmds-config']['version'] == "V2"
 
 
+# pylint: disable=too-many-statements
 def test_api_machine_config(test_microvm_with_api):
     """
     Test /machine_config PUT/PATCH scenarios that unit tests can't cover.
@@ -425,6 +426,22 @@ def test_api_machine_config(test_microvm_with_api):
     )
     assert test_microvm.api_session.is_status_bad_request(response.status_code)
 
+    # Test that ht_enabled=True errors on ARM.
+    response = test_microvm.machine_cfg.patch(
+        ht_enabled=True
+    )
+    if platform.machine() == "x86_64":
+        assert test_microvm.api_session.is_status_no_content(
+            response.status_code
+        )
+    else:
+        assert test_microvm.api_session.is_status_bad_request(
+            response.status_code
+        )
+        assert "Enabling HyperThreading is not supported on aarch64" in\
+            response.text
+
+    # Test that CPU template errors on ARM.
     response = test_microvm.machine_cfg.patch(
         cpu_template='C3'
     )

tests/integration_tests/functional/test_topology.py

@@ -152,6 +152,8 @@ def test_cache_topology(test_microvm_with_api, network_config, num_vcpus, htt):
 
     @type: functional
     """
+    if htt and PLATFORM == 'aarch64':
+        pytest.skip("HyperThreading is configurable only on x86.")
     vm = test_microvm_with_api
     vm.spawn()
     vm.basic_config(vcpu_count=num_vcpus, ht_enabled=htt)