| Message ID | 20171012225447.256-8-michal.wajdeczko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/13/2017 4:24 AM, Michal Wajdeczko wrote: > Checking GuC firmware size can be done in GuC specific code > right before DMA copy as it is unlikely to fail anyway. > > v2: rebased > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> #1 > --- > drivers/gpu/drm/i915/intel_guc_fw.c | 7 ++++++- > drivers/gpu/drm/i915/intel_uc_fw.c | 8 -------- > 2 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c > index 71dacc3..020ce26 100644 > --- a/drivers/gpu/drm/i915/intel_guc_fw.c > +++ b/drivers/gpu/drm/i915/intel_guc_fw.c > @@ -133,6 +133,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, > unsigned long offset; > struct sg_table *sg = vma->pages; > u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT]; > + u32 size = guc_fw->header_size + guc_fw->ucode_size; > int i, ret = 0; > > /* where RSA signature starts */ > @@ -145,7 +146,11 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, > > /* The header plus uCode will be copied to WOPCM via DMA, excluding any > * other components */ > - I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size); > + if (size > intel_guc_wopcm_size(dev_priv)) { This size check better be done before RSA copy. With that: Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > + DRM_ERROR("Firmware is too large to fit in WOPCM\n"); > + return -EFBIG; > + } > + I915_WRITE(DMA_COPY_SIZE, size); > > /* Set the source address for the new blob */ > offset = guc_ggtt_offset(vma) + guc_fw->header_offset; > diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c > index 766b1cb..482115b 100644 > --- a/drivers/gpu/drm/i915/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/intel_uc_fw.c > @@ -108,14 +108,6 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, > */ > switch (uc_fw->type) { > case INTEL_UC_FW_TYPE_GUC: > - /* Header and uCode will be loaded to WOPCM. Size of the two. */ > - size = uc_fw->header_size + uc_fw->ucode_size; > - > - /* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */ > - if (size > intel_guc_wopcm_size(dev_priv)) { > - DRM_ERROR("Firmware is too large to fit in WOPCM\n"); > - goto fail; > - } > uc_fw->major_ver_found = css->guc.sw_version >> 16; > uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF; > break;
On Fri, 13 Oct 2017 08:49:22 +0200, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > > > On 10/13/2017 4:24 AM, Michal Wajdeczko wrote: >> Checking GuC firmware size can be done in GuC specific code >> right before DMA copy as it is unlikely to fail anyway. >> >> v2: rebased >> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> #1 >> --- >> drivers/gpu/drm/i915/intel_guc_fw.c | 7 ++++++- >> drivers/gpu/drm/i915/intel_uc_fw.c | 8 -------- >> 2 files changed, 6 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c >> b/drivers/gpu/drm/i915/intel_guc_fw.c >> index 71dacc3..020ce26 100644 >> --- a/drivers/gpu/drm/i915/intel_guc_fw.c >> +++ b/drivers/gpu/drm/i915/intel_guc_fw.c >> @@ -133,6 +133,7 @@ static int guc_ucode_xfer_dma(struct >> drm_i915_private *dev_priv, >> unsigned long offset; >> struct sg_table *sg = vma->pages; >> u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT]; >> + u32 size = guc_fw->header_size + guc_fw->ucode_size; >> int i, ret = 0; >> /* where RSA signature starts */ >> @@ -145,7 +146,11 @@ static int guc_ucode_xfer_dma(struct >> drm_i915_private *dev_priv, >> /* The header plus uCode will be copied to WOPCM via DMA, >> excluding any >> * other components */ >> - I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size); >> + if (size > intel_guc_wopcm_size(dev_priv)) { > This size check better be done before RSA copy. With that: > Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Hmm, I was planning to separate handling of RSA signature out of this function and focus here only on DMA xfer, so that move will be void. And I still think that size check is better to be done closer to the place where that size will be used in HW commands. Michal >> + DRM_ERROR("Firmware is too large to fit in WOPCM\n"); >> + return -EFBIG; >> + } >> + I915_WRITE(DMA_COPY_SIZE, size); >> /* Set the source address for the new blob */ >> offset = guc_ggtt_offset(vma) + guc_fw->header_offset; >> diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c >> b/drivers/gpu/drm/i915/intel_uc_fw.c >> index 766b1cb..482115b 100644 >> --- a/drivers/gpu/drm/i915/intel_uc_fw.c >> +++ b/drivers/gpu/drm/i915/intel_uc_fw.c >> @@ -108,14 +108,6 @@ void intel_uc_fw_fetch(struct drm_i915_private >> *dev_priv, >> */ >> switch (uc_fw->type) { >> case INTEL_UC_FW_TYPE_GUC: >> - /* Header and uCode will be loaded to WOPCM. Size of the two. */ >> - size = uc_fw->header_size + uc_fw->ucode_size; >> - >> - /* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */ >> - if (size > intel_guc_wopcm_size(dev_priv)) { >> - DRM_ERROR("Firmware is too large to fit in WOPCM\n"); >> - goto fail; >> - } >> uc_fw->major_ver_found = css->guc.sw_version >> 16; >> uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF; >> break;
diff --git a/drivers/gpu/drm/i915/intel_guc_fw.c b/drivers/gpu/drm/i915/intel_guc_fw.c index 71dacc3..020ce26 100644 --- a/drivers/gpu/drm/i915/intel_guc_fw.c +++ b/drivers/gpu/drm/i915/intel_guc_fw.c @@ -133,6 +133,7 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, unsigned long offset; struct sg_table *sg = vma->pages; u32 status, rsa[UOS_RSA_SCRATCH_MAX_COUNT]; + u32 size = guc_fw->header_size + guc_fw->ucode_size; int i, ret = 0; /* where RSA signature starts */ @@ -145,7 +146,11 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv, /* The header plus uCode will be copied to WOPCM via DMA, excluding any * other components */ - I915_WRITE(DMA_COPY_SIZE, guc_fw->header_size + guc_fw->ucode_size); + if (size > intel_guc_wopcm_size(dev_priv)) { + DRM_ERROR("Firmware is too large to fit in WOPCM\n"); + return -EFBIG; + } + I915_WRITE(DMA_COPY_SIZE, size); /* Set the source address for the new blob */ offset = guc_ggtt_offset(vma) + guc_fw->header_offset; diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c index 766b1cb..482115b 100644 --- a/drivers/gpu/drm/i915/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/intel_uc_fw.c @@ -108,14 +108,6 @@ void intel_uc_fw_fetch(struct drm_i915_private *dev_priv, */ switch (uc_fw->type) { case INTEL_UC_FW_TYPE_GUC: - /* Header and uCode will be loaded to WOPCM. Size of the two. */ - size = uc_fw->header_size + uc_fw->ucode_size; - - /* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */ - if (size > intel_guc_wopcm_size(dev_priv)) { - DRM_ERROR("Firmware is too large to fit in WOPCM\n"); - goto fail; - } uc_fw->major_ver_found = css->guc.sw_version >> 16; uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF; break;