[Intel-gfx] [PATCH v3 2/5] drm/i915/wopcm: Check WOPCM layout separately from calculations

[Michal Wajdeczko]

On Fri, 16 Aug 2019 13:21:03 +0200, Chris Wilson  
<chris at chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2019-08-16 11:54:58)
>> +static inline bool __check_layout(struct drm_i915_private *i915, u32  
>> wopcm_size,
>> +                                 u32 guc_wopcm_base, u32  
>> guc_wopcm_size,
>> +                                 u32 guc_fw_size, u32 huc_fw_size)
>> +{
>> +       const u32 ctx_rsvd = context_reserved_size(i915);
>> +       u32 size;
>> +
>> +       size = wopcm_size - ctx_rsvd;
>
> I didn't spot the paranoia for
>
> if (ctx_rsvd > wopcm_size)
> 	return false;
>
> Is that built in earlier? Even so, probably still wise to include it here
> as well to fit in with the overflow checks.

was added to intel_wopcm_init() that calls this function, look for:

+	GEM_BUG_ON(ctx_rsvd + WOPCM_RESERVED_SIZE >= wopcm->size);

>
>> +       if (unlikely(range_overflows(guc_wopcm_base, guc_wopcm_size,  
>> size))) {
>> +               dev_err(i915->drm.dev,
>> +                       "WOPCM: invalid GuC region layout: %uK + %uK >  
>> %uK\n",
>> +                       guc_wopcm_base / SZ_1K, guc_wopcm_size / SZ_1K,
>> +                       size / SZ_1K);
>> +               return false;
>> +       }
>> +
>> +       size = guc_fw_size + GUC_WOPCM_RESERVED +  
>> GUC_WOPCM_STACK_RESERVED;
>> +       if (unlikely(guc_wopcm_size < size)) {
>> +               dev_err(i915->drm.dev, "WOPCM: no space for %s: %uK <  
>> %uK\n",
>> +                       intel_uc_fw_type_repr(INTEL_UC_FW_TYPE_GUC),
>> +                       guc_wopcm_size / SZ_1K, size / SZ_1K);
>> +               return false;
>> +       }
>> +
>> +       size = huc_fw_size + WOPCM_RESERVED_SIZE;
>> +       if (unlikely(guc_wopcm_base < size)) {
>> +               dev_err(i915->drm.dev, "WOPCM: no space for %s: %uK <  
>> %uK\n",
>> +                       intel_uc_fw_type_repr(INTEL_UC_FW_TYPE_HUC),
>> +                       guc_wopcm_base / SZ_1K, size / SZ_1K);
>> +               return false;
>> +       }
>> +
>> +       return check_hw_restrictions(i915, guc_wopcm_base,  
>> guc_wopcm_size,
>> +                                    huc_fw_size);
>>  }
>
> Looks safely paranoid to me,
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> -Chris