On Thu, 2009-09-17 at 10:24 +1000, Dave Airlie wrote: > As long as someone tests quake 1 still works ;-)
Ok, the DGA situation is actually worse than I thought. Any driver supporting framebuffer resizing that uses the xf86DiDGAInit function will expose the system to potentially catastrophic failures. The xf86DiDGAInit function takes a static address (in the server's address space) for the base of the scanout buffer. This value is given to clients to mmap from /dev/mem themselves. It's important to note here that we're returning a random internal X server address space value to applications and encouraging them to mmap that part of CPU physical address space and write pixels there. Clearly, this code hasn't ever been used by an application as it could never have worked. The other operation that DGA supports is to offer a few rendering operations on the framebuffer as well as the ability to create a pixmap representing the framebuffer for use by regular X drawing functions. I'm betting the only client using this is the DGA2 example application 'skull_dga' -- it's not exactly a compelling use case (a magic mechanism to create a drawable representing the screen just doesn't seem interesting to me). I know that when I googled around, I couldn't find anything other than the DGA man page referencing these functions. I'm sure this could all be fixed with sufficient energy and time, but I suggest that it would be better to just not do this anymore and leave DGA as a way for old applications to switch video modes and get direct mouse input. For these two operations, it seems to work just fine (although I did manage to uncover a bug in the intel driver for DGA mode switching). The trick, however, is how to make these 'safe' operations available while not exposing the 'bad' parts. The xf86DiDGA code doesn't allow this -- you either get all of DGA, including the bad parts, or you get none of it. So, a cautious driver must never initialize DGA for fear that it will be run against an old X server. The solution that I came up with is to have xf86CrtcInit call xf86DiDGAInit and remove the call to xf86DiDGAInit from the video driver. Thus, the driver running against an old server gets no DGA (ether bad or safe parts) and when run against a new server gets just the safe parts. I've tested this with gl-117 and warsow, both of which appear to use DGA input. Additional testing is encouraged, of course. I don't think this patch is 'optional' for either X server 1.7 or 1.6.4; it removes the potential for some fairly serious system-wide consequences while appearing to keep existing applications running. -keith
From 8fd84838477db6195f60aa00a029dd8988b69cbc Mon Sep 17 00:00:00 2001 From: Keith Packard <[email protected]> Date: Fri, 18 Sep 2009 21:12:17 -0700 Subject: [PATCH] xfree86/modes: Remove all framebuffer support from DGA This removes all rendering and mapping code from xf86DiDGA, leaving just mode setting and raw input device access. The mapping code didn't have the offset within /dev/mem for the frame buffer and the pixmap support assumed that the framebuffer was never reallocated. Signed-off-by: Keith Packard <[email protected]> --- hw/xfree86/modes/xf86Crtc.c | 7 ++ hw/xfree86/modes/xf86DiDGA.c | 119 +++++++--------------------------------- hw/xfree86/modes/xf86RandR12.c | 8 +-- 3 files changed, 29 insertions(+), 105 deletions(-) diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c index c6dfd8c..c1e31e0 100644 --- a/hw/xfree86/modes/xf86Crtc.c +++ b/hw/xfree86/modes/xf86Crtc.c @@ -805,6 +805,9 @@ xf86CrtcScreenInit (ScreenPtr screen) config->CloseScreen = screen->CloseScreen; screen->CloseScreen = xf86CrtcCloseScreen; +#ifdef XFreeXDGA + xf86DiDGAInit(screen, 0); +#endif #ifdef RANDR_13_INTERFACE return RANDR_INTERFACE_VERSION; #else @@ -1923,6 +1926,10 @@ xf86SetScrnInfoModes (ScrnInfoPtr scrn) } } scrn->currentMode = scrn->modes; +#ifdef XFreeXDGA + if (scrn->pScreen) + xf86DiDGAReInit(scrn->pScreen); +#endif } static void diff --git a/hw/xfree86/modes/xf86DiDGA.c b/hw/xfree86/modes/xf86DiDGA.c index 0964cef..0f7b834 100644 --- a/hw/xfree86/modes/xf86DiDGA.c +++ b/hw/xfree86/modes/xf86DiDGA.c @@ -72,8 +72,7 @@ xf86_dga_get_modes (ScreenPtr pScreen) mode = modes + num++; mode->mode = display_mode; - mode->flags = DGA_CONCURRENT_ACCESS | DGA_PIXMAP_AVAILABLE; - mode->flags |= DGA_FILL_RECT | DGA_BLIT_RECT; + mode->flags = DGA_CONCURRENT_ACCESS; if (display_mode->Flags & V_DBLSCAN) mode->flags |= DGA_DOUBLESCAN; if (display_mode->Flags & V_INTERLACE) @@ -91,14 +90,14 @@ xf86_dga_get_modes (ScreenPtr pScreen) mode->yViewportStep = 1; mode->viewportFlags = DGA_FLIP_RETRACE; mode->offset = 0; - mode->address = (unsigned char *) xf86_config->dga_address; - mode->bytesPerScanline = xf86_config->dga_stride; - mode->imageWidth = xf86_config->dga_width; - mode->imageHeight = xf86_config->dga_height; + mode->address = 0; + mode->imageWidth = mode->viewportWidth; + mode->imageHeight = mode->viewportHeight; + mode->bytesPerScanline = (mode->imageWidth * scrn->bitsPerPixel) >> 3; mode->pixmapWidth = mode->imageWidth; mode->pixmapHeight = mode->imageHeight; - mode->maxViewportX = mode->imageWidth - mode->viewportWidth; - mode->maxViewportY = mode->imageHeight - mode->viewportHeight; + mode->maxViewportX = 0; + mode->maxViewportY = 0; display_mode = display_mode->next; if (display_mode == scrn->modes) @@ -149,93 +148,11 @@ xf86_dga_set_viewport(ScrnInfoPtr scrn, int x, int y, int flags) } static Bool -xf86_dga_get_drawable_and_gc (ScrnInfoPtr scrn, DrawablePtr *ppDrawable, GCPtr *ppGC) -{ - ScreenPtr pScreen = scrn->pScreen; - xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn); - PixmapPtr pPixmap; - GCPtr pGC; - - pPixmap = GetScratchPixmapHeader (pScreen, xf86_config->dga_width, xf86_config->dga_height, - scrn->depth, scrn->bitsPerPixel, xf86_config->dga_stride, - (char *) scrn->memPhysBase + scrn->fbOffset); - if (!pPixmap) - return FALSE; - pGC = GetScratchGC (scrn->depth, pScreen); - if (!pGC) - { - FreeScratchPixmapHeader (pPixmap); - return FALSE; - } - *ppDrawable = &pPixmap->drawable; - *ppGC = pGC; - return TRUE; -} - -static void -xf86_dga_release_drawable_and_gc (ScrnInfoPtr scrn, DrawablePtr pDrawable, GCPtr pGC) -{ - FreeScratchGC (pGC); - FreeScratchPixmapHeader ((PixmapPtr) pDrawable); -} - -static void -xf86_dga_fill_rect(ScrnInfoPtr scrn, int x, int y, int w, int h, unsigned long color) -{ - GCPtr pGC; - DrawablePtr pDrawable; - XID vals[1]; - xRectangle r; - - if (!xf86_dga_get_drawable_and_gc (scrn, &pDrawable, &pGC)) - return; - vals[0] = color; - ChangeGC (pGC, GCForeground, vals); - ValidateGC (pDrawable, pGC); - r.x = x; - r.y = y; - r.width = w; - r.height = h; - pGC->ops->PolyFillRect (pDrawable, pGC, 1, &r); - xf86_dga_release_drawable_and_gc (scrn, pDrawable, pGC); -} - -static void -xf86_dga_sync(ScrnInfoPtr scrn) -{ - ScreenPtr pScreen = scrn->pScreen; - WindowPtr pRoot = WindowTable [pScreen->myNum]; - char buffer[4]; - - pScreen->GetImage (&pRoot->drawable, 0, 0, 1, 1, ZPixmap, ~0L, buffer); -} - -static void -xf86_dga_blit_rect(ScrnInfoPtr scrn, int srcx, int srcy, int w, int h, int dstx, int dsty) -{ - DrawablePtr pDrawable; - GCPtr pGC; - - if (!xf86_dga_get_drawable_and_gc (scrn, &pDrawable, &pGC)) - return; - ValidateGC (pDrawable, pGC); - pGC->ops->CopyArea (pDrawable, pDrawable, pGC, srcx, srcy, w, h, dstx, dsty); - xf86_dga_release_drawable_and_gc (scrn, pDrawable, pGC); -} - -static Bool xf86_dga_open_framebuffer(ScrnInfoPtr scrn, char **name, unsigned char **mem, int *size, int *offset, int *flags) { - xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn); - - *size = xf86_config->dga_stride * xf86_config->dga_height; - *mem = (unsigned char *) (xf86_config->dga_address); - *offset = 0; - *flags = DGA_NEED_ROOT; - - return TRUE; + return FALSE; } static void @@ -249,9 +166,9 @@ static DGAFunctionRec xf86_dga_funcs = { xf86_dga_set_mode, xf86_dga_set_viewport, xf86_dga_get_viewport, - xf86_dga_sync, - xf86_dga_fill_rect, - xf86_dga_blit_rect, + NULL, + NULL, + NULL, NULL }; @@ -261,6 +178,9 @@ xf86DiDGAReInit (ScreenPtr pScreen) ScrnInfoPtr scrn = xf86Screens[pScreen->myNum]; xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn); + if (!DGAAvailable(pScreen->myNum)) + return TRUE; + if (!xf86_dga_get_modes (pScreen)) return FALSE; @@ -273,11 +193,14 @@ xf86DiDGAInit (ScreenPtr pScreen, unsigned long dga_address) ScrnInfoPtr scrn = xf86Screens[pScreen->myNum]; xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn); + if (DGAAvailable(pScreen->myNum)) + return TRUE; + xf86_config->dga_flags = 0; - xf86_config->dga_address = dga_address; - xf86_config->dga_width = scrn->virtualX; - xf86_config->dga_height = scrn->virtualY; - xf86_config->dga_stride = scrn->displayWidth * scrn->bitsPerPixel >> 3; + xf86_config->dga_address = 0; + xf86_config->dga_width = 0; + xf86_config->dga_height = 0; + xf86_config->dga_stride = 0; if (!xf86_dga_get_modes (pScreen)) return FALSE; diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c index c2465bc..6ea9d26 100644 --- a/hw/xfree86/modes/xf86RandR12.c +++ b/hw/xfree86/modes/xf86RandR12.c @@ -1,5 +1,5 @@ /* - * Copyright � 2002 Keith Packard, member of The XFree86 Project, Inc. + * Copyright © 2002 Keith Packard, member of The XFree86 Project, Inc. * * Permission to use, copy, modify, distribute, and sell this software and its * documentation for any purpose is hereby granted without fee, provided that @@ -467,9 +467,6 @@ xf86RandR12GetInfo (ScreenPtr pScreen, Rotation *rotations) { xf86ProbeOutputModes (scrp, 0, 0); xf86SetScrnInfoModes (scrp); -#ifdef XFreeXDGA - xf86DiDGAReInit (pScreen); -#endif } for (mode = scrp->modes; ; mode = mode->next) @@ -1528,9 +1525,6 @@ xf86RandR12GetInfo12 (ScreenPtr pScreen, Rotation *rotations) return TRUE; xf86ProbeOutputModes (pScrn, 0, 0); xf86SetScrnInfoModes (pScrn); -#ifdef XFreeXDGA - xf86DiDGAReInit (pScreen); -#endif return xf86RandR12SetInfo12 (pScreen); } -- 1.6.3.3
signature.asc
Description: This is a digitally signed message part
_______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
