This is the second patch in a series intended to let Cloud Hypervisor
support more than 255 vCPUs in guest VMs; the first patch/commit is
https://github.com/cloud-hypervisor/cloud-hypervisor/pull/7231
At the moment, CPU topology in Cloud Hypervisor is using
u8 for components, and somewhat inconsistently:
- struct CpuTopology in vmm/src/vm_config.rs uses four components
(threads_per_core, cores_per_die, dies_per_package, packages);
- when passed around as a tuple, it is a 3-tuple of u8, with
some inconsistency:
- in get_x2apic_id in arch/src/x86_64/mod.rs the three u8
are assumed to be (correctly)
threads_per_core, cores_per_die, and dies_per_package, but
- in get_vcpu_topology() in vmm/src/cpu.rs the three-tuple is
threads_per_core, cores_per_die, and packages (dies_per_package
is assumed to always be one? not clear).
So for consistency, a 4-tuple is always passed around.
In addition, the types of the tuple components is changed from u8 to
u16, as on x86_64 subcomponents can consume up to 16 bits.
Again, config constraints have not been changed, so this patch
is mostly NOOP.
Signed-off-by: Barret Rhoden <brho@google.com>
Signed-off-by: Neel Natu <neelnatu@google.com>
Signed-off-by: Ofir Weisse <oweisse@google.com>
Signed-off-by: Peter Oskolkov <posk@google.com>
This is the first change to Cloud Hypervisor in a series of changes
intended to increase the max number of supported vCPUs in guest VMs,
which is currently limited to 255 (254 on x86_64).
No user-visible/behavior changes are expected as a result of
applying this patch, as the type of boot_cpus and related
fields in config structs remains u8 for now, and all configuration
validations remain the same.
Signed-off-by: Barret Rhoden <brho@google.com>
Signed-off-by: Neel Natu <neelnatu@google.com>
Signed-off-by: Ofir Weisse <oweisse@google.com>
Signed-off-by: Peter Oskolkov <posk@google.com>
This allows the fw_cfg device to be recognized by the guest linux
kernel. This becomes more relavnt in the following cl where I add
the option to load files into the guest via fw_cfg. The Linux kernel
already has a fw_cfg driver that will automatically load these files
under /sys when CONFIG_FW_CFG_SYSFS is enabled in the kernel config
For arm we must add fw_cfg to the devices tree
Signed-off-by: Alex Orozco <alexorozco@google.com>
Checksumming a type that has padding would use the padding in the
checksum, which is definitely wrong and is often unsound.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
There are a little many cpuid logs now. When starting a vm with
64 vcpu, we can get more than four thousand INFO messages:
cat vm1.log |grep 'arch/src/x86_64/mod.rs:891' |wc -l
4352
Signed-off-by: Yi Wang <foxywang@tencent.com>
This streamlines the Error implementation in the Cloud Hypervisor code
base to match the remaining parts so that everything follows the agreed
conventions. These are leftovers missed in the previous commits.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
When booting a Linux guest in SMP configuration,
the following kernel warning can be observed:
[Firmware Bug]: CPUID leaf 0xb subleaf 1 APIC ID mismatch 1 != 0
The reason is that we announce the presence of the extended topology
leaf, but fail to announce the x2apic ID in EDX.
Signed-off-by: Thomas Prescher <thomas.prescher@cyberus-technology.de>
On-behalf-of: SAP thomas.prescher@sap.com
This streamlines the code base to follow best practices for
error handling in Rust: Each error struct implements
std::error::Error (most due via thiserror::Error derive macro)
and sets its source accordingly.
This allows future work that nicely prints the error chains,
for example.
So far, the convention is that each error prints its
sub error as part of its Display::fmt() impl.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This streamlines the code base to follow best practices for
error handling in Rust: Each error struct implements
std::error::Error (most due via thiserror::Error derive macro)
and sets its source accordingly.
This allows future work that nicely prints the error chains,
for example.
So far, the convention is that each error prints its
sub error as part of its Display::fmt() impl.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This streamlines the code base to follow best practices for
error handling in Rust: Each error struct implements
std::error::Error (most due via thiserror::Error derive macro)
and sets its source accordingly.
This allows future work that nicely prints the error chains,
for example.
So far, the convention is that each error prints its
sub error as part of its Display::fmt() impl.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This streamlines the code base to follow best practices for
error handling in Rust: Each error struct implements
std::error::Error (most due via thiserror::Error derive macro)
and sets its source accordingly.
This allows future work that nicely prints the error chains,
for example.
So far, the convention is that each error prints its
sub error as part of its Display::fmt() impl.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Fixing the following clippy issue using `cargo clippy --fix`:
error: variables can be used directly in the `format!` string
--> build.rs:25:27
|
25 | version.push_str(&format!("-{}", extra_version));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Num SPI and Base SPI nodes should be added before the end node to be
included in the device tree generation.
Fixes: eac44e6 (arch: Extend FDT for GICv2M device for ARM64 on MSHV)
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
Since this would be used in other places inside the hypervisor and
hypervisor crate cannot take a dependency on arch crate, as that creates
cyclic dependency.
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
Currently PPI interrupt ID are hardcoded as numbers, it would be ideal
to define them as constants and could be reused in other parts of the
hypervisor crate.
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
GICv2M requires two additional properties to be exposed via FDT:
1) Base SPI number and 2) Total number of SPIs. SPIs in general starts
from 32 and goes upto 1019. But currently we are limiting the range to
96 as that should be good enough for any normal Linux guest to function.
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
There are other potential users of these registers definitions in the
hypervisor crate. And hypervisor crate cannot use definitions from arch
crate because it creates cyclic dependency.
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
This is a typo here, `interrupt-controller` attribute of each CPU node
should not have a `#` prepended.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Integrate fdt setup and layout for `riscv64` to `arch` crate, provide
definitions of `riscv64` platform specific error and reuse
`MmioDeviceInfo`, `PciSpaceInfo` structures and `DeviceInfoForFdt` impl
block for `riscv64`.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Reused layout set for `aarch64` as much as possible to provide a concise
and compatible (for devices) layout for RISC-V 64-bit.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Asserting on .is_ok()/.is_err() leads to hard to debug failures (as if
the test fails, it will only say "assertion failed: false". We replace
these with `.unwrap()`, which also prints the exact error variant that
was unexpectedly encountered (we can to this these days thanks to
efforts to implement Display and Debug for our error types). If the
assert!((...).is_ok()) was followed by an .unwrap() anyway, we just drop
the assert.
Inspired by and quoted from @roypat.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
By introducing `imports_granularity="Module"` format strategy,
effectively groups imports from the same module into one line or block,
improving maintainability and readability.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Historically the Cloud Hypervisor coding style has been to ensure that
all imports are ordered and placed in a single group. Unfortunately
cargo fmt has no support for ensuring that all imports are in a single
group so if whitespace lines were added as part of the import statements
then they would only be odered correctly in the group.
By adopting "group_imports="StdExternalCrate" we can enforce a style
where imports are placed in at most three groups for std, external
crates and the crate itself. Choosing a style enforceable by the tooling
reduces the reviewer burden.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
An example warning output is:
error: first doc comment paragraph is too long
--> virtio-devices/src/lib.rs:158:1
|
158 | / /// Convert an absolute address into an address space (GuestMemory)
159 | | /// to a host pointer and verify that the provided size define a valid
160 | | /// range within a single memory region.
161 | | /// Return None if it is out of bounds or if addr+size overlaps a single region.
| |_
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
= note: `-D clippy::too-long-first-doc-paragraph` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::too_long_first_doc_paragraph)]`
Signed-off-by: Bo Chen <chen.bo@intel.com>
With this we are removing the CloudHypervisor definition of
StandardRegisters instead using an enum which contains different
variants of StandardRegisters coming from their bindigs crate.
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
HV APIC(i.e., synthetic APIC controller exposed by Microsoft Hypervisor)
does not support one-shot operation using a TSC deadline value. Due to
which we see the following backtrace inside the guest when running with
hypervisor-fw/OVMF:
[ 0.560765] unchecked MSR access error: WRMSR to 0x832 (tried to
write 0x00000000000400ec) at rIP: 0xffffffff8f473594
(native_write_msr+0x4/0x30)
[ 0.560765] Call Trace:
[ 0.560765] ? native_apic_msr_write+0x2b/0x30
[ 0.560765] __setup_APIC_LVTT+0xbc/0xe0
[ 0.560765] lapic_timer_set_oneshot+0x27/0x30
[ 0.560765] clockevents_switch_state+0xaf/0xf0
[ 0.560765] tick_setup_periodic+0x47/0x90
[ 0.560765] tick_setup_device.isra.0+0x7c/0x110
[ 0.560765] tick_check_new_device+0xce/0xf0
[ 0.560765] clockevents_register_device+0x82/0x170
[ 0.560765] clockevents_config_and_register+0x2f/0x40
[ 0.560765] setup_APIC_timer+0xe1/0xf0
[ 0.560765] setup_boot_APIC_clock+0x5f/0x66
[ 0.560765] native_smp_prepare_cpus+0x1d6/0x286
[ 0.560765] kernel_init_freeable+0xcf/0x255
[ 0.560765] ? rest_init+0xb0/0xb0
[ 0.560765] kernel_init+0xe/0x110
[ 0.560765] ret_from_fork+0x22/0x40
Also, if this feature is exposed guest would not finish booting and get
stuck right before unpacking the root filesystem.
Fixes: 06e8d1c40 ("hypervisor: mshv: fix topology for Intel HW on MSHV")
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
If the KVM version is older than v6.6, KVM_GET_SUPPORTED_CPUID will omit
the L1 cache information in CPUID function 0x8000_0005. Fall back to
the host L1 cache information if it is omitted by KVM.
Signed-off-by: Sean Banko <sbanko@crusoe.ai>
Rather than calling cpuid and then updating the APIC ID field - use the
existing common CPUID data which already includes CPUID data for eax=1
(aka function = 1). This removes the need to call cpuid per vCPU thread
created. This has a positive impact on boot time with multiple vCPUs as
the cpuid instruction is serialising.
Fixes: #5646
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Clippy complained one use of the char_array macro.
Instead of fixing that use case, the observation is that the macro was
needed only because MP table types mixed c_char and c_uchar for no
particular reason.
Only use c_uchar in those types, and drop char_array.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
In accordance with reuse requirements:
- Place each license file in the LICENSES/ directory
- Add missing SPDX-License-Identifier to files.
- Add .reuse/dep5 to bulk-license files
Fixes: #5887
Signed-off-by: Ruslan Mstoi <ruslan.mstoi@intel.com>
With the nightly toolchain (2024-02-18) cargo check will flag up
redundant imports either because they are pulled in by the prelude on
earlier match.
Remove those redundant imports.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Allow cloud-hypervisor to direct boot the bzImage kernel format using
the regular 32 bit entry point. This can share the memory and vcpu
setup with the regular PVH boot code, but requires the setup of the
'zero page'.
Signed-off-by: Stefan Nuernberger <stefan.nuernberger@cyberus-technology.de>
The 'generate_ram_ranges' function currently hardcodes the assumption
that there are only 2 E820 RAM entries. This is not flexible enough to
handle vendor specific memory holes. Returning a Vec is also more
convenient for users of this function.
Signed-off-by: Thomas Barrett <tbarrett@crusoeenergy.com>
The `ByteValued` trait implementations for the data structures from the
'linux_loader' crate are no longer needed, and hence their wrappers can
be removed.
Signed-off-by: Bo Chen <chen.bo@intel.com>