| Message ID | 20220826030553.2611574-2-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Drop version numbers from firmware files | expand |
On 8/25/2022 8:05 PM, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > There was a misunderstanding in how firmware file compatibility should > be managed within i915. This has been clarified as: > i915 must support all existing firmware releases forever > new minor firmware releases should replace prior versions > only backwards compatibility breaking releases should be a new file > > This patch cleans up the single fallback file support that was added > as a quick fix emergency effort. That is now removed in preference to > supporting arbitrary numbers of firmware files per platform. > > The patch also adds support for having GuC firmware files that are > named by major version only (because the major version indicates > backwards breaking changes that affect the KMD) and for having HuC > firmware files with no version number at all (because the KMD has no > interface requirements with the HuC). > > For GuC, the driver will report via dmesg if the found file is older than > expected. For HuC, the KMD will no longer require updating for any new > HuC release so will not be able to report what the latest expected > version is. > > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 10 +- > drivers/gpu/drm/i915/gt/uc/intel_uc.c | 4 +- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 442 ++++++++++++------ > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 33 +- > drivers/gpu/drm/i915/i915_gpu_error.c | 16 +- > 5 files changed, 319 insertions(+), 186 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > index 0d56b615bf78e..04393932623c7 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -1868,7 +1868,7 @@ int intel_guc_submission_init(struct intel_guc *guc) > if (guc->submission_initialized) > return 0; > > - if (guc->fw.major_ver_found < 70) { > + if (guc->fw.file_selected.major_ver < 70) { > ret = guc_lrc_desc_pool_create_v69(guc); > if (ret) > return ret; > @@ -2303,7 +2303,7 @@ static int register_context(struct intel_context *ce, bool loop) > GEM_BUG_ON(intel_context_is_child(ce)); > trace_intel_context_register(ce); > > - if (guc->fw.major_ver_found >= 70) > + if (guc->fw.file_selected.major_ver >= 70) > ret = register_context_v70(guc, ce, loop); > else > ret = register_context_v69(guc, ce, loop); > @@ -2315,7 +2315,7 @@ static int register_context(struct intel_context *ce, bool loop) > set_context_registered(ce); > spin_unlock_irqrestore(&ce->guc_state.lock, flags); > > - if (guc->fw.major_ver_found >= 70) > + if (guc->fw.file_selected.major_ver >= 70) > guc_context_policy_init_v70(ce, loop); > } > > @@ -2921,7 +2921,7 @@ static void __guc_context_set_preemption_timeout(struct intel_guc *guc, > u16 guc_id, > u32 preemption_timeout) > { > - if (guc->fw.major_ver_found >= 70) { > + if (guc->fw.file_selected.major_ver >= 70) { > struct context_policy policy; > > __guc_context_policy_start_klv(&policy, guc_id); > @@ -3186,7 +3186,7 @@ static int guc_context_alloc(struct intel_context *ce) > static void __guc_context_set_prio(struct intel_guc *guc, > struct intel_context *ce) > { > - if (guc->fw.major_ver_found >= 70) { > + if (guc->fw.file_selected.major_ver >= 70) { > struct context_policy policy; > > __guc_context_policy_start_klv(&policy, ce->guc_id.id); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > index f2e7c82985efd..d965ac4832d60 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c > @@ -436,8 +436,8 @@ static void print_fw_ver(struct intel_uc *uc, struct intel_uc_fw *fw) > struct drm_i915_private *i915 = uc_to_gt(uc)->i915; > > drm_info(&i915->drm, "%s firmware %s version %u.%u\n", > - intel_uc_fw_type_repr(fw->type), fw->path, > - fw->major_ver_found, fw->minor_ver_found); > + intel_uc_fw_type_repr(fw->type), fw->file_selected.path, > + fw->file_selected.major_ver, fw->file_selected.minor_ver); > } > > static int __uc_init_hw(struct intel_uc *uc) > 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 58547292efa0a..94cf2d4a46e6f 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > @@ -41,7 +41,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw, > "%s firmware -> %s\n", > intel_uc_fw_type_repr(uc_fw->type), > status == INTEL_UC_FIRMWARE_SELECTED ? > - uc_fw->path : intel_uc_fw_status_repr(status)); > + uc_fw->file_selected.path : intel_uc_fw_status_repr(status)); > } > #endif > > @@ -51,84 +51,149 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw, > * > * Note that RKL and ADL-S have the same GuC/HuC device ID's and use the same > * firmware as TGL. > + * > + * Version numbers: > + * Originally, the driver required an exact match major/minor/patch furmware > + * file and only supported that one version for any given platform. However, > + * the new direction from upstream is to be becakwards compatible with all typo becakwards > + * prior releases and to be as flexible as possible as to what firmware is > + * loaded. > + * > + * For GuC, the major version number signifies a backwards breaking API change. > + * So, new format GuC firmware files are labelled by their major version only. > + * For HuC, there is no KMD interaction, hence no version matching requirement. > + * So, new format HuC firmware files have no version number at all. > + * > + * All of which means that the table below must keep all old format files with > + * full three point version number. But newer files have reduced requirements. > + * Having said that, the driver still needs to track the minor version number > + * for GuC at least. As it is useful to report to the user that they are not > + * running with a recent enough version for all KMD supported features, > + * security fixes, etc. to be enabled. > */ > -#define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_def) \ > - fw_def(DG2, 0, guc_def(dg2, 70, 4, 1)) \ > - fw_def(ALDERLAKE_P, 0, guc_def(adlp, 70, 1, 1)) \ > - fw_def(ALDERLAKE_S, 0, guc_def(tgl, 70, 1, 1)) \ > - fw_def(DG1, 0, guc_def(dg1, 70, 1, 1)) \ > - fw_def(ROCKETLAKE, 0, guc_def(tgl, 70, 1, 1)) \ > - fw_def(TIGERLAKE, 0, guc_def(tgl, 70, 1, 1)) \ > - fw_def(JASPERLAKE, 0, guc_def(ehl, 70, 1, 1)) \ > - fw_def(ELKHARTLAKE, 0, guc_def(ehl, 70, 1, 1)) \ > - fw_def(ICELAKE, 0, guc_def(icl, 70, 1, 1)) \ > - fw_def(COMETLAKE, 5, guc_def(cml, 70, 1, 1)) \ > - fw_def(COMETLAKE, 0, guc_def(kbl, 70, 1, 1)) \ > - fw_def(COFFEELAKE, 0, guc_def(kbl, 70, 1, 1)) \ > - fw_def(GEMINILAKE, 0, guc_def(glk, 70, 1, 1)) \ > - fw_def(KABYLAKE, 0, guc_def(kbl, 70, 1, 1)) \ > - fw_def(BROXTON, 0, guc_def(bxt, 70, 1, 1)) \ > - fw_def(SKYLAKE, 0, guc_def(skl, 70, 1, 1)) > - > -#define INTEL_GUC_FIRMWARE_DEFS_FALLBACK(fw_def, guc_def) \ > - fw_def(ALDERLAKE_P, 0, guc_def(adlp, 69, 0, 3)) \ > - fw_def(ALDERLAKE_S, 0, guc_def(tgl, 69, 0, 3)) > - > -#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_def) \ > - fw_def(ALDERLAKE_P, 0, huc_def(tgl, 7, 9, 3)) \ > - fw_def(ALDERLAKE_S, 0, huc_def(tgl, 7, 9, 3)) \ > - fw_def(DG1, 0, huc_def(dg1, 7, 9, 3)) \ > - fw_def(ROCKETLAKE, 0, huc_def(tgl, 7, 9, 3)) \ > - fw_def(TIGERLAKE, 0, huc_def(tgl, 7, 9, 3)) \ > - fw_def(JASPERLAKE, 0, huc_def(ehl, 9, 0, 0)) \ > - fw_def(ELKHARTLAKE, 0, huc_def(ehl, 9, 0, 0)) \ > - fw_def(ICELAKE, 0, huc_def(icl, 9, 0, 0)) \ > - fw_def(COMETLAKE, 5, huc_def(cml, 4, 0, 0)) \ > - fw_def(COMETLAKE, 0, huc_def(kbl, 4, 0, 0)) \ > - fw_def(COFFEELAKE, 0, huc_def(kbl, 4, 0, 0)) \ > - fw_def(GEMINILAKE, 0, huc_def(glk, 4, 0, 0)) \ > - fw_def(KABYLAKE, 0, huc_def(kbl, 4, 0, 0)) \ > - fw_def(BROXTON, 0, huc_def(bxt, 2, 0, 0)) \ > - fw_def(SKYLAKE, 0, huc_def(skl, 2, 0, 0)) > - > -#define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \ > +#define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \ > + fw_def(DG2, 0, guc_mmp(dg2, 70, 4, 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_mmp(tgl, 70, 1, 1)) \ > + fw_def(ALDERLAKE_S, 0, guc_mmp(tgl, 69, 0, 3)) \ > + fw_def(DG1, 0, guc_mmp(dg1, 70, 1, 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)) \ > + fw_def(ELKHARTLAKE, 0, guc_mmp(ehl, 70, 1, 1)) \ > + fw_def(ICELAKE, 0, guc_mmp(icl, 70, 1, 1)) \ > + fw_def(COMETLAKE, 5, guc_mmp(cml, 70, 1, 1)) \ > + fw_def(COMETLAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ > + fw_def(COFFEELAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ > + fw_def(GEMINILAKE, 0, guc_mmp(glk, 70, 1, 1)) \ > + fw_def(KABYLAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ > + fw_def(BROXTON, 0, guc_mmp(bxt, 70, 1, 1)) \ > + fw_def(SKYLAKE, 0, guc_mmp(skl, 70, 1, 1)) > + > +#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp) \ > + fw_def(ALDERLAKE_P, 0, huc_mmp(tgl, 7, 9, 3)) \ > + fw_def(ALDERLAKE_S, 0, huc_mmp(tgl, 7, 9, 3)) \ > + fw_def(DG1, 0, huc_mmp(dg1, 7, 9, 3)) \ > + fw_def(ROCKETLAKE, 0, huc_mmp(tgl, 7, 9, 3)) \ > + fw_def(TIGERLAKE, 0, huc_mmp(tgl, 7, 9, 3)) \ > + fw_def(JASPERLAKE, 0, huc_mmp(ehl, 9, 0, 0)) \ > + fw_def(ELKHARTLAKE, 0, huc_mmp(ehl, 9, 0, 0)) \ > + fw_def(ICELAKE, 0, huc_mmp(icl, 9, 0, 0)) \ > + fw_def(COMETLAKE, 5, huc_mmp(cml, 4, 0, 0)) \ > + fw_def(COMETLAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ > + fw_def(COFFEELAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ > + fw_def(GEMINILAKE, 0, huc_mmp(glk, 4, 0, 0)) \ > + fw_def(KABYLAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ > + fw_def(BROXTON, 0, huc_mmp(bxt, 2, 0, 0)) \ > + fw_def(SKYLAKE, 0, huc_mmp(skl, 2, 0, 0)) > + > +/* > + * Set of macros for producing a list of filenames from the above table. > + */ > +#define __MAKE_UC_FW_PATH_BLANK(prefix_, name_) \ > + "i915/" \ > + __stringify(prefix_) name_ ".bin" > + > +#define __MAKE_UC_FW_PATH_MAJOR(prefix_, name_, major_) \ > + "i915/" \ > + __stringify(prefix_) name_ \ > + __stringify(major_) ".bin" > + > +#define __MAKE_UC_FW_PATH_MMP(prefix_, name_, major_, minor_, patch_) \ > "i915/" \ > __stringify(prefix_) name_ \ > __stringify(major_) "." \ > __stringify(minor_) "." \ > __stringify(patch_) ".bin" > > -#define MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \ > - __MAKE_UC_FW_PATH(prefix_, "_guc_", major_, minor_, patch_) > +/* Minor for internal driver use, not part of file name */ > +#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_) \ > + __MAKE_UC_FW_PATH_MAJOR(prefix_, "_guc_", major_) > > -#define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \ > - __MAKE_UC_FW_PATH(prefix_, "_huc_", major_, minor_, bld_num_) > +#define MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \ > + __MAKE_UC_FW_PATH_MMP(prefix_, "_guc_", major_, minor_, patch_) > > -/* All blobs need to be declared via MODULE_FIRMWARE() */ > +#define MAKE_HUC_FW_PATH_BLANK(prefix_) \ > + __MAKE_UC_FW_PATH_BLANK(prefix_, "_huc") > + > +#define MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \ > + __MAKE_UC_FW_PATH_MMP(prefix_, "_huc_", major_, minor_, patch_) > + > +/* > + * All blobs need to be declared via MODULE_FIRMWARE(). > + * This first expansion of the table macros is solely to provide > + * that declaration. > + */ > #define INTEL_UC_MODULE_FW(platform_, revid_, uc_) \ > MODULE_FIRMWARE(uc_); > > -INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH) > -INTEL_GUC_FIRMWARE_DEFS_FALLBACK(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH) > -INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH) > +INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH_MAJOR, MAKE_GUC_FW_PATH_MMP) > +INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK, MAKE_HUC_FW_PATH_MMP) > > -/* The below structs and macros are used to iterate across the list of blobs */ > +/* > + * The next expansion of the table macros (in __uc_fw_auto_select below) provides > + * actual data structures with both the filename and the version information. > + * These structure arrays are then iterated over to the list of suitable files > + * for the current platform and to then attempt to load those files, in the order > + * listed, until one is successfully found. > + */ > struct __packed uc_fw_blob { > + const char *path; > + bool legacy; > u8 major; > u8 minor; > - const char *path; > + u8 patch; > }; > > -#define UC_FW_BLOB(major_, minor_, path_) \ > - { .major = major_, .minor = minor_, .path = path_ } > +#define UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ > + .major = major_, \ > + .minor = minor_, \ > + .patch = patch_, \ > + .path = path_, > + > +#define UC_FW_BLOB_NEW(major_, minor_, patch_, path_) \ > + { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ > + .legacy = false } > + > +#define UC_FW_BLOB_OLD(major_, minor_, patch_, path_) \ > + { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ > + .legacy = true } > > -#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \ > - UC_FW_BLOB(major_, minor_, \ > - MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_)) > +#define GUC_FW_BLOB(prefix_, major_, minor_) \ > + UC_FW_BLOB_NEW(major_, minor_, 0, \ > + MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_)) > > -#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \ > - UC_FW_BLOB(major_, minor_, \ > - MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_)) > +#define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \ > + UC_FW_BLOB_OLD(major_, minor_, patch_, \ > + MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_)) > + > +#define HUC_FW_BLOB(prefix_) \ > + UC_FW_BLOB_NEW(0, 0, 0, MAKE_HUC_FW_PATH_BLANK(prefix_)) > + > +#define HUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \ > + UC_FW_BLOB_OLD(major_, minor_, patch_, \ > + MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_)) > > struct __packed uc_fw_platform_requirement { > enum intel_platform p; > @@ -152,18 +217,16 @@ 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) > - }; > - static const struct uc_fw_platform_requirement blobs_guc_fallback[] = { > - INTEL_GUC_FIRMWARE_DEFS_FALLBACK(MAKE_FW_LIST, GUC_FW_BLOB) > + 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) > + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP) > }; > 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; is this static to only run the test once per boot? > const struct uc_fw_platform_requirement *fw_blobs; > enum intel_platform p = INTEL_INFO(i915)->platform; > u32 fw_count; > @@ -184,49 +247,94 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) > fw_count = blobs_all[uc_fw->type].count; > > for (i = 0; i < fw_count && p <= fw_blobs[i].p; i++) { > - if (p == fw_blobs[i].p && rev >= fw_blobs[i].rev) { > - const struct uc_fw_blob *blob = &fw_blobs[i].blob; > - uc_fw->path = blob->path; > - uc_fw->wanted_path = blob->path; > - uc_fw->major_ver_wanted = blob->major; > - uc_fw->minor_ver_wanted = blob->minor; > - break; > - } > - } > + const struct uc_fw_blob *blob = &fw_blobs[i].blob; > > - if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) { > - const struct uc_fw_platform_requirement *blobs = blobs_guc_fallback; > - u32 count = ARRAY_SIZE(blobs_guc_fallback); > + if (p != fw_blobs[i].p) > + continue; > > - for (i = 0; i < count && p <= blobs[i].p; i++) { > - if (p == blobs[i].p && rev >= blobs[i].rev) { > - const struct uc_fw_blob *blob = &blobs[i].blob; > + if (rev < fw_blobs[i].rev) > + continue; > > - uc_fw->fallback.path = blob->path; > - uc_fw->fallback.major_ver = blob->major; > - uc_fw->fallback.minor_ver = blob->minor; > - break; > - } > + if (uc_fw->file_selected.path) { > + if (uc_fw->file_selected.path == blob->path) > + uc_fw->file_selected.path = NULL; > + > + continue; > } Does this work with 3 versions for the same platform if the 2 newer ones are not available? let's say we have v1, v2 and v3. v3 (newer) gets selected first but not found, so we call back in this function again and match v3 in the path check, clear the selected and continue, selecting v2 in the next round. if v2 is also not found, we call back in here starting from v3, which will not match the path check, so we'll continue below and assign it. Or am I missing something? everything else looks ok. Daniele > + > + uc_fw->file_selected.path = blob->path; > + uc_fw->file_wanted.path = blob->path; > + uc_fw->file_wanted.major_ver = blob->major; > + uc_fw->file_wanted.minor_ver = blob->minor; > + break; > } > > /* make sure the list is ordered as expected */ > - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) { > + if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified) { > + verified = true; > + > for (i = 1; i < fw_count; i++) { > + /* Next platform is good: */ > if (fw_blobs[i].p < fw_blobs[i - 1].p) > continue; > > + /* 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; > > - drm_err(&i915->drm, "Invalid FW blob order: %s r%u comes before %s r%u\n", > - intel_platform_name(fw_blobs[i - 1].p), > - fw_blobs[i - 1].rev, > - intel_platform_name(fw_blobs[i].p), > - fw_blobs[i].rev); > + /* 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; > + > + /* Next major version is good: */ > + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) > + continue; > + > + /* New must be before legacy: */ > + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy) > + goto bad; > + > + /* 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; > + > + 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; > > - uc_fw->path = NULL; > + /* 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, "\x1B[35;1mInvalid FW blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", > + 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; > } > } > } > @@ -259,7 +367,7 @@ static void __uc_fw_user_override(struct drm_i915_private *i915, struct intel_uc > } > > if (unlikely(path)) { > - uc_fw->path = path; > + uc_fw->file_selected.path = path; > uc_fw->user_overridden = true; > } > } > @@ -283,7 +391,7 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, > */ > BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); > GEM_BUG_ON(uc_fw->status); > - GEM_BUG_ON(uc_fw->path); > + GEM_BUG_ON(uc_fw->file_selected.path); > > uc_fw->type = type; > > @@ -292,7 +400,7 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, > __uc_fw_user_override(i915, uc_fw); > } > > - intel_uc_fw_change_status(uc_fw, uc_fw->path ? *uc_fw->path ? > + intel_uc_fw_change_status(uc_fw, uc_fw->file_selected.path ? *uc_fw->file_selected.path ? > INTEL_UC_FIRMWARE_SELECTED : > INTEL_UC_FIRMWARE_DISABLED : > INTEL_UC_FIRMWARE_NOT_SUPPORTED); > @@ -305,32 +413,32 @@ static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e) > > if (i915_inject_probe_error(i915, e)) { > /* non-existing blob */ > - uc_fw->path = "<invalid>"; > + uc_fw->file_selected.path = "<invalid>"; > uc_fw->user_overridden = user; > } else if (i915_inject_probe_error(i915, e)) { > /* require next major version */ > - uc_fw->major_ver_wanted += 1; > - uc_fw->minor_ver_wanted = 0; > + uc_fw->file_wanted.major_ver += 1; > + uc_fw->file_wanted.minor_ver = 0; > uc_fw->user_overridden = user; > } else if (i915_inject_probe_error(i915, e)) { > /* require next minor version */ > - uc_fw->minor_ver_wanted += 1; > + uc_fw->file_wanted.minor_ver += 1; > uc_fw->user_overridden = user; > - } else if (uc_fw->major_ver_wanted && > + } else if (uc_fw->file_wanted.major_ver && > i915_inject_probe_error(i915, e)) { > /* require prev major version */ > - uc_fw->major_ver_wanted -= 1; > - uc_fw->minor_ver_wanted = 0; > + uc_fw->file_wanted.major_ver -= 1; > + uc_fw->file_wanted.minor_ver = 0; > uc_fw->user_overridden = user; > - } else if (uc_fw->minor_ver_wanted && > + } else if (uc_fw->file_wanted.minor_ver && > i915_inject_probe_error(i915, e)) { > /* require prev minor version - hey, this should work! */ > - uc_fw->minor_ver_wanted -= 1; > + uc_fw->file_wanted.minor_ver -= 1; > uc_fw->user_overridden = user; > } else if (user && i915_inject_probe_error(i915, e)) { > /* officially unsupported platform */ > - uc_fw->major_ver_wanted = 0; > - uc_fw->minor_ver_wanted = 0; > + uc_fw->file_wanted.major_ver = 0; > + uc_fw->file_wanted.minor_ver = 0; > uc_fw->user_overridden = true; > } > } > @@ -341,8 +449,8 @@ static int check_gsc_manifest(const struct firmware *fw, > u32 *dw = (u32 *)fw->data; > u32 version = dw[HUC_GSC_VERSION_DW]; > > - uc_fw->major_ver_found = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version); > - uc_fw->minor_ver_found = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version); > + uc_fw->file_selected.major_ver = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version); > + uc_fw->file_selected.minor_ver = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version); > > return 0; > } > @@ -357,7 +465,7 @@ static int check_ccs_header(struct drm_i915_private *i915, > /* Check the size of the blob before examining buffer contents */ > if (unlikely(fw->size < sizeof(struct uc_css_header))) { > drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, > + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, > fw->size, sizeof(struct uc_css_header)); > return -ENODATA; > } > @@ -370,7 +478,7 @@ static int check_ccs_header(struct drm_i915_private *i915, > if (unlikely(size != sizeof(struct uc_css_header))) { > drm_warn(&i915->drm, > "%s firmware %s: unexpected header size: %zu != %zu\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, > + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, > fw->size, sizeof(struct uc_css_header)); > return -EPROTO; > } > @@ -385,7 +493,7 @@ static int check_ccs_header(struct drm_i915_private *i915, > size = sizeof(struct uc_css_header) + uc_fw->ucode_size + uc_fw->rsa_size; > if (unlikely(fw->size < size)) { > drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, > + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, > fw->size, size); > return -ENOEXEC; > } > @@ -394,16 +502,16 @@ static int check_ccs_header(struct drm_i915_private *i915, > size = __intel_uc_fw_get_upload_size(uc_fw); > if (unlikely(size >= i915->wopcm.size)) { > drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu > %zu\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, > + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, > size, (size_t)i915->wopcm.size); > return -E2BIG; > } > > /* Get version numbers from the CSS header */ > - uc_fw->major_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, > - css->sw_version); > - uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR, > - css->sw_version); > + uc_fw->file_selected.major_ver = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, > + css->sw_version); > + uc_fw->file_selected.minor_ver = FIELD_GET(CSS_SW_VERSION_UC_MINOR, > + css->sw_version); > > if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) > uc_fw->private_data_size = css->private_data_size; > @@ -422,9 +530,11 @@ static int check_ccs_header(struct drm_i915_private *i915, > int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) > { > struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915; > + struct intel_uc_fw_file file_ideal; > struct device *dev = i915->drm.dev; > struct drm_i915_gem_object *obj; > const struct firmware *fw = NULL; > + bool old_ver = false; > int err; > > GEM_BUG_ON(!i915->wopcm.size); > @@ -437,27 +547,33 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) > __force_fw_fetch_failures(uc_fw, -EINVAL); > __force_fw_fetch_failures(uc_fw, -ESTALE); > > - err = firmware_request_nowarn(&fw, uc_fw->path, dev); > - if (err && !intel_uc_fw_is_overridden(uc_fw) && uc_fw->fallback.path) { > - err = firmware_request_nowarn(&fw, uc_fw->fallback.path, dev); > - if (!err) { > - drm_notice(&i915->drm, > - "%s firmware %s is recommended, but only %s was found\n", > - intel_uc_fw_type_repr(uc_fw->type), > - uc_fw->wanted_path, > - uc_fw->fallback.path); > - drm_info(&i915->drm, > - "Consider updating your linux-firmware pkg or downloading from %s\n", > - INTEL_UC_FIRMWARE_URL); > - > - uc_fw->path = uc_fw->fallback.path; > - uc_fw->major_ver_wanted = uc_fw->fallback.major_ver; > - uc_fw->minor_ver_wanted = uc_fw->fallback.minor_ver; > + err = firmware_request_nowarn(&fw, uc_fw->file_selected.path, dev); > + memcpy(&file_ideal, &uc_fw->file_wanted, sizeof(file_ideal)); > + if (!err || intel_uc_fw_is_overridden(uc_fw)) > + goto done; > + > + while (err == -ENOENT) { > + __uc_fw_auto_select(i915, uc_fw); > + if (!uc_fw->file_selected.path) { > + /* > + * No more options! But set the path back to something > + * valid just in case it gets dereferenced. > + */ > + uc_fw->file_selected.path = file_ideal.path; > + > + /* Also, preserve the version that was really wanted */ > + memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted)); > + break; > } > + > + err = firmware_request_nowarn(&fw, uc_fw->file_selected.path, dev); > } > + > if (err) > goto fail; > > + old_ver = true; > +done: > if (uc_fw->loaded_via_gsc) > err = check_gsc_manifest(fw, uc_fw); > else > @@ -465,18 +581,39 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) > if (err) > goto fail; > > - if (uc_fw->major_ver_found != uc_fw->major_ver_wanted || > - uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) { > - drm_notice(&i915->drm, "%s firmware %s: unexpected version: %u.%u != %u.%u\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, > - uc_fw->major_ver_found, uc_fw->minor_ver_found, > - uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted); > - if (!intel_uc_fw_is_overridden(uc_fw)) { > - err = -ENOEXEC; > - goto fail; > + if (uc_fw->file_wanted.major_ver) { > + /* Check the file's major version was as it claimed */ > + if (uc_fw->file_selected.major_ver != uc_fw->file_wanted.major_ver) { > + drm_notice(&i915->drm, "%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.major_ver, uc_fw->file_selected.minor_ver, > + uc_fw->file_wanted.major_ver, uc_fw->file_wanted.minor_ver); > + if (!intel_uc_fw_is_overridden(uc_fw)) { > + err = -ENOEXEC; > + goto fail; > + } > + } else { > + if (uc_fw->file_selected.minor_ver < uc_fw->file_wanted.minor_ver) > + old_ver = true; > } > } > > + if (old_ver) { > + /* Preserve the version that was really wanted */ > + memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted)); > + > + drm_notice(&i915->drm, > + "%s firmware %s (%d.%d) is recommended, but only %s (%d.%d) was found\n", > + intel_uc_fw_type_repr(uc_fw->type), > + uc_fw->file_wanted.path, > + uc_fw->file_wanted.major_ver, uc_fw->file_wanted.minor_ver, > + uc_fw->file_selected.path, > + uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver); > + drm_info(&i915->drm, > + "Consider updating your linux-firmware pkg or downloading from %s\n", > + INTEL_UC_FIRMWARE_URL); > + } > + > if (HAS_LMEM(i915)) { > obj = i915_gem_object_create_lmem_from_data(i915, fw->data, fw->size); > if (!IS_ERR(obj)) > @@ -503,7 +640,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) > INTEL_UC_FIRMWARE_ERROR); > > i915_probe_error(i915, "%s firmware %s: fetch failed with error %d\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err); > + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, err); > drm_info(&i915->drm, "%s firmware(s) can be downloaded from %s\n", > intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL); > > @@ -645,7 +782,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, u32 dst_offset, u32 dma_flags) > > fail: > i915_probe_error(gt->i915, "Failed to load %s firmware %s (%d)\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, > + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, > err); > intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_LOAD_FAIL); > return err; > @@ -864,18 +1001,19 @@ size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len) > void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p) > { > drm_printf(p, "%s firmware: %s\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->wanted_path); > - if (uc_fw->fallback.path) { > - drm_printf(p, "%s firmware fallback: %s\n", > - intel_uc_fw_type_repr(uc_fw->type), uc_fw->fallback.path); > - drm_printf(p, "fallback selected: %s\n", > - str_yes_no(uc_fw->path == uc_fw->fallback.path)); > - } > + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path); > + if (uc_fw->file_selected.path != uc_fw->file_wanted.path) > + drm_printf(p, "%s firmware wanted: %s\n", > + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_wanted.path); > drm_printf(p, "\tstatus: %s\n", > intel_uc_fw_status_repr(uc_fw->status)); > - drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n", > - uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted, > - uc_fw->major_ver_found, uc_fw->minor_ver_found); > + if (uc_fw->file_wanted.major_ver) > + drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n", > + uc_fw->file_wanted.major_ver, uc_fw->file_wanted.minor_ver, > + uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver); > + else > + drm_printf(p, "\tversion: found %u.%u\n", > + uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver); > drm_printf(p, "\tuCode: %u bytes\n", uc_fw->ucode_size); > drm_printf(p, "\tRSA: %u bytes\n", uc_fw->rsa_size); > } > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h > index 7aa2644400b98..344763c942e37 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h > @@ -64,6 +64,17 @@ enum intel_uc_fw_type { > }; > #define INTEL_UC_FW_NUM_TYPES 2 > > +/* > + * The firmware build process will generate a version header file with major and > + * minor version defined. The versions are built into CSS header of firmware. > + * i915 kernel driver set the minimal firmware version required per platform. > + */ > +struct intel_uc_fw_file { > + const char *path; > + u16 major_ver; > + u16 minor_ver; > +}; > + > /* > * This structure encapsulates all the data needed during the process > * of fetching, caching, and loading the firmware image into the uC. > @@ -74,11 +85,12 @@ struct intel_uc_fw { > const enum intel_uc_fw_status status; > enum intel_uc_fw_status __status; /* no accidental overwrites */ > }; > - const char *wanted_path; > - const char *path; > + struct intel_uc_fw_file file_wanted; > + struct intel_uc_fw_file file_selected; > bool user_overridden; > size_t size; > struct drm_i915_gem_object *obj; > + > /** > * @dummy: A vma used in binding the uc fw to ggtt. We can't define this > * vma on the stack as it can lead to a stack overflow, so we define it > @@ -89,25 +101,8 @@ struct intel_uc_fw { > struct i915_vma_resource dummy; > struct i915_vma *rsa_data; > > - /* > - * The firmware build process will generate a version header file with major and > - * minor version defined. The versions are built into CSS header of firmware. > - * i915 kernel driver set the minimal firmware version required per platform. > - */ > - u16 major_ver_wanted; > - u16 minor_ver_wanted; > - u16 major_ver_found; > - u16 minor_ver_found; > - > - struct { > - const char *path; > - u16 major_ver; > - u16 minor_ver; > - } fallback; > - > u32 rsa_size; > u32 ucode_size; > - > u32 private_data_size; > > bool loaded_via_gsc; > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c > index b5fbc2252784a..9ea2fe34e7d30 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -1022,8 +1022,10 @@ static void cleanup_params(struct i915_gpu_coredump *error) > > static void cleanup_uc(struct intel_uc_coredump *uc) > { > - kfree(uc->guc_fw.path); > - kfree(uc->huc_fw.path); > + kfree(uc->guc_fw.file_selected.path); > + kfree(uc->huc_fw.file_selected.path); > + kfree(uc->guc_fw.file_wanted.path); > + kfree(uc->huc_fw.file_wanted.path); > i915_vma_coredump_free(uc->guc.vma_log); > i915_vma_coredump_free(uc->guc.vma_ctb); > > @@ -1705,12 +1707,10 @@ gt_record_uc(struct intel_gt_coredump *gt, > memcpy(&error_uc->guc_fw, &uc->guc.fw, sizeof(uc->guc.fw)); > memcpy(&error_uc->huc_fw, &uc->huc.fw, sizeof(uc->huc.fw)); > > - /* Non-default firmware paths will be specified by the modparam. > - * As modparams are generally accesible from the userspace make > - * explicit copies of the firmware paths. > - */ > - error_uc->guc_fw.path = kstrdup(uc->guc.fw.path, ALLOW_FAIL); > - error_uc->huc_fw.path = kstrdup(uc->huc.fw.path, ALLOW_FAIL); > + error_uc->guc_fw.file_selected.path = kstrdup(uc->guc.fw.file_selected.path, ALLOW_FAIL); > + error_uc->huc_fw.file_selected.path = kstrdup(uc->huc.fw.file_selected.path, ALLOW_FAIL); > + error_uc->guc_fw.file_wanted.path = kstrdup(uc->guc.fw.file_wanted.path, ALLOW_FAIL); > + error_uc->huc_fw.file_wanted.path = kstrdup(uc->huc.fw.file_wanted.path, ALLOW_FAIL); > > /* > * Save the GuC log and include a timestamp reference for converting the
On 8/26/2022 09:35, Ceraolo Spurio, Daniele wrote: > On 8/25/2022 8:05 PM, John.C.Harrison@Intel.com wrote: >> From: John Harrison <John.C.Harrison@Intel.com> >> >> There was a misunderstanding in how firmware file compatibility should >> be managed within i915. This has been clarified as: >> i915 must support all existing firmware releases forever >> new minor firmware releases should replace prior versions >> only backwards compatibility breaking releases should be a new file >> >> This patch cleans up the single fallback file support that was added >> as a quick fix emergency effort. That is now removed in preference to >> supporting arbitrary numbers of firmware files per platform. >> >> The patch also adds support for having GuC firmware files that are >> named by major version only (because the major version indicates >> backwards breaking changes that affect the KMD) and for having HuC >> firmware files with no version number at all (because the KMD has no >> interface requirements with the HuC). >> >> For GuC, the driver will report via dmesg if the found file is older >> than >> expected. For HuC, the KMD will no longer require updating for any new >> HuC release so will not be able to report what the latest expected >> version is. >> >> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> >> --- >> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 10 +- >> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 4 +- >> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 442 ++++++++++++------ >> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 33 +- >> drivers/gpu/drm/i915/i915_gpu_error.c | 16 +- >> 5 files changed, 319 insertions(+), 186 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >> index 0d56b615bf78e..04393932623c7 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >> @@ -1868,7 +1868,7 @@ int intel_guc_submission_init(struct intel_guc >> *guc) >> if (guc->submission_initialized) >> return 0; >> - if (guc->fw.major_ver_found < 70) { >> + if (guc->fw.file_selected.major_ver < 70) { >> ret = guc_lrc_desc_pool_create_v69(guc); >> if (ret) >> return ret; >> @@ -2303,7 +2303,7 @@ static int register_context(struct >> intel_context *ce, bool loop) >> GEM_BUG_ON(intel_context_is_child(ce)); >> trace_intel_context_register(ce); >> - if (guc->fw.major_ver_found >= 70) >> + if (guc->fw.file_selected.major_ver >= 70) >> ret = register_context_v70(guc, ce, loop); >> else >> ret = register_context_v69(guc, ce, loop); >> @@ -2315,7 +2315,7 @@ static int register_context(struct >> intel_context *ce, bool loop) >> set_context_registered(ce); >> spin_unlock_irqrestore(&ce->guc_state.lock, flags); >> - if (guc->fw.major_ver_found >= 70) >> + if (guc->fw.file_selected.major_ver >= 70) >> guc_context_policy_init_v70(ce, loop); >> } >> @@ -2921,7 +2921,7 @@ static void >> __guc_context_set_preemption_timeout(struct intel_guc *guc, >> u16 guc_id, >> u32 preemption_timeout) >> { >> - if (guc->fw.major_ver_found >= 70) { >> + if (guc->fw.file_selected.major_ver >= 70) { >> struct context_policy policy; >> __guc_context_policy_start_klv(&policy, guc_id); >> @@ -3186,7 +3186,7 @@ static int guc_context_alloc(struct >> intel_context *ce) >> static void __guc_context_set_prio(struct intel_guc *guc, >> struct intel_context *ce) >> { >> - if (guc->fw.major_ver_found >= 70) { >> + if (guc->fw.file_selected.major_ver >= 70) { >> struct context_policy policy; >> __guc_context_policy_start_klv(&policy, ce->guc_id.id); >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c >> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c >> index f2e7c82985efd..d965ac4832d60 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c >> @@ -436,8 +436,8 @@ static void print_fw_ver(struct intel_uc *uc, >> struct intel_uc_fw *fw) >> struct drm_i915_private *i915 = uc_to_gt(uc)->i915; >> drm_info(&i915->drm, "%s firmware %s version %u.%u\n", >> - intel_uc_fw_type_repr(fw->type), fw->path, >> - fw->major_ver_found, fw->minor_ver_found); >> + intel_uc_fw_type_repr(fw->type), fw->file_selected.path, >> + fw->file_selected.major_ver, fw->file_selected.minor_ver); >> } >> static int __uc_init_hw(struct intel_uc *uc) >> 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 58547292efa0a..94cf2d4a46e6f 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >> @@ -41,7 +41,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw >> *uc_fw, >> "%s firmware -> %s\n", >> intel_uc_fw_type_repr(uc_fw->type), >> status == INTEL_UC_FIRMWARE_SELECTED ? >> - uc_fw->path : intel_uc_fw_status_repr(status)); >> + uc_fw->file_selected.path : intel_uc_fw_status_repr(status)); >> } >> #endif >> @@ -51,84 +51,149 @@ void intel_uc_fw_change_status(struct >> intel_uc_fw *uc_fw, >> * >> * Note that RKL and ADL-S have the same GuC/HuC device ID's and >> use the same >> * firmware as TGL. >> + * >> + * Version numbers: >> + * Originally, the driver required an exact match major/minor/patch >> furmware >> + * file and only supported that one version for any given platform. >> However, >> + * the new direction from upstream is to be becakwards compatible >> with all > > typo becakwards > >> + * prior releases and to be as flexible as possible as to what >> firmware is >> + * loaded. >> + * >> + * For GuC, the major version number signifies a backwards breaking >> API change. >> + * So, new format GuC firmware files are labelled by their major >> version only. >> + * For HuC, there is no KMD interaction, hence no version matching >> requirement. >> + * So, new format HuC firmware files have no version number at all. >> + * >> + * All of which means that the table below must keep all old format >> files with >> + * full three point version number. But newer files have reduced >> requirements. >> + * Having said that, the driver still needs to track the minor >> version number >> + * for GuC at least. As it is useful to report to the user that they >> are not >> + * running with a recent enough version for all KMD supported features, >> + * security fixes, etc. to be enabled. >> */ >> -#define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_def) \ >> - fw_def(DG2, 0, guc_def(dg2, 70, 4, 1)) \ >> - fw_def(ALDERLAKE_P, 0, guc_def(adlp, 70, 1, 1)) \ >> - fw_def(ALDERLAKE_S, 0, guc_def(tgl, 70, 1, 1)) \ >> - fw_def(DG1, 0, guc_def(dg1, 70, 1, 1)) \ >> - fw_def(ROCKETLAKE, 0, guc_def(tgl, 70, 1, 1)) \ >> - fw_def(TIGERLAKE, 0, guc_def(tgl, 70, 1, 1)) \ >> - fw_def(JASPERLAKE, 0, guc_def(ehl, 70, 1, 1)) \ >> - fw_def(ELKHARTLAKE, 0, guc_def(ehl, 70, 1, 1)) \ >> - fw_def(ICELAKE, 0, guc_def(icl, 70, 1, 1)) \ >> - fw_def(COMETLAKE, 5, guc_def(cml, 70, 1, 1)) \ >> - fw_def(COMETLAKE, 0, guc_def(kbl, 70, 1, 1)) \ >> - fw_def(COFFEELAKE, 0, guc_def(kbl, 70, 1, 1)) \ >> - fw_def(GEMINILAKE, 0, guc_def(glk, 70, 1, 1)) \ >> - fw_def(KABYLAKE, 0, guc_def(kbl, 70, 1, 1)) \ >> - fw_def(BROXTON, 0, guc_def(bxt, 70, 1, 1)) \ >> - fw_def(SKYLAKE, 0, guc_def(skl, 70, 1, 1)) >> - >> -#define INTEL_GUC_FIRMWARE_DEFS_FALLBACK(fw_def, guc_def) \ >> - fw_def(ALDERLAKE_P, 0, guc_def(adlp, 69, 0, 3)) \ >> - fw_def(ALDERLAKE_S, 0, guc_def(tgl, 69, 0, 3)) >> - >> -#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_def) \ >> - fw_def(ALDERLAKE_P, 0, huc_def(tgl, 7, 9, 3)) \ >> - fw_def(ALDERLAKE_S, 0, huc_def(tgl, 7, 9, 3)) \ >> - fw_def(DG1, 0, huc_def(dg1, 7, 9, 3)) \ >> - fw_def(ROCKETLAKE, 0, huc_def(tgl, 7, 9, 3)) \ >> - fw_def(TIGERLAKE, 0, huc_def(tgl, 7, 9, 3)) \ >> - fw_def(JASPERLAKE, 0, huc_def(ehl, 9, 0, 0)) \ >> - fw_def(ELKHARTLAKE, 0, huc_def(ehl, 9, 0, 0)) \ >> - fw_def(ICELAKE, 0, huc_def(icl, 9, 0, 0)) \ >> - fw_def(COMETLAKE, 5, huc_def(cml, 4, 0, 0)) \ >> - fw_def(COMETLAKE, 0, huc_def(kbl, 4, 0, 0)) \ >> - fw_def(COFFEELAKE, 0, huc_def(kbl, 4, 0, 0)) \ >> - fw_def(GEMINILAKE, 0, huc_def(glk, 4, 0, 0)) \ >> - fw_def(KABYLAKE, 0, huc_def(kbl, 4, 0, 0)) \ >> - fw_def(BROXTON, 0, huc_def(bxt, 2, 0, 0)) \ >> - fw_def(SKYLAKE, 0, huc_def(skl, 2, 0, 0)) >> - >> -#define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \ >> +#define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \ >> + fw_def(DG2, 0, guc_mmp(dg2, 70, 4, 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_mmp(tgl, 70, 1, 1)) \ >> + fw_def(ALDERLAKE_S, 0, guc_mmp(tgl, 69, 0, 3)) \ >> + fw_def(DG1, 0, guc_mmp(dg1, 70, 1, 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)) \ >> + fw_def(ELKHARTLAKE, 0, guc_mmp(ehl, 70, 1, 1)) \ >> + fw_def(ICELAKE, 0, guc_mmp(icl, 70, 1, 1)) \ >> + fw_def(COMETLAKE, 5, guc_mmp(cml, 70, 1, 1)) \ >> + fw_def(COMETLAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ >> + fw_def(COFFEELAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ >> + fw_def(GEMINILAKE, 0, guc_mmp(glk, 70, 1, 1)) \ >> + fw_def(KABYLAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ >> + fw_def(BROXTON, 0, guc_mmp(bxt, 70, 1, 1)) \ >> + fw_def(SKYLAKE, 0, guc_mmp(skl, 70, 1, 1)) >> + >> +#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp) \ >> + fw_def(ALDERLAKE_P, 0, huc_mmp(tgl, 7, 9, 3)) \ >> + fw_def(ALDERLAKE_S, 0, huc_mmp(tgl, 7, 9, 3)) \ >> + fw_def(DG1, 0, huc_mmp(dg1, 7, 9, 3)) \ >> + fw_def(ROCKETLAKE, 0, huc_mmp(tgl, 7, 9, 3)) \ >> + fw_def(TIGERLAKE, 0, huc_mmp(tgl, 7, 9, 3)) \ >> + fw_def(JASPERLAKE, 0, huc_mmp(ehl, 9, 0, 0)) \ >> + fw_def(ELKHARTLAKE, 0, huc_mmp(ehl, 9, 0, 0)) \ >> + fw_def(ICELAKE, 0, huc_mmp(icl, 9, 0, 0)) \ >> + fw_def(COMETLAKE, 5, huc_mmp(cml, 4, 0, 0)) \ >> + fw_def(COMETLAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ >> + fw_def(COFFEELAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ >> + fw_def(GEMINILAKE, 0, huc_mmp(glk, 4, 0, 0)) \ >> + fw_def(KABYLAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ >> + fw_def(BROXTON, 0, huc_mmp(bxt, 2, 0, 0)) \ >> + fw_def(SKYLAKE, 0, huc_mmp(skl, 2, 0, 0)) >> + >> +/* >> + * Set of macros for producing a list of filenames from the above >> table. >> + */ >> +#define __MAKE_UC_FW_PATH_BLANK(prefix_, name_) \ >> + "i915/" \ >> + __stringify(prefix_) name_ ".bin" >> + >> +#define __MAKE_UC_FW_PATH_MAJOR(prefix_, name_, major_) \ >> + "i915/" \ >> + __stringify(prefix_) name_ \ >> + __stringify(major_) ".bin" >> + >> +#define __MAKE_UC_FW_PATH_MMP(prefix_, name_, major_, minor_, patch_) \ >> "i915/" \ >> __stringify(prefix_) name_ \ >> __stringify(major_) "." \ >> __stringify(minor_) "." \ >> __stringify(patch_) ".bin" >> -#define MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \ >> - __MAKE_UC_FW_PATH(prefix_, "_guc_", major_, minor_, patch_) >> +/* Minor for internal driver use, not part of file name */ >> +#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_) \ >> + __MAKE_UC_FW_PATH_MAJOR(prefix_, "_guc_", major_) >> -#define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \ >> - __MAKE_UC_FW_PATH(prefix_, "_huc_", major_, minor_, bld_num_) >> +#define MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \ >> + __MAKE_UC_FW_PATH_MMP(prefix_, "_guc_", major_, minor_, patch_) >> -/* All blobs need to be declared via MODULE_FIRMWARE() */ >> +#define MAKE_HUC_FW_PATH_BLANK(prefix_) \ >> + __MAKE_UC_FW_PATH_BLANK(prefix_, "_huc") >> + >> +#define MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \ >> + __MAKE_UC_FW_PATH_MMP(prefix_, "_huc_", major_, minor_, patch_) >> + >> +/* >> + * All blobs need to be declared via MODULE_FIRMWARE(). >> + * This first expansion of the table macros is solely to provide >> + * that declaration. >> + */ >> #define INTEL_UC_MODULE_FW(platform_, revid_, uc_) \ >> MODULE_FIRMWARE(uc_); >> -INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH) >> -INTEL_GUC_FIRMWARE_DEFS_FALLBACK(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH) >> -INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH) >> +INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH_MAJOR, >> MAKE_GUC_FW_PATH_MMP) >> +INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK, >> MAKE_HUC_FW_PATH_MMP) >> -/* The below structs and macros are used to iterate across the >> list of blobs */ >> +/* >> + * The next expansion of the table macros (in __uc_fw_auto_select >> below) provides >> + * actual data structures with both the filename and the version >> information. >> + * These structure arrays are then iterated over to the list of >> suitable files >> + * for the current platform and to then attempt to load those files, >> in the order >> + * listed, until one is successfully found. >> + */ >> struct __packed uc_fw_blob { >> + const char *path; >> + bool legacy; >> u8 major; >> u8 minor; >> - const char *path; >> + u8 patch; >> }; >> -#define UC_FW_BLOB(major_, minor_, path_) \ >> - { .major = major_, .minor = minor_, .path = path_ } >> +#define UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ >> + .major = major_, \ >> + .minor = minor_, \ >> + .patch = patch_, \ >> + .path = path_, >> + >> +#define UC_FW_BLOB_NEW(major_, minor_, patch_, path_) \ >> + { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ >> + .legacy = false } >> + >> +#define UC_FW_BLOB_OLD(major_, minor_, patch_, path_) \ >> + { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ >> + .legacy = true } >> -#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \ >> - UC_FW_BLOB(major_, minor_, \ >> - MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_)) >> +#define GUC_FW_BLOB(prefix_, major_, minor_) \ >> + UC_FW_BLOB_NEW(major_, minor_, 0, \ >> + MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_)) >> -#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \ >> - UC_FW_BLOB(major_, minor_, \ >> - MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_)) >> +#define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \ >> + UC_FW_BLOB_OLD(major_, minor_, patch_, \ >> + MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_)) >> + >> +#define HUC_FW_BLOB(prefix_) \ >> + UC_FW_BLOB_NEW(0, 0, 0, MAKE_HUC_FW_PATH_BLANK(prefix_)) >> + >> +#define HUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \ >> + UC_FW_BLOB_OLD(major_, minor_, patch_, \ >> + MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_)) >> struct __packed uc_fw_platform_requirement { >> enum intel_platform p; >> @@ -152,18 +217,16 @@ 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) >> - }; >> - static const struct uc_fw_platform_requirement >> blobs_guc_fallback[] = { >> - INTEL_GUC_FIRMWARE_DEFS_FALLBACK(MAKE_FW_LIST, GUC_FW_BLOB) >> + 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) >> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, >> HUC_FW_BLOB_MMP) >> }; >> 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; > > is this static to only run the test once per boot? Yes. No point in doing the same test per firmware blob attempt per GT per device when the list itself is static and immutable. > >> const struct uc_fw_platform_requirement *fw_blobs; >> enum intel_platform p = INTEL_INFO(i915)->platform; >> u32 fw_count; >> @@ -184,49 +247,94 @@ __uc_fw_auto_select(struct drm_i915_private >> *i915, struct intel_uc_fw *uc_fw) >> fw_count = blobs_all[uc_fw->type].count; >> for (i = 0; i < fw_count && p <= fw_blobs[i].p; i++) { >> - if (p == fw_blobs[i].p && rev >= fw_blobs[i].rev) { >> - const struct uc_fw_blob *blob = &fw_blobs[i].blob; >> - uc_fw->path = blob->path; >> - uc_fw->wanted_path = blob->path; >> - uc_fw->major_ver_wanted = blob->major; >> - uc_fw->minor_ver_wanted = blob->minor; >> - break; >> - } >> - } >> + const struct uc_fw_blob *blob = &fw_blobs[i].blob; >> - if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) { >> - const struct uc_fw_platform_requirement *blobs = >> blobs_guc_fallback; >> - u32 count = ARRAY_SIZE(blobs_guc_fallback); >> + if (p != fw_blobs[i].p) >> + continue; >> - for (i = 0; i < count && p <= blobs[i].p; i++) { >> - if (p == blobs[i].p && rev >= blobs[i].rev) { >> - const struct uc_fw_blob *blob = &blobs[i].blob; >> + if (rev < fw_blobs[i].rev) >> + continue; >> - uc_fw->fallback.path = blob->path; >> - uc_fw->fallback.major_ver = blob->major; >> - uc_fw->fallback.minor_ver = blob->minor; >> - break; >> - } >> + if (uc_fw->file_selected.path) { >> + if (uc_fw->file_selected.path == blob->path) >> + uc_fw->file_selected.path = NULL; >> + >> + continue; >> } > > Does this work with 3 versions for the same platform if the 2 newer > ones are not available? let's say we have v1, v2 and v3. v3 (newer) > gets selected first but not found, so we call back in this function > again and match v3 in the path check, clear the selected and continue, > selecting v2 in the next round. if v2 is also not found, we call back > in here starting from v3, which will not match the path check, so > we'll continue below and assign it. Or am I missing something? The second round will call back in with v2 as 'selected' not v3. So it will skip over v3, match on v2 and return v1. John. > > everything else looks ok. > > Daniele > >> + >> + uc_fw->file_selected.path = blob->path; >> + uc_fw->file_wanted.path = blob->path; >> + uc_fw->file_wanted.major_ver = blob->major; >> + uc_fw->file_wanted.minor_ver = blob->minor; >> + break; >> } >> /* make sure the list is ordered as expected */ >> - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) { >> + if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified) { >> + verified = true; >> + >> for (i = 1; i < fw_count; i++) { >> + /* Next platform is good: */ >> if (fw_blobs[i].p < fw_blobs[i - 1].p) >> continue; >> + /* 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; >> - drm_err(&i915->drm, "Invalid FW blob order: %s r%u >> comes before %s r%u\n", >> - intel_platform_name(fw_blobs[i - 1].p), >> - fw_blobs[i - 1].rev, >> - intel_platform_name(fw_blobs[i].p), >> - fw_blobs[i].rev); >> + /* 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; >> + >> + /* Next major version is good: */ >> + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) >> + continue; >> + >> + /* New must be before legacy: */ >> + if (!fw_blobs[i].blob.legacy && fw_blobs[i - >> 1].blob.legacy) >> + goto bad; >> + >> + /* 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; >> + >> + 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; >> - uc_fw->path = NULL; >> + /* 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, "\x1B[35;1mInvalid FW blob order: %s >> r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", >> + 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; >> } >> } >> } >> @@ -259,7 +367,7 @@ static void __uc_fw_user_override(struct >> drm_i915_private *i915, struct intel_uc >> } >> if (unlikely(path)) { >> - uc_fw->path = path; >> + uc_fw->file_selected.path = path; >> uc_fw->user_overridden = true; >> } >> } >> @@ -283,7 +391,7 @@ void intel_uc_fw_init_early(struct intel_uc_fw >> *uc_fw, >> */ >> BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); >> GEM_BUG_ON(uc_fw->status); >> - GEM_BUG_ON(uc_fw->path); >> + GEM_BUG_ON(uc_fw->file_selected.path); >> uc_fw->type = type; >> @@ -292,7 +400,7 @@ void intel_uc_fw_init_early(struct intel_uc_fw >> *uc_fw, >> __uc_fw_user_override(i915, uc_fw); >> } >> - intel_uc_fw_change_status(uc_fw, uc_fw->path ? *uc_fw->path ? >> + intel_uc_fw_change_status(uc_fw, uc_fw->file_selected.path ? >> *uc_fw->file_selected.path ? >> INTEL_UC_FIRMWARE_SELECTED : >> INTEL_UC_FIRMWARE_DISABLED : >> INTEL_UC_FIRMWARE_NOT_SUPPORTED); >> @@ -305,32 +413,32 @@ static void __force_fw_fetch_failures(struct >> intel_uc_fw *uc_fw, int e) >> if (i915_inject_probe_error(i915, e)) { >> /* non-existing blob */ >> - uc_fw->path = "<invalid>"; >> + uc_fw->file_selected.path = "<invalid>"; >> uc_fw->user_overridden = user; >> } else if (i915_inject_probe_error(i915, e)) { >> /* require next major version */ >> - uc_fw->major_ver_wanted += 1; >> - uc_fw->minor_ver_wanted = 0; >> + uc_fw->file_wanted.major_ver += 1; >> + uc_fw->file_wanted.minor_ver = 0; >> uc_fw->user_overridden = user; >> } else if (i915_inject_probe_error(i915, e)) { >> /* require next minor version */ >> - uc_fw->minor_ver_wanted += 1; >> + uc_fw->file_wanted.minor_ver += 1; >> uc_fw->user_overridden = user; >> - } else if (uc_fw->major_ver_wanted && >> + } else if (uc_fw->file_wanted.major_ver && >> i915_inject_probe_error(i915, e)) { >> /* require prev major version */ >> - uc_fw->major_ver_wanted -= 1; >> - uc_fw->minor_ver_wanted = 0; >> + uc_fw->file_wanted.major_ver -= 1; >> + uc_fw->file_wanted.minor_ver = 0; >> uc_fw->user_overridden = user; >> - } else if (uc_fw->minor_ver_wanted && >> + } else if (uc_fw->file_wanted.minor_ver && >> i915_inject_probe_error(i915, e)) { >> /* require prev minor version - hey, this should work! */ >> - uc_fw->minor_ver_wanted -= 1; >> + uc_fw->file_wanted.minor_ver -= 1; >> uc_fw->user_overridden = user; >> } else if (user && i915_inject_probe_error(i915, e)) { >> /* officially unsupported platform */ >> - uc_fw->major_ver_wanted = 0; >> - uc_fw->minor_ver_wanted = 0; >> + uc_fw->file_wanted.major_ver = 0; >> + uc_fw->file_wanted.minor_ver = 0; >> uc_fw->user_overridden = true; >> } >> } >> @@ -341,8 +449,8 @@ static int check_gsc_manifest(const struct >> firmware *fw, >> u32 *dw = (u32 *)fw->data; >> u32 version = dw[HUC_GSC_VERSION_DW]; >> - uc_fw->major_ver_found = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, >> version); >> - uc_fw->minor_ver_found = FIELD_GET(HUC_GSC_MINOR_VER_MASK, >> version); >> + uc_fw->file_selected.major_ver = >> FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version); >> + uc_fw->file_selected.minor_ver = >> FIELD_GET(HUC_GSC_MINOR_VER_MASK, version); >> return 0; >> } >> @@ -357,7 +465,7 @@ static int check_ccs_header(struct >> drm_i915_private *i915, >> /* Check the size of the blob before examining buffer contents */ >> if (unlikely(fw->size < sizeof(struct uc_css_header))) { >> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < >> %zu\n", >> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >> + intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_selected.path, >> fw->size, sizeof(struct uc_css_header)); >> return -ENODATA; >> } >> @@ -370,7 +478,7 @@ static int check_ccs_header(struct >> drm_i915_private *i915, >> if (unlikely(size != sizeof(struct uc_css_header))) { >> drm_warn(&i915->drm, >> "%s firmware %s: unexpected header size: %zu != %zu\n", >> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >> + intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_selected.path, >> fw->size, sizeof(struct uc_css_header)); >> return -EPROTO; >> } >> @@ -385,7 +493,7 @@ static int check_ccs_header(struct >> drm_i915_private *i915, >> size = sizeof(struct uc_css_header) + uc_fw->ucode_size + >> uc_fw->rsa_size; >> if (unlikely(fw->size < size)) { >> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < >> %zu\n", >> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >> + intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_selected.path, >> fw->size, size); >> return -ENOEXEC; >> } >> @@ -394,16 +502,16 @@ static int check_ccs_header(struct >> drm_i915_private *i915, >> size = __intel_uc_fw_get_upload_size(uc_fw); >> if (unlikely(size >= i915->wopcm.size)) { >> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu > >> %zu\n", >> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >> + intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_selected.path, >> size, (size_t)i915->wopcm.size); >> return -E2BIG; >> } >> /* Get version numbers from the CSS header */ >> - uc_fw->major_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, >> - css->sw_version); >> - uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR, >> - css->sw_version); >> + uc_fw->file_selected.major_ver = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, >> + css->sw_version); >> + uc_fw->file_selected.minor_ver = FIELD_GET(CSS_SW_VERSION_UC_MINOR, >> + css->sw_version); >> if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) >> uc_fw->private_data_size = css->private_data_size; >> @@ -422,9 +530,11 @@ static int check_ccs_header(struct >> drm_i915_private *i915, >> int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >> { >> struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915; >> + struct intel_uc_fw_file file_ideal; >> struct device *dev = i915->drm.dev; >> struct drm_i915_gem_object *obj; >> const struct firmware *fw = NULL; >> + bool old_ver = false; >> int err; >> GEM_BUG_ON(!i915->wopcm.size); >> @@ -437,27 +547,33 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >> __force_fw_fetch_failures(uc_fw, -EINVAL); >> __force_fw_fetch_failures(uc_fw, -ESTALE); >> - err = firmware_request_nowarn(&fw, uc_fw->path, dev); >> - if (err && !intel_uc_fw_is_overridden(uc_fw) && >> uc_fw->fallback.path) { >> - err = firmware_request_nowarn(&fw, uc_fw->fallback.path, dev); >> - if (!err) { >> - drm_notice(&i915->drm, >> - "%s firmware %s is recommended, but only %s was >> found\n", >> - intel_uc_fw_type_repr(uc_fw->type), >> - uc_fw->wanted_path, >> - uc_fw->fallback.path); >> - drm_info(&i915->drm, >> - "Consider updating your linux-firmware pkg or >> downloading from %s\n", >> - INTEL_UC_FIRMWARE_URL); >> - >> - uc_fw->path = uc_fw->fallback.path; >> - uc_fw->major_ver_wanted = uc_fw->fallback.major_ver; >> - uc_fw->minor_ver_wanted = uc_fw->fallback.minor_ver; >> + err = firmware_request_nowarn(&fw, uc_fw->file_selected.path, dev); >> + memcpy(&file_ideal, &uc_fw->file_wanted, sizeof(file_ideal)); >> + if (!err || intel_uc_fw_is_overridden(uc_fw)) >> + goto done; >> + >> + while (err == -ENOENT) { >> + __uc_fw_auto_select(i915, uc_fw); >> + if (!uc_fw->file_selected.path) { >> + /* >> + * No more options! But set the path back to something >> + * valid just in case it gets dereferenced. >> + */ >> + uc_fw->file_selected.path = file_ideal.path; >> + >> + /* Also, preserve the version that was really wanted */ >> + memcpy(&uc_fw->file_wanted, &file_ideal, >> sizeof(uc_fw->file_wanted)); >> + break; >> } >> + >> + err = firmware_request_nowarn(&fw, >> uc_fw->file_selected.path, dev); >> } >> + >> if (err) >> goto fail; >> + old_ver = true; >> +done: >> if (uc_fw->loaded_via_gsc) >> err = check_gsc_manifest(fw, uc_fw); >> else >> @@ -465,18 +581,39 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >> if (err) >> goto fail; >> - if (uc_fw->major_ver_found != uc_fw->major_ver_wanted || >> - uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) { >> - drm_notice(&i915->drm, "%s firmware %s: unexpected version: >> %u.%u != %u.%u\n", >> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >> - uc_fw->major_ver_found, uc_fw->minor_ver_found, >> - uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted); >> - if (!intel_uc_fw_is_overridden(uc_fw)) { >> - err = -ENOEXEC; >> - goto fail; >> + if (uc_fw->file_wanted.major_ver) { >> + /* Check the file's major version was as it claimed */ >> + if (uc_fw->file_selected.major_ver != >> uc_fw->file_wanted.major_ver) { >> + drm_notice(&i915->drm, "%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.major_ver, >> uc_fw->file_selected.minor_ver, >> + uc_fw->file_wanted.major_ver, >> uc_fw->file_wanted.minor_ver); >> + if (!intel_uc_fw_is_overridden(uc_fw)) { >> + err = -ENOEXEC; >> + goto fail; >> + } >> + } else { >> + if (uc_fw->file_selected.minor_ver < >> uc_fw->file_wanted.minor_ver) >> + old_ver = true; >> } >> } >> + if (old_ver) { >> + /* Preserve the version that was really wanted */ >> + memcpy(&uc_fw->file_wanted, &file_ideal, >> sizeof(uc_fw->file_wanted)); >> + >> + drm_notice(&i915->drm, >> + "%s firmware %s (%d.%d) is recommended, but only %s >> (%d.%d) was found\n", >> + intel_uc_fw_type_repr(uc_fw->type), >> + uc_fw->file_wanted.path, >> + uc_fw->file_wanted.major_ver, >> uc_fw->file_wanted.minor_ver, >> + uc_fw->file_selected.path, >> + uc_fw->file_selected.major_ver, >> uc_fw->file_selected.minor_ver); >> + drm_info(&i915->drm, >> + "Consider updating your linux-firmware pkg or >> downloading from %s\n", >> + INTEL_UC_FIRMWARE_URL); >> + } >> + >> if (HAS_LMEM(i915)) { >> obj = i915_gem_object_create_lmem_from_data(i915, fw->data, >> fw->size); >> if (!IS_ERR(obj)) >> @@ -503,7 +640,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >> INTEL_UC_FIRMWARE_ERROR); >> i915_probe_error(i915, "%s firmware %s: fetch failed with >> error %d\n", >> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err); >> + intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_selected.path, err); >> drm_info(&i915->drm, "%s firmware(s) can be downloaded from %s\n", >> intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL); >> @@ -645,7 +782,7 @@ int intel_uc_fw_upload(struct intel_uc_fw >> *uc_fw, u32 dst_offset, u32 dma_flags) >> fail: >> i915_probe_error(gt->i915, "Failed to load %s firmware %s (%d)\n", >> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >> + intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_selected.path, >> err); >> intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_LOAD_FAIL); >> return err; >> @@ -864,18 +1001,19 @@ size_t intel_uc_fw_copy_rsa(struct intel_uc_fw >> *uc_fw, void *dst, u32 max_len) >> void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct >> drm_printer *p) >> { >> drm_printf(p, "%s firmware: %s\n", >> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->wanted_path); >> - if (uc_fw->fallback.path) { >> - drm_printf(p, "%s firmware fallback: %s\n", >> - intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->fallback.path); >> - drm_printf(p, "fallback selected: %s\n", >> - str_yes_no(uc_fw->path == uc_fw->fallback.path)); >> - } >> + intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_selected.path); >> + if (uc_fw->file_selected.path != uc_fw->file_wanted.path) >> + drm_printf(p, "%s firmware wanted: %s\n", >> + intel_uc_fw_type_repr(uc_fw->type), >> uc_fw->file_wanted.path); >> drm_printf(p, "\tstatus: %s\n", >> intel_uc_fw_status_repr(uc_fw->status)); >> - drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n", >> - uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted, >> - uc_fw->major_ver_found, uc_fw->minor_ver_found); >> + if (uc_fw->file_wanted.major_ver) >> + drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n", >> + uc_fw->file_wanted.major_ver, >> uc_fw->file_wanted.minor_ver, >> + uc_fw->file_selected.major_ver, >> uc_fw->file_selected.minor_ver); >> + else >> + drm_printf(p, "\tversion: found %u.%u\n", >> + uc_fw->file_selected.major_ver, >> uc_fw->file_selected.minor_ver); >> drm_printf(p, "\tuCode: %u bytes\n", uc_fw->ucode_size); >> drm_printf(p, "\tRSA: %u bytes\n", uc_fw->rsa_size); >> } >> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >> index 7aa2644400b98..344763c942e37 100644 >> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >> @@ -64,6 +64,17 @@ enum intel_uc_fw_type { >> }; >> #define INTEL_UC_FW_NUM_TYPES 2 >> +/* >> + * The firmware build process will generate a version header file >> with major and >> + * minor version defined. The versions are built into CSS header of >> firmware. >> + * i915 kernel driver set the minimal firmware version required per >> platform. >> + */ >> +struct intel_uc_fw_file { >> + const char *path; >> + u16 major_ver; >> + u16 minor_ver; >> +}; >> + >> /* >> * This structure encapsulates all the data needed during the process >> * of fetching, caching, and loading the firmware image into the uC. >> @@ -74,11 +85,12 @@ struct intel_uc_fw { >> const enum intel_uc_fw_status status; >> enum intel_uc_fw_status __status; /* no accidental >> overwrites */ >> }; >> - const char *wanted_path; >> - const char *path; >> + struct intel_uc_fw_file file_wanted; >> + struct intel_uc_fw_file file_selected; >> bool user_overridden; >> size_t size; >> struct drm_i915_gem_object *obj; >> + >> /** >> * @dummy: A vma used in binding the uc fw to ggtt. We can't >> define this >> * vma on the stack as it can lead to a stack overflow, so we >> define it >> @@ -89,25 +101,8 @@ struct intel_uc_fw { >> struct i915_vma_resource dummy; >> struct i915_vma *rsa_data; >> - /* >> - * The firmware build process will generate a version header >> file with major and >> - * minor version defined. The versions are built into CSS header >> of firmware. >> - * i915 kernel driver set the minimal firmware version required >> per platform. >> - */ >> - u16 major_ver_wanted; >> - u16 minor_ver_wanted; >> - u16 major_ver_found; >> - u16 minor_ver_found; >> - >> - struct { >> - const char *path; >> - u16 major_ver; >> - u16 minor_ver; >> - } fallback; >> - >> u32 rsa_size; >> u32 ucode_size; >> - >> u32 private_data_size; >> bool loaded_via_gsc; >> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c >> b/drivers/gpu/drm/i915/i915_gpu_error.c >> index b5fbc2252784a..9ea2fe34e7d30 100644 >> --- a/drivers/gpu/drm/i915/i915_gpu_error.c >> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c >> @@ -1022,8 +1022,10 @@ static void cleanup_params(struct >> i915_gpu_coredump *error) >> static void cleanup_uc(struct intel_uc_coredump *uc) >> { >> - kfree(uc->guc_fw.path); >> - kfree(uc->huc_fw.path); >> + kfree(uc->guc_fw.file_selected.path); >> + kfree(uc->huc_fw.file_selected.path); >> + kfree(uc->guc_fw.file_wanted.path); >> + kfree(uc->huc_fw.file_wanted.path); >> i915_vma_coredump_free(uc->guc.vma_log); >> i915_vma_coredump_free(uc->guc.vma_ctb); >> @@ -1705,12 +1707,10 @@ gt_record_uc(struct intel_gt_coredump *gt, >> memcpy(&error_uc->guc_fw, &uc->guc.fw, sizeof(uc->guc.fw)); >> memcpy(&error_uc->huc_fw, &uc->huc.fw, sizeof(uc->huc.fw)); >> - /* Non-default firmware paths will be specified by the modparam. >> - * As modparams are generally accesible from the userspace make >> - * explicit copies of the firmware paths. >> - */ >> - error_uc->guc_fw.path = kstrdup(uc->guc.fw.path, ALLOW_FAIL); >> - error_uc->huc_fw.path = kstrdup(uc->huc.fw.path, ALLOW_FAIL); >> + error_uc->guc_fw.file_selected.path = >> kstrdup(uc->guc.fw.file_selected.path, ALLOW_FAIL); >> + error_uc->huc_fw.file_selected.path = >> kstrdup(uc->huc.fw.file_selected.path, ALLOW_FAIL); >> + error_uc->guc_fw.file_wanted.path = >> kstrdup(uc->guc.fw.file_wanted.path, ALLOW_FAIL); >> + error_uc->huc_fw.file_wanted.path = >> kstrdup(uc->huc.fw.file_wanted.path, ALLOW_FAIL); >> /* >> * Save the GuC log and include a timestamp reference for >> converting the >
On 8/26/2022 9:40 AM, John Harrison wrote: > On 8/26/2022 09:35, Ceraolo Spurio, Daniele wrote: >> On 8/25/2022 8:05 PM, John.C.Harrison@Intel.com wrote: >>> From: John Harrison <John.C.Harrison@Intel.com> >>> >>> There was a misunderstanding in how firmware file compatibility should >>> be managed within i915. This has been clarified as: >>> i915 must support all existing firmware releases forever >>> new minor firmware releases should replace prior versions >>> only backwards compatibility breaking releases should be a new file >>> >>> This patch cleans up the single fallback file support that was added >>> as a quick fix emergency effort. That is now removed in preference to >>> supporting arbitrary numbers of firmware files per platform. >>> >>> The patch also adds support for having GuC firmware files that are >>> named by major version only (because the major version indicates >>> backwards breaking changes that affect the KMD) and for having HuC >>> firmware files with no version number at all (because the KMD has no >>> interface requirements with the HuC). >>> >>> For GuC, the driver will report via dmesg if the found file is older >>> than >>> expected. For HuC, the KMD will no longer require updating for any new >>> HuC release so will not be able to report what the latest expected >>> version is. >>> >>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com> >>> --- >>> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 10 +- >>> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 4 +- >>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 442 >>> ++++++++++++------ >>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 33 +- >>> drivers/gpu/drm/i915/i915_gpu_error.c | 16 +- >>> 5 files changed, 319 insertions(+), 186 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >>> index 0d56b615bf78e..04393932623c7 100644 >>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c >>> @@ -1868,7 +1868,7 @@ int intel_guc_submission_init(struct intel_guc >>> *guc) >>> if (guc->submission_initialized) >>> return 0; >>> - if (guc->fw.major_ver_found < 70) { >>> + if (guc->fw.file_selected.major_ver < 70) { >>> ret = guc_lrc_desc_pool_create_v69(guc); >>> if (ret) >>> return ret; >>> @@ -2303,7 +2303,7 @@ static int register_context(struct >>> intel_context *ce, bool loop) >>> GEM_BUG_ON(intel_context_is_child(ce)); >>> trace_intel_context_register(ce); >>> - if (guc->fw.major_ver_found >= 70) >>> + if (guc->fw.file_selected.major_ver >= 70) >>> ret = register_context_v70(guc, ce, loop); >>> else >>> ret = register_context_v69(guc, ce, loop); >>> @@ -2315,7 +2315,7 @@ static int register_context(struct >>> intel_context *ce, bool loop) >>> set_context_registered(ce); >>> spin_unlock_irqrestore(&ce->guc_state.lock, flags); >>> - if (guc->fw.major_ver_found >= 70) >>> + if (guc->fw.file_selected.major_ver >= 70) >>> guc_context_policy_init_v70(ce, loop); >>> } >>> @@ -2921,7 +2921,7 @@ static void >>> __guc_context_set_preemption_timeout(struct intel_guc *guc, >>> u16 guc_id, >>> u32 preemption_timeout) >>> { >>> - if (guc->fw.major_ver_found >= 70) { >>> + if (guc->fw.file_selected.major_ver >= 70) { >>> struct context_policy policy; >>> __guc_context_policy_start_klv(&policy, guc_id); >>> @@ -3186,7 +3186,7 @@ static int guc_context_alloc(struct >>> intel_context *ce) >>> static void __guc_context_set_prio(struct intel_guc *guc, >>> struct intel_context *ce) >>> { >>> - if (guc->fw.major_ver_found >= 70) { >>> + if (guc->fw.file_selected.major_ver >= 70) { >>> struct context_policy policy; >>> __guc_context_policy_start_klv(&policy, ce->guc_id.id); >>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c >>> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c >>> index f2e7c82985efd..d965ac4832d60 100644 >>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c >>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c >>> @@ -436,8 +436,8 @@ static void print_fw_ver(struct intel_uc *uc, >>> struct intel_uc_fw *fw) >>> struct drm_i915_private *i915 = uc_to_gt(uc)->i915; >>> drm_info(&i915->drm, "%s firmware %s version %u.%u\n", >>> - intel_uc_fw_type_repr(fw->type), fw->path, >>> - fw->major_ver_found, fw->minor_ver_found); >>> + intel_uc_fw_type_repr(fw->type), fw->file_selected.path, >>> + fw->file_selected.major_ver, fw->file_selected.minor_ver); >>> } >>> static int __uc_init_hw(struct intel_uc *uc) >>> 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 58547292efa0a..94cf2d4a46e6f 100644 >>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c >>> @@ -41,7 +41,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw >>> *uc_fw, >>> "%s firmware -> %s\n", >>> intel_uc_fw_type_repr(uc_fw->type), >>> status == INTEL_UC_FIRMWARE_SELECTED ? >>> - uc_fw->path : intel_uc_fw_status_repr(status)); >>> + uc_fw->file_selected.path : intel_uc_fw_status_repr(status)); >>> } >>> #endif >>> @@ -51,84 +51,149 @@ void intel_uc_fw_change_status(struct >>> intel_uc_fw *uc_fw, >>> * >>> * Note that RKL and ADL-S have the same GuC/HuC device ID's and >>> use the same >>> * firmware as TGL. >>> + * >>> + * Version numbers: >>> + * Originally, the driver required an exact match major/minor/patch >>> furmware >>> + * file and only supported that one version for any given platform. >>> However, >>> + * the new direction from upstream is to be becakwards compatible >>> with all >> >> typo becakwards >> >>> + * prior releases and to be as flexible as possible as to what >>> firmware is >>> + * loaded. >>> + * >>> + * For GuC, the major version number signifies a backwards breaking >>> API change. >>> + * So, new format GuC firmware files are labelled by their major >>> version only. >>> + * For HuC, there is no KMD interaction, hence no version matching >>> requirement. >>> + * So, new format HuC firmware files have no version number at all. >>> + * >>> + * All of which means that the table below must keep all old format >>> files with >>> + * full three point version number. But newer files have reduced >>> requirements. >>> + * Having said that, the driver still needs to track the minor >>> version number >>> + * for GuC at least. As it is useful to report to the user that >>> they are not >>> + * running with a recent enough version for all KMD supported >>> features, >>> + * security fixes, etc. to be enabled. >>> */ >>> -#define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_def) \ >>> - fw_def(DG2, 0, guc_def(dg2, 70, 4, 1)) \ >>> - fw_def(ALDERLAKE_P, 0, guc_def(adlp, 70, 1, 1)) \ >>> - fw_def(ALDERLAKE_S, 0, guc_def(tgl, 70, 1, 1)) \ >>> - fw_def(DG1, 0, guc_def(dg1, 70, 1, 1)) \ >>> - fw_def(ROCKETLAKE, 0, guc_def(tgl, 70, 1, 1)) \ >>> - fw_def(TIGERLAKE, 0, guc_def(tgl, 70, 1, 1)) \ >>> - fw_def(JASPERLAKE, 0, guc_def(ehl, 70, 1, 1)) \ >>> - fw_def(ELKHARTLAKE, 0, guc_def(ehl, 70, 1, 1)) \ >>> - fw_def(ICELAKE, 0, guc_def(icl, 70, 1, 1)) \ >>> - fw_def(COMETLAKE, 5, guc_def(cml, 70, 1, 1)) \ >>> - fw_def(COMETLAKE, 0, guc_def(kbl, 70, 1, 1)) \ >>> - fw_def(COFFEELAKE, 0, guc_def(kbl, 70, 1, 1)) \ >>> - fw_def(GEMINILAKE, 0, guc_def(glk, 70, 1, 1)) \ >>> - fw_def(KABYLAKE, 0, guc_def(kbl, 70, 1, 1)) \ >>> - fw_def(BROXTON, 0, guc_def(bxt, 70, 1, 1)) \ >>> - fw_def(SKYLAKE, 0, guc_def(skl, 70, 1, 1)) >>> - >>> -#define INTEL_GUC_FIRMWARE_DEFS_FALLBACK(fw_def, guc_def) \ >>> - fw_def(ALDERLAKE_P, 0, guc_def(adlp, 69, 0, 3)) \ >>> - fw_def(ALDERLAKE_S, 0, guc_def(tgl, 69, 0, 3)) >>> - >>> -#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_def) \ >>> - fw_def(ALDERLAKE_P, 0, huc_def(tgl, 7, 9, 3)) \ >>> - fw_def(ALDERLAKE_S, 0, huc_def(tgl, 7, 9, 3)) \ >>> - fw_def(DG1, 0, huc_def(dg1, 7, 9, 3)) \ >>> - fw_def(ROCKETLAKE, 0, huc_def(tgl, 7, 9, 3)) \ >>> - fw_def(TIGERLAKE, 0, huc_def(tgl, 7, 9, 3)) \ >>> - fw_def(JASPERLAKE, 0, huc_def(ehl, 9, 0, 0)) \ >>> - fw_def(ELKHARTLAKE, 0, huc_def(ehl, 9, 0, 0)) \ >>> - fw_def(ICELAKE, 0, huc_def(icl, 9, 0, 0)) \ >>> - fw_def(COMETLAKE, 5, huc_def(cml, 4, 0, 0)) \ >>> - fw_def(COMETLAKE, 0, huc_def(kbl, 4, 0, 0)) \ >>> - fw_def(COFFEELAKE, 0, huc_def(kbl, 4, 0, 0)) \ >>> - fw_def(GEMINILAKE, 0, huc_def(glk, 4, 0, 0)) \ >>> - fw_def(KABYLAKE, 0, huc_def(kbl, 4, 0, 0)) \ >>> - fw_def(BROXTON, 0, huc_def(bxt, 2, 0, 0)) \ >>> - fw_def(SKYLAKE, 0, huc_def(skl, 2, 0, 0)) >>> - >>> -#define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \ >>> +#define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \ >>> + fw_def(DG2, 0, guc_mmp(dg2, 70, 4, 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_mmp(tgl, 70, 1, 1)) \ >>> + fw_def(ALDERLAKE_S, 0, guc_mmp(tgl, 69, 0, 3)) \ >>> + fw_def(DG1, 0, guc_mmp(dg1, 70, 1, 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)) \ >>> + fw_def(ELKHARTLAKE, 0, guc_mmp(ehl, 70, 1, 1)) \ >>> + fw_def(ICELAKE, 0, guc_mmp(icl, 70, 1, 1)) \ >>> + fw_def(COMETLAKE, 5, guc_mmp(cml, 70, 1, 1)) \ >>> + fw_def(COMETLAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ >>> + fw_def(COFFEELAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ >>> + fw_def(GEMINILAKE, 0, guc_mmp(glk, 70, 1, 1)) \ >>> + fw_def(KABYLAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ >>> + fw_def(BROXTON, 0, guc_mmp(bxt, 70, 1, 1)) \ >>> + fw_def(SKYLAKE, 0, guc_mmp(skl, 70, 1, 1)) >>> + >>> +#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp) \ >>> + fw_def(ALDERLAKE_P, 0, huc_mmp(tgl, 7, 9, 3)) \ >>> + fw_def(ALDERLAKE_S, 0, huc_mmp(tgl, 7, 9, 3)) \ >>> + fw_def(DG1, 0, huc_mmp(dg1, 7, 9, 3)) \ >>> + fw_def(ROCKETLAKE, 0, huc_mmp(tgl, 7, 9, 3)) \ >>> + fw_def(TIGERLAKE, 0, huc_mmp(tgl, 7, 9, 3)) \ >>> + fw_def(JASPERLAKE, 0, huc_mmp(ehl, 9, 0, 0)) \ >>> + fw_def(ELKHARTLAKE, 0, huc_mmp(ehl, 9, 0, 0)) \ >>> + fw_def(ICELAKE, 0, huc_mmp(icl, 9, 0, 0)) \ >>> + fw_def(COMETLAKE, 5, huc_mmp(cml, 4, 0, 0)) \ >>> + fw_def(COMETLAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ >>> + fw_def(COFFEELAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ >>> + fw_def(GEMINILAKE, 0, huc_mmp(glk, 4, 0, 0)) \ >>> + fw_def(KABYLAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ >>> + fw_def(BROXTON, 0, huc_mmp(bxt, 2, 0, 0)) \ >>> + fw_def(SKYLAKE, 0, huc_mmp(skl, 2, 0, 0)) >>> + >>> +/* >>> + * Set of macros for producing a list of filenames from the above >>> table. >>> + */ >>> +#define __MAKE_UC_FW_PATH_BLANK(prefix_, name_) \ >>> + "i915/" \ >>> + __stringify(prefix_) name_ ".bin" >>> + >>> +#define __MAKE_UC_FW_PATH_MAJOR(prefix_, name_, major_) \ >>> + "i915/" \ >>> + __stringify(prefix_) name_ \ >>> + __stringify(major_) ".bin" >>> + >>> +#define __MAKE_UC_FW_PATH_MMP(prefix_, name_, major_, minor_, >>> patch_) \ >>> "i915/" \ >>> __stringify(prefix_) name_ \ >>> __stringify(major_) "." \ >>> __stringify(minor_) "." \ >>> __stringify(patch_) ".bin" >>> -#define MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \ >>> - __MAKE_UC_FW_PATH(prefix_, "_guc_", major_, minor_, patch_) >>> +/* Minor for internal driver use, not part of file name */ >>> +#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_) \ >>> + __MAKE_UC_FW_PATH_MAJOR(prefix_, "_guc_", major_) >>> -#define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \ >>> - __MAKE_UC_FW_PATH(prefix_, "_huc_", major_, minor_, bld_num_) >>> +#define MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \ >>> + __MAKE_UC_FW_PATH_MMP(prefix_, "_guc_", major_, minor_, patch_) >>> -/* All blobs need to be declared via MODULE_FIRMWARE() */ >>> +#define MAKE_HUC_FW_PATH_BLANK(prefix_) \ >>> + __MAKE_UC_FW_PATH_BLANK(prefix_, "_huc") >>> + >>> +#define MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \ >>> + __MAKE_UC_FW_PATH_MMP(prefix_, "_huc_", major_, minor_, patch_) >>> + >>> +/* >>> + * All blobs need to be declared via MODULE_FIRMWARE(). >>> + * This first expansion of the table macros is solely to provide >>> + * that declaration. >>> + */ >>> #define INTEL_UC_MODULE_FW(platform_, revid_, uc_) \ >>> MODULE_FIRMWARE(uc_); >>> -INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH) >>> -INTEL_GUC_FIRMWARE_DEFS_FALLBACK(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH) >>> -INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH) >>> +INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH_MAJOR, >>> MAKE_GUC_FW_PATH_MMP) >>> +INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK, >>> MAKE_HUC_FW_PATH_MMP) >>> -/* The below structs and macros are used to iterate across the >>> list of blobs */ >>> +/* >>> + * The next expansion of the table macros (in __uc_fw_auto_select >>> below) provides >>> + * actual data structures with both the filename and the version >>> information. >>> + * These structure arrays are then iterated over to the list of >>> suitable files >>> + * for the current platform and to then attempt to load those >>> files, in the order >>> + * listed, until one is successfully found. >>> + */ >>> struct __packed uc_fw_blob { >>> + const char *path; >>> + bool legacy; >>> u8 major; >>> u8 minor; >>> - const char *path; >>> + u8 patch; >>> }; >>> -#define UC_FW_BLOB(major_, minor_, path_) \ >>> - { .major = major_, .minor = minor_, .path = path_ } >>> +#define UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ >>> + .major = major_, \ >>> + .minor = minor_, \ >>> + .patch = patch_, \ >>> + .path = path_, >>> + >>> +#define UC_FW_BLOB_NEW(major_, minor_, patch_, path_) \ >>> + { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ >>> + .legacy = false } >>> + >>> +#define UC_FW_BLOB_OLD(major_, minor_, patch_, path_) \ >>> + { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ >>> + .legacy = true } >>> -#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \ >>> - UC_FW_BLOB(major_, minor_, \ >>> - MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_)) >>> +#define GUC_FW_BLOB(prefix_, major_, minor_) \ >>> + UC_FW_BLOB_NEW(major_, minor_, 0, \ >>> + MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_)) >>> -#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \ >>> - UC_FW_BLOB(major_, minor_, \ >>> - MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_)) >>> +#define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \ >>> + UC_FW_BLOB_OLD(major_, minor_, patch_, \ >>> + MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_)) >>> + >>> +#define HUC_FW_BLOB(prefix_) \ >>> + UC_FW_BLOB_NEW(0, 0, 0, MAKE_HUC_FW_PATH_BLANK(prefix_)) >>> + >>> +#define HUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \ >>> + UC_FW_BLOB_OLD(major_, minor_, patch_, \ >>> + MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_)) >>> struct __packed uc_fw_platform_requirement { >>> enum intel_platform p; >>> @@ -152,18 +217,16 @@ 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) >>> - }; >>> - static const struct uc_fw_platform_requirement >>> blobs_guc_fallback[] = { >>> - INTEL_GUC_FIRMWARE_DEFS_FALLBACK(MAKE_FW_LIST, GUC_FW_BLOB) >>> + 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) >>> + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, >>> HUC_FW_BLOB_MMP) >>> }; >>> 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; >> >> is this static to only run the test once per boot? > Yes. > > No point in doing the same test per firmware blob attempt per GT per > device when the list itself is static and immutable. > >> >>> const struct uc_fw_platform_requirement *fw_blobs; >>> enum intel_platform p = INTEL_INFO(i915)->platform; >>> u32 fw_count; >>> @@ -184,49 +247,94 @@ __uc_fw_auto_select(struct drm_i915_private >>> *i915, struct intel_uc_fw *uc_fw) >>> fw_count = blobs_all[uc_fw->type].count; >>> for (i = 0; i < fw_count && p <= fw_blobs[i].p; i++) { >>> - if (p == fw_blobs[i].p && rev >= fw_blobs[i].rev) { >>> - const struct uc_fw_blob *blob = &fw_blobs[i].blob; >>> - uc_fw->path = blob->path; >>> - uc_fw->wanted_path = blob->path; >>> - uc_fw->major_ver_wanted = blob->major; >>> - uc_fw->minor_ver_wanted = blob->minor; >>> - break; >>> - } >>> - } >>> + const struct uc_fw_blob *blob = &fw_blobs[i].blob; >>> - if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) { >>> - const struct uc_fw_platform_requirement *blobs = >>> blobs_guc_fallback; >>> - u32 count = ARRAY_SIZE(blobs_guc_fallback); >>> + if (p != fw_blobs[i].p) >>> + continue; >>> - for (i = 0; i < count && p <= blobs[i].p; i++) { >>> - if (p == blobs[i].p && rev >= blobs[i].rev) { >>> - const struct uc_fw_blob *blob = &blobs[i].blob; >>> + if (rev < fw_blobs[i].rev) >>> + continue; >>> - uc_fw->fallback.path = blob->path; >>> - uc_fw->fallback.major_ver = blob->major; >>> - uc_fw->fallback.minor_ver = blob->minor; >>> - break; >>> - } >>> + if (uc_fw->file_selected.path) { >>> + if (uc_fw->file_selected.path == blob->path) >>> + uc_fw->file_selected.path = NULL; >>> + >>> + continue; >>> } >> >> Does this work with 3 versions for the same platform if the 2 newer >> ones are not available? let's say we have v1, v2 and v3. v3 (newer) >> gets selected first but not found, so we call back in this function >> again and match v3 in the path check, clear the selected and >> continue, selecting v2 in the next round. if v2 is also not found, we >> call back in here starting from v3, which will not match the path >> check, so we'll continue below and assign it. Or am I missing something? > The second round will call back in with v2 as 'selected' not v3. So it > will skip over v3, match on v2 and return v1. My bad, I had read this as if the "continue" was in the inner if and not the outer one, hence my confusion. It's ok like this. I do think we might want still want to add a test or something to check that fallbacks with more than one version work, but we can do that as a follow up as such case doesn't currently exist. with that typo fixed: Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > > John. > >> >> everything else looks ok. >> >> Daniele >> >>> + >>> + uc_fw->file_selected.path = blob->path; >>> + uc_fw->file_wanted.path = blob->path; >>> + uc_fw->file_wanted.major_ver = blob->major; >>> + uc_fw->file_wanted.minor_ver = blob->minor; >>> + break; >>> } >>> /* make sure the list is ordered as expected */ >>> - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) { >>> + if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified) { >>> + verified = true; >>> + >>> for (i = 1; i < fw_count; i++) { >>> + /* Next platform is good: */ >>> if (fw_blobs[i].p < fw_blobs[i - 1].p) >>> continue; >>> + /* 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; >>> - drm_err(&i915->drm, "Invalid FW blob order: %s r%u >>> comes before %s r%u\n", >>> - intel_platform_name(fw_blobs[i - 1].p), >>> - fw_blobs[i - 1].rev, >>> - intel_platform_name(fw_blobs[i].p), >>> - fw_blobs[i].rev); >>> + /* 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; >>> + >>> + /* Next major version is good: */ >>> + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) >>> + continue; >>> + >>> + /* New must be before legacy: */ >>> + if (!fw_blobs[i].blob.legacy && fw_blobs[i - >>> 1].blob.legacy) >>> + goto bad; >>> + >>> + /* 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; >>> + >>> + 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; >>> - uc_fw->path = NULL; >>> + /* 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, "\x1B[35;1mInvalid FW blob order: >>> %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", >>> + 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; >>> } >>> } >>> } >>> @@ -259,7 +367,7 @@ static void __uc_fw_user_override(struct >>> drm_i915_private *i915, struct intel_uc >>> } >>> if (unlikely(path)) { >>> - uc_fw->path = path; >>> + uc_fw->file_selected.path = path; >>> uc_fw->user_overridden = true; >>> } >>> } >>> @@ -283,7 +391,7 @@ void intel_uc_fw_init_early(struct intel_uc_fw >>> *uc_fw, >>> */ >>> BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); >>> GEM_BUG_ON(uc_fw->status); >>> - GEM_BUG_ON(uc_fw->path); >>> + GEM_BUG_ON(uc_fw->file_selected.path); >>> uc_fw->type = type; >>> @@ -292,7 +400,7 @@ void intel_uc_fw_init_early(struct intel_uc_fw >>> *uc_fw, >>> __uc_fw_user_override(i915, uc_fw); >>> } >>> - intel_uc_fw_change_status(uc_fw, uc_fw->path ? *uc_fw->path ? >>> + intel_uc_fw_change_status(uc_fw, uc_fw->file_selected.path ? >>> *uc_fw->file_selected.path ? >>> INTEL_UC_FIRMWARE_SELECTED : >>> INTEL_UC_FIRMWARE_DISABLED : >>> INTEL_UC_FIRMWARE_NOT_SUPPORTED); >>> @@ -305,32 +413,32 @@ static void __force_fw_fetch_failures(struct >>> intel_uc_fw *uc_fw, int e) >>> if (i915_inject_probe_error(i915, e)) { >>> /* non-existing blob */ >>> - uc_fw->path = "<invalid>"; >>> + uc_fw->file_selected.path = "<invalid>"; >>> uc_fw->user_overridden = user; >>> } else if (i915_inject_probe_error(i915, e)) { >>> /* require next major version */ >>> - uc_fw->major_ver_wanted += 1; >>> - uc_fw->minor_ver_wanted = 0; >>> + uc_fw->file_wanted.major_ver += 1; >>> + uc_fw->file_wanted.minor_ver = 0; >>> uc_fw->user_overridden = user; >>> } else if (i915_inject_probe_error(i915, e)) { >>> /* require next minor version */ >>> - uc_fw->minor_ver_wanted += 1; >>> + uc_fw->file_wanted.minor_ver += 1; >>> uc_fw->user_overridden = user; >>> - } else if (uc_fw->major_ver_wanted && >>> + } else if (uc_fw->file_wanted.major_ver && >>> i915_inject_probe_error(i915, e)) { >>> /* require prev major version */ >>> - uc_fw->major_ver_wanted -= 1; >>> - uc_fw->minor_ver_wanted = 0; >>> + uc_fw->file_wanted.major_ver -= 1; >>> + uc_fw->file_wanted.minor_ver = 0; >>> uc_fw->user_overridden = user; >>> - } else if (uc_fw->minor_ver_wanted && >>> + } else if (uc_fw->file_wanted.minor_ver && >>> i915_inject_probe_error(i915, e)) { >>> /* require prev minor version - hey, this should work! */ >>> - uc_fw->minor_ver_wanted -= 1; >>> + uc_fw->file_wanted.minor_ver -= 1; >>> uc_fw->user_overridden = user; >>> } else if (user && i915_inject_probe_error(i915, e)) { >>> /* officially unsupported platform */ >>> - uc_fw->major_ver_wanted = 0; >>> - uc_fw->minor_ver_wanted = 0; >>> + uc_fw->file_wanted.major_ver = 0; >>> + uc_fw->file_wanted.minor_ver = 0; >>> uc_fw->user_overridden = true; >>> } >>> } >>> @@ -341,8 +449,8 @@ static int check_gsc_manifest(const struct >>> firmware *fw, >>> u32 *dw = (u32 *)fw->data; >>> u32 version = dw[HUC_GSC_VERSION_DW]; >>> - uc_fw->major_ver_found = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, >>> version); >>> - uc_fw->minor_ver_found = FIELD_GET(HUC_GSC_MINOR_VER_MASK, >>> version); >>> + uc_fw->file_selected.major_ver = >>> FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version); >>> + uc_fw->file_selected.minor_ver = >>> FIELD_GET(HUC_GSC_MINOR_VER_MASK, version); >>> return 0; >>> } >>> @@ -357,7 +465,7 @@ static int check_ccs_header(struct >>> drm_i915_private *i915, >>> /* Check the size of the blob before examining buffer contents */ >>> if (unlikely(fw->size < sizeof(struct uc_css_header))) { >>> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < >>> %zu\n", >>> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >>> + intel_uc_fw_type_repr(uc_fw->type), >>> uc_fw->file_selected.path, >>> fw->size, sizeof(struct uc_css_header)); >>> return -ENODATA; >>> } >>> @@ -370,7 +478,7 @@ static int check_ccs_header(struct >>> drm_i915_private *i915, >>> if (unlikely(size != sizeof(struct uc_css_header))) { >>> drm_warn(&i915->drm, >>> "%s firmware %s: unexpected header size: %zu != %zu\n", >>> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >>> + intel_uc_fw_type_repr(uc_fw->type), >>> uc_fw->file_selected.path, >>> fw->size, sizeof(struct uc_css_header)); >>> return -EPROTO; >>> } >>> @@ -385,7 +493,7 @@ static int check_ccs_header(struct >>> drm_i915_private *i915, >>> size = sizeof(struct uc_css_header) + uc_fw->ucode_size + >>> uc_fw->rsa_size; >>> if (unlikely(fw->size < size)) { >>> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < >>> %zu\n", >>> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >>> + intel_uc_fw_type_repr(uc_fw->type), >>> uc_fw->file_selected.path, >>> fw->size, size); >>> return -ENOEXEC; >>> } >>> @@ -394,16 +502,16 @@ static int check_ccs_header(struct >>> drm_i915_private *i915, >>> size = __intel_uc_fw_get_upload_size(uc_fw); >>> if (unlikely(size >= i915->wopcm.size)) { >>> drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu > >>> %zu\n", >>> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >>> + intel_uc_fw_type_repr(uc_fw->type), >>> uc_fw->file_selected.path, >>> size, (size_t)i915->wopcm.size); >>> return -E2BIG; >>> } >>> /* Get version numbers from the CSS header */ >>> - uc_fw->major_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, >>> - css->sw_version); >>> - uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR, >>> - css->sw_version); >>> + uc_fw->file_selected.major_ver = >>> FIELD_GET(CSS_SW_VERSION_UC_MAJOR, >>> + css->sw_version); >>> + uc_fw->file_selected.minor_ver = >>> FIELD_GET(CSS_SW_VERSION_UC_MINOR, >>> + css->sw_version); >>> if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) >>> uc_fw->private_data_size = css->private_data_size; >>> @@ -422,9 +530,11 @@ static int check_ccs_header(struct >>> drm_i915_private *i915, >>> int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >>> { >>> struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915; >>> + struct intel_uc_fw_file file_ideal; >>> struct device *dev = i915->drm.dev; >>> struct drm_i915_gem_object *obj; >>> const struct firmware *fw = NULL; >>> + bool old_ver = false; >>> int err; >>> GEM_BUG_ON(!i915->wopcm.size); >>> @@ -437,27 +547,33 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >>> __force_fw_fetch_failures(uc_fw, -EINVAL); >>> __force_fw_fetch_failures(uc_fw, -ESTALE); >>> - err = firmware_request_nowarn(&fw, uc_fw->path, dev); >>> - if (err && !intel_uc_fw_is_overridden(uc_fw) && >>> uc_fw->fallback.path) { >>> - err = firmware_request_nowarn(&fw, uc_fw->fallback.path, dev); >>> - if (!err) { >>> - drm_notice(&i915->drm, >>> - "%s firmware %s is recommended, but only %s was >>> found\n", >>> - intel_uc_fw_type_repr(uc_fw->type), >>> - uc_fw->wanted_path, >>> - uc_fw->fallback.path); >>> - drm_info(&i915->drm, >>> - "Consider updating your linux-firmware pkg or >>> downloading from %s\n", >>> - INTEL_UC_FIRMWARE_URL); >>> - >>> - uc_fw->path = uc_fw->fallback.path; >>> - uc_fw->major_ver_wanted = uc_fw->fallback.major_ver; >>> - uc_fw->minor_ver_wanted = uc_fw->fallback.minor_ver; >>> + err = firmware_request_nowarn(&fw, uc_fw->file_selected.path, >>> dev); >>> + memcpy(&file_ideal, &uc_fw->file_wanted, sizeof(file_ideal)); >>> + if (!err || intel_uc_fw_is_overridden(uc_fw)) >>> + goto done; >>> + >>> + while (err == -ENOENT) { >>> + __uc_fw_auto_select(i915, uc_fw); >>> + if (!uc_fw->file_selected.path) { >>> + /* >>> + * No more options! But set the path back to something >>> + * valid just in case it gets dereferenced. >>> + */ >>> + uc_fw->file_selected.path = file_ideal.path; >>> + >>> + /* Also, preserve the version that was really wanted */ >>> + memcpy(&uc_fw->file_wanted, &file_ideal, >>> sizeof(uc_fw->file_wanted)); >>> + break; >>> } >>> + >>> + err = firmware_request_nowarn(&fw, >>> uc_fw->file_selected.path, dev); >>> } >>> + >>> if (err) >>> goto fail; >>> + old_ver = true; >>> +done: >>> if (uc_fw->loaded_via_gsc) >>> err = check_gsc_manifest(fw, uc_fw); >>> else >>> @@ -465,18 +581,39 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >>> if (err) >>> goto fail; >>> - if (uc_fw->major_ver_found != uc_fw->major_ver_wanted || >>> - uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) { >>> - drm_notice(&i915->drm, "%s firmware %s: unexpected version: >>> %u.%u != %u.%u\n", >>> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >>> - uc_fw->major_ver_found, uc_fw->minor_ver_found, >>> - uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted); >>> - if (!intel_uc_fw_is_overridden(uc_fw)) { >>> - err = -ENOEXEC; >>> - goto fail; >>> + if (uc_fw->file_wanted.major_ver) { >>> + /* Check the file's major version was as it claimed */ >>> + if (uc_fw->file_selected.major_ver != >>> uc_fw->file_wanted.major_ver) { >>> + drm_notice(&i915->drm, "%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.major_ver, >>> uc_fw->file_selected.minor_ver, >>> + uc_fw->file_wanted.major_ver, >>> uc_fw->file_wanted.minor_ver); >>> + if (!intel_uc_fw_is_overridden(uc_fw)) { >>> + err = -ENOEXEC; >>> + goto fail; >>> + } >>> + } else { >>> + if (uc_fw->file_selected.minor_ver < >>> uc_fw->file_wanted.minor_ver) >>> + old_ver = true; >>> } >>> } >>> + if (old_ver) { >>> + /* Preserve the version that was really wanted */ >>> + memcpy(&uc_fw->file_wanted, &file_ideal, >>> sizeof(uc_fw->file_wanted)); >>> + >>> + drm_notice(&i915->drm, >>> + "%s firmware %s (%d.%d) is recommended, but only %s >>> (%d.%d) was found\n", >>> + intel_uc_fw_type_repr(uc_fw->type), >>> + uc_fw->file_wanted.path, >>> + uc_fw->file_wanted.major_ver, >>> uc_fw->file_wanted.minor_ver, >>> + uc_fw->file_selected.path, >>> + uc_fw->file_selected.major_ver, >>> uc_fw->file_selected.minor_ver); >>> + drm_info(&i915->drm, >>> + "Consider updating your linux-firmware pkg or >>> downloading from %s\n", >>> + INTEL_UC_FIRMWARE_URL); >>> + } >>> + >>> if (HAS_LMEM(i915)) { >>> obj = i915_gem_object_create_lmem_from_data(i915, >>> fw->data, fw->size); >>> if (!IS_ERR(obj)) >>> @@ -503,7 +640,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) >>> INTEL_UC_FIRMWARE_ERROR); >>> i915_probe_error(i915, "%s firmware %s: fetch failed with >>> error %d\n", >>> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err); >>> + intel_uc_fw_type_repr(uc_fw->type), >>> uc_fw->file_selected.path, err); >>> drm_info(&i915->drm, "%s firmware(s) can be downloaded from >>> %s\n", >>> intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL); >>> @@ -645,7 +782,7 @@ int intel_uc_fw_upload(struct intel_uc_fw >>> *uc_fw, u32 dst_offset, u32 dma_flags) >>> fail: >>> i915_probe_error(gt->i915, "Failed to load %s firmware %s >>> (%d)\n", >>> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, >>> + intel_uc_fw_type_repr(uc_fw->type), >>> uc_fw->file_selected.path, >>> err); >>> intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_LOAD_FAIL); >>> return err; >>> @@ -864,18 +1001,19 @@ size_t intel_uc_fw_copy_rsa(struct >>> intel_uc_fw *uc_fw, void *dst, u32 max_len) >>> void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct >>> drm_printer *p) >>> { >>> drm_printf(p, "%s firmware: %s\n", >>> - intel_uc_fw_type_repr(uc_fw->type), uc_fw->wanted_path); >>> - if (uc_fw->fallback.path) { >>> - drm_printf(p, "%s firmware fallback: %s\n", >>> - intel_uc_fw_type_repr(uc_fw->type), >>> uc_fw->fallback.path); >>> - drm_printf(p, "fallback selected: %s\n", >>> - str_yes_no(uc_fw->path == uc_fw->fallback.path)); >>> - } >>> + intel_uc_fw_type_repr(uc_fw->type), >>> uc_fw->file_selected.path); >>> + if (uc_fw->file_selected.path != uc_fw->file_wanted.path) >>> + drm_printf(p, "%s firmware wanted: %s\n", >>> + intel_uc_fw_type_repr(uc_fw->type), >>> uc_fw->file_wanted.path); >>> drm_printf(p, "\tstatus: %s\n", >>> intel_uc_fw_status_repr(uc_fw->status)); >>> - drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n", >>> - uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted, >>> - uc_fw->major_ver_found, uc_fw->minor_ver_found); >>> + if (uc_fw->file_wanted.major_ver) >>> + drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n", >>> + uc_fw->file_wanted.major_ver, >>> uc_fw->file_wanted.minor_ver, >>> + uc_fw->file_selected.major_ver, >>> uc_fw->file_selected.minor_ver); >>> + else >>> + drm_printf(p, "\tversion: found %u.%u\n", >>> + uc_fw->file_selected.major_ver, >>> uc_fw->file_selected.minor_ver); >>> drm_printf(p, "\tuCode: %u bytes\n", uc_fw->ucode_size); >>> drm_printf(p, "\tRSA: %u bytes\n", uc_fw->rsa_size); >>> } >>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >>> index 7aa2644400b98..344763c942e37 100644 >>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h >>> @@ -64,6 +64,17 @@ enum intel_uc_fw_type { >>> }; >>> #define INTEL_UC_FW_NUM_TYPES 2 >>> +/* >>> + * The firmware build process will generate a version header file >>> with major and >>> + * minor version defined. The versions are built into CSS header of >>> firmware. >>> + * i915 kernel driver set the minimal firmware version required per >>> platform. >>> + */ >>> +struct intel_uc_fw_file { >>> + const char *path; >>> + u16 major_ver; >>> + u16 minor_ver; >>> +}; >>> + >>> /* >>> * This structure encapsulates all the data needed during the process >>> * of fetching, caching, and loading the firmware image into the uC. >>> @@ -74,11 +85,12 @@ struct intel_uc_fw { >>> const enum intel_uc_fw_status status; >>> enum intel_uc_fw_status __status; /* no accidental >>> overwrites */ >>> }; >>> - const char *wanted_path; >>> - const char *path; >>> + struct intel_uc_fw_file file_wanted; >>> + struct intel_uc_fw_file file_selected; >>> bool user_overridden; >>> size_t size; >>> struct drm_i915_gem_object *obj; >>> + >>> /** >>> * @dummy: A vma used in binding the uc fw to ggtt. We can't >>> define this >>> * vma on the stack as it can lead to a stack overflow, so we >>> define it >>> @@ -89,25 +101,8 @@ struct intel_uc_fw { >>> struct i915_vma_resource dummy; >>> struct i915_vma *rsa_data; >>> - /* >>> - * The firmware build process will generate a version header >>> file with major and >>> - * minor version defined. The versions are built into CSS >>> header of firmware. >>> - * i915 kernel driver set the minimal firmware version required >>> per platform. >>> - */ >>> - u16 major_ver_wanted; >>> - u16 minor_ver_wanted; >>> - u16 major_ver_found; >>> - u16 minor_ver_found; >>> - >>> - struct { >>> - const char *path; >>> - u16 major_ver; >>> - u16 minor_ver; >>> - } fallback; >>> - >>> u32 rsa_size; >>> u32 ucode_size; >>> - >>> u32 private_data_size; >>> bool loaded_via_gsc; >>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c >>> b/drivers/gpu/drm/i915/i915_gpu_error.c >>> index b5fbc2252784a..9ea2fe34e7d30 100644 >>> --- a/drivers/gpu/drm/i915/i915_gpu_error.c >>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c >>> @@ -1022,8 +1022,10 @@ static void cleanup_params(struct >>> i915_gpu_coredump *error) >>> static void cleanup_uc(struct intel_uc_coredump *uc) >>> { >>> - kfree(uc->guc_fw.path); >>> - kfree(uc->huc_fw.path); >>> + kfree(uc->guc_fw.file_selected.path); >>> + kfree(uc->huc_fw.file_selected.path); >>> + kfree(uc->guc_fw.file_wanted.path); >>> + kfree(uc->huc_fw.file_wanted.path); >>> i915_vma_coredump_free(uc->guc.vma_log); >>> i915_vma_coredump_free(uc->guc.vma_ctb); >>> @@ -1705,12 +1707,10 @@ gt_record_uc(struct intel_gt_coredump *gt, >>> memcpy(&error_uc->guc_fw, &uc->guc.fw, sizeof(uc->guc.fw)); >>> memcpy(&error_uc->huc_fw, &uc->huc.fw, sizeof(uc->huc.fw)); >>> - /* Non-default firmware paths will be specified by the modparam. >>> - * As modparams are generally accesible from the userspace make >>> - * explicit copies of the firmware paths. >>> - */ >>> - error_uc->guc_fw.path = kstrdup(uc->guc.fw.path, ALLOW_FAIL); >>> - error_uc->huc_fw.path = kstrdup(uc->huc.fw.path, ALLOW_FAIL); >>> + error_uc->guc_fw.file_selected.path = >>> kstrdup(uc->guc.fw.file_selected.path, ALLOW_FAIL); >>> + error_uc->huc_fw.file_selected.path = >>> kstrdup(uc->huc.fw.file_selected.path, ALLOW_FAIL); >>> + error_uc->guc_fw.file_wanted.path = >>> kstrdup(uc->guc.fw.file_wanted.path, ALLOW_FAIL); >>> + error_uc->huc_fw.file_wanted.path = >>> kstrdup(uc->huc.fw.file_wanted.path, ALLOW_FAIL); >>> /* >>> * Save the GuC log and include a timestamp reference for >>> converting the >> >
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 0d56b615bf78e..04393932623c7 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1868,7 +1868,7 @@ int intel_guc_submission_init(struct intel_guc *guc) if (guc->submission_initialized) return 0; - if (guc->fw.major_ver_found < 70) { + if (guc->fw.file_selected.major_ver < 70) { ret = guc_lrc_desc_pool_create_v69(guc); if (ret) return ret; @@ -2303,7 +2303,7 @@ static int register_context(struct intel_context *ce, bool loop) GEM_BUG_ON(intel_context_is_child(ce)); trace_intel_context_register(ce); - if (guc->fw.major_ver_found >= 70) + if (guc->fw.file_selected.major_ver >= 70) ret = register_context_v70(guc, ce, loop); else ret = register_context_v69(guc, ce, loop); @@ -2315,7 +2315,7 @@ static int register_context(struct intel_context *ce, bool loop) set_context_registered(ce); spin_unlock_irqrestore(&ce->guc_state.lock, flags); - if (guc->fw.major_ver_found >= 70) + if (guc->fw.file_selected.major_ver >= 70) guc_context_policy_init_v70(ce, loop); } @@ -2921,7 +2921,7 @@ static void __guc_context_set_preemption_timeout(struct intel_guc *guc, u16 guc_id, u32 preemption_timeout) { - if (guc->fw.major_ver_found >= 70) { + if (guc->fw.file_selected.major_ver >= 70) { struct context_policy policy; __guc_context_policy_start_klv(&policy, guc_id); @@ -3186,7 +3186,7 @@ static int guc_context_alloc(struct intel_context *ce) static void __guc_context_set_prio(struct intel_guc *guc, struct intel_context *ce) { - if (guc->fw.major_ver_found >= 70) { + if (guc->fw.file_selected.major_ver >= 70) { struct context_policy policy; __guc_context_policy_start_klv(&policy, ce->guc_id.id); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c index f2e7c82985efd..d965ac4832d60 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c @@ -436,8 +436,8 @@ static void print_fw_ver(struct intel_uc *uc, struct intel_uc_fw *fw) struct drm_i915_private *i915 = uc_to_gt(uc)->i915; drm_info(&i915->drm, "%s firmware %s version %u.%u\n", - intel_uc_fw_type_repr(fw->type), fw->path, - fw->major_ver_found, fw->minor_ver_found); + intel_uc_fw_type_repr(fw->type), fw->file_selected.path, + fw->file_selected.major_ver, fw->file_selected.minor_ver); } static int __uc_init_hw(struct intel_uc *uc) 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 58547292efa0a..94cf2d4a46e6f 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c @@ -41,7 +41,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw, "%s firmware -> %s\n", intel_uc_fw_type_repr(uc_fw->type), status == INTEL_UC_FIRMWARE_SELECTED ? - uc_fw->path : intel_uc_fw_status_repr(status)); + uc_fw->file_selected.path : intel_uc_fw_status_repr(status)); } #endif @@ -51,84 +51,149 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw, * * Note that RKL and ADL-S have the same GuC/HuC device ID's and use the same * firmware as TGL. + * + * Version numbers: + * Originally, the driver required an exact match major/minor/patch furmware + * file and only supported that one version for any given platform. However, + * the new direction from upstream is to be becakwards compatible with all + * prior releases and to be as flexible as possible as to what firmware is + * loaded. + * + * For GuC, the major version number signifies a backwards breaking API change. + * So, new format GuC firmware files are labelled by their major version only. + * For HuC, there is no KMD interaction, hence no version matching requirement. + * So, new format HuC firmware files have no version number at all. + * + * All of which means that the table below must keep all old format files with + * full three point version number. But newer files have reduced requirements. + * Having said that, the driver still needs to track the minor version number + * for GuC at least. As it is useful to report to the user that they are not + * running with a recent enough version for all KMD supported features, + * security fixes, etc. to be enabled. */ -#define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_def) \ - fw_def(DG2, 0, guc_def(dg2, 70, 4, 1)) \ - fw_def(ALDERLAKE_P, 0, guc_def(adlp, 70, 1, 1)) \ - fw_def(ALDERLAKE_S, 0, guc_def(tgl, 70, 1, 1)) \ - fw_def(DG1, 0, guc_def(dg1, 70, 1, 1)) \ - fw_def(ROCKETLAKE, 0, guc_def(tgl, 70, 1, 1)) \ - fw_def(TIGERLAKE, 0, guc_def(tgl, 70, 1, 1)) \ - fw_def(JASPERLAKE, 0, guc_def(ehl, 70, 1, 1)) \ - fw_def(ELKHARTLAKE, 0, guc_def(ehl, 70, 1, 1)) \ - fw_def(ICELAKE, 0, guc_def(icl, 70, 1, 1)) \ - fw_def(COMETLAKE, 5, guc_def(cml, 70, 1, 1)) \ - fw_def(COMETLAKE, 0, guc_def(kbl, 70, 1, 1)) \ - fw_def(COFFEELAKE, 0, guc_def(kbl, 70, 1, 1)) \ - fw_def(GEMINILAKE, 0, guc_def(glk, 70, 1, 1)) \ - fw_def(KABYLAKE, 0, guc_def(kbl, 70, 1, 1)) \ - fw_def(BROXTON, 0, guc_def(bxt, 70, 1, 1)) \ - fw_def(SKYLAKE, 0, guc_def(skl, 70, 1, 1)) - -#define INTEL_GUC_FIRMWARE_DEFS_FALLBACK(fw_def, guc_def) \ - fw_def(ALDERLAKE_P, 0, guc_def(adlp, 69, 0, 3)) \ - fw_def(ALDERLAKE_S, 0, guc_def(tgl, 69, 0, 3)) - -#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_def) \ - fw_def(ALDERLAKE_P, 0, huc_def(tgl, 7, 9, 3)) \ - fw_def(ALDERLAKE_S, 0, huc_def(tgl, 7, 9, 3)) \ - fw_def(DG1, 0, huc_def(dg1, 7, 9, 3)) \ - fw_def(ROCKETLAKE, 0, huc_def(tgl, 7, 9, 3)) \ - fw_def(TIGERLAKE, 0, huc_def(tgl, 7, 9, 3)) \ - fw_def(JASPERLAKE, 0, huc_def(ehl, 9, 0, 0)) \ - fw_def(ELKHARTLAKE, 0, huc_def(ehl, 9, 0, 0)) \ - fw_def(ICELAKE, 0, huc_def(icl, 9, 0, 0)) \ - fw_def(COMETLAKE, 5, huc_def(cml, 4, 0, 0)) \ - fw_def(COMETLAKE, 0, huc_def(kbl, 4, 0, 0)) \ - fw_def(COFFEELAKE, 0, huc_def(kbl, 4, 0, 0)) \ - fw_def(GEMINILAKE, 0, huc_def(glk, 4, 0, 0)) \ - fw_def(KABYLAKE, 0, huc_def(kbl, 4, 0, 0)) \ - fw_def(BROXTON, 0, huc_def(bxt, 2, 0, 0)) \ - fw_def(SKYLAKE, 0, huc_def(skl, 2, 0, 0)) - -#define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \ +#define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \ + fw_def(DG2, 0, guc_mmp(dg2, 70, 4, 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_mmp(tgl, 70, 1, 1)) \ + fw_def(ALDERLAKE_S, 0, guc_mmp(tgl, 69, 0, 3)) \ + fw_def(DG1, 0, guc_mmp(dg1, 70, 1, 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)) \ + fw_def(ELKHARTLAKE, 0, guc_mmp(ehl, 70, 1, 1)) \ + fw_def(ICELAKE, 0, guc_mmp(icl, 70, 1, 1)) \ + fw_def(COMETLAKE, 5, guc_mmp(cml, 70, 1, 1)) \ + fw_def(COMETLAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ + fw_def(COFFEELAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ + fw_def(GEMINILAKE, 0, guc_mmp(glk, 70, 1, 1)) \ + fw_def(KABYLAKE, 0, guc_mmp(kbl, 70, 1, 1)) \ + fw_def(BROXTON, 0, guc_mmp(bxt, 70, 1, 1)) \ + fw_def(SKYLAKE, 0, guc_mmp(skl, 70, 1, 1)) + +#define INTEL_HUC_FIRMWARE_DEFS(fw_def, huc_raw, huc_mmp) \ + fw_def(ALDERLAKE_P, 0, huc_mmp(tgl, 7, 9, 3)) \ + fw_def(ALDERLAKE_S, 0, huc_mmp(tgl, 7, 9, 3)) \ + fw_def(DG1, 0, huc_mmp(dg1, 7, 9, 3)) \ + fw_def(ROCKETLAKE, 0, huc_mmp(tgl, 7, 9, 3)) \ + fw_def(TIGERLAKE, 0, huc_mmp(tgl, 7, 9, 3)) \ + fw_def(JASPERLAKE, 0, huc_mmp(ehl, 9, 0, 0)) \ + fw_def(ELKHARTLAKE, 0, huc_mmp(ehl, 9, 0, 0)) \ + fw_def(ICELAKE, 0, huc_mmp(icl, 9, 0, 0)) \ + fw_def(COMETLAKE, 5, huc_mmp(cml, 4, 0, 0)) \ + fw_def(COMETLAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ + fw_def(COFFEELAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ + fw_def(GEMINILAKE, 0, huc_mmp(glk, 4, 0, 0)) \ + fw_def(KABYLAKE, 0, huc_mmp(kbl, 4, 0, 0)) \ + fw_def(BROXTON, 0, huc_mmp(bxt, 2, 0, 0)) \ + fw_def(SKYLAKE, 0, huc_mmp(skl, 2, 0, 0)) + +/* + * Set of macros for producing a list of filenames from the above table. + */ +#define __MAKE_UC_FW_PATH_BLANK(prefix_, name_) \ + "i915/" \ + __stringify(prefix_) name_ ".bin" + +#define __MAKE_UC_FW_PATH_MAJOR(prefix_, name_, major_) \ + "i915/" \ + __stringify(prefix_) name_ \ + __stringify(major_) ".bin" + +#define __MAKE_UC_FW_PATH_MMP(prefix_, name_, major_, minor_, patch_) \ "i915/" \ __stringify(prefix_) name_ \ __stringify(major_) "." \ __stringify(minor_) "." \ __stringify(patch_) ".bin" -#define MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_) \ - __MAKE_UC_FW_PATH(prefix_, "_guc_", major_, minor_, patch_) +/* Minor for internal driver use, not part of file name */ +#define MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_) \ + __MAKE_UC_FW_PATH_MAJOR(prefix_, "_guc_", major_) -#define MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_) \ - __MAKE_UC_FW_PATH(prefix_, "_huc_", major_, minor_, bld_num_) +#define MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \ + __MAKE_UC_FW_PATH_MMP(prefix_, "_guc_", major_, minor_, patch_) -/* All blobs need to be declared via MODULE_FIRMWARE() */ +#define MAKE_HUC_FW_PATH_BLANK(prefix_) \ + __MAKE_UC_FW_PATH_BLANK(prefix_, "_huc") + +#define MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_) \ + __MAKE_UC_FW_PATH_MMP(prefix_, "_huc_", major_, minor_, patch_) + +/* + * All blobs need to be declared via MODULE_FIRMWARE(). + * This first expansion of the table macros is solely to provide + * that declaration. + */ #define INTEL_UC_MODULE_FW(platform_, revid_, uc_) \ MODULE_FIRMWARE(uc_); -INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH) -INTEL_GUC_FIRMWARE_DEFS_FALLBACK(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH) -INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH) +INTEL_GUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_GUC_FW_PATH_MAJOR, MAKE_GUC_FW_PATH_MMP) +INTEL_HUC_FIRMWARE_DEFS(INTEL_UC_MODULE_FW, MAKE_HUC_FW_PATH_BLANK, MAKE_HUC_FW_PATH_MMP) -/* The below structs and macros are used to iterate across the list of blobs */ +/* + * The next expansion of the table macros (in __uc_fw_auto_select below) provides + * actual data structures with both the filename and the version information. + * These structure arrays are then iterated over to the list of suitable files + * for the current platform and to then attempt to load those files, in the order + * listed, until one is successfully found. + */ struct __packed uc_fw_blob { + const char *path; + bool legacy; u8 major; u8 minor; - const char *path; + u8 patch; }; -#define UC_FW_BLOB(major_, minor_, path_) \ - { .major = major_, .minor = minor_, .path = path_ } +#define UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ + .major = major_, \ + .minor = minor_, \ + .patch = patch_, \ + .path = path_, + +#define UC_FW_BLOB_NEW(major_, minor_, patch_, path_) \ + { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ + .legacy = false } + +#define UC_FW_BLOB_OLD(major_, minor_, patch_, path_) \ + { UC_FW_BLOB_BASE(major_, minor_, patch_, path_) \ + .legacy = true } -#define GUC_FW_BLOB(prefix_, major_, minor_, patch_) \ - UC_FW_BLOB(major_, minor_, \ - MAKE_GUC_FW_PATH(prefix_, major_, minor_, patch_)) +#define GUC_FW_BLOB(prefix_, major_, minor_) \ + UC_FW_BLOB_NEW(major_, minor_, 0, \ + MAKE_GUC_FW_PATH_MAJOR(prefix_, major_, minor_)) -#define HUC_FW_BLOB(prefix_, major_, minor_, bld_num_) \ - UC_FW_BLOB(major_, minor_, \ - MAKE_HUC_FW_PATH(prefix_, major_, minor_, bld_num_)) +#define GUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \ + UC_FW_BLOB_OLD(major_, minor_, patch_, \ + MAKE_GUC_FW_PATH_MMP(prefix_, major_, minor_, patch_)) + +#define HUC_FW_BLOB(prefix_) \ + UC_FW_BLOB_NEW(0, 0, 0, MAKE_HUC_FW_PATH_BLANK(prefix_)) + +#define HUC_FW_BLOB_MMP(prefix_, major_, minor_, patch_) \ + UC_FW_BLOB_OLD(major_, minor_, patch_, \ + MAKE_HUC_FW_PATH_MMP(prefix_, major_, minor_, patch_)) struct __packed uc_fw_platform_requirement { enum intel_platform p; @@ -152,18 +217,16 @@ 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) - }; - static const struct uc_fw_platform_requirement blobs_guc_fallback[] = { - INTEL_GUC_FIRMWARE_DEFS_FALLBACK(MAKE_FW_LIST, GUC_FW_BLOB) + 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) + INTEL_HUC_FIRMWARE_DEFS(MAKE_FW_LIST, HUC_FW_BLOB, HUC_FW_BLOB_MMP) }; 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; const struct uc_fw_platform_requirement *fw_blobs; enum intel_platform p = INTEL_INFO(i915)->platform; u32 fw_count; @@ -184,49 +247,94 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw) fw_count = blobs_all[uc_fw->type].count; for (i = 0; i < fw_count && p <= fw_blobs[i].p; i++) { - if (p == fw_blobs[i].p && rev >= fw_blobs[i].rev) { - const struct uc_fw_blob *blob = &fw_blobs[i].blob; - uc_fw->path = blob->path; - uc_fw->wanted_path = blob->path; - uc_fw->major_ver_wanted = blob->major; - uc_fw->minor_ver_wanted = blob->minor; - break; - } - } + const struct uc_fw_blob *blob = &fw_blobs[i].blob; - if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) { - const struct uc_fw_platform_requirement *blobs = blobs_guc_fallback; - u32 count = ARRAY_SIZE(blobs_guc_fallback); + if (p != fw_blobs[i].p) + continue; - for (i = 0; i < count && p <= blobs[i].p; i++) { - if (p == blobs[i].p && rev >= blobs[i].rev) { - const struct uc_fw_blob *blob = &blobs[i].blob; + if (rev < fw_blobs[i].rev) + continue; - uc_fw->fallback.path = blob->path; - uc_fw->fallback.major_ver = blob->major; - uc_fw->fallback.minor_ver = blob->minor; - break; - } + if (uc_fw->file_selected.path) { + if (uc_fw->file_selected.path == blob->path) + uc_fw->file_selected.path = NULL; + + continue; } + + uc_fw->file_selected.path = blob->path; + uc_fw->file_wanted.path = blob->path; + uc_fw->file_wanted.major_ver = blob->major; + uc_fw->file_wanted.minor_ver = blob->minor; + break; } /* make sure the list is ordered as expected */ - if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST)) { + if (IS_ENABLED(CONFIG_DRM_I915_SELFTEST) && !verified) { + verified = true; + for (i = 1; i < fw_count; i++) { + /* Next platform is good: */ if (fw_blobs[i].p < fw_blobs[i - 1].p) continue; + /* 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; - drm_err(&i915->drm, "Invalid FW blob order: %s r%u comes before %s r%u\n", - intel_platform_name(fw_blobs[i - 1].p), - fw_blobs[i - 1].rev, - intel_platform_name(fw_blobs[i].p), - fw_blobs[i].rev); + /* 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; + + /* Next major version is good: */ + if (fw_blobs[i].blob.major < fw_blobs[i - 1].blob.major) + continue; + + /* New must be before legacy: */ + if (!fw_blobs[i].blob.legacy && fw_blobs[i - 1].blob.legacy) + goto bad; + + /* 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; + + 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; - uc_fw->path = NULL; + /* 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, "\x1B[35;1mInvalid FW blob order: %s r%u %s%d.%d.%d comes before %s r%u %s%d.%d.%d\n", + 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; } } } @@ -259,7 +367,7 @@ static void __uc_fw_user_override(struct drm_i915_private *i915, struct intel_uc } if (unlikely(path)) { - uc_fw->path = path; + uc_fw->file_selected.path = path; uc_fw->user_overridden = true; } } @@ -283,7 +391,7 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, */ BUILD_BUG_ON(INTEL_UC_FIRMWARE_UNINITIALIZED); GEM_BUG_ON(uc_fw->status); - GEM_BUG_ON(uc_fw->path); + GEM_BUG_ON(uc_fw->file_selected.path); uc_fw->type = type; @@ -292,7 +400,7 @@ void intel_uc_fw_init_early(struct intel_uc_fw *uc_fw, __uc_fw_user_override(i915, uc_fw); } - intel_uc_fw_change_status(uc_fw, uc_fw->path ? *uc_fw->path ? + intel_uc_fw_change_status(uc_fw, uc_fw->file_selected.path ? *uc_fw->file_selected.path ? INTEL_UC_FIRMWARE_SELECTED : INTEL_UC_FIRMWARE_DISABLED : INTEL_UC_FIRMWARE_NOT_SUPPORTED); @@ -305,32 +413,32 @@ static void __force_fw_fetch_failures(struct intel_uc_fw *uc_fw, int e) if (i915_inject_probe_error(i915, e)) { /* non-existing blob */ - uc_fw->path = "<invalid>"; + uc_fw->file_selected.path = "<invalid>"; uc_fw->user_overridden = user; } else if (i915_inject_probe_error(i915, e)) { /* require next major version */ - uc_fw->major_ver_wanted += 1; - uc_fw->minor_ver_wanted = 0; + uc_fw->file_wanted.major_ver += 1; + uc_fw->file_wanted.minor_ver = 0; uc_fw->user_overridden = user; } else if (i915_inject_probe_error(i915, e)) { /* require next minor version */ - uc_fw->minor_ver_wanted += 1; + uc_fw->file_wanted.minor_ver += 1; uc_fw->user_overridden = user; - } else if (uc_fw->major_ver_wanted && + } else if (uc_fw->file_wanted.major_ver && i915_inject_probe_error(i915, e)) { /* require prev major version */ - uc_fw->major_ver_wanted -= 1; - uc_fw->minor_ver_wanted = 0; + uc_fw->file_wanted.major_ver -= 1; + uc_fw->file_wanted.minor_ver = 0; uc_fw->user_overridden = user; - } else if (uc_fw->minor_ver_wanted && + } else if (uc_fw->file_wanted.minor_ver && i915_inject_probe_error(i915, e)) { /* require prev minor version - hey, this should work! */ - uc_fw->minor_ver_wanted -= 1; + uc_fw->file_wanted.minor_ver -= 1; uc_fw->user_overridden = user; } else if (user && i915_inject_probe_error(i915, e)) { /* officially unsupported platform */ - uc_fw->major_ver_wanted = 0; - uc_fw->minor_ver_wanted = 0; + uc_fw->file_wanted.major_ver = 0; + uc_fw->file_wanted.minor_ver = 0; uc_fw->user_overridden = true; } } @@ -341,8 +449,8 @@ static int check_gsc_manifest(const struct firmware *fw, u32 *dw = (u32 *)fw->data; u32 version = dw[HUC_GSC_VERSION_DW]; - uc_fw->major_ver_found = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version); - uc_fw->minor_ver_found = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version); + uc_fw->file_selected.major_ver = FIELD_GET(HUC_GSC_MAJOR_VER_MASK, version); + uc_fw->file_selected.minor_ver = FIELD_GET(HUC_GSC_MINOR_VER_MASK, version); return 0; } @@ -357,7 +465,7 @@ static int check_ccs_header(struct drm_i915_private *i915, /* Check the size of the blob before examining buffer contents */ if (unlikely(fw->size < sizeof(struct uc_css_header))) { drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, fw->size, sizeof(struct uc_css_header)); return -ENODATA; } @@ -370,7 +478,7 @@ static int check_ccs_header(struct drm_i915_private *i915, if (unlikely(size != sizeof(struct uc_css_header))) { drm_warn(&i915->drm, "%s firmware %s: unexpected header size: %zu != %zu\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, fw->size, sizeof(struct uc_css_header)); return -EPROTO; } @@ -385,7 +493,7 @@ static int check_ccs_header(struct drm_i915_private *i915, size = sizeof(struct uc_css_header) + uc_fw->ucode_size + uc_fw->rsa_size; if (unlikely(fw->size < size)) { drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu < %zu\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, fw->size, size); return -ENOEXEC; } @@ -394,16 +502,16 @@ static int check_ccs_header(struct drm_i915_private *i915, size = __intel_uc_fw_get_upload_size(uc_fw); if (unlikely(size >= i915->wopcm.size)) { drm_warn(&i915->drm, "%s firmware %s: invalid size: %zu > %zu\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, size, (size_t)i915->wopcm.size); return -E2BIG; } /* Get version numbers from the CSS header */ - uc_fw->major_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, - css->sw_version); - uc_fw->minor_ver_found = FIELD_GET(CSS_SW_VERSION_UC_MINOR, - css->sw_version); + uc_fw->file_selected.major_ver = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, + css->sw_version); + uc_fw->file_selected.minor_ver = FIELD_GET(CSS_SW_VERSION_UC_MINOR, + css->sw_version); if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) uc_fw->private_data_size = css->private_data_size; @@ -422,9 +530,11 @@ static int check_ccs_header(struct drm_i915_private *i915, int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) { struct drm_i915_private *i915 = __uc_fw_to_gt(uc_fw)->i915; + struct intel_uc_fw_file file_ideal; struct device *dev = i915->drm.dev; struct drm_i915_gem_object *obj; const struct firmware *fw = NULL; + bool old_ver = false; int err; GEM_BUG_ON(!i915->wopcm.size); @@ -437,27 +547,33 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) __force_fw_fetch_failures(uc_fw, -EINVAL); __force_fw_fetch_failures(uc_fw, -ESTALE); - err = firmware_request_nowarn(&fw, uc_fw->path, dev); - if (err && !intel_uc_fw_is_overridden(uc_fw) && uc_fw->fallback.path) { - err = firmware_request_nowarn(&fw, uc_fw->fallback.path, dev); - if (!err) { - drm_notice(&i915->drm, - "%s firmware %s is recommended, but only %s was found\n", - intel_uc_fw_type_repr(uc_fw->type), - uc_fw->wanted_path, - uc_fw->fallback.path); - drm_info(&i915->drm, - "Consider updating your linux-firmware pkg or downloading from %s\n", - INTEL_UC_FIRMWARE_URL); - - uc_fw->path = uc_fw->fallback.path; - uc_fw->major_ver_wanted = uc_fw->fallback.major_ver; - uc_fw->minor_ver_wanted = uc_fw->fallback.minor_ver; + err = firmware_request_nowarn(&fw, uc_fw->file_selected.path, dev); + memcpy(&file_ideal, &uc_fw->file_wanted, sizeof(file_ideal)); + if (!err || intel_uc_fw_is_overridden(uc_fw)) + goto done; + + while (err == -ENOENT) { + __uc_fw_auto_select(i915, uc_fw); + if (!uc_fw->file_selected.path) { + /* + * No more options! But set the path back to something + * valid just in case it gets dereferenced. + */ + uc_fw->file_selected.path = file_ideal.path; + + /* Also, preserve the version that was really wanted */ + memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted)); + break; } + + err = firmware_request_nowarn(&fw, uc_fw->file_selected.path, dev); } + if (err) goto fail; + old_ver = true; +done: if (uc_fw->loaded_via_gsc) err = check_gsc_manifest(fw, uc_fw); else @@ -465,18 +581,39 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) if (err) goto fail; - if (uc_fw->major_ver_found != uc_fw->major_ver_wanted || - uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) { - drm_notice(&i915->drm, "%s firmware %s: unexpected version: %u.%u != %u.%u\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, - uc_fw->major_ver_found, uc_fw->minor_ver_found, - uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted); - if (!intel_uc_fw_is_overridden(uc_fw)) { - err = -ENOEXEC; - goto fail; + if (uc_fw->file_wanted.major_ver) { + /* Check the file's major version was as it claimed */ + if (uc_fw->file_selected.major_ver != uc_fw->file_wanted.major_ver) { + drm_notice(&i915->drm, "%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.major_ver, uc_fw->file_selected.minor_ver, + uc_fw->file_wanted.major_ver, uc_fw->file_wanted.minor_ver); + if (!intel_uc_fw_is_overridden(uc_fw)) { + err = -ENOEXEC; + goto fail; + } + } else { + if (uc_fw->file_selected.minor_ver < uc_fw->file_wanted.minor_ver) + old_ver = true; } } + if (old_ver) { + /* Preserve the version that was really wanted */ + memcpy(&uc_fw->file_wanted, &file_ideal, sizeof(uc_fw->file_wanted)); + + drm_notice(&i915->drm, + "%s firmware %s (%d.%d) is recommended, but only %s (%d.%d) was found\n", + intel_uc_fw_type_repr(uc_fw->type), + uc_fw->file_wanted.path, + uc_fw->file_wanted.major_ver, uc_fw->file_wanted.minor_ver, + uc_fw->file_selected.path, + uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver); + drm_info(&i915->drm, + "Consider updating your linux-firmware pkg or downloading from %s\n", + INTEL_UC_FIRMWARE_URL); + } + if (HAS_LMEM(i915)) { obj = i915_gem_object_create_lmem_from_data(i915, fw->data, fw->size); if (!IS_ERR(obj)) @@ -503,7 +640,7 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) INTEL_UC_FIRMWARE_ERROR); i915_probe_error(i915, "%s firmware %s: fetch failed with error %d\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, err); + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, err); drm_info(&i915->drm, "%s firmware(s) can be downloaded from %s\n", intel_uc_fw_type_repr(uc_fw->type), INTEL_UC_FIRMWARE_URL); @@ -645,7 +782,7 @@ int intel_uc_fw_upload(struct intel_uc_fw *uc_fw, u32 dst_offset, u32 dma_flags) fail: i915_probe_error(gt->i915, "Failed to load %s firmware %s (%d)\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->path, + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path, err); intel_uc_fw_change_status(uc_fw, INTEL_UC_FIRMWARE_LOAD_FAIL); return err; @@ -864,18 +1001,19 @@ size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len) void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p) { drm_printf(p, "%s firmware: %s\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->wanted_path); - if (uc_fw->fallback.path) { - drm_printf(p, "%s firmware fallback: %s\n", - intel_uc_fw_type_repr(uc_fw->type), uc_fw->fallback.path); - drm_printf(p, "fallback selected: %s\n", - str_yes_no(uc_fw->path == uc_fw->fallback.path)); - } + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path); + if (uc_fw->file_selected.path != uc_fw->file_wanted.path) + drm_printf(p, "%s firmware wanted: %s\n", + intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_wanted.path); drm_printf(p, "\tstatus: %s\n", intel_uc_fw_status_repr(uc_fw->status)); - drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n", - uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted, - uc_fw->major_ver_found, uc_fw->minor_ver_found); + if (uc_fw->file_wanted.major_ver) + drm_printf(p, "\tversion: wanted %u.%u, found %u.%u\n", + uc_fw->file_wanted.major_ver, uc_fw->file_wanted.minor_ver, + uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver); + else + drm_printf(p, "\tversion: found %u.%u\n", + uc_fw->file_selected.major_ver, uc_fw->file_selected.minor_ver); drm_printf(p, "\tuCode: %u bytes\n", uc_fw->ucode_size); drm_printf(p, "\tRSA: %u bytes\n", uc_fw->rsa_size); } diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h index 7aa2644400b98..344763c942e37 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h @@ -64,6 +64,17 @@ enum intel_uc_fw_type { }; #define INTEL_UC_FW_NUM_TYPES 2 +/* + * The firmware build process will generate a version header file with major and + * minor version defined. The versions are built into CSS header of firmware. + * i915 kernel driver set the minimal firmware version required per platform. + */ +struct intel_uc_fw_file { + const char *path; + u16 major_ver; + u16 minor_ver; +}; + /* * This structure encapsulates all the data needed during the process * of fetching, caching, and loading the firmware image into the uC. @@ -74,11 +85,12 @@ struct intel_uc_fw { const enum intel_uc_fw_status status; enum intel_uc_fw_status __status; /* no accidental overwrites */ }; - const char *wanted_path; - const char *path; + struct intel_uc_fw_file file_wanted; + struct intel_uc_fw_file file_selected; bool user_overridden; size_t size; struct drm_i915_gem_object *obj; + /** * @dummy: A vma used in binding the uc fw to ggtt. We can't define this * vma on the stack as it can lead to a stack overflow, so we define it @@ -89,25 +101,8 @@ struct intel_uc_fw { struct i915_vma_resource dummy; struct i915_vma *rsa_data; - /* - * The firmware build process will generate a version header file with major and - * minor version defined. The versions are built into CSS header of firmware. - * i915 kernel driver set the minimal firmware version required per platform. - */ - u16 major_ver_wanted; - u16 minor_ver_wanted; - u16 major_ver_found; - u16 minor_ver_found; - - struct { - const char *path; - u16 major_ver; - u16 minor_ver; - } fallback; - u32 rsa_size; u32 ucode_size; - u32 private_data_size; bool loaded_via_gsc; diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index b5fbc2252784a..9ea2fe34e7d30 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1022,8 +1022,10 @@ static void cleanup_params(struct i915_gpu_coredump *error) static void cleanup_uc(struct intel_uc_coredump *uc) { - kfree(uc->guc_fw.path); - kfree(uc->huc_fw.path); + kfree(uc->guc_fw.file_selected.path); + kfree(uc->huc_fw.file_selected.path); + kfree(uc->guc_fw.file_wanted.path); + kfree(uc->huc_fw.file_wanted.path); i915_vma_coredump_free(uc->guc.vma_log); i915_vma_coredump_free(uc->guc.vma_ctb); @@ -1705,12 +1707,10 @@ gt_record_uc(struct intel_gt_coredump *gt, memcpy(&error_uc->guc_fw, &uc->guc.fw, sizeof(uc->guc.fw)); memcpy(&error_uc->huc_fw, &uc->huc.fw, sizeof(uc->huc.fw)); - /* Non-default firmware paths will be specified by the modparam. - * As modparams are generally accesible from the userspace make - * explicit copies of the firmware paths. - */ - error_uc->guc_fw.path = kstrdup(uc->guc.fw.path, ALLOW_FAIL); - error_uc->huc_fw.path = kstrdup(uc->huc.fw.path, ALLOW_FAIL); + error_uc->guc_fw.file_selected.path = kstrdup(uc->guc.fw.file_selected.path, ALLOW_FAIL); + error_uc->huc_fw.file_selected.path = kstrdup(uc->huc.fw.file_selected.path, ALLOW_FAIL); + error_uc->guc_fw.file_wanted.path = kstrdup(uc->guc.fw.file_wanted.path, ALLOW_FAIL); + error_uc->huc_fw.file_wanted.path = kstrdup(uc->huc.fw.file_wanted.path, ALLOW_FAIL); /* * Save the GuC log and include a timestamp reference for converting the