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

[Wang, Zhi A]

This is just a workaround and will be removed soon. Better keep the change as small as it can be. Can you take this patch first? Since I'm going to let Joonas know that we don't need to change i915 if this patch works.

Thanks,
Zhi.

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

On 2017.10.19 02:30:09 +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;
>  }
>  

Totally ignore return value is not good, could isolate invalid case and bypass specially for that case but keep possible err path?

>  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

--
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827