Re: [PATCH v3] drm/i915/gvt: return the actual aperture size under gvt environment

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ti, 2017-05-09 at 03:10 +0000, Li, Weinan Z wrote:

> > > > @@ -242,6 +242,9 @@ int intel_vgt_balloon(struct drm_i915_private
> > > > *dev_priv)
> > > >  			goto err;
> > > >  	}
> > > > 
> > > > +	for (i = 0; i < ARRAY_SIZE(bl_info.space); i++)
> > > > +		ggtt->base.reserved += bl_info.space[i].size;
> > > > +
> > 
> > There should be an equal decrease when deballooning is done. And for that to
> > be correct, you need to add proper onion teardown to this function to make
> > sure the count stays correct (can't call deballoon on failure or the count will
> > become negative which will result in huge number marked as reserved).
> Oh, that's my fault. Should add clean up in intel_vgt_deballoon().
> @@ -114,6 +114,7 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
>         }
> 
>         memset(&bl_info, 0, sizeof(bl_info));
> +       dev_priv->ggtt.reserved = 0;
> }
> Since if any steps in intel_vgt_balloon() fail, it will deal as error and run
> intel_vgt_deballoon() for clean up, no partial success happen.
> So we only calculate the reserved when balloon success, it can ensure it's correct.

Onion teardown should be used according to kernel coding style, there's
really no excuse not to.

Just add to the ggtt->base.reserved in increments, and remove in
increments during teardown or in the deballoon function. ggtt.reserved
is not exclusively for GVT-g to use, so you can't simply zero it. There
needs to be incremental additions and substractions as objects are
added and removed for the variable to stay general.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx