Re: [PATCH 6/7] drm/i915: guest i915 notification for Intel-GVTg

From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Zhiyuan Lv <zhiyuan.lv@intel.com>, intel-gfx@lists.freedesktop.org
Cc: igvt-g@lists.01.org
Subject: Re: [PATCH 6/7] drm/i915: guest i915 notification for Intel-GVTg
Date: Thu, 20 Aug 2015 16:11:57 +0300	[thread overview]
Message-ID: <1440076317.5168.19.camel@linux.intel.com> (raw)
In-Reply-To: <1440056724-26976-7-git-send-email-zhiyuan.lv@intel.com>

Hi,

Notes below.

On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> When i915 drivers run inside a VM with Intel-GVTg, some explicit
> notifications are needed from guest to host device model through PV
> INFO page write. The notifications include:
> 
> 	PPGTT create/destroy
> 	EXECLIST create/destroy
> 
> They are used for the shadow implementation of PPGTT and EXECLIST
> context. Intel GVT-g needs to write-protect the guest pages of PPGTT
> and contexts, and clear the write protection when they end their life
> cycle.
> 
> Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 41 
> +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c    | 25 ++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 823005c..00dafb0 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -896,6 +896,41 @@ static int gen8_init_scratch(struct 
> i915_address_space *vm)
>  	return 0;
>  }
>  
> +static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool 
> create)
> +{
> +	enum vgt_g2v_type msg;
> +	struct drm_device *dev = ppgtt->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	unsigned int offset = vgtif_reg(pdp0_lo);
> +	int i;
> +
> +	if (USES_FULL_48BIT_PPGTT(dev)) {

With regards the patch "preallocate pdps for 32 bit vgpu", is this code
path ever taken?

> +		u64 daddr = px_dma(&ppgtt->pml4);
> +
> +		I915_WRITE(offset, daddr & 0xffffffff);
> +		I915_WRITE(offset + 4, daddr >> 32);
> +
> +		msg = (create ? VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE :
> +				VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY)
> ;
> +	} else {
> +		for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
> +			u64 daddr = i915_page_dir_dma_addr(ppgtt, 
> i);
> +
> +			I915_WRITE(offset, daddr & 0xffffffff);
> +			I915_WRITE(offset + 4, daddr >> 32);
> +
> +			offset += 8;
> +		}
> +
> +		msg = (create ? VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE :
> +				VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY)
> ;
> +	}
> +
> +	I915_WRITE(vgtif_reg(g2v_notify), msg);
> +
> +	return 0;
> +}
> +
>  static void gen8_free_scratch(struct i915_address_space *vm)
>  {
>  	struct drm_device *dev = vm->dev;
> @@ -942,6 +977,9 @@ static void gen8_ppgtt_cleanup(struct 
> i915_address_space *vm)
>  	struct i915_hw_ppgtt *ppgtt =
>  		container_of(vm, struct i915_hw_ppgtt, base);
>  
> +	if (intel_vgpu_active(vm->dev))
> +		gen8_ppgtt_notify_vgt(ppgtt, false);
> +
>  	if (!USES_FULL_48BIT_PPGTT(ppgtt->base.dev))
>  		gen8_ppgtt_cleanup_3lvl(ppgtt->base.dev, &ppgtt
> ->pdp);
>  	else
> @@ -1516,6 +1554,9 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt 
> *ppgtt)
>  		}
>  	}
>  
> +	if (intel_vgpu_active(ppgtt->base.dev))
> +		gen8_ppgtt_notify_vgt(ppgtt, true);
> +
>  	return 0;
>  
>  free_scratch:
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 4b2ac37..80d424b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -136,6 +136,7 @@
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
>  #include "intel_mocs.h"
> +#include "i915_vgpu.h"
>  
>  #define GEN9_LR_CONTEXT_RENDER_SIZE (22 * PAGE_SIZE)
>  #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
> @@ -2122,6 +2123,22 @@ make_rpcs(struct drm_device *dev)
>  	return rpcs;
>  }
>  
> +static void intel_lr_context_notify_vgt(struct intel_context *ctx,
> +					struct intel_engine_cs 
> *ring,
> +					int msg)
> +{
> +	struct drm_device *dev = ring->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u64 tmp = intel_lr_context_descriptor(ctx, ring);
> +
> +	I915_WRITE(vgtif_reg(execlist_context_descriptor_lo),
> +			tmp & 0xffffffff);
> +	I915_WRITE(vgtif_reg(execlist_context_descriptor_hi),
> +			tmp >> 32);
> +
> +	I915_WRITE(vgtif_reg(g2v_notify), msg);
> +}
> +

Why the other interface has bool for action and the other msg?

Regards, Joonas

>  static int
>  populate_lr_context(struct intel_context *ctx, struct 
> drm_i915_gem_object *ctx_obj,
>  		    struct intel_engine_cs *ring, struct 
> intel_ringbuffer *ringbuf)
> @@ -2282,6 +2299,10 @@ void intel_lr_context_free(struct 
> intel_context *ctx)
>  					ctx->engine[i].ringbuf;
>  			struct intel_engine_cs *ring = ringbuf
> ->ring;
>  
> +			if (intel_vgpu_active(ringbuf->ring->dev))
> +				intel_lr_context_notify_vgt(ctx, 
> ring,
> +					VGT_G2V_EXECLIST_CONTEXT_DES
> TROY);
> +
>  			if ((ctx == ring->default_context) ||
>  			    (intel_vgpu_active(ring->dev))) {
>  				intel_unpin_ringbuffer_obj(ringbuf);
> @@ -2439,6 +2460,10 @@ int intel_lr_context_deferred_create(struct 
> intel_context *ctx,
>  	ctx->engine[ring->id].ringbuf = ringbuf;
>  	ctx->engine[ring->id].state = ctx_obj;
>  
> +	if (intel_vgpu_active(dev))
> +		intel_lr_context_notify_vgt(ctx, ring,
> +				VGT_G2V_EXECLIST_CONTEXT_CREATE);
> +
>  	if (ctx == ring->default_context)
>  		lrc_setup_hardware_status_page(ring, ctx_obj);
>  	else if (ring->id == RCS && !ctx->rcs_initialized) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx