| Message ID | 20221123223108.1696415-4-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | More GuC firmware version improvements | expand |
On 11/23/2022 2:31 PM, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > The GuC firmware includes an extra version number to specify the > submission API level. So use that rather than the main firmware > version number for submission related checks. > > Also, while it is guaranteed that GuC version number components are > only 8-bits in size, other firmwares do not have that restriction. So > stop making assumptions about them generically fitting in a u16 > individually, or in a u32 as a combined 8.8.8. > > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/gt/uc/intel_guc.h | 11 ++ > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 ++- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 124 ++++++++++++++++-- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 10 +- > drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 3 +- > 5 files changed, 137 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > index 1bb3f98292866..bb4dfe707a7d0 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > @@ -158,6 +158,9 @@ struct intel_guc { > bool submission_selected; > /** @submission_initialized: tracks whether GuC submission has been initialised */ > bool submission_initialized; > + /** @submission_version: Submission API version of the currently loaded firmware */ > + struct intel_uc_fw_ver submission_version; > + > /** > * @rc_supported: tracks whether we support GuC rc on the current platform > */ > @@ -268,6 +271,14 @@ struct intel_guc { > #endif > }; > > +/* > + * GuC version number components are only 8-bit, so converting to a 32bit 8.8.8 > + * integer works. > + */ > +#define MAKE_GUC_VER(maj, min, pat) (((maj) << 16) | ((min) << 8) | (pat)) > +#define MAKE_GUC_VER_STRUCT(ver) MAKE_GUC_VER((ver).major, (ver).minor, (ver).patch) > +#define GUC_SUBMIT_VER(guc) MAKE_GUC_VER_STRUCT((guc)->submission_version) > + > static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) > { > return container_of(log, struct intel_guc, log); > 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 0a42f1807f52c..53f7f599cde3a 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -1890,7 +1890,7 @@ int intel_guc_submission_init(struct intel_guc *guc) > if (guc->submission_initialized) > return 0; > > - if (GET_UC_VER(guc) < MAKE_UC_VER(70, 0, 0)) { > + if (GUC_SUBMIT_VER(guc) < MAKE_GUC_VER(1, 0, 0)) { > ret = guc_lrc_desc_pool_create_v69(guc); > if (ret) > return ret; > @@ -2330,7 +2330,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 (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) > + if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) > ret = register_context_v70(guc, ce, loop); > else > ret = register_context_v69(guc, ce, loop); > @@ -2342,7 +2342,7 @@ static int register_context(struct intel_context *ce, bool loop) > set_context_registered(ce); > spin_unlock_irqrestore(&ce->guc_state.lock, flags); > > - if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) > + if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) > guc_context_policy_init_v70(ce, loop); > } > > @@ -2956,7 +2956,7 @@ static void __guc_context_set_preemption_timeout(struct intel_guc *guc, > u16 guc_id, > u32 preemption_timeout) > { > - if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) { > + if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) { > struct context_policy policy; > > __guc_context_policy_start_klv(&policy, guc_id); > @@ -3283,7 +3283,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 (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) { > + if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) { > struct context_policy policy; > > __guc_context_policy_start_klv(&policy, ce->guc_id.id); > @@ -4366,7 +4366,7 @@ static int guc_init_global_schedule_policy(struct intel_guc *guc) > intel_wakeref_t wakeref; > int ret = 0; > > - if (GET_UC_VER(guc) < MAKE_UC_VER(70, 3, 0)) > + if (GUC_SUBMIT_VER(guc) < MAKE_GUC_VER(1, 1, 0)) > return 0; > > __guc_scheduling_policy_start_klv(&policy); > @@ -4905,6 +4905,9 @@ void intel_guc_submission_print_info(struct intel_guc *guc, > if (!sched_engine) > return; > > + drm_printf(p, "GuC Submission API Version: %d.%d.%d\n", > + guc->submission_version.major, guc->submission_version.minor, > + guc->submission_version.patch); > drm_printf(p, "GuC Number Outstanding Submission G2H: %u\n", > atomic_read(&guc->outstanding_submission_g2h)); > drm_printf(p, "GuC tasklet count: %u\n", > 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 5e2ee1ac89514..7d2349647b593 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c > @@ -478,6 +478,62 @@ static int check_gsc_manifest(const struct firmware *fw, > return 0; > } > > +static void uc_unpack_css_version(struct intel_uc_fw_ver *ver, u32 css_value) > +{ > + /* Get version numbers from the CSS header */ > + ver->major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, css_value); > + ver->minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR, css_value); > + ver->patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, css_value); > +} > + > +static void guc_read_css_info(struct intel_uc_fw *uc_fw, struct uc_css_header *css) > +{ > + struct intel_guc *guc = container_of(uc_fw, struct intel_guc, fw); > + > + /* > + * The GuC firmware includes an extra version number to specify the > + * submission API level. This allows submission code to work with > + * multiple GuC versions without having to know the absolute firmware > + * version number (there are likely to be multiple firmware releases > + * which all support the same submission API level). > + * > + * Note that the spec for the CSS header defines this version number > + * as 'vf_version' as it was originally intended for virtualisation. > + * However, it is applicable to native submission as well. > + * > + * Unfortunately, due to an oversight, this version number was only > + * exposed in the CSS header from v70.6.0. > + */ > + if (uc_fw->file_selected.ver.major >= 70) { > + if (uc_fw->file_selected.ver.minor >= 6) { > + /* v70.6.0 adds CSS header support */ > + uc_unpack_css_version(&guc->submission_version, css->vf_version); > + } else if (uc_fw->file_selected.ver.minor >= 3) { > + /* v70.3.0 introduced v1.1.0 */ > + guc->submission_version.major = 1; > + guc->submission_version.minor = 1; > + guc->submission_version.patch = 0; > + } else { > + /* v70.0.0 introduced v1.0.0 */ > + guc->submission_version.major = 1; > + guc->submission_version.minor = 0; > + guc->submission_version.patch = 0; > + } > + } else if (uc_fw->file_selected.ver.major >= 69) { > + /* v69.0.0 introduced v0.10.0 */ > + guc->submission_version.major = 0; > + guc->submission_version.minor = 10; > + guc->submission_version.patch = 0; > + } else { > + /* Prior versions were v0.1.0 */ > + guc->submission_version.major = 0; > + guc->submission_version.minor = 1; > + guc->submission_version.patch = 0; > + } > + > + uc_fw->private_data_size = css->private_data_size; > +} > + > static int check_ccs_header(struct intel_gt *gt, > const struct firmware *fw, > struct intel_uc_fw *uc_fw) > @@ -531,20 +587,50 @@ static int check_ccs_header(struct intel_gt *gt, > return -E2BIG; > } > > - /* Get version numbers from the CSS header */ > - uc_fw->file_selected.ver.major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, > - css->sw_version); > - uc_fw->file_selected.ver.minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR, > - css->sw_version); > - uc_fw->file_selected.ver.patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, > - css->sw_version); > + uc_unpack_css_version(&uc_fw->file_selected.ver, css->sw_version); > > if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) > - uc_fw->private_data_size = css->private_data_size; > + guc_read_css_info(uc_fw, css); > > return 0; > } > > +static bool is_ver_8bit(struct intel_uc_fw_ver *ver) > +{ > + return ver->major < 0xFF && ver->minor < 0xFF && ver->patch < 0xFF; > +} > + > +static bool gyc_check_version_range(struct intel_uc_fw *uc_fw) typo gyc. With this fixed: Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Daniele > +{ > + struct intel_guc *guc = container_of(uc_fw, struct intel_guc, fw); > + > + /* > + * GuC version number components are defined as being 8-bits. > + * The submission code relies on this to optimise version comparison > + * tests. So enforce the restriction here. > + */ > + > + if (!is_ver_8bit(&uc_fw->file_selected.ver)) { > + drm_warn(&__uc_fw_to_gt(uc_fw)->i915->drm, "%s firmware: invalid file version: 0x%02X:%02X:%02X\n", > + intel_uc_fw_type_repr(uc_fw->type), > + uc_fw->file_selected.ver.major, > + uc_fw->file_selected.ver.minor, > + uc_fw->file_selected.ver.patch); > + return false; > + } > + > + if (!is_ver_8bit(&guc->submission_version)) { > + drm_warn(&__uc_fw_to_gt(uc_fw)->i915->drm, "%s firmware: invalid submit version: 0x%02X:%02X:%02X\n", > + intel_uc_fw_type_repr(uc_fw->type), > + guc->submission_version.major, > + guc->submission_version.minor, > + guc->submission_version.patch); > + return false; > + } > + > + return true; > +} > + > /** > * intel_uc_fw_fetch - fetch uC firmware > * @uc_fw: uC firmware > @@ -621,6 +707,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) > if (err) > goto fail; > > + if (uc_fw->type == INTEL_UC_FW_TYPE_GUC && !gyc_check_version_range(uc_fw)) > + goto fail; > + > if (uc_fw->file_wanted.ver.major) { > /* Check the file's major version was as it claimed */ > if (uc_fw->file_selected.ver.major != uc_fw->file_wanted.ver.major) { > @@ -1054,7 +1143,7 @@ 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) > { > - u32 ver_sel, ver_want; > + bool got_wanted; > > drm_printf(p, "%s firmware: %s\n", > intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path); > @@ -1063,9 +1152,20 @@ void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p) > 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)); > - ver_sel = MAKE_UC_VER_STRUCT(uc_fw->file_selected.ver); > - ver_want = MAKE_UC_VER_STRUCT(uc_fw->file_wanted.ver); > - if (ver_sel < ver_want) > + > + if (uc_fw->file_selected.ver.major < uc_fw->file_wanted.ver.major) > + got_wanted = false; > + else if ((uc_fw->file_selected.ver.major == uc_fw->file_wanted.ver.major) && > + (uc_fw->file_selected.ver.minor < uc_fw->file_wanted.ver.minor)) > + got_wanted = false; > + else if ((uc_fw->file_selected.ver.major == uc_fw->file_wanted.ver.major) && > + (uc_fw->file_selected.ver.minor == uc_fw->file_wanted.ver.minor) && > + (uc_fw->file_selected.ver.patch < uc_fw->file_wanted.ver.patch)) > + got_wanted = false; > + else > + got_wanted = true; > + > + if (!got_wanted) > drm_printf(p, "\tversion: wanted %u.%u.%u, found %u.%u.%u\n", > uc_fw->file_wanted.ver.major, > uc_fw->file_wanted.ver.minor, > 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 6501d6f1fbdff..3ab87c54a3987 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h > @@ -66,9 +66,9 @@ enum intel_uc_fw_type { > #define INTEL_UC_FW_NUM_TYPES 2 > > struct intel_uc_fw_ver { > - u16 major; > - u16 minor; > - u16 patch; > + u32 major; > + u32 minor; > + u32 patch; > }; > > /* > @@ -114,10 +114,6 @@ struct intel_uc_fw { > bool loaded_via_gsc; > }; > > -#define MAKE_UC_VER(maj, min, pat) ((pat) | ((min) << 8) | ((maj) << 16)) > -#define MAKE_UC_VER_STRUCT(ver) MAKE_UC_VER((ver).major, (ver).minor, (ver).patch) > -#define GET_UC_VER(uc) (MAKE_UC_VER_STRUCT((uc)->fw.file_selected.ver)) > - > /* > * When we load the uC binaries, we pin them in a reserved section at the top of > * the GGTT, which is ~18 MBs. On multi-GT systems where the GTs share the GGTT, > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h > index 7a411178bdbf2..646fa8aa6cf19 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h > @@ -74,7 +74,8 @@ struct uc_css_header { > #define CSS_SW_VERSION_UC_MAJOR (0xFF << 16) > #define CSS_SW_VERSION_UC_MINOR (0xFF << 8) > #define CSS_SW_VERSION_UC_PATCH (0xFF << 0) > - u32 reserved0[13]; > + u32 vf_version; > + u32 reserved0[12]; > union { > u32 private_data_size; /* only applies to GuC */ > u32 reserved1;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 1bb3f98292866..bb4dfe707a7d0 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -158,6 +158,9 @@ struct intel_guc { bool submission_selected; /** @submission_initialized: tracks whether GuC submission has been initialised */ bool submission_initialized; + /** @submission_version: Submission API version of the currently loaded firmware */ + struct intel_uc_fw_ver submission_version; + /** * @rc_supported: tracks whether we support GuC rc on the current platform */ @@ -268,6 +271,14 @@ struct intel_guc { #endif }; +/* + * GuC version number components are only 8-bit, so converting to a 32bit 8.8.8 + * integer works. + */ +#define MAKE_GUC_VER(maj, min, pat) (((maj) << 16) | ((min) << 8) | (pat)) +#define MAKE_GUC_VER_STRUCT(ver) MAKE_GUC_VER((ver).major, (ver).minor, (ver).patch) +#define GUC_SUBMIT_VER(guc) MAKE_GUC_VER_STRUCT((guc)->submission_version) + static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) { return container_of(log, struct intel_guc, log); 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 0a42f1807f52c..53f7f599cde3a 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1890,7 +1890,7 @@ int intel_guc_submission_init(struct intel_guc *guc) if (guc->submission_initialized) return 0; - if (GET_UC_VER(guc) < MAKE_UC_VER(70, 0, 0)) { + if (GUC_SUBMIT_VER(guc) < MAKE_GUC_VER(1, 0, 0)) { ret = guc_lrc_desc_pool_create_v69(guc); if (ret) return ret; @@ -2330,7 +2330,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 (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) + if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) ret = register_context_v70(guc, ce, loop); else ret = register_context_v69(guc, ce, loop); @@ -2342,7 +2342,7 @@ static int register_context(struct intel_context *ce, bool loop) set_context_registered(ce); spin_unlock_irqrestore(&ce->guc_state.lock, flags); - if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) + if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) guc_context_policy_init_v70(ce, loop); } @@ -2956,7 +2956,7 @@ static void __guc_context_set_preemption_timeout(struct intel_guc *guc, u16 guc_id, u32 preemption_timeout) { - if (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) { + if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) { struct context_policy policy; __guc_context_policy_start_klv(&policy, guc_id); @@ -3283,7 +3283,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 (GET_UC_VER(guc) >= MAKE_UC_VER(70, 0, 0)) { + if (GUC_SUBMIT_VER(guc) >= MAKE_GUC_VER(1, 0, 0)) { struct context_policy policy; __guc_context_policy_start_klv(&policy, ce->guc_id.id); @@ -4366,7 +4366,7 @@ static int guc_init_global_schedule_policy(struct intel_guc *guc) intel_wakeref_t wakeref; int ret = 0; - if (GET_UC_VER(guc) < MAKE_UC_VER(70, 3, 0)) + if (GUC_SUBMIT_VER(guc) < MAKE_GUC_VER(1, 1, 0)) return 0; __guc_scheduling_policy_start_klv(&policy); @@ -4905,6 +4905,9 @@ void intel_guc_submission_print_info(struct intel_guc *guc, if (!sched_engine) return; + drm_printf(p, "GuC Submission API Version: %d.%d.%d\n", + guc->submission_version.major, guc->submission_version.minor, + guc->submission_version.patch); drm_printf(p, "GuC Number Outstanding Submission G2H: %u\n", atomic_read(&guc->outstanding_submission_g2h)); drm_printf(p, "GuC tasklet count: %u\n", 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 5e2ee1ac89514..7d2349647b593 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c @@ -478,6 +478,62 @@ static int check_gsc_manifest(const struct firmware *fw, return 0; } +static void uc_unpack_css_version(struct intel_uc_fw_ver *ver, u32 css_value) +{ + /* Get version numbers from the CSS header */ + ver->major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, css_value); + ver->minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR, css_value); + ver->patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, css_value); +} + +static void guc_read_css_info(struct intel_uc_fw *uc_fw, struct uc_css_header *css) +{ + struct intel_guc *guc = container_of(uc_fw, struct intel_guc, fw); + + /* + * The GuC firmware includes an extra version number to specify the + * submission API level. This allows submission code to work with + * multiple GuC versions without having to know the absolute firmware + * version number (there are likely to be multiple firmware releases + * which all support the same submission API level). + * + * Note that the spec for the CSS header defines this version number + * as 'vf_version' as it was originally intended for virtualisation. + * However, it is applicable to native submission as well. + * + * Unfortunately, due to an oversight, this version number was only + * exposed in the CSS header from v70.6.0. + */ + if (uc_fw->file_selected.ver.major >= 70) { + if (uc_fw->file_selected.ver.minor >= 6) { + /* v70.6.0 adds CSS header support */ + uc_unpack_css_version(&guc->submission_version, css->vf_version); + } else if (uc_fw->file_selected.ver.minor >= 3) { + /* v70.3.0 introduced v1.1.0 */ + guc->submission_version.major = 1; + guc->submission_version.minor = 1; + guc->submission_version.patch = 0; + } else { + /* v70.0.0 introduced v1.0.0 */ + guc->submission_version.major = 1; + guc->submission_version.minor = 0; + guc->submission_version.patch = 0; + } + } else if (uc_fw->file_selected.ver.major >= 69) { + /* v69.0.0 introduced v0.10.0 */ + guc->submission_version.major = 0; + guc->submission_version.minor = 10; + guc->submission_version.patch = 0; + } else { + /* Prior versions were v0.1.0 */ + guc->submission_version.major = 0; + guc->submission_version.minor = 1; + guc->submission_version.patch = 0; + } + + uc_fw->private_data_size = css->private_data_size; +} + static int check_ccs_header(struct intel_gt *gt, const struct firmware *fw, struct intel_uc_fw *uc_fw) @@ -531,20 +587,50 @@ static int check_ccs_header(struct intel_gt *gt, return -E2BIG; } - /* Get version numbers from the CSS header */ - uc_fw->file_selected.ver.major = FIELD_GET(CSS_SW_VERSION_UC_MAJOR, - css->sw_version); - uc_fw->file_selected.ver.minor = FIELD_GET(CSS_SW_VERSION_UC_MINOR, - css->sw_version); - uc_fw->file_selected.ver.patch = FIELD_GET(CSS_SW_VERSION_UC_PATCH, - css->sw_version); + uc_unpack_css_version(&uc_fw->file_selected.ver, css->sw_version); if (uc_fw->type == INTEL_UC_FW_TYPE_GUC) - uc_fw->private_data_size = css->private_data_size; + guc_read_css_info(uc_fw, css); return 0; } +static bool is_ver_8bit(struct intel_uc_fw_ver *ver) +{ + return ver->major < 0xFF && ver->minor < 0xFF && ver->patch < 0xFF; +} + +static bool gyc_check_version_range(struct intel_uc_fw *uc_fw) +{ + struct intel_guc *guc = container_of(uc_fw, struct intel_guc, fw); + + /* + * GuC version number components are defined as being 8-bits. + * The submission code relies on this to optimise version comparison + * tests. So enforce the restriction here. + */ + + if (!is_ver_8bit(&uc_fw->file_selected.ver)) { + drm_warn(&__uc_fw_to_gt(uc_fw)->i915->drm, "%s firmware: invalid file version: 0x%02X:%02X:%02X\n", + intel_uc_fw_type_repr(uc_fw->type), + uc_fw->file_selected.ver.major, + uc_fw->file_selected.ver.minor, + uc_fw->file_selected.ver.patch); + return false; + } + + if (!is_ver_8bit(&guc->submission_version)) { + drm_warn(&__uc_fw_to_gt(uc_fw)->i915->drm, "%s firmware: invalid submit version: 0x%02X:%02X:%02X\n", + intel_uc_fw_type_repr(uc_fw->type), + guc->submission_version.major, + guc->submission_version.minor, + guc->submission_version.patch); + return false; + } + + return true; +} + /** * intel_uc_fw_fetch - fetch uC firmware * @uc_fw: uC firmware @@ -621,6 +707,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw) if (err) goto fail; + if (uc_fw->type == INTEL_UC_FW_TYPE_GUC && !gyc_check_version_range(uc_fw)) + goto fail; + if (uc_fw->file_wanted.ver.major) { /* Check the file's major version was as it claimed */ if (uc_fw->file_selected.ver.major != uc_fw->file_wanted.ver.major) { @@ -1054,7 +1143,7 @@ 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) { - u32 ver_sel, ver_want; + bool got_wanted; drm_printf(p, "%s firmware: %s\n", intel_uc_fw_type_repr(uc_fw->type), uc_fw->file_selected.path); @@ -1063,9 +1152,20 @@ void intel_uc_fw_dump(const struct intel_uc_fw *uc_fw, struct drm_printer *p) 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)); - ver_sel = MAKE_UC_VER_STRUCT(uc_fw->file_selected.ver); - ver_want = MAKE_UC_VER_STRUCT(uc_fw->file_wanted.ver); - if (ver_sel < ver_want) + + if (uc_fw->file_selected.ver.major < uc_fw->file_wanted.ver.major) + got_wanted = false; + else if ((uc_fw->file_selected.ver.major == uc_fw->file_wanted.ver.major) && + (uc_fw->file_selected.ver.minor < uc_fw->file_wanted.ver.minor)) + got_wanted = false; + else if ((uc_fw->file_selected.ver.major == uc_fw->file_wanted.ver.major) && + (uc_fw->file_selected.ver.minor == uc_fw->file_wanted.ver.minor) && + (uc_fw->file_selected.ver.patch < uc_fw->file_wanted.ver.patch)) + got_wanted = false; + else + got_wanted = true; + + if (!got_wanted) drm_printf(p, "\tversion: wanted %u.%u.%u, found %u.%u.%u\n", uc_fw->file_wanted.ver.major, uc_fw->file_wanted.ver.minor, 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 6501d6f1fbdff..3ab87c54a3987 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h @@ -66,9 +66,9 @@ enum intel_uc_fw_type { #define INTEL_UC_FW_NUM_TYPES 2 struct intel_uc_fw_ver { - u16 major; - u16 minor; - u16 patch; + u32 major; + u32 minor; + u32 patch; }; /* @@ -114,10 +114,6 @@ struct intel_uc_fw { bool loaded_via_gsc; }; -#define MAKE_UC_VER(maj, min, pat) ((pat) | ((min) << 8) | ((maj) << 16)) -#define MAKE_UC_VER_STRUCT(ver) MAKE_UC_VER((ver).major, (ver).minor, (ver).patch) -#define GET_UC_VER(uc) (MAKE_UC_VER_STRUCT((uc)->fw.file_selected.ver)) - /* * When we load the uC binaries, we pin them in a reserved section at the top of * the GGTT, which is ~18 MBs. On multi-GT systems where the GTs share the GGTT, diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h index 7a411178bdbf2..646fa8aa6cf19 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h @@ -74,7 +74,8 @@ struct uc_css_header { #define CSS_SW_VERSION_UC_MAJOR (0xFF << 16) #define CSS_SW_VERSION_UC_MINOR (0xFF << 8) #define CSS_SW_VERSION_UC_PATCH (0xFF << 0) - u32 reserved0[13]; + u32 vf_version; + u32 reserved0[12]; union { u32 private_data_size; /* only applies to GuC */ u32 reserved1;