Re: [PATCH 1/3] drm/i915: Move the release of PT page to the upper caller

[Zhi Wang <zhi.a.wang@intel.com>]

Thanks for the efforts! The commits in linux tree are:

3e52d71e drm/i915: A hotfix for making aliasing PPGTT work for GVT-g
26d12c6 drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.
a18dbba drm/i915: Move the release of PT page to the upper caller

Thanks,
Zhi.

On 04/05/17 14:36, Zhi Wang wrote:
> Hi Greg:
>      Would you mind to take these three patches into next
> linux-stable-4.10.x? Another two patches are:
>
> [PATCH 2/3] drm/i915: Let execlist_update_context() cover !FULL_PPGTT mode.
>
> [PATCH 3/3] drm/i915: A hotfix for making aliasing PPGTT work for GVT-g
>
> Thank you so much! :P
>
> Thanks,
> Zhi.
>
> On 04/05/17 14:33, 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.
>>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Cc: Michel Thierry <michel.thierry@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
>> Cc: Zhiyuan Lv <zhiyuan.lv@intel.com>
>> Cc: Greg KH <gregkh@linuxfoundation.org> #v4.10+
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
>> Link:
>> https://patchwork.freedesktop.org/patch/122697/msgid/1479728666-25333-1-git-send-email-zhi.a.wang@intel.com
>>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Link:
>> http://patchwork.freedesktop.org/patch/msgid/1480402516-22275-1-git-send-email-zhi.a.wang@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);
>>           }
>>       }
>>   }
>>