Commit graph

185 commits

Author SHA1 Message Date
Daniel Verkamp
ad213f44e0 cros_async: port some uring_source tests to io_source
This allows the tests to run on all executor kinds supported by the
generic io_source test harness (io_uring, epoll, and Windows handle).

readmulti is modified to write different data to each region so the test
can verify the correct part of the file is actually read.

fsync is removed since it is already covered by io_source's fsync test.

readwrite_current_file_position is split into two tests that validate
the async io_source behavior using normal non-async File operations
(read_current_file_position and write_current_file_position).

BUG=b:288967124
TEST=tools/dev_container tools/presubmit

Change-Id: I59ce8dd3934135499062fed4cb7e0f54bf446d09
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4655444
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
2023-07-10 23:46:14 +00:00
David Stevens
5b031ce8b0 base: Introduce TimerTrait
Introduce a TimerTrait that is implemented by both Timer and FakeTimer.
This trait can be used to write testable code without needing to
condition use statements on cfg(test).

BUG=None
TEST=CQ

Change-Id: I874ffe19fafe73e5c17fd5bb689e5b571e9f5b12
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4664188
Reviewed-by: Junichi Uekawa <uekawa@chromium.org>
Commit-Queue: David Stevens <stevensd@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2023-07-05 01:12:35 +00:00
Daniel Verkamp
024a5e44f2 cros_async: add MemRegion iterator
MemRegionIter allows iteration over a &[MemRegion] list, with additional
helper functions to skip bytes and truncate the list.

In particular, this is useful for the virtio DescriptorChain utilities
(Reader/Writer) that previously needed to call collect() to allocate a
new copy of the regions list when it was necessary to limit the length
or skip part of the beginning of the chain.

This also replaces the functionality of MemRegion::truncate(), which
used Cow<[MemRegion]>, meaning it would sometimes need to allocate and
copy. The new MemRegionIter implementation never needs to copy the list
of regions; it just maintains a couple of internal counters and adjusts
the returned MemRegions on the fly.

BUG=None
TEST=tools/dev_container tools/presubmit
TEST=cargo test -p cros_async

Change-Id: Ie66c7a8c4061a166c986c6d6937e21549ddb205e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4559367
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-06-28 19:45:38 +00:00
Daniel Verkamp
f1885ff06b cros_async: uring_executor: create iovecs in one pass
The previous method required calling `get_volatile_slice()` twice for
each memory region - once to validate that each region is valid, and
again to convert the MemRegion into an IoBufMut. It also relied on
`get_volatile_slice()` returning the same non-Err value twice to avoid a
panic in the `unwrap()` used in the second pass.

Instead, we can iterate over the regions once and convert the iterator
directly into a pinned boxed slice.

BUG=None
TEST=tools/dev_container tools/presubmit
TEST=cargo test -p cros_async

Change-Id: I16e0bd38998b97e072a2d0fa5a72a2dea2d082f9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4646722
Reviewed-by: Frederick Mayle <fmayle@google.com>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-06-28 19:45:38 +00:00
Frederick Mayle
ee82228475 cros_async: add TaskHandle docs and TaskHandle::cancel method
Change-Id: Ib235ac810bb70a0027b9ff431b76b1c5fe14ada2
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4567121
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Richard Zhang <rizhang@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-05-31 00:47:23 +00:00
Dennis Kempin
4cd4043fd1 Upgrade to 1.68.2: Fix clippy nits
The new rust toolchain comes with improved clippy warnings that
catch more unnecessary casts than before.
Some cases have to be whitelisted since libc types have varying size
based on the architecture we are compiling for.

BUG=None
TEST=presubmit

Change-Id: Ia473b0bfe6e74bdb7b0d4ca15d2c9fb410fc353e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4568144
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-05-25 18:28:14 +00:00
Vikram Auradkar
f054575a8b clippy: Disallow len_without_is_empty and len_zero
BUG=b:283512997
TEST=none

