diff mbox

[v2] drm/i915: don't whitelist oacontrol in cmd parser

Message ID 20161108125148.25007-1-robert@sixbynine.org (mailing list archive)
State New, archived
Headers show

Commit Message

Robert Bragg Nov. 8, 2016, 12:51 p.m. UTC
This v2 patch bumps the command parser version so it can be referenced in
corresponding i-g-t gem_exec_parse changes.

--- >8 ---

Being able to program OACONTROL from a non-privileged batch buffer is
not sufficient to be able to configure the OA unit. This was originally
allowed to help enable Mesa to expose OA counters via the
INTEL_performance_query extension, but the current implementation based
on programming OACONTROL via a batch buffer isn't able to report useable
data without a more complete OA unit configuration. Mesa handles the
possibility that writes to OACONTROL may not be allowed and so only
advertises the extension after explicitly testing that a write to
OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
should be ok for userspace.

Removing this simplifies adding a new kernel api for configuring the OA
unit without needing to consider the possibility that userspace might
trample on OACONTROL state which we'd like to start managing within
the kernel instead. In particular running any Mesa based GL application
currently results in clearing OACONTROL when initializing which would
disable the capturing of metrics.

v2:
    This bumps the command parser version from 8 to 9, as the change is
    visible to userspace.

Signed-off-by: Robert Bragg <robert@sixbynine.org>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Sourab Gupta <sourab.gupta@intel.com>
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 42 ++++------------------------------
 1 file changed, 5 insertions(+), 37 deletions(-)

Comments

Daniel Vetter Nov. 22, 2016, 1:34 p.m. UTC | #1
On Tue, Nov 08, 2016 at 12:51:48PM +0000, Robert Bragg wrote:
> This v2 patch bumps the command parser version so it can be referenced in
> corresponding i-g-t gem_exec_parse changes.
> 
> --- >8 ---

Scissors cut everything below, not everything above, hence next time
around pls switch around your comment and the commit message, as-is not
much left ;-)

Fixed up while applying.
-Daniel

