Age | Commit message (Collapse) | Author | Files | Lines |
|
Fixes the following Coverity reports:
________________________________________________________________________________________________________
*** CID 417161: Memory - corruptions (ARRAY_VS_SINGLETON)
/samples/server.c: 438 in migration_write_data()
432 }
433
434 /* write to bar0, if any */
435 if (write_end > server_data->bar1_size) {
436 length_in_bar0 = write_end - write_start;
437 write_start -= server_data->bar1_size;
CID 417161: Memory - corruptions (ARRAY_VS_SINGLETON)
Using "&server_data->bar0" as an array. This might corrupt or misinterpret adjacent memory locations.
438 memcpy(&server_data->bar0 + write_start, buf + length_in_bar1,
439 length_in_bar0);
440 }
441
442 server_data->migration.bytes_transferred += bytes_written;
443
________________________________________________________________________________________________________
*** CID 417160: Memory - corruptions (ARRAY_VS_SINGLETON)
/samples/server.c: 394 in migration_read_data()
388 }
389
390 /* read bar0, if any */
391 if (read_end > server_data->bar1_size) {
392 length_in_bar0 = read_end - read_start;
393 read_start -= server_data->bar1_size;
CID 417160: Memory - corruptions (ARRAY_VS_SINGLETON)
Using "&server_data->bar0" as an array. This might corrupt or misinterpret adjacent memory locations.
394 memcpy(buf + length_in_bar1, &server_data->bar0 + read_start,
395 length_in_bar0);
396 }
397
398 server_data->migration.bytes_transferred += bytes_read;
399
________________________________________________________________________________________________________
*** CID 417159: Possible Control flow issues (DEADCODE)
/lib/libvfio-user.c: 121 in dev_get_caps()
115
116 header = (struct vfio_info_cap_header*)(vfio_reg + 1);
117
118 if (vfu_reg->mmap_areas != NULL) {
119 int i, nr_mmap_areas = vfu_reg->nr_mmap_areas;
120 if (type != NULL) {
CID 417159: Possible Control flow issues (DEADCODE)
Execution cannot reach this statement: "type->header.next = vfio_re...".
121 type->header.next = vfio_reg->cap_offset + sizeof(struct vfio_region_info_cap_type);
122 sparse = (struct vfio_region_info_cap_sparse_mmap*)(type + 1);
123 } else {
124 vfio_reg->cap_offset = sizeof(struct vfio_region_info);
125 sparse = (struct vfio_region_info_cap_sparse_mmap*)header;
126 }
Signed-off-by: William Henderson <william.henderson@nutanix.com>
|
|
This commit adapts the vfio-user protocol specification and the libvfio-user
implementation to v2 of the VFIO live migration interface, as used in the kernel
and QEMU.
The differences between v1 and v2 are discussed in this email thread [1], and we
slightly differ from upstream VFIO v2 in that instead of transferring data over
a new FD, we use the existing UNIX socket with new commands
VFIO_USER_MIG_DATA_READ/WRITE. We also don't yet use P2P states.
The updated spec was submitted to qemu-devel [2].
[1] https://lore.kernel.org/all/20220130160826.32449-9-yishaih@nvidia.com/
[2] https://lore.kernel.org/all/20230718094150.110183-1-william.henderson@nutanix.com/
Signed-off-by: William Henderson <william.henderson@nutanix.com>
|
|
The server sample is supposed to demonstrate dirty page logging, but it was not marking dirty pages. This commit both adds client-side dirty page tracking for pages dirtied with `vfu_sgl_write` and server-side dirty page tracking for pages directly dirtied by the server using `vfu_sgl_get/put`.
Signed-off-by: William Henderson <william.henderson@nutanix.com>
|
|
Signed-off-by: William Henderson <william.henderson@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
The correct DMA address is formed by adding base and offset - the latter
was accidentally missing. Change the server example to read and write
blocks at non-zero offsets, such that `test-client-server.sh` exercises
offset handling.
Signed-off-by: Mattias Nissler <mnissler@rivosinc.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
Reviewed-by: John Levon <john.levon@nutanix.com>
Reported-by: Eduardo Lima <eblima@gmail.com>
|
|
Harmonize and rename the vfu_*sg() APIs to better reflect their functionality:
in our case, there is no mapping happening as part of these calls, they are
merely housekeeping for range splitting, dirty tracking, and so on.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
These functions from openssl are deprecated, and hence break builds with
openssl 3.0, which now has a compiler warning for them. We only use them
to check buffer contents; replace them with CRC code from DPDK instead,
and entirely drop use of openssl.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
To support fuzzing with AFL++, add a "pipe" transport that reads from stdin and
outputs to stdout: this is the most convenient way of doing fuzzing.
Add some docs on how to run a fuzzing session.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Fix three remaining low priority coverity issues; they do not represent bugs.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
Reviewed-by: John Leon <john.levon@nutanix.com>
|
|
Fix a few coverity-identified issues.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
|
|
dirty (#551)
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
Reviewed-by: John Levon <john.levon@nutanix.com>
|
|
The specification states that the region offset given in the region info should
be used as the "offset" when mmap()ing the region from the client side. However,
the library instead implemented a fixed offset scheme similar to that of vfio -
and no clients actually set up the file like that.
Instead, let servers define their own offsets, and pass them through to clients
as is. It's up to the server to decide how its backing file or files is
organized.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
We were accidentally mapping bar0 twice.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
As we are now pure userspace, there is no need for us to use non-standard
integer types. This leaves the copied defines from Linux's vfio.h alone,
however.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
The first in a series excising the use of the "return -errno" idiom. This is a
non-standard usage, and in userspace, we have "errno" for delivering side-band
error values. As there have been multiple bugs from not using standard error
return methods like -1+errno or NULL+errno, let's do that.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Changed variable type for getopt() to fix compiler warning when
compiling on arm
Signed-off-by: mpiszczek <mpiszczek@ddn.com>
Reviewed-by: John Levon <john.levon@nutanix.com>
|
|
vfu_log() and err() should not take newlines.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Give API users an opportunity to clean up when a client disconnects from the
vfio-user socket.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Prefix logs with client/server; run the tests verbosely.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
This fixes a number of issues with how DMA is handled, based on some changes by
Thanos Makatos:
- rename callbacks to register/unregister, as there is not necessarily
any mapping
- provide the (large) page-aligned mapped start and size, the page size used,
as well as the protection flags: some API users need these
- for convenience, provide the virtual address separately that corresponds to
the mapped region
- we should only require a DMA controller to use vfu_addr_to_sg(),
not an unregister callback
- the callbacks should return errno not -errno
- region removal was incorrectly updating the region array
- various other cleanups and clarifications
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
This avoids any issues with multiple definitions when passing CFLAGS in.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
|
|
This is used by SPDK, and it's generally useful. This also uncovered some issues
in the test mocking.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
|
|
This sends a message to a vfio-user client to trigger an IRQ, instead of writing
to an eventfd. However, this isn't necessary on the cases we care about, where
eventfds *are* available. Furthermore, this isn't something an API user should
need to know about: if we ever care, the better way to do this is to make
vfu_irq_trigger() automatically use a message if an eventfd isn't available.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
The most common way we have written this is as "sizeof()"; use this form
consistently.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Regardless of what we do internally, most of our API uses standard mechanisms
for reporting errors. Fix vfu_run_ctx() to do so properly as well, and fix a
couple of other references for user-provided callbacks.
This will require a small fix to SPDK.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
|
|
This patch exposes the fact that live migration is implemented as a
special device region. Hiding this from the user doesn't offer much
benefit since it only takes just a little bit of extra code for the user
to handle it as a region. We do keep the migration callback
functionality since this feature substantially simplifies supporting
live migration from the device implementation's perspective.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
Co-authored-by: John Levon <john.levon@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
When the device is set to stop-and-copy state it must stop operating, which means that if it has been previously programmed to triggers it should refrain from doing so.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Handling data_offset and data_size internally is wrong: we can't simply
assume that the migration data should be appending to the migration
region, devices might have their own requirements.
This also requires a way for the device to return the data_offset, we
do this by making the prepare_data callback applicable in resume state.
Also, allow migration read/write callabcks to return errors.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
* add fake guest thread in migration sample
This patch adds a thread that pretends to be the guest VM (while the
client itself is the VMM) in the live migration sample. Our goal is to
have the fake guest modifying device state while live migration is
ongoing and when live migration has finished the device state should be
correct in the destination server.
Currently this doesn't work because the server blindly applies device
state when resuming, which is wrong because some device state must be
applied to specific offsets. To fix this we have to include the offset
and length (along with BAR1 data) in the migration stream.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
We don't need data_size, it only complicates things.
We don't use migr_data at all.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
This patch adds a simplistic pre-copy phase in the live migration
sample. The end goal is have a separate thread in the client to
modify device state while the device is in the pre-copy phase. This will
be done in later patches.
BAR1 is saved during the pre-copy phase and BAR0 is saved during the
stop-and-copy phase. This is purely for convenience. There are quite a
few assumptions and FIXMEs, even in the client code. We plan to address
them in future patches.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
* Misc fixes for DMA_MAP region prot
1. Validate prot passed in vfu_addr_to_sg()
2. Let user know region prot via vfu_unmap_dma_cb_t
Signed-off-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Split up vfu_pci_setup_config_hdr(): individual "helpers" like vfu_pci_set_id()
are much simpler to use than making the user specify the values in
header-formatted structs; and this way if we want to add additional helpers, we
won't need to modify the existing functions.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
It's easy (with the new vfu_get_private()) to go from a vfu_ctx to the private
pointer, but not the reverse; pass the ctx into all the callbacks.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
This patch returns region capabilities the same way VFIO does: if argsz
is not large enough then it returns only region info and sets argsz to
what it should be in order to fit the capabilities, the client then
retries with a large enough argsz. The protocol specification has been
updated as well.
Plus unit tests.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Make valgrind happy.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
* Drop vfu_ctx_drive() and use vfu_run_ctx()
Renamed vfu_ctx_poll() to vfu_run_ctx().
Updated vfu_run_ctx() to also handle blocking ctx.
Instead of having separate functions for blocking and
non-blocking ctx, better to have one.
This way user can call same set of functions for both cases.
Signed-off-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Misc changes for vfu_ctx_try_attach()
* Rename to vfu_attach_ctx()
* Removed call to vfu_realize_ctx(), should be called separately
* Now vfu_attach_ctx() must also be called for blocking ctx.
Misc changes for vfu_realize_ctx()
* Made calling vfu_realize_ctx() mandatory
* vfu_ctx_drive() and vfu_poll_ctx() returns EINVAL if the device is not
realized.
* Renamed vfu_ctx->ready to vfu_ctx->realized
Added unit test for vfu_attach_ctx() and vfu_realize_ctx()
Signed-off-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
We renamed other code to be "REGION" instead of "REG" so it's less ambiguous. Do
the same for VFU_REG_FLAG_*.
Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Swapnil Ingle <swapnil.ingle@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|
|
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
|