Bug 21022

Summary:

mga/G450: Driver segfaults during startup when using MergedFB

Product:

xorg

Reporter:

Stefan Dirsch <sndirsch>

Component:

Driver/mga

Assignee:

Tilman Sauerbeck <tilman>

Status:

RESOLVED FIXED

QA Contact:

Xorg Project Team <xorg-team>

Severity:

normal

Priority:

medium

CC:

eich, idr, mat

Version:

git

Hardware:

Other

OS:

All

Whiteboard:

i915 platform:

i915 features:

Attachments:

Description	Flags
xf86-video-mga-hal.diff	none
HALlib-4.1.tar.gz	none

Description Stefan Dirsch 2009-04-02 19:16:49 UTC

This is on a G450 with current git using MergedFB.

# X

[...]
(EE) MGA(0):  -- Hardware Cursor disabled.

Backtrace:
0: X(xf86SigHandler+0x79) [0x80cbbf9]
1: [0xffffe400]
2: /usr/lib/xorg/modules//drivers/mga_drv.so(mga_read_and_process_bios+0x37) [0xb7affaf7]
3: /usr/lib/xorg/modules//drivers/mga_drv.so(MGAPreInitMergedFB+0x3c8) [0xb7b0f648]
4: /usr/lib/xorg/modules//drivers/mga_drv.so [0xb7b0a8be]
5: X(InitOutput+0x99f) [0x80a8aaf]
6: X(main+0x279) [0x8070da9]
7: /lib/libc.so.6(__libc_start_main+0xe5) [0xb7c46705]
8: X [0x8070391]

Fatal server error:
Caught signal 11.  Server aborting

Aborted

After reverting the following commit driver startup works again.

commit 074a4aa1985219910a96b022368067e3ed5641e6
Author: Ian Romanick <idr@us.ibm.com>
Date:   Fri May 30 18:23:59 2008 -0700

    Initialize default BIOS values from a data table instead of from code

It crashes in mga_read_and_process_bios()
-->    (void) memcpy(& pMga->bios, & pMga->chip_attribs->default_bios_values,
                 sizeof(struct mga_bios_values));

the second time this function is called. First time pMga->chip_attribs is
ok. Second time it's a NULL pointer. :-(

Comment 1 Stefan Dirsch 2009-04-02 19:39:45 UTC

Well, reverting the patch isn't that easy any longer, since new elements for memory probing have been added to struct mga_device_attributes meanwhile. So a proper fix would be appreciated. Unfortunately I can't figure out the bugzilla account of Ian Romanick.

Comment 2 Stefan Dirsch 2009-04-02 19:43:33 UTC

In case nobody can still remember how to configure MergedFB with mga driver. Just
as an example.

Section "Device"
  BoardName    "G450 DH G450"
  BusID        "2:0:0"
  Driver       "mga"
  Identifier   "Device[0]"
  Option       "MergedFB"
  Option       "Monitor2HSync" "30-80"
  Option       "MetaModes" "1280x1024-1280x1024"
  Option       "Monitor2Position" "LeftOf"
  Option       "Monitor2VRefresh" "59-75"
  Screen       0
  VendorName   "Matrox"
EndSection

Comment 3 Stefan Dirsch 2009-04-02 19:51:13 UTC

I'm attaching the HAL patch I'm using and the HALlib itself. HALlib needs to be extracted in src/. This command line is required to link mgahal module after building the driver:

gcc -m32 \
    -o /usr/lib/xorg/modules/drivers/mgahal_drv.so \
    -shared \
    .libs/mga_halmod.o .libs/clientlx.o HALlib/mgaHALlib.a \
    -Wl,-Bstatic -Wl,-Bdynamic

Comment 4 Stefan Dirsch 2009-04-02 19:53:26 UTC

Created attachment 24484 [details] [review]
xf86-video-mga-hal.diff

HAL patch

Comment 5 Stefan Dirsch 2009-04-02 19:54:19 UTC

Created attachment 24485 [details]
HALlib-4.1.tar.gz

HAL library.

Comment 6 Stefan Dirsch 2009-04-16 06:45:20 UTC

I just committed a patch, which works for me.

commit 32bc0bcec815a363a47b9e7337d06370baf0c0d4
Author: Stefan Dirsch <sndirsch@suse.de>
Date:   Thu Apr 16 15:34:12 2009 +0200

    Prevent MergedFB setups from crashing.
    
    Second time mga_read_and_process_bios() is called pMga->chip_attribs
    is a NULL pointer for some reason. (#21022)

diff --git a/src/mga_bios.c b/src/mga_bios.c
index b78890e..c015077 100644
--- a/src/mga_bios.c
+++ b/src/mga_bios.c
@@ -326,8 +326,9 @@ Bool mga_read_and_process_bios( ScrnInfoPtr pScrn )
      * isn't found or can't be read we'll still have some reasonable values
      * to use.
      */
-    (void) memcpy(& pMga->bios, & pMga->chip_attribs->default_bios_values,
-                 sizeof(struct mga_bios_values));
+    if (pMga->chip_attribs)
+       (void) memcpy(& pMga->bios, & pMga->chip_attribs->default_bios_values,
+                      sizeof(struct mga_bios_values));
 
 
     /* If the BIOS address was probed, it was found from the PCI config space

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.