Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> It is required that the caller declare the exact number of dwords they
> wish to write into the ring. This is required for two reasons, we need
> to allocate sufficient space for the entire command packet and we need
> to be sure that the contents are completely written to avoid executing
> stale data. The current interface requires for any bug to be caught in
> review, the reader has to carefully count the number of
> intel_ring_emit() between intel_ring_begin() and intel_ring_advance().
> If we record the end of the packet of each intel_ring_begin() we can
> also have CI check for us.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem.h | 9 +++++++++
> drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
> drivers/gpu/drm/i915/intel_ringbuffer.h | 3 +++
> 3 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index a585d47c420a..412fa2794cbb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -28,9 +28,18 @@
> #ifdef CONFIG_DRM_I915_DEBUG_GEM
> #define GEM_BUG_ON(expr) BUG_ON(expr)
> #define GEM_WARN_ON(expr) WARN_ON(expr)
> +
> +#define GEM_BUG_ONLY(expr) expr
> +#define GEM_BUG_ONLY_DECLARE(var) var
> +#define GEM_BUG_ONLY_ON(expr) GEM_BUG_ON(expr)
> +
> #else
> #define GEM_BUG_ON(expr) BUILD_BUG_ON_INVALID(expr)
> #define GEM_WARN_ON(expr) (BUILD_BUG_ON_INVALID(expr), 0)
> +
> +#define GEM_BUG_ONLY(expr) do { } while (0)
> +#define GEM_BUG_ONLY_DECLARE(var)
> +#define GEM_BUG_ONLY_ON(expr)
> #endif
>
> #define I915_NUM_ENGINES 5
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index d32cbba25d98..383083ef2210 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2238,6 +2238,7 @@ int intel_ring_begin(struct drm_i915_gem_request *req, int num_dwords)
>
> ring->space -= bytes;
> GEM_BUG_ON(ring->space < 0);
> + GEM_BUG_ONLY(ring->advance = ring->tail + bytes);
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index b9c15cd40fbf..2c6d3655985e 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -144,6 +144,8 @@ struct intel_ring {
>
> u32 head;
> u32 tail;
> + GEM_BUG_ONLY_DECLARE(u32 advance);
> +
> int space;
> int size;
> int effective_size;
> @@ -516,6 +518,7 @@ static inline void intel_ring_advance(struct intel_ring *ring)
> * reserved for the command packet (i.e. the value passed to
> * intel_ring_begin()).
> */
> + GEM_BUG_ONLY_ON(ring->tail != ring->advance);
> }
>
> static inline u32 intel_ring_offset(struct intel_ring *ring, void *addr)
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
