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

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, Apr 12, 2017 at 04:36:57PM +0800, Weinan Li wrote:
> I915_GEM_GET_APERTURE ioctl is used to probe aperture size from userspace.
> Some applications like OpenCL use this information to know how much GM
> resource can it use.

That's a userspace bug.

> In gvt environment, each vm only use the ballooned
> part of aperture, so we should return the actual aperture size exclude
> the reserved part by balloon.
> 
> I915_GEM_CONTEXT_GETPARAM ioctl query the I915_CONTEXT_PARAM_GTT_SIZE, we
> also need to exclude the reserved part in GTT.
> 
> Signed-off-by: Weinan Li <weinan.z.li@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c         |  7 +++----
>  drivers/gpu/drm/i915/i915_gem_context.c |  4 +++-
>  drivers/gpu/drm/i915/i915_vgpu.c        | 18 +++++++++++++++++-
>  drivers/gpu/drm/i915/i915_vgpu.h        |  2 ++
>  4 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 84ea249..b3fb424 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -145,9 +145,8 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
>  	struct drm_i915_gem_get_aperture *args = data;
>  	struct i915_vma *vma;
> -	size_t pinned;
> +	size_t pinned = 0;
>  
> -	pinned = 0;
>  	mutex_lock(&dev->struct_mutex);
>  	list_for_each_entry(vma, &ggtt->base.active_list, vm_link)
>  		if (i915_vma_is_pinned(vma))
> @@ -157,9 +156,9 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
>  			pinned += vma->node.size;
>  	mutex_unlock(&dev->struct_mutex);
>  
> -	args->aper_size = ggtt->base.total;
> +	args->aper_size = ggtt->base.total -
> +		 intel_vgt_reserved_size_by_balloon(dev_priv);
>  	args->aper_available_size = args->aper_size - pinned;
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 8bd0c49..9f3280d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -88,6 +88,7 @@
>  #include <drm/drmP.h>
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include "i915_vgpu.h"
>  #include "i915_trace.h"
>  
>  #define ALL_L3_SLICES(dev) (1 << NUM_L3_SLICES(dev)) - 1
> @@ -1053,7 +1054,8 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
>  		else if (to_i915(dev)->mm.aliasing_ppgtt)
>  			args->value = to_i915(dev)->mm.aliasing_ppgtt->base.total;
>  		else
> -			args->value = to_i915(dev)->ggtt.base.total;
> +			args->value = to_i915(dev)->ggtt.base.total -
> +			  intel_vgt_reserved_size_by_balloon(dev->dev_private);
>  		break;
>  	case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
>  		args->value = i915_gem_context_no_error_capture(ctx);
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index 4ab8a97..ce722d8 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -88,6 +88,11 @@ struct _balloon_info_ {
>  	 * graphic memory, 2/3 for unmappable graphic memory.
>  	 */
>  	struct drm_mm_node space[4];
> +	/*
> +	 * Total space size exclude ballooned named reserved_total, it's
> +	 * invisible for vGPU.
> +	 */
> +	size_t reserved_total;

What is size_t?

>  };
>  
>  static struct _balloon_info_ bl_info;
> @@ -116,6 +121,14 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
>  	memset(&bl_info, 0, sizeof(bl_info));
>  }
>  
> +size_t intel_vgt_reserved_size_by_balloon(struct drm_i915_private *dev_priv)
> +{
> +	if (!intel_vgpu_active(dev_priv))
> +		return 0;
> +
> +	return bl_info.reserved_total;
> +}

Or just return bl_info.reserved_total.

Why is there a global here anyway?

Better would be to track dev_priv->ggtt.reserved

Then the core code becomes
	gtt_size = dev_priv->ggtt.total - dev_priv->ggtt.reserved;

and doesn't need to know the identity of every possible consumer.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx