[PATCH 2/2] drm/i915/gvt: A workaround for an regression caused by one i915 PPGTT optimization

[Wang, Zhi A]

Practically yes. But we cannot assume guest is always behaving good. That's the reason why I call it a workaround and the root cause of fix should be the lazy shadow, which I hope I can make it work this week.

Thanks,
Zhi.

-----Original Message-----
From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On Behalf Of Du, Changbin
Sent: Thursday, October 19, 2017 5:55 AM
To: Zhi Wang <zhi.wang.linux at gmail.com>
Cc: Du, Changbin <changbin.du at intel.com>; intel-gvt-dev at lists.freedesktop.org; Wang, Zhi A <zhi.a.wang at intel.com>
Subject: Re: [PATCH 2/2] drm/i915/gvt: A workaround for an regression caused by one i915 PPGTT optimization

I think this is not a workaround but a fix. GTE always has valid value is not a true assumption. Shadowing code should cover this case, and it is not an error (the error msg should remove).

On Thu, Oct 19, 2017 at 02:30:09AM +0800, Zhi Wang wrote:
> In the commit:
> 
> commit 14826673247eaf36b16fd821fac27efa663f3fa6
> Author: Chris Wilson <chris at chris-wilson.co.uk>
> Date:   Fri Sep 8 19:16:22 2017 +0100
> 
>     drm/i915: Only initialize partially filled pagetables
> 
>     If we know that we will completely fill a pagetable (i.e. we are
>     inserting a complete set of 512 pages), we can skip prefilling that PT
>     with scratch entries. If we have to abort the insertion prior to writing
>     the real entries, we will teardown the pagetable and remove it from the
>     page directory (so that we will restart the allocation next time).
> 
>     We could do similar tricks for the PD and PDP, but the likelihood of a
>     single insertion covering the entire 512 entries diminishes, as do the
>     cycle savings. The saving are even greater (relatively) when we are
>     preallocating page tables for huge pages, as then we never need to fill
>     the page table.
> 
> It will link an un-initialized PTE page into a PPGTT page table 
> tracked by GVT-g, which leads to linux guest failing to boot. Since 
> the fix of root casue still needs some time to be ready, a temporary 
> workaround is introduced first.
> 
> Signed-off-by: Zhi Wang <zhi.a.wang at intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/gtt.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c 
> b/drivers/gpu/drm/i915/gvt/gtt.c index 6fa9271..d24d52d 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -767,7 +767,6 @@ static int ppgtt_write_protection_handler(void 
> *data, u64 pa,  {
>  	struct intel_vgpu_page_track *t = data;
>  	struct intel_vgpu_guest_page *p = page_track_to_guest_page(t);
> -	int ret;
>  
>  	if (bytes != 4 && bytes != 8)
>  		return -EINVAL;
> @@ -775,11 +774,9 @@ static int ppgtt_write_protection_handler(void *data, u64 pa,
>  	if (!t->tracked)
>  		return -EINVAL;
>  
> -	ret = ppgtt_handle_guest_write_page_table_bytes(p,
> +	ppgtt_handle_guest_write_page_table_bytes(p,
>  		pa, p_data, bytes);
> -	if (ret)
> -		return ret;
> -	return ret;
> +	return 0;
>  }
>  
>  static int reclaim_one_mm(struct intel_gvt *gvt);
> --
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

--
Thanks,
Changbin Du