[RFC PATCH 0/2] Enable OA perf for vGPU

[RFC PATCH 0/2] Enable OA perf for vGPU

[Zhenyu Wang]

This is to enable OA based profiling for vGPU context
which try to expose vGPU hw id and make i915 perf utilize
it for target single context based profiling.

Zhenyu Wang (2):
  drm/i915: Add perf property support for context HW id
  drm/i915/gvt: expose vGPU context hw id

 drivers/gpu/drm/i915/gvt/kvmgt.c | 17 +++++++++++++++++
 drivers/gpu/drm/i915/i915_perf.c | 37 +++++++++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h      |  5 +++++
 3 files changed, 59 insertions(+)

-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Zhenyu Wang]

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);
+		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);
+
+		if (!specific_ctx) {
+			DRM_DEBUG("Failed to look up HW context id.\n");
+			goto err;
+		}
+	}
+
 	/*
 	 * On Haswell the OA unit supports clock gating off for a specific
 	 * context and in this mode there's no visibility of metrics for the
@@ -2708,6 +2732,14 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
 			props->single_context = 1;
 			props->ctx_handle = value;
 			break;
+		case DRM_I915_PERF_PROP_CTX_HW_ID:
+			if (value >= MAX_CONTEXT_HW_ID) {
+				DRM_DEBUG("Invalid OA HW context ID\n");
+				return -EINVAL;
+			}
+			props->single_context_hw = 1;
+			props->ctx_hw_id = value;
+			break;
 		case DRM_I915_PERF_PROP_SAMPLE_OA:
 			props->sample_flags |= SAMPLE_OA_REPORT;
 			break;
@@ -2779,6 +2811,11 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
 		uprop += 2;
 	}
 
+	if (props->single_context && props->single_context_hw) {
+		DRM_DEBUG("Assign context handler and HW id simultaneously\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 7ccbd6a2bbe0..ddafa556e290 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1381,6 +1381,11 @@ enum drm_i915_perf_property_id {
 	 */
 	DRM_I915_PERF_PROP_OA_EXPONENT,
 
+	/**
+	 * The value specifies ctx with hw_id
+	 */
+	DRM_I915_PERF_PROP_CTX_HW_ID,
+
 	DRM_I915_PERF_PROP_MAX /* non-ABI */
 };
 
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 2/2] drm/i915/gvt: expose vGPU context hw id

[Zhenyu Wang]

This exposes vGPU context hw id in mdev sysfs which is used to
do vGPU based profiling. Retrieved vGPU context hw id can be set
through i915 perf ioctl to set profiling for target vGPU.

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/gvt/kvmgt.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index fd0c85f9ef3c..83e88c70272a 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -1170,10 +1170,27 @@ vgpu_id_show(struct device *dev, struct device_attribute *attr,
 	return sprintf(buf, "\n");
 }
 
+static ssize_t
+hw_id_show(struct device *dev, struct device_attribute *attr,
+	   char *buf)
+{
+	struct mdev_device *mdev = mdev_from_dev(dev);
+
+	if (mdev) {
+		struct intel_vgpu *vgpu = (struct intel_vgpu *)
+			mdev_get_drvdata(mdev);
+		return sprintf(buf, "%u\n",
+			       vgpu->shadow_ctx->hw_id);
+	}
+	return sprintf(buf, "\n");
+}
+
 static DEVICE_ATTR_RO(vgpu_id);
+static DEVICE_ATTR_RO(hw_id);
 
 static struct attribute *intel_vgpu_attrs[] = {
 	&dev_attr_vgpu_id.attr,
+	&dev_attr_hw_id.attr,
 	NULL
 };
 
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Enable OA perf for vGPU

[Patchwork]

== Series Details ==

Series: Enable OA perf for vGPU
URL   : https://patchwork.freedesktop.org/series/27474/
State : failure

== Summary ==

  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/bounds.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/compile.h
  CHK     kernel/config_data.h
  CC [M]  drivers/gpu/drm/i915/i915_perf.o
drivers/gpu/drm/i915/i915_perf.c: In function ‘i915_perf_open_ioctl’:
drivers/gpu/drm/i915/i915_perf.c:2857:9: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  return ret;
         ^
cc1: all warnings being treated as errors
scripts/Makefile.build:302: recipe for target 'drivers/gpu/drm/i915/i915_perf.o' failed
make[4]: *** [drivers/gpu/drm/i915/i915_perf.o] Error 1
scripts/Makefile.build:561: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:561: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:561: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1016: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Lionel Landwerlin]

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?

