[PATCH] Automatically load video drivers based on PCI ID's
Aaron Plattner
aplattner at nvidia.com
Thu Feb 21 18:17:39 PST 2008
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi David,
Sorry to take so long to look at this again. This looks better, I just
have a few remaining suggestions.
I'd check that strlen(direntry->d_name) > 3, or else bad stuff could happen
when you do strcmp(&direntry->d_name[len-3], ".so").
strncpy followed by strlen is not safe if the source string (in this case
direntry->d_name) is too long because it won't write a terminating '\0'.
You've got an extraneous "drec = NULL" right before "drec =
(DriverPtr)...".
Instead of
retval = (char*)xalloc(strlen(drec->driverName));
strcpy(retval, drec->driverName);
you could simply do
retval = xstrdup(direc->driverName);
For constructing the driver path, XNFprintf looks pretty convenient, if it
works:
driverpath = XNFprintf("%s/drivers/", xf86ModulePath);
Otherwise, you should probably use xnfalloc or better yet, fail gracefully
if xalloc fails.
I don't think
char *matches[matchmax]
works on all compilers, though I could be wrong.
- -- Aaron
On Tue, Feb 05, 2008 at 09:19:29PM -0500, David Nusinow wrote:
> Drivers must be pci-reworked and claim a set of ID's in their driver rec.
>
> This patch also removes previous attempts at driver autodetection,
> including my misguided text file based version and the previous heuristic
> based only on the vendor ID.
> ---
> configure.ac | 1 -
> hw/xfree86/common/xf86AutoConfig.c | 271 +++++++++++++-----------------------
> hw/xfree86/common/xf86Config.h | 1 -
> include/xorg-config.h.in | 3 -
> 4 files changed, 99 insertions(+), 177 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 566ddcb..7ee97c8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1043,7 +1043,6 @@ if test "x$XDMAUTH" = xyes; then
> fi
>
> AC_DEFINE_DIR(COMPILEDDEFAULTFONTPATH, FONTPATH, [Default font path])
> -AC_DEFINE_DIR(PCI_TXT_IDS_PATH, PCI_TXT_IDS_DIR, [Default PCI text file ID path])
> AC_DEFINE_DIR(SERVER_MISC_CONFIG_PATH, SERVERCONFIG, [Server miscellaneous config path])
> AC_DEFINE_DIR(BASE_FONT_PATH, FONTDIR, [Default base font path])
> AC_DEFINE_DIR(DRI_DRIVER_PATH, DRI_DRIVER_PATH, [Default DRI driver path])
> diff --git a/hw/xfree86/common/xf86AutoConfig.c b/hw/xfree86/common/xf86AutoConfig.c
> index c6e1972..1c74904 100644
> --- a/hw/xfree86/common/xf86AutoConfig.c
> +++ b/hw/xfree86/common/xf86AutoConfig.c
> @@ -33,6 +33,7 @@
> #include <xorg-config.h>
> #endif
>
> +#include <dlfcn.h>
> #include "xf86.h"
> #include "xf86Parser.h"
> #include "xf86tokens.h"
> @@ -136,61 +137,6 @@ AppendToConfig(const char *s)
> AppendToList(s, &builtinConfig, &builtinLines);
> }
>
> -static const char *
> -videoPtrToDriverName(struct pci_device *dev)
> -{
> - /*
> - * things not handled yet:
> - * amd/cyrix/nsc
> - * xgi
> - */
> -
> - switch (dev->vendor_id)
> - {
> - case 0x1142: return "apm";
> - case 0xedd8: return "ark";
> - case 0x1a03: return "ast";
> - case 0x1002: return "ati";
> - case 0x102c: return "chips";
> - case 0x1013: return "cirrus";
> - case 0x8086:
> - if ((dev->device_id == 0x00d1) || (dev->device_id == 0x7800))
> - return "i740";
> - else return "intel";
> - case 0x102b: return "mga";
> - case 0x10c8: return "neomagic";
> - case 0x105d: return "i128";
> - case 0x10de: case 0x12d2: return "nv";
> - case 0x1163: return "rendition";
> - case 0x5333:
> - switch (dev->device_id)
> - {
> - case 0x88d0: case 0x88d1: case 0x88f0: case 0x8811:
> - case 0x8812: case 0x8814: case 0x8901:
> - return "s3";
> - case 0x5631: case 0x883d: case 0x8a01: case 0x8a10:
> - case 0x8c01: case 0x8c03: case 0x8904: case 0x8a13:
> - return "s3virge";
> - default:
> - return "savage";
> - }
> - case 0x1039: return "sis";
> - case 0x126f: return "siliconmotion";
> - case 0x121a:
> - if (dev->device_id < 0x0003)
> - return "voodoo";
> - else
> - return "tdfx";
> - case 0x3d3d: return "glint";
> - case 0x1023: return "trident";
> - case 0x100c: return "tseng";
> - case 0x1106: return "via";
> - case 0x15ad: return "vmware";
> - default: break;
> - }
> - return NULL;
> -}
> -
> Bool
> xf86AutoConfig(void)
> {
> @@ -250,22 +196,6 @@ xf86AutoConfig(void)
> return (ret == CONFIG_OK);
> }
>
> -int
> -xchomp(char *line)
> -{
> - size_t len = 0;
> -
> - if (!line) {
> - return 1;
> - }
> -
> - len = strlen(line);
> - if (line[len - 1] == '\n' && len > 0) {
> - line[len - 1] = '\0';
> - }
> - return 0;
> -}
> -
> GDevPtr
> autoConfigDevice(GDevPtr preconf_device)
> {
> @@ -307,109 +237,108 @@ autoConfigDevice(GDevPtr preconf_device)
> return ptr;
> }
>
> -#ifdef __linux__
> -/* This function is used to provide a workaround for binary drivers that
> - * don't export their PCI ID's properly. If distros don't end up using this
> - * feature it can and should be removed because the symbol-based resolution
> - * scheme should be the primary one */
> +static char*
> +driverClaimsID(struct dirent *direntry, struct pci_device *match, char *driverpath)
> +{
> + char *retval = NULL;
> + char *modulepath = NULL;
> + char *drivername = NULL;
> + int len;
> + int buflen = 256;
> + int tmp, i;
> + void *handle;
> + const struct pci_id_match *driver_pci;
> + DriverPtr drec;
> +
> + if (!direntry)
> + goto end;
> + len = strlen(direntry->d_name);
> + if ((direntry->d_name[0] == '.') ||
> + (strcmp(&(direntry->d_name[len-3]), ".so") != 0 ) ) {
> + goto end;
> + }
> +
> + drivername = (char*)xalloc(buflen);
> + modulepath = (char*)xalloc(strlen(driverpath) + buflen);
> +
> + strcpy(modulepath, driverpath);
> + strncat(modulepath, direntry->d_name, buflen);
> + handle = dlopen(modulepath, RTLD_LAZY);
> + if (!handle) {
> + xf86Msg(X_ERROR, "Could not dlopen the module %s\n", direntry->d_name);
> + goto end;
> + }
> +
> + /* Figure out driver name to get the right symbol */
> + strncpy(drivername, direntry->d_name, buflen);
> + tmp = strlen(drivername);
> + for (i = 0 ; i < tmp ; i++) {
> + if (isalnum(drivername[i]))
> + drivername[i] = toupper(drivername[i]);
> + else
> + drivername[i] = '\0';
> + }
> + /* Get the DriverRec and look for a PCI ID match */
> + drec = NULL;
> + drec = (DriverPtr)dlsym(handle, drivername);
> + if (!drec) {
> + goto end;
> + }
> +
> + driver_pci = drec->supported_devices;
> + while (driver_pci->vendor_id != 0) {
> + if (PCI_ID_COMPARE(driver_pci->vendor_id, match->vendor_id) &&
> + PCI_ID_COMPARE(driver_pci->device_id, match->device_id) &&
> + PCI_ID_COMPARE(driver_pci->subvendor_id, match->subvendor_id) &&
> + PCI_ID_COMPARE(driver_pci->subdevice_id, match->subdevice_id)) {
> + xf86Msg(X_DEFAULT, "Driver %s supports the primary video device.\n", drec->driverName);
> + retval = (char*)xalloc(strlen(drec->driverName));
> + strcpy(retval, drec->driverName);
> + break;
> + }
> + driver_pci++;
> + }
> + dlclose(handle);
> +
> + end:
> + xfree(modulepath);
> + xfree(drivername);
> + return retval;
> +}
> +
> static void
> -matchDriverFromFiles (char** matches, uint16_t match_vendor, uint16_t match_chip)
> +matchDriverFromSymbols (char** matches, int matchmax, struct pci_device *match)
> {
> - DIR *idsdir;
> - FILE *fp;
> + char *driverpath, *matcheddriver;
> + DIR *dir;
> struct dirent *direntry;
> - char *line = NULL;
> - size_t len;
> - ssize_t read;
> - char path_name[256], vendor_str[5], chip_str[5];
> - uint16_t vendor, chip;
> - int i, j;
> -
> - idsdir = opendir(PCI_TXT_IDS_PATH);
> - if (idsdir) {
> - xf86Msg(X_INFO, "Scanning %s directory for additional PCI ID's supported by the drivers\n", PCI_TXT_IDS_PATH);
> - direntry = readdir(idsdir);
> - /* Read the directory */
> + int i;
> +
> + xf86Msg(X_INFO, "Searching for drivers that support the primary pci video\n");
> + driverpath = (char*)xalloc(strlen(xf86ModulePath) + strlen("/drivers/") + 1);
> + strcpy(driverpath, xf86ModulePath);
> + strcat(driverpath, "/drivers/");
> + dir = opendir(driverpath);
> + if (dir) {
> + direntry = readdir(dir);
> while (direntry) {
> - if (direntry->d_name[0] == '.') {
> - direntry = readdir(idsdir);
> - continue;
> - }
> - len = strlen(direntry->d_name);
> - /* A tiny bit of sanity checking. We should probably do better */
> - if (strncmp(&(direntry->d_name[len-4]), ".ids", 4) == 0) {
> - /* We need the full path name to open the file */
> - strncpy(path_name, PCI_TXT_IDS_PATH, 256);
> - strncat(path_name, "/", 1);
> - strncat(path_name, direntry->d_name, (256 - strlen(path_name) - 1));
> - fp = fopen(path_name, "r");
> - if (fp == NULL) {
> - xf86Msg(X_ERROR, "Could not open %s for reading. Exiting.\n", path_name);
> - goto end;
> - }
> - /* Read the file */
> - #ifdef __GLIBC__
> - while ((read = getline(&line, &len, fp)) != -1) {
> - #else
> - while ((line = fgetln(fp, &len)) != (char *)NULL) {
> - #endif /* __GLIBC __ */
> - xchomp(line);
> - if (isdigit(line[0])) {
> - strncpy(vendor_str, line, 4);
> - vendor_str[4] = '\0';
> - vendor = (int)strtol(vendor_str, NULL, 16);
> - if ((strlen(&line[4])) == 0) {
> - chip_str[0] = '\0';
> - chip = -1;
> - } else {
> - /* Handle trailing whitespace */
> - if (isspace(line[4])) {
> - chip_str[0] = '\0';
> - chip = -1;
> - } else {
> - /* Ok, it's a real ID */
> - strncpy(chip_str, &line[4], 4);
> - chip_str[4] = '\0';
> - chip = (int)strtol(chip_str, NULL, 16);
> - }
> - }
> - if (vendor == match_vendor && chip == match_chip ) {
> - i = 0;
> - while (matches[i]) {
> - i++;
> - }
> - matches[i] = (char*)xalloc(sizeof(char) * strlen(direntry->d_name) - 3);
> - if (!matches[i]) {
> - xf86Msg(X_ERROR, "Could not allocate space for the module name. Exiting.\n");
> - goto end;
> - }
> - /* hack off the .ids suffix. This should guard
> - * against other problems, but it will end up
> - * taking off anything after the first '.' */
> - for (j = 0; j < (strlen(direntry->d_name) - 3) ; j++) {
> - if (direntry->d_name[j] == '.') {
> - matches[i][j] = '\0';
> - break;
> - } else {
> - matches[i][j] = direntry->d_name[j];
> - }
> - }
> - xf86Msg(X_INFO, "Matched %s from file name %s\n", matches[i], direntry->d_name);
> - }
> - } else {
> - /* TODO Handle driver overrides here */
> + matcheddriver = driverClaimsID(direntry, match, driverpath);
> + if (matcheddriver) {
> + for (i = 0 ; i < matchmax ; i++) {
> + if (matches[i] == NULL) {
> + matches[i] = matcheddriver;
> + break;
> }
> }
> - fclose(fp);
> }
> - direntry = readdir(idsdir);
> - }
> + direntry = readdir(dir);
> + }
> + } else {
> + xf86Msg(X_ERROR, "Could not open driver dir %s for reading.\n", driverpath);
> }
> - end:
> - xfree(line);
> - closedir(idsdir);
> + closedir(dir);
> + xfree(driverpath);
> }
> -#endif /* __linux__ */
>
> char*
> chooseVideoDriver(void)
> @@ -418,9 +347,10 @@ chooseVideoDriver(void)
> struct pci_device_iterator *iter;
> char *chosen_driver = NULL;
> int i;
> - char *matches[20]; /* If we have more than 20 drivers we're in trouble */
> + int matchmax = 20; /* If we have more than 20 drivers we're in trouble */
> + char *matches[matchmax];
>
> - for (i=0 ; i<20 ; i++)
> + for (i=0 ; i < matchmax ; i++)
> matches[i] = NULL;
>
> /* Find the primary device, and get some information about it. */
> @@ -437,17 +367,14 @@ chooseVideoDriver(void)
> ErrorF("Primary device is not PCI\n");
> }
>
> -#ifdef __linux__
> - matchDriverFromFiles(matches, info->vendor_id, info->device_id);
> -#endif /* __linux__ */
> + matchDriverFromSymbols(matches, matchmax, info);
>
> /* TODO Handle multiple drivers claiming to support the same PCI ID */
> if (matches[0]) {
> chosen_driver = matches[0];
> } else {
> - if (info != NULL)
> - chosen_driver = videoPtrToDriverName(info);
> if (chosen_driver == NULL) {
> + xf86Msg(X_DEFAULT, "Could not find a driver that explicitly supports the primary video card. Falling back to the default driver.\n");
> #if defined __i386__ || defined __amd64__ || defined __hurd__
> chosen_driver = "vesa";
> #elif defined __alpha__
> diff --git a/hw/xfree86/common/xf86Config.h b/hw/xfree86/common/xf86Config.h
> index a174e46..3411423 100644
> --- a/hw/xfree86/common/xf86Config.h
> +++ b/hw/xfree86/common/xf86Config.h
> @@ -69,6 +69,5 @@ ConfigStatus xf86HandleConfigFile(Bool);
> Bool xf86AutoConfig(void);
> GDevPtr autoConfigDevice(GDevPtr preconf_device);
> char* chooseVideoDriver(void);
> -int xchomp(char *line);
>
> #endif /* _xf86_config_h */
> diff --git a/include/xorg-config.h.in b/include/xorg-config.h.in
> index b91ea92..97d53a2 100644
> --- a/include/xorg-config.h.in
> +++ b/include/xorg-config.h.in
> @@ -112,7 +112,4 @@
> /* Have execinfo.h */
> #undef HAVE_EXECINFO_H
>
> -/* Path to text files containing PCI IDs */
> -#undef PCI_TXT_IDS_PATH
> -
> #endif /* _XORG_CONFIG_H_ */
> --
> 1.5.4
>
> _______________________________________________
> xorg mailing list
> xorg at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.7 (GNU/Linux)
iQEVAwUBR74ww3YgpP6LHaLQAQK8Xwf/akV70KUgPacI/7q34qTsmUwqCrNPoIJi
td6KR6o5fMENfiVd9Pmfuq3CWUIFBfB4x4+m+yS4MMDO/DBqc5Pq1fyCCchNsIbO
WqRSf5ZS+733mmcpVjsqeqcfAK32VS6/arhcTMEdsqzQsByIbJm1+3+WLzytdMt6
fJhsBztZSIloBkAWqddbF69HOCUSruM7QLL3LvLYBEJlMBFEiou4PT5fVGVQIOpX
3PTJLGO5Zq8j2pJkIkUmCCItn5NKwuGp+9R4KGCmxXYP1hptE/89k+5hKIN2PPHB
6gJbTnMlRj3XRiceg/sAF9RKcBdYvGegYGjPGZRzpJjPwESjYjAYVg==
=7kFD
-----END PGP SIGNATURE-----
More information about the xorg
mailing list