vc4: Bring over cleanups from submitting to the kernel.
authorEric Anholt <eric@anholt.net>
Sat, 5 Dec 2015 20:25:25 +0000 (12:25 -0800)
committerEric Anholt <eric@anholt.net>
Sat, 5 Dec 2015 21:12:27 +0000 (13:12 -0800)
src/gallium/drivers/vc4/kernel/vc4_render_cl.c
src/gallium/drivers/vc4/kernel/vc4_validate.c
src/gallium/drivers/vc4/kernel/vc4_validate_shaders.c
src/gallium/drivers/vc4/vc4_simulator_validate.h

index b827eb7e9e1f147daf4f10afb33755cea52a451a..31784b797711f9ddc14fa4043482fa1a4b2a4326 100644 (file)
@@ -62,7 +62,6 @@ static inline void rcl_u32(struct vc4_rcl_setup *setup, u32 val)
        setup->next_offset += 4;
 }
 
-
 /*
  * Emits a no-op STORE_TILE_BUFFER_GENERAL.
  *
@@ -255,6 +254,7 @@ static int vc4_create_rcl_bo(struct drm_device *dev, struct vc4_exec_info *exec,
                for (x = min_x_tile; x <= max_x_tile; x++) {
                        bool first = (x == min_x_tile && y == min_y_tile);
                        bool last = (x == max_x_tile && y == max_y_tile);
+
                        emit_tile(exec, setup, x, y, first, last);
                }
        }
@@ -414,7 +414,8 @@ int vc4_get_rcl(struct drm_device *dev, struct vc4_exec_info *exec)
        if (has_bin &&
            (args->max_x_tile > exec->bin_tiles_x ||
             args->max_y_tile > exec->bin_tiles_y)) {
-               DRM_ERROR("Render tiles (%d,%d) outside of bin config (%d,%d)\n",
+               DRM_ERROR("Render tiles (%d,%d) outside of bin config "
+                         "(%d,%d)\n",
                          args->max_x_tile, args->max_y_tile,
                          exec->bin_tiles_x, exec->bin_tiles_y);
                return -EINVAL;
index b248831113cb2a0526cab235b7f9d8a7974774d2..a50e9c382616654b17de9be45c7a922c5ac81d4c 100644 (file)
@@ -47,7 +47,6 @@
        void *validated,                                \
        void *untrusted
 
-
 /** Return the width in pixels of a 64-byte microtile. */
 static uint32_t
 utile_width(int cpp)
