summary refs log tree commit diff
path: root/devices/src/virtio/block.rs
Commit message (Collapse)AuthorAge
* Make lots of things DebugAlyssa Ross2020-07-02
|
* devices: return Vec from queue_max_sizesAlyssa Ross2020-06-15
|
* crosvm: fix deadlock on early VmRequestAlyssa Ross2020-06-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a DiskCommand was received on the crosvm socket before the virtio-block device was activated, the Token::VmRequest case in the main event loop would forward the request to the block device socket, and then wait syncronously for a response. That response would never come because the device hadn't been activated, and it would never be activated because the event loop would never continue, and therefore never be able to respond to the event that causes the device to be activated. crosvm would therefore just hang forever, waiting for a response that would never come. This patch fixes this deadlock by keeping track of whether devices that send a response in this way have been activated yet. If they have already been activated, messages are sent and responses are received as normal. If they have not been activated, messages are instead put into a per-device queue. Once the device is activated, queued messages are processed all at once, and then the device is marked as ready, and the queue is dropped. Future messages are processed immediately as they come in, with no further queueing. A device indicates that it is ready by sending a message on its socket. The main crosvm event loop can then poll the socket, to be notified when the device is ready. This poll event will only trigger once -- once it has been received, it is removed from the poll context. Currently, the only device type that responds to external control messages AND needs to be activated by an event is the block device. The balloon device does not respond to messages, and the xhci controller device is activated up front. The code is nevertheless structured so that it should be very easy to drop another kind of device in to the queuing system, should that be required. Message-Id: <20200614114344.22642-3-hi@alyssa.is> Notes: Reviewed-by: Cole Helbling <cole.e.helbling@outlook.com>
* 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>
* Virtio: Add blk VIRTIO_RING_F_EVENT_IDX featureXiong Zhang2020-03-05
| | | | | | | | | | | | | | | | | | | | | | Previous interrupt suppress patch only supply crude interrupt suppress, VIRTIO_RING_F_EVENT_IDX feature supply a more performant alternative: 1) where the driver specifies how far the device can progress before a notification is required 2) where the device specifies how far the driver can progress before a interrrupt is required. This patch add this feature into blk. For gpu and network, this could be added also, but gpu and network performance don't get better. BUG=None TEST=run benchmark for blk in guest Change-Id: I73fe3f8b72a9e88fd6073890bc6ab2bee891d51d Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2008341 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Xiong Zhang <xiong.y.zhang@intel.corp-partner.google.com>
* devices: block: let resize convert to non-sparseDaniel Verkamp2020-02-27
| | | | | | | | | | | | | | | | | | | | | Change the behavior of the resize operation on virtio-block devices so that it causes a disk to become fully allocated (non-sparse) even if it had previously been sparse. This means that we could have a disk that was previously sparse and is now non-sparse, so treat discard requests for sparse disks as a no-op instead of an error. This is acceptable since discard is a hint and doing nothing is a valid implementation. BUG=chromium:858815 TEST=`crosvm disk resize` a sparse disk Change-Id: I8b79e460e5432cc71bed98172527fe1cd2e726ed Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2070846 Reviewed-by: David Munro <davidmunro@google.com> Reviewed-by: Dylan Reid <dgreid@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
* Virtio: Add virtio block irq suppressXiong Zhang2020-02-19
| | | | | | | | | | | | | | | The flag in avail descriptor supplies irq suppress, it could reduce irq injection from device, so many redundant interrupts could be removed from guest, then improve guest performance. BUG=None TEST=run fio read and fio write in guest Change-Id: I68789d8ca24d0e84d0b446db65057f4da2fac56f Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.corp-partner.google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2008339 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
* virtio: Inject virtio-blk interrupt quicklyXiong Zhang2020-02-19
| | | | | | | | | | | | | | | | | | | Current blk interrupt is injected into guest after device handle a batch of requests. While this patch injects interrupt at the end of each request. So guest block will get much more interrupts and could handle request more quickly. With this patch, the guest fio read test improves 13%, while fio write doesn't get better. BUG=none TEST=run fio_read and fio_write in guest Change-Id: Ib0bd23e624dfc5d940d6cc124468b898d8ba128e Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.corp-partner.google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2008338 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: virtio: block: keep disk allocated on resizeDaniel Verkamp2020-01-24
| | | | | | | | | | | | | | | | When a non-sparse disk is resized, we should allocate storage for the newly-expanded space when the disk is grown to maintain the non-sparseness. To accomplish this, add a call to allocate in the resize function in the block device. BUG=chromium:858815 TEST=`crosvm disk resize ...` and verify disk image is fully allocated Change-Id: If263aa2b5c9da11b8bfc0586e4ac1575f2bd7084 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2015829 Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Daniel Verkamp <dverkamp@chromium.org> Reviewed-by: Dylan Reid <dgreid@chromium.org>
* virtio: resolve some new introduced clippy warningsChuanxiao Dong2020-01-10
| | | | | | | | | | | | BUG=None TEST=./bin/clippy TEST=cargo test -p devices Change-Id: Ic5183ef887a85bc14357fd29bc7ea70caded61a8 Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.corp-partner.google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1988704 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* virtio: implement reset for balloon/rng/blk/netChuanxiao Dong2020-01-03
| | | | | | | | | | | | | | | | | | | | The reset method will be called when guest virtio driver is resetting the device. Currently the balloon/Rng/block/net virtio drivers will re-configure the virt queue during the reset so they required to be re-activated for using the new virt queue configurations. To support this, need these device models to return back the moved ownership of the important variables so that they can do the re-activate. BUG=chromium:1030609 TEST=Launch linux guest and follow the reproduce steps in BUG#1030609 to check if balloon/Rng/block/net driver still complain failure. Change-Id: I5b40fd303ea334484c590982e3e0874ea4e854ee Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.corp-partner.google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1971097 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: block: add block_size option for disksDaniel Verkamp2019-12-10
| | | | | | | | | | | | | | | This allows overriding the default logical block size (512 bytes) with other values, such as 4096 for 4K block size disks. BUG=chromium:942700 TEST=crosvm run -r vm_rootfs,block_size=4096 vm_kernel TEST=verify block size with lsblk --output-all Change-Id: Ia6db05f369a76557a2afb8b48b5cc2b66cf84b01 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1954220 Reviewed-by: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
* devices: virtio: enable MSI-X for all devicesDaniel Verkamp2019-12-06
| | | | | | | | | | | | | | | | | | All virtio devices can use the same generic calculation for number of MSI-X vectors required: number of queues plus one for configuration changes. Move this calculation to the VirtioPciDevice implementation and remove the Option to unconditionally enable MSI-X support for all PCI virtio devices. BUG=chromium:854765 TEST=Verify all virtio interrupts in /proc/interrupts are PCI-MSI Change-Id: I5905ab52840e7617b0b342ec6ca3f75dccd16e4d Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1925169 Reviewed-by: Zide Chen <zide.chen@intel.com> Reviewed-by: Dylan Reid <dgreid@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* disk: add get_len() to eliminate need for SeekDaniel Verkamp2019-11-27
| | | | | | | | | | | | | | | This new trait allows DiskFile implementors to provide the length of the file directly rather than using SeekFrom::End with seek(). BUG=None TEST=./build_test TEST=Boot Termina in crosvm Change-Id: I9447ebb43dbd5fbb32a3a6b6d2fc969b9406cdbc Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1913961 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Dylan Reid <dgreid@chromium.org>
* disk: switch from WriteZeroes to WriteZeroesAtDaniel Verkamp2019-11-27
| | | | | | | | | | | | | | | | This eliminates an extra seek per guest write zeroes request. Additionally, it allows us to stop depending on the file cursor and pass the offset directly, making multi-queue implementation easier. BUG=chromium:858815 TEST=Boot Termina in crosvm Change-Id: I8b15a39752a1b68597a2b1e1fd72382a484a3cb2 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1913521 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Dylan Reid <dgreid@chromium.org>
* devices: block: add option to control sparsenessDaniel Verkamp2019-11-18
| | | | | | | | | | | | | | | | | | Extend the --disk option and other related options to allow a particular disk to have the sparse operations (virtio-blk's discard command) enabled or disabled. By default, the sparse flag will be enabled for virtio-blk devices, matching current behavior. BUG=chromium:858815 TEST=Run `crosvm with --rwdisk file.img,sparse=false` and try to discard Change-Id: Ib72c949711fbe869a3f444d7f929a80d0e039f72 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1906750 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org>
* Revert "devices: virtio: disable MSI-X for block and net"Daniel Verkamp2019-11-18
| | | | | | | | | | | | | | | | | Re-enable MSI-X for virtio-blk and virtio-net now that the underlying issue causing hangs at startup has been fixed (CL:1917495). BUG=chromium:1019986 TEST=Boot Termina on nami This reverts commit 85858f580eada8dd85c8c798ef3e98f18d92dc1e. Change-Id: I5a5e197243a16aee2b2aaf3145a1180749b097b2 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1918261 Reviewed-by: Zide Chen <zide.chen@intel.com> Reviewed-by: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: virtio: pass Interrupt to activate()Daniel Verkamp2019-11-18
| | | | | | | | | | | | | | Factor out the common creation of struct Interrupt. No functional change. BUG=chromium:854765 TEST=./build_test Change-Id: Idf8804771ba1af5181818f643e15e1b42918258a Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1898268 Tested-by: kokoro <noreply+kokoro@google.com>
* devices: virtio: block: refactor status_writer setupDaniel Verkamp2019-11-18
| | | | | | | | | | | | | | | | | | | | | | | This consolidates the status byte manipulation in process_one_request() instead of requiring both that function and execute_request() to deal with it. The tests are modified to run the full process_one_request() function instead of just execute_request() to exercise the full descriptor parsing logic, and they are adapted to read the status of the request from the status byte in the buffer from the descriptor since process_one_request() returns successfully as long as the descriptor parsing succeeded, even if the requested I/O failed. BUG=None TEST=./build_test Change-Id: I17affabc2d3c30c810643ce260152cf34893b772 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1918479 Reviewed-by: Dylan Reid <dgreid@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>
* devices: virtio: block: advertise seg_maxDaniel Verkamp2019-11-08
| | | | | | | | | | | | | | | | | | | | | | The virtio-blk configuration space has a `seg_max` field that lets the device inform the driver of the maximum number of segments allowed within a single request. The Linux virtio block driver assumes that if the corresponding feature (VIRTIO_BLK_F_SEG_MAX) is not advertised, then only one segment can be used. Add a segment limit based on sysconf(_SC_IOV_MAX) to allow the Linux block stack to make use of multiple segments in a single request, which will get translated into a single readv/writev call in the crosvm block device. BUG=None TEST=strace crosvm virtio-blk process and note preadv with iov_cnt > 1 Change-Id: Ia14ebebb85daa21e2d43437bb74886f32e6e8187 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1876806 Reviewed-by: Stephen Barber <smbarber@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: virtio: disable MSI-X for block and netDaniel Verkamp2019-10-31
| | | | | | | | | | | | | | | | | | | | | | | | | Temporarily turn off MSI-X support in the block and net devices since it seems this is responsible for some test flakiness that manifests as timeouts/hangs in the ProxyDevice read handler, e.g.: [devices/src/proxy.rs:238] failed read from child device process virtio-pci (virtio-block): failed to receive request or response: Resource temporarily unavailable (os error 11) This is a minimally-invasive change to disable MSI-X without a full revert of the relevant patches by just changing the relevant devices so that they no longer request MSI-X vectors. BUG=chromium:1019986 BUG=chromium:854765 TEST=check /proc/interrupts inside crosvm does not contain "PCI-MSI" Change-Id: Ib37b503e609e2b9e22265370bcfe5804f04057ef Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1891643 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org> Reviewed-by: Zide Chen <zide.chen@intel.corp-partner.google.com>
* devices: virtio: block: use FileReadWriteAtVolatileDaniel Verkamp2019-10-29
| | | | | | | | | | | | | | | | | | | Use the "at" variants of the read/write functions in the block device. This reduces the number of syscalls on the host per I/O to one (pread64/pwrite64) rather than two (lseek + read/write). The CompositeDiskFile implementation is also updated in this commit, since it's both a producer and consumer of DiskFile, and it isn't trivial to update it in a separate commit without breaking compilation. BUG=None TEST=Start Crostini on kevin, banon, and nami Change-Id: I031e7e87cd6c99504db8c56b1725ea51c1e27a53 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1845948 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Stephen Barber <smbarber@chromium.org>
* devices: implement dedicated Interrupt struct for virtio WorkerZide Chen2019-10-25
| | | | | | | | | | | | | | | | | | | | | | The code to inject interrupt to the guest can be generic to all virtio devices. This patch: - move those guest interrupt related fields out of Worker structure and put in a separate file, making the worker code cleaner. - remove redandant functions across virtio devices: signal_used_queue(), signal_config_changed(), etc. BUG=chromium:854765 TEST=sanity test on eve and Linux TEST=cargo test -p devices Change-Id: I8e9f760f2057f192fdc74d16a59fea2e6b08c194 Signed-off-by: Zide Chen <zide.chen@intel.corp-partner.google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1869553 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
* devices: enable MSI-X for virtio-net and viotio-block devicesXiong Zhang2019-10-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - signal_used_queue(): trigger MSI-X interrupts to the guest if MSI-X is enabled, otherwise trigger INTx interrupts - enable MSI-X on vhost-net: allocate one vhost_interrupt for every MSI-X vector. Performance wise, fio random R/W test on eve pixelbook: INTx MSI-X delta fio write 8.13MiB/s 9.79MiB/s +1.66MiB/s (+20%) fio read 24.35MiB/s 29.3MiB/s +4.95MiB/s (+20%) For networking performance (TCP stream), test results on eve pixelbook: INTx MSI-X delta iperf3 5.93Gbits/s 6.57Gbits/s +0.64Gbits/s (+10.7%) iperf3 -R 5.68Gbits/s 7.37Gbits/s +1.30Gbits/s (+22.8%) iperf test results on VM launched from Ubuntu host (client sends only): INTx MSI-X delta virtio-net 9.53Gbits/s 11.4 Gbits/s +1.87Gbits/s (+19.5%) vhost 28.34Gbits/s 44.43Gbits/s +16.09Gbits/s (+56.7%) BUG=chromium:854765 TEST=cargo test -p devices TEST=tested virtio-net and block on Linux VM and eve pixelbook Change-Id: Ic4952a094327e6b977f446def8209ea2f796878c Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.corp-partner.google.com> Signed-off-by: Zide Chen <zide.chen@intel.corp-partner.google.com> Signed-off-by: Sainath Grandhi <sainath.grandhi@intel.corp-partner.google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1828340 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: 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>
* devices: implement MsixConfig struct and generic MSI-X functionsZide Chen2019-10-24
| | | | | | | | | | | | | | | | | | | | The MsixConfig struct is responsible for all the operations of MSI-X Capability Structure and MSI-X Table. A msix_config object is created for each virtio device. BUG=chromium:854765 TEST=cargo test -p devices Change-Id: Ide7c34d335d49a201f20b0a4307bcda97d1d61b7 Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com> Signed-off-by: Xiong Zhang <xiong.y.zhang@intel.corp-partner.google.com> Signed-off-by: Zide Chen <zide.chen@intel.corp-partner.google.com> Signed-off-by: Sainath Grandhi <sainath.grandhi@intel.corp-partner.google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1828337 Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Tested-by: Daniel Verkamp <dverkamp@chromium.org> Commit-Queue: Stephen Barber <smbarber@chromium.org>
* Add explicit `dyn` for trait objectsDaniel Verkamp2019-10-17
| | | | | | | | | | | | | | | Fix "trait objects without an explicit `dyn` are deprecated" warnings introduced in Rust 1.38. BUG=None TEST=emerge-nami crosvm Change-Id: I8ca6aa747475268ae898adddd5d091d401326ceb Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1862999 Reviewed-by: Dylan Reid <dgreid@chromium.org> Reviewed-by: Stephen Barber <smbarber@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* 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>
* sys_util: add write_zeroes_all() functionDaniel Verkamp2019-09-25
| | | | | | | | | | | | | | | | | | In the same spirit as write_all() for the standard io::Write::write() function, add a write_zeroes_all() function with a default implementation that calls write_zeroes() in a loop until the requested length is met. This will allow write_zeroes implementations that don't necessarily fulfill the entire requested length. BUG=None TEST=cargo test -p sys_util write_zeroes Change-Id: I0fc3a4b3fe8904946e253ab8a2687555b12657be Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1811466 Reviewed-by: Zach Reizner <zachr@chromium.org> Reviewed-by: Cody Schuffelen <schuffelen@google.com> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: join worker threads in drop()Daniel Verkamp2019-09-17
| | | | | | | | | | | | | | Make sure all devices join any threads they spawn before returning from the drop() handler after signaling the exit event. BUG=chromium:992494 TEST=crosvm exits without errors Change-Id: I6bc91c32a08f568b041765044caa9aff6f7cf4a9 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1802156 Reviewed-by: Stephen Barber <smbarber@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* Replace "AsRawFd" with "AsRawFds" for disks.Cody Schuffelen2019-08-30
| | | | | | | | | | | | This supports virtio disks that depend on multiple file descriptors. All of the file descriptors are passed to the jail when relevant. Bug: b/133432409 Change-Id: Idf2e24cd2984c0d12a47a523c13d24c1ba8d173e Signed-off-by: Cody Schuffelen <schuffelen@google.com> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1691761 Reviewed-by: Daniel Verkamp <dverkamp@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: add copy_config() helper functionDaniel Verkamp2019-08-16
| | | | | | | | | | | | | | | | | | | | | | | Add a new virtio configuration copying function to replace all of the slightly varying read_config() and write_config() implementations in our virtio devices. This replaces a lot of tricky bounds-checking code with a single central implementation, simplifying the devices to a single call to copy_config() in most cases. The balloon device is also changed to represent its config space as a DataInit struct to match most other devices and remove several unwrap() calls. BUG=None TEST=./build_test TEST=Boot vm_kernel+vm_rootfs in crosvm TEST=Start Crostini on nami Change-Id: Ia49bd6dbe609d17455b9562086bc0b24f327be3f Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1749562 Reviewed-by: Dylan Reid <dgreid@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com>
* devices: virtio: block: use descriptor chain utilsDaniel Verkamp2019-08-13
| | | | | | | | | | | | | | | | | | | Rewrite the virtio block device to use the descriptor Reader/Writer interfaces - this greatly simplifes the block device code. This also lets the block device handle arbitrary descriptor layouts, since the descriptor reader/writer handles that transparently for us. BUG=chromium:990546 TEST=./build_test TEST=Boot crosvm with vm_kernel+vm_rootfs on workstation TEST=Boot full Crostini environment on nami Change-Id: Ie9a2ba70a6c7ed0ae731660fd991fb88242e275f Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1721371 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Stephen Barber <smbarber@chromium.org>
* tree-wide: use PollContext::build_with where possibleZach Reizner2019-07-24
| | | | | | | | | | | | | | | | | | The old method of creating a PollContext and calling `add` inside of `and_then` chains was an ugly way handle the Results that can crop up after each call. The `build_with` function is equivalent but operates on a slice which has way less boilerplate. TEST=./build_test BUG=None Change-Id: I8b0d6532680e04c501187397bd211014a2363c25 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1715581 Tested-by: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Auto-Submit: Zach Reizner <zachr@chromium.org> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org> Commit-Queue: Zach Reizner <zachr@chromium.org>
* tempfile: Unify the two tempdir implementationsDavid Tolnay2019-07-11
| | | | | | | | | | | | | | | | | | | | | | Looks like we ended up with two totally different tempdir implementations: one from CL:520706 and the other from CL:1409705. This CL consolidates them into one implementation. BUG=chromium:974059 TEST=tempfile: cargo test TEST=crosvm: cargo check --all-features TEST=devices: cargo check --tests TEST=sys_util: cargo check --tests TEST=local kokoro TEST=./build_test Cq-Depend: chromium:1574668 Change-Id: Id70e963c9986ed2fc5f160819c4a7f9f16092b3b Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1573227 Tested-by: kokoro <noreply+kokoro@google.com> Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
* eliminate mut from non-mut referencesZach Reizner2019-06-04
| | | | | | | | | | | | | | | | | | | | | | | This manifested itself in a couple places that were turning shared memory buffers into slices for the purposes of passing these slices to `Read` and `Write` trait methods. However, this required the removal of the methods that took `Read` and `Write` instances. This was a convenient interface but impossible to implement safely because making slices from raw pointers without enforcing safety guarantees causes undefined behaviour in Rust. It turns out lots of code in crosvm was using these interfaces indirectly, which explains why this CL touches so much. TEST=crosvm run BUG=chromium:938767 Change-Id: I4ff40c98da6ed08a4a42f4c31f0717f81b1c5863 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1636685 Reviewed-by: Zach Reizner <zachr@chromium.org> Tested-by: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Commit-Queue: Zach Reizner <zachr@chromium.org>
* devices: block: issue fsync when FlushTimer expiresDaniel Verkamp2019-05-29
| | | | | | | | | | | | | | | | | | The RequestType::Flush handler correctly uses fsync(), which issues an fsync to the underlying disk image file. However, the flush timer (started on write and cancelled if a flush request is executed) was only calling flush(), which is insufficient when the disk image is a raw file - it just flushes in-memory buffers and does not issue an fsync. BUG=None TEST=Issue writes in crosvm; verify fsync in strace output Change-Id: I1de8a35615031b5fdf5599dd6b49015d0b245c31 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1632876 Tested-by: kokoro <noreply+kokoro@google.com> Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org> Reviewed-by: Dylan Reid <dgreid@chromium.org>
* crosvm: Removes used_desc_heads arrays from various virtio devices.Jakub Staron2019-05-23
| | | | | | | | | | | | BUG=None TEST=tast run ${IP} vm.CrostiniStartEverything Change-Id: I14bdc330bff23ef3397251a81bdf63e37c1e1dfd Reviewed-on: https://chromium-review.googlesource.com/1611014 Commit-Ready: Jakub Staroń <jstaron@google.com> Tested-by: kokoro <noreply+kokoro@google.com> Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org> Reviewed-by: Zach Reizner <zachr@chromium.org>
* devices: block: clarify read/write error messagesDaniel Verkamp2019-04-22
| | | | | | | | | | | | | | | Fix up the Display impl for ExecuteError so that it's clear which direction data is moving for the Read and Write variants. BUG=None TEST=cargo build Change-Id: Ide4ea5cb453e4d7f6bd2812a1696df96daec511b Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1574963 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org>
* Extracts DiskResize from VmRequest to a new type.Jakub Staron2019-04-19
| | | | | | | | | | | | | | | BUG=None TEST=cargo test TEST=cargo test --package msg_socket TEST=cargo test --package devices TEST=cargo test --package vm_control TEST=tast -verbose run ${IP} vm.CrostiniStartEverything Change-Id: Icf26f53d3fd813ab43b8f14079f90628d245eed7 Reviewed-on: https://chromium-review.googlesource.com/1565297 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
* edition: Eliminate ref keywordDavid Tolnay2019-04-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As described in: https://doc.rust-lang.org/edition-guide/rust-2018/ownership-and-lifetimes/default-match-bindings.html which also covers the new mental model that the Rust Book will use for teaching binding modes and has been found to be more friendly for both beginners and experienced users. Before: match *opt { Some(ref v) => ..., None => ..., } After: match opt { Some(v) => ..., None => ..., } TEST=cargo check --all-features TEST=local kokoro Change-Id: I3c5800a9be36aaf5d3290ae3bd3116f699cb00b7 Reviewed-on: https://chromium-review.googlesource.com/1566669 Commit-Ready: David Tolnay <dtolnay@chromium.org> Tested-by: David Tolnay <dtolnay@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
* clippy: Resolve unneeded_field_patternDavid Tolnay2019-04-17
| | | | | | | | | | | TEST=bin/clippy Change-Id: Ia0e0163441fafd4ce44fef7ebaa18d1cc947e20e Reviewed-on: https://chromium-review.googlesource.com/1566891 Commit-Ready: David Tolnay <dtolnay@chromium.org> Tested-by: David Tolnay <dtolnay@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
* edition: Fill in macro importsDavid Tolnay2019-04-15
| | | | | | | | | | | | | | | | | | | | Macros were previously imported through `#[macro_use] extern crate`, which is basically a glob import of all macros from the crate. As of 2018 edition of Rust, `extern crate` is no longer required and macros are imported individually like any other item from a dependency. This CL fills in all the appropriate macro imports that will allow us to remove our use of `extern crate` in a subsequent CL. TEST=cargo check --all-features --tests TEST=kokoro Change-Id: If2ec08b06b743abf5f62677c6a9927c3d5d90a54 Reviewed-on: https://chromium-review.googlesource.com/1565546 Commit-Ready: David Tolnay <dtolnay@chromium.org> Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: David Tolnay <dtolnay@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: David Tolnay <dtolnay@chromium.org>
* Wrap the UnixSeqpacket with a more descriptive type.Jakub Staron2019-04-10
| | | | | | | | | | | | | | | | Host/device sockets are now created as a pairs of MsgSockets instead of UnixSeqpacket sockets. BUG=chromium:950663 TEST=cargo check TEST=cargo test Change-Id: I8f61a711fe3c2547bf5d18fcfa23bfd0dc0ef5fd Reviewed-on: https://chromium-review.googlesource.com/1559041 Commit-Ready: Jakub Staroń <jstaron@google.com> Tested-by: kokoro <noreply+kokoro@google.com> Tested-by: Jakub Staroń <jstaron@google.com> Reviewed-by: Stephen Barber <smbarber@chromium.org> Reviewed-by: Zach Reizner <zachr@chromium.org>
* devices: block: report block size in configDaniel Verkamp2019-03-28
| | | | | | | | | | | | | | | | | Set up the infrastructure for reporting block size in the virtio-blk device model. For now, we'll keep reporting SECTOR_SIZE (512), which is the default block size that is assumed if we don't report it. This prepares us to easily switch the reported block size in the future. BUG=chromium:942700 TEST=Boot Crostini on nami with an existing VM and container Change-Id: I983817743c40e8278fe6cb9a10498011a8887ec9 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1526334 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Zach Reizner <zachr@chromium.org>
* crosvm: use seqpacket rather than datagram socketsZach Reizner2019-02-28
| | | | | | | | | | | | | | | | | | | | The advantage of seqpacket is that they are connection oriented. A listener can be created that accepts new connections, useful for the path based VM control sockets. Previously, the only bidirectional sockets in crosvm were either stream based or made using socketpair. This change also whitelists sendmsg and recvmsg for the common device policy. TEST=cargo test BUG=chromium:848187 Change-Id: I83fd46f54bce105a7730632cd013b5e7047db22b Reviewed-on: https://chromium-review.googlesource.com/1470917 Commit-Ready: Zach Reizner <zachr@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Tested-by: Zach Reizner <zachr@chromium.org> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
* error: Print errors using Display implDavid Tolnay2019-02-20
| | | | | | | | | | | | | | | | | | | | | | | | | I have been running into Debug-printed error messages too often and needing to look up in the source code each level of nested errors to find out from the comment on the error variant what the short name of the variant means in human terms. Worse, many errors (like the one shown below) already had error strings written but were being printed from the calling code in the less helpful Debug representation anyway. Before: [ERROR:src/main.rs:705] The architecture failed to build the vm: NoVarEmpty After: [ERROR:src/main.rs:705] The architecture failed to build the vm: /var/empty doesn't exist, can't jail devices. TEST=cargo check --all-features TEST=FEATURES=test emerge-amd64-generic crosvm Change-Id: I77122c7d6861b2d610de2fff718896918ab21e10 Reviewed-on: https://chromium-review.googlesource.com/1469225 Commit-Ready: David Tolnay <dtolnay@chromium.org> Tested-by: David Tolnay <dtolnay@chromium.org> Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
* devices: block: add bounds checksDaniel Verkamp2019-01-31
| | | | | | | | | | | | | | | | | | | | | | | As reported by the Firecracker team, the block device model doesn't check if an I/O request starts before the end of the disk but extends beyond it. For writes to disks backed by raw files, this could end up unintentionally extending the size of the disk. Add bounds checks to the request execution path to catch these out-of-bounds I/Os and fail them. While we're here, fix a few other minor issues: only seek for read and write requests (the 'sector' field of the request should be ignored for flush, write zeroes, and discard), and check for overflow when performing the shifts to convert from sectors to bytes. BUG=chromium:927393 TEST=cargo test -p devices block Change-Id: I0dd19299d03a4f0716093091f173a5c507529963 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1448852 Tested-by: kokoro <noreply+kokoro@google.com> Reviewed-by: Dylan Reid <dgreid@chromium.org>
generated by cgit-pink 1.4.1 (git 2.36.1) at 2024-05-27 07:26:00 +0000