On Fri, Jan 27, 2017 at 07:05:57PM +0200, Ville Syrjälä wrote:
> On Fri, Jan 27, 2017 at 04:55:31PM +0000, Chris Wilson wrote:
> > Just sanity check that the value we deduce from the stolen memory
> > register fits within the kernel's dma_addr_t and doesn't overflow.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_gem_stolen.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > index 42bbc4b04fd6..4f1f3090c0ed 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> > @@ -211,6 +211,14 @@ static dma_addr_t i915_stolen_to_dma(struct drm_i915_private *dev_priv)
> > ggtt_start &= PGTBL_ADDRESS_LO_MASK;
> > ggtt_end = ggtt_start + ggtt_total_entries(ggtt) * 4;
> >
> > + if (ggtt_end <= ggtt_start ||
> > + overflows_type(ggtt_end, dma_addr_t)) {
> > + DRM_ERROR("DMA address for reserved igfx memory [%llx - %llx] does not fit within the kernel's %db dma_addr_t\n",
> > + ggtt_start, ggtt_end,
> > + (int)sizeof(dma_addr_t) * 8);
> > + return 0;
> > + }
>
> This would only check if the ggtt location fits into dma_addr_t.
> We don't need that since we never touch the ggtt directly with
> either the CPU or GPU. So I think this piece of code should keep
> on using u64 as it needs to be able to hold the 36 address we read
> from the hardware/firmware.
Ok, I was just looking for u64 in the vicinity of dma_addr_t. So we are
just checking that the ggtt doesn't overlap stolen, not creating stolen
addresses from ggtt. Yup, we don't need to check against dma_addr_t
here - that I can leave for future register checking.
The sanity check for stolen would actually just be:
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -189,7 +189,7 @@ static dma_addr_t i915_stolen_to_dma(struct drm_i915_private *dev_priv)
base = tom - tseg_size - ggtt->stolen_size;
}
- if (base == 0)
+ if (base == 0 || ggtt->stolen_size + base < base)
return 0;
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
