[PATCH 0/7] Hardening firmware fetch

[PATCH 0/7] Hardening firmware fetch

[Michal Wajdeczko]

More probe failures inside uc loading path.

Michal Wajdeczko (7):
  drm/i915/uc: Prefer dev_info for reporting options
  drm/i915/uc: HuC firmware can't be supported without GuC
  drm/i915/uc: Don't fetch HuC fw if GuC fw fetch already failed
  drm/i915: Don't try to partition WOPCM without GuC firmware
  drm/i915: Make wopcm_to_i915() private
  drm/i915/uc: WOPCM programming errors are not always real
  drm/i915/uc: Hardening firmware fetch

 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.c     |  42 +++---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 156 +++++++++++++++-------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  |  11 +-
 drivers/gpu/drm/i915/i915_drv.h           |   5 -
 drivers/gpu/drm/i915/intel_wopcm.c        |   7 +-
 7 files changed, 156 insertions(+), 78 deletions(-)

-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/7] drm/i915/uc: Prefer dev_info for reporting options

[Michal Wajdeczko]

While modparams are global for the i915 module, we are reporting
status of the params applied against specific device instance.

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_uc.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index e87b7904ab7a..3c007e0e1a20 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -59,11 +59,14 @@ static int __intel_uc_reset_hw(struct intel_uc *uc)
 
 static void __confirm_options(struct intel_uc *uc)
 {
-	DRM_DEBUG_DRIVER("enable_guc=%d (guc:%s submission:%s huc:%s)\n",
-			 i915_modparams.enable_guc,
-			 yesno(intel_uc_supports_guc(uc)),
-			 yesno(intel_uc_supports_guc_submission(uc)),
-			 yesno(intel_uc_supports_huc(uc)));
+	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
+
+	DRM_DEV_DEBUG_DRIVER(i915->drm.dev,
+			     "enable_guc=%d (guc:%s submission:%s huc:%s)\n",
+			     i915_modparams.enable_guc,
+			     yesno(intel_uc_supports_guc(uc)),
+			     yesno(intel_uc_supports_guc_submission(uc)),
+			     yesno(intel_uc_supports_huc(uc)));
 
 	if (i915_modparams.enable_guc == -1)
 		return;
@@ -76,22 +79,26 @@ static void __confirm_options(struct intel_uc *uc)
 	}
 
 	if (!intel_uc_supports_guc(uc))
-		DRM_INFO("Incompatible option enable_guc=%d - %s\n",
+		dev_info(i915->drm.dev,
+			 "Incompatible option enable_guc=%d - %s\n",
 			 i915_modparams.enable_guc, "GuC is not supported!");
 
 	if (i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC &&
 	    !intel_uc_supports_huc(uc))
-		DRM_INFO("Incompatible option enable_guc=%d - %s\n",
+		dev_info(i915->drm.dev,
+			 "Incompatible option enable_guc=%d - %s\n",
 			 i915_modparams.enable_guc, "HuC is not supported!");
 
 	if (i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION &&
 	    !intel_uc_supports_guc_submission(uc))
-		DRM_INFO("Incompatible option enable_guc=%d - %s\n",
+		dev_info(i915->drm.dev,
+			 "Incompatible option enable_guc=%d - %s\n",
 			 i915_modparams.enable_guc, "GuC submission is N/A");
 
 	if (i915_modparams.enable_guc & ~(ENABLE_GUC_SUBMISSION |
 					  ENABLE_GUC_LOAD_HUC))
-		DRM_INFO("Incompatible option enable_guc=%d - %s\n",
+		dev_info(i915->drm.dev,
+			 "Incompatible option enable_guc=%d - %s\n",
 			 i915_modparams.enable_guc, "undocumented flag");
 }
 
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Michal Wajdeczko]

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(-)

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);
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/7] drm/i915/uc: Don't fetch HuC fw if GuC fw fetch already failed

[Michal Wajdeczko]

When we failed to fetch GuC firmware there is no point in fetching
HuC firmware as we will not be able to use it without working GuC.

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_uc.c    | 5 ++++-
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 8 +++++---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 3 +--
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 3c007e0e1a20..c40eab290342 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -283,11 +283,14 @@ static void guc_disable_communication(struct intel_guc *guc)
 void intel_uc_fetch_firmwares(struct intel_uc *uc)
 {
 	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
+	int err;
 
 	if (!intel_uc_supports_guc(uc))
 		return;
 
-	intel_uc_fw_fetch(&uc->guc.fw, i915);
+	err = intel_uc_fw_fetch(&uc->guc.fw, i915);
+	if (err)
+		return;
 
 	if (intel_uc_supports_huc(uc))
 		intel_uc_fw_fetch(&uc->huc.fw, 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 00235cac84aa..3a3803bfa5a8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -205,13 +205,14 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 
 /**
  * intel_uc_fw_fetch - fetch uC firmware
- *
  * @uc_fw: uC firmware
  * @i915: device private
  *
  * Fetch uC firmware into GEM obj.
+ *
+ * Return: 0 on success, a negative errno code on failure.
  */
-void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
+int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 {
 	struct drm_i915_gem_object *obj;
 	const struct firmware *fw = NULL;
@@ -322,7 +323,7 @@ void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	uc_fw->status = INTEL_UC_FIRMWARE_AVAILABLE;
 
 	release_firmware(fw);
-	return;
+	return 0;
 
 fail:
 	uc_fw->status = INTEL_UC_FIRMWARE_MISSING;
@@ -333,6 +334,7 @@ void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 		 intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL);
 
 	release_firmware(fw);		/* OK even if fw is NULL */
+	return err;
 }
 
 static u32 uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, struct i915_ggtt *ggtt)
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 7a858710d446..fae45bc16bc7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -173,8 +173,7 @@ 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, bool supported,
 			    enum intel_platform platform, u8 rev);
-void intel_uc_fw_fetch(struct intel_uc_fw *uc_fw,
-		       struct drm_i915_private *i915);
+int 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);
 int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, struct intel_gt *gt,
 		       u32 wopcm_offset, u32 dma_flags);
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/7] drm/i915: Don't try to partition WOPCM without GuC firmware

[Michal Wajdeczko]

For meaningful WOPCM partitioning we need GuC (and optionally HuC)
firmware size(s) and we shouldn't just rely on GuC support flag,
as we might fail to fetch GuC firmware and it's size will be 0
and all calculations will be just wrong/useless.

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/intel_wopcm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index 4c22143ee84f..5e5c3fd3472d 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -170,7 +170,7 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
 	u32 guc_wopcm_rsvd;
 	int err;
 
-	if (!USES_GUC(i915))
+	if (!guc_fw_size)
 		return;
 
 	GEM_BUG_ON(!wopcm->size);
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/7] drm/i915: Make wopcm_to_i915() private

[Michal Wajdeczko]

No need to define it globally as we're only using it in wopcm.c

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/i915_drv.h    | 5 -----
 drivers/gpu/drm/i915/intel_wopcm.c | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c9476f24f5c1..d516e9099d15 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1907,11 +1907,6 @@ static inline struct drm_i915_private *pdev_to_i915(struct pci_dev *pdev)
 	return pci_get_drvdata(pdev);
 }
 
-static inline struct drm_i915_private *wopcm_to_i915(struct intel_wopcm *wopcm)
-{
-	return container_of(wopcm, struct drm_i915_private, wopcm);
-}
-
 /* Simple iterator over all initialised engines */
 #define for_each_engine(engine__, dev_priv__, id__) \
 	for ((id__) = 0; \
diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index 5e5c3fd3472d..2bda24200498 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -64,6 +64,11 @@
 #define GEN9_GUC_FW_RESERVED	SZ_128K
 #define GEN9_GUC_WOPCM_OFFSET	(GUC_WOPCM_RESERVED + GEN9_GUC_FW_RESERVED)
 
+static inline struct drm_i915_private *wopcm_to_i915(struct intel_wopcm *wopcm)
+{
+	return container_of(wopcm, struct drm_i915_private, wopcm);
+}
+
 /**
  * intel_wopcm_init_early() - Early initialization of the WOPCM.
  * @wopcm: pointer to intel_wopcm.
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/7] drm/i915/uc: WOPCM programming errors are not always real

[Michal Wajdeczko]

WOPCM programming error might be due to inserted earlier probe
failure that could affects HuC firmware loading and thus impacts
result of WOPCM partitioning that would be now incompatible with
previously programmed values.

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_uc.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index c40eab290342..32aa4509ba1d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -415,11 +415,13 @@ static int uc_init_wopcm(struct intel_uc *uc)
 	return 0;
 
 err_out:
-	DRM_ERROR("Failed to init uC WOPCM registers:\n");
-	DRM_ERROR("DMA_GUC_WOPCM_OFFSET=%#x\n",
-		  intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
-	DRM_ERROR("GUC_WOPCM_SIZE=%#x\n",
-		  intel_uncore_read(uncore, GUC_WOPCM_SIZE));
+	i915_probe_error(gt->i915, "Failed to init uC WOPCM registers!\n");
+	i915_probe_error(gt->i915, "%s(%#x)=%#x\n", "DMA_GUC_WOPCM_OFFSET",
+			 i915_mmio_reg_offset(DMA_GUC_WOPCM_OFFSET),
+			 intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET));
+	i915_probe_error(gt->i915, "%s(%#x)=%#x\n", "GUC_WOPCM_SIZE",
+			 i915_mmio_reg_offset(GUC_WOPCM_SIZE),
+			 intel_uncore_read(uncore, GUC_WOPCM_SIZE));
 
 	return err;
 }
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/7] drm/i915/uc: Hardening firmware fetch

[Michal Wajdeczko]

Insert few more failure points into firmware fetch procedure to check
use of the wrong blob name or use of the mismatched firmware versions.

Also update some messages (remove ptr, duplicated infos) and stop
treating all fetch errors as missing firmware case.

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_uc_fw.c | 137 ++++++++++++++++-------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h |   3 +
 2 files changed, 97 insertions(+), 43 deletions(-)

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 3a3803bfa5a8..4faff1010398 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -153,20 +153,23 @@ static const char *__override_huc_firmware_path(void)
 	return "";
 }
 
-static bool
-__uc_fw_override(struct intel_uc_fw *uc_fw)
+static void __uc_fw_user_override(struct intel_uc_fw *uc_fw)
 {
+	const char *path = NULL;
+
 	switch (uc_fw->type) {
 	case INTEL_UC_FW_TYPE_GUC:
-		uc_fw->path = __override_guc_firmware_path();
+		path = __override_guc_firmware_path();
 		break;
 	case INTEL_UC_FW_TYPE_HUC:
-		uc_fw->path = __override_huc_firmware_path();
+		path = __override_huc_firmware_path();
 		break;
 	}
 
-	uc_fw->user_overridden = uc_fw->path;
-	return uc_fw->user_overridden;
+	if (unlikely(path)) {
+		uc_fw->path = path;
+		uc_fw->user_overridden = true;
+	}
 }
 
 /**
@@ -194,8 +197,10 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 
 	uc_fw->type = type;
 
-	if (supported && likely(!__uc_fw_override(uc_fw)))
+	if (supported) {
 		__uc_fw_auto_select(uc_fw, platform, rev);
+		__uc_fw_user_override(uc_fw);
+	}
 
 	if (uc_fw->path && *uc_fw->path)
 		uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
@@ -203,6 +208,41 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 		uc_fw->status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
 }
 
+static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw,
+				      struct drm_i915_private *i915, bool user)
+{
+	int e = user ? -EINVAL : -ESTALE;
+
+	if (i915_inject_load_error(i915, e)) {
+		/* non-existing blob */
+		uc_fw->path = "<invalid>";
+		uc_fw->user_overridden = user;
+	} else if (i915_inject_load_error(i915, e)) {
+		/* require next major version */
+		uc_fw->major_ver_wanted += 1;
+		uc_fw->minor_ver_wanted = 0;
+		uc_fw->user_overridden = user;
+	} else if (i915_inject_load_error(i915, e)) {
+		/* require next minor version */
+		uc_fw->minor_ver_wanted += 1;
+		uc_fw->user_overridden = user;
+	} else if (uc_fw->major_ver_wanted && i915_inject_load_error(i915, e)) {
+		/* require prev major version */
+		uc_fw->major_ver_wanted -= 1;
+		uc_fw->minor_ver_wanted = 0;
+		uc_fw->user_overridden = user;
+	} else if (uc_fw->minor_ver_wanted && i915_inject_load_error(i915, e)) {
+		/* require prev minor version - hey, this should work! */
+		uc_fw->minor_ver_wanted -= 1;
+		uc_fw->user_overridden = user;
+	} else if (user && i915_inject_load_error(i915, e)) {
+		/* officially unsupported platform */
+		uc_fw->major_ver_wanted = 0;
+		uc_fw->minor_ver_wanted = 0;
+		uc_fw->user_overridden = true;
+	}
+}
+
 /**
  * intel_uc_fw_fetch - fetch uC firmware
  * @uc_fw: uC firmware
@@ -222,17 +262,22 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 
 	GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
 
+	err = i915_inject_load_error(i915, -ENXIO);
+	if (err)
+		return err;
+
+	__force_fw_fetch_failures(uc_fw, i915, true);
+	__force_fw_fetch_failures(uc_fw, i915, false);
+
 	err = request_firmware(&fw, uc_fw->path, i915->drm.dev);
 	if (err)
 		goto fail;
 
-	DRM_DEBUG_DRIVER("%s fw size %zu ptr %p\n",
-			 intel_uc_fw_type_repr(uc_fw->type), fw->size, fw);
-
 	/* Check the size of the blob before examining buffer contents */
-	if (fw->size < sizeof(struct uc_css_header)) {
-		DRM_WARN("%s: Unexpected firmware size (%zu, min %zu)\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
+	if (unlikely(fw->size < sizeof(struct uc_css_header))) {
+		dev_warn(i915->drm.dev,
+			 "%s firmware %s: invalid size: %zu < %zu\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
 			 fw->size, sizeof(struct uc_css_header));
 		err = -ENODATA;
 		goto fail;
@@ -243,10 +288,12 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	/* Check integrity of size values inside CSS header */
 	size = (css->header_size_dw - css->key_size_dw - css->modulus_size_dw -
 		css->exponent_size_dw) * sizeof(u32);
-	if (size != sizeof(struct uc_css_header)) {
-		DRM_WARN("%s: Mismatched firmware header definition\n",
-			 intel_uc_fw_type_repr(uc_fw->type));
-		err = -ENOEXEC;
+	if (unlikely(size != sizeof(struct uc_css_header))) {
+		dev_warn(i915->drm.dev,
+			 "%s firmware %s: unexpected header size: %zu != %zu\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			 fw->size, sizeof(struct uc_css_header));
+		err = -EPROTO;
 		goto fail;
 	}
 
@@ -254,19 +301,23 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
 
 	/* now RSA */
-	if (css->key_size_dw != UOS_RSA_SCRATCH_COUNT) {
-		DRM_WARN("%s: Mismatched firmware RSA key size (%u)\n",
-			 intel_uc_fw_type_repr(uc_fw->type), css->key_size_dw);
-		err = -ENOEXEC;
+	if (unlikely(css->key_size_dw != UOS_RSA_SCRATCH_COUNT)) {
+		dev_warn(i915->drm.dev,
+			 "%s firmware %s: unexpected RSA key size: %u != %u\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			 css->key_size_dw, UOS_RSA_SCRATCH_COUNT);
+		err = -EPROTO;
 		goto fail;
 	}
 	uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
 
 	/* At least, it should have header, uCode and RSA. Size of all three. */
 	size = sizeof(struct uc_css_header) + uc_fw->ucode_size + uc_fw->rsa_size;
-	if (fw->size < size) {
-		DRM_WARN("%s: Truncated firmware (%zu, expected %zu)\n",
-			 intel_uc_fw_type_repr(uc_fw->type), fw->size, size);
+	if (unlikely(fw->size < size)) {
+		dev_warn(i915->drm.dev,
+			 "%s firmware %s: invalid size: %zu < %zu\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			 fw->size, size);
 		err = -ENOEXEC;
 		goto fail;
 	}
@@ -292,29 +343,22 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 		break;
 	}
 
-	DRM_DEBUG_DRIVER("%s fw version %u.%u (wanted %u.%u)\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
-			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-
-	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
-		DRM_NOTE("%s: Skipping firmware version check\n",
-			 intel_uc_fw_type_repr(uc_fw->type));
-	} else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
-		   uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-		DRM_NOTE("%s: Wrong firmware version (%u.%u, required %u.%u)\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
+	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
+	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
+		dev_warn(i915->drm.dev,
+			 "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
 			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
 			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-		err = -ENOEXEC;
-		goto fail;
+		if (!intel_uc_fw_is_overridden(uc_fw)) {
+			err = -ENOEXEC;
+			goto fail;
+		}
 	}
 
 	obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
 	if (IS_ERR(obj)) {
 		err = PTR_ERR(obj);
-		DRM_DEBUG_DRIVER("%s fw object_create err=%d\n",
-				 intel_uc_fw_type_repr(uc_fw->type), err);
 		goto fail;
 	}
 
@@ -322,15 +366,22 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	uc_fw->size = fw->size;
 	uc_fw->status = INTEL_UC_FIRMWARE_AVAILABLE;
 
+	DRM_DEV_DEBUG_DRIVER(i915->drm.dev, "%s firmware %s: %s\n",
+			     intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			     intel_uc_fw_status_repr(uc_fw->status));
+
 	release_firmware(fw);
 	return 0;
 
 fail:
-	uc_fw->status = INTEL_UC_FIRMWARE_MISSING;
+	if (err == -ENOENT)
+		uc_fw->status = INTEL_UC_FIRMWARE_MISSING;
+	else
+		uc_fw->status = INTEL_UC_FIRMWARE_ERROR;
 
-	DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
+	dev_info(i915->drm.dev, "%s firmware %s: fetch failed with error %d\n",
 		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
-	DRM_INFO("%s: Firmware can be downloaded from %s\n",
+	dev_info(i915->drm.dev, "%s firmware(s) can be downloaded from %s\n",
 		 intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL);
 
 	release_firmware(fw);		/* OK even if fw is NULL */
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 fae45bc16bc7..0d22e73dff15 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -42,6 +42,7 @@ enum intel_uc_fw_status {
 	INTEL_UC_FIRMWARE_UNINITIALIZED = 0, /* used to catch checks done too early */
 	INTEL_UC_FIRMWARE_SELECTED, /* selected the blob we want to load */
 	INTEL_UC_FIRMWARE_MISSING, /* blob not found on the system */
+	INTEL_UC_FIRMWARE_ERROR, /* invalid format or version */
 	INTEL_UC_FIRMWARE_AVAILABLE, /* blob found and copied in mem */
 	INTEL_UC_FIRMWARE_FAIL, /* failed to xfer or init/auth the fw */
 	INTEL_UC_FIRMWARE_TRANSFERRED, /* dma xfer done */
@@ -92,6 +93,8 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 		return "SELECTED";
 	case INTEL_UC_FIRMWARE_MISSING:
 		return "MISSING";
+	case INTEL_UC_FIRMWARE_ERROR:
+		return "ERROR";
 	case INTEL_UC_FIRMWARE_AVAILABLE:
 		return "AVAILABLE";
 	case INTEL_UC_FIRMWARE_FAIL:
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915/uc: Prefer dev_info for reporting options

[Chris Wilson]

Quoting Michal Wajdeczko (2019-08-07 18:00:28)
> While modparams are global for the i915 module, we are reporting
> status of the params applied against specific device instance.
> 
> 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_uc.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index e87b7904ab7a..3c007e0e1a20 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -59,11 +59,14 @@ static int __intel_uc_reset_hw(struct intel_uc *uc)
>  
>  static void __confirm_options(struct intel_uc *uc)
>  {
> -       DRM_DEBUG_DRIVER("enable_guc=%d (guc:%s submission:%s huc:%s)\n",
> -                        i915_modparams.enable_guc,
> -                        yesno(intel_uc_supports_guc(uc)),
> -                        yesno(intel_uc_supports_guc_submission(uc)),
> -                        yesno(intel_uc_supports_huc(uc)));
> +       struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> +
> +       DRM_DEV_DEBUG_DRIVER(i915->drm.dev,
> +                            "enable_guc=%d (guc:%s submission:%s huc:%s)\n",
> +                            i915_modparams.enable_guc,
> +                            yesno(intel_uc_supports_guc(uc)),
> +                            yesno(intel_uc_supports_guc_submission(uc)),
> +                            yesno(intel_uc_supports_huc(uc)));

DDD_DRIVER()? :)

>         if (i915_modparams.enable_guc == -1)
>                 return;
> @@ -76,22 +79,26 @@ static void __confirm_options(struct intel_uc *uc)
>         }
>  
>         if (!intel_uc_supports_guc(uc))
> -               DRM_INFO("Incompatible option enable_guc=%d - %s\n",
> +               dev_info(i915->drm.dev,
> +                        "Incompatible option enable_guc=%d - %s\n",
>                          i915_modparams.enable_guc, "GuC is not supported!");
>  
>         if (i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC &&
>             !intel_uc_supports_huc(uc))
> -               DRM_INFO("Incompatible option enable_guc=%d - %s\n",
> +               dev_info(i915->drm.dev,
> +                        "Incompatible option enable_guc=%d - %s\n",
>                          i915_modparams.enable_guc, "HuC is not supported!");
>  
>         if (i915_modparams.enable_guc & ENABLE_GUC_SUBMISSION &&
>             !intel_uc_supports_guc_submission(uc))
> -               DRM_INFO("Incompatible option enable_guc=%d - %s\n",
> +               dev_info(i915->drm.dev,
> +                        "Incompatible option enable_guc=%d - %s\n",
>                          i915_modparams.enable_guc, "GuC submission is N/A");
>  
>         if (i915_modparams.enable_guc & ~(ENABLE_GUC_SUBMISSION |
>                                           ENABLE_GUC_LOAD_HUC))
> -               DRM_INFO("Incompatible option enable_guc=%d - %s\n",
> +               dev_info(i915->drm.dev,
> +                        "Incompatible option enable_guc=%d - %s\n",
>                          i915_modparams.enable_guc, "undocumented flag");

Yes, "confirm_options" fits the device model, as opposed to the old
sanitize that adjusted the global parameter.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915/uc: Don't fetch HuC fw if GuC fw fetch already failed

[Chris Wilson]

Quoting Michal Wajdeczko (2019-08-07 18:00:30)
> When we failed to fetch GuC firmware there is no point in fetching
> HuC firmware as we will not be able to use it without working GuC.
> 
> 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_uc.c    | 5 ++++-
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 8 +++++---
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 3 +--
>  3 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 3c007e0e1a20..c40eab290342 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -283,11 +283,14 @@ static void guc_disable_communication(struct intel_guc *guc)
>  void intel_uc_fetch_firmwares(struct intel_uc *uc)
>  {
>         struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
> +       int err;
>  
>         if (!intel_uc_supports_guc(uc))
>                 return;
>  
> -       intel_uc_fw_fetch(&uc->guc.fw, i915);
> +       err = intel_uc_fw_fetch(&uc->guc.fw, i915);
> +       if (err)
> +               return;

We still don't care about the err, just that it exists.

if (intel_uc_fw_fetch() return; ?

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915: Don't try to partition WOPCM without GuC firmware

[Chris Wilson]

Quoting Michal Wajdeczko (2019-08-07 18:00:31)
> For meaningful WOPCM partitioning we need GuC (and optionally HuC)
> firmware size(s) and we shouldn't just rely on GuC support flag,
> as we might fail to fetch GuC firmware and it's size will be 0
> and all calculations will be just wrong/useless.
> 
> 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/intel_wopcm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
> index 4c22143ee84f..5e5c3fd3472d 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -170,7 +170,7 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
>         u32 guc_wopcm_rsvd;
>         int err;
>  
> -       if (!USES_GUC(i915))
> +       if (!guc_fw_size)
>                 return;

It looks like the calculations with huc_fw_size=0 are fine, just hope
the fw agrees.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/7] drm/i915: Make wopcm_to_i915() private

[Chris Wilson]

Quoting Michal Wajdeczko (2019-08-07 18:00:32)
> No need to define it globally as we're only using it in wopcm.c
> 
> 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>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915/uc: WOPCM programming errors are not always real

[Chris Wilson]

Quoting Michal Wajdeczko (2019-08-07 18:00:33)
> WOPCM programming error might be due to inserted earlier probe
> failure that could affects HuC firmware loading and thus impacts
> result of WOPCM partitioning that would be now incompatible with
> previously programmed values.
> 
> 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>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Look, I made no comments regarding whether the errors were real anyway.
Oops.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/uc: Hardening firmware fetch

[Chris Wilson]

Quoting Michal Wajdeczko (2019-08-07 18:00:34)
> Insert few more failure points into firmware fetch procedure to check
> use of the wrong blob name or use of the mismatched firmware versions.
> 
> Also update some messages (remove ptr, duplicated infos) and stop
> treating all fetch errors as missing firmware case.
> 
> 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_uc_fw.c | 137 ++++++++++++++++-------
>  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h |   3 +
>  2 files changed, 97 insertions(+), 43 deletions(-)
> 
> 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 3a3803bfa5a8..4faff1010398 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -153,20 +153,23 @@ static const char *__override_huc_firmware_path(void)
>         return "";
>  }
>  
> -static bool
> -__uc_fw_override(struct intel_uc_fw *uc_fw)
> +static void __uc_fw_user_override(struct intel_uc_fw *uc_fw)
>  {
> +       const char *path = NULL;
> +
>         switch (uc_fw->type) {
>         case INTEL_UC_FW_TYPE_GUC:
> -               uc_fw->path = __override_guc_firmware_path();
> +               path = __override_guc_firmware_path();
>                 break;
>         case INTEL_UC_FW_TYPE_HUC:
> -               uc_fw->path = __override_huc_firmware_path();
> +               path = __override_huc_firmware_path();
>                 break;
>         }
>  
> -       uc_fw->user_overridden = uc_fw->path;
> -       return uc_fw->user_overridden;
> +       if (unlikely(path)) {
> +               uc_fw->path = path;
> +               uc_fw->user_overridden = true;
> +       }

The code got longer for no real improvement in clarity?
Oh, because now uc_fw->path may be non-NULL on entry due to call
reordering.

>  }
>  
>  /**
> @@ -194,8 +197,10 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
>  
>         uc_fw->type = type;
>  
> -       if (supported && likely(!__uc_fw_override(uc_fw)))
> +       if (supported) {
>                 __uc_fw_auto_select(uc_fw, platform, rev);
> +               __uc_fw_user_override(uc_fw);
> +       }
>  
>         if (uc_fw->path && *uc_fw->path)
>                 uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
> @@ -203,6 +208,41 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
>                 uc_fw->status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
>  }
>  
> +static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw,
> +                                     struct drm_i915_private *i915, bool user)
> +{
> +       int e = user ? -EINVAL : -ESTALE;
> +
> +       if (i915_inject_load_error(i915, e)) {
> +               /* non-existing blob */
> +               uc_fw->path = "<invalid>";
> +               uc_fw->user_overridden = user;
> +       } else if (i915_inject_load_error(i915, e)) {
> +               /* require next major version */
> +               uc_fw->major_ver_wanted += 1;
> +               uc_fw->minor_ver_wanted = 0;
> +               uc_fw->user_overridden = user;
> +       } else if (i915_inject_load_error(i915, e)) {
> +               /* require next minor version */
> +               uc_fw->minor_ver_wanted += 1;
> +               uc_fw->user_overridden = user;
> +       } else if (uc_fw->major_ver_wanted && i915_inject_load_error(i915, e)) {
> +               /* require prev major version */
> +               uc_fw->major_ver_wanted -= 1;
> +               uc_fw->minor_ver_wanted = 0;
> +               uc_fw->user_overridden = user;
> +       } else if (uc_fw->minor_ver_wanted && i915_inject_load_error(i915, e)) {
> +               /* require prev minor version - hey, this should work! */
> +               uc_fw->minor_ver_wanted -= 1;
> +               uc_fw->user_overridden = user;
> +       } else if (user && i915_inject_load_error(i915, e)) {
> +               /* officially unsupported platform */
> +               uc_fw->major_ver_wanted = 0;
> +               uc_fw->minor_ver_wanted = 0;
> +               uc_fw->user_overridden = true;
> +       }

Taking self-abuse to a whole new level :)

> +}
> +
>  /**
>   * intel_uc_fw_fetch - fetch uC firmware
>   * @uc_fw: uC firmware
> @@ -222,17 +262,22 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
>  
>         GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
>  
> +       err = i915_inject_load_error(i915, -ENXIO);
> +       if (err)
> +               return err;
> +
> +       __force_fw_fetch_failures(uc_fw, i915, true);
> +       __force_fw_fetch_failures(uc_fw, i915, false);

Ok. The set of major/minor/user_override make some sort of sense to me,
but I couldn't say if that's a complete set or not.

> @@ -292,29 +343,22 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
>                 break;
>         }
>  
> -       DRM_DEBUG_DRIVER("%s fw version %u.%u (wanted %u.%u)\n",
> -                        intel_uc_fw_type_repr(uc_fw->type),
> -                        uc_fw->major_ver_found, uc_fw->minor_ver_found,
> -                        uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
> -
> -       if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
> -               DRM_NOTE("%s: Skipping firmware version check\n",
> -                        intel_uc_fw_type_repr(uc_fw->type));
> -       } else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> -                  uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> -               DRM_NOTE("%s: Wrong firmware version (%u.%u, required %u.%u)\n",
> -                        intel_uc_fw_type_repr(uc_fw->type),
> +       if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
> +           uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
> +               dev_warn(i915->drm.dev,

Still a dev_notice(), especially as this is still user controllable.

> +                        "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
> +                        intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
>                          uc_fw->major_ver_found, uc_fw->minor_ver_found,
>                          uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
> -               err = -ENOEXEC;
> -               goto fail;
> +               if (!intel_uc_fw_is_overridden(uc_fw)) {
> +                       err = -ENOEXEC;
> +                       goto fail;
> +               }
>         }

> -       DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
> +       dev_info(i915->drm.dev, "%s firmware %s: fetch failed with error %d\n",
>                  intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);

WARN -> info ? Not at least a notice?

> -       DRM_INFO("%s: Firmware can be downloaded from %s\n",
> +       dev_info(i915->drm.dev, "%s firmware(s) can be downloaded from %s\n",
>                  intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL);
>  
>         release_firmware(fw);           /* OK even if fw is NULL */

Just a question of log levels.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for Hardening firmware fetch

[Patchwork]

== Series Details ==

Series: Hardening firmware fetch
URL   : https://patchwork.freedesktop.org/series/64856/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6649 -> Patchwork_13903
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13903 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13903, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13903:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload:
    - fi-skl-6260u:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-skl-6260u/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-skl-6260u/igt@i915_module_load@reload.html

  
Known issues
------------

  Here are the changes found in Patchwork_13903 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_basic@bad-close:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-icl-u3/igt@gem_basic@bad-close.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-icl-u3/igt@gem_basic@bad-close.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [PASS][5] -> [INCOMPLETE][6] ([fdo#107718])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_selftest@live_reset:
    - fi-icl-u2:          [PASS][7] -> [INCOMPLETE][8] ([fdo#107713])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-icl-u2/igt@i915_selftest@live_reset.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-icl-u2/igt@i915_selftest@live_reset.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       [PASS][9] -> [WARN][10] ([fdo#109380])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [PASS][11] -> [FAIL][12] ([fdo#103167])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-c:
    - fi-kbl-7567u:       [PASS][13] -> [SKIP][14] ([fdo#109271]) +23 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-cml-u:           [INCOMPLETE][15] ([fdo#110566]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-cml-u/igt@gem_ctx_create@basic-files.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-cml-u/igt@gem_ctx_create@basic-files.html

  * igt@gem_tiled_blits@basic:
    - fi-icl-u3:          [DMESG-WARN][17] ([fdo#107724]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-icl-u3/igt@gem_tiled_blits@basic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-icl-u3/igt@gem_tiled_blits@basic.html

  * igt@prime_vgem@basic-wait-default:
    - {fi-icl-dsi}:       [DMESG-WARN][19] ([fdo#106107]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-icl-dsi/igt@prime_vgem@basic-wait-default.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/fi-icl-dsi/igt@prime_vgem@basic-wait-default.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566


Participating hosts (55 -> 46)
------------------------------

  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6649 -> Patchwork_13903

  CI-20190529: 20190529
  CI_DRM_6649: 47b76298b3b6a5240d37f4e3f0182a2d47069d42 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5125: 35d81d01b1599b4bc4df0e09e25f6f531eed4f8a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13903: 16753ab0a94d7cc9d4f6d135c275b8e5fa72a54a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

16753ab0a94d drm/i915/uc: Hardening firmware fetch
db89f65ff7f6 drm/i915/uc: WOPCM programming errors are not always real
2b00681bed79 drm/i915: Make wopcm_to_i915() private
29a7a0e5a233 drm/i915: Don't try to partition WOPCM without GuC firmware
db1dd662941e drm/i915/uc: Don't fetch HuC fw if GuC fw fetch already failed
6d6f260831f4 drm/i915/uc: HuC firmware can't be supported without GuC
05e3959f9690 drm/i915/uc: Prefer dev_info for reporting options

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13903/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/7] drm/i915/uc: Hardening firmware fetch

[Michal Wajdeczko]

Insert few more failure points into firmware fetch procedure to check
use of the wrong blob name or use of the mismatched firmware versions.

Also update some messages (remove ptr, duplicated infos) and stop
treating all fetch errors as missing firmware case.

v2: update log levels (Chris)

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_uc_fw.c | 142 +++++++++++++++--------
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h |   3 +
 2 files changed, 98 insertions(+), 47 deletions(-)

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 3a3803bfa5a8..a8007d622f57 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -153,20 +153,23 @@ static const char *__override_huc_firmware_path(void)
 	return "";
 }
 
-static bool
-__uc_fw_override(struct intel_uc_fw *uc_fw)
+static void __uc_fw_user_override(struct intel_uc_fw *uc_fw)
 {
+	const char *path = NULL;
+
 	switch (uc_fw->type) {
 	case INTEL_UC_FW_TYPE_GUC:
-		uc_fw->path = __override_guc_firmware_path();
+		path = __override_guc_firmware_path();
 		break;
 	case INTEL_UC_FW_TYPE_HUC:
-		uc_fw->path = __override_huc_firmware_path();
+		path = __override_huc_firmware_path();
 		break;
 	}
 
-	uc_fw->user_overridden = uc_fw->path;
-	return uc_fw->user_overridden;
+	if (unlikely(path)) {
+		uc_fw->path = path;
+		uc_fw->user_overridden = true;
+	}
 }
 
 /**
@@ -194,8 +197,10 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 
 	uc_fw->type = type;
 
-	if (supported && likely(!__uc_fw_override(uc_fw)))
+	if (supported) {
 		__uc_fw_auto_select(uc_fw, platform, rev);
+		__uc_fw_user_override(uc_fw);
+	}
 
 	if (uc_fw->path && *uc_fw->path)
 		uc_fw->status = INTEL_UC_FIRMWARE_SELECTED;
@@ -203,6 +208,41 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
 		uc_fw->status = INTEL_UC_FIRMWARE_NOT_SUPPORTED;
 }
 
+static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw,
+				      struct drm_i915_private *i915, bool user)
+{
+	int e = user ? -EINVAL : -ESTALE;
+
+	if (i915_inject_load_error(i915, e)) {
+		/* non-existing blob */
+		uc_fw->path = "<invalid>";
+		uc_fw->user_overridden = user;
+	} else if (i915_inject_load_error(i915, e)) {
+		/* require next major version */
+		uc_fw->major_ver_wanted += 1;
+		uc_fw->minor_ver_wanted = 0;
+		uc_fw->user_overridden = user;
+	} else if (i915_inject_load_error(i915, e)) {
+		/* require next minor version */
+		uc_fw->minor_ver_wanted += 1;
+		uc_fw->user_overridden = user;
+	} else if (uc_fw->major_ver_wanted && i915_inject_load_error(i915, e)) {
+		/* require prev major version */
+		uc_fw->major_ver_wanted -= 1;
+		uc_fw->minor_ver_wanted = 0;
+		uc_fw->user_overridden = user;
+	} else if (uc_fw->minor_ver_wanted && i915_inject_load_error(i915, e)) {
+		/* require prev minor version - hey, this should work! */
+		uc_fw->minor_ver_wanted -= 1;
+		uc_fw->user_overridden = user;
+	} else if (user && i915_inject_load_error(i915, e)) {
+		/* officially unsupported platform */
+		uc_fw->major_ver_wanted = 0;
+		uc_fw->minor_ver_wanted = 0;
+		uc_fw->user_overridden = true;
+	}
+}
+
 /**
  * intel_uc_fw_fetch - fetch uC firmware
  * @uc_fw: uC firmware
@@ -214,6 +254,7 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
  */
 int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 {
+	struct device *dev = i915->drm.dev;
 	struct drm_i915_gem_object *obj;
 	const struct firmware *fw = NULL;
 	struct uc_css_header *css;
@@ -222,17 +263,21 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 
 	GEM_BUG_ON(!intel_uc_fw_supported(uc_fw));
 
-	err = request_firmware(&fw, uc_fw->path, i915->drm.dev);
+	err = i915_inject_load_error(i915, -ENXIO);
 	if (err)
-		goto fail;
+		return err;
 
-	DRM_DEBUG_DRIVER("%s fw size %zu ptr %p\n",
-			 intel_uc_fw_type_repr(uc_fw->type), fw->size, fw);
+	__force_fw_fetch_failures(uc_fw, i915, true);
+	__force_fw_fetch_failures(uc_fw, i915, false);
+
+	err = request_firmware(&fw, uc_fw->path, dev);
+	if (err)
+		goto fail;
 
 	/* Check the size of the blob before examining buffer contents */
-	if (fw->size < sizeof(struct uc_css_header)) {
-		DRM_WARN("%s: Unexpected firmware size (%zu, min %zu)\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
+	if (unlikely(fw->size < sizeof(struct uc_css_header))) {
+		dev_warn(dev, "%s firmware %s: invalid size: %zu < %zu\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
 			 fw->size, sizeof(struct uc_css_header));
 		err = -ENODATA;
 		goto fail;
@@ -243,10 +288,12 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	/* Check integrity of size values inside CSS header */
 	size = (css->header_size_dw - css->key_size_dw - css->modulus_size_dw -
 		css->exponent_size_dw) * sizeof(u32);
-	if (size != sizeof(struct uc_css_header)) {
-		DRM_WARN("%s: Mismatched firmware header definition\n",
-			 intel_uc_fw_type_repr(uc_fw->type));
-		err = -ENOEXEC;
+	if (unlikely(size != sizeof(struct uc_css_header))) {
+		dev_warn(dev,
+			 "%s firmware %s: unexpected header size: %zu != %zu\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			 fw->size, sizeof(struct uc_css_header));
+		err = -EPROTO;
 		goto fail;
 	}
 
@@ -254,19 +301,21 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
 
 	/* now RSA */
-	if (css->key_size_dw != UOS_RSA_SCRATCH_COUNT) {
-		DRM_WARN("%s: Mismatched firmware RSA key size (%u)\n",
-			 intel_uc_fw_type_repr(uc_fw->type), css->key_size_dw);
-		err = -ENOEXEC;
+	if (unlikely(css->key_size_dw != UOS_RSA_SCRATCH_COUNT)) {
+		dev_warn(dev, "%s firmware %s: unexpected key size: %u != %u\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			 css->key_size_dw, UOS_RSA_SCRATCH_COUNT);
+		err = -EPROTO;
 		goto fail;
 	}
 	uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
 
 	/* At least, it should have header, uCode and RSA. Size of all three. */
 	size = sizeof(struct uc_css_header) + uc_fw->ucode_size + uc_fw->rsa_size;
-	if (fw->size < size) {
-		DRM_WARN("%s: Truncated firmware (%zu, expected %zu)\n",
-			 intel_uc_fw_type_repr(uc_fw->type), fw->size, size);
+	if (unlikely(fw->size < size)) {
+		dev_warn(dev, "%s firmware %s: invalid size: %zu < %zu\n",
+			 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			 fw->size, size);
 		err = -ENOEXEC;
 		goto fail;
 	}
@@ -292,29 +341,21 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 		break;
 	}
 
-	DRM_DEBUG_DRIVER("%s fw version %u.%u (wanted %u.%u)\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
-			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-
-	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
-		DRM_NOTE("%s: Skipping firmware version check\n",
-			 intel_uc_fw_type_repr(uc_fw->type));
-	} else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
-		   uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-		DRM_NOTE("%s: Wrong firmware version (%u.%u, required %u.%u)\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
-			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-		err = -ENOEXEC;
-		goto fail;
+	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
+	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
+		dev_notice(dev, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
+			   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			   uc_fw->major_ver_found, uc_fw->minor_ver_found,
+			   uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
+		if (!intel_uc_fw_is_overridden(uc_fw)) {
+			err = -ENOEXEC;
+			goto fail;
+		}
 	}
 
 	obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
 	if (IS_ERR(obj)) {
 		err = PTR_ERR(obj);
-		DRM_DEBUG_DRIVER("%s fw object_create err=%d\n",
-				 intel_uc_fw_type_repr(uc_fw->type), err);
 		goto fail;
 	}
 
@@ -322,15 +363,22 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
 	uc_fw->size = fw->size;
 	uc_fw->status = INTEL_UC_FIRMWARE_AVAILABLE;
 
+	DRM_DEV_DEBUG_DRIVER(dev, "%s firmware %s: %s\n",
+			     intel_uc_fw_type_repr(uc_fw->type), uc_fw->path,
+			     intel_uc_fw_status_repr(uc_fw->status));
+
 	release_firmware(fw);
 	return 0;
 
 fail:
-	uc_fw->status = INTEL_UC_FIRMWARE_MISSING;
+	if (err == -ENOENT)
+		uc_fw->status = INTEL_UC_FIRMWARE_MISSING;
+	else
+		uc_fw->status = INTEL_UC_FIRMWARE_ERROR;
 
-	DRM_WARN("%s: Failed to fetch firmware %s (error %d)\n",
-		 intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
-	DRM_INFO("%s: Firmware can be downloaded from %s\n",
+	dev_notice(dev, "%s firmware %s: fetch failed with error %d\n",
+		   intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err);
+	dev_info(dev, "%s firmware(s) can be downloaded from %s\n",
 		 intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL);
 
 	release_firmware(fw);		/* OK even if fw is NULL */
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 fae45bc16bc7..0d22e73dff15 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -42,6 +42,7 @@ enum intel_uc_fw_status {
 	INTEL_UC_FIRMWARE_UNINITIALIZED = 0, /* used to catch checks done too early */
 	INTEL_UC_FIRMWARE_SELECTED, /* selected the blob we want to load */
 	INTEL_UC_FIRMWARE_MISSING, /* blob not found on the system */
+	INTEL_UC_FIRMWARE_ERROR, /* invalid format or version */
 	INTEL_UC_FIRMWARE_AVAILABLE, /* blob found and copied in mem */
 	INTEL_UC_FIRMWARE_FAIL, /* failed to xfer or init/auth the fw */
 	INTEL_UC_FIRMWARE_TRANSFERRED, /* dma xfer done */
@@ -92,6 +93,8 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 		return "SELECTED";
 	case INTEL_UC_FIRMWARE_MISSING:
 		return "MISSING";
+	case INTEL_UC_FIRMWARE_ERROR:
+		return "ERROR";
 	case INTEL_UC_FIRMWARE_AVAILABLE:
 		return "AVAILABLE";
 	case INTEL_UC_FIRMWARE_FAIL:
-- 
2.19.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/uc: Hardening firmware fetch

[Chris Wilson]

Quoting Michal Wajdeczko (2019-08-07 19:37:59)
> Insert few more failure points into firmware fetch procedure to check
> use of the wrong blob name or use of the mismatched firmware versions.
> 
> Also update some messages (remove ptr, duplicated infos) and stop
> treating all fetch errors as missing firmware case.
> 
> v2: update log levels (Chris)
> 
> 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>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for Hardening firmware fetch (rev2)

[Patchwork]

== Series Details ==

Series: Hardening firmware fetch (rev2)
URL   : https://patchwork.freedesktop.org/series/64856/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6649 -> Patchwork_13907
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/

Known issues
------------

  Here are the changes found in Patchwork_13907 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [PASS][1] -> [INCOMPLETE][2] ([fdo#107718])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_module_load@reload:
    - fi-skl-6600u:       [PASS][3] -> [INCOMPLETE][4] ([fdo#104108])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-skl-6600u/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-skl-6600u/igt@i915_module_load@reload.html

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-snb-2600:        [PASS][5] -> [DMESG-WARN][6] ([fdo#110684] / [fdo#111115])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-snb-2600/igt@i915_module_load@reload-with-fault-injection.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-snb-2600/igt@i915_module_load@reload-with-fault-injection.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       [PASS][7] -> [WARN][8] ([fdo#109380])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-kbl-7567u/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [PASS][9] -> [FAIL][10] ([fdo#103167])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-c:
    - fi-kbl-7567u:       [PASS][11] -> [SKIP][12] ([fdo#109271]) +23 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-kbl-7567u/igt@kms_pipe_crc_basic@read-crc-pipe-c.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-kbl-7500u:       [PASS][13] -> [SKIP][14] ([fdo#109271]) +23 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-kbl-7500u/igt@prime_vgem@basic-fence-flip.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-kbl-7500u/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-write:
    - fi-icl-u3:          [PASS][15] -> [DMESG-WARN][16] ([fdo#107724])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-icl-u3/igt@prime_vgem@basic-write.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-icl-u3/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-cml-u:           [INCOMPLETE][17] ([fdo#110566]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-cml-u/igt@gem_ctx_create@basic-files.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-cml-u/igt@gem_ctx_create@basic-files.html

  * igt@gem_tiled_blits@basic:
    - fi-icl-u3:          [DMESG-WARN][19] ([fdo#107724]) -> [PASS][20] +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-icl-u3/igt@gem_tiled_blits@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-icl-u3/igt@gem_tiled_blits@basic.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][21] ([fdo#109485]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@prime_vgem@basic-wait-default:
    - {fi-icl-dsi}:       [DMESG-WARN][23] ([fdo#106107]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/fi-icl-dsi/igt@prime_vgem@basic-wait-default.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/fi-icl-dsi/igt@prime_vgem@basic-wait-default.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#106350]: https://bugs.freedesktop.org/show_bug.cgi?id=106350
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
  [fdo#110684]: https://bugs.freedesktop.org/show_bug.cgi?id=110684
  [fdo#111115]: https://bugs.freedesktop.org/show_bug.cgi?id=111115


Participating hosts (55 -> 47)
------------------------------

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6649 -> Patchwork_13907

  CI-20190529: 20190529
  CI_DRM_6649: 47b76298b3b6a5240d37f4e3f0182a2d47069d42 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5125: 35d81d01b1599b4bc4df0e09e25f6f531eed4f8a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13907: faa6e663714ea7512ff43176c092f5c8d1ade52a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

faa6e663714e drm/i915/uc: Hardening firmware fetch
e6110e42a76e drm/i915/uc: WOPCM programming errors are not always real
93eaeb159c70 drm/i915: Make wopcm_to_i915() private
a44689eb2699 drm/i915: Don't try to partition WOPCM without GuC firmware
2b177eace14e drm/i915/uc: Don't fetch HuC fw if GuC fw fetch already failed
c808091061d7 drm/i915/uc: HuC firmware can't be supported without GuC
ae51148746d6 drm/i915/uc: Prefer dev_info for reporting options

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Michal Wajdeczko]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.BAT: success for Hardening firmware fetch (rev2)

[Chris Wilson]

Quoting Patchwork (2019-08-07 20:42:47)
> == Series Details ==
> 
> Series: Hardening firmware fetch (rev2)
> URL   : https://patchwork.freedesktop.org/series/64856/
> State : success
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_6649 -> Patchwork_13907
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.

None found, but still some worrying warnings for me. Oh well, pushed with
one minor edit for a compile warning,

 static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw,
-                                     struct drm_i915_private *i915, bool user)
+                                     struct drm_i915_private *i915,
+                                     int e)
 {
-       int e = user ? -EINVAL : -ESTALE;
+       bool user = e == -EINVAL;

        if (i915_inject_load_error(i915, e)) {
                /* non-existing blob */
@@ -267,8 +268,8 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw, struct drm_i915_private *i915)
        if (err)
                return err;

-       __force_fw_fetch_failures(uc_fw, i915, true);
-       __force_fw_fetch_failures(uc_fw, i915, false);
+       __force_fw_fetch_failures(uc_fw, i915, -EINVAL);
+       __force_fw_fetch_failures(uc_fw, i915, -ESTALE);
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Kumar Valsan, Prathap]

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>
________________________________________
> 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

✗ Fi.CI.IGT: failure for Hardening firmware fetch (rev2)

[Patchwork]

== Series Details ==

Series: Hardening firmware fetch (rev2)
URL   : https://patchwork.freedesktop.org/series/64856/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6649_full -> Patchwork_13907_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13907_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13907_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13907_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_schedule@pi-ringfull-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb5/igt@gem_exec_schedule@pi-ringfull-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb2/igt@gem_exec_schedule@pi-ringfull-bsd.html

  
Known issues
------------

  Here are the changes found in Patchwork_13907_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@preempt-other-bsd2:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#109276]) +7 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb4/igt@gem_exec_schedule@preempt-other-bsd2.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb3/igt@gem_exec_schedule@preempt-other-bsd2.html

  * igt@gem_exec_schedule@preemptive-hang-bsd:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#111325]) +2 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb5/igt@gem_exec_schedule@preemptive-hang-bsd.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb2/igt@gem_exec_schedule@preemptive-hang-bsd.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-apl4/igt@i915_suspend@fence-restore-tiled2untiled.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-apl1/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_flip@dpms-vs-vblank-race:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([fdo#103060])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-glk9/igt@kms_flip@dpms-vs-vblank-race.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-glk6/igt@kms_flip@dpms-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103167]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-mmap-gtt:
    - shard-hsw:          [PASS][13] -> [SKIP][14] ([fdo#109271])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-hsw4/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-mmap-gtt.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-hsw6/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-shrfb-draw-mmap-gtt.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-kbl:          [PASS][15] -> [DMESG-WARN][16] ([fdo#108566]) +4 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-kbl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_psr@psr2_primary_blt:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109441]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb2/igt@kms_psr@psr2_primary_blt.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb1/igt@kms_psr@psr2_primary_blt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][19] -> [FAIL][20] ([fdo#99912])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-apl6/igt@kms_setmode@basic.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-apl8/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-skl:          [PASS][21] -> [INCOMPLETE][22] ([fdo#104108])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-skl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-skl9/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-iclb:         [PASS][23] -> [INCOMPLETE][24] ([fdo#107713]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb4/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb3/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  * igt@perf@blocking:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#110728])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-skl7/igt@perf@blocking.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-skl4/igt@perf@blocking.html

  
#### Possible fixes ####

  * igt@gem_exec_schedule@deep-bsd:
    - shard-iclb:         [INCOMPLETE][27] ([fdo#107713]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb7/igt@gem_exec_schedule@deep-bsd.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb8/igt@gem_exec_schedule@deep-bsd.html

  * igt@gem_exec_schedule@wide-bsd:
    - shard-iclb:         [SKIP][29] ([fdo#111325]) -> [PASS][30] +2 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb1/igt@gem_exec_schedule@wide-bsd.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb8/igt@gem_exec_schedule@wide-bsd.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-kbl:          [DMESG-WARN][31] ([fdo#108686]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-kbl2/igt@gem_tiled_swapping@non-threaded.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-kbl2/igt@gem_tiled_swapping@non-threaded.html

  * igt@i915_pm_rps@reset:
    - shard-kbl:          [FAIL][33] ([fdo#102250]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-kbl4/igt@i915_pm_rps@reset.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-kbl7/igt@i915_pm_rps@reset.html
    - shard-glk:          [FAIL][35] ([fdo#102250]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-glk6/igt@i915_pm_rps@reset.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-glk2/igt@i915_pm_rps@reset.html

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-kbl:          [FAIL][37] -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-kbl4/igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-kbl1/igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x256-random:
    - shard-apl:          [INCOMPLETE][39] ([fdo#103927]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-apl5/igt@kms_cursor_crc@pipe-a-cursor-256x256-random.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-apl4/igt@kms_cursor_crc@pipe-a-cursor-256x256-random.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-skl:          [INCOMPLETE][41] ([fdo#110741]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-skl1/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-skl3/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * igt@kms_flip@2x-modeset-vs-vblank-race-interruptible:
    - shard-glk:          [FAIL][43] ([fdo#103060]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-glk1/igt@kms_flip@2x-modeset-vs-vblank-race-interruptible.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-glk5/igt@kms_flip@2x-modeset-vs-vblank-race-interruptible.html

  * igt@kms_flip@plain-flip-fb-recreate:
    - shard-skl:          [FAIL][45] ([fdo#100368]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-skl6/igt@kms_flip@plain-flip-fb-recreate.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-skl3/igt@kms_flip@plain-flip-fb-recreate.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         [FAIL][47] ([fdo#103167]) -> [PASS][48] +4 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@psr-rgb565-draw-mmap-wc:
    - shard-skl:          [FAIL][49] ([fdo#103167]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-skl6/igt@kms_frontbuffer_tracking@psr-rgb565-draw-mmap-wc.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-skl10/igt@kms_frontbuffer_tracking@psr-rgb565-draw-mmap-wc.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-skl:          [INCOMPLETE][51] ([fdo#104108]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-skl3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-skl5/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][53] ([fdo#108566]) -> [PASS][54] +1 similar issue
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-apl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [FAIL][55] ([fdo#108145]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][57] ([fdo#108145] / [fdo#110403]) -> [PASS][58] +1 similar issue
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][59] ([fdo#109441]) -> [PASS][60] +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb5/igt@kms_psr@psr2_sprite_plane_move.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][61] ([fdo#108566]) -> [PASS][62] +4 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-kbl1/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@prime_vgem@wait-bsd2:
    - shard-iclb:         [SKIP][63] ([fdo#109276]) -> [PASS][64] +9 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb5/igt@prime_vgem@wait-bsd2.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb2/igt@prime_vgem@wait-bsd2.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-isolation-bsd2:
    - shard-iclb:         [SKIP][65] ([fdo#109276]) -> [FAIL][66] ([fdo#111330])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6649/shard-iclb6/igt@gem_mocs_settings@mocs-isolation-bsd2.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/shard-iclb2/igt@gem_mocs_settings@mocs-isolation-bsd2.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102250]: https://bugs.freedesktop.org/show_bug.cgi?id=102250
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110741]: https://bugs.freedesktop.org/show_bug.cgi?id=110741
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6649 -> Patchwork_13907

  CI-20190529: 20190529
  CI_DRM_6649: 47b76298b3b6a5240d37f4e3f0182a2d47069d42 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5125: 35d81d01b1599b4bc4df0e09e25f6f531eed4f8a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13907: faa6e663714ea7512ff43176c092f5c8d1ade52a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13907/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx