* [Intel-gfx] [PATCH 1/6] drm/i915/guc: Decode another GuC load failure case
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
@ 2023-04-21 1:15 ` John.C.Harrison
2023-04-21 1:15 ` [Intel-gfx] [PATCH 2/6] drm/i915/guc: Print status register when waiting for GuC to load John.C.Harrison
` (14 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: John.C.Harrison @ 2023-04-21 1:15 UTC (permalink / raw)
To: Intel-GFX; +Cc: DRI-Devel
From: John Harrison <John.C.Harrison@Intel.com>
Explain another potential firmware failure mode and early exit the
long wait if hit.
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
index bcb1129b36102..dabeaf4f245f3 100644
--- a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h
@@ -44,6 +44,7 @@ enum intel_guc_load_status {
enum intel_bootrom_load_status {
INTEL_BOOTROM_STATUS_NO_KEY_FOUND = 0x13,
INTEL_BOOTROM_STATUS_AES_PROD_KEY_FOUND = 0x1A,
+ INTEL_BOOTROM_STATUS_PROD_KEY_CHECK_FAILURE = 0x2B,
INTEL_BOOTROM_STATUS_RSA_FAILED = 0x50,
INTEL_BOOTROM_STATUS_PAVPC_FAILED = 0x73,
INTEL_BOOTROM_STATUS_WOPCM_FAILED = 0x74,
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 6fda3aec5c66a..0ff088a5e51a8 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -129,6 +129,7 @@ static inline bool guc_load_done(struct intel_uncore *uncore, u32 *status, bool
case INTEL_BOOTROM_STATUS_RC6CTXCONFIG_FAILED:
case INTEL_BOOTROM_STATUS_MPUMAP_INCORRECT:
case INTEL_BOOTROM_STATUS_EXCEPTION:
+ case INTEL_BOOTROM_STATUS_PROD_KEY_CHECK_FAILURE:
*success = false;
return true;
}
@@ -219,6 +220,11 @@ static int guc_wait_ucode(struct intel_guc *guc)
guc_info(guc, "firmware signature verification failed\n");
ret = -ENOEXEC;
break;
+
+ case INTEL_BOOTROM_STATUS_PROD_KEY_CHECK_FAILURE:
+ guc_info(guc, "firmware production part check failure\n");
+ ret = -ENOEXEC;
+ break;
}
switch (ukernel) {
--
2.39.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Intel-gfx] [PATCH 2/6] drm/i915/guc: Print status register when waiting for GuC to load
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
2023-04-21 1:15 ` [Intel-gfx] [PATCH 1/6] drm/i915/guc: Decode another GuC load failure case John.C.Harrison
@ 2023-04-21 1:15 ` John.C.Harrison
2023-04-21 1:15 ` [Intel-gfx] [PATCH 3/6] drm/i915/uc: Track patch level versions on reduced version firmware files John.C.Harrison
` (13 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: John.C.Harrison @ 2023-04-21 1:15 UTC (permalink / raw)
To: Intel-GFX; +Cc: DRI-Devel
From: John Harrison <John.C.Harrison@Intel.com>
If the GuC load is taking an excessively long time, the wait loop
currently prints the GT frequency. Extend that to include the GuC
status as well so we can see if the GuC is actually making progress or
not.
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 6 ++++--
1 file changed, 4 insertions(+), 2 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 0ff088a5e51a8..364d0d546ec82 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -191,8 +191,10 @@ static int guc_wait_ucode(struct intel_guc *guc)
if (!ret || !success)
break;
- guc_dbg(guc, "load still in progress, count = %d, freq = %dMHz\n",
- count, intel_rps_read_actual_frequency(&uncore->gt->rps));
+ guc_dbg(guc, "load still in progress, count = %d, freq = %dMHz, status = 0x%08X [0x%02X/%02X]\n",
+ count, intel_rps_read_actual_frequency(&uncore->gt->rps), status,
+ REG_FIELD_GET(GS_BOOTROM_MASK, status),
+ REG_FIELD_GET(GS_UKERNEL_MASK, status));
}
after = ktime_get();
delta = ktime_sub(after, before);
--
2.39.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [Intel-gfx] [PATCH 3/6] drm/i915/uc: Track patch level versions on reduced version firmware files
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
2023-04-21 1:15 ` [Intel-gfx] [PATCH 1/6] drm/i915/guc: Decode another GuC load failure case John.C.Harrison
2023-04-21 1:15 ` [Intel-gfx] [PATCH 2/6] drm/i915/guc: Print status register when waiting for GuC to load John.C.Harrison
@ 2023-04-21 1:15 ` John.C.Harrison
2023-04-28 23:53 ` Ceraolo Spurio, Daniele
2023-04-21 1:15 ` [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation John.C.Harrison
` (12 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: John.C.Harrison @ 2023-04-21 1:15 UTC (permalink / raw)
To: Intel-GFX; +Cc: DRI-Devel
From: John Harrison <John.C.Harrison@Intel.com>
When reduced version firmware files were added (matching major
component being the only strict requirement), the minor version was
still tracked and a notification reported if it was older. However,
the patch version should really be tracked as well for the same
reasons. The KMD can work without the change but if the effort has
been taken to release a new firmware with the change then there must
be a valid reason for doing so - important bug fix, security fix, etc.
And in that case it would be good to alert the user if they are
missing out on that new fix.
v2: Use correct patch version number and drop redunant debug print
(review by Daniele / CI results).
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 30 +++++++++++++++---------
1 file changed, 19 insertions(+), 11 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 a82a53dbbc86d..c9cd9bb47577f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -80,14 +80,14 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
*/
#define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \
fw_def(METEORLAKE, 0, guc_mmp(mtl, 70, 6, 5)) \
- fw_def(DG2, 0, guc_maj(dg2, 70, 5)) \
- fw_def(ALDERLAKE_P, 0, guc_maj(adlp, 70, 5)) \
+ fw_def(DG2, 0, guc_maj(dg2, 70, 5, 1)) \
+ fw_def(ALDERLAKE_P, 0, guc_maj(adlp, 70, 5, 1)) \
fw_def(ALDERLAKE_P, 0, guc_mmp(adlp, 70, 1, 1)) \
fw_def(ALDERLAKE_P, 0, guc_mmp(adlp, 69, 0, 3)) \
- fw_def(ALDERLAKE_S, 0, guc_maj(tgl, 70, 5)) \
+ fw_def(ALDERLAKE_S, 0, guc_maj(tgl, 70, 5, 1)) \
fw_def(ALDERLAKE_S, 0, guc_mmp(tgl, 70, 1, 1)) \
fw_def(ALDERLAKE_S, 0, guc_mmp(tgl, 69, 0, 3)) \
- fw_def(DG1, 0, guc_maj(dg1, 70, 5)) \
+ fw_def(DG1, 0, guc_maj(dg1, 70, 5, 1)) \
fw_def(ROCKETLAKE, 0, guc_mmp(tgl, 70, 1, 1)) \
fw_def(TIGERLAKE, 0, guc_mmp(tgl, 70, 1, 1)) \
fw_def(JASPERLAKE, 0, guc_mmp(ehl, 70, 1, 1)) \
@@ -141,7 +141,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
__stringify(patch_) ".bin"
/* Minor for internal driver use, not part of file name */
-#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_) \
+#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_) \
__MAKE_UC_FW_PATH_MAJOR(prefix_, "guc", major_)
#define MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \
@@ -197,9 +197,9 @@ struct __packed uc_fw_blob {
{ UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
.legacy = true }
-#define GUC_FW_BLOB(prefix_, major_, minor_) \
- UC_FW_BLOB_NEW(major_, minor_, 0, false, \
- MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_))
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
#define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \
UC_FW_BLOB_OLD(major_, minor_, patch_, \
@@ -296,6 +296,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
uc_fw->file_wanted.path = blob->path;
uc_fw->file_wanted.ver.major = blob->major;
uc_fw->file_wanted.ver.minor = blob->minor;
+ uc_fw->file_wanted.ver.patch = blob->patch;
uc_fw->loaded_via_gsc = blob->loaded_via_gsc;
found = true;
break;
@@ -790,6 +791,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
} else {
if (uc_fw->file_selected.ver.minor < uc_fw->file_wanted.ver.minor)
old_ver = true;
+ else if ((uc_fw->file_selected.ver.minor == uc_fw->file_wanted.ver.minor) &&
+ (uc_fw->file_selected.ver.patch < uc_fw->file_wanted.ver.patch))
+ old_ver = true;
}
}
@@ -797,12 +801,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
/* Preserve the version that was really wanted */
memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted));
- gt_notice(gt, "%s firmware %s (%d.%d) is recommended, but only %s (%d.%d) was found\n",
+ gt_notice(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n",
intel_uc_fw_type_repr(uc_fw->type),
uc_fw->file_wanted.path,
- uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor,
+ uc_fw->file_wanted.ver.major,
+ uc_fw->file_wanted.ver.minor,
+ uc_fw->file_wanted.ver.patch,
uc_fw->file_selected.path,
- uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor);
+ uc_fw->file_selected.ver.major,
+ uc_fw->file_selected.ver.minor,
+ uc_fw->file_selected.ver.patch);
gt_info(gt, "Consider updating your linux-firmware pkg or downloading from %s\n",
INTEL_UC_FIRMWARE_URL);
}
--
2.39.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 3/6] drm/i915/uc: Track patch level versions on reduced version firmware files
2023-04-21 1:15 ` [Intel-gfx] [PATCH 3/6] drm/i915/uc: Track patch level versions on reduced version firmware files John.C.Harrison
@ 2023-04-28 23:53 ` Ceraolo Spurio, Daniele
0 siblings, 0 replies; 27+ messages in thread
From: Ceraolo Spurio, Daniele @ 2023-04-28 23:53 UTC (permalink / raw)
To: John.C.Harrison, Intel-GFX; +Cc: DRI-Devel
On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> When reduced version firmware files were added (matching major
> component being the only strict requirement), the minor version was
> still tracked and a notification reported if it was older. However,
> the patch version should really be tracked as well for the same
> reasons. The KMD can work without the change but if the effort has
> been taken to release a new firmware with the change then there must
> be a valid reason for doing so - important bug fix, security fix, etc.
> And in that case it would be good to alert the user if they are
> missing out on that new fix.
>
> v2: Use correct patch version number and drop redunant debug print
> (review by Daniele / CI results).
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Daniele
> ---
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 30 +++++++++++++++---------
> 1 file changed, 19 insertions(+), 11 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 a82a53dbbc86d..c9cd9bb47577f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -80,14 +80,14 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
> */
> #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \
> fw_def(METEORLAKE, 0, guc_mmp(mtl, 70, 6, 5)) \
> - fw_def(DG2, 0, guc_maj(dg2, 70, 5)) \
> - fw_def(ALDERLAKE_P, 0, guc_maj(adlp, 70, 5)) \
> + fw_def(DG2, 0, guc_maj(dg2, 70, 5, 1)) \
> + fw_def(ALDERLAKE_P, 0, guc_maj(adlp, 70, 5, 1)) \
> fw_def(ALDERLAKE_P, 0, guc_mmp(adlp, 70, 1, 1)) \
> fw_def(ALDERLAKE_P, 0, guc_mmp(adlp, 69, 0, 3)) \
> - fw_def(ALDERLAKE_S, 0, guc_maj(tgl, 70, 5)) \
> + fw_def(ALDERLAKE_S, 0, guc_maj(tgl, 70, 5, 1)) \
> fw_def(ALDERLAKE_S, 0, guc_mmp(tgl, 70, 1, 1)) \
> fw_def(ALDERLAKE_S, 0, guc_mmp(tgl, 69, 0, 3)) \
> - fw_def(DG1, 0, guc_maj(dg1, 70, 5)) \
> + fw_def(DG1, 0, guc_maj(dg1, 70, 5, 1)) \
> fw_def(ROCKETLAKE, 0, guc_mmp(tgl, 70, 1, 1)) \
> fw_def(TIGERLAKE, 0, guc_mmp(tgl, 70, 1, 1)) \
> fw_def(JASPERLAKE, 0, guc_mmp(ehl, 70, 1, 1)) \
> @@ -141,7 +141,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
> __stringify(patch_) ".bin"
>
> /* Minor for internal driver use, not part of file name */
> -#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_) \
> +#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_) \
> __MAKE_UC_FW_PATH_MAJOR(prefix_, "guc", major_)
>
> #define MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \
> @@ -197,9 +197,9 @@ struct __packed uc_fw_blob {
> { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \
> .legacy = true }
>
> -#define GUC_FW_BLOB(prefix_, major_, minor_) \
> - UC_FW_BLOB_NEW(major_, minor_, 0, false, \
> - MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_))
> +#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
> + UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
> + MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
>
> #define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \
> UC_FW_BLOB_OLD(major_, minor_, patch_, \
> @@ -296,6 +296,7 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
> uc_fw->file_wanted.path = blob->path;
> uc_fw->file_wanted.ver.major = blob->major;
> uc_fw->file_wanted.ver.minor = blob->minor;
> + uc_fw->file_wanted.ver.patch = blob->patch;
> uc_fw->loaded_via_gsc = blob->loaded_via_gsc;
> found = true;
> break;
> @@ -790,6 +791,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> } else {
> if (uc_fw->file_selected.ver.minor < uc_fw->file_wanted.ver.minor)
> old_ver = true;
> + else if ((uc_fw->file_selected.ver.minor == uc_fw->file_wanted.ver.minor) &&
> + (uc_fw->file_selected.ver.patch < uc_fw->file_wanted.ver.patch))
> + old_ver = true;
> }
> }
>
> @@ -797,12 +801,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> /* Preserve the version that was really wanted */
> memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted));
>
> - gt_notice(gt, "%s firmware %s (%d.%d) is recommended, but only %s (%d.%d) was found\n",
> + gt_notice(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n",
> intel_uc_fw_type_repr(uc_fw->type),
> uc_fw->file_wanted.path,
> - uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor,
> + uc_fw->file_wanted.ver.major,
> + uc_fw->file_wanted.ver.minor,
> + uc_fw->file_wanted.ver.patch,
> uc_fw->file_selected.path,
> - uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor);
> + uc_fw->file_selected.ver.major,
> + uc_fw->file_selected.ver.minor,
> + uc_fw->file_selected.ver.patch);
> gt_info(gt, "Consider updating your linux-firmware pkg or downloading from %s\n",
> INTEL_UC_FIRMWARE_URL);
> }
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (2 preceding siblings ...)
2023-04-21 1:15 ` [Intel-gfx] [PATCH 3/6] drm/i915/uc: Track patch level versions on reduced version firmware files John.C.Harrison
@ 2023-04-21 1:15 ` John.C.Harrison
2023-04-29 0:04 ` Ceraolo Spurio, Daniele
2023-04-21 1:15 ` [Intel-gfx] [PATCH 5/6] drm/i915/uc: Reject duplicate entries in firmware table John.C.Harrison
` (11 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: John.C.Harrison @ 2023-04-21 1:15 UTC (permalink / raw)
To: Intel-GFX; +Cc: DRI-Devel
From: John Harrison <John.C.Harrison@Intel.com>
The validation of the firmware table was being done inside the code
for scanning the table for the next available firmware blob. Which is
unnecessary. So pull it out into a separate function that is only
called once per blob type at init time.
Also, drop the CONFIG_SELFTEST requirement and make errors terminal.
It was mentioned that potential issues with backports would not be
caught by regular pre-merge CI as that only occurs on tip not stable
branches. Making the validation unconditional and failing driver load
on detecting of a problem ensures that such backports will also be
validated correctly.
v2: Change to unconditionally fail module load on a validation error
(review feedback/discussion with Daniele).
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 157 +++++++++++++----------
1 file changed, 92 insertions(+), 65 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 c9cd9bb47577f..eb52e8db9ae0b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -233,20 +233,22 @@ struct fw_blobs_by_type {
u32 count;
};
+static const struct uc_fw_platform_requirement blobs_guc[] = {
+ INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP)
+};
+
+static const struct uc_fw_platform_requirement blobs_huc[] = {
+ INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
+};
+
+static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = {
+ [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
+ [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
+};
+
static void
__uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
{
- static const struct uc_fw_platform_requirement blobs_guc[] = {
- INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP)
- };
- static const struct uc_fw_platform_requirement blobs_huc[] = {
- INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
- };
- static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = {
- [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
- [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
- };
- static bool verified[INTEL_UC_FW_NUM_TYPES];
const struct uc_fw_platform_requirement *fw_blobs;
enum intel_platform p = INTEL_INFO(i915)->platform;
u32 fw_count;
@@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
continue;
if (uc_fw->file_selected.path) {
+ /*
+ * Continuing an earlier search after a found blob failed to load.
+ * Once the previously chosen path has been found, clear it out
+ * and let the search continue from there.
+ */
if (uc_fw->file_selected.path == blob->path)
uc_fw->file_selected.path = NULL;
@@ -306,76 +313,91 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
/* Failed to find a match for the last attempt?! */
uc_fw->file_selected.path = NULL;
}
+}
- /* make sure the list is ordered as expected */
- if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified[uc_fw->type]) {
- verified[uc_fw->type] = true;
+static bool validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_fw_type type)
+{
+ const struct uc_fw_platform_requirement *fw_blobs;
+ u32 fw_count;
+ int i;
- for (i = 1; i < fw_count; i++) {
- /* Next platform is good: */
- if (fw_blobs[i].p < fw_blobs[i - 1].p)
- continue;
+ if (type >= ARRAY_SIZE(blobs_all)) {
+ drm_err(&i915->drm, "No blob array for %s\n", intel_uc_fw_type_repr(type));
+ return false;
+ }
- /* Next platform revision is good: */
- if (fw_blobs[i].p == fw_blobs[i - 1].p &&
- fw_blobs[i].rev < fw_blobs[i - 1].rev)
- continue;
+ fw_blobs = blobs_all[type].blobs;
+ fw_count = blobs_all[type].count;
- /* Platform/revision must be in order: */
- if (fw_blobs[i].p != fw_blobs[i - 1].p ||
- fw_blobs[i].rev != fw_blobs[i - 1].rev)
- goto bad;
+ if (!fw_count)
+ return true;
- /* Next major version is good: */
- if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
- continue;
+ /* make sure the list is ordered as expected */
+ for (i = 1; i < fw_count; i++) {
+ /* Next platform is good: */
+ if (fw_blobs[i].p < fw_blobs[i - 1].p)
+ continue;
- /* New must be before legacy: */
- if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy)
- goto bad;
+ /* Next platform revision is good: */
+ if (fw_blobs[i].p == fw_blobs[i - 1].p &&
+ fw_blobs[i].rev < fw_blobs[i - 1].rev)
+ continue;
- /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y (GuC) */
- if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) {
- if (!fw_blobs[i - 1].blob.major)
- continue;
+ /* Platform/revision must be in order: */
+ if (fw_blobs[i].p != fw_blobs[i - 1].p ||
+ fw_blobs[i].rev != fw_blobs[i - 1].rev)
+ goto bad;
- if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major)
- continue;
- }
+ /* Next major version is good: */
+ if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
+ continue;
- /* Major versions must be in order: */
- if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
- goto bad;
+ /* New must be before legacy: */
+ if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy)
+ goto bad;
- /* Next minor version is good: */
- if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
+ /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y (GuC) */
+ if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) {
+ if (!fw_blobs[i - 1].blob.major)
continue;
- /* Minor versions must be in order: */
- if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
- goto bad;
-
- /* Patch versions must be in order: */
- if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
+ if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major)
continue;
+ }
+
+ /* Major versions must be in order: */
+ if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
+ goto bad;
+
+ /* Next minor version is good: */
+ if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
+ continue;
+
+ /* Minor versions must be in order: */
+ if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
+ goto bad;
+
+ /* Patch versions must be in order: */
+ if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
+ continue;
bad:
- drm_err(&i915->drm, "Invalid %s blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
- intel_uc_fw_type_repr(uc_fw->type),
- intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - 1].rev,
- fw_blobs[i - 1].blob.legacy ? "L" : "v",
- fw_blobs[i - 1].blob.major,
- fw_blobs[i - 1].blob.minor,
- fw_blobs[i - 1].blob.patch,
- intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
- fw_blobs[i].blob.legacy ? "L" : "v",
- fw_blobs[i].blob.major,
- fw_blobs[i].blob.minor,
- fw_blobs[i].blob.patch);
-
- uc_fw->file_selected.path = NULL;
- }
+ drm_err(&i915->drm, "Invalid %s blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
+ intel_uc_fw_type_repr(type),
+ intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - 1].rev,
+ fw_blobs[i - 1].blob.legacy ? "L" : "v",
+ fw_blobs[i - 1].blob.major,
+ fw_blobs[i - 1].blob.minor,
+ fw_blobs[i - 1].blob.patch,
+ intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
+ fw_blobs[i].blob.legacy ? "L" : "v",
+ fw_blobs[i].blob.major,
+ fw_blobs[i].blob.minor,
+ fw_blobs[i].blob.patch);
+ return false;
}
+
+ return true;
}
static const char *__override_guc_firmware_path(struct drm_i915_private *i915)
@@ -443,6 +465,11 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
uc_fw->type = type;
if (HAS_GT_UC(i915)) {
+ if (!validate_fw_table_type(i915, type)) {
+ intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_ERROR);
+ return;
+ }
+
__uc_fw_auto_select(i915, uc_fw);
__uc_fw_user_override(i915, uc_fw);
}
--
2.39.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation
2023-04-21 1:15 ` [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation John.C.Harrison
@ 2023-04-29 0:04 ` Ceraolo Spurio, Daniele
2023-04-29 0:16 ` John Harrison
0 siblings, 1 reply; 27+ messages in thread
From: Ceraolo Spurio, Daniele @ 2023-04-29 0:04 UTC (permalink / raw)
To: John.C.Harrison, Intel-GFX; +Cc: DRI-Devel
On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> The validation of the firmware table was being done inside the code
> for scanning the table for the next available firmware blob. Which is
> unnecessary. So pull it out into a separate function that is only
> called once per blob type at init time.
>
> Also, drop the CONFIG_SELFTEST requirement and make errors terminal.
> It was mentioned that potential issues with backports would not be
> caught by regular pre-merge CI as that only occurs on tip not stable
> branches. Making the validation unconditional and failing driver load
> on detecting of a problem ensures that such backports will also be
> validated correctly.
>
> v2: Change to unconditionally fail module load on a validation error
> (review feedback/discussion with Daniele).
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 157 +++++++++++++----------
> 1 file changed, 92 insertions(+), 65 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 c9cd9bb47577f..eb52e8db9ae0b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -233,20 +233,22 @@ struct fw_blobs_by_type {
> u32 count;
> };
>
> +static const struct uc_fw_platform_requirement blobs_guc[] = {
> + INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP)
> +};
> +
> +static const struct uc_fw_platform_requirement blobs_huc[] = {
> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
> +};
> +
> +static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = {
> + [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
> + [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
> +};
> +
> static void
> __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
> {
> - static const struct uc_fw_platform_requirement blobs_guc[] = {
> - INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP)
> - };
> - static const struct uc_fw_platform_requirement blobs_huc[] = {
> - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
> - };
> - static const struct fw_blobs_by_type blobs_all[INTEL_UC_FW_NUM_TYPES] = {
> - [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
> - [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
> - };
> - static bool verified[INTEL_UC_FW_NUM_TYPES];
> const struct uc_fw_platform_requirement *fw_blobs;
> enum intel_platform p = INTEL_INFO(i915)->platform;
> u32 fw_count;
> @@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
> continue;
>
> if (uc_fw->file_selected.path) {
> + /*
> + * Continuing an earlier search after a found blob failed to load.
> + * Once the previously chosen path has been found, clear it out
> + * and let the search continue from there.
> + */
> if (uc_fw->file_selected.path == blob->path)
> uc_fw->file_selected.path = NULL;
>
> @@ -306,76 +313,91 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
> /* Failed to find a match for the last attempt?! */
> uc_fw->file_selected.path = NULL;
> }
> +}
>
> - /* make sure the list is ordered as expected */
> - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified[uc_fw->type]) {
> - verified[uc_fw->type] = true;
> +static bool validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_fw_type type)
> +{
> + const struct uc_fw_platform_requirement *fw_blobs;
> + u32 fw_count;
> + int i;
>
> - for (i = 1; i < fw_count; i++) {
> - /* Next platform is good: */
> - if (fw_blobs[i].p < fw_blobs[i - 1].p)
> - continue;
> + if (type >= ARRAY_SIZE(blobs_all)) {
> + drm_err(&i915->drm, "No blob array for %s\n", intel_uc_fw_type_repr(type));
> + return false;
> + }
>
> - /* Next platform revision is good: */
> - if (fw_blobs[i].p == fw_blobs[i - 1].p &&
> - fw_blobs[i].rev < fw_blobs[i - 1].rev)
> - continue;
> + fw_blobs = blobs_all[type].blobs;
> + fw_count = blobs_all[type].count;
>
> - /* Platform/revision must be in order: */
> - if (fw_blobs[i].p != fw_blobs[i - 1].p ||
> - fw_blobs[i].rev != fw_blobs[i - 1].rev)
> - goto bad;
> + if (!fw_count)
> + return true;
>
> - /* Next major version is good: */
> - if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
> - continue;
> + /* make sure the list is ordered as expected */
> + for (i = 1; i < fw_count; i++) {
> + /* Next platform is good: */
> + if (fw_blobs[i].p < fw_blobs[i - 1].p)
> + continue;
>
> - /* New must be before legacy: */
> - if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy)
> - goto bad;
> + /* Next platform revision is good: */
> + if (fw_blobs[i].p == fw_blobs[i - 1].p &&
> + fw_blobs[i].rev < fw_blobs[i - 1].rev)
> + continue;
>
> - /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y (GuC) */
> - if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) {
> - if (!fw_blobs[i - 1].blob.major)
> - continue;
> + /* Platform/revision must be in order: */
> + if (fw_blobs[i].p != fw_blobs[i - 1].p ||
> + fw_blobs[i].rev != fw_blobs[i - 1].rev)
> + goto bad;
>
> - if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major)
> - continue;
> - }
> + /* Next major version is good: */
> + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
> + continue;
>
> - /* Major versions must be in order: */
> - if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
> - goto bad;
> + /* New must be before legacy: */
> + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy)
> + goto bad;
>
> - /* Next minor version is good: */
> - if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
> + /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y (GuC) */
> + if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) {
> + if (!fw_blobs[i - 1].blob.major)
> continue;
>
> - /* Minor versions must be in order: */
> - if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
> - goto bad;
> -
> - /* Patch versions must be in order: */
> - if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
> + if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major)
> continue;
> + }
> +
> + /* Major versions must be in order: */
> + if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
> + goto bad;
> +
> + /* Next minor version is good: */
> + if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
> + continue;
> +
> + /* Minor versions must be in order: */
> + if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
> + goto bad;
> +
> + /* Patch versions must be in order: */
> + if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
> + continue;
>
> bad:
> - drm_err(&i915->drm, "Invalid %s blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
> - intel_uc_fw_type_repr(uc_fw->type),
> - intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - 1].rev,
> - fw_blobs[i - 1].blob.legacy ? "L" : "v",
> - fw_blobs[i - 1].blob.major,
> - fw_blobs[i - 1].blob.minor,
> - fw_blobs[i - 1].blob.patch,
> - intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
> - fw_blobs[i].blob.legacy ? "L" : "v",
> - fw_blobs[i].blob.major,
> - fw_blobs[i].blob.minor,
> - fw_blobs[i].blob.patch);
> -
> - uc_fw->file_selected.path = NULL;
> - }
> + drm_err(&i915->drm, "Invalid %s blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
> + intel_uc_fw_type_repr(type),
> + intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i - 1].rev,
> + fw_blobs[i - 1].blob.legacy ? "L" : "v",
> + fw_blobs[i - 1].blob.major,
> + fw_blobs[i - 1].blob.minor,
> + fw_blobs[i - 1].blob.patch,
> + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
> + fw_blobs[i].blob.legacy ? "L" : "v",
> + fw_blobs[i].blob.major,
> + fw_blobs[i].blob.minor,
> + fw_blobs[i].blob.patch);
> + return false;
> }
> +
> + return true;
> }
>
> static const char *__override_guc_firmware_path(struct drm_i915_private *i915)
> @@ -443,6 +465,11 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw,
> uc_fw->type = type;
>
> if (HAS_GT_UC(i915)) {
> + if (!validate_fw_table_type(i915, type)) {
> + intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_ERROR);
In our hierarchy of firmware statuses, INTEL_UC_FIRMWARE_ERROR includes
the fact that the fw has been selected, which in turns implies that
file_selected.path is valid. this means that even with enable_guc=0 the
wants/uses_guc macro might end up returning true, which is not something
we want.
Daniele
> + return;
> + }
> +
> __uc_fw_auto_select(i915, uc_fw);
> __uc_fw_user_override(i915, uc_fw);
> }
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation
2023-04-29 0:04 ` Ceraolo Spurio, Daniele
@ 2023-04-29 0:16 ` John Harrison
2023-04-29 0:26 ` Ceraolo Spurio, Daniele
0 siblings, 1 reply; 27+ messages in thread
From: John Harrison @ 2023-04-29 0:16 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, Intel-GFX; +Cc: DRI-Devel
On 4/28/2023 17:04, Ceraolo Spurio, Daniele wrote:
> On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> The validation of the firmware table was being done inside the code
>> for scanning the table for the next available firmware blob. Which is
>> unnecessary. So pull it out into a separate function that is only
>> called once per blob type at init time.
>>
>> Also, drop the CONFIG_SELFTEST requirement and make errors terminal.
>> It was mentioned that potential issues with backports would not be
>> caught by regular pre-merge CI as that only occurs on tip not stable
>> branches. Making the validation unconditional and failing driver load
>> on detecting of a problem ensures that such backports will also be
>> validated correctly.
>>
>> v2: Change to unconditionally fail module load on a validation error
>> (review feedback/discussion with Daniele).
>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 157 +++++++++++++----------
>> 1 file changed, 92 insertions(+), 65 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 c9cd9bb47577f..eb52e8db9ae0b 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> @@ -233,20 +233,22 @@ struct fw_blobs_by_type {
>> u32 count;
>> };
>> +static const struct uc_fw_platform_requirement blobs_guc[] = {
>> + INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB, GUC_FW_BLOB_MMP)
>> +};
>> +
>> +static const struct uc_fw_platform_requirement blobs_huc[] = {
>> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>> +};
>> +
>> +static const struct fw_blobs_by_type
>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>> + [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
>> + [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
>> +};
>> +
>> static void
>> __uc_fw_auto_select(struct drm_i915_private *i915, struct
>> intel_uc_fw *uc_fw)
>> {
>> - static const struct uc_fw_platform_requirement blobs_guc[] = {
>> - INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
>> GUC_FW_BLOB_MMP)
>> - };
>> - static const struct uc_fw_platform_requirement blobs_huc[] = {
>> - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>> - };
>> - static const struct fw_blobs_by_type
>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>> - [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
>> - [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
>> - };
>> - static bool verified[INTEL_UC_FW_NUM_TYPES];
>> const struct uc_fw_platform_requirement *fw_blobs;
>> enum intel_platform p = INTEL_INFO(i915)->platform;
>> u32 fw_count;
>> @@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private
>> *i915, struct intel_uc_fw *uc_fw)
>> continue;
>> if (uc_fw->file_selected.path) {
>> + /*
>> + * Continuing an earlier search after a found blob
>> failed to load.
>> + * Once the previously chosen path has been found, clear
>> it out
>> + * and let the search continue from there.
>> + */
>> if (uc_fw->file_selected.path == blob->path)
>> uc_fw->file_selected.path = NULL;
>> @@ -306,76 +313,91 @@ __uc_fw_auto_select(struct drm_i915_private
>> *i915, struct intel_uc_fw *uc_fw)
>> /* Failed to find a match for the last attempt?! */
>> uc_fw->file_selected.path = NULL;
>> }
>> +}
>> - /* make sure the list is ordered as expected */
>> - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) &&
>> !verified[uc_fw->type]) {
>> - verified[uc_fw->type] = true;
>> +static bool validate_fw_table_type(struct drm_i915_private *i915,
>> enum intel_uc_fw_type type)
>> +{
>> + const struct uc_fw_platform_requirement *fw_blobs;
>> + u32 fw_count;
>> + int i;
>> - for (i = 1; i < fw_count; i++) {
>> - /* Next platform is good: */
>> - if (fw_blobs[i].p < fw_blobs[i - 1].p)
>> - continue;
>> + if (type >= ARRAY_SIZE(blobs_all)) {
>> + drm_err(&i915->drm, "No blob array for %s\n",
>> intel_uc_fw_type_repr(type));
>> + return false;
>> + }
>> - /* Next platform revision is good: */
>> - if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>> - fw_blobs[i].rev < fw_blobs[i - 1].rev)
>> - continue;
>> + fw_blobs = blobs_all[type].blobs;
>> + fw_count = blobs_all[type].count;
>> - /* Platform/revision must be in order: */
>> - if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>> - fw_blobs[i].rev != fw_blobs[i - 1].rev)
>> - goto bad;
>> + if (!fw_count)
>> + return true;
>> - /* Next major version is good: */
>> - if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
>> - continue;
>> + /* make sure the list is ordered as expected */
>> + for (i = 1; i < fw_count; i++) {
>> + /* Next platform is good: */
>> + if (fw_blobs[i].p < fw_blobs[i - 1].p)
>> + continue;
>> - /* New must be before legacy: */
>> - if (!fw_blobs[i].blob.legacy && fw_blobs[i -
>> 1].blob.legacy)
>> - goto bad;
>> + /* Next platform revision is good: */
>> + if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>> + fw_blobs[i].rev < fw_blobs[i - 1].rev)
>> + continue;
>> - /* New to legacy also means 0.0 to X.Y (HuC), or X.0
>> to X.Y (GuC) */
>> - if (fw_blobs[i].blob.legacy && !fw_blobs[i -
>> 1].blob.legacy) {
>> - if (!fw_blobs[i - 1].blob.major)
>> - continue;
>> + /* Platform/revision must be in order: */
>> + if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>> + fw_blobs[i].rev != fw_blobs[i - 1].rev)
>> + goto bad;
>> - if (fw_blobs[i].blob.major == fw_blobs[i -
>> 1].blob.major)
>> - continue;
>> - }
>> + /* Next major version is good: */
>> + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
>> + continue;
>> - /* Major versions must be in order: */
>> - if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
>> - goto bad;
>> + /* New must be before legacy: */
>> + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy)
>> + goto bad;
>> - /* Next minor version is good: */
>> - if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
>> + /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y
>> (GuC) */
>> + if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) {
>> + if (!fw_blobs[i - 1].blob.major)
>> continue;
>> - /* Minor versions must be in order: */
>> - if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
>> - goto bad;
>> -
>> - /* Patch versions must be in order: */
>> - if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
>> + if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major)
>> continue;
>> + }
>> +
>> + /* Major versions must be in order: */
>> + if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
>> + goto bad;
>> +
>> + /* Next minor version is good: */
>> + if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
>> + continue;
>> +
>> + /* Minor versions must be in order: */
>> + if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
>> + goto bad;
>> +
>> + /* Patch versions must be in order: */
>> + if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
>> + continue;
>> bad:
>> - drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>> - intel_uc_fw_type_repr(uc_fw->type),
>> - intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i -
>> 1].rev,
>> - fw_blobs[i - 1].blob.legacy ? "L" : "v",
>> - fw_blobs[i - 1].blob.major,
>> - fw_blobs[i - 1].blob.minor,
>> - fw_blobs[i - 1].blob.patch,
>> - intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
>> - fw_blobs[i].blob.legacy ? "L" : "v",
>> - fw_blobs[i].blob.major,
>> - fw_blobs[i].blob.minor,
>> - fw_blobs[i].blob.patch);
>> -
>> - uc_fw->file_selected.path = NULL;
>> - }
>> + drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>> + intel_uc_fw_type_repr(type),
>> + intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i -
>> 1].rev,
>> + fw_blobs[i - 1].blob.legacy ? "L" : "v",
>> + fw_blobs[i - 1].blob.major,
>> + fw_blobs[i - 1].blob.minor,
>> + fw_blobs[i - 1].blob.patch,
>> + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
>> + fw_blobs[i].blob.legacy ? "L" : "v",
>> + fw_blobs[i].blob.major,
>> + fw_blobs[i].blob.minor,
>> + fw_blobs[i].blob.patch);
>> + return false;
>> }
>> +
>> + return true;
>> }
>> static const char *__override_guc_firmware_path(struct
>> drm_i915_private *i915)
>> @@ -443,6 +465,11 @@ void intel_uc_fw_init_early(struct intel_uc_fw
>> *uc_fw,
>> uc_fw->type = type;
>> if (HAS_GT_UC(i915)) {
>> + if (!validate_fw_table_type(i915, type)) {
>> + intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_ERROR);
>
> In our hierarchy of firmware statuses, INTEL_UC_FIRMWARE_ERROR
> includes the fact that the fw has been selected, which in turns
> implies that file_selected.path is valid. this means that even with
> enable_guc=0 the wants/uses_guc macro might end up returning true,
> which is not something we want.
>
> Daniele
Suggestions for a better plan? Add an another status enum? Nothing
earlier in the sequence seems appropriate. And the init_early stack does
not support returning error codes.
John.
>
>> + return;
>> + }
>> +
>> __uc_fw_auto_select(i915, uc_fw);
>> __uc_fw_user_override(i915, uc_fw);
>> }
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation
2023-04-29 0:16 ` John Harrison
@ 2023-04-29 0:26 ` Ceraolo Spurio, Daniele
2023-04-29 0:30 ` John Harrison
0 siblings, 1 reply; 27+ messages in thread
From: Ceraolo Spurio, Daniele @ 2023-04-29 0:26 UTC (permalink / raw)
To: John Harrison, Intel-GFX; +Cc: DRI-Devel
On 4/28/2023 5:16 PM, John Harrison wrote:
> On 4/28/2023 17:04, Ceraolo Spurio, Daniele wrote:
>> On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>
>>> The validation of the firmware table was being done inside the code
>>> for scanning the table for the next available firmware blob. Which is
>>> unnecessary. So pull it out into a separate function that is only
>>> called once per blob type at init time.
>>>
>>> Also, drop the CONFIG_SELFTEST requirement and make errors terminal.
>>> It was mentioned that potential issues with backports would not be
>>> caught by regular pre-merge CI as that only occurs on tip not stable
>>> branches. Making the validation unconditional and failing driver load
>>> on detecting of a problem ensures that such backports will also be
>>> validated correctly.
>>>
>>> v2: Change to unconditionally fail module load on a validation error
>>> (review feedback/discussion with Daniele).
>>>
>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 157
>>> +++++++++++++----------
>>> 1 file changed, 92 insertions(+), 65 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 c9cd9bb47577f..eb52e8db9ae0b 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> @@ -233,20 +233,22 @@ struct fw_blobs_by_type {
>>> u32 count;
>>> };
>>> +static const struct uc_fw_platform_requirement blobs_guc[] = {
>>> + INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
>>> GUC_FW_BLOB_MMP)
>>> +};
>>> +
>>> +static const struct uc_fw_platform_requirement blobs_huc[] = {
>>> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>>> +};
>>> +
>>> +static const struct fw_blobs_by_type
>>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>>> + [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
>>> + [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
>>> +};
>>> +
>>> static void
>>> __uc_fw_auto_select(struct drm_i915_private *i915, struct
>>> intel_uc_fw *uc_fw)
>>> {
>>> - static const struct uc_fw_platform_requirement blobs_guc[] = {
>>> - INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
>>> GUC_FW_BLOB_MMP)
>>> - };
>>> - static const struct uc_fw_platform_requirement blobs_huc[] = {
>>> - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>>> - };
>>> - static const struct fw_blobs_by_type
>>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>>> - [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
>>> - [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
>>> - };
>>> - static bool verified[INTEL_UC_FW_NUM_TYPES];
>>> const struct uc_fw_platform_requirement *fw_blobs;
>>> enum intel_platform p = INTEL_INFO(i915)->platform;
>>> u32 fw_count;
>>> @@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private
>>> *i915, struct intel_uc_fw *uc_fw)
>>> continue;
>>> if (uc_fw->file_selected.path) {
>>> + /*
>>> + * Continuing an earlier search after a found blob
>>> failed to load.
>>> + * Once the previously chosen path has been found,
>>> clear it out
>>> + * and let the search continue from there.
>>> + */
>>> if (uc_fw->file_selected.path == blob->path)
>>> uc_fw->file_selected.path = NULL;
>>> @@ -306,76 +313,91 @@ __uc_fw_auto_select(struct drm_i915_private
>>> *i915, struct intel_uc_fw *uc_fw)
>>> /* Failed to find a match for the last attempt?! */
>>> uc_fw->file_selected.path = NULL;
>>> }
>>> +}
>>> - /* make sure the list is ordered as expected */
>>> - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) &&
>>> !verified[uc_fw->type]) {
>>> - verified[uc_fw->type] = true;
>>> +static bool validate_fw_table_type(struct drm_i915_private *i915,
>>> enum intel_uc_fw_type type)
>>> +{
>>> + const struct uc_fw_platform_requirement *fw_blobs;
>>> + u32 fw_count;
>>> + int i;
>>> - for (i = 1; i < fw_count; i++) {
>>> - /* Next platform is good: */
>>> - if (fw_blobs[i].p < fw_blobs[i - 1].p)
>>> - continue;
>>> + if (type >= ARRAY_SIZE(blobs_all)) {
>>> + drm_err(&i915->drm, "No blob array for %s\n",
>>> intel_uc_fw_type_repr(type));
>>> + return false;
>>> + }
>>> - /* Next platform revision is good: */
>>> - if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>>> - fw_blobs[i].rev < fw_blobs[i - 1].rev)
>>> - continue;
>>> + fw_blobs = blobs_all[type].blobs;
>>> + fw_count = blobs_all[type].count;
>>> - /* Platform/revision must be in order: */
>>> - if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>>> - fw_blobs[i].rev != fw_blobs[i - 1].rev)
>>> - goto bad;
>>> + if (!fw_count)
>>> + return true;
>>> - /* Next major version is good: */
>>> - if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
>>> - continue;
>>> + /* make sure the list is ordered as expected */
>>> + for (i = 1; i < fw_count; i++) {
>>> + /* Next platform is good: */
>>> + if (fw_blobs[i].p < fw_blobs[i - 1].p)
>>> + continue;
>>> - /* New must be before legacy: */
>>> - if (!fw_blobs[i].blob.legacy && fw_blobs[i -
>>> 1].blob.legacy)
>>> - goto bad;
>>> + /* Next platform revision is good: */
>>> + if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>>> + fw_blobs[i].rev < fw_blobs[i - 1].rev)
>>> + continue;
>>> - /* New to legacy also means 0.0 to X.Y (HuC), or X.0
>>> to X.Y (GuC) */
>>> - if (fw_blobs[i].blob.legacy && !fw_blobs[i -
>>> 1].blob.legacy) {
>>> - if (!fw_blobs[i - 1].blob.major)
>>> - continue;
>>> + /* Platform/revision must be in order: */
>>> + if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>>> + fw_blobs[i].rev != fw_blobs[i - 1].rev)
>>> + goto bad;
>>> - if (fw_blobs[i].blob.major == fw_blobs[i -
>>> 1].blob.major)
>>> - continue;
>>> - }
>>> + /* Next major version is good: */
>>> + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
>>> + continue;
>>> - /* Major versions must be in order: */
>>> - if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
>>> - goto bad;
>>> + /* New must be before legacy: */
>>> + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy)
>>> + goto bad;
>>> - /* Next minor version is good: */
>>> - if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
>>> + /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to X.Y
>>> (GuC) */
>>> + if (fw_blobs[i].blob.legacy && !fw_blobs[i - 1].blob.legacy) {
>>> + if (!fw_blobs[i - 1].blob.major)
>>> continue;
>>> - /* Minor versions must be in order: */
>>> - if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
>>> - goto bad;
>>> -
>>> - /* Patch versions must be in order: */
>>> - if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
>>> + if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major)
>>> continue;
>>> + }
>>> +
>>> + /* Major versions must be in order: */
>>> + if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
>>> + goto bad;
>>> +
>>> + /* Next minor version is good: */
>>> + if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
>>> + continue;
>>> +
>>> + /* Minor versions must be in order: */
>>> + if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
>>> + goto bad;
>>> +
>>> + /* Patch versions must be in order: */
>>> + if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
>>> + continue;
>>> bad:
>>> - drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>>> - intel_uc_fw_type_repr(uc_fw->type),
>>> - intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i
>>> - 1].rev,
>>> - fw_blobs[i - 1].blob.legacy ? "L" : "v",
>>> - fw_blobs[i - 1].blob.major,
>>> - fw_blobs[i - 1].blob.minor,
>>> - fw_blobs[i - 1].blob.patch,
>>> - intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
>>> - fw_blobs[i].blob.legacy ? "L" : "v",
>>> - fw_blobs[i].blob.major,
>>> - fw_blobs[i].blob.minor,
>>> - fw_blobs[i].blob.patch);
>>> -
>>> - uc_fw->file_selected.path = NULL;
>>> - }
>>> + drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>>> + intel_uc_fw_type_repr(type),
>>> + intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i -
>>> 1].rev,
>>> + fw_blobs[i - 1].blob.legacy ? "L" : "v",
>>> + fw_blobs[i - 1].blob.major,
>>> + fw_blobs[i - 1].blob.minor,
>>> + fw_blobs[i - 1].blob.patch,
>>> + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
>>> + fw_blobs[i].blob.legacy ? "L" : "v",
>>> + fw_blobs[i].blob.major,
>>> + fw_blobs[i].blob.minor,
>>> + fw_blobs[i].blob.patch);
>>> + return false;
>>> }
>>> +
>>> + return true;
>>> }
>>> static const char *__override_guc_firmware_path(struct
>>> drm_i915_private *i915)
>>> @@ -443,6 +465,11 @@ void intel_uc_fw_init_early(struct intel_uc_fw
>>> *uc_fw,
>>> uc_fw->type = type;
>>> if (HAS_GT_UC(i915)) {
>>> + if (!validate_fw_table_type(i915, type)) {
>>> + intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_ERROR);
>>
>> In our hierarchy of firmware statuses, INTEL_UC_FIRMWARE_ERROR
>> includes the fact that the fw has been selected, which in turns
>> implies that file_selected.path is valid. this means that even with
>> enable_guc=0 the wants/uses_guc macro might end up returning true,
>> which is not something we want.
>>
>> Daniele
> Suggestions for a better plan? Add an another status enum? Nothing
> earlier in the sequence seems appropriate. And the init_early stack
> does not support returning error codes.
I think the question here is: what are you expecting to happen in case
of error and on what platforms? let's say we have an invalid table entry
for ADLP, would the expectation be that all GuC platforms won't boot, or
just ADLP? And is that only if we have enable_guc set to a positive
value, or even if enable_guc=0?
Daniele
>
> John.
>
>
>>
>>> + return;
>>> + }
>>> +
>>> __uc_fw_auto_select(i915, uc_fw);
>>> __uc_fw_user_override(i915, uc_fw);
>>> }
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation
2023-04-29 0:26 ` Ceraolo Spurio, Daniele
@ 2023-04-29 0:30 ` John Harrison
2023-04-29 0:32 ` John Harrison
0 siblings, 1 reply; 27+ messages in thread
From: John Harrison @ 2023-04-29 0:30 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, Intel-GFX; +Cc: DRI-Devel
On 4/28/2023 17:26, Ceraolo Spurio, Daniele wrote:
> On 4/28/2023 5:16 PM, John Harrison wrote:
>> On 4/28/2023 17:04, Ceraolo Spurio, Daniele wrote:
>>> On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
>>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>>
>>>> The validation of the firmware table was being done inside the code
>>>> for scanning the table for the next available firmware blob. Which is
>>>> unnecessary. So pull it out into a separate function that is only
>>>> called once per blob type at init time.
>>>>
>>>> Also, drop the CONFIG_SELFTEST requirement and make errors terminal.
>>>> It was mentioned that potential issues with backports would not be
>>>> caught by regular pre-merge CI as that only occurs on tip not stable
>>>> branches. Making the validation unconditional and failing driver load
>>>> on detecting of a problem ensures that such backports will also be
>>>> validated correctly.
>>>>
>>>> v2: Change to unconditionally fail module load on a validation error
>>>> (review feedback/discussion with Daniele).
>>>>
>>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 157
>>>> +++++++++++++----------
>>>> 1 file changed, 92 insertions(+), 65 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 c9cd9bb47577f..eb52e8db9ae0b 100644
>>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>>> @@ -233,20 +233,22 @@ struct fw_blobs_by_type {
>>>> u32 count;
>>>> };
>>>> +static const struct uc_fw_platform_requirement blobs_guc[] = {
>>>> + INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
>>>> GUC_FW_BLOB_MMP)
>>>> +};
>>>> +
>>>> +static const struct uc_fw_platform_requirement blobs_huc[] = {
>>>> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>>>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>>>> +};
>>>> +
>>>> +static const struct fw_blobs_by_type
>>>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>>>> + [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
>>>> + [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
>>>> +};
>>>> +
>>>> static void
>>>> __uc_fw_auto_select(struct drm_i915_private *i915, struct
>>>> intel_uc_fw *uc_fw)
>>>> {
>>>> - static const struct uc_fw_platform_requirement blobs_guc[] = {
>>>> - INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
>>>> GUC_FW_BLOB_MMP)
>>>> - };
>>>> - static const struct uc_fw_platform_requirement blobs_huc[] = {
>>>> - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>>>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>>>> - };
>>>> - static const struct fw_blobs_by_type
>>>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>>>> - [INTEL_UC_FW_TYPE_GUC] = { blobs_guc,
>>>> ARRAY_SIZE(blobs_guc) },
>>>> - [INTEL_UC_FW_TYPE_HUC] = { blobs_huc,
>>>> ARRAY_SIZE(blobs_huc) },
>>>> - };
>>>> - static bool verified[INTEL_UC_FW_NUM_TYPES];
>>>> const struct uc_fw_platform_requirement *fw_blobs;
>>>> enum intel_platform p = INTEL_INFO(i915)->platform;
>>>> u32 fw_count;
>>>> @@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private
>>>> *i915, struct intel_uc_fw *uc_fw)
>>>> continue;
>>>> if (uc_fw->file_selected.path) {
>>>> + /*
>>>> + * Continuing an earlier search after a found blob
>>>> failed to load.
>>>> + * Once the previously chosen path has been found,
>>>> clear it out
>>>> + * and let the search continue from there.
>>>> + */
>>>> if (uc_fw->file_selected.path == blob->path)
>>>> uc_fw->file_selected.path = NULL;
>>>> @@ -306,76 +313,91 @@ __uc_fw_auto_select(struct drm_i915_private
>>>> *i915, struct intel_uc_fw *uc_fw)
>>>> /* Failed to find a match for the last attempt?! */
>>>> uc_fw->file_selected.path = NULL;
>>>> }
>>>> +}
>>>> - /* make sure the list is ordered as expected */
>>>> - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) &&
>>>> !verified[uc_fw->type]) {
>>>> - verified[uc_fw->type] = true;
>>>> +static bool validate_fw_table_type(struct drm_i915_private *i915,
>>>> enum intel_uc_fw_type type)
>>>> +{
>>>> + const struct uc_fw_platform_requirement *fw_blobs;
>>>> + u32 fw_count;
>>>> + int i;
>>>> - for (i = 1; i < fw_count; i++) {
>>>> - /* Next platform is good: */
>>>> - if (fw_blobs[i].p < fw_blobs[i - 1].p)
>>>> - continue;
>>>> + if (type >= ARRAY_SIZE(blobs_all)) {
>>>> + drm_err(&i915->drm, "No blob array for %s\n",
>>>> intel_uc_fw_type_repr(type));
>>>> + return false;
>>>> + }
>>>> - /* Next platform revision is good: */
>>>> - if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>>>> - fw_blobs[i].rev < fw_blobs[i - 1].rev)
>>>> - continue;
>>>> + fw_blobs = blobs_all[type].blobs;
>>>> + fw_count = blobs_all[type].count;
>>>> - /* Platform/revision must be in order: */
>>>> - if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>>>> - fw_blobs[i].rev != fw_blobs[i - 1].rev)
>>>> - goto bad;
>>>> + if (!fw_count)
>>>> + return true;
>>>> - /* Next major version is good: */
>>>> - if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
>>>> - continue;
>>>> + /* make sure the list is ordered as expected */
>>>> + for (i = 1; i < fw_count; i++) {
>>>> + /* Next platform is good: */
>>>> + if (fw_blobs[i].p < fw_blobs[i - 1].p)
>>>> + continue;
>>>> - /* New must be before legacy: */
>>>> - if (!fw_blobs[i].blob.legacy && fw_blobs[i -
>>>> 1].blob.legacy)
>>>> - goto bad;
>>>> + /* Next platform revision is good: */
>>>> + if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>>>> + fw_blobs[i].rev < fw_blobs[i - 1].rev)
>>>> + continue;
>>>> - /* New to legacy also means 0.0 to X.Y (HuC), or X.0
>>>> to X.Y (GuC) */
>>>> - if (fw_blobs[i].blob.legacy && !fw_blobs[i -
>>>> 1].blob.legacy) {
>>>> - if (!fw_blobs[i - 1].blob.major)
>>>> - continue;
>>>> + /* Platform/revision must be in order: */
>>>> + if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>>>> + fw_blobs[i].rev != fw_blobs[i - 1].rev)
>>>> + goto bad;
>>>> - if (fw_blobs[i].blob.major == fw_blobs[i -
>>>> 1].blob.major)
>>>> - continue;
>>>> - }
>>>> + /* Next major version is good: */
>>>> + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
>>>> + continue;
>>>> - /* Major versions must be in order: */
>>>> - if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
>>>> - goto bad;
>>>> + /* New must be before legacy: */
>>>> + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy)
>>>> + goto bad;
>>>> - /* Next minor version is good: */
>>>> - if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
>>>> + /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to
>>>> X.Y (GuC) */
>>>> + if (fw_blobs[i].blob.legacy && !fw_blobs[i -
>>>> 1].blob.legacy) {
>>>> + if (!fw_blobs[i - 1].blob.major)
>>>> continue;
>>>> - /* Minor versions must be in order: */
>>>> - if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
>>>> - goto bad;
>>>> -
>>>> - /* Patch versions must be in order: */
>>>> - if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
>>>> + if (fw_blobs[i].blob.major == fw_blobs[i - 1].blob.major)
>>>> continue;
>>>> + }
>>>> +
>>>> + /* Major versions must be in order: */
>>>> + if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
>>>> + goto bad;
>>>> +
>>>> + /* Next minor version is good: */
>>>> + if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
>>>> + continue;
>>>> +
>>>> + /* Minor versions must be in order: */
>>>> + if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
>>>> + goto bad;
>>>> +
>>>> + /* Patch versions must be in order: */
>>>> + if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
>>>> + continue;
>>>> bad:
>>>> - drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>>>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>>>> - intel_uc_fw_type_repr(uc_fw->type),
>>>> - intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i
>>>> - 1].rev,
>>>> - fw_blobs[i - 1].blob.legacy ? "L" : "v",
>>>> - fw_blobs[i - 1].blob.major,
>>>> - fw_blobs[i - 1].blob.minor,
>>>> - fw_blobs[i - 1].blob.patch,
>>>> - intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
>>>> - fw_blobs[i].blob.legacy ? "L" : "v",
>>>> - fw_blobs[i].blob.major,
>>>> - fw_blobs[i].blob.minor,
>>>> - fw_blobs[i].blob.patch);
>>>> -
>>>> - uc_fw->file_selected.path = NULL;
>>>> - }
>>>> + drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>>>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>>>> + intel_uc_fw_type_repr(type),
>>>> + intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i -
>>>> 1].rev,
>>>> + fw_blobs[i - 1].blob.legacy ? "L" : "v",
>>>> + fw_blobs[i - 1].blob.major,
>>>> + fw_blobs[i - 1].blob.minor,
>>>> + fw_blobs[i - 1].blob.patch,
>>>> + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
>>>> + fw_blobs[i].blob.legacy ? "L" : "v",
>>>> + fw_blobs[i].blob.major,
>>>> + fw_blobs[i].blob.minor,
>>>> + fw_blobs[i].blob.patch);
>>>> + return false;
>>>> }
>>>> +
>>>> + return true;
>>>> }
>>>> static const char *__override_guc_firmware_path(struct
>>>> drm_i915_private *i915)
>>>> @@ -443,6 +465,11 @@ void intel_uc_fw_init_early(struct intel_uc_fw
>>>> *uc_fw,
>>>> uc_fw->type = type;
>>>> if (HAS_GT_UC(i915)) {
>>>> + if (!validate_fw_table_type(i915, type)) {
>>>> + intel_uc_fw_change_status(uc_fw,
>>>> INTEL_UC_FIRMWARE_ERROR);
>>>
>>> In our hierarchy of firmware statuses, INTEL_UC_FIRMWARE_ERROR
>>> includes the fact that the fw has been selected, which in turns
>>> implies that file_selected.path is valid. this means that even with
>>> enable_guc=0 the wants/uses_guc macro might end up returning true,
>>> which is not something we want.
>>>
>>> Daniele
>> Suggestions for a better plan? Add an another status enum? Nothing
>> earlier in the sequence seems appropriate. And the init_early stack
>> does not support returning error codes.
>
> I think the question here is: what are you expecting to happen in case
> of error and on what platforms? let's say we have an invalid table
> entry for ADLP, would the expectation be that all GuC platforms won't
> boot, or just ADLP? And is that only if we have enable_guc set to a
> positive value, or even if enable_guc=0?
The intention is to totally break driver load on any table error.
The reason being that someone is back porting a firmware update to ADL-P
but breaks DG2 in the process. However, the are only intending to change
ADL-P and so don't test on DG2. They therefore don't realise that the
driver is now broken for someone else. Whereas, if we make any table
error a fatal load failure irrespective of tested platform, enable_guc
or other module params, etc. then it is guaranteed to be caught no
matter what platform they test on.
John.
>
> Daniele
>
>>
>> John.
>>
>>
>>>
>>>> + return;
>>>> + }
>>>> +
>>>> __uc_fw_auto_select(i915, uc_fw);
>>>> __uc_fw_user_override(i915, uc_fw);
>>>> }
>>>
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation
2023-04-29 0:30 ` John Harrison
@ 2023-04-29 0:32 ` John Harrison
2023-04-29 1:21 ` Ceraolo Spurio, Daniele
0 siblings, 1 reply; 27+ messages in thread
From: John Harrison @ 2023-04-29 0:32 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, Intel-GFX; +Cc: DRI-Devel
On 4/28/2023 17:30, John Harrison wrote:
> On 4/28/2023 17:26, Ceraolo Spurio, Daniele wrote:
>> On 4/28/2023 5:16 PM, John Harrison wrote:
>>> On 4/28/2023 17:04, Ceraolo Spurio, Daniele wrote:
>>>> On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
>>>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>>>
>>>>> The validation of the firmware table was being done inside the code
>>>>> for scanning the table for the next available firmware blob. Which is
>>>>> unnecessary. So pull it out into a separate function that is only
>>>>> called once per blob type at init time.
>>>>>
>>>>> Also, drop the CONFIG_SELFTEST requirement and make errors terminal.
>>>>> It was mentioned that potential issues with backports would not be
>>>>> caught by regular pre-merge CI as that only occurs on tip not stable
>>>>> branches. Making the validation unconditional and failing driver load
>>>>> on detecting of a problem ensures that such backports will also be
>>>>> validated correctly.
>>>>>
>>>>> v2: Change to unconditionally fail module load on a validation error
>>>>> (review feedback/discussion with Daniele).
>>>>>
>>>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 157
>>>>> +++++++++++++----------
>>>>> 1 file changed, 92 insertions(+), 65 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 c9cd9bb47577f..eb52e8db9ae0b 100644
>>>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>>>> @@ -233,20 +233,22 @@ struct fw_blobs_by_type {
>>>>> u32 count;
>>>>> };
>>>>> +static const struct uc_fw_platform_requirement blobs_guc[] = {
>>>>> + INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
>>>>> GUC_FW_BLOB_MMP)
>>>>> +};
>>>>> +
>>>>> +static const struct uc_fw_platform_requirement blobs_huc[] = {
>>>>> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>>>>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>>>>> +};
>>>>> +
>>>>> +static const struct fw_blobs_by_type
>>>>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>>>>> + [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
>>>>> + [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
>>>>> +};
>>>>> +
>>>>> static void
>>>>> __uc_fw_auto_select(struct drm_i915_private *i915, struct
>>>>> intel_uc_fw *uc_fw)
>>>>> {
>>>>> - static const struct uc_fw_platform_requirement blobs_guc[] = {
>>>>> - INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
>>>>> GUC_FW_BLOB_MMP)
>>>>> - };
>>>>> - static const struct uc_fw_platform_requirement blobs_huc[] = {
>>>>> - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>>>>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>>>>> - };
>>>>> - static const struct fw_blobs_by_type
>>>>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>>>>> - [INTEL_UC_FW_TYPE_GUC] = { blobs_guc,
>>>>> ARRAY_SIZE(blobs_guc) },
>>>>> - [INTEL_UC_FW_TYPE_HUC] = { blobs_huc,
>>>>> ARRAY_SIZE(blobs_huc) },
>>>>> - };
>>>>> - static bool verified[INTEL_UC_FW_NUM_TYPES];
>>>>> const struct uc_fw_platform_requirement *fw_blobs;
>>>>> enum intel_platform p = INTEL_INFO(i915)->platform;
>>>>> u32 fw_count;
>>>>> @@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private
>>>>> *i915, struct intel_uc_fw *uc_fw)
>>>>> continue;
>>>>> if (uc_fw->file_selected.path) {
>>>>> + /*
>>>>> + * Continuing an earlier search after a found blob
>>>>> failed to load.
>>>>> + * Once the previously chosen path has been found,
>>>>> clear it out
>>>>> + * and let the search continue from there.
>>>>> + */
>>>>> if (uc_fw->file_selected.path == blob->path)
>>>>> uc_fw->file_selected.path = NULL;
>>>>> @@ -306,76 +313,91 @@ __uc_fw_auto_select(struct
>>>>> drm_i915_private *i915, struct intel_uc_fw *uc_fw)
>>>>> /* Failed to find a match for the last attempt?! */
>>>>> uc_fw->file_selected.path = NULL;
>>>>> }
>>>>> +}
>>>>> - /* make sure the list is ordered as expected */
>>>>> - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) &&
>>>>> !verified[uc_fw->type]) {
>>>>> - verified[uc_fw->type] = true;
>>>>> +static bool validate_fw_table_type(struct drm_i915_private *i915,
>>>>> enum intel_uc_fw_type type)
>>>>> +{
>>>>> + const struct uc_fw_platform_requirement *fw_blobs;
>>>>> + u32 fw_count;
>>>>> + int i;
>>>>> - for (i = 1; i < fw_count; i++) {
>>>>> - /* Next platform is good: */
>>>>> - if (fw_blobs[i].p < fw_blobs[i - 1].p)
>>>>> - continue;
>>>>> + if (type >= ARRAY_SIZE(blobs_all)) {
>>>>> + drm_err(&i915->drm, "No blob array for %s\n",
>>>>> intel_uc_fw_type_repr(type));
>>>>> + return false;
>>>>> + }
>>>>> - /* Next platform revision is good: */
>>>>> - if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>>>>> - fw_blobs[i].rev < fw_blobs[i - 1].rev)
>>>>> - continue;
>>>>> + fw_blobs = blobs_all[type].blobs;
>>>>> + fw_count = blobs_all[type].count;
>>>>> - /* Platform/revision must be in order: */
>>>>> - if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>>>>> - fw_blobs[i].rev != fw_blobs[i - 1].rev)
>>>>> - goto bad;
>>>>> + if (!fw_count)
>>>>> + return true;
>>>>> - /* Next major version is good: */
>>>>> - if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
>>>>> - continue;
>>>>> + /* make sure the list is ordered as expected */
>>>>> + for (i = 1; i < fw_count; i++) {
>>>>> + /* Next platform is good: */
>>>>> + if (fw_blobs[i].p < fw_blobs[i - 1].p)
>>>>> + continue;
>>>>> - /* New must be before legacy: */
>>>>> - if (!fw_blobs[i].blob.legacy && fw_blobs[i -
>>>>> 1].blob.legacy)
>>>>> - goto bad;
>>>>> + /* Next platform revision is good: */
>>>>> + if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>>>>> + fw_blobs[i].rev < fw_blobs[i - 1].rev)
>>>>> + continue;
>>>>> - /* New to legacy also means 0.0 to X.Y (HuC), or
>>>>> X.0 to X.Y (GuC) */
>>>>> - if (fw_blobs[i].blob.legacy && !fw_blobs[i -
>>>>> 1].blob.legacy) {
>>>>> - if (!fw_blobs[i - 1].blob.major)
>>>>> - continue;
>>>>> + /* Platform/revision must be in order: */
>>>>> + if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>>>>> + fw_blobs[i].rev != fw_blobs[i - 1].rev)
>>>>> + goto bad;
>>>>> - if (fw_blobs[i].blob.major == fw_blobs[i -
>>>>> 1].blob.major)
>>>>> - continue;
>>>>> - }
>>>>> + /* Next major version is good: */
>>>>> + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
>>>>> + continue;
>>>>> - /* Major versions must be in order: */
>>>>> - if (fw_blobs[i].blob.major != fw_blobs[i -
>>>>> 1].blob.major)
>>>>> - goto bad;
>>>>> + /* New must be before legacy: */
>>>>> + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy)
>>>>> + goto bad;
>>>>> - /* Next minor version is good: */
>>>>> - if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
>>>>> + /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to
>>>>> X.Y (GuC) */
>>>>> + if (fw_blobs[i].blob.legacy && !fw_blobs[i -
>>>>> 1].blob.legacy) {
>>>>> + if (!fw_blobs[i - 1].blob.major)
>>>>> continue;
>>>>> - /* Minor versions must be in order: */
>>>>> - if (fw_blobs[i].blob.minor != fw_blobs[i -
>>>>> 1].blob.minor)
>>>>> - goto bad;
>>>>> -
>>>>> - /* Patch versions must be in order: */
>>>>> - if (fw_blobs[i].blob.patch <= fw_blobs[i -
>>>>> 1].blob.patch)
>>>>> + if (fw_blobs[i].blob.major == fw_blobs[i -
>>>>> 1].blob.major)
>>>>> continue;
>>>>> + }
>>>>> +
>>>>> + /* Major versions must be in order: */
>>>>> + if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
>>>>> + goto bad;
>>>>> +
>>>>> + /* Next minor version is good: */
>>>>> + if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
>>>>> + continue;
>>>>> +
>>>>> + /* Minor versions must be in order: */
>>>>> + if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
>>>>> + goto bad;
>>>>> +
>>>>> + /* Patch versions must be in order: */
>>>>> + if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
>>>>> + continue;
>>>>> bad:
>>>>> - drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>>>>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>>>>> - intel_uc_fw_type_repr(uc_fw->type),
>>>>> - intel_platform_name(fw_blobs[i - 1].p),
>>>>> fw_blobs[i - 1].rev,
>>>>> - fw_blobs[i - 1].blob.legacy ? "L" : "v",
>>>>> - fw_blobs[i - 1].blob.major,
>>>>> - fw_blobs[i - 1].blob.minor,
>>>>> - fw_blobs[i - 1].blob.patch,
>>>>> - intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
>>>>> - fw_blobs[i].blob.legacy ? "L" : "v",
>>>>> - fw_blobs[i].blob.major,
>>>>> - fw_blobs[i].blob.minor,
>>>>> - fw_blobs[i].blob.patch);
>>>>> -
>>>>> - uc_fw->file_selected.path = NULL;
>>>>> - }
>>>>> + drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>>>>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>>>>> + intel_uc_fw_type_repr(type),
>>>>> + intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i -
>>>>> 1].rev,
>>>>> + fw_blobs[i - 1].blob.legacy ? "L" : "v",
>>>>> + fw_blobs[i - 1].blob.major,
>>>>> + fw_blobs[i - 1].blob.minor,
>>>>> + fw_blobs[i - 1].blob.patch,
>>>>> + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
>>>>> + fw_blobs[i].blob.legacy ? "L" : "v",
>>>>> + fw_blobs[i].blob.major,
>>>>> + fw_blobs[i].blob.minor,
>>>>> + fw_blobs[i].blob.patch);
>>>>> + return false;
>>>>> }
>>>>> +
>>>>> + return true;
>>>>> }
>>>>> static const char *__override_guc_firmware_path(struct
>>>>> drm_i915_private *i915)
>>>>> @@ -443,6 +465,11 @@ void intel_uc_fw_init_early(struct
>>>>> intel_uc_fw *uc_fw,
>>>>> uc_fw->type = type;
>>>>> if (HAS_GT_UC(i915)) {
>>>>> + if (!validate_fw_table_type(i915, type)) {
>>>>> + intel_uc_fw_change_status(uc_fw,
>>>>> INTEL_UC_FIRMWARE_ERROR);
>>>>
>>>> In our hierarchy of firmware statuses, INTEL_UC_FIRMWARE_ERROR
>>>> includes the fact that the fw has been selected, which in turns
>>>> implies that file_selected.path is valid. this means that even with
>>>> enable_guc=0 the wants/uses_guc macro might end up returning true,
>>>> which is not something we want.
>>>>
>>>> Daniele
>>> Suggestions for a better plan? Add an another status enum? Nothing
>>> earlier in the sequence seems appropriate. And the init_early stack
>>> does not support returning error codes.
>>
>> I think the question here is: what are you expecting to happen in
>> case of error and on what platforms? let's say we have an invalid
>> table entry for ADLP, would the expectation be that all GuC platforms
>> won't boot, or just ADLP? And is that only if we have enable_guc set
>> to a positive value, or even if enable_guc=0?
> The intention is to totally break driver load on any table error.
>
> The reason being that someone is back porting a firmware update to
> ADL-P but breaks DG2 in the process. However, the are only intending
> to change ADL-P and so don't test on DG2. They therefore don't realise
> that the driver is now broken for someone else. Whereas, if we make
> any table error a fatal load failure irrespective of tested platform,
> enable_guc or other module params, etc. then it is guaranteed to be
> caught no matter what platform they test on.
>
Well, I guess if you are testing on a platform that does'nt use GuC/HuC
at all (or have enalble_guc=0) then none of this code would even run?
But then, if you are patching the firmware loading code then it is
reasonable to expect a test run on at least one firmware enabled platform.
John.
> John.
>
>>
>> Daniele
>>
>>>
>>> John.
>>>
>>>
>>>>
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> __uc_fw_auto_select(i915, uc_fw);
>>>>> __uc_fw_user_override(i915, uc_fw);
>>>>> }
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation
2023-04-29 0:32 ` John Harrison
@ 2023-04-29 1:21 ` Ceraolo Spurio, Daniele
0 siblings, 0 replies; 27+ messages in thread
From: Ceraolo Spurio, Daniele @ 2023-04-29 1:21 UTC (permalink / raw)
To: John Harrison, Intel-GFX; +Cc: DRI-Devel
On 4/28/2023 5:32 PM, John Harrison wrote:
> On 4/28/2023 17:30, John Harrison wrote:
>> On 4/28/2023 17:26, Ceraolo Spurio, Daniele wrote:
>>> On 4/28/2023 5:16 PM, John Harrison wrote:
>>>> On 4/28/2023 17:04, Ceraolo Spurio, Daniele wrote:
>>>>> On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
>>>>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>>>>
>>>>>> The validation of the firmware table was being done inside the code
>>>>>> for scanning the table for the next available firmware blob.
>>>>>> Which is
>>>>>> unnecessary. So pull it out into a separate function that is only
>>>>>> called once per blob type at init time.
>>>>>>
>>>>>> Also, drop the CONFIG_SELFTEST requirement and make errors terminal.
>>>>>> It was mentioned that potential issues with backports would not be
>>>>>> caught by regular pre-merge CI as that only occurs on tip not stable
>>>>>> branches. Making the validation unconditional and failing driver
>>>>>> load
>>>>>> on detecting of a problem ensures that such backports will also be
>>>>>> validated correctly.
>>>>>>
>>>>>> v2: Change to unconditionally fail module load on a validation error
>>>>>> (review feedback/discussion with Daniele).
>>>>>>
>>>>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 157
>>>>>> +++++++++++++----------
>>>>>> 1 file changed, 92 insertions(+), 65 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 c9cd9bb47577f..eb52e8db9ae0b 100644
>>>>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>>>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>>>>> @@ -233,20 +233,22 @@ struct fw_blobs_by_type {
>>>>>> u32 count;
>>>>>> };
>>>>>> +static const struct uc_fw_platform_requirement blobs_guc[] = {
>>>>>> + INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
>>>>>> GUC_FW_BLOB_MMP)
>>>>>> +};
>>>>>> +
>>>>>> +static const struct uc_fw_platform_requirement blobs_huc[] = {
>>>>>> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>>>>>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>>>>>> +};
>>>>>> +
>>>>>> +static const struct fw_blobs_by_type
>>>>>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>>>>>> + [INTEL_UC_FW_TYPE_GUC] = { blobs_guc, ARRAY_SIZE(blobs_guc) },
>>>>>> + [INTEL_UC_FW_TYPE_HUC] = { blobs_huc, ARRAY_SIZE(blobs_huc) },
>>>>>> +};
>>>>>> +
>>>>>> static void
>>>>>> __uc_fw_auto_select(struct drm_i915_private *i915, struct
>>>>>> intel_uc_fw *uc_fw)
>>>>>> {
>>>>>> - static const struct uc_fw_platform_requirement blobs_guc[] = {
>>>>>> - INTEL_GUC_FIRMWARE_DEFS(MAKE_FW_LIST, GUC_FW_BLOB,
>>>>>> GUC_FW_BLOB_MMP)
>>>>>> - };
>>>>>> - static const struct uc_fw_platform_requirement blobs_huc[] = {
>>>>>> - INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB,
>>>>>> HUC_FW_BLOB_MMP, HUC_FW_BLOB_GSC)
>>>>>> - };
>>>>>> - static const struct fw_blobs_by_type
>>>>>> blobs_all[INTEL_UC_FW_NUM_TYPES] = {
>>>>>> - [INTEL_UC_FW_TYPE_GUC] = { blobs_guc,
>>>>>> ARRAY_SIZE(blobs_guc) },
>>>>>> - [INTEL_UC_FW_TYPE_HUC] = { blobs_huc,
>>>>>> ARRAY_SIZE(blobs_huc) },
>>>>>> - };
>>>>>> - static bool verified[INTEL_UC_FW_NUM_TYPES];
>>>>>> const struct uc_fw_platform_requirement *fw_blobs;
>>>>>> enum intel_platform p = INTEL_INFO(i915)->platform;
>>>>>> u32 fw_count;
>>>>>> @@ -286,6 +288,11 @@ __uc_fw_auto_select(struct drm_i915_private
>>>>>> *i915, struct intel_uc_fw *uc_fw)
>>>>>> continue;
>>>>>> if (uc_fw->file_selected.path) {
>>>>>> + /*
>>>>>> + * Continuing an earlier search after a found blob
>>>>>> failed to load.
>>>>>> + * Once the previously chosen path has been found,
>>>>>> clear it out
>>>>>> + * and let the search continue from there.
>>>>>> + */
>>>>>> if (uc_fw->file_selected.path == blob->path)
>>>>>> uc_fw->file_selected.path = NULL;
>>>>>> @@ -306,76 +313,91 @@ __uc_fw_auto_select(struct
>>>>>> drm_i915_private *i915, struct intel_uc_fw *uc_fw)
>>>>>> /* Failed to find a match for the last attempt?! */
>>>>>> uc_fw->file_selected.path = NULL;
>>>>>> }
>>>>>> +}
>>>>>> - /* make sure the list is ordered as expected */
>>>>>> - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) &&
>>>>>> !verified[uc_fw->type]) {
>>>>>> - verified[uc_fw->type] = true;
>>>>>> +static bool validate_fw_table_type(struct drm_i915_private
>>>>>> *i915, enum intel_uc_fw_type type)
>>>>>> +{
>>>>>> + const struct uc_fw_platform_requirement *fw_blobs;
>>>>>> + u32 fw_count;
>>>>>> + int i;
>>>>>> - for (i = 1; i < fw_count; i++) {
>>>>>> - /* Next platform is good: */
>>>>>> - if (fw_blobs[i].p < fw_blobs[i - 1].p)
>>>>>> - continue;
>>>>>> + if (type >= ARRAY_SIZE(blobs_all)) {
>>>>>> + drm_err(&i915->drm, "No blob array for %s\n",
>>>>>> intel_uc_fw_type_repr(type));
>>>>>> + return false;
>>>>>> + }
>>>>>> - /* Next platform revision is good: */
>>>>>> - if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>>>>>> - fw_blobs[i].rev < fw_blobs[i - 1].rev)
>>>>>> - continue;
>>>>>> + fw_blobs = blobs_all[type].blobs;
>>>>>> + fw_count = blobs_all[type].count;
>>>>>> - /* Platform/revision must be in order: */
>>>>>> - if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>>>>>> - fw_blobs[i].rev != fw_blobs[i - 1].rev)
>>>>>> - goto bad;
>>>>>> + if (!fw_count)
>>>>>> + return true;
>>>>>> - /* Next major version is good: */
>>>>>> - if (fw_blobs[i].blob.major < fw_blobs[i -
>>>>>> 1].blob.major)
>>>>>> - continue;
>>>>>> + /* make sure the list is ordered as expected */
>>>>>> + for (i = 1; i < fw_count; i++) {
>>>>>> + /* Next platform is good: */
>>>>>> + if (fw_blobs[i].p < fw_blobs[i - 1].p)
>>>>>> + continue;
>>>>>> - /* New must be before legacy: */
>>>>>> - if (!fw_blobs[i].blob.legacy && fw_blobs[i -
>>>>>> 1].blob.legacy)
>>>>>> - goto bad;
>>>>>> + /* Next platform revision is good: */
>>>>>> + if (fw_blobs[i].p == fw_blobs[i - 1].p &&
>>>>>> + fw_blobs[i].rev < fw_blobs[i - 1].rev)
>>>>>> + continue;
>>>>>> - /* New to legacy also means 0.0 to X.Y (HuC), or
>>>>>> X.0 to X.Y (GuC) */
>>>>>> - if (fw_blobs[i].blob.legacy && !fw_blobs[i -
>>>>>> 1].blob.legacy) {
>>>>>> - if (!fw_blobs[i - 1].blob.major)
>>>>>> - continue;
>>>>>> + /* Platform/revision must be in order: */
>>>>>> + if (fw_blobs[i].p != fw_blobs[i - 1].p ||
>>>>>> + fw_blobs[i].rev != fw_blobs[i - 1].rev)
>>>>>> + goto bad;
>>>>>> - if (fw_blobs[i].blob.major == fw_blobs[i -
>>>>>> 1].blob.major)
>>>>>> - continue;
>>>>>> - }
>>>>>> + /* Next major version is good: */
>>>>>> + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major)
>>>>>> + continue;
>>>>>> - /* Major versions must be in order: */
>>>>>> - if (fw_blobs[i].blob.major != fw_blobs[i -
>>>>>> 1].blob.major)
>>>>>> - goto bad;
>>>>>> + /* New must be before legacy: */
>>>>>> + if (!fw_blobs[i].blob.legacy && fw_blobs[i -
>>>>>> 1].blob.legacy)
>>>>>> + goto bad;
>>>>>> - /* Next minor version is good: */
>>>>>> - if (fw_blobs[i].blob.minor < fw_blobs[i -
>>>>>> 1].blob.minor)
>>>>>> + /* New to legacy also means 0.0 to X.Y (HuC), or X.0 to
>>>>>> X.Y (GuC) */
>>>>>> + if (fw_blobs[i].blob.legacy && !fw_blobs[i -
>>>>>> 1].blob.legacy) {
>>>>>> + if (!fw_blobs[i - 1].blob.major)
>>>>>> continue;
>>>>>> - /* Minor versions must be in order: */
>>>>>> - if (fw_blobs[i].blob.minor != fw_blobs[i -
>>>>>> 1].blob.minor)
>>>>>> - goto bad;
>>>>>> -
>>>>>> - /* Patch versions must be in order: */
>>>>>> - if (fw_blobs[i].blob.patch <= fw_blobs[i -
>>>>>> 1].blob.patch)
>>>>>> + if (fw_blobs[i].blob.major == fw_blobs[i -
>>>>>> 1].blob.major)
>>>>>> continue;
>>>>>> + }
>>>>>> +
>>>>>> + /* Major versions must be in order: */
>>>>>> + if (fw_blobs[i].blob.major != fw_blobs[i - 1].blob.major)
>>>>>> + goto bad;
>>>>>> +
>>>>>> + /* Next minor version is good: */
>>>>>> + if (fw_blobs[i].blob.minor < fw_blobs[i - 1].blob.minor)
>>>>>> + continue;
>>>>>> +
>>>>>> + /* Minor versions must be in order: */
>>>>>> + if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
>>>>>> + goto bad;
>>>>>> +
>>>>>> + /* Patch versions must be in order: */
>>>>>> + if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
>>>>>> + continue;
>>>>>> bad:
>>>>>> - drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>>>>>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>>>>>> - intel_uc_fw_type_repr(uc_fw->type),
>>>>>> - intel_platform_name(fw_blobs[i - 1].p),
>>>>>> fw_blobs[i - 1].rev,
>>>>>> - fw_blobs[i - 1].blob.legacy ? "L" : "v",
>>>>>> - fw_blobs[i - 1].blob.major,
>>>>>> - fw_blobs[i - 1].blob.minor,
>>>>>> - fw_blobs[i - 1].blob.patch,
>>>>>> - intel_platform_name(fw_blobs[i].p),
>>>>>> fw_blobs[i].rev,
>>>>>> - fw_blobs[i].blob.legacy ? "L" : "v",
>>>>>> - fw_blobs[i].blob.major,
>>>>>> - fw_blobs[i].blob.minor,
>>>>>> - fw_blobs[i].blob.patch);
>>>>>> -
>>>>>> - uc_fw->file_selected.path = NULL;
>>>>>> - }
>>>>>> + drm_err(&i915->drm, "Invalid %s blob order: %s r%u
>>>>>> %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n",
>>>>>> + intel_uc_fw_type_repr(type),
>>>>>> + intel_platform_name(fw_blobs[i - 1].p), fw_blobs[i -
>>>>>> 1].rev,
>>>>>> + fw_blobs[i - 1].blob.legacy ? "L" : "v",
>>>>>> + fw_blobs[i - 1].blob.major,
>>>>>> + fw_blobs[i - 1].blob.minor,
>>>>>> + fw_blobs[i - 1].blob.patch,
>>>>>> + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
>>>>>> + fw_blobs[i].blob.legacy ? "L" : "v",
>>>>>> + fw_blobs[i].blob.major,
>>>>>> + fw_blobs[i].blob.minor,
>>>>>> + fw_blobs[i].blob.patch);
>>>>>> + return false;
>>>>>> }
>>>>>> +
>>>>>> + return true;
>>>>>> }
>>>>>> static const char *__override_guc_firmware_path(struct
>>>>>> drm_i915_private *i915)
>>>>>> @@ -443,6 +465,11 @@ void intel_uc_fw_init_early(struct
>>>>>> intel_uc_fw *uc_fw,
>>>>>> uc_fw->type = type;
>>>>>> if (HAS_GT_UC(i915)) {
>>>>>> + if (!validate_fw_table_type(i915, type)) {
>>>>>> + intel_uc_fw_change_status(uc_fw,
>>>>>> INTEL_UC_FIRMWARE_ERROR);
>>>>>
>>>>> In our hierarchy of firmware statuses, INTEL_UC_FIRMWARE_ERROR
>>>>> includes the fact that the fw has been selected, which in turns
>>>>> implies that file_selected.path is valid. this means that even
>>>>> with enable_guc=0 the wants/uses_guc macro might end up returning
>>>>> true, which is not something we want.
>>>>>
>>>>> Daniele
>>>> Suggestions for a better plan? Add an another status enum? Nothing
>>>> earlier in the sequence seems appropriate. And the init_early stack
>>>> does not support returning error codes.
>>>
>>> I think the question here is: what are you expecting to happen in
>>> case of error and on what platforms? let's say we have an invalid
>>> table entry for ADLP, would the expectation be that all GuC
>>> platforms won't boot, or just ADLP? And is that only if we have
>>> enable_guc set to a positive value, or even if enable_guc=0?
>> The intention is to totally break driver load on any table error.
>>
>> The reason being that someone is back porting a firmware update to
>> ADL-P but breaks DG2 in the process. However, the are only intending
>> to change ADL-P and so don't test on DG2. They therefore don't
>> realise that the driver is now broken for someone else. Whereas, if
>> we make any table error a fatal load failure irrespective of tested
>> platform, enable_guc or other module params, etc. then it is
>> guaranteed to be caught no matter what platform they test on.
>>
> Well, I guess if you are testing on a platform that does'nt use
> GuC/HuC at all (or have enalble_guc=0) then none of this code would
> even run? But then, if you are patching the firmware loading code then
> it is reasonable to expect a test run on at least one firmware enabled
> platform.
I was thinking we could add a new status code, but things then might
become complicated because both the submission mode (GuC vs execlists)
and the FW fetching are tied to the "wants" flag, so I'm not sure how
easy it would be to keep one but skip the other. An alternative option
could be to set the path to a known "invalid" value and fail the fetch
if we get that path (or just fail the fetch if path is NULL and/or
status is error).
Daniele
>
> John.
>
>> John.
>>
>>>
>>> Daniele
>>>
>>>>
>>>> John.
>>>>
>>>>
>>>>>
>>>>>> + return;
>>>>>> + }
>>>>>> +
>>>>>> __uc_fw_auto_select(i915, uc_fw);
>>>>>> __uc_fw_user_override(i915, uc_fw);
>>>>>> }
>>>>>
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Intel-gfx] [PATCH 5/6] drm/i915/uc: Reject duplicate entries in firmware table
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (3 preceding siblings ...)
2023-04-21 1:15 ` [Intel-gfx] [PATCH 4/6] drm/i915/uc: Enhancements to firmware table validation John.C.Harrison
@ 2023-04-21 1:15 ` John.C.Harrison
2023-04-29 0:08 ` Ceraolo Spurio, Daniele
2023-04-21 1:15 ` [Intel-gfx] [PATCH 6/6] drm/i915/uc: Make unexpected firmware versions an error in debug builds John.C.Harrison
` (10 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: John.C.Harrison @ 2023-04-21 1:15 UTC (permalink / raw)
To: Intel-GFX; +Cc: DRI-Devel
From: John Harrison <John.C.Harrison@Intel.com>
It was noticed that duplicate entries in the firmware table could cause
an infinite loop in the firmware loading code if that entry failed to
load. Duplicate entries are a bug anyway and so should never happen.
Ensure they don't by tweaking the table validation code to reject
duplicates.
For full m/m/p files, that can be done by simply tweaking the patch
level check to reject matching values. For reduced version entries,
the filename itself must be compared.
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 27 +++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 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 eb52e8db9ae0b..bc4011d55667c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -319,7 +319,7 @@ static bool validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_
{
const struct uc_fw_platform_requirement *fw_blobs;
u32 fw_count;
- int i;
+ int i, j;
if (type >= ARRAY_SIZE(blobs_all)) {
drm_err(&i915->drm, "No blob array for %s\n", intel_uc_fw_type_repr(type));
@@ -334,6 +334,27 @@ static bool validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_
/* make sure the list is ordered as expected */
for (i = 1; i < fw_count; i++) {
+ /* Versionless file names must be unique per platform: */
+ for (j = i + 1; j < fw_count; j++) {
+ /* Same platform? */
+ if (fw_blobs[i].p != fw_blobs[j].p)
+ continue;
+
+ if (fw_blobs[i].blob.path != fw_blobs[j].blob.path)
+ continue;
+
+ drm_err(&i915->drm, "Diplicaate %s blobs: %s r%u %s%d.%d.%d [%s] matches %s r%u %s%d.%d.%d [%s]\n",
+ intel_uc_fw_type_repr(type),
+ intel_platform_name(fw_blobs[j].p), fw_blobs[j].rev,
+ fw_blobs[j].blob.legacy ? "L" : "v",
+ fw_blobs[j].blob.major, fw_blobs[j].blob.minor,
+ fw_blobs[j].blob.patch, fw_blobs[j].blob.path,
+ intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
+ fw_blobs[i].blob.legacy ? "L" : "v",
+ fw_blobs[i].blob.major, fw_blobs[i].blob.minor,
+ fw_blobs[i].blob.patch, fw_blobs[i].blob.path);
+ }
+
/* Next platform is good: */
if (fw_blobs[i].p < fw_blobs[i - 1].p)
continue;
@@ -377,8 +398,8 @@ static bool validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_
if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
goto bad;
- /* Patch versions must be in order: */
- if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
+ /* Patch versions must be in order and unique: */
+ if (fw_blobs[i].blob.patch < fw_blobs[i - 1].blob.patch)
continue;
bad:
--
2.39.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 5/6] drm/i915/uc: Reject duplicate entries in firmware table
2023-04-21 1:15 ` [Intel-gfx] [PATCH 5/6] drm/i915/uc: Reject duplicate entries in firmware table John.C.Harrison
@ 2023-04-29 0:08 ` Ceraolo Spurio, Daniele
0 siblings, 0 replies; 27+ messages in thread
From: Ceraolo Spurio, Daniele @ 2023-04-29 0:08 UTC (permalink / raw)
To: John.C.Harrison, Intel-GFX; +Cc: DRI-Devel
On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> It was noticed that duplicate entries in the firmware table could cause
> an infinite loop in the firmware loading code if that entry failed to
> load. Duplicate entries are a bug anyway and so should never happen.
> Ensure they don't by tweaking the table validation code to reject
> duplicates.
>
> For full m/m/p files, that can be done by simply tweaking the patch
> level check to reject matching values. For reduced version entries,
> the filename itself must be compared.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 27 +++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 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 eb52e8db9ae0b..bc4011d55667c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -319,7 +319,7 @@ static bool validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_
> {
> const struct uc_fw_platform_requirement *fw_blobs;
> u32 fw_count;
> - int i;
> + int i, j;
>
> if (type >= ARRAY_SIZE(blobs_all)) {
> drm_err(&i915->drm, "No blob array for %s\n", intel_uc_fw_type_repr(type));
> @@ -334,6 +334,27 @@ static bool validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_
>
> /* make sure the list is ordered as expected */
> for (i = 1; i < fw_count; i++) {
> + /* Versionless file names must be unique per platform: */
> + for (j = i + 1; j < fw_count; j++) {
> + /* Same platform? */
> + if (fw_blobs[i].p != fw_blobs[j].p)
> + continue;
> +
> + if (fw_blobs[i].blob.path != fw_blobs[j].blob.path)
> + continue;
> +
> + drm_err(&i915->drm, "Diplicaate %s blobs: %s r%u %s%d.%d.%d [%s] matches %s r%u %s%d.%d.%d [%s]\n",
typo Diplicaate
> + intel_uc_fw_type_repr(type),
> + intel_platform_name(fw_blobs[j].p), fw_blobs[j].rev,
> + fw_blobs[j].blob.legacy ? "L" : "v",
> + fw_blobs[j].blob.major, fw_blobs[j].blob.minor,
> + fw_blobs[j].blob.patch, fw_blobs[j].blob.path,
> + intel_platform_name(fw_blobs[i].p), fw_blobs[i].rev,
nit: we could avoid printing the platform twice because you're
explicitly checking that it is the same earlier on. Not a blocked.
With the typo fixed:
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Daniele
> + fw_blobs[i].blob.legacy ? "L" : "v",
> + fw_blobs[i].blob.major, fw_blobs[i].blob.minor,
> + fw_blobs[i].blob.patch, fw_blobs[i].blob.path);
> + }
> +
> /* Next platform is good: */
> if (fw_blobs[i].p < fw_blobs[i - 1].p)
> continue;
> @@ -377,8 +398,8 @@ static bool validate_fw_table_type(struct drm_i915_private *i915, enum intel_uc_
> if (fw_blobs[i].blob.minor != fw_blobs[i - 1].blob.minor)
> goto bad;
>
> - /* Patch versions must be in order: */
> - if (fw_blobs[i].blob.patch <= fw_blobs[i - 1].blob.patch)
> + /* Patch versions must be in order and unique: */
> + if (fw_blobs[i].blob.patch < fw_blobs[i - 1].blob.patch)
> continue;
>
> bad:
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Intel-gfx] [PATCH 6/6] drm/i915/uc: Make unexpected firmware versions an error in debug builds
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (4 preceding siblings ...)
2023-04-21 1:15 ` [Intel-gfx] [PATCH 5/6] drm/i915/uc: Reject duplicate entries in firmware table John.C.Harrison
@ 2023-04-21 1:15 ` John.C.Harrison
2023-04-29 0:19 ` Ceraolo Spurio, Daniele
2023-04-21 2:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to uc firmare management (rev2) Patchwork
` (9 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: John.C.Harrison @ 2023-04-21 1:15 UTC (permalink / raw)
To: Intel-GFX; +Cc: DRI-Devel
From: John Harrison <John.C.Harrison@Intel.com>
If the DEBUG_GEM config option is set then escalate the 'unexpected
firmware version' message from a notice to an error. This will ensure
that the CI system treats such occurences as a failure and logs a bug
about it (or fails the pre-merge testing).
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 34 ++++++++++++++----------
1 file changed, 20 insertions(+), 14 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 bc4011d55667c..c5b21d17ca437 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -17,6 +17,12 @@
#include "i915_drv.h"
#include "i915_reg.h"
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
+#define UNEXPECTED gt_err
+#else
+#define UNEXPECTED gt_notice
+#endif
+
static inline struct intel_gt *
____uc_fw_to_gt(struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type)
{
@@ -828,10 +834,10 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
if (uc_fw->file_wanted.ver.major && uc_fw->file_selected.ver.major) {
/* Check the file's major version was as it claimed */
if (uc_fw->file_selected.ver.major != uc_fw->file_wanted.ver.major) {
- gt_notice(gt, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
- intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
- uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor,
- uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor);
+ UNEXPECTED(gt, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
+ intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
+ uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor,
+ uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor);
if (!intel_uc_fw_is_overridden(uc_fw)) {
err = -ENOEXEC;
goto fail;
@@ -849,16 +855,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
/* Preserve the version that was really wanted */
memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted));
- gt_notice(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n",
- intel_uc_fw_type_repr(uc_fw->type),
- uc_fw->file_wanted.path,
- uc_fw->file_wanted.ver.major,
- uc_fw->file_wanted.ver.minor,
- uc_fw->file_wanted.ver.patch,
- uc_fw->file_selected.path,
- uc_fw->file_selected.ver.major,
- uc_fw->file_selected.ver.minor,
- uc_fw->file_selected.ver.patch);
+ UNEXPECTED(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n",
+ intel_uc_fw_type_repr(uc_fw->type),
+ uc_fw->file_wanted.path,
+ uc_fw->file_wanted.ver.major,
+ uc_fw->file_wanted.ver.minor,
+ uc_fw->file_wanted.ver.patch,
+ uc_fw->file_selected.path,
+ uc_fw->file_selected.ver.major,
+ uc_fw->file_selected.ver.minor,
+ uc_fw->file_selected.ver.patch);
gt_info(gt, "Consider updating your linux-firmware pkg or downloading from %s\n",
INTEL_UC_FIRMWARE_URL);
}
--
2.39.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 6/6] drm/i915/uc: Make unexpected firmware versions an error in debug builds
2023-04-21 1:15 ` [Intel-gfx] [PATCH 6/6] drm/i915/uc: Make unexpected firmware versions an error in debug builds John.C.Harrison
@ 2023-04-29 0:19 ` Ceraolo Spurio, Daniele
2023-04-29 0:21 ` John Harrison
0 siblings, 1 reply; 27+ messages in thread
From: Ceraolo Spurio, Daniele @ 2023-04-29 0:19 UTC (permalink / raw)
To: John.C.Harrison, Intel-GFX; +Cc: DRI-Devel
On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> If the DEBUG_GEM config option is set then escalate the 'unexpected
> firmware version' message from a notice to an error. This will ensure
> that the CI system treats such occurences as a failure and logs a bug
> about it (or fails the pre-merge testing).
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 34 ++++++++++++++----------
> 1 file changed, 20 insertions(+), 14 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 bc4011d55667c..c5b21d17ca437 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -17,6 +17,12 @@
> #include "i915_drv.h"
> #include "i915_reg.h"
>
> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
> +#define UNEXPECTED gt_err
> +#else
> +#define UNEXPECTED gt_notice
I tried to find an alternative word that had the same number of
characters as "gt_notice" so that we could avoid the indent in the diff,
but nothing came to mind, so:
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Daniele
> +#endif
> +
> static inline struct intel_gt *
> ____uc_fw_to_gt(struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type)
> {
> @@ -828,10 +834,10 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> if (uc_fw->file_wanted.ver.major && uc_fw->file_selected.ver.major) {
> /* Check the file's major version was as it claimed */
> if (uc_fw->file_selected.ver.major != uc_fw->file_wanted.ver.major) {
> - gt_notice(gt, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
> - uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor,
> - uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor);
> + UNEXPECTED(gt, "%s firmware %s: unexpected version: %u.%u != %u.%u\n",
> + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path,
> + uc_fw->file_selected.ver.major, uc_fw->file_selected.ver.minor,
> + uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor);
> if (!intel_uc_fw_is_overridden(uc_fw)) {
> err = -ENOEXEC;
> goto fail;
> @@ -849,16 +855,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> /* Preserve the version that was really wanted */
> memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted));
>
> - gt_notice(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n",
> - intel_uc_fw_type_repr(uc_fw->type),
> - uc_fw->file_wanted.path,
> - uc_fw->file_wanted.ver.major,
> - uc_fw->file_wanted.ver.minor,
> - uc_fw->file_wanted.ver.patch,
> - uc_fw->file_selected.path,
> - uc_fw->file_selected.ver.major,
> - uc_fw->file_selected.ver.minor,
> - uc_fw->file_selected.ver.patch);
> + UNEXPECTED(gt, "%s firmware %s (%d.%d.%d) is recommended, but only %s (%d.%d.%d) was found\n",
> + intel_uc_fw_type_repr(uc_fw->type),
> + uc_fw->file_wanted.path,
> + uc_fw->file_wanted.ver.major,
> + uc_fw->file_wanted.ver.minor,
> + uc_fw->file_wanted.ver.patch,
> + uc_fw->file_selected.path,
> + uc_fw->file_selected.ver.major,
> + uc_fw->file_selected.ver.minor,
> + uc_fw->file_selected.ver.patch);
> gt_info(gt, "Consider updating your linux-firmware pkg or downloading from %s\n",
> INTEL_UC_FIRMWARE_URL);
> }
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [Intel-gfx] [PATCH 6/6] drm/i915/uc: Make unexpected firmware versions an error in debug builds
2023-04-29 0:19 ` Ceraolo Spurio, Daniele
@ 2023-04-29 0:21 ` John Harrison
0 siblings, 0 replies; 27+ messages in thread
From: John Harrison @ 2023-04-29 0:21 UTC (permalink / raw)
To: Ceraolo Spurio, Daniele, Intel-GFX; +Cc: DRI-Devel
On 4/28/2023 17:19, Ceraolo Spurio, Daniele wrote:
> On 4/20/2023 6:15 PM, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> If the DEBUG_GEM config option is set then escalate the 'unexpected
>> firmware version' message from a notice to an error. This will ensure
>> that the CI system treats such occurences as a failure and logs a bug
>> about it (or fails the pre-merge testing).
>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 34 ++++++++++++++----------
>> 1 file changed, 20 insertions(+), 14 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 bc4011d55667c..c5b21d17ca437 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> @@ -17,6 +17,12 @@
>> #include "i915_drv.h"
>> #include "i915_reg.h"
>> +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM)
>> +#define UNEXPECTED gt_err
>> +#else
>> +#define UNEXPECTED gt_notice
>
> I tried to find an alternative word that had the same number of
> characters as "gt_notice" so that we could avoid the indent in the
> diff, but nothing came to mind, so:
Yup, already went through that! And it didn't seem worth it to
abbreviate by dropping a single random vowel.
John.
>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> Daniele
>
>> +#endif
>> +
>> static inline struct intel_gt *
>> ____uc_fw_to_gt(struct intel_uc_fw *uc_fw, enum intel_uc_fw_type type)
>> {
>> @@ -828,10 +834,10 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>> if (uc_fw->file_wanted.ver.major &&
>> uc_fw->file_selected.ver.major) {
>> /* Check the file's major version was as it claimed */
>> if (uc_fw->file_selected.ver.major !=
>> uc_fw->file_wanted.ver.major) {
>> - gt_notice(gt, "%s firmware %s: unexpected version: %u.%u
>> != %u.%u\n",
>> - intel_uc_fw_type_repr(uc_fw->type),
>> uc_fw->file_selected.path,
>> - uc_fw->file_selected.ver.major,
>> uc_fw->file_selected.ver.minor,
>> - uc_fw->file_wanted.ver.major,
>> uc_fw->file_wanted.ver.minor);
>> + UNEXPECTED(gt, "%s firmware %s: unexpected version:
>> %u.%u != %u.%u\n",
>> + intel_uc_fw_type_repr(uc_fw->type),
>> uc_fw->file_selected.path,
>> + uc_fw->file_selected.ver.major,
>> uc_fw->file_selected.ver.minor,
>> + uc_fw->file_wanted.ver.major,
>> uc_fw->file_wanted.ver.minor);
>> if (!intel_uc_fw_is_overridden(uc_fw)) {
>> err = -ENOEXEC;
>> goto fail;
>> @@ -849,16 +855,16 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>> /* Preserve the version that was really wanted */
>> memcpy(&uc_fw->file_wanted, &file_ideal,
>> sizeof(uc_fw->file_wanted));
>> - gt_notice(gt, "%s firmware %s (%d.%d.%d) is recommended,
>> but only %s (%d.%d.%d) was found\n",
>> - intel_uc_fw_type_repr(uc_fw->type),
>> - uc_fw->file_wanted.path,
>> - uc_fw->file_wanted.ver.major,
>> - uc_fw->file_wanted.ver.minor,
>> - uc_fw->file_wanted.ver.patch,
>> - uc_fw->file_selected.path,
>> - uc_fw->file_selected.ver.major,
>> - uc_fw->file_selected.ver.minor,
>> - uc_fw->file_selected.ver.patch);
>> + UNEXPECTED(gt, "%s firmware %s (%d.%d.%d) is recommended,
>> but only %s (%d.%d.%d) was found\n",
>> + intel_uc_fw_type_repr(uc_fw->type),
>> + uc_fw->file_wanted.path,
>> + uc_fw->file_wanted.ver.major,
>> + uc_fw->file_wanted.ver.minor,
>> + uc_fw->file_wanted.ver.patch,
>> + uc_fw->file_selected.path,
>> + uc_fw->file_selected.ver.major,
>> + uc_fw->file_selected.ver.minor,
>> + uc_fw->file_selected.ver.patch);
>> gt_info(gt, "Consider updating your linux-firmware pkg or
>> downloading from %s\n",
>> INTEL_UC_FIRMWARE_URL);
>> }
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to uc firmare management (rev2)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (5 preceding siblings ...)
2023-04-21 1:15 ` [Intel-gfx] [PATCH 6/6] drm/i915/uc: Make unexpected firmware versions an error in debug builds John.C.Harrison
@ 2023-04-21 2:26 ` Patchwork
2023-04-21 2:26 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
` (8 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-21 2:26 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
== Series Details ==
Series: Improvements to uc firmare management (rev2)
URL : https://patchwork.freedesktop.org/series/116517/
State : warning
== Summary ==
Error: dim checkpatch failed
5f4add3122b0 drm/i915/guc: Decode another GuC load failure case
2b35e460865b drm/i915/guc: Print status register when waiting for GuC to load
5fcf4600dbe0 drm/i915/uc: Track patch level versions on reduced version firmware files
-:61: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'major_' - possible side-effects?
#61: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:200:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
-:61: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'minor_' - possible side-effects?
#61: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:200:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
-:61: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'patch_' - possible side-effects?
#61: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:200:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
total: 0 errors, 0 warnings, 3 checks, 73 lines checked
0ad23ab7f74c drm/i915/uc: Enhancements to firmware table validation
8d3a53230905 drm/i915/uc: Reject duplicate entries in firmware table
0eae0615810a drm/i915/uc: Make unexpected firmware versions an error in debug builds
^ permalink raw reply [flat|nested] 27+ messages in thread* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Improvements to uc firmare management (rev2)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (6 preceding siblings ...)
2023-04-21 2:26 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to uc firmare management (rev2) Patchwork
@ 2023-04-21 2:26 ` Patchwork
2023-04-21 2:43 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
` (7 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-21 2:26 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
== Series Details ==
Series: Improvements to uc firmare management (rev2)
URL : https://patchwork.freedesktop.org/series/116517/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 27+ messages in thread* [Intel-gfx] ✗ Fi.CI.BAT: failure for Improvements to uc firmare management (rev2)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (7 preceding siblings ...)
2023-04-21 2:26 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2023-04-21 2:43 ` Patchwork
2023-04-21 20:54 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to uc firmare management (rev3) Patchwork
` (6 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-21 2:43 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 5949 bytes --]
== Series Details ==
Series: Improvements to uc firmare management (rev2)
URL : https://patchwork.freedesktop.org/series/116517/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_13036 -> Patchwork_116517v2
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_116517v2 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_116517v2, 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_116517v2/index.html
Participating hosts (37 -> 37)
------------------------------
Additional (1): fi-kbl-soraka
Missing (1): fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_116517v2:
### IGT changes ###
#### Possible regressions ####
* igt@runner@aborted:
- fi-apl-guc: NOTRUN -> [FAIL][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/fi-apl-guc/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_116517v2 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_huc_copy@huc-copy:
- fi-kbl-soraka: NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@basic:
- fi-kbl-soraka: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html
* igt@i915_selftest@live@gt_pm:
- fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][4] ([i915#1886])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html
* igt@i915_selftest@live@slpc:
- bat-rpls-1: NOTRUN -> [DMESG-FAIL][5] ([i915#6367] / [i915#7996])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/bat-rpls-1/igt@i915_selftest@live@slpc.html
* igt@kms_chamelium_frames@hdmi-crc-fast:
- fi-kbl-soraka: NOTRUN -> [SKIP][6] ([fdo#109271]) +16 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html
* igt@kms_chamelium_hpd@common-hpd-after-suspend:
- bat-rpls-1: NOTRUN -> [SKIP][7] ([i915#7828])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/bat-rpls-1/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
* igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1:
- bat-dg2-8: [PASS][8] -> [FAIL][9] ([i915#7932])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13036/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-d-dp-1.html
* igt@kms_pipe_crc_basic@suspend-read-crc:
- bat-rpls-1: NOTRUN -> [SKIP][10] ([i915#1845])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/bat-rpls-1/igt@kms_pipe_crc_basic@suspend-read-crc.html
#### Possible fixes ####
* igt@i915_selftest@live@hangcheck:
- fi-skl-guc: [DMESG-WARN][11] ([i915#8073]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13036/fi-skl-guc/igt@i915_selftest@live@hangcheck.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/fi-skl-guc/igt@i915_selftest@live@hangcheck.html
* igt@i915_selftest@live@reset:
- bat-rpls-1: [ABORT][13] ([i915#4983] / [i915#7981]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13036/bat-rpls-1/igt@i915_selftest@live@reset.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/bat-rpls-1/igt@i915_selftest@live@reset.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
[i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981
[i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996
[i915#8073]: https://gitlab.freedesktop.org/drm/intel/issues/8073
Build changes
-------------
* Linux: CI_DRM_13036 -> Patchwork_116517v2
CI-20190529: 20190529
CI_DRM_13036: f373f2faeb2f25a27dc83bd1326b43315be1a6a4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7263: a6bd8f415c4ec41b5a014c7db47e46c81ffd0074 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116517v2: f373f2faeb2f25a27dc83bd1326b43315be1a6a4 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
f4f328619600 drm/i915/uc: Make unexpected firmware versions an error in debug builds
6b2de09e0554 drm/i915/uc: Reject duplicate entries in firmware table
21490bd9fc18 drm/i915/uc: Enhancements to firmware table validation
680e844808c0 drm/i915/uc: Track patch level versions on reduced version firmware files
cea29917fe2d drm/i915/guc: Print status register when waiting for GuC to load
d47587460f75 drm/i915/guc: Decode another GuC load failure case
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v2/index.html
[-- Attachment #2: Type: text/html, Size: 7000 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to uc firmare management (rev3)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (8 preceding siblings ...)
2023-04-21 2:43 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2023-04-21 20:54 ` Patchwork
2023-04-21 20:54 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
` (5 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-21 20:54 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
== Series Details ==
Series: Improvements to uc firmare management (rev3)
URL : https://patchwork.freedesktop.org/series/116517/
State : warning
== Summary ==
Error: dim checkpatch failed
c9c1858ce1b5 drm/i915/guc: Decode another GuC load failure case
78137da72e02 drm/i915/guc: Print status register when waiting for GuC to load
b5a9b016719f drm/i915/uc: Track patch level versions on reduced version firmware files
-:61: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'major_' - possible side-effects?
#61: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:200:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
-:61: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'minor_' - possible side-effects?
#61: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:200:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
-:61: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'patch_' - possible side-effects?
#61: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:200:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
total: 0 errors, 0 warnings, 3 checks, 73 lines checked
1280d8f010c6 drm/i915/uc: Enhancements to firmware table validation
a4d2663e570b drm/i915/uc: Reject duplicate entries in firmware table
cd22dd08e779 drm/i915/uc: Make unexpected firmware versions an error in debug builds
^ permalink raw reply [flat|nested] 27+ messages in thread* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Improvements to uc firmare management (rev3)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (9 preceding siblings ...)
2023-04-21 20:54 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to uc firmare management (rev3) Patchwork
@ 2023-04-21 20:54 ` Patchwork
2023-04-21 21:05 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
` (4 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-21 20:54 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
== Series Details ==
Series: Improvements to uc firmare management (rev3)
URL : https://patchwork.freedesktop.org/series/116517/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 27+ messages in thread* [Intel-gfx] ✗ Fi.CI.BAT: failure for Improvements to uc firmare management (rev3)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (10 preceding siblings ...)
2023-04-21 20:54 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2023-04-21 21:05 ` Patchwork
2023-04-25 1:21 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to uc firmare management (rev4) Patchwork
` (3 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-21 21:05 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 4901 bytes --]
== Series Details ==
Series: Improvements to uc firmare management (rev3)
URL : https://patchwork.freedesktop.org/series/116517/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_13043 -> Patchwork_116517v3
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_116517v3 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_116517v3, 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_116517v3/index.html
Participating hosts (36 -> 36)
------------------------------
Additional (1): bat-mtlp-8
Missing (1): fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_116517v3:
### IGT changes ###
#### Possible regressions ####
* igt@kms_flip@basic-flip-vs-wf_vblank@a-hdmi-a1:
- fi-elk-e7500: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13043/fi-elk-e7500/igt@kms_flip@basic-flip-vs-wf_vblank@a-hdmi-a1.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v3/fi-elk-e7500/igt@kms_flip@basic-flip-vs-wf_vblank@a-hdmi-a1.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@i915_selftest@live@hugepages:
- {bat-mtlp-8}: NOTRUN -> [DMESG-FAIL][3]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v3/bat-mtlp-8/igt@i915_selftest@live@hugepages.html
Known issues
------------
Here are the changes found in Patchwork_116517v3 that come from known issues:
### IGT changes ###
#### Warnings ####
* igt@i915_selftest@live@slpc:
- bat-rpls-1: [DMESG-FAIL][4] ([i915#6367] / [i915#7996]) -> [DMESG-FAIL][5] ([i915#6367])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13043/bat-rpls-1/igt@i915_selftest@live@slpc.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v3/bat-rpls-1/igt@i915_selftest@live@slpc.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
[i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
[i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
[i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996
[i915#8346]: https://gitlab.freedesktop.org/drm/intel/issues/8346
[i915#8368]: https://gitlab.freedesktop.org/drm/intel/issues/8368
[i915#8369]: https://gitlab.freedesktop.org/drm/intel/issues/8369
[i915#8379]: https://gitlab.freedesktop.org/drm/intel/issues/8379
Build changes
-------------
* Linux: CI_DRM_13043 -> Patchwork_116517v3
CI-20190529: 20190529
CI_DRM_13043: 2fa9c266135355c9993507d7c27cc6722956bfec @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7264: 2f0a07378e58e5c7d7b589b39ace7e3a2317f6b2 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116517v3: 2fa9c266135355c9993507d7c27cc6722956bfec @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
9124e14cc346 drm/i915/uc: Make unexpected firmware versions an error in debug builds
285925456608 drm/i915/uc: Reject duplicate entries in firmware table
9d456229f40e drm/i915/uc: Enhancements to firmware table validation
f88b23c6bd08 drm/i915/uc: Track patch level versions on reduced version firmware files
0230c2d1ec3d drm/i915/guc: Print status register when waiting for GuC to load
99d1c102d0f1 drm/i915/guc: Decode another GuC load failure case
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v3/index.html
[-- Attachment #2: Type: text/html, Size: 4323 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to uc firmare management (rev4)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (11 preceding siblings ...)
2023-04-21 21:05 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2023-04-25 1:21 ` Patchwork
2023-04-25 1:21 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-25 1:21 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
== Series Details ==
Series: Improvements to uc firmare management (rev4)
URL : https://patchwork.freedesktop.org/series/116517/
State : warning
== Summary ==
Error: dim checkpatch failed
767fe16369d4 drm/i915/guc: Decode another GuC load failure case
95f43703948d drm/i915/guc: Print status register when waiting for GuC to load
739f30be54fc drm/i915/uc: Track patch level versions on reduced version firmware files
-:61: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'major_' - possible side-effects?
#61: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:200:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
-:61: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'minor_' - possible side-effects?
#61: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:200:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
-:61: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'patch_' - possible side-effects?
#61: FILE: drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c:200:
+#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \
+ UC_FW_BLOB_NEW(major_, minor_, patch_, false, \
+ MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_, patch_))
total: 0 errors, 0 warnings, 3 checks, 73 lines checked
e4b70f34344b drm/i915/uc: Enhancements to firmware table validation
1f175721171c drm/i915/uc: Reject duplicate entries in firmware table
4d4fd1a1efe2 drm/i915/uc: Make unexpected firmware versions an error in debug builds
^ permalink raw reply [flat|nested] 27+ messages in thread* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for Improvements to uc firmare management (rev4)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (12 preceding siblings ...)
2023-04-25 1:21 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Improvements to uc firmare management (rev4) Patchwork
@ 2023-04-25 1:21 ` Patchwork
2023-04-25 1:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-04-25 8:54 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-25 1:21 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
== Series Details ==
Series: Improvements to uc firmare management (rev4)
URL : https://patchwork.freedesktop.org/series/116517/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 27+ messages in thread* [Intel-gfx] ✓ Fi.CI.BAT: success for Improvements to uc firmare management (rev4)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (13 preceding siblings ...)
2023-04-25 1:21 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2023-04-25 1:34 ` Patchwork
2023-04-25 8:54 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-25 1:34 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 3641 bytes --]
== Series Details ==
Series: Improvements to uc firmare management (rev4)
URL : https://patchwork.freedesktop.org/series/116517/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13056 -> Patchwork_116517v4
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/index.html
Participating hosts (39 -> 38)
------------------------------
Missing (1): fi-snb-2520m
Known issues
------------
Here are the changes found in Patchwork_116517v4 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-kbl-soraka: [PASS][1] -> [DMESG-FAIL][2] ([i915#5334] / [i915#7872])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
* igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1:
- bat-dg2-8: [PASS][3] -> [FAIL][4] ([i915#7932]) +1 similar issue
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-d-dp-1.html
#### Possible fixes ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-glk-j4005: [DMESG-FAIL][5] ([i915#5334]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/fi-glk-j4005/igt@i915_selftest@live@gt_heartbeat.html
* igt@kms_addfb_basic@unused-modifier:
- fi-kbl-soraka: [INCOMPLETE][7] -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/fi-kbl-soraka/igt@kms_addfb_basic@unused-modifier.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/fi-kbl-soraka/igt@kms_addfb_basic@unused-modifier.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
[i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
[i915#8176]: https://gitlab.freedesktop.org/drm/intel/issues/8176
[i915#8361]: https://gitlab.freedesktop.org/drm/intel/issues/8361
Build changes
-------------
* Linux: CI_DRM_13056 -> Patchwork_116517v4
CI-20190529: 20190529
CI_DRM_13056: 308c0163d9e46238948942260e6d2abcad3d8bff @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7267: a267f0236e06fc282e3dc3b8c7d76f9ed6088d9b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116517v4: 308c0163d9e46238948942260e6d2abcad3d8bff @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
73551022be0b drm/i915/uc: Make unexpected firmware versions an error in debug builds
88e712f9b743 drm/i915/uc: Reject duplicate entries in firmware table
167ae011abe2 drm/i915/uc: Enhancements to firmware table validation
840667ca3eca drm/i915/uc: Track patch level versions on reduced version firmware files
bc3e7163bbb3 drm/i915/guc: Print status register when waiting for GuC to load
5f41dc0364f0 drm/i915/guc: Decode another GuC load failure case
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/index.html
[-- Attachment #2: Type: text/html, Size: 4310 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* [Intel-gfx] ✓ Fi.CI.IGT: success for Improvements to uc firmare management (rev4)
2023-04-21 1:15 [Intel-gfx] [PATCH 0/6] Improvements to uc firmare management John.C.Harrison
` (14 preceding siblings ...)
2023-04-25 1:34 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2023-04-25 8:54 ` Patchwork
15 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2023-04-25 8:54 UTC (permalink / raw)
To: john.c.harrison; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 20051 bytes --]
== Series Details ==
Series: Improvements to uc firmare management (rev4)
URL : https://patchwork.freedesktop.org/series/116517/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_13056_full -> Patchwork_116517v4_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (7 -> 7)
------------------------------
No changes in participating hosts
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_116517v4_full:
### IGT changes ###
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@i915_module_load@reload-with-fault-injection:
- {shard-dg1}: NOTRUN -> [DMESG-WARN][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-dg1-12/igt@i915_module_load@reload-with-fault-injection.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-pwrite:
- {shard-dg1}: [PASS][2] -> [SKIP][3] +5 similar issues
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-dg1-16/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-pwrite.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-dg1-12/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-shrfb-draw-pwrite.html
* igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-mmap-wc:
- {shard-dg1}: [SKIP][4] ([fdo#111825]) -> [SKIP][5] +19 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-dg1-17/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-mmap-wc.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-dg1-12/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-mmap-wc.html
* igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-gtt:
- {shard-dg1}: [SKIP][6] ([i915#4833]) -> [SKIP][7] +5 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-dg1-16/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-gtt.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-dg1-12/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-gtt.html
* igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-blt:
- {shard-dg1}: NOTRUN -> [SKIP][8] +50 similar issues
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-dg1-12/igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-blt.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
- {shard-dg1}: [SKIP][9] ([i915#3458]) -> [SKIP][10] +8 similar issues
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-dg1-16/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-dg1-12/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc.html
Known issues
------------
Here are the changes found in Patchwork_116517v4_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_fair@basic-pace-share@rcs0:
- shard-glk: [PASS][11] -> [FAIL][12] ([i915#2842])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-glk5/igt@gem_exec_fair@basic-pace-share@rcs0.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-glk5/igt@gem_exec_fair@basic-pace-share@rcs0.html
* igt@gem_exec_fair@basic-pace-solo@rcs0:
- shard-apl: [PASS][13] -> [FAIL][14] ([i915#2842])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-apl1/igt@gem_exec_fair@basic-pace-solo@rcs0.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-apl6/igt@gem_exec_fair@basic-pace-solo@rcs0.html
* igt@i915_pm_rps@reset:
- shard-snb: [PASS][15] -> [INCOMPLETE][16] ([i915#7790])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-snb4/igt@i915_pm_rps@reset.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-snb4/igt@i915_pm_rps@reset.html
* igt@i915_selftest@live@gt_heartbeat:
- shard-glk: [PASS][17] -> [DMESG-FAIL][18] ([i915#5334])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-glk1/igt@i915_selftest@live@gt_heartbeat.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-glk1/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@migrate:
- shard-glk: [PASS][19] -> [DMESG-FAIL][20] ([i915#7384])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-glk1/igt@i915_selftest@live@migrate.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-glk1/igt@i915_selftest@live@migrate.html
* igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
- shard-apl: [PASS][21] -> [FAIL][22] ([i915#2346])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-apl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-apl7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
- shard-glk: [PASS][23] -> [FAIL][24] ([i915#2346])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-glk4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-glk4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
* igt@kms_dither@fb-8bpc-vs-panel-6bpc@pipe-a-hdmi-a-1:
- shard-glk: NOTRUN -> [SKIP][25] ([fdo#109271])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-glk9/igt@kms_dither@fb-8bpc-vs-panel-6bpc@pipe-a-hdmi-a-1.html
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-a-vga-1:
- shard-snb: NOTRUN -> [SKIP][26] ([fdo#109271]) +36 similar issues
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-snb5/igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-25@pipe-a-vga-1.html
#### Possible fixes ####
* igt@drm_fdinfo@most-busy-idle-check-all@rcs0:
- {shard-rkl}: [FAIL][27] ([i915#7742]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-rkl-3/igt@drm_fdinfo@most-busy-idle-check-all@rcs0.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-rkl-7/igt@drm_fdinfo@most-busy-idle-check-all@rcs0.html
* igt@gem_barrier_race@remote-request@rcs0:
- {shard-tglu}: [ABORT][29] ([i915#8211]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-tglu-6/igt@gem_barrier_race@remote-request@rcs0.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-tglu-6/igt@gem_barrier_race@remote-request@rcs0.html
* igt@gem_exec_fair@basic-deadline:
- {shard-rkl}: [FAIL][31] ([i915#2846]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-rkl-6/igt@gem_exec_fair@basic-deadline.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-rkl-1/igt@gem_exec_fair@basic-deadline.html
* igt@gem_exec_fair@basic-none-share@rcs0:
- shard-glk: [FAIL][33] ([i915#2842]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-glk2/igt@gem_exec_fair@basic-none-share@rcs0.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-glk5/igt@gem_exec_fair@basic-none-share@rcs0.html
* igt@gem_exec_fair@basic-pace-share@rcs0:
- {shard-tglu}: [FAIL][35] ([i915#2842]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-tglu-3/igt@gem_exec_fair@basic-pace-share@rcs0.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-tglu-4/igt@gem_exec_fair@basic-pace-share@rcs0.html
* igt@gem_ppgtt@blt-vs-render-ctxn:
- shard-snb: [FAIL][37] ([i915#8295]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-snb1/igt@gem_ppgtt@blt-vs-render-ctxn.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-snb5/igt@gem_ppgtt@blt-vs-render-ctxn.html
* igt@i915_pm_dc@dc6-dpms:
- {shard-tglu}: [FAIL][39] ([i915#3989] / [i915#454]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-tglu-3/igt@i915_pm_dc@dc6-dpms.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-tglu-9/igt@i915_pm_dc@dc6-dpms.html
* igt@i915_pm_rpm@dpms-lpsp:
- {shard-rkl}: [SKIP][41] ([i915#1397]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-rkl-4/igt@i915_pm_rpm@dpms-lpsp.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-rkl-7/igt@i915_pm_rpm@dpms-lpsp.html
* igt@kms_cursor_legacy@forked-move@pipe-b:
- {shard-dg1}: [INCOMPLETE][43] ([i915#8011] / [i915#8347]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-dg1-14/igt@kms_cursor_legacy@forked-move@pipe-b.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-dg1-16/igt@kms_cursor_legacy@forked-move@pipe-b.html
* igt@kms_flip@2x-flip-vs-expired-vblank@ac-hdmi-a1-hdmi-a2:
- shard-glk: [FAIL][45] ([i915#79]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-glk4/igt@kms_flip@2x-flip-vs-expired-vblank@ac-hdmi-a1-hdmi-a2.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank@ac-hdmi-a1-hdmi-a2.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1:
- shard-apl: [FAIL][47] ([i915#79]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13056/shard-apl1/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_116517v4/shard-apl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-dp1.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
[fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
[fdo#109302]: https://bugs.freedesktop.org/show_bug.cgi?id=109302
[fdo#109307]: https://bugs.freedesktop.org/show_bug.cgi?id=109307
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
[fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
[fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
[i915#1839]: https://gitlab.freedesktop.org/drm/intel/issues/1839
[i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
[i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2433]: https://gitlab.freedesktop.org/drm/intel/issues/2433
[i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
[i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
[i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
[i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
[i915#284]: https://gitlab.freedesktop.org/drm/intel/issues/284
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
[i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
[i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
[i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
[i915#3361]: https://gitlab.freedesktop.org/drm/intel/issues/3361
[i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
[i915#3469]: https://gitlab.freedesktop.org/drm/intel/issues/3469
[i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
[i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
[i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
[i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
[i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
[i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
[i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#3938]: https://gitlab.freedesktop.org/drm/intel/issues/3938
[i915#3952]: https://gitlab.freedesktop.org/drm/intel/issues/3952
[i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
[i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
[i915#404]: https://gitlab.freedesktop.org/drm/intel/issues/404
[i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
[i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
[i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215
[i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
[i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
[i915#4387]: https://gitlab.freedesktop.org/drm/intel/issues/4387
[i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
[i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
[i915#4565]: https://gitlab.freedesktop.org/drm/intel/issues/4565
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4771]: https://gitlab.freedesktop.org/drm/intel/issues/4771
[i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
[i915#4816]: https://gitlab.freedesktop.org/drm/intel/issues/4816
[i915#4818]: https://gitlab.freedesktop.org/drm/intel/issues/4818
[i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
[i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
[i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
[i915#4873]: https://gitlab.freedesktop.org/drm/intel/issues/4873
[i915#4879]: https://gitlab.freedesktop.org/drm/intel/issues/4879
[i915#4880]: https://gitlab.freedesktop.org/drm/intel/issues/4880
[i915#4881]: https://gitlab.freedesktop.org/drm/intel/issues/4881
[i915#4885]: https://gitlab.freedesktop.org/drm/intel/issues/4885
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
[i915#5289]: https://gitlab.freedesktop.org/drm/intel/issues/5289
[i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
[i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
[i915#5493]: https://gitlab.freedesktop.org/drm/intel/issues/5493
[i915#5563]: https://gitlab.freedesktop.org/drm/intel/issues/5563
[i915#5723]: https://gitlab.freedesktop.org/drm/intel/issues/5723
[i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
[i915#6227]: https://gitlab.freedesktop.org/drm/intel/issues/6227
[i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
[i915#6433]: https://gitlab.freedesktop.org/drm/intel/issues/6433
[i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6590]: https://gitlab.freedesktop.org/drm/intel/issues/6590
[i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
[i915#6946]: https://gitlab.freedesktop.org/drm/intel/issues/6946
[i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
[i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
[i915#7178]: https://gitlab.freedesktop.org/drm/intel/issues/7178
[i915#7384]: https://gitlab.freedesktop.org/drm/intel/issues/7384
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7697]: https://gitlab.freedesktop.org/drm/intel/issues/7697
[i915#7701]: https://gitlab.freedesktop.org/drm/intel/issues/7701
[i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
[i915#7733]: https://gitlab.freedesktop.org/drm/intel/issues/7733
[i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
[i915#7790]: https://gitlab.freedesktop.org/drm/intel/issues/7790
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
[i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
[i915#8153]: https://gitlab.freedesktop.org/drm/intel/issues/8153
[i915#8154]: https://gitlab.freedesktop.org/drm/intel/issues/8154
[i915#8155]: https://gitlab.freedesktop.org/drm/intel/issues/8155
[i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
[i915#8292]: https://gitlab.freedesktop.org/drm/intel/issues/8292
[i915#8295]: https://gitlab.freedesktop.org/drm/intel/issues/8295
[i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
[i915#8381]: https://gitlab.freedesktop.org/drm/intel/issues/8381
Build changes
-------------
* Linux: CI_DRM_13056 -> Patchwork_116517v4
CI-20190529: 20190529
CI_DRM_13056: 308c0163d9e46238948942260e6d2abcad3d8bff @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7267: a267f0236e06fc282e3dc3b8c7d76f9ed6088d9b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_116517v4: 308c0163d9e46238948942260e6d2abcad3d8bff @ 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_116517v4/index.html
[-- Attachment #2: Type: text/html, Size: 14038 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread