[Mesa-dev] [PATCH] i965: Implement bounds checking for transform feedback output.

[Paul Berry]

On 15 December 2011 15:20, Kenneth Graunke <kenneth at whitecape.org> wrote:

> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_context.c |    1 +
>  src/mesa/drivers/dri/i965/brw_context.h |    3 ++
>  src/mesa/drivers/dri/i965/brw_gs_emit.c |   10 ++++++++
>  src/mesa/drivers/dri/i965/gen6_sol.c    |   38
> +++++++++++++++++++++++++++++++
>  4 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 7d360b0..fd60853 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -117,6 +117,7 @@ static void brwInitDriverFunctions( struct
> dd_function_table *functions )
>    brw_init_queryobj_functions(functions);
>
>    functions->PrepareExecBegin = brwPrepareExecBegin;
> +   functions->BeginTransformFeedback = brw_begin_transform_feedback;
>    functions->EndTransformFeedback = brw_end_transform_feedback;
>  }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 8e52488..20623d4 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1073,6 +1073,9 @@ brw_fprog_uses_noperspective(const struct
> gl_fragment_program *fprog);
>
>  /* gen6_sol.c */
>  void
> +brw_begin_transform_feedback(struct gl_context *ctx,
> +                            struct gl_transform_feedback_object *obj);
> +void
>  brw_end_transform_feedback(struct gl_context *ctx,
>                            struct gl_transform_feedback_object *obj);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_gs_emit.c
> b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> index 72d4eca..5dd3734 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs_emit.c
> @@ -352,6 +352,15 @@ gen6_sol_program(struct brw_gs_compile *c, struct
> brw_gs_prog_key *key,
>        */
>       brw_MOV(p, get_element_ud(c->reg.header, 5),
>               get_element_ud(c->reg.SVBI, 0));
> +
> +      /* Make sure that the buffers have enough room for all the
> vertices. */
> +      brw_ADD(p, get_element_ud(c->reg.temp, 0),
> +                get_element_ud(c->reg.SVBI, 0), brw_imm_ud(num_verts));
> +      brw_CMP(p, vec1(brw_null_reg()), BRW_CONDITIONAL_L,
> +                get_element_ud(c->reg.temp, 0),
> +                get_element_ud(c->reg.SVBI, 4));
> +      brw_IF(p, BRW_EXECUTE_1);
> +
>       /* For each vertex, generate code to output each varying using the
>        * appropriate binding table entry.
>        */
> @@ -392,6 +401,7 @@ gen6_sol_program(struct brw_gs_compile *c, struct
> brw_gs_prog_key *key,
>                     get_element_ud(c->reg.header, 5), brw_imm_ud(1));
>          }
>       }
> +      brw_ENDIF(p);
>
>       /* Now, reinitialize the header register from R0 to restore the
> parts of
>        * the register that we overwrote while streaming out transform
> feedback
> diff --git a/src/mesa/drivers/dri/i965/gen6_sol.c
> b/src/mesa/drivers/dri/i965/gen6_sol.c
> index b11bce2..56d4a6a 100644
> --- a/src/mesa/drivers/dri/i965/gen6_sol.c
> +++ b/src/mesa/drivers/dri/i965/gen6_sol.c
> @@ -26,6 +26,7 @@
>  * Code to initialize the binding table entries used by transform feedback.
>  */
>
> +#include "main/macros.h"
>  #include "brw_context.h"
>  #include "intel_buffer_objects.h"
>  #include "intel_batchbuffer.h"
> @@ -101,6 +102,43 @@ const struct brw_tracked_state gen6_sol_surface = {
>  };
>
>  void
> +brw_begin_transform_feedback(struct gl_context *ctx,
> +                            struct gl_transform_feedback_object *obj)
> +{
> +   struct intel_context *intel = intel_context(ctx);
> +   const struct gl_shader_program *vs_prog =
> +      ctx->Shader.CurrentVertexProgram;
> +   const struct gl_transform_feedback_info *linked_xfb_info =
> +      &vs_prog->LinkedTransformFeedback;
> +   struct gl_transform_feedback_object *xfb_obj =
> +      ctx->TransformFeedback.CurrentObject;
> +
> +   unsigned max_index = 0xffffffff;
> +
> +   /* Compute the maximum number of vertices that we can write without
> +    * overflowing any of the buffers currently being used for feedback.
> +    */
> +   for (int i = 0; i < MAX_FEEDBACK_ATTRIBS; ++i) {
>

Minor nit: MAX_FEEDBACK_ATTRIBS (32) is the correct bound for generic Mesa
code, but in i965, we only support 4 buffers, so this loop bound should
probably be BRW_MAX_SOL_BUFFERS.


> +      unsigned stride = linked_xfb_info->BufferStride[i];
> +
> +      /* Skip any inactive buffers, which have a stride of 0. */
> +      if (stride == 0)
> +        continue;
> +
> +      unsigned max_for_this_buffer = xfb_obj->Size[i] / (4 * stride);
> +      max_index = MIN2(max_index, max_for_this_buffer);
> +   }
> +
> +   /* Initialize the SVBI 0 register to zero and set the maximum index. */
> +   BEGIN_BATCH(4);
> +   OUT_BATCH(_3DSTATE_GS_SVB_INDEX << 16 | (4 - 2));
> +   OUT_BATCH(0); /* SVBI 0 */
> +   OUT_BATCH(0);
> +   OUT_BATCH(max_index);
> +   ADVANCE_BATCH();
> +}
> +
> +void
>  brw_end_transform_feedback(struct gl_context *ctx,
>                            struct gl_transform_feedback_object *obj)
>  {
> --
> 1.7.7.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

Other than that one minor comment, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111216/b5bedce2/attachment-0001.htm>