[Intel-gfx] [PATCH] drm/i915/gvt: Fix relocation of shadow bb

[Zhenyu Wang]

On 2017.01.06 19:58:16 +0000, Chris Wilson wrote:
> set_gma_to_bb_cmd() is completely bogus - it is (incorrectly) applying
> the rules to read a GTT offset from a command as opposed to writing the
> GTT offset. And to cap it all set_gma_to_bb_cmd() is called within a list
> iterator of the most strange construction.
>

Looks fine to me. I'll pick up this after validating with batch buffer scan on.

Thanks.

> Fixes: be1da7070aea ("drm/i915/gvt: vGPU command scanner")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang at intel.com>
> Cc: Yulei Zhang <yulei.zhang at intel.com>
> Cc: <drm-intel-fixes at lists.freedesktop.org> # v4.10-rc1+
> ---
>  drivers/gpu/drm/i915/gvt/execlist.c  | 64 ++++++++++--------------------------
>  drivers/gpu/drm/i915/gvt/scheduler.h |  2 +-
>  2 files changed, 19 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
> index 9a98553ad742..4b8454b445cd 100644
> --- a/drivers/gpu/drm/i915/gvt/execlist.c
> +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> @@ -364,58 +364,30 @@ static void free_workload(struct intel_vgpu_workload *workload)
>  #define get_desc_from_elsp_dwords(ed, i) \
>  	((struct execlist_ctx_descriptor_format *)&((ed)->data[i * 2]))
>  
> -
> -#define BATCH_BUFFER_ADDR_MASK ((1UL << 32) - (1U << 2))
> -#define BATCH_BUFFER_ADDR_HIGH_MASK ((1UL << 16) - (1U))
> -static int set_gma_to_bb_cmd(struct intel_shadow_bb_entry *entry_obj,
> -			     unsigned long add, int gmadr_bytes)
> -{
> -	if (WARN_ON(gmadr_bytes != 4 && gmadr_bytes != 8))
> -		return -1;
> -
> -	*((u32 *)(entry_obj->bb_start_cmd_va + (1 << 2))) = add &
> -		BATCH_BUFFER_ADDR_MASK;
> -	if (gmadr_bytes == 8) {
> -		*((u32 *)(entry_obj->bb_start_cmd_va + (2 << 2))) =
> -			add & BATCH_BUFFER_ADDR_HIGH_MASK;
> -	}
> -
> -	return 0;
> -}
> -
>  static void prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload)
>  {
> -	int gmadr_bytes = workload->vgpu->gvt->device_info.gmadr_bytes_in_cmd;
> +	const int gmadr_bytes = workload->vgpu->gvt->device_info.gmadr_bytes_in_cmd;
> +	struct intel_shadow_bb_entry *entry_obj;
>  
>  	/* pin the gem object to ggtt */
> -	if (!list_empty(&workload->shadow_bb)) {
> -		struct intel_shadow_bb_entry *entry_obj =
> -			list_first_entry(&workload->shadow_bb,
> -					 struct intel_shadow_bb_entry,
> -					 list);
> -		struct intel_shadow_bb_entry *temp;
> +	list_for_each_entry(entry_obj, &workload->shadow_bb, list) {
> +		struct i915_vma *vma;
>  
> -		list_for_each_entry_safe(entry_obj, temp, &workload->shadow_bb,
> -				list) {
> -			struct i915_vma *vma;
> -
> -			vma = i915_gem_object_ggtt_pin(entry_obj->obj, NULL, 0,
> -						       4, 0);
> -			if (IS_ERR(vma)) {
> -				gvt_err("Cannot pin\n");
> -				return;
> -			}
> -
> -			/* FIXME: we are not tracking our pinned VMA leaving it
> -			 * up to the core to fix up the stray pin_count upon
> -			 * free.
> -			 */
> -
> -			/* update the relocate gma with shadow batch buffer*/
> -			set_gma_to_bb_cmd(entry_obj,
> -					  i915_ggtt_offset(vma),
> -					  gmadr_bytes);
> +		vma = i915_gem_object_ggtt_pin(entry_obj->obj, NULL, 0, 4, 0);
> +		if (IS_ERR(vma)) {
> +			gvt_err("Cannot pin\n");
> +			return;
>  		}
> +
> +		/* FIXME: we are not tracking our pinned VMA leaving it
> +		 * up to the core to fix up the stray pin_count upon
> +		 * free.
> +		 */
> +
> +		/* update the relocate gma with shadow batch buffer*/
> +		entry_obj->bb_start_cmd_va[1] = i915_ggtt_offset(vma);
> +		if (gmadr_bytes == 8)
> +			entry_obj->bb_start_cmd_va[2] = 0;
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.h b/drivers/gpu/drm/i915/gvt/scheduler.h
> index 3b30c28bff51..2833dfa8c9ae 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.h
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.h
> @@ -113,7 +113,7 @@ struct intel_shadow_bb_entry {
>  	struct drm_i915_gem_object *obj;
>  	void *va;
>  	unsigned long len;
> -	void *bb_start_cmd_va;
> +	u32 *bb_start_cmd_va;
>  };
>  
>  #define workload_q_head(vgpu, ring_id) \
> -- 
> 2.11.0
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20170109/9f98056d/attachment.sig>