On Wed, May 29, 2019 at 01:03:35PM +0300, Dan Carpenter wrote:
> Hi Syrjälä,
>
> I had a question about commit c457d9cf256e: ("drm/i915: Make sure we have
> enough memory bandwidth on ICL").
>
> drivers/gpu/drm/i915/intel_bw.c
> 64 static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
> 65 struct intel_qgv_point *sp,
> 66 int point)
> 67 {
> 68 u32 val = 0, val2;
> ^^^^
> "val2" is uninitialized.
>
> 69 int ret;
> 70
> 71 ret = sandybridge_pcode_read(dev_priv,
> 72 ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> 73 ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point),
> 74 &val, &val2);
> 75 if (ret)
> 76 return ret;
> 77
> 78 sp->dclk = val & 0xffff;
> 79 sp->t_rp = (val & 0xff0000) >> 16;
> 80 sp->t_rcd = (val & 0xff000000) >> 24;
> 81
> 82 sp->t_rdpre = val2 & 0xff;
> 83 sp->t_ras = (val2 & 0xff00) >> 8;
> 84
> 85 sp->t_rc = sp->t_rp + sp->t_ras;
> 86
> 87 return 0;
> 88 }
>
> drivers/gpu/drm/i915/intel_sideband.c
> 376 static int __sandybridge_pcode_rw(struct drm_i915_private *i915,
> 377 u32 mbox, u32 *val, u32 *val1,
> 378 int fast_timeout_us,
> 379 int slow_timeout_ms,
> 380 bool is_read)
> 381 {
> 382 struct intel_uncore *uncore = &i915->uncore;
> 383
> 384 lockdep_assert_held(&i915->sb_lock);
> 385
> 386 /*
> 387 * GEN6_PCODE_* are outside of the forcewake domain, we can
> 388 * use te fw I915_READ variants to reduce the amount of work
> 389 * required when reading/writing.
> 390 */
> 391
> 392 if (intel_uncore_read_fw(uncore, GEN6_PCODE_MAILBOX) & GEN6_PCODE_READY)
> 393 return -EAGAIN;
> 394
> 395 intel_uncore_write_fw(uncore, GEN6_PCODE_DATA, *val);
> 396 intel_uncore_write_fw(uncore, GEN6_PCODE_DATA1, val1 ? *val1 : 0);
> ^^^^^
> We write uninitialized value out here. I'm sort of surprised that
> UBSan doesn't complain. I don't know the code well enough to say if
> this is a problem.
Thanks for catching this. I suspect this pcode command doesn't care
about the initial value in that register, but I can't be 100% sure.
So we should initialize it just in case.
>
> 397 intel_uncore_write_fw(uncore,
> 398 GEN6_PCODE_MAILBOX, GEN6_PCODE_READY | mbox);
> 399
> 400 if (__intel_wait_for_register_fw(uncore,
> 401 GEN6_PCODE_MAILBOX,
> 402 GEN6_PCODE_READY, 0,
> 403 fast_timeout_us,
> 404 slow_timeout_ms,
> 405 &mbox))
> 406 return -ETIMEDOUT;
>
> regards,
> dan carpenter
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
