Emil Velikov <[email protected]> writes: > Hi Eric, > > On 24 August 2018 at 14:11, Emil Velikov <[email protected]> wrote: >> From: Emil Velikov <[email protected]> >> >> 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: [email protected] >> Signed-off-by: Emil Velikov <[email protected]> >> --- >> 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
