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

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: Matthew Auld <matthew.auld@intel.com>
Subject: Re: [PATCH 1/2] drm/i915: Remove variable length arrays from sseu debugfs printers
Date: Tue, 13 Mar 2018 09:35:03 +0000	[thread overview]
Message-ID: <ea54ac2a-e77b-3712-3363-eba085ebbf3c@linux.intel.com> (raw)
In-Reply-To: <20180313004055.25411-1-chris@chris-wilson.co.uk>

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@chris-wilson.co.uk>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Matthew Auld <matthew.auld@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@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx