* [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.