Hi Eric,
On 24 August 2018 at 14:11, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> From: Emil Velikov <emil.veli...@collabora.com>
>
> As the comment above globalDriverAPI (in dri_util.c) says, if the loader
> is unaware of createNewScreen2 there is a race condition.
>
> In which globalDriverAPI, will be set in the driver driDriverGetExtensions*
> function and used in createNewScreen(). If we call another drivers'
> driDriverGetExtensions, the createNewScreen will use the latter's API
> instead of the former.
>
> To make it more convoluting, the driver _must_ also expose
> __DRI_DRIVER_VTABLE, as that one exposes the correct API.
>
> The race also occurs, for loaders which use the pre megadrivers
> driDriverGetExtensions entrypoint.
>
> Cc: mesa-sta...@lists.freedesktop.org
> Signed-off-by: Emil Velikov <emil.veli...@collabora.com>
> ---
> src/gallium/state_trackers/dri/dri2.c | 21 +++++++++++++++++++++
> src/gallium/state_trackers/dri/dri_screen.h | 1 +
> src/gallium/state_trackers/dri/drisw.c | 6 ++++++
> src/gallium/targets/dri/target.c | 2 +-
> 4 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/dri/dri2.c
> b/src/gallium/state_trackers/dri/dri2.c
> index 3cbca4e5dc3..b21e6815796 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -2318,11 +2318,32 @@ const struct __DriverAPIRec dri_kms_driver_api = {
> .ReleaseBuffer = dri2_release_buffer,
> };
>
> +static const struct __DRIDriverVtableExtensionRec gallium_drm_vtable = {
> + .base = { __DRI_DRIVER_VTABLE, 1 },
> + .vtable = &galliumdrm_driver_api,
> +};
> +
> +static const struct __DRIDriverVtableExtensionRec dri_kms_vtable = {
> + .base = { __DRI_DRIVER_VTABLE, 1 },
> + .vtable = &dri_kms_driver_api,
> +};
> +
> /* This is the table of extensions that the loader will dlsym() for. */
> const __DRIextension *galliumdrm_driver_extensions[] = {
> &driCoreExtension.base,
> &driImageDriverExtension.base,
> &driDRI2Extension.base,
> + &gallium_drm_vtable.base,
> + &gallium_config_options.base,
> + NULL
> +};
> +
> +/* This is the table of extensions that the loader will dlsym() for. */
> +const __DRIextension *dri_kms_driver_extensions[] = {
> + &driCoreExtension.base,
> + &driImageDriverExtension.base,
> + &driDRI2Extension.base,
> + &dri_kms_vtable.base,
> &gallium_config_options.base,
> NULL
> };
> diff --git a/src/gallium/state_trackers/dri/dri_screen.h
> b/src/gallium/state_trackers/dri/dri_screen.h
> index 8d2d9c02892..fde3b4088a7 100644
> --- a/src/gallium/state_trackers/dri/dri_screen.h
> +++ b/src/gallium/state_trackers/dri/dri_screen.h
> @@ -147,6 +147,7 @@ void
> dri_destroy_screen(__DRIscreen * sPriv);
>
> extern const struct __DriverAPIRec dri_kms_driver_api;
> +extern const __DRIextension *dri_kms_driver_extensions[];
>
> extern const struct __DriverAPIRec galliumdrm_driver_api;
> extern const __DRIextension *galliumdrm_driver_extensions[];
> diff --git a/src/gallium/state_trackers/dri/drisw.c
> b/src/gallium/state_trackers/dri/drisw.c
> index 1fba71bdd97..76a06b36664 100644
> --- a/src/gallium/state_trackers/dri/drisw.c
> +++ b/src/gallium/state_trackers/dri/drisw.c
> @@ -513,11 +513,17 @@ const struct __DriverAPIRec galliumsw_driver_api = {
> .CopySubBuffer = drisw_copy_sub_buffer,
> };
>
> +static const struct __DRIDriverVtableExtensionRec galliumsw_vtable = {
> + .base = { __DRI_DRIVER_VTABLE, 1 },
> + .vtable = &galliumsw_driver_api,
> +};
> +
> /* This is the table of extensions that the loader will dlsym() for. */
> const __DRIextension *galliumsw_driver_extensions[] = {
> &driCoreExtension.base,
> &driSWRastExtension.base,
> &driCopySubBufferExtension.base,
> + &galliumsw_vtable.base,
> &gallium_config_options.base,
> NULL
> };
> diff --git a/src/gallium/targets/dri/target.c
> b/src/gallium/targets/dri/target.c
> index 835d125f21e..e943cae6a16 100644
> --- a/src/gallium/targets/dri/target.c
> +++ b/src/gallium/targets/dri/target.c
> @@ -28,7 +28,7 @@ const __DRIextension
> **__driDriverGetExtensions_kms_swrast(void);
> PUBLIC const __DRIextension **__driDriverGetExtensions_kms_swrast(void)
> {
> globalDriverAPI = &dri_kms_driver_api;
> - return galliumdrm_driver_extensions;
> + return dri_kms_driver_extensions;
> }
>
Can you please skim through the above?
Seems like I forgot to implement this while making the gallium mega-drivers.
I did not notice any issues in practise, but with EGLDevice this will
become more likely to hit.
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
