[PATCH v2 0/2] drm/etnaviv: Add GPU reset counters for robustness

[PATCH v2 0/2] drm/etnaviv: Add GPU reset counters for robustness

[Christian Gmeiner]

The OpenGL robustness extensions (GL_KHR_robustness) and Vulkan
(VK_ERROR_DEVICE_LOST) let an application detect a GPU reset and check
whether its own context caused it, so it can throw away the broken
context and build a new one. etnaviv already resets the GPU after a
hang, but userspace has no way to learn about it.

This series adds two counters and a RESET_QUERY ioctl that returns both
in one call:

- a global counter that counts every reset of a GPU core.
- a context counter that only counts the resets the calling context was
  guilty of.

Userspace samples both values and compares them later: if the context
counter moved the context was guilty, if only the global counter moved
the context was an innocent victim. That is all that is needed to
implement glGetGraphicsResetStatus() and Vulkan device loss.

The global counter is kept 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.

The first patch is preparation: the counters are updated from the
scheduler timeout worker, which can race with the DRM file being
closed, so struct etnaviv_file_private becomes reference counted and
every submit holds a reference.

Link to the Mesa MR implementing the userspace side:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/42826

Signed-off-by: Christian Gmeiner <cgmeiner@igalia.com>
---
Changes in v2:
- Replace the two GET_PARAM values with a dedicated RESET_QUERY ioctl
  that returns both counters in one call. The ioctl has a flags field
  that must be zero for now, so the query can later be extended to name
  a specific context once a DRM file can hold more than one context
  (Lucas).
- Make the reset counters plain u32 instead of atomics (Lucas).
- Use __u32 for the ioctl counter fields to match the internal
  counters, following i915 GET_RESET_STATS.
- Link to v1: https://lore.kernel.org/r/20260709-etnaviv-reset-notification-v1-0-64c617496958@igalia.com

---
Christian Gmeiner (2):
      drm/etnaviv: Reference count struct etnaviv_file_private
      drm/etnaviv: Add GPU reset counters

 drivers/gpu/drm/etnaviv/etnaviv_drv.c        | 44 ++++++++++++++++++++++++++--
 drivers/gpu/drm/etnaviv/etnaviv_drv.h        | 12 ++++++++
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  5 +++-
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h        |  2 ++
 drivers/gpu/drm/etnaviv/etnaviv_sched.c      |  3 ++
 include/uapi/drm/etnaviv_drm.h               | 19 +++++++++++-
 6 files changed, 81 insertions(+), 4 deletions(-)
---
base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
change-id: 20260708-etnaviv-reset-notification-b037153a1aab

Best regards,
-- 
Christian Gmeiner <cgmeiner@igalia.com>

[PATCH v2 1/2] drm/etnaviv: Reference count struct etnaviv_file_private

[Christian Gmeiner]

From: Christian Gmeiner <cgmeiner@igalia.com>

The next commit updates per-context data from the GPU reset path, which
runs in the scheduler timeout worker. This can race with closing the DRM
file: drm_sched_entity_flush() only waits until the entity queue is
empty, it does not wait for jobs still running on the hardware. So the
context can already be freed while the reset path still needs it.

Reference count the context and let every submit hold a reference, the
same way a submit already keeps its mmu context and pid alive. No
functional change.

Signed-off-by: Christian Gmeiner <cgmeiner@igalia.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c        | 17 ++++++++++++++++-
 drivers/gpu/drm/etnaviv/etnaviv_drv.h        | 11 +++++++++++
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  5 ++++-
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 08aca9035fc1..a27ed014fb4e 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -60,6 +60,19 @@ static void load_gpu(struct drm_device *dev)
 	}
 }
 
