All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment
@ 2017-05-10  2:59 Weinan Li
  2017-05-10  3:24 ` ✓ Fi.CI.BAT: success for drm/i915/gvt: return the correct usable aperture size under gvt environment (rev2) Patchwork
  2017-05-10 10:42 ` [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment Joonas Lahtinen
  0 siblings, 2 replies; 10+ messages in thread
From: Weinan Li @ 2017-05-10  2:59 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev

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.

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.

v4: add onion teardown to balloon and deballoon to make sure the reserved
stays correct. Code style refine.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Weinan Li <weinan.z.li@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c     | 4 ++--
 drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
 drivers/gpu/drm/i915/i915_vgpu.c    | 8 +++++++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 33fb11c..8d8d9c0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -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;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index fb15684..da9aa9f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -255,6 +255,7 @@ struct i915_address_space {
 	struct drm_i915_file_private *file;
 	struct list_head global_link;
 	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
+	u64 reserved;		/* size addr space reserved */
 
 	bool closed;
 
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 4ab8a97..25bed9b 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -109,8 +109,10 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
 	DRM_DEBUG("VGT deballoon.\n");
 
 	for (i = 0; i < 4; i++) {
-		if (bl_info.space[i].allocated)
+		if (bl_info.space[i].allocated) {
+			dev_priv->ggtt.base.reserved -= bl_info.space[i].size;
 			drm_mm_remove_node(&bl_info.space[i]);
+		}
 	}
 
 	memset(&bl_info, 0, sizeof(bl_info));
@@ -216,6 +218,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 
 		if (ret)
 			goto err;
+		ggtt->base.reserved += bl_info.space[2].size;
 	}
 
 	if (unmappable_end < ggtt_end) {
@@ -223,6 +226,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 					unmappable_end, ggtt_end);
 		if (ret)
 			goto err;
+		ggtt->base.reserved += bl_info.space[3].size;
 	}
 
 	/* Mappable graphic memory ballooning */
@@ -232,6 +236,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 
 		if (ret)
 			goto err;