> 
> Being able to program OACONTROL from a non-privileged batch buffer is
> not sufficient to be able to configure the OA unit. This was originally
> allowed to help enable Mesa to expose OA counters via the
> INTEL_performance_query extension, but the current implementation based
> on programming OACONTROL via a batch buffer isn't able to report useable
> data without a more complete OA unit configuration. Mesa handles the
> possibility that writes to OACONTROL may not be allowed and so only
> advertises the extension after explicitly testing that a write to
> OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
> should be ok for userspace.
> 
> Removing this simplifies adding a new kernel api for configuring the OA
> unit without needing to consider the possibility that userspace might
> trample on OACONTROL state which we'd like to start managing within
> the kernel instead. In particular running any Mesa based GL application
> currently results in clearing OACONTROL when initializing which would
> disable the capturing of metrics.
> 
> v2:
>     This bumps the command parser version from 8 to 9, as the change is
>     visible to userspace.
> 
> Signed-off-by: Robert Bragg <robert@sixbynine.org>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> Reviewed-by: Sourab Gupta <sourab.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_cmd_parser.c | 42 ++++------------------------------
>  1 file changed, 5 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index c9d2ecd..f5762cd 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -450,7 +450,6 @@ static const struct drm_i915_reg_descriptor gen7_render_regs[] = {
>  	REG64(PS_INVOCATION_COUNT),
>  	REG64(PS_DEPTH_COUNT),
>  	REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
> -	REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below. */
>  	REG64(MI_PREDICATE_SRC0),
>  	REG64(MI_PREDICATE_SRC1),
>  	REG32(GEN7_3DPRIM_END_OFFSET),
> @@ -1060,8 +1059,7 @@ bool intel_engine_needs_cmd_parser(struct intel_engine_cs *engine)
>  static bool check_cmd(const struct intel_engine_cs *engine,
>  		      const struct drm_i915_cmd_descriptor *desc,
>  		      const u32 *cmd, u32 length,
> -		      const bool is_master,
> -		      bool *oacontrol_set)
> +		      const bool is_master)
>  {
>  	if (desc->flags & CMD_DESC_SKIP)
>  		return true;
> @@ -1099,31 +1097,6 @@ static bool check_cmd(const struct intel_engine_cs *engine,
>  			}
>  
>  			/*
> -			 * 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 == i915_mmio_reg_offset(GEN7_OACONTROL)) {
> -				if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
> -					DRM_DEBUG_DRIVER("CMD: Rejected LRM to OACONTROL\n");
> -					return false;
> -				}
> -
> -				if (desc->cmd.value == MI_LOAD_REGISTER_REG) {
> -					DRM_DEBUG_DRIVER("CMD: Rejected LRR to OACONTROL\n");
> -					return false;
> -				}
> -
> -				if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1))
> -					*oacontrol_set = (cmd[offset + 1] != 0);
> -			}
> -
> -			/*
>  			 * Check the value written to the register against the
>  			 * allowed mask/value pair given in the whitelist entry.
>  			 */
> @@ -1214,7 +1187,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
>  	u32 *cmd, *batch_end;
>  	struct drm_i915_cmd_descriptor default_desc = noop_desc;
>  	const struct drm_i915_cmd_descriptor *desc = &default_desc;
> -	bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */
>  	bool needs_clflush_after = false;
>  	int ret = 0;
>  
> @@ -1270,8 +1242,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
>  			break;
>  		}
>  
> -		if (!check_cmd(engine, desc, cmd, length, is_master,
> -			       &oacontrol_set)) {
> +		if (!check_cmd(engine, desc, cmd, length, is_master)) {
>  			ret = -EACCES;
>  			break;
>  		}
> @@ -1279,11 +1250,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
>  		cmd += length;
>  	}
>  
> -	if (oacontrol_set) {
> -		DRM_DEBUG_DRIVER("CMD: batch set OACONTROL but did not clear it\n");
> -		ret = -EINVAL;
> -	}
> -
>  	if (cmd >= batch_end) {
>  		DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a BBE cmd!\n");
>  		ret = -EINVAL;
> @@ -1336,6 +1302,8 @@ int i915_cmd_parser_get_version(struct drm_i915_private *dev_priv)
>  	 * 8. Don't report cmd_check() failures as EINVAL errors to userspace;
>  	 *    rely on the HW to NOOP disallowed commands as it would without
>  	 *    the parser enabled.
> +	 * 9. Don't whitelist or handle oacontrol specially, as ownership
> +	 *    for oacontrol state is moving to i915-perf.
>  	 */
> -	return 8;
> +	return 9;
>  }
> -- 
> 2.10.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Robert Bragg Nov. 22, 2016, 2:09 p.m. UTC | #2
On Tue, Nov 22, 2016 at 1:34 PM, Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Nov 08, 2016 at 12:51:48PM +0000, Robert Bragg wrote:
> > This v2 patch bumps the command parser version so it can be referenced in
> > corresponding i-g-t gem_exec_parse changes.
> >
> > --- >8 ---
>
> Scissors cut everything below, not everything above, hence next time
> around pls switch around your comment and the commit message, as-is not
> much left ;-)
>

Hmm, they cut away what's above and keep what's below in my experience -
what command are you seeing the opposite with?

I just double checked this with git am --scissors

- Robert



>
> Fixed up while applying.
> -Daniel
>
> >
> > Being able to program OACONTROL from a non-privileged batch buffer is
> > not sufficient to be able to configure the OA unit. This was originally
> > allowed to help enable Mesa to expose OA counters via the
> > INTEL_performance_query extension, but the current implementation based
> > on programming OACONTROL via a batch buffer isn't able to report useable
> > data without a more complete OA unit configuration. Mesa handles the
> > possibility that writes to OACONTROL may not be allowed and so only
> > advertises the extension after explicitly testing that a write to
> > OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
> > should be ok for userspace.
> >
> > Removing this simplifies adding a new kernel api for configuring the OA
> > unit without needing to consider the possibility that userspace might
> > trample on OACONTROL state which we'd like to start managing within
> > the kernel instead. In particular running any Mesa based GL application
> > currently results in clearing OACONTROL when initializing which would
> > disable the capturing of metrics.
> >
> > v2:
> >     This bumps the command parser version from 8 to 9, as the change is
> >     visible to userspace.
> >
> > Signed-off-by: Robert Bragg <robert@sixbynine.org>
> > Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> > Reviewed-by: Sourab Gupta <sourab.gupta@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_cmd_parser.c | 42
> ++++------------------------------
> >  1 file changed, 5 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
> b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > index c9d2ecd..f5762cd 100644
> > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > @@ -450,7 +450,6 @@ static const struct drm_i915_reg_descriptor
> gen7_render_regs[] = {
> >       REG64(PS_INVOCATION_COUNT),
> >       REG64(PS_DEPTH_COUNT),
> >       REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
> > -     REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below.
> */
> >       REG64(MI_PREDICATE_SRC0),
> >       REG64(MI_PREDICATE_SRC1),
> >       REG32(GEN7_3DPRIM_END_OFFSET),
> > @@ -1060,8 +1059,7 @@ bool intel_engine_needs_cmd_parser(struct
> intel_engine_cs *engine)
> >  static bool check_cmd(const struct intel_engine_cs *engine,
> >                     const struct drm_i915_cmd_descriptor *desc,
> >                     const u32 *cmd, u32 length,
> > -                   const bool is_master,
> > -                   bool *oacontrol_set)
> > +                   const bool is_master)
> >  {
> >       if (desc->flags & CMD_DESC_SKIP)
> >               return true;
> > @@ -1099,31 +1097,6 @@ static bool check_cmd(const struct
> intel_engine_cs *engine,
> >                       }
> >
> >                       /*
> > -                      * 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 == i915_mmio_reg_offset(GEN7_OACONTROL))
> {
> > -                             if (desc->cmd.value ==
> MI_LOAD_REGISTER_MEM) {
> > -                                     DRM_DEBUG_DRIVER("CMD: Rejected
> LRM to OACONTROL\n");
> > -                                     return false;
> > -                             }
> > -
> > -                             if (desc->cmd.value ==
> MI_LOAD_REGISTER_REG) {
> > -                                     DRM_DEBUG_DRIVER("CMD: Rejected
> LRR to OACONTROL\n");
> > -                                     return false;
> > -                             }
> > -
> > -                             if (desc->cmd.value ==
> MI_LOAD_REGISTER_IMM(1))
> > -                                     *oacontrol_set = (cmd[offset + 1]
> != 0);
> > -                     }
> > -
> > -                     /*
> >                        * Check the value written to the register against
> the
> >                        * allowed mask/value pair given in the whitelist
> entry.
> >                        */
> > @@ -1214,7 +1187,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs
> *engine,
> >       u32 *cmd, *batch_end;
> >       struct drm_i915_cmd_descriptor default_desc = noop_desc;
> >       const struct drm_i915_cmd_descriptor *desc = &default_desc;
> > -     bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd()
> */
> >       bool needs_clflush_after = false;
> >       int ret = 0;
> >
> > @@ -1270,8 +1242,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs
> *engine,
> >                       break;
> >               }
> >
> > -             if (!check_cmd(engine, desc, cmd, length, is_master,
> > -                            &oacontrol_set)) {
> > +             if (!check_cmd(engine, desc, cmd, length, is_master)) {
> >                       ret = -EACCES;
> >                       break;
> >               }
> > @@ -1279,11 +1250,6 @@ int intel_engine_cmd_parser(struct
> intel_engine_cs *engine,
> >               cmd += length;
> >       }
> >
> > -     if (oacontrol_set) {
> > -             DRM_DEBUG_DRIVER("CMD: batch set OACONTROL but did not
> clear it\n");
> > -             ret = -EINVAL;
> > -     }
> > -
> >       if (cmd >= batch_end) {
> >               DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a
> BBE cmd!\n");
> >               ret = -EINVAL;
> > @@ -1336,6 +1302,8 @@ int i915_cmd_parser_get_version(struct
> drm_i915_private *dev_priv)
> >        * 8. Don't report cmd_check() failures as EINVAL errors to
> userspace;
> >        *    rely on the HW to NOOP disallowed commands as it would
> without
> >        *    the parser enabled.
> > +      * 9. Don't whitelist or handle oacontrol specially, as ownership
> > +      *    for oacontrol state is moving to i915-perf.
> >        */
> > -     return 8;
> > +     return 9;
> >  }
> > --
> > 2.10.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
Daniel Vetter Nov. 22, 2016, 3:36 p.m. UTC | #3
On Tue, Nov 22, 2016 at 02:09:08PM +0000, Robert Bragg wrote:
> On Tue, Nov 22, 2016 at 1:34 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Tue, Nov 08, 2016 at 12:51:48PM +0000, Robert Bragg wrote:
> > > This v2 patch bumps the command parser version so it can be referenced in
> > > corresponding i-g-t gem_exec_parse changes.
> > >
> > > --- >8 ---
> >
> > Scissors cut everything below, not everything above, hence next time
> > around pls switch around your comment and the commit message, as-is not
> > much left ;-)
> >
> 
> Hmm, they cut away what's above and keep what's below in my experience -
> what command are you seeing the opposite with?
> 
> I just double checked this with git am --scissors

Plain git am works the other way round, that's why I never noticed there's
a special option. TIL ;-)
-Daniel

