[Mesa-dev] [PATCH 5/5] loader: Factor out the common driver opening logic from each loader.
Eric Engestrom
eric.engestrom at intel.com
Fri Nov 16 11:43:20 UTC 2018
On Thursday, 2018-11-15 15:05:27 -0800, Eric Anholt wrote:
> I copied the code from egl_dri2.c, but the functionality was equivalent
> between all the loaders other than their particular environment variables.
> ---
> src/egl/Makefile.am | 3 +-
> src/egl/drivers/dri2/egl_dri2.c | 75 +++-----------------------
> src/egl/meson.build | 3 --
> src/gbm/Makefile.am | 1 -
> src/gbm/backends/dri/gbm_dri.c | 96 +++++++++------------------------
> src/gbm/meson.build | 1 -
> src/glx/Makefile.am | 1 -
> src/glx/SConscript | 1 -
> src/glx/dri_common.c | 91 +++----------------------------
> src/glx/meson.build | 1 -
> src/loader/Makefile.am | 1 +
> src/loader/loader.c | 90 +++++++++++++++++++++++++++++++
> src/loader/loader.h | 7 +++
> src/loader/meson.build | 4 +-
> 14 files changed, 142 insertions(+), 233 deletions(-)
>
> diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
> index 24a8e96a8e1e..7269912d96f3 100644
> --- a/src/egl/Makefile.am
> +++ b/src/egl/Makefile.am
> @@ -119,8 +119,7 @@ AM_CFLAGS += \
> -I$(top_srcdir)/src/egl/drivers/dri2 \
> -I$(top_srcdir)/src/gbm/backends/dri \
> -I$(top_builddir)/src/egl/wayland/wayland-drm \
> - -I$(top_srcdir)/src/egl/wayland/wayland-drm \
> - -DDEFAULT_DRIVER_DIR=\"$(DRI_DRIVER_SEARCH_DIR)\"
> + -I$(top_srcdir)/src/egl/wayland/wayland-drm
>
> nodist_libEGL_common_la_SOURCES = \
> $(dri2_backend_GENERATED_FILES)
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index f05c39126c85..f998655d2ae0 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -483,75 +483,14 @@ static const __DRIextension **
> dri2_open_driver(_EGLDisplay *disp)
> {
> struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> - const __DRIextension **extensions = NULL;
> - char path[PATH_MAX], *search_paths, *next, *end;
> - char *get_extensions_name;
> - const __DRIextension **(*get_extensions)(void);
> -
> - search_paths = NULL;
> - if (geteuid() == getuid()) {
> - /* don't allow setuid apps to use LIBGL_DRIVERS_PATH */
> - search_paths = getenv("LIBGL_DRIVERS_PATH");
> - }
> - if (search_paths == NULL)
> - search_paths = DEFAULT_DRIVER_DIR;
> -
> - dri2_dpy->driver = NULL;
> - end = search_paths + strlen(search_paths);
> - for (char *p = search_paths; p < end; p = next + 1) {
> - int len;
> - next = strchr(p, ':');
> - if (next == NULL)
> - next = end;
> -
> - len = next - p;
> -#if GLX_USE_TLS
> - snprintf(path, sizeof path,
> - "%.*s/tls/%s_dri.so", len, p, dri2_dpy->driver_name);
> - dri2_dpy->driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL);
> -#endif
> - if (dri2_dpy->driver == NULL) {
> - snprintf(path, sizeof path,
> - "%.*s/%s_dri.so", len, p, dri2_dpy->driver_name);
> - dri2_dpy->driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL);
> - if (dri2_dpy->driver == NULL)
> - _eglLog(_EGL_DEBUG, "failed to open %s: %s\n", path, dlerror());
> - }
> - /* not need continue to loop all paths once the driver is found */
> - if (dri2_dpy->driver != NULL)
> - break;
> - }
> -
> - if (dri2_dpy->driver == NULL) {
> - _eglLog(_EGL_WARNING,
> - "DRI2: failed to open %s (search paths %s)",
> - dri2_dpy->driver_name, search_paths);
> - return NULL;
> - }
> -
> - _eglLog(_EGL_DEBUG, "DRI2: dlopen(%s)", path);
> -
> - get_extensions_name = loader_get_extensions_name(dri2_dpy->driver_name);
> - if (get_extensions_name) {
> - get_extensions = dlsym(dri2_dpy->driver, get_extensions_name);
> - if (get_extensions) {
> - extensions = get_extensions();
> - } else {
> - _eglLog(_EGL_DEBUG, "driver does not expose %s(): %s\n",
> - get_extensions_name, dlerror());
> - }
> - free(get_extensions_name);
> - }
> -
> - if (!extensions)
> - extensions = dlsym(dri2_dpy->driver, __DRI_DRIVER_EXTENSIONS);
> - if (extensions == NULL) {
> - _eglLog(_EGL_WARNING,
> - "DRI2: driver exports no extensions (%s)", dlerror());
> - dlclose(dri2_dpy->driver);
> - }
> + static const char *search_path_vars[] = {
> + "LIBGL_DRIVERS_PATH",
> + NULL,
> + };
>
> - return extensions;
> + return loader_open_driver(dri2_dpy->driver_name,
> + &dri2_dpy->driver,
> + search_path_vars);
> }
>
> EGLBoolean
> diff --git a/src/egl/meson.build b/src/egl/meson.build
> index 8c0ffea8b40a..372842967d37 100644
> --- a/src/egl/meson.build
> +++ b/src/egl/meson.build
> @@ -93,9 +93,6 @@ if with_dri2
> 'drivers/dri2/egl_dri2.h',
> 'drivers/dri2/egl_dri2_fallbacks.h',
> )
> - c_args_for_egl += [
> - '-DDEFAULT_DRIVER_DIR="@0@"'.format(dri_search_path),
> - ]
>
> if with_platform_x11
> files_egl += files('drivers/dri2/platform_x11.c')
> diff --git a/src/gbm/Makefile.am b/src/gbm/Makefile.am
> index 5097212cda0a..bb246ecebf52 100644
> --- a/src/gbm/Makefile.am
> +++ b/src/gbm/Makefile.am
> @@ -42,7 +42,6 @@ libgbm_la_SOURCES += \
> $(gbm_dri_FILES)
>
> AM_CFLAGS += \
> - -DDEFAULT_DRIVER_DIR='"$(DRI_DRIVER_SEARCH_DIR)"' \
> $(LIBDRM_CFLAGS) \
> $(PTHREADSTUBS_CFLAGS)
>
> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
> index f32c0cd9885f..8f035b7c7d84 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -301,31 +301,21 @@ dri_bind_extensions(struct gbm_dri_device *dri,
> return ret;
> }
>
> +static void
> +gbm_loader_log(int level, const char *fmt, ...)
> +{
> + if (level > _LOADER_WARNING)
> + return;
> +
> + va_list ap;
> + va_start(ap, fmt);
> + vfprintf(stderr, fmt, ap);
> + va_end(ap);
> +}
Why? This is the exact behaviour of the default_logger() :)
If you still want to add it, I'd do it in a separate commit, since it
has nothing to do with the rest of this patch.
With gbm_loader_log() split out, this is:
Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>
>
> static const __DRIextension **
> dri_open_driver(struct gbm_dri_device *dri)
> {
> - const __DRIextension **extensions = NULL;
> - char path[PATH_MAX], *search_paths, *p, *next, *end;
> - char *get_extensions_name;
> -
> - search_paths = NULL;
> - /* don't allow setuid apps to use LIBGL_DRIVERS_PATH or GBM_DRIVERS_PATH */
> - if (geteuid() == getuid()) {
> - /* Read GBM_DRIVERS_PATH first for compatibility, but LIBGL_DRIVERS_PATH
> - * is recommended over GBM_DRIVERS_PATH.
> - */
> - search_paths = getenv("GBM_DRIVERS_PATH");
> -
> - /* Read LIBGL_DRIVERS_PATH if GBM_DRIVERS_PATH was not set.
> - * LIBGL_DRIVERS_PATH is recommended over GBM_DRIVERS_PATH.
> - */
> - if (search_paths == NULL) {
> - search_paths = getenv("LIBGL_DRIVERS_PATH");
> - }
> - }
> - if (search_paths == NULL)
> - search_paths = DEFAULT_DRIVER_DIR;
>
> /* Temporarily work around dri driver libs that need symbols in libglapi
> * but don't automatically link it in.
> */
> @@ -334,56 +324,20 @@ dri_open_driver(struct gbm_dri_device *dri)
> */
> dlopen("libglapi.so.0", RTLD_LAZY | RTLD_GLOBAL);
>
> - dri->driver = NULL;
> - end = search_paths + strlen(search_paths);
> - for (p = search_paths; p < end && dri->driver == NULL; p = next + 1) {
> - int len;
> - next = strchr(p, ':');
> - if (next == NULL)
> - next = end;
> -
> - len = next - p;
> -#if GLX_USE_TLS
> - snprintf(path, sizeof path,
> - "%.*s/tls/%s_dri.so", len, p, dri->driver_name);
> - dri->driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL);
> -#endif
> - if (dri->driver == NULL) {
> - snprintf(path, sizeof path,
> - "%.*s/%s_dri.so", len, p, dri->driver_name);
> - dri->driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL);
> - }
> - /* not need continue to loop all paths once the driver is found */
> - if (dri->driver != NULL)
> - break;
> - }
> -
> - if (dri->driver == NULL) {
> - fprintf(stderr, "gbm: failed to open any driver (search paths %s)\n",
> - search_paths);
> - fprintf(stderr, "gbm: Last dlopen error: %s\n", dlerror());
> - return NULL;
> - }
> -
> - get_extensions_name = loader_get_extensions_name(dri->driver_name);
> - if (get_extensions_name) {
> - const __DRIextension **(*get_extensions)(void);
> -
> - get_extensions = dlsym(dri->driver, get_extensions_name);
> - free(get_extensions_name);
> -
> - if (get_extensions)
> - extensions = get_extensions();
> - }
> -
> - if (!extensions)
> - extensions = dlsym(dri->driver, __DRI_DRIVER_EXTENSIONS);
> - if (extensions == NULL) {
> - fprintf(stderr, "gbm: driver exports no extensions (%s)", dlerror());
> - dlclose(dri->driver);
> - }
> + loader_set_logger(gbm_loader_log);
>
> - return extensions;
> + static const char *search_path_vars[] = {
> + /* Read GBM_DRIVERS_PATH first for compatibility, but LIBGL_DRIVERS_PATH
> + * is recommended over GBM_DRIVERS_PATH.
> + */
> + "GBM_DRIVERS_PATH"
> + /* Read LIBGL_DRIVERS_PATH if GBM_DRIVERS_PATH was not set.
> + * LIBGL_DRIVERS_PATH is recommended over GBM_DRIVERS_PATH.
> + */
> + "LIBGL_DRIVERS_PATH",
> + NULL
> + };
> + return loader_open_driver(dri->driver_name, &dri->driver, search_path_vars);
> }
>
> static int
> diff --git a/src/gbm/meson.build b/src/gbm/meson.build
> index 719f9c1a9b8a..007f50a9ae32 100644
> --- a/src/gbm/meson.build
> +++ b/src/gbm/meson.build
> @@ -37,7 +37,6 @@ incs_gbm = [
> if with_dri2
> files_gbm += files('backends/dri/gbm_dri.c', 'backends/dri/gbm_driint.h')
> deps_gbm += dep_libdrm # TODO: pthread-stubs
> - args_gbm += '-DDEFAULT_DRIVER_DIR="@0@"'.format(dri_search_path)
> endif
> if with_platform_wayland
> deps_gbm += dep_wayland_server
> diff --git a/src/glx/Makefile.am b/src/glx/Makefile.am
> index 8f9d80c9f415..d208ce14bb79 100644
> --- a/src/glx/Makefile.am
> +++ b/src/glx/Makefile.am
> @@ -40,7 +40,6 @@ AM_CFLAGS = \
> $(VISIBILITY_CFLAGS) \
> $(EXTRA_DEFINES_XF86VIDMODE) \
> -D_REENTRANT \
> - -DDEFAULT_DRIVER_DIR=\"$(DRI_DRIVER_SEARCH_DIR)\" \
> $(DEFINES) \
> $(LIBDRM_CFLAGS) \
> $(DRI2PROTO_CFLAGS) \
> diff --git a/src/glx/SConscript b/src/glx/SConscript
> index 8ce177158140..7555fb0568c8 100644
> --- a/src/glx/SConscript
> +++ b/src/glx/SConscript
> @@ -24,7 +24,6 @@ env.Prepend(CPPPATH = [
>
> env.Append(CPPDEFINES = [
> '_REENTRANT',
> - #('DEFAULT_DRIVER_DIR', 'DRI_DRIVER_SEARCH_DIR')
> ])
>
> env.Prepend(LIBS = [
> diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
> index 75a5e5025761..fb8a29f09934 100644
> --- a/src/glx/dri_common.c
> +++ b/src/glx/dri_common.c
> @@ -94,98 +94,23 @@ dri_message(int level, const char *f, ...)
> _X_HIDDEN const __DRIextension **
> driOpenDriver(const char *driverName, void **out_driver_handle)
> {
> - void *glhandle, *handle;
> - const char *libPaths, *p, *next;
> - char realDriverName[200];
> - int len;
> + void *glhandle;
>
> /* Attempt to make sure libGL symbols will be visible to the driver */
> glhandle = dlopen(GL_LIB_NAME, RTLD_NOW | RTLD_GLOBAL);
>
> - libPaths = NULL;
> - if (geteuid() == getuid()) {
> - /* don't allow setuid apps to use LIBGL_DRIVERS_PATH */
> - libPaths = getenv("LIBGL_DRIVERS_PATH");
> - if (!libPaths)
> - libPaths = getenv("LIBGL_DRIVERS_DIR"); /* deprecated */
> - }
> - if (libPaths == NULL)
> - libPaths = DEFAULT_DRIVER_DIR;
> -
> - handle = NULL;
> - for (p = libPaths; *p; p = next) {
> - next = strchr(p, ':');
> - if (next == NULL) {
> - len = strlen(p);
> - next = p + len;
> - }
> - else {
> - len = next - p;
> - next++;
> - }
> + static const char *search_path_vars[] = {
> + "LIBGL_DRIVERS_PATH",
> + "LIBGL_DRIVERS_DIR", /* deprecated */
> + NULL
> + };
>
> -#ifdef GLX_USE_TLS
> - snprintf(realDriverName, sizeof realDriverName,
> - "%.*s/tls/%s_dri.so", len, p, driverName);
> - InfoMessageF("OpenDriver: trying %s\n", realDriverName);
> - handle = dlopen(realDriverName, RTLD_NOW | RTLD_GLOBAL);
> -#endif
> -
> - if (handle == NULL) {
> - snprintf(realDriverName, sizeof realDriverName,
> - "%.*s/%s_dri.so", len, p, driverName);
> - InfoMessageF("OpenDriver: trying %s\n", realDriverName);
> - handle = dlopen(realDriverName, RTLD_NOW | RTLD_GLOBAL);
> - }
> -
> - if (handle != NULL)
> - break;
> - else
> - InfoMessageF("dlopen %s failed (%s)\n", realDriverName, dlerror());
> - }
> -
> - if (!handle)
> - ErrorMessageF("unable to load driver: %s_dri.so\n", driverName);
> + const __DRIextension **extensions =
> + loader_open_driver(driverName, out_driver_handle, search_path_vars);
>
> if (glhandle)
> dlclose(glhandle);
>
> - const __DRIextension **extensions = driGetDriverExtensions(handle,
> - driverName);
> - if (!extensions) {
> - dlclose(handle);
> - handle = NULL;
> - }
> -
> - *out_driver_handle = handle;
> - return extensions;
> -}
> -
> -static const __DRIextension **
> -driGetDriverExtensions(void *handle, const char *driver_name)
> -{
> - const __DRIextension **extensions = NULL;
> - const __DRIextension **(*get_extensions)(void);
> - char *get_extensions_name = loader_get_extensions_name(driver_name);
> -
> - if (get_extensions_name) {
> - get_extensions = dlsym(handle, get_extensions_name);
> - if (get_extensions) {
> - free(get_extensions_name);
> - return get_extensions();
> - } else {
> - InfoMessageF("driver does not expose %s(): %s\n",
> - get_extensions_name, dlerror());
> - free(get_extensions_name);
> - }
> - }
> -
> - extensions = dlsym(handle, __DRI_DRIVER_EXTENSIONS);
> - if (extensions == NULL) {
> - ErrorMessageF("driver exports no extensions (%s)\n", dlerror());
> - return NULL;
> - }
> -
> return extensions;
> }
>
> diff --git a/src/glx/meson.build b/src/glx/meson.build
> index dd8ba60ad80c..1de35fca6bc2 100644
> --- a/src/glx/meson.build
> +++ b/src/glx/meson.build
> @@ -134,7 +134,6 @@ endif
>
> gl_lib_cargs = [
> '-D_REENTRANT',
> - '-DDEFAULT_DRIVER_DIR="@0@"'.format(dri_search_path),
> ]
>
> if dep_xxf86vm.found()
> diff --git a/src/loader/Makefile.am b/src/loader/Makefile.am
> index 762525642819..3c22e0ae7e60 100644
> --- a/src/loader/Makefile.am
> +++ b/src/loader/Makefile.am
> @@ -28,6 +28,7 @@ noinst_LTLIBRARIES = libloader.la
> AM_CPPFLAGS = \
> -I$(top_builddir)/src/util/ \
> -DUSE_DRICONF \
> + -DDEFAULT_DRIVER_DIR=\"$(DRI_DRIVER_SEARCH_DIR)\" \
> $(DEFINES) \
> -I$(top_srcdir)/include \
> -I$(top_srcdir)/include/drm-uapi \
> diff --git a/src/loader/loader.c b/src/loader/loader.c
> index 041a59212be7..05140b6d18fd 100644
> --- a/src/loader/loader.c
> +++ b/src/loader/loader.c
> @@ -26,6 +26,7 @@
> * Rob Clark <robclark at freedesktop.org>
> */
>
> +#include <dlfcn.h>
> #include <errno.h>
> #include <fcntl.h>
> #include <sys/stat.h>
> @@ -35,6 +36,7 @@
> #include <string.h>
> #include <unistd.h>
> #include <stdlib.h>
> +#include <sys/param.h>
> #ifdef MAJOR_IN_MKDEV
> #include <sys/mkdev.h>
> #endif
> @@ -510,3 +512,91 @@ loader_get_extensions_name(const char *driver_name)
>
> return name;
> }
> +
> +/**
> + * Opens a DRI driver using its driver name, returning the __DRIextension
> + * entrypoints.
> + *
> + * \param driverName - a name like "i965", "radeon", "nouveau", etc.
> + * \param out_driver - Address where the dlopen() return value will be stored.
> + * \param search_path_vars - NULL-terminated list of env vars that can be used
> + * to override the DEFAULT_DRIVER_DIR search path.
> + */
> +const struct __DRIextensionRec **
> +loader_open_driver(const char *driver_name,
> + void **out_driver_handle,
> + const char **search_path_vars)
> +{
> + char path[PATH_MAX], *search_paths, *next, *end;
> + char *get_extensions_name;
> + const struct __DRIextensionRec **extensions = NULL;
> + const struct __DRIextensionRec **(*get_extensions)(void);
> +
> + search_paths = NULL;
> + if (geteuid() == getuid() && search_path_vars) {
> + for (int i = 0; search_path_vars[i] != NULL; i++) {
> + search_paths = getenv(search_path_vars[i]);
> + if (search_paths)
> + break;
> + }
> + }
> + if (search_paths == NULL)
> + search_paths = DEFAULT_DRIVER_DIR;
> +
> + void *driver = NULL;
> + end = search_paths + strlen(search_paths);
> + for (char *p = search_paths; p < end; p = next + 1) {
> + int len;
> + next = strchr(p, ':');
> + if (next == NULL)
> + next = end;
> +
> + len = next - p;
> +#if GLX_USE_TLS
> + snprintf(path, sizeof(path), "%.*s/tls/%s_dri.so", len, p, driver_name);
> + driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL);
> +#endif
> + if (driver == NULL) {
> + snprintf(path, sizeof(path), "%.*s/%s_dri.so", len, p, driver_name);
> + driver = dlopen(path, RTLD_NOW | RTLD_GLOBAL);
> + if (driver == NULL)
> + log_(_LOADER_DEBUG, "MESA-LOADER: failed to open %s: %s\n",
> + path, dlerror());
> + }
> + /* not need continue to loop all paths once the driver is found */
> + if (driver != NULL)
> + break;
> + }
> +
> + if (driver == NULL) {
> + log_(_LOADER_WARNING, "MESA-LOADER: failed to open %s (search paths %s)\n",
> + driver_name, search_paths);
> + *out_driver_handle = NULL;
> + return NULL;
> + }
> +
> + log_(_LOADER_DEBUG, "MESA-LOADER: dlopen(%s)\n", path);
> +
> + get_extensions_name = loader_get_extensions_name(driver_name);
> + if (get_extensions_name) {
> + get_extensions = dlsym(driver, get_extensions_name);
> + if (get_extensions) {
> + extensions = get_extensions();
> + } else {
> + log_(_LOADER_DEBUG, "MESA-LOADER: driver does not expose %s(): %s\n",
> + get_extensions_name, dlerror());
> + }
> + free(get_extensions_name);
> + }
> +
> + if (!extensions)
> + extensions = dlsym(driver, __DRI_DRIVER_EXTENSIONS);
> + if (extensions == NULL) {
> + log_(_LOADER_WARNING,
> + "MESA-LOADER: driver exports no extensions (%s)\n", dlerror());
> + dlclose(driver);
> + }
> +
> + *out_driver_handle = driver;
> + return extensions;
> +}
> diff --git a/src/loader/loader.h b/src/loader/loader.h
> index 7b4dd01144f8..779ec44b75d4 100644
> --- a/src/loader/loader.h
> +++ b/src/loader/loader.h
> @@ -33,6 +33,8 @@
> extern "C" {
> #endif
>
> +struct __DRIextensionRec;
> +
> /* Helpers to figure out driver and device name, eg. from pci-id, etc. */
>
> int
> @@ -47,6 +49,11 @@ loader_get_pci_id_for_fd(int fd, int *vendor_id, int *chip_id);
> char *
> loader_get_driver_for_fd(int fd);
>
> +const struct __DRIextensionRec **
> +loader_open_driver(const char *driver_name,
> + void **out_driver_handle,
> + const char **search_path_vars);
> +
> char *
> loader_get_device_name_for_fd(int fd);
>
> diff --git a/src/loader/meson.build b/src/loader/meson.build
> index 76fcae2cb44a..e280ba41cc2b 100644
> --- a/src/loader/meson.build
> +++ b/src/loader/meson.build
> @@ -39,7 +39,9 @@ libloader = static_library(
> 'loader',
> ['loader.c', 'loader.h', 'pci_id_driver_map.c', 'pci_id_driver_map.h',
> xmlpool_options_h],
> - c_args : [c_vis_args, '-DUSE_DRICONF'],
> + c_args : [c_vis_args, '-DUSE_DRICONF',
> + '-DDEFAULT_DRIVER_DIR="@0@"'.format(dri_search_path),
> +],
> include_directories : [inc_include, inc_src, inc_util],
> dependencies : [dep_libdrm, dep_thread],
> build_by_default : false,
> --
> 2.19.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list