Commit graph

180 commits

Author SHA1 Message Date
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
Daniel Verkamp
ac0fc378a3 Fix remaining Chrome/Chromium OS instances
These should be written as ChromeOS and ChromiumOS (without the space)
to match the updated branding. The copyright headers were already
migrated to the new style (https://crrev.com/c/3894243), but there were
some more instances left over.

BUG=None
TEST=tools/cargo-doc

Change-Id: I8c76aea2eb33b2e370ab71ee9b5cc0a4cfd00585
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4129934
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-01-03 22:14:30 +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
Daniel Verkamp
10990c89af Rust 1.65: Fix clippy derive_partial_eq_without_eq lints
BUG=b:260784028
TEST=tools/clippy

Change-Id: Ib2b595385ed04b9480b22549334ce798d980d347
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4064717
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
2022-12-01 01:32:30 +00:00
Clarissa Garvey
76cf0e6bf6 Fix straggler needless_return violations
These are not caught during presubmit or when running cargo clippy but
nonetheless exist and should be cleaned up.

BUG=b:157245930, crbug:908640
TEST=cargo build

Change-Id: I8b4d79bf224b800fbdfeaa7729c5f283d42908e9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4039522
Auto-Submit: Clarissa Garvey <clarissagarvey@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2022-11-21 19:00:07 +00:00
Clarissa Garvey
ba3e2fa852 Add clippy tag for safety docs
This CL is a documentation-only change to add # Safety in pre-existing
safety documenation so clippy picks it up and does not flag it as
undocumented.

BUG=b:157245930, chromium:908640
TEST=remove suppression -> cargo clippy -> warnings removed

Change-Id: Iaf79839e2c8e4d8e7f74d18f244f4d02f1576bb2
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4021747
Commit-Queue: Clarissa Garvey <clarissagarvey@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2022-11-12 01:27:46 +00:00
Clarissa Garvey
0292091f52 Fix needless return violations
...round 2  because building locally missed some :)

Most are safe, similar to crrev.com/c/4014832

Two require an exception, similar to crrev.com/c/4018392

BUG=b:157245930, crbug:908640
TEST=CQ with needless_return suppression disabled

Change-Id: I0f54ee1e4688afe9c9f596bd32a98070aac20612
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4019273
Commit-Queue: Clarissa Garvey <clarissagarvey@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2022-11-10 18:10:31 +00:00
Dennis Kempin
6027bee9ba cros_async: Reduce timeout for CancellablePool in testing
The tests are explicitly verifying the timeout behavior, which
by default is 5s. We can speed up the tests by using a smaller
timeout when running a test.

Reduces the time to run the test from 6s to 1s

BUG=None
TEST=tools/run_tests cros_async:cros_async -v

Change-Id: I6262b653878171c6746a1ca90139b00a099dd91e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4004299
Auto-Submit: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
2022-11-09 18:39:33 +00:00
Takaya Saeki
b9295d8ba9 cros_async: show debug log when setting the default executor.
Show the debug log to tell the configured default async executor
backend.

BUG=b:251289312
TEST=confirmed the debug log is shown by --async-executor option

Change-Id: I82f040d2ad19739cd486945bae2b5d99492a62a1
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4006066
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2022-11-08 06:34:08 +00:00
Takaya Saeki
303c285b7f cros_async: Move --async-executor to run/device/devices subcommands
Currently, --async-executor option to switch the backend of
cros_async::Executor is a global option of crosvm main command. This is
because --async-executor is an option to switch the async runtime
backend engine of cros_async crate for a whole crosvm.

However, in practice, only run, device, and devices subcommands have the
motivation to switch the backends for the performance. Other subcommands
may or may not use async functions, but it's unlikely they want to
switch the backends. So, it makes sense to make --async-executor an
option of those three subcommands, not of the global option. Also, it
allows us to switch --async-executor in arvm_dev.conf, which overrides
the options for `crosvm run` command, and to control the async executor
by the coming configuration file feature.

