[PATCH v3 01/11] drm/i915/gvt: parse init context to update cmd accessible reg whitelist

Zhenyu Wang zhenyuw at linux.intel.com
Tue Dec 22 05:23:22 UTC 2020


On 2020.12.09 12:40:29 +0800, Yan Zhao wrote:
> Logical Context is actually a big batch buffer consisting of multiple
> LRI commands + saved registers. It comprises Ring Context (the first
> 0x50 dwords) and Engine Context. The registers defined in Engine Context
> are command accessible, and safe to execute in VM Context.
> However, not all of them are currently tracked in existing register
> whitelist. Here we kick hardware to generate a dummy Engine Context and
> then scan the dummy Engine context to update whitelist dynamically. Based
> on updated list, later patches will audit future VM Engine Contexts to
> disallow undesired LRIs (if out of what hardware generates).
> 
> Cc: Kevin Tian <kevin.tian at intel.com>
> Signed-off-by: Wang Zhi <zhi.a.wang at intel.com>
> Signed-off-by: Yan Zhao <yan.y.zhao at intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/cmd_parser.c | 143 +++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/gvt/cmd_parser.h |   2 +
>  drivers/gpu/drm/i915/gvt/gvt.h        |   4 +
>  drivers/gpu/drm/i915/gvt/vgpu.c       |   4 +-
>  4 files changed, 149 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index 16b582cb97ed..9d2fdaca92bd 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -38,10 +38,15 @@
>  
>  #include "i915_drv.h"
>  #include "gt/intel_ring.h"
> +#include "gt/intel_gt_requests.h"
>  #include "gvt.h"
>  #include "i915_pvinfo.h"
>  #include "trace.h"
>  
> +#include "gem/i915_gem_context.h"
> +#include "gem/i915_gem_pm.h"
> +#include "gt/intel_context.h"
> +
>  #define INVALID_OP    (~0U)
>  
>  #define OP_LEN_MI           9
> @@ -454,6 +459,7 @@ enum {
>  	RING_BUFFER_INSTRUCTION,
>  	BATCH_BUFFER_INSTRUCTION,
>  	BATCH_BUFFER_2ND_LEVEL,
> +	RING_BUFFER_CTX,
>  };
>  
>  enum {
> @@ -495,6 +501,7 @@ struct parser_exec_state {
>  	 */
>  	int saved_buf_addr_type;
>  	bool is_ctx_wa;
> +	bool is_init_ctx;
>  
>  	const struct cmd_info *info;
>  
> @@ -708,6 +715,11 @@ static inline u32 cmd_val(struct parser_exec_state *s, int index)
>  	return *cmd_ptr(s, index);
>  }
>  
> +static inline bool is_init_ctx(struct parser_exec_state *s)
> +{
> +	return (s->buf_type == RING_BUFFER_CTX && s->is_init_ctx);
> +}
> +
>  static void parser_exec_state_dump(struct parser_exec_state *s)
>  {
>  	int cnt = 0;
> @@ -721,7 +733,8 @@ static void parser_exec_state_dump(struct parser_exec_state *s)
>  
>  	gvt_dbg_cmd("  %s %s ip_gma(%08lx) ",
>  			s->buf_type == RING_BUFFER_INSTRUCTION ?
> -			"RING_BUFFER" : "BATCH_BUFFER",
> +			"RING_BUFFER" : ((s->buf_type == RING_BUFFER_CTX) ?
> +				"CTX_BUFFER" : "BATCH_BUFFER"),
>  			s->buf_addr_type == GTT_BUFFER ?
>  			"GTT" : "PPGTT", s->ip_gma);
>  
> @@ -756,7 +769,8 @@ static inline void update_ip_va(struct parser_exec_state *s)
>  	if (WARN_ON(s->ring_head == s->ring_tail))
>  		return;
>  
> -	if (s->buf_type == RING_BUFFER_INSTRUCTION) {
> +	if (s->buf_type == RING_BUFFER_INSTRUCTION ||
> +			s->buf_type == RING_BUFFER_CTX) {
>  		unsigned long ring_top = s->ring_start + s->ring_size;
>  
>  		if (s->ring_head > s->ring_tail) {
> @@ -936,6 +950,11 @@ static int cmd_reg_handler(struct parser_exec_state *s,
>  		return -EFAULT;
>  	}
>  
> +	if (is_init_ctx(s)) {
> +		intel_gvt_mmio_set_cmd_accessible(gvt, offset);
> +		return 0;
> +	}
> +
>  	if (!intel_gvt_mmio_is_cmd_accessible(gvt, offset)) {
>  		gvt_vgpu_err("%s access to non-render register (%x)\n",
>  				cmd, offset);
> @@ -1215,6 +1234,8 @@ static int cmd_handler_mi_batch_buffer_end(struct parser_exec_state *s)
>  		s->buf_type = BATCH_BUFFER_INSTRUCTION;
>  		ret = ip_gma_set(s, s->ret_ip_gma_bb);
>  		s->buf_addr_type = s->saved_buf_addr_type;
> +	} else if (s->buf_type == RING_BUFFER_CTX) {
> +		ret = ip_gma_set(s, s->ring_tail);
>  	} else {
>  		s->buf_type = RING_BUFFER_INSTRUCTION;
>  		s->buf_addr_type = GTT_BUFFER;
> @@ -2763,7 +2784,8 @@ static int command_scan(struct parser_exec_state *s,
>  	gma_bottom = rb_start +  rb_len;
>  
>  	while (s->ip_gma != gma_tail) {
> -		if (s->buf_type == RING_BUFFER_INSTRUCTION) {
> +		if (s->buf_type == RING_BUFFER_INSTRUCTION ||
> +				s->buf_type == RING_BUFFER_CTX) {
>  			if (!(s->ip_gma >= rb_start) ||
>  				!(s->ip_gma < gma_bottom)) {
>  				gvt_vgpu_err("ip_gma %lx out of ring scope."
> @@ -3056,6 +3078,121 @@ int intel_gvt_scan_and_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx)
>  	return 0;
>  }
>  
> +/* generate dummy contexts by sending empty requests to HW, and let
> + * the HW to fill Engine Contexts. This dummy contexts are used for
> + * initialization purpose (update reg whitelist), so referred to as
> + * init context here
> + */
> +void intel_gvt_update_reg_whitelist(struct intel_vgpu *vgpu)
> +{
> +	struct intel_gvt *gvt = vgpu->gvt;
> +	struct drm_i915_private *dev_priv = gvt->gt->i915;
> +	struct intel_engine_cs *engine;
> +	enum intel_engine_id id;
> +	const unsigned long start = LRC_STATE_PN * PAGE_SIZE;
> +	struct i915_request *rq;
> +	struct intel_vgpu_submission *s = &vgpu->submission;
> +	struct i915_request *requests[I915_NUM_ENGINES] = {};
> +	bool is_ctx_pinned[I915_NUM_ENGINES] = {};
> +	int ret;
> +
> +	if (gvt->is_reg_whitelist_updated)
> +		return;
> +
> +	for_each_engine(engine, &dev_priv->gt, id) {
> +		ret = intel_context_pin(s->shadow[id]);
> +		if (ret) {
> +			gvt_vgpu_err("fail to pin shadow ctx\n");
> +			goto out;
> +		}
> +		is_ctx_pinned[id] = true;
> +
> +		rq = i915_request_create(s->shadow[id]);
> +		if (IS_ERR(rq)) {
> +			gvt_vgpu_err("fail to alloc default request\n");
> +			ret = -EIO;
> +			goto out;
> +		}
> +		requests[id] = i915_request_get(rq);
> +		i915_request_add(rq);
> +	}
> +
> +	if (intel_gt_wait_for_idle(&dev_priv->gt,
> +				I915_GEM_IDLE_TIMEOUT) == -ETIME) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	/* scan init ctx to update cmd accessible list */
> +	for_each_engine(engine, &dev_priv->gt, id) {
> +		int size = engine->context_size - PAGE_SIZE;
> +		void *vaddr;
> +		struct parser_exec_state s;
> +		struct drm_i915_gem_object *obj;
> +		struct i915_request *rq;
> +
> +		rq = requests[id];
> +		GEM_BUG_ON(!i915_request_completed(rq));
> +		GEM_BUG_ON(!intel_context_is_pinned(rq->context));
> +		obj = rq->context->state->obj;
> +
> +		if (!obj) {
> +			ret = -EIO;
> +			goto out;
> +		}
> +
> +		i915_gem_object_set_cache_coherency(obj,
> +						    I915_CACHE_LLC);
> +
> +		vaddr = i915_gem_object_pin_map(obj, I915_MAP_WB);
> +		if (IS_ERR(vaddr)) {
> +			gvt_err("failed to pin init ctx obj, ring=%d, err=%lx\n",
> +				id, PTR_ERR(vaddr));
> +			goto out;
> +		}
> +
> +		s.buf_type = RING_BUFFER_CTX;
> +		s.buf_addr_type = GTT_BUFFER;
> +		s.vgpu = vgpu;
> +		s.engine = engine;
> +		s.ring_start = 0;
> +		s.ring_size = size;
> +		s.ring_head = 0;
> +		s.ring_tail = size;
> +		s.rb_va = vaddr + start;
> +		s.workload = NULL;
> +		s.is_ctx_wa = false;
> +		s.is_init_ctx = true;
> +
> +		/* skipping the first RING_CTX_SIZE(0x50) dwords */
> +		ret = ip_gma_set(&s, RING_CTX_SIZE);
> +		if (ret) {
> +			i915_gem_object_unpin_map(obj);
> +			goto out;
> +		}
> +
> +		ret = command_scan(&s, 0, size, 0, size);
> +		if (ret)
> +			gvt_err("Scan init ctx error\n");
> +
> +		i915_gem_object_unpin_map(obj);
> +	}
> +
> +out:
> +	if (!ret)
> +		gvt->is_reg_whitelist_updated = true;
> +
> +	for (id = 0; id < I915_NUM_ENGINES ; id++) {
> +		if (!requests[id])
> +			continue;
> +		i915_request_put(requests[id]);
> +
> +		if (!is_ctx_pinned[id])
> +			continue;
> +		intel_context_unpin(s->shadow[id]);

In error path, these two steps looks inconsistent which could skip ctx unpin...

> +	}
> +}
> +
>  static int init_cmd_table(struct intel_gvt *gvt)
>  {
>  	unsigned int gen_type = intel_gvt_get_device_type(gvt);
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.h b/drivers/gpu/drm/i915/gvt/cmd_parser.h
> index ab25d151932a..09ca2b8a63c8 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.h
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.h
> @@ -50,4 +50,6 @@ int intel_gvt_scan_and_shadow_ringbuffer(struct intel_vgpu_workload *workload);
>  
>  int intel_gvt_scan_and_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx);
>  
> +void intel_gvt_update_reg_whitelist(struct intel_vgpu *vgpu);
> +
>  #endif
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index cf3578e3f4dd..fad412d19f9c 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -329,6 +329,7 @@ struct intel_gvt {
>  		u32 *mocs_mmio_offset_list;
>  		u32 mocs_mmio_offset_list_cnt;
>  	} engine_mmio_list;
> +	bool is_reg_whitelist_updated;
>  
>  	struct dentry *debugfs_root;
>  };
> @@ -412,6 +413,9 @@ int intel_gvt_load_firmware(struct intel_gvt *gvt);
>  #define vgpu_fence_base(vgpu) (vgpu->fence.base)
>  #define vgpu_fence_sz(vgpu) (vgpu->fence.size)
>  
> +/* ring context size i.e. the first 0x50 dwords*/
> +#define RING_CTX_SIZE 320
> +
>  struct intel_vgpu_creation_params {
>  	__u64 handle;
>  	__u64 low_gm_sz;  /* in MB */
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index e49944fde333..5a7226339cf4 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -500,9 +500,11 @@ struct intel_vgpu *intel_gvt_create_vgpu(struct intel_gvt *gvt,
>  
>  	mutex_lock(&gvt->lock);
>  	vgpu = __intel_gvt_create_vgpu(gvt, &param);
> -	if (!IS_ERR(vgpu))
> +	if (!IS_ERR(vgpu)) {
>  		/* calculate left instance change for types */
>  		intel_gvt_update_vgpu_types(gvt);
> +		intel_gvt_update_reg_whitelist(vgpu);
> +	}
>  	mutex_unlock(&gvt->lock);
>  
>  	return vgpu;
> -- 
> 2.17.1
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20201222/9211ca6e/attachment.sig>


More information about the intel-gvt-dev mailing list