drm/i915: Fix GVT balloon fail path handling

Submitted by Zhenyu Wang on June 10, 2019, 9:28 a.m.

Details

Message ID 20190610092819.11371-1-zhenyuw@linux.intel.com
State New
Headers show
Series "drm/i915: Fix GVT balloon fail path handling" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Zhenyu Wang June 10, 2019, 9:28 a.m.
For any reason if GVT balloon failed, deballoon would be called.
This adds a simple mask to check validity of balloon spaces. When
failure happens, that mask is used to track for deballoon, so it
won't cause any invalid space reference in fail path, which fixed
kernel oops when balloon error happened.

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_vgpu.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 94d3992b599d..5aec34db1aaa 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -94,6 +94,7 @@  struct _balloon_info_ {
 	 * graphic memory, 2/3 for unmappable graphic memory.
 	 */
 	struct drm_mm_node space[4];
+	u8 valid;
 };
 
 static struct _balloon_info_ bl_info;
@@ -126,8 +127,12 @@  void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
 
 	DRM_DEBUG("VGT deballoon.\n");
 
-	for (i = 0; i < 4; i++)
-		vgt_deballoon_space(&dev_priv->ggtt, &bl_info.space[i]);
+	for (i = 0; i < 4; i++) {
+		if (bl_info.valid & BIT(i)) {
+			vgt_deballoon_space(&dev_priv->ggtt, &bl_info.space[i]);
+			bl_info.valid &= ~(u8)BIT(i);
+		}
+	}
 }
 
 static int vgt_balloon_space(struct i915_ggtt *ggtt,
@@ -232,16 +237,17 @@  int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 	if (unmappable_base > ggtt->mappable_end) {
 		ret = vgt_balloon_space(ggtt, &bl_info.space[2],
 					ggtt->mappable_end, unmappable_base);
-
 		if (ret)
 			goto err;
+		bl_info.valid |= BIT(2);
 	}
 
 	if (unmappable_end < ggtt_end) {
 		ret = vgt_balloon_space(ggtt, &bl_info.space[3],
 					unmappable_end, ggtt_end);
 		if (ret)
-			goto err_upon_mappable;
+			goto err;
+		bl_info.valid |= BIT(3);
 	}
 
 	/* Mappable graphic memory ballooning */
@@ -250,7 +256,8 @@  int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 					0, mappable_base);
 
 		if (ret)
-			goto err_upon_unmappable;
+			goto err;
+		bl_info.valid |= BIT(0);
 	}
 
 	if (mappable_end < ggtt->mappable_end) {
@@ -258,19 +265,15 @@  int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 					mappable_end, ggtt->mappable_end);
 
 		if (ret)
-			goto err_below_mappable;
+			goto err;
+		bl_info.valid |= BIT(1);
 	}
 
 	DRM_INFO("VGT balloon successfully\n");
 	return 0;
 
-err_below_mappable:
-	vgt_deballoon_space(ggtt, &bl_info.space[0]);
-err_upon_unmappable:
-	vgt_deballoon_space(ggtt, &bl_info.space[3]);
-err_upon_mappable:
-	vgt_deballoon_space(ggtt, &bl_info.space[2]);
 err:
+	intel_vgt_deballoon(dev_priv);
 	DRM_ERROR("VGT balloon fail\n");
 	return ret;
 }

Comments

Quoting Zhenyu Wang (2019-06-10 10:28:19)
> For any reason if GVT balloon failed, deballoon would be called.
> This adds a simple mask to check validity of balloon spaces. When
> failure happens, that mask is used to track for deballoon, so it
> won't cause any invalid space reference in fail path, which fixed
> kernel oops when balloon error happened.

Against the upstream, it uses an onion unwind. Only those nodes
successfully reserved are removed.

> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_vgpu.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index 94d3992b599d..5aec34db1aaa 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -94,6 +94,7 @@ struct _balloon_info_ {
>          * graphic memory, 2/3 for unmappable graphic memory.
>          */
>         struct drm_mm_node space[4];
> +       u8 valid;
>  };
>  
>  static struct _balloon_info_ bl_info;
> @@ -126,8 +127,12 @@ void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
>  
>         DRM_DEBUG("VGT deballoon.\n");
>  
> -       for (i = 0; i < 4; i++)
> -               vgt_deballoon_space(&dev_priv->ggtt, &bl_info.space[i]);
> +       for (i = 0; i < 4; i++) {
> +               if (bl_info.valid & BIT(i)) {
> +                       vgt_deballoon_space(&dev_priv->ggtt, &bl_info.space[i]);
> +                       bl_info.valid &= ~(u8)BIT(i);

Is the problem that you are calling it twice? As far I can see we only
call it from i915_ggtt_cleanup_hw... Oh, note that i915_gem_init_ggtt()
fails to unwind correctly, and intel_vgt_deballoon is called from the
wrong point.

Fwiw, you could just use
	if (!drm_mm_node_alloced(node))
		return;

-Chris