Re: [PATCH 1/3] drm/i915: Fix command parser to validate multiple register access with the same command.

From: Francisco Jerez <currojerez@riseup.net>
To: "Daniel Vetter" <daniel@ffwll.ch>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/i915: Fix command parser to validate multiple register access with the same command.
Date: Mon, 15 Jun 2015 15:05:24 +0300	[thread overview]
Message-ID: <87a8w1kwhn.fsf@riseup.net> (raw)
In-Reply-To: <20150615114048.GD8341@phenom.ffwll.local>

[-- Attachment #1.1.1: Type: text/plain, Size: 1735 bytes --]

Daniel Vetter <daniel@ffwll.ch> writes:

> On Mon, Jun 15, 2015 at 02:26:22PM +0300, Ville Syrjälä wrote:
>> On Mon, Jun 15, 2015 at 02:18:01PM +0300, Francisco Jerez wrote:
>> > Daniel Vetter <daniel@ffwll.ch> writes:
>> > 
>> > > On Tue, Jun 02, 2015 at 05:36:26PM +0800, Zhigang Gong wrote:
>> > >> The patchset LGTM and works well with beignet. The 80%+ performance regression issue in darktable also has been fixed
>> > >> after this patchset applied and enable the atomic in L3 at beignet side. So,
>> > >> 
>> > >> Reviewed-by: Zhigang Gong <zhigang.gong@linux.intel.com>
>> > >
>> > > All three merged.
>> > 
>> > Thanks Daniel.
>> > 
>> > > Aside: Dont we need an increment for the cmd parser version for
>> > > userspace to be able to detect this?
>> > >
>> > Yeah, that would be a good idea, patch attached.
>> 
>> The old version alloweed userspace to write basically any register, the
>> new version allows only the whitelisted registers. I don't see how a
>> version number bump would help anyone.
>
> Oops, totally missed the context of patch 1. Jani I think that one's for
> you too ...
>
IMHO the version bump is still useful for userspace to find out whether
it can use plain LRIs to write the L3 atomic chicken bits.  It's true
that as Ville said it would have been possible for userspace to write
the same bits before this series by building a batch specifically
crafted to cheat the command parser, but I don't think we want userspace
to rely on a command parser bug (e.g. because we may want to back-port
the fix to earlier kernel versions).

> Thanks for pointing this out.
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 212 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx