[Intel-gfx] [PATCH] drm/i915/guc: Document that the ads blob entries only lie within the first page

[Michal Wajdeczko]

On Thu, Mar 16, 2017 at 10:58:20AM +0000, Chris Wilson wrote:
> On Thu, Mar 16, 2017 at 10:55:06AM +0000, Chris Wilson wrote:
> > guc_addon_create() makes the assumption that it need only kmap the
> > initial page in order to write all of the configuration data used by the
> > guc. Confusingly it also allocates many scratch pages in the same vma
> > and passes that to the guc. Reassure the reader that all is well with a
> > BUILD_BUG_ON() that we do not access outside of the kmapped page.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Oscar Mateo <oscar.mateo at intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_guc_submission.c | 5 +++++
> >  drivers/gpu/drm/i915/i915_utils.h          | 1 +
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> > index 97726fcb1230..91d7ab0df0cd 100644
> > --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> > @@ -888,13 +888,16 @@ static void guc_addon_create(struct intel_guc *guc)
> >  		guc->ads_vma = vma;
> >  	}
> >  
> > +	/* First members are assumed to be in a single page */
> 
> s/First/Written/

Btw, maybe it would be better to move all these BUILD_BUGs here?

> 
> >  	page = i915_vma_first_page(vma);
> >  	blob = kmap(page);
> >  
> >  	/* GuC scheduling policies */
> > +	BUILD_BUG_ON(ptr_offset_end(blob, policies) > PAGE_SIZE);
> >  	guc_policies_init(&blob->policies);
> >  
> >  	/* MMIO reg state */
> > +	BUILD_BUG_ON(ptr_offset_end(blob, reg_state) > PAGE_SIZE);
> >  	for_each_engine(engine, dev_priv, id) {
> >  		blob->reg_state.mmio_white_list[engine->guc_id].mmio_start =
> >  			engine->mmio_base + GUC_MMIO_WHITE_LIST_START;
> > @@ -903,6 +906,8 @@ static void guc_addon_create(struct intel_guc *guc)
> >  		blob->reg_state.mmio_white_list[engine->guc_id].count = 0;
> >  	}
> >  
> > +	BUILD_BUG_ON(ptr_offset_end(blob, reg_state) > PAGE_SIZE);

s/reg_state/ads


-Michal

> > +
> >  	/*
> >  	 * The GuC requires a "Golden Context" when it reinitialises
> >  	 * engines after a reset. Here we use the Render ring default
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre