From 14c18ed5d2e22d058cf3cb1ff6e330a29bb4e399 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Mon, 25 Nov 2024 10:26:13 +0000 Subject: [PATCH 1/3] replace `set([...])` with set literal My IDE was complaining about this Signed-off-by: Patrick Roy --- tests/framework/utils_cpu_templates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/framework/utils_cpu_templates.py b/tests/framework/utils_cpu_templates.py index 96bf197efda..e57ecfe72c7 100644 --- a/tests/framework/utils_cpu_templates.py +++ b/tests/framework/utils_cpu_templates.py @@ -29,7 +29,7 @@ def get_supported_cpu_templates(): 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: - return sorted(set(INTEL_TEMPLATES) - set(["T2CL"])) + return sorted(set(INTEL_TEMPLATES) - {"T2CL"}) case CpuVendor.INTEL, _: return INTEL_TEMPLATES case CpuVendor.AMD, _: From 265729e5f664401d63bd59616bb84902030f099d Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Mon, 25 Nov 2024 10:51:36 +0000 Subject: [PATCH 2/3] chore: make sure hexadecimal numbers are prefixed by '0x' I got an error message saying 'Start microvm error: Internal error while starting microVM: Vm error: Missing KVM capabilities: aa', which just looked like KVM was screaming. Thus, look through where we print hex numbers, and make sure they are prefixed using '0x', and do so uniformly by using Rust's `:#x` format modifier [1] [1]: https://doc.rust-lang.org/std/fmt/#sign0 Signed-off-by: Patrick Roy --- src/vmm/src/devices/virtio/device.rs | 2 +- src/vmm/src/devices/virtio/mmio.rs | 22 +++++++--------------- src/vmm/src/devices/virtio/vsock/device.rs | 2 +- src/vmm/src/gdb/target.rs | 6 +++--- src/vmm/src/vstate/vcpu/x86_64.rs | 4 ++-- src/vmm/src/vstate/vm.rs | 2 +- 6 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/vmm/src/devices/virtio/device.rs b/src/vmm/src/devices/virtio/device.rs index b5af6862af9..5adf4873dc6 100644 --- a/src/vmm/src/devices/virtio/device.rs +++ b/src/vmm/src/devices/virtio/device.rs @@ -156,7 +156,7 @@ pub trait VirtioDevice: AsAny + Send { let avail_features = self.avail_features(); let unrequested_features = v & !avail_features; if unrequested_features != 0 { - warn!("Received acknowledge request for unknown feature: {:x}", v); + warn!("Received acknowledge request for unknown feature: {:#x}", v); // Don't count these features as acked. v &= !unrequested_features; } diff --git a/src/vmm/src/devices/virtio/mmio.rs b/src/vmm/src/devices/virtio/mmio.rs index 463d11ca2e2..f734058c5b1 100644 --- a/src/vmm/src/devices/virtio/mmio.rs +++ b/src/vmm/src/devices/virtio/mmio.rs @@ -138,7 +138,7 @@ impl MmioTransport { self.with_queue_mut(f); } else { warn!( - "update virtio queue in invalid state 0x{:x}", + "update virtio queue in invalid state {:#x}", self.device_status ); } @@ -227,7 +227,7 @@ impl MmioTransport { } _ => { warn!( - "invalid virtio driver status transition: 0x{:x} -> 0x{:x}", + "invalid virtio driver status transition: {:#x} -> {:#x}", self.device_status, status ); } @@ -282,7 +282,7 @@ impl MmioTransport { 0x70 => self.device_status, 0xfc => self.config_generation, _ => { - warn!("unknown virtio mmio register read: 0x{:x}", offset); + warn!("unknown virtio mmio register read: {:#x}", offset); return; } }; @@ -290,11 +290,7 @@ impl MmioTransport { } 0x100..=0xfff => self.locked_device().read_config(offset - 0x100, data), _ => { - warn!( - "invalid virtio mmio read: 0x{:x}:0x{:x}", - offset, - data.len() - ); + warn!("invalid virtio mmio read: {:#x}:{:#x}", offset, data.len()); } }; } @@ -324,7 +320,7 @@ impl MmioTransport { .ack_features_by_page(self.acked_features_select, v); } else { warn!( - "ack virtio features in invalid state 0x{:x}", + "ack virtio features in invalid state {:#x}", self.device_status ); } @@ -346,7 +342,7 @@ impl MmioTransport { 0xa0 => self.update_queue_field(|q| lo(&mut q.used_ring_address, v)), 0xa4 => self.update_queue_field(|q| hi(&mut q.used_ring_address, v)), _ => { - warn!("unknown virtio mmio register write: 0x{:x}", offset); + warn!("unknown virtio mmio register write: {:#x}", offset); } } } @@ -361,11 +357,7 @@ impl MmioTransport { } } _ => { - warn!( - "invalid virtio mmio write: 0x{:x}:0x{:x}", - offset, - data.len() - ); + warn!("invalid virtio mmio write: {:#x}:{:#x}", offset, data.len()); } } } diff --git a/src/vmm/src/devices/virtio/vsock/device.rs b/src/vmm/src/devices/virtio/vsock/device.rs index bf438aca99f..eb7c144c4be 100644 --- a/src/vmm/src/devices/virtio/vsock/device.rs +++ b/src/vmm/src/devices/virtio/vsock/device.rs @@ -322,7 +322,7 @@ where fn write_config(&mut self, offset: u64, data: &[u8]) { METRICS.cfg_fails.inc(); warn!( - "vsock: guest driver attempted to write device config (offset={:x}, len={:x})", + "vsock: guest driver attempted to write device config (offset={:#x}, len={:#x})", offset, data.len() ); diff --git a/src/vmm/src/gdb/target.rs b/src/vmm/src/gdb/target.rs index fc2da289e8a..b8230342b27 100644 --- a/src/vmm/src/gdb/target.rs +++ b/src/vmm/src/gdb/target.rs @@ -390,7 +390,7 @@ impl MultiThreadBase for FirecrackerTarget { while !data.is_empty() { let gpa = arch::translate_gva(&vcpu_state.vcpu_fd, gva, &vmm).map_err(|e| { - error!("Error {e:?} translating gva on read address: {gva:X}"); + error!("Error {e:?} translating gva on read address: {gva:#X}"); })?; // Compute the amount space left in the page after the gpa @@ -424,7 +424,7 @@ impl MultiThreadBase for FirecrackerTarget { while !data.is_empty() { let gpa = arch::translate_gva(&vcpu_state.vcpu_fd, gva, &vmm).map_err(|e| { - error!("Error {e:?} translating gva on read address: {gva:X}"); + error!("Error {e:?} translating gva on read address: {gva:#X}"); })?; // Compute the amount space left in the page after the gpa @@ -436,7 +436,7 @@ impl MultiThreadBase for FirecrackerTarget { vmm.guest_memory() .write(&data[..write_len], GuestAddress(gpa)) .map_err(|e| { - error!("Error {e:?} writing memory at {gpa:X}"); + error!("Error {e:?} writing memory at {gpa:#X}"); })?; data = &data[write_len..]; diff --git a/src/vmm/src/vstate/vcpu/x86_64.rs b/src/vmm/src/vstate/vcpu/x86_64.rs index 6fee3933435..a1bb22d1bb7 100644 --- a/src/vmm/src/vstate/vcpu/x86_64.rs +++ b/src/vmm/src/vstate/vcpu/x86_64.rs @@ -61,7 +61,7 @@ pub enum KvmVcpuError { VcpuGetLapic(kvm_ioctls::Error), /// Failed to get KVM vcpu mp state: {0} VcpuGetMpState(kvm_ioctls::Error), - /// Failed to get KVM vcpu msr: 0x{0:x} + /// Failed to get KVM vcpu msr: {0:#x} VcpuGetMsr(u32), /// Failed to get KVM vcpu msrs: {0} VcpuGetMsrs(kvm_ioctls::Error), @@ -321,7 +321,7 @@ impl KvmVcpu { .filter(|msr| msr.index == MSR_IA32_TSC_DEADLINE && msr.data == 0) .for_each(|msr| { warn!( - "MSR_IA32_TSC_DEADLINE is 0, replacing with {:x}.", + "MSR_IA32_TSC_DEADLINE is 0, replacing with {:#x}.", tsc_value ); msr.data = tsc_value; diff --git a/src/vmm/src/vstate/vm.rs b/src/vmm/src/vstate/vm.rs index 0f72abcf68f..3d24dc6f9ac 100644 --- a/src/vmm/src/vstate/vm.rs +++ b/src/vmm/src/vstate/vm.rs @@ -34,7 +34,7 @@ use crate::vstate::memory::{Address, GuestMemory, GuestMemoryMmap, GuestMemoryRe pub enum VmError { /// The host kernel reports an invalid KVM API version: {0} ApiVersion(i32), - /// Missing KVM capabilities: {0:x?} + /// Missing KVM capabilities: {0:#x?} Capabilities(u32), /** Error creating KVM object: {0} Make sure the user launching the firecracker process is \ configured on the /dev/kvm file's ACL. */ From 22e18704b19466bfac2a73b5432d68b95872a999 Mon Sep 17 00:00:00 2001 From: Patrick Roy Date: Mon, 25 Nov 2024 10:12:59 +0000 Subject: [PATCH 3/3] fix: make aarch64_with_sve_and_pac template work with >=2 vcpus Setting the least significant 4 bits to zero overwrites some configuration that Firecracker sets on secondary CPUs that is needed for them to be able to be booted. With these 4 bits set to 0, the CPUs will never show up as online inside the guest, and KVM will just infinitely spin inside KVM_RUN for them. Fix by using 'x' in the template, which preserves the defaults set by Firecracker. Signed-off-by: Patrick Roy --- tests/data/static_cpu_templates/aarch64_with_sve_and_pac.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/data/static_cpu_templates/aarch64_with_sve_and_pac.json b/tests/data/static_cpu_templates/aarch64_with_sve_and_pac.json index b155d81dc34..29e47be4a92 100644 --- a/tests/data/static_cpu_templates/aarch64_with_sve_and_pac.json +++ b/tests/data/static_cpu_templates/aarch64_with_sve_and_pac.json @@ -1,4 +1,4 @@ { "kvm_capabilities": ["170", "171", "172"], - "vcpu_features": [{ "index": 0, "bitmap": "0b1110000" }] + "vcpu_features": [{ "index": 0, "bitmap": "0b111xxxx" }] }