[Mesa-dev] [PATCH] st/dri: implement the __DRI_DRIVER_VTABLE extension
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
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
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