[bug report] drm/i915: Make sure we have enough memory bandwidth on ICL

From: Dan Carpenter <dan.carpenter@oracle.com>
To: ville.syrjala@linux.intel.com
Cc: intel-gfx@lists.freedesktop.org
Subject: [bug report] drm/i915: Make sure we have enough memory bandwidth on ICL
Date: Wed, 29 May 2019 13:03:35 +0300	[thread overview]
Message-ID: <20190529100335.GA10365@mwanda> (raw)

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.

   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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx