[PATCH v2] drm/i915/gvt: refine vgpu gtt sync policy to reduce invalidate times

[Wang, Zhi A]

This won't be a problem. It's only delayed WC buffer flush, not delay shadow. Maybe Weinan can change the word to delay_gtt_invalidate instead of delay gtt update to prevent confusing.

The virtual GGTT is still updated when guest write the virtual GGTT. We can still address the correct framebuffer.

-----Original Message-----
From: Tian, Kevin 
Sent: Friday, November 3, 2017 4:04 AM
To: Li, Weinan Z <weinan.z.li at intel.com>; intel-gvt-dev at lists.freedesktop.org
Cc: Zhenyu Wang <zhenyuw at linux.intel.com>; Wang, Zhi A <zhi.a.wang at intel.com>; Li, Weinan Z <weinan.z.li at intel.com>
Subject: RE: [PATCH v2] drm/i915/gvt: refine vgpu gtt sync policy to reduce invalidate times

The assumption of this change is that, all GGTT addresses beyond aperture are only accessed by device when executing a workload, so we can delay update of those entries until the point of detecting a workload submission. However such assumption may not be true, at least for display. What about guest chooses a way to flip framebuffers not by changing base address while instead by changing GGTT mapping on the same base address? If such guest framebuffer is already programmed to physical display plane (as required in local display usage e.g. automotive), then delay of update would lead to incorrect content being shown. Current driver might not behave like that, but in GVT-g side we should make sure it's sane.

> From: Weinan Li
> Sent: Friday, November 3, 2017 9:01 AM
> 
> Add delayed_update flag in gvt->gtt to indicate there is cached gtt 
> entries need to be flushed before vgpu workload submittion, then don't 
> need to trigger invalidate gtt every time gtt entry write, it can save 
> time in vgpu reset or vgpu has frequently gtt entries updating case.
> 
> v2 : use flag instead of atomic_t, code refine (Zhi)
> 
> Signed-off-by: Weinan Li <weinan.z.li at intel.com>
> Cc: Zhi Wang <zhi.a.wang at intel.com>
> Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/gtt.c | 20 ++++++++++++++++++--  
> drivers/gpu/drm/i915/gvt/gtt.h |  1 +
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c 
> b/drivers/gpu/drm/i915/gvt/gtt.c index 6fa9271..b4cb5ae 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -1293,6 +1293,15 @@ static int ppgtt_set_guest_page_oos(struct 
> intel_vgpu *vgpu,
>  	return intel_gvt_hypervisor_disable_page_track(vgpu, &gpt->track);  
> }
> 
> +int intel_vgpu_flush_mm(struct intel_gvt *gvt, bool force) {
> +	if (force || gvt->gtt.delayed_update) {
> +		gvt->gtt.delayed_update = false;
> +		gtt_invalidate(gvt->dev_priv);
> +	}
> +	return 0;
> +}
> +
>  /**
>   * intel_vgpu_sync_oos_pages - sync all the out-of-synced shadow for 
> vGPU
>   * @vgpu: a vGPU
> @@ -1405,6 +1414,8 @@ int intel_vgpu_flush_post_shadow(struct
> intel_vgpu *vgpu)
>  	unsigned long index;
>  	int ret;
> 
> +	intel_vgpu_flush_mm(vgpu->gvt, false);
> +
>  	list_for_each_safe(pos, n, &vgpu->gtt.post_shadow_list_head) {
>  		spt = container_of(pos, struct intel_vgpu_ppgtt_spt,
>  				post_shadow_list);
> @@ -1953,7 +1964,6 @@ static int emulate_gtt_mmio_write(struct 
> intel_vgpu *vgpu, unsigned int off,
>  	/* the VM may configure the whole GM space when ballooning is used 
> */
>  	if (!vgpu_gmadr_is_valid(vgpu, gma))
>  		return 0;
> -
>  	ggtt_get_guest_entry(ggtt_mm, &e, g_gtt_index);
> 
>  	memcpy((void *)&e.val64 + (off & (info->gtt_entry_size - 1)), 
> p_data, @@ -1975,8 +1985,14 @@ static int 
> emulate_gtt_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
>  	}
> 
>  	ggtt_set_shadow_entry(ggtt_mm, &m, g_gtt_index);
> -	gtt_invalidate(gvt->dev_priv);
> +
> +	if (vgpu_gmadr_is_aperture(vgpu, gma))
> +		gtt_invalidate(gvt->dev_priv);
> +	else
> +		gvt->gtt.delayed_update = true;
> +
>  	ggtt_set_guest_entry(ggtt_mm, &e, g_gtt_index);
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.h 
> b/drivers/gpu/drm/i915/gvt/gtt.h index 416b2f8..f36f485 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.h
> +++ b/drivers/gpu/drm/i915/gvt/gtt.h
> @@ -88,6 +88,7 @@ struct intel_gvt_gtt {
> 
>  	struct page *scratch_page;
>  	unsigned long scratch_mfn;
> +	bool delayed_update;
>  };
> 
>  enum {
> --
> 1.9.1
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev