[Intel-gfx] [PATCH] drm/i915: Move the release of PT page to the upper caller

[Zhi Wang]

Hi guys:
     Would you mind to have a quick review on this patch? :P The linux 
guest under GVT-g couldn't boot up without this patch in the newer kernel.

Thanks,
Zhi.

On 11/21/16 19:44, Zhi Wang wrote:
> a PT page will be released if it doesn't contain any meaningful mappings
> during PPGTT page table shrinking. The PT entry in the upper level will
> be set to a scratch entry.
>
> Normally this works nicely, but in virtualization world, the PPGTT page
> table is tracked by hypervisor. Releasing the PT page before modifying
> the upper level PT entry would cause extra efforts.
>
> As the tracked page has been returned to OS before losing track from
> hypervisor, it could be written in any pattern. Hypervisor has to recognize
> if a page is still being used as a PT page by validating these writing
> patterns. It's complicated. Better let the guest modify the PT entry in
> upper level PT first, then release the PT page.
>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> Cc: Zhiyuan Lv <zhiyuan.lv at intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++-----------
>   1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b4bde14..6cee707 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -736,10 +736,8 @@ static bool gen8_ppgtt_clear_pt(struct i915_address_space *vm,
>
>   	bitmap_clear(pt->used_ptes, pte, num_entries);
>
> -	if (bitmap_empty(pt->used_ptes, GEN8_PTES)) {
> -		free_pt(to_i915(vm->dev), pt);
> +	if (bitmap_empty(pt->used_ptes, GEN8_PTES))
>   		return true;
> -	}
>
>   	pt_vaddr = kmap_px(pt);
>
> @@ -775,13 +773,12 @@ static bool gen8_ppgtt_clear_pd(struct i915_address_space *vm,
>   			pde_vaddr = kmap_px(pd);
>   			pde_vaddr[pde] = scratch_pde;
>   			kunmap_px(ppgtt, pde_vaddr);
> +			free_pt(to_i915(vm->dev), pt);
>   		}
>   	}
>
> -	if (bitmap_empty(pd->used_pdes, I915_PDES)) {
> -		free_pd(to_i915(vm->dev), pd);
> +	if (bitmap_empty(pd->used_pdes, I915_PDES))
>   		return true;
> -	}
>
>   	return false;
>   }
> @@ -795,7 +792,6 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
>   				 uint64_t length)
>   {
>   	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
> -	struct drm_i915_private *dev_priv = to_i915(vm->dev);
>   	struct i915_page_directory *pd;
>   	uint64_t pdpe;
>   	gen8_ppgtt_pdpe_t *pdpe_vaddr;
> @@ -813,16 +809,14 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
>   				pdpe_vaddr[pdpe] = scratch_pdpe;
>   				kunmap_px(ppgtt, pdpe_vaddr);
>   			}
> +			free_pd(to_i915(vm->dev), pd);
>   		}
>   	}
>
>   	mark_tlbs_dirty(ppgtt);
>
> -	if (USES_FULL_48BIT_PPGTT(dev_priv) &&
> -	    bitmap_empty(pdp->used_pdpes, I915_PDPES_PER_PDP(dev_priv))) {
> -		free_pdp(dev_priv, pdp);
> +	if (bitmap_empty(pdp->used_pdpes, I915_PDPES_PER_PDP(dev_priv)))
>   		return true;
> -	}
>
>   	return false;
>   }
> @@ -836,6 +830,7 @@ static void gen8_ppgtt_clear_pml4(struct i915_address_space *vm,
>   				  uint64_t start,
>   				  uint64_t length)
>   {
> +	struct drm_i915_private *dev_priv = to_i915(vm->dev);
>   	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
>   	struct i915_page_directory_pointer *pdp;
>   	uint64_t pml4e;
> @@ -854,6 +849,7 @@ static void gen8_ppgtt_clear_pml4(struct i915_address_space *vm,
>   			pml4e_vaddr = kmap_px(pml4);
>   			pml4e_vaddr[pml4e] = scratch_pml4e;
>   			kunmap_px(ppgtt, pml4e_vaddr);
> +			free_pdp(dev_priv, pdp);
>   		}
>   	}
>   }
>