On pe, 2017-05-26 at 09:37 +0800, Weinan Li wrote:
> I915_GEM_GET_APERTURE ioctl is used to probe aperture size from userspace.
> In gvt environment, each vm only use the ballooned part of aperture, so we
> should return the correct available aperture size exclude the reserved part
> by balloon.
>
> v2: add 'reserved' in struct i915_address_space to record the reserved size
> in ggtt (Chris)
>
> v3: remain aper_size as total, adjust aper_available_size exclude reserved
> and pinned. UMD driver need to adjust the max allocation size according to
> the available aperture size but not total size. KMD return the correct
> usable aperture size any time (Chris, Joonas)
>
> v4: decrease reserved in deballoon (Joonas)
>
> v5: add onion teardown in balloon, add vgt_deballoon_space (Joonas)
>
> v6: change title name (Zhenyu)
>
> Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
> Signed-off-by: Weinan Li <weinan.z.li@xxxxxxxxx>
<SNIP>
> @@ -156,8 +156,8 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
> mutex_unlock(&dev->struct_mutex);
>
> args->aper_size = ggtt->base.total;
> - args->aper_available_size = args->aper_size - pinned;
> -
> + args->aper_available_size = args->aper_size -
> + ggtt->base.reserved - pinned;
Wrong indentation here.
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -92,6 +92,17 @@ struct _balloon_info_ {
>
> static struct _balloon_info_ bl_info;
>
> +static void vgt_deballoon_space(struct i915_ggtt *ggtt,
> + struct drm_mm_node *node)
Ditto.
> +{
> + DRM_INFO("deballoon space: range [ 0x%llx - 0x%llx ] %llu KiB.\n",
> + node->start, node->start + node->size, node->size / 1024);
> +
> + ggtt->base.reserved -= node->size;
> + drm_mm_remove_node(node);
> + memset(node, 0, sizeof(*node));
memset() is not needed.
> @@ -127,9 +135,14 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt,
>
> DRM_INFO("balloon space: range [ 0x%lx - 0x%lx ] %lu KiB.\n",
> start, end, size / 1024);
> - return i915_gem_gtt_reserve(&ggtt->base, node,
> + ret = i915_gem_gtt_reserve(&ggtt->base, node,
> size, start, I915_COLOR_UNEVICTABLE,
> 0);
> + if (!ret)
> + ggtt->base.reserved += size;
> + else
> + memset(node, 0, sizeof(*node));
memset() is not needed.
> + return ret;
> }
>
> /**
> @@ -215,14 +228,14 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
> ggtt->mappable_end, unmappable_base);
>
> if (ret)
> - goto err;
> + goto err_out;
"err" is a good enough label for last level error path. "out" is used
when the path is also taken on a successful call of the function.
> }
>
> if (unmappable_end < ggtt_end) {
> ret = vgt_balloon_space(ggtt, &bl_info.space[3],
> unmappable_end, ggtt_end);
> if (ret)
> - goto err;
> + goto err_deballoon_upon_mappable;
This function is about ballooning, so "deballoon" is bit verbose. Just
"err_upon_mappable".
I commented about the whitespace issues and memsets in previous
revisions already, so please go through the review comments
systematically to expedite further reviews.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