+static void etnaviv_file_private_release(struct kref *kref)
+{
+	struct etnaviv_file_private *ctx =
+		container_of(kref, struct etnaviv_file_private, refcount);
+
+	kfree(ctx);
+}
+
+void etnaviv_file_private_put(struct etnaviv_file_private *ctx)
+{
+	kref_put(&ctx->refcount, etnaviv_file_private_release);
+}
+
 static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
 {
 	struct etnaviv_drm_private *priv = dev->dev_private;
@@ -70,6 +83,8 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
 	if (!ctx)
 		return -ENOMEM;
 
+	kref_init(&ctx->refcount);
+
 	ret = xa_alloc_cyclic(&priv->active_contexts, &ctx->id, ctx,
 			      xa_limit_32b, &priv->next_context_id, GFP_KERNEL);
 	if (ret < 0)
@@ -120,7 +135,7 @@ static void etnaviv_postclose(struct drm_device *dev, struct drm_file *file)
 
 	xa_erase(&priv->active_contexts, ctx->id);
 
-	kfree(ctx);
+	etnaviv_file_private_put(ctx);
 }
 
 /*
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index 55a9e745604d..cba4323ae589 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -7,6 +7,7 @@
 #define __ETNAVIV_DRV_H__
 
 #include <linux/io.h>
+#include <linux/kref.h>
 #include <linux/list.h>
 #include <linux/mm_types.h>
 #include <linux/sizes.h>
@@ -29,6 +30,7 @@ struct etnaviv_iommu_global;
 #define ETNAVIV_SOFTPIN_START_ADDRESS	SZ_4M /* must be >= SUBALLOC_SIZE */
 
 struct etnaviv_file_private {
+	struct kref refcount;
 	int id;
 	struct etnaviv_iommu_context	*mmu;
 	struct drm_sched_entity		sched_entity[ETNA_MAX_PIPES];
@@ -53,6 +55,15 @@ struct etnaviv_drm_private {
 	struct etnaviv_cmdbuf *flop_reset_data_ppu;
 };
 
+void etnaviv_file_private_put(struct etnaviv_file_private *ctx);
+
+static inline struct etnaviv_file_private *
+etnaviv_file_private_get(struct etnaviv_file_private *ctx)
+{
+	kref_get(&ctx->refcount);
+	return ctx;
+}
+
 int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 		struct drm_file *file);
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 1a77a09b3377..98f1f59a8b05 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -398,6 +398,9 @@ static void submit_cleanup(struct kref *kref)
 
 	put_pid(submit->pid);
 
+	if (submit->ctx)
+		etnaviv_file_private_put(submit->ctx);
+
 	kfree(submit->pmrs);
 	kfree(submit);
 }
@@ -526,7 +529,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void *data,
 	if (ret)
 		goto err_submit_put;
 
-	submit->ctx = file->driver_priv;
+	submit->ctx = etnaviv_file_private_get(file->driver_priv);
 	submit->mmu_context = etnaviv_iommu_context_get(submit->ctx->mmu);
 	submit->exec_state = args->exec_state;
 	submit->flags = args->flags;

-- 
2.54.0

Re: [PATCH v2 1/2] drm/etnaviv: Reference count struct etnaviv_file_private

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked return value of drm_sched_entity_init leads to NULL pointer dereference on file close.
--

--- Patch [1]: [PATCH v2 1/2] drm/etnaviv: Reference count struct etnaviv_file_private ---
commit ac09092a81050d73fa591427824ad978bbce1e24
Author: Christian Gmeiner <cgmeiner@igalia.com>

drm/etnaviv: Reference count struct etnaviv_file_private

This commit introduces reference counting for etnaviv_file_private to ensure
the context remains alive while the GPU reset path is still running. The
scheduler timeout worker now holds a reference, avoiding a race with closing
the DRM file.

> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 08aca9035fc1a..a27ed014fb4e7 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
[ ... ]
> @@ -70,6 +83,8 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
>  	if (!ctx)
>  		return -ENOMEM;
>  
> +	kref_init(&ctx->refcount);
> +
>  	ret = xa_alloc_cyclic(&priv->active_contexts, &ctx->id, ctx,
>  			      xa_limit_32b, &priv->next_context_id, GFP_KERNEL);

[Severity: High]
This is a pre-existing issue, but does etnaviv_open() need to check the
return value of drm_sched_entity_init()?

Looking at etnaviv_open() just below this block, the driver initializes a
scheduler entity for each valid GPU without checking for failure:

etnaviv_open() {
        ...
        drm_sched_entity_init(&ctx->sched_entity[i],
                              DRM_SCHED_PRIORITY_NORMAL, &sched,
                              1, NULL);
        ...
}

If drm_sched_entity_init() fails (e.g., due to memory exhaustion),
entity->stats remains NULL. 

Later, etnaviv_postclose() unconditionally tears down all entities:

etnaviv_postclose() {
        ...
        if (gpu)
                drm_sched_entity_destroy(&ctx->sched_entity[i]);
        ...
}

This eventually invokes drm_sched_entity_fini() in
drivers/gpu/drm/scheduler/sched_entity.c, which calls:

        drm_sched_entity_stats_put(entity->stats);

Could this cause a NULL pointer dereference in kref_put() when it attempts
to operate on the NULL entity->stats pointer during file close?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-etnaviv-reset-notification-v2-0-90cfebcf22a2@igalia.com?part=1

[PATCH v2 2/2] drm/etnaviv: Add GPU reset counters

[Christian Gmeiner]

From: Christian Gmeiner <cgmeiner@igalia.com>

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 <cgmeiner@igalia.com>
---
 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;
+
+	return 0;
+}
+
 static int etnaviv_ioctl_gem_new(struct drm_device *dev, void *data,
 		struct drm_file *file)
 {
@@ -502,6 +526,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
 	ETNA_IOCTL(GEM_WAIT,     gem_wait,     DRM_RENDER_ALLOW),
 	ETNA_IOCTL(PM_QUERY_DOM, pm_query_dom, DRM_RENDER_ALLOW),
 	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
+	ETNA_IOCTL(RESET_QUERY,  reset_query,  DRM_RENDER_ALLOW),
 };
 
 static void etnaviv_show_fdinfo(struct drm_printer *p, struct drm_file *file)
@@ -530,7 +555,7 @@ static const struct drm_driver etnaviv_drm_driver = {
 	.name               = "etnaviv",
 	.desc               = "etnaviv DRM",
 	.major              = 1,
-	.minor              = 4,
+	.minor              = 5,
 };
 
 /*
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index cba4323ae589..9c348aa7f8d3 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -34,6 +34,7 @@ struct etnaviv_file_private {
 	int id;
 	struct etnaviv_iommu_context	*mmu;
 	struct drm_sched_entity		sched_entity[ETNA_MAX_PIPES];
+	u32 reset_counter;
 };
 
 struct etnaviv_drm_private {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 5cb46c84e03a..a97780131426 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -148,6 +148,8 @@ struct etnaviv_gpu {
 	u32 hangcheck_primid;
 	u32 hangcheck_fence;
 
+	u32 reset_counter;
+
 	void __iomem *mmio;
 	int irq;
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 139e6e38784b..bd1335a17f46 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -79,6 +79,9 @@ static enum drm_gpu_sched_stat etnaviv_sched_timedout_job(struct drm_sched_job
 	if(sched_job)
 		drm_sched_increase_karma(sched_job);
 
+	gpu->reset_counter++;
+	submit->ctx->reset_counter++;
+
 	/* get the GPU back into the init state */
 	etnaviv_core_dump(submit);
 	etnaviv_gpu_recover_hang(submit);
diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
index af024d90453d..b50a3a0799e6 100644
--- a/include/uapi/drm/etnaviv_drm.h
+++ b/include/uapi/drm/etnaviv_drm.h
@@ -265,6 +265,21 @@ struct drm_etnaviv_pm_signal {
 	char  name[64];   /* out, name of domain */
 };
 
