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

[Zhenyu Wang]

On 2018.05.07 11:29:34 +0800, Du, Changbin wrote:
> 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.
>

I mean use fetch value for the trace and return dec value, better inline this as well.

> > > +
> > >  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
> _______________________________________________
> 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: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20180507/0d7cc27c/attachment.sig>