Re: [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


Anusha
		if (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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux