From 426790e4001ac7315554f876637298fb37fb85a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Sun, 20 Oct 2024 13:36:58 +0200 Subject: [PATCH 01/16] tests: save metadata on snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is some information in the Microvm class that we don't save in the snapshot. Some tests do depend on those, so to make the booted/restored case homogenous, make room in the snapshot to save metadata that we can then restore. Signed-off-by: Pablo Barbáchano --- tests/framework/microvm.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py index ed02488bc7b..c1be701665a 100644 --- a/tests/framework/microvm.py +++ b/tests/framework/microvm.py @@ -75,6 +75,7 @@ class Snapshot: disks: dict ssh_key: Path snapshot_type: SnapshotType + meta: dict @property def is_diff(self) -> bool: @@ -110,6 +111,7 @@ def copy_to_chroot(self, chroot) -> "Snapshot": disks=self.disks, ssh_key=self.ssh_key, snapshot_type=self.snapshot_type, + meta=self.meta, ) @classmethod @@ -125,6 +127,7 @@ def load_from(cls, src: Path) -> "Snapshot": disks={dsk: src / p for dsk, p in obj["disks"].items()}, ssh_key=src / obj["ssh_key"], snapshot_type=SnapshotType(obj["snapshot_type"]), + meta=obj["meta"], ) def save_to(self, dst: Path): @@ -917,6 +920,9 @@ def make_snapshot( net_ifaces=[x["iface"] for ifname, x in self.iface.items()], ssh_key=self.ssh_key, snapshot_type=snapshot_type, + meta={ + "kernel_file": self.kernel_file, + }, ) def snapshot_diff(self, *, mem_path: str = "mem", vmstate_path="vmstate"): @@ -954,6 +960,9 @@ def restore_from_snapshot( if uffd_path is not None: mem_backend = {"backend_type": "Uffd", "backend_path": str(uffd_path)} + for key, value in snapshot.meta.items(): + setattr(self, key, value) + self.api.snapshot_load.put( mem_backend=mem_backend, snapshot_path=str(jailed_vmstate), From d34f15dca4c5238ca3a1bfdb810168e06ab4dbac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Sun, 20 Oct 2024 12:55:38 +0200 Subject: [PATCH 02/16] tests: refactor test_vulnerabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some further simplifications: - Simplify the "_host" tests as they will provide the same result in both A/B situations. - Add a second MicrovmFactory fixture with the "A" firecracker. - Add a uvm_any fixture that includes all CPU templates and also a boot/restored dimension. This decreases the need for separate tests. For example the guest test test_check_vulnerability_files_ab can now test all variants: 2 (restored/booted) * 3 (kernels) * 9 (cpu templates) = 54 tests Signed-off-by: Pablo Barbáchano --- tests/README.md | 12 +- tests/conftest.py | 35 ++ tests/framework/ab_test.py | 53 +- tests/framework/microvm.py | 18 +- tests/framework/properties.py | 5 +- tests/framework/utils_cpu_templates.py | 4 +- .../integration_tests/functional/test_net.py | 43 +- .../security/test_vulnerabilities.py | 498 +++++------------- 8 files changed, 210 insertions(+), 458 deletions(-) diff --git a/tests/README.md b/tests/README.md index ea46ff56786..bf7aba9a547 100644 --- a/tests/README.md +++ b/tests/README.md @@ -306,10 +306,14 @@ that are pre-initialized with specific guest kernels and rootfs: 24.04 squashfs as rootfs, - `uvm_plain` yields a Firecracker process pre-initialized with a 5.10 kernel and the same Ubuntu 24.04 squashfs. - -Generally, tests should use the former if you are testing some interaction -between the guest and Firecracker, while the latter should be used if -Firecracker functionality unrelated to the guest is being tested. +- `uvm_any` yields started microvms, parametrized by all supported kernels, all + CPU templates (static, custom and none), and either booted or restored from a + snapshot. +- `uvm_any_booted` works the same as `uvm_any`, but only for booted VMs. + +Generally, tests should use `uvm_plain_any` if you are testing some interaction +between the guest and Firecracker, and `uvm_plain` should be used if Firecracker +functionality unrelated to the guest is being tested. ### Markers diff --git a/tests/conftest.py b/tests/conftest.py index 8f4c2e51ff5..5ad291d01a4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -459,3 +459,38 @@ def uvm_with_initrd( uvm = microvm_factory.build(guest_kernel_linux_5_10) uvm.initrd_file = fs yield uvm + + +def uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template): + """Return a booted uvm""" + uvm = microvm_factory.build(guest_kernel, rootfs) + uvm.spawn() + uvm.basic_config(vcpu_count=2, mem_size_mib=256) + uvm.set_cpu_template(cpu_template) + uvm.add_net_iface() + uvm.start() + return uvm + + +def uvm_restored(microvm_factory, guest_kernel, rootfs, cpu_template): + """Return a restored uvm""" + uvm = uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template) + snapshot = uvm.snapshot_full() + uvm.kill() + uvm2 = microvm_factory.build() + uvm2.spawn() + uvm2.restore_from_snapshot(snapshot, resume=True) + uvm2.cpu_template_name = uvm.cpu_template_name + return uvm2 + + +@pytest.fixture(params=[uvm_booted, uvm_restored]) +def uvm_ctor(request): + """Fixture to return uvms with different constructors""" + return request.param + + +@pytest.fixture +def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs, cpu_template_any): + """Return booted and restored uvms""" + return uvm_ctor(microvm_factory, guest_kernel, rootfs, cpu_template_any) diff --git a/tests/framework/ab_test.py b/tests/framework/ab_test.py index cf909d44fa6..f92d62aeaa2 100644 --- a/tests/framework/ab_test.py +++ b/tests/framework/ab_test.py @@ -31,10 +31,9 @@ from framework import utils from framework.defs import FC_WORKSPACE_DIR -from framework.microvm import Microvm from framework.utils import CommandReturn from framework.with_filelock import with_filelock -from host_tools.cargo_build import DEFAULT_TARGET_DIR, get_firecracker_binaries +from host_tools.cargo_build import DEFAULT_TARGET_DIR # Locally, this will always compare against main, even if we try to merge into, say, a feature branch. # We might want to do a more sophisticated way to determine a "parent" branch here. @@ -176,56 +175,6 @@ def set_did_not_grow_comparator( ) -def precompiled_ab_test_guest_command( - microvm_factory: Callable[[Path, Path], Microvm], - command: str, - *, - comparator: Callable[[CommandReturn, CommandReturn], bool] = default_comparator, - a_revision: str = DEFAULT_A_REVISION, - b_revision: Optional[str] = None, -): - """The same as git_ab_test_command, but via SSH. The closure argument should setup a microvm using the passed - paths to firecracker and jailer binaries.""" - b_directory = ( - DEFAULT_B_DIRECTORY - if b_revision is None - else FC_WORKSPACE_DIR / "build" / b_revision - ) - - def test_runner(bin_dir, _is_a: bool): - microvm = microvm_factory(bin_dir / "firecracker", bin_dir / "jailer") - return microvm.ssh.run(command) - - (_, old_out, old_err), (_, new_out, new_err), the_same = binary_ab_test( - test_runner, - comparator, - a_directory=FC_WORKSPACE_DIR / "build" / a_revision, - b_directory=b_directory, - ) - - assert ( - the_same - ), f"The output of running command `{command}` changed:\nOld:\nstdout:\n{old_out}\nstderr\n{old_err}\n\nNew:\nstdout:\n{new_out}\nstderr:\n{new_err}" - - -def precompiled_ab_test_guest_command_if_pr( - microvm_factory: Callable[[Path, Path], Microvm], - command: str, - *, - comparator=default_comparator, - check_in_nonpr=True, -): - """The same as git_ab_test_command_if_pr, but via SSH""" - if is_pr(): - precompiled_ab_test_guest_command( - microvm_factory, command, comparator=comparator - ) - return None - - microvm = microvm_factory(*get_firecracker_binaries()) - return microvm.ssh.run(command, check=check_in_nonpr) - - def check_regression( a_samples: List[float], b_samples: List[float], *, n_resamples: int = 9999 ): diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py index c1be701665a..832f56a877e 100644 --- a/tests/framework/microvm.py +++ b/tests/framework/microvm.py @@ -244,6 +244,7 @@ def __init__( self.disks_vhost_user = {} self.vcpus_count = None self.mem_size_bytes = None + self.cpu_template_name = None self._pre_cmd = [] if numa_node: @@ -735,12 +736,14 @@ def basic_config( smt=smt, mem_size_mib=mem_size_mib, track_dirty_pages=track_dirty_pages, - cpu_template=cpu_template, huge_pages=huge_pages, ) self.vcpus_count = vcpu_count self.mem_size_bytes = mem_size_mib * 2**20 + if cpu_template is not None: + self.set_cpu_template(cpu_template) + if self.memory_monitor: self.memory_monitor.start() @@ -773,6 +776,19 @@ def basic_config( if enable_entropy_device: self.enable_entropy_device() + def set_cpu_template(self, cpu_template): + """Set guest CPU template.""" + if cpu_template is None: + return + # static CPU template + if isinstance(cpu_template, str): + self.api.machine_config.patch(cpu_template=cpu_template) + self.cpu_template_name = cpu_template.lower() + # custom CPU template + elif isinstance(cpu_template, dict): + self.api.cpu_config.put(**cpu_template["template"]) + self.cpu_template_name = cpu_template["name"].lower() + def add_drive( self, drive_id, diff --git a/tests/framework/properties.py b/tests/framework/properties.py index b40df56249e..7dd1cb46588 100644 --- a/tests/framework/properties.py +++ b/tests/framework/properties.py @@ -72,11 +72,14 @@ def __init__(self): # major.minor.patch self.host_linux_patch = get_kernel_version(2) self.os = get_os_version() - self.host_os = get_host_os() + self.host_os = get_host_os() or "NA" self.libc_ver = "-".join(platform.libc_ver()) self.rust_version = run_cmd("rustc --version |awk '{print $2}'") + # Buildkite/PR information self.buildkite_pipeline_slug = os.environ.get("BUILDKITE_PIPELINE_SLUG") self.buildkite_build_number = os.environ.get("BUILDKITE_BUILD_NUMBER") + self.buildkite_pr = os.environ.get("BUILDKITE_PULL_REQUEST", "false") != "false" + self.buildkite_revision_a = os.environ.get("BUILDKITE_PULL_REQUEST_BASE_BRANCH") if self._in_git_repo(): self.git_commit_id = run_cmd("git rev-parse HEAD") diff --git a/tests/framework/utils_cpu_templates.py b/tests/framework/utils_cpu_templates.py index e57ecfe72c7..89326c1da5a 100644 --- a/tests/framework/utils_cpu_templates.py +++ b/tests/framework/utils_cpu_templates.py @@ -3,6 +3,8 @@ """Utilities for CPU template related functionality.""" +# pylint:disable=too-many-return-statements + import json from pathlib import Path @@ -23,9 +25,7 @@ def get_supported_cpu_templates(): """ Return the list of CPU templates supported by the platform. """ - # pylint:disable=too-many-return-statements host_linux = global_props.host_linux_version_tpl - match get_cpu_vendor(), global_props.cpu_codename: # T2CL template is only supported on Cascade Lake and newer CPUs. case CpuVendor.INTEL, CpuModel.INTEL_SKYLAKE: diff --git a/tests/integration_tests/functional/test_net.py b/tests/integration_tests/functional/test_net.py index a804b8f90a8..2072d015ca2 100644 --- a/tests/integration_tests/functional/test_net.py +++ b/tests/integration_tests/functional/test_net.py @@ -84,14 +84,23 @@ def test_multi_queue_unsupported(uvm_plain): ) -def run_udp_offload_test(vm): +@pytest.fixture +def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs): + """Return booted and restored uvm with no CPU templates""" + return uvm_ctor(microvm_factory, guest_kernel, rootfs, None) + + +def test_tap_offload(uvm_any): """ + Verify that tap offload features are configured for a booted/restored VM. + - Start a socat UDP server in the guest. - Try to send a UDP message with UDP offload enabled. If tap offload features are not configured, an attempt to send a message will fail with EIO "Input/output error". More info (search for "TUN_F_CSUM is a must"): https://blog.cloudflare.com/fr-fr/virtual-networking-101-understanding-tap/ """ + vm = uvm_any port = "81" out_filename = "/tmp/out.txt" message = "x" @@ -112,35 +121,3 @@ def run_udp_offload_test(vm): # Check that the server received the message ret = vm.ssh.run(f"cat {out_filename}") assert ret.stdout == message, f"{ret.stdout=} {ret.stderr=}" - - -def test_tap_offload_booted(uvm_plain_any): - """ - Verify that tap offload features are configured for a booted VM. - """ - vm = uvm_plain_any - vm.spawn() - vm.basic_config() - vm.add_net_iface() - vm.start() - - run_udp_offload_test(vm) - - -def test_tap_offload_restored(microvm_factory, guest_kernel, rootfs): - """ - Verify that tap offload features are configured for a restored VM. - """ - src = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False) - src.spawn() - src.basic_config() - src.add_net_iface() - src.start() - snapshot = src.snapshot_full() - src.kill() - - dst = microvm_factory.build() - dst.spawn() - dst.restore_from_snapshot(snapshot, resume=True) - - run_udp_offload_test(dst) diff --git a/tests/integration_tests/security/test_vulnerabilities.py b/tests/integration_tests/security/test_vulnerabilities.py index fed9dbc96d7..7cf9bc78958 100644 --- a/tests/integration_tests/security/test_vulnerabilities.py +++ b/tests/integration_tests/security/test_vulnerabilities.py @@ -5,21 +5,17 @@ # script from the third party "Spectre & Meltdown Checker" project. This script is under the # GPL-3.0-only license. """Tests vulnerabilities mitigations.""" + import json -import os +from pathlib import Path import pytest import requests from framework import utils -from framework.ab_test import ( - is_pr, - precompiled_ab_test_guest_command, - precompiled_ab_test_guest_command_if_pr, - set_did_not_grow_comparator, -) +from framework.ab_test import git_clone +from framework.microvm import MicroVMFactory from framework.properties import global_props -from framework.utils import CommandReturn CHECKER_URL = "https://meltdown.ovh" CHECKER_FILENAME = "spectre-meltdown-checker.sh" @@ -29,119 +25,72 @@ VULN_DIR = "/sys/devices/system/cpu/vulnerabilities" -def configure_microvm( - factory, - kernel, - rootfs, - *, - firecracker=None, - jailer=None, - cpu_template=None, - custom_cpu_template=None, -): - """Build a microvm for vulnerability tests""" - assert not (cpu_template and custom_cpu_template) - # Either both or neither are specified - assert firecracker and jailer or not firecracker and not jailer - - if firecracker: - microvm = factory.build( - kernel, rootfs, fc_binary_path=firecracker, jailer_binary_path=jailer - ) - else: - microvm = factory.build(kernel, rootfs) - - microvm.spawn() - microvm.basic_config(vcpu_count=2, mem_size_mib=256, cpu_template=cpu_template) - if custom_cpu_template: - microvm.api.cpu_config.put(**custom_cpu_template["template"]) - microvm.cpu_template = cpu_template - if cpu_template is None and custom_cpu_template is not None: - microvm.cpu_template = custom_cpu_template["name"] - microvm.add_net_iface() - microvm.start() - return microvm +class SpectreMeltdownChecker: + """Helper class to use Spectre & Meltdown Checker""" + def __init__(self, path): + self.path = path -@pytest.fixture -def build_microvm( - microvm_factory, - guest_kernel_linux_5_10, - rootfs, -): - """Fixture returning a factory function for a normal microvm""" - return lambda firecracker=None, jailer=None: configure_microvm( - microvm_factory, - guest_kernel_linux_5_10, - rootfs, - firecracker=firecracker, - jailer=jailer, - ) - - -@pytest.fixture -def build_microvm_with_template( - microvm_factory, guest_kernel_linux_5_10, rootfs, cpu_template -): - """Fixture returning a factory function for microvms with our built-in template""" - return lambda firecracker=None, jailer=None: configure_microvm( - microvm_factory, - guest_kernel_linux_5_10, - rootfs, - firecracker=firecracker, - jailer=jailer, - cpu_template=cpu_template, - ) - - -@pytest.fixture -def build_microvm_with_custom_template( - microvm_factory, guest_kernel_linux_5_10, rootfs, custom_cpu_template -): - """Fixture returning a factory function for microvms with custom cpu templates""" - return lambda firecracker=None, jailer=None: configure_microvm( - microvm_factory, - guest_kernel_linux_5_10, - rootfs, - firecracker=firecracker, - jailer=jailer, - custom_cpu_template=custom_cpu_template, - ) - - -def with_restore(factory, microvm_factory): - """Turns the given microvm factory into one that makes the microvm go through a snapshot-restore cycle""" - - def restore(firecracker=None, jailer=None): - microvm = factory(firecracker, jailer) - - snapshot = microvm.snapshot_full() - - if firecracker: - dst_vm = microvm_factory.build( - fc_binary_path=firecracker, jailer_binary_path=jailer - ) - else: - dst_vm = microvm_factory.build() - dst_vm.spawn() - # Restore the destination VM from the snapshot - dst_vm.restore_from_snapshot(snapshot, resume=True) - dst_vm.cpu_template = microvm.cpu_template - - return dst_vm - - return restore - - -def with_checker(factory, spectre_meltdown_checker): - """Turns the given microvm factory function into one that also contains the spectre-meltdown checker script""" + def _parse_output(self, output): + return { + json.dumps(entry) # dict is unhashable + for entry in json.loads(output) + if entry["VULNERABLE"] + } - def download_checker(firecracker, jailer): - microvm = factory(firecracker, jailer) - microvm.ssh.scp_put(spectre_meltdown_checker, REMOTE_CHECKER_PATH) - return microvm + def get_report_for_guest(self, vm) -> set: + """Parses the output of `spectre-meltdown-checker.sh --batch json` + and returns the set of issues for which it reported 'Vulnerable'. - return download_checker + Sample stdout: + ``` + [ + { + "NAME": "SPECTRE VARIANT 1", + "CVE": "CVE-2017-5753", + "VULNERABLE": false, + "INFOS": "Mitigation: usercopy/swapgs barriers and __user pointer sanitization" + }, + { ... } + ] + ``` + """ + vm.ssh.scp_put(self.path, REMOTE_CHECKER_PATH) + res = vm.ssh.run(REMOTE_CHECKER_COMMAND) + return self._parse_output(res.stdout) + + def get_report_for_host(self) -> set: + """Runs `spectre-meltdown-checker.sh` in the host and returns the set of + issues for which it reported 'Vulnerable'. + """ + + res = utils.check_output(f"sh {self.path} --batch json") + assert res.returncode == 0 + return self._parse_output(res.stdout) + + def expected_vulnerabilities(self, cpu_template_name): + """ + There is a REPTAR exception reported on INTEL_ICELAKE when spectre-meltdown-checker.sh + script is run inside the guest from below the tests: + test_spectre_meltdown_checker_on_guest and + test_spectre_meltdown_checker_on_restored_guest + The same script when run on host doesn't report the + exception which means the instances are actually not vulnerable to REPTAR. + The only reason why the script cannot determine if the guest + is vulnerable or not because Firecracker does not expose the microcode + version to the guest. + + The check in spectre_meltdown_checker is here: + https://github.com/speed47/spectre-meltdown-checker/blob/0f2edb1a71733c1074550166c5e53abcfaa4d6ca/spectre-meltdown-checker.sh#L6635-L6637 + + Since we have a test on host and the exception in guest is not valid, + we add a check to ignore this exception. + """ + if global_props.cpu_codename == "INTEL_ICELAKE" and cpu_template_name is None: + return { + '{"NAME": "REPTAR", "CVE": "CVE-2023-23583", "VULNERABLE": true, "INFOS": "Your microcode is too old to mitigate the vulnerability"}' + } + return set() @pytest.fixture(scope="session", name="spectre_meltdown_checker") @@ -149,189 +98,32 @@ def download_spectre_meltdown_checker(tmp_path_factory): """Download spectre / meltdown checker script.""" resp = requests.get(CHECKER_URL, timeout=5) resp.raise_for_status() - path = tmp_path_factory.mktemp("tmp", True) / CHECKER_FILENAME path.write_bytes(resp.content) - - return path - - -def spectre_meltdown_reported_vulnerablities( - spectre_meltdown_checker_output: CommandReturn, -) -> set: - """ - Parses the output of `spectre-meltdown-checker.sh --batch json` and returns the set of issues - for which it reported 'Vulnerable'. - - Sample stdout: - ``` - [ - { - "NAME": "SPECTRE VARIANT 1", - "CVE": "CVE-2017-5753", - "VULNERABLE": false, - "INFOS": "Mitigation: usercopy/swapgs barriers and __user pointer sanitization" - }, - { - ... - } - ] - ``` - """ - return { - json.dumps(entry) # dict is unhashable - for entry in json.loads(spectre_meltdown_checker_output.stdout) - if entry["VULNERABLE"] - } - - -def check_vulnerabilities_on_guest(status): - """ - There is a REPTAR exception reported on INTEL_ICELAKE when spectre-meltdown-checker.sh - script is run inside the guest from below the tests: - test_spectre_meltdown_checker_on_guest and - test_spectre_meltdown_checker_on_restored_guest - The same script when run on host doesn't report the - exception which means the instances are actually not vulnerable to REPTAR. - The only reason why the script cannot determine if the guest - is vulnerable or not because Firecracker does not expose the microcode - version to the guest. - - The check in spectre_meltdown_checker is here: - https://github.com/speed47/spectre-meltdown-checker/blob/0f2edb1a71733c1074550166c5e53abcfaa4d6ca/spectre-meltdown-checker.sh#L6635-L6637 - - Since we have a test on host and the exception in guest is not valid, - we add a check to ignore this exception. - """ - report_guest_vulnerabilities = spectre_meltdown_reported_vulnerablities(status) - known_guest_vulnerabilities = set() - if global_props.cpu_codename == "INTEL_ICELAKE": - known_guest_vulnerabilities = { - '{"NAME": "REPTAR", "CVE": "CVE-2023-23583", "VULNERABLE": true, "INFOS": "Your microcode is too old to mitigate the vulnerability"}' - } - assert report_guest_vulnerabilities == known_guest_vulnerabilities + return SpectreMeltdownChecker(path) # Nothing can be sensibly tested in a PR context here @pytest.mark.skipif( - is_pr(), reason="Test depends solely on factors external to GitHub repository" + global_props.buildkite_pr, + reason="Test depends solely on factors external to GitHub repository", ) def test_spectre_meltdown_checker_on_host(spectre_meltdown_checker): - """ - Test with the spectre / meltdown checker on host. - """ - rc, output, _ = utils.run_cmd(f"sh {spectre_meltdown_checker} --batch json") - - if output and rc != 0: - report = spectre_meltdown_reported_vulnerablities(output) - expected = {} - assert report == expected, f"Unexpected vulnerabilities: {report} vs {expected}" - - -def test_spectre_meltdown_checker_on_guest(spectre_meltdown_checker, build_microvm): - """ - Test with the spectre / meltdown checker on guest. - """ - - status = precompiled_ab_test_guest_command_if_pr( - with_checker(build_microvm, spectre_meltdown_checker), - REMOTE_CHECKER_COMMAND, - comparator=set_did_not_grow_comparator( - spectre_meltdown_reported_vulnerablities - ), - check_in_nonpr=False, - ) - if status and status.returncode != 0: - check_vulnerabilities_on_guest(status) - - -def test_spectre_meltdown_checker_on_restored_guest( - spectre_meltdown_checker, build_microvm, microvm_factory -): - """ - Test with the spectre / meltdown checker on a restored guest. - """ - status = precompiled_ab_test_guest_command_if_pr( - with_checker( - with_restore(build_microvm, microvm_factory), spectre_meltdown_checker - ), - REMOTE_CHECKER_COMMAND, - comparator=set_did_not_grow_comparator( - spectre_meltdown_reported_vulnerablities - ), - check_in_nonpr=False, - ) - if status and status.returncode != 0: - check_vulnerabilities_on_guest(status) - - -def test_spectre_meltdown_checker_on_guest_with_template( - spectre_meltdown_checker, build_microvm_with_template -): - """ - Test with the spectre / meltdown checker on guest with CPU template. - """ + """Test with the spectre / meltdown checker on host.""" + report = spectre_meltdown_checker.get_report_for_host() + assert report == set(), f"Unexpected vulnerabilities: {report}" - precompiled_ab_test_guest_command_if_pr( - with_checker(build_microvm_with_template, spectre_meltdown_checker), - REMOTE_CHECKER_COMMAND, - comparator=set_did_not_grow_comparator( - spectre_meltdown_reported_vulnerablities - ), - ) - -def test_spectre_meltdown_checker_on_guest_with_custom_template( - spectre_meltdown_checker, build_microvm_with_custom_template -): - """ - Test with the spectre / meltdown checker on guest with a custom CPU template. - """ - precompiled_ab_test_guest_command_if_pr( - with_checker(build_microvm_with_custom_template, spectre_meltdown_checker), - REMOTE_CHECKER_COMMAND, - comparator=set_did_not_grow_comparator( - spectre_meltdown_reported_vulnerablities - ), - ) - - -def test_spectre_meltdown_checker_on_restored_guest_with_template( - spectre_meltdown_checker, build_microvm_with_template, microvm_factory -): - """ - Test with the spectre / meltdown checker on a restored guest with a CPU template. - """ - precompiled_ab_test_guest_command_if_pr( - with_checker( - with_restore(build_microvm_with_template, microvm_factory), - spectre_meltdown_checker, - ), - REMOTE_CHECKER_COMMAND, - comparator=set_did_not_grow_comparator( - spectre_meltdown_reported_vulnerablities - ), - ) - - -def test_spectre_meltdown_checker_on_restored_guest_with_custom_template( - spectre_meltdown_checker, - build_microvm_with_custom_template, - microvm_factory, -): - """ - Test with the spectre / meltdown checker on a restored guest with a custom CPU template. - """ - precompiled_ab_test_guest_command_if_pr( - with_checker( - with_restore(build_microvm_with_custom_template, microvm_factory), - spectre_meltdown_checker, - ), - REMOTE_CHECKER_COMMAND, - comparator=set_did_not_grow_comparator( - spectre_meltdown_reported_vulnerablities - ), - ) +# Nothing can be sensibly tested here in a PR context +@pytest.mark.skipif( + global_props.buildkite_pr, + reason="Test depends solely on factors external to GitHub repository", +) +def test_vulnerabilities_on_host(): + """Test vulnerability files on host.""" + res = utils.run_cmd(f"grep -r Vulnerable {VULN_DIR}") + # if grep finds no matching lines, it exits with status 1 + assert res.returncode == 1, res.stdout def get_vuln_files_exception_dict(template): @@ -372,25 +164,14 @@ def get_vuln_files_exception_dict(template): # Since those bits are not set on Intel Skylake and C3 template makes guests pretend to be AWS # C3 instance (quite old processor now) by overwriting CPUID.1H:EAX, it is impossible to avoid # this "Unknown" state. - if global_props.cpu_codename == "INTEL_SKYLAKE" and template == "C3": + if global_props.cpu_codename == "INTEL_SKYLAKE" and template == "c3": exception_dict["mmio_stale_data"] = "Unknown: No mitigations" - elif global_props.cpu_codename == "INTEL_SKYLAKE" or template == "T2S": + elif global_props.cpu_codename == "INTEL_SKYLAKE" or template == "t2s": exception_dict["mmio_stale_data"] = "Clear CPU buffers" return exception_dict -# Nothing can be sensibly tested here in a PR context -@pytest.mark.skipif( - is_pr(), reason="Test depends solely on factors external to GitHub repository" -) -def test_vulnerabilities_on_host(): - """ - Test vulnerabilities files on host. - """ - utils.check_output(f"! grep -r Vulnerable {VULN_DIR}") - - def check_vulnerabilities_files_on_guest(microvm): """ Check that the guest's vulnerabilities files do not contain `Vulnerable`. @@ -400,88 +181,75 @@ def check_vulnerabilities_files_on_guest(microvm): # Retrieve a list of vulnerabilities files available inside guests. vuln_dir = "/sys/devices/system/cpu/vulnerabilities" _, stdout, _ = microvm.ssh.check_output(f"find -D all {vuln_dir} -type f") - vuln_files = stdout.split("\n") + vuln_files = stdout.splitlines() # Fixtures in this file (test_vulnerabilities.py) add this special field. - template = microvm.cpu_template + template = microvm.cpu_template_name # Check that vulnerabilities files in the exception dictionary have the expected values and # the others do not contain "Vulnerable". exceptions = get_vuln_files_exception_dict(template) + results = [] for vuln_file in vuln_files: - filename = os.path.basename(vuln_file) + filename = Path(vuln_file).name if filename in exceptions: _, stdout, _ = microvm.ssh.run(f"cat {vuln_file}") assert exceptions[filename] in stdout else: cmd = f"grep Vulnerable {vuln_file}" - ecode, stdout, stderr = microvm.ssh.run(cmd) - assert ecode == 1, f"{vuln_file}: stdout:\n{stdout}\nstderr:\n{stderr}\n" - - -def check_vulnerabilities_files_ab(builder): - """Does an A/B test on the contents of the /sys/devices/system/cpu/vulnerabilities files in the guest if - running in a PR pipeline, and otherwise calls `check_vulnerabilities_files_on_guest` - """ - if is_pr(): - precompiled_ab_test_guest_command( - builder, - f"! grep -r Vulnerable {VULN_DIR}", - comparator=set_did_not_grow_comparator( - lambda output: set(output.stdout.splitlines()) - ), - ) - else: - check_vulnerabilities_files_on_guest(builder()) + _ecode, stdout, _stderr = microvm.ssh.run(cmd) + results.append({"file": vuln_file, "stdout": stdout}) + return results -def test_vulnerabilities_files_on_guest(build_microvm): - """ - Test vulnerabilities files on guest. - """ - check_vulnerabilities_files_ab(build_microvm) - - -def test_vulnerabilities_files_on_restored_guest(build_microvm, microvm_factory): - """ - Test vulnerabilities files on a restored guest. - """ - check_vulnerabilities_files_ab(with_restore(build_microvm, microvm_factory)) +@pytest.fixture +def microvm_factory_a(record_property): + """MicroVMFactory using revision A binaries""" + revision_a = global_props.buildkite_revision_a + bin_dir = git_clone(Path("../build") / revision_a, revision_a).resolve() + fc_bin = bin_dir / "firecracker" + jailer_bin = bin_dir / "jailer" + record_property("firecracker_bin", str(fc_bin)) + uvm_factory = MicroVMFactory(fc_bin, jailer_bin) + yield uvm_factory + uvm_factory.kill() -def test_vulnerabilities_files_on_guest_with_template(build_microvm_with_template): - """ - Test vulnerabilities files on guest with CPU template. - """ - check_vulnerabilities_files_ab(build_microvm_with_template) - +@pytest.fixture +def uvm_any_a(microvm_factory_a, uvm_ctor, guest_kernel, rootfs, cpu_template_any): + """Return uvm with revision A firecracker -def test_vulnerabilities_files_on_guest_with_custom_template( - build_microvm_with_custom_template, -): + Since pytest caches fixtures, this guarantees uvm_any_a will match a vm from uvm_any. + See https://docs.pytest.org/en/stable/how-to/fixtures.html#fixtures-can-be-requested-more-than-once-per-test-return-values-are-cached """ - Test vulnerabilities files on guest with a custom CPU template. - """ - check_vulnerabilities_files_ab(build_microvm_with_custom_template) + return uvm_ctor(microvm_factory_a, guest_kernel, rootfs, cpu_template_any) -def test_vulnerabilities_files_on_restored_guest_with_template( - build_microvm_with_template, microvm_factory -): - """ - Test vulnerabilities files on a restored guest with a CPU template. - """ - check_vulnerabilities_files_ab( - with_restore(build_microvm_with_template, microvm_factory) - ) +def test_check_vulnerability_files_ab(request, uvm_any): + """Test vulnerability files on guests""" + res_b = check_vulnerabilities_files_on_guest(uvm_any) + if global_props.buildkite_pr: + # we only get the uvm_any_a fixtures if we need it + uvm_a = request.getfixturevalue("uvm_any_a") + res_a = check_vulnerabilities_files_on_guest(uvm_a) + assert res_b <= res_a + else: + assert not [x for x in res_b if "Vulnerable" in x["stdout"]] -def test_vulnerabilities_files_on_restored_guest_with_custom_template( - build_microvm_with_custom_template, microvm_factory +def test_spectre_meltdown_checker_on_guest( + request, + uvm_any, + spectre_meltdown_checker, ): - """ - Test vulnerabilities files on a restored guest with a custom CPU template. - """ - check_vulnerabilities_files_ab( - with_restore(build_microvm_with_custom_template, microvm_factory) - ) + """Test with the spectre / meltdown checker on any supported guest.""" + res_b = spectre_meltdown_checker.get_report_for_guest(uvm_any) + if global_props.buildkite_pr: + # we only get the uvm_any_a fixtures if we need it + uvm_a = request.getfixturevalue("uvm_any_a") + res_a = spectre_meltdown_checker.get_report_for_guest(uvm_a) + assert res_b <= res_a + else: + assert res_b == spectre_meltdown_checker.expected_vulnerabilities( + uvm_any.cpu_template_name + ) From 41b7341071cb720eb8e0efb98836cef45c8952a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Thu, 31 Oct 2024 22:49:26 +0100 Subject: [PATCH 03/16] tests: refactor test_nv.py to use uvm_any_booted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a new uvm_any_booted fixture to make the test simpler. Signed-off-by: Pablo Barbáchano --- tests/conftest.py | 6 +++++ tests/integration_tests/security/test_nv.py | 29 +++------------------ 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5ad291d01a4..a2990a56c98 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -494,3 +494,9 @@ def uvm_ctor(request): def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs, cpu_template_any): """Return booted and restored uvms""" return uvm_ctor(microvm_factory, guest_kernel, rootfs, cpu_template_any) + + +@pytest.fixture +def uvm_any_booted(microvm_factory, guest_kernel, rootfs, cpu_template_any): + """Return booted uvms""" + return uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template_any) diff --git a/tests/integration_tests/security/test_nv.py b/tests/integration_tests/security/test_nv.py index 5dd5acb2308..5d874cb57a6 100644 --- a/tests/integration_tests/security/test_nv.py +++ b/tests/integration_tests/security/test_nv.py @@ -16,31 +16,8 @@ start providing the feature by mistake. """ -import pytest - -@pytest.fixture -def uvm_with_cpu_template(microvm_factory, guest_kernel, rootfs, cpu_template_any): - """A microvm fixture parametrized with all possible templates""" - vm = microvm_factory.build(guest_kernel, rootfs) - vm.spawn() - cpu_template = None - if isinstance(cpu_template_any, str): - cpu_template = cpu_template_any - vm.basic_config(cpu_template=cpu_template) - if isinstance(cpu_template_any, dict): - vm.api.cpu_config.put(**cpu_template_any["template"]) - vm.add_net_iface() - vm.start() - yield vm - - -def test_no_nv_when_using_cpu_templates(uvm_with_cpu_template): - """ - Double-check that guests using CPU templates don't have Nested Virtualization - enabled. - """ - - vm = uvm_with_cpu_template - rc, _, _ = vm.ssh.run("[ ! -e /dev/kvm ]") +def test_no_nv(uvm_any_booted): + """Validate that guests don't have Nested Virtualization enabled.""" + rc, _, _ = uvm_any_booted.ssh.run("[ ! -e /dev/kvm ]") assert rc == 0, "/dev/kvm exists" From 451238a43ee63f795eb54ca39f82a5f417b97e28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Tue, 29 Oct 2024 16:06:17 +0100 Subject: [PATCH 04/16] tests: refactor test_cpu_features_aarch64 to use uvm_any MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the new uvm_any fixture to make the tests simpler Signed-off-by: Pablo Barbáchano --- tests/framework/utils_cpu_templates.py | 10 +- .../functional/test_cpu_features_aarch64.py | 157 ++++-------------- 2 files changed, 34 insertions(+), 133 deletions(-) diff --git a/tests/framework/utils_cpu_templates.py b/tests/framework/utils_cpu_templates.py index 89326c1da5a..56021a21aaa 100644 --- a/tests/framework/utils_cpu_templates.py +++ b/tests/framework/utils_cpu_templates.py @@ -22,9 +22,7 @@ def get_supported_cpu_templates(): - """ - Return the list of CPU templates supported by the platform. - """ + """Return the list of static CPU templates supported by the platform.""" host_linux = global_props.host_linux_version_tpl match get_cpu_vendor(), global_props.cpu_codename: # T2CL template is only supported on Cascade Lake and newer CPUs. @@ -44,12 +42,8 @@ def get_supported_cpu_templates(): def get_supported_custom_cpu_templates(): - """ - Return the list of custom CPU templates supported by the platform. - """ - # pylint:disable=too-many-return-statements + """Return the list of custom CPU templates supported by the platform.""" host_linux = global_props.host_linux_version_tpl - match get_cpu_vendor(), global_props.cpu_codename: # T2CL template is only supported on Cascade Lake and newer CPUs. case CpuVendor.INTEL, CpuModel.INTEL_SKYLAKE: diff --git a/tests/integration_tests/functional/test_cpu_features_aarch64.py b/tests/integration_tests/functional/test_cpu_features_aarch64.py index 8357f54b568..1edcb39f151 100644 --- a/tests/integration_tests/functional/test_cpu_features_aarch64.py +++ b/tests/integration_tests/functional/test_cpu_features_aarch64.py @@ -3,83 +3,63 @@ """Tests for the CPU features for aarch64.""" import os -import platform -import re import pytest -import framework.utils_cpuid as cpuid_utils from framework import utils from framework.properties import global_props from framework.utils_cpuid import CPU_FEATURES_CMD, CpuModel -PLATFORM = platform.machine() +pytestmark = pytest.mark.skipif( + global_props.cpu_architecture != "aarch64", reason="Only run in aarch64" +) -DEFAULT_G2_FEATURES = set( +G2_FEATS = set( ( "fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp " "asimdhp cpuid asimdrdm lrcpc dcpop asimddp ssbs" - ).split(" ") + ).split() ) -DEFAULT_G3_FEATURES_5_10 = DEFAULT_G2_FEATURES | set( - "sha512 asimdfhm dit uscat ilrcpc flagm jscvt fcma sha3 sm3 sm4 rng dcpodp i8mm bf16 dgh".split( - " " - ) +G3_FEATS = G2_FEATS | set( + "sha512 asimdfhm dit uscat ilrcpc flagm jscvt fcma sha3 sm3 sm4 rng dcpodp i8mm bf16 dgh".split() ) -DEFAULT_G3_FEATURES_WITH_SVE_AND_PAC_5_10 = DEFAULT_G3_FEATURES_5_10 | set( - "paca pacg sve svebf16 svei8mm".split(" ") -) +G3_SVE_AND_PAC = set("paca pacg sve svebf16 svei8mm".split()) -DEFAULT_G3_FEATURES_V1N1 = DEFAULT_G2_FEATURES +def test_guest_cpu_features(uvm_any): + """Check the CPU features for a microvm with different CPU templates""" -def _check_cpu_features_arm(test_microvm, guest_kv, template_name=None): - expected_cpu_features = {"Flags": []} - match cpuid_utils.get_cpu_model_name(), guest_kv, template_name: - case CpuModel.ARM_NEOVERSE_N1, _, "v1n1": - expected_cpu_features = DEFAULT_G2_FEATURES - case CpuModel.ARM_NEOVERSE_N1, _, None: - expected_cpu_features = DEFAULT_G2_FEATURES + vm = uvm_any + expected_cpu_features = set() + match global_props.cpu_model, vm.cpu_template_name: + case CpuModel.ARM_NEOVERSE_N1, "v1n1": + expected_cpu_features = G2_FEATS + case CpuModel.ARM_NEOVERSE_N1, None: + expected_cpu_features = G2_FEATS # [cm]7g with guest kernel 5.10 and later - case CpuModel.ARM_NEOVERSE_V1, _, "v1n1": - expected_cpu_features = DEFAULT_G3_FEATURES_V1N1 - case CpuModel.ARM_NEOVERSE_V1, _, "aarch64_with_sve_and_pac": - expected_cpu_features = DEFAULT_G3_FEATURES_WITH_SVE_AND_PAC_5_10 - case CpuModel.ARM_NEOVERSE_V1, _, None: - expected_cpu_features = DEFAULT_G3_FEATURES_5_10 - - _, stdout, _ = test_microvm.ssh.check_output(CPU_FEATURES_CMD) - flags = set(stdout.strip().split(" ")) - assert flags == expected_cpu_features + case CpuModel.ARM_NEOVERSE_V1, "v1n1": + expected_cpu_features = G2_FEATS + case CpuModel.ARM_NEOVERSE_V1, "aarch64_with_sve_and_pac": + expected_cpu_features = G3_FEATS | G3_SVE_AND_PAC + case CpuModel.ARM_NEOVERSE_V1, None: + expected_cpu_features = G3_FEATS + guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split()) + assert guest_feats == expected_cpu_features -def get_cpu_template_dir(cpu_template): - """ - Utility function to return a valid string which will be used as - name of the directory where snapshot artifacts are stored during - snapshot test and loaded from during restore test. - """ - return cpu_template if cpu_template else "none" - - -@pytest.mark.skipif( - PLATFORM != "aarch64", - reason="This is aarch64 specific test.", -) -def test_host_vs_guest_cpu_features_aarch64(uvm_nano): +def test_host_vs_guest_cpu_features(uvm_nano): """Check CPU features host vs guest""" vm = uvm_nano vm.add_net_iface() vm.start() - host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.strip().split(" ")) - guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.strip().split(" ")) - - cpu_model = cpuid_utils.get_cpu_model_name() + host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.split()) + guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split()) + cpu_model = global_props.cpu_model match cpu_model: case CpuModel.ARM_NEOVERSE_N1: expected_guest_minus_host = set() @@ -141,79 +121,6 @@ def test_host_vs_guest_cpu_features_aarch64(uvm_nano): assert guest_feats - host_feats == expected_guest_minus_host case _: if os.environ.get("BUILDKITE") is not None: - assert False, f"Cpu model {cpu_model} is not supported" - - -@pytest.mark.skipif( - PLATFORM != "aarch64", - reason="This is aarch64 specific test.", -) -def test_default_cpu_features(microvm_factory, guest_kernel, rootfs): - """ - Check the CPU features for a microvm with the specified config. - """ - - vm = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False) - vm.spawn() - vm.basic_config() - vm.add_net_iface() - vm.start() - guest_kv = re.search(r"vmlinux-(\d+\.\d+)", guest_kernel.name).group(1) - _check_cpu_features_arm(vm, guest_kv) - - -@pytest.mark.skipif( - PLATFORM != "aarch64", - reason="This is aarch64 specific test.", -) -def test_cpu_features_with_static_template( - microvm_factory, guest_kernel, rootfs, cpu_template -): - """ - Check the CPU features for a microvm with the specified config. - """ - - vm = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False) - vm.spawn() - vm.basic_config(cpu_template=cpu_template) - vm.add_net_iface() - vm.start() - guest_kv = re.search(r"vmlinux-(\d+\.\d+)", guest_kernel.name).group(1) - _check_cpu_features_arm(vm, guest_kv, "v1n1") - - # Check that cpu features are still correct - # after snap/restore cycle. - snapshot = vm.snapshot_full() - restored_vm = microvm_factory.build() - restored_vm.spawn() - restored_vm.restore_from_snapshot(snapshot, resume=True) - _check_cpu_features_arm(restored_vm, guest_kv, "v1n1") - - -@pytest.mark.skipif( - PLATFORM != "aarch64", - reason="This is aarch64 specific test.", -) -def test_cpu_features_with_custom_template( - microvm_factory, guest_kernel, rootfs, custom_cpu_template -): - """ - Check the CPU features for a microvm with the specified config. - """ - - vm = microvm_factory.build(guest_kernel, rootfs, monitor_memory=False) - vm.spawn() - vm.basic_config() - vm.api.cpu_config.put(**custom_cpu_template["template"]) - vm.add_net_iface() - vm.start() - guest_kv = re.search(r"vmlinux-(\d+\.\d+)", guest_kernel.name).group(1) - _check_cpu_features_arm(vm, guest_kv, custom_cpu_template["name"]) - - # Check that cpu features are still correct - # after snap/restore cycle. - snapshot = vm.snapshot_full() - restored_vm = microvm_factory.build() - restored_vm.spawn() - restored_vm.restore_from_snapshot(snapshot, resume=True) - _check_cpu_features_arm(restored_vm, guest_kv, custom_cpu_template["name"]) + assert ( + False + ), f"Cpu model {cpu_model} is not supported, please onboard it." From 582498c2e1dd4ed0066b00252d10637b4092c022 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Mon, 18 Nov 2024 22:31:29 +0100 Subject: [PATCH 05/16] tests: skip x86_64 tests at module level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All tests in the file are x86_64 specific, so do the skip at the top-level, once. Signed-off-by: Pablo Barbáchano --- .../functional/test_cpu_features_x86_64.py | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/tests/integration_tests/functional/test_cpu_features_x86_64.py b/tests/integration_tests/functional/test_cpu_features_x86_64.py index 84d84bd8659..bb86df2eade 100644 --- a/tests/integration_tests/functional/test_cpu_features_x86_64.py +++ b/tests/integration_tests/functional/test_cpu_features_x86_64.py @@ -30,6 +30,10 @@ ) DATA_FILES = Path("./data/msr") +pytestmark = pytest.mark.skipif( + global_props.cpu_architecture != "x86_64", reason="Only run in x86_64" +) + def read_msr_csv(fd): """Read a CSV of MSRs""" @@ -105,7 +109,6 @@ def skip_test_based_on_artifacts(snapshot_artifacts_dir): pytest.skip(re.sub(" +", " ", reason)) -@pytest.mark.skipif(PLATFORM != "x86_64", reason="CPUID is only supported on x86_64.") @pytest.mark.parametrize( "num_vcpus", [1, 2, 16], @@ -126,7 +129,6 @@ def test_cpuid(uvm_plain_any, num_vcpus, htt): _check_cpuid_x86(vm, num_vcpus, "true" if num_vcpus > 1 else "false") -@pytest.mark.skipif(PLATFORM != "x86_64", reason="CPUID is only supported on x86_64.") @pytest.mark.skipif( cpuid_utils.get_cpu_vendor() != cpuid_utils.CpuVendor.AMD, reason="L3 cache info is only present in 0x80000006 for AMD", @@ -143,9 +145,6 @@ def test_extended_cache_features(uvm_plain_any): _check_extended_cache_features(vm) -@pytest.mark.skipif( - PLATFORM != "x86_64", reason="The CPU brand string is masked only on x86_64." -) def test_brand_string(uvm_plain_any): """ Ensure good formatting for the guest brand string. @@ -204,10 +203,6 @@ def test_brand_string(uvm_plain_any): assert False -@pytest.mark.skipif( - PLATFORM != "x86_64", - reason="This is x86_64 specific test.", -) def test_host_vs_guest_cpu_features_x86_64(uvm_nano): """Check CPU features host vs guest""" @@ -929,9 +924,6 @@ def test_cpu_cpuid_restore(microvm_factory, guest_kernel, msr_cpu_template): ) -@pytest.mark.skipif( - PLATFORM != "x86_64", reason="CPU features are masked only on x86_64." -) @pytest.mark.parametrize("cpu_template", ["T2", "T2S", "C3"]) def test_cpu_template(uvm_plain_any, cpu_template, microvm_factory): """ @@ -1249,7 +1241,6 @@ def check_enabled_features(test_microvm, cpu_template): ) -@pytest.mark.skipif(PLATFORM != "x86_64", reason="This test is specific to x86_64.") def test_c3_on_skylake_show_warning(uvm_plain, cpu_template): """ This test verifies that the warning message about MMIO stale data mitigation From 14e6739223b30a891a585d4b8f57bf4dfdfd56d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Mon, 18 Nov 2024 22:32:55 +0100 Subject: [PATCH 06/16] tests: refactor test_cpu_features_x86_64.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the common feature flags to get some clarity on what are the differences between the CPUs. Signed-off-by: Pablo Barbáchano --- .../functional/test_cpu_features_x86_64.py | 203 +++++------------- 1 file changed, 56 insertions(+), 147 deletions(-) diff --git a/tests/integration_tests/functional/test_cpu_features_x86_64.py b/tests/integration_tests/functional/test_cpu_features_x86_64.py index bb86df2eade..f8bbe1ef52e 100644 --- a/tests/integration_tests/functional/test_cpu_features_x86_64.py +++ b/tests/integration_tests/functional/test_cpu_features_x86_64.py @@ -203,6 +203,57 @@ def test_brand_string(uvm_plain_any): assert False +INTEL_HOST_ONLY_FEATS = { + "acpi", + "aperfmperf", + "arch_perfmon", + "art", + "bts", + "cat_l3", + "cdp_l3", + "cqm", + "cqm_llc", + "cqm_mbm_local", + "cqm_mbm_total", + "cqm_occup_llc", + "dca", + "ds_cpl", + "dtes64", + "dtherm", + "dts", + "epb", + "ept", + "ept_ad", + "est", + "flexpriority", + "flush_l1d", + "hwp", + "hwp_act_window", + "hwp_epp", + "hwp_pkg_req", + "ida", + "intel_ppin", + "intel_pt", + "mba", + "monitor", + "pbe", + "pdcm", + "pebs", + "pln", + "pts", + "rdt_a", + "sdbg", + "smx", + "tm", + "tm2", + "tpr_shadow", + "vmx", + "vnmi", + "vpid", + "xtpr", +} + + def test_host_vs_guest_cpu_features_x86_64(uvm_nano): """Check CPU features host vs guest""" @@ -283,110 +334,14 @@ def test_host_vs_guest_cpu_features_x86_64(uvm_nano): "tsc_known_freq", } case CpuModel.INTEL_SKYLAKE: - assert host_feats - guest_feats == { - "acpi", - "aperfmperf", - "arch_perfmon", - "art", - "bts", - "cat_l3", - "cdp_l3", - "cqm", - "cqm_llc", - "cqm_mbm_local", - "cqm_mbm_total", - "cqm_occup_llc", - "dca", - "ds_cpl", - "dtes64", - "dtherm", - "dts", - "epb", - "ept", - "ept_ad", - "est", - "flexpriority", - "flush_l1d", - "hwp", - "hwp_act_window", - "hwp_epp", - "hwp_pkg_req", - "ida", - "intel_ppin", - "intel_pt", - "mba", - "monitor", - "pbe", - "pdcm", - "pebs", - "pln", - "pts", - "rdt_a", - "sdbg", - "smx", - "tm", - "tm2", - "tpr_shadow", - "vmx", - "vnmi", - "vpid", - "xtpr", - } + assert host_feats - guest_feats == INTEL_HOST_ONLY_FEATS assert guest_feats - host_feats == { "hypervisor", "tsc_known_freq", "umip", } case CpuModel.INTEL_CASCADELAKE: - expected_host_minus_guest = { - "acpi", - "aperfmperf", - "arch_perfmon", - "art", - "bts", - "cat_l3", - "cdp_l3", - "cqm", - "cqm_llc", - "cqm_mbm_local", - "cqm_mbm_total", - "cqm_occup_llc", - "dca", - "ds_cpl", - "dtes64", - "dtherm", - "dts", - "epb", - "ept", - "ept_ad", - "est", - "flexpriority", - "flush_l1d", - "hwp", - "hwp_act_window", - "hwp_epp", - "hwp_pkg_req", - "ida", - "intel_ppin", - "intel_pt", - "mba", - "monitor", - "pbe", - "pdcm", - "pebs", - "pln", - "pts", - "rdt_a", - "sdbg", - "smx", - "tm", - "tm2", - "tpr_shadow", - "vmx", - "vnmi", - "vpid", - "xtpr", - } + expected_host_minus_guest = INTEL_HOST_ONLY_FEATS expected_guest_minus_host = { "hypervisor", "tsc_known_freq", @@ -414,56 +369,10 @@ def test_host_vs_guest_cpu_features_x86_64(uvm_nano): assert host_feats - guest_feats == expected_host_minus_guest assert guest_feats - host_feats == expected_guest_minus_host case CpuModel.INTEL_ICELAKE: - host_guest_diff_5_10 = { - "dtes64", - "hwp_act_window", - "pdcm", - "acpi", - "aperfmperf", - "arch_perfmon", - "art", - "bts", - "cat_l3", - "cqm", - "cqm_llc", - "cqm_mbm_local", - "cqm_mbm_total", - "cqm_occup_llc", - "dca", - "ds_cpl", - "dtherm", - "dts", - "epb", - "ept", - "ept_ad", - "est", - "flexpriority", - "flush_l1d", - "hwp", - "hwp_epp", - "hwp_pkg_req", - "ida", - "intel_ppin", - "intel_pt", - "mba", - "monitor", - "pbe", + host_guest_diff_5_10 = INTEL_HOST_ONLY_FEATS - {"cdp_l3"} | { "pconfig", - "pebs", - "pln", - "pts", - "rdt_a", - "sdbg", - "smx", - "split_lock_detect", - "tm", - "tm2", "tme", - "tpr_shadow", - "vmx", - "vnmi", - "vpid", - "xtpr", + "split_lock_detect", } host_guest_diff_6_1 = host_guest_diff_5_10 - { "bts", @@ -1261,7 +1170,7 @@ def test_c3_on_skylake_show_warning(uvm_plain, cpu_template): "does not apply the mitigation against MMIO stale data " "vulnerability." ) - if cpu_template == "C3" and global_props.cpu_codename == "INTEL_SKYLAKE": + if uvm.cpu_template_name == "c3" and global_props.cpu_codename == "INTEL_SKYLAKE": assert message in uvm.log_data else: assert message not in uvm.log_data From 40b953d12e14ed14c186d3606acf98d5e8f1516a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Mon, 18 Nov 2024 23:10:09 +0100 Subject: [PATCH 07/16] tests: move host vs guest cpu features into a single test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the same test spread over two files. Putting it together in a new file so they don't drift over time. Signed-off-by: Pablo Barbáchano --- .../functional/test_cpu_features_aarch64.py | 78 ----- .../test_cpu_features_host_vs_guest.py | 272 ++++++++++++++++++ .../functional/test_cpu_features_x86_64.py | 198 ------------- 3 files changed, 272 insertions(+), 276 deletions(-) create mode 100644 tests/integration_tests/functional/test_cpu_features_host_vs_guest.py diff --git a/tests/integration_tests/functional/test_cpu_features_aarch64.py b/tests/integration_tests/functional/test_cpu_features_aarch64.py index 1edcb39f151..2068194a894 100644 --- a/tests/integration_tests/functional/test_cpu_features_aarch64.py +++ b/tests/integration_tests/functional/test_cpu_features_aarch64.py @@ -2,11 +2,8 @@ # SPDX-License-Identifier: Apache-2.0 """Tests for the CPU features for aarch64.""" -import os - import pytest -from framework import utils from framework.properties import global_props from framework.utils_cpuid import CPU_FEATURES_CMD, CpuModel @@ -49,78 +46,3 @@ def test_guest_cpu_features(uvm_any): guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split()) assert guest_feats == expected_cpu_features - - -def test_host_vs_guest_cpu_features(uvm_nano): - """Check CPU features host vs guest""" - - vm = uvm_nano - vm.add_net_iface() - vm.start() - host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.split()) - guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split()) - cpu_model = global_props.cpu_model - match cpu_model: - case CpuModel.ARM_NEOVERSE_N1: - expected_guest_minus_host = set() - expected_host_minus_guest = set() - - # Upstream kernel v6.11+ hides "ssbs" from "lscpu" on Neoverse-N1 and Neoverse-V1 since - # they have an errata whereby an MSR to the SSBS special-purpose register does not - # affect subsequent speculative instructions, permitting speculative store bypassing for - # a window of time. - # https://github.com/torvalds/linux/commit/adeec61a4723fd3e39da68db4cc4d924e6d7f641 - # - # While Amazon Linux kernels (v5.10 and v6.1) backported the above commit, our test - # ubuntu kernel (v6.8) and our guest kernels (v5.10 and v6.1) don't pick it. - host_has_ssbs = global_props.host_os not in { - "amzn2", - "amzn2023", - } and global_props.host_linux_version_tpl < (6, 11) - guest_has_ssbs = vm.guest_kernel_version < (6, 11) - - if host_has_ssbs and not guest_has_ssbs: - expected_host_minus_guest |= {"ssbs"} - if not host_has_ssbs and guest_has_ssbs: - expected_guest_minus_host |= {"ssbs"} - - assert host_feats - guest_feats == expected_host_minus_guest - assert guest_feats - host_feats == expected_guest_minus_host - case CpuModel.ARM_NEOVERSE_V1: - expected_guest_minus_host = set() - # KVM does not enable PAC or SVE features by default - # and Firecracker does not enable them either. - expected_host_minus_guest = { - "paca", - "pacg", - "sve", - "svebf16", - "svei8mm", - } - - # Upstream kernel v6.11+ hides "ssbs" from "lscpu" on Neoverse-N1 and Neoverse-V1 since - # they have an errata whereby an MSR to the SSBS special-purpose register does not - # affect subsequent speculative instructions, permitting speculative store bypassing for - # a window of time. - # https://github.com/torvalds/linux/commit/adeec61a4723fd3e39da68db4cc4d924e6d7f641 - # - # While Amazon Linux kernels (v5.10 and v6.1) backported the above commit, our test - # ubuntu kernel (v6.8) and our guest kernels (v5.10 and v6.1) don't pick it. - host_has_ssbs = global_props.host_os not in { - "amzn2", - "amzn2023", - } and global_props.host_linux_version_tpl < (6, 11) - guest_has_ssbs = vm.guest_kernel_version < (6, 11) - - if host_has_ssbs and not guest_has_ssbs: - expected_host_minus_guest |= {"ssbs"} - if not host_has_ssbs and guest_has_ssbs: - expected_guest_minus_host |= {"ssbs"} - - assert host_feats - guest_feats == expected_host_minus_guest - assert guest_feats - host_feats == expected_guest_minus_host - case _: - if os.environ.get("BUILDKITE") is not None: - assert ( - False - ), f"Cpu model {cpu_model} is not supported, please onboard it." diff --git a/tests/integration_tests/functional/test_cpu_features_host_vs_guest.py b/tests/integration_tests/functional/test_cpu_features_host_vs_guest.py new file mode 100644 index 00000000000..bbf4ed57d21 --- /dev/null +++ b/tests/integration_tests/functional/test_cpu_features_host_vs_guest.py @@ -0,0 +1,272 @@ +# Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +# pylint: disable=too-many-statements + +""" +Check CPU features in the host vs the guest. + +This test can highlight differences between the host and what the guest sees. + +No CPU templates as we are interested only on what is passed through to the guest by default. +For that, check test_feat_parity.py +""" + +import os + +from framework import utils +from framework.properties import global_props +from framework.utils_cpuid import CPU_FEATURES_CMD, CpuModel + +CPU_MODEL = global_props.cpu_codename + +INTEL_HOST_ONLY_FEATS = { + "acpi", + "aperfmperf", + "arch_perfmon", + "art", + "bts", + "cat_l3", + "cdp_l3", + "cqm", + "cqm_llc", + "cqm_mbm_local", + "cqm_mbm_total", + "cqm_occup_llc", + "dca", + "ds_cpl", + "dtes64", + "dtherm", + "dts", + "epb", + "ept", + "ept_ad", + "est", + "flexpriority", + "flush_l1d", + "hwp", + "hwp_act_window", + "hwp_epp", + "hwp_pkg_req", + "ida", + "intel_ppin", + "intel_pt", + "mba", + "monitor", + "pbe", + "pdcm", + "pebs", + "pln", + "pts", + "rdt_a", + "sdbg", + "smx", + "tm", + "tm2", + "tpr_shadow", + "vmx", + "vnmi", + "vpid", + "xtpr", +} + + +def test_host_vs_guest_cpu_features(uvm_nano): + """Check CPU features host vs guest""" + + vm = uvm_nano + vm.add_net_iface() + vm.start() + host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.split()) + guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.split()) + + match CPU_MODEL: + case CpuModel.AMD_MILAN: + host_guest_diff_5_10 = { + "amd_ppin", + "aperfmperf", + "bpext", + "cat_l3", + "cdp_l3", + "cpb", + "cqm", + "cqm_llc", + "cqm_mbm_local", + "cqm_mbm_total", + "cqm_occup_llc", + "decodeassists", + "extapic", + "extd_apicid", + "flushbyasid", + "hw_pstate", + "ibs", + "irperf", + "lbrv", + "mba", + "monitor", + "mwaitx", + "overflow_recov", + "pausefilter", + "perfctr_llc", + "perfctr_nb", + "pfthreshold", + "rdpru", + "rdt_a", + "sev", + "sev_es", + "skinit", + "smca", + "sme", + "succor", + "svm_lock", + "tce", + "tsc_scale", + "v_vmsave_vmload", + "vgif", + "vmcb_clean", + "wdt", + } + + host_guest_diff_6_1 = host_guest_diff_5_10 - { + "lbrv", + "pausefilter", + "pfthreshold", + "sme", + "tsc_scale", + "v_vmsave_vmload", + "vgif", + "vmcb_clean", + } | {"brs", "rapl", "v_spec_ctrl"} + + if global_props.host_linux_version_tpl < (6, 1): + assert host_feats - guest_feats == host_guest_diff_5_10 + else: + assert host_feats - guest_feats == host_guest_diff_6_1 + + assert guest_feats - host_feats == { + "hypervisor", + "tsc_adjust", + "tsc_deadline_timer", + "tsc_known_freq", + } + + case CpuModel.INTEL_SKYLAKE: + assert host_feats - guest_feats == INTEL_HOST_ONLY_FEATS + assert guest_feats - host_feats == { + "hypervisor", + "tsc_known_freq", + "umip", + } + + case CpuModel.INTEL_CASCADELAKE: + expected_host_minus_guest = INTEL_HOST_ONLY_FEATS + expected_guest_minus_host = { + "hypervisor", + "tsc_known_freq", + "umip", + } + + # Linux kernel v6.4+ passes through the CPUID bit for "flush_l1d" to guests. + # https://github.com/torvalds/linux/commit/45cf86f26148e549c5ba4a8ab32a390e4bde216e + # + # Our test ubuntu host kernel is v6.8 and has the commit. + if global_props.host_linux_version_tpl >= (6, 4): + expected_host_minus_guest -= {"flush_l1d"} + + # Linux kernel v6.6+ drops the "invpcid_single" synthetic feature bit. + # https://github.com/torvalds/linux/commit/54e3d9434ef61b97fd3263c141b928dc5635e50d + # + # Our test ubuntu host kernel is v6.8 and has the commit. + host_has_invpcid_single = global_props.host_linux_version_tpl < (6, 6) + guest_has_invpcid_single = vm.guest_kernel_version < (6, 6) + if host_has_invpcid_single and not guest_has_invpcid_single: + expected_host_minus_guest |= {"invpcid_single"} + if not host_has_invpcid_single and guest_has_invpcid_single: + expected_guest_minus_host |= {"invpcid_single"} + + assert host_feats - guest_feats == expected_host_minus_guest + assert guest_feats - host_feats == expected_guest_minus_host + + case CpuModel.INTEL_ICELAKE: + host_guest_diff_5_10 = INTEL_HOST_ONLY_FEATS - {"cdp_l3"} | { + "pconfig", + "tme", + "split_lock_detect", + } + host_guest_diff_6_1 = host_guest_diff_5_10 - { + "bts", + "dtes64", + "dts", + "pebs", + } + + if global_props.host_linux_version_tpl < (6, 1): + assert host_feats - guest_feats == host_guest_diff_5_10 + else: + assert host_feats - guest_feats == host_guest_diff_6_1 + + assert guest_feats - host_feats == { + "hypervisor", + "tsc_known_freq", + } + + case CpuModel.ARM_NEOVERSE_N1: + expected_guest_minus_host = set() + expected_host_minus_guest = set() + + # Upstream kernel v6.11+ hides "ssbs" from "lscpu" on Neoverse-N1 and Neoverse-V1 since + # they have an errata whereby an MSR to the SSBS special-purpose register does not + # affect subsequent speculative instructions, permitting speculative store bypassing for + # a window of time. + # https://github.com/torvalds/linux/commit/adeec61a4723fd3e39da68db4cc4d924e6d7f641 + # + # While Amazon Linux kernels (v5.10 and v6.1) backported the above commit, our test + # ubuntu kernel (v6.8) and our guest kernels (v5.10 and v6.1) don't pick it. + host_has_ssbs = global_props.host_os not in { + "amzn2", + "amzn2023", + } and global_props.host_linux_version_tpl < (6, 11) + guest_has_ssbs = vm.guest_kernel_version < (6, 11) + + if host_has_ssbs and not guest_has_ssbs: + expected_host_minus_guest |= {"ssbs"} + if not host_has_ssbs and guest_has_ssbs: + expected_guest_minus_host |= {"ssbs"} + + assert host_feats - guest_feats == expected_host_minus_guest + assert guest_feats - host_feats == expected_guest_minus_host + + case CpuModel.ARM_NEOVERSE_V1: + expected_guest_minus_host = set() + # KVM does not enable PAC or SVE features by default + # and Firecracker does not enable them either. + expected_host_minus_guest = {"paca", "pacg", "sve", "svebf16", "svei8mm"} + + # Upstream kernel v6.11+ hides "ssbs" from "lscpu" on Neoverse-N1 and Neoverse-V1 since + # they have an errata whereby an MSR to the SSBS special-purpose register does not + # affect subsequent speculative instructions, permitting speculative store bypassing for + # a window of time. + # https://github.com/torvalds/linux/commit/adeec61a4723fd3e39da68db4cc4d924e6d7f641 + # + # While Amazon Linux kernels (v5.10 and v6.1) backported the above commit, our test + # ubuntu kernel (v6.8) and our guest kernels (v5.10 and v6.1) don't pick it. + host_has_ssbs = global_props.host_os not in { + "amzn2", + "amzn2023", + } and global_props.host_linux_version_tpl < (6, 11) + guest_has_ssbs = vm.guest_kernel_version < (6, 11) + + if host_has_ssbs and not guest_has_ssbs: + expected_host_minus_guest |= {"ssbs"} + if not host_has_ssbs and guest_has_ssbs: + expected_guest_minus_host |= {"ssbs"} + + assert host_feats - guest_feats == expected_host_minus_guest + assert guest_feats - host_feats == expected_guest_minus_host + + case _: + # only fail if running in CI + if os.environ.get("BUILDKITE") is not None: + assert ( + guest_feats == host_feats + ), f"Cpu model {CPU_MODEL} is not supported" diff --git a/tests/integration_tests/functional/test_cpu_features_x86_64.py b/tests/integration_tests/functional/test_cpu_features_x86_64.py index f8bbe1ef52e..a966e253c46 100644 --- a/tests/integration_tests/functional/test_cpu_features_x86_64.py +++ b/tests/integration_tests/functional/test_cpu_features_x86_64.py @@ -22,7 +22,6 @@ from framework.defs import SUPPORTED_HOST_KERNELS from framework.properties import global_props from framework.utils_cpu_templates import SUPPORTED_CPU_TEMPLATES -from framework.utils_cpuid import CPU_FEATURES_CMD, CpuModel PLATFORM = platform.machine() UNSUPPORTED_HOST_KERNEL = ( @@ -203,203 +202,6 @@ def test_brand_string(uvm_plain_any): assert False -INTEL_HOST_ONLY_FEATS = { - "acpi", - "aperfmperf", - "arch_perfmon", - "art", - "bts", - "cat_l3", - "cdp_l3", - "cqm", - "cqm_llc", - "cqm_mbm_local", - "cqm_mbm_total", - "cqm_occup_llc", - "dca", - "ds_cpl", - "dtes64", - "dtherm", - "dts", - "epb", - "ept", - "ept_ad", - "est", - "flexpriority", - "flush_l1d", - "hwp", - "hwp_act_window", - "hwp_epp", - "hwp_pkg_req", - "ida", - "intel_ppin", - "intel_pt", - "mba", - "monitor", - "pbe", - "pdcm", - "pebs", - "pln", - "pts", - "rdt_a", - "sdbg", - "smx", - "tm", - "tm2", - "tpr_shadow", - "vmx", - "vnmi", - "vpid", - "xtpr", -} - - -def test_host_vs_guest_cpu_features_x86_64(uvm_nano): - """Check CPU features host vs guest""" - - vm = uvm_nano - vm.add_net_iface() - vm.start() - host_feats = set(utils.check_output(CPU_FEATURES_CMD).stdout.strip().split(" ")) - guest_feats = set(vm.ssh.check_output(CPU_FEATURES_CMD).stdout.strip().split(" ")) - - cpu_model = cpuid_utils.get_cpu_codename() - match cpu_model: - case CpuModel.AMD_MILAN: - host_guest_diff_5_10 = { - "amd_ppin", - "aperfmperf", - "bpext", - "cat_l3", - "cdp_l3", - "cpb", - "cqm", - "cqm_llc", - "cqm_mbm_local", - "cqm_mbm_total", - "cqm_occup_llc", - "decodeassists", - "extapic", - "extd_apicid", - "flushbyasid", - "hw_pstate", - "ibs", - "irperf", - "lbrv", - "mba", - "monitor", - "mwaitx", - "overflow_recov", - "pausefilter", - "perfctr_llc", - "perfctr_nb", - "pfthreshold", - "rdpru", - "rdt_a", - "sev", - "sev_es", - "skinit", - "smca", - "sme", - "succor", - "svm_lock", - "tce", - "tsc_scale", - "v_vmsave_vmload", - "vgif", - "vmcb_clean", - "wdt", - } - - host_guest_diff_6_1 = host_guest_diff_5_10 - { - "lbrv", - "pausefilter", - "pfthreshold", - "sme", - "tsc_scale", - "v_vmsave_vmload", - "vgif", - "vmcb_clean", - } | {"brs", "rapl", "v_spec_ctrl"} - - if global_props.host_linux_version_tpl < (6, 1): - assert host_feats - guest_feats == host_guest_diff_5_10 - else: - assert host_feats - guest_feats == host_guest_diff_6_1 - - assert guest_feats - host_feats == { - "hypervisor", - "tsc_adjust", - "tsc_deadline_timer", - "tsc_known_freq", - } - case CpuModel.INTEL_SKYLAKE: - assert host_feats - guest_feats == INTEL_HOST_ONLY_FEATS - assert guest_feats - host_feats == { - "hypervisor", - "tsc_known_freq", - "umip", - } - case CpuModel.INTEL_CASCADELAKE: - expected_host_minus_guest = INTEL_HOST_ONLY_FEATS - expected_guest_minus_host = { - "hypervisor", - "tsc_known_freq", - "umip", - } - - # Linux kernel v6.4+ passes through the CPUID bit for "flush_l1d" to guests. - # https://github.com/torvalds/linux/commit/45cf86f26148e549c5ba4a8ab32a390e4bde216e - # - # Our test ubuntu host kernel is v6.8 and has the commit. - if global_props.host_linux_version_tpl >= (6, 4): - expected_host_minus_guest -= {"flush_l1d"} - - # Linux kernel v6.6+ drops the "invpcid_single" synthetic feature bit. - # https://github.com/torvalds/linux/commit/54e3d9434ef61b97fd3263c141b928dc5635e50d - # - # Our test ubuntu host kernel is v6.8 and has the commit. - host_has_invpcid_single = global_props.host_linux_version_tpl < (6, 6) - guest_has_invpcid_single = vm.guest_kernel_version < (6, 6) - if host_has_invpcid_single and not guest_has_invpcid_single: - expected_host_minus_guest |= {"invpcid_single"} - if not host_has_invpcid_single and guest_has_invpcid_single: - expected_guest_minus_host |= {"invpcid_single"} - - assert host_feats - guest_feats == expected_host_minus_guest - assert guest_feats - host_feats == expected_guest_minus_host - case CpuModel.INTEL_ICELAKE: - host_guest_diff_5_10 = INTEL_HOST_ONLY_FEATS - {"cdp_l3"} | { - "pconfig", - "tme", - "split_lock_detect", - } - host_guest_diff_6_1 = host_guest_diff_5_10 - { - "bts", - "dtes64", - "dts", - "pebs", - } - - if global_props.host_linux_version_tpl < (6, 1): - assert host_feats - guest_feats == host_guest_diff_5_10 - else: - assert host_feats - guest_feats == host_guest_diff_6_1 - - assert guest_feats - host_feats == { - "hypervisor", - "tsc_known_freq", - } - case CpuModel.AMD_GENOA: - # Return here to allow the test to pass until CPU features to enable are confirmed - return - case _: - if os.environ.get("BUILDKITE") is not None: - assert ( - guest_feats == host_feats - ), f"Cpu model {cpu_model} is not supported" - - # From the `Intel® 64 Architecture x2APIC Specification` # (https://courses.cs.washington.edu/courses/cse451/24wi/documentation/x2apic.pdf): # > The X2APIC MSRs cannot to be loaded and stored on VMX transitions. A VMX transition fails From f27293edbe27ac7c60c5b271edb552d7c40b2e26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Sat, 23 Nov 2024 11:59:20 +0100 Subject: [PATCH 08/16] tests: add helper method MicrovmFactory.build_from_snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a fairly common repeated pattern, so extract it into a method to make writing tests simpler. Signed-off-by: Pablo Barbáchano --- tests/conftest.py | 4 +- tests/framework/microvm.py | 7 +++ .../integration_tests/functional/test_api.py | 5 +- .../functional/test_balloon.py | 4 +- .../functional/test_cmd_line_parameters.py | 4 +- .../functional/test_snapshot_basic.py | 52 ++++++------------- .../functional/test_snapshot_editor.py | 4 +- .../test_snapshot_not_losing_dirty_pages.py | 7 +-- .../functional/test_vsock.py | 9 +--- 9 files changed, 31 insertions(+), 65 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index a2990a56c98..7cacedb9918 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -477,9 +477,7 @@ def uvm_restored(microvm_factory, guest_kernel, rootfs, cpu_template): uvm = uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template) snapshot = uvm.snapshot_full() uvm.kill() - uvm2 = microvm_factory.build() - uvm2.spawn() - uvm2.restore_from_snapshot(snapshot, resume=True) + uvm2 = microvm_factory.build_from_snapshot(snapshot) uvm2.cpu_template_name = uvm.cpu_template_name return uvm2 diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py index 832f56a877e..572437c758b 100644 --- a/tests/framework/microvm.py +++ b/tests/framework/microvm.py @@ -1084,6 +1084,13 @@ def build(self, kernel=None, rootfs=None, **kwargs): vm.ssh_key = ssh_key return vm + def build_from_snapshot(self, snapshot: Snapshot): + """Build a microvm from a snapshot""" + vm = self.build() + vm.spawn() + vm.restore_from_snapshot(snapshot, resume=True) + return vm + def kill(self): """Clean up all built VMs""" for vm in self.vms: diff --git a/tests/integration_tests/functional/test_api.py b/tests/integration_tests/functional/test_api.py index 27366529c39..1e54c7b4fb1 100644 --- a/tests/integration_tests/functional/test_api.py +++ b/tests/integration_tests/functional/test_api.py @@ -1166,10 +1166,7 @@ def test_get_full_config_after_restoring_snapshot(microvm_factory, uvm_nano): ] snapshot = uvm_nano.snapshot_full() - uvm2 = microvm_factory.build() - uvm2.spawn() - uvm2.restore_from_snapshot(snapshot, resume=True) - + uvm2 = microvm_factory.build_from_snapshot(snapshot) expected_cfg = setup_cfg.copy() # We expect boot-source to be set with the following values diff --git a/tests/integration_tests/functional/test_balloon.py b/tests/integration_tests/functional/test_balloon.py index ee750dcac7d..2c59fd2e814 100644 --- a/tests/integration_tests/functional/test_balloon.py +++ b/tests/integration_tests/functional/test_balloon.py @@ -484,9 +484,7 @@ def test_balloon_snapshot(microvm_factory, guest_kernel, rootfs): assert first_reading > second_reading snapshot = vm.snapshot_full() - microvm = microvm_factory.build() - microvm.spawn() - microvm.restore_from_snapshot(snapshot, resume=True) + microvm = microvm_factory.build_from_snapshot(snapshot) # Get the firecracker from snapshot pid, and open an ssh connection. firecracker_pid = microvm.firecracker_pid diff --git a/tests/integration_tests/functional/test_cmd_line_parameters.py b/tests/integration_tests/functional/test_cmd_line_parameters.py index 25e47a50e17..79af938b1f7 100644 --- a/tests/integration_tests/functional/test_cmd_line_parameters.py +++ b/tests/integration_tests/functional/test_cmd_line_parameters.py @@ -96,9 +96,7 @@ def test_cli_metrics_if_resume_no_metrics(uvm_plain, microvm_factory): snapshot = uvm1.snapshot_full() # When: restoring from the snapshot - uvm2 = microvm_factory.build() - uvm2.spawn() - uvm2.restore_from_snapshot(snapshot) + uvm2 = microvm_factory.build_from_snapshot(snapshot) # Then: the old metrics configuration does not exist metrics2 = Path(uvm2.jailer.chroot_path()) / metrics_path.name diff --git a/tests/integration_tests/functional/test_snapshot_basic.py b/tests/integration_tests/functional/test_snapshot_basic.py index ac596440f67..4030bb4e981 100644 --- a/tests/integration_tests/functional/test_snapshot_basic.py +++ b/tests/integration_tests/functional/test_snapshot_basic.py @@ -50,39 +50,25 @@ def _get_guest_drive_size(ssh_connection, guest_dev_name="/dev/vdb"): return lines[1].strip() -def test_resume_after_restoration(uvm_nano, microvm_factory): - """Tests snapshot is resumable after restoration. +@pytest.mark.parametrize("resume_at_restore", [True, False]) +def test_resume(uvm_nano, microvm_factory, resume_at_restore): + """Tests snapshot is resumable at or after restoration. - Check that a restored microVM is resumable by calling PATCH /vm with Resumed - after PUT /snapshot/load with `resume_vm=False`. + Check that a restored microVM is resumable by either + a. PUT /snapshot/load with `resume_vm=False`, then calling PATCH /vm resume=True + b. PUT /snapshot/load with `resume_vm=True` """ vm = uvm_nano vm.add_net_iface() vm.start() - - snapshot = vm.snapshot_full() - - restored_vm = microvm_factory.build() - restored_vm.spawn() - restored_vm.restore_from_snapshot(snapshot) - restored_vm.resume() - - -def test_resume_at_restoration(uvm_nano, microvm_factory): - """Tests snapshot is resumable at restoration. - - Check that a restored microVM is resumable by calling PUT /snapshot/load - with `resume_vm=True`. - """ - vm = uvm_nano - vm.add_net_iface() - vm.start() - snapshot = vm.snapshot_full() - restored_vm = microvm_factory.build() restored_vm.spawn() - restored_vm.restore_from_snapshot(snapshot, resume=True) + restored_vm.restore_from_snapshot(snapshot, resume=resume_at_restore) + if not resume_at_restore: + assert restored_vm.state == "Paused" + restored_vm.resume() + assert restored_vm.state == "Running" def test_snapshot_current_version(uvm_nano): @@ -228,9 +214,7 @@ def test_patch_drive_snapshot(uvm_nano, microvm_factory): # Load snapshot in a new Firecracker microVM. logger.info("Load snapshot, mem %s", snapshot.mem) - vm = microvm_factory.build() - vm.spawn() - vm.restore_from_snapshot(snapshot, resume=True) + vm = microvm_factory.build_from_snapshot(snapshot) # Attempt to connect to resumed microvm and verify the new microVM has the # right scratch drive. @@ -319,9 +303,7 @@ def test_negative_postload_api(uvm_plain, microvm_factory): basevm.kill() # Do not resume, just load, so we can still call APIs that work. - microvm = microvm_factory.build() - microvm.spawn() - microvm.restore_from_snapshot(snapshot, resume=True) + microvm = microvm_factory.build_from_snapshot(snapshot) fail_msg = "The requested operation is not supported after starting the microVM" with pytest.raises(RuntimeError, match=fail_msg): @@ -486,9 +468,7 @@ def test_diff_snapshot_overlay(guest_kernel, rootfs, microvm_factory): assert not filecmp.cmp(merged_snapshot.mem, first_snapshot_backup, shallow=False) - new_vm = microvm_factory.build() - new_vm.spawn() - new_vm.restore_from_snapshot(merged_snapshot, resume=True) + _ = microvm_factory.build_from_snapshot(merged_snapshot) # Check that the restored VM works @@ -510,9 +490,7 @@ def test_snapshot_overwrite_self(guest_kernel, rootfs, microvm_factory): snapshot = base_vm.snapshot_full() base_vm.kill() - vm = microvm_factory.build() - vm.spawn() - vm.restore_from_snapshot(snapshot, resume=True) + vm = microvm_factory.build_from_snapshot(snapshot) # When restoring a snapshot, vm.restore_from_snapshot first copies # the memory file (inside of the jailer) to /mem.src diff --git a/tests/integration_tests/functional/test_snapshot_editor.py b/tests/integration_tests/functional/test_snapshot_editor.py index 4d466a441ce..9323695628c 100644 --- a/tests/integration_tests/functional/test_snapshot_editor.py +++ b/tests/integration_tests/functional/test_snapshot_editor.py @@ -68,6 +68,4 @@ def test_remove_regs(uvm_nano, microvm_factory): assert MIDR_EL1 not in stdout # test that we can restore from a snapshot - new_vm = microvm_factory.build() - new_vm.spawn() - new_vm.restore_from_snapshot(snapshot, resume=True) + _ = microvm_factory.build_from_snapshot(snapshot) diff --git a/tests/integration_tests/functional/test_snapshot_not_losing_dirty_pages.py b/tests/integration_tests/functional/test_snapshot_not_losing_dirty_pages.py index 5584cfceac8..79366f13f0b 100644 --- a/tests/integration_tests/functional/test_snapshot_not_losing_dirty_pages.py +++ b/tests/integration_tests/functional/test_snapshot_not_losing_dirty_pages.py @@ -63,9 +63,6 @@ def test_diff_snapshot_works_after_error( # Now there is enough space for it to work snap2 = uvm.snapshot_diff() - - vm2 = microvm_factory.build() - vm2.spawn() - vm2.restore_from_snapshot(snap2, resume=True) - uvm.kill() + + _vm2 = microvm_factory.build_from_snapshot(snap2) diff --git a/tests/integration_tests/functional/test_vsock.py b/tests/integration_tests/functional/test_vsock.py index 2d540b8f934..dfa02510b37 100644 --- a/tests/integration_tests/functional/test_vsock.py +++ b/tests/integration_tests/functional/test_vsock.py @@ -199,10 +199,7 @@ def test_vsock_transport_reset_h2g( test_vm.kill() # Load snapshot. - - vm2 = microvm_factory.build() - vm2.spawn() - vm2.restore_from_snapshot(snapshot, resume=True) + vm2 = microvm_factory.build_from_snapshot(snapshot) # Check that vsock device still works. # Test guest-initiated connections. @@ -231,9 +228,7 @@ def test_vsock_transport_reset_g2h(uvm_nano, microvm_factory): for _ in range(5): # Load snapshot. - new_vm = microvm_factory.build() - new_vm.spawn() - new_vm.restore_from_snapshot(snapshot, resume=True) + new_vm = microvm_factory.build_from_snapshot(snapshot) # After snap restore all vsock connections should be # dropped. This means guest socat should exit same way From d993691ccbbe8bc765a1e6129c5fead51432e716 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Sat, 23 Nov 2024 12:03:31 +0100 Subject: [PATCH 09/16] tests: refactor test_rng with uvm_any MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite test_rng using uvm_any. Signed-off-by: Pablo Barbáchano --- .../integration_tests/functional/test_rng.py | 66 ++++++++++--------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/tests/integration_tests/functional/test_rng.py b/tests/integration_tests/functional/test_rng.py index b40aa66033d..a5b80157109 100644 --- a/tests/integration_tests/functional/test_rng.py +++ b/tests/integration_tests/functional/test_rng.py @@ -8,11 +8,9 @@ from host_tools.network import SSHConnection -@pytest.fixture(params=[None]) -def uvm_with_rng(uvm_plain, request): +def uvm_with_rng_booted(microvm_factory, guest_kernel, rootfs, rate_limiter): """Fixture of a microvm with virtio-rng configured""" - rate_limiter = request.param - uvm = uvm_plain + uvm = microvm_factory.build(guest_kernel, rootfs) uvm.spawn(log_level="INFO") uvm.basic_config(vcpu_count=2, mem_size_mib=256) uvm.add_net_iface() @@ -23,6 +21,34 @@ def uvm_with_rng(uvm_plain, request): return uvm +def uvm_with_rng_restored(microvm_factory, guest_kernel, rootfs, rate_limiter): + """Return a restored uvm""" + uvm = uvm_with_rng_booted(microvm_factory, guest_kernel, rootfs, rate_limiter) + snapshot = uvm.snapshot_full() + uvm.kill() + uvm2 = microvm_factory.build_from_snapshot(snapshot) + uvm2.rng_rate_limiter = uvm.rng_rate_limiter + return uvm2 + + +@pytest.fixture(params=[uvm_with_rng_booted, uvm_with_rng_restored]) +def uvm_ctor(request): + """Fixture to return uvms with different constructors""" + return request.param + + +@pytest.fixture(params=[None]) +def rate_limiter(request): + """Fixture to return different rate limiters""" + return request.param + + +@pytest.fixture +def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs, rate_limiter): + """Return booted and restored uvms""" + return uvm_ctor(microvm_factory, guest_kernel, rootfs, rate_limiter) + + def list_rng_available(ssh_connection: SSHConnection) -> list[str]: """Returns a list of rng devices available in the VM""" return ( @@ -62,35 +88,17 @@ def test_rng_not_present(uvm_nano): ), "virtio_rng device should not be available in the uvm" -def test_rng_present(uvm_with_rng): +def test_rng_present(uvm_any): """ Test a guest microVM with an entropy defined configured and ensure that we can access `/dev/hwrng` """ - vm = uvm_with_rng + vm = uvm_any assert_virtio_rng_is_current_hwrng_device(vm.ssh) check_entropy(vm.ssh) -def test_rng_snapshot(uvm_with_rng, microvm_factory): - """ - Test that a virtio-rng device is functional after resuming from - a snapshot - """ - - vm = uvm_with_rng - assert_virtio_rng_is_current_hwrng_device(vm.ssh) - check_entropy(vm.ssh) - snapshot = vm.snapshot_full() - - new_vm = microvm_factory.build() - new_vm.spawn() - new_vm.restore_from_snapshot(snapshot, resume=True) - assert_virtio_rng_is_current_hwrng_device(new_vm.ssh) - check_entropy(new_vm.ssh) - - def _get_percentage_difference(measured, base): """Return the percentage delta between the arguments.""" if measured == base: @@ -199,7 +207,7 @@ def _rate_limiter_id(rate_limiter): # parametrize the RNG rate limiter @pytest.mark.parametrize( - "uvm_with_rng", + "rate_limiter", [ {"bandwidth": {"size": 1000, "refill_time": 100}}, {"bandwidth": {"size": 10000, "refill_time": 100}}, @@ -208,16 +216,14 @@ def _rate_limiter_id(rate_limiter): indirect=True, ids=_rate_limiter_id, ) -def test_rng_bw_rate_limiter(uvm_with_rng): +@pytest.mark.parametrize("uvm_ctor", [uvm_with_rng_booted], indirect=True) +def test_rng_bw_rate_limiter(uvm_any): """ Test that rate limiter without initial burst budget works """ - vm = uvm_with_rng - # _start_vm_with_rng(vm, rate_limiter) - + vm = uvm_any size = vm.rng_rate_limiter["bandwidth"]["size"] refill_time = vm.rng_rate_limiter["bandwidth"]["refill_time"] - expected_kbps = size / refill_time assert_virtio_rng_is_current_hwrng_device(vm.ssh) From bea83246114b8489211d7b9588bf67b92e2dd584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Sat, 23 Nov 2024 13:55:58 +0100 Subject: [PATCH 10/16] tests: drop unused static cpu_templates fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixture is not used anymore, as it is mostly superseded by cpu_template_any. Signed-off-by: Pablo Barbáchano --- tests/conftest.py | 7 ------- .../functional/test_cpu_features_x86_64.py | 15 +++++++-------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 7cacedb9918..b599ffc79ab 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -292,13 +292,6 @@ def microvm_factory(request, record_property, results_dir): uvm_factory.kill() -@pytest.fixture(params=static_cpu_templates_params()) -def cpu_template(request, record_property): - """Return all static CPU templates supported by the vendor.""" - record_property("static_cpu_template", request.param) - return request.param - - @pytest.fixture(params=custom_cpu_templates_params()) def custom_cpu_template(request, record_property): """Return all dummy custom CPU templates supported by the vendor.""" diff --git a/tests/integration_tests/functional/test_cpu_features_x86_64.py b/tests/integration_tests/functional/test_cpu_features_x86_64.py index a966e253c46..1af6a39f83a 100644 --- a/tests/integration_tests/functional/test_cpu_features_x86_64.py +++ b/tests/integration_tests/functional/test_cpu_features_x86_64.py @@ -952,18 +952,16 @@ def check_enabled_features(test_microvm, cpu_template): ) -def test_c3_on_skylake_show_warning(uvm_plain, cpu_template): +def test_c3_on_skylake_show_warning(uvm_plain, cpu_template_any): """ This test verifies that the warning message about MMIO stale data mitigation - is displayed only on Intel Skylake with C3 template. + is displayed only on Intel Skylake with static C3 template. """ uvm = uvm_plain uvm.spawn() - uvm.basic_config( - vcpu_count=2, - mem_size_mib=256, - cpu_template=cpu_template, - ) + uvm.basic_config(vcpu_count=2, mem_size_mib=256) + uvm.add_net_iface() + uvm.set_cpu_template(cpu_template_any) uvm.start() message = ( @@ -972,7 +970,8 @@ def test_c3_on_skylake_show_warning(uvm_plain, cpu_template): "does not apply the mitigation against MMIO stale data " "vulnerability." ) - if uvm.cpu_template_name == "c3" and global_props.cpu_codename == "INTEL_SKYLAKE": + + if cpu_template_any == "C3" and global_props.cpu_codename == "INTEL_SKYLAKE": assert message in uvm.log_data else: assert message not in uvm.log_data From a2b659c9255a63ba3a2a38e7bfd0abc6d441bb32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Thu, 28 Nov 2024 23:25:56 +0100 Subject: [PATCH 11/16] tests: simplify rootfs fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we only have one rootfs, simplify the fixture to return a single value. This will have also not show it as part of the test instance name. Signed-off-by: Pablo Barbáchano --- tests/conftest.py | 23 ++++++++++++----------- tests/framework/artifacts.py | 10 ++-------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index b599ffc79ab..5ed61083014 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -35,7 +35,7 @@ import host_tools.cargo_build as build_tools from framework import defs, utils -from framework.artifacts import kernel_params, rootfs_params +from framework.artifacts import disks, kernel_params from framework.microvm import MicroVMFactory from framework.properties import global_props from framework.utils_cpu_templates import ( @@ -354,13 +354,6 @@ def guest_kernel_fxt(request, record_property): return kernel -def rootfs_fxt(request, record_property): - """Return all supported rootfs.""" - fs = request.param - record_property("rootfs", fs.name) - return fs - - # Fixtures for all guest kernels, and specific versions guest_kernel = pytest.fixture(guest_kernel_fxt, params=kernel_params("vmlinux-*")) guest_kernel_acpi = pytest.fixture( @@ -380,9 +373,17 @@ def rootfs_fxt(request, record_property): params=kernel_params("vmlinux-6.1*"), ) -# Fixtures for all Ubuntu rootfs, and specific versions -rootfs = pytest.fixture(rootfs_fxt, params=rootfs_params("ubuntu-24*.squashfs")) -rootfs_rw = pytest.fixture(rootfs_fxt, params=rootfs_params("*.ext4")) + +@pytest.fixture +def rootfs(): + """Return an Ubuntu 24.04 read-only rootfs""" + return disks("ubuntu-24.04.squashfs")[0] + + +@pytest.fixture +def rootfs_rw(): + """Return an Ubuntu 24.04 ext4 rootfs""" + return disks("ubuntu-24.04.ext4")[0] @pytest.fixture diff --git a/tests/framework/artifacts.py b/tests/framework/artifacts.py index 77584f02129..0ed27c16b61 100644 --- a/tests/framework/artifacts.py +++ b/tests/framework/artifacts.py @@ -44,9 +44,9 @@ def kernels(glob, artifact_dir: Path = ARTIFACT_DIR) -> Iterator: break -def disks(glob) -> Iterator: +def disks(glob) -> list: """Return supported rootfs""" - yield from sorted(ARTIFACT_DIR.glob(glob)) + return sorted(ARTIFACT_DIR.glob(glob)) def kernel_params( @@ -57,12 +57,6 @@ def kernel_params( yield pytest.param(kernel, id=kernel.name) -def rootfs_params(glob="ubuntu-*.squashfs") -> Iterator: - """Return supported rootfs as pytest parameters""" - for rootfs in disks(glob=glob): - yield pytest.param(rootfs, id=rootfs.name) - - @dataclass(frozen=True, repr=True) class FirecrackerArtifact: """Utility class for Firecracker binary artifacts.""" From 1825aa491cc67b0628280ed3dd7fb1b0b9b3a2d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Barb=C3=A1chano?= Date: Fri, 29 Nov 2024 09:32:07 +0100 Subject: [PATCH 12/16] tests: parametrize uvm_any fixture family with vcpus and memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a parameter so that we can control vcpu number and memory. Right now it uses a hardcoded value, but it can be overridden per test. Signed-off-by: Pablo Barbáchano --- tests/conftest.py | 54 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5ed61083014..ac73b4bd8ab 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -455,20 +455,34 @@ def uvm_with_initrd( yield uvm -def uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template): +@pytest.fixture +def vcpu_count(): + """Return default vcpu_count. Use indirect parametrization to override.""" + return 2 + + +@pytest.fixture +def mem_size_mib(): + """Return memory size. Use indirect parametrization to override.""" + return 256 + + +def uvm_booted( + microvm_factory, guest_kernel, rootfs, cpu_template, vcpu_count=2, mem_size_mib=256 +): """Return a booted uvm""" uvm = microvm_factory.build(guest_kernel, rootfs) uvm.spawn() - uvm.basic_config(vcpu_count=2, mem_size_mib=256) + uvm.basic_config(vcpu_count=vcpu_count, mem_size_mib=mem_size_mib) uvm.set_cpu_template(cpu_template) uvm.add_net_iface() uvm.start() return uvm -def uvm_restored(microvm_factory, guest_kernel, rootfs, cpu_template): +def uvm_restored(microvm_factory, guest_kernel, rootfs, cpu_template, **kwargs): """Return a restored uvm""" - uvm = uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template) + uvm = uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template, **kwargs) snapshot = uvm.snapshot_full() uvm.kill() uvm2 = microvm_factory.build_from_snapshot(snapshot) @@ -483,12 +497,36 @@ def uvm_ctor(request): @pytest.fixture -def uvm_any(microvm_factory, uvm_ctor, guest_kernel, rootfs, cpu_template_any): +def uvm_any( + microvm_factory, + uvm_ctor, + guest_kernel, + rootfs, + cpu_template_any, + vcpu_count, + mem_size_mib, +): """Return booted and restored uvms""" - return uvm_ctor(microvm_factory, guest_kernel, rootfs, cpu_template_any) + return uvm_ctor( + microvm_factory, + guest_kernel, + rootfs, + cpu_template_any, + vcpu_count=vcpu_count, + mem_size_mib=mem_size_mib, + ) @pytest.fixture -def uvm_any_booted(microvm_factory, guest_kernel, rootfs, cpu_template_any): +def uvm_any_booted( + microvm_factory, guest_kernel, rootfs, cpu_template_any, vcpu_count, mem_size_mib +): """Return booted uvms""" - return uvm_booted(microvm_factory, guest_kernel, rootfs, cpu_template_any) + return uvm_booted( + microvm_factory, + guest_kernel, + rootfs, + cpu_template_any, + vcpu_count=vcpu_count, + mem_size_mib=mem_size_mib, + ) From 1e4002f77ac34d4e5ad7c956b162d0c329df3709 Mon Sep 17 00:00:00 2001 From: Takahiro Itazuri Date: Thu, 5 Dec 2024 05:47:07 +0000 Subject: [PATCH 13/16] test: Enable to test custom CPU templates w/o python code modification Previously to test custom CPU templates for temporary testing purpose in development, we needed to modify Python code (especially tests/framework/utils_cpu_templates.py) manually. That is very cumbersome and slows down our development / test / debug cycle, since it requires us to make a commit and push it manually. With this change, we become able to inject custom CPU templates into test (not only from local but also from buildkite) without modifying python code. Signed-off-by: Takahiro Itazuri --- tests/data/custom_cpu_templates/.gitkeep | 0 tests/framework/utils_cpu_templates.py | 9 +++++++++ 2 files changed, 9 insertions(+) create mode 100644 tests/data/custom_cpu_templates/.gitkeep diff --git a/tests/data/custom_cpu_templates/.gitkeep b/tests/data/custom_cpu_templates/.gitkeep new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/framework/utils_cpu_templates.py b/tests/framework/utils_cpu_templates.py index 56021a21aaa..1dc0375bd56 100644 --- a/tests/framework/utils_cpu_templates.py +++ b/tests/framework/utils_cpu_templates.py @@ -64,6 +64,7 @@ def get_supported_custom_cpu_templates(): def custom_cpu_templates_params(): """Return Custom CPU templates as pytest parameters""" + # Return custom CPU templates that are equivalent to the supported static CPU templates. for name in sorted(get_supported_custom_cpu_templates()): tmpl = Path(f"./data/static_cpu_templates/{name.lower()}.json") yield pytest.param( @@ -71,6 +72,14 @@ def custom_cpu_templates_params(): id="custom_" + name, ) + # Return custom CPU templates for temporary testing purpose. + for tmpl in Path("./data/custom_cpu_templates").glob("*.json"): + name = tmpl.stem + yield pytest.param( + {"name": name, "template": json.loads(tmpl.read_text("utf-8"))}, + id="custom_" + name, + ) + def static_cpu_templates_params(): """Return Static CPU templates as pytest parameters""" From b195cd1da491f3de3f4447077e8cc69c1acb5e95 Mon Sep 17 00:00:00 2001 From: Takahiro Itazuri Date: Thu, 5 Dec 2024 06:30:55 +0000 Subject: [PATCH 14/16] test: Allow to add additional commands to prepend We usually run the CI through the python scripts generating buildkite pipeline steps. To enable to inject custom CPU templates through the python scripts, adds a new command line argument `--additional-prepend` that specifies additional commands to prepend to the generated steps. Signed-off-by: Takahiro Itazuri --- .buildkite/common.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.buildkite/common.py b/.buildkite/common.py index a8913d5d6ef..7533194c300 100644 --- a/.buildkite/common.py +++ b/.buildkite/common.py @@ -176,6 +176,14 @@ def __call__(self, parser, namespace, value, option_string=None): default=None, type=str, ) +COMMON_PARSER.add_argument( + "--additional-prepend", + help="Commands to be prepended additionally", + required=False, + nargs="+", + default=[], + type=str, +) def random_str(k: int): @@ -288,6 +296,7 @@ def _adapt_group(self, group): f"chmod -v a+x {self.binary_dir}/**/*", ] ) + prepend.extend(self.args.additional_prepend) for step in group["steps"]: step["command"] = prepend + step["command"] From 078be2a7039bf4ab2fe40056fa574a298bf25356 Mon Sep 17 00:00:00 2001 From: Takahiro Itazuri Date: Thu, 5 Dec 2024 06:56:00 +0000 Subject: [PATCH 15/16] test: Enable to filter pytest tests to run Previously, when using the python scripts generating buildkite steps, we were only able to run the entire tests. However, in some cases, we would like to run only a subset of them. Adds a new argument `-k` (or `--keywords`) to filter pytest tests to run. Signed-off-by: Takahiro Itazuri --- .buildkite/common.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.buildkite/common.py b/.buildkite/common.py index 7533194c300..ae1147711e6 100644 --- a/.buildkite/common.py +++ b/.buildkite/common.py @@ -184,6 +184,14 @@ def __call__(self, parser, namespace, value, option_string=None): default=[], type=str, ) +COMMON_PARSER.add_argument( + "-k", + "--keywords", + help="Keywords to filter pytest tests to run", + required=False, + default=None, + type=str, +) def random_str(k: int): @@ -360,6 +368,8 @@ def devtool_test(self, devtool_opts=None, pytest_opts=None): parts.append("--") if self.binary_dir is not None: parts.append(f"--binary-dir=../{self.binary_dir}/$(uname -m)") + if self.args.keywords is not None: + parts.append(f"-k {self.args.keywords}") if pytest_opts: parts.append(pytest_opts) cmds.append(" ".join(parts)) From 6d1470cf78d0859492169d8332edef26ca4d7872 Mon Sep 17 00:00:00 2001 From: Takahiro Itazuri Date: Thu, 5 Dec 2024 08:55:35 +0000 Subject: [PATCH 16/16] test: How to run python tests with custom CPU templates Add documentation explaining how to run python integration tests with custom CPU templates. Signed-off-by: Takahiro Itazuri --- tests/README.md | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/README.md b/tests/README.md index bf7aba9a547..bcf82c6e90f 100644 --- a/tests/README.md +++ b/tests/README.md @@ -646,3 +646,49 @@ sudo env PYTHONPATH=tests HOME=$HOME ~/.local/bin/ipython3 -i tools/sandbox.py - > \[!WARNING\] > > **Notice this runs as root!** + +## How to run python tests with custom CPU templates + +By placing custom CPU templates under `tests/data/custom_cpu_templates/` +directory, you can run the CI with them for testing / debugging purposes. + +Using the pytest keyword filtering option `-k`, you can run only python tests +that use a specific CPU template. For example: + +```sh +tools/devtool -y test -- integration_tests/functional -k unique_template_name +``` + +You can also do it from buildkite using the scripts under `.buildkite/` +directory. The easiest way is to commit custom CPU template JSON files in +question to your forked repo. Note that you should specify platforms on which +the custom CPU templates are expected to work. For example: + +```yaml +steps: +- label: "Run test with custom CPU templates" + command: | + .buildkite/pipeline_pr.py \ + --instances m6g.metal m7g.metal \ + -k unique_template_name \ + | buildkite-agent pipeline upload +``` + +Even without making any commit, you can inject the custom CPU template at +runtime via `--additional-prepend` option of the buildkite step generation +scripts. For example: + +```yaml +steps: +- label: "Run test with custom CPU templates" + command: | + .buildkite/pipeline_pr.py \ + --instances m6g.metal m7g.metal \ + -k unique_template_name \ + --additional-prepend 'echo "{\"kvm_capabilities\": [\"170\", \"171\", \"172\"], \"vcpu_features\": [{\"index\": 0, \"bitmap\": \"0b111xxxx\"}]}" > tests/data/custom_cpu_templates/unique_template_name.json \ + | buildkite-agent pipeline upload +``` + +In case that a CPU template written directly looks ugly or too lengthy, an +alternative way is to download or copy it from somewhere at runtime also via the +prepended command, although it is almost same as committing to your forked repo.