Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move in x112virtgpu as muvm-x11bridge #97

Merged
merged 6 commits into from
Nov 4, 2024
Merged

Move in x112virtgpu as muvm-x11bridge #97

merged 6 commits into from
Nov 4, 2024

Conversation

asahilina
Copy link
Member

Per discussion with @WhatAmISupposedToPutHere and @slp, let's move x112virtgpu development here since muvm is the only consumer.

@WhatAmISupposedToPutHere
Copy link
Collaborator

Acked-by: Sasha Finkelstein fnkl.kernel@gmail.com

@asahilina asahilina force-pushed the x11bridge branch 5 times, most recently from c8f2239 to a0d1c2e Compare October 31, 2024 17:22
@asahilina asahilina force-pushed the x11bridge branch 2 times, most recently from cb1442d to fcf006d Compare October 31, 2024 18:00
@slp
Copy link
Collaborator

slp commented Nov 4, 2024

Do we want to publish muvm-x11bridge independently? If not (and I'd strongly suggest against it, given the past experience with muvm-guest and muvm-server), I'd suggest putting it under crates/muvm/src/x11bridge and to follow the same organization as muvm-guest and muvm-server. This way muvm, composed by the muvm, muvm-guest, muvm-server and muvm-x11bridge binaries, will be published as a single crate to crates.io and as a single package in Fedora.

@teohhanhui
Copy link
Collaborator

If it's moved into the same crate, I think it'd make sense to have a x11bridge feature gate that's included in default.

@asahilina
muvm-x11bridge: Vendor in x112virtgpu and rename it
4d31e80 
This is based on x112virtgpu commit bd78728493d8f19:

WhatAmISupposedToPutHere/x112virtgpu@bd78728

With the following changes:

- Replaced util::create_shm_file() with nix::unistd::mkstemp()
- Replaced clap with bpaf (so we don't have two cmdline parsing crates)
- Fixed the clippy warnings
- Dropped the other targets (preload, wrapper)

Co-developed-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
Signed-off-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
Acked-by: Sasha Finkelstein <fnkl.kernel@gmail.com>
Signed-off-by: Asahi Lina <lina@asahilina.net>
@asahilina
CODEOWNERS: Add @WhatAmISupposedToPutHere
6092323 
Signed-off-by: Asahi Lina <lina@asahilina.net>
@asahilina
Copy link
Member Author

Moved it into the same crate and gated it behind a feature. I can't make the --direct-x11 option disappear entirely if the feature is disabled (the construct!() macro does not support it), so instead I hide it from help and error out if it's enabled (which is probably more user-friendly anyway).

@asahilina
Cargo authors: Add @WhatAmISupposedToPutHere and @asahilina
4b0cfe8 
Signed-off-by: Asahi Lina <lina@asahilina.net>
@asahilina
x11bridge: Fix shmem read race & stale shm files
4fd6c00 
Without the patched libxshmfence, a failed ptrace replacement due to
e.g. `strace glxgears` would leak the shm file.

Fix this and the slightly racy shm data copy in one go, by moving it
into replace_futex_storage() and removing the file before checking for
errors.

Signed-off-by: Asahi Lina <lina@asahilina.net>
@asahilina
x11: Use muvm-x11bridge instead of x112virtgpu
9dffe23 
Signed-off-by: Asahi Lina <lina@asahilina.net>
@asahilina
x11bridge: Gate off aarch64-specific code on !aarch64
d385f36 
Signed-off-by: Asahi Lina <lina@asahilina.net>
@asahilina
Copy link
Member Author

FWIW I also prototyped optionally dropping in sommelier for pure Wayland forwarding, but it's quite broken. It seems --parent mode does not work at all with --virtgpu-channel (it tries to open /dev/wl0 and fails). Per-app mode can be tested by just doing stuff like /usr/bin/sommelier --virtgpu-channel --log-level=3 firefox and seems to mostly work, but not all apps support the fd-based Wayland connection that requires, and it needs a per-app launch and doesn't support apps launching other apps, so it's basically a non-starter.

So I suggest we just leave Wayland out of muvm for now until we have a sommelier alternative. People who want to try the fd-based mode can just launch it explicitly with an app.

slp
slp approved these changes Nov 4, 2024
View reviewed changes
Copy link
Collaborator

@slp slp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@slp slp merged commit 5fae96c into AsahiLinux:main Nov 4, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teohhanhui teohhanhui teohhanhui left review comments

@alyssarosenzweig alyssarosenzweig alyssarosenzweig approved these changes

@slp slp slp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@asahilina @WhatAmISupposedToPutHere @slp @teohhanhui @alyssarosenzweig