@@ -191,7 +190,7 @@ vc4_check_tex_size(struct vc4_exec_info *exec, struct drm_gem_cma_object *fbo,
 
        if (size + offset < size ||
            size + offset > fbo->base.size) {
-               DRM_ERROR("Overflow in %dx%d (%dx%d) fbo size (%d + %d > %d)\n",
+               DRM_ERROR("Overflow in %dx%d (%dx%d) fbo size (%d + %d > %zd)\n",
                          width, height,
                          aligned_width, aligned_height,
                          size, offset, fbo->base.size);
@@ -201,7 +200,6 @@ vc4_check_tex_size(struct vc4_exec_info *exec, struct drm_gem_cma_object *fbo,
        return true;
 }
 
-
 static int
 validate_flush(VALIDATE_ARGS)
 {
@@ -270,7 +268,7 @@ validate_indexed_prim_list(VALIDATE_ARGS)
 
        if (offset > ib->base.size ||
            (ib->base.size - offset) / index_size < length) {
-               DRM_ERROR("IB access overflow (%d + %d*%d > %d)\n",
+               DRM_ERROR("IB access overflow (%d + %d*%d > %zd)\n",
                          offset, length, index_size, ib->base.size);
                return -EINVAL;
        }
@@ -424,8 +422,8 @@ validate_gem_handles(VALIDATE_ARGS)
        return 0;
 }
 
-#define VC4_DEFINE_PACKET(packet, name, func) \
-       [packet] = { packet ## _SIZE, name, func }
+#define VC4_DEFINE_PACKET(packet, func) \
+       [packet] = { packet ## _SIZE, #packet, func }
 
 static const struct cmd_info {
        uint16_t len;
@@ -433,42 +431,42 @@ static const struct cmd_info {
        int (*func)(struct vc4_exec_info *exec, void *validated,
                    void *untrusted);
 } cmd_info[] = {
-       VC4_DEFINE_PACKET(VC4_PACKET_HALT, "halt", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_NOP, "nop", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_FLUSH, "flush", validate_flush),
-       VC4_DEFINE_PACKET(VC4_PACKET_FLUSH_ALL, "flush all state", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_START_TILE_BINNING, "start tile binning", validate_start_tile_binning),
-       VC4_DEFINE_PACKET(VC4_PACKET_INCREMENT_SEMAPHORE, "increment semaphore", validate_increment_semaphore),
-
-       VC4_DEFINE_PACKET(VC4_PACKET_GL_INDEXED_PRIMITIVE, "Indexed Primitive List", validate_indexed_prim_list),
-
-       VC4_DEFINE_PACKET(VC4_PACKET_GL_ARRAY_PRIMITIVE, "Vertex Array Primitives", validate_gl_array_primitive),
-
-       /* This is only used by clipped primitives (packets 48 and 49), which
-        * we don't support parsing yet.
-        */
-       VC4_DEFINE_PACKET(VC4_PACKET_PRIMITIVE_LIST_FORMAT, "primitive list format", NULL),
-
-       VC4_DEFINE_PACKET(VC4_PACKET_GL_SHADER_STATE, "GL Shader State", validate_gl_shader_state),
-       /* We don't support validating NV shader states. */
-
-       VC4_DEFINE_PACKET(VC4_PACKET_CONFIGURATION_BITS, "configuration bits", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_FLAT_SHADE_FLAGS, "flat shade flags", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_POINT_SIZE, "point size", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_LINE_WIDTH, "line width", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_RHT_X_BOUNDARY, "RHT X boundary", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_DEPTH_OFFSET, "Depth Offset", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_CLIP_WINDOW, "Clip Window", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_VIEWPORT_OFFSET, "Viewport Offset", NULL),
-       VC4_DEFINE_PACKET(VC4_PACKET_CLIPPER_XY_SCALING, "Clipper XY Scaling", NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_HALT, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_NOP, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_FLUSH, validate_flush),
+       VC4_DEFINE_PACKET(VC4_PACKET_FLUSH_ALL, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_START_TILE_BINNING,
+                         validate_start_tile_binning),
+       VC4_DEFINE_PACKET(VC4_PACKET_INCREMENT_SEMAPHORE,
+                         validate_increment_semaphore),
+
+       VC4_DEFINE_PACKET(VC4_PACKET_GL_INDEXED_PRIMITIVE,
+                         validate_indexed_prim_list),
+       VC4_DEFINE_PACKET(VC4_PACKET_GL_ARRAY_PRIMITIVE,
+                         validate_gl_array_primitive),
+
+       VC4_DEFINE_PACKET(VC4_PACKET_PRIMITIVE_LIST_FORMAT, NULL),
+
+       VC4_DEFINE_PACKET(VC4_PACKET_GL_SHADER_STATE, validate_gl_shader_state),
+
+       VC4_DEFINE_PACKET(VC4_PACKET_CONFIGURATION_BITS, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_FLAT_SHADE_FLAGS, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_POINT_SIZE, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_LINE_WIDTH, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_RHT_X_BOUNDARY, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_DEPTH_OFFSET, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_CLIP_WINDOW, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_VIEWPORT_OFFSET, NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_CLIPPER_XY_SCALING, NULL),
        /* Note: The docs say this was also 105, but it was 106 in the
         * initial userland code drop.
         */
-       VC4_DEFINE_PACKET(VC4_PACKET_CLIPPER_Z_SCALING, "Clipper Z Scale and Offset", NULL),
+       VC4_DEFINE_PACKET(VC4_PACKET_CLIPPER_Z_SCALING, NULL),
 
-       VC4_DEFINE_PACKET(VC4_PACKET_TILE_BINNING_MODE_CONFIG, "tile binning configuration", validate_tile_binning_config),
+       VC4_DEFINE_PACKET(VC4_PACKET_TILE_BINNING_MODE_CONFIG,
+                         validate_tile_binning_config),
 
-       VC4_DEFINE_PACKET(VC4_PACKET_GEM_HANDLES, "GEM handles", validate_gem_handles),
+       VC4_DEFINE_PACKET(VC4_PACKET_GEM_HANDLES, validate_gem_handles),
 };
 
 int
@@ -500,11 +498,6 @@ vc4_validate_bin_cl(struct drm_device *dev,
                        return -EINVAL;
                }
 
-#if 0
-               DRM_INFO("0x%08x: packet %d (%s) size %d processing...\n",
-                        src_offset, cmd, info->name, info->len);
-#endif
-
                if (src_offset + info->len > len) {
                        DRM_ERROR("0x%08x: packet %d (%s) length 0x%08x "
                                  "exceeds bounds (0x%08x)\n",
@@ -519,8 +512,7 @@ vc4_validate_bin_cl(struct drm_device *dev,
                if (info->func && info->func(exec,
                                             dst_pkt + 1,
                                             src_pkt + 1)) {
-                       DRM_ERROR("0x%08x: packet %d (%s) failed to "
-                                 "validate\n",
+                       DRM_ERROR("0x%08x: packet %d (%s) failed to validate\n",
                                  src_offset, cmd, info->name);
                        return -EINVAL;
                }
@@ -588,12 +580,14 @@ reloc_tex(struct vc4_exec_info *exec,
 
        if (sample->is_direct) {
                uint32_t remaining_size = tex->base.size - p0;
+
                if (p0 > tex->base.size - 4) {
                        DRM_ERROR("UBO offset greater than UBO size\n");
                        goto fail;
                }
                if (p1 > remaining_size - 4) {
-                       DRM_ERROR("UBO clamp would allow reads outside of UBO\n");
+                       DRM_ERROR("UBO clamp would allow reads "
+                                 "outside of UBO\n");
                        goto fail;
                }
                *validated_p0 = tex->paddr + p0;
@@ -875,7 +869,8 @@ validate_gl_shader_rec(struct drm_device *dev,
                        max_index = ((vbo->base.size - offset - attr_size) /
                                     stride);
                        if (state->max_index > max_index) {
-                               DRM_ERROR("primitives use index %d out of supplied %d\n",
+                               DRM_ERROR("primitives use index %d out of "
+                                         "supplied %d\n",
                                          state->max_index, max_index);
                                return -EINVAL;
                        }
index ab9a6512e828e8a4d6c4247b494a3c2b34338689..868a0ad1a3ab95a7a34c883a3459ae106bb5ee9f 100644 (file)
 /**
  * DOC: Shader validator for VC4.
  *
- * The VC4 has no IOMMU between it and system memory.  So, a user with access
- * to execute shaders could escalate privilege by overwriting system memory
- * (using the VPM write address register in the general-purpose DMA mode) or
- * reading system memory it shouldn't (reading it as a texture, or uniform
- * data, or vertex data).
+ * The VC4 has no IOMMU between it and system memory, so a user with
+ * access to execute shaders could escalate privilege by overwriting
+ * system memory (using the VPM write address register in the
+ * general-purpose DMA mode) or reading system memory it shouldn't
+ * (reading it as a texture, or uniform data, or vertex data).
  *
- * This walks over a shader starting from some offset within a BO, ensuring
- * that its accesses are appropriately bounded, and recording how many texture
- * accesses are made and where so that we can do relocations for them in the
+ * This walks over a shader BO, ensuring that its accesses are
+ * appropriately bounded, and recording how many texture accesses are
+ * made and where so that we can do relocations for them in the
  * uniform stream.
- *
- * The kernel API has shaders stored in user-mapped BOs.  The BOs will be
- * forcibly unmapped from the process before validation, and any cache of
- * validated state will be flushed if the mapping is faulted back in.
- *
- * Storing the shaders in BOs means that the validation process will be slow
- * due to uncached reads, but since shaders are long-lived and shader BOs are
- * never actually modified, this shouldn't be a problem.
  */
 
 #include "vc4_drv.h"
@@ -71,7 +63,6 @@ waddr_to_live_reg_index(uint32_t waddr, bool is_b)
                else
                        return waddr;
        } else if (waddr <= QPU_W_ACC3) {
-
                return 64 + waddr - QPU_W_ACC0;
        } else {
                return ~0;
@@ -86,15 +77,14 @@ raddr_add_a_to_live_reg_index(uint64_t inst)
        uint32_t raddr_a = QPU_GET_FIELD(inst, QPU_RADDR_A);
        uint32_t raddr_b = QPU_GET_FIELD(inst, QPU_RADDR_B);
 
-       if (add_a == QPU_MUX_A) {
+       if (add_a == QPU_MUX_A)
                return raddr_a;
-       } else if (add_a == QPU_MUX_B && sig != QPU_SIG_SMALL_IMM) {
+       else if (add_a == QPU_MUX_B && sig != QPU_SIG_SMALL_IMM)
                return 32 + raddr_b;
-       } else if (add_a <= QPU_MUX_R3) {
+       else if (add_a <= QPU_MUX_R3)
                return 64 + add_a;
-       } else {
+       else
                return ~0;
-       }
 }
 
 static bool
@@ -112,9 +102,9 @@ is_tmu_write(uint32_t waddr)
 }
 
 static bool
-record_validated_texture_sample(struct vc4_validated_shader_info *validated_shader,
-                               struct vc4_shader_validation_state *validation_state,
-                               int tmu)
+record_texture_sample(struct vc4_validated_shader_info *validated_shader,
+                     struct vc4_shader_validation_state *validation_state,
+                     int tmu)
 {
        uint32_t s = validated_shader->num_texture_samples;
        int i;
@@ -227,8 +217,8 @@ check_tmu_write(uint64_t inst,
                validated_shader->uniforms_size += 4;
 
        if (submit) {
-               if (!record_validated_texture_sample(validated_shader,
-                                                    validation_state, tmu)) {
+               if (!record_texture_sample(validated_shader,
+                                          validation_state, tmu)) {
                        return false;
                }
 
@@ -239,10 +229,10 @@ check_tmu_write(uint64_t inst,
 }
 
 static bool
-check_register_write(uint64_t inst,
-                    struct vc4_validated_shader_info *validated_shader,
-                    struct vc4_shader_validation_state *validation_state,
-                    bool is_mul)
+check_reg_write(uint64_t inst,
+               struct vc4_validated_shader_info *validated_shader,
+               struct vc4_shader_validation_state *validation_state,
+               bool is_mul)
 {
        uint32_t waddr = (is_mul ?
                          QPU_GET_FIELD(inst, QPU_WADDR_MUL) :
@@ -298,7 +288,7 @@ check_register_write(uint64_t inst,
                return true;
 
        case QPU_W_TLB_STENCIL_SETUP:
-                return true;
+               return true;
        }
 
        return true;
@@ -361,7 +351,7 @@ track_live_clamps(uint64_t inst,
                }
 
                validation_state->live_max_clamp_regs[lri_add] = true;
-       } if (op_add == QPU_A_MIN) {
+       } else if (op_add == QPU_A_MIN) {
                /* Track live clamps of a value clamped to a minimum of 0 and
                 * a maximum of some uniform's offset.
                 */
@@ -393,8 +383,10 @@ check_instruction_writes(uint64_t inst,
                return false;
        }
 
-       ok = (check_register_write(inst, validated_shader, validation_state, false) &&
-             check_register_write(inst, validated_shader, validation_state, true));
+       ok = (check_reg_write(inst, validated_shader, validation_state,
+                             false) &&
+             check_reg_write(inst, validated_shader, validation_state,
+                             true));
 
        track_live_clamps(inst, validated_shader, validation_state);
 
@@ -442,7 +434,7 @@ vc4_validate_shader(struct drm_gem_cma_object *shader_obj)
        shader = shader_obj->vaddr;
        max_ip = shader_obj->base.size / sizeof(uint64_t);
 
-       validated_shader = kcalloc(sizeof(*validated_shader), 1, GFP_KERNEL);
+       validated_shader = kcalloc(1, sizeof(*validated_shader), GFP_KERNEL);
        if (!validated_shader)
                return NULL;
 
@@ -498,7 +490,7 @@ vc4_validate_shader(struct drm_gem_cma_object *shader_obj)
 
        if (ip == max_ip) {
                DRM_ERROR("shader failed to terminate before "
-                         "shader BO end at %d\n",
+                         "shader BO end at %zd\n",
                          shader_obj->base.size);
                goto fail;
        }
@@ -514,6 +506,9 @@ vc4_validate_shader(struct drm_gem_cma_object *shader_obj)
        return validated_shader;
 
 fail:
-       kfree(validated_shader);
+       if (validated_shader) {
+               kfree(validated_shader->texture_samples);
+               kfree(validated_shader);
+       }
        return NULL;
 }
index 68ace0216aaeffb655952237554a4b3cbd12efe4..40d3ada6ca2883295def4efc5e621cc8c211e61b 100644 (file)
@@ -65,7 +65,7 @@ struct drm_device {
 };
 
 struct drm_gem_object {
-        uint32_t size;
+        size_t size;
         struct drm_device *dev;
 };