[Mesa-dev] [PATCH v2 1/2] panfrost: Jobs must be per context, not per screen

Rohan Garg rohan.garg at collabora.com
Fri Aug 30 16:00:12 UTC 2019


Jobs _must_ only be shared across the same context, having
the last_job tracked in a screen causes use-after-free issues
and memory corruptions.

Signed-off-by: Rohan Garg <rohan.garg at collabora.com>
---
 src/gallium/drivers/panfrost/pan_context.c | 10 +++++-----
 src/gallium/drivers/panfrost/pan_context.h |  6 ++++++
 src/gallium/drivers/panfrost/pan_drm.c     |  6 +++---
 src/gallium/drivers/panfrost/pan_screen.c  |  3 ---
 src/gallium/drivers/panfrost/pan_screen.h  |  6 ------
 5 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index fa9c92af9f6..94ee9b5bdb2 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -1329,9 +1329,6 @@ panfrost_submit_frame(struct panfrost_context *ctx, bool flush_immediate,
                       struct pipe_fence_handle **fence,
                       struct panfrost_job *job)
 {
-        struct pipe_context *gallium = (struct pipe_context *) ctx;
-        struct panfrost_screen *screen = pan_screen(gallium->screen);
-
         panfrost_job_submit(ctx, job);
 
         /* If visual, we can stall a frame */
@@ -1339,8 +1336,8 @@ panfrost_submit_frame(struct panfrost_context *ctx, bool flush_immediate,
         if (!flush_immediate)
                 panfrost_drm_force_flush_fragment(ctx, fence);
 
-        screen->last_fragment_flushed = false;
-        screen->last_job = job;
+        ctx->last_fragment_flushed = false;
+        ctx->last_job = job;
 
         /* If readback, flush now (hurts the pipelined performance) */
         if (flush_immediate)
@@ -2856,6 +2853,9 @@ panfrost_create_context(struct pipe_screen *screen, void *priv, unsigned flags)
         assert(ctx->blitter);
         assert(ctx->blitter_wallpaper);
 
+        ctx->last_fragment_flushed = true;
+        ctx->last_job = NULL;
+
         /* Prepare for render! */
 
         panfrost_job_init(ctx);
diff --git a/src/gallium/drivers/panfrost/pan_context.h b/src/gallium/drivers/panfrost/pan_context.h
index 4c1580b3393..9f96e983a86 100644
--- a/src/gallium/drivers/panfrost/pan_context.h
+++ b/src/gallium/drivers/panfrost/pan_context.h
@@ -203,6 +203,12 @@ struct panfrost_context {
         bool is_t6xx;
 
         uint32_t out_sync;
+
+        /* While we're busy building up the job for frame N, the GPU is
+         * still busy executing frame N-1. So hold a reference to
+         * yesterjob */
+        int last_fragment_flushed;
+        struct panfrost_job *last_job;
 };
 
 /* Corresponds to the CSO */
diff --git a/src/gallium/drivers/panfrost/pan_drm.c b/src/gallium/drivers/panfrost/pan_drm.c
index c3693bff56a..f89bc1d26eb 100644
--- a/src/gallium/drivers/panfrost/pan_drm.c
+++ b/src/gallium/drivers/panfrost/pan_drm.c
@@ -349,12 +349,12 @@ panfrost_drm_force_flush_fragment(struct panfrost_context *ctx,
         struct pipe_context *gallium = (struct pipe_context *) ctx;
         struct panfrost_screen *screen = pan_screen(gallium->screen);
 
-        if (!screen->last_fragment_flushed) {
+        if (!ctx->last_fragment_flushed) {
                 drmSyncobjWait(screen->fd, &ctx->out_sync, 1, INT64_MAX, 0, NULL);
-                screen->last_fragment_flushed = true;
+                ctx->last_fragment_flushed = true;
 
                 /* The job finished up, so we're safe to clean it up now */
-                panfrost_free_job(ctx, screen->last_job);
+                panfrost_free_job(ctx, ctx->last_job);
         }
 
         if (fence) {
diff --git a/src/gallium/drivers/panfrost/pan_screen.c b/src/gallium/drivers/panfrost/pan_screen.c
index 36c91a1572e..5c288f52bbd 100644
--- a/src/gallium/drivers/panfrost/pan_screen.c
+++ b/src/gallium/drivers/panfrost/pan_screen.c
@@ -665,9 +665,6 @@ panfrost_create_screen(int fd, struct renderonly *ro)
         screen->base.fence_finish = panfrost_fence_finish;
         screen->base.set_damage_region = panfrost_resource_set_damage_region;
 
-        screen->last_fragment_flushed = true;
-        screen->last_job = NULL;
-
         panfrost_resource_screen_init(screen);
 
         return &screen->base;
diff --git a/src/gallium/drivers/panfrost/pan_screen.h b/src/gallium/drivers/panfrost/pan_screen.h
index 35fb8de2628..7991b395f54 100644
--- a/src/gallium/drivers/panfrost/pan_screen.h
+++ b/src/gallium/drivers/panfrost/pan_screen.h
@@ -118,12 +118,6 @@ struct panfrost_screen {
          * Each bucket is a linked list of free panfrost_bo objects. */
 
         struct list_head bo_cache[NR_BO_CACHE_BUCKETS];
-
-        /* While we're busy building up the job for frame N, the GPU is
-         * still busy executing frame N-1. So hold a reference to
-         * yesterjob */
-        int last_fragment_flushed;
-        struct panfrost_job *last_job;
 };
 
 static inline struct panfrost_screen *
-- 
2.17.1



More information about the mesa-dev mailing list