Windows 11 PnP rebalance rewrites peer BARs into the same range CH
packed the initial layout at, causing move_bar() failures and boot
deadlock. Pack Mmio64 BARs at 8 MiB stride. Mmio32 isn't wide enough
for the same stride, but its BARs don't participate in guest BAR
rebalancing.
On restore, pin the BAR to the snapshot address (alignment=None) so a
guest-relocated BAR with smaller alignment is accepted.
Signed-off-by: CMGS <ilskdw@gmail.com>
IommuEpollHandler::request_queue() can fail because the guest put in a
bad request or because of a fatal error. Handle those cases differently,
letting the guest continue, but see the error if it can.
This makes debugging from the guest easier as one mistake doesn't cause
a VM reset if it's avoidable.
Signed-off-by: Dylan Reid <dgreid@fb.com>
Assert VirtioPciNotifyCap and VirtioPciCap64 size cap_len from
their own type. Catches a future regression of the same shape as
the VirtioPciCfgCap one in any of the sibling capabilities.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Assert the emitted VirtioPciCfgCap carries cfg_type 5, the value
assigned to PciCapabilityType::Pci by virtio 1.2 section 4.1.4.1.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Regression test for the cap_len fix. The emitted VirtioPciCfgCap
must report cap_len 20, covering the trailing pci_cfg_data window
per virtio 1.2 section 4.1.4.9.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
VirtioPciCfgCap::new built its inner header via VirtioPciCap::new,
which sized cap_len from the bare virtio_pci_cap layout, yielding
16. The emitted capability is VirtioPciCfgCap, which appends a four
byte pci_cfg_data window, so the correct value is 20.
The virtio 1.2 specification defines this cap as virtio_pci_cap
followed by pci_cfg_data[4] and requires cap_len to
cover the whole structure. Build the header inline so cap_len
reflects the actual emitted size, matching VirtioPciNotifyCap and
VirtioPciCap64.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
I started by looking at all `Option<Vec<T>>` values in config.rs and
vm_config.rs, and replaced them with `Option<Box<[T]>>`. This has the
advantage that one now can see at a glance if this field will ever
resize during operation or not, reducing cognitive load and increasing
maintainability. All fields that need the properties of a Ver or where
this change was not trivial are kept intact.
On-behalf-of: SAP philipp.schuster@sap.com
Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de>
Some block devices (ZFS volume) may require BLKDISCARD and BLKZEROOUT
ioctls for discard and write_zeroes operations respectively.
There is no good way to probe whether fallocate is supported on a block
device. Arguably, punch_hole and write_zeroes are rare. Instead of
having a complex scheme for the IO uring backend, we force it to always
use ioctls. The code can be changed if the synchronized ioctls become a
performance issue.
Changes:
- Detect block devices at construction time
- Use BLKDISCARD ioctl for punch_hole (discard) on block devices
- Use BLKZEROOUT ioctl for write_zeroes on block devices
- Add BLKDISCARD/BLKZEROOUT to VirtioBlock seccomp whitelist
- Keep fallocate() path for regular files (no behavior change)
- Consolidate some helper functions to the new sparse module
Signed-off-by: Wei Liu <liuwe@microsoft.com>
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>
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>
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>
Signed-off-by: Bo Chen <bchen@crusoe.ai>
When no image_type is specified, sector 0 writes are disabled as a
safety measure for autodetected raw images. Extend this protection
to autodetected fixed VHD images, which carry metadata in the last
sector and are equally susceptible to accidental overwrites of the
first sector when the format is not explicitly acknowledged.
Update the corresponding warning in the virtio block worker to be
format agnostic.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Quoting the spec:
> If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, and the back-end
> sets the VHOST_USER_NEED_REPLY flag, the front-end must respond with
> zero when operation is successfully completed, or non-zero
> otherwise.
cloud-hypervisor would previously not send a response to a
VHOST_USER_BACKEND_CONFIG_CHANGE_MSG message, even if
VHOST_USER_PROTOCOL_F_REPLY_ACK had been negotiated and
VHOST_USER_NEED_REPLY was set, in violation of the spec.
Link: https://qemu-project.gitlab.io/qemu/interop/vhost-user.html#back-end-message-types
Fixes: 8d6213338 ("virtio-devices: generic-vhost-user: Config change notification")
Signed-off-by: Alyssa Ross <hi@alyssa.is>
If a guest observes DEVICE_NEEDS_RESET, resets the device, and tries to
re-initialize it, but the VMM knows the backend is disconnected, we can
short-circuit the doomed activation.
This is not incorrect, but saves the VMM from making several round-trip
calls to a peer process that doesn't exist. It'll also make the logs
cleaner.
Signed-off-by: Dylan Reid <dgreid@fb.com>
resume() mirrors pause() for backend communication: it skips the
vhost-user backend call when the device is already disconnected, and it
marks newly failed resume_vhost_user() calls disconnected only when the
classifier identifies transport loss.
Signed-off-by: Dylan Reid <dgreid@fb.com>
pause() returns DeviceDisconnected without calling into the backend when
VhostUserCommon already knows the socket is gone. DeviceManager treats
only that sentinel as log-and-continue, so one dead vhost-user device
does not abort the whole pause iteration.
Signed-off-by: Dylan Reid <dgreid@fb.com>
For add memory region, if the backend is disconnected or returns an
error, forward the appropriate error type to the caller. If the error
indicates that the vhost user backend has disconnected, mark it as such.
Signed-off-by: Dylan Reid <dgreid@fb.com>
reset() is teardown and must still clean up local state even if the
vhost-user backend has already gone away. When the disconnected flag is
already set, it skips reset_vhost_user() and proceeds with kill-event,
worker-unblock, event logging, and interrupt callback cleanup.
Signed-off-by: Dylan Reid <dgreid@fb.com>
This function hasn't been used since '22.
All callers removed with:
1f0e5eb66 vmm: virtio-devices: Restore every VirtioDevice upon creation
TEST: build and cargo test all still pass, grep returns no results.
Signed-off-by: Dylan Reid <dgreid@fb.com>
Add two explicit disconnected-backend error paths before wiring them
into the call sites.
MigratableError::DeviceDisconnected is the lifecycle sentinel for
operations that were skipped because a component is already known to be
disconnected. It lets the caller log and continue without treating it as
a VMM-fatal condition.
Error::BackendDisconnected is the vhost-user-local error used when
VhostUserCommon refuses to call a backend after its disconnected flag is
set. The transport classifier treats socket close/reset/EOF and
vhost-user partial-message/disconnected cases as transport loss, while
backend NACKs, invalid protocol state, and retry-able socket errors
remain ordinary operation failures.
Signed-off-by: Dylan Reid <dgreid@fb.com>
Add a 'disconnected' flag shared between VhostUserCommon and
VhostUserEpollHandler. This flag is set whenever the run loop hits an
error that would cause an exit (failed reconnect, broken backend req
handler, unknown event).
Following commits will use this to gate backend calls in order to avoid
repeated timeouts and errors when a backend disappears. This will
simplify shutdown sequencing for orchestrators using vhost-user devices.
Signed-off-by: Dylan Reid <dgreid@fb.com>
When a guest resets a device by writing status=0 and reinitializes
without enabling queues before writing DRIVER_OK, the activation
path would collect zero ready queues and treat that as a fatal
error, killing the entire VMM process.
The PCI transport now checks that at least one queue is ready
before reporting that the device needs activation. This prevents
a spurious activation attempt that would fatally fail when no
queues are enabled.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
config_generation is a u8 and the spec mandates wrap around.
Verify the increment past 0xff lands on 0x00 without panicking
under overflow checks.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Cover the three observable cases of consume_config_change. With
the flag set the counter advances by one and the flag is cleared.
With the flag clear the call is a no-op. A burst of flag sets
between two reads results in only one bump, which is the wrap
hazard mitigation the spec asks for.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
A Config trigger must set the config_changed flag so the next
device specific configuration read can bump config_generation. A
Queue trigger must leave the flag alone.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Set a config_changed flag in VirtioInterruptMsix when a Config
interrupt fires, and increment config_generation only when the
driver next reads the device specific configuration region. The
flag is cleared by that read so the driver observes a stable
value across the read and a fresh value on any later read.
This avoids the wrap hazard of incrementing on every Config
event, where a burst of interrupts could roll the 8 bit counter
back to its previous value between two driver reads.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Convert the u8 field to Arc<AtomicU8> so the interrupt path can
mutate it without holding the common config mutex. Reads at
offset 0x15 use Acquire ordering. State serialization preserves
the value.
Update the three unit tests that construct
VirtioPciCommonConfig directly.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Currently when the socket is disconnected by the other end there is no
logging of such an event. Add a log to aid identifying when this has
happened.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Abandon the reconnection to the vhost-user socket if the kill_evt is
fired because e.g. a device removal request has come in during the
reconnection.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Replace the use of sleeps with a TimerFd. Initially this is functionally
equivalent but it can be extended to also handle other events.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Drop inflate or deflate descriptors whose len exceeds the Linux
driver maximum of VIRTIO_BALLOON_ARRAY_PFNS_MAX PFN entries of 4
bytes each. Without the cap, a guest can submit a descriptor with
a huge len over a small backing and drive an unbounded warn loop
in the device thread.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
block::Error implements Display via thiserror, so the user facing
log lines do not need the Debug formatter.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
With the per handler needs_reset() gates removed from net and
block, nothing reads DEVICE_NEEDS_RESET anymore. Drop the
device_needs_reset helper and refresh the doc comment on
mark_device_needs_reset to reflect the central call site in
spawn_virtio_thread, where it runs after the worker has already
exited.
No functional change.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
With virtqueue iterator errors now killing the worker and
spawn_virtio_thread marking NEEDS_RESET centrally, the per
handler needs_reset() gate on process_queue_submit and
process_queue_complete is unreachable.
Drop needs_reset(), the two early returns, the unused
device_status field on BlockEpollHandler and its initializer,
and the device_needs_reset import.
No functional change.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
queue.iter() errors used to be swallowed by
handle_queue_iterator_error(), which marked the device as
NEEDS_RESET and returned Ok so the worker kept running while
disabled. spawn_virtio_thread now does the NEEDS_RESET marking
when the worker exits with an error.
Propagate the iterator error as Error::QueueIterator and escalate
it to EpollHelperError::HandleEvent in
process_queue_submit_and_signal so the worker exits. Per request
errors stay logged. Drop the now unused
handle_queue_iterator_error helper.
No functional change for the guest. NEEDS_RESET is still set and
the config interrupt is still raised on virtqueue corruption.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
spawn_virtio_thread now marks the device as NEEDS_RESET and notifies
the guest whenever the worker thread exits with an error, so the
per handler needs_reset() gate and the handle_queue_iterator_error()
helper in net are redundant.
Let virtqueue iterator errors propagate out of the worker thread
through DeviceError::NetQueuePair. Drop the unused device_status
field and the device_needs_reset and mark_device_needs_reset
imports.
No functional change for the guest. NEEDS_RESET is still set and
the config interrupt is still raised on virtqueue corruption.
Signed-off-by: Anatol Belski <anbelski@linux.microsoft.com>
Request::parse rejected unknown request types with an error, causing
process_queue to report the chain as used with no response written.
The device should write an error response so the driver knows the
request was handled. Move type validation out of parse into
process_queue where a proper response can be constructed.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Address translation, guest memory read, write, and flush failures on
the transmitq propagated errors that killed the console device thread.
A host-side I/O error such as a PTY disconnect would permanently
disable the console. Log warnings and continue processing instead.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Address translation or guest memory write failures on the receiveq
propagated errors that killed the console device thread. Log a
warning and break out of the descriptor loop instead.
Also fix a data-loss bug: bytes were drained from the input buffer
before the write to guest memory, so a failed write would silently
discard the data. Copy first, write, then drain only on success.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
The output queue handler read data from every descriptor without
checking the write-only flag. The driver must not put device-writable
buffers in the transmitq. Skip them with a warning.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
The input queue handler wrote data to every descriptor without
checking the write-only flag. The device must not write to
device-readable buffers. Skip them with a warning.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Remove UnknownRequestType (no longer returned after responding with
ERROR), EventFdWriteFail, EventFdTryCloneFail, MpscRecvFail, and
NotActivatedByGuest which have no call sites. Drop the now-unused
mpsc import.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
A malformed descriptor chain — wrong read/write flags, missing
descriptors, or undersized buffers — caused Request::parse to return
an error that killed the device thread. Log a warning, report the
chain as used with zero length, and continue processing.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
is_range_state() and set_range() took nb_blocks as u16 to match the
wire format, but unplug_all() computed the total block count from
region_size / block_size and cast to u16, silently truncating for
regions larger than 128 GiB. Widen the internal parameter to usize
so unplug_all() resets the full bitmap.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
When is_valid_range() returns false, the handler still computed
offset = addr - config.addr which can underflow if addr is below the
region base, then queried the bitmap at a meaningless index. Return
(ERROR, 0) immediately so no arithmetic runs on invalid input.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
An unknown request type caused process_queue to return an error that
killed the device thread. The request/response descriptors are already
parsed at this point, so respond with VIRTIO_MEM_RESP_ERROR and
continue processing.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Address translation or memory release errors on the free page
reporting queue propagated up and killed the balloon device thread.
Log a warning and skip the offending descriptor so the device
continues operating.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
A single malformed descriptor — device-writable where device-readable
is expected, non-aligned length, overflowed address, unmapped PFN, or
a failed fallocate/madvise — propagated an error that killed the
balloon device thread. Skip bad descriptors and PFNs with a warning
so the device keeps operating.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
Only the first descriptor in each chain was processed. If the PFN
array spanned multiple chained descriptors the remaining PFNs were
silently dropped. Iterate the full chain so every descriptor is
validated and its PFNs are acted on.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
The inflate and deflate queues carry device-readable PFN arrays: the
driver writes them and the device only reads. The device never writes
to the descriptor buffers, so the used ring entry should report 0
bytes written rather than the descriptor length.
Signed-off-by: Rob Bradford <rbradford@meta.com>
Assisted-by: Claude:claude-opus-4-6
