[PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g

[PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g

[Zhiyuan Lv]

Intel GVT-g will perform EXECLIST context shadowing and ring buffer
shadowing. The shadow copy is created when guest creates a context.
If a context changes its LRCA address, the hypervisor is hard to know
whether it is a new context or not. We always pin context objects to
global GTT to make life easier.

Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 39df304..4b2ac37 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2282,7 +2282,8 @@ void intel_lr_context_free(struct intel_context *ctx)
 					ctx->engine[i].ringbuf;
 			struct intel_engine_cs *ring = ringbuf->ring;
 
-			if (ctx == ring->default_context) {
+			if ((ctx == ring->default_context) ||
+			    (intel_vgpu_active(ring->dev))) {
 				intel_unpin_ringbuffer_obj(ringbuf);
 				i915_gem_object_ggtt_unpin(ctx_obj);
 			}
@@ -2353,6 +2354,8 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 				     struct intel_engine_cs *ring)
 {
 	const bool is_global_default_ctx = (ctx == ring->default_context);
+	const bool need_to_pin_ctx = (is_global_default_ctx ||
+				      (intel_vgpu_active(ring->dev)));
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *ctx_obj;
@@ -2374,7 +2377,7 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 		return -ENOMEM;
 	}
 
-	if (is_global_default_ctx) {
+	if (need_to_pin_ctx) {
 		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
 				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
 		if (ret) {
@@ -2415,7 +2418,7 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 			goto error_free_rbuf;
 		}
 
-		if (is_global_default_ctx) {
+		if (need_to_pin_ctx) {
 			ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
 			if (ret) {
 				DRM_ERROR(
@@ -2464,14 +2467,14 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 	return 0;
 
 error:
-	if (is_global_default_ctx)
+	if (need_to_pin_ctx)
 		intel_unpin_ringbuffer_obj(ringbuf);
 error_destroy_rbuf:
 	intel_destroy_ringbuffer_obj(ringbuf);
 error_free_rbuf:
 	kfree(ringbuf);
 error_unpin_ctx:
-	if (is_global_default_ctx)
+	if (need_to_pin_ctx)
 		i915_gem_object_ggtt_unpin(ctx_obj);
 	drm_gem_object_unreference(&ctx_obj->base);
 	return ret;
-- 
1.9.1

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

Re: [PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g

[Zhiyuan Lv]

Hi Chris,

If we cannot always pin lr context into GGTT, the LRCA cannot be used
as a context identifier for us. Then we have to consider a proper
interface for i915 in VM to notify GVT-g device model.

The context creation might be OK. We can pin context first, then
notify the context creation, after that, unpin the context. In GVT-g,
we can get the context's guest physical addresses from GTT table, and
use that to identify a guest context.

But the destroy can be a problem. It does not make sense to pin
context and unpin that time right?

Do you have any suggestions/comments? Thanks in advance!

Regards,
-Zhiyuan

On Thu, Aug 20, 2015 at 05:16:54PM +0800, Zhiyuan Lv wrote:
> Hi Chris,
> 
> On Thu, Aug 20, 2015 at 09:36:00AM +0100, Chris Wilson wrote:
> > On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote:
> > > Intel GVT-g will perform EXECLIST context shadowing and ring buffer
> > > shadowing. The shadow copy is created when guest creates a context.
> > > If a context changes its LRCA address, the hypervisor is hard to know
> > > whether it is a new context or not. We always pin context objects to
> > > global GTT to make life easier.
> > 
> > Nak. Please explain why we need to workaround a bug in the host. We
> > cannot pin the context as that breaks userspace (e.g. synmark) who can
> > and will try to use more contexts than we have room.
> 
> This change is only for i915 running inside a VM. Host i915 driver's
> behavior will not be impacted.
> 
> The reason we want to pin context is that our hypervisor identifies
> guest contexts with their LRCA, and need LRCA unchanged during
> contexts' life cycle so that shadow copy of contexts can easily find
> their counterparts. If we cannot pin them, we have to consider more
> complicated shadow implementation ...
> 
> BTW Chris, are there many apps like synmark that may used up GGTT with
> contexts if they are all pinned? Thanks!
> 
> Regards,
> -Zhiyuan
> 
> > -Chris
> > 
> > -- 
> > Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g

[Zhiyuan Lv]

Hi Chris,

On Thu, Aug 20, 2015 at 09:36:00AM +0100, Chris Wilson wrote:
> On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote:
> > Intel GVT-g will perform EXECLIST context shadowing and ring buffer
> > shadowing. The shadow copy is created when guest creates a context.
> > If a context changes its LRCA address, the hypervisor is hard to know
> > whether it is a new context or not. We always pin context objects to
> > global GTT to make life easier.
> 
> Nak. Please explain why we need to workaround a bug in the host. We
> cannot pin the context as that breaks userspace (e.g. synmark) who can
> and will try to use more contexts than we have room.

This change is only for i915 running inside a VM. Host i915 driver's
behavior will not be impacted.

The reason we want to pin context is that our hypervisor identifies
guest contexts with their LRCA, and need LRCA unchanged during
contexts' life cycle so that shadow copy of contexts can easily find
their counterparts. If we cannot pin them, we have to consider more
complicated shadow implementation ...

BTW Chris, are there many apps like synmark that may used up GGTT with
contexts if they are all pinned? Thanks!

Regards,
-Zhiyuan

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g

[Chris Wilson]

On Thu, Aug 20, 2015 at 03:45:21PM +0800, Zhiyuan Lv wrote:
> Intel GVT-g will perform EXECLIST context shadowing and ring buffer
> shadowing. The shadow copy is created when guest creates a context.
> If a context changes its LRCA address, the hypervisor is hard to know
> whether it is a new context or not. We always pin context objects to
> global GTT to make life easier.

Nak. Please explain why we need to workaround a bug in the host. We
cannot pin the context as that breaks userspace (e.g. synmark) who can
and will try to use more contexts than we have room.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/7] drm/i915: always pin lrc context for vgpu with Intel GVT-g

[Zhiyuan Lv]

Intel GVT-g will perform EXECLIST context shadowing and ring buffer
shadowing. The shadow copy is created when guest creates a context.
If a context changes its LRCA address, the hypervisor is hard to know
whether it is a new context or not. We always pin context objects to
global GTT to make life easier.

Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 39df304..4b2ac37 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2282,7 +2282,8 @@ void intel_lr_context_free(struct intel_context *ctx)
 					ctx->engine[i].ringbuf;
 			struct intel_engine_cs *ring = ringbuf->ring;
 
-			if (ctx == ring->default_context) {
+			if ((ctx == ring->default_context) ||
+			    (intel_vgpu_active(ring->dev))) {
 				intel_unpin_ringbuffer_obj(ringbuf);
 				i915_gem_object_ggtt_unpin(ctx_obj);
 			}
@@ -2353,6 +2354,8 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 				     struct intel_engine_cs *ring)
 {
 	const bool is_global_default_ctx = (ctx == ring->default_context);
+	const bool need_to_pin_ctx = (is_global_default_ctx ||
+				      (intel_vgpu_active(ring->dev)));
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *ctx_obj;
@@ -2374,7 +2377,7 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 		return -ENOMEM;
 	}
 
-	if (is_global_default_ctx) {
+	if (need_to_pin_ctx) {
 		ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN,
 				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
 		if (ret) {
@@ -2415,7 +2418,7 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 			goto error_free_rbuf;
 		}
 
-		if (is_global_default_ctx) {
+		if (need_to_pin_ctx) {
 			ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf);
 			if (ret) {
 				DRM_ERROR(
@@ -2464,14 +2467,14 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 	return 0;
 
 error:
-	if (is_global_default_ctx)
+	if (need_to_pin_ctx)
 		intel_unpin_ringbuffer_obj(ringbuf);
 error_destroy_rbuf:
 	intel_destroy_ringbuffer_obj(ringbuf);
 error_free_rbuf:
 	kfree(ringbuf);
 error_unpin_ctx:
-	if (is_global_default_ctx)
+	if (need_to_pin_ctx)
 		i915_gem_object_ggtt_unpin(ctx_obj);
 	drm_gem_object_unreference(&ctx_obj->base);
 	return ret;
-- 
1.9.1

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