From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kenneth Graunke Subject: Re: [PATCH 3/3] drm/i915: Track OACONTROL register enable/disable during parsing Date: Thu, 27 Mar 2014 14:58:01 -0700 Message-ID: <53349EE9.6090509@whitecape.org> References: <1395945820-20376-1-git-send-email-bradley.d.volkin@intel.com> <1395945820-20376-4-git-send-email-bradley.d.volkin@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1505439410==" Return-path: Received: from homiemail-a38.g.dreamhost.com (caiajhbdcaib.dreamhost.com [208.97.132.81]) by gabe.freedesktop.org (Postfix) with ESMTP id D93426EB23 for ; Thu, 27 Mar 2014 14:57:17 -0700 (PDT) In-Reply-To: <1395945820-20376-4-git-send-email-bradley.d.volkin@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: bradley.d.volkin@intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============1505439410== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="MIq5ggt0x8elIjTJxrnVPJesRl9o6OJ9s" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --MIq5ggt0x8elIjTJxrnVPJesRl9o6OJ9s Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 03/27/2014 11:43 AM, bradley.d.volkin@intel.com wrote: > From: Brad Volkin >=20 > There is some thought that the data from the performance counters enabl= ed > via OACONTROL should only be available to the process that enabled coun= ting. > To limit snooping, require that any batch buffer which sets OACONTROL t= o a > non-zero value also sets it back to 0 before the end of the batch. >=20 > This requires limiting OACONTROL writes to happen via MI_LOAD_REGISTER_= IMM > so that we can access the value being written. This should be in line w= ith > the expected use case for writing OACONTROL. >=20 > Cc: Kenneth Graunke > Signed-off-by: Brad Volkin > --- > drivers/gpu/drm/i915/i915_cmd_parser.c | 35 ++++++++++++++++++++++++++= -------- > 1 file changed, 27 insertions(+), 8 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i= 915/i915_cmd_parser.c > index 2eb2aca..779e14c 100644 > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c > @@ -407,12 +407,7 @@ static const u32 gen7_render_regs[] =3D { > REG64(CL_PRIMITIVES_COUNT), > REG64(PS_INVOCATION_COUNT), > REG64(PS_DEPTH_COUNT), > - /* > - * FIXME: This is just to keep mesa working for now, we need to check= > - * that mesa resets this again and that it doesn't use any of the > - * special modes which write into the gtt. > - */ > - OACONTROL, > + OACONTROL, /* Only allowed for LRI and SRM. See below. */ > REG64(GEN7_SO_NUM_PRIMS_WRITTEN(0)), > REG64(GEN7_SO_NUM_PRIMS_WRITTEN(1)), > REG64(GEN7_SO_NUM_PRIMS_WRITTEN(2)), > @@ -761,7 +756,8 @@ bool i915_needs_cmd_parser(struct intel_ring_buffer= *ring) > static bool check_cmd(const struct intel_ring_buffer *ring, > const struct drm_i915_cmd_descriptor *desc, > const u32 *cmd, > - const bool is_master) > + const bool is_master, > + bool *oacontrol_set) > { > if (desc->flags & CMD_DESC_REJECT) { > DRM_DEBUG_DRIVER("CMD: Rejected command: 0x%08X\n", *cmd); > @@ -777,6 +773,23 @@ static bool check_cmd(const struct intel_ring_buff= er *ring, > if (desc->flags & CMD_DESC_REGISTER) { > u32 reg_addr =3D cmd[desc->reg.offset] & desc->reg.mask; > =20 > + /* > + * OACONTROL requires some special handling for writes. We > + * want to make sure that any batch which enables OA also > + * disables it before the end of the batch. The goal is to > + * prevent one process from snooping on the perf data from > + * another process. To do that, we need to check the value > + * that will be written to the register. Hence, limit > + * OACONTROL writes to only MI_LOAD_REGISTER_IMM commands. > + */ > + if (reg_addr =3D=3D OACONTROL) { > + if (desc->cmd.value =3D=3D MI_LOAD_REGISTER_MEM) > + return false; While I don't need the ability to use LRM on OACONTROL, I don't see any specific reason to prohibit it, as long as there's a later LRI of 0. I think you could just do: if (desc->cmd.value =3D=3D MI_LOAD_REGISTER_MEM) oacontrol_set =3D true; which will later get cleared if it sees a LRI of 0, and if not, you reject the batch. > + > + if (desc->cmd.value =3D=3D MI_LOAD_REGISTER_IMM(1)) > + *oacontrol_set =3D (cmd[2] !=3D 0) ? true : false; You shouldn't need to do both !=3D 0 and ? true : false... *oacontrol_set =3D cmd[2] !=3D 0; Thanks for implementing this! That was surprisingly less code than I thought. > + } > + > if (!valid_reg(ring->reg_table, > ring->reg_count, reg_addr)) { > if (!is_master || > @@ -851,6 +864,7 @@ int i915_parse_cmds(struct intel_ring_buffer *ring,= > u32 *cmd, *batch_base, *batch_end; > struct drm_i915_cmd_descriptor default_desc =3D { 0 }; > int needs_clflush =3D 0; > + bool oacontrol_set =3D false; /* OACONTROL tracking. See check_cmd() = */ > =20 > ret =3D i915_gem_obj_prepare_shmem_read(batch_obj, &needs_clflush); > if (ret) { > @@ -900,7 +914,7 @@ int i915_parse_cmds(struct intel_ring_buffer *ring,= > break; > } > =20 > - if (!check_cmd(ring, desc, cmd, is_master)) { > + if (!check_cmd(ring, desc, cmd, is_master, &oacontrol_set)) { > ret =3D -EINVAL; > break; > } > @@ -908,6 +922,11 @@ int i915_parse_cmds(struct intel_ring_buffer *ring= , > cmd +=3D length; > } > =20 > + if (oacontrol_set) { > + DRM_DEBUG_DRIVER("CMD: batch set OACONTROL but did not clear it\n");= > + ret =3D -EINVAL; > + } > + > if (cmd >=3D batch_end) { > DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a BBE cmd!\n= "); > ret =3D -EINVAL; >=20 --MIq5ggt0x8elIjTJxrnVPJesRl9o6OJ9s Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJTNJ7uAAoJEFtb2gcdScw4BYAP/jtlRg446FPrpaZP0vmqB1pi xnBerIjjS7h3tvA7QC43MKwMgSTasuIrLdnF7nBsR4vK6t2h25Uvy2nS5H9xGdKt M1zsdcVqQEXhSXebSXU+fbpoQG4PCdqn9jRplwZaVhzB6D2nH4jYZxqPR+RcBNqQ IrvcfgpevqW5+LkjbFtMFQb7FZjoxdeC4Rc1DQL/lK+u7zxGCWwmSEkyjpOM4Cpo FoIBH2y0IUnZQImeIW9spJlpCZ0bGoeEiXRnxNzdrSR0NAQ4JzvSNt1+pKy4xV8e KZMtkA7xOb77r8k1aN+Frq1ORkRoVJw2f9xJcnE2X+Q4xAR5rhstpxi3EXCXurw8 zhz4BoxAo8f8QsH7fHTZZKcQvcLk7Mu1pH/q3U3SpPnuxBj+4Hg68NL/C+LdJYWw ovarUdEWSSL77T+9itAGZLPdQ2OMwnk3NenCpl8TSe8ZFci2wIJ0P1agf0e6Je2Y C/2Trf8fUPYblPc0jUtWZKi7537HbCU8/SAKnnNt+I/E0T2PiD+jOMdMHSrIHCtk w1ghYRdiRsy3NfxJ6kG+CrdBMUaVFkm15CJU1CTjG2diz1UDPmccZJLRKrVJbNG/ 7int4sRwwKdEXvX+uOy4n/wrrrqnDkDjdbWOkVvpwUQgRA/MvAsB4h2vxphEmnll zaiMKYkr8PFbLzSDviwu =R7Me -----END PGP SIGNATURE----- --MIq5ggt0x8elIjTJxrnVPJesRl9o6OJ9s-- --===============1505439410== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1505439410==--