submit_batch_requests pushed each BatchRequest into the io_uring SQ in
turn and used `?` to bail on the first push failure.
Leaving the initial SQEs visible to the kernel — but submitter.submit()
was never called, and every other call site in this file gates submit()
behind a preceding sq.push() that now also fails on the full ring.
This could allow a guest to DoS it's own queue or worse if the buffer is
freed early.
Signed-off-by: Dylan Reid <dgreid@fb.com>
(cherry picked from commit ee315d2e7c)
The bounce buffer for an unaligned descriptor was allocated in
execute_async and leaked on error paths, even though, for the sync
case the kernel already had a pointer to the buffer.
Clean this up by moving ownership of the buffer to the
AlignedOperation type. To make it actually safe, stop stashing a
guest memory pointer for the duration of the op. Instead, save the
guest address and pass guest memory back to the complete function.
Signed-off-by: Dylan Reid <dgreid@fb.com>
(cherry picked from commit 1b8c92dd5e3c0c58316826486ce5ee30eeb71407))
[backport: adapted to stable/v51.x; v51.x has no block/src/request.rs
split, so the new aligned_operation module is added next to
block/src/lib.rs and the in tree struct, alloc, free path is
replaced in place.]
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
For non-batch backends execute_async submits the kernel I/O inline
before returning. An early return while processing before inserting in
inflight_requests, meant the request went untracked, the local batch
list was never appended to inflight_requests, even though the request is
pending in the kernel.
To track it, insert into self.inflight_requests as soon as execute_async
returns Ok. The completion path's find_inflight_request now matches the
orphan and the bounce buffer is freed only after the kernel signals it
is done.
Signed-off-by: Dylan Reid <dgreid@fb.com>
(cherry picked from commit fa8acbd712)
A malicious or buggy guest can violate virtio by making the same
descriptor head available twice before the first chain has been placed
on the used ring. The submit path pushed both chains onto the
VecDeque-backed inflight_requests keyed by head_index, and on completion
find_inflight_request() returned the first linear match. That Request's
complete_async() freed its bounce buffer while the other chain's
io_uring op was still targeting it, producing a use-after-free the
kernel could then scribble into.
Signed-off-by: Dylan Reid <dgreid@fb.com>
(cherry picked from commit 544fa4aa76)
A buggy or malicious guest may write an inappropriate value into
virtqueue's next_avail field. This will result in an error
when iterating over the queue:
863837ef86/virtio-queue/src/queue.rs (L708)
but this error is (logged and) ignored if pop_descriptor_chain()
is used:
863837ef86/virtio-queue/src/queue.rs (L583)
A reasonable approach, implemented here, is to mark the device as
NEEDS_RESET and ignore further queue events until the guest
reinitializes the device.
How this patch was tested:
Linux kernel was patched to trigger a bad next_avail when the
virtqueue queue counter reaches 5000:
--------------- START OF LINUX KERNEL PATCH ----------
$ git diff
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b784aab668670..989f2a0c64a77 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -15,6 +15,9 @@
#include <linux/spinlock.h>
#include <xen/xen.h>
+
+void virtqueue_kick_always(struct virtqueue *vq);
+
#ifdef DEBUG
/* For development, we want to crash whenever the ring is screwed. */
#define BAD_RING(_vq, fmt, args...) \
@@ -677,6 +680,12 @@ static inline int virtqueue_add_split(
struct virtqueue *_vq,
* new available array entries. */
virtio_wmb(vq->weak_barriers);
vq->split.avail_idx_shadow++;
+ {
+ if ((vq->split.avail_idx_shadow % 100) == 0)
+ printk(KERN_ERR "avail idx: %d",
+ (int)vq->split.avail_idx_shadow);
+ if (vq->split.avail_idx_shadow == 5000)
+ vq->split.avail_idx_shadow = 0;
+ }
vq->split.vring.avail->idx = cpu_to_virtio16(_vq->vdev,
vq->split.avail_idx_shadow);
vq->num_added++;
@@ -689,6 +698,11 @@ static inline int virtqueue_add_split(
struct virtqueue *_vq,
if (unlikely(vq->num_added == (1 << 16) - 1))
virtqueue_kick(_vq);
+ {
+ if (unlikely(vq->split.avail_idx_shadow == 0))
+ virtqueue_kick_always(_vq);
+ }
+
return 0;
unmap_release:
@@ -2515,6 +2529,11 @@ bool virtqueue_kick(struct virtqueue *vq)
}
EXPORT_SYMBOL_GPL(virtqueue_kick);
+void virtqueue_kick_always(struct virtqueue *vq)
+{
+ virtqueue_kick_prepare(vq);
+ virtqueue_notify(vq);
+}
/**
* virtqueue_get_buf_ctx - get the next used buffer
* @_vq: the struct virtqueue we're talking about.
--------------- END OF LINUX KERNEL PATCH ----------
Then the kernel was booted, and the host pinged until the
nic became unresponsive:
ping -i 0.002 192.168.4.1
Device status was confirmed using
cat /sys/class/net/eth0/device/status
(it was 0x4f).
Then the device was re-initialized:
DEV_NAME=$(basename $(readlink -f /sys/class/net/eth0/device))
echo $DEV_NAME | tee /sys/bus/virtio/drivers/virtio_net/unbind
echo $DEV_NAME | tee /sys/bus/virtio/drivers/virtio_net/bind
ip link set eth0 up
At this point networking became healthly again.
Signed-off-by: Peter Oskolkov <posk@google.com>
(cherry picked from commit 563303b50a)
As well as rejecting writes to sector 0 in the case of raw files where
the user hasn't specified the image_type also reject virtio requests of
type discard and write_zeroes.
Signed-off-by: Rob Bradford <rbradford@meta.com>
If the disk image was autodetected to raw (not specified with image_type
= 0) then in the virtio-block subsystem generate errors for writes to
block 0 (treat as if read-only). This gives an immediate error vs using
the image implementations in the block subsystem.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Add an image_type to DiskConfig to specify the image type. If none is
specified autodetect the image type but disable potentially unsafe
behaviour in the QCOW2 backend by disabling the backing file support.
If the image type is autodetected then fix it in the config so that it
will be persistant across reboots and migrations/snapshot & restores.
This also handles the case where the image type was not specified as
part of the disk configuration.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Verify that opening a QCOW2 image with a backing file reference
through QcowDiskSync with backing_files=off produces the user-facing
BackingFilesDisabled error rather than MaxNestingDepthExceeded.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
When a QCOW2 image has a backing file but backing_files=on is not set,
the error was MaxNestingDepthExceeded which gives no indication that
this is a policy decision or how to resolve it.
Add a BackingFilesDisabled error variant whose message indicates that
backing file support is disabled and references the backing_files
option. The translation from MaxNestingDepthExceeded to
BackingFilesDisabled happens at the QcowDiskSync boundary where the
policy decision is made, preserving the original error for genuine
recursive depth exhaustion.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The 0.6.7 version of the mshv crates introduced a new version of
make_default_partition_create_arg inside `struct Mshv`. This version
queries the available processor features on the host and gives the same
feature set to the guests.
Move Cloud Hypervisor to this new function.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
test_guest_numa_generic_initiator was missing the #[test] attribute and
did not match the VFIO CI filter pattern "vfio::test_nvidia"
Add #[test] and rename to test_nvidia_guest_numa_generic_initiator so
the existing CI infrastructure picks it up on the vfio-nvidia runner.
Fixes: #7718
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
The device_id field was added to both NumaConfig and NumaNode as part
of the Generic Initiator support, but create_numa_nodes() change
was missed when the commits were reorganized.
As a result, node.device_id is never propogated from the config to
the runtime node and the ACPI SRAT Type 5 (Generic Initiator Affinity)
entries were never emitted.
Add the missing propogation so that create_srat_table() can resolve
the device and emit the correct affinity structure
Fixes: #7717
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
Add performance tests for standalone qcow2 images without backing
files - uncompressed, zlib and zstd compressed. Each variant
includes single queue and multiqueue tests for sequential
read, random read and warmed up sequential read.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add multiqueue num_queues=4 performance tests for qcow2 overlay
images with both qcow2 and raw backing files - sequential read,
random read, and warm read variants.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The backing_files option defaults to false, so qcow2 overlay
tests fail with MaxNestingDepthExceeded. Pass backing_files=on
when the test file is an overlay.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The focal image checksums have been moved in the -common
sha1sums file. Use the correct file for metrics.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Unlike most virtio feature bits, VIRTIO_BLK_F_RO is not optional.
It indicates that the host is refusing to permit write operations, and
the guest must not be allowed to override it.
However, the block device currently does not enforce this. If the guest
does not negotiate VIRTIO_BLK_F_RO, the block device will think the
device is writable and forward write requests to the backend.
This is not a security problem right now because the backing device of a
read-only device is always opened read-only. The kernel will thus
reject the write operations with EBADF. If support is added for
receiving the backing device file descriptor via SCM_RIGHTS (#7704),
it will be possible to have a read-only block device backed by a
writable file descriptor. This would make the bug a genuine security
vulnerability.
Fix the bug by explicitly checking if VIRTIO_BLK_F_RO was offered but
not negotiated. In this case, log a warning and proceed as if the guest
did acknowledge the feature. This always indicates a guest driver bug.
Fixes: #7697
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
OVMF sends FLUSH requests to read-only virtio-block devices. Refusing
these requests prevents OVMF from accessing the EFI System Partition and
therefore makes VMs unable to boot. Accept these requests instead.
them.
Ignoring these requests is possible, but inconsistent with fsync(2)
which honors them.
Fixes: #7698
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
Add handling for GHCB_INFO_SPECIAL_DBGPRINT VMG exit in the SEV-SNP
guest exit handler. This exit occurs when the guest sends debug print
requests through the GHCB interface.
Without this handler, SEV-SNP guests fail to boot when debug output
is triggered, such as when a debugger is attached to the guest image.
The handler acknowledges the exit without printing to avoid performance
degradation from frequent debug print requests.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
The --net help text documented fd as fd=<fd1,fd2...>, but
comma-separated FD lists in option values must be bracketed to avoid
top-level option splitting.
Update NetConfig::SYNTAX to use fd=<[fd1,fd2,...]>, matching parser
behavior and existing net parsing tests:
`cargo test -p vmm test_net_parsing`
On-behalf-of: SAP leander.kohler@sap.com
Signed-off-by: Leander Kohler <leander.kohler@cyberus-technology.de>
Add comprehensive integration tests for DISCARD and WRITE_ZEROES:
Multiqueue stress tests verify concurrent operations across queues,
testing scattered writes with simultaneous fstrim, and write/discard
races that stress refcount table locking.
Format specific tests verify QCOW2 deallocates clusters after DISCARD,
raw files create holes using fallocate, and unsupported formats VHD
and VHDX correctly reject DISCARD requests.
Tests for sparse=off verify raw files preallocate full disk size and
QCOW2 uses zero flag instead of deallocating clusters.
Add helper functions to verify sparse files, count QCOW2 zero flagged
regions using qemu-img map, and verify guest reads zeros from
discarded regions.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add comprehensive tests for DISCARD and WRITE_ZEROES operations:
QCOW2 zero flag test validates the complete workflow: allocate
cluster, DISCARD it, verify reads return zeros, write new data,
verify cluster reallocated.
QcowSync tests verify punch_hole and write_zeroes with Arc<Mutex<>>
sharing, including tests for cache consistency with multiple async
I/O operations.
RawFileSync tests verify punch_hole and write_zeroes using
fallocate.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Implement punch_hole() and write_zeroes() for raw file backends using
io_uring and fallocate.
punch_hole() uses FALLOC_FL_PUNCH_HOLE to deallocate storage.
write_zeroes() uses FALLOC_FL_ZERO_RANGE to write zeros efficiently.
Both use FALLOC_FL_KEEP_SIZE to maintain file size.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Implement punch_hole and write_zeroes for QcowSync backend by
delegating to QcowFile::punch_hole which triggers cluster
deallocation. write_zeroes delegates to punch_hole as unallocated
clusters read as zeros in QCOW2.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add VIRTIO_BLK_T_DISCARD and VIRTIO_BLK_T_WRITE_ZEROES request types.
Parse discard/write_zeroes descriptors (sector, num_sectors, flags),
convert to byte offsets, and call punch_hole/write_zeroes on the disk
backend. Mark as unsupported in sync mode.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Implement DISCARD using QCOW2 zero flag (bit 0 of L2 entries) with
sparse aware behavior.
When sparse=true - fully deallocate clusters by decrementing
refcount, clearing L2 entry, and reclaiming storage via punch_hole
when refcount reaches zero.
When sparse=false - use zero flag to keep storage allocated while
marking as reading zeros. Only works when cluster is not shared.
Shared clusters are fully deallocated.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
When sparse=false is configured, preallocate the entire raw disk file
at startup using fallocate(). This provides space reservation and
reduces fragmentation.
Only applies to raw disks. QCOW2/VHD/VHDX formats manage their own
allocation.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add sparse parameter to QcowFile constructors and propagate it from
device_manager through QcowDiskSync. This makes the sparse configuration
available throughout the QCOW2 implementation for controlling allocation
and deallocation behavior.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add supports_zero_flag() to DiskFile trait to indicate whether a disk
format can mark clusters/blocks as reading zeros without deallocating
storage.
QCOW2 supports this via the zero flag in L2 entries. VHDX also has
PAYLOAD_BLOCK_ZERO state for this, though it's not yet implemented in
cloud-hypervisor.
This enables DISCARD to be advertised even with sparse=false for formats
with zero-flag support, since they can mark regions as zeros (keeps
storage allocated) instead of requiring full deallocation.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add sparse boolean configuration option to DiskConfig with a default
value of true to control disk space allocation behavior.
When sparse is true, the disk uses sparse allocation where deallocated
blocks are returned to the filesystem, and the DISCARD feature is
advertised to the guest.
When sparse is false, disk space is kept fully allocated and DISCARD
is not advertised.
WRITE_ZEROES is always advertised when the backend supports it,
regardless of the sparse setting.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add capability query to DiskFile trait to check backend
support for sparse operations (punch hole, write zeroes,
discard). Only advertise VIRTIO_BLK_F_DISCARD and
VIRTIO_BLK_F_WRITE_ZEROES when the backend supports these
operations.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add functions to probe whether a file or block device actually
supports PUNCH_HOLE and ZERO_RANGE operations at runtime. The
probe is performed at file open time by testing the operations
at EOF with a zero-length range, which is a safe no-op.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add punch_hole() and write_zeroes() methods to the AsyncIo trait
with stub implementations for all backends. These will be used to
support DISCARD and WRITE_ZEROES operations.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Decompose the monolithic `new_from_memory_manager` function into
smaller, focused helper methods to improve code readability,
maintainability, and testability.
Changes:
- Extract `should_force_iommu()` to determine IOMMU requirements for
confidential computing (TDX/SEV-SNP)
- Extract `should_stop_on_boot()` to check debug pause configuration
- Extract `create_cpu_manager()` to encapsulate CPU manager creation
and CPUID population
- Extract `init_tdx_if_enabled()` for TDX-specific VM initialization
- Extract `create_device_manager()` to encapsulate device manager setup
- Extract `hypervisor_specific_init()` to orchestrate initialization
sequences for different hypervisors (KVM, MSHV, SEV-SNP)
- Extract `init_sev_snp()` for SEV-SNP confidential VM setup
- Extract `init_mshv()` for MSHV hypervisor initialization
- Extract `init_kvm()` for KVM hypervisor initialization
- Extract `create_fw_cfg_if_enabled()` for fw_cfg device creation
This refactoring replaces complex nested `cfg_if!` blocks with cleaner
conditional method calls, providing clear separation between hypervisor-
specific initialization paths while preserving existing functionality.
No functional changes intended.
Issue: https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7598
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Update .lychee.toml to exclude the following patterns:
- ARM domains (developer.arm.com, infocenter.arm.com) which return 403
Forbidden due to anti-bot protections in CI.
- Local TCP addresses (192.168.1.10) which are unsupported by the
link-checker tool.
- The .lychee.toml file itself, to prevent the tool from recursively
checking its own regex exclusion patterns as valid URLs.
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
Document device_id parameter in NumaConfig, automatic
guest_numa_id assignment, default NUMA distances and
restrictions on Generoc Initiator NUMA nodes
Add numa configuration examples with GPU device and distance
relationships.
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
Add test_guest_numa_generic_initiator to validate ACPI Generic
Initiator Affinity (SRAT Type 5) support for VFIO device.
The test verifies the following :
- Guest VM boots with a VFIO device associated with a {cpu,
memort}-less NUMA node
- Guest Kernel correctly detects Generic Initiator through
ACPI tables SRAT, SLIT
- NUMA topology in the guest includes the device-only node
with correct distances
Invoked via :
./scripts/dev_cli.sh tests --integration -- --hypervisor kvm \
--test-filter test_guest_numa_generic_initiator
The test requires a real VFIO device bound to vfio-pci driver and
skips gracefully if hardware is unavailable.
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
Update FDT generation to skip NUMA properties when Generic Initiator
nodes are present, preventing conflicts between FDT and ACPI NUMA
information. FDT cannot represent Generic Initiator nodes, so ACPI
(via SRAT Type 5) becomes the authoritative source for the entire
NUMA topology when Generic Initiators exist.
Skip FDT numa-node-id properties in CPU and memory nodes
when Generic Initiator is present
Distance map bug fix : iterate over actual NUMA node IDs instead
of 0..len()
Use distance symmetry to derive distance when forward config is
missing
Default to distance cost 20 when neither direction specified
Only create memory nodes if NUMA node has memory region
Added unit tests
ARM64 boot protocol:
https://docs.kernel.org/arch/arm64/booting.html
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
Support ACPI Generic Initiator Affinity to associate
PCI devices with NUMA proximity domains
Add GenericInitiatorAffinity struct
Add from_pci_bdf() to encode PCI Segment:Bus:Device.Function
Add from_acpi_device() for ACPI device handles (future use)
Generate SRAT Type 5 entries for nodes with device_id
Improve create_slit_table() to check distance symmetry when
forward distance is missing
Track device ID to BDF mappings in DeviceManager
Includes comprehensive unit tests
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
Validate device_id in numa config is mutually
exclusive with cpus and memory_zones
Add NumaConfig::validate() and modify NumaConfig::parse()
Add ValidationError::InvalidNumaConfig for detailed error
messages
Include unit tests covering valid and invalid configs
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
Add an optional device_id string field to NumaConfig for identifying
PCI devices associated with a NUMA node. This is used by the Generic
Initiator support to map devices to their proximity domain.
Update OpenAPI spec (cloud-hypervisor.yaml) to include the
new device_id field in the NumaConfig schema.
The device_id is optional and parsed from the --numa parameter:
--numa "device_id=<device_id>,distances=[...],..."
The optional field is accepted but not used.
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
The documentation says guest_numa_id is required to be unique and
therefore the parser() giving default value for non-existing
guest_numa_id with .unwrap_or(0) is dangerous.
Return a validation error if guest_numa_id is not provided instead
of silently defaulting to 0.
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
Verify live resize of QCOW2 disks works via the API, including
resizing that requires L1 table growth.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add support for live resizing QCOW2 images. This enables growing
the virtual size of a QCOW2 disk while the VM is running.
Key features:
- Growing the image automatically expands the L1 table if needed
- Shrinking is not supported
- Resizing for images with backing files is not supported
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Provide a stub implementation for save_data_tables() to unblock pause
functionality. Without this, pausing a VM causes Cloud Hypervisor to
panic due to the unimplemented!() macro. This unblocks the
test_api_http_pause_resume testcase. We don't need to save any state
just to pause and resume.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
Instead of closing a file descriptor that belongs to the vhost-user
frontend, drop the vu_common_ctrl::VhostUserHandle and the
vhost::vhost_user::Frontend it contains. This causes the destructor to
drop the file descriptor.
This breaks the last DPDK test, so disable it. See #7689.
Fixes: #7163
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
When a VFIO device with multiple MMIO regions is hot-unplugged, each
region must be individually matched and removed from the DeviceManager's
mmio_regions list. Compare per-region rather than building an aggregate
across all regions, which would never match any individual entry.
Also remove the now-unused HashSet import.
Signed-off-by: Damian Barabonkov <dbctl@pm.me>
Change has_matching_slots() to compare two MmioRegion instances
directly rather than requiring callers to construct an intermediate
HashSet of slot numbers. Remove the now-unused
user_memory_region_slots() method and HashSet import.
Signed-off-by: Damian Barabonkov <dbctl@pm.me>
Replace `clone()` with `take()` when retrieving device configurations
from `DeviceManager.config`.
This avoids unnecessarily copying the device configuration lists (e.g.,
`disks`, `net`, `fs`) when they are being processed and subsequently
moved out of the configuration. This optimization improves performance
by reducing memory allocations and cloning overhead.
Signed-off-by: Changyuan Lyu <changyuanl@google.com>
Since kernel commit 6693731487a8 ("vsock/virtio: Allocate nonlinear SKBs
for handling large transmit buffers"), a large vsock packet can be split
into multiple descriptors.
If we encounter such TX packets, pull the content into an owned buffer.
Fixes: #7672
Signed-off-by: Wei Liu <liuwe@microsoft.com>
When running manual tests locally, it is sometimes necessary to
generate a cloud-init file at a custom path instead of defaulting
to /tmp. This is useful for developers and higher-level management
layers where files in /tmp may be cleaned up automatically.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Backing files (e.g. for QCOW2) interact badly with landlock since they
are not obvious from the initial VM configuration. Only enable their use
with an explicit option.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Test reading from overlay at offsets beyond backing file returns
zeros. Covers reads within backing range, beyond backing, and
boundary spanning.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
When an overlay QCOW2 image is larger than its backing file, reads
from offsets beyond the backing file virtual size would previously
fail with an I/O error.
The backing file virtual size is determined at open time and stored
for bounds checking during read operations:
- If the entire read is beyond the backing size, return all zeros
- If the read spans the boundary, read available data from backing and
fill the remainder with zeros
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Set HV_PARTITION_CREATION_FLAG_SMT_ENABLED_GUEST when the guest
topology has more than one thread per core. This allows the
hypervisor to schedule guest VPs correctly on SMT-enabled hosts.
Without this flag, the hypervisor schedules guest VPs incorrectly,
causing SMT unusable.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The kernel allows madvise on shared memory if
/sys/kernel/mm/transparent_hugepage/shmem_enabled is set.
Always try and configure THP via madvise when
the user requests THP be enabled.
If this fails, only a warning log is emitted and THP won't be enabled.
Signed-off-by: Champ-Goblem <cameron@northflank.com>
When restoring from snapshot with shared=false, write access to the
backing file is not required. Opening it read-only allows restore to
succeed on read-only media and overlay lower layers while preserving
MAP_PRIVATE semantics.
Signed-off-by: Rowen-Ye <rowenye1@gmail.com>
This patch adds the skeleton of the CVM test
support and modify existing scripts and test framework
to enable such scenario. Split the sha1sum to support both
regular and CVM guest. Add one test case for CVM. Will further
add more test cases.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
X64_64 image download steps is being used for both
regular and CVM guest. Keeping the steps withing a function
in the test-util.sh
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Now Guest struct has an option to set timeout.
No need to pass timeout while booting the guest.
If no timeout is set, default is used.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Modify Guest struct to keep some test specific
data so that test cases could be shared between
regular guest and CVM.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Testing generic vhost-user devices will require virtiofsd to support the
--tag option, which v1.8.0 does not support.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
Replace generic WritingHeader error with specific SyncingHeader
error for header fsync operations. This provides more precise
error reporting when syncing QCOW2 header changes to disk fails.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Wrap QcowFile in Arc<Mutex<>> to ensure thread safety when multiple
virtio queues access the same QCOW2 image concurrently.
Previously, each queue received its own QcowSync instance via
new_async_io() that shared the underlying QcowFile through Clone.
However, cloned QcowFile instances share internal mutable state
(L2 cache, reference counts, file seek position) without
synchronization, leading to data corruption under concurrent I/O.
This change serializes all QCOW2 operations through a mutex, which
ensures correctness at the cost of parallelism. A more performant
solution would require separating metadata locking from actual I/O
operations, tracked in #7560.
Related: #7560
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
TL;DR: Would reduce CI pressure by cancelling more "unnecessary" runs
but I can't verify without running a merge queue.
A common development pattern is to push a change and then immediately
check CI results. Follow-up fix pushes are quite common, which leads to
multiple CI runs being queued for the same pull request.
In Cloud Hypervisor, the size and cost of the CI matrix means that
several consecutive pushes (for example 3-4 in a short time) put
significant pressure on CI runners and noticeably increase feedback
latency.
In practice, concurrency handling is especially tricky for the merge
queue. From personal experience: If one does not take special care, CI
runs triggered by a `merge_group` can cancel each other, as in a merge
queue there are two runs for each job by default: one for the normal PR
and one for the merge commit. This is easy to run into, also because the
available documentation and best practices for this feature are not very
good.
At the same time, our workflows do not run on `push` events, but only
on `pull_request` and `merge_group`. Because of this, using
`${{ github.ref }}` alone as a concurrency key is not very meaningful,
and in practice only few runs are actually cancelled for successive PR
updates. Therefore, we should improve the usage of this feature.
This change tries to improve the situation by refining the concurrency
group key. The goal is to keep cancellation for multiple PR pushes,
while at the same time preventing unintended cancellations in the merge
queue by separating `merge_group` runs from regular PR runs.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
In kata-containers we use the api_client crate, but it's currently
failing our cargo deny check due to missing license, and there aren't
any license files within the crate, so I haven't found a good way to
work around this.
Alternatively I'd be happy to add the license to the workspace crate
and then reference it here, but that seems to clash with the direction
of the project in #7525.
Signed-off-by: stevenhorsman <steven@uk.ibm.com>
QCOW2 v3 autoclear_features field contains bits for features whose
metadata becomes invalid when the image is modified by software that
doesn't understand them. Defined bits:
- Bit 0: Bitmaps extension
- Bit 1: Raw external data
Cloud-hypervisor doesn't support bitmaps or external data files, so
all autoclear bits are cleared on writable open. This signals other
tools that these features' data may be stale.
Readonly opens preserve autoclear bits unchanged.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
This adds some missing features that are useful. In particular it adds
VIRTIO_F_RING_INDIRECT_DESC which gives a performance improvement.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Reported-by: Daniel Farina <daniel@ubicloud.com>
TDX builds its own ACPI tables in `create_acpi_tables_tdx` so it will
return None in the standard `create_acpi_tables` function and the
assertion for `rsdp_addr` will fail.
Signed-off-by: Zhibin Li <banlu.lzb@antgroup.com>
Add tests for corrupt bit behavior during I/O operations.
- Unaligned L2 table address triggers corrupt bit on read
- Unaligned cluster address triggers corrupt bit on read and write
- Normal operations do not set the corrupt bit
- V2 images work correctly without feature bits
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
MSHV doesn't present an ITS to guests. So, /proc/interrupts would never
have "ITS-PCI-MSIX".
Instead, a Gicv2m frame is presented to guests. So expect
"GICv2m-PCI-MSIX" in testcases.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
Add integration tests for QCOW2 corrupt bit handling. Verify that
images with the corrupt bit set are rejected for writable access but
allowed for read-only access with a warning.
Helper functions are added to read and modify the corrupt flag in the
QCOW2 v3 header.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Validate that L2 table offsets and refcount block offsets are cluster
aligned. Set the corrupt bit when unaligned offsets are detected, as
this indicates corrupted L1 or refcount table entries.
Validate that data cluster offsets from L2 entries are cluster aligned
during both reads and writes to existing clusters. Set the corrupt bit
when unaligned data cluster offsets are detected.
Prevent allocation of clusters at offset 0, which contains the QCOW2
header and should never be allocated. This catches corruption in the
available clusters list. Set the corrupt bit when this condition is
detected.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Set the QCOW2 corrupt bit when internal inconsistencies are detected
that indicate image metadata may be corrupted:
- Decompression decode failure, meaning compressed cluster data is
invalid
- Decompression size mismatch, where decompressed data doesn't match
expected cluster size
- Partial write after decompression, where L2 table was updated but
data cluster not fully written, leaving metadata inconsistent
- Invalid refcount index, where cluster address is outside valid
refcount table range, indicating a corrupted L2 entry
- Dirty L2 with zero L1 address, where L2 table is marked dirty but
L1 has no address for it
Note: Marking decompression failures as corrupt is more conservative
than QEMU, which returns EIO without setting the corrupt bit. This is
debatable since corrupted compressed data doesn't necessarily indicate
metadata corruption, but it provides a stronger safety guarantee by
preventing further writes to potentially damaged images.
Once set, the image can only be opened read-only until repaired with
qemu-img check -r.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add comprehensive tests for the corrupt bit handling. Cover writable
rejection, read-only access, persistence, and dirty bit
coexistence.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Implement proper handling of the QCOW2 corrupt bit (incompatible feature
bit 1) according to the specification:
- Add Error::CorruptImage for rejecting writable opens of corrupt images
- Add CORRUPT to SUPPORTED features (handled specially, not rejected)
- Add QcowHeader::set_corrupt_bit() to mark images as corrupt
- Add QcowHeader::is_corrupt() helper method
- Reject writable opens of corrupt images with Error::CorruptImage
- Allow readonly opens of corrupt images with a warning
The corrupt bit indicates that image metadata may be inconsistent. Per
spec, such images must not be written to until repaired by external
tools like qemu-img. Read-only access is permitted to allow data
recovery.
Users can open corrupt images read-only using:
--disk path=/path/to/image.qcow2,readonly=on
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
VsockPacket::hdr holds a raw pointer to the address of the VSock packet
header, which is in guest memory. It opens the door to double-fetch
(or TOCTOU) race conditions. Therefore, VSockPacket::hdr content can't
be trusted since it can be arbitrarily changed by the guest, at any
time.
To mitigate this, we can copy the header content to an array in VMM's
memory that the guest can't modify.
Signed-off-by: Thomas Leroy <thomas.leroy.mp@gmail.com>
Update QcowHeader and other related places to use BeUint methods
internally for reading/writing header fields.
This removes the byteorder dependency from mod.rs and consolidates
all big-endian file I/O through the shared BeUint trait.
Suggested-by: Rob Bradford <rbradford@rivosinc.com>
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add a read_be() method to the BeUint trait and make it pub(super)
so it can be used across the qcow module. Change BeUint::write_be()
to take Self instead of u64, providing type safety through TryFrom
conversion.
Suggested-by: Rob Bradford <rbradford@rivosinc.com>
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add tests to verify dirty bit is set while VM runs and cleared on
clean shutdown. As part of it, ensure graceful shutdown when OS
disk verification requires consistent image state.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Verify dirty bit is set on open and cleared on close for v3 images.
Ensure v2 and read-only files are not affected. Update existing
tests.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add support for the dirty bit (bit 0 of incompatible_features) which
indicates the image was not closed cleanly. This improves data
integrity by allowing detection of potentially corrupted images.
On open:
- If dirty bit is already set, log a warning and trigger
refcount rebuild
- Set the dirty bit and write it to disk immediately
- Sync to ensure persistence before any writes
- Skip dirty bit and refcount rebuild for readonly files
On clean close:
- Clear the dirty bit in the header
- Write it to disk and sync
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Test all refcount_order values (0-6):
- Basic open for each width
- Write/read roundtrip
- Overwrite and multi-cluster allocation
- L2 cache eviction under memory pressure
- Sub-byte and byte-aligned max value handling
- Overflow error detection
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Reject refcount values exceeding the maximum for the image's
refcount_order. This prevents silent truncation when storing
refcounts in narrow widths (e.g., 1-bit max is 1, 4-bit max is 15,
etc.).
Returns RefcountOverflow error with the attempted value, maximum,
and bit width. Propagates as EINVAL to the guest.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
QCOW2 v3 specifies refcount_order 0-6 with
refcount_bits = 1 << refcount_order. Previously only 16-bit (order 4)
was supported.
Changes:
- RefcountBytes trait handles byte-aligned types (8/16/32/64-bit)
- Generic pack/unpack for sub-byte widths (1/2/4-bit)
- Function pointers for read/write selected at open time
- Internal refcount type widened from u16 to u64
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Parse the feature name table header extension to provide descriptive
error messages when unsupported incompatible features are detected.
Currently only the compression bit (bit 3, zstd) is supported.
This prevents opening qcow2 images with features that would cause
incorrect behavior or data corruption (e.g., dirty bit, corrupt bit,
external data file, extended L2 entries).
Feature names are defined as follows:
1. The image's feature name table header extension (if present)
2. Hardcoded fallback names for known features
3. Generic "unknown feature bit N" for undefined features
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Co-developed-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
Implement read support for bit 0 in QCOW2 L2 table entries.
When this flag is set, the cluster reads as zeros without accessing
disk. This improves compatibility with QCOW2 images that use this
optimization.
According to the QCOW2 specification, bit 0 of the standard cluster
descriptor indicates that the cluster reads as zeros. Unlike
l2_entry == 0 indicating a completely unallocated entry, bit 0 can
be set on an allocated cluster.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
build_edk2 was leaving behind .built markers
even when compilation failed.
Gate creation of.built marker to occur only on
successful build
Modify build_edk2() to exit with error code
when arm64 firmware artifact : CLOUDHV_EFI.fd
is not produced
Fixes#7608
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
build_edk2() module in scripts/common-aarch64.sh
does not produce the UEFI firmware for aarch64 as
the commits used to assemble sources for acpica,
edk2-platforms and edk2 do not compile after GCC
version upgraded from 11.4.0 to 13.3.0 in the
developer container (ubuntu 22.04 to 24.04)
Apply minimum upgrade to EDK2_REPO and ACPICA_REPO
required to compile with GCC 13.3.0
while still assuring guest VM boot for all
integration tests
BaseTools: Brotli compression submodule that was
previously failing has been fixed following commit
bump
Developers can now produce UEFI firmware for
aarch64 using the following commands
```
./scripts/dev_cli.sh shell
source scripts/test-util.sh
source scripts/common-aarch64.sh
build_edk2
```
Update docs/uefi.md
Fixes#7608
Signed-off-by: Saravanan D <saravanand@crusoe.ai>
Recent changes related to arm64 support in MSHV exposed
inconsistencies in the VM initialization and CVM boot paths.
The VM creation flow currently diverges across multiple scenarios,
including regular MSHV, CVM, and arm64, with each path performing
guest initialization steps in a different order.
Certain platform-specific requirements further constrain the ordering
of operations, such as the timing of address space creation,
IGVM loading, interrupt controller setup, and payload loading. For
CVM case address-space creation must be done after IGVM loading, and
PSP measurement. For Regular and arm64 this memory initialization
must be done early. For MSHV, vm.init() and sev_snp.init() are called in
different order which is run time and build time conditionally checked.
Additionally, while the KVM initialization path differs slightly
from MSHV, it shares common logic that is currently split across
separate conditional and build-time code paths, contributing to
fragmentation of the overall flow.
This change restructures the VM creation and initialization sequence
to better align shared logic, enforce scenario-specific ordering
constraints, and ensure consistent and correct behavior across all
supported configurations. In doing so, it restores proper CVM boot
behavior and improves the maintainability of the initialization code.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
The spec says simply that that an empty payload should be returned on
error. Be slightly more helpful by adding a warning.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Based upon the discussion and in
https://github.com/rust-vmm/vhost/issues/29#issue-830820820 and the QEMU
behaviour the get_config offset should be zero. This was not caught by
our integration tests as the vhost-user-blk backend as implemented in
this repository does not use the offset.
Fixes: #7615
Signed-off-by: Rob Bradford <rbradford@meta.com>
Since the mshv integration workflow has been stable for a long time,
make the workflows no longer optional.
Signed-off-by: Aastha Rawat <aastharawat@microsoft.com>
Add warmup_iterations field to run iterations before measuring
performance. This complements existing cold start tests
by separating cache effects from steady state throughput.
New tests with 2 warmup iterations:
- block_qcow2_backing_qcow2_read_warm_MiBps
- block_qcow2_backing_raw_read_warm_MiBps
Results show warm cache is much faster and more consistent:
- QCOW2: 1766 MiB/s (4% variance) vs cold 960 MiB/s (73% variance)
- RAW: 1822 MiB/s (6% variance) vs cold 1300 MiB/s (55% variance)
RAW backing is 3% faster than QCOW2 in steady state.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The write_to() function is used by test code to create qcow2 files for
testing. For v3 headers with extended header_size (>104), it needs to:
1. Write the mandatory compression_type field at bytes 104-111
2. Write the header extension end marker at the header_size offset
3. Seek to backing_file_offset before writing the backing file path
Additionally, create_for_size_and_path() must set backing_file_offset
to account for the 8 byte extension end marker in v3 files, so the
backing file path doesn't overwrite the extension area.
Add unit tests for read_header_extensions() covering backing format
parsing (raw/qcow2), unknown extensions, and error cases (invalid
formats, invalid UTF-8). These tests depend on the header writing fixes
to create properly formatted v3 test files.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add support for parsing QCOW v3 header extensions to read the
backing file format. The QCOW v3 spec allows optional header
extensions between the fixed header and the backing file name.
Implement read_header_extensions() to parse the extension area,
which starts at the header_size offset. At the moment it is
used to read the backing file format. Further extension
processing is open in folow up implementations.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add support for raw backing files in addition to qcow2 backing
files. This enables QCOW2 overlays to use raw images as their
backing store.
The backing file format is auto-detected when not specified,
using the existing detect_image_type() function.
Add backing_file_format field to QcowHeader to store the format
type, which will be populated from header extensions by a
subsequent patch.
Modify new_from_backing() to accept a backing_format parameter,
consolidating support for both raw and qcow2 backing files in a
single function. The backing_file_size parameter allows overlay
creation without opening the backing file multiple times.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
In most of the cases, special registers don't change after emulations,
but current code sets them back unconditionally, and although some of
them are set over the register page, others require a system call and a
hypervisor to be updated, which is a waste it there were not changes.
Introduce and CPU update method for Microsoft Hypervisor emulator and
set special registers only when they were changed. This change reduces
guest boot time by 4% for a single VP guest boot (in L1VH partition) in
my experiments.
Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
This is a precursor change to overall ioctl and hypercall reduction
effort. The old (current) CPU state can be compared to the new to
determine what has changed and avoid unnecessary register updates.
Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
Minor modifications were made to make the sentences sound more natural.
Also fixed some parameter usage issues in bash code block.
Signed-off-by: Yi Wang <foxywang@tencent.com>
There are some syntax and format issues in tdx/sev documents.
Make some modification to make the description more natural.
And the link of SEV-SNP is invalid, fix it.
Signed-off-by: Yi Wang <foxywang@tencent.com>
There are some minor syntax and command issues in debug-port document.
As commit 5febdec81a (vmm: Enable `gdbstub` on AArch64) supported
aarch64, the docs should keep consistent.
Signed-off-by: Yi Wang <foxywang@tencent.com>
Fix some minor syntax issues in api/building document to make
the sentences more fluent and easy to read.
Signed-off-by: Yi Wang <foxywang@tencent.com>
Some description in the device document were inconsistent with the
source code. Also fix some syntax issues to make the sentences more
fluent.
Signed-off-by: Yi Wang <foxywang@tencent.com>
CI reports:
In scripts/test-util.sh line 216:
cleanup() {
^-- SC2329 (info): This function is never invoked. Check usage (or ignored if invoked indirectly).
The shellcheck can't trace calling in trap, so we need add hint
to make it happy.
Signed-off-by: Yi Wang <foxywang@tencent.com>
The IORT table's ID mapping uses a 256-ID offset per PCI segment to
ensure unique device IDs across all segments. This partitioning scheme
(output_base = 256 * segment_id) must match the device ID encoding used
in KVM MSI routing configuration [1].
This mapping assumes one bus per PCI segment, and supports up to 256 PCI
segments in the system.
[1] c9374d87ac
Signed-off-by: Bo Chen <bchen@crusoe.ai>
The IORT specification (Revision E.b, Table 12) defines the ITS Group
Node structure with an ITS Identifiers array following the node header.
Although the IORT table is zero-initialized, this commit adds an
explicit write of the ITS identifier value (0) for clarity and spec
compliance.
This ITS identifier must match the `translation_id` field in the MADT
GIC ITS structure to ensure proper interrupt routing on ARM platforms.
Signed-off-by: Bo Chen <bchen@crusoe.ai>
The current IORT table implementation is based on IORT Spec revision E.b
[1], as evidenced by:
* The PCI root complex node revision being set to `3`
* The code being updated in late 2021 [2] when revision E.b was the
latest version
This patch ensures the IORT table is properly generated according to
this specification revision, fixing three issues:
1. The IORT table revision should be `3` rather than `2` (see Table 2 in
the spec [1])
2. The GIC ITS group node revision should be `1` rather than `0`
(see Table 12 in the spec [1])
3. The "Memory access properties" and "ATS Attribute" fields of the PCI
root complex node was set incorrectly - specifically the MAF (Memory
Access Flags) including CPM and DACS bits (see Tables 14, 15, and 17
in the spec [1])
[1] https://developer.arm.com/documentation/den0049/eb/?lang=en
[2] https://github.com/cloud-hypervisor/cloud-hypervisor/pull/3356
Signed-off-by: Bo Chen <bchen@crusoe.ai>
It should always succeed and is apparently implicitly called by libc or
some dependency somewhere.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
x86::__cpuid is safe on Rust ≥1.94 but unsafe on older versions. This
causes unused_unsafe warnings when compiling with Rust ≥1.94. However,
on earlier Rust versions, the code won’t compile if the unsafe blocks
are absent.
Work around this by adding #[allow(unused_unsafe)] where needed to
suppress the warnings.
See #7588 for more discussion.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
On MSHV, exposing multithreaded CPU topologies requires setting the
PROCESSORS_PER_SOCKET partition property so that CPUID.0xB reports
correct logical processor counts and topology levels to the guest.
This property must be set after all vCPUs are configured, as the
hypervisor uses the complete vCPU layout to derive and report CPU
topology information.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
This change is a prerequisite for live disk resizing. Before this
commit, the epoll-handler threads just got a copy of the sector
size which we cannot update during runtime.
On-behalf-of: SAP thomas.prescher@sap.com
Signed-off-by: Thomas Prescher <thomas.prescher@cyberus-technology.de>
Add sequential and random read performance tests for QCOW2 overlays
with QCOW2 backing files.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Introduce a new BlockControl struct to encapsulate fio operation
parameters. This replaces the tuple-based fio_control with a more
extensible structure that includes:
- fio_ops: The FIO operation type
- bandwidth: Whether to measure bandwidth or IOPS
- test_file: The file path to test against
This refactoring enables reusing performance_block_io with different
test files.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Virtio PCI devices are created in a set of nested functions. In each
of this functions a vector is created to add created devices to, only
to be appended to the vector of the higher nesting level. Those nested
vectors are unnecessary as we can directly write to the member of
`DeviceManager`.
Signed-off-by: Pascal Scholz <pascal.scholz@cyberus-technology.de>
On-behalf-of: SAP pascal.scholz@sap.com
The TILE data state of AMX may require 8KB+ space, calling the legacy
KVM_GET_XSAVE will encounter an error since KVM_GET_XSAVE only can get
4KB space. This patch adds KVM_GET_XSAVE2 support to allow snapping more
data.
Fixes: #7533
Signed-off-by: Songqian Li <sionli@tencent.com>
Refactor write_pointer_table to accept iterators instead of requiring
materialized vectors, eliminating temporary allocations in L1 table
sync operations.
Changes:
- Modified write_pointer_table() to take Iterator<Item = &T> and
dereference internally before passing owned values to the callback
- Added write_pointer_table_direct() convenience wrapper for cases
without value transformation
- Updated sync_caches() to use l1_table.iter() directly instead of
.get_values().iter().copied()
- Implemented Deref<Target = [T]> for VecCache to enable direct .iter()
Performance impact:
- Eliminates L1 table allocation during sync (~2KB per 100GB disk)
- L2 and refcount table writes already used slices, no change there
- Zero performance overhead: iterator dereferencing is equivalent to
.copied() and optimizes identically
The L1 sync previously collected entries into a Vec to apply the
OFLAG_COPIED flag. The new iterator+callback pattern computes this
on-the-fly, avoiding the allocation entirely.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add Deref<Target = [T]> implementation for VecCache<T> to allow direct
slice operations without explicitly calling get_values(). This enables
cleaner code patterns like cache.iter() instead
of cache.get_values().iter().
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Add fcntl to virtio_block_thread_rules to allow try_clone() on file
descriptors. The try_clone() method uses fcntl(fd, F_DUPFD_CLOEXEC)
to duplicate file descriptors, which is needed for efficient QCOW2
L1 table sync that avoids temporary allocations.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Creating a vdpa_net with an associated MAC address and setting the right
amount of queues in order to fix the integration test related to
vdpa_net.
Fixes: #5756
Signed-off-by: Sebastien Boeuf <seb@rivosinc.com>
In [0] we refactored some Arc<Mutex<T>> parameters to &Mutex<T>> to
satisfy clippy's needless_pass_by_value lint. Nevertheless, this is also
not so idiomatic, so as a follow-up, we put the responsibility to lock
objects to the caller side (only where this is not strictly needed by
the callee).
While on it, I also tried to pass vm_config directly into
pre_create_console_devices() which would clean up some code, but then
we have interleaving mutable and immutable borrows of the Vmm, which
are denied by the borrow checker.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Our CI test execution system finds some mergeable pages in the system
directly on boot since a recent update. Remove the broken assumption in
the test_memory_mergeable integration test and only check if the guest
VMs influence the shared pages count.
Signed-off-by: Stefan Nürnberger <stefan.nuernberger@cyberus-technology.de>
https://github.com/cloud-hypervisor/cloud-hypervisor/pull/7294 adjusted
the checks for read-only requests made to virtio-blk devices and started
rejecting VIRTIO_BLK_T_GET_ID requests. These requests do not perform
any writes and are needed in order to access device serials from within
the guest.
Signed-off-by: Connor Brewster <cbrewster@hey.com>
`cargo rustc` is incompatible with virtual manifests, so the CI needs to
use cargo build instead. However, passing `RUSTFLAGS="-D warnings"` via
the environment would propagate to all dependencies, and some of them
currently fail to build under ``-D warnings` due to issues like [0]:
```
error: creating a mutable reference to mutable static
--> src/temp.rs:97:5
|
97 | DIRS.pop()
| ^^^^^^^^^^ mutable reference to mutable static
```
To resolve this, apply ``-D warnings` only to the `cargo clippy`
commands (which apply to our workspace only) and avoid enforcing it for
the entire cargo build.
[0]: https://github.com/cloud-hypervisor/cloud-hypervisor/actions/runs/19962283528/job/57245376263?pr=7525
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
TL;DR: cargo clippy|check|... now runs on whole workspace by default.
## Steps
- add new workspace member `cloud-hypervisor`
- move `./src` to new workspace member
- move `./tests` to new workspace member
- move relevant parts from Cargo.toml to new workspace member
- kept necessary parts in main Cargo.toml, such as profile
configurations
## About
The main Cargo.toml historically mixes workspace and crate definitions
for cloud-hypervisor and ch-remote. This makes it hard to read and
requires `--workspace` to run cargo clippy or cargo test on all
workspace members, which is counter-intuitive.
This patch separates the workspace from the crate definition in the main
Cargo.toml file. After this, cargo clippy, cargo test, etc., work on the
whole workspace naturally, giving a smoother developer experience. The
Cargo.toml without a package definition is also called a virtual
workspace or virtual manifest by Cargo [0].
Backporting is not a concern: CHV no longer backports, but the affected
files are rarely modified anyway.
[0] https://doc.rust-lang.org/cargo/reference/workspaces.html#virtual-workspace
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This will also prevent some useless rebuilds. Using `--verbose` we can
observe that the build.rs causes frequent useless rebuilds - having
less is a good thing. They come from the dependency of `build.rs` to
the local git repository.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
The BufWriter must be flushed explicitly to handle errors
properly. Without explicit flush, errors during the implicit
drop flush are ignored.
This is the same issue fixed for write_pointer_table
in commit 85556951a.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The MSR enumeration logic in MSHV does not accurately
account for the capabilities of the underlying
hardware and hypervisor. Previously, integration tests
passed because only the default MSRs—universally supported
across platforms—were involved. However, recent feature
additions in MSHV have introduced MSRs that are not
supported on Intel hardware, causing failures.
This patch retrieves the MSRs from the VM specific
API call that filters out the unsupported MSRs.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Add set_cluster_refcount_track_freed() helper to consolidate the
common pattern of setting a cluster refcount and tracking freed
refblocks. This reduces code duplication and improves readability.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
The image passed for the guest construction is copied. Previously,
check-img has been checking the unchanged image from the workspace dir,
which is supposed to be error free.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Freed clusters correctly have refcount=0. Remove the assertion that
expected no clusters with zero refcount, as it was validating the
buggy behavior.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
When converting a compressed cluster to standard during write
operations, the old compressed cluster's refcount was never
decremented, causing leak warnings by `qemu-img check ..`
`Leaked cluster X refcount=N reference=M`
Additionally, compressed data can span multiple physical clusters,
not just one. The compressed cluster address and size are encoded
in the L2 entry, and the data may cross cluster boundaries.
The proper handling is implemented as follows:
- Extract compressed cluster address and size before overwriting
L2 entry
- Identify all clusters occupied by the compressed data
- Decrement refcount for each cluster in the range
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
When a refcount block is evicted from cache and replaced with a new
one, the old refcount block cluster was added to unref_clusters but
its refcount was never decremented to 0 on disk. This left the cluster
with refcount=1 while no metadata referenced it, causing errors in
qemu-img check
`Leaked cluster X refcount=1 reference=0`
This fix recursively calls set_cluster_refcount(freed_cluster, 0) to
properly decrement the freed refcount block's refcount on disk. The
recursion handles cascading replacements where freeing one refcount
block may trigger the replacement of another.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
"Signed-off-by" is the only variant that is accepted. So we
should remove the inconsistency to prevent:
- user forgets this at all
- CI complains
- user adds "Signed-Off-By"
- CI still complains because of the wrong format
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
We enable AMX tile state components via the hypervisor (as introduced
in the previous commit) instead of doing this inline in the body of
`CpuManager::new`.
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de>
On-behalf-of: SAP oliver.anderson@sap.com
Currently when the user configures AMX the corresponding state
components get dynamically enabled directly inside the function body of
vmm::cpu::CpuManager::new.
With our ongoing work on CPU templates/profiles, there will (likely) be
one more binary crate for producing CPU profiles that also needs to do
this (without creating a CpuManager) and it may also be the case that
we will need to call this function prior to `CpuManager::new` during
live migrations.
We thus add a method for enabling the AMX tile state components on the
hypervisor trait that may be called wherever necessary. We argue that
this is beneficial for code clarity independently of the upcoming CPU
templates/profiles PR that we are working on.
The astute reader will notice that the logic introduced here is not 100%
the same as what is done inside the vmm::cpu::Cpumanager::new method. We
claim that our approach is more in-line with the official documentation.
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de>
On-behalf-of: SAP oliver.anderson@sap.com
This patch updates the documentation to reflect the newly added
nested CPU feature option in the CLI.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Add an option in the CLI to enable nested support.
Adding an option --cpu nested=on|off to the CPU
argument to enable nested support for Microsoft
Hypervisor. By default it is enabled for both KVM
and MSHV.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Create HypervisorVmConfig early and pass the
struct to VM creation API in the vmm crate. Getting
rid of multiple conditional parameter.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
The OFLAG_COPIED bit (bit 63) indicates a cluster's refcount is exactly
1 and doesn't need copy-on-write. This bit must be set in L1 entries
when their referenced L2 clusters have refcount=1.
Previously, L1 entries were always written as raw addresses without the
OFLAG_COPIED bit, violating the QCOW2 specification and causing qemu-img
check to report errors like
`ERROR OFLAG_COPIED L2 cluster: l1_index=X .... refcount=1`
The implementation queries each L2 cluster's refcount in sync_caches()
and sets OFLAG_COPIED appropriately when writing the L1 table. This
ensures QCOW2 images are specification compliant and maintain correct
COW semantics to avoid data corruption.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Previously the code relies on the implicit flush when BufWriter is
dropped. That's not safe.
Per BufWriter's document:
```
It is critical to call flush before BufWriter<W> is dropped. Though
dropping will attempt to flush the contents of the buffer, any errors
that happen in the process of dropping will be ignored. Calling flush
ensures that the buffer is empty and thus dropping will not even attempt
file operations.
```
Signed-off-by: Wei Liu <liuwe@microsoft.com>
The MSRs are constant at boot time so rather than creating a vector in
the boot_msr_entries() method instead reaturn a reference to static MSR
array data.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Previously, state that we accumulated during the migration process in
the receiver was kept in `mut Option` variables or HashMaps. The
problem is that it is unclear in the code when these variables can be
safely used. It's also difficult to add new state, such as the state
we need to handle the upcoming feature for performing the migration
using multiple connections.
To solve this, I've refactored the code to use the state machine
pattern. Each state carries the data it needs to. Any state that
didn't arrive yet (memory_files, memory_manager) cannot be accessed
until we are in the proper state.
Some benefits that fall out of this:
- We now respond to all requests, even invalid ones, at least with an
error message.
- Any error handling a request will result in an error message being
sent.
- There is only a single place where responses are sent and thus it's
very hard to mess this up in the code.
- The main protocol state machine fits on a screen.
I would argue that especially the error cases are now much more
consistent. There is still a lot to be done. There is still state
transfer via self.vm and similar. In an ideal world, this would also
be carried by the state machine. I also want to see better handling of
payloads, which are still handled all over the place, but this change
is already too big. :)
Co-authored-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP julian.stecklina@sap.com
On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Julian Stecklina <julian.stecklina@cyberus-technology.de>
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
... and nuke some Option<> while I was there. Given that HashMap has a
usable default and we end up passing an empty HashMap anyway, just get
rid of the Option.
On-behalf-of: SAP julian.stecklina@sap.com
Signed-off-by: Julian Stecklina <julian.stecklina@cyberus-technology.de>
During the lifecycle of a confidential VM, the guest may revoke
previously shared pages via an attribute-intercept VM exit.
When this happens, the host-side cache must be updated so that any
subsequent access by the VMM triggers a fresh request for the guest
to re-share the page.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
This is a follow-up of [0].
# Advantages
- This saves dozens of unneeded clone()s across the whole code base
- Makes it much easier to reason about how parameters are used
(often we passed owned Arc/Rc versions without actually needing
ownership)
# Exceptions
For certain code paths, the alternatives would require awkward or overly
complex code, and in some cases the functions are the logical owners of
the values they take. In those cases, I've added
#[allow(clippy::needless_pass_by_value)].
This does not mean that one should not improve this in the future.
[0] 6a86c157af
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Fix the condition to correctly identify a new PTY connection.
Fixes: 287887c99 (vmm: fix console IO safety)
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
Previously, `into_raw_fd()` transferred fd ownership to epoll while
keeping separate clones in `reader` and `writer`, causing leaks when
the stream was closed. Now, `reader` owns the fd and epoll borrows it.
When the FD is closed in the kernel, reader will be reset to None.
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
The Intel Granite Rapids processors include more AMX related features
that are advertised in leaf 0x7 subleaf 0x1. If the VM is not
configured to support AMX (the default) then these feature bits need
to be masked out.
Furthermore Tile information and TMUL information
in leaves 0x1d and 0x1e respectively are also purely related to AMX
and should also be zeroed whenever AMX support is disabled.
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de>
On-behalf-of: SAP <oliver.anderson@sap.com>
When enabling amx feature, we should call arch_prctl to request
permission to use tile data for guest. The permission should be
requested before the first vcpu is created, so we need to call
arch_prctl in vmm thread. This patch adds the arch_prctl syscall for
vmm_thread_rules.
Fixes: #7516
Signed-off-by: Songqian Li <sionli@tencent.com>
This helps to uncover expensive and needless clones in the code base.
For example, I prevented extensive clones in the snapshot path where
(nested) BTreeMap's have been cloned over and over again. Further,
the lint helps devs to much better reason about the ownership of
parameters.
All of these changes have been done manually with the necessary
caution. A few structs that are cheap to clone are now `copy` so that
this lint won't trigger for them.
I didn't enable the lint so far as it is a massive rabbit hole and
needs much more fixes. Nevertheless, it is very useful.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
We've encountered issues that Cloud Hypervisor corrupts disk images
after use. Those issues may not be immediately obvious until the
corrupted images are used again.
Run consistency checks over the disk images in the test cases to catch
issues as early as possible.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
It is a common use case to run a subset of tests locally to verify
certain functionalities.
The default behaviour for nextest is to error out if no tests are run.
That causes the test scripts to return a non-zero value (failure). Pass
`--no-tests=pass` to nextest to match what `cargo test` does if no tests
are run.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
This is continuing the work from [0]. Some places in CHV
"misuse" Result::map_err for side-effects. inspect_err is
a better suited alternative for exactly that use-case.
Unfortunately, there is no clippy lint for this yet.
[0] f02745a7ed
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This commit is the first in a series of similar commits to clean up
obsolete `extern crate` statements
Since Rust 1.30, normal macros can be imported via `use`, and with Rust
1.31 and edition 2018 this has become the preferred approach.
`extern crate` is only needed for `alloc` in `no_std` crates, which does
not apply here.
By dropping these (often redundant or odd) `extern crate` lines, we
expose the actual dependencies more clearly and reduce technical debt.
## Auto-generation of the series
Most of this series was produced automatically:
1. Removed all "extern crate" references
2. Run the script [0] to add missing `use` statements
3. Run `cargo +nightly fmt --all`
4. Fix the remaining problems manually
The treewide changes were then split into per-folder commits.
[0]
```python
import os
import re
# Mapping of macro/function usage to imports
MACRO_IMPORTS = {
"info!": "use log::info;\n",
"debug!": "use log::debug;\n",
"error!": "use log::error;\n",
"trace!": "use log::trace;\n",
"warn!": "use log::warn;\n",
"event!": "use event_monitor::event;\n",
"anyhow!(": "use anyhow::anyhow;\n",
"bitflags!(": "use bitflags::bitflags;\n",
"ioctl_ior_nr!": "use vmm_sys_util::{ioctl_ior_nr};\n",
"ioctl_iow_nr!": "use vmm_sys_util::{ioctl_iow_nr};\n",
}
# Regex for finding the first use statement
USE_REGEX = re.compile(r"^\s*(use|pub use) .+?;")
def process_file(path):
with open(path, "r", encoding="utf-8") as f:
lines = f.readlines()
content = "".join(lines)
existing_imports = set(lines)
needed_imports = set()
# Check macros/functions against mapping, only add if not already present
for key, import_stmt in MACRO_IMPORTS.items():
if key in content and import_stmt not in existing_imports:
needed_imports.add(import_stmt)
if not needed_imports:
print(f"Unmodified {path} (no new imports needed)")
return # Nothing to do
# Find first use or pub use statement
for i, line in enumerate(lines):
if USE_REGEX.match(line):
insertion_index = i + 1
break
else:
print(f"Unmodified {path} (no use or pub use statement found)")
return # No use statement found, skip file
# Insert imports
lines[insertion_index:insertion_index] = list(needed_imports)
# Write back file
with open(path, "w", encoding="utf-8") as f:
f.writelines(lines)
print(f"Modified {path}, added imports: {''.join(needed_imports).strip()}")
for root, _, files in os.walk("."):
for file in files:
if file.endswith(".rs"):
process_file(os.path.join(root, file))
```
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Add tests:
- zlib: test_virtio_block_qcow2_zlib()
- zstd: test_virtio_block_qcow2_zstd()
Both these tests use zlib- and zstd-compressed images as OS image.
Modify test_virtio_block_qcow2_backing_file() test: it is practical
to test qcow2 file-backing with compression, so use zlib-compressed
image as a backing file.
Signed-off-by: Eugene Korenevsky <ekorenevsky@aliyun.com>
Add support of reading and writing compressed clusters.
Support zlib and zstd compressions.
L2 cache: store entire L2 entries, not only standard cluster addresses.
Read path. Offsets of compressed clusters cannot be determined,
therefore replace QcowFile.file_offset_read() with QcowFile.file_read().
This method reads the cluster, decompresses it if necessary and returns
the data to the caller.
Write path. QcowFile.file_offset_write(): since writing to compressed
clusters is not generally possible, allocate a new standard
(non-compressed) cluster if compressed L2 entry is encountered; then
decompress compressed cluster into new cluster; then return offset
inside new cluster to the caller. Processing of standard clusters is
not changed.
Signed-off-by: Eugene Korenevsky <ekorenevsky@aliyun.com>
As well as saving the MSRs as it is currently does ensure that the KVM
capability is enabled along with keeping the internal state updated.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Co-authored-by: Chengyu Fu <chengyu.fu@linux.alibaba.com>
The granularity has significant implications in typical cloud
deployments with network storage. The Linux kernel will sync advisory
locks to network file systems, but these backends may have different
policies and handle locks differently. For example, Netapp speaks a NFS
API but will treat advisory OFD locks for the whole file as mandatory
locks, whereas byte-range locks for the whole file will remain
advisory [0].
As it is a valid use case to prevent multiple CHV instances from
accessing the same disk but disk management software (e.g., Cinder in
OpenStack) should be able to snapshot disks while VMs are running, we
need special control over the lock granularity. Therefore, it is a valid
use case to lock the whole byte range of a disk image without
technically locking the whole file - to get the best of both worlds.
This also brings CHVs behavior in line with QEMU [1].
Whole-file locks remain a valid use case and could be supported later.
This patch only provides the necessary groundwork; making it
configurable is out of scope for now.
[0] https://kb.netapp.com/on-prem/ontap/da/NAS/NAS-KBs/How_is_Mandatory_Locking_supported_for_NFSv4_on_ONTAP_9
[1] <qemu>/util/osdep.c::qemu_lock_fcntl()
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This should be guaranteed by GuestMemory and GuestMemoryRegion, but
those traits are currently safe, so add checks to guard against
incorrect implementations of them.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
It takes a pointer to a userspace address that it accesses, so it should
be marked unsafe. This was missed earlier.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
Callers of get_host_address_range() rely on it returning a pointer to at
least size bytes of memory. mem.get_host_address() is an overrideable
method of a safe trait, so it is better for safe code to not rely on its
correctness for safety. Instead, use mem.get_slice(), which returns a
VolatileSlice whose invariants guarantee that it points to a sufficient
amount of memory. If mem.check_range() succeeds but mem.get_slice()
returns a slice that is too small, this means that there is either a
logic error or a situation the code cannot support yet, so panic.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
The kernel will validate that the size is page-aligned. The file offset
is always zero, so the kernel will also validate that the offset is
page-aligned.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
This API passes a u64 to a kernel API that treats the u64 as a userspace
address. Therefore, it should be marked unsafe, but it currently is not
[1]. Wrap the call in an unsafe block to document that invariants must
be upheld to avoid undefined behavior. This causes a compiler warning,
so suppress the warning with #[allow(unused_unsafe)].
[1]: https://github.com/rust-vmm/vfio/issues/100
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
I believe that its only caller used it safely, but it is still better to
mark the code as unsafe. Also add additional validity checks.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
Also drop support for building the TDX code for 32-bit targets. All
CPUs with TDX support are 64-bit so supporting 32-bit targets is not
needed.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
To ensure that struct sizes are the same on 32-bit and 64-bit, various
kernel APIs use __u64 (Rust u64) to represent userspace pointers.
Userspace is expected to cast pointers to __u64 before passing them to
the kernel, and cast kernel-provided __u64 to a pointer before using
them. However, various safe APIs in Cloud Hypervisor took
caller-provided u64 values and passed them to syscalls that interpret
them as userspace addresses. Therefore, passing bad u64 values would
cause memory disclosure or corruption.
Fix the bug by using usize and pointer types as appropriate. To make
soundness of the code easier to reason about, the PCI code gains a new
MmapRegion abstraction that ensures the validity of pointers. The rest
of the code already has an MmapRegion abstraction it can use. To avoid
having to reason about whether something is keeping the MmapRegion
alive, reference counting is added. MmapRegion cannot hold references
to other objects, so the reference counting cannot introduce cycles.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
To ensure that struct sizes are the same on 32-bit and 64-bit, various
kernel APIs use __u64 (Rust u64) to represent userspace pointers.
Userspace is expected to cast pointers to __u64 before passing them to
the kernel, and cast kernel-provided __u64 to a pointer before using
them. However, various safe APIs in Cloud Hypervisor took
caller-provided u64 values and passed them to syscalls that treat them
as userspace addresses. Therefore, passing bad u64 values would cause
memory disclosure or corruption. The memory region APIs are one example
of this, so mark them as unsafe.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
When booting a Linux guest in SMP configuration, on sapphire rapids
and granite rapids the following kernel warning can be observed:
[Firmware Bug]: CPUID leaf 0x1f subleaf 1 APIC ID mismatch 1 != 0
[Firmware Bug]: CPUID leaf 0x1f subleaf 2 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 for each subleaf.
Signed-off-by: Thomas Prescher <thomas.prescher@cyberus-technology.de>
On-behalf-of: SAP thomas.prescher@sap.com
as of rust 1.90, writes to unix sockets use the sendto syscall. This
affects the vcpu threads when vfio_user devices are accessed.
Signed-off-by: Stefan Nürnberger <stefan.nuernberger@cyberus-technology.de>
This is the first commit in a series of commits to improve the Code
Quality in Cloud Hypervisor in a sustainable way. These are the
default rules from `clippy::all` but written here to be more explicit.
`clippy::all` refers to all "default sensible" lints, not all
existing lints.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This better aligns with the rest of the code and makes it clearer
that these tests can run "as is" in a normal hosted environments
without the special test environment.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
TL;DR: Massive quality of life improvement for devs
Cloud Hypervisor uses the Cargo test framework for multiple tests:
- normal unit tests
- unit tests requiring special environment (the Tap device tests)
- integration tests requiring a special environment
This prevented the execution of `cargo test --workspace`, which results
in a very poor developer experience. Although
`./scripts/run_unit_tests.sh` exists, there are valid reasons why devs
cannot or even don't want to use it.
By adding a new `chv_testenv` rustc config, we can conditionally only
activate tests when the `./scripts/` magic runs them. This improves
the general developer experience by a lot.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Commit 5ec47d4883 was intended to patch ebx bits 23-16 in cpuid leaf
0x1, but it was not working as expected, as in rust, operator << has a
stronger precedence than & [1]. Later commit b6667f948e fixed the
operator precedence clippy warning, but did not fix the actual issue. As
a result, the current code is not changing ebx,
```
cpu_ebx |= ((dies_per_package as u32) * (cores_per_die as u32) * (threads_per_core as u32))
& (0xff << 16);
```
Since the total number of logical processors is generally less than
65536, the right hand side of the expression is 0 in most cases.
[1] https://doc.rust-lang.org/reference/expressions.html#expression-precedence
Fixes: 5ec47d4883 ("arch: x86_64: enable HTT flag")
Signed-off-by: Changyuan Lyu <changyuanl@google.com>
A major improvement to the developer experience of clippy in
Cloud Hypervisor.
1. Make `cargo clippy` just work with the same lints we use in CI
2. Simplify adding new lints
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
... by just passing the iterator along. For large VMs this bitmap is
gigantic. A 12TB VM has 384MB of dirty bitmap.
With all these optimizations from the previous commits in place, we
see quite the improvement when it comes to scanning the dirty bitmap.
For a bitmap with 1% bits (randomly) set, dirty_log() takes:
Original code: 2166ms (100.0%)
New code: 382ms ( 17.6%)
on my system. The sparser the dirty bitmap the faster. Scanning an
empty bitmap is 100x faster. For a 5% populated bitmap we are still 3x
faster.
If someone wants to play with this, there is a benchmark harness here:
https://github.com/blitz/chv-bitmap-bench
On-behalf-of: SAP julian.stecklina@sap.com
Signed-off-by: Julian Stecklina <julian.stecklina@cyberus-technology.de>
This would be a good opportunity to optimize another pointless vector
away, but I don't have a good way to test this at the moment. But
maybe someone else gives it a shot.
On-behalf-of: SAP julian.stecklina@sap.com
Signed-off-by: Julian Stecklina <julian.stecklina@cyberus-technology.de>
Adding itertools as dependency improves the iteration code in the
following significantly.
With this change, we don't need a copy of the vector. Just something
that can be coerced into an iterator. We also use the bit position
iterator to make the code somewhat clearer. The new code is much
faster, because it will not iterate over every bit, just each 1 bit in
the input.
The next commit will complete this optimization and have some concrete
numbers.
On-behalf-of: SAP julian.stecklina@sap.com
Signed-off-by: Julian Stecklina <julian.stecklina@cyberus-technology.de>
Due to the fw_cfg test being disabled on MSHV this results in no tests
being runnable which results in an error in nextest. Reduce that error
to a warning use --no-tests=warn
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
cargo nextest is an improved test runner that allows retries as well a
reporting the times for the test runs.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
When using nextest for running tests each test is run in its own process
so the old solution of using a static variable for the guest ID (used to
determine the network segment) no longer works.
Instead use a text file on the filesystem protected with an exclusive
lock. The test process will read from it and then write back the next ID
that can be used. It wraps around at the limit of u8 and skips ID 0.
This function intentionally panics rather than propagate errors as it
should only be called for testing purposes and there the panic handler
will give a useful backtrace and cleanup.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
This is required to support exclusive locking on files which is needed
for safe test ID generation when using nextest (since it runs each test
as a separate process.)
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
as of rust 1.90, writes to unix socket streams use send_with_flags
instead of write, so it uses a sendto syscall instead of write.
Signed-off-by: Matt Moriarity <matt@mattmoriarity.com>
To get that list, I've used
```
git log | grep --fixed-strings -- "-by:" | head -n 100000 | sort | less
```
on the Linux kernel's git repository.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
The set of extensions supported by a RISC-V system needs to be exposed
to the guest - currently that is a fixed, minimal set of extensions.
These extensions are not sufficient to boot Ubuntu 25.10 which now has a
mininimum requirement of RVA23S64 (which is a minimum set of extensions
that make sense for server use cases.)
The easiest way to convey the extensions that the guest should use is to
copy those that the host kernel understands (and thus includes in the
/proc/cpuinfo) data.
However since nested virtualisation is not currently possible - exclude
the "H" (Hypervisor) extension from the list of short (single letter)
extensions.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
After updating the Linux kernel to 6.19.6 the second segment (segment=1)
is now under the 2nd IOMMU group (which it a more logical setup) and as
such the added device which is on that segment is in that second IOMMU
group.
The same check is made in test_vdpa_block so also test there.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Resignal every 10ms the thread if it has not acknowledged the signal via
setting the atomic when the vCPU thread was acknowledged. Further, avoid
an infinite loop by generating an error if it takes more than 1000ms to
interrupt the thread.
The retry helps mitigate a race condition where the signal is received
between checking the pause atomic and entering KVM_RUN ioctl when
pausing. Hitting this race condition would leave the
wait_untial_signal_acknowledged() method spinning indefinitely.
The timeout error avoids the VMM process being blocked indefinitely.
See: #7427
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Dump kernel logs after running the tests in the MSHV workflow to help
debug failures.
In addition to getting the kernel logs using `dmesg` also use AzCli to
retrieve the serial console logs. If the VM is hung or panicked, the
workflow would be unable to SSH into it and execute `dmesg`. In this
case the serial console logs would be helpful.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
Replace the use of `guestmount and `guestunmount` with standard tools
(losetup, mount) to modify cloud disk image. This eliminates
dependency on guestmount which requires /dev/kvm and is not available
in MSHV root partition.
Signed-off-by: Aastha Rawat <aastharawat@microsoft.com>
Refactor MSHV hypervisor selection logic to enable aarch64 support
instead of exiting. Use `--features mshv` to conditionally build &
test when hypervisor is mshv replacing hard exit for aarch64 on mshv.
Signed-off-by: Aastha Rawat <aastharawat@microsoft.com>
There are several copy-pasted code fragments in impl QcowFile. All of
them add L2 entry to the cache and one of them (in file_offset_write())
does also allocating new L2 entry if necessary.
Fold all these code fragments (except of one in l2_table() which does
error handling in special way) into cache_l2_cluster() method without
changing the logic.
This will make the code more compact and clean.
Signed-off-by: Eugene Korenevsky <ekorenevsky@aliyun.com>
With the current syntax docker gives 'docker: invalid reference format'
error. Also during parsing /xxx:/yyy in process_volumes_args
with \" inside variable i.e arr_vols=("${arg_vols//#/ }")
gives wrong output.
Example:
scripts/dev_cli.sh tests --integration --volumes /mshv:/mshv
Error: The volume /mshv /mshv does not exist.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
The API documentation tells users to expect a message when Cloud
Hypervisor is launched, this message was removed in commit 13724db
This change updates the documentation to reflect how the program
actually functions, which is to say no message.
Signed-off-by: Ariel Chenet <achenet@fastmail.com>
Consuming `&Arc<T>` as argument is almost always an antipattern as it
hides whether the callee is going to take over (shared) ownership
(by .clone()) or not. Instead, it is better to consume `&dyn T` or
`Arc<dyn T>` to be more explicit. This commit cleans up the code.
The change is very mechanic and was very easy to implement across the
code base.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Disable test_snapshot_restore_hotplug_virtiomem for mshv. It is failing
frequently in the CI. It needs to be stabilized before enabling again.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
The ivshmem tests involving snapshot/restore & live migration are
failing in the MSHV CI with this error:
Could not get vCPU state GetMsrEntries(
Hypercall 80 failed with 0x5 : InvalidParameter ...<snip>)
This needs more investigation. It is worth noting that the general live
migration tests are also not run in the CI for MSHV.
Disable these tests for MSHV for the time being.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
The ivshmem tests are all failing in the CI for MSHV because Cloud
Hypervisor is built without the mshv feature.
Error: Cloud Hypervisor exited with the following chain of errors:
0: Failed to open hypervisor interface (is hypervisor interface
available?)
1: Failed to create the hypervisor
2: no supported hypervisor
Modify the build command to include the mshv feature.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
The existing concurrency configuration causes workflow runs for one Pull
Request to cancel in-progress runs for another Pull Request. When the
workflow is triggered by the `pull_request_target event`, `github.ref`
is always set to the base branch, regardless of which branch raised the
PR. This causes every new pull request to share the same concurrency key
and thus cancel the running CI of any other open pull request.
This commit updates the `concurrency.group` to use
`github.event.pull_request.number` for unique isolation when running on
PR, falling back to `github.ref` for non-PR push (e.g., merge group).
Signed-off-by: AASTHA RAWAT <aastharawat@microsoft.com>
Disable the following common parallel and common sequential tests for
mshv since these are failing consistently in the CI.
Common parallel:
- test_tpm
- test_cpu_topology_421
- test_cpu_topology_142
- test_cpu_topology_262
- test_cpu_hotplug
Common sequential:
- test_snapshot_restore_basic
- test_snapshot_restore_with_fd
- test_snapshot_restore_pvpanic
Signed-off-by: AASTHA RAWAT <aastharawat@microsoft.com>
test_fw_cfg is frequently failing in the CI for MSHV. Exclude it for
now. It needs further investigation. See issue #7434 for details.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
Move the GICv2M MSI frame SPI base from 32 to 128 to avoid conflicts
with platform device interrupts.
While at it, rename the constants to make it explicit that they are
associated with the GICv2m MSI Frame.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
This was added in 7be69edf51 to deal with
changes to the KVM bindings that made run() and set_immediate_exit()
take &mut self. Instead adopt a Box<> value in Vcpu allowing the removal
of this internal Mutex.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
The virtio spec defines the feature bit range and 4 x 32-bit pages.
However no features are currently defined with bits > 63. However Linux
has now started acking features in those higher pages. Since this is
harmless (we drop the acks to those feature pages) and quietly return 0
for available features in those higher pages this warning can be
removed.
Fixes: #7414
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Fix a race condition that happens in resume()-pause() cycles.
It is odd that for pause(), the CpuManager waited via `state.paused`
for the vCPU thread to ACK the state change but not for `resume()`.
In the `resume()` case, oddly CpuManager "owned" the state change
in `state.paused`. This commit changes this so that the vCPU ACKs
its state change itself in `state.paused` when it transitions from
pause->run. Further, `CpuManager::resume()` now gracefully waits for
the vCPU to be resumed.
More technical:
This change ensures proper synchronization and prevents situations
in that park() follows right after unpark(), causing deadlocks and
other weird behavior due to race conditions.
Calling resume() now takes slightly longer, very similar to pause().
This is, however, even for 254 vCPUs in the range of less than 10ms,
and ultimately we now have correct behaviour.
## Reproducer
Since [0] is merged, the underlying problem can be tested without this
commit by modifying the pause() API call to run `CpuManager::pause()`
and `CpuManager::resume()` in a loop a thousand times.
`ch-remote --api-socket ... pause`
```patch
diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs
index d7bba25cc..35557d58f 100644
--- a/vmm/src/vm.rs
+++ b/vmm/src/vm.rs
@@ -2687,6 +2687,10 @@ impl Pausable for Vm {
MigratableError::Pause(anyhow!("Error activating pending virtio devices: {:?}", e))
})?;
+ for _ in 0..1000 {
+ self.cpu_manager.lock().unwrap().pause()?;
+ self.cpu_manager.lock().unwrap().resume()?;
+ }
self.cpu_manager.lock().unwrap().pause()?;
self.device_manager.lock().unwrap().pause()?;
```
## Outlook
Decades of experience in VMM development showed us that using
many AtomicBools is a footgun. They are not synchronized with each
other at all. On the long term, we might want to refactor things to
have a single shared AtomicU64 with different bits having different
meanings.
[0] https://github.com/cloud-hypervisor/cloud-hypervisor/pull/7290
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
These bindings actually refer to atomic bool shared across all
vCPUs to instruct single vCPUs with their next action. As there
are already enough Arc<AtomicBool>, this helps while debugging
things to see that different bindings refer to the same atomic
bool.
In other words: This naming really confused us while debugging.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This gives a reliable way of identifying if the VM has booted as well as
a reliable way to validate the addition of the network interface.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Add a second L1 network interface definition to the guest Cloud Init
configuration, including an additional host IP. Do this by splitting the
network range into two /25s. For clarity the network struct members have
also been renamed.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Previous RISC-V ACPI related change introduced MSIS which will be
rejected by typo check, let's extend the typos to accept it.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Add necessary definitions and RISC-V ACPI tables to enable ACPI feature.
More specifically, this commit add MADT definitions for RISC-V AIA
interrupt chips.
Signed-off-by: Can Zhang <icloud9957@gmail.com>
Rename the MSHV CI workflow to better align with the existing naming
scheme and also make the arch explicit.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
Add step-level timeout for running integration tests so that the
workflow doesn't get stuck on any test.
Signed-off-by: AASTHA RAWAT <aastharawat@microsoft.com>
We are actually using v0.3.0 of vm-fdt since main branch of that crate
already includes everything. Let's pin vm-fdt to v0.3.0 instead.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
Pull requests from forked repos cannot access GitHub secrets which
results in failure of MSHV CI. Switching to pull_request_target
resolves this. It allows the workflow to run with access to repo secrets
and ensures that code from the base branch is used instead of forked
code, preventing potential security risks.
Signed-off-by: AASTHA RAWAT <aastharawat@microsoft.com>
Add workflow to run integration tests on mshv. It calls the azure
infra setup workflow and executes integration tests in the
provisioned environment.
Signed-off-by: AASTHA RAWAT <aastharawat@microsoft.com>
Add workflow to setup Azure infra to validate MSHV.
This is used to provision the environment before executing tests
on it.
Signed-off-by: AASTHA RAWAT <aastharawat@microsoft.com>
This provides clearer life-cycle management of resources around vfio,
and aligns better with the kernel behavior as reported below with vfio
legacy mode (with vfio container/group).
Fixes: #7328
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Signed-off-by: Yi Wang <foxywang@tencent.com>
We recently moved many of our tests to use focal to jammy as the guest
images, including rate-limiter tests (#7367). We forgot to update the
rate-limiter scripts to reflect such change.
Our CI pipeline failed to report such error because our self-host runner
happened to be not working at the time we land the changes (see #7405).
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Only use 75% of the available threads - this will reduce dislk and
memory pressure. Reducing the chance of flaky tests.
See: #7405
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
While an IGVM is provided validation fails as there is
no kernel or firmware. This patch fixes the sev_snp boot
failure.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
MSHV (Microsoft Hypervisor) doesn't expose an ITS to guests. Instead it
exposes a GICv2m MSI Frame for mapping MSIs to SPIs.
Update the MADT generation code to create the GIC MSI Frame structure
instead of the ITS structure whenever the guest vGIC is using a GICv2m
frame. This allows booting guests via firmware on MSHV.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
In preparation for removing focal support - start by using a more
generic "disk_config" variable name for the DiskConfig struct vs
embedding the image type.
Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
Rust's aliasing rules do not permit mutating a `usize` through a shared
reference. Unsafe blocks and FFI are no exception.
This PR fixes such a violation detected in the AMX support checks by
passing a raw mutable pointer instead of a shared reference in the
syscall.
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de>
On-behalf-of: SAP <oliver.anderson@sap.com>
For a graceful resource management of externally provided FDs in Cloud
Hypervisor, corresponding FDs need to be closed on a device removal.
This is the case for virtio-net devices using external FDs, for example.
With the fix introduced in this commit, we allow management software to
properly clean up resources, e.g., libvirt can clean up tap devices.
PS: CHV uses "added" and "removed", which has the same meaning as
hot device attach/hotplug and hot device detach/unplug.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Fix clippy warning `uninlined_format_args` reported by rustc rustc
1.89.0 (29483883e 2025-08-04).
```console
warning: variables can be used directly in the `format!` string
--> block/src/lib.rs:649:17
|
649 | info!("{} failed to create io_uring instance: {}", error_msg, e);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
= note: `#[warn(clippy::uninlined_format_args)]` on by default
help: change this to
|
649 - info!("{} failed to create io_uring instance: {}", error_msg, e);
649 + info!("{error_msg} failed to create io_uring instance: {e}");
|
```
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
"Update configs must have a unique combination of 'package-ecosystem',
'directory', and 'target-branch'. Ecosystem 'cargo' has overlapping
directories for target branch 'main'."
Drop `target-branch` in rust-vmm group to make those two unique.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
## TL;DR
This unifies error handling, implementation, and logging of config
objects that are populated with additional FDs received as part of
the HTTP API request from the management software. This prevents current
and future code in repetition.
In the future, if we want to support let's say virtio-blk with external
FDs, the AddBlk API handler simply needs a call to `attach_fds_to_cfg()`
and gets all the magic for free.
## Motivation
The motivation of the new abstraction is to provide a verbose, solid,
and bulletproof solution for a complex domain. The interaction between
- the management layer,
- the passing of file descriptors over UNIX domain sockets via
SCM_RIGHTS,
- the attachment of configurations to those FDs,
- and the ability to give new developers clear insights into what
happens under the hood
is not trivial. These factors justify encapsulating the complexity
behind a convenient and well-documented abstraction, making the system
both robust in production and approachable for new developers.
In addition, it allows us to perform unit testing. Further, We get rid
of existing partial code duplication and inconsistencies.
Finally, while this approach may initially result in more code, every
new handler that accepts FDs benefits from reduced duplication and a
correct implementation by relying on the shared abstraction. This will
also enable future functionality, such as virtio-blk devices backed by
FDs which can then be integrated with ease.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Deserializing values as `-1` makes sense to prevent errors, so let's
keep it. However, serializing them differently adds confusion. For
example, a `ch-remote info` call should not report `-1` but the actual
FDs.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
(1) The old messages are missing the "why" part. With this change, users
of Cloud Hypervisor have somehow more context and people looking at
the code perfectly know what's going on.
(2) Using warn! implies that the user should take action, but in this
case, there’s nothing the user can do. If the API is used correctly
and file descriptors are passed via an SCM_RIGHTS message over a
UNIX domain socket, everything works as intended. In that case,
there's no need to issue a warning — debug! is sufficient.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Live migration, state save/resume, and hotplug are not trivial when it
comes to virtio-net devices backed by externally provided FDs. As the
mechanism behind it can be considered as quite "multi-step magic" even
for experienced programmers, it makes sense to thoroughly document this
to ease debugging and to improve the mental model of developers working
on this in the future.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This change enables easier integration with third-party
tools by removing the requirement for a dummy IP address
when configuring tap devices. The modification applies to
both CLI and API interactions.
Previously, cloud-hypervisor would automatically set a
default static IP address (192.168.249.1) if none was provided.
This could lead to:
* multiple devices without explicit IP configurations
would end up with the same default IP
* unnecessary inclusion of this IP in firewall rules
* the IP address could clash with host networking and
routing
This introduces a new constraint:
When providing an IP, the mask must also be provided.
Removes warnings introduced in #7179.
Closes issue #7083.
Signed-off-by: Maximilian Güntner <code@mguentner.de>
The function `read_local_stream_port` had no proper handling for
unexpected or incomplete input.
When the control socket of the VSOCK device was closed without sending
the expected `CONNECT <PORT>\n` statement completely, the thread
got stuck in an infinite loop as it attempted to read from a closed
socket over and over again which never returned any data.
This resulted in the thread responsible for `epoll` being completely
blocked. New VSOCK connections could not be established and existing
ones became defunct, effectively leading to a Denial of Service of
the entire VSOCK device.
The issue can be reproduced by opening a socket and immediately
closing it.
```
socat - UNIX-CONNECT:/socket.vsock
<Ctrl-C>
```
Instead of applying a quick fix by handling the `EPOLLHUP` event before
reading, the function is refactored to remove the error-prone `while`
loop and multiple `read`s.
Notably, we now check if the number of bytes read is zero, which occurs
when `event_set == EPOLLHUP | EPOLLIN`, indicating that the socket has
been closed by the client.
Additionally, the actual parsing code is now extracted into a dedicated
function that is tested.
Fixes: #6798
Signed-off-by: Maximilian Güntner <code@mguentner.de>
For MSHV customers don't want to make everything
default during partition creation. For example
nested support, some synthetic features could be
controlled from CLI through platform argument.
Create_vm API getting messy after adding more flags.
This patch introduces common data struct to be passed
from vmm crate to hypervisor crate during partition creation.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Handle MSHV reset intercepts on arm64 such that guests can gracefully
shutdown/reboot instead of panicking due to unhandled intercept.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
Reference link to set_device_attribut for aarch64
is invalid. Looks like the code browsing does not
have the reference anymore.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
This message makes it easier to confirm a successful shutdown when
scanning logs during development and in production. If the message is
missing and the last log entry is not an error, the most likely cause
is a livelock (e.g. contention on a lock) introduced while developing
new Cloud Hypervisor features.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This commit includes all simple clippy fixes excluding the
collapsing of nested ifs using the let-chains feature. This
follows in the next commit.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Bump mshv-ioctls and mshv-bindings to 0.6.0. Most notably, this version
contains fixes and new bindings for arm64 guests.
Bump the vfio crates too so that they point to the latest mshv crates.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
Virtio console is activated much later in boot. The output it spits out
lacks the initial CPU configuration of the guest.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
These can differ between platforms, so it's better to use centralized
definitions of them. We can't currently do this for the KVM and VFIO
ioctls, because the corresponding crates don't publicly expose the
ioctl numbers.
Signed-off-by: Alyssa Ross <hi@alyssa.is>
The previously linked file was converted to ReStructuredText, then
later removed from the kernel, with part of the justification being
"Most of what booting-without-of.rst contains is now in the DT
specification", so point to that instead.
Signed-off-by: Alyssa Ross <hi@alyssa.is>
Since the user has to explicitly ask for Landlock to be enabled in
Cloud Hypervisor, it's surprising that, even when they've done that,
Landlock will silently not be enabled if the kernel doesn't support
it.
To prevent accidental absence of a desired security protection, exit
with an error if Landlock, or the one feature of it we use in Cloud
Hypervisor (file access) is not supported.
Signed-off-by: Alyssa Ross <hi@alyssa.is>
arch::aarch64::fdt::create_cpu_nodes will always look at this if it
exists. (If it doesn't exist, this is a no-op —
add_rule_with_access() won't add rules for paths that don't exist.)
Fixes: b3e5738b4 ("vmm: Introduce ApplyLandlock trait")
Signed-off-by: Alyssa Ross <hi@alyssa.is>
MSHV now supports movable pages i.e VA backed guest.
Also with more features and stability in the MSHV
Kernel drives MSHV now supports more test scenario.
This patch enables more integration test cases.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Without write access to the directory the socket will go in, it's not
possible to create the socket.
I've tested outgoing connections from the VM, and they don't seem to
need read permissions on that directory to connect to a socket on the
host.
Fixes: b3e5738b4 ("vmm: Introduce ApplyLandlock trait")
Signed-off-by: Alyssa Ross <hi@alyssa.is>
path_beneath_rules() just needs the paths given to it to be
AsRef<Path>, so there's no need to create new PathBufs for it.
Signed-off-by: Alyssa Ross <hi@alyssa.is>
Raise the max number of supported (v)CPUs on kvm x86_64 hosts
to 8192 (the max allowed value of CONFIG_NR_CPUS in the Linux kernel).
Other platfroms keep their existing CPU limits pending further
development and testing.
The change has been tested on Intel and AMD hosts.
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>
These are now used by Cloud Hypervisor when linked with Glibc 2.42.
These values should be correct for all currently supported Cloud
Hypervisor platforms, although they are not for all Linux platforms.
Closes: https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7276
Signed-off-by: Alyssa Ross <hi@alyssa.is>
This commit removes the SGX support from cloud hypervisor. SGX support
was deprecated in May as part of #7090.
Signed-off-by: Shubham Chakrawar <schakrawar@crusoe.ai>
This patch gives user an option to override the
default migratable version to any later release.
This option makes MSHV specific tests suitable for
tests since MSHV is stable after some breaking changes.
This patch is also necessary for MSHV CI.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
This step was done manually by searching for "unsafe fn" in
the code base and adding corresponding Safety sections.
`clippy::missing_safety_doc` only works for public functions
but none of the corresponding functions is public.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Implement the batch submission function for raw disk, default it is
enabled. After parsing the requests this method is
called for better IO latency and bandwidth.
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Cache and batch IO requests after parsing all
items in the queue, improving performance—especially
for small block sizes—by reducing per-request overhead.
Introduced two methods in the AsyncIo trait for batch
submission, with implementation in the raw disk backend.
This method should be called during/after parsing all block IO requests
in the available queue. If the batch submission is not enabled, by
default it does the old way of submitting requests.
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Instead of returning boolean return an struct of completion status
so that it can be cached for batch submission.
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Signed-off-by: Muminul Islam <muislam@microsoft.com>
This patch changes the read-only check using acked features bit, which
will help to check more features.
Signed-off-by: Songqian Li <sionli@tencent.com>
Also tweak extended interrupt handlings, as needed.
Most credit should go to Neel Natu, who figured out the magic
bits needed to make things work and provided detailed comments.
This patch is still NOOP, as VM config allows only up to
254 vCPUs on x86_64.
Note: changes in this and related previous patches/PRs have
only been tested on Linux hosts running on Intel x86_64 hardware.
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>
test_direct_kernel_boot_bzimage runs only on x86, so the cfg!() branch
for selecting grep_cmd is unnecessary. Remove it for clarity.
Signed-off-by: Anirudh Rayabharam <anrayabh@microsoft.com>
One can call `to_vec()` anyway if one needs an owned copy. This change
further helps to prevent needless copies in upcoming changes.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
On aarch64 and RISC-V, calling load_firmware() through load_kernel()
provides no benefit and only duplicates checks already performed in
load_payload(). load_payload() now directly invokes load_firmware() or
load_kernel(), removing unnecessary indirection and redundancy.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Both functions are defined separately for the two architecture with
minor differences.
* `load_firmware()`: call `arch::uefi::load_uefi` which are available on
both architecture;
* `load_kernel()`: manually align to `arch::layout::KERNEL_START` 2MB
for both architecture (e.g. no-op for `aarch64`);
Signed-off-by: Bo Chen <bchen@crusoe.ai>
With 254 vCPUs, pausing now takes ~4ms instead of >254ms. This
improvement is visible when running `ch-remote pause` and is
particularly important for live migration, where every millisecond
of downtime matters.
For the wait logic, it is fine to stick to the approach of
sleeping 1ms on the first missed ACK as:
1) we have to wait anyway
2) we give time to the OS, enabling it to schedule a vCPU thread next
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Enable kvm build test and clippy test on RISC-V 64-bit platform to
ensure whole projects builds properly.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
MP table is a legacy device that is incompatible
with x2apic CPU IDs exceeding 254. The Linux kernel
is perfectly happy without MP table in these cases.
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>
Currently, the following scenarios are supported by Cloud Hypervisor to
bootstrap a VM:
1. provide firmware
2. provide kernel
3. provide kernel + cmdline
4. provide kernel + initrd
5. provide kernel + cmdline + initrd
As the difference between `--firmware` and `--kernel` is not very clear
currently, especially as both use/support a Xen PVH entry, adding this
helps to identify the cause of misconfiguration.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This is necessary to use the let-chains feature in a
follow-up. After upgrading to Rust edition 2024, clippy
wants to collapse various if's with let-chains.
Update image to 20250815-0 since MSRV in Dockerfile is updated.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
This patch introduces the inter-vm shared memory(ivshmem) device
to share a memory region between multiple processes running
different guests and the host.
This patch supports the basic ivshmem functions like ivshmem-plain
in QEMU[1].
[1] https://www.qemu.org/docs/master/specs/ivshmem-spec.html
Signed-off-by: Yi Wang <foxywang@tencent.com>
Signed-off-by: Songqian Li <sionli@tencent.com>
Move UserspaceMapping to vm-device to avoid redefinition since
UserspaceMapping is used by both `virtio-devices` and `device`
crate.
Signed-off-by: Songqian Li <sionli@tencent.com>
It is always called with topology provided, so there is no
need to pass topology as an Option. Simplifying the signature
makes further topology-related changes to arc/src/x86_64 module
simpler.
Signed-off-by: Peter Oskolkov <posk@google.com>
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 us to enable/disable the fw_cfg device via the cli
We can also now upload files into the guest vm using fw_cfg_items
via the cli
Signed-off-by: Alex Orozco <alexorozco@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>
We pass a reference to the guest memory when we create the device
in DeviceManager. This allows us to access the guest memory for DMA.
Signed-off-by: Alex Orozco <alexorozco@google.com>
The acpi tables are created in the same place the acpi tables would be
created for the regular bootflow, except here we add them to the
fw_cfg device to be measured by the fw and then the fw will put the
acpi tables into memory.
Signed-off-by: Alex Orozco <alexorozco@google.com>
The kernel and initramfs are passed to the fw_cfg device as
file references. The cmdline is passed directly.
Signed-off-by: Alex Orozco <alexorozco@google.com>
Here we add the fw_cfg device as a legacy device to the device manager.
It is guarded behind a fw_cfg flag in vmm at creation of the
DeviceManager. In this cl we implement the fw_cfg device with one
function (signature).
Signed-off-by: Alex Orozco <alexorozco@google.com>
This partially reverts
ed8f347fe6 from #7183 and
6277d7d5f2 from #7201.
# Output how it was merged for v47 (#7066)
```
Error: Cloud Hypervisor exited with the following chain of errors:
0: Error booting VM
1: The VM could not boot
2: Error manipulating firmware file
3: No such file or directory (os error 2)
Debug Info: VmBoot(VmBoot(FirmwareFile(Os { code: 2, kind: NotFound, message: "No such file or directory" })))
```
# Output after #7183 and #7201
```
cloud-hypervisor: 31.385730ms: <main> ERROR:/home/pschuster/dev/cloud-hypervisor/src/lib.rs:27 -- Error: Cloud Hypervisor exited with the following chain of errors:
cloud-hypervisor: 31.417961ms: <main> ERROR:/home/pschuster/dev/cloud-hypervisor/src/lib.rs:39 -- 0: Error booting VM
cloud-hypervisor: 31.448078ms: <main> ERROR:/home/pschuster/dev/cloud-hypervisor/src/lib.rs:39 -- 1: The VM could not boot
cloud-hypervisor: 31.486711ms: <main> ERROR:/home/pschuster/dev/cloud-hypervisor/src/lib.rs:39 -- 2: Error manipulating firmware file
cloud-hypervisor: 31.513331ms: <main> ERROR:/home/pschuster/dev/cloud-hypervisor/src/lib.rs:39 -- 3: No such file or directory (os error 2)
cloud-hypervisor: 31.548037ms: <main> ERROR:/home/pschuster/dev/cloud-hypervisor/src/lib.rs:44 --
cloud-hypervisor: 31.568045ms: <main> ERROR:/home/pschuster/dev/cloud-hypervisor/src/lib.rs:45 -- Debug Info: VmBoot(VmBoot(FirmwareFile(Os { code: 2, kind: NotFound, message: "No such file or directory" })))
```
The "proper logger" has indeed the advantage that messages can
be gracefully redirected to log files etc. However, this makes the
error message hardly readable.
Therefore, I propose to use error!() only for runtime errors messages
but not a pretty-printed version of those.
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
On-behalf-of: SAP philipp.schuster@sap.com
Error::UefiLoad is required for load_firmware to propagate errors
encountered, define it for riscv64.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
uefi_flash field in memory_manager is required for uefi loading and
booting, enable it for riscv64 architecture.
Signed-off-by: Ruoqing He <heruoqing@iscas.ac.cn>
The option parser did not allow quoting any special characters, which
means that some strings could not be passed via command-line options.
Fix this by allowing double quotes to quote all the special characters,
such as '@', ',', '[', and ']'. '"' can be escaped by doubling it. The
implementation uses some tricks to stay modular.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
rustc 1.90.0-beta.1 (788da80fc 2025-08-04) suggests using library
feature `unsigned_is_multiple_of`. It is stabled in Rust 1.87.0.
Update image to 20250807-0 since MSRV in Dockerfile is updated.
Signed-off-by: Songqian Li <sionli@tencent.com>
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Now 'create_acpi_tables_internal()' can generate ACPI tables with
different dsdt offset, so that it can be reused for generating ACPI
tables for `fw_cfg`.
Signed-off-by: Bo Chen <bchen@crusoe.ai>
Signed-off-by: Alex Orozco <alexorozco@google.com>
Optimize lychee workflow to check only changed
files in pull requests, avoiding excessive API
calls to prevent rate limits.
Fixes#7056
Signed-off-by: Shubham Chakrawar <schakrawar@crusoe.ai>
In case of CVM guest rsdp is set to none. Unwrapping it
make the vmm crashed. Don't call configure system if the
rsdb address is none.
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Many of the workspace members in the Cloud-hypervisor workspace share
common dependencies. Making these workspace dependencies reduces
duplication and improves maintainability.
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de>
On-behalf-of: SAP oliver.anderson@sap.com
The current documentation only mentions Windows
server support. Since only minimal adjustments
are needed for Windows 11 support, these were
added where applicable.
Also contains whitespace fixes, adds missing `\`
in example code and adds more cross references.
Signed-off-by: Lucas Grosche <lucas.grosche@cyberus-technology.de>
The other workspace members in the Cloud-hypervisor workspace
currently declare log version 0.4.22, but the tpm crate
has an older version. This inconsistency is addressed by this PR
which opens the door for declaring log as a workspace dependency.
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de>
On-behalf-of: SAP oliver.anderson@sap.com
Running one or two tests in a tight loop can cause the download
functions to quickly hit GitHub's API rate limit. That causes the test
script to fail for no apparent reason.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
Generate the data disk under a temporary directory so that multiple
instances of the test suites can run at the same time.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
The other workspace members in the Cloud-hypervisor workspace
currently declare libc crate version 0.2.167, but the tpm crate
has an older version. This inconsistency is addressed by this PR
which opens the door for declaring libc as a workspace dependency.
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de>
On-behalf-of: SAP oliver.anderson@sap.com
The other workspace members in the Cloud-hypervisor workspace
currently declare anyhow version 1.0.94, but the tpm crate
has an older version. This inconsistency is addressed by this PR
which opens the door for declaring anyhow as a workspace dependency.
Signed-off-by: Oliver Anderson <oliver.anderson@cyberus-technology.de>
On-behalf-of: SAP oliver.anderson@sap.com
It shouldn't be necessary to lock the file for the adaptor. This removes
two layers of indirection for QcowDiskSync and VhdxDiskSync.
Signed-off-by: Wei Liu <liuwe@microsoft.com>
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