+/*
+ * Reset status query:
+ *
+ * Both counters start at zero and only ever increase. Userspace saves
+ * both values and compares them on a later query: if the context
+ * counter moved this context caused a reset, if only the global
+ * counter moved the GPU was reset on behalf of another context.
+ */
+struct drm_etnaviv_reset_query {
+	__u32 pipe;			/* in */
+	__u32 flags;			/* in, must be 0 */
+	__u32 global_reset_counter;	/* out, all resets of this GPU core */
+	__u32 context_reset_counter;	/* out, resets caused by this context */
+};
+
 #define DRM_ETNAVIV_GET_PARAM          0x00
 /* placeholder:
 #define DRM_ETNAVIV_SET_PARAM          0x01
@@ -279,7 +294,8 @@ struct drm_etnaviv_pm_signal {
 #define DRM_ETNAVIV_GEM_WAIT           0x09
 #define DRM_ETNAVIV_PM_QUERY_DOM       0x0a
 #define DRM_ETNAVIV_PM_QUERY_SIG       0x0b
-#define DRM_ETNAVIV_NUM_IOCTLS         0x0c
+#define DRM_ETNAVIV_RESET_QUERY        0x0c
+#define DRM_ETNAVIV_NUM_IOCTLS         0x0d
 
 #define DRM_IOCTL_ETNAVIV_GET_PARAM    DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_GET_PARAM, struct drm_etnaviv_param)
 #define DRM_IOCTL_ETNAVIV_GEM_NEW      DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_GEM_NEW, struct drm_etnaviv_gem_new)
@@ -292,6 +308,7 @@ struct drm_etnaviv_pm_signal {
 #define DRM_IOCTL_ETNAVIV_GEM_WAIT     DRM_IOW(DRM_COMMAND_BASE + DRM_ETNAVIV_GEM_WAIT, struct drm_etnaviv_gem_wait)
 #define DRM_IOCTL_ETNAVIV_PM_QUERY_DOM DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_PM_QUERY_DOM, struct drm_etnaviv_pm_domain)
 #define DRM_IOCTL_ETNAVIV_PM_QUERY_SIG DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_PM_QUERY_SIG, struct drm_etnaviv_pm_signal)
+#define DRM_IOCTL_ETNAVIV_RESET_QUERY  DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_RESET_QUERY, struct drm_etnaviv_reset_query)
 
 #if defined(__cplusplus)
 }

-- 
2.54.0

Re: [PATCH v2 2/2] drm/etnaviv: Add GPU reset counters

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Concurrent modification and unannotated lockless reads of `gpu->reset_counter` and `ctx->reset_counter` lead to data races and lost increments.
--

--- Patch [2]: [PATCH v2 2/2] drm/etnaviv: Add GPU reset counters ---
commit 0f536ab6e10b2d00764a49473b750d556dcda8ee
Author: Christian Gmeiner <cgmeiner@igalia.com>

drm/etnaviv: Add GPU reset counters

This commit adds per-GPU and per-context reset counters to etnaviv to allow
userspace to detect GPU resets and determine if their context was guilty.

> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
[ ... ]
> +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;

[Severity: Medium]
Is it possible for these counters to be updated concurrently while being
read here? 

Since etnaviv_sched_timedout_job() can increment the counters on another CPU,
it seems this lockless read might need READ_ONCE() to prevent load tearing and
ensure the compiler doesn't optimize the read incorrectly.

> +
> +	return 0;
> +}
[ ... ]
> --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
[ ... ]
> @@ -79,6 +79,9 @@ static enum drm_gpu_sched_stat etnaviv_sched_timedout_job(struct drm_sched_job
>  	if(sched_job)
>  		drm_sched_increase_karma(sched_job);
>  
> +	gpu->reset_counter++;
> +	submit->ctx->reset_counter++;

[Severity: Medium]
Can a single userspace context submit jobs to multiple GPU pipes that
might hang at the same time? 

If so, could multiple per-GPU scheduler timeout workers execute
etnaviv_sched_timedout_job() simultaneously? If they do, both workers would
evaluate submit->ctx->reset_counter++ concurrently, which could lead to a
multi-writer data race and lost increments. 

Would it make sense to use atomic_t for the context reset counter, or at
least WRITE_ONCE() if exact counts are less critical than preventing data
races?

>  	/* get the GPU back into the init state */
>  	etnaviv_core_dump(submit);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-etnaviv-reset-notification-v2-0-90cfebcf22a2@igalia.com?part=2

Re: [PATCH v2 2/2] drm/etnaviv: Add GPU reset counters

[Lucas Stach]

Am Freitag, dem 10.07.2026 um 11:00 +0200 schrieb Christian Gmeiner:
> From: Christian Gmeiner <cgmeiner@igalia.com>
> 
> 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 <cgmeiner@igalia.com>
> ---
>  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.

Regards,
Lucas

> +
> +	return 0;
> +}
> +
>  static int etnaviv_ioctl_gem_new(struct drm_device *dev, void *data,
>  		struct drm_file *file)
>  {
> @@ -502,6 +526,7 @@ static const struct drm_ioctl_desc etnaviv_ioctls[] = {
>  	ETNA_IOCTL(GEM_WAIT,     gem_wait,     DRM_RENDER_ALLOW),
>  	ETNA_IOCTL(PM_QUERY_DOM, pm_query_dom, DRM_RENDER_ALLOW),
>  	ETNA_IOCTL(PM_QUERY_SIG, pm_query_sig, DRM_RENDER_ALLOW),
> +	ETNA_IOCTL(RESET_QUERY,  reset_query,  DRM_RENDER_ALLOW),
>  };
>  
>  static void etnaviv_show_fdinfo(struct drm_printer *p, struct drm_file *file)
> @@ -530,7 +555,7 @@ static const struct drm_driver etnaviv_drm_driver = {
>  	.name               = "etnaviv",
>  	.desc               = "etnaviv DRM",
>  	.major              = 1,
> -	.minor              = 4,
> +	.minor              = 5,
>  };
>  
>  /*
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index cba4323ae589..9c348aa7f8d3 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -34,6 +34,7 @@ struct etnaviv_file_private {
>  	int id;
>  	struct etnaviv_iommu_context	*mmu;
>  	struct drm_sched_entity		sched_entity[ETNA_MAX_PIPES];
> +	u32 reset_counter;
>  };
>  
>  struct etnaviv_drm_private {
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 5cb46c84e03a..a97780131426 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -148,6 +148,8 @@ struct etnaviv_gpu {
>  	u32 hangcheck_primid;
>  	u32 hangcheck_fence;
>  
> +	u32 reset_counter;
> +
>  	void __iomem *mmio;
>  	int irq;
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> index 139e6e38784b..bd1335a17f46 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> @@ -79,6 +79,9 @@ static enum drm_gpu_sched_stat etnaviv_sched_timedout_job(struct drm_sched_job
>  	if(sched_job)
>  		drm_sched_increase_karma(sched_job);
>  
> +	gpu->reset_counter++;
> +	submit->ctx->reset_counter++;
> +
>  	/* get the GPU back into the init state */
>  	etnaviv_core_dump(submit);
>  	etnaviv_gpu_recover_hang(submit);
> diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> index af024d90453d..b50a3a0799e6 100644
> --- a/include/uapi/drm/etnaviv_drm.h
> +++ b/include/uapi/drm/etnaviv_drm.h
> @@ -265,6 +265,21 @@ struct drm_etnaviv_pm_signal {
>  	char  name[64];   /* out, name of domain */
>  };
>  
> +/*
> + * Reset status query:
> + *
> + * Both counters start at zero and only ever increase. Userspace saves
> + * both values and compares them on a later query: if the context
> + * counter moved this context caused a reset, if only the global
> + * counter moved the GPU was reset on behalf of another context.
> + */
> +struct drm_etnaviv_reset_query {
> +	__u32 pipe;			/* in */
> +	__u32 flags;			/* in, must be 0 */
> +	__u32 global_reset_counter;	/* out, all resets of this GPU core */
> +	__u32 context_reset_counter;	/* out, resets caused by this context */
> +};
> +
>  #define DRM_ETNAVIV_GET_PARAM          0x00
>  /* placeholder:
>  #define DRM_ETNAVIV_SET_PARAM          0x01
> @@ -279,7 +294,8 @@ struct drm_etnaviv_pm_signal {
>  #define DRM_ETNAVIV_GEM_WAIT           0x09
>  #define DRM_ETNAVIV_PM_QUERY_DOM       0x0a
>  #define DRM_ETNAVIV_PM_QUERY_SIG       0x0b
> -#define DRM_ETNAVIV_NUM_IOCTLS         0x0c
> +#define DRM_ETNAVIV_RESET_QUERY        0x0c
> +#define DRM_ETNAVIV_NUM_IOCTLS         0x0d
>  
>  #define DRM_IOCTL_ETNAVIV_GET_PARAM    DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_GET_PARAM, struct drm_etnaviv_param)
>  #define DRM_IOCTL_ETNAVIV_GEM_NEW      DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_GEM_NEW, struct drm_etnaviv_gem_new)
> @@ -292,6 +308,7 @@ struct drm_etnaviv_pm_signal {
>  #define DRM_IOCTL_ETNAVIV_GEM_WAIT     DRM_IOW(DRM_COMMAND_BASE + DRM_ETNAVIV_GEM_WAIT, struct drm_etnaviv_gem_wait)
>  #define DRM_IOCTL_ETNAVIV_PM_QUERY_DOM DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_PM_QUERY_DOM, struct drm_etnaviv_pm_domain)
>  #define DRM_IOCTL_ETNAVIV_PM_QUERY_SIG DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_PM_QUERY_SIG, struct drm_etnaviv_pm_signal)
> +#define DRM_IOCTL_ETNAVIV_RESET_QUERY  DRM_IOWR(DRM_COMMAND_BASE + DRM_ETNAVIV_RESET_QUERY, struct drm_etnaviv_reset_query)
>  
>  #if defined(__cplusplus)
>  }

Re: [PATCH v2 2/2] drm/etnaviv: Add GPU reset counters

[Christian Gmeiner]

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 <cgmeiner@igalia.com>
> > ---
> >  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