diff mbox series

[2/7] drm/i915/uc: HuC firmware can't be supported without GuC

Message ID 20190807170034.8440-3-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show
Series Hardening firmware fetch | expand

Commit Message

Michal Wajdeczko Aug. 7, 2019, 5 p.m. UTC 
There is no point in selecting HuC firmware if GuC is unsupported
or it was already disabled, as we need GuC to authenticate HuC.

While around, make uc_fw_init_early work without direct access
to whole i915, pass only needed platform/rev info.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c |  5 ++++-
 drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c |  8 +++++++-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 13 +++++++------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  |  5 +++--
 4 files changed, 21 insertions(+), 10 deletions(-)

Comments

Chris Wilson Aug. 7, 2019, 5:27 p.m. UTC | #1 
Quoting Michal Wajdeczko (2019-08-07 18:00:29)
> There is no point in selecting HuC firmware if GuC is unsupported
> or it was already disabled, as we need GuC to authenticate HuC.

Makes sense.

> While around, make uc_fw_init_early work without direct access
> to whole i915, pass only needed platform/rev info.

Hmm. When I've been briefly thinking about this, I've contemplated
passing around the *{ intel_device_info, intel_runtime_info }
 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

Looks ok, but I wouldn't be surprised if we came up with an alternative
approach to passing the feature flags around later (on a wider scale).

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Michal Wajdeczko Aug. 7, 2019, 8:12 p.m. UTC | #2 
On Wed, 07 Aug 2019 22:21:29 +0200, Kumar Valsan, Prathap  
<prathap.kumar.valsan@intel.com> wrote:

> On Wed, Aug 07, 2019 at 05:00:29PM +0000, Michal Wajdeczko wrote:
>> There is no point in selecting HuC firmware if GuC is unsupported
>> or it was already disabled, as we need GuC to authenticate HuC.
>>
>
> We are calling  intel_uc_init() irrespctive of  
> intel_uc_fetch_firmwares() is
> successful. Is this ok?

No need to change this, as intel_guc_init (and intel_huc_init)
will call intel_uc_fw_init() which will with -ENOEXEC if firmware
is not available (aka fetched)

Michal

>
> Thanks,
> Prathap
>> While around, make uc_fw_init_early work without direct access
>> to whole i915, pass only needed platform/rev info.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Kumar Valsan, Prathap Aug. 7, 2019, 8:21 p.m. UTC | #3 
On Wed, Aug 07, 2019 at 05:00:29PM +0000, Michal Wajdeczko wrote:
> There is no point in selecting HuC firmware if GuC is unsupported
> or it was already disabled, as we need GuC to authenticate HuC.
> 

We are calling  intel_uc_init() irrespctive of intel_uc_fetch_firmwares() is
successful. Is this ok?

Thanks,
Prathap
> While around, make uc_fw_init_early work without direct access
> to whole i915, pass only needed platform/rev info.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index 28735c14b9a0..11765cfb0498 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -39,7 +39,10 @@ 
  */
 void intel_guc_fw_init_early(struct intel_guc *guc)
 {
-	intel_uc_fw_init_early(&guc->fw, INTEL_UC_FW_TYPE_GUC, guc_to_gt(guc)->i915);
+	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
+
+	intel_uc_fw_init_early(&guc->fw, INTEL_UC_FW_TYPE_GUC, HAS_GT_UC(i915),
+			       INTEL_INFO(i915)->platform, INTEL_REVID(i915));
 }
 
 static void guc_prepare_xfer(struct intel_uncore *uncore)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
index 0e885859c828..88dfed837827 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc_fw.c
@@ -31,7 +31,13 @@ 
  */
 void intel_huc_fw_init_early(struct intel_huc *huc)
 {
-	intel_uc_fw_init_early(&huc->fw, INTEL_UC_FW_TYPE_HUC, huc_to_gt(huc)->i915);
+	struct intel_gt *gt = huc_to_gt(huc);
+	struct intel_uc *uc = &gt->uc;
+	struct drm_i915_private *i915 = gt->i915;
+
+	intel_uc_fw_init_early(&huc->fw, INTEL_UC_FW_TYPE_HUC,
+			       intel_uc_supports_guc(uc),
+			       INTEL_INFO(i915)->platform, INTEL_REVID(i915));
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index a3a22a26016c..00235cac84aa 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -171,16 +171,18 @@  __uc_fw_override(struct intel_uc_fw *uc_fw)
 
 /**
  * intel_uc_fw_init_early - initialize the uC object and select the firmware
- * @i915: device private
  * @uc_fw: uC firmware
  * @type: type of uC
+ * @supported: is uC support possible
+ * @platform: platform identifier
+ * @rev: hardware revision
  *
  * Initialize the state of our uC object and relevant tracking and select the
  * firmware to fetch and load.
  */
 void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
-			    enum intel_uc_fw_type type,
-			    struct drm_i915_private *i915)
+			    enum intel_uc_fw_type type, bool supported,
+			    enum intel_platform platform, u8 rev)
 {
 	/*
 	 * we use FIRMWARE_UNINITIALIZED to detect checks against uc_fw->status
@@ -192,9 +194,8 @@  void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 
 	uc_fw->type = type;
 
-	if (HAS_GT_UC(i915) && likely(!__uc_fw_override(uc_fw)))
-		__uc_fw_auto_select(uc_fw, INTEL_INFO(i915)->platform,
-				    INTEL_REVID(i915));
+	if (supported && likely(!__uc_fw_override(uc_fw)))
+		__uc_fw_auto_select(uc_fw, platform, rev);
 
 	if (uc_fw->path && *uc_fw->path)
 		uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index bfe3614613b7..7a858710d446 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -27,6 +27,7 @@ 
 
 #include <linux/types.h>
 #include "intel_uc_fw_abi.h"
+#include "intel_device_info.h"
 #include "i915_gem.h"
 
 struct drm_printer;
@@ -170,8 +171,8 @@  static inline u32 intel_uc_fw_get_upload_size(struct intel_uc_fw *uc_fw)
 }
 
 void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
-			    enum intel_uc_fw_type type,
-			    struct drm_i915_private *i915);
+			    enum intel_uc_fw_type type, bool supported,
+			    enum intel_platform platform, u8 rev);
 void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw,
 		       struct drm_i915_private *i915);
 void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw);