Make it possible to retrieve the display order by adding a
display_order() member to the DynDecodedHandle trait.
This is in preparation for further CrosVM changes where the handles
will be sorted by the display order.
The decoders will issue the display order from a monotonically
increasing counter after applying any codec-specific reordering
algorithm. We will build upon the monotonically increasing counter to
identify gaps in CrosVM and wait until said gaps are filled by newer
frames.
BUG=b:214478588
TEST="cargo build --features=video-decoder,vaapi" successfully builds
the libva crate
Change-Id: I4e4b1becb9aea124b6b942069dcc676385bd6986
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3907528
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Add a convertion for the VP8 VA profile, otherwise the profile will be
skipped when the Capabilities are built and VP8 will be unsupported.
BUG=b:214478588
TEST="Decoder reports VP8 as supported (e.g.: if using GStreamer,
gst-inspect-1.0 video4linux2 lists a VP8 decoder)"
Change-Id: I6af62b475d840f309c3bdc7cc05e7722149c9b3b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3907527
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Do not drain the ready queue if we have not been provided buffers by
CrosVM.
The previous implementation would attempt to drain as soon as a picture
was ready to be outputted, but this does not account for the fact that
there is a window of time between ProvidePictureBuffers being emitted
and set_output_parameters() being called, as the guest needs to process
the request.
This meant that ready frames would be dequeued, but the call to
output_picture would return Err, effectively dropping the frames before
they could be outputted.
Simply return early if OutputQueueState != OutputQueueState::Decoding,
leaving the queue as is.
BUG=b:214478588
TEST="Decoding test-25fps.h264 yields exactly 250 frames."
Change-Id: I6042f4556933d4b59a54c6ca986510356a040e18
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3903094
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
The reset implementation for the VAAPI backend was wrong for a couple of
reasons.
It would create a new session, thereby invalidating the eventfd file
descriptor passed to the WaitContext. This meant that events would not
get signalled from that point on.
Also it would recreate the codec instance, but the right thing to do
here is to flush, which resets the internal state without needlessly
invalidating the allocations already made.
The above is also the same logic done by the ffmpeg decoder, which calls
avcodec_flush_buffers instead of recreating the AVCodecContext.
The previous implementation would also not clear the submit and ready
queues.
Not clearing the submit queue meant we could not return the buffers to
crosvm, which is against the contract for reset(). This is because this
queue keeps a reference to GuestResourceHandle.
Not clearing the ready queue meant that we would return stale buffers
once/if decoding resumed.
Lastly, it would reset the state of the output queue to
AwaitingOutputBuffers, but we can resume decoding with no new buffers
provided, so this is incorrect.
In particular, this change eliminates the following error reported by
the virtio video kernel driver:
virtio-video virtio-video.5: timed out waiting for queue clear
And also the related CrosVM virtio video worker error: returning async
error response: invalid operation
As the ResetCompleted event is correctly signalled now.
BUG=b:214478588
TEST="Decoding in the guest does not cause these two warnings to be
emitted: virtio-video virtio-video.5: timed out waiting for queue clear
and returning async error response: invalid operation right after that"
Change-Id: I0ed2fc59819875fb3cc4462d401811f3126ab686
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3903093
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Introduce try_make_progress() as a way to enforce the following
invariant between the submit and ready queues: the ready queue must
always be drained first.
This is to avoid deadlocks: draining the submit queue will fail if the
ready queue is full enough, since this prevents the deallocation of the
VASurfaces embedded in the handles stored in the ready queue. This
means that no progress gets done.
While we are at it, make sure to call it in places where we were only
erroneously draining the submit queue (which again, means that no
progress could be made)
BUG=b:214478588
TEST="The decoder does not randomly stalls if the guest app is too slow
to retrieve frames in the ready queue"
Change-Id: I9ea34089b7df4417272cdc775d42422b865a993c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3901198
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
We are always done with the input buffer even if no frames are generated
by the decoder. Reflect this by always releasing the buffer after a call
to self.codec.decode()
While we are at it, note that it's the resource_id that has to be passed
to NotifyEndOfBitstreamBuffer.
BUG=b:214478588
TEST="Guest does not complain about unknown resource_ids when processing
NotifyEndOfBitstreamBuffer"
Change-Id: I61c0d2f0e7ea6a4907e853bb2f005bb421447042
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3901197
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
The current code will add an input format even though it might not be
able to be decoded to NV12. This will be fixed in a later patch, but for
now just drop these formats instead.
BUG=b:214478588
TEST=cargo test -p devices --features=video-decoder,vaapi virtio::video::decoder::backend::vaapi::tests::test_get_capabilities -- --include-ignored
Change-Id: I89e7261fdc935207f1fcb11d18e7701cdaf54ba9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3901274
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
We had a manual parsing function that is strictly equivalent to how
serde_keyvalue would deserialize, so do the latter instead.
Also add some tests to make sure we don't regress in the future.
BUG=b:218223240
TEST=./tools/health-check
Change-Id: I5b6317774368fa4256a1944e7aec54e8fe8f210a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3979494
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
We had a manual parsing function that is strictly equivalent to how
serde_keyvalue would deserialize, so do the latter instead.
Also add some tests to make sure we don't regress in the future.
BUG=b:218223240
TEST=./tools/health-check
Change-Id: If1b6531f570e3512d41638d88f66c863b8213385
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3977491
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
The max width and height from the coded side are the best approximation
for the max width and height of the raw side if the actual values are
not available in VA-API.
In particular, it's extremely unlikely to have 1x1 as acceptable values
for max_width and max_height.
BUG=b:214478588
TEST=cargo test -p devices --features=video-decoder,vaapi virtio::video::decoder::backend::vaapi::tests::test_get_capabilities -- --include-ignored
Change-Id: I6d9830b4da69f29f5e2e11c3fa688db4a7007726
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3901273
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
The raw capabilities should not depend on the value for rt_format.
Both in the Intel and Mesa implementations it can be seen that setting
VAConfigAttribRTFormat as a parameter to vaCreateConfig only serves as a
way to validate that the to-be-generated config supports it.
This means that the Intel implementation will only use it in
CheckAttribList, while Mesa will AND it with a precomputed bitmask of
supported values in vaCreateConfig.
Thus, creating one RawCaps instance per value of RT_FORMAT only serves
to create needless duplication.
Not only that, but it frequently makes the call to QUERY_CAPABILITIES
fail, as the resulting structures become too large to fit into the 1024
byte buffer.
Remove all traces of VA_RT_FORMAT_* from this code path. While we are at
it, note that the libva crate API for vaQuerySurfaceAttribute changed to
reflect that this call can return more than one attribute at once.
This is particularly important for VASurfaceAttribPixelFormat, as one
value per supported fourcc is returned. We now properly return one
instance of RawCaps per VA_FOURCC supported.
Also reflect that Surface attributes can be defined more than once
This is particularly the case with VASurfaceAttribPixelFormat, where the
driver will define one attrib per pixel format supported.
This can be seen in the Intel driver, for example, in
MediaLibvaCaps::QuerySurfaceAttributes.
The previous code assumed that a given attribute could only be defined
once, and thus used find(). It was refactored to use filter() instead.
The name of the method was changed to reflect that it can now return
more than one attribute per call. The return type was changed to Vec for
the same reason.
BUG=b:214478588
TEST="cargo test --package devices --lib --features video-decoder --features vaapi -- virtio::video::decoder::backend::vaapi::tests::test_get_capabilities --include-ignored"
Change-Id: I914376beb026e30b2a52ca8f7d01c81e6a9a2775
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3946301
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Queue incoming requests so that they can be retried if the backend runs
out of resources.
Previously the VA-API backend would just refuse to decode if all
allocated surfaces were in use. Notably this can be the case if the
surfaces are kept in use by virtue of being in the ready queue. An
example of which could be seen whenever the guest would not retrieve the
decoded images as fast as the hardware decoder would produce them.
This issue can be worked around by keeping a queue of the decode
requests in the order they were submitted by the guest. The operation
can then be retried either on the next call to decode or when a new
output buffer becomes available, whatever comes first.
BUG=b:214478588
TEST="Decoding from the guest does not fail with
StatelessBackendError::OutOfResources"
Change-Id: I53b972f6520bdc704be6eaca14c5343391122914
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3875045
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Port the VAAPI backend to the new cros-codecs crate. This crate now
contains all codec related code and is independent from the rest of the
CrosVM code.
BUG=b:214478588
TEST="cargo test --package devices --lib --features video-decoder --features vaapi -- virtio::video::decoder::backend::vaapi::tests::test_get_capabilities --include-ignored"
Change-Id: Id207c53c0c4200e03ce8793d7c37cb5fbe808829
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3875044
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Introduce the cros-codecs crate. This crate contains all the
codec-related code and does not depend on CrosVM. The decoders are
decoupled from the backends, which allows for the implementation of new
backends without touching the decoder code.
This crate comes with dummy backends to test the decoder functionality
in isolation, but in order to decode frames, a real backend is needed.
Currently this backend is the VAAPI backend. Using it adds a dependency
on the libva crate.
This change adds support for VP8, H264 and VP9.
BUG=b:214478588
TEST="cd media/cros-codecs && cargo test --release --features vaapi -- --include-ignored"
TEST="emerge-hatch chromeos-base/crosvm" completes successfully.
Change-Id: I596d5db4dabcc96dcfdbce1f41c8092e01b64271
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3875043
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
We are going a add a couple of .vp9 and .ivf files, these are binary and
do not need to end with a newline.
BUG=b:214478588
TEST=./tools/health-check passes with crrev.com/c/3875043
Change-Id: Ic4e434616ed880dbff5dae76108e8d6692d80584
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3990145
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Ensure all of the cfg checks for whpx also validate the target is
Windows when used in generic (non-Windows-platform-specific) code. This
will allow all builds to enable the whpx feature by default.
BUG=b:213151419
TEST=tools/dev_container tools/presubmit --all
Change-Id: I1faebeed227ac5653697195b195b0884e043f110
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3989384
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This allows ChromeOS/AOSP to keep using their slightly older bugfix
version number, while our Cargo.lock file is updated to the latest
version that includes the build.rs fix to prevent unnecessary re-builds.
BUG=None
TEST=presubmit
Change-Id: Ibe0a46632d9766cad7fb6bc5b6b4042da92313bf
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3984415
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
This will help us running audio tests under wine.
BUG=b:237011316
TEST=presubmit
Change-Id: Iba297159291abd135fb1972a19fa5b5c216fa956
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3971028
Commit-Queue: Vikram Auradkar <auradkar@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
BUG=b:240692674
TESTED=no test, led missed this in the last cl
Change-Id: I725083b7d8b62eef5c411acb7113fd5f18059410
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3984414
Auto-Submit: Zihan Chen <zihanchen@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
The 9p lcreate operation takes a directory fid as input and creates a
file in that directory; when the operation completes, the same fid
becomes a reference to the newly-created file. We updated the internal
self.fids structure's file and path fields to point to the new file, but
we neglected to update the filetype field, which would remain as the
original FileType::Directory.
This caused an issue with commit 53cd18e062 ("p9: use *at() functions
for set_attr"), since that change causes set_attr requests to validate
the filetype is not a directory when attempting to set its length.
BUG=b:253838039
TEST=tast run <...>.DefaultSharedFolder
Change-Id: Ie46a660dd4616d669c924014e704e9b5703eb7e9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3983116
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Add a new builder to build crosvm in crOS tree, and all the
depencies of this new builder.
BUG=b:240692674
TESTED=led get-builder luci.crosvm.ci:chromeos_amd64-generic | led edit-cr-cl https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3966928 | led edit-recipe-bundle | led edit -r build_chromeos_hatch | led launch
Change-Id: Id2f284139922916edd2dd584f576da9fb3445518
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3966928
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Zihan Chen <zihanchen@google.com>
reland note: Added wineboot for Dockerfile.user, since it prepares
directories for wine on a per-user basis.
To enable podman, the Dockerfile has been split into a root
run Dockerfile and one that adds a non-root user.
The following combinations have been tested:
./tools/dev_container -v --clean --podman --unprivileged
./tools/dev_container -v --clean --podman
./tools/dev_container -v --clean --unprivileged
And warnings have been added to ensure users are aware that
the only fully supported variant is running a privileged
docker container:
./tools/dev_container -v --clean
The unprivileged containers will allow us to validate if
unit tests require privileged system access.
BUG=None
TEST=See above
Change-Id: Ifd70c1e30ef266e39bf517e315dc88fccecc8a62
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3983255
Auto-Submit: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Zihan Chen <zihanchen@google.com>
Commit-Queue: Zihan Chen <zihanchen@google.com>
This change moves most ircchip tests into an integration test.
These tests rely on kvm, and if they do not - reuse much of the
test code from each other, so they need to move together.
Note: This removes the apic_timer test. The test has been disabled
for a while due to it's use of sleep in unit tests.
We cannot support it as an integration test either, since it combines
the sleep with a FakeClock. userspace.rs swaps the real clock for
FakeClock when compiled with cfg(test), but integration tests do not
compile with cfg(test), so we cannot use the FakeClock.
The biggest side-effect is faster execution as we can run all other 300+
tests in parallel and via user-space emulation, significantly cutting
down on the test times. It also allows those tests to run in a
podman container.
BUG=b:244620308
TEST=CQ
Change-Id: I1728a736d27e924daf228752711435885dacfa6a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3977111
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Structs can be parsed as part of the command-line input, if they are
enclosed within braces.
BUG=b:218223240
TEST=cargo test -p serde_keyvalue
Change-Id: I05d9d1237036c6ba464408d56c216072e285d801
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3979490
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Pujun Lun <lunpujun@google.com>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Group related bits closer and comment a bit. This does not change the
behavior of the code.
BUG=None
TEST=cargo test -p serde_keyvalue
Change-Id: I01136fbebaa3790311255492f87848e058a8ae8f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3979489
Reviewed-by: Pujun Lun <lunpujun@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
There is no technical reason to limit the number of video decoders and
encoders that can be instantiated, so remove the current artificial
limitation of one.
BUG=b:255223604
TEST=`cargo run --features "video-decoder,ffmpeg,vaapi" -- ... --video-decoder ffmpeg --video-decoder vaapi`
results in two usable decoder devices in the guest.
Change-Id: I71cd344db6827b57daa324ccb467425fe8337b65
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3974354
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Other device parameters with a backend type use `backend` as field name,
so do the same for video devices. This is backward-compatible as the
backend field is implicit for the command-line.
BUG=b:255223604
TEST=cargo build --features "video-decoder,video-encoder,ffmpeg"
Change-Id: I5133080714d3292295468a2c7152a06fc669da1c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3974353
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Virtio devices can typically be instantiated several times. This should
particularly be the case for the GPU device, but we currently have an
artificial limitation to 1 device due to the way we handle resource
bridges.
This limitation should hopefully be lifted in the future, and meanwhile
we would like to enable instantiating the GPU from the configuration
file with its final syntax ; so turn the `gpu` field of `RunCommand`
into a vector, and throw a custom error if more than one GPU has been
instantiated.
BUG=b:255223604
TEST=cargo build
Change-Id: Ia2f76e52efaddeffb1a23c86088c123587985b94
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3974352
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Configuration file support for the `run` command will be done by
deserializising a file into a `RunCommand` instance, where all the
parameters will be optional.
Make sure that all parameters that are not an `Option` are optional by
giving them a default value if they are not explicitly specified to make
the structure deserializable.
The set of parameters we are confident enough about to allow them in the
configuration file is still limited, so most parameters are skipped. We
will enable them after review and ensuring they are suitable for a
configuration file (see b/255223604).
There are also a few parameters that are definitely deprecated ; these
ones are also skipped pending their removal from the command-line.
BUG=b:218223240
TEST=./tools/health-check
Change-Id: Ia261e7e4d2d5957cf96265d9117355dc352fdd3c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3970367
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
This reverts commit 6a2b1fda85.
Reason for revert: Broke mingw64 builds
Original change's description:
> dev_container: Fix podman and enable unprivileged containers
>
> To enable podman, the Dockerfile has been split into a root
> run Dockerfile and one that adds a non-root user.
>
> The following combinations have been tested:
>
> ./tools/dev_container -v --clean --podman --unprivileged
> ./tools/dev_container -v --clean --podman
> ./tools/dev_container -v --clean --unprivileged
>
> And warnings have been added to ensure users are aware that
> the only fully supported variant is running a privileged
> docker container:
>
> ./tools/dev_container -v --clean
>
> The unprivileged containers will allow us to validate if
> unit tests require privileged system access.
>
> BUG=None
> TEST=See above
>
> Change-Id: I185b1d9c3829674986305b0e72a39b1a4ba11b98
> Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3971029
> Reviewed-by: Zihan Chen <zihanchen@google.com>
> Commit-Queue: Dennis Kempin <denniskempin@google.com>
> Reviewed-by: Dennis Kempin <denniskempin@google.com>
Bug: None
Change-Id: Id57686ed869abcfb54431aa328c54234b9465eb7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3979385
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Auto-Submit: Dennis Kempin <denniskempin@google.com>
To enable podman, the Dockerfile has been split into a root
run Dockerfile and one that adds a non-root user.
The following combinations have been tested:
./tools/dev_container -v --clean --podman --unprivileged
./tools/dev_container -v --clean --podman
./tools/dev_container -v --clean --unprivileged
And warnings have been added to ensure users are aware that
the only fully supported variant is running a privileged
docker container:
./tools/dev_container -v --clean
The unprivileged containers will allow us to validate if
unit tests require privileged system access.
BUG=None
TEST=See above
Change-Id: I185b1d9c3829674986305b0e72a39b1a4ba11b98
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3971029
Reviewed-by: Zihan Chen <zihanchen@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Parsed GPU parameters need to be fixed up using a platform-dependent
function. This was done using a custom argh parsing function, but doing
so only applies the fixup to command-line arguments, and we will want to
deserialize GPU parameters from configuration files as well.
Move the fixup to the serde deserialization stage by using a proxy type
which fixes up GpuParameters while being converted from it. Since
GPU command-line parameters are also parsed using serde, this ensures
fixup is performed from both paths.
BUG=b:218223240
TEST=`cargo test --features "gpu,virgl_renderer,virgl_renderer_next" parse_gpu` passes.
Change-Id: I54b73bf0bddc933c012cc00f70608afd575c3352
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3977490
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Pujun Lun <lunpujun@google.com>
If crosvm's connection to the host compositor is suddenly broken
(e.g. from a compositor crash), a single POLLHUP is sent on the listened
file descriptor, but it is ignored. Then the VirtioGpu backend is
repeatedly awoken to handle display events, although there are none.
Since the compositor is lost and there is currently no path for
recovery, we can at least prevent runaway CPU usage by removing the
display from VirtioGpu's WaitContext. For VMs that can continue to
function without a guest display (headless crostini), this is
non-lethal. For VMs that require a display (ARCVM), other mechanisms
already exist for shutting down the VM under such unrecoverable
conditions.
BUG=b:250923109
TEST=Run Crostini and trigger a chrome crash; inspect virtio_gpu CPU
usage
Change-Id: I8b2261a093191dfe142697c4c4adc4e9ffab751a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3975942
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Ryan Neph <ryanneph@google.com>
crrev.com/c/3971026 put GpuMode::ModeVirglRenderer behind the
`virgl_renderer` feature, but did not disable all the tests that try to
parse the virgl renderer.
BUG=None
TEST=`cargo test --features "gpu" parse_gpu` passes.
TEST=`cargo test --features "gpu,virgl_renderer,virgl_renderer_next" parse_gpu` passes.
Change-Id: I1018d5021b81484b6d747e15aa60edf005c1f53a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3977489
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Pujun Lun <lunpujun@google.com>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
We want to support configuration files where configuration options will
have the same name as our command-line arguments. For fields of
RunCommand which name is not identical to the argument itself, this
would mean adding a new serde helper attribute to make the name match we
one we already explicitly specified to argh.
It is just simpler and consistent to have the fields named exactly after
their corresponding argument, so do this. Some fields need to be
reordered in order to satisfy the #[remain::sorted] requirement, but no
other change is done otherwise.
BUG=b:218223240
TEST=`cargo run --features all-x86_64 -- run --help |sort` yields
identical output before and after with CL.
Change-Id: I1bf9d1e6a443afd61dd6d8180da42e485699353e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3970366
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
We currently have 4 different arguments for specifying block devices:
`disk`, `rwdisk`, `root`, and `rwroot`. These arguments basically do the
same thing save for setting an additional flag to the disk
configuration.
This is both inefficient and confusing, so add a new `block` argument
that allows to specify both the root and read-only properties as flags.
It works similarly to the argument of the same name of the `devices`
command and is easier to use in the context of a configuration file.
We will eventually deprecate the 4 previous arguments after all users
have transitioned to the new one.
Change-Id: I87451aee57c714b5d47df9d8823c0b769137d426
BUG=b:218223240
TEST=`--block PATH,root` can be specified in place of `--rwroot PATH`
and the right device is mounted as root, read-write.
Change-Id: Ic186616344abac8af86040fc6b1613fd8e66a6a0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3970364
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
The `root` and `rwroot` command-line parameters allow to specify a block
device for which the right parameters are passed to the kernel to mount
it as the root filesystem. This approach relies purely on the name of
the parameter given and won't allow us to pass a "root" flag to a
unified block device command-line option.
Address this by adding a "root" member to `DiskOption`. On top of
allowing us to specify the will to mount a particular device as root
through a flag, it also clarifies the code a bit as we deal with one
less unnamed tuple.
BUG=b:218223240
TEST=Guest Linux boots as expected with the `--rwroot` option.
Change-Id: I5d5eda1cc8b1fc2f08e798064fc0db3b17f000a3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3970363
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Since we have several command-line options for specifying disks (disk,
rwdisk, root, and rwroot), we add an increasing disk ID each time we
encounter one of these options so we can preserve the order of
declaration later.
This ID is currently handle as a tuple alongside the actual DiskOption,
which forces us to use the `from_str_fn` argh attribute and requires the
ID to be explicitly specified if we deserialize RunCommand.
Replace the tuple by a dedicated local structure with its own FromStr
and From<DiskOption> implementations that assign the ID transparently.
This allows us to stop using `from_str_fn` and will also make
deserialization behave as we want for supporting configuration files.
BUG=b:218223240
TEST=Guest Linux boots as expected with the `--rwroot` option.
Change-Id: I291c119e6e9c4d76b71a2d6982d1504a3d689160
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3970362
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Since the virtio allocates all available IRQ lines for VGIC,
system_allocator::allocate_irq() returns 32 for Goldfish battery,
but that is not handled by the VGIC. In the result, the interrupts
from Goldfish Battery device are dropped in the host kernel. Thus
even if the crosvm detects the AC unplug, it is not notified to
the guest.
To fix this issue, assign a static IRQ number (#3) to Goldfish
battery device as same as other devices, so that it can deriver the
interrupts correctly to the guest side via VGIC.
BUG=b:252582345
TEST=Boot the ARCVM and run 'dumpsys battery' and unplug/re-plug
AC connector several times, and confirm the AC status is updated.
Change-Id: Icdf3713cdf615d0039dd4e7719b80cad32333094
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3971137
Reviewed-by: Masami Hiramatsu <mhiramat@google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Auto-Submit: Masami Hiramatsu <mhiramat@google.com>
Only gfxstream is being actively used on Windows for now, so we
either use it or fall back to 2D rendering by default.
This CL also puts ModeVirglRenderer under the "virglrenderer"
feature flag, so that the arg parser would reject it when specified
by mistake.
BUG=b:254284360
TEST=presubmit
Change-Id: Ifa39e4a528acf1bd45a85e7327b3edded3a4e7d6
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3971026
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Kaiyi Li <kaiyili@google.com>
Commit-Queue: Pujun Lun <lunpujun@google.com>
The extracted tests rely on access to system devices
or global state that prevent them from being run in parallel
or in restricted environments.
As an integration test they will be executed separately and
single threaded.
Updates the test runner to ensure integration tests are actually
run single threaded as intended.
BUG=b:244623061
TEST=./tools/run_tests base:\* --repeat 100 -p x86_64/mingw64/aarch64
Change-Id: I4267b9f79055208aca86796d902da251816bcada
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3971025
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
We do receive WM_ENTERSIZEMOVE when the window is about to be resized or
moved, but it doesn't tell us whether resizing or moving should be
expected. We won't know that until later we receive WM_SIZING or
WM_MOVING. There are also corner cases where we don't receive either
WM_SIZING or WM_MOVING in the modal loop, or receive both of them.
This CL adds an enum SizeMoveLoopState to track this state, so that we
can know whether the window is resizing or moving.
One alternative is to use WM_NCHITTEST to test whether the cursor is on
the window title bar (which implies moving the window) or window
borders/corners (which implies resizing) when WM_ENTERSIZEMOVE is
received. However, the user may also trigger resizing/moving from the
system menu (e.g. by right-clicking on the title bar and selecting it
from the drop down list), so this is not always reliable.
BUG=b:254702853
TEST=Tested in the Windows downstream
Change-Id: I8c8d97a7542b291c57dbddb75d785b324ff2776e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3975933
Commit-Queue: Pujun Lun <lunpujun@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
This CL bumps to the latest Rust-scudo version and re-enables
the corresponding feature, after it was disabled in crrev/c/3964927.
The fix introduced in https://github.com/google/rust-scudo/pull/6 was
submitted and is part of the 0.1.3 release of scudo.
BUG=None
TEST=health-check
Change-Id: I9c658cde9ea2d4cdf0d03110e2d015c8339e5267
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3973490
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
We will deserialize AddressRange from RunConfig, so make sure we error
on invalid parameters.
BUG=None
TEST=./tools/health-check
Change-Id: Ia4b5c62e61f3e12e40f400e199fc401773dbbdcc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3970365
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