+		ggtt->base.reserved += bl_info.space[0].size;
 	}
 
 	if (mappable_end < ggtt->mappable_end) {
@@ -240,6 +245,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 
 		if (ret)
 			goto err;
+		ggtt->base.reserved += bl_info.space[1].size;
 	}
 
 	DRM_INFO("VGT balloon successfully\n");
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/gvt: return the correct usable aperture size under gvt environment (rev2)
  2017-05-10  2:59 [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment Weinan Li
@ 2017-05-10  3:24 ` Patchwork
  2017-05-10 10:42 ` [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment Joonas Lahtinen
  1 sibling, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-05-10  3:24 UTC (permalink / raw)
  To: Li, Weinan Z; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gvt: return the correct usable aperture size under gvt environment (rev2)
URL   : https://patchwork.freedesktop.org/series/24206/
State : success

== Summary ==

Series 24206v2 drm/i915/gvt: return the correct usable aperture size under gvt environment
https://patchwork.freedesktop.org/api/1.0/series/24206/revisions/2/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:436s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:437s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:594s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:523s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:535s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:497s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:500s
fi-elk-e7500     total:278  pass:229  dwarn:0   dfail:0   fail:0   skip:49  time:426s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:417s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:419s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:498s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:473s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:468s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:572s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:460s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:583s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:469s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:498s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:434s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:410s

417babaa12ad98dad2c39f361612f1afe6894816 drm-tip: 2017y-05m-09d-13h-13m-23s UTC integration manifest
25b4a18 drm/i915/gvt: return the correct usable aperture size under gvt environment

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4654/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment
  2017-05-10  2:59 [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment Weinan Li
  2017-05-10  3:24 ` ✓ Fi.CI.BAT: success for drm/i915/gvt: return the correct usable aperture size under gvt environment (rev2) Patchwork
@ 2017-05-10 10:42 ` Joonas Lahtinen
  2017-05-11  6:51   ` Li, Weinan Z
  1 sibling, 1 reply; 10+ messages in thread
From: Joonas Lahtinen @ 2017-05-10 10:42 UTC (permalink / raw)
  To: Weinan Li, intel-gfx, intel-gvt-dev

On ke, 2017-05-10 at 10:59 +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.
> 
> 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.
> 
> v4: add onion teardown to balloon and deballoon to make sure the reserved
> stays correct. Code style refine.

There's no onion teardown seen yet, please read:

https://www.kernel.org/doc/html/v4.10/process/coding-style.html#central
ized-exiting-of-functions

Please incorporate the addition to vgt_balloon_space function to reduce
code duplication.

Once the proper teardown is implemented, intel_vgt_deballoon function
should unconditionally remove the drm_mm nodes as there's no condition
when only one of them would be allocated. And intel_vgt_balloon
definitely should not call intel_vgt_deballoon in error path as per the
coding style above.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment
  2017-05-10 10:42 ` [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment Joonas Lahtinen
@ 2017-05-11  6:51   ` Li, Weinan Z
  2017-05-11 12:55     ` Joonas Lahtinen
  0 siblings, 1 reply; 10+ messages in thread
From: Li, Weinan Z @ 2017-05-11  6:51 UTC (permalink / raw)
  To: Joonas Lahtinen, intel-gfx, intel-gvt-dev

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Wednesday, May 10, 2017 6:43 PM
> To: Li, Weinan Z <weinan.z.li@intel.com>; intel-gfx@lists.freedesktop.org; intel-
> gvt-dev@lists.freedesktop.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Subject: Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size
> under gvt environment
> 
> On ke, 2017-05-10 at 10:59 +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.
> >
> > 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.
> >
> > v4: add onion teardown to balloon and deballoon to make sure the
> > reserved stays correct. Code style refine.
> 
> There's no onion teardown seen yet, please read:
> 
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#central
> ized-exiting-of-functions
> 
> Please incorporate the addition to vgt_balloon_space function to reduce code
> duplication.
> 
> Once the proper teardown is implemented, intel_vgt_deballoon function should
> unconditionally remove the drm_mm nodes as there's no condition when only
> one of them would be allocated. And intel_vgt_balloon definitely should not call
> intel_vgt_deballoon in error path as per the coding style above.

Thanks Joonas. A little change is brought in if move the fail checking into balloon_space()
There are 4 balloon spaces, current policy is if any one fail clean up all the success ones, with
this change it won't clean up the success ones. It should not impact the driver's behavior.

@@ -120,6 +122,7 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt,
                             struct drm_mm_node *node,
                             unsigned long start, unsigned long end)
 {
+       int ret;
        unsigned long size = end - start;

        if (start >= end)
@@ -127,9 +130,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));
+       return ret;
 }

 /**
@@ -247,6 +255,5 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)

 err:
        DRM_ERROR("VGT balloon fail\n");
-       intel_vgt_deballoon(dev_priv);
        return ret;
 }
> 
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment
  2017-05-11  6:51   ` Li, Weinan Z
@ 2017-05-11 12:55     ` Joonas Lahtinen
  2017-05-12  3:20       ` Li, Weinan Z
  0 siblings, 1 reply; 10+ messages in thread
From: Joonas Lahtinen @ 2017-05-11 12:55 UTC (permalink / raw)
  To: Li, Weinan Z, intel-gfx, intel-gvt-dev

On to, 2017-05-11 at 06:51 +0000, Li, Weinan Z wrote:
> > 
> > -----Original Message-----
> > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> > Sent: Wednesday, May 10, 2017 6:43 PM
> > To: Li, Weinan Z <weinan.z.li@intel.com>; intel-gfx@lists.freedesktop.org; intel-
> > gvt-dev@lists.freedesktop.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Subject: Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size
> > under gvt environment
> > 
> > On ke, 2017-05-10 at 10:59 +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.
> > > 
> > > 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.
> > > 
> > > v4: add onion teardown to balloon and deballoon to make sure the
> > > reserved stays correct. Code style refine.
> > 
> > There's no onion teardown seen yet, please read:
> > 
> > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#central
> > ized-exiting-of-functions
> > 
> > Please incorporate the addition to vgt_balloon_space function to reduce code
> > duplication.
> > 
> > Once the proper teardown is implemented, intel_vgt_deballoon function should
> > unconditionally remove the drm_mm nodes as there's no condition when only
> > one of them would be allocated. And intel_vgt_balloon definitely should not call
> > intel_vgt_deballoon in error path as per the coding style above.
> 
> Thanks Joonas. A little change is brought in if move the fail checking into balloon_space()
> There are 4 balloon spaces, current policy is if any one fail clean up all the success ones, with
> this change it won't clean up the success ones. It should not impact the driver's behavior.

Please re-read the "Centralized exiting of function", in this case
you'd have three labels for onion teardown if any of the space
reservations fails, you jump to the right one. Please take a look in
the i915 to similar initialization functions for examples.

> @@ -127,9 +130,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));

This should not be needed. Either all of the nodes have been
successfully initialize or not. The only partial states are in the
intel_vgt_balloon function, which should use different labels to back
off from different stages of initialization.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment
  2017-05-11 12:55     ` Joonas Lahtinen
@ 2017-05-12  3:20       ` Li, Weinan Z
  2017-05-18  8:28         ` Joonas Lahtinen
  0 siblings, 1 reply; 10+ messages in thread
From: Li, Weinan Z @ 2017-05-12  3:20 UTC (permalink / raw)
  To: Joonas Lahtinen, intel-gfx, intel-gvt-dev



Thanks.
Best Regards.
Weinan, LI


> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Thursday, May 11, 2017 8:56 PM
> To: Li, Weinan Z <weinan.z.li@intel.com>; intel-gfx@lists.freedesktop.org; intel-
> gvt-dev@lists.freedesktop.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Subject: Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size
> under gvt environment
> 
> On to, 2017-05-11 at 06:51 +0000, Li, Weinan Z wrote:
> > >
> > > -----Original Message-----
> > > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> > > Sent: Wednesday, May 10, 2017 6:43 PM
> > > To: Li, Weinan Z <weinan.z.li@intel.com>;
> > > intel-gfx@lists.freedesktop.org; intel-
> > > gvt-dev@lists.freedesktop.org
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Subject: Re: [PATCH v4] drm/i915/gvt: return the correct usable
> > > aperture size under gvt environment
> > >
> > > On ke, 2017-05-10 at 10:59 +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.
> > > >
> > > > 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.
> > > >
> > > > v4: add onion teardown to balloon and deballoon to make sure the
> > > > reserved stays correct. Code style refine.
> > >
> > > There's no onion teardown seen yet, please read:
> > >
> > > https://www.kernel.org/doc/html/v4.10/process/coding-style.html#cent
> > > ral
> > > ized-exiting-of-functions
> > >
> > > Please incorporate the addition to vgt_balloon_space function to
> > > reduce code duplication.
> > >
> > > Once the proper teardown is implemented, intel_vgt_deballoon
> > > function should unconditionally remove the drm_mm nodes as there's
> > > no condition when only one of them would be allocated. And
> > > intel_vgt_balloon definitely should not call intel_vgt_deballoon in error
> path as per the coding style above.
> >
> > Thanks Joonas. A little change is brought in if move the fail checking
> > into balloon_space() There are 4 balloon spaces, current policy is if
> > any one fail clean up all the success ones, with this change it won't clean up
> the success ones. It should not impact the driver's behavior.
> 
> Please re-read the "Centralized exiting of function", in this case you'd have
> three labels for onion teardown if any of the space reservations fails, you jump
> to the right one. Please take a look in the i915 to similar initialization functions
> for examples.
> 
> > @@ -127,9 +130,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));
> 
> This should not be needed. Either all of the nodes have been successfully
> initialize or not. The only partial states are in the intel_vgt_balloon function,
> which should use different labels to back off from different stages of
> initialization.
Thanks Joonas' guidance.
I add 4 labels in intel_vgt_balloon for cleaning up ballooned space, the reserved size
will increase when one balloon space reserve success, and will clean up if anyone reserve
fail step by step.

diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 4ab8a97..e21cfff 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -109,8 +109,8 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
        DRM_DEBUG("VGT deballoon.\n");

        for (i = 0; i < 4; i++) {
-               if (bl_info.space[i].allocated)
-                       drm_mm_remove_node(&bl_info.space[i]);
+               dev_priv->ggtt.base.reserved -= bl_info.space[i].size;
+               drm_mm_remove_node(&bl_info.space[i]);
        }

        memset(&bl_info, 0, sizeof(bl_info));
@@ -120,6 +120,7 @@ static int vgt_balloon_space(struct i915_ggtt *ggtt,
                             struct drm_mm_node *node,
                             unsigned long start, unsigned long end)
 {
+       int ret;
        unsigned long size = end - start;

        if (start >= end)
@@ -127,9 +128,12 @@ 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;
+       return ret;
 }

 /**
@@ -215,14 +219,14 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
                                        ggtt->mappable_end, unmappable_base);

                if (ret)
-                       goto err;
+                       goto out_err;
        }

        if (unmappable_end < ggtt_end) {
                ret = vgt_balloon_space(ggtt, &bl_info.space[3],
                                        unmappable_end, ggtt_end);
                if (ret)
-                       goto err;
+                       goto deballoon_upon_mappable;
        }

        /* Mappable graphic memory ballooning */
@@ -231,7 +235,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
                                        0, mappable_base);

                if (ret)