Change-Id: I0477583b3f8c8eefeb1d79f11302b1b28a8bd770
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4549999
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Vikram Auradkar <auradkar@google.com>
2023-05-22 21:27:21 +00:00
Takaya Saeki
293bdf3f24 cros_async: Rename Mutex to RwLock
Currently, cros_async::sync::Mutex is named "Mutex", but it is actually
a read-write lock, which is confusing to readers. As a result, readers
often mistakenly think that the part of the code which should use a
read-write lock is using a Mutex.

This change renames Mutex to RwLock to eliminate the confusion. In
addition, this change updates the documentation to recommend developers
to use the standard `futures:🔒:Mutex` if they just need a Mutex,
not a read-write lock.

BUG=None
TEST=./tools/dev_container ./tools/presubmit

Change-Id: Ic008d2448308066d9c5cabb0b6c499a62994e5e2
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4546967
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
2023-05-22 07:01:38 +00:00
Frederick Mayle
cb450f4996 cros_async: factor out common executor code
fd_executor, uring_executor, and handle_executor had a lot of duplicate
code. A new trait `Reactor` is introduced to abstract away the
differences between the IO backends. There is now a single
`RawExecutor`, which is generic over `Reactor`s.

No behavior changes are intended, with some small exceptions:

* A new error variant is used in some cases, e.g. instead of
  `Error::PollExecutor` an error might be `Error::Io` now. Nothing seems
  to inspect these errors, so it is unlikely to cause an issue.
* There is an extra alloc when the executor is dropped (for the boxed
  future).
* The Unix backends had an `AtomicI32` state field but the Windows
  backend did not. Now Windows does as well. This might save a syscall
  here and there.

Change-Id: Ieeff9d9888ed082cf3e0213e586c55c9d8a1f8bf
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4331554
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-05-18 17:40:14 +00:00
Frederick Mayle
b5a2e2065e cros_async: fix waker implementation for epoll backend
I broke this in https://crrev.com/c/4331553 because I didn't notice the
`mem::replace` call. The first wake would switch the special wake event
op status to "complete" and then a second wake would panic because
nothing should complete twice.

Bug: 282757779
Change-Id: Iff9120edccd9d128f44586f50ac4dacae6223843
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4535106
Commit-Queue: Frederick Mayle <fmayle@google.com>
Auto-Submit: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-05-17 15:52:50 +00:00
Takaya Saeki
7781380f01 devices: virtio: block: use fdatasync for FLUSH
Currently, virtio-blk translates a FLUSH request to fsync system call.
However, fsync always updates the metadata of a file, which causes extra
journaling for metadata update. Since virtio-blk cares only the data of
the given block image file, we should avoid updating extra metadata.

After this change, virtio-blk now calls fdatasync for a FLUSH request.
If a write operation wrote to a data block that is already allocated
before, fdatasync can avoid triggering jdb2 journaling on an ext4 file
system.

Note there are structs which fall back to fsync for fdatasync. In
addition, io_uring executor does not implement fdatasync yet.

We observed statistically significant play_store_shown_time improvement
on trogdor-arc-r by -1.614% with this change. Including non
statistically significant data, we observed the following improvements.

|         Name          |  delta  | Count |
|-----------------------|---------|-------|
| trogdor-arc-r (lazor) | -1.614% |  170  |
| brya-arc-t (primus)   | -1.478% |  200  |
| octopus-arc-t (apel)  | -0.887% |  190  |
| kukui-arc-r (kakadu)  | -0.451% |  170  |

BUG=b:281609112
TEST=run a vm with block and confirmed fdatasync is called by strace

Change-Id: Idc94a3ec169e9a5e04394079967f6d79ff4c32db
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4523029
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-05-12 18:15:54 +00:00
Frederick Mayle
82ce7f3131 cros_async: delete run_one
It isn't widely used, probably because it isn't very useful: you need a
reference to the Executor to do IO. The same argument could be made for
`block_on`, but it is more widely used in tests.

Change-Id: I3e7b5783866d42a18103025f85912a9abeab5327
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4513228
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-05-11 03:36:08 +00:00
Frederick Mayle
60392a2483 cros_async: fd_executor: implement wake up notify directly in poll loop
This dovetails with an upcoming refactoring which splits the generic
executor code from the IO "reactor" code. It is also simpler and uses
marginally less resources.

The `RawExecutor::wake` call was removed from `RawExecutor::drop`. It
was previously needed only so that the (now deleted) async function
`notify_task` would break out of its loop (even then, it didn't seem
strictly necessary).

(Could we possibly need the `wake` call for something else? I don't
think so. `wake`'s propose is to wake up the `RawExecutor` on *other*
threads, however, in `drop`, it is impossible for other threads to have
access to `self` because we've got a `&mut`, so there will never be
anything to wake up.)

Change-Id: Ic61e304b360d0e33320b486d811504213c2321fe
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4331553
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-05-11 03:36:08 +00:00
Frederick Mayle
547f9a222d cros_async: test blocking pool handle drop behavior
This was implicitly tested before, but seems worth an explicit test.

Change-Id: I0d31051a8422d61f91938fc5975f230346bbce65
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4478450
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-04-27 05:16:37 +00:00
Dennis Kempin
bbb6a24d51 tools/presubmit: Add doctests
Adds an invocation to run doctests to the health_checks builder.

To make this not run by default on every `tools/presubmit`
invocation, this CL splits default_health_checks out of the
health_checks group.

BUG=b:242635135
TEST=tools/presubmit health_checks

Change-Id: Ic850c461eb79e81c4b8118fc3182614fac907569
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4476090
Reviewed-by: Zihan Chen <zihanchen@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
2023-04-26 00:09:33 +00:00
Frederick Mayle
91d4d643ed cros_async: rename TimerAsync::next_val to TimerAsync::wait
Change-Id: Ibc3c4ee9584062ed77d60c947751d3a83c2fc6ee
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4409941
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2023-04-19 08:44:15 +00:00
Frederick Mayle
a3750e1c07 cros_async: return () from IoSource::wait_for_handle and Timer::next_val
wait_for_handle always returned 0.

Timer::next_val's return wasn't used and its meaning was platform
dependent.

Change-Id: I2278859c19505398c3dd01b90e69e20894c38e25
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4409940
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-04-19 08:44:15 +00:00
Frederick Mayle
e6c584f08d cros_async: trim IoSource API
`wait_for_handle` only makes sense on Windows.

`read_u64` can be implemented in terms of the other read methods. We now
have to allocate a `Vec` which shouldn't technically be needed for the
epoll backend. We do this for all other kinds of IO and timers/events
are generally low throughput, so expect the performance loss won't be
noticable.

Change-Id: I1c1f1d5d1fc84073b940bc71425fe150196c4441
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4405314
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-04-19 08:44:15 +00:00
Daniel Verkamp
2c1d17fe41 tree-wide: remove unnecessary casts
Fixes clippy lint in Rust 1.68

BUG=b:276487055
TEST=tools/clippy # with rust 1.68

Change-Id: Ib9d83b8dc15a93e815600391e93b8bb1788f7dc4
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4391107
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-04-17 23:43:59 +00:00
Daniel Verkamp
a319ecb25e cros_async: tweak tests to satisfy clippy
Give the unused futures names to avoid triggering the new clippy
let_underscore_future warning.

No functional change.

BUG=b:276487055
TEST=tools/clippy # with rust 1.68

Change-Id: I61631261e5d8acee1fe123c808a0e76ff24a0a6c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4391113
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-04-12 19:36:47 +00:00
Daniel Verkamp
b4b48218cf devices: virtio-blk: disarm timer on flush request
The block device has a flush timer that is started when a write request
is received. This is intended to flush any outstanding writes to the
disk image if the guest does not issue its own explicit flush request
before the timeout. However, the existing implementation did not
actually disarm the tirm if the guest submitted a flush, so the flush
timer would always fire 60 seconds after the most recent write, even if
all previous writes had already been flushed by guest request.

Eliminate the extra flush by disarming the flush timer when a
VIRTIO_BLK_T_FLUSH request is processed.

BUG=None
TEST=strace crosvm block device while writing and syncing
TEST=dd if=/dev/random of=/dev/vdb conv=fsync bs=4096 count=1

Change-Id: I2cc754c84b4f3f85468381133b40f59c460dd309
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4316953
Reviewed-by: Takaya Saeki <takayas@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2023-04-06 18:10:15 +00:00
Maciek Swiech
eebc110fbf balloon: implement wss reporting virtio-balloon extensions
this patch implements the virtio-balloon extensions proposed in
go/roziere - namely adding the Working Set Size vqueues and listener on
the wss vqueue. in addition we add a crosvm_control command which allows
a user to request that the balloon device send a request to the driver
to produce a WSS report, similar to asking for balloon stats.

BUG=b:271757427,b:273212204
TEST=sidecar process, available at https://x20.corp.google.com/users/dr/drmasquatch/socket-pinger
TEST=deploy to DUT, restart termina and arcvm

Change-Id: I45753f523aae97d14bfc5dcbad5ddfcb70d10beb
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4260158
Reviewed-by: T.J. Alumbaugh <talumbau@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Maciek Swiech <drmasquatch@google.com>
2023-04-05 22:23:04 +00:00
Frederick Mayle
78c50795a8 cros_async: backend independent async tests
This consolidates many of the unit tests for async backends into
io_source.rs to reduce code duplication and to ensure consistent
behavior.

Notes:

* Deleted `read_eventfds` because it is unix specific and we have
  separate tests for the `AsyncEvent` interface.
* Moved uring specific tests into uring_source.rs to reduce `#[cfg]`
  noise.
* There are more tests that should be consolidated, e.g. the
  `IoSource::fallocate` tests.
* I hadn't realize some of the complexity of OverlappedSource until I
  had done most of the work, so this initial passes excludes it.

Change-Id: I5b11605246a049398688f57d81fc48579a2287c9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4391794
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
2023-04-04 23:34:43 +00:00
Daniel Verkamp
10a487a725 tree-wide: replace manual enum Defaults with derive
This is supported since Rust 1.62.

Fixes the new derivable_impls clippy lint, added in Rust 1.67.

BUG=b:276487055
TEST=tools/clippy # with rust 1.68

Change-Id: I3696dd3cf4acbf9b5a2cfa8c2046e7656e13dbbd
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4390735
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-04-04 22:12:39 +00:00
Frederick Mayle
2dc11a85f2 cros_async: delete Executor::async_from_local
One `async_from` function can handle both `Send` and `!Send` IO sources
now.

Note that, before, when we were using `IoSourceExt`, having two
functions for this wasn't really useful anyways because the returned
trait object was always `!Send`.

Change-Id: I2655657988b30a4fb4ec9c4ee543724d4e594de3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4391793
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2023-04-04 20:17:23 +00:00
Frederick Mayle
7e41126c86 cros_async: rename AllocateMode's Default to Allocate
Suggested in https://crrev.com/c/4312808

Change-Id: Ia3a31375bfa22b99b5c59279f1132ad3898f1df0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4396702
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-04-04 20:17:23 +00:00
Frederick Mayle
0ec62b285e cros_async: delete async IO traits in favor of IoSource
Change-Id: I66ef7ae128603049380890e71042b1003af7d25a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4312808
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
2023-04-04 20:17:23 +00:00
Frederick Mayle
6245e630b8 cros_async: introduce concrete type IoSource for async IO
This removes two layers of boxing and trait objects. One for the IO
source objects and another for the futures generated by calling methods
on them.

Benefits:

* IO futures will now be `Send` (if the underlying source is `Send`,
  which is generally the case).
* It is easier to work with a concrete type than a `Box<dyn ...>`.
* More efficient (less boxing and virtual function calls).

Slightly smaller binary size:

  $ cargo build --all-features --profile chromeos && ls -l target/chromeos/crosvm
  before: 10,811,576
  after:  10,692,792 (1.1% smaller)

