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

[Lionel Landwerlin]

On 18/07/17 12:34, Chris Wilson wrote:
> 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 at intel.com>
>>> Cc: Niu, Bing <bing.niu at intel.com>
>>> Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>>> Cc: intel-gvt-dev at lists.freedesktop.org
>>> Signed-off-by: Zhenyu Wang <zhenyuw at 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
>

Thanks Chris!