From 9a04fadaf63e6b1e1dd92172eda1ac04eb15c01b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 8 Jun 2025 22:12:31 +0200 Subject: [PATCH 1/3] Do not allocate grants for framebuffer by default If glamor is not used (which is default), FBBasePriv seems to be unused. GUI agent doesn't share the root window with GUI daemon. If glamor is used, FBBasePriv probably unused in practice too - the GUI agent doesn't support sharing part of the pixmap anyway, but I'm not 100% sure about it (see qubes_create_screen_resources() - the "pixmap" of the root window is passed to glamor, and it isn't clear if it wouldn't end up used in some window that is handled by the GUI agent). Based on this observation, use normal memory for the framebuffer. The main benefit is not having the framebuffer mlock()-ed, which helps especially with small VMs. But also, allocating it as normal userspace memory gives the kernel more flexibility in finding memory for it (it doesn't need to be physically continuous, etc). Due to the glamor case, leave the grant-based FBBase allocation in the code, but disabled by default. And leave a #define to re-enable it, with appropriate comment. Fixes QubesOS/qubes-issues#9992 --- xf86-video-dummy/src/dummy_driver.c | 38 +++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/xf86-video-dummy/src/dummy_driver.c b/xf86-video-dummy/src/dummy_driver.c index 283aece3..6a560efd 100644 --- a/xf86-video-dummy/src/dummy_driver.c +++ b/xf86-video-dummy/src/dummy_driver.c @@ -87,6 +87,16 @@ static Bool dummyDriverFunc(ScrnInfoPtr pScrn, xorgDriverFuncOp op, /* static void DUMMYDisplayPowerManagementSet(ScrnInfoPtr pScrn, */ /* int PowerManagementMode, int flags); */ +/* + * Make FBBase grant-table backed if glamor is initialized. Currently it's + * believed to be unneeded, but if you get the + * "can't dump window without grant table allocation" message for a root + * window, set DUMMY_GLAMOR_GNT_BACKED_FBBASE to 1 and report an issue. + * See discussion at: + * https://github.com/QubesOS/qubes-gui-agent-linux/pull/233 + */ +#define DUMMY_GLAMOR_GNT_BACKED_FBBASE 0 + #define DUMMY_VERSION 4000 #define DUMMY_NAME "DUMMYQBS" #define DUMMY_DRIVER_NAME "dummyqbs" @@ -915,9 +925,10 @@ qubes_create_screen_resources(ScreenPtr pScreen) { if (ret) { PixmapPtr pixmap = pScreen->GetScreenPixmap(pScreen); if (dPtr->glamor) - glamor_egl_create_textured_pixmap_from_gbm_bo(pixmap, dPtr->front_bo, FALSE); - xf86_qubes_pixmap_set_private(pixmap, - dPtr->FBBasePriv); + glamor_egl_create_textured_pixmap_from_gbm_bo(pixmap, dPtr->front_bo, FALSE); + if (dPtr->FBBasePriv) + xf86_qubes_pixmap_set_private(pixmap, + dPtr->FBBasePriv); } return ret; @@ -1015,10 +1026,17 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL) return FALSE; } - dPtr->FBBasePriv = qubes_alloc_pixmap_private(pScrn->videoRam * 1024); - if (dPtr->FBBasePriv == NULL) - return FALSE; - dPtr->FBBase = (void *) dPtr->FBBasePriv->data; + if (DUMMY_GLAMOR_GNT_BACKED_FBBASE && dPtr->glamor) { + dPtr->FBBasePriv = qubes_alloc_pixmap_private(pScrn->videoRam * 1024); + if (dPtr->FBBasePriv == NULL) + return FALSE; + dPtr->FBBase = (void *) dPtr->FBBasePriv->data; + } else { + dPtr->FBBase = calloc(1, pScrn->videoRam * 1024); + if (dPtr->FBBase == NULL) + return FALSE; + } + /* * next we save the current state and setup the first mode @@ -1290,8 +1308,12 @@ DUMMYCloseScreen(CLOSE_SCREEN_ARGS_DECL) if (dPtr->FBBasePriv) { xf86_qubes_free_pixmap_private(dPtr->FBBasePriv); dPtr->FBBasePriv = NULL; - dPtr->FBBase = NULL; + } else { + if (dPtr->FBBase) { + free(dPtr->FBBase); + } } + dPtr->FBBase = NULL; } if (dPtr->CursorInfo) From 7cf04f3e8ba2304d1d1b0e6d661a41c6868f9701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 11 Jun 2025 20:21:07 +0200 Subject: [PATCH 2/3] Drop DGA extension support It appears to be broken forever. Since nobody complained, it looks like nobody actually need it. Drop it. See discussion at https://github.com/QubesOS/qubes-gui-agent-linux/pull/233 --- xf86-video-dummy/configure.ac | 8 -- xf86-video-dummy/src/Makefile.am | 5 - xf86-video-dummy/src/dummy.h | 7 -- xf86-video-dummy/src/dummy_dga.c | 175 ---------------------------- xf86-video-dummy/src/dummy_driver.c | 8 -- 5 files changed, 203 deletions(-) delete mode 100644 xf86-video-dummy/src/dummy_dga.c diff --git a/xf86-video-dummy/configure.ac b/xf86-video-dummy/configure.ac index 5463382c..98b56141 100644 --- a/xf86-video-dummy/configure.ac +++ b/xf86-video-dummy/configure.ac @@ -44,7 +44,6 @@ LT_INIT AH_TOP([#include "xorg-server.h"]) # Define a configure option for an alternate module directory -AC_ARG_ENABLE(dga, AS_HELP_STRING([--disable-dga], [Build DGA extension (default: yes)]), [DGA=$enableval], [DGA=yes]) AC_ARG_WITH(xorg-module-dir, [ --with-xorg-module-dir=DIR ], [ moduledir="$withval" ], [ moduledir="$libdir/xorg/modules" ]) @@ -56,13 +55,6 @@ XORG_DRIVER_CHECK_EXT(RANDR, randrproto) XORG_DRIVER_CHECK_EXT(RENDER, renderproto) XORG_DRIVER_CHECK_EXT(XV, videoproto) -if test "x$DGA" = xyes; then - XORG_DRIVER_CHECK_EXT(XFreeXDGA, xf86dgaproto) - AC_DEFINE(USE_DGA, 1, [Support DGA extension]) -fi -AC_SUBST([DGA]) -AM_CONDITIONAL([DGA], [test "x$DGA" = xyes]) - # Obtain compiler/linker options for the driver dependencies PKG_CHECK_MODULES(XORG, [xorg-server >= 1.0.99.901] xproto fontsproto $REQUIRED_MODULES) diff --git a/xf86-video-dummy/src/Makefile.am b/xf86-video-dummy/src/Makefile.am index 237a3f16..fa90b383 100644 --- a/xf86-video-dummy/src/Makefile.am +++ b/xf86-video-dummy/src/Makefile.am @@ -39,8 +39,3 @@ dummyqbs_drv_la_SOURCES = \ dummy_driver.c \ dummy.h \ ../../gui-agent/list.c - -if DGA -dummyqbs_drv_la_SOURCES += \ - dummy_dga.c -endif diff --git a/xf86-video-dummy/src/dummy.h b/xf86-video-dummy/src/dummy.h index 0ccae928..280f0725 100644 --- a/xf86-video-dummy/src/dummy.h +++ b/xf86-video-dummy/src/dummy.h @@ -34,9 +34,6 @@ extern Bool DUMMYCursorInit(ScreenPtr pScrn); extern void DUMMYShowCursor(ScrnInfoPtr pScrn); extern void DUMMYHideCursor(ScrnInfoPtr pScrn); -/* in dummy_dga.c */ -Bool DUMMYDGAInit(ScreenPtr pScreen); - /* in dummy_video.c */ extern void DUMMYInitVideo(ScreenPtr pScreen); @@ -53,10 +50,6 @@ struct gbm_bo; typedef struct dummyRec { - DGAModePtr DGAModes; - int numDGAModes; - Bool DGAactive; - int DGAViewportStatus; /* options */ OptionInfoPtr Options; Bool swCursor; diff --git a/xf86-video-dummy/src/dummy_dga.c b/xf86-video-dummy/src/dummy_dga.c deleted file mode 100644 index d16d09f1..00000000 --- a/xf86-video-dummy/src/dummy_dga.c +++ /dev/null @@ -1,175 +0,0 @@ -#ifdef HAVE_CONFIG_H -#include "config.h" -#endif - -#include "xf86.h" -#include "xf86_OSproc.h" -#include "dgaproc.h" -#include "dummy.h" - -static Bool DUMMY_OpenFramebuffer(ScrnInfoPtr, char **, unsigned char **, - int *, int *, int *); -static Bool DUMMY_SetMode(ScrnInfoPtr, DGAModePtr); -static int DUMMY_GetViewport(ScrnInfoPtr); -static void DUMMY_SetViewport(ScrnInfoPtr, int, int, int); - -static -DGAFunctionRec DUMMYDGAFuncs = { - DUMMY_OpenFramebuffer, - NULL, - DUMMY_SetMode, - DUMMY_SetViewport, - DUMMY_GetViewport, - NULL, - NULL, - NULL, -#if 0 - DUMMY_BlitTransRect -#else - NULL -#endif -}; - -Bool -DUMMYDGAInit(ScreenPtr pScreen) -{ - ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen); - DUMMYPtr pDUMMY = DUMMYPTR(pScrn); - DGAModePtr modes = NULL, newmodes = NULL, currentMode; - DisplayModePtr pMode, firstMode; - int Bpp = pScrn->bitsPerPixel >> 3; - int num = 0, imlines, pixlines; - - imlines = (pScrn->videoRam * 1024) / - (pScrn->displayWidth * (pScrn->bitsPerPixel >> 3)); - - pixlines = imlines; - - pMode = firstMode = pScrn->modes; - - while(pMode) { - - newmodes = realloc(modes, (num + 1) * sizeof(DGAModeRec)); - - if(!newmodes) { - free(modes); - return FALSE; - } - modes = newmodes; - - currentMode = modes + num; - num++; - - currentMode->mode = pMode; - currentMode->flags = DGA_CONCURRENT_ACCESS | DGA_PIXMAP_AVAILABLE; - if(pMode->Flags & V_DBLSCAN) - currentMode->flags |= DGA_DOUBLESCAN; - if(pMode->Flags & V_INTERLACE) - currentMode->flags |= DGA_INTERLACED; - currentMode->byteOrder = pScrn->imageByteOrder; - currentMode->depth = pScrn->depth; - currentMode->bitsPerPixel = pScrn->bitsPerPixel; - currentMode->red_mask = pScrn->mask.red; - currentMode->green_mask = pScrn->mask.green; - currentMode->blue_mask = pScrn->mask.blue; - currentMode->visualClass = (Bpp == 1) ? PseudoColor : TrueColor; - currentMode->viewportWidth = pMode->HDisplay; - currentMode->viewportHeight = pMode->VDisplay; - currentMode->xViewportStep = 1; - currentMode->yViewportStep = 1; - currentMode->viewportFlags = DGA_FLIP_RETRACE; - currentMode->offset = 0; - currentMode->address = (unsigned char *)pDUMMY->FBBase; - - currentMode->bytesPerScanline = - ((pScrn->displayWidth * Bpp) + 3) & ~3L; - currentMode->imageWidth = pScrn->displayWidth; - currentMode->imageHeight = imlines; - currentMode->pixmapWidth = currentMode->imageWidth; - currentMode->pixmapHeight = pixlines; - currentMode->maxViewportX = currentMode->imageWidth - - currentMode->viewportWidth; - currentMode->maxViewportY = currentMode->imageHeight - - currentMode->viewportHeight; - - pMode = pMode->next; - if(pMode == firstMode) - break; - } - - pDUMMY->numDGAModes = num; - pDUMMY->DGAModes = modes; - - return DGAInit(pScreen, &DUMMYDGAFuncs, modes, num); -} - -static DisplayModePtr DUMMYSavedDGAModes[MAXSCREENS]; - -static Bool -DUMMY_SetMode( - ScrnInfoPtr pScrn, - DGAModePtr pMode -){ - int index = pScrn->pScreen->myNum; - DUMMYPtr pDUMMY = DUMMYPTR(pScrn); - - if(!pMode) { /* restore the original mode */ - if(pDUMMY->DGAactive) { - pScrn->currentMode = DUMMYSavedDGAModes[index]; - DUMMYSwitchMode(SWITCH_MODE_ARGS(pScrn, pScrn->currentMode)); - DUMMYAdjustFrame(ADJUST_FRAME_ARGS(pScrn, 0, 0)); - pDUMMY->DGAactive = FALSE; - } - } else { - if(!pDUMMY->DGAactive) { /* save the old parameters */ - DUMMYSavedDGAModes[index] = pScrn->currentMode; - pDUMMY->DGAactive = TRUE; - } - - DUMMYSwitchMode(SWITCH_MODE_ARGS(pScrn, pMode->mode)); - } - - return TRUE; -} - -static int -DUMMY_GetViewport( - ScrnInfoPtr pScrn -){ - DUMMYPtr pDUMMY = DUMMYPTR(pScrn); - - return pDUMMY->DGAViewportStatus; -} - -static void -DUMMY_SetViewport( - ScrnInfoPtr pScrn, - int x, int y, - int flags -){ - DUMMYPtr pDUMMY = DUMMYPTR(pScrn); - - DUMMYAdjustFrame(ADJUST_FRAME_ARGS(pScrn, x, y)); - pDUMMY->DGAViewportStatus = 0; -} - - -static Bool -DUMMY_OpenFramebuffer( - ScrnInfoPtr pScrn, - char **name, - unsigned char **mem, - int *size, - int *offset, - int *flags -){ - DUMMYPtr pDUMMY = DUMMYPTR(pScrn); - - *name = NULL; /* no special device */ - *mem = (unsigned char*)pDUMMY->FBBase; - *size = pScrn->videoRam * 1024; - *offset = 0; - *flags = DGA_NEED_ROOT; - - return TRUE; -} diff --git a/xf86-video-dummy/src/dummy_driver.c b/xf86-video-dummy/src/dummy_driver.c index 6a560efd..6b3dce86 100644 --- a/xf86-video-dummy/src/dummy_driver.c +++ b/xf86-video-dummy/src/dummy_driver.c @@ -48,10 +48,6 @@ #include #include "scrnintstr.h" #include "servermd.h" -#ifdef USE_DGA -#define _XF86DGA_SERVER_ -#include -#endif /* glamor support */ #define GLAMOR_FOR_XORG @@ -1137,10 +1133,6 @@ DUMMYScreenInit(SCREEN_INIT_ARGS_DECL) dPtr->CreateScreenResources = pScreen->CreateScreenResources; pScreen->CreateScreenResources = qubes_create_screen_resources; -#ifdef USE_DGA - DUMMYDGAInit(pScreen); -#endif - /* initialize XRANDR */ xf86CrtcConfigInit(pScrn, &DUMMYCrtcConfigFuncs); /* FIXME */ From 49420c53aa0d212c5e2b7fcd02b6cdac9f4e68ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Sun, 8 Jun 2025 22:31:15 +0200 Subject: [PATCH 3/3] Increase screen pixmap size beyond initial video RAM if needed If FBBase is not backed by grant tables, it's now possible to reallocate it. This is especially useful when new external display is connected and the overall resolution is increased. Note the realloc() call may change FBBase address. In practice, FBBase is given to fbScreenInit, and it does save it at some point (fbScreenInit->fbFinishScreenInit->miScreenInit->miScreenDevPrivateInit), but then it's used only to attach it to the screen pixmap, which is updated via ModifyPixmapHeader() call few lines below anyway. This change adjusts also pScrn->videoRam. But fortunately X server seems to use it only at startup, and only to validate initial modes list. Fixes QubesOS/qubes-issues#7448 --- xf86-video-dummy/src/dummy_driver.c | 36 +++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/xf86-video-dummy/src/dummy_driver.c b/xf86-video-dummy/src/dummy_driver.c index 6b3dce86..5af1c6fe 100644 --- a/xf86-video-dummy/src/dummy_driver.c +++ b/xf86-video-dummy/src/dummy_driver.c @@ -416,13 +416,41 @@ Bool DUMMYAdjustScreenPixmap(ScrnInfoPtr pScrn, int width, int height) "Failed to get the screen pixmap.\n"); return FALSE; } - if (cbLine > UINT32_MAX || cbLine * height > pScrn->videoRam * 1024) - { + if (cbLine > UINT32_MAX) { xf86DrvMsg(pScrn->scrnIndex, X_ERROR, - "Unable to set up a virtual screen size of %dx%d with %d Kb of video memory available. Please increase the video memory size.\n", - width, height, pScrn->videoRam); + "Unable to set up a virtual screen size of %dx%d, cbLine " + "overflow\n", + width, height); return FALSE; } + if (cbLine * height > pScrn->videoRam * 1024) { + if (!dPtr->FBBasePriv) { + /* If there is no backing grant entries, it's easy enough to extend + */ + pointer *newFBBase; + size_t new_size = (cbLine * height + 1023) & ~1023; + + newFBBase = realloc(dPtr->FBBase, new_size); + if (!newFBBase) { + xf86DrvMsg(pScrn->scrnIndex, X_ERROR, + "Unable to set up a virtual screen size of %dx%d, " + "cannot allocate memory (%zu bytes)\n", + width, height, new_size); + return FALSE; + } + memset((char*)newFBBase + pScrn->videoRam * 1024, + 0, + new_size - pScrn->videoRam * 1024); + dPtr->FBBase = newFBBase; + pScrn->videoRam = new_size / 1024; + } else { + xf86DrvMsg(pScrn->scrnIndex, X_ERROR, + "Unable to set up a virtual screen size of %dx%d with %d Kb of video memory available. Please increase the video memory size.\n", + width, height, pScrn->videoRam); + return FALSE; + } + } + pScreen->ModifyPixmapHeader(pPixmap, width, height, pScrn->depth, xf86GetBppFromDepth(pScrn, pScrn->depth), cbLine, dPtr->FBBase);