summary refs log tree commit diff
path: root/devices/src/virtio/descriptor_utils.rs
Commit message (Collapse)AuthorAge
* Merge remote-tracking branch 'origin/master'Alyssa Ross2020-06-14
|\
| * sys_util: Refactor IntoIovecChirantan Ekbote2020-05-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The original stated purpose of this trait was to reduce memory allocations but having the `into_iovec` method return a Vec kind of defeats that purpose. Refactor the trait so that it can either convert a T into an iovec or convert a &[T] into a &[iovec]. Implement the trait for VolatileSlice, IoSlice, and IoSliceMut and update all the callers. BUG=none TEST=unit tests Cq-Depend: chromium:2210272 Change-Id: I9d0d617a23030d241d50411f4a5a16e7cba4bcee Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2208527 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-by: Zach Reizner <zachr@chromium.org> Commit-Queue: Chirantan Ekbote <chirantan@chromium.org> Tested-by: Chirantan Ekbote <chirantan@chromium.org>
| * Make VolatileSlice ABI-compatible with iovecChirantan Ekbote2020-05-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change VolatileSlice so that it is ABI-compatible with iovec. This allows us to directly pass in a VolatileSlice for a C function that expects an iovec without having to create temporaries that convert from one to the other. Also change all the parameters from u64 to usize. It's not possible to address more memory than fits into a usize so having u64 here didn't really provide much benefit and led to a lot of tedious casting back and forth all over the place. BUG=none TEST=unit tests Cq-Depend: chromium:2206621 Change-Id: I258f9123c603d9a4c6c5e2d4d10eb4aedf74466d Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2203998 Tested-by: Chirantan Ekbote <chirantan@chromium.org> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
* | Merge remote-tracking branch 'origin/master'Alyssa Ross2020-05-22
|\|
| * descriptor_utils: Remove need for temporary vectorsChirantan Ekbote2020-05-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor the DescriptorChainConsumer, Reader, and Writer structs so that we don't have to allocate a Vec on the heap every time we read or write from a DescriptorChain. This should hopefully give us some small performance improvements as well as simplify the code in some places. Also switch from VolatileSlices to iovecs so that it's easier to use these structs with io_uring. Otherwise we would end up allocating temporary vectors to convert from VolatlieSlice to iovec. BUG=none TEST=unit tests Change-Id: I1657bc76cfff084df825dbbdc8ff414740b71a8f Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2190106 Reviewed-by: Dylan Reid <dgreid@chromium.org> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Dylan Reid <dgreid@chromium.org>
* | Merge remote-tracking branch 'origin/master'Alyssa Ross2020-05-10
|\|
| * descriptor_utils: Add write_iter methodChirantan Ekbote2020-05-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a method to write a series of objects produced by an iterator. Unlike the current `consume` method, this doesn't require the caller to first store the objects in an intermediate collection. Also change `consume` to be implemented using `write_iter`. This is the Writer equivalent of the `iter` and `collect` methods of the Reader struct. BUG=none TEST=unit tests Change-Id: I36bf2fef4d40e13a4741fa55fc35dd556c13a53b Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2172841 Commit-Queue: Chirantan Ekbote <chirantan@chromium.org> Commit-Queue: Dylan Reid <dgreid@chromium.org> Auto-Submit: Chirantan Ekbote <chirantan@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Dylan Reid <dgreid@chromium.org> Reviewed-by: Stephen Barber <smbarber@chromium.org>
* | Merge remote-tracking branch 'origin/master'Alyssa Ross2020-03-26
|\|
| * descriptor_utils: Add `iter` methodChirantan Ekbote2020-03-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add an `iter` method to the `Reader` struct so that callers can iterate over the objects in place rather than having to store them in a separate collection. BUG=none TEST=unit tests Change-Id: I29671910a4244a8d7786ca2eb241416ae72b8c9f Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2093966 Tested-by: Chirantan Ekbote <chirantan@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
* | devices: don't make virtio device Results pubAlyssa Ross2020-03-20
|/ | | | | These would pollute the module, and would override std's Result in modules that imported super::* like virtio_device.
* devices: virtio: Implement Reader::collect() and Writer::consume()Charles William Dick2020-02-19
| | | | | | | | | | | | | | Adds a method Reader::collect() to read a collection of DataInit types, and a method Writer::consume() to write a collection of DataInit types. BUG=b:147334004 TEST=cargo test -p devices Change-Id: Ib5947d30b44b74dc6cf0474e5b87778aad6f08a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2061516 Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org> Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org> Tested-by: Keiichi Watanabe <keiichiw@chromium.org>
* devices: use File rather than shm in testsDaniel Verkamp2019-12-06
| | | | | | | | | | | | | | | | | | | | | | | | | Two virtio descriptor_utils tests were using SharedMemory to stand in for I/o targets with a fixed size; replace these with File to avoid needing the FileReadWriteVolatile impl for SharedMemory, which isn't used anywhere else in the crosvm code base. This slightly changes the behavior under test in the reader_failing_io test, since it was previously using the SharedMemory seal functionality to make the region ungrowable; this is an unusual corner case, and (as mentioned in the comment that was previously at the end of the test) it is testing implementation details of write() on shared memory on Linux. Instead, just use a read-only file so that write() to it will fail and cause the same observable result. BUG=None TEST=./build_test.py Change-Id: I6d62cd70791f1dec625b750ecd01cc51e307f971 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1939783 Reviewed-by: Stephen Barber <smbarber@chromium.org> Reviewed-by: Chirantan Ekbote <chirantan@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: virtio: ensure all block data is transferredDaniel Verkamp2019-11-11
| | | | | | | | | | | | | | | | | | Add _exact/_all variants of the FileReadWriteAtVolatile functions on descriptor Reader/Writer, and use them in the block device to replace the short read/short write error cases. This ensures all data is read/written even if the underlying implementation (in particular, qcow2) does not transfer the full amount of data in one read_vectored_at_volatile/write_vectored_at_volatile call. BUG=chromium:1023422 TEST=`mkfs.btrfs /dev/vdb` with a qcow2 disk Change-Id: Ia37a333947f6f63faf3d4a06cfcc297309d5aff6 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1907443 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Stephen Barber <smbarber@chromium.org>
* Remove duplicated semicolonsDaniel Verkamp2019-11-08
| | | | | | | | | | | | | This will be checked by Rust 1.39.0's rustfmt. BUG=None TEST=bin/fmt --check Change-Id: I8f037207af39f7de1c346365259a10dbe044450b Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1904162 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org>
* devices: add into_iovec() for DescriptorUtilsDaniel Verkamp2019-11-05
| | | | | | | | | | | | | | This allows the conversion of (part of) a descriptor chain into an iovec suitable for use with sys_util functions like send_with_fds(). BUG=None TEST=./build_test.py Change-Id: I4e3f7d9c1175c1173661b0661d3fa15d1da72d1a Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1815417 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org>
* descriptor_utils: Use copy_nonoverlappingChirantan Ekbote2019-10-28
| | | | | | | | | | | | | | | | | | | | Replace the copy_{to,from} calls for VolatileSlice with ptr::copy_nonoverlapping. The copy_{to,from} implementations were doing a volatile read/write per byte, which is significantly slower than just using a memcpy. Using copy_nonoverlapping should be safe here as that's how this was implemented before the refactor. BUG=chromium:1014999 TEST=unit tests Change-Id: Iad29e76056ff3064a5fe7e816b517b4ac75eaaef Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1866894 Commit-Queue: Chirantan Ekbote <chirantan@chromium.org> Tested-by: Chirantan Ekbote <chirantan@chromium.org> Reviewed-by: Stephen Barber <smbarber@chromium.org> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
* descriptor_utils: check for size overflow in new()Daniel Verkamp2019-10-25
| | | | | | | | | | | | | | | | | | | Move the check for length overflow that was in available_bytes() into Reader::new() and Writer::new(). This simplifies callers, since they can assume that once a valid Reader or Writer has been constructed, available_bytes() cannot fail. Since we are walking the descriptor chain during new() anyway, this extra check should be essentially free. BUG=None TEST=cargo test -p devices descriptor_utils Change-Id: Ibeb1defd3728e7b71356650094b0885f3419ed47 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1873142 Reviewed-by: Dylan Reid <dgreid@chromium.org> Reviewed-by: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Stephen Barber <smbarber@chromium.org>
* descriptor_utils: Consume all buffers when reading or writingChirantan Ekbote2019-10-23
| | | | | | | | | | | | | | | | | | | The consume function in both the read and write methods should consume all the VolatileSlices that are given to it rather than just the first one. The previous implementation was not wrong, just inefficient. This should fix that. Also add a test to make sure that this doesn't regress in the future. BUG=none TEST=unit tests Change-Id: I02ec22269cdd6cdc329dd62367b99352a4dc1245 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1865271 Tested-by: Chirantan Ekbote <chirantan@chromium.org> Commit-Queue: Chirantan Ekbote <chirantan@chromium.org> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-by: Stephen Barber <smbarber@chromium.org>
* devices: Refactor DescriptorChainConsumer, Reader, and WriterChirantan Ekbote2019-10-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor the Reader and Writer implementations for DescriptorChains. This has several changes: * Change the DescriptorChainConsumer to keep a VecDeque<VolatileSlice> instead of an iterator. This delegates the fiddly business of sub-slicing chunks of memory to the VolatileSlice implementation. * Read in the entire DescriptorChain once when the Reader or Writer is first constructed. This allows us to validate the DescriptorChain in the beginning rather than having to deal with an invalid DescriptorChain in the middle of the device operating on it. Combined with the check that enforces the ordering of read/write descriptors in a previous change we can be sure that the entire descriptor chain that we have copied in is valid. * Add a new `split_at` method so that we can split the Reader/Writer into multiple pieces, each responsible for reading/writing a separate part of the DescriptorChain. This is particularly useful for implementing zero-copy data transfer as we sometimes need to write the data first and then update an earlier part of the buffer with the number of bytes written. * Stop caching the available bytes in the DescriptorChain. The previous implementation iterated over the remaining descriptors in the chain and then only updated the cached value. If a mis-behaving guest then changed one of the later descriptors, the cached value would no longer be valid. * Check for integer overflow when calculating the number of bytes available in the chain. A guest could fill a chain with five 1GB descriptors and cause an integer overflow on a 32-bit machine. This would previously crash the device process since we compile with integer overflow checks enabled but it would be better to return an error instead. * Clean up the Read/Write impls. Having 2 different functions called `read`, with different behavior is just confusing. Consolidate on the Read/Write traits from `std::io`. * Change the `read_to` and `write_from` functions to be generic over types that implement `FileReadWriteVolatile` since we are not allowed to assume that it's safe to call read or write on something just because it implements `AsRawFd`. Also add `*at` variants that read or write to a particular offset rather than the kernel offset. * Change the callback passed to the `consume` function of `DescriptorChainConsumer` to take a `&[VolatileSlice]` instead. This way we can use the `*vectored` versions of some methods to reduce the number of I/O syscalls we need to make. * Change the `Result` types that are returned. Functions that perform I/O return an `io::Result`. Functions that only work on guest memory return a `guest_memory::Result`. This makes it easier to inter-operate with the functions from `std::io`. * Change some u64/u32 parameters to usize to avoid having to convert back and forth between the two in various places. BUG=b:136128319 TEST=unit tests Change-Id: I15102f7b4035d66b5ce0891df42b656411e8279f Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1757240 Auto-Submit: Chirantan Ekbote <chirantan@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Stephen Barber <smbarber@chromium.org> Reviewed-by: Zach Reizner <zachr@chromium.org> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
* virtio: queue: Enforce DescriptorChain orderingChirantan Ekbote2019-10-06
| | | | | | | | | | | | | | | | | | | The virtio spec requires that all read-only descriptors appear in the chain before any write-only descriptors. Enforce this in the `checked_new` function by adding a new `required_flags` parameter. The `next_descriptor` function will set this to `VIRTQ_DESC_F_WRITE` if the current descriptor is write-only. This ensures that once we see a write-only descriptor, all following descriptors must be write-only. BUG=b:136127316 TEST=none Change-Id: Id8f942a4236a20f62f35439f3648dbec17e14c00 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1757239 Auto-Submit: Chirantan Ekbote <chirantan@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Stephen Barber <smbarber@chromium.org> Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
* virtio: Make Reader interface cloneable.David Riley2019-09-17
| | | | | | | | | | | | | BUG=chromium:993452 TEST=apitrace replay Change-Id: If7dc8ef93d9e6b9783f2f8f124fcee5e016b3eb4 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1775364 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-by: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Tested-by: David Riley <davidriley@chromium.org> Commit-Queue: David Riley <davidriley@chromium.org>
* use `SharedMemory::{named, anon}` to replace `::new`Zach Reizner2019-09-11
| | | | | | | | | | | | | | | | The new constructors are shorter and omit the bare `None` in the `anon` call sites which gave no clues to the reader what the effect of that `None` was. This should improve readability. TEST=./build_test BUG=None Change-Id: I2e34e7df9a4ccc5da50edf4e963a6a42e3d84b22 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1797188 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Commit-Queue: Zach Reizner <zachr@chromium.org> Tested-by: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* Extract disk creation logic out of qcow and src.Cody Schuffelen2019-08-28
| | | | | | | | | Bug: b/133432409 Change-Id: Iba25d5f6bb5f60619bb2f5a3d72ddfd3a81650b4 Signed-off-by: Cody Schuffelen <schuffelen@google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1691460 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: virtio: make create_descriptor_chain pubDaniel Verkamp2019-08-13
| | | | | | | | | | | | | | Allow use of this helper function in other virtio devices that want to write virtio descriptor chains as part of their tests. BUG=chromium:990546 TEST=./build_test Change-Id: Ib986646dc36b6406c88f20950586e1c665adf167 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1732851 Reviewed-by: Stephen Barber <smbarber@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: virtio: add volatile read/write for desc chainsDaniel Verkamp2019-08-13
| | | | | | | | | | | | | | This will allow streaming data between a FileReadWriteVolatile and the descriptor chain Reader/Writer types. BUG=chromium:990546 TEST=./build_test Change-Id: Idc97ce99dd1cc340444298f705df4f12e339095d Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1721370 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org>
* devices: virtio: add seek() for descriptor chainsDaniel Verkamp2019-08-13
| | | | | | | | | | | | | | This allows moving the read/write cursor around within a chain of descriptors through the standard io::Seek interface. BUG=chromium:990546 TEST=./build_test Change-Id: I26ed368d3c7592188241a343dfeb922f3423d935 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1721369 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org>
* devices: virtio: add Error type for descriptorsDaniel Verkamp2019-08-13
| | | | | | | | | | | | | | | | Add an error type to describe descriptor Errors in more detail. This lets us return a more accurate error in a later CL in this chain by adding a VolatileMemoryError variant. BUG=chromium:990546 TEST=./build_test Change-Id: I08680d0cb64bfc3667bac7b2ad8a8bc0e78e8058 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1733988 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org>
* tree-wide: Use new trait object syntaxDylan Reid2019-07-24
| | | | | | | | | | | A few places were using the old syntax without `dyn`. Nightly compilers have started warning more aggressively, so fix up the last of those. Signed-off-by: Dylan Reid <dgreid@chromium.org> Change-Id: I4df49b4a27a62acfd8c542cec903e4c5b31bedcc Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1715576 Reviewed-by: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: remove use of mem::uninitializedDaniel Verkamp2019-07-09
| | | | | | | | | | | | | | | mem::uninitialized is unsafe, and we already replaced most instances of it with alternate implementations; however, another one slipped in since then. Replace it with Default::default() as a safe alterantive. BUG=None TEST=./build_test Change-Id: Idacdcb0ebe197cc93fba4b15c3dda774bb56e73e Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1691233 Reviewed-by: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* crosvm: Implement Reader/Writer interface over DescriptorChain.Jakub Staron2019-06-21
This change adds a convinient interface over DescriptorChain. It hides the complexity of DescriptorChain and allows to treat it as a pair of read-only and write-only buffers. In the future, it will also allow to easily support indirect descriptors. BUG=chromium:966258 TEST=cargo test --package devices descriptor_utils TEST=run crosvm without sandbox, share a directory, compare checksum of shared file between host and guest Change-Id: I9fb722ee2024c8d7d40f560571ec7d7c454bfc2b Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1647370 Reviewed-by: Zach Reizner <zachr@chromium.org> Reviewed-by: Stephen Barber <smbarber@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Jakub StaroĊ„ <jstaron@google.com>
generated by cgit-pink 1.4.1 (git 2.36.1) at 2024-06-05 04:46:20 +0000