> +
> +		if (!specific_ctx) {
> +			DRM_DEBUG("Failed to look up HW context id.\n");
> +			goto err;
> +		}
> +	}
> +
>   	/*
>   	 * On Haswell the OA unit supports clock gating off for a specific
>   	 * context and in this mode there's no visibility of metrics for the
> @@ -2708,6 +2732,14 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
>   			props->single_context = 1;
>   			props->ctx_handle = value;
>   			break;
> +		case DRM_I915_PERF_PROP_CTX_HW_ID:
> +			if (value >= MAX_CONTEXT_HW_ID) {
> +				DRM_DEBUG("Invalid OA HW context ID\n");
> +				return -EINVAL;
> +			}
> +			props->single_context_hw = 1;
> +			props->ctx_hw_id = value;
> +			break;
>   		case DRM_I915_PERF_PROP_SAMPLE_OA:
>   			props->sample_flags |= SAMPLE_OA_REPORT;
>   			break;
> @@ -2779,6 +2811,11 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
>   		uprop += 2;
>   	}
>   
> +	if (props->single_context && props->single_context_hw) {
> +		DRM_DEBUG("Assign context handler and HW id simultaneously\n");
> +		return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 7ccbd6a2bbe0..ddafa556e290 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1381,6 +1381,11 @@ enum drm_i915_perf_property_id {
>   	 */
>   	DRM_I915_PERF_PROP_OA_EXPONENT,
>   
> +	/**
> +	 * The value specifies ctx with hw_id
> +	 */
> +	DRM_I915_PERF_PROP_CTX_HW_ID,
> +
>   	DRM_I915_PERF_PROP_MAX /* non-ABI */
>   };
>   


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Lionel Landwerlin]

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);
> +		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);
> +
> +		if (!specific_ctx) {
> +			DRM_DEBUG("Failed to look up HW context id.\n");

And you're missing setting ret = -ENOENT here :)

> +			goto err;
> +		}
> +	}
> +
>   	/*
>   	 * On Haswell the OA unit supports clock gating off for a specific
>   	 * context and in this mode there's no visibility of metrics for the
> @@ -2708,6 +2732,14 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
>   			props->single_context = 1;
>   			props->ctx_handle = value;
>   			break;
> +		case DRM_I915_PERF_PROP_CTX_HW_ID:
> +			if (value >= MAX_CONTEXT_HW_ID) {
> +				DRM_DEBUG("Invalid OA HW context ID\n");
> +				return -EINVAL;
> +			}
> +			props->single_context_hw = 1;
> +			props->ctx_hw_id = value;
> +			break;
>   		case DRM_I915_PERF_PROP_SAMPLE_OA:
>   			props->sample_flags |= SAMPLE_OA_REPORT;
>   			break;
> @@ -2779,6 +2811,11 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
>   		uprop += 2;
>   	}
>   
> +	if (props->single_context && props->single_context_hw) {
> +		DRM_DEBUG("Assign context handler and HW id simultaneously\n");
> +		return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 7ccbd6a2bbe0..ddafa556e290 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1381,6 +1381,11 @@ enum drm_i915_perf_property_id {
>   	 */
>   	DRM_I915_PERF_PROP_OA_EXPONENT,
>   
> +	/**
> +	 * The value specifies ctx with hw_id
> +	 */
> +	DRM_I915_PERF_PROP_CTX_HW_ID,
> +
>   	DRM_I915_PERF_PROP_MAX /* non-ABI */
>   };
>   


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

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

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

[Lionel Landwerlin]

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);
> +		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);
> +
> +		if (!specific_ctx) {
> +			DRM_DEBUG("Failed to look up HW context id.\n");
> +			goto err;
> +		}
> +	}

Chris pointed something really important.
The condition below :

if (IS_HASWELL(dev_priv) && specific_ctx)

needs to be moved into the if (props->single_context) so we ensure that 
only an operation using a context handle can be considered non 
privileged on Haswell.

On Gen8+, everything requires privileged access though.

> +
>   	/*
>   	 * On Haswell the OA unit supports clock gating off for a specific
>   	 * context and in this mode there's no visibility of metrics for the
> @@ -2708,6 +2732,14 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
>   			props->single_context = 1;
>   			props->ctx_handle = value;
>   			break;
> +		case DRM_I915_PERF_PROP_CTX_HW_ID:
> +			if (value >= MAX_CONTEXT_HW_ID) {
> +				DRM_DEBUG("Invalid OA HW context ID\n");
> +				return -EINVAL;
> +			}
> +			props->single_context_hw = 1;
> +			props->ctx_hw_id = value;
> +			break;
>   		case DRM_I915_PERF_PROP_SAMPLE_OA:
>   			props->sample_flags |= SAMPLE_OA_REPORT;
>   			break;
> @@ -2779,6 +2811,11 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv,
>   		uprop += 2;
>   	}
>   
> +	if (props->single_context && props->single_context_hw) {
> +		DRM_DEBUG("Assign context handler and HW id simultaneously\n");
> +		return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 7ccbd6a2bbe0..ddafa556e290 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -1381,6 +1381,11 @@ enum drm_i915_perf_property_id {
>   	 */
>   	DRM_I915_PERF_PROP_OA_EXPONENT,
>   
> +	/**
> +	 * The value specifies ctx with hw_id
> +	 */
> +	DRM_I915_PERF_PROP_CTX_HW_ID,
> +
>   	DRM_I915_PERF_PROP_MAX /* non-ABI */
>   };
>   


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [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@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
>

Thanks Chris!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx