flatpak / xdg-desktop-portal

Desktop integration portal
https://flatpak.github.io/xdg-desktop-portal/
GNU Lesser General Public License v2.1
603 stars 194 forks source link

Allow for an XR runtime to access needed resources #953

Closed Obsessee closed 1 year ago

Obsessee commented 1 year ago

Based on discussion between @swick and @jadahl over in the GNOME GitLab. Further continued in the VR support under Wayland issue

From the GNOME side there seems to be a desire to use portals rather than the Wayland protocol for DRM leasing XR devices. A Monado developer chimed in to say they're interested in a one click portal that allows them access to all needed devices (USB/hidraw and DRM).

There are disagreements between Valve and GNOME devs (and potentially other DEs) on using portals for this. This issue could be used as a place for cross-desktop discussion for a universal way to handle XR devices, as well as sharing ideas on what capabilities a potential portal could have.

orowith2os commented 1 year ago

I'd like to mention that a lot of this work would still need to be put in for each compositor - they just need to expose it in a slightly different way to their clients.

It would also be possible for a portal to build on top of the Wayland protocol, I think.

Mikenux commented 1 year ago

From what I've read, it seems to be about taking full control of a VR device and the necessary resources. But what does this mean?

And what about "installing" udev rules? and what are those?

orowith2os commented 1 year ago

Dedicate system resources when using the device?

The software would normally want to have exclusive access to those resources, yes.

Does this involve turning off the desktop monitor when the device is in use?

Not necessary. Displays get noted as non-desktop in the DRM API if e.g. they're from a VR headset (thanks @joshua-ashton for talking me through that :)), and the desktop can lease them to the necessary clients without worry of a malicious client taking over the primary display(s).

You can lease desktop displays, but it's not required here, and in fact would be out of scope.

Does it give an app the ability to flash firmware on it?

It shouldn't, imo, but the Monado dev(s) should comment on that first. If any software needs to do so, it can go through a (hopefully existing in the future) USB portal.

misyltoad commented 1 year ago

It shouldn't, imo, but the Monado dev(s) should comment on that first. If any software needs to do so, it can go through a (hopefully existing in the future) USB portal.

The hidraw devices you get already just allow you to do that.

Mikenux commented 1 year ago

That's the point: it would be better to first have something that only allows normal use of a device, and then ask permission only for features that can potentially harm a device. This would be better than asking to access the device to use it while saying that it could potentially be broken...

misyltoad commented 1 year ago

There is no such thing. Hidraw access in SteamVR is non-negotiable. Its just not possible to make what SteamVR does work any other way.

Mikenux commented 1 year ago

If it is not possible to have such a separation, you can have:

Or just have the appropriate permission, but the app won't be labeled as fully sandboxed.

orowith2os commented 1 year ago

If any of us were going to go for a static permission, we wouldn't be here. And the problem with static permissions here is that it won't work fully. You can't just get device=all and access everything necessary without intervention from the compositor and some udev rules.

A portal could improve the situation in that regard, assuming it would allow hidraw access from a sandbox (and without any udev rules or root, but maybe the portal could have a udev rule dependency there).

And the user knowing what a portal allows is implied by the fact that it's a portal.

Mikenux commented 1 year ago

If a portal is right for you, then let's just talk about that.

And the user knowing what a portal allows is implied by the fact that it's a portal.

I'm not talking about that. I doubt it's implicit to the user that allowing access to full control of a device also means the app can flash firmware onto it. This means that both information should appear in the portal UI.

That said, it might be a good idea to list everything an app can potentially do with a device when it has full control of it.

Note that informing the user well is only my opinion. This requires agreement from all parties (because I'm not even a part of any of them per se).

lordkitsuna commented 1 year ago

Not to put too fine of a point on it but have people learned literally nothing from Android permissions, uac, and every other attempt at some type of user interacting-based permission system.

When you start asking for it for literally God damn everything, and overloading the window with every possible thing this permission might allow. users just click yes without even looking at what it is because they're tired of seeing it. It should be reserved for places where it is actually really important. this just isn't one of them.

also, keeping this separate because I could potentially be wrong. However I'm fairly certain that accessing hidraw already requires making a udev rule which requires superuser privileges to create, so it's not something that can just be applied arbitrarily without the user explicitly granting an application elevated permissions already.

orowith2os commented 1 year ago

@lordkitsuna please try and keep this discussion constructive. You're being unnecessarily hostile.

orowith2os commented 1 year ago

More details on some of the technical stuff here...

Privileges are generally a non-issue for this portal.

Access to DRM resources without privilege escalation is done by way of the DRM leasing protocol on Wayland, but the portal doesn't need to concern itself with that detail here. Why? Because all it needs is a DRM-compatible file descriptor. This portal would work on both Xorg and Wayland because of that - the portal backend can go through the DRM lease protocol on Wayland or some private compositor API, and the Xorg extension on Xorg. It's the same end result: a file descriptor for clients to use to access resources directly, in an unprivileged manner.

For input devices, this is a backend-specific detail too. One could implement this with udev rules or a private compositor API. Again, same end result - raw access to the input devices in an unprivileged manner. The compositor already has exclusive control over raw input devices normally, and it can lease them out too. See the inputfd protocol (draft) for one such example.

So let's talk design. What should the API look like? How should it be exposed to the user? What details need to be conveyed? One could title the prompt with "Allow x to (exclusively) access VR devices?" and leave it at that.

misyltoad commented 1 year ago

A portal for leasing should have nothing to do with USB devices or hidraw fwiw.

Mikenux commented 1 year ago

@lordkitsuna:

When you start asking for it for literally God damn everything, and overloading the window with every possible thing this permission might allow. users just click yes without even looking at what it is because they're tired of seeing it. It should be reserved for places where it is actually really important. this just isn't one of them.

Yes, and I am aware of it. That's why I first asked if it was possible to distinguish the device in a "normal" state (e.g. playing a game with the device) from a state where we can upload a firmware to it. If it were possible, it might be simpler. Also, I'm asking for a list if there are any things (other than allowing the device to change its state from "normal" to "firmware"), which is a start, not the end, because all information is needed before making decisions on a user interface.

@Joshua-Ashton:

Before using leasing, you must gain access to the relevant devices. You cannot have access to all devices and then request the activation of leasing if it can be applied to all devices (is this correct?).

misyltoad commented 1 year ago

Before using leasing, you must gain access to the relevant devices. You cannot have access to all devices and then request the activation of leasing if it can be applied to all devices (is this correct?).

The only device relevant to leasing is the display.

Mikenux commented 1 year ago

Ok, thanks. Anyway, from a UI perspective, it seems to me better to ask for full control of a device (plus what that means) than to request access to a device then (or only ) to ask for leasing for the display.

swick commented 1 year ago

The portal must be able to allow VR compositors running in flatpak to work without any legacy static permissions. This means specifically no --device-all, and thus no /dev/hidraw*.

It is obvious that VR compositors need access to hidraw file descriptions to function. They just cannot come in the form of opening the hosts /dev/hidraw.

hidraw devices on the host must continue to be not user accessible but instead get arbitrated to the active session by something like logind, just like other input devices (with the exception of evdev joysticks for backwards compat right now). For arbitration to work, hidraw needs to be revocable.

To avoid arbitrary apps e.g. bricking the underlying device, hidraw also needs to be firewalled by restricting the functionality to input and output. Those file descriptions would be suitable to be consumed by wayland's inputfd protocol as well as leased out (read: removed from the desktop and moving exclusively to the VR compositor) besides a DRM lease for a VR portal.

swick commented 1 year ago

Does it give an app the ability to flash firmware on it?

The hidraw devices you get already just allow you to do that.

Exactly one of the problems.

There is no such thing. Hidraw access in SteamVR is non-negotiable. Its just not possible to make what SteamVR does work any other way.

No one argues that VR compositors should not have access to hidraw devices. The question here is how to give them access. Just taking a sledge hammer and bringing down walls until your app works is not a sustainable strategy.

davibu commented 1 year ago

Does it give an app the ability to flash firmware on it?

The hidraw devices you get already just allow you to do that.

Exactly one of the problems.

But SteamVR manages the firmware as well, to my understanding SteamVR acts like a driver in userland for VR, the reason being needing GPU-API access (Vulkan/OpenGL) for viewport distortion, while still needing low level access to the hi-draw devices to calculate the player and controller position in real time which is needed for the camera position and viewport distortion. The low latency being very important for avoiding motion sickness.

Especially your demand of updating firmware through at least a different application if not an OS tool instead of SteamVR is very weird. Under your demand a user would need to activate the base stations, the VR headset and the wireless controller to then update them through a desktop interface or different application. And potentially needing to turn them off and on again to then use them in SteamVR. And even then it would also be the question how the user would even know a new firmware is available for a device not always connected. This will most likely result in extremely unfriendly UX or a workaround where SteamVR calls a service which does the flashing, which would probably mean needing another portal for flashing and reacquiring the device.

I would also bring up concerns for your accessing IO for VR through a session service, as these would introduce a lot of latency. Low latency is quite important for VR to avoid motion sickness. Such a demand would need to be compensated, by either even more constrained rendering time for applications or even higher CPU requirements. VR has in it's nature already very high hardware requirements, just demanding to pass all IO through a session layer is quite inconsiderate.

I would also add that the VR compositor might not be run in userland for standalone VR headsets, which would with your demands lead to two separate code paths one for standalone VR and one for userland VR.

I must also add I think your tone is very inappropriate, SteamVR solved a problem of being able to use VR headsets on Linux, it works as it does, because these system you are talking about did not exist or not appropriate for VR use case. Now demanding VR compositors have to work under the constraint you find reasonable without considering the potential domain specific problems or even just asking for them is not nice. Especially while being actively against the drm-lease wayland version, which atleast works under KDE and Sway.

Mikenux commented 1 year ago

A way to separate normal device state from "firmware" state will be needed in the long term.

The thing to understand is this:

That said, if such a separation takes too long and GNOME wants to offer the use of VR to its users, there is the solution of accessing the device and putting the appropriate warnings (perhaps without being too alarming).

wsippel commented 1 year ago

As far as I understand the inner workings, only the runtimes access headsets directly, so either SteamVR or Monado. Those runtimes are also effectively the device drivers for the hardware, and as such need full access to everything. End user VR applications don't access the headsets directly, they go through the runtimes using the OpenVR or OpenXR APIs, which don't expose functions like firmware updates.

swick commented 1 year ago

Regarding firmware updates:

Firmware updates are typically handled by a system daemon such as fwupd. When the device is connected and there is an update available, the user is notified and can update the firmware.

Generally, upgrading the firmware does require a reboot of the device to become active, no matter where it is done.

For a desktop system you will need a working desktop environment to start the VR compositor anyway so I don't see how a firmware update from the system daemon would be any worse then from the app itself.

My main problem with putting firmware updates into a VR compositor is that it now binds a specific device to a specific VR compositor. Want to update your Index? Use Steam VR. Everyone else would have to implement the same firmware update code, find some way to legally redistribute the firmware itself.

With fwupd you can update your firmware and use any VR compositor.

Regarding latency: The daemon would pass file descriptors which are potentially filtered by a bpf program. There is no latency issue to be expected.

Regarding non-desktop systems: you can ship whatever you want on them. I'm totally fine with non-desktop systems which directly open something. I really don't care and neither have the people who want to ship their own special system.

Those runtimes are also effectively the device drivers for the hardware, and as such need full access to everything

They are not device drivers. They need input and output from hidraw devices to function properly and they implement a lot of code to make sense of the sensor readings from those devices, but they are not drivers.

Mikenux commented 1 year ago

Isn't this issue can be closed, now that @swick opens https://github.com/flatpak/xdg-desktop-portal/discussions/1165?