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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