Change-Id: I7b52cd24945cfb4ff0a6c377739b5d4ab9cacb0b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4004980
Reviewed-by: Noah Gold <nkgold@google.com>
2023-04-04 20:17:23 +00:00
Daniel Verkamp
083dcf75f4 tree-wide: apply nightly rustfmt
BUG=None
TEST=tools/fmt --nightly

Change-Id: Ifb08dd55ccf2a74ef739d7517a64970d24a82405
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4375640
Auto-Submit: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-03-27 21:30:45 +00:00
Daniel Verkamp
7e803c87ca base: use SafeDescriptor in unix socket wrappers
The SafeDescriptor type manages the lifetime of the file descriptor,
which simplifies the socket wrappers (each wrapper no longer needs to
manually implement Drop to close the descriptor, for example).

Also remove unneeded RawFd-related functions, replacing them with
RawDescriptor and SafeDescriptor equivalents.

BUG=None
TEST=tools/dev_container tools/presubmit

Change-Id: I634a19922ec24d06071b21247c79761cfc21a79a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4322266
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
2023-03-17 18:12:49 +00:00
Noah Gold
8de14602fe cros_async: document IO completion ports.
BUG=b:272614458
TEST=presubmit

Change-Id: I86ebf2e718378a2efcee4a7dd4eb9813eca20fcd
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4335239
Commit-Queue: Noah Gold <nkgold@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
2023-03-14 20:21:35 +00:00
Frederick Mayle
c842b3ee2d cros_async: block_on: simplify errno handling
Change-Id: I9971911239a0c2e1cc0bbebeabc687025d3c2212
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4328233
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-03-14 20:09:58 +00:00
Noah Gold
1806805c06 cros_async: add experimental OverlappedSource.
Following the addition of an overlapped capable executor, here is the
experimental overlapped IO source. This is how we'll request overlapped
IO that runs on the overlapped executor. This isn't ready for production use
quite yet, but it's something we'll want in the future.

BUG=b:272614458
TEST=presubmit

Change-Id: Iaa97cf70924334dbe82d4562f43c0daf8888cb35
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4328234
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Commit-Queue: Noah Gold <nkgold@google.com>
2023-03-14 18:32:52 +00:00
Noah Gold
c15ad34453 cros_async: convert HandleExecutor to use IOCP.
This is a first step towards using overlapped IO (truly async IO) on
Windows. Dead code allows are removed in the next CL, which adds the
OverlappedSource.

BUG=b:272614458
TEST=presubmit

Change-Id: I19e20e957462357cecdabc08757ed44ea662767e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4326943
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Commit-Queue: Noah Gold <nkgold@google.com>
2023-03-14 18:24:44 +00:00
Frederick Mayle
b31f613dbe cros_async: optimize and fix race in drop_detached_blocking_pool
Sometimes the blocking pool worker thread wouldn't start the blocking
task in time before the shutdown sequence. Now it uses a channel to wait
for it to start.

Also, the test took 10 seconds (BlockingPool::shutdown timeout) and
leaked a thread. Now it uses a channel to cleanly shutdown the thread.
Takes 20 ms on my machine. I've verified that the test still fails for
the old, leaky detach implementation.

BUG=b:272587712,b:270986145

Change-Id: Ibcc39a7129deaabbfed8023276da8c6dc48605bc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4327466
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-03-10 03:06:56 +00:00
Frederick Mayle
807d845d49 devices: remove unnecessary Executor::spawn_local calls
Change-Id: I4a6e3f9be41810c514c76990f636ee3ab1bd9dec
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4312805
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2023-03-08 02:37:51 +00:00
Frederick Mayle
22319de99e cros_async: BlockingPool: use oneshot channel instead of Task
I think this ultimately does about the same thing under the hood, but I
find it easier to understand and reason about.

BUG=b:270986145

Change-Id: Idca0bd93e63e12814381fefab70554eabb52b8dd
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4295169
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-03-07 18:33:34 +00:00
Frederick Mayle
2ee0885695 cros_async: TaskHandle abstraction + new detach semantics
`Task::detach` can be dangerous, e.g. in combination with `BlockingPool`
it can cause leaks.

