Add libseat support (run Xserver as ordinary user) #935
Conversation
cepelinas9000
requested review from
a team,
ONykyf,
algrid,
artist4xlibre,
b-aaz,
metux,
mikedld,
notbabaisyou,
probonopd,
stefan11111 and
steven-vd
|
@cepelinas9000 Did you have a look at comment 3220492682 · Add seatd/libseat support · Issue #202 · X11Libre/xserver and especially LeePen/xlibre - xlibre - Devuan git store? |
Now I have looked :) Missed that comment, Mark have fixed additional blocking bugs (in this PR Xserver refuses to start when there are no '-seat' argument given) |
Excellent. 😉 I added a "Fixes: ..." line to the description above so the feature request ticket gets a link to here. |
| if (xorgHWOpenConsole) | ||
| xf86CloseConsole(); | ||
|
|
||
| seatd_libseat_fini(); |
There was a problem hiding this comment.
does that compile w/o seatd enabled ?
There was a problem hiding this comment.
Yes, the CI tests where passing. The original patch have #ifdef and following style code (see, seatd-libseat.h:
#ifdef SEATD_LIBSEAT
...
#else
#define seatd_libseat_init()
#define seatd_libseat_fini()
#endif
|
The terminology "rootless" is ambiguous here, because it usually refers to an Xserver without an actual root window. |
cepelinas9000
changed the title
I changed PR title to less confusing |
|
@cepelinas9000 I think the target branch of this pull request should be changed from |
I change this to master when will be ready. Now it temporary measure to not deal with intermediate merge conflicts🤞. |
|
Yes it is available on FreeBSD. |
| if (may_fail) | ||
| return 0; | ||
| FatalError("parse_vt_settings: Cannot open /dev/tty0 (%s)\n", | ||
| FatalError("parse_vt_settings: Cannot open /dev/tty0 (%s), maybe missing fot ex. '-seat seat0' parameter? (in case trying to run in rootless mode) \n", |
There was a problem hiding this comment.
"fot ex." ? is this a spelling error?
|
This is a large change, In my opinion it needs as large of a change in documentation too, things like new flags and configs should be noted. Another thing that I forgot to mention is that FreeBSD has seatd but elogind is NOT available. |
cepelinas9000
force-pushed
the
seatd-rootless
branch
from
488ed4a to
0ec080d
Compare
|
This is progress update, there need more work and testing:
|
Yes, I concur! |
|
@cepelinas9000 Would you mind to involve the Devuan devs or did you already? Basically the patchset against Xorg was created by them, wasn't it? |
I invited the Devuan devs by posting to libera/#devuan-dev/ Thursday, 2025-09-18. |
I don't know anything about libseat/seatd, but would like to try it. Some questions on the topic:
|
To answer thesese questions, I want to test myself too (time shortage etc...) and familiarize myself. Libseat doing following:
p.s seatd only open device nodes which recognizes: /dev/dri/, hid devices, evdev devices |
cepelinas9000
force-pushed
the
seatd-rootless
branch
2 times, most recently
from
02f5d3c to
409243e
Compare
|
Now if libseat active, the modesetting driver tries to open gpu device node trough Added short image, as long as you can access seatd.sock - you can start Xorg server as any user: 
Refactored code should work on freebsd (or any bsd) as long there is |
| #ifdef SEATD_LIBSEAT | ||
| // try get device from seatd arbiter | ||
| if (dev){ | ||
| fd = seatd_libseat_open_graphics(dev); |
There was a problem hiding this comment.
@X11Libre/wranglers ping. @metux @stefan11111
I took liberty and increased public sdk surface with seatd_libseat_open_graphics export (it asks seatd server to perform open operation and via fd passing returns file fd), my question is: shouldn't (in far future) be some sort api* o deal with device opening. Or I missed something?
- for example amdgpu calls open directly, same intel
|
I just wanted to comment that I tried this PR on my Void Linux system and it works perfectly on greetd + tuigreet! I'm so happy that I can now run rootless X11 on my machine ❤️. Many thanks to the XLibre community for this beautiful work. |
cepelinas9000
force-pushed
the
seatd-rootless
branch
from
409243e to
07ee126
Compare
|
moved opening device node down - now tries open as current process firstly then tries call seatd and improved failure case message (see: https://github.com/X11Libre/xserver/pull/935/files#diff-54b8e9df3c29f5a215f2ffe63e1d897036c6263c7699a3be0d1bbb53e7e040f6R120 ) |
hw/xfree86/common/xf86Init.c
Outdated
| xf86OpenConsole(); | ||
| else | ||
| if (xorgHWOpenConsole) { | ||
| if (!seatd_libseat_controls_session()) { |
There was a problem hiding this comment.
Indentation looks wrong on github here for some reason.
Did you use tabs instead of spaces?
There was a problem hiding this comment.
That my mixup didn't notice that original diff had tabs here, i just copied from it.
hw/xfree86/common/xf86Init.c
Outdated
| if (xorgHWOpenConsole) { | ||
| if (!seatd_libseat_controls_session()) { | ||
| xf86CloseConsole(); | ||
| } |
There was a problem hiding this comment.
Indentation looks wrong on github here for some reason.
Did you use tabs instead of spaces?
| } | ||
|
|
||
| #ifdef SEATD_LIBSEAT | ||
| // try get device from seatd arbiter |
|
|
||
| #ifdef SEATD_LIBSEAT | ||
| // try get device from seatd arbiter | ||
| if (seatd_libseat_controls_session()){ |
There was a problem hiding this comment.
This should only be done if we don't have an fd from above. Otherwise, we have a leak.
There was a problem hiding this comment.
Also, does this have to be implemented by the drivers?
It can't be handled by the X server alone?
There was a problem hiding this comment.
This should only be done if we don't have an fd from above. Otherwise, we have a leak.
Good catch, i moved that block code without thinking...
There was a problem hiding this comment.
Also, does this have to be implemented by the drivers? It can't be handled by the X server alone?
I'm wondering too, but going through drivers all of them manually calls too open, or missing I something?
There was a problem hiding this comment.
I'm not familiar with this, but perhaps you can take a look at the logind code?
I don't see any logind-specific code in the drivers.
Also, how exactly does fd = seatd_libseat_open_graphics(dev); work?
You can't just pass fd's from the daemon to the X server.
Does it open a socket to the seatd process and pass the data to/from the real fd? What about the ioctl's we use on the graphics device?
There was a problem hiding this comment.
I'm not familiar with this, but perhaps you can take a look at the logind code?
I don't see any logind-specific code in the drivers.
When everything works™, device is opened with same seatd_libseat_open_graphics(dev), during get_drm_info probing ( hw/xfree86/os-support/shared/drm_platform.c#L26 ) in driver with help xf86_platform_device_odev_attributes (search in driver.c) call.
But if there is some failure, then driver call hw_open function and tries manually open device. The systemd-logind code doesn't handle this case at all...
Also, how exactly does fd = seatd_libseat_open_graphics(dev); work?
You can't just pass fd's from the daemon to the X server.
It work exactly passing fd's: opens fd from elevated process, via unix socket sends fd to xserver. The twist is that it saves fd inside and additionally calls drop master / revoke ioctls (why it works - 'dup semantics here', you can play with example https://gist.github.com/cepelinas9000/8f30a41894adb84c2786b4d658ac73c9 )
Does it open a socket to the seatd process and pass the data to/from the real fd? What about the ioctl's we use on the graphics device?
Yes it opens session during initialization, for seatd unix socket or dbus connection if systemd-logind. The ioctl business as usual - it return fd and that all. Obviously if seat/logind crashes - worse case scenario during chvt you won't return from tty text mode.
hw/xfree86/os-support/meson.build
Outdated
| ] | ||
| endif | ||
|
|
||
| if host_machine.system() == 'linux' or host_machine.system().endswith('bsd') |
There was a problem hiding this comment.
@b-aaz Is this the correct way to check for bsd?
|
|
||
| if (fd == -1) | ||
| if (fd == -1) { | ||
| // Try opening the path directly |
|
|
||
| #include "seatd-libseat.h" | ||
|
|
||
| // ============ libseat client adapter ====================== |
| xf86EnableInputDeviceForVTSwitch(pInfo); | ||
| } | ||
| } | ||
| xf86InputEnableVTProbe(); // Add any paused input devices |
| } | ||
| } | ||
| xf86InputEnableVTProbe(); // Add any paused input devices | ||
| xf86platformVTProbe(); // Probe for outputs |
| free(xfmt); | ||
| } | ||
|
|
||
| // ============== seatd-libseat.h API functions ============= |
|
@cepelinas9000 This pr is intended for maint? Not master? |
To master - when it will be +/- ok then i rebase it. |
This is initial patch for libseat. Technicaly verbatim code from Devuan xorg server moved to os-support/shared directory as it can support other oses than linux. The original repository: https://git.devuan.org/devuan/xorg-server.git The patch between fc24510f17e89a5bbac1065abab758a4d0c42634 and 6bf6238 Co-Authored-By: Mark Hindley <[email protected]> Co-Authored-By: Ralph Ronnquist <[email protected]> Signed-off-By: Tautvis <[email protected]>
Format libseat.c style to align with Xlibre Signed-off-By: Tautvis <[email protected]>
Refactor libseat code for Xlibre and add to meson.build:
* update meson for seat-libseat
* refactor code:
* seatd-libseat.c - add aditional private include headers
* add cast to libseat_set_log_handler argument 1 for (libseat_log_func),
for warning silencing
* make seatd_libseat_init to accept keeptty state as parameter
* include xf86Events, xf86_priv - make visible xf86VTLeave for seatd code (it
is neccesary because seatd can force console leave)
Signed-off-By: Tautvis <[email protected]>
Add xf86VTKeepTtyIsSet function to export KeepTty state. When seatd activates, it takes control of current vt (in global sense)!, this code only activates sesion control code when XServer started using same vt (for more context see c88a325 commit ). Signed-off-By: Tautvis <[email protected]>
Add necessary code into Xserver to support libseat (this time enabling functionality). Code used from Devuan repository + local changes to make functional in Xlibre Co-Authored-By: Mark Hindley <[email protected]> Co-Authored-By: Ralph Ronnquist <[email protected]> Signed-off-By: Tautvis <[email protected]>
The device node with seatd like with systemd-logind are opened during probe (see get_drm_info in hw/xfree86/os-support/shared/drm_platform.c), but in case of failure, the modesetting driver tries to open device node directly (open_hw function in modesetting/driver.c). This patch aligns functionality and as last resort ask's seatd arbiter to open device for us. To make it functional seatd_libseat_controls_session, seatd_libseat_open_graphics needs _X_EXPORT Signed-off-By: Tautvis <[email protected]>
cepelinas9000
force-pushed
the
seatd-rootless
branch
from
07ee126 to
727dc53
Compare
|
|
@stefan11111 @X11Libre/dev ping |
|
Other than a couple of unnecessary newlines seemingly added for no reason or an unrelated reason (eg. fixing whitespace consistency should probably be a separate mr) (which aren't a huge issue in my eyes), it looks good to me. |
| #ifdef SEATD_LIBSEAT | ||
| #include <xf86Xinput.h> | ||
| extern int seatd_libseat_init(void); | ||
| extern int seatd_libseat_init(BOOL KeepTty_state); |
There was a problem hiding this comment.
IMHO, we can safely use stdbool here.
No need to use protocol header types here, if we're not directly dealing with the X11 protocol.
| dbus_required = get_option('systemd_logind') == 'true' | ||
| dbus_dep = dependency('dbus-1', version: '>= 1.0', required: dbus_required) | ||
|
|
||
| seat_libseat_required = get_option('seatd_libseat') == 'true' |
There was a problem hiding this comment.
could we perhaps move those to the hw/xfree86/common meson file ?
| extern void seatd_libseat_close_device(InputInfoPtr p); | ||
| extern int seatd_libseat_switch_session(int session); | ||
| extern Bool seatd_libseat_controls_session(void); | ||
| extern _X_EXPORT Bool seatd_libseat_controls_session(void); |
There was a problem hiding this comment.
please add a comment that these symbols are only allowed for in-tree drivers and NOT part of public SDK.
|
|
||
| #ifdef SEATD_LIBSEAT | ||
| /* try get device from seatd arbiter */ | ||
| if (fd == -1 && seatd_libseat_controls_session()){ |
There was a problem hiding this comment.
perhaps move the whole thing into the seatd support code ? (instead of potentially duplicated within drivers)
8 participants
Adds libseat support, it is adapted devuan patches for Xlibre.
Firstly, currently draft mode - technically it is nearly verbatium patch (with Xlibre adaptations) from https://git.devuan.org/devuan/xorg-server/compare/suites/experimental...support-libseat-upstream
Secondly, with extra work and additional patches: it should be possible to start xserver from AppImage/chroot - as long there are working device enumeration (or nodes configured beforehand - seatd opens device nodes for xserver and sents via unix socket as fd's)
@b-aaz there are mentioned at https://sr.ht/~kennylevinsen/seatd/ FreeBSD support it working or in early stages (I didn't check deeply) ?
Fixes: #202
To test it, you need libseat and seatd running (doesn't matter as service - you can start as root via tmux). Only problem, that if you have already running X server libseatd will try revoke display driver node and fails (you need initialize with X running).
If you don't want to install seatd:
then just start if
but don't forget when compiling Xlibre you need to provide seatd directory:
and when running LD_LIBRARY_PATH
these parameters necessary for now (that why draft):
edit: added test instructions