[Mesa-dev] [PATCH] st/dri: implement the __DRI_DRIVER_VTABLE extension

2018-08-24 Thread Emil Velikov 
From: Emil Velikov 

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 
---
 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;
 }
 
 #endif
-- 
2.18.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/dri: implement the __DRI_DRIVER_VTABLE extension

2018-09-03 Thread Emil Velikov 
Hi Eric,

On 24 August 2018 at 14:11, Emil Velikov  wrote:
> From: Emil Velikov 
>
> 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 
> ---
>  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

Re: [Mesa-dev] [PATCH] st/dri: implement the __DRI_DRIVER_VTABLE extension

2018-09-05 Thread Eric Anholt 
Emil Velikov  writes:

> Hi Eric,
>
> On 24 August 2018 at 14:11, Emil Velikov  wrote:
>> From: Emil Velikov 
>>
>> 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 
>> ---
>>  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.

With having a new baby, I'm getting maybe an hour a day for software
stuff.  I won't have time to review this.


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo