[Intel-gfx] [PATCH 2/2] drm/i915/gt: Always try to reserve GGTT address 0x0

[Matthew Auld]

On Sun, 24 Jan 2021 at 13:54, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>
> Since writing to address 0 is a very common mistake, let's try to avoid
> putting anything sensitive there.
>
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/2989
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_ggtt.c | 40 +++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index dac07d66f658..3a737d4fbc3c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -535,16 +535,32 @@ static int init_ggtt(struct i915_ggtt *ggtt)
>
>         mutex_init(&ggtt->error_mutex);
>         if (ggtt->mappable_end) {
> -               /* Reserve a mappable slot for our lockless error capture */
> -               ret = drm_mm_insert_node_in_range(&ggtt->vm.mm,
> -                                                 &ggtt->error_capture,
> -                                                 PAGE_SIZE, 0,
> -                                                 I915_COLOR_UNEVICTABLE,
> -                                                 0, ggtt->mappable_end,
> -                                                 DRM_MM_INSERT_LOW);
> -               if (ret)
> -                       return ret;
> +               /*
> +                * Reserve a mappable slot for our lockless error capture.
> +                *
> +                * We strongly prefer taking address 0x0 in order to protect
> +                * other critical buffers against accidental overwrites,
> +                * as writing to address 0 is a very common mistake.
> +                *
> +                * Since 0 may already be in use by the system (e.g. the BIOS
> +                * framebuffer), we let the reservation fail quietly and hope
> +                * 0 remains reserved always.
> +                */
> +               ggtt->error_capture.size = I915_GTT_PAGE_SIZE;
> +               ggtt->error_capture.color = I915_COLOR_UNEVICTABLE;
> +               if (drm_mm_reserve_node(&ggtt->vm.mm, &ggtt->error_capture))
> +                       drm_mm_insert_node_in_range(&ggtt->vm.mm,
> +                                                   &ggtt->error_capture,
> +                                                   ggtt->error_capture.size, 0,
> +                                                   ggtt->error_capture.color,
> +                                                   0, ggtt->mappable_end,
> +                                                   DRM_MM_INSERT_LOW);

We don't want to check the in_range for an error? It should be
impossible I guess?

Reviewed-by: Matthew Auld <matthew.auld at intel.com>

>         }
> +       if (drm_mm_node_allocated(&ggtt->error_capture))
> +               drm_dbg(&ggtt->vm.i915->drm,
> +                       "Reserved GGTT:[%llx, %llx] for use by error capture\n",
> +                       ggtt->error_capture.start,
> +                       ggtt->error_capture.start + ggtt->error_capture.size);
>
>         /*
>          * The upper portion of the GuC address space has a sizeable hole
> @@ -557,9 +573,9 @@ static int init_ggtt(struct i915_ggtt *ggtt)
>
>         /* Clear any non-preallocated blocks */
>         drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) {
> -               drm_dbg_kms(&ggtt->vm.i915->drm,
> -                           "clearing unused GTT space: [%lx, %lx]\n",
> -                           hole_start, hole_end);
> +               drm_dbg(&ggtt->vm.i915->drm,
> +                       "clearing unused GTT space: [%lx, %lx]\n",
> +                       hole_start, hole_end);
>                 ggtt->vm.clear_range(&ggtt->vm, hole_start,
>                                      hole_end - hole_start);
>         }
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx