Bug 100011 - src/intel_device.c reverses return value meaning of xorg server xf86LoadKernelModule()
Summary: src/intel_device.c reverses return value meaning of xorg server xf86LoadKerne...
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/intel (show other bugs)
Version: git
Hardware: Other FreeBSD
: medium normal
Assignee: Chris Wilson
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-28 20:29 UTC by David Shao
Modified: 2017-02-28 20:53 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description David Shao 2017-02-28 20:29:24 UTC
The function load_i915_kernel_module() in file src/intel_device.c
of xf86-video-intel reverses the meaning of the return value of
xorg server's xf86LoadKernelModule().  xf86LoadKernelModule()
actually returns 0 if the loading fails, non-zero if success.

For example, from xorg server 1.19.1's source code,
file hw/xfree86/os-support/linux/lnx_kmod.c:

 * Input:
 *    modName - name of the kernel module (Ex: "tdfx")
 * Return:
 *    0 for failure, 1 for success
 */
int
xf86LoadKernelModule(const char *modName)

        if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
            return 1;           /* success! */
        }
        else {
            return 0;
        }

And from file hw/xfree86/os-support/bsd/bsd_kmod.c

 * Return:
 *    0 for failure, 1 for success
 */
int
xf86LoadKernelModule(const char *modName)
{
    if (kldload(modName) != -1)
        return 1;
    else
        return 0;
}

On FreeBSD there is an additional complication that if
"i915kms" is added to kernel_module_names, the inverted use
of the return value causes both i915kms.ko and i915.ko to
be loaded.

Therefore I believe a patch similar to the following needs to be made
to src/intel_device.c:

@@ -233,9 +242,8 @@ static int load_i915_kernel_module(void)
 	const char **kn;
 
 	for (kn = kernel_module_names; *kn; kn++)
-		if (xf86LoadKernelModule(*kn) == 0)
+		if (xf86LoadKernelModule(*kn))
 			return 0;
-
 	return -1;
 }
Comment 1 Chris Wilson 2017-02-28 20:53:17 UTC
commit 78d7a09b0343829c81257024b164b0b3764392ac
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Feb 28 20:48:43 2017 +0000

    intel: Fix checking xf86LoadKernelModule for success
    
    Originally introduced in
    
    commit f66e25def3431a900068cc1c23a6b1e8322ef046
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Mon Jun 23 16:14:28 2014 +0100
    
        intel: Wait for the DRM device to load
    
    the legacy path was checking the return xf86LoadKernelModule
    incorrectly. This error was then copy into the common loader in
    
    commit 6a2dcb388e6b549c3175ccfbcd3f1751e25de40a
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Fri Jul 31 10:40:32 2015 +0100
    
        intel: Refactor i915.ko loading support
    
    xf86LoadKernelModule() returns 1 on success and zero on failure.
    
    Reported-by: David Shao <davshao@gmail.com>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100011
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>


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.