-                       goto err;
+                       goto deballoon_upon_unmappable;
        }

        if (mappable_end < ggtt->mappable_end) {
@@ -239,14 +243,23 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
                                        mappable_end, ggtt->mappable_end);

                if (ret)
-                       goto err;
+                       goto deballoon_below_mappable;
        }

        DRM_INFO("VGT balloon successfully\n");
        return 0;

-err:
+deballoon_below_mappable:
+       ggtt->base.reserved -= bl_info.space[0].size;
+       drm_mm_remove_node(&bl_info.space[0]);
+deballoon_upon_unmappable:
+       ggtt->base.reserved -= bl_info.space[3].size;
+       drm_mm_remove_node(&bl_info.space[3]);
+deballoon_upon_mappable:
+       ggtt->base.reserved -= bl_info.space[2].size;
+       drm_mm_remove_node(&bl_info.space[2]);
+out_err:
        DRM_ERROR("VGT balloon fail\n");
-       intel_vgt_deballoon(dev_priv);
+       memset(&bl_info, 0, sizeof(bl_info));
        return ret;
 }
> 
> 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment
  2017-05-12  3:20       ` Li, Weinan Z
@ 2017-05-18  8:28         ` Joonas Lahtinen
  0 siblings, 0 replies; 10+ messages in thread
From: Joonas Lahtinen @ 2017-05-18  8:28 UTC (permalink / raw)
  To: Li, Weinan Z, intel-gfx, intel-gvt-dev

On pe, 2017-05-12 at 03:20 +0000, Li, Weinan Z wrote:
> 
> Thanks Joonas' guidance.
> I add 4 labels in intel_vgt_balloon for cleaning up ballooned space, the reserved size
> will increase when one balloon space reserve success, and will clean up if anyone reserve
> fail step by step.

Yes, there could still be vgt_deballoon_space function to further
reduce code duplication. The labels should begin with err_, like
elsewhere in in the driver.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment
  2017-05-10  2:47 Weinan Li
  2017-05-10  3:01 ` Li, Weinan Z
@ 2017-05-12  0:08 ` kbuild test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kbuild test robot @ 2017-05-12  0:08 UTC (permalink / raw)
  To: Weinan Li; +Cc: intel-gfx, intel-gvt-dev, kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1887 bytes --]

Hi Weinan,

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.11 next-20170511]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Weinan-Li/drm-i915-gvt-return-the-correct-usable-aperture-size-under-gvt-environment/20170511-170356
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/i915/i915_vgpu.c: In function 'intel_vgt_deballoon':
>> drivers/gpu//drm/i915/i915_vgpu.c:113:18: error: invalid type argument of '->' (have 'struct i915_ggtt')
       dev_priv->ggtt->base.reserved -= bl_info.space[i].size;
                     ^~

