[Intel-gfx] [PATCH 1/2] drm/i915: Remove variable length arrays from sseu debugfs printers

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-03-13 09:35:03)
> 
> On 13/03/2018 00:40, Chris Wilson wrote:
> > In order to enable -Wvla to prevent new variable length arrays being
> > used in i915.ko, we first must remove the existing VLA. Inside
> > i915_print_sseu_info(), VLA are used as the actual size of the sseu
> > depends on platform. Replace the VLA with the maximum required.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: Matthew Auld <matthew.auld at intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++------
> >   1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index c4cc8fef11a0..0eac7dcdddbf 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -4312,9 +4312,10 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_cache_sharing_fops,
> >   static void cherryview_sseu_device_status(struct drm_i915_private *dev_priv,
> >                                         struct sseu_dev_info *sseu)
> >   {
> > -     int ss_max = 2;
> > +#define SS_MAX 2
> > +     const int ss_max = SS_MAX;
> > +     u32 sig1[SS_MAX], sig2[SS_MAX];
> >       int ss;
> > -     u32 sig1[ss_max], sig2[ss_max];
> 
> Even const, sigh.
> 
> >   
> >       sig1[0] = I915_READ(CHV_POWER_SS0_SIG1);
> >       sig1[1] = I915_READ(CHV_POWER_SS1_SIG1);
> > @@ -4338,15 +4339,15 @@ static void cherryview_sseu_device_status(struct drm_i915_private *dev_priv,
> >               sseu->eu_per_subslice = max_t(unsigned int,
> >                                             sseu->eu_per_subslice, eu_cnt);
> >       }
> > +#undef SS_MAX
> >   }
> >   
> >   static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
> >                                    struct sseu_dev_info *sseu)
> >   {
> >       const struct intel_device_info *info = INTEL_INFO(dev_priv);
> > +     u32 s_reg[6], eu_reg[2 * 4], eu_mask[2];
> 
> For future improvement I think pulling out 6 and 4 to something like 
> GEN10_MAX_SLICES and GEN10_MAX_SUBSLICES would be good. Just so numbers 
> are centralized and we remove any possibility of walking out of bounds. 
> Actually for all of the impacted platforms.
> 
> >       int s, ss;
> > -     u32 s_reg[info->sseu.max_slices];
> > -     u32 eu_reg[2 * info->sseu.max_subslices], eu_mask[2];
> >   
> >       for (s = 0; s < info->sseu.max_slices; s++) {
> >               /*
> > @@ -4399,9 +4400,8 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
> >                                   struct sseu_dev_info *sseu)
> >   {
> >       const struct intel_device_info *info = INTEL_INFO(dev_priv);
> > +     u32 s_reg[3], eu_reg[2 * 4], eu_mask[2];
> >       int s, ss;
> > -     u32 s_reg[info->sseu.max_slices];
> > -     u32 eu_reg[2 * info->sseu.max_subslices], eu_mask[2];
> >   
> >       for (s = 0; s < info->sseu.max_slices; s++) {
> >               s_reg[s] = I915_READ(GEN9_SLICE_PGCTL_ACK(s));
> > 
> 
> Maximums are correct so for now it is workable:
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

I figure I'd let someone else figure out how to write GENX_MAX_SLICES
nicely and push this pair while we have no more VLA in the driver. :)

Thanks for the review,
-Chris