Re: [RFC PATCH 1/2] drm/i915: Add perf property support for context HW id

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Lionel Landwerlin (2017-07-18 12:30:10)
> Looks fine to me, down there a couple of suggestions.
> 
> Cheers,
> 
> -
> Lionel
> 
> On 18/07/17 08:51, Zhenyu Wang wrote:
> > In order to support profiling for special context e.g vGPU context,
> > we can expose vGPU context hw id and enable i915 perf property to
> > get target context for profiling. This adds new perf property to
> > assign target context with hw id.
> >
> > Jiao Pengyuan has helped to fix context reference bug in original code.
> >
> > Cc: Jiao, Pengyuan <pengyuan.jiao@intel.com>
> > Cc: Niu, Bing <bing.niu@intel.com>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Cc: intel-gvt-dev@lists.freedesktop.org
> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_perf.c | 37 +++++++++++++++++++++++++++++++++++++
> >   include/uapi/drm/i915_drm.h      |  5 +++++
> >   2 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> > index d9f77a4d85db..350fd259b2d0 100644
> > --- a/drivers/gpu/drm/i915/i915_perf.c
> > +++ b/drivers/gpu/drm/i915/i915_perf.c
> > @@ -348,7 +348,9 @@ struct perf_open_properties {
> >       u32 sample_flags;
> >   
> >       u64 single_context:1;
> > +     u64 single_context_hw:1;
> >       u64 ctx_handle;
> > +     u64 ctx_hw_id;
> >   
> >       /* OA sampling state */
> >       int metrics_set;
> > @@ -2555,6 +2557,28 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv,
> >               }
> >       }
> >   
> > +     if (props->single_context_hw) {
> > +             struct i915_gem_context *ctx;
> > +
> > +             mutex_lock(&dev_priv->drm.struct_mutex);
> 
> Maybe use i915_mutex_lock_interruptible() ?
> 
> > +             list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
> > +                     if (!i915_gem_context_is_default(ctx))
> > +                             continue;
> > +
> > +                     if (ctx->hw_id == props->ctx_hw_id) {
> > +                             specific_ctx = ctx;
> > +                             i915_gem_context_get(specific_ctx);
> > +                             break;
> > +                     }
> > +             }
> > +             mutex_unlock(&dev_priv->drm.struct_mutex);
> 
> Maybe you could put the logic above into a function?

Please, please make sure this guarded by extreme paranoia. This indeed
has the opposite effect and allows any user to snoop on another.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx