Re: [Intel-gfx] [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
>-Original Message-
>From: De Marchi, Lucas
>Sent: Friday, June 14, 2019 2:37 PM
>To: Srivatsa, Anusha
>Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
>Subject: Re: [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
>
>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@lists.freedesktop.org
>>>Cc: Vivi, Rodrigo ; Srivatsa, Anusha
>>>; De Marchi, Lucas
>>>
>>>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
>>>---
>>> 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.
Ok. Makes sense.
Reviewed-by: Anusha Srivatsa
>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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
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@lists.freedesktop.org
Cc: Vivi, Rodrigo ; Srivatsa, Anusha
; De Marchi, Lucas
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
---
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 9/9] drm/i915/dmc: protect against loading wrong firmware
>-Original Message-
>From: De Marchi, Lucas
>Sent: Friday, June 7, 2019 2:13 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo ; Srivatsa, Anusha
>; De Marchi, Lucas
>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
>---
> 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?.."
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [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
---
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;
+
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