This commit adds a `TaskHandle` type to replace `Task` in the cros_async
API. It is essentially the same thing, except that the `detach` method
simply moves the underlying `Task` into the `RawExecutor` so that when
the `RawExecutor` is dropped, we will also drop the `Task`s.

BUG=b:270986145

Change-Id: I727deb1f9f615aa222952dea1fee4a1c0e443fde
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4294679
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-03-07 18:33:34 +00:00
Frederick Mayle
6b4cc2525b cros_async: dedup Executor::run implementations
Change-Id: Ib9a3343e7cfde9d7bbe73ea642d0201ddc6bcf5b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4297127
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Frederick Mayle <fmayle@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2023-02-28 20:48:28 +00:00
Daniel Verkamp
ad7e0ad740 cros_async: add select9 and select10
These will be used to wait on larger numbers of futures simultaneously.

BUG=None
TEST=tools/presubmit --all

Change-Id: I9dd632c3e3971d47b9c393e06c98b2b77ecc5c86
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4265533
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-02-21 22:03:59 +00:00
Zihan Chen
28ce4e5423 crosvm: Uprev rust toolchain and dev container
- Rust toolchain is updated to 1.65.0
- Catapult dashboard upload tool is added to dev_container
- Bindgen is updated to latest version to support custom derive
- Derive Eq when PartialEq is derived as required by new Clippy

TEST=CQ, bindgen-all-the-things

FIXED=b:260784028
BUG=b:257303497

Change-Id: I2034cd09e0aed84d4e9b30f2e85d84d94a442ea4
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4228427
Auto-Submit: Zihan Chen <zihanchen@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Zihan Chen <zihanchen@google.com>
2023-02-08 20:26:30 +00:00
Frederick Mayle
ffd74530ae disk: native AsyncDisk impl for CompositeDiskFile
There is a lot of code and test duplication w.r.t. the non-async impl
but we expect to delete the non-async impl in the near future, so the
temporary duplication is preferable vs increased code complexity.

Bug: 219595052
Change-Id: I402166331606d41178c1557c66be591d95c98c45
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4158596
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2023-01-13 20:38:20 +00:00
Frederick Mayle
b64fc1dfc6 disk: double buffered read/write methods for AsyncDisk
Change-Id: I8fe7d197951ab287d0c0c6c412034af3b8e43868
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4158595
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-01-13 00:54:26 +00:00
Frederick Mayle
7b101d6cf2 cros_async: doc fix for MemRegion
Change-Id: Ie877d60a2c0c9da9fd0aac6e13d8faa2317457f2
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4158594
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2023-01-12 01:16:25 +00:00
Takaya Saeki
a85d391fbb Revert "devices: virtio: create the Executor for the block before jail"
This reverts commit ca90b4d1bd.

Reason for revert: This causes issues for Cattlefish

Original change's description:
> devices: virtio: create the Executor for the block before jail
>
> To utilize the io_uring-based Executor with the sandbox, each device
> must create any `Executor`s before jailing.
>
> This commit has the virtio-blk device make its `Executor` in `new`
> method, which is called in the main process before jailing. This change
> allows the virtio-block device to run in the sandbox with io_uring.
>
> BUG=b:251039942
> BUG=b:262341514
> TEST=`crosvm run --rwdisk path=blkdisk.img,async_executor=uring` runs
> with and without the sandbox
> TEST=`crosvm run --rwdisk path=blkdisk.img,async_executor=epoll` runs
> with and without the sandbox
>
> Change-Id: Iadb002d436a9d45f63220528273f9d2e283fbd88
> Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4098933
> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>

Bug: b:251039942
Bug: b:262341514
Change-Id: I23097d929a0e9f680e3aaa02f4d83d8f7e1ddb26
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4114052
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Takaya Saeki <takayas@chromium.org>
2022-12-20 02:16:17 +00:00
Takaya Saeki
ca90b4d1bd devices: virtio: create the Executor for the block before jail
To utilize the io_uring-based Executor with the sandbox, each device
must create any `Executor`s before jailing.

