[PATCH v3] drm/i915/gvt: clear the vGPU reset logic

[Zhenyu Wang]

On 2017.02.21 14:30:07 +0800, Du, Changbin wrote:
> On Tue, Feb 21, 2017 at 01:37:39PM +0800, Ping Gao wrote:
> > Releasing shadow PPGTT pages is not enough when vGPU reset, the
> > guest page table tracking data should has same life-cycle with
> > all the shadow PPGTT pages; Otherwise there is no chance to
> > re-shadow the PPGTT pages without free the guest page table
> > tracking data.
> > 
> > This patch clear the PPGTT reset logic and make the vGPU reset in
> > working order.
> > 
> > v2: refactor some logic to avoid code duplicated.
> > v3: remove useless macro and add comments from Christophe.
> > 
> > Signed-off-by: Ping Gao <ping.a.gao at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/gtt.c | 39 ++++++++++++++++++++++++++++-----------
> >  1 file changed, 28 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> > index 28c9234..acd02f7 100644
> > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > @@ -2015,6 +2015,22 @@ int intel_vgpu_init_gtt(struct intel_vgpu *vgpu)
> >  	return create_scratch_page_tree(vgpu);
> >  }
> >  
> > +static void intel_vgpu_free_mm(struct intel_vgpu *vgpu, int type)
> > +{
> > +	struct list_head *pos, *n;
> > +	struct intel_vgpu_mm *mm;
> > +
> > +	list_for_each_safe(pos, n, &vgpu->gtt.mm_list_head) {
> > +		mm = container_of(pos, struct intel_vgpu_mm, list);
> A list_for_each_entry_safe can be used here. :)
>

yep, could leave for another one.

> > +		if (mm->type == type) {
> > +			vgpu->gvt->gtt.mm_free_page_table(mm);
> > +			list_del(&mm->list);
> > +			list_del(&mm->lru_list);
> > +			kfree(mm);
> > +		}
> > +	}
> > +}
> > +
> >  /**
> >   * intel_vgpu_clean_gtt - clean up per-vGPU graphics memory virulization
> >   * @vgpu: a vGPU
> > @@ -2027,18 +2043,18 @@ int intel_vgpu_init_gtt(struct intel_vgpu *vgpu)
> >   */
> >  void intel_vgpu_clean_gtt(struct intel_vgpu *vgpu)
> >  {
> > -	struct list_head *pos, *n;
> > -	struct intel_vgpu_mm *mm;
> > -
> >  	ppgtt_free_all_shadow_page(vgpu);
> > -	release_scratch_page_tree(vgpu);
> >  
> > -	list_for_each_safe(pos, n, &vgpu->gtt.mm_list_head) {
> > -		mm = container_of(pos, struct intel_vgpu_mm, list);
> > -		vgpu->gvt->gtt.mm_free_page_table(mm);
> > -		list_del(&mm->list);
> > -		list_del(&mm->lru_list);
> > -		kfree(mm);
> > +	/* Shadow pages are only created when there is no page
> > +	 * table tracking data, so remove page tracking data after
> > +	 * removing the shadow pages.This comment is for vGPU reset
> > +	 * path.
> > +	 */
> > +	intel_vgpu_free_mm(vgpu, INTEL_GVT_MM_PPGTT);
> > +
> > +	if (vgpu->resetting == false) {
> > +		intel_vgpu_free_mm(vgpu, INTEL_GVT_MM_GGTT);
> > +		release_scratch_page_tree(vgpu);
> >  	}
> This func may read a little confusing. It try to mix reset work with
> clean work, while the name is 'clean'. How about leave
> intel_vgpu_clean_gtt just do clean work, and add below 2 statements
> into intel_vgpu_reset_gtt()?
>   ppgtt_free_all_shadow_page(vgpu);
>   intel_vgpu_free_mm(vgpu, INTEL_GVT_MM_PPGTT);
> 
> This is more clear for me.
> 

Sounds better.

> >  }
> >  
> > @@ -2321,7 +2337,8 @@ void intel_vgpu_reset_gtt(struct intel_vgpu *vgpu, bool dmlr)
> >  {
> >  	int i;
> >  
> > -	ppgtt_free_all_shadow_page(vgpu);
> > +	intel_vgpu_clean_gtt(vgpu);
> > +
> >  	if (!dmlr)
> >  		return;
> >  
> > -- 
> > 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



> _______________________________________________
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20170221/5ff72d36/attachment.sig>