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>
These structs directly embed another struct and then implement
ByteValued on that struct to implement ByteValued for the inner struct.
As such the inner struct is never directly accessed so to avoid the dead
code analysis mark this as allowed.
Beta clippy fix:
warning: field `0` is never read
--> arch/src/x86_64/mod.rs:129:32
|
129 | struct MemmapTableEntryWrapper(hvm_memmap_table_entry);
| ----------------------- ^^^^^^^^^^^^^^^^^^^^^^
| |
| field in this struct
|
= note: `MemmapTableEntryWrapper` has a derived impl for the trait `Clone`, but this is intentionally ignored during dead code analysis
= note: `#[warn(dead_code)]` on by default
help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field
|
129 | struct MemmapTableEntryWrapper(());
|
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
When the HTT flag CPUID.1.EDX[HTT] is 0, it indicates that there is
only a single logical processor in the package. When HTT is 1, it
indicates that CPUID.1.EBX[23:16] contains the number of logical
processors in the package.
When this information is not included in CPUID leaf 0x1, some cpu
topology enumeration software such as hwloc are known to crash.
Signed-off-by: Thomas Barrett <tbarrett@crusoeenergy.com>
When using amd topology, the svm feature flag on cpuid leaf
0x8000_0001.ecx is overwritten. We update the amd cpu topology
logic to use the flag values that originated in
KVM_GET_SUPPORTED_CPUID ioctl and override as necessary.
Signed-off-by: Thomas Barrett <tbarrett@crusoeenergy.com>
warning: this URL is not a hyperlink
--> arch/src/aarch64/layout.rs:114:58
|
114 | ...in https://www.kernel.org/doc/Documentation/arm64/booting.txt.
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://www.kernel.org/doc/Documentation/arm64/booting.txt.>`
|
= note: bare URLs are not automatically turned into clickable links
= note: `#[warn(rustdoc::bare_urls)]` on by default
I also noticed that it looks like this comment was supposed to be
applied to FDT_MAX_SIZE, not FDT_START, so I moved it.
Signed-off-by: Alyssa Ross <hi@alyssa.is>
Under the fuzzer this code appears dead:
error: field `0` is never read
--> /home/rob/src/cloud-hypervisor/arch/src/x86_64/mod.rs:128:32
|
128 | struct MemmapTableEntryWrapper(hvm_memmap_table_entry);
| ----------------------- ^^^^^^^^^^^^^^^^^^^^^^
| |
| field in this struct
|
= note: `MemmapTableEntryWrapper` has a derived impl for the trait `Clone`, but this is intentionally ignored during dead code analysis
= note: `-D dead-code` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(dead_code)]`
help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field
|
128 | struct MemmapTableEntryWrapper(());
| ~~
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
CI reports errors:
error: writing `&Vec` instead of `&[_]` involves a new object where a slice will do
--> arch/src/x86_64/mod.rs:1351:19
|
1351 | epc_sections: &Vec<SgxEpcSection>,
| ^^^^^^^^^^^^^^^^^^^ help: change this to: `&[SgxEpcSection]`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg
= note: `-D clippy::ptr-arg` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::ptr_arg)]`
Signed-off-by: Yi Wang <foxywang@tencent.com>
