On Fri, Jun 14, 2019 at 12:44:50PM -0700, Anusha Srivatsa wrote:
-----Original Message----- From: De Marchi, Lucas Sent: Friday, June 7, 2019 2:13 AM To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx Cc: Vivi, Rodrigo <rodrigo.vivi@xxxxxxxxx>; Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>; De Marchi, Lucas <lucas.demarchi@xxxxxxxxx> Subject: [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware In intel_package_header version 2 there's a new field in the fw_info table that must be 0, otherwise it's not the correct DMC firmware. Add a check for version 2 or later. Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> --- drivers/gpu/drm/i915/intel_csr.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c index 7608e4e2950d..864531aae1a5 100644 --- a/drivers/gpu/drm/i915/intel_csr.c +++ b/drivers/gpu/drm/i915/intel_csr.c @@ -120,7 +120,10 @@ struct intel_css_header { } __packed; struct intel_fw_info { - u16 reserved1; + u8 reserved1; + + /* reserved on package_header version 1, must be 0 on version 2 */ + u8 dmc_id; /* Stepping (A, B, C, ..., *). * is a wildcard */ char stepping; @@ -325,12 +328,16 @@ void intel_csr_load_program(struct drm_i915_private *dev_priv) */ static u32 find_dmc_fw_offset(const struct intel_fw_info *fw_info, unsigned int num_entries, - const struct stepping_info *si) + const struct stepping_info *si, + u8 package_ver) { u32 dmc_offset = CSR_DEFAULT_FW_OFFSET; unsigned int i; for (i = 0; i < num_entries; i++) { + if (package_ver > 1 && fw_info[i].dmc_id != 0) + continue;Wont we need a message here? "Wrong DMC version, not loading v_x.0x" or something?.."
I don't think so. It's normal to have other blobs inside the firmware that we don't care about. At most I could put a debug, but I don't think we really care. As long as we find one we should be fine. And if we don't, then we will complain loudly later in this function with a DRM_ERROR(). dmc_offset remaining as CSR_DEFAULT_FW_OFFSET has a double meaning here: either we didn't find any entry or we found one that had the offset set to this value. Lucas De Marchi
Anushaif (fw_info[i].substepping == '*' && si->stepping == fw_info[i].stepping) { dmc_offset = fw_info[i].offset; @@ -508,7 +515,8 @@ parse_csr_fw_package(struct intel_csr *csr, fw_info = (const struct intel_fw_info *) ((u8 *)package_header + sizeof(*package_header)); - dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si); + dmc_offset = find_dmc_fw_offset(fw_info, num_entries, si, + package_header->header_ver); if (dmc_offset == CSR_DEFAULT_FW_OFFSET) { DRM_ERROR("DMC firmware not supported for %c stepping\n", si->stepping); -- 2.21.0
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