vim +113 drivers/gpu//drm/i915/i915_vgpu.c

    97	 * @dev_priv: i915 device private data
    98	 *
    99	 * This function is called to deallocate the ballooned-out graphic memory, when
   100	 * driver is unloaded or when ballooning fails.
   101	 */
   102	void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
   103	{
   104		int i;
   105	
   106		if (!intel_vgpu_active(dev_priv))
   107			return;
   108	
   109		DRM_DEBUG("VGT deballoon.\n");
   110	
   111		for (i = 0; i < 4; i++) {
   112			if (bl_info.space[i].allocated) {
 > 113				dev_priv->ggtt->base.reserved -= bl_info.space[i].size;
   114				drm_mm_remove_node(&bl_info.space[i]);
   115			}
   116		}
   117	
   118		memset(&bl_info, 0, sizeof(bl_info));
   119	}
   120	
   121	static int vgt_balloon_space(struct i915_ggtt *ggtt,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38888 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment
  2017-05-10  2:47 Weinan Li
@ 2017-05-10  3:01 ` Li, Weinan Z
  2017-05-12  0:08 ` kbuild test robot
  1 sibling, 0 replies; 10+ messages in thread
From: Li, Weinan Z @ 2017-05-10  3:01 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev

Really sorry, please ignore this mail with wrong patch. Will send the correct one then.

Thanks.
Best Regards.
Weinan, LI


> -----Original Message-----
> From: Li, Weinan Z
> Sent: Wednesday, May 10, 2017 10:48 AM
> To: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Cc: Li, Weinan Z <weinan.z.li@intel.com>; Chris Wilson <chris@chris-
> wilson.co.uk>; Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Subject: [PATCH v4] drm/i915/gvt: return the correct usable aperture size
> under gvt environment
> 
> 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.
> 
> 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.
> 
> v4: add onion teardown to balloon and deballoon to make sure the reserved
> stays correct. Code style refine.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Weinan Li <weinan.z.li@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c     | 4 ++--
>  drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
>  drivers/gpu/drm/i915/i915_vgpu.c    | 8 +++++++-
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c index 33fb11c..8d8d9c0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -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;
>  	return 0;
>  }
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
> b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index fb15684..da9aa9f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -255,6 +255,7 @@ struct i915_address_space {
>  	struct drm_i915_file_private *file;
>  	struct list_head global_link;
>  	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
> +	u64 reserved;		/* size addr space reserved */
> 
>  	bool closed;
> 
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> b/drivers/gpu/drm/i915/i915_vgpu.c
> index 4ab8a97..b144cf6 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -109,8 +109,10 @@ void intel_vgt_deballoon(struct drm_i915_private
> *dev_priv)
>  	DRM_DEBUG("VGT deballoon.\n");
> 
>  	for (i = 0; i < 4; i++) {
> -		if (bl_info.space[i].allocated)
> +		if (bl_info.space[i].allocated) {
> +			dev_priv->ggtt->base.reserved -= bl_info.space[i].size;
>  			drm_mm_remove_node(&bl_info.space[i]);
> +		}
>  	}
> 
>  	memset(&bl_info, 0, sizeof(bl_info));
> @@ -216,6 +218,7 @@ int intel_vgt_balloon(struct drm_i915_private
> *dev_priv)
> 
>  		if (ret)
>  			goto err;
> +		ggtt->base.reserved += bl_info.space[2].size;
>  	}
> 
>  	if (unmappable_end < ggtt_end) {
> @@ -223,6 +226,7 @@ int intel_vgt_balloon(struct drm_i915_private
> *dev_priv)
>  					unmappable_end, ggtt_end);
>  		if (ret)
>  			goto err;
> +		ggtt->base.reserved += bl_info.space[3].size;
>  	}
> 
>  	/* Mappable graphic memory ballooning */ @@ -232,6 +236,7 @@ int
> intel_vgt_balloon(struct drm_i915_private *dev_priv)
> 
>  		if (ret)
>  			goto err;
> +		ggtt->base.reserved += bl_info.space[0].size;
>  	}
> 
>  	if (mappable_end < ggtt->mappable_end) { @@ -240,6 +245,7 @@ int
> intel_vgt_balloon(struct drm_i915_private *dev_priv)
> 
>  		if (ret)
>  			goto err;
> +		ggtt->base.reserved += bl_info.space[1].size;
>  	}
> 
>  	DRM_INFO("VGT balloon successfully\n");
> --
> 1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment
@ 2017-05-10  2:47 Weinan Li
  2017-05-10  3:01 ` Li, Weinan Z
  2017-05-12  0:08 ` kbuild test robot
  0 siblings, 2 replies; 10+ messages in thread
From: Weinan Li @ 2017-05-10  2:47 UTC (permalink / raw)
  To: intel-gfx, intel-gvt-dev

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.

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.

v4: add onion teardown to balloon and deballoon to make sure the reserved
stays correct. Code style refine.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Weinan Li <weinan.z.li@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c     | 4 ++--
 drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
 drivers/gpu/drm/i915/i915_vgpu.c    | 8 +++++++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 33fb11c..8d8d9c0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -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;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index fb15684..da9aa9f 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -255,6 +255,7 @@ struct i915_address_space {
 	struct drm_i915_file_private *file;
 	struct list_head global_link;
 	u64 total;		/* size addr space maps (ex. 2GB for ggtt) */
+	u64 reserved;		/* size addr space reserved */
 
 	bool closed;
 
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 4ab8a97..b144cf6 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -109,8 +109,10 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
 	DRM_DEBUG("VGT deballoon.\n");
 
 	for (i = 0; i < 4; i++) {
-		if (bl_info.space[i].allocated)
+		if (bl_info.space[i].allocated) {
+			dev_priv->ggtt->base.reserved -= bl_info.space[i].size;
 			drm_mm_remove_node(&bl_info.space[i]);
+		}
 	}
 
 	memset(&bl_info, 0, sizeof(bl_info));
@@ -216,6 +218,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 
 		if (ret)
 			goto err;
+		ggtt->base.reserved += bl_info.space[2].size;
 	}
 
 	if (unmappable_end < ggtt_end) {
@@ -223,6 +226,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 					unmappable_end, ggtt_end);
 		if (ret)
 			goto err;
+		ggtt->base.reserved += bl_info.space[3].size;
 	}
 
 	/* Mappable graphic memory ballooning */
@@ -232,6 +236,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 
 		if (ret)
 			goto err;
+		ggtt->base.reserved += bl_info.space[0].size;
 	}
 
 	if (mappable_end < ggtt->mappable_end) {
@@ -240,6 +245,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 
 		if (ret)
 			goto err;
+		ggtt->base.reserved += bl_info.space[1].size;
 	}
 
 	DRM_INFO("VGT balloon successfully\n");
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-05-18  8:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-10  2:59 [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment Weinan Li
2017-05-10  3:24 ` ✓ Fi.CI.BAT: success for drm/i915/gvt: return the correct usable aperture size under gvt environment (rev2) Patchwork
2017-05-10 10:42 ` [PATCH v4] drm/i915/gvt: return the correct usable aperture size under gvt environment Joonas Lahtinen
2017-05-11  6:51   ` Li, Weinan Z
2017-05-11 12:55     ` Joonas Lahtinen
2017-05-12  3:20       ` Li, Weinan Z
2017-05-18  8:28         ` Joonas Lahtinen
  -- strict thread matches above, loose matches on Subject: below --
2017-05-10  2:47 Weinan Li
2017-05-10  3:01 ` Li, Weinan Z
2017-05-12  0:08 ` kbuild test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.