This commit has the virtio-blk device make its `Executor` in `new`
method, which is called in the main process before jailing. This change
allows the virtio-block device to run in the sandbox with io_uring.

BUG=b:251039942
BUG=b:262341514
TEST=`crosvm run --rwdisk path=blkdisk.img,async_executor=uring` runs
with and without the sandbox
TEST=`crosvm run --rwdisk path=blkdisk.img,async_executor=epoll` runs
with and without the sandbox

Change-Id: Iadb002d436a9d45f63220528273f9d2e283fbd88
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4098933
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2022-12-19 08:09:27 +00:00
Daniel Verkamp
a506de7ee6 Reformat with nightly rustfmt
BUG=None
TEST=tools/fmt --nightly

Change-Id: I7c74b9190bf28e3d7e3ac4401a2b79800a9e4b76
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4087003
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2022-12-07 19:33:09 +00:00
Takaya Saeki
266724ea60 io_uring: restrict io_uring operations that can be sumitted
io_uring allows you to submit a lot of file operations. Those operations
have corresponding syscalls, but bypass the seccomp filter since they
are submitted by single syscall. To address this security hole, the
IORING_REGISTER_RESTRICTIONS can be used to apply an allow-list of
operations to a given io_uring queue.

This commit introduces `IORING_REGISTER_RESTRICTIONS` support for
`URingContext`, and has URingExecutor apply a basic allow-list to the
UringContexts that it creates. This basic allow-list is limited to
operations allowed by common_device.policy plus fsync and fallocate.

However, note that this commit is only one step for the effective jail
of io_uring. Since `IORING_REGISTER_RESTRICTIONS` is applicable only
per-queue, a compromised device process can still easily bypass this
restriction just by creating another io_uring queue. The effective jail
is completed with coming changes that allow executor creation only
before jailing.

This commit also updates the policy file with `io_uring_register`.

BUG=b:251039942
TEST=`cargo test -p io_uring` passed
TEST=`crosvm run --disable-sandbox --async-executor uring ...` booted
TEST=`crosvm run --async-executor uring ...` with the jail booted on x86
TEST=`cargo build` succeded on aarch64.
TEST=`cargo check` succeded on armv7.

Change-Id: I6e024f240eaf9fb6ce16f1a3d46b7f923a5f5169
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4071824
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
2022-12-07 02:02:16 +00:00
Pattara Teerapong
a87d48c12f audio_streams,cros_async: Add async create stream methods
Add async_new_async_{playback,capture}_stream to StreamSource trait.
These methods are used to create new async playback or capture stream
asynchronously. Default implementation is to call their sync version.

cros_async: Add wait_fd_readable to AudioStreamsExecutor trait, this
will be used by libcras to implement the new async_new_async_*_stream
method.

BUG=b:258759773
TEST=emerge audio_streams cros_async
TEST=FEATURES=test emerge-amd64-generic audio_streams

Change-Id: I8dfc743eb8fa6552a1a6b2953b79666e2909e417
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4066242
Commit-Queue: Pattara Teerapong <pteerapong@chromium.org>
Reviewed-by: Chih-Yang Hsia <paulhsia@chromium.org>
2022-12-05 16:46:49 +00:00
Takaya Saeki
6fd4433f5d devices: virtio: block: introduce per-device async-executor option for
block device

Currently, crosvm has `--async-executor` option to switch the async
runtime engine for the whole crosvm. On the other hand, it is reported
that io_uring executor causes regression for some devices such as audio
(b:207621853). Thus, for incrementally enabling the io_uring executor,
we want per-device async-executor option to enable it for devices which
we observed the improvement.

This CL adds the per-device async-executor option for the block device.

BUG=b:251105847
TEST=confirmed that the async is switched by `crosvm run ... --block
...,async-executor=uring`
TEST=confirmed that the async is switched by `crosvm devices --block
...,async-executor=uring`

Change-Id: I0ff12741c2113b9aa5999357a0ce52e8dbd11933
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4054806
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
2022-12-02 03:09:39 +00:00