| Message ID | 20180301233805.4779-1-keithp@keithp.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
"modesetting: Allow a DRM fd to be passed on command line with -masterfd"
( rev:
1
)
in
X.org (DEPRECATED - USE GITLAB) |
diff --git a/hw/xfree86/common/xf86Globals.c b/hw/xfree86/common/xf86Globals.c index e890f05c2..7cc7401a2 100644 --- a/hw/xfree86/common/xf86Globals.c +++ b/hw/xfree86/common/xf86Globals.c @@ -53,6 +53,8 @@ DevPrivateKeyRec xf86ScreenKeyRec; ScrnInfoPtr *xf86Screens = NULL; /* List of ScrnInfos */ ScrnInfoPtr *xf86GPUScreens = NULL; /* List of ScrnInfos */ +int xf86DRMMasterFd = -1; /* Command line argument for DRM master file descriptor */ + const unsigned char byte_reversed[256] = { 0x00, 0x80, 0x40, 0xc0, 0x20, 0xa0, 0x60, 0xe0, 0x10, 0x90, 0x50, 0xd0, 0x30, 0xb0, 0x70, 0xf0, diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h index 4fe2b5f33..393af3900 100644 --- a/hw/xfree86/common/xf86Priv.h +++ b/hw/xfree86/common/xf86Priv.h @@ -93,6 +93,7 @@ extern _X_EXPORT int xf86LogVerbose; /* log file verbosity level */ extern ScrnInfoPtr *xf86GPUScreens; /* List of pointers to ScrnInfoRecs */ extern int xf86NumGPUScreens; +extern _X_EXPORT int xf86DRMMasterFd; /* Command line argument for DRM master file descriptor */ #ifndef DEFAULT_VERBOSE #define DEFAULT_VERBOSE 0 #endif diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c index ec2aa9a27..1d84e113d 100644 --- a/hw/xfree86/drivers/modesetting/driver.c +++ b/hw/xfree86/drivers/modesetting/driver.c @@ -36,6 +36,7 @@ #include <unistd.h> #include <fcntl.h> #include "xf86.h" +#include "xf86Priv.h" #include "xf86_OSproc.h" #include "compiler.h" #include "xf86Pci.h" @@ -194,11 +195,24 @@ modesettingEntPtr ms_ent_priv(ScrnInfoPtr scrn) return pPriv->ptr; } +static int +get_passed_fd(void) +{ + if (xf86DRMMasterFd >= 0) { + xf86DrvMsg(-1, X_INFO, "Using passed DRM master file descriptor %d\n", xf86DRMMasterFd); + return dup(xf86DRMMasterFd); + } + return -1; +} + static int open_hw(const char *dev) { int fd; + if ((fd = get_passed_fd()) != -1) + return fd; + if (dev) fd = open(dev, O_RDWR, 0); else { @@ -822,6 +836,12 @@ ms_get_drm_master_fd(ScrnInfoPtr pScrn) return TRUE; } + ms->fd_passed = FALSE; + if ((ms->fd = get_passed_fd()) >= 0) { + ms->fd_passed = TRUE; + return TRUE; + } + #ifdef XSERVER_PLATFORM_BUS if (pEnt->location.type == BUS_PLATFORM) { if (pEnt->location.id.plat->flags & XF86_PDEV_SERVER_FD) @@ -1495,6 +1515,9 @@ SetMaster(ScrnInfoPtr pScrn) (ms->pEnt->location.id.plat->flags & XF86_PDEV_SERVER_FD)) return TRUE; + if (ms->fd_passed) + return TRUE; + ret = drmSetMaster(ms->fd); if (ret) xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "drmSetMaster failed: %s\n", @@ -1746,7 +1769,8 @@ LeaveVT(ScrnInfoPtr pScrn) (ms->pEnt->location.id.plat->flags & XF86_PDEV_SERVER_FD)) return; - drmDropMaster(ms->fd); + if (!ms->fd_passed) + drmDropMaster(ms->fd); } /* diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h index fe835918b..6be51e01b 100644 --- a/hw/xfree86/drivers/modesetting/driver.h +++ b/hw/xfree86/drivers/modesetting/driver.h @@ -84,6 +84,7 @@ struct ms_drm_queue { typedef struct _modesettingRec { int fd; + Bool fd_passed; int Chipset; EntityInfoPtr pEnt; diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c index 9e5ddcd50..b654f7618 100644 --- a/hw/xfree86/os-support/linux/lnx_init.c +++ b/hw/xfree86/os-support/linux/lnx_init.c @@ -356,6 +356,13 @@ xf86CloseConsole(void) close(xf86Info.consoleFd); /* make the vt-manager happy */ } +#define CHECK_FOR_REQUIRED_ARGUMENT() \ + if (((i + 1) >= argc) || (!argv[i + 1])) { \ + ErrorF("Required argument to %s not specified\n", argv[i]); \ + UseMsg(); \ + FatalError("Required argument to %s not specified\n", argv[i]); \ + } + int xf86ProcessArgument(int argc, char *argv[], int i) { @@ -376,6 +383,19 @@ xf86ProcessArgument(int argc, char *argv[], int i) } return 1; } + + if (!strcmp(argv[i], "-masterfd")) { + CHECK_FOR_REQUIRED_ARGUMENT(); + if (xf86PrivsElevated()) + FatalError("\nCannot specify -masterfd when server is setuid/setgid\n"); + if (sscanf(argv[++i], "%d", &xf86DRMMasterFd) != 1) { + UseMsg(); + xf86DRMMasterFd = -1; + return 0; + } + return 2; + } + return 0; } @@ -385,4 +405,6 @@ xf86UseMsg(void) ErrorF("vtXX use the specified VT number\n"); ErrorF("-keeptty "); ErrorF("don't detach controlling tty (for debugging only)\n"); + if (!xf86PrivsElevated()) + ErrorF("-masterfd <fd> use the specified fd as the DRM master fd\n"); }
Looks good! One nitpick I'm not 100% sure about: On Thu, 2018-03-01 at 15:38 -0800, Keith Packard wrote: > This lets an application open a suitable DRM device and pass the file > descriptor to the mode setting driver through an X server command line > option, '-masterfd'. > > There's a companion application, xlease, which creates a DRM master by > leasing an output from another X server. That is available at > > git clone git://people.freedesktop.org/~keithp/xlease > > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > hw/xfree86/common/xf86Globals.c | 2 ++ > hw/xfree86/common/xf86Priv.h | 1 + > hw/xfree86/drivers/modesetting/driver.c | 26 +++++++++++++++++++++++++- > hw/xfree86/drivers/modesetting/driver.h | 1 + > hw/xfree86/os-support/linux/lnx_init.c | 22 ++++++++++++++++++++++ > 5 files changed, 51 insertions(+), 1 deletion(-) > > diff --git a/hw/xfree86/common/xf86Globals.c > b/hw/xfree86/common/xf86Globals.c > index e890f05c2..7cc7401a2 100644 > --- a/hw/xfree86/common/xf86Globals.c > +++ b/hw/xfree86/common/xf86Globals.c > @@ -53,6 +53,8 @@ DevPrivateKeyRec xf86ScreenKeyRec; > ScrnInfoPtr *xf86Screens = NULL; /* List of ScrnInfos */ > ScrnInfoPtr *xf86GPUScreens = NULL; /* List of ScrnInfos */ > > +int xf86DRMMasterFd = -1; /* Command line argument for DRM master file > descriptor */ > + > const unsigned char byte_reversed[256] = { > 0x00, 0x80, 0x40, 0xc0, 0x20, 0xa0, 0x60, 0xe0, > 0x10, 0x90, 0x50, 0xd0, 0x30, 0xb0, 0x70, 0xf0, > diff --git a/hw/xfree86/common/xf86Priv.h b/hw/xfree86/common/xf86Priv.h > index 4fe2b5f33..393af3900 100644 > --- a/hw/xfree86/common/xf86Priv.h > +++ b/hw/xfree86/common/xf86Priv.h > @@ -93,6 +93,7 @@ extern _X_EXPORT int xf86LogVerbose; /* log file > verbosity level */ > > extern ScrnInfoPtr *xf86GPUScreens; /* List of pointers to > ScrnInfoRecs */ > extern int xf86NumGPUScreens; > +extern _X_EXPORT int xf86DRMMasterFd; /* Command line argument > for DRM master file descriptor */ > #ifndef DEFAULT_VERBOSE > #define DEFAULT_VERBOSE 0 > #endif > diff --git a/hw/xfree86/drivers/modesetting/driver.c > b/hw/xfree86/drivers/modesetting/driver.c > index ec2aa9a27..1d84e113d 100644 > --- a/hw/xfree86/drivers/modesetting/driver.c > +++ b/hw/xfree86/drivers/modesetting/driver.c > @@ -36,6 +36,7 @@ > #include <unistd.h> > #include <fcntl.h> > #include "xf86.h" > +#include "xf86Priv.h" > #include "xf86_OSproc.h" > #include "compiler.h" > #include "xf86Pci.h" > @@ -194,11 +195,24 @@ modesettingEntPtr ms_ent_priv(ScrnInfoPtr scrn) > return pPriv->ptr; > } > > +static int > +get_passed_fd(void) > +{ > + if (xf86DRMMasterFd >= 0) { > + xf86DrvMsg(-1, X_INFO, "Using passed DRM master file descriptor > %d\n", xf86DRMMasterFd); > + return dup(xf86DRMMasterFd); > + } > + return -1; > +} > + > static int > open_hw(const char *dev) > { > int fd; > > + if ((fd = get_passed_fd()) != -1) > + return fd; > + > if (dev) > fd = open(dev, O_RDWR, 0); > else { > @@ -822,6 +836,12 @@ ms_get_drm_master_fd(ScrnInfoPtr pScrn) > return TRUE; > } > > + ms->fd_passed = FALSE; > + if ((ms->fd = get_passed_fd()) >= 0) { > + ms->fd_passed = TRUE; > + return TRUE; > + } > + > #ifdef XSERVER_PLATFORM_BUS > if (pEnt->location.type == BUS_PLATFORM) { > if (pEnt->location.id.plat->flags & XF86_PDEV_SERVER_FD) > @@ -1495,6 +1515,9 @@ SetMaster(ScrnInfoPtr pScrn) > (ms->pEnt->location.id.plat->flags & XF86_PDEV_SERVER_FD)) > return TRUE; > > + if (ms->fd_passed) > + return TRUE; > + > ret = drmSetMaster(ms->fd); > if (ret) > xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "drmSetMaster failed: %s\n", > @@ -1746,7 +1769,8 @@ LeaveVT(ScrnInfoPtr pScrn) > (ms->pEnt->location.id.plat->flags & XF86_PDEV_SERVER_FD)) > return; > > - drmDropMaster(ms->fd); > + if (!ms->fd_passed) > + drmDropMaster(ms->fd); > } > > /* > diff --git a/hw/xfree86/drivers/modesetting/driver.h > b/hw/xfree86/drivers/modesetting/driver.h > index fe835918b..6be51e01b 100644 > --- a/hw/xfree86/drivers/modesetting/driver.h > +++ b/hw/xfree86/drivers/modesetting/driver.h > @@ -84,6 +84,7 @@ struct ms_drm_queue { > > typedef struct _modesettingRec { > int fd; > + Bool fd_passed; > > int Chipset; > EntityInfoPtr pEnt; > diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os- > support/linux/lnx_init.c > index 9e5ddcd50..b654f7618 100644 > --- a/hw/xfree86/os-support/linux/lnx_init.c > +++ b/hw/xfree86/os-support/linux/lnx_init.c > @@ -356,6 +356,13 @@ xf86CloseConsole(void) > close(xf86Info.consoleFd); /* make the vt-manager happy */ > } > > +#define CHECK_FOR_REQUIRED_ARGUMENT() \ > + if (((i + 1) >= argc) || (!argv[i + 1])) { > \ > + ErrorF("Required argument to %s not specified\n", argv[i]); \ > + UseMsg(); \ > + FatalError("Required argument to %s not specified\n", argv[i]); Is the double printing of "Required argument to %s not specified" here intentional? > \ > + } > + > int > xf86ProcessArgument(int argc, char *argv[], int i) > { > @@ -376,6 +383,19 @@ xf86ProcessArgument(int argc, char *argv[], int i) > } > return 1; > } > + > + if (!strcmp(argv[i], "-masterfd")) { > + CHECK_FOR_REQUIRED_ARGUMENT(); > + if (xf86PrivsElevated()) > + FatalError("\nCannot specify -masterfd when server is > setuid/setgid\n"); > + if (sscanf(argv[++i], "%d", &xf86DRMMasterFd) != 1) { > + UseMsg(); > + xf86DRMMasterFd = -1; > + return 0; > + } > + return 2; > + } > + > return 0; > } > > @@ -385,4 +405,6 @@ xf86UseMsg(void) > ErrorF("vtXX use the specified VT number\n"); > ErrorF("-keeptty "); > ErrorF("don't detach controlling tty (for debugging only)\n"); > + if (!xf86PrivsElevated()) > + ErrorF("-masterfd <fd> use the specified fd as the DRM > master fd\n"); I think it would be a better idea for us to show this argument description unconditionally, along with adding a note about setuid/setgid not being allowed with it > }
This lets an application open a suitable DRM device and pass the file descriptor to the mode setting driver through an X server command line option, '-masterfd'. There's a companion application, xlease, which creates a DRM master by leasing an output from another X server. That is available at git clone git://people.freedesktop.org/~keithp/xlease Signed-off-by: Keith Packard <keithp@keithp.com> --- hw/xfree86/common/xf86Globals.c | 2 ++ hw/xfree86/common/xf86Priv.h | 1 + hw/xfree86/drivers/modesetting/driver.c | 26 +++++++++++++++++++++++++- hw/xfree86/drivers/modesetting/driver.h | 1 + hw/xfree86/os-support/linux/lnx_init.c | 22 ++++++++++++++++++++++ 5 files changed, 51 insertions(+), 1 deletion(-)