Thus, this commit moves the --async-executor option from the global
command to the run, device, and devices subcommands. This is a breaking
change, but it is unlikely to have many users using this relatively new
option. Takayas also confirmed there's no usage on codesearch.

BUG=b:251289312
TEST=confirmed `crosvm --log-level debug run --async-executor ...`
switches the executor with additional debug log in the coming CL.
TEST=confirmed `crosvm --log-level debug device --async-executor ...`
switches the executor with additional debug log in the coming CL.
TEST=confirmed `crosvm --log-level debug devices --async-executor ...`
switches the executor with additional debug log in the coming CL.
TEST=./tools/dev_container ./tools/presubmit

Change-Id: Ia25f2d0b296b8d73f31d71c362e5f90678166d96
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005473
Auto-Submit: Takaya Saeki <takayas@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
2022-11-08 06:29:20 +00:00
Richard Zhang
24ed80fd99 win_audio: Upstream Windows audio backend code
The most notable changes are:

* Playback Async support
* Audio Client flags to hide app from SndVol when audio session has
  expired
* Updates to audio_streams and cros_async crates for playback async
  support

BUG=b:256655413
TEST=Verified to work downstream

Change-Id: Ifbe9a15791feaa41d6e1d5eaf2c5824b2c7c25b8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3994882
Commit-Queue: Richard Zhang <rizhang@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
2022-11-02 21:07:37 +00:00
Daniel Verkamp
86148a9ceb cros_async: use tempfile instead of fixed path in tests
Previously, several tests used the same hard-coded filename for test
files ("/tmp/write_from_vec"), which could cause flaky tests when
running in parallel. Replace these with a tempfile invocation so the
tests can run simultaneously.

BUG=None
TEST=cargo test -p cros_async

Change-Id: I2b32d998583f9a44359345628d803b2397af33b1
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3993927
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2022-10-31 21:57:39 +00:00
Dennis Kempin
f8eb24052e Reformat all files with nightly
Run tools/fmt --nightly

BUG=None
TEST=None

Change-Id: Iaccfc5fe141c512f4b508c699f89686a4552bf96
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3988327
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
2022-10-31 21:33:33 +00:00
Daniel Verkamp
28d0f67858 base: event: add Event::reset() API
This allows resetting an Event without waiting on it. Windows already
had this functionality in its EventExt, and we can provide an equivalent
implementation for eventfd on Linux, so promote this function to the
cross-platform Event type.

BUG=b:231344063
TEST=tools/presubmit --all

Change-Id: I6dba3cb2e0b2d702e8a3f0dacf5c25c1dd044a13
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3966480
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
2022-10-20 21:04:14 +00:00
Frederick Mayle
3d4b4809ba base: cross platform Event API
Trying to reconcile the difference between the linux and windows
implementations.

Code relying on the eventfd count must now use the linux specific
`EventExt` interface.

BUG=b:231344063
TEST=presubmits

Change-Id: I14eb50f7a02d766a00f27aca388823309633e193
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3864030
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
2022-10-19 20:17:42 +00:00
Idan Raiter
cbf1e0c61b devices: stop resetting ReadNotifiers in vhost user
Upstreaming a fix from nkgold@ for the following issue:

ReadNotifiers shouldn't ever be reset as their state is managed by the
type that exposed the notifier (e.g. StreamChannel). Resetting it can
lead to lose notifications. idanr@ discovered that EventAsync::clone_raw
was creating a regular EventAsync (e.g. with reset), and it was being
used with read notifiers. This CL replaces clone_raw with a new factory
that produces `without_reset` EventAsyncs.

BUG=b:253255209
TEST=presubmit, downstream

Change-Id: I0b9de2f12cc3597ba05a8ac108f4cd0a8e726b3f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3960857
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Idan Raiter <idanr@google.com>
2022-10-19 17:13:15 +00:00
Allen Webb
07041ace37 Adding missing # provided by ebuild to Cargo.tomls
cros_async and by extension serde_keyvalue are needed on ChromeOS, so
add the `# provided by ebuild` comments where appropriate to make it
possible to uprev cros_async (which requires pulling in serde_keyvalue).

