The Req trait and related code relied on transmuting arbitrary u32
values into an enum that might not be one of the valid enum branches;
this has always been undefined behavior in Rust, and now it is actually
causing failures at runtime (tests compiled with Rust 1.71.1 in release
mode crash, since the compiler intentionally emits an illegal
instruction for the transmute in a test).
Rework the code so instead of transmuting it first and checking for the
is_valid() function later, we require Req to implement TryFrom<u32> and
make VhostUserMsgHeader::get_code() return a Result so that callers must
check that the code is indeed valid.
Ideally, this code should be refactored so that the VhostUserMsgHeader
is checked once when it is received rather than each time get_code() is
called, but that would be a larger change, so it is left as a potential
future cleanup.
BUG=b:297923761
TEST=cargo test --release -p vmm_vhost # w/rust-toolchain = 1.71.1
TEST=tools/dev_container tools/presubmit
Change-Id: I8d24dd9d81ee107f9ab43ea7bf68145c380364db
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4824412
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
After go/playcl/83699, the WndProc thread is the only thread that
accesses this tube, so we could change its type from Arc<Mutex<Tube>> to
Rc<Tube>, so that we no longer need to worry about deadlocking.
This tube was a field of struct DisplayParameters. That struct is used
for CLI arg parsing, however, we don't get this tube from CLI so the
code looks awkward. Besides, DisplayParameters specifies params for each
individual window, but we will likely only have one tube and share it
among multiple windows when we support multi-windowing. So, we can
remove it from DisplayParameters, and WindowProcedureThread will
distribute the clones of the tube to surfaces.
Bug: 257486521
Test: Ran GPG
Change-Id: Ic7adeb4a41c89d79e40820ad4d50e8d85c4e385c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4804685
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Kaiyi Li <kaiyili@google.com>
Report the error from fs::read() if it fails, in addition to the path.
Also add an error for when both string and path are missing, and remove
the validation for this case in config.rs. This simplifies the cmdline
parsing code because it can now use the default deserialization method
instead of a custom from_str_fn.
BUG=b:283990685
TEST=tools/dev_container tools/presubmit
TEST=crosvm run --fw-cfg with string= and path= options
Change-Id: I37104ae00c9acda55f10fdd382ea727d5b59b6cd
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4795353
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Sebastian Hereu <sebastianhereu@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Also add a config test with a valid --fw-cfg with path, and check the
values returned by the valid config parse tests.
BUG=b:283990685
TEST=cargo test parse_fw_cfg
Change-Id: I80ccefa9f449e6fa4bdd56d55cf519e15a213d32
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4795352
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This CL change the message when failed to get THP size from warning
to info, due to it will always use the fallback value on the THP
disabled host environment.
BUG=b:296176956
TEST=CQ
Change-Id: I751d58d12b2ab571e0362221eb35f5d37adf916f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4816246
Reviewed-by: Shin Kawamura <kawasin@google.com>
Commit-Queue: Shengsong Tan <sstan@chromium.org>
Reviewed-by: David Stevens <stevensd@chromium.org>
Partly fixes the mingw --no-default-features build. The network config
code is still broken because net structs are only available when the
slirp feature is enabled on Windows, but the uses of the config structs
are not protected with the same cfg check.
BUG=b:260607247
TEST=cargo build --target x86_64-pc-windows-gnu --no-default-features \
--features=slirp
Change-Id: Ia2f6e7baf769245135ef5ff48ee6fb2c4578f319
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4818989
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
We used to spawn a separate thread to poll gpu_main_display_tube for
messages from the external supervisor, and then wrap those messages in
thread messages (WM_USER_HANDLE_SERVICE_MESSAGE_INTERNAL) that are
posted to the WndProc thread. Now that we have replaced GetMessageW() with
MsgWaitForMultipleObjects(), we can poll that tube and the message queue
at the same time in the WndProc thread, and then route service messages
to WindowMessageDispatcher::process_service_message().
The newly added MsgWaitContext looks similar to EventContext but it is
much simpler, since we don't need to worry about removing handles,
setting timeout, etc. It may get more complex when we start to add more
handles to it (for event devices, etc.), and we might look for ways to
unify the logic with EventContext, but this simple struct should suffice
for now.
Bug: 244489783
Bug: 244491590
Test: flat run emulator
Test: flat run battlestar
Change-Id: I5722523ad1424a1860f2883d33728a1baf9550cc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4719209
Reviewed-by: Pujun Lun <lunpujun@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Kaiyi Li <kaiyili@google.com>
Currently we have two display event loops, one in the GPU worker thread
(mainly used by other platforms; we just poll event devices and the
display closed event there) and one in the WndProc thread (for handling
Windows specific window messages). We are exploring the possibilities of
merging those loops.
MsgWaitForMultipleObjects() allows us to wait for window messages and
other events at the same time. As the first step of our experiments,
we will use it to replace GetMessageW() while keeping everything else
unchanged. Simple performance tests show that there is no regression in
the average latency between we posting a message to the message queue
and we finishing retrieving it from the queue.
Bug: 244489783
Test: Ran GPG
Change-Id: I2ccea348459fa7af0a2ee0ac55e7c817f222a2f6
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4719208
Reviewed-by: Pujun Lun <lunpujun@google.com>
Commit-Queue: Kaiyi Li <kaiyili@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
This will be run by default by CI builders, but not locally as it
invalidates the build cache.
BUG=b:260607247
TEST=dev_container presubmit --no-delta all
Change-Id: I75f3d97bda6881a343364875e53e529a39ff168a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4818791
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
In contrast to docker, podman will not pass-through SIGINT unless
--interactive is specified.
This change enables interactive mode by default if a tty is present,
and allows Luci builders to disable this behavior with the new
--no-interactive flag.
BUG=b:275613273
TEST=dev_container presubmit - then CTRL-C
Change-Id: Ic4900a0669b8c423316196abb289516aa618101d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4818790
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
If PeekMessageW() does retrieve a message, we are responsibe for
dropping it.
Bug: 244489783
Test: Ran GPG
Change-Id: I69250387cc93698513a47d38de52d161a0026692
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4719207
Reviewed-by: Pujun Lun <lunpujun@google.com>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Commit-Queue: Kaiyi Li <kaiyili@google.com>
This flag is used by luci builders to force non-interactive mode.
The logic is temporary to allow us to change the defaults for the
interactive mode / tty logic in a follow-up CL.
BUG=b:275613273
TEST=dev_container --no-interactive run_tests
Change-Id: I93f315d5f65ed184e9c400531b115cc94312c95f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4818789
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Similar to virtio-balloon, feature acks were not being tracked properly
and caused snapshotting to fail. This CL applies the same fix we used on
the balloon device.
BUG=b:297294476
TEST=snapshotting doesn't fail on this device anymore.
Change-Id: I6a4faa088eadf043fcc49479e3e565af18dbaa96
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4814771
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Noah Gold <nkgold@google.com>
This is not used by vm_concierge after https://crrev.com/c/4781331 and
there are no other users that I can find, so remove the unused code.
BUG=b:296117425
TEST=emerge-brya crosvm vm_host_tools
Change-Id: Icd81a977fdfa856522a9cf0ab91d1e5291a5c4e2
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4808256
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Support passing guest fences to host for waiting by using extended
VIRTIO_GPU_CMD_SUBMIT_3D command that replaces previously unused
padding field with a new 'num_in_fences' field. The in-fence IDs
are prepended to the cmdbuf data. There is new corresponding
VIRTIO_GPU_F_FENCE_PASSING feature flag, it must be supported by
guest in order to activate fence passing on host. There is new
VIRTIO_GPU_FLAG_FENCE_SHAREABLE flag for VIRTIO_GPU_CMD_SUBMIT_3D
command that is set if job's fence is shareable.
Fence passing allows guest to offload dma-fence waiting to host,
removing unnecessary stalls in the GPU pipeline. Guest kernel doesn't
have to stall job submission anymore until all in-fences are signalled
on guest side.
This new feature improves GL/VL interop performance up to 2-3x times
for native contexts. It will also allow to share host GPU fences with
host display subsystem easily for explicit fencing of framebuffer flips
within the host kernel display driver, which is useful for virtio-wl.
TEST=none
BUG=b:253779836
Link: https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1138
Change-Id: Ie547614c26ff5f3f75ccf203096e9087827be542
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4679609
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>
Commit-Queue: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Ryan Neph <ryanneph@google.com>
Remove definition of deprecated resource-sync feature in a favor
of upcoming fence-passing alternative feature that will achieve
same goals in a simpler and more generic manner.
TEST=none
BUG=b:253779836
Suggested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Change-Id: I4ec45cb778d055e28e10d204e76b083089b88e61
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4759864
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>
Commit-Queue: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Ryan Neph <ryanneph@google.com>
To help debug some spam of credit update messages, we need to know which
port they are being sent for.
BUG=b:294074231
TEST=ran emulator & verified ports appeared as expected in the new log
message.
Change-Id: Ib9061425f8cd5f2338cc9950ee15c19a85847963
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4814431
Commit-Queue: Noah Gold <nkgold@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
This was only used by crosvm-direct.
BUG=b:279663365
TEST=tools/dev_container tools/presubmit
Change-Id: I5c7b7827da92d9becda339df3e61852675d83009
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4811783
Reviewed-by: Junichi Uekawa <uekawa@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This option was only used by crosvm_direct, which has been removed, and
there are no other tests for this functionality. Remove the dead code
before it bitrots.
BUG=b:279663365
TEST=tools/dev_container tools/presubmit
Change-Id: Ie3b33a5133dafc2425184092179c08455c02fd21
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4811782
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Junichi Uekawa <uekawa@chromium.org>
The BIOS-specific startup information is contained in VcpuInit now.
Change-Id: I698975a33e254165c78a06b97a24c7a36d9f8d1d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4811781
Reviewed-by: Junichi Uekawa <uekawa@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Symlink destination is not by default cached in FUSE. However every
binary fork/exec will try to load the same symlink over and over due to
jemalloc symlinks on ARCVM. It was costing approx 1ms on every app start
up on brya. Turn on the symlink caching behavior so that jemalloc
symlink does not cost as much.
Before, every guest side fork exec used to cause a read/readlinkat/write on the
pcivirtio-fs jemalloc mount point on ARCVM.
1690885281.169417 read(11, "\1\0\0\0\0\0\0\0", 8) = 8 <0.000118>
1690885281.169780 readlinkat(20, "", "narenas:12,tcache:true,lg_tcache"..., 4096) = 39 <0.000103>
1690885281.170075 write(6, "\1\0\0\0\0\0\0\0", 8) = 8 <0.000180>
BUG=b:294008709
TEST=on ARCVM strace pcivirtio-fs process threads for jemalloc:0 and
jemalloc:1 and observe syscalls. # none after being cached.
Pair: keiichiw@chromium.org
Change-Id: I832fb61c7b917f218697cb5807519af81794dc33
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4753629
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Junichi Uekawa <uekawa@chromium.org>
certain files such as "bootindex" and "etc/acpi/rsdp" are automatically
generated by crosvm and are expected by the OVMF guest. We would like an
agrument that acts as a switch to enable fw_cfg so that we can add the
automatically generated files without having to add a file to device.
Towards this end, this CL introduces the --enable-fw-cfg argument.
Additionally, we automatically enable fw_cfg if the user specifies the
bootindex argument from the command line. The bootindex argument is
useless if the "bootorder" file is not made available to the guest b/c
fw_cfg is not present.
BUG=b:283990685
Change-Id: Ie60e27a20a94d7d52bd5af8d301f77f9f83cdcde
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4797982
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Denise Costin <dcostin@google.com>
Commit-Queue: Sebastian Hereu <sebastianhereu@google.com>
