From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robert Bragg Subject: Re: [PATCH v8 02/12] drm/i915: Add i915 perf infrastructure Date: Mon, 31 Oct 2016 16:27:49 +0000 Message-ID: References: <20161028021430.2177-1-robert@sixbynine.org> <20161028021430.2177-3-robert@sixbynine.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0671108388==" Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Matthew Auld Cc: ML dri-devel , Intel Graphics Development , Sourab Gupta , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org --===============0671108388== Content-Type: multipart/alternative; boundary=001a1147032e8dcbac05402bb10c --001a1147032e8dcbac05402bb10c Content-Type: text/plain; charset=UTF-8 On Fri, Oct 28, 2016 at 3:27 PM, Matthew Auld < matthew.william.auld@gmail.com> wrote: > > +/* Note we copy the properties from userspace outside of the i915 perf > > + * mutex to avoid an awkward lockdep with mmap_sem. > > + * > > + * Note this function only validates properties in isolation it doesn't > > + * validate that the combination of properties makes sense or that all > > + * properties necessary for a particular kind of stream have been set. > > + */ > > +static int read_properties_unlocked(struct drm_i915_private *dev_priv, > > + u64 __user *uprops, > > + u32 n_props, > > + struct perf_open_properties *props) > > +{ > > + u64 __user *uprop = uprops; > > + int i; > > + > > + memset(props, 0, sizeof(struct perf_open_properties)); > > + > > + if (!n_props) { > > + DRM_ERROR("No i915 perf properties given"); > > + return -EINVAL; > > + } > > + > > + if (n_props > DRM_I915_PERF_PROP_MAX) { > Ah but DRM_I915_PERF_PROP_MAX is not a property itself. > I'm not sure I follow what your implied concern is? This is just a sanity check for the number properties given by userspace, based on the assumption that there's currently no reason for multiple values with a particular property id. --001a1147032e8dcbac05402bb10c Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Fri, Oct 28, 2016 at 3:27 PM, Matthew Auld <matthew.wi= lliam.auld@gmail.com> wrote:
> +/* Note we copy the properties from userspace outs= ide of the i915 perf
> + * mutex to avoid an awkward lockdep with mmap_sem.
> + *
> + * Note this function only validates properties in isolation it doesn= 't
> + * validate that the combination of properties makes sense or that al= l
> + * properties necessary for a particular kind of stream have been set= .
> + */
> +static int read_properties_unlocked(struct drm_i915_private *dev= _priv,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0u64 __user *uprops,=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0u32 n_props,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0struct perf_open_pr= operties *props)
> +{
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0u64 __user *uprop =3D uprops;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0int i;
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0memset(props, 0, sizeof(struct perf_open_p= roperties));
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!n_props) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DRM_ERROR(&quo= t;No i915 perf properties given");
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL= ;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0}
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0if (n_props > DRM_I915_PERF_PROP_MAX) {=
Ah but DRM_I915_PERF_PROP_MAX is not a property itself.

I'm not sure I follow what your implied concern= is?

This is just a sanity check for the number propertie= s given by userspace, based on the assumption that there's currently no= reason for multiple values with a particular property id.
<= br>
--001a1147032e8dcbac05402bb10c-- --===============0671108388== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0671108388==--