BUG=b:250883877
TEST=kokoro passes

Change-Id: I423cac0c4e5e30afac058b914358caca7d27edbf
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3953082
Auto-Submit: Allen Webb <allenwebb@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2022-10-14 16:42:38 +00:00
Keiichi Watanabe
98ff121e5b cros_async: Use argh_derive feature in serde_keyvalue
This is a followup of crrev.com/c/3931568.

Since `derive(serde_keyvalue::FromKeyValues)` used for `ExecutorKind` is
guarded by the serde_keyvalue's `argh_derive` feature, we need to
specify the feature in cros_async's Cargo.toml.

The original CL didn't break the crosvm build because our builder always
enable the `argh_derive` feature.

BUG=b:239154263
TEST=cargo test in /cros_async

Change-Id: I10c77373241271e3002fe574c69e0118059a7f1a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3945519
Auto-Submit: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2022-10-11 17:26:59 +00:00
Takaya Saeki
1be9cb2ef5 cros_async: refactor FromStr for ExecutorKind
FromStr trait is manually implemented for ExecutorKind, but
serd_keyvalue should be preferred. This commits refactors ExecutorKind
so that derives serd_keyvalue.

BUG=None
TEST=`crosvm --async-executor (epoll|uring) run ...` works
TEST=test passes on linux and windows

Change-Id: I5aafb6704c20a17120b30c290e1a4069dff9e5ce
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3924880
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
2022-10-04 09:58:49 +00:00
Takaya Saeki
119ace0dca Reland "cros_async: add a command-line option to switch Executor
backend"

This reverts commit 03fdffb447.

Now b:249692652 is fixed by crrev.com/c/3842811/, we can bring back the
command-line option to switch Executor backend.

This commit is identical with the original CL, except it resolving the
conflict with ccrev.com/c/3841945/; it renames the old `use_uring` in a
test to the new name, `is_uring_stable`.
(cros_async/src/sys/unix/uring_source.rs:652)

BUG=b:249692652
BUG=b:239154263
TEST=confirmed crosvm --async-executor correcly switchs the Executor for
`crosvm --async-executor epoll (or uring) device gpu --socket
/tmp/gpu.sock` command. Used additional debug log to tell which
executoris running, which is not commited)
TEST=all tests passed

Change-Id: Iafb6921c1a7594a178712eb46792dc54d5d99f71
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3931568
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2022-10-03 09:31:24 +00:00
Frederick Mayle
ef70f71256 cros_async: fix io at current file pos for io_uring
The `ReadAsync` and `WriteAsync` traits accept file offsets as an
`Option<u64>`. The epoll executor interprets `None` as "current file
position" and the io_uring executor interprets `None` as position zero.
This change updates the io_uring executor in favor of the epoll executor
behavior.

It is important to differentiate between position zero and "current file
position" when you don't know whether a file is seekable. For example,
console and serial devices should support both types of files as input.

BUG=b:243198718
TEST=presubmit --all

Change-Id: I01634e6bbdff352c0a6055419d0fafd10c0c40a6
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3841945
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Steven Moreland <smoreland@google.com>
Reviewed-by: Takaya Saeki <takayas@chromium.org>
2022-09-30 20:32:02 +00:00
Colin Downs-Razouk
e0de18a6a5 base: timer: handle 0 interval explicitly
Make it official that setting a Timer's interval to 0 is the same as
setting it to None, meaning it will not be periodic. Fixed the windows
timer to behave this way again, and updated Timer docstring.

Also updated APIC timer to use None for interval which is clearer.

Bug: b:248612697
Test: tested with UserspaceIrqChip in dynamic tick mode
Change-Id: I57713c3ace345a8c64b9219a8e60f36c54a9481c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3925006
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Colin Downs-Razouk <colindr@google.com>
2022-09-30 16:50:22 +00:00