linux-kernel - RE: [Intel-gfx] drm/i915: Avoid accessing the stolen address when it is unavailable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 25 Oct 2013 00:27:42 +0000
From:	"Liu, Chuansheng" <chuansheng.liu@...el.com>
To:	Ben Widawsky <ben@...dawsk.net>,
	Chris Wilson <chris@...is-wilson.co.uk>,
	"daniel.vetter@...ll.ch" <daniel.vetter@...ll.ch>,
	"airlied@...ux.ie" <airlied@...ux.ie>,
	"intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Li, Fei" <fei.li@...el.com>
Subject: RE: [Intel-gfx] drm/i915: Avoid accessing the stolen address when
 it is unavailable

Hello Chris and Ben,

> -----Original Message-----
> From: Ben Widawsky [mailto:ben@...dawsk.net]
> Sent: Friday, October 25, 2013 4:57 AM
> To: Chris Wilson; Liu, Chuansheng; daniel.vetter@...ll.ch; airlied@...ux.ie;
> intel-gfx@...ts.freedesktop.org; dri-devel@...ts.freedesktop.org;
> linux-kernel@...r.kernel.org; Li, Fei
> Subject: Re: [Intel-gfx] drm/i915: Avoid accessing the stolen address when it is
> unavailable
> 
> On Thu, Oct 24, 2013 at 01:17:06PM +0100, Chris Wilson wrote:
> > On Fri, Oct 25, 2013 at 12:33:47AM +0800, Chuansheng Liu wrote:
> > >
> > > In our platform, we hit the the stolen region initialization failure case,
> > > such as below log:
> > > [drm:i915_stolen_to_physical] *ERROR* conflict detected with stolen
> region: [0x7b000000]
> > >
> > > And it causes the dev_priv->mm.stolen_base is NULL, in this case, we
> should
> > > avoid accessing it any more.
> > >
> > > Here is possible call trace:
> > > intel_enable_gt_powersave -- >
> > > valleyview_enable_rps -- >
> > > valleyview_setup_pctx
> >
> > The two create_stolen routines are no-ops in that case so all that
> > happens instead is that we read VLV_PCBR. However, really if
> > i915_gem_object_create_stolen_for_preallocated() fails we should abort
> > loading the driver as it means we have a hardware conflict and undefined
> > behaviour.
In case of dev_priv->mm.stolen_base == NULL, and the valleyview_setup_pctx() is called
at the first time, it will call i915_gem_object_create_stolen_for_preallocated(), which should
should return NULL always due to (!drm_mm_initialized(&dev_priv->mm.stolen)).

After that, every time specially when doing pm operation, the above scenario will
be called again and again.

Here this patch is to save some time for PM operation, we do not need to read
VLV_PCBR and pcbr_offset calculation in case of stolen_base == NULL.

Is it making sense? Thanks.

> 
> I agree. We should start treating these things as errors since no
> RPS/RC6 is essentially not what anyone wants.
> 
> DRM_DEBUG_DRIVER, and add a check in valleyview_enable_rps for the pctx
> value.
The pctx is already checked in valleyview_disable_rps().
Do we need more checking in case of pctx == NULL?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/