[PATCH 2/2] drm/i915/gvt: support inconsecutive partial gtt entry write

Zhenyu Wang zhenyuw at linux.intel.com
Mon Oct 8 09:58:11 UTC 2018


On 2018.09.19 06:54:41 +0000, Zhang, Xiaolin wrote:
> It looks fine for me. 
> 
> Reviewed-by: Xiaolin Zhang <xiaolin.zhang at intel.com>
> 

Applied, thanks!

> 
> -----Original Message-----
> From: hang.yuan at linux.intel.com [mailto:hang.yuan at linux.intel.com] 
> Sent: Wednesday, September 19, 2018 2:42 PM
> To: intel-gvt-dev at lists.freedesktop.org
> Cc: Hang Yuan <hang.yuan at linux.intel.com>; Zhao, Yan Y <yan.y.zhao at intel.com>; Zhang, Xiaolin <xiaolin.zhang at intel.com>; Wang, Zhenyu Z <zhenyu.z.wang at intel.com>
> Subject: [PATCH 2/2] drm/i915/gvt: support inconsecutive partial gtt entry write
> 
> From: Hang Yuan <hang.yuan at linux.intel.com>
> 
> Previously we assumed two 4-byte writes to the same PTE coming in sequence.
> But recently we observed inconsecutive partial write happening as well. So this patch enhances the previous solution. It now uses a list to save more partial writes. If one partial write can be combined with another one in the list to construct a full PTE, update its shadow entry. Otherwise, save the partial write in the list.
> 
> v2: invalidate old entry and flush ggtt (Zhenyu)
> v3: split old ggtt page unmap to another patch (Zhenyu)
> v4: refine codes (Zhenyu)
> 
> Signed-off-by: Hang Yuan <hang.yuan at linux.intel.com>
> Cc: Yan Zhao <yan.y.zhao at intel.com>
> Cc: Xiaolin Zhang <xiaolin.zhang at intel.com>
> Cc: Zhenyu Wang <zhenyu.z.wang at intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/gtt.c | 107 ++++++++++++++++++++---------------------
>  drivers/gpu/drm/i915/gvt/gtt.h |   9 +++-
>  2 files changed, 60 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 3a725ac..58e166e 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1905,7 +1905,6 @@ static struct intel_vgpu_mm *intel_vgpu_create_ggtt_mm(struct intel_vgpu *vgpu)
>  		vgpu_free_mm(mm);
>  		return ERR_PTR(-ENOMEM);
>  	}
> -	mm->ggtt_mm.last_partial_off = -1UL;
>  
>  	return mm;
>  }
> @@ -1930,7 +1929,6 @@ void _intel_vgpu_mm_release(struct kref *mm_ref)
>  		invalidate_ppgtt_mm(mm);
>  	} else {
>  		vfree(mm->ggtt_mm.virtual_ggtt);
> -		mm->ggtt_mm.last_partial_off = -1UL;
>  	}
>  
>  	vgpu_free_mm(mm);
> @@ -2168,6 +2166,8 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
>  	struct intel_gvt_gtt_entry e, m;
>  	dma_addr_t dma_addr;
>  	int ret;
> +	struct intel_gvt_partial_pte *partial_pte, *pos, *n;
> +	bool partial_update = false;
>  
>  	if (bytes != 4 && bytes != 8)
>  		return -EINVAL;
> @@ -2178,68 +2178,57 @@ static int emulate_ggtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
>  	if (!vgpu_gmadr_is_valid(vgpu, gma))
>  		return 0;
>  
> -	ggtt_get_guest_entry(ggtt_mm, &e, g_gtt_index);
> -
> +	e.type = GTT_TYPE_GGTT_PTE;
>  	memcpy((void *)&e.val64 + (off & (info->gtt_entry_size - 1)), p_data,
>  			bytes);
>  
>  	/* If ggtt entry size is 8 bytes, and it's split into two 4 bytes
> -	 * write, we assume the two 4 bytes writes are consecutive.
> -	 * Otherwise, we abort and report error
> +	 * write, save the first 4 bytes in a list and update virtual
> +	 * PTE. Only update shadow PTE when the second 4 bytes comes.
>  	 */
>  	if (bytes < info->gtt_entry_size) {
> -		if (ggtt_mm->ggtt_mm.last_partial_off == -1UL) {
> -			/* the first partial part*/
> -			ggtt_mm->ggtt_mm.last_partial_off = off;
> -			ggtt_mm->ggtt_mm.last_partial_data = e.val64;
> -			return 0;
> -		} else if ((g_gtt_index ==
> -				(ggtt_mm->ggtt_mm.last_partial_off >>
> -				info->gtt_entry_size_shift)) &&
> -			(off !=	ggtt_mm->ggtt_mm.last_partial_off)) {
> -			/* the second partial part */
> -
> -			int last_off = ggtt_mm->ggtt_mm.last_partial_off &
> -				(info->gtt_entry_size - 1);
> -
> -			memcpy((void *)&e.val64 + last_off,
> -				(void *)&ggtt_mm->ggtt_mm.last_partial_data +
> -				last_off, bytes);
> -
> -			ggtt_mm->ggtt_mm.last_partial_off = -1UL;
> -		} else {
> -			int last_offset;
> -
> -			gvt_vgpu_err("failed to populate guest ggtt entry: abnormal ggtt entry write sequence, last_partial_off=%lx, offset=%x, bytes=%d, ggtt entry size=%d\n",
> -					ggtt_mm->ggtt_mm.last_partial_off, off,
> -					bytes, info->gtt_entry_size);
> -
> -			/* set host ggtt entry to scratch page and clear
> -			 * virtual ggtt entry as not present for last
> -			 * partially write offset
> -			 */
> -			last_offset = ggtt_mm->ggtt_mm.last_partial_off &
> -					(~(info->gtt_entry_size - 1));
> -
> -			ggtt_get_host_entry(ggtt_mm, &m, last_offset);
> -			ggtt_invalidate_pte(vgpu, &m);
> -			ops->set_pfn(&m, gvt->gtt.scratch_mfn);
> -			ops->clear_present(&m);
> -			ggtt_set_host_entry(ggtt_mm, &m, last_offset);
> -			ggtt_invalidate(gvt->dev_priv);
> -
> -			ggtt_get_guest_entry(ggtt_mm, &e, last_offset);
> -			ops->clear_present(&e);
> -			ggtt_set_guest_entry(ggtt_mm, &e, last_offset);
> -
> -			ggtt_mm->ggtt_mm.last_partial_off = off;
> -			ggtt_mm->ggtt_mm.last_partial_data = e.val64;
> +		bool found = false;
> +
> +		list_for_each_entry_safe(pos, n,
> +				&ggtt_mm->ggtt_mm.partial_pte_list, list) {
> +			if (g_gtt_index == pos->offset >>
> +					info->gtt_entry_size_shift) {
> +				if (off != pos->offset) {
> +					/* the second partial part*/
> +					int last_off = pos->offset &
> +						(info->gtt_entry_size - 1);
> +
> +					memcpy((void *)&e.val64 + last_off,
> +						(void *)&pos->data + last_off,
> +						bytes);
> +
> +					list_del(&pos->list);
> +					kfree(pos);
> +					found = true;
> +					break;
> +				}
> +
> +				/* update of the first partial part */
> +				pos->data = e.val64;
> +				ggtt_set_guest_entry(ggtt_mm, &e, g_gtt_index);
> +				return 0;
> +			}
> +		}
>  
> -			return 0;
> +		if (!found) {
> +			/* the first partial part */
> +			partial_pte = kzalloc(sizeof(*partial_pte), GFP_KERNEL);
> +			if (!partial_pte)
> +				return -ENOMEM;
> +			partial_pte->offset = off;
> +			partial_pte->data = e.val64;
> +			list_add_tail(&partial_pte->list,
> +				&ggtt_mm->ggtt_mm.partial_pte_list);
> +			partial_update = true;
>  		}
>  	}
>  
> -	if (ops->test_present(&e)) {
> +	if (!partial_update && (ops->test_present(&e))) {
>  		gfn = ops->get_pfn(&e);
>  		m = e;
>  
> @@ -2432,6 +2421,8 @@ int intel_vgpu_init_gtt(struct intel_vgpu *vgpu)
>  
>  	intel_vgpu_reset_ggtt(vgpu, false);
>  
> +	INIT_LIST_HEAD(&gtt->ggtt_mm->ggtt_mm.partial_pte_list);
> +
>  	return create_scratch_page_tree(vgpu);  }
>  
> @@ -2456,6 +2447,14 @@ static void intel_vgpu_destroy_all_ppgtt_mm(struct intel_vgpu *vgpu)
>  
>  static void intel_vgpu_destroy_ggtt_mm(struct intel_vgpu *vgpu)  {
> +	struct intel_gvt_partial_pte *pos;
> +
> +	list_for_each_entry(pos,
> +			&vgpu->gtt.ggtt_mm->ggtt_mm.partial_pte_list, list) {
> +		gvt_dbg_mm("partial PTE update on hold 0x%lx : 0x%llx\n",
> +			pos->offset, pos->data);
> +		kfree(pos);
> +	}
>  	intel_vgpu_destroy_mm(vgpu->gtt.ggtt_mm);
>  	vgpu->gtt.ggtt_mm = NULL;
>  }
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h b/drivers/gpu/drm/i915/gvt/gtt.h index 7a9b361..a11bfee 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.h
> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
> @@ -133,6 +133,12 @@ enum intel_gvt_mm_type {
>  
>  #define GVT_RING_CTX_NR_PDPS	GEN8_3LVL_PDPES
>  
> +struct intel_gvt_partial_pte {
> +	unsigned long offset;
> +	u64 data;
> +	struct list_head list;
> +};
> +
>  struct intel_vgpu_mm {
>  	enum intel_gvt_mm_type type;
>  	struct intel_vgpu *vgpu;
> @@ -157,8 +163,7 @@ struct intel_vgpu_mm {
>  		} ppgtt_mm;
>  		struct {
>  			void *virtual_ggtt;
> -			unsigned long last_partial_off;
> -			u64 last_partial_data;
> +			struct list_head partial_pte_list;
>  		} ggtt_mm;
>  	};
>  };
> --
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
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: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20181008/5b67e896/attachment.sig>


More information about the intel-gvt-dev mailing list