* [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@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org > -----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@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org 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@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org 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@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org 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
* [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
* 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@lists.freedesktop.org, intel-gvt-dev@lists.freedesktop.org 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
* 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox