Re: [Intel-gfx] [PATCH] drm/i915: Don't deballoon unused ggtt drm_mm_node in linux guest
On 2018.03.30 07:01:02 +, Zhang, Xiong Y wrote: > > + Zhi and Zhenyu > > > > Quoting Xiong Zhang (2018-03-29 13:58:41) > > > Four drm_mm_node are used to reserve guest ggtt space, but some of > > > them may aren't initialized and used in intel_vgt_balloon(), so these > > > unused drm_mm_node couldn't be removed through > > drm_mm_remove_node(). > > > > I'm not sure how this slipped by in previous review, but is there an > > explanation why we have; > > > > static struct _balloon_info_ bl_info; > > > > ... and are not even initializing it? > > > > This should definitely find it's way into dev_priv and be properly > > initialized. > [Zhang, Xiong Y] how about move bl_info into struct i915_virtual_gpu ? so > that we could get it through dev_priv->vgpu.bl_info > yep, seems fine to me. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't deballoon unused ggtt drm_mm_node in linux guest
> + Zhi and Zhenyu
>
> Quoting Xiong Zhang (2018-03-29 13:58:41)
> > Four drm_mm_node are used to reserve guest ggtt space, but some of
> > them may aren't initialized and used in intel_vgt_balloon(), so these
> > unused drm_mm_node couldn't be removed through
> drm_mm_remove_node().
>
> I'm not sure how this slipped by in previous review, but is there an
> explanation why we have;
>
> static struct _balloon_info_ bl_info;
>
> ... and are not even initializing it?
>
> This should definitely find it's way into dev_priv and be properly
> initialized.
[Zhang, Xiong Y] how about move bl_info into struct i915_virtual_gpu ? so that
we could get it through dev_priv->vgpu.bl_info
thanks
>
> Regards, Joonas
>
> >
> > Fixes: ff8f797557c7("drm/i915: return the correct usable aperture size
> > under gvt environment")
> > Signed-off-by: Xiong Zhang
> > ---
> > drivers/gpu/drm/i915/i915_vgpu.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> > b/drivers/gpu/drm/i915/i915_vgpu.c
> > index 5fe9f3f..7545686 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.c
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> > @@ -100,6 +100,9 @@ static struct _balloon_info_ bl_info; static void
> > vgt_deballoon_space(struct i915_ggtt *ggtt,
> > struct drm_mm_node *node) {
> > + if (!node->allocated)
> > + return;
> > +
> > DRM_DEBUG_DRIVER("deballoon space: range [0x%llx -
> 0x%llx] %llu KiB.\n",
> > node->start,
> > node->start + node->size,
> > --
> > 2.7.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> ___
> intel-gvt-dev mailing list
> intel-gvt-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't deballoon unused ggtt drm_mm_node in linux guest
+ Zhi and Zhenyu
Quoting Xiong Zhang (2018-03-29 13:58:41)
> Four drm_mm_node are used to reserve guest ggtt space, but some of them
> may aren't initialized and used in intel_vgt_balloon(), so these unused
> drm_mm_node couldn't be removed through drm_mm_remove_node().
I'm not sure how this slipped by in previous review, but is there an
explanation why we have;
static struct _balloon_info_ bl_info;
... and are not even initializing it?
This should definitely find it's way into dev_priv and be properly
initialized.
Regards, Joonas
>
> Fixes: ff8f797557c7("drm/i915: return the correct usable aperture size under
> gvt environment")
> Signed-off-by: Xiong Zhang
> ---
> drivers/gpu/drm/i915/i915_vgpu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> b/drivers/gpu/drm/i915/i915_vgpu.c
> index 5fe9f3f..7545686 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -100,6 +100,9 @@ static struct _balloon_info_ bl_info;
> static void vgt_deballoon_space(struct i915_ggtt *ggtt,
> struct drm_mm_node *node)
> {
> + if (!node->allocated)
> + return;
> +
> DRM_DEBUG_DRIVER("deballoon space: range [0x%llx - 0x%llx] %llu
> KiB.\n",
> node->start,
> node->start + node->size,
> --
> 2.7.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't deballoon unused ggtt drm_mm_node in linux guest
> Quoting Xiong Zhang (2018-03-29 11:58:41)
> > Four drm_mm_node are used to reserve guest ggtt space, but some of
> > them may aren't initialized and used in intel_vgt_balloon(), so these
> > unused
>
> may be skipped and not initialised due to space constraints,
[Zhang, Xiong Y] OK, I will apply it. thanks
>
> > drm_mm_node couldn't be removed through drm_mm_remove_node().
> >
> > Fixes: ff8f797557c7("drm/i915: return the correct usable aperture size
> > under gvt environment")
> > Signed-off-by: Xiong Zhang
>
> Had to read through and work out what the problem was; whether it was a
> bug elsewhere in vgpu or deliberate, so amending the commit msg to be
> clear would be helpful.
[Zhang, Xiong Y] bl_info.space[i] is initialized only on specific condition in
intel_vgt_balloon(). When guest i915 driver is unloaded, intel_vgt_deballon()
go through all four bl_info.space[3:0] and call drm_mm_remove_node() for each.
If one isn't initialized, warning and reference null pointer occur in
drm_mm_remove_node().
I will update commit message. Thanks.
>
> Reviewed-by: Chris Wilson
>
> Who actually consumes bl_info? It just looks like being a balloon being set
> adrift.
[Zhang, Xiong Y] bl_info is internal static variable, only
intel_vgt_deballoon() consume it at driver unload, it deballoon reserved ggtt
space.
> -Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't deballoon unused ggtt drm_mm_node in linux guest
Quoting Xiong Zhang (2018-03-29 11:58:41)
> Four drm_mm_node are used to reserve guest ggtt space, but some of them
> may aren't initialized and used in intel_vgt_balloon(), so these unused
may be skipped and not initialised due to space constraints,
> drm_mm_node couldn't be removed through drm_mm_remove_node().
>
> Fixes: ff8f797557c7("drm/i915: return the correct usable aperture size under
> gvt environment")
> Signed-off-by: Xiong Zhang
Had to read through and work out what the problem was; whether it was
a bug elsewhere in vgpu or deliberate, so amending the commit msg to be
clear would be helpful.
Reviewed-by: Chris Wilson
Who actually consumes bl_info? It just looks like being a balloon being
set adrift.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Don't deballoon unused ggtt drm_mm_node in linux guest
Four drm_mm_node are used to reserve guest ggtt space, but some of them
may aren't initialized and used in intel_vgt_balloon(), so these unused
drm_mm_node couldn't be removed through drm_mm_remove_node().
Fixes: ff8f797557c7("drm/i915: return the correct usable aperture size under
gvt environment")
Signed-off-by: Xiong Zhang
---
drivers/gpu/drm/i915/i915_vgpu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 5fe9f3f..7545686 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -100,6 +100,9 @@ static struct _balloon_info_ bl_info;
static void vgt_deballoon_space(struct i915_ggtt *ggtt,
struct drm_mm_node *node)
{
+ if (!node->allocated)
+ return;
+
DRM_DEBUG_DRIVER("deballoon space: range [0x%llx - 0x%llx] %llu KiB.\n",
node->start,
node->start + node->size,
--
2.7.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
