Quoting Matthew Auld (2021-01-25 11:28:16) > On Sun, 24 Jan 2021 at 13:54, Chris Wilson <[email protected]> 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 <[email protected]> > > --- > > 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?
For the purpose of error-capture, we will survive without. (I guess we should include a note in the error capture as to which path was used to retrieve the data.) Far better to risk error capture not being fully functional than aborting the driver load. (Although what could go wrong here... A driver bug?) For the secondary purpose of preventing use of 0x0, if the reservation fails, then we know something is at 0x0 and we have to trust that it remains in place preventing our user. -Chris _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