> 
> - Robert
> 
> 
> 
> >
> > Fixed up while applying.
> > -Daniel
> >
> > >
> > > Being able to program OACONTROL from a non-privileged batch buffer is
> > > not sufficient to be able to configure the OA unit. This was originally
> > > allowed to help enable Mesa to expose OA counters via the
> > > INTEL_performance_query extension, but the current implementation based
> > > on programming OACONTROL via a batch buffer isn't able to report useable
> > > data without a more complete OA unit configuration. Mesa handles the
> > > possibility that writes to OACONTROL may not be allowed and so only
> > > advertises the extension after explicitly testing that a write to
> > > OACONTROL succeeds. Based on this; removing OACONTROL from the whitelist
> > > should be ok for userspace.
> > >
> > > Removing this simplifies adding a new kernel api for configuring the OA
> > > unit without needing to consider the possibility that userspace might
> > > trample on OACONTROL state which we'd like to start managing within
> > > the kernel instead. In particular running any Mesa based GL application
> > > currently results in clearing OACONTROL when initializing which would
> > > disable the capturing of metrics.
> > >
> > > v2:
> > >     This bumps the command parser version from 8 to 9, as the change is
> > >     visible to userspace.
> > >
> > > Signed-off-by: Robert Bragg <robert@sixbynine.org>
> > > Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> > > Reviewed-by: Sourab Gupta <sourab.gupta@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_cmd_parser.c | 42
> > ++++------------------------------
> > >  1 file changed, 5 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > > index c9d2ecd..f5762cd 100644
> > > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > > @@ -450,7 +450,6 @@ static const struct drm_i915_reg_descriptor
> > gen7_render_regs[] = {
> > >       REG64(PS_INVOCATION_COUNT),
> > >       REG64(PS_DEPTH_COUNT),
> > >       REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
> > > -     REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below.
> > */
> > >       REG64(MI_PREDICATE_SRC0),
> > >       REG64(MI_PREDICATE_SRC1),
> > >       REG32(GEN7_3DPRIM_END_OFFSET),
> > > @@ -1060,8 +1059,7 @@ bool intel_engine_needs_cmd_parser(struct
> > intel_engine_cs *engine)
> > >  static bool check_cmd(const struct intel_engine_cs *engine,
> > >                     const struct drm_i915_cmd_descriptor *desc,
> > >                     const u32 *cmd, u32 length,
> > > -                   const bool is_master,
> > > -                   bool *oacontrol_set)
> > > +                   const bool is_master)
> > >  {
> > >       if (desc->flags & CMD_DESC_SKIP)
> > >               return true;
> > > @@ -1099,31 +1097,6 @@ static bool check_cmd(const struct
> > intel_engine_cs *engine,
> > >                       }
> > >
> > >                       /*
> > > -                      * 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 == i915_mmio_reg_offset(GEN7_OACONTROL))
> > {
> > > -                             if (desc->cmd.value ==
> > MI_LOAD_REGISTER_MEM) {
> > > -                                     DRM_DEBUG_DRIVER("CMD: Rejected
> > LRM to OACONTROL\n");
> > > -                                     return false;
> > > -                             }
> > > -
> > > -                             if (desc->cmd.value ==
> > MI_LOAD_REGISTER_REG) {
> > > -                                     DRM_DEBUG_DRIVER("CMD: Rejected
> > LRR to OACONTROL\n");
> > > -                                     return false;
> > > -                             }
> > > -
> > > -                             if (desc->cmd.value ==
> > MI_LOAD_REGISTER_IMM(1))
> > > -                                     *oacontrol_set = (cmd[offset + 1]
> > != 0);
> > > -                     }
> > > -
> > > -                     /*
> > >                        * Check the value written to the register against
> > the
> > >                        * allowed mask/value pair given in the whitelist
> > entry.
> > >                        */
> > > @@ -1214,7 +1187,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs
> > *engine,
> > >       u32 *cmd, *batch_end;
> > >       struct drm_i915_cmd_descriptor default_desc = noop_desc;
> > >       const struct drm_i915_cmd_descriptor *desc = &default_desc;
> > > -     bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd()
> > */
> > >       bool needs_clflush_after = false;
> > >       int ret = 0;
> > >
> > > @@ -1270,8 +1242,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs
> > *engine,
> > >                       break;
> > >               }
> > >
> > > -             if (!check_cmd(engine, desc, cmd, length, is_master,
> > > -                            &oacontrol_set)) {
> > > +             if (!check_cmd(engine, desc, cmd, length, is_master)) {
> > >                       ret = -EACCES;
> > >                       break;
> > >               }
> > > @@ -1279,11 +1250,6 @@ int intel_engine_cmd_parser(struct
> > intel_engine_cs *engine,
> > >               cmd += length;
> > >       }
> > >
> > > -     if (oacontrol_set) {
> > > -             DRM_DEBUG_DRIVER("CMD: batch set OACONTROL but did not
> > clear it\n");
> > > -             ret = -EINVAL;
> > > -     }
> > > -
> > >       if (cmd >= batch_end) {
> > >               DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a
> > BBE cmd!\n");
> > >               ret = -EINVAL;
> > > @@ -1336,6 +1302,8 @@ int i915_cmd_parser_get_version(struct
> > drm_i915_private *dev_priv)
> > >        * 8. Don't report cmd_check() failures as EINVAL errors to
> > userspace;
> > >        *    rely on the HW to NOOP disallowed commands as it would
> > without
> > >        *    the parser enabled.
> > > +      * 9. Don't whitelist or handle oacontrol specially, as ownership
> > > +      *    for oacontrol state is moving to i915-perf.
> > >        */
> > > -     return 8;
> > > +     return 9;
> > >  }
> > > --
> > > 2.10.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> >
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index c9d2ecd..f5762cd 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -450,7 +450,6 @@  static const struct drm_i915_reg_descriptor gen7_render_regs[] = {
 	REG64(PS_INVOCATION_COUNT),
 	REG64(PS_DEPTH_COUNT),
 	REG64_IDX(RING_TIMESTAMP, RENDER_RING_BASE),
-	REG32(GEN7_OACONTROL), /* Only allowed for LRI and SRM. See below. */
 	REG64(MI_PREDICATE_SRC0),
 	REG64(MI_PREDICATE_SRC1),
 	REG32(GEN7_3DPRIM_END_OFFSET),
@@ -1060,8 +1059,7 @@  bool intel_engine_needs_cmd_parser(struct intel_engine_cs *engine)
 static bool check_cmd(const struct intel_engine_cs *engine,
 		      const struct drm_i915_cmd_descriptor *desc,
 		      const u32 *cmd, u32 length,
-		      const bool is_master,
-		      bool *oacontrol_set)
+		      const bool is_master)
 {
 	if (desc->flags & CMD_DESC_SKIP)
 		return true;
@@ -1099,31 +1097,6 @@  static bool check_cmd(const struct intel_engine_cs *engine,
 			}
 
 			/*
-			 * 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 == i915_mmio_reg_offset(GEN7_OACONTROL)) {
-				if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
-					DRM_DEBUG_DRIVER("CMD: Rejected LRM to OACONTROL\n");
-					return false;
-				}
-
-				if (desc->cmd.value == MI_LOAD_REGISTER_REG) {
-					DRM_DEBUG_DRIVER("CMD: Rejected LRR to OACONTROL\n");
-					return false;
-				}
-
-				if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1))
-					*oacontrol_set = (cmd[offset + 1] != 0);
-			}
-
-			/*
 			 * Check the value written to the register against the
 			 * allowed mask/value pair given in the whitelist entry.
 			 */
@@ -1214,7 +1187,6 @@  int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 	u32 *cmd, *batch_end;
 	struct drm_i915_cmd_descriptor default_desc = noop_desc;
 	const struct drm_i915_cmd_descriptor *desc = &default_desc;
-	bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */
 	bool needs_clflush_after = false;
 	int ret = 0;
 
@@ -1270,8 +1242,7 @@  int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 			break;
 		}
 
-		if (!check_cmd(engine, desc, cmd, length, is_master,
-			       &oacontrol_set)) {
+		if (!check_cmd(engine, desc, cmd, length, is_master)) {
 			ret = -EACCES;
 			break;
 		}
@@ -1279,11 +1250,6 @@  int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		cmd += length;
 	}
 
-	if (oacontrol_set) {
-		DRM_DEBUG_DRIVER("CMD: batch set OACONTROL but did not clear it\n");
-		ret = -EINVAL;
-	}
-
 	if (cmd >= batch_end) {
 		DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a BBE cmd!\n");
 		ret = -EINVAL;
@@ -1336,6 +1302,8 @@  int i915_cmd_parser_get_version(struct drm_i915_private *dev_priv)
 	 * 8. Don't report cmd_check() failures as EINVAL errors to userspace;
 	 *    rely on the HW to NOOP disallowed commands as it would without
 	 *    the parser enabled.
+	 * 9. Don't whitelist or handle oacontrol specially, as ownership
+	 *    for oacontrol state is moving to i915-perf.
 	 */
-	return 8;
+	return 9;
 }