drm/i915/guc: init submission structures as part of guc_init

Commit Message

Daniele Ceraolo Spurio July 25, 2019, 5:46 p.m. UTC

guc->stage_desc_pool is required as part of the init parameters and
there is no reason we have to init them after HuC. This fixes a NULL
ptr dereference due to guc->stage_desc_pool not being set (no fixes
tag since GuC submission can't be enabled yet).

Signed-off-by: Daniele Ceraolo Spurio <[e-mail]>
Cc: Michal Wajdeczko <[e-mail]>
Cc: Chris Wilson <[e-mail]>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c | 15 +++++++++++++++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c  | 16 ----------------
 2 files changed, 15 insertions(+), 16 deletions(-)

Comments

Michal Wajdeczko July 26, 2019, 5:11 a.m. UTC | #1

On Thu, 25 Jul 2019 19:46:55 +0200, Daniele Ceraolo Spurio  
<[e-mail]> wrote:

> guc->stage_desc_pool is required as part of the init parameters and
> there is no reason we have to init them after HuC. This fixes a NULL
> ptr dereference due to guc->stage_desc_pool not being set (no fixes
> tag since GuC submission can't be enabled yet).
>
> Signed-off-by: Daniele Ceraolo Spurio <[e-mail]>
> Cc: Michal Wajdeczko <[e-mail]>
> Cc: Chris Wilson <[e-mail]>
> ---

Reviewed-by: Michal Wajdeczko <[e-mail]>

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 1ea6a9e50c02..13fbbffd05c7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -290,6 +290,16 @@  int intel_guc_init(struct intel_guc *guc)
 	if (ret)
 		goto err_ads;
 
+	if (intel_uc_is_using_guc_submission(&gt->uc)) {
+		/*
+		 * This is stuff we need to have available at fw load time
+		 * if we are planning to enable submission later
+		 */
+		ret = intel_guc_submission_init(guc);
+		if (ret)
+			goto err_ct;
+	}
+
 	/* now that everything is perma-pinned, initialize the parameters */
 	guc_init_params(guc);
 
@@ -298,6 +308,8 @@  int intel_guc_init(struct intel_guc *guc)
 
 	return 0;
 
+err_ct:
+	intel_guc_ct_fini(&guc->ct);
 err_ads:
 	intel_guc_ads_destroy(guc);
 err_log:
@@ -317,6 +329,9 @@  void intel_guc_fini(struct intel_guc *guc)
 
 	i915_ggtt_disable_guc(gt->ggtt);
 
+	if (intel_uc_is_using_guc_submission(&gt->uc))
+		intel_guc_submission_fini(guc);
+
 	intel_guc_ct_fini(&guc->ct);
 
 	intel_guc_ads_destroy(guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index b1815abecf30..8eef85696ec2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -397,21 +397,8 @@  int intel_uc_init(struct intel_uc *uc)
 			goto err_guc;
 	}
 
-	if (intel_uc_is_using_guc_submission(uc)) {
-		/*
-		 * This is stuff we need to have available at fw load time
-		 * if we are planning to enable submission later
-		 */
-		ret = intel_guc_submission_init(guc);
-		if (ret)
-			goto err_huc;
-	}
-
 	return 0;
 
-err_huc:
-	if (intel_uc_is_using_huc(uc))
-		intel_huc_fini(huc);
 err_guc:
 	intel_guc_fini(guc);
 	return ret;
@@ -426,9 +413,6 @@  void intel_uc_fini(struct intel_uc *uc)
 
 	GEM_BUG_ON(!intel_uc_fw_supported(&guc->fw));
 
-	if (intel_uc_is_using_guc_submission(uc))
-		intel_guc_submission_fini(guc);
-
 	if (intel_uc_is_using_huc(uc))
 		intel_huc_fini(&uc->huc);