[PATCH v4 13/14] drm/i915/gvt: Fix error handling in ppgtt_populate_spt_by_guest_entry

[Du, Changbin]

On Mon, May 07, 2018 at 11:17:31AM +0800, Zhenyu Wang wrote:
> On 2018.05.04 17:50:39 +0800, changbin.du at intel.com wrote:
> > From: Changbin Du <changbin.du at intel.com>
> > 
> > Don't forget to free allocated spt if shadowing failed.
> > 
> > Signed-off-by: Changbin Du <changbin.du at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/gtt.c | 31 ++++++++++++++++++++-----------
> >  1 file changed, 20 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> > index 2bf85be..4af6fee3 100644
> > --- a/drivers/gpu/drm/i915/gvt/gtt.c
> > +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> > @@ -912,10 +912,17 @@ static void ppgtt_get_spt(struct intel_vgpu_ppgtt_spt *spt)
> >  	int v = atomic_read(&spt->refcount);
> >  
> >  	trace_spt_refcount(spt->vgpu->id, "inc", spt, v, (v + 1));
> > -
> >  	atomic_inc(&spt->refcount);
> >  }
> >  
> > +static int ppgtt_put_spt(struct intel_vgpu_ppgtt_spt *spt)
> > +{
> > +	int v = atomic_read(&spt->refcount);
> > +
> > +	trace_spt_refcount(spt->vgpu->id, "dec", spt, v, (v - 1));
> > +	return atomic_dec_return(&spt->refcount);
> > +}
> 
> Use atomic_fetch_dec()
>
Then it will be wrong. The caller expects the value after dec op.

> > +
> >  static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt);
> >  
> >  static int ppgtt_invalidate_spt_by_shadow_entry(struct intel_vgpu *vgpu,
> > @@ -967,14 +974,11 @@ static int ppgtt_invalidate_spt(struct intel_vgpu_ppgtt_spt *spt)
> >  	struct intel_gvt_gtt_entry e;
> >  	unsigned long index;
> >  	int ret;
> > -	int v = atomic_read(&spt->refcount);
> >  
> >  	trace_spt_change(spt->vgpu->id, "die", spt,
> >  			spt->guest_page.gfn, spt->shadow_page.type);
> >  
> > -	trace_spt_refcount(spt->vgpu->id, "dec", spt, v, (v - 1));
> > -
> > -	if (atomic_dec_return(&spt->refcount) > 0)
> > +	if (ppgtt_put_spt(spt) > 0)
> >  		return 0;
> >  
> >  	for_each_present_shadow_entry(spt, &e, index) {
> > @@ -1056,8 +1060,10 @@ static struct intel_vgpu_ppgtt_spt *ppgtt_populate_spt_by_guest_entry(
> >  			gvt_dbg_mm("reshadow PDE since ips changed\n");
> >  			clear_page(spt->shadow_page.vaddr);
> >  			ret = ppgtt_populate_spt(spt);
> > -			if (ret)
> > -				goto fail;
> > +			if (ret) {
> > +				ppgtt_put_spt(spt);
> > +				goto err;
> > +			}
> >  		}
> >  	} else {
> >  		int type = get_next_pt_type(we->type);
> > @@ -1065,22 +1071,25 @@ static struct intel_vgpu_ppgtt_spt *ppgtt_populate_spt_by_guest_entry(
> >  		spt = ppgtt_alloc_spt_gfn(vgpu, type, ops->get_pfn(we), ips);
> >  		if (IS_ERR(spt)) {
> >  			ret = PTR_ERR(spt);
> > -			goto fail;
> > +			goto err;
> >  		}
> >  
> >  		ret = intel_vgpu_enable_page_track(vgpu, spt->guest_page.gfn);
> >  		if (ret)
> > -			goto fail;
> > +			goto err_free_spt;
> >  
> >  		ret = ppgtt_populate_spt(spt);
> >  		if (ret)
> > -			goto fail;
> > +			goto err_free_spt;
> >  
> >  		trace_spt_change(vgpu->id, "new", spt, spt->guest_page.gfn,
> >  				 spt->shadow_page.type);
> >  	}
> >  	return spt;
> > -fail:
> > +
> > +err_free_spt:
> > +	ppgtt_free_spt(spt);
> > +err:
> >  	gvt_vgpu_err("fail: shadow page %p guest entry 0x%llx type %d\n",
> >  		     spt, we->val64, we->type);
> >  	return ERR_PTR(ret);
> > -- 
> > 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



-- 
Thanks,
Changbin Du