[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 11:18:12AM +0000, Chris Wilson wrote:
> On Thu, Mar 16, 2017 at 12:06:20PM +0100, Michal Wajdeczko wrote:
> > 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?
> 
> I was thinking next to the write so that if they were changed, copied,
> it would be more obvious to update; as well as the check being clearly
> associated with the write.

Your intention was clear ;)
But, since they would be spread across over the function, it would be
easy to forget about the origin of these BUILD_BUGs as they will be far
from the introduction comment ... One can easily forget to add them for
new fields, or even drop on rename or something.
Note that by moving them here, we also make this comment more visible
and associated with strong enforcement (at least to the level that we
can handle today)

-Michal