Bug 21022 - mga/G450: Driver segfaults during startup when using MergedFB
Summary: mga/G450: Driver segfaults during startup when using MergedFB
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/mga (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Tilman Sauerbeck
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-02 19:16 UTC by Stefan Dirsch
Modified: 2009-04-16 06:45 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
xf86-video-mga-hal.diff (3.95 KB, patch)
2009-04-02 19:53 UTC, Stefan Dirsch
no flags Details | Splinter Review
HALlib-4.1.tar.gz (134.29 KB, application/octet-stream)
2009-04-02 19:54 UTC, Stefan Dirsch
no flags Details

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.