Hi Lucas, > > > > The OpenGL and Vulkan robustness extensions let an application detect a > > GPU reset and check if its own context caused it, so the application can > > drop the broken context and build a new one. The kernel knows both > > facts, but etnaviv has no way to report them to userspace. > > > > Add two counters and a RESET_QUERY ioctl that returns both in one call: > > a per-GPU counter that counts every reset of that GPU, and a per-context > > counter that only counts the resets this context was guilty of. > > Userspace compares the counters with saved values: if the context > > counter moved the context was guilty, if only the GPU counter moved the > > context was an innocent victim. > > > > The GPU counter is per GPU core and not per device, so a hang on one > > pipe does not look like an innocent reset to contexts that only use > > another pipe. > > > > Bump the driver minor version so userspace can detect the feature. > > > > Signed-off-by: Christian Gmeiner <[email protected]> > > --- > > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 27 ++++++++++++++++++++++++++- > > drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 + > > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 2 ++ > > drivers/gpu/drm/etnaviv/etnaviv_sched.c | 3 +++ > > include/uapi/drm/etnaviv_drm.h | 19 ++++++++++++++++++- > > 5 files changed, 50 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > index a27ed014fb4e..a48698463fc1 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > @@ -296,6 +296,30 @@ static int etnaviv_ioctl_get_param(struct drm_device > > *dev, void *data, > > return etnaviv_gpu_get_param(gpu, args->param, &args->value); > > } > > > > +static int etnaviv_ioctl_reset_query(struct drm_device *dev, void *data, > > + struct drm_file *file) > > +{ > > + struct etnaviv_drm_private *priv = dev->dev_private; > > + struct etnaviv_file_private *ctx = file->driver_priv; > > + struct drm_etnaviv_reset_query *args = data; > > + struct etnaviv_gpu *gpu; > > + > > + if (args->flags) > > + return -EINVAL; > > + > > + if (args->pipe >= ETNA_MAX_PIPES) > > + return -EINVAL; > > + > > + gpu = priv->gpu[args->pipe]; > > + if (!gpu) > > + return -ENXIO; > > + > > + args->global_reset_counter = gpu->reset_counter; > > + args->context_reset_counter = ctx->reset_counter; > > Reading those values still has the chance to race with the timeout > handler updating the values. Failure mode would be timeout handler > running up until update of the global reset counter, then the readout > passing back both values to userspace, timeout handler updating the > context reset counter. Userspace would see a innocent context reset due > to only the global counter moving, while in reality it was guilty > context reset. > > I guess the easiest way to ensure atomic updates of both values would > be to take the gpu mutex here and across the update of both values. >
Right, that window exists and it reports exactly the wrong thing to the guilty context. I now take the gpu lock across both increments in the timeout handler and across the readout here, so the query can never see the global counter move without the context counter. -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info/privacypolicy
