* [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-27 0:35 ` Anusha Srivatsa
0 siblings, 0 replies; 34+ messages in thread
From: Anusha Srivatsa @ 2022-04-27 0:35 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, stable
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
2 files changed, 72 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..ac7896835bfa 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
#define BXT_DMC_MAX_FW_SIZE 0x3000
MODULE_FIRMWARE(BXT_DMC_PATH);
-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
-#define DMC_V1_MAX_MMIO_COUNT 8
-#define DMC_V3_MAX_MMIO_COUNT 20
-#define DMC_V1_MMIO_START_RANGE 0x80000
-
struct intel_css_header {
/* 0x09 for DMC */
u32 module_type;
@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ int i;
+
+ if (header_ver == 1) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < DMC_MMIO_START_RANGE || mmioaddr[i] > DMC_MMIO_END_RANGE)
+ return false;
+ }
+ }
+
+ /* Main DMC MMIO check */
+ if (dmc_id == DMC_FW_MAIN) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
+ return false;
+ }
+ }
+
+ /* Pipe DMC MMIO check */
+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START && mmioaddr[i] > ADLP_PIPE_MMIO_END)
+ return false;
+ }
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
+ return false;
+ }
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..235d1b721334 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -11,12 +11,43 @@
#define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
#define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
#define DMC_HTP_ADDR_SKL 0x00500034
+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
#define DMC_SSP_BASE _MMIO(0x8F074)
#define DMC_HTP_SKL _MMIO(0x8F004)
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
#define DMC_MMIO_END_RANGE 0x8FFFF
+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
+#define DMC_V1_MAX_MMIO_COUNT 8
+#define DMC_V3_MAX_MMIO_COUNT 20
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define _TGL_MAIN_MMIO_START 0x8F000
+#define _TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define _TGL_PIPEC_MMIO_START 0x9A000
+#define _TGL_PIPEC_MMIO_END 0x9BFFF
+#define _TGL_PIPED_MMIO_START 0x9E000
+#define _TGL_PIPED_MMIO_END 0x9FFFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe, _TGL_MAIN_MMIO_START,\
+ _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START,\
+ _TGL_PIPEC_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_DMC_MMIO_END(pipe) _PICK(pipe, _TGL_MAIN_MMIO_END,\
+ _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END,\
+ _TGL_PIPEC_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-27 0:35 ` Anusha Srivatsa
0 siblings, 0 replies; 34+ messages in thread
From: Anusha Srivatsa @ 2022-04-27 0:35 UTC (permalink / raw)
To: intel-gfx; +Cc: Anusha Srivatsa, stable, Lucas De Marchi
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
2 files changed, 72 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..ac7896835bfa 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
#define BXT_DMC_MAX_FW_SIZE 0x3000
MODULE_FIRMWARE(BXT_DMC_PATH);
-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
-#define DMC_V1_MAX_MMIO_COUNT 8
-#define DMC_V3_MAX_MMIO_COUNT 20
-#define DMC_V1_MMIO_START_RANGE 0x80000
-
struct intel_css_header {
/* 0x09 for DMC */
u32 module_type;
@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ int i;
+
+ if (header_ver == 1) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < DMC_MMIO_START_RANGE || mmioaddr[i] > DMC_MMIO_END_RANGE)
+ return false;
+ }
+ }
+
+ /* Main DMC MMIO check */
+ if (dmc_id == DMC_FW_MAIN) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
+ return false;
+ }
+ }
+
+ /* Pipe DMC MMIO check */
+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START && mmioaddr[i] > ADLP_PIPE_MMIO_END)
+ return false;
+ }
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
+ return false;
+ }
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..235d1b721334 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -11,12 +11,43 @@
#define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
#define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
#define DMC_HTP_ADDR_SKL 0x00500034
+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
#define DMC_SSP_BASE _MMIO(0x8F074)
#define DMC_HTP_SKL _MMIO(0x8F004)
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
#define DMC_MMIO_END_RANGE 0x8FFFF
+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
+#define DMC_V1_MAX_MMIO_COUNT 8
+#define DMC_V3_MAX_MMIO_COUNT 20
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define _TGL_MAIN_MMIO_START 0x8F000
+#define _TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define _TGL_PIPEC_MMIO_START 0x9A000
+#define _TGL_PIPEC_MMIO_END 0x9BFFF
+#define _TGL_PIPED_MMIO_START 0x9E000
+#define _TGL_PIPED_MMIO_END 0x9FFFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe, _TGL_MAIN_MMIO_START,\
+ _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START,\
+ _TGL_PIPEC_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_DMC_MMIO_END(pipe) _PICK(pipe, _TGL_MAIN_MMIO_END,\
+ _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END,\
+ _TGL_PIPEC_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dmc: Add MMIO range restrictions (rev3)
2022-04-27 0:35 ` Anusha Srivatsa
(?)
@ 2022-04-27 1:44 ` Patchwork
-1 siblings, 0 replies; 34+ messages in thread
From: Patchwork @ 2022-04-27 1:44 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/dmc: Add MMIO range restrictions (rev3)
URL : https://patchwork.freedesktop.org/series/102168/
State : warning
== Summary ==
Error: dim checkpatch failed
f6bceb457fc4 drm/i915/dmc: Add MMIO range restrictions
-:58: WARNING:LONG_LINE: line length of 111 exceeds 100 columns
#58: FILE: drivers/gpu/drm/i915/display/intel_dmc.c:386:
+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
-:71: WARNING:LONG_LINE: line length of 111 exceeds 100 columns
#71: FILE: drivers/gpu/drm/i915/display/intel_dmc.c:399:
+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
total: 0 errors, 2 warnings, 0 checks, 109 lines checked
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/dmc: Add MMIO range restrictions (rev3)
2022-04-27 0:35 ` Anusha Srivatsa
(?)
(?)
@ 2022-04-27 2:10 ` Patchwork
-1 siblings, 0 replies; 34+ messages in thread
From: Patchwork @ 2022-04-27 2:10 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 29756 bytes --]
== Series Details ==
Series: drm/i915/dmc: Add MMIO range restrictions (rev3)
URL : https://patchwork.freedesktop.org/series/102168/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_11550 -> Patchwork_102168v3
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_102168v3 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_102168v3, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/index.html
Participating hosts (43 -> 44)
------------------------------
Additional (2): fi-kbl-x1275 bat-dg1-6
Missing (1): fi-bsw-cyan
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_102168v3:
### IGT changes ###
#### Possible regressions ####
* igt@gem_exec_suspend@basic-s0@smem:
- fi-kbl-guc: [PASS][1] -> [DMESG-WARN][2] +1 similar issue
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-guc/igt@gem_exec_suspend@basic-s0@smem.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-guc/igt@gem_exec_suspend@basic-s0@smem.html
- fi-glk-j4005: [PASS][3] -> [DMESG-WARN][4] +2 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-glk-j4005/igt@gem_exec_suspend@basic-s0@smem.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-glk-j4005/igt@gem_exec_suspend@basic-s0@smem.html
- fi-kbl-x1275: NOTRUN -> [DMESG-WARN][5] +2 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-x1275/igt@gem_exec_suspend@basic-s0@smem.html
- fi-skl-6700k2: [PASS][6] -> [DMESG-WARN][7]
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-skl-6700k2/igt@gem_exec_suspend@basic-s0@smem.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-skl-6700k2/igt@gem_exec_suspend@basic-s0@smem.html
- fi-kbl-7567u: [PASS][8] -> [DMESG-WARN][9] +1 similar issue
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-7567u/igt@gem_exec_suspend@basic-s0@smem.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-7567u/igt@gem_exec_suspend@basic-s0@smem.html
- fi-rkl-11600: [PASS][10] -> [DMESG-WARN][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-rkl-11600/igt@gem_exec_suspend@basic-s0@smem.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@gem_exec_suspend@basic-s0@smem.html
* igt@gem_exec_suspend@basic-s3@smem:
- fi-skl-guc: [PASS][12] -> [DMESG-WARN][13] +2 similar issues
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-skl-guc/igt@gem_exec_suspend@basic-s3@smem.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-skl-guc/igt@gem_exec_suspend@basic-s3@smem.html
- fi-cfl-guc: [PASS][14] -> [DMESG-WARN][15] +2 similar issues
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-cfl-guc/igt@gem_exec_suspend@basic-s3@smem.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-cfl-guc/igt@gem_exec_suspend@basic-s3@smem.html
- fi-bxt-dsi: [PASS][16] -> [DMESG-WARN][17] +2 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-bxt-dsi/igt@gem_exec_suspend@basic-s3@smem.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-bxt-dsi/igt@gem_exec_suspend@basic-s3@smem.html
- fi-rkl-guc: [PASS][18] -> [DMESG-WARN][19] +2 similar issues
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-rkl-guc/igt@gem_exec_suspend@basic-s3@smem.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-guc/igt@gem_exec_suspend@basic-s3@smem.html
- fi-adl-ddr5: [PASS][20] -> [DMESG-WARN][21] +2 similar issues
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-adl-ddr5/igt@gem_exec_suspend@basic-s3@smem.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-adl-ddr5/igt@gem_exec_suspend@basic-s3@smem.html
- fi-tgl-1115g4: [PASS][22] -> [DMESG-WARN][23] +2 similar issues
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s3@smem.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s3@smem.html
- fi-kbl-7500u: [PASS][24] -> [DMESG-WARN][25] +2 similar issues
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-7500u/igt@gem_exec_suspend@basic-s3@smem.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-7500u/igt@gem_exec_suspend@basic-s3@smem.html
* igt@i915_pm_rpm@basic-rte:
- fi-rkl-guc: [PASS][26] -> [SKIP][27] +2 similar issues
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-rkl-guc/igt@i915_pm_rpm@basic-rte.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-guc/igt@i915_pm_rpm@basic-rte.html
- fi-adl-ddr5: [PASS][28] -> [SKIP][29] +2 similar issues
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-adl-ddr5/igt@i915_pm_rpm@basic-rte.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-adl-ddr5/igt@i915_pm_rpm@basic-rte.html
- fi-tgl-1115g4: [PASS][30] -> [SKIP][31] +2 similar issues
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-tgl-1115g4/igt@i915_pm_rpm@basic-rte.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-tgl-1115g4/igt@i915_pm_rpm@basic-rte.html
* igt@i915_pm_rpm@module-reload:
- fi-rkl-11600: NOTRUN -> [SKIP][32] +2 similar issues
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@i915_pm_rpm@module-reload.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-kbl-soraka: [PASS][33] -> [DMESG-WARN][34] +2 similar issues
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-soraka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-soraka/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
- fi-rkl-11600: NOTRUN -> [DMESG-WARN][35]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
- fi-cfl-8700k: [PASS][36] -> [DMESG-WARN][37] +2 similar issues
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-cfl-8700k/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-cfl-8700k/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
- fi-cfl-8109u: [PASS][38] -> [DMESG-WARN][39] +2 similar issues
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-cfl-8109u/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-cfl-8109u/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
#### Warnings ####
* igt@gem_exec_suspend@basic-s3@smem:
- fi-rkl-11600: [INCOMPLETE][40] ([i915#5127]) -> [DMESG-WARN][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@gem_exec_suspend@basic-s0@smem:
- {bat-rpls-2}: [PASS][42] -> [DMESG-WARN][43] +2 similar issues
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-rpls-2/igt@gem_exec_suspend@basic-s0@smem.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-rpls-2/igt@gem_exec_suspend@basic-s0@smem.html
- {bat-jsl-2}: [PASS][44] -> [DMESG-WARN][45] +2 similar issues
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-jsl-2/igt@gem_exec_suspend@basic-s0@smem.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-jsl-2/igt@gem_exec_suspend@basic-s0@smem.html
- {bat-rpls-1}: [PASS][46] -> [DMESG-WARN][47] +2 similar issues
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-rpls-1/igt@gem_exec_suspend@basic-s0@smem.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-rpls-1/igt@gem_exec_suspend@basic-s0@smem.html
* igt@gem_exec_suspend@basic-s3@smem:
- {fi-ehl-2}: [PASS][48] -> [DMESG-WARN][49] +2 similar issues
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-ehl-2/igt@gem_exec_suspend@basic-s3@smem.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-ehl-2/igt@gem_exec_suspend@basic-s3@smem.html
- {fi-jsl-1}: [PASS][50] -> [DMESG-WARN][51] +2 similar issues
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-jsl-1/igt@gem_exec_suspend@basic-s3@smem.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-jsl-1/igt@gem_exec_suspend@basic-s3@smem.html
- {bat-jsl-1}: [PASS][52] -> [DMESG-WARN][53] +2 similar issues
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-jsl-1/igt@gem_exec_suspend@basic-s3@smem.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-jsl-1/igt@gem_exec_suspend@basic-s3@smem.html
* igt@i915_pm_rpm@basic-pci-d3-state:
- {bat-jsl-1}: [PASS][54] -> [SKIP][55] +2 similar issues
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-jsl-1/igt@i915_pm_rpm@basic-pci-d3-state.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-jsl-1/igt@i915_pm_rpm@basic-pci-d3-state.html
- {bat-rpls-2}: [PASS][56] -> [SKIP][57] +1 similar issue
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-rpls-2/igt@i915_pm_rpm@basic-pci-d3-state.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-rpls-2/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_pm_rpm@basic-rte:
- {bat-rpls-1}: [PASS][58] -> [SKIP][59] +2 similar issues
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-rpls-1/igt@i915_pm_rpm@basic-rte.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-rpls-1/igt@i915_pm_rpm@basic-rte.html
- {fi-jsl-1}: [PASS][60] -> [SKIP][61] +2 similar issues
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-jsl-1/igt@i915_pm_rpm@basic-rte.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-jsl-1/igt@i915_pm_rpm@basic-rte.html
- {bat-jsl-2}: [PASS][62] -> [SKIP][63] +2 similar issues
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-jsl-2/igt@i915_pm_rpm@basic-rte.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-jsl-2/igt@i915_pm_rpm@basic-rte.html
- {fi-tgl-dsi}: [PASS][64] -> [SKIP][65] +2 similar issues
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-tgl-dsi/igt@i915_pm_rpm@basic-rte.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-tgl-dsi/igt@i915_pm_rpm@basic-rte.html
* igt@i915_pm_rpm@module-reload:
- {fi-ehl-2}: [PASS][66] -> [SKIP][67] +2 similar issues
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-ehl-2/igt@i915_pm_rpm@module-reload.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-ehl-2/igt@i915_pm_rpm@module-reload.html
- {bat-adlp-6}: [PASS][68] -> [SKIP][69] +2 similar issues
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-adlp-6/igt@i915_pm_rpm@module-reload.html
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-adlp-6/igt@i915_pm_rpm@module-reload.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- {bat-adlp-6}: [PASS][70] -> [DMESG-WARN][71] +2 similar issues
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-adlp-6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-adlp-6/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
Known issues
------------
Here are the changes found in Patchwork_102168v3 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s0@smem:
- bat-dg1-6: NOTRUN -> [INCOMPLETE][72] ([i915#5827])
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-dg1-6/igt@gem_exec_suspend@basic-s0@smem.html
* igt@gem_exec_suspend@basic-s3@smem:
- fi-skl-6700k2: [PASS][73] -> [INCOMPLETE][74] ([i915#4939] / [i915#5680])
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-skl-6700k2/igt@gem_exec_suspend@basic-s3@smem.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-skl-6700k2/igt@gem_exec_suspend@basic-s3@smem.html
* igt@gem_huc_copy@huc-copy:
- fi-kbl-x1275: NOTRUN -> [SKIP][75] ([fdo#109271] / [i915#2190])
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-x1275/igt@gem_huc_copy@huc-copy.html
- fi-rkl-11600: NOTRUN -> [SKIP][76] ([i915#2190])
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@basic:
- fi-rkl-11600: NOTRUN -> [SKIP][77] ([i915#4613]) +3 similar issues
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@gem_lmem_swapping@basic.html
- fi-kbl-guc: NOTRUN -> [SKIP][78] ([fdo#109271] / [i915#4613]) +3 similar issues
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-guc/igt@gem_lmem_swapping@basic.html
* igt@gem_lmem_swapping@verify-random:
- fi-kbl-x1275: NOTRUN -> [SKIP][79] ([fdo#109271] / [i915#4613]) +3 similar issues
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-x1275/igt@gem_lmem_swapping@verify-random.html
* igt@gem_tiled_pread_basic:
- fi-rkl-11600: NOTRUN -> [SKIP][80] ([i915#3282])
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@gem_tiled_pread_basic.html
* igt@i915_pm_backlight@basic-brightness:
- fi-rkl-11600: NOTRUN -> [SKIP][81] ([i915#3012])
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-kbl-soraka: [PASS][82] -> [SKIP][83] ([fdo#109271]) +2 similar issues
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-soraka/igt@i915_pm_rpm@basic-pci-d3-state.html
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-soraka/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_pm_rpm@basic-rte:
- fi-kbl-7500u: [PASS][84] -> [SKIP][85] ([fdo#109271]) +2 similar issues
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-7500u/igt@i915_pm_rpm@basic-rte.html
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-7500u/igt@i915_pm_rpm@basic-rte.html
- fi-bxt-dsi: [PASS][86] -> [SKIP][87] ([fdo#109271]) +2 similar issues
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-bxt-dsi/igt@i915_pm_rpm@basic-rte.html
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-bxt-dsi/igt@i915_pm_rpm@basic-rte.html
* igt@i915_pm_rpm@module-reload:
- fi-cfl-8109u: [PASS][88] -> [SKIP][89] ([fdo#109271]) +2 similar issues
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
- fi-cfl-8700k: [PASS][90] -> [SKIP][91] ([fdo#109271]) +2 similar issues
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-cfl-8700k/igt@i915_pm_rpm@module-reload.html
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-cfl-8700k/igt@i915_pm_rpm@module-reload.html
- fi-skl-guc: [PASS][92] -> [SKIP][93] ([fdo#109271]) +2 similar issues
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-skl-guc/igt@i915_pm_rpm@module-reload.html
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-skl-guc/igt@i915_pm_rpm@module-reload.html
- fi-kbl-7567u: [PASS][94] -> [SKIP][95] ([fdo#109271])
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html
- fi-glk-j4005: [PASS][96] -> [SKIP][97] ([fdo#109271]) +2 similar issues
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-glk-j4005/igt@i915_pm_rpm@module-reload.html
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-glk-j4005/igt@i915_pm_rpm@module-reload.html
- fi-cfl-guc: [PASS][98] -> [SKIP][99] ([fdo#109271]) +2 similar issues
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-cfl-guc/igt@i915_pm_rpm@module-reload.html
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-cfl-guc/igt@i915_pm_rpm@module-reload.html
- fi-kbl-8809g: [PASS][100] -> [SKIP][101] ([fdo#109271])
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-8809g/igt@i915_pm_rpm@module-reload.html
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-8809g/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live@hangcheck:
- fi-adl-ddr5: [PASS][102] -> [DMESG-WARN][103] ([i915#5591])
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-adl-ddr5/igt@i915_selftest@live@hangcheck.html
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-adl-ddr5/igt@i915_selftest@live@hangcheck.html
* igt@kms_chamelium@dp-crc-fast:
- fi-rkl-11600: NOTRUN -> [SKIP][104] ([fdo#111827]) +8 similar issues
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@kms_chamelium@dp-crc-fast.html
* igt@kms_chamelium@hdmi-crc-fast:
- fi-kbl-x1275: NOTRUN -> [SKIP][105] ([fdo#109271] / [fdo#111827]) +8 similar issues
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-x1275/igt@kms_chamelium@hdmi-crc-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-rkl-11600: NOTRUN -> [SKIP][106] ([i915#4070] / [i915#4103]) +1 similar issue
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
* igt@kms_force_connector_basic@force-load-detect:
- fi-rkl-11600: NOTRUN -> [SKIP][107] ([fdo#109285] / [i915#4098])
[107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
- fi-rkl-11600: NOTRUN -> [SKIP][108] ([i915#4070] / [i915#533])
[108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
- fi-kbl-guc: NOTRUN -> [SKIP][109] ([fdo#109271] / [i915#533])
[109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-guc/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
- fi-kbl-x1275: NOTRUN -> [SKIP][110] ([fdo#109271] / [i915#533])
[110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-x1275/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
* igt@kms_pipe_crc_basic@read-crc-pipe-c:
- fi-kbl-guc: NOTRUN -> [SKIP][111] ([fdo#109271]) +23 similar issues
[111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-guc/igt@kms_pipe_crc_basic@read-crc-pipe-c.html
* igt@kms_psr@primary_mmap_gtt:
- fi-rkl-11600: NOTRUN -> [SKIP][112] ([i915#1072]) +3 similar issues
[112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@kms_psr@primary_mmap_gtt.html
* igt@kms_psr@primary_page_flip:
- fi-kbl-x1275: NOTRUN -> [SKIP][113] ([fdo#109271]) +15 similar issues
[113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-x1275/igt@kms_psr@primary_page_flip.html
* igt@kms_setmode@basic-clone-single-crtc:
- fi-rkl-11600: NOTRUN -> [SKIP][114] ([i915#3555] / [i915#4098])
[114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-userptr:
- fi-rkl-11600: NOTRUN -> [SKIP][115] ([i915#3301] / [i915#3708])
[115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@prime_vgem@basic-userptr.html
* igt@prime_vgem@basic-write:
- fi-rkl-11600: NOTRUN -> [SKIP][116] ([i915#3291] / [i915#3708]) +2 similar issues
[116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-rkl-11600/igt@prime_vgem@basic-write.html
#### Possible fixes ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-cfl-guc: [DMESG-FAIL][117] ([i915#5334]) -> [PASS][118]
[117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-cfl-guc/igt@i915_selftest@live@gt_heartbeat.html
[118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-cfl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@hangcheck:
- fi-hsw-4770: [INCOMPLETE][119] ([i915#4785]) -> [PASS][120]
[119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
[120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
* igt@kms_busy@basic@modeset:
- {bat-adlp-6}: [DMESG-WARN][121] ([i915#3576]) -> [PASS][122]
[121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/bat-adlp-6/igt@kms_busy@basic@modeset.html
[122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/bat-adlp-6/igt@kms_busy@basic@modeset.html
#### Warnings ####
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
- fi-kbl-7567u: [SKIP][123] ([fdo#109271] / [i915#5341]) -> [SKIP][124] ([fdo#109271])
[123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-7567u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-7567u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-pnv-d510: [SKIP][125] ([fdo#109271] / [i915#5341]) -> [SKIP][126] ([fdo#109271])
[125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-pnv-d510/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-pnv-d510/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-snb-2520m: [SKIP][127] ([fdo#109271] / [i915#5341]) -> [SKIP][128] ([fdo#109271])
[127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-snb-2520m/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-snb-2520m/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-bsw-kefka: [SKIP][129] ([fdo#109271] / [i915#5341]) -> [SKIP][130] ([fdo#109271])
[129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-bsw-kefka/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-bsw-kefka/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-kbl-8809g: [SKIP][131] ([fdo#109271] / [i915#5341]) -> [SKIP][132] ([fdo#109271])
[131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-kbl-8809g/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-kbl-8809g/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-bsw-nick: [SKIP][133] ([fdo#109271] / [i915#5341]) -> [SKIP][134] ([fdo#109271])
[133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-bsw-nick/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-bsw-nick/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-bwr-2160: [SKIP][135] ([fdo#109271] / [i915#5341]) -> [SKIP][136] ([fdo#109271])
[135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-bwr-2160/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-bwr-2160/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-snb-2600: [SKIP][137] ([fdo#109271] / [i915#5341]) -> [SKIP][138] ([fdo#109271])
[137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-snb-2600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-snb-2600/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-elk-e7500: [SKIP][139] ([fdo#109271] / [i915#5341]) -> [SKIP][140] ([fdo#109271])
[139]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-elk-e7500/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-elk-e7500/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-blb-e6850: [SKIP][141] ([fdo#109271] / [i915#5341]) -> [SKIP][142] ([fdo#109271])
[141]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-blb-e6850/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[142]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-blb-e6850/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
- fi-ilk-650: [SKIP][143] ([fdo#109271] / [i915#5341]) -> [SKIP][144] ([fdo#109271])
[143]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11550/fi-ilk-650/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
[144]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/fi-ilk-650/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
[i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
[i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
[i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
[i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
[i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
[i915#4939]: https://gitlab.freedesktop.org/drm/intel/issues/4939
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#5127]: https://gitlab.freedesktop.org/drm/intel/issues/5127
[i915#5329]: https://gitlab.freedesktop.org/drm/intel/issues/5329
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#5338]: https://gitlab.freedesktop.org/drm/intel/issues/5338
[i915#5341]: https://gitlab.freedesktop.org/drm/intel/issues/5341
[i915#5537]: https://gitlab.freedesktop.org/drm/intel/issues/5537
[i915#5591]: https://gitlab.freedesktop.org/drm/intel/issues/5591
[i915#5680]: https://gitlab.freedesktop.org/drm/intel/issues/5680
[i915#5827]: https://gitlab.freedesktop.org/drm/intel/issues/5827
Build changes
-------------
* Linux: CI_DRM_11550 -> Patchwork_102168v3
CI-20190529: 20190529
CI_DRM_11550: 56b089ae03ef8ea8ab7f474eaa70367898891ef0 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6451: f055bd83bd831a938d639718c2359516224f15f9 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_102168v3: 56b089ae03ef8ea8ab7f474eaa70367898891ef0 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
d019fe2b8edd drm/i915/dmc: Add MMIO range restrictions
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102168v3/index.html
[-- Attachment #2: Type: text/html, Size: 35382 bytes --]
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-27 0:35 ` Anusha Srivatsa
@ 2022-04-27 4:26 ` kernel test robot
-1 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2022-04-27 4:26 UTC (permalink / raw)
To: Anusha Srivatsa, intel-gfx; +Cc: kbuild-all, Lucas De Marchi, stable
Hi Anusha,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip next-20220426]
[cannot apply to v5.18-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220427-084021
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-a012-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271216.6t3eWj4f-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/f79241ea04e8815b3c1b0ab6b9d6136efc8646d3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220427-084021
git checkout f79241ea04e8815b3c1b0ab6b9d6136efc8646d3
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_dmc.c: In function 'parse_dmc_fw_header':
>> drivers/gpu/drm/i915/display/intel_dmc.c:476:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
476 | if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
| ^~
drivers/gpu/drm/i915/display/intel_dmc.c:478:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
478 | return 0;
| ^~~~~~
vim +/if +476 drivers/gpu/drm/i915/display/intel_dmc.c
406
407 static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
408 const struct intel_dmc_header_base *dmc_header,
409 size_t rem_size, u8 dmc_id)
410 {
411 struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
412 struct dmc_fw_info *dmc_info = &dmc->dmc_info[dmc_id];
413 unsigned int header_len_bytes, dmc_header_size, payload_size, i;
414 const u32 *mmioaddr, *mmiodata;
415 u32 mmio_count, mmio_count_max, start_mmioaddr;
416 u8 *payload;
417
418 BUILD_BUG_ON(ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V3_MAX_MMIO_COUNT ||
419 ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V1_MAX_MMIO_COUNT);
420
421 /*
422 * Check if we can access common fields, we will checkc again below
423 * after we have read the version
424 */
425 if (rem_size < sizeof(struct intel_dmc_header_base))
426 goto error_truncated;
427
428 /* Cope with small differences between v1 and v3 */
429 if (dmc_header->header_ver == 3) {
430 const struct intel_dmc_header_v3 *v3 =
431 (const struct intel_dmc_header_v3 *)dmc_header;
432
433 if (rem_size < sizeof(struct intel_dmc_header_v3))
434 goto error_truncated;
435
436 mmioaddr = v3->mmioaddr;
437 mmiodata = v3->mmiodata;
438 mmio_count = v3->mmio_count;
439 mmio_count_max = DMC_V3_MAX_MMIO_COUNT;
440 /* header_len is in dwords */
441 header_len_bytes = dmc_header->header_len * 4;
442 start_mmioaddr = v3->start_mmioaddr;
443 dmc_header_size = sizeof(*v3);
444 } else if (dmc_header->header_ver == 1) {
445 const struct intel_dmc_header_v1 *v1 =
446 (const struct intel_dmc_header_v1 *)dmc_header;
447
448 if (rem_size < sizeof(struct intel_dmc_header_v1))
449 goto error_truncated;
450
451 mmioaddr = v1->mmioaddr;
452 mmiodata = v1->mmiodata;
453 mmio_count = v1->mmio_count;
454 mmio_count_max = DMC_V1_MAX_MMIO_COUNT;
455 header_len_bytes = dmc_header->header_len;
456 start_mmioaddr = DMC_V1_MMIO_START_RANGE;
457 dmc_header_size = sizeof(*v1);
458 } else {
459 drm_err(&i915->drm, "Unknown DMC fw header version: %u\n",
460 dmc_header->header_ver);
461 return 0;
462 }
463
464 if (header_len_bytes != dmc_header_size) {
465 drm_err(&i915->drm, "DMC firmware has wrong dmc header length "
466 "(%u bytes)\n", header_len_bytes);
467 return 0;
468 }
469
470 /* Cache the dmc header info. */
471 if (mmio_count > mmio_count_max) {
472 drm_err(&i915->drm, "DMC firmware has wrong mmio count %u\n", mmio_count);
473 return 0;
474 }
475
> 476 if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
477 drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
478 return 0;
479
480 for (i = 0; i < mmio_count; i++) {
481 dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
482 dmc_info->mmiodata[i] = mmiodata[i];
483 }
484 dmc_info->mmio_count = mmio_count;
485 dmc_info->start_mmioaddr = start_mmioaddr;
486
487 rem_size -= header_len_bytes;
488
489 /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
490 payload_size = dmc_header->fw_size * 4;
491 if (rem_size < payload_size)
492 goto error_truncated;
493
494 if (payload_size > dmc->max_fw_size) {
495 drm_err(&i915->drm, "DMC FW too big (%u bytes)\n", payload_size);
496 return 0;
497 }
498 dmc_info->dmc_fw_size = dmc_header->fw_size;
499
500 dmc_info->payload = kmalloc(payload_size, GFP_KERNEL);
501 if (!dmc_info->payload)
502 return 0;
503
504 payload = (u8 *)(dmc_header) + header_len_bytes;
505 memcpy(dmc_info->payload, payload, payload_size);
506
507 return header_len_bytes + payload_size;
508
509 error_truncated:
510 drm_err(&i915->drm, "Truncated DMC firmware, refusing.\n");
511 return 0;
512 }
513
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-27 4:26 ` kernel test robot
0 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2022-04-27 4:26 UTC (permalink / raw)
To: Anusha Srivatsa, intel-gfx; +Cc: Lucas De Marchi, kbuild-all, stable
Hi Anusha,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip next-20220426]
[cannot apply to v5.18-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220427-084021
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-a012-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271216.6t3eWj4f-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/f79241ea04e8815b3c1b0ab6b9d6136efc8646d3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220427-084021
git checkout f79241ea04e8815b3c1b0ab6b9d6136efc8646d3
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_dmc.c: In function 'parse_dmc_fw_header':
>> drivers/gpu/drm/i915/display/intel_dmc.c:476:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
476 | if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
| ^~
drivers/gpu/drm/i915/display/intel_dmc.c:478:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
478 | return 0;
| ^~~~~~
vim +/if +476 drivers/gpu/drm/i915/display/intel_dmc.c
406
407 static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
408 const struct intel_dmc_header_base *dmc_header,
409 size_t rem_size, u8 dmc_id)
410 {
411 struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
412 struct dmc_fw_info *dmc_info = &dmc->dmc_info[dmc_id];
413 unsigned int header_len_bytes, dmc_header_size, payload_size, i;
414 const u32 *mmioaddr, *mmiodata;
415 u32 mmio_count, mmio_count_max, start_mmioaddr;
416 u8 *payload;
417
418 BUILD_BUG_ON(ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V3_MAX_MMIO_COUNT ||
419 ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V1_MAX_MMIO_COUNT);
420
421 /*
422 * Check if we can access common fields, we will checkc again below
423 * after we have read the version
424 */
425 if (rem_size < sizeof(struct intel_dmc_header_base))
426 goto error_truncated;
427
428 /* Cope with small differences between v1 and v3 */
429 if (dmc_header->header_ver == 3) {
430 const struct intel_dmc_header_v3 *v3 =
431 (const struct intel_dmc_header_v3 *)dmc_header;
432
433 if (rem_size < sizeof(struct intel_dmc_header_v3))
434 goto error_truncated;
435
436 mmioaddr = v3->mmioaddr;
437 mmiodata = v3->mmiodata;
438 mmio_count = v3->mmio_count;
439 mmio_count_max = DMC_V3_MAX_MMIO_COUNT;
440 /* header_len is in dwords */
441 header_len_bytes = dmc_header->header_len * 4;
442 start_mmioaddr = v3->start_mmioaddr;
443 dmc_header_size = sizeof(*v3);
444 } else if (dmc_header->header_ver == 1) {
445 const struct intel_dmc_header_v1 *v1 =
446 (const struct intel_dmc_header_v1 *)dmc_header;
447
448 if (rem_size < sizeof(struct intel_dmc_header_v1))
449 goto error_truncated;
450
451 mmioaddr = v1->mmioaddr;
452 mmiodata = v1->mmiodata;
453 mmio_count = v1->mmio_count;
454 mmio_count_max = DMC_V1_MAX_MMIO_COUNT;
455 header_len_bytes = dmc_header->header_len;
456 start_mmioaddr = DMC_V1_MMIO_START_RANGE;
457 dmc_header_size = sizeof(*v1);
458 } else {
459 drm_err(&i915->drm, "Unknown DMC fw header version: %u\n",
460 dmc_header->header_ver);
461 return 0;
462 }
463
464 if (header_len_bytes != dmc_header_size) {
465 drm_err(&i915->drm, "DMC firmware has wrong dmc header length "
466 "(%u bytes)\n", header_len_bytes);
467 return 0;
468 }
469
470 /* Cache the dmc header info. */
471 if (mmio_count > mmio_count_max) {
472 drm_err(&i915->drm, "DMC firmware has wrong mmio count %u\n", mmio_count);
473 return 0;
474 }
475
> 476 if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
477 drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
478 return 0;
479
480 for (i = 0; i < mmio_count; i++) {
481 dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
482 dmc_info->mmiodata[i] = mmiodata[i];
483 }
484 dmc_info->mmio_count = mmio_count;
485 dmc_info->start_mmioaddr = start_mmioaddr;
486
487 rem_size -= header_len_bytes;
488
489 /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
490 payload_size = dmc_header->fw_size * 4;
491 if (rem_size < payload_size)
492 goto error_truncated;
493
494 if (payload_size > dmc->max_fw_size) {
495 drm_err(&i915->drm, "DMC FW too big (%u bytes)\n", payload_size);
496 return 0;
497 }
498 dmc_info->dmc_fw_size = dmc_header->fw_size;
499
500 dmc_info->payload = kmalloc(payload_size, GFP_KERNEL);
501 if (!dmc_info->payload)
502 return 0;
503
504 payload = (u8 *)(dmc_header) + header_len_bytes;
505 memcpy(dmc_info->payload, payload, payload_size);
506
507 return header_len_bytes + payload_size;
508
509 error_truncated:
510 drm_err(&i915->drm, "Truncated DMC firmware, refusing.\n");
511 return 0;
512 }
513
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-27 0:35 ` Anusha Srivatsa
@ 2022-04-27 5:41 ` Lucas De Marchi
-1 siblings, 0 replies; 34+ messages in thread
From: Lucas De Marchi @ 2022-04-27 5:41 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx, stable
On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
>Bspec has added some steps that check forDMC MMIO range before
>programming them
>
>v2: Fix for CI
>v3: move register defines to .h (Anusha)
>- Check MMIO restrictions per pipe
>- Add MMIO restricton for v1 dmc header as well (Lucas)
>
>BSpec: 49193
>
>Cc: <stable@vger.kernel.org>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
> drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
> 2 files changed, 72 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>index 257cf662f9f4..ac7896835bfa 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
> #define BXT_DMC_MAX_FW_SIZE 0x3000
> MODULE_FIRMWARE(BXT_DMC_PATH);
>
>-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>-#define DMC_V1_MAX_MMIO_COUNT 8
>-#define DMC_V3_MAX_MMIO_COUNT 20
>-#define DMC_V1_MMIO_START_RANGE 0x80000
>-
> struct intel_css_header {
> /* 0x09 for DMC */
> u32 module_type;
>@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
> }
> }
>
>+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
>+ u32 mmio_count, int header_ver, u8 dmc_id)
>+{
>+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
>+ int i;
>+
>+ if (header_ver == 1) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < DMC_MMIO_START_RANGE || mmioaddr[i] > DMC_MMIO_END_RANGE)
>+ return false;
>+ }
return missing here
>+ }
>+
>+ /* Main DMC MMIO check */
>+ if (dmc_id == DMC_FW_MAIN) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>+ return false;
>+ }
>+ }
>+
>+ /* Pipe DMC MMIO check */
>+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START && mmioaddr[i] > ADLP_PIPE_MMIO_END)
>+ return false;
>+ }
for DG2, main should use TGL_DMC_MMIO_START? and then fail here because
of another missing return above?
>+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>+ return false;
this is handling DMC_FW_MAIN twice.
Maybe something like this:
u32 start, end;
if (header_ver == 1) {
start = DMC_MMIO_START_RANGE;
end = DMC_MMIO_END_RANGE;
} else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
start = TGL_DMC_MMIO_START(dmc_id);
end = TGL_DMC_MMIO_END(dmc_id);
} else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
start = ADLP_PIPE_MMIO_START;
end = ADLP_PIPE_MMIO_END;
} else {
drm_warn(&i915->drm, "Unknown mmio range for sanity check");
return false;
}
for (i = 0; i < mmio_count; i++)
if (mmioaddr[i] < start || mmioaddr[i] > end)
return false;
return true;
... untested, and may need tweaks depending on the answer to the
question above on what range to use for ADL-P/DG2 on main DMC.
>+ }
>+ }
>+
>+ return true;
>+}
>+
> static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> const struct intel_dmc_header_base *dmc_header,
> size_t rem_size, u8 dmc_id)
>@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> return 0;
> }
>
>+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
>+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
>+ return 0;
you don't like DMC and decided to fail it for all platforms?
>+
> for (i = 0; i < mmio_count; i++) {
> dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> dmc_info->mmiodata[i] = mmiodata[i];
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>index d65e698832eb..235d1b721334 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>@@ -11,12 +11,43 @@
> #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
> #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
> #define DMC_HTP_ADDR_SKL 0x00500034
>+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> #define DMC_SSP_BASE _MMIO(0x8F074)
> #define DMC_HTP_SKL _MMIO(0x8F004)
> #define DMC_LAST_WRITE _MMIO(0x8F034)
> #define DMC_LAST_WRITE_VALUE 0xc003b400
> #define DMC_MMIO_START_RANGE 0x80000
> #define DMC_MMIO_END_RANGE 0x8FFFF
>+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>+#define DMC_V1_MAX_MMIO_COUNT 8
>+#define DMC_V3_MAX_MMIO_COUNT 20
why are you moving these to _regs? these describe the DMC header/blob
>+#define DMC_V1_MMIO_START_RANGE 0x80000
>+#define _TGL_MAIN_MMIO_START 0x8F000
>+#define _TGL_MAIN_MMIO_END 0x8FFFF
>+#define _TGL_PIPEA_MMIO_START 0x92000
>+#define _TGL_PIPEA_MMIO_END 0x93FFF
>+#define _TGL_PIPEB_MMIO_START 0x96000
>+#define _TGL_PIPEB_MMIO_END 0x97FFF
>+#define _TGL_PIPEC_MMIO_START 0x9A000
>+#define _TGL_PIPEC_MMIO_END 0x9BFFF
>+#define _TGL_PIPED_MMIO_START 0x9E000
>+#define _TGL_PIPED_MMIO_END 0x9FFFF
>+#define ADLP_PIPE_MMIO_START 0x5F000
>+#define ADLP_PIPE_MMIO_END 0x5FFFF
>+
>+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe, _TGL_MAIN_MMIO_START,\
_PICK? Did you miss my previous review?
Lucas De Marchi
>+ _TGL_PIPEA_MMIO_START,\
>+ _TGL_PIPEB_MMIO_START,\
>+ _TGL_PIPEC_MMIO_START,\
>+ _TGL_PIPEB_MMIO_START)
>+
>+#define TGL_DMC_MMIO_END(pipe) _PICK(pipe, _TGL_MAIN_MMIO_END,\
>+ _TGL_PIPEA_MMIO_END,\
>+ _TGL_PIPEB_MMIO_END,\
>+ _TGL_PIPEC_MMIO_END,\
>+ _TGL_PIPEB_MMIO_END)
>+
> #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
>--
>2.25.1
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-27 5:41 ` Lucas De Marchi
0 siblings, 0 replies; 34+ messages in thread
From: Lucas De Marchi @ 2022-04-27 5:41 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx, stable
On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
>Bspec has added some steps that check forDMC MMIO range before
>programming them
>
>v2: Fix for CI
>v3: move register defines to .h (Anusha)
>- Check MMIO restrictions per pipe
>- Add MMIO restricton for v1 dmc header as well (Lucas)
>
>BSpec: 49193
>
>Cc: <stable@vger.kernel.org>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
> drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
> 2 files changed, 72 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>index 257cf662f9f4..ac7896835bfa 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
> #define BXT_DMC_MAX_FW_SIZE 0x3000
> MODULE_FIRMWARE(BXT_DMC_PATH);
>
>-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>-#define DMC_V1_MAX_MMIO_COUNT 8
>-#define DMC_V3_MAX_MMIO_COUNT 20
>-#define DMC_V1_MMIO_START_RANGE 0x80000
>-
> struct intel_css_header {
> /* 0x09 for DMC */
> u32 module_type;
>@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
> }
> }
>
>+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
>+ u32 mmio_count, int header_ver, u8 dmc_id)
>+{
>+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
>+ int i;
>+
>+ if (header_ver == 1) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < DMC_MMIO_START_RANGE || mmioaddr[i] > DMC_MMIO_END_RANGE)
>+ return false;
>+ }
return missing here
>+ }
>+
>+ /* Main DMC MMIO check */
>+ if (dmc_id == DMC_FW_MAIN) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>+ return false;
>+ }
>+ }
>+
>+ /* Pipe DMC MMIO check */
>+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START && mmioaddr[i] > ADLP_PIPE_MMIO_END)
>+ return false;
>+ }
for DG2, main should use TGL_DMC_MMIO_START? and then fail here because
of another missing return above?
>+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id) || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>+ return false;
this is handling DMC_FW_MAIN twice.
Maybe something like this:
u32 start, end;
if (header_ver == 1) {
start = DMC_MMIO_START_RANGE;
end = DMC_MMIO_END_RANGE;
} else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
start = TGL_DMC_MMIO_START(dmc_id);
end = TGL_DMC_MMIO_END(dmc_id);
} else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
start = ADLP_PIPE_MMIO_START;
end = ADLP_PIPE_MMIO_END;
} else {
drm_warn(&i915->drm, "Unknown mmio range for sanity check");
return false;
}
for (i = 0; i < mmio_count; i++)
if (mmioaddr[i] < start || mmioaddr[i] > end)
return false;
return true;
... untested, and may need tweaks depending on the answer to the
question above on what range to use for ADL-P/DG2 on main DMC.
>+ }
>+ }
>+
>+ return true;
>+}
>+
> static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> const struct intel_dmc_header_base *dmc_header,
> size_t rem_size, u8 dmc_id)
>@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> return 0;
> }
>
>+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
>+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
>+ return 0;
you don't like DMC and decided to fail it for all platforms?
>+
> for (i = 0; i < mmio_count; i++) {
> dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> dmc_info->mmiodata[i] = mmiodata[i];
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>index d65e698832eb..235d1b721334 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>@@ -11,12 +11,43 @@
> #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
> #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
> #define DMC_HTP_ADDR_SKL 0x00500034
>+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> #define DMC_SSP_BASE _MMIO(0x8F074)
> #define DMC_HTP_SKL _MMIO(0x8F004)
> #define DMC_LAST_WRITE _MMIO(0x8F034)
> #define DMC_LAST_WRITE_VALUE 0xc003b400
> #define DMC_MMIO_START_RANGE 0x80000
> #define DMC_MMIO_END_RANGE 0x8FFFF
>+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>+#define DMC_V1_MAX_MMIO_COUNT 8
>+#define DMC_V3_MAX_MMIO_COUNT 20
why are you moving these to _regs? these describe the DMC header/blob
>+#define DMC_V1_MMIO_START_RANGE 0x80000
>+#define _TGL_MAIN_MMIO_START 0x8F000
>+#define _TGL_MAIN_MMIO_END 0x8FFFF
>+#define _TGL_PIPEA_MMIO_START 0x92000
>+#define _TGL_PIPEA_MMIO_END 0x93FFF
>+#define _TGL_PIPEB_MMIO_START 0x96000
>+#define _TGL_PIPEB_MMIO_END 0x97FFF
>+#define _TGL_PIPEC_MMIO_START 0x9A000
>+#define _TGL_PIPEC_MMIO_END 0x9BFFF
>+#define _TGL_PIPED_MMIO_START 0x9E000
>+#define _TGL_PIPED_MMIO_END 0x9FFFF
>+#define ADLP_PIPE_MMIO_START 0x5F000
>+#define ADLP_PIPE_MMIO_END 0x5FFFF
>+
>+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe, _TGL_MAIN_MMIO_START,\
_PICK? Did you miss my previous review?
Lucas De Marchi
>+ _TGL_PIPEA_MMIO_START,\
>+ _TGL_PIPEB_MMIO_START,\
>+ _TGL_PIPEC_MMIO_START,\
>+ _TGL_PIPEB_MMIO_START)
>+
>+#define TGL_DMC_MMIO_END(pipe) _PICK(pipe, _TGL_MAIN_MMIO_END,\
>+ _TGL_PIPEA_MMIO_END,\
>+ _TGL_PIPEB_MMIO_END,\
>+ _TGL_PIPEC_MMIO_END,\
>+ _TGL_PIPEB_MMIO_END)
>+
> #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
>--
>2.25.1
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-27 0:35 ` Anusha Srivatsa
@ 2022-04-27 7:49 ` kernel test robot
-1 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2022-04-27 7:49 UTC (permalink / raw)
To: Anusha Srivatsa, intel-gfx; +Cc: Lucas De Marchi, kbuild-all, stable
Hi Anusha,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip next-20220426]
[cannot apply to v5.18-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220427-084021
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20220427/202204271502.BuTprbqW-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/f79241ea04e8815b3c1b0ab6b9d6136efc8646d3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220427-084021
git checkout f79241ea04e8815b3c1b0ab6b9d6136efc8646d3
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_dmc.c: In function 'parse_dmc_fw_header':
>> drivers/gpu/drm/i915/display/intel_dmc.c:476:9: error: this 'if' clause does not guard... [-Werror=misleading-indentation]
476 | if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
| ^~
drivers/gpu/drm/i915/display/intel_dmc.c:478:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
478 | return 0;
| ^~~~~~
cc1: all warnings being treated as errors
vim +/if +476 drivers/gpu/drm/i915/display/intel_dmc.c
406
407 static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
408 const struct intel_dmc_header_base *dmc_header,
409 size_t rem_size, u8 dmc_id)
410 {
411 struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
412 struct dmc_fw_info *dmc_info = &dmc->dmc_info[dmc_id];
413 unsigned int header_len_bytes, dmc_header_size, payload_size, i;
414 const u32 *mmioaddr, *mmiodata;
415 u32 mmio_count, mmio_count_max, start_mmioaddr;
416 u8 *payload;
417
418 BUILD_BUG_ON(ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V3_MAX_MMIO_COUNT ||
419 ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V1_MAX_MMIO_COUNT);
420
421 /*
422 * Check if we can access common fields, we will checkc again below
423 * after we have read the version
424 */
425 if (rem_size < sizeof(struct intel_dmc_header_base))
426 goto error_truncated;
427
428 /* Cope with small differences between v1 and v3 */
429 if (dmc_header->header_ver == 3) {
430 const struct intel_dmc_header_v3 *v3 =
431 (const struct intel_dmc_header_v3 *)dmc_header;
432
433 if (rem_size < sizeof(struct intel_dmc_header_v3))
434 goto error_truncated;
435
436 mmioaddr = v3->mmioaddr;
437 mmiodata = v3->mmiodata;
438 mmio_count = v3->mmio_count;
439 mmio_count_max = DMC_V3_MAX_MMIO_COUNT;
440 /* header_len is in dwords */
441 header_len_bytes = dmc_header->header_len * 4;
442 start_mmioaddr = v3->start_mmioaddr;
443 dmc_header_size = sizeof(*v3);
444 } else if (dmc_header->header_ver == 1) {
445 const struct intel_dmc_header_v1 *v1 =
446 (const struct intel_dmc_header_v1 *)dmc_header;
447
448 if (rem_size < sizeof(struct intel_dmc_header_v1))
449 goto error_truncated;
450
451 mmioaddr = v1->mmioaddr;
452 mmiodata = v1->mmiodata;
453 mmio_count = v1->mmio_count;
454 mmio_count_max = DMC_V1_MAX_MMIO_COUNT;
455 header_len_bytes = dmc_header->header_len;
456 start_mmioaddr = DMC_V1_MMIO_START_RANGE;
457 dmc_header_size = sizeof(*v1);
458 } else {
459 drm_err(&i915->drm, "Unknown DMC fw header version: %u\n",
460 dmc_header->header_ver);
461 return 0;
462 }
463
464 if (header_len_bytes != dmc_header_size) {
465 drm_err(&i915->drm, "DMC firmware has wrong dmc header length "
466 "(%u bytes)\n", header_len_bytes);
467 return 0;
468 }
469
470 /* Cache the dmc header info. */
471 if (mmio_count > mmio_count_max) {
472 drm_err(&i915->drm, "DMC firmware has wrong mmio count %u\n", mmio_count);
473 return 0;
474 }
475
> 476 if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
477 drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
478 return 0;
479
480 for (i = 0; i < mmio_count; i++) {
481 dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
482 dmc_info->mmiodata[i] = mmiodata[i];
483 }
484 dmc_info->mmio_count = mmio_count;
485 dmc_info->start_mmioaddr = start_mmioaddr;
486
487 rem_size -= header_len_bytes;
488
489 /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
490 payload_size = dmc_header->fw_size * 4;
491 if (rem_size < payload_size)
492 goto error_truncated;
493
494 if (payload_size > dmc->max_fw_size) {
495 drm_err(&i915->drm, "DMC FW too big (%u bytes)\n", payload_size);
496 return 0;
497 }
498 dmc_info->dmc_fw_size = dmc_header->fw_size;
499
500 dmc_info->payload = kmalloc(payload_size, GFP_KERNEL);
501 if (!dmc_info->payload)
502 return 0;
503
504 payload = (u8 *)(dmc_header) + header_len_bytes;
505 memcpy(dmc_info->payload, payload, payload_size);
506
507 return header_len_bytes + payload_size;
508
509 error_truncated:
510 drm_err(&i915->drm, "Truncated DMC firmware, refusing.\n");
511 return 0;
512 }
513
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-27 7:49 ` kernel test robot
0 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2022-04-27 7:49 UTC (permalink / raw)
To: Anusha Srivatsa, intel-gfx; +Cc: kbuild-all, Lucas De Marchi, stable
Hi Anusha,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip next-20220426]
[cannot apply to v5.18-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220427-084021
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20220427/202204271502.BuTprbqW-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/f79241ea04e8815b3c1b0ab6b9d6136efc8646d3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220427-084021
git checkout f79241ea04e8815b3c1b0ab6b9d6136efc8646d3
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_dmc.c: In function 'parse_dmc_fw_header':
>> drivers/gpu/drm/i915/display/intel_dmc.c:476:9: error: this 'if' clause does not guard... [-Werror=misleading-indentation]
476 | if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
| ^~
drivers/gpu/drm/i915/display/intel_dmc.c:478:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
478 | return 0;
| ^~~~~~
cc1: all warnings being treated as errors
vim +/if +476 drivers/gpu/drm/i915/display/intel_dmc.c
406
407 static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
408 const struct intel_dmc_header_base *dmc_header,
409 size_t rem_size, u8 dmc_id)
410 {
411 struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
412 struct dmc_fw_info *dmc_info = &dmc->dmc_info[dmc_id];
413 unsigned int header_len_bytes, dmc_header_size, payload_size, i;
414 const u32 *mmioaddr, *mmiodata;
415 u32 mmio_count, mmio_count_max, start_mmioaddr;
416 u8 *payload;
417
418 BUILD_BUG_ON(ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V3_MAX_MMIO_COUNT ||
419 ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V1_MAX_MMIO_COUNT);
420
421 /*
422 * Check if we can access common fields, we will checkc again below
423 * after we have read the version
424 */
425 if (rem_size < sizeof(struct intel_dmc_header_base))
426 goto error_truncated;
427
428 /* Cope with small differences between v1 and v3 */
429 if (dmc_header->header_ver == 3) {
430 const struct intel_dmc_header_v3 *v3 =
431 (const struct intel_dmc_header_v3 *)dmc_header;
432
433 if (rem_size < sizeof(struct intel_dmc_header_v3))
434 goto error_truncated;
435
436 mmioaddr = v3->mmioaddr;
437 mmiodata = v3->mmiodata;
438 mmio_count = v3->mmio_count;
439 mmio_count_max = DMC_V3_MAX_MMIO_COUNT;
440 /* header_len is in dwords */
441 header_len_bytes = dmc_header->header_len * 4;
442 start_mmioaddr = v3->start_mmioaddr;
443 dmc_header_size = sizeof(*v3);
444 } else if (dmc_header->header_ver == 1) {
445 const struct intel_dmc_header_v1 *v1 =
446 (const struct intel_dmc_header_v1 *)dmc_header;
447
448 if (rem_size < sizeof(struct intel_dmc_header_v1))
449 goto error_truncated;
450
451 mmioaddr = v1->mmioaddr;
452 mmiodata = v1->mmiodata;
453 mmio_count = v1->mmio_count;
454 mmio_count_max = DMC_V1_MAX_MMIO_COUNT;
455 header_len_bytes = dmc_header->header_len;
456 start_mmioaddr = DMC_V1_MMIO_START_RANGE;
457 dmc_header_size = sizeof(*v1);
458 } else {
459 drm_err(&i915->drm, "Unknown DMC fw header version: %u\n",
460 dmc_header->header_ver);
461 return 0;
462 }
463
464 if (header_len_bytes != dmc_header_size) {
465 drm_err(&i915->drm, "DMC firmware has wrong dmc header length "
466 "(%u bytes)\n", header_len_bytes);
467 return 0;
468 }
469
470 /* Cache the dmc header info. */
471 if (mmio_count > mmio_count_max) {
472 drm_err(&i915->drm, "DMC firmware has wrong mmio count %u\n", mmio_count);
473 return 0;
474 }
475
> 476 if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
477 drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
478 return 0;
479
480 for (i = 0; i < mmio_count; i++) {
481 dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
482 dmc_info->mmiodata[i] = mmiodata[i];
483 }
484 dmc_info->mmio_count = mmio_count;
485 dmc_info->start_mmioaddr = start_mmioaddr;
486
487 rem_size -= header_len_bytes;
488
489 /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
490 payload_size = dmc_header->fw_size * 4;
491 if (rem_size < payload_size)
492 goto error_truncated;
493
494 if (payload_size > dmc->max_fw_size) {
495 drm_err(&i915->drm, "DMC FW too big (%u bytes)\n", payload_size);
496 return 0;
497 }
498 dmc_info->dmc_fw_size = dmc_header->fw_size;
499
500 dmc_info->payload = kmalloc(payload_size, GFP_KERNEL);
501 if (!dmc_info->payload)
502 return 0;
503
504 payload = (u8 *)(dmc_header) + header_len_bytes;
505 memcpy(dmc_info->payload, payload, payload_size);
506
507 return header_len_bytes + payload_size;
508
509 error_truncated:
510 drm_err(&i915->drm, "Truncated DMC firmware, refusing.\n");
511 return 0;
512 }
513
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-27 0:35 ` Anusha Srivatsa
` (5 preceding siblings ...)
(?)
@ 2022-04-27 12:42 ` Andi Shyti
-1 siblings, 0 replies; 34+ messages in thread
From: Andi Shyti @ 2022-04-27 12:42 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx, Lucas De Marchi, stable
[...]
> + if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id))
> + drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
> + return 0;
> +
mh? :)
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-27 5:41 ` [Intel-gfx] " Lucas De Marchi
@ 2022-04-29 20:39 ` Srivatsa, Anusha
-1 siblings, 0 replies; 34+ messages in thread
From: Srivatsa, Anusha @ 2022-04-29 20:39 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx, stable
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Tuesday, April 26, 2022 10:42 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
> >Bspec has added some steps that check forDMC MMIO range before
> >programming them
> >
> >v2: Fix for CI
> >v3: move register defines to .h (Anusha)
> >- Check MMIO restrictions per pipe
> >- Add MMIO restricton for v1 dmc header as well (Lucas)
> >
> >BSpec: 49193
> >
> >Cc: <stable@vger.kernel.org>
> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >---
> > drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
> > 2 files changed, 72 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >index 257cf662f9f4..ac7896835bfa 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
> > #define BXT_DMC_MAX_FW_SIZE 0x3000
> > MODULE_FIRMWARE(BXT_DMC_PATH);
> >
> >-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> >-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >-#define DMC_V1_MAX_MMIO_COUNT 8
> >-#define DMC_V3_MAX_MMIO_COUNT 20
> >-#define DMC_V1_MMIO_START_RANGE 0x80000
> >-
> > struct intel_css_header {
> > /* 0x09 for DMC */
> > u32 module_type;
> >@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
> *dmc,
> > }
> > }
> >
> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> u32 *mmioaddr,
> >+ u32 mmio_count, int header_ver, u8
> dmc_id) {
> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> dmc);
> >+ int i;
> >+
> >+ if (header_ver == 1) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
> mmioaddr[i] > DMC_MMIO_END_RANGE)
> >+ return false;
> >+ }
>
> return missing here
>
> >+ }
> >+
> >+ /* Main DMC MMIO check */
> >+ if (dmc_id == DMC_FW_MAIN) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >+ return false;
> >+ }
> >+ }
> >+
> >+ /* Pipe DMC MMIO check */
> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
> mmioaddr[i] > ADLP_PIPE_MMIO_END)
> >+ return false;
> >+ }
>
> for DG2, main should use TGL_DMC_MMIO_START? and then fail here
> because of another missing return above?
>
> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> IS_ALDERLAKE_S(i915)) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >+ return false;
>
> this is handling DMC_FW_MAIN twice.
>
>
> Maybe something like this:
>
> u32 start, end;
>
> if (header_ver == 1) {
> start = DMC_MMIO_START_RANGE;
> end = DMC_MMIO_END_RANGE;
> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
> IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
> start = TGL_DMC_MMIO_START(dmc_id);
> end = TGL_DMC_MMIO_END(dmc_id);
> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> start = ADLP_PIPE_MMIO_START;
> end = ADLP_PIPE_MMIO_END;
> } else {
> drm_warn(&i915->drm, "Unknown mmio range for sanity
> check");
> return false;
> }
>
> for (i = 0; i < mmio_count; i++)
> if (mmioaddr[i] < start || mmioaddr[i] > end)
> return false;
>
> return true;
>
>
> ... untested, and may need tweaks depending on the answer to the question
> above on what range to use for ADL-P/DG2 on main DMC.
The above approach is definitely neater.
The main DMC offset is the same for all platforms.
> >+ }
> >+ }
> >+
> >+ return true;
> >+}
> >+
> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> > const struct intel_dmc_header_base
> *dmc_header,
> > size_t rem_size, u8 dmc_id)
> >@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc
> *dmc,
> > return 0;
> > }
> >
> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
> dmc_header->header_ver, dmc_id))
> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
> Addresses\n");
> >+ return 0;
>
> you don't like DMC and decided to fail it for all platforms?
<facepalm>
> >+
> > for (i = 0; i < mmio_count; i++) {
> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >index d65e698832eb..235d1b721334 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >@@ -11,12 +11,43 @@
> > #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
> > #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
> > #define DMC_HTP_ADDR_SKL 0x00500034
> >+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> > #define DMC_SSP_BASE _MMIO(0x8F074)
> > #define DMC_HTP_SKL _MMIO(0x8F004)
> > #define DMC_LAST_WRITE _MMIO(0x8F034)
> > #define DMC_LAST_WRITE_VALUE 0xc003b400
> > #define DMC_MMIO_START_RANGE 0x80000
> > #define DMC_MMIO_END_RANGE 0x8FFFF
> >+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >+#define DMC_V1_MAX_MMIO_COUNT 8
> >+#define DMC_V3_MAX_MMIO_COUNT 20
>
>
> why are you moving these to _regs? these describe the DMC header/blob
Yeah my mistake. While making the change they seemed like the right thing to do.
>
> >+#define DMC_V1_MMIO_START_RANGE 0x80000
> >+#define _TGL_MAIN_MMIO_START 0x8F000
> >+#define _TGL_MAIN_MMIO_END 0x8FFFF
> >+#define _TGL_PIPEA_MMIO_START 0x92000
> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
> >+#define _TGL_PIPEB_MMIO_START 0x96000
> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
> >+#define _TGL_PIPEC_MMIO_START 0x9A000
> >+#define _TGL_PIPEC_MMIO_END 0x9BFFF
> >+#define _TGL_PIPED_MMIO_START 0x9E000
> >+#define _TGL_PIPED_MMIO_END 0x9FFFF
> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
> >+
> >+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
> _TGL_MAIN_MMIO_START,\
>
> _PICK? Did you miss my previous review?
No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So using PICK_EVEN is not the right choice here. We also don't need to do _MMIO really.....
Unless I am missing something, this seems like the right approach.
Anusha
> Lucas De Marchi
>
> >+ _TGL_PIPEA_MMIO_START,\
> >+ _TGL_PIPEB_MMIO_START,\
> >+ _TGL_PIPEC_MMIO_START,\
> >+ _TGL_PIPEB_MMIO_START)
> >+
> >+#define TGL_DMC_MMIO_END(pipe) _PICK(pipe,
> _TGL_MAIN_MMIO_END,\
> >+ _TGL_PIPEA_MMIO_END,\
> >+ _TGL_PIPEB_MMIO_END,\
> >+ _TGL_PIPEC_MMIO_END,\
> >+ _TGL_PIPEB_MMIO_END)
> >+
> > #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> > #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> > #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
> >--
> >2.25.1
> >
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-29 20:39 ` Srivatsa, Anusha
0 siblings, 0 replies; 34+ messages in thread
From: Srivatsa, Anusha @ 2022-04-29 20:39 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx, stable
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Tuesday, April 26, 2022 10:42 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
> >Bspec has added some steps that check forDMC MMIO range before
> >programming them
> >
> >v2: Fix for CI
> >v3: move register defines to .h (Anusha)
> >- Check MMIO restrictions per pipe
> >- Add MMIO restricton for v1 dmc header as well (Lucas)
> >
> >BSpec: 49193
> >
> >Cc: <stable@vger.kernel.org>
> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >---
> > drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
> > 2 files changed, 72 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >index 257cf662f9f4..ac7896835bfa 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
> > #define BXT_DMC_MAX_FW_SIZE 0x3000
> > MODULE_FIRMWARE(BXT_DMC_PATH);
> >
> >-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> >-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >-#define DMC_V1_MAX_MMIO_COUNT 8
> >-#define DMC_V3_MAX_MMIO_COUNT 20
> >-#define DMC_V1_MMIO_START_RANGE 0x80000
> >-
> > struct intel_css_header {
> > /* 0x09 for DMC */
> > u32 module_type;
> >@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
> *dmc,
> > }
> > }
> >
> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> u32 *mmioaddr,
> >+ u32 mmio_count, int header_ver, u8
> dmc_id) {
> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> dmc);
> >+ int i;
> >+
> >+ if (header_ver == 1) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
> mmioaddr[i] > DMC_MMIO_END_RANGE)
> >+ return false;
> >+ }
>
> return missing here
>
> >+ }
> >+
> >+ /* Main DMC MMIO check */
> >+ if (dmc_id == DMC_FW_MAIN) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >+ return false;
> >+ }
> >+ }
> >+
> >+ /* Pipe DMC MMIO check */
> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
> mmioaddr[i] > ADLP_PIPE_MMIO_END)
> >+ return false;
> >+ }
>
> for DG2, main should use TGL_DMC_MMIO_START? and then fail here
> because of another missing return above?
>
> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> IS_ALDERLAKE_S(i915)) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >+ return false;
>
> this is handling DMC_FW_MAIN twice.
>
>
> Maybe something like this:
>
> u32 start, end;
>
> if (header_ver == 1) {
> start = DMC_MMIO_START_RANGE;
> end = DMC_MMIO_END_RANGE;
> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
> IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
> start = TGL_DMC_MMIO_START(dmc_id);
> end = TGL_DMC_MMIO_END(dmc_id);
> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> start = ADLP_PIPE_MMIO_START;
> end = ADLP_PIPE_MMIO_END;
> } else {
> drm_warn(&i915->drm, "Unknown mmio range for sanity
> check");
> return false;
> }
>
> for (i = 0; i < mmio_count; i++)
> if (mmioaddr[i] < start || mmioaddr[i] > end)
> return false;
>
> return true;
>
>
> ... untested, and may need tweaks depending on the answer to the question
> above on what range to use for ADL-P/DG2 on main DMC.
The above approach is definitely neater.
The main DMC offset is the same for all platforms.
> >+ }
> >+ }
> >+
> >+ return true;
> >+}
> >+
> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> > const struct intel_dmc_header_base
> *dmc_header,
> > size_t rem_size, u8 dmc_id)
> >@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc
> *dmc,
> > return 0;
> > }
> >
> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
> dmc_header->header_ver, dmc_id))
> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
> Addresses\n");
> >+ return 0;
>
> you don't like DMC and decided to fail it for all platforms?
<facepalm>
> >+
> > for (i = 0; i < mmio_count; i++) {
> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >index d65e698832eb..235d1b721334 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >@@ -11,12 +11,43 @@
> > #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
> > #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
> > #define DMC_HTP_ADDR_SKL 0x00500034
> >+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> > #define DMC_SSP_BASE _MMIO(0x8F074)
> > #define DMC_HTP_SKL _MMIO(0x8F004)
> > #define DMC_LAST_WRITE _MMIO(0x8F034)
> > #define DMC_LAST_WRITE_VALUE 0xc003b400
> > #define DMC_MMIO_START_RANGE 0x80000
> > #define DMC_MMIO_END_RANGE 0x8FFFF
> >+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >+#define DMC_V1_MAX_MMIO_COUNT 8
> >+#define DMC_V3_MAX_MMIO_COUNT 20
>
>
> why are you moving these to _regs? these describe the DMC header/blob
Yeah my mistake. While making the change they seemed like the right thing to do.
>
> >+#define DMC_V1_MMIO_START_RANGE 0x80000
> >+#define _TGL_MAIN_MMIO_START 0x8F000
> >+#define _TGL_MAIN_MMIO_END 0x8FFFF
> >+#define _TGL_PIPEA_MMIO_START 0x92000
> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
> >+#define _TGL_PIPEB_MMIO_START 0x96000
> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
> >+#define _TGL_PIPEC_MMIO_START 0x9A000
> >+#define _TGL_PIPEC_MMIO_END 0x9BFFF
> >+#define _TGL_PIPED_MMIO_START 0x9E000
> >+#define _TGL_PIPED_MMIO_END 0x9FFFF
> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
> >+
> >+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
> _TGL_MAIN_MMIO_START,\
>
> _PICK? Did you miss my previous review?
No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So using PICK_EVEN is not the right choice here. We also don't need to do _MMIO really.....
Unless I am missing something, this seems like the right approach.
Anusha
> Lucas De Marchi
>
> >+ _TGL_PIPEA_MMIO_START,\
> >+ _TGL_PIPEB_MMIO_START,\
> >+ _TGL_PIPEC_MMIO_START,\
> >+ _TGL_PIPEB_MMIO_START)
> >+
> >+#define TGL_DMC_MMIO_END(pipe) _PICK(pipe,
> _TGL_MAIN_MMIO_END,\
> >+ _TGL_PIPEA_MMIO_END,\
> >+ _TGL_PIPEB_MMIO_END,\
> >+ _TGL_PIPEC_MMIO_END,\
> >+ _TGL_PIPEB_MMIO_END)
> >+
> > #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> > #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> > #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
> >--
> >2.25.1
> >
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-29 20:39 ` Srivatsa, Anusha
@ 2022-04-29 20:49 ` Lucas De Marchi
-1 siblings, 0 replies; 34+ messages in thread
From: Lucas De Marchi @ 2022-04-29 20:49 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx, stable
On Fri, Apr 29, 2022 at 01:39:03PM -0700, Anusha Srivatsa wrote:
>
>
>> -----Original Message-----
>> From: De Marchi, Lucas <lucas.demarchi@intel.com>
>> Sent: Tuesday, April 26, 2022 10:42 PM
>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
>> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>>
>> On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
>> >Bspec has added some steps that check forDMC MMIO range before
>> >programming them
>> >
>> >v2: Fix for CI
>> >v3: move register defines to .h (Anusha)
>> >- Check MMIO restrictions per pipe
>> >- Add MMIO restricton for v1 dmc header as well (Lucas)
>> >
>> >BSpec: 49193
>> >
>> >Cc: <stable@vger.kernel.org>
>> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> >---
>> > drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
>> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
>> > 2 files changed, 72 insertions(+), 7 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >index 257cf662f9f4..ac7896835bfa 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
>> > #define BXT_DMC_MAX_FW_SIZE 0x3000
>> > MODULE_FIRMWARE(BXT_DMC_PATH);
>> >
>> >-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>> >-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>> >-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>> >-#define DMC_V1_MAX_MMIO_COUNT 8
>> >-#define DMC_V3_MAX_MMIO_COUNT 20
>> >-#define DMC_V1_MMIO_START_RANGE 0x80000
>> >-
>> > struct intel_css_header {
>> > /* 0x09 for DMC */
>> > u32 module_type;
>> >@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
>> *dmc,
>> > }
>> > }
>> >
>> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
>> u32 *mmioaddr,
>> >+ u32 mmio_count, int header_ver, u8
>> dmc_id) {
>> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
>> dmc);
>> >+ int i;
>> >+
>> >+ if (header_ver == 1) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
>> mmioaddr[i] > DMC_MMIO_END_RANGE)
>> >+ return false;
>> >+ }
>>
>> return missing here
>>
>> >+ }
>> >+
>> >+ /* Main DMC MMIO check */
>> >+ if (dmc_id == DMC_FW_MAIN) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
>> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>> >+ return false;
>> >+ }
>> >+ }
>> >+
>> >+ /* Pipe DMC MMIO check */
>> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
>> mmioaddr[i] > ADLP_PIPE_MMIO_END)
>> >+ return false;
>> >+ }
>>
>> for DG2, main should use TGL_DMC_MMIO_START? and then fail here
>> because of another missing return above?
>>
>> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
>> IS_ALDERLAKE_S(i915)) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
>> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>> >+ return false;
>>
>> this is handling DMC_FW_MAIN twice.
>>
>>
>> Maybe something like this:
>>
>> u32 start, end;
>>
>> if (header_ver == 1) {
>> start = DMC_MMIO_START_RANGE;
>> end = DMC_MMIO_END_RANGE;
>> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
>> IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
>> start = TGL_DMC_MMIO_START(dmc_id);
>> end = TGL_DMC_MMIO_END(dmc_id);
>> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>> start = ADLP_PIPE_MMIO_START;
>> end = ADLP_PIPE_MMIO_END;
>> } else {
>> drm_warn(&i915->drm, "Unknown mmio range for sanity
>> check");
>> return false;
>> }
>>
>> for (i = 0; i < mmio_count; i++)
>> if (mmioaddr[i] < start || mmioaddr[i] > end)
>> return false;
>>
>> return true;
>>
>>
>> ... untested, and may need tweaks depending on the answer to the question
>> above on what range to use for ADL-P/DG2 on main DMC.
>The above approach is definitely neater.
>The main DMC offset is the same for all platforms.
>
>> >+ }
>> >+ }
>> >+
>> >+ return true;
>> >+}
>> >+
>> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
>> > const struct intel_dmc_header_base
>> *dmc_header,
>> > size_t rem_size, u8 dmc_id)
>> >@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc
>> *dmc,
>> > return 0;
>> > }
>> >
>> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
>> dmc_header->header_ver, dmc_id))
>> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
>> Addresses\n");
>> >+ return 0;
>>
>> you don't like DMC and decided to fail it for all platforms?
>
><facepalm>
>
>> >+
>> > for (i = 0; i < mmio_count; i++) {
>> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
>> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
>> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >index d65e698832eb..235d1b721334 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >@@ -11,12 +11,43 @@
>> > #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
>> > #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
>> > #define DMC_HTP_ADDR_SKL 0x00500034
>> >+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>> > #define DMC_SSP_BASE _MMIO(0x8F074)
>> > #define DMC_HTP_SKL _MMIO(0x8F004)
>> > #define DMC_LAST_WRITE _MMIO(0x8F034)
>> > #define DMC_LAST_WRITE_VALUE 0xc003b400
>> > #define DMC_MMIO_START_RANGE 0x80000
>> > #define DMC_MMIO_END_RANGE 0x8FFFF
>> >+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>> >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>> >+#define DMC_V1_MAX_MMIO_COUNT 8
>> >+#define DMC_V3_MAX_MMIO_COUNT 20
>>
>>
>> why are you moving these to _regs? these describe the DMC header/blob
>
>Yeah my mistake. While making the change they seemed like the right thing to do.
>
>>
>> >+#define DMC_V1_MMIO_START_RANGE 0x80000
>> >+#define _TGL_MAIN_MMIO_START 0x8F000
>> >+#define _TGL_MAIN_MMIO_END 0x8FFFF
>> >+#define _TGL_PIPEA_MMIO_START 0x92000
>> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
>> >+#define _TGL_PIPEB_MMIO_START 0x96000
>> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
>> >+#define _TGL_PIPEC_MMIO_START 0x9A000
>> >+#define _TGL_PIPEC_MMIO_END 0x9BFFF
>> >+#define _TGL_PIPED_MMIO_START 0x9E000
>> >+#define _TGL_PIPED_MMIO_END 0x9FFFF
>> >+#define ADLP_PIPE_MMIO_START 0x5F000
>> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
>> >+
>> >+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
>> _TGL_MAIN_MMIO_START,\
>>
>> _PICK? Did you miss my previous review?
>
>No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So using PICK_EVEN is not the right choice here. We also don't need to do _MMIO really.....
>Unless I am missing something, this seems like the right approach.
Because the name you chose for your variable:
TGL_DMC_MMIO_START(pipe) _PICK(pipe,
I was expecting this to be used only with the pipe DMC address, which
are evenly spaced. It seems the argument you're expecting here is a
dmc_id. But.... you said:
>The main DMC offset is the same for all platforms.
So, maybe just handle that separately and keep using pipe here? Then you
can switch to _PICK_EVEN()
Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-29 20:49 ` Lucas De Marchi
0 siblings, 0 replies; 34+ messages in thread
From: Lucas De Marchi @ 2022-04-29 20:49 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx, stable
On Fri, Apr 29, 2022 at 01:39:03PM -0700, Anusha Srivatsa wrote:
>
>
>> -----Original Message-----
>> From: De Marchi, Lucas <lucas.demarchi@intel.com>
>> Sent: Tuesday, April 26, 2022 10:42 PM
>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
>> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>>
>> On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
>> >Bspec has added some steps that check forDMC MMIO range before
>> >programming them
>> >
>> >v2: Fix for CI
>> >v3: move register defines to .h (Anusha)
>> >- Check MMIO restrictions per pipe
>> >- Add MMIO restricton for v1 dmc header as well (Lucas)
>> >
>> >BSpec: 49193
>> >
>> >Cc: <stable@vger.kernel.org>
>> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> >---
>> > drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
>> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
>> > 2 files changed, 72 insertions(+), 7 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >index 257cf662f9f4..ac7896835bfa 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
>> > #define BXT_DMC_MAX_FW_SIZE 0x3000
>> > MODULE_FIRMWARE(BXT_DMC_PATH);
>> >
>> >-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>> >-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>> >-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>> >-#define DMC_V1_MAX_MMIO_COUNT 8
>> >-#define DMC_V3_MAX_MMIO_COUNT 20
>> >-#define DMC_V1_MMIO_START_RANGE 0x80000
>> >-
>> > struct intel_css_header {
>> > /* 0x09 for DMC */
>> > u32 module_type;
>> >@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
>> *dmc,
>> > }
>> > }
>> >
>> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
>> u32 *mmioaddr,
>> >+ u32 mmio_count, int header_ver, u8
>> dmc_id) {
>> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
>> dmc);
>> >+ int i;
>> >+
>> >+ if (header_ver == 1) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
>> mmioaddr[i] > DMC_MMIO_END_RANGE)
>> >+ return false;
>> >+ }
>>
>> return missing here
>>
>> >+ }
>> >+
>> >+ /* Main DMC MMIO check */
>> >+ if (dmc_id == DMC_FW_MAIN) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
>> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>> >+ return false;
>> >+ }
>> >+ }
>> >+
>> >+ /* Pipe DMC MMIO check */
>> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
>> mmioaddr[i] > ADLP_PIPE_MMIO_END)
>> >+ return false;
>> >+ }
>>
>> for DG2, main should use TGL_DMC_MMIO_START? and then fail here
>> because of another missing return above?
>>
>> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
>> IS_ALDERLAKE_S(i915)) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
>> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>> >+ return false;
>>
>> this is handling DMC_FW_MAIN twice.
>>
>>
>> Maybe something like this:
>>
>> u32 start, end;
>>
>> if (header_ver == 1) {
>> start = DMC_MMIO_START_RANGE;
>> end = DMC_MMIO_END_RANGE;
>> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
>> IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
>> start = TGL_DMC_MMIO_START(dmc_id);
>> end = TGL_DMC_MMIO_END(dmc_id);
>> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>> start = ADLP_PIPE_MMIO_START;
>> end = ADLP_PIPE_MMIO_END;
>> } else {
>> drm_warn(&i915->drm, "Unknown mmio range for sanity
>> check");
>> return false;
>> }
>>
>> for (i = 0; i < mmio_count; i++)
>> if (mmioaddr[i] < start || mmioaddr[i] > end)
>> return false;
>>
>> return true;
>>
>>
>> ... untested, and may need tweaks depending on the answer to the question
>> above on what range to use for ADL-P/DG2 on main DMC.
>The above approach is definitely neater.
>The main DMC offset is the same for all platforms.
>
>> >+ }
>> >+ }
>> >+
>> >+ return true;
>> >+}
>> >+
>> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
>> > const struct intel_dmc_header_base
>> *dmc_header,
>> > size_t rem_size, u8 dmc_id)
>> >@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc
>> *dmc,
>> > return 0;
>> > }
>> >
>> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
>> dmc_header->header_ver, dmc_id))
>> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
>> Addresses\n");
>> >+ return 0;
>>
>> you don't like DMC and decided to fail it for all platforms?
>
><facepalm>
>
>> >+
>> > for (i = 0; i < mmio_count; i++) {
>> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
>> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
>> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >index d65e698832eb..235d1b721334 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>> >@@ -11,12 +11,43 @@
>> > #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
>> > #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
>> > #define DMC_HTP_ADDR_SKL 0x00500034
>> >+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>> > #define DMC_SSP_BASE _MMIO(0x8F074)
>> > #define DMC_HTP_SKL _MMIO(0x8F004)
>> > #define DMC_LAST_WRITE _MMIO(0x8F034)
>> > #define DMC_LAST_WRITE_VALUE 0xc003b400
>> > #define DMC_MMIO_START_RANGE 0x80000
>> > #define DMC_MMIO_END_RANGE 0x8FFFF
>> >+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>> >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>> >+#define DMC_V1_MAX_MMIO_COUNT 8
>> >+#define DMC_V3_MAX_MMIO_COUNT 20
>>
>>
>> why are you moving these to _regs? these describe the DMC header/blob
>
>Yeah my mistake. While making the change they seemed like the right thing to do.
>
>>
>> >+#define DMC_V1_MMIO_START_RANGE 0x80000
>> >+#define _TGL_MAIN_MMIO_START 0x8F000
>> >+#define _TGL_MAIN_MMIO_END 0x8FFFF
>> >+#define _TGL_PIPEA_MMIO_START 0x92000
>> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
>> >+#define _TGL_PIPEB_MMIO_START 0x96000
>> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
>> >+#define _TGL_PIPEC_MMIO_START 0x9A000
>> >+#define _TGL_PIPEC_MMIO_END 0x9BFFF
>> >+#define _TGL_PIPED_MMIO_START 0x9E000
>> >+#define _TGL_PIPED_MMIO_END 0x9FFFF
>> >+#define ADLP_PIPE_MMIO_START 0x5F000
>> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
>> >+
>> >+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
>> _TGL_MAIN_MMIO_START,\
>>
>> _PICK? Did you miss my previous review?
>
>No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So using PICK_EVEN is not the right choice here. We also don't need to do _MMIO really.....
>Unless I am missing something, this seems like the right approach.
Because the name you chose for your variable:
TGL_DMC_MMIO_START(pipe) _PICK(pipe,
I was expecting this to be used only with the pipe DMC address, which
are evenly spaced. It seems the argument you're expecting here is a
dmc_id. But.... you said:
>The main DMC offset is the same for all platforms.
So, maybe just handle that separately and keep using pipe here? Then you
can switch to _PICK_EVEN()
Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* RE: [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-29 20:49 ` [Intel-gfx] " Lucas De Marchi
@ 2022-04-29 22:57 ` Srivatsa, Anusha
-1 siblings, 0 replies; 34+ messages in thread
From: Srivatsa, Anusha @ 2022-04-29 22:57 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx, stable
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Friday, April 29, 2022 1:50 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Fri, Apr 29, 2022 at 01:39:03PM -0700, Anusha Srivatsa wrote:
> >
> >
> >> -----Original Message-----
> >> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> >> Sent: Tuesday, April 26, 2022 10:42 PM
> >> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> >> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> >> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
> >>
> >> On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
> >> >Bspec has added some steps that check forDMC MMIO range before
> >> >programming them
> >> >
> >> >v2: Fix for CI
> >> >v3: move register defines to .h (Anusha)
> >> >- Check MMIO restrictions per pipe
> >> >- Add MMIO restricton for v1 dmc header as well (Lucas)
> >> >
> >> >BSpec: 49193
> >> >
> >> >Cc: <stable@vger.kernel.org>
> >> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >> >---
> >> > drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
> >> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
> >> > 2 files changed, 72 insertions(+), 7 deletions(-)
> >> >
> >> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >index 257cf662f9f4..ac7896835bfa 100644
> >> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
> >> > #define BXT_DMC_MAX_FW_SIZE 0x3000
> >> > MODULE_FIRMWARE(BXT_DMC_PATH);
> >> >
> >> >-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> >> >-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >> >-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >> >-#define DMC_V1_MAX_MMIO_COUNT 8
> >> >-#define DMC_V3_MAX_MMIO_COUNT 20
> >> >-#define DMC_V1_MMIO_START_RANGE 0x80000
> >> >-
> >> > struct intel_css_header {
> >> > /* 0x09 for DMC */
> >> > u32 module_type;
> >> >@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
> >> *dmc,
> >> > }
> >> > }
> >> >
> >> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> >> u32 *mmioaddr,
> >> >+ u32 mmio_count, int header_ver, u8
> >> dmc_id) {
> >> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> >> dmc);
> >> >+ int i;
> >> >+
> >> >+ if (header_ver == 1) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
> >> mmioaddr[i] > DMC_MMIO_END_RANGE)
> >> >+ return false;
> >> >+ }
> >>
> >> return missing here
> >>
> >> >+ }
> >> >+
> >> >+ /* Main DMC MMIO check */
> >> >+ if (dmc_id == DMC_FW_MAIN) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> >> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >> >+ return false;
> >> >+ }
> >> >+ }
> >> >+
> >> >+ /* Pipe DMC MMIO check */
> >> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
> >> mmioaddr[i] > ADLP_PIPE_MMIO_END)
> >> >+ return false;
> >> >+ }
> >>
> >> for DG2, main should use TGL_DMC_MMIO_START? and then fail here
> >> because of another missing return above?
> >>
> >> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> >> IS_ALDERLAKE_S(i915)) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> >> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >> >+ return false;
> >>
> >> this is handling DMC_FW_MAIN twice.
> >>
> >>
> >> Maybe something like this:
> >>
> >> u32 start, end;
> >>
> >> if (header_ver == 1) {
> >> start = DMC_MMIO_START_RANGE;
> >> end = DMC_MMIO_END_RANGE;
> >> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
> >> IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
> >> start = TGL_DMC_MMIO_START(dmc_id);
> >> end = TGL_DMC_MMIO_END(dmc_id);
> >> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >> start = ADLP_PIPE_MMIO_START;
> >> end = ADLP_PIPE_MMIO_END;
> >> } else {
> >> drm_warn(&i915->drm, "Unknown mmio range for sanity
> check");
> >> return false;
> >> }
> >>
> >> for (i = 0; i < mmio_count; i++)
> >> if (mmioaddr[i] < start || mmioaddr[i] > end)
> >> return false;
> >>
> >> return true;
> >>
> >>
> >> ... untested, and may need tweaks depending on the answer to the
> >> question above on what range to use for ADL-P/DG2 on main DMC.
> >The above approach is definitely neater.
> >The main DMC offset is the same for all platforms.
> >
> >> >+ }
> >> >+ }
> >> >+
> >> >+ return true;
> >> >+}
> >> >+
> >> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> >> > const struct intel_dmc_header_base
> >> *dmc_header,
> >> > size_t rem_size, u8 dmc_id) @@ -443,6 +473,10
> @@ static
> >> >u32 parse_dmc_fw_header(struct intel_dmc
> >> *dmc,
> >> > return 0;
> >> > }
> >> >
> >> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
> >> dmc_header->header_ver, dmc_id))
> >> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
> >> Addresses\n");
> >> >+ return 0;
> >>
> >> you don't like DMC and decided to fail it for all platforms?
> >
> ><facepalm>
> >
> >> >+
> >> > for (i = 0; i < mmio_count; i++) {
> >> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> >> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
> >> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >index d65e698832eb..235d1b721334 100644
> >> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >@@ -11,12 +11,43 @@
> >> > #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
> >> > #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
> >> > #define DMC_HTP_ADDR_SKL 0x00500034
> >> >+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> >> > #define DMC_SSP_BASE _MMIO(0x8F074)
> >> > #define DMC_HTP_SKL _MMIO(0x8F004)
> >> > #define DMC_LAST_WRITE _MMIO(0x8F034)
> >> > #define DMC_LAST_WRITE_VALUE 0xc003b400
> >> > #define DMC_MMIO_START_RANGE 0x80000
> >> > #define DMC_MMIO_END_RANGE 0x8FFFF
> >> >+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >> >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >> >+#define DMC_V1_MAX_MMIO_COUNT 8
> >> >+#define DMC_V3_MAX_MMIO_COUNT 20
> >>
> >>
> >> why are you moving these to _regs? these describe the DMC
> >> header/blob
> >
> >Yeah my mistake. While making the change they seemed like the right thing
> to do.
> >
> >>
> >> >+#define DMC_V1_MMIO_START_RANGE 0x80000
> >> >+#define _TGL_MAIN_MMIO_START 0x8F000
> >> >+#define _TGL_MAIN_MMIO_END 0x8FFFF
> >> >+#define _TGL_PIPEA_MMIO_START 0x92000
> >> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
> >> >+#define _TGL_PIPEB_MMIO_START 0x96000
> >> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
> >> >+#define _TGL_PIPEC_MMIO_START 0x9A000
> >> >+#define _TGL_PIPEC_MMIO_END 0x9BFFF
> >> >+#define _TGL_PIPED_MMIO_START 0x9E000
> >> >+#define _TGL_PIPED_MMIO_END 0x9FFFF
> >> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
> >> >+
> >> >+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
> >> _TGL_MAIN_MMIO_START,\
> >>
> >> _PICK? Did you miss my previous review?
> >
> >No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So
> using PICK_EVEN is not the right choice here. We also don't need to do
> _MMIO really.....
> >Unless I am missing something, this seems like the right approach.
>
> Because the name you chose for your variable:
>
> TGL_DMC_MMIO_START(pipe) _PICK(pipe,
>
> I was expecting this to be used only with the pipe DMC address, which are
> evenly spaced. It seems the argument you're expecting here is a dmc_id.
Ah, yes. I see the confusion now. It is expecting the dmc_id ,yes. In the usage of the macro in the beginning of the patch though, I am using dmc_id. Thought that would make it clearer, still pipe was the wrong choice of var name.
> But.... you said:
>
> >The main DMC offset is the same for all platforms.
>
> So, maybe just handle that separately and keep using pipe here? Then you
> can switch to _PICK_EVEN()
While the Pipe DMC s are evenly spaced and _PICK_EVEN is the right choice. The dmc_id for PipeA, PipeB ....will be 1,2....and not 0,1,2 so the helper will return the wrong values.
But you are suggesting to use PIPE_A, PIPE_B etc which will be 0 indexed. But here in the code we are parsing the dmc binary to see if it has Pipe DMC and if so what the MMIO offsets are they in and if it is a valid blob or not. The data we can use at this point is the dmc_id...... Unless we do a conversion from dmc-id to the pipe:
If (DMC_FW_PIPEA)
TGL_DMC_MMIO_START(PIPE_A) _PICK(
That will lead to individual such conditions per pipe.
Anusha
> Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-29 22:57 ` Srivatsa, Anusha
0 siblings, 0 replies; 34+ messages in thread
From: Srivatsa, Anusha @ 2022-04-29 22:57 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx, stable
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Friday, April 29, 2022 1:50 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Fri, Apr 29, 2022 at 01:39:03PM -0700, Anusha Srivatsa wrote:
> >
> >
> >> -----Original Message-----
> >> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> >> Sent: Tuesday, April 26, 2022 10:42 PM
> >> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> >> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> >> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
> >>
> >> On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
> >> >Bspec has added some steps that check forDMC MMIO range before
> >> >programming them
> >> >
> >> >v2: Fix for CI
> >> >v3: move register defines to .h (Anusha)
> >> >- Check MMIO restrictions per pipe
> >> >- Add MMIO restricton for v1 dmc header as well (Lucas)
> >> >
> >> >BSpec: 49193
> >> >
> >> >Cc: <stable@vger.kernel.org>
> >> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >> >---
> >> > drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
> >> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
> >> > 2 files changed, 72 insertions(+), 7 deletions(-)
> >> >
> >> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >index 257cf662f9f4..ac7896835bfa 100644
> >> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
> >> > #define BXT_DMC_MAX_FW_SIZE 0x3000
> >> > MODULE_FIRMWARE(BXT_DMC_PATH);
> >> >
> >> >-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> >> >-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >> >-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >> >-#define DMC_V1_MAX_MMIO_COUNT 8
> >> >-#define DMC_V3_MAX_MMIO_COUNT 20
> >> >-#define DMC_V1_MMIO_START_RANGE 0x80000
> >> >-
> >> > struct intel_css_header {
> >> > /* 0x09 for DMC */
> >> > u32 module_type;
> >> >@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
> >> *dmc,
> >> > }
> >> > }
> >> >
> >> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> >> u32 *mmioaddr,
> >> >+ u32 mmio_count, int header_ver, u8
> >> dmc_id) {
> >> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> >> dmc);
> >> >+ int i;
> >> >+
> >> >+ if (header_ver == 1) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
> >> mmioaddr[i] > DMC_MMIO_END_RANGE)
> >> >+ return false;
> >> >+ }
> >>
> >> return missing here
> >>
> >> >+ }
> >> >+
> >> >+ /* Main DMC MMIO check */
> >> >+ if (dmc_id == DMC_FW_MAIN) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> >> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >> >+ return false;
> >> >+ }
> >> >+ }
> >> >+
> >> >+ /* Pipe DMC MMIO check */
> >> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
> >> mmioaddr[i] > ADLP_PIPE_MMIO_END)
> >> >+ return false;
> >> >+ }
> >>
> >> for DG2, main should use TGL_DMC_MMIO_START? and then fail here
> >> because of another missing return above?
> >>
> >> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> >> IS_ALDERLAKE_S(i915)) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
> >> || mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
> >> >+ return false;
> >>
> >> this is handling DMC_FW_MAIN twice.
> >>
> >>
> >> Maybe something like this:
> >>
> >> u32 start, end;
> >>
> >> if (header_ver == 1) {
> >> start = DMC_MMIO_START_RANGE;
> >> end = DMC_MMIO_END_RANGE;
> >> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
> >> IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
> >> start = TGL_DMC_MMIO_START(dmc_id);
> >> end = TGL_DMC_MMIO_END(dmc_id);
> >> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >> start = ADLP_PIPE_MMIO_START;
> >> end = ADLP_PIPE_MMIO_END;
> >> } else {
> >> drm_warn(&i915->drm, "Unknown mmio range for sanity
> check");
> >> return false;
> >> }
> >>
> >> for (i = 0; i < mmio_count; i++)
> >> if (mmioaddr[i] < start || mmioaddr[i] > end)
> >> return false;
> >>
> >> return true;
> >>
> >>
> >> ... untested, and may need tweaks depending on the answer to the
> >> question above on what range to use for ADL-P/DG2 on main DMC.
> >The above approach is definitely neater.
> >The main DMC offset is the same for all platforms.
> >
> >> >+ }
> >> >+ }
> >> >+
> >> >+ return true;
> >> >+}
> >> >+
> >> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> >> > const struct intel_dmc_header_base
> >> *dmc_header,
> >> > size_t rem_size, u8 dmc_id) @@ -443,6 +473,10
> @@ static
> >> >u32 parse_dmc_fw_header(struct intel_dmc
> >> *dmc,
> >> > return 0;
> >> > }
> >> >
> >> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
> >> dmc_header->header_ver, dmc_id))
> >> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
> >> Addresses\n");
> >> >+ return 0;
> >>
> >> you don't like DMC and decided to fail it for all platforms?
> >
> ><facepalm>
> >
> >> >+
> >> > for (i = 0; i < mmio_count; i++) {
> >> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> >> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
> >> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >index d65e698832eb..235d1b721334 100644
> >> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >> >@@ -11,12 +11,43 @@
> >> > #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
> >> > #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
> >> > #define DMC_HTP_ADDR_SKL 0x00500034
> >> >+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
> >> > #define DMC_SSP_BASE _MMIO(0x8F074)
> >> > #define DMC_HTP_SKL _MMIO(0x8F004)
> >> > #define DMC_LAST_WRITE _MMIO(0x8F034)
> >> > #define DMC_LAST_WRITE_VALUE 0xc003b400
> >> > #define DMC_MMIO_START_RANGE 0x80000
> >> > #define DMC_MMIO_END_RANGE 0x8FFFF
> >> >+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
> >> >+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
> >> >+#define DMC_V1_MAX_MMIO_COUNT 8
> >> >+#define DMC_V3_MAX_MMIO_COUNT 20
> >>
> >>
> >> why are you moving these to _regs? these describe the DMC
> >> header/blob
> >
> >Yeah my mistake. While making the change they seemed like the right thing
> to do.
> >
> >>
> >> >+#define DMC_V1_MMIO_START_RANGE 0x80000
> >> >+#define _TGL_MAIN_MMIO_START 0x8F000
> >> >+#define _TGL_MAIN_MMIO_END 0x8FFFF
> >> >+#define _TGL_PIPEA_MMIO_START 0x92000
> >> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
> >> >+#define _TGL_PIPEB_MMIO_START 0x96000
> >> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
> >> >+#define _TGL_PIPEC_MMIO_START 0x9A000
> >> >+#define _TGL_PIPEC_MMIO_END 0x9BFFF
> >> >+#define _TGL_PIPED_MMIO_START 0x9E000
> >> >+#define _TGL_PIPED_MMIO_END 0x9FFFF
> >> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
> >> >+
> >> >+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
> >> _TGL_MAIN_MMIO_START,\
> >>
> >> _PICK? Did you miss my previous review?
> >
> >No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So
> using PICK_EVEN is not the right choice here. We also don't need to do
> _MMIO really.....
> >Unless I am missing something, this seems like the right approach.
>
> Because the name you chose for your variable:
>
> TGL_DMC_MMIO_START(pipe) _PICK(pipe,
>
> I was expecting this to be used only with the pipe DMC address, which are
> evenly spaced. It seems the argument you're expecting here is a dmc_id.
Ah, yes. I see the confusion now. It is expecting the dmc_id ,yes. In the usage of the macro in the beginning of the patch though, I am using dmc_id. Thought that would make it clearer, still pipe was the wrong choice of var name.
> But.... you said:
>
> >The main DMC offset is the same for all platforms.
>
> So, maybe just handle that separately and keep using pipe here? Then you
> can switch to _PICK_EVEN()
While the Pipe DMC s are evenly spaced and _PICK_EVEN is the right choice. The dmc_id for PipeA, PipeB ....will be 1,2....and not 0,1,2 so the helper will return the wrong values.
But you are suggesting to use PIPE_A, PIPE_B etc which will be 0 indexed. But here in the code we are parsing the dmc binary to see if it has Pipe DMC and if so what the MMIO offsets are they in and if it is a valid blob or not. The data we can use at this point is the dmc_id...... Unless we do a conversion from dmc-id to the pipe:
If (DMC_FW_PIPEA)
TGL_DMC_MMIO_START(PIPE_A) _PICK(
That will lead to individual such conditions per pipe.
Anusha
> Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-29 20:49 ` [Intel-gfx] " Lucas De Marchi
(?)
(?)
@ 2022-05-02 18:09 ` Lucas De Marchi
-1 siblings, 0 replies; 34+ messages in thread
From: Lucas De Marchi @ 2022-05-02 18:09 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx, stable
On Fri, Apr 29, 2022 at 01:49:37PM -0700, Lucas De Marchi wrote:
>On Fri, Apr 29, 2022 at 01:39:03PM -0700, Anusha Srivatsa wrote:
>>
>>
>>>-----Original Message-----
>>>From: De Marchi, Lucas <lucas.demarchi@intel.com>
>>>Sent: Tuesday, April 26, 2022 10:42 PM
>>>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>>>Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
>>>Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>>>
>>>On Tue, Apr 26, 2022 at 05:35:09PM -0700, Anusha Srivatsa wrote:
>>>>Bspec has added some steps that check forDMC MMIO range before
>>>>programming them
>>>>
>>>>v2: Fix for CI
>>>>v3: move register defines to .h (Anusha)
>>>>- Check MMIO restrictions per pipe
>>>>- Add MMIO restricton for v1 dmc header as well (Lucas)
>>>>
>>>>BSpec: 49193
>>>>
>>>>Cc: <stable@vger.kernel.org>
>>>>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>>>Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>>>---
>>>> drivers/gpu/drm/i915/display/intel_dmc.c | 48 ++++++++++++++++---
>>>> drivers/gpu/drm/i915/display/intel_dmc_regs.h | 31 ++++++++++++
>>>> 2 files changed, 72 insertions(+), 7 deletions(-)
>>>>
>>>>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
>>>>b/drivers/gpu/drm/i915/display/intel_dmc.c
>>>>index 257cf662f9f4..ac7896835bfa 100644
>>>>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>>>>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>>>>@@ -97,13 +97,6 @@ MODULE_FIRMWARE(SKL_DMC_PATH);
>>>> #define BXT_DMC_MAX_FW_SIZE 0x3000
>>>> MODULE_FIRMWARE(BXT_DMC_PATH);
>>>>
>>>>-#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>>>>-#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>>>>-#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>>>>-#define DMC_V1_MAX_MMIO_COUNT 8
>>>>-#define DMC_V3_MAX_MMIO_COUNT 20
>>>>-#define DMC_V1_MMIO_START_RANGE 0x80000
>>>>-
>>>> struct intel_css_header {
>>>> /* 0x09 for DMC */
>>>> u32 module_type;
>>>>@@ -374,6 +367,43 @@ static void dmc_set_fw_offset(struct intel_dmc
>>>*dmc,
>>>> }
>>>> }
>>>>
>>>>+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
>>>u32 *mmioaddr,
>>>>+ u32 mmio_count, int header_ver, u8
>>>dmc_id) {
>>>>+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
>>>dmc);
>>>>+ int i;
>>>>+
>>>>+ if (header_ver == 1) {
>>>>+ for (i = 0; i < mmio_count; i++) {
>>>>+ if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
>>>mmioaddr[i] > DMC_MMIO_END_RANGE)
>>>>+ return false;
>>>>+ }
>>>
>>>return missing here
>>>
>>>>+ }
>>>>+
>>>>+ /* Main DMC MMIO check */
>>>>+ if (dmc_id == DMC_FW_MAIN) {
>>>>+ for (i = 0; i < mmio_count; i++) {
>>>>+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
>>>|| mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>>>>+ return false;
>>>>+ }
>>>>+ }
>>>>+
>>>>+ /* Pipe DMC MMIO check */
>>>>+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>>>>+ for (i = 0; i < mmio_count; i++) {
>>>>+ if (mmioaddr[i] < ADLP_PIPE_MMIO_START &&
>>>mmioaddr[i] > ADLP_PIPE_MMIO_END)
>>>>+ return false;
>>>>+ }
>>>
>>>for DG2, main should use TGL_DMC_MMIO_START? and then fail here
>>>because of another missing return above?
>>>
>>>>+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
>>>IS_ALDERLAKE_S(i915)) {
>>>>+ for (i = 0; i < mmio_count; i++) {
>>>>+ if (mmioaddr[i] < TGL_DMC_MMIO_START(dmc_id)
>>>|| mmioaddr[i] > TGL_DMC_MMIO_END(dmc_id))
>>>>+ return false;
>>>
>>>this is handling DMC_FW_MAIN twice.
>>>
>>>
>>>Maybe something like this:
>>>
>>> u32 start, end;
>>>
>>> if (header_ver == 1) {
>>> start = DMC_MMIO_START_RANGE;
>>> end = DMC_MMIO_END_RANGE;
>>> } else if (dmc_id == DMC_FW_MAIN || IS_TIGERLAKE(i915) ||
>>>IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
>>> start = TGL_DMC_MMIO_START(dmc_id);
>>> end = TGL_DMC_MMIO_END(dmc_id);
>>> } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>>> start = ADLP_PIPE_MMIO_START;
>>> end = ADLP_PIPE_MMIO_END;
>>> } else {
>>> drm_warn(&i915->drm, "Unknown mmio range for sanity
>>>check");
>>> return false;
>>> }
>>>
>>> for (i = 0; i < mmio_count; i++)
>>> if (mmioaddr[i] < start || mmioaddr[i] > end)
>>> return false;
>>>
>>> return true;
>>>
>>>
>>>... untested, and may need tweaks depending on the answer to the question
>>>above on what range to use for ADL-P/DG2 on main DMC.
>>The above approach is definitely neater.
>>The main DMC offset is the same for all platforms.
>>
>>>>+ }
>>>>+ }
>>>>+
>>>>+ return true;
>>>>+}
>>>>+
>>>> static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
>>>> const struct intel_dmc_header_base
>>>*dmc_header,
>>>> size_t rem_size, u8 dmc_id)
>>>>@@ -443,6 +473,10 @@ static u32 parse_dmc_fw_header(struct intel_dmc
>>>*dmc,
>>>> return 0;
>>>> }
>>>>
>>>>+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
>>>dmc_header->header_ver, dmc_id))
>>>>+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
>>>Addresses\n");
>>>>+ return 0;
>>>
>>>you don't like DMC and decided to fail it for all platforms?
>>
>><facepalm>
>>
>>>>+
>>>> for (i = 0; i < mmio_count; i++) {
>>>> dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
>>>> dmc_info->mmiodata[i] = mmiodata[i]; diff --git
>>>>a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>>>>b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>>>>index d65e698832eb..235d1b721334 100644
>>>>--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>>>>+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>>>>@@ -11,12 +11,43 @@
>>>> #define DMC_PROGRAM(addr, i) _MMIO((addr) + (i) * 4)
>>>> #define DMC_SSP_BASE_ADDR_GEN9 0x00002FC0
>>>> #define DMC_HTP_ADDR_SKL 0x00500034
>>>>+#define DMC_DEFAULT_FW_OFFSET 0xFFFFFFFF
>>>> #define DMC_SSP_BASE _MMIO(0x8F074)
>>>> #define DMC_HTP_SKL _MMIO(0x8F004)
>>>> #define DMC_LAST_WRITE _MMIO(0x8F034)
>>>> #define DMC_LAST_WRITE_VALUE 0xc003b400
>>>> #define DMC_MMIO_START_RANGE 0x80000
>>>> #define DMC_MMIO_END_RANGE 0x8FFFF
>>>>+#define PACKAGE_MAX_FW_INFO_ENTRIES 20
>>>>+#define PACKAGE_V2_MAX_FW_INFO_ENTRIES 32
>>>>+#define DMC_V1_MAX_MMIO_COUNT 8
>>>>+#define DMC_V3_MAX_MMIO_COUNT 20
>>>
>>>
>>>why are you moving these to _regs? these describe the DMC header/blob
>>
>>Yeah my mistake. While making the change they seemed like the right thing to do.
>>
>>>
>>>>+#define DMC_V1_MMIO_START_RANGE 0x80000
>>>>+#define _TGL_MAIN_MMIO_START 0x8F000
>>>>+#define _TGL_MAIN_MMIO_END 0x8FFFF
>>>>+#define _TGL_PIPEA_MMIO_START 0x92000
>>>>+#define _TGL_PIPEA_MMIO_END 0x93FFF
>>>>+#define _TGL_PIPEB_MMIO_START 0x96000
>>>>+#define _TGL_PIPEB_MMIO_END 0x97FFF
>>>>+#define _TGL_PIPEC_MMIO_START 0x9A000
>>>>+#define _TGL_PIPEC_MMIO_END 0x9BFFF
>>>>+#define _TGL_PIPED_MMIO_START 0x9E000
>>>>+#define _TGL_PIPED_MMIO_END 0x9FFFF
>>>>+#define ADLP_PIPE_MMIO_START 0x5F000
>>>>+#define ADLP_PIPE_MMIO_END 0x5FFFF
>>>>+
>>>>+#define TGL_DMC_MMIO_START(pipe) _PICK(pipe,
>>>_TGL_MAIN_MMIO_START,\
>>>
>>>_PICK? Did you miss my previous review?
>>
>>No. the thing is Main DMC with the Pipe DMCs are not evenly spaced. So using PICK_EVEN is not the right choice here. We also don't need to do _MMIO really.....
>>Unless I am missing something, this seems like the right approach.
>
>Because the name you chose for your variable:
>
> TGL_DMC_MMIO_START(pipe) _PICK(pipe,
>
>I was expecting this to be used only with the pipe DMC address, which
>are evenly spaced. It seems the argument you're expecting here is a
>dmc_id. But.... you said:
>
>>The main DMC offset is the same for all platforms.
>
>So, maybe just handle that separately and keep using pipe here? Then you
>can switch to _PICK_EVEN()
usually we have helper functions/macros to do that kind of conversion.
dmc_id to pipe is id - 1. *With the proper range checks* somewhere to
ensure you aren't accessing the wrong address you could even embed the
conversion:
#define TGL_PIPE_DMC_MMIO_START(dmc_id) _PICK_EVEN((dmc_id - 1), ...
or add a helper macro to avoid the repetition and document what this is
about.
Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-06 17:35 Anusha Srivatsa
0 siblings, 0 replies; 34+ messages in thread
From: Anusha Srivatsa @ 2022-05-06 17:35 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, stable
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
- Add MMIO range for RKL as well.(Anusha)
v5: Use DISPLAY_VER instead of per platform check (Lucas)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 42 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 2f01aca4d981..f545cc7367e3 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -378,6 +378,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (DISPLAY_VER(i915) >= 13) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (DISPLAY_VER(i915) >= 12) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -447,6 +484,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-04 18:32 Anusha Srivatsa
0 siblings, 0 replies; 34+ messages in thread
From: Anusha Srivatsa @ 2022-05-04 18:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, stable
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
- Add MMIO range for RKL as well.(Anusha)
v5: Use DISPLAY_VER instead of per platform check (Lucas)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 42 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..e7437ed2597e 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -374,6 +374,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (DISPLAY_VER(i915) >= 13) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (DISPLAY_VER(i915) >= 12) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +480,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-05-04 0:31 ` Lucas De Marchi
@ 2022-05-04 0:36 ` Srivatsa, Anusha
0 siblings, 0 replies; 34+ messages in thread
From: Srivatsa, Anusha @ 2022-05-04 0:36 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx, stable
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Tuesday, May 3, 2022 5:31 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; stable@vger.kernel.org
> Subject: Re: [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Tue, May 03, 2022 at 05:13:46PM -0700, Anusha Srivatsa wrote:
> >Bspec has added some steps that check forDMC MMIO range before
> >programming them
> >
> >v2: Fix for CI
> >v3: move register defines to .h (Anusha)
> >- Check MMIO restrictions per pipe
> >- Add MMIO restricton for v1 dmc header as well (Lucas)
> >v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
> >- clean up sanity check logic.(Lucas)
> >- Add MMIO range for RKL as well.(Anusha)
> >
> >BSpec: 49193
> >
> >Cc: <stable@vger.kernel.org>
> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >---
> > drivers/gpu/drm/i915/display/intel_dmc.c | 43 +++++++++++++++++++
> > drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
> > 2 files changed, 60 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >index 257cf662f9f4..e37ba75e68da 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >@@ -374,6 +374,44 @@ static void dmc_set_fw_offset(struct intel_dmc
> *dmc,
> > }
> > }
> >
> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> u32 *mmioaddr,
> >+ u32 mmio_count, int header_ver, u8
> dmc_id) {
> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> dmc);
> >+ u32 start_range, end_range;
> >+ int i;
> >+
> >+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
>
> dmc_id is unsigned and DMC_FW_MAIN is 0. dmc_id < DMC_FW_MAIN can't
> ever possibly happen so you can remove it.
>
> >+ drm_warn(&i915->drm, "Unsupported firmware id %u\n",
> dmc_id);
> >+ return false;
> >+ }
> >+
> >+ if (header_ver == 1) {
> >+ start_range = DMC_MMIO_START_RANGE;
> >+ end_range = DMC_MMIO_END_RANGE;
> >+ } else if (dmc_id == DMC_FW_MAIN) {
> >+ start_range = TGL_MAIN_MMIO_START;
> >+ end_range = TGL_MAIN_MMIO_END;
> >+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>
> } else if (DISPLAY_VER(i915) >= 13) {
>
> ?
>
> >+ start_range = ADLP_PIPE_MMIO_START;
> >+ end_range = ADLP_PIPE_MMIO_END;
> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> IS_ALDERLAKE_S(i915) ||
> >+ IS_ROCKETLAKE(i915)) {
>
> } else if (DISPLAY_VER(i915) >= 12) {
>
> ?
>
> maintaining the if/else ladder fine grained by platform is somewhat painful.
Agreed.
> >+ start_range = TGL_PIPE_MMIO_START(dmc_id);
> >+ end_range = TGL_PIPE_MMIO_END(dmc_id);
> >+ } else {
> >+ drm_warn(&i915->drm, "Unknown mmio range for sanity
> check");
> >+ return false;
> >+ }
> >+
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
> >+ return false;
> >+ }
> >+
> >+ return true;
> >+}
> >+
> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> > const struct intel_dmc_header_base
> *dmc_header,
> > size_t rem_size, u8 dmc_id)
> >@@ -443,6 +481,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc
> *dmc,
> > return 0;
> > }
> >
> >+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count,
> dmc_header->header_ver, dmc_id)) {
> >+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO
> Addresses\n");
> >+ return 0;
> >+ }
> >+
> > for (i = 0; i < mmio_count; i++) {
> > dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> > dmc_info->mmiodata[i] = mmiodata[i]; diff --git
> >a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >index d65e698832eb..67e14eb96a7a 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
> >@@ -16,7 +16,23 @@
> > #define DMC_LAST_WRITE _MMIO(0x8F034)
> > #define DMC_LAST_WRITE_VALUE 0xc003b400
> > #define DMC_MMIO_START_RANGE 0x80000
> >-#define DMC_MMIO_END_RANGE 0x8FFFF
> >+#define DMC_MMIO_END_RANGE 0x8FFFF
> >+#define DMC_V1_MMIO_START_RANGE 0x80000
> >+#define TGL_MAIN_MMIO_START 0x8F000
> >+#define TGL_MAIN_MMIO_END 0x8FFFF
> >+#define _TGL_PIPEA_MMIO_START 0x92000
> >+#define _TGL_PIPEA_MMIO_END 0x93FFF
> >+#define _TGL_PIPEB_MMIO_START 0x96000
> >+#define _TGL_PIPEB_MMIO_END 0x97FFF
> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
>
> don't we have per-pipe range for ADLP_? Or is there only pipe A?
>
We don't have per-pipe range. We have one big chunk of range for all pipe DMC MMIOs.
> with the above fixes, feel free to add my Reviewed-by: Lucas De Marchi
> <lucas.demarchi@intel.com> in the next version.
Thanks!
> Lucas De Marchi
>
> >+
> >+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1),
> _TGL_PIPEA_MMIO_START,\
> >+ _TGL_PIPEB_MMIO_START)
> >+
> >+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1),
> _TGL_PIPEA_MMIO_END,\
> >+ _TGL_PIPEB_MMIO_END)
> >+
> > #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> > #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> > #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
> >--
> >2.25.1
> >
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-05-04 0:13 Anusha Srivatsa
@ 2022-05-04 0:31 ` Lucas De Marchi
2022-05-04 0:36 ` Srivatsa, Anusha
0 siblings, 1 reply; 34+ messages in thread
From: Lucas De Marchi @ 2022-05-04 0:31 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx, stable
On Tue, May 03, 2022 at 05:13:46PM -0700, Anusha Srivatsa wrote:
>Bspec has added some steps that check forDMC MMIO range before
>programming them
>
>v2: Fix for CI
>v3: move register defines to .h (Anusha)
>- Check MMIO restrictions per pipe
>- Add MMIO restricton for v1 dmc header as well (Lucas)
>v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
>- clean up sanity check logic.(Lucas)
>- Add MMIO range for RKL as well.(Anusha)
>
>BSpec: 49193
>
>Cc: <stable@vger.kernel.org>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dmc.c | 43 +++++++++++++++++++
> drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
> 2 files changed, 60 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>index 257cf662f9f4..e37ba75e68da 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>@@ -374,6 +374,44 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
> }
> }
>
>+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
>+ u32 mmio_count, int header_ver, u8 dmc_id)
>+{
>+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
>+ u32 start_range, end_range;
>+ int i;
>+
>+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
dmc_id is unsigned and DMC_FW_MAIN is 0. dmc_id < DMC_FW_MAIN can't ever
possibly happen so you can remove it.
>+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
>+ return false;
>+ }
>+
>+ if (header_ver == 1) {
>+ start_range = DMC_MMIO_START_RANGE;
>+ end_range = DMC_MMIO_END_RANGE;
>+ } else if (dmc_id == DMC_FW_MAIN) {
>+ start_range = TGL_MAIN_MMIO_START;
>+ end_range = TGL_MAIN_MMIO_END;
>+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
} else if (DISPLAY_VER(i915) >= 13) {
?
>+ start_range = ADLP_PIPE_MMIO_START;
>+ end_range = ADLP_PIPE_MMIO_END;
>+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915) ||
>+ IS_ROCKETLAKE(i915)) {
} else if (DISPLAY_VER(i915) >= 12) {
?
maintaining the if/else ladder fine grained by platform is somewhat painful.
>+ start_range = TGL_PIPE_MMIO_START(dmc_id);
>+ end_range = TGL_PIPE_MMIO_END(dmc_id);
>+ } else {
>+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
>+ return false;
>+ }
>+
>+ for (i = 0; i < mmio_count; i++) {
>+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
>+ return false;
>+ }
>+
>+ return true;
>+}
>+
> static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> const struct intel_dmc_header_base *dmc_header,
> size_t rem_size, u8 dmc_id)
>@@ -443,6 +481,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> return 0;
> }
>
>+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
>+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
>+ return 0;
>+ }
>+
> for (i = 0; i < mmio_count; i++) {
> dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> dmc_info->mmiodata[i] = mmiodata[i];
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>index d65e698832eb..67e14eb96a7a 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
>@@ -16,7 +16,23 @@
> #define DMC_LAST_WRITE _MMIO(0x8F034)
> #define DMC_LAST_WRITE_VALUE 0xc003b400
> #define DMC_MMIO_START_RANGE 0x80000
>-#define DMC_MMIO_END_RANGE 0x8FFFF
>+#define DMC_MMIO_END_RANGE 0x8FFFF
>+#define DMC_V1_MMIO_START_RANGE 0x80000
>+#define TGL_MAIN_MMIO_START 0x8F000
>+#define TGL_MAIN_MMIO_END 0x8FFFF
>+#define _TGL_PIPEA_MMIO_START 0x92000
>+#define _TGL_PIPEA_MMIO_END 0x93FFF
>+#define _TGL_PIPEB_MMIO_START 0x96000
>+#define _TGL_PIPEB_MMIO_END 0x97FFF
>+#define ADLP_PIPE_MMIO_START 0x5F000
>+#define ADLP_PIPE_MMIO_END 0x5FFFF
don't we have per-pipe range for ADLP_? Or is there only pipe A?
with the above fixes, feel free to add my Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
in the next version.
Lucas De Marchi
>+
>+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
>+ _TGL_PIPEB_MMIO_START)
>+
>+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
>+ _TGL_PIPEB_MMIO_END)
>+
> #define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
> #define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
> #define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
>--
>2.25.1
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-04 0:13 Anusha Srivatsa
2022-05-04 0:31 ` Lucas De Marchi
0 siblings, 1 reply; 34+ messages in thread
From: Anusha Srivatsa @ 2022-05-04 0:13 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, stable
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
- Add MMIO range for RKL as well.(Anusha)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 43 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..e37ba75e68da 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -374,6 +374,44 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915) ||
+ IS_ROCKETLAKE(i915)) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +481,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-03 23:36 Anusha Srivatsa
0 siblings, 0 replies; 34+ messages in thread
From: Anusha Srivatsa @ 2022-05-03 23:36 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, stable
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
- Add MMIO range for RKL as well.(Anusha)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 43 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..12d5cb850e39 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -374,6 +374,44 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915) ||
+ IS_ROCKETLAKE(dev_priv)) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +481,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-05-03 22:04 Anusha Srivatsa
0 siblings, 0 replies; 34+ messages in thread
From: Anusha Srivatsa @ 2022-05-03 22:04 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi, stable
Bspec has added some steps that check forDMC MMIO range before
programming them
v2: Fix for CI
v3: move register defines to .h (Anusha)
- Check MMIO restrictions per pipe
- Add MMIO restricton for v1 dmc header as well (Lucas)
v4: s/_PICK/_PICK_EVEN and use it only for Pipe DMC scenario.
- clean up sanity check logic.(Lucas)
BSpec: 49193
Cc: <stable@vger.kernel.org>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 42 +++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dmc_regs.h | 18 +++++++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..7abe4418eeb0 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -374,6 +374,43 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+ u32 mmio_count, int header_ver, u8 dmc_id)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ u32 start_range, end_range;
+ int i;
+
+ if (dmc_id >= DMC_FW_MAX || dmc_id < DMC_FW_MAIN) {
+ drm_warn(&i915->drm, "Unsupported firmware id %u\n", dmc_id);
+ return false;
+ }
+
+ if (header_ver == 1) {
+ start_range = DMC_MMIO_START_RANGE;
+ end_range = DMC_MMIO_END_RANGE;
+ } else if (dmc_id == DMC_FW_MAIN) {
+ start_range = TGL_MAIN_MMIO_START;
+ end_range = TGL_MAIN_MMIO_END;
+ } else if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ start_range = ADLP_PIPE_MMIO_START;
+ end_range = ADLP_PIPE_MMIO_END;
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915)) {
+ start_range = TGL_PIPE_MMIO_START(dmc_id);
+ end_range = TGL_PIPE_MMIO_END(dmc_id);
+ } else {
+ drm_warn(&i915->drm, "Unknown mmio range for sanity check");
+ return false;
+ }
+
+ for (i = 0; i < mmio_count; i++) {
+ if (mmioaddr[i] < start_range || mmioaddr[i] > end_range)
+ return false;
+ }
+
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +480,11 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count, dmc_header->header_ver, dmc_id)) {
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
diff --git a/drivers/gpu/drm/i915/display/intel_dmc_regs.h b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
index d65e698832eb..67e14eb96a7a 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dmc_regs.h
@@ -16,7 +16,23 @@
#define DMC_LAST_WRITE _MMIO(0x8F034)
#define DMC_LAST_WRITE_VALUE 0xc003b400
#define DMC_MMIO_START_RANGE 0x80000
-#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_MMIO_END_RANGE 0x8FFFF
+#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define _TGL_PIPEA_MMIO_START 0x92000
+#define _TGL_PIPEA_MMIO_END 0x93FFF
+#define _TGL_PIPEB_MMIO_START 0x96000
+#define _TGL_PIPEB_MMIO_END 0x97FFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
+
+#define TGL_PIPE_MMIO_START(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_START,\
+ _TGL_PIPEB_MMIO_START)
+
+#define TGL_PIPE_MMIO_END(dmc_id) _PICK_EVEN(((dmc_id) - 1), _TGL_PIPEA_MMIO_END,\
+ _TGL_PIPEB_MMIO_END)
+
#define SKL_DMC_DC3_DC5_COUNT _MMIO(0x80030)
#define SKL_DMC_DC5_DC6_COUNT _MMIO(0x8002C)
#define BXT_DMC_DC3_DC5_COUNT _MMIO(0x80038)
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-06 20:53 ` Srivatsa, Anusha
@ 2022-04-25 18:16 ` Lucas De Marchi
0 siblings, 0 replies; 34+ messages in thread
From: Lucas De Marchi @ 2022-04-25 18:16 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx
On Wed, Apr 06, 2022 at 08:53:56PM +0000, Anusha Srivatsa wrote:
>
>
>> -----Original Message-----
>> From: De Marchi, Lucas <lucas.demarchi@intel.com>
>> Sent: Wednesday, April 6, 2022 10:46 AM
>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
>>
>> On Wed, Apr 06, 2022 at 10:16:55AM -0700, Anusha Srivatsa wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: De Marchi, Lucas <lucas.demarchi@intel.com>
>> >> Sent: Tuesday, April 5, 2022 11:03 AM
>> >> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>> >> Cc: intel-gfx@lists.freedesktop.org
>> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range
>> >> restrictions
>> >>
>> >> On Tue, Apr 05, 2022 at 10:14:29AM -0700, Anusha Srivatsa wrote:
>> >> >Bspec has added some steps that check for DMC MMIO range before
>> >> >programming them.
>> >> >
>> >> >v2: Fix for CI failure for v1
>> >> >
>> >> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> >> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> >> >---
>> >> > drivers/gpu/drm/i915/display/intel_dmc.c | 42
>> >> ++++++++++++++++++++++++
>> >> > 1 file changed, 42 insertions(+)
>> >> >
>> >> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >> >b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >> >index 257cf662f9f4..05d8e90854ec 100644
>> >> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >> >@@ -103,6 +103,18 @@ MODULE_FIRMWARE(BXT_DMC_PATH);
>> >> > #define DMC_V1_MAX_MMIO_COUNT 8
>> >> > #define DMC_V3_MAX_MMIO_COUNT 20
>> >> > #define DMC_V1_MMIO_START_RANGE 0x80000
>> >> >+#define TGL_MAIN_MMIO_START 0x8F000
>> >> >+#define TGL_MAIN_MMIO_END 0x8FFFF
>> >> >+#define TGL_PIPEA_MMIO_START 0x92000
>> >> >+#define TGL_PIPEA_MMIO_END 0x93FFF
>> >> >+#define TGL_PIPEB_MMIO_START 0x96000
>> >> >+#define TGL_PIPEB_MMIO_END 0x97FFF
>> >> >+#define TGL_PIPEC_MMIO_START 0x9A000
>> >> >+#define TGL_PIPEC_MMIO_END 0x9BFFF
>> >> >+#define TGL_PIPED_MMIO_START 0x9E000
>> >> >+#define TGL_PIPED_MMIO_END 0x9FFFF
>> >> >+#define ADLP_PIPE_MMIO_START 0x5F000
>> >> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
>> >> >
>> >> > struct intel_css_header {
>> >> > /* 0x09 for DMC */
>> >> >@@ -374,6 +386,30 @@ static void dmc_set_fw_offset(struct intel_dmc
>> >> *dmc,
>> >> > }
>> >> > }
>> >> >
>> >> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
>> >> >+u32 *mmioaddr,
>> >> >+u32 mmio_count)
>> >> >+{
>> >> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
>> >> dmc);
>> >> >+ int i;
>> >> >+
>> >> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>> >> >+ for (i = 0; i < mmio_count; i++) {
>> >> >+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START &&
>> >> mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
>> >> >+ (mmioaddr[i] >= ADLP_PIPE_MMIO_START &&
>> >> mmioaddr[i] <= ADLP_PIPE_MMIO_END)))
>> >> >+ return false;
>> >> >+ }
>> >> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
>> >> IS_ALDERLAKE_S(i915))
>> >> >+ for (i = 0; i < mmio_count; i++) {
>> >> >+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START &&
>> >> mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
>> >> >+ (mmioaddr[i] >= TGL_PIPEA_MMIO_START &&
>> >> mmioaddr[i] <= TGL_PIPEA_MMIO_END) ||
>> >> >+ (mmioaddr[i] >= TGL_PIPEB_MMIO_START &&
>> >> mmioaddr[i] <= TGL_PIPEB_MMIO_END) ||
>> >> >+ (mmioaddr[i] >= TGL_PIPEC_MMIO_START &&
>> >> mmioaddr[i] <= TGL_PIPEC_MMIO_END) ||
>> >> >+ (mmioaddr[i] >= TGL_PIPED_MMIO_START &&
>> >> mmioaddr[i] <= TGL_PIPEC_MMIO_END)))
>> >> >+ return false;
>> >>
>> >> wonder if we should check for each pipe DMC range independently
>> >> rather than just checking all the ranges.
>> > Can convert this to a switch case in that scenario. "If it is PIPE A then it must
>> be within this range". But it will be 2 switches one for DG2 and ADLP and one
>> for TGL and the rest which have individual ranges for every pipe.
>>
>> I was thinking more about like this:
>>
>> #define _TGL_PIPEA_MMIO 0x92000
>> #define _TGL_PIPEB_MMIO 0x96000
>> #define TGL_PIPE_MMIO(pipe) _MMIO_PIPE(pipe, _TGL_PIPEA_MMIO,
>> _TGL_PIPEB_MMIO)
>> #define TGL_PIPE_MMIO_SIZE 0x1000
>
>Hmm, does it make sense to add something like:
>
>#define DMC_MMIO(dmc_id) _MMIO(_PICK(DMC_ID, DMC_FW_MAIN, DMC_FW_PIPEA, DMC_FW_PIPEB, DMC_FW_PIPEC, DMC_FW_PIPED)
typo here ----------------------------------^^^^^^
_PICK(dmc_id, DMC_FW_MAIN, DMC_FW_PIPEA, ...) would return 0, 1, ....
Why are you converting it to _MMIO? Did you mean to use the address?
If the main blob is not handled differently than it could make sense,
yes.
Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-06 17:46 ` Lucas De Marchi
@ 2022-04-06 20:53 ` Srivatsa, Anusha
2022-04-25 18:16 ` Lucas De Marchi
0 siblings, 1 reply; 34+ messages in thread
From: Srivatsa, Anusha @ 2022-04-06 20:53 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Wednesday, April 6, 2022 10:46 AM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Wed, Apr 06, 2022 at 10:16:55AM -0700, Anusha Srivatsa wrote:
> >
> >
> >> -----Original Message-----
> >> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> >> Sent: Tuesday, April 5, 2022 11:03 AM
> >> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> >> Cc: intel-gfx@lists.freedesktop.org
> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range
> >> restrictions
> >>
> >> On Tue, Apr 05, 2022 at 10:14:29AM -0700, Anusha Srivatsa wrote:
> >> >Bspec has added some steps that check for DMC MMIO range before
> >> >programming them.
> >> >
> >> >v2: Fix for CI failure for v1
> >> >
> >> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >> >---
> >> > drivers/gpu/drm/i915/display/intel_dmc.c | 42
> >> ++++++++++++++++++++++++
> >> > 1 file changed, 42 insertions(+)
> >> >
> >> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >index 257cf662f9f4..05d8e90854ec 100644
> >> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >> >@@ -103,6 +103,18 @@ MODULE_FIRMWARE(BXT_DMC_PATH);
> >> > #define DMC_V1_MAX_MMIO_COUNT 8
> >> > #define DMC_V3_MAX_MMIO_COUNT 20
> >> > #define DMC_V1_MMIO_START_RANGE 0x80000
> >> >+#define TGL_MAIN_MMIO_START 0x8F000
> >> >+#define TGL_MAIN_MMIO_END 0x8FFFF
> >> >+#define TGL_PIPEA_MMIO_START 0x92000
> >> >+#define TGL_PIPEA_MMIO_END 0x93FFF
> >> >+#define TGL_PIPEB_MMIO_START 0x96000
> >> >+#define TGL_PIPEB_MMIO_END 0x97FFF
> >> >+#define TGL_PIPEC_MMIO_START 0x9A000
> >> >+#define TGL_PIPEC_MMIO_END 0x9BFFF
> >> >+#define TGL_PIPED_MMIO_START 0x9E000
> >> >+#define TGL_PIPED_MMIO_END 0x9FFFF
> >> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
> >> >
> >> > struct intel_css_header {
> >> > /* 0x09 for DMC */
> >> >@@ -374,6 +386,30 @@ static void dmc_set_fw_offset(struct intel_dmc
> >> *dmc,
> >> > }
> >> > }
> >> >
> >> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> >> >+u32 *mmioaddr,
> >> >+u32 mmio_count)
> >> >+{
> >> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> >> dmc);
> >> >+ int i;
> >> >+
> >> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START &&
> >> mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
> >> >+ (mmioaddr[i] >= ADLP_PIPE_MMIO_START &&
> >> mmioaddr[i] <= ADLP_PIPE_MMIO_END)))
> >> >+ return false;
> >> >+ }
> >> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> >> IS_ALDERLAKE_S(i915))
> >> >+ for (i = 0; i < mmio_count; i++) {
> >> >+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START &&
> >> mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
> >> >+ (mmioaddr[i] >= TGL_PIPEA_MMIO_START &&
> >> mmioaddr[i] <= TGL_PIPEA_MMIO_END) ||
> >> >+ (mmioaddr[i] >= TGL_PIPEB_MMIO_START &&
> >> mmioaddr[i] <= TGL_PIPEB_MMIO_END) ||
> >> >+ (mmioaddr[i] >= TGL_PIPEC_MMIO_START &&
> >> mmioaddr[i] <= TGL_PIPEC_MMIO_END) ||
> >> >+ (mmioaddr[i] >= TGL_PIPED_MMIO_START &&
> >> mmioaddr[i] <= TGL_PIPEC_MMIO_END)))
> >> >+ return false;
> >>
> >> wonder if we should check for each pipe DMC range independently
> >> rather than just checking all the ranges.
> > Can convert this to a switch case in that scenario. "If it is PIPE A then it must
> be within this range". But it will be 2 switches one for DG2 and ADLP and one
> for TGL and the rest which have individual ranges for every pipe.
>
> I was thinking more about like this:
>
> #define _TGL_PIPEA_MMIO 0x92000
> #define _TGL_PIPEB_MMIO 0x96000
> #define TGL_PIPE_MMIO(pipe) _MMIO_PIPE(pipe, _TGL_PIPEA_MMIO,
> _TGL_PIPEB_MMIO)
> #define TGL_PIPE_MMIO_SIZE 0x1000
Hmm, does it make sense to add something like:
#define DMC_MMIO(dmc_id) _MMIO(_PICK(DMC_ID, DMC_FW_MAIN, DMC_FW_PIPEA, DMC_FW_PIPEB, DMC_FW_PIPEC, DMC_FW_PIPED)
> This of course means that each blob is supposed to update only addresses
> on their own ranges. Is this true? Do we care?
Yes, pipe A will update its own range and so on.
> >
> >> >+ }
> >> >+ return true;
> >> >+}
> >> >+
> >> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> >> > const struct intel_dmc_header_base
> >> *dmc_header,
> >> > size_t rem_size, u8 dmc_id) @@ -443,6 +479,12
> @@ static
> >> >u32 parse_dmc_fw_header(struct intel_dmc
> >> *dmc,
> >> > return 0;
> >> > }
> >> >
> >> >+ if (dmc_header->header_ver == 3) {
> >>
> >> this also needs to be done for ver 2
> >For v2 though there has been no update about the start range. As in this
> mmio range is different from the RAM_MMIO_START range.
>
> it is the same situation as v3. We read it from firmware. Why do you simply
> trust the value in v2 but you don't trust it in v3? You removed the check in
> 3d5928a168a9 ("drm/i915/xelpd: Pipe A DMC plugging")
>
> for (i = 0; i < mmio_count; i++) {
> - if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
> - mmioaddr[i] > DMC_MMIO_END_RANGE) {
> - drm_err(&i915->drm, "DMC firmware has wrong mmio address
> 0x%x\n",
> - mmioaddr[i]);
> - return 0;
> - }
> dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
> dmc_info->mmiodata[i] = mmiodata[i];
> }
>
> I remember mentioning this during review, but let it pass.
Thanks for the above patch, will do the needful.
Anusha
> Lucas De Marchi
>
> >
> >Anusha
> >
> >> Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-06 17:16 ` Srivatsa, Anusha
@ 2022-04-06 17:46 ` Lucas De Marchi
2022-04-06 20:53 ` Srivatsa, Anusha
0 siblings, 1 reply; 34+ messages in thread
From: Lucas De Marchi @ 2022-04-06 17:46 UTC (permalink / raw)
To: Srivatsa, Anusha; +Cc: intel-gfx
On Wed, Apr 06, 2022 at 10:16:55AM -0700, Anusha Srivatsa wrote:
>
>
>> -----Original Message-----
>> From: De Marchi, Lucas <lucas.demarchi@intel.com>
>> Sent: Tuesday, April 5, 2022 11:03 AM
>> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>> Cc: intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
>>
>> On Tue, Apr 05, 2022 at 10:14:29AM -0700, Anusha Srivatsa wrote:
>> >Bspec has added some steps that check for DMC MMIO range before
>> >programming them.
>> >
>> >v2: Fix for CI failure for v1
>> >
>> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> >---
>> > drivers/gpu/drm/i915/display/intel_dmc.c | 42
>> ++++++++++++++++++++++++
>> > 1 file changed, 42 insertions(+)
>> >
>> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >index 257cf662f9f4..05d8e90854ec 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>> >@@ -103,6 +103,18 @@ MODULE_FIRMWARE(BXT_DMC_PATH);
>> > #define DMC_V1_MAX_MMIO_COUNT 8
>> > #define DMC_V3_MAX_MMIO_COUNT 20
>> > #define DMC_V1_MMIO_START_RANGE 0x80000
>> >+#define TGL_MAIN_MMIO_START 0x8F000
>> >+#define TGL_MAIN_MMIO_END 0x8FFFF
>> >+#define TGL_PIPEA_MMIO_START 0x92000
>> >+#define TGL_PIPEA_MMIO_END 0x93FFF
>> >+#define TGL_PIPEB_MMIO_START 0x96000
>> >+#define TGL_PIPEB_MMIO_END 0x97FFF
>> >+#define TGL_PIPEC_MMIO_START 0x9A000
>> >+#define TGL_PIPEC_MMIO_END 0x9BFFF
>> >+#define TGL_PIPED_MMIO_START 0x9E000
>> >+#define TGL_PIPED_MMIO_END 0x9FFFF
>> >+#define ADLP_PIPE_MMIO_START 0x5F000
>> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
>> >
>> > struct intel_css_header {
>> > /* 0x09 for DMC */
>> >@@ -374,6 +386,30 @@ static void dmc_set_fw_offset(struct intel_dmc
>> *dmc,
>> > }
>> > }
>> >
>> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
>> >+u32 *mmioaddr,
>> >+u32 mmio_count)
>> >+{
>> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
>> dmc);
>> >+ int i;
>> >+
>> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START &&
>> mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
>> >+ (mmioaddr[i] >= ADLP_PIPE_MMIO_START &&
>> mmioaddr[i] <= ADLP_PIPE_MMIO_END)))
>> >+ return false;
>> >+ }
>> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
>> IS_ALDERLAKE_S(i915))
>> >+ for (i = 0; i < mmio_count; i++) {
>> >+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START &&
>> mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
>> >+ (mmioaddr[i] >= TGL_PIPEA_MMIO_START &&
>> mmioaddr[i] <= TGL_PIPEA_MMIO_END) ||
>> >+ (mmioaddr[i] >= TGL_PIPEB_MMIO_START &&
>> mmioaddr[i] <= TGL_PIPEB_MMIO_END) ||
>> >+ (mmioaddr[i] >= TGL_PIPEC_MMIO_START &&
>> mmioaddr[i] <= TGL_PIPEC_MMIO_END) ||
>> >+ (mmioaddr[i] >= TGL_PIPED_MMIO_START &&
>> mmioaddr[i] <= TGL_PIPEC_MMIO_END)))
>> >+ return false;
>>
>> wonder if we should check for each pipe DMC range independently rather
>> than just checking all the ranges.
> Can convert this to a switch case in that scenario. "If it is PIPE A then it must be within this range". But it will be 2 switches one for DG2 and ADLP and one for TGL and the rest which have individual ranges for every pipe.
I was thinking more about like this:
#define _TGL_PIPEA_MMIO 0x92000
#define _TGL_PIPEB_MMIO 0x96000
#define TGL_PIPE_MMIO(pipe) _MMIO_PIPE(pipe, _TGL_PIPEA_MMIO, _TGL_PIPEB_MMIO)
#define TGL_PIPE_MMIO_SIZE 0x1000
This of course means that each blob is supposed to update only addresses
on their own ranges. Is this true? Do we care?
>
>> >+ }
>> >+ return true;
>> >+}
>> >+
>> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
>> > const struct intel_dmc_header_base
>> *dmc_header,
>> > size_t rem_size, u8 dmc_id)
>> >@@ -443,6 +479,12 @@ static u32 parse_dmc_fw_header(struct intel_dmc
>> *dmc,
>> > return 0;
>> > }
>> >
>> >+ if (dmc_header->header_ver == 3) {
>>
>> this also needs to be done for ver 2
>For v2 though there has been no update about the start range. As in this mmio range is different from the RAM_MMIO_START range.
it is the same situation as v3. We read it from firmware. Why do you
simply trust the value in v2 but you don't trust it in v3? You removed
the check in 3d5928a168a9 ("drm/i915/xelpd: Pipe A DMC plugging")
for (i = 0; i < mmio_count; i++) {
- if (mmioaddr[i] < DMC_MMIO_START_RANGE ||
- mmioaddr[i] > DMC_MMIO_END_RANGE) {
- drm_err(&i915->drm, "DMC firmware has wrong mmio address 0x%x\n",
- mmioaddr[i]);
- return 0;
- }
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
}
I remember mentioning this during review, but let it pass.
Lucas De Marchi
>
>Anusha
>
>> Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-05 18:02 ` Lucas De Marchi
@ 2022-04-06 17:16 ` Srivatsa, Anusha
2022-04-06 17:46 ` Lucas De Marchi
0 siblings, 1 reply; 34+ messages in thread
From: Srivatsa, Anusha @ 2022-04-06 17:16 UTC (permalink / raw)
To: De Marchi, Lucas; +Cc: intel-gfx
> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi@intel.com>
> Sent: Tuesday, April 5, 2022 11:03 AM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
>
> On Tue, Apr 05, 2022 at 10:14:29AM -0700, Anusha Srivatsa wrote:
> >Bspec has added some steps that check for DMC MMIO range before
> >programming them.
> >
> >v2: Fix for CI failure for v1
> >
> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> >---
> > drivers/gpu/drm/i915/display/intel_dmc.c | 42
> ++++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c
> >b/drivers/gpu/drm/i915/display/intel_dmc.c
> >index 257cf662f9f4..05d8e90854ec 100644
> >--- a/drivers/gpu/drm/i915/display/intel_dmc.c
> >+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
> >@@ -103,6 +103,18 @@ MODULE_FIRMWARE(BXT_DMC_PATH);
> > #define DMC_V1_MAX_MMIO_COUNT 8
> > #define DMC_V3_MAX_MMIO_COUNT 20
> > #define DMC_V1_MMIO_START_RANGE 0x80000
> >+#define TGL_MAIN_MMIO_START 0x8F000
> >+#define TGL_MAIN_MMIO_END 0x8FFFF
> >+#define TGL_PIPEA_MMIO_START 0x92000
> >+#define TGL_PIPEA_MMIO_END 0x93FFF
> >+#define TGL_PIPEB_MMIO_START 0x96000
> >+#define TGL_PIPEB_MMIO_END 0x97FFF
> >+#define TGL_PIPEC_MMIO_START 0x9A000
> >+#define TGL_PIPEC_MMIO_END 0x9BFFF
> >+#define TGL_PIPED_MMIO_START 0x9E000
> >+#define TGL_PIPED_MMIO_END 0x9FFFF
> >+#define ADLP_PIPE_MMIO_START 0x5F000
> >+#define ADLP_PIPE_MMIO_END 0x5FFFF
> >
> > struct intel_css_header {
> > /* 0x09 for DMC */
> >@@ -374,6 +386,30 @@ static void dmc_set_fw_offset(struct intel_dmc
> *dmc,
> > }
> > }
> >
> >+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const
> >+u32 *mmioaddr,
> >+u32 mmio_count)
> >+{
> >+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915),
> dmc);
> >+ int i;
> >+
> >+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START &&
> mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
> >+ (mmioaddr[i] >= ADLP_PIPE_MMIO_START &&
> mmioaddr[i] <= ADLP_PIPE_MMIO_END)))
> >+ return false;
> >+ }
> >+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) ||
> IS_ALDERLAKE_S(i915))
> >+ for (i = 0; i < mmio_count; i++) {
> >+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START &&
> mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
> >+ (mmioaddr[i] >= TGL_PIPEA_MMIO_START &&
> mmioaddr[i] <= TGL_PIPEA_MMIO_END) ||
> >+ (mmioaddr[i] >= TGL_PIPEB_MMIO_START &&
> mmioaddr[i] <= TGL_PIPEB_MMIO_END) ||
> >+ (mmioaddr[i] >= TGL_PIPEC_MMIO_START &&
> mmioaddr[i] <= TGL_PIPEC_MMIO_END) ||
> >+ (mmioaddr[i] >= TGL_PIPED_MMIO_START &&
> mmioaddr[i] <= TGL_PIPEC_MMIO_END)))
> >+ return false;
>
> wonder if we should check for each pipe DMC range independently rather
> than just checking all the ranges.
Can convert this to a switch case in that scenario. "If it is PIPE A then it must be within this range". But it will be 2 switches one for DG2 and ADLP and one for TGL and the rest which have individual ranges for every pipe.
> >+ }
> >+ return true;
> >+}
> >+
> > static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> > const struct intel_dmc_header_base
> *dmc_header,
> > size_t rem_size, u8 dmc_id)
> >@@ -443,6 +479,12 @@ static u32 parse_dmc_fw_header(struct intel_dmc
> *dmc,
> > return 0;
> > }
> >
> >+ if (dmc_header->header_ver == 3) {
>
> this also needs to be done for ver 2
For v2 though there has been no update about the start range. As in this mmio range is different from the RAM_MMIO_START range.
Anusha
> Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-05 17:14 Anusha Srivatsa
2022-04-05 18:02 ` Lucas De Marchi
2022-04-05 23:18 ` kernel test robot
@ 2022-04-06 6:58 ` kernel test robot
2 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2022-04-06 6:58 UTC (permalink / raw)
To: Anusha Srivatsa, intel-gfx; +Cc: Lucas De Marchi, kbuild-all
Hi Anusha,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on drm-tip/drm-tip v5.18-rc1 next-20220405]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220406-011821
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220406/202204061418.8NyZZGOk-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-19) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/53017f72f2ddb095da8d5ef1cb92d0b1d02c8a2b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220406-011821
git checkout 53017f72f2ddb095da8d5ef1cb92d0b1d02c8a2b
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_dmc.c: In function 'parse_dmc_fw_header':
>> drivers/gpu/drm/i915/display/intel_dmc.c:476:17: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
476 | if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count))
| ^~
drivers/gpu/drm/i915/display/intel_dmc.c:478:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
478 | return 0;
| ^~~~~~
vim +/if +476 drivers/gpu/drm/i915/display/intel_dmc.c
405
406 static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
407 const struct intel_dmc_header_base *dmc_header,
408 size_t rem_size, u8 dmc_id)
409 {
410 struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
411 struct dmc_fw_info *dmc_info = &dmc->dmc_info[dmc_id];
412 unsigned int header_len_bytes, dmc_header_size, payload_size, i;
413 const u32 *mmioaddr, *mmiodata;
414 u32 mmio_count, mmio_count_max, start_mmioaddr;
415 u8 *payload;
416
417 BUILD_BUG_ON(ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V3_MAX_MMIO_COUNT ||
418 ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V1_MAX_MMIO_COUNT);
419
420 /*
421 * Check if we can access common fields, we will checkc again below
422 * after we have read the version
423 */
424 if (rem_size < sizeof(struct intel_dmc_header_base))
425 goto error_truncated;
426
427 /* Cope with small differences between v1 and v3 */
428 if (dmc_header->header_ver == 3) {
429 const struct intel_dmc_header_v3 *v3 =
430 (const struct intel_dmc_header_v3 *)dmc_header;
431
432 if (rem_size < sizeof(struct intel_dmc_header_v3))
433 goto error_truncated;
434
435 mmioaddr = v3->mmioaddr;
436 mmiodata = v3->mmiodata;
437 mmio_count = v3->mmio_count;
438 mmio_count_max = DMC_V3_MAX_MMIO_COUNT;
439 /* header_len is in dwords */
440 header_len_bytes = dmc_header->header_len * 4;
441 start_mmioaddr = v3->start_mmioaddr;
442 dmc_header_size = sizeof(*v3);
443 } else if (dmc_header->header_ver == 1) {
444 const struct intel_dmc_header_v1 *v1 =
445 (const struct intel_dmc_header_v1 *)dmc_header;
446
447 if (rem_size < sizeof(struct intel_dmc_header_v1))
448 goto error_truncated;
449
450 mmioaddr = v1->mmioaddr;
451 mmiodata = v1->mmiodata;
452 mmio_count = v1->mmio_count;
453 mmio_count_max = DMC_V1_MAX_MMIO_COUNT;
454 header_len_bytes = dmc_header->header_len;
455 start_mmioaddr = DMC_V1_MMIO_START_RANGE;
456 dmc_header_size = sizeof(*v1);
457 } else {
458 drm_err(&i915->drm, "Unknown DMC fw header version: %u\n",
459 dmc_header->header_ver);
460 return 0;
461 }
462
463 if (header_len_bytes != dmc_header_size) {
464 drm_err(&i915->drm, "DMC firmware has wrong dmc header length "
465 "(%u bytes)\n", header_len_bytes);
466 return 0;
467 }
468
469 /* Cache the dmc header info. */
470 if (mmio_count > mmio_count_max) {
471 drm_err(&i915->drm, "DMC firmware has wrong mmio count %u\n", mmio_count);
472 return 0;
473 }
474
475 if (dmc_header->header_ver == 3) {
> 476 if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count))
477 drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
478 return 0;
479 }
480
481 for (i = 0; i < mmio_count; i++) {
482 dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
483 dmc_info->mmiodata[i] = mmiodata[i];
484 }
485 dmc_info->mmio_count = mmio_count;
486 dmc_info->start_mmioaddr = start_mmioaddr;
487
488 rem_size -= header_len_bytes;
489
490 /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
491 payload_size = dmc_header->fw_size * 4;
492 if (rem_size < payload_size)
493 goto error_truncated;
494
495 if (payload_size > dmc->max_fw_size) {
496 drm_err(&i915->drm, "DMC FW too big (%u bytes)\n", payload_size);
497 return 0;
498 }
499 dmc_info->dmc_fw_size = dmc_header->fw_size;
500
501 dmc_info->payload = kmalloc(payload_size, GFP_KERNEL);
502 if (!dmc_info->payload)
503 return 0;
504
505 payload = (u8 *)(dmc_header) + header_len_bytes;
506 memcpy(dmc_info->payload, payload, payload_size);
507
508 return header_len_bytes + payload_size;
509
510 error_truncated:
511 drm_err(&i915->drm, "Truncated DMC firmware, refusing.\n");
512 return 0;
513 }
514
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-05 17:14 Anusha Srivatsa
2022-04-05 18:02 ` Lucas De Marchi
@ 2022-04-05 23:18 ` kernel test robot
2022-04-06 6:58 ` kernel test robot
2 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2022-04-05 23:18 UTC (permalink / raw)
To: Anusha Srivatsa, intel-gfx; +Cc: Lucas De Marchi, kbuild-all
Hi Anusha,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on drm-tip/drm-tip v5.18-rc1 next-20220405]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220406-011821
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-randconfig-c002 (https://download.01.org/0day-ci/archive/20220406/202204060704.ALs2bEHY-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-19) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/53017f72f2ddb095da8d5ef1cb92d0b1d02c8a2b
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Anusha-Srivatsa/drm-i915-dmc-Add-MMIO-range-restrictions/20220406-011821
git checkout 53017f72f2ddb095da8d5ef1cb92d0b1d02c8a2b
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/gpu/drm/i915/display/intel_dmc.c: In function 'parse_dmc_fw_header':
>> drivers/gpu/drm/i915/display/intel_dmc.c:476:17: error: this 'if' clause does not guard... [-Werror=misleading-indentation]
476 | if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count))
| ^~
drivers/gpu/drm/i915/display/intel_dmc.c:478:25: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
478 | return 0;
| ^~~~~~
cc1: all warnings being treated as errors
vim +/if +476 drivers/gpu/drm/i915/display/intel_dmc.c
405
406 static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
407 const struct intel_dmc_header_base *dmc_header,
408 size_t rem_size, u8 dmc_id)
409 {
410 struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
411 struct dmc_fw_info *dmc_info = &dmc->dmc_info[dmc_id];
412 unsigned int header_len_bytes, dmc_header_size, payload_size, i;
413 const u32 *mmioaddr, *mmiodata;
414 u32 mmio_count, mmio_count_max, start_mmioaddr;
415 u8 *payload;
416
417 BUILD_BUG_ON(ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V3_MAX_MMIO_COUNT ||
418 ARRAY_SIZE(dmc_info->mmioaddr) < DMC_V1_MAX_MMIO_COUNT);
419
420 /*
421 * Check if we can access common fields, we will checkc again below
422 * after we have read the version
423 */
424 if (rem_size < sizeof(struct intel_dmc_header_base))
425 goto error_truncated;
426
427 /* Cope with small differences between v1 and v3 */
428 if (dmc_header->header_ver == 3) {
429 const struct intel_dmc_header_v3 *v3 =
430 (const struct intel_dmc_header_v3 *)dmc_header;
431
432 if (rem_size < sizeof(struct intel_dmc_header_v3))
433 goto error_truncated;
434
435 mmioaddr = v3->mmioaddr;
436 mmiodata = v3->mmiodata;
437 mmio_count = v3->mmio_count;
438 mmio_count_max = DMC_V3_MAX_MMIO_COUNT;
439 /* header_len is in dwords */
440 header_len_bytes = dmc_header->header_len * 4;
441 start_mmioaddr = v3->start_mmioaddr;
442 dmc_header_size = sizeof(*v3);
443 } else if (dmc_header->header_ver == 1) {
444 const struct intel_dmc_header_v1 *v1 =
445 (const struct intel_dmc_header_v1 *)dmc_header;
446
447 if (rem_size < sizeof(struct intel_dmc_header_v1))
448 goto error_truncated;
449
450 mmioaddr = v1->mmioaddr;
451 mmiodata = v1->mmiodata;
452 mmio_count = v1->mmio_count;
453 mmio_count_max = DMC_V1_MAX_MMIO_COUNT;
454 header_len_bytes = dmc_header->header_len;
455 start_mmioaddr = DMC_V1_MMIO_START_RANGE;
456 dmc_header_size = sizeof(*v1);
457 } else {
458 drm_err(&i915->drm, "Unknown DMC fw header version: %u\n",
459 dmc_header->header_ver);
460 return 0;
461 }
462
463 if (header_len_bytes != dmc_header_size) {
464 drm_err(&i915->drm, "DMC firmware has wrong dmc header length "
465 "(%u bytes)\n", header_len_bytes);
466 return 0;
467 }
468
469 /* Cache the dmc header info. */
470 if (mmio_count > mmio_count_max) {
471 drm_err(&i915->drm, "DMC firmware has wrong mmio count %u\n", mmio_count);
472 return 0;
473 }
474
475 if (dmc_header->header_ver == 3) {
> 476 if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count))
477 drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
478 return 0;
479 }
480
481 for (i = 0; i < mmio_count; i++) {
482 dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
483 dmc_info->mmiodata[i] = mmiodata[i];
484 }
485 dmc_info->mmio_count = mmio_count;
486 dmc_info->start_mmioaddr = start_mmioaddr;
487
488 rem_size -= header_len_bytes;
489
490 /* fw_size is in dwords, so multiplied by 4 to convert into bytes. */
491 payload_size = dmc_header->fw_size * 4;
492 if (rem_size < payload_size)
493 goto error_truncated;
494
495 if (payload_size > dmc->max_fw_size) {
496 drm_err(&i915->drm, "DMC FW too big (%u bytes)\n", payload_size);
497 return 0;
498 }
499 dmc_info->dmc_fw_size = dmc_header->fw_size;
500
501 dmc_info->payload = kmalloc(payload_size, GFP_KERNEL);
502 if (!dmc_info->payload)
503 return 0;
504
505 payload = (u8 *)(dmc_header) + header_len_bytes;
506 memcpy(dmc_info->payload, payload, payload_size);
507
508 return header_len_bytes + payload_size;
509
510 error_truncated:
511 drm_err(&i915->drm, "Truncated DMC firmware, refusing.\n");
512 return 0;
513 }
514
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
2022-04-05 17:14 Anusha Srivatsa
@ 2022-04-05 18:02 ` Lucas De Marchi
2022-04-06 17:16 ` Srivatsa, Anusha
2022-04-05 23:18 ` kernel test robot
2022-04-06 6:58 ` kernel test robot
2 siblings, 1 reply; 34+ messages in thread
From: Lucas De Marchi @ 2022-04-05 18:02 UTC (permalink / raw)
To: Anusha Srivatsa; +Cc: intel-gfx
On Tue, Apr 05, 2022 at 10:14:29AM -0700, Anusha Srivatsa wrote:
>Bspec has added some steps that check for DMC MMIO range before
>programming them.
>
>v2: Fix for CI failure for v1
>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dmc.c | 42 ++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>index 257cf662f9f4..05d8e90854ec 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>@@ -103,6 +103,18 @@ MODULE_FIRMWARE(BXT_DMC_PATH);
> #define DMC_V1_MAX_MMIO_COUNT 8
> #define DMC_V3_MAX_MMIO_COUNT 20
> #define DMC_V1_MMIO_START_RANGE 0x80000
>+#define TGL_MAIN_MMIO_START 0x8F000
>+#define TGL_MAIN_MMIO_END 0x8FFFF
>+#define TGL_PIPEA_MMIO_START 0x92000
>+#define TGL_PIPEA_MMIO_END 0x93FFF
>+#define TGL_PIPEB_MMIO_START 0x96000
>+#define TGL_PIPEB_MMIO_END 0x97FFF
>+#define TGL_PIPEC_MMIO_START 0x9A000
>+#define TGL_PIPEC_MMIO_END 0x9BFFF
>+#define TGL_PIPED_MMIO_START 0x9E000
>+#define TGL_PIPED_MMIO_END 0x9FFFF
>+#define ADLP_PIPE_MMIO_START 0x5F000
>+#define ADLP_PIPE_MMIO_END 0x5FFFF
>
> struct intel_css_header {
> /* 0x09 for DMC */
>@@ -374,6 +386,30 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
> }
> }
>
>+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
>+u32 mmio_count)
>+{
>+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
>+ int i;
>+
>+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
>+ for (i = 0; i < mmio_count; i++) {
>+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START && mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
>+ (mmioaddr[i] >= ADLP_PIPE_MMIO_START && mmioaddr[i] <= ADLP_PIPE_MMIO_END)))
>+ return false;
>+ }
>+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915))
>+ for (i = 0; i < mmio_count; i++) {
>+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START && mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
>+ (mmioaddr[i] >= TGL_PIPEA_MMIO_START && mmioaddr[i] <= TGL_PIPEA_MMIO_END) ||
>+ (mmioaddr[i] >= TGL_PIPEB_MMIO_START && mmioaddr[i] <= TGL_PIPEB_MMIO_END) ||
>+ (mmioaddr[i] >= TGL_PIPEC_MMIO_START && mmioaddr[i] <= TGL_PIPEC_MMIO_END) ||
>+ (mmioaddr[i] >= TGL_PIPED_MMIO_START && mmioaddr[i] <= TGL_PIPEC_MMIO_END)))
>+ return false;
wonder if we should check for each pipe DMC range independently rather
than just checking all the ranges.
>+ }
>+ return true;
>+}
>+
> static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> const struct intel_dmc_header_base *dmc_header,
> size_t rem_size, u8 dmc_id)
>@@ -443,6 +479,12 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
> return 0;
> }
>
>+ if (dmc_header->header_ver == 3) {
this also needs to be done for ver 2
Lucas De Marchi
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-05 17:14 Anusha Srivatsa
2022-04-05 18:02 ` Lucas De Marchi
` (2 more replies)
0 siblings, 3 replies; 34+ messages in thread
From: Anusha Srivatsa @ 2022-04-05 17:14 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Bspec has added some steps that check for DMC MMIO range before
programming them.
v2: Fix for CI failure for v1
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 42 ++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..05d8e90854ec 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -103,6 +103,18 @@ MODULE_FIRMWARE(BXT_DMC_PATH);
#define DMC_V1_MAX_MMIO_COUNT 8
#define DMC_V3_MAX_MMIO_COUNT 20
#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define TGL_PIPEA_MMIO_START 0x92000
+#define TGL_PIPEA_MMIO_END 0x93FFF
+#define TGL_PIPEB_MMIO_START 0x96000
+#define TGL_PIPEB_MMIO_END 0x97FFF
+#define TGL_PIPEC_MMIO_START 0x9A000
+#define TGL_PIPEC_MMIO_END 0x9BFFF
+#define TGL_PIPED_MMIO_START 0x9E000
+#define TGL_PIPED_MMIO_END 0x9FFFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
struct intel_css_header {
/* 0x09 for DMC */
@@ -374,6 +386,30 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+u32 mmio_count)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ int i;
+
+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ for (i = 0; i < mmio_count; i++) {
+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START && mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
+ (mmioaddr[i] >= ADLP_PIPE_MMIO_START && mmioaddr[i] <= ADLP_PIPE_MMIO_END)))
+ return false;
+ }
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915))
+ for (i = 0; i < mmio_count; i++) {
+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START && mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
+ (mmioaddr[i] >= TGL_PIPEA_MMIO_START && mmioaddr[i] <= TGL_PIPEA_MMIO_END) ||
+ (mmioaddr[i] >= TGL_PIPEB_MMIO_START && mmioaddr[i] <= TGL_PIPEB_MMIO_END) ||
+ (mmioaddr[i] >= TGL_PIPEC_MMIO_START && mmioaddr[i] <= TGL_PIPEC_MMIO_END) ||
+ (mmioaddr[i] >= TGL_PIPED_MMIO_START && mmioaddr[i] <= TGL_PIPEC_MMIO_END)))
+ return false;
+ }
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +479,12 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (dmc_header->header_ver == 3) {
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count))
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions
@ 2022-04-05 0:35 Anusha Srivatsa
0 siblings, 0 replies; 34+ messages in thread
From: Anusha Srivatsa @ 2022-04-05 0:35 UTC (permalink / raw)
To: intel-gfx; +Cc: Lucas De Marchi
Bspec has added some steps that check for DMC MMIO range before
programming them.
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
drivers/gpu/drm/i915/display/intel_dmc.c | 42 ++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 257cf662f9f4..dc4ff43e9467 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -103,6 +103,18 @@ MODULE_FIRMWARE(BXT_DMC_PATH);
#define DMC_V1_MAX_MMIO_COUNT 8
#define DMC_V3_MAX_MMIO_COUNT 20
#define DMC_V1_MMIO_START_RANGE 0x80000
+#define TGL_MAIN_MMIO_START 0x8F000
+#define TGL_MAIN_MMIO_END 0x8FFFF
+#define TGL_PIPEA_MMIO_START 0x92000
+#define TGL_PIPEA_MMIO_END 0x93FFF
+#define TGL_PIPEB_MMIO_START 0x96000
+#define TGL_PIPEB_MMIO_END 0x97FFF
+#define TGL_PIPEC_MMIO_START 0x9A000
+#define TGL_PIPEC_MMIO_END 0x9BFFF
+#define TGL_PIPED_MMIO_START 0x9E000
+#define TGL_PIPED_MMIO_END 0x9FFFF
+#define ADLP_PIPE_MMIO_START 0x5F000
+#define ADLP_PIPE_MMIO_END 0x5FFFF
struct intel_css_header {
/* 0x09 for DMC */
@@ -374,6 +386,30 @@ static void dmc_set_fw_offset(struct intel_dmc *dmc,
}
}
+static bool dmc_mmio_addr_sanity_check(struct intel_dmc *dmc, const u32 *mmioaddr,
+u32 mmio_count)
+{
+ struct drm_i915_private *i915 = container_of(dmc, typeof(*i915), dmc);
+ int i;
+
+ if (IS_DG2(i915) || IS_ALDERLAKE_P(i915)) {
+ for (i = 0; i < mmio_count; i++) {
+ if (!((mmioaddr[i] >= TGL_MAIN_MMIO_START && mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
+ (mmioaddr[i] >= ADLP_PIPE_MMIO_START && mmioaddr[i] <= ADLP_PIPE_MMIO_END)))
+ return false;
+ }
+ } else if (IS_TIGERLAKE(i915) || IS_DG1(i915) || IS_ALDERLAKE_S(i915))
+ for (i = 0; i < mmio_count; i++) {
+ if ((!(mmioaddr[i] >= TGL_MAIN_MMIO_START && mmioaddr[i] <= TGL_MAIN_MMIO_END) ||
+ (mmioaddr[i] >= TGL_PIPEA_MMIO_START && mmioaddr[i] <= TGL_PIPEA_MMIO_END) ||
+ (mmioaddr[i] >= TGL_PIPEB_MMIO_START && mmioaddr[i] <= TGL_PIPEB_MMIO_END) ||
+ (mmioaddr[i] >= TGL_PIPEC_MMIO_START && mmioaddr[i] <= TGL_PIPEC_MMIO_END) ||
+ (mmioaddr[i] >= TGL_PIPED_MMIO_START && mmioaddr[i] <= TGL_PIPEC_MMIO_END)))
+ return false;
+ }
+ return true;
+}
+
static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
const struct intel_dmc_header_base *dmc_header,
size_t rem_size, u8 dmc_id)
@@ -443,6 +479,12 @@ static u32 parse_dmc_fw_header(struct intel_dmc *dmc,
return 0;
}
+ if (dmc_header->header_ver == 3) {
+ if (!dmc_mmio_addr_sanity_check(dmc, mmioaddr, mmio_count))
+ drm_err(&i915->drm, "DMC firmware has Wrong MMIO Addresses\n");
+ return 0;
+ }
+
for (i = 0; i < mmio_count; i++) {
dmc_info->mmioaddr[i] = _MMIO(mmioaddr[i]);
dmc_info->mmiodata[i] = mmiodata[i];
--
2.25.1
^ permalink raw reply related [flat|nested] 34+ messages in thread
end of thread, other threads:[~2022-05-06 17:39 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-27 0:35 [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions Anusha Srivatsa
2022-04-27 0:35 ` Anusha Srivatsa
2022-04-27 1:44 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/dmc: Add MMIO range restrictions (rev3) Patchwork
2022-04-27 2:10 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-04-27 4:26 ` [Intel-gfx] [PATCH] drm/i915/dmc: Add MMIO range restrictions kernel test robot
2022-04-27 4:26 ` kernel test robot
2022-04-27 5:41 ` Lucas De Marchi
2022-04-27 5:41 ` [Intel-gfx] " Lucas De Marchi
2022-04-29 20:39 ` Srivatsa, Anusha
2022-04-29 20:39 ` Srivatsa, Anusha
2022-04-29 20:49 ` Lucas De Marchi
2022-04-29 20:49 ` [Intel-gfx] " Lucas De Marchi
2022-04-29 22:57 ` Srivatsa, Anusha
2022-04-29 22:57 ` [Intel-gfx] " Srivatsa, Anusha
2022-05-02 18:09 ` Lucas De Marchi
2022-04-27 7:49 ` kernel test robot
2022-04-27 7:49 ` kernel test robot
2022-04-27 12:42 ` Andi Shyti
-- strict thread matches above, loose matches on Subject: below --
2022-05-06 17:35 Anusha Srivatsa
2022-05-04 18:32 Anusha Srivatsa
2022-05-04 0:13 Anusha Srivatsa
2022-05-04 0:31 ` Lucas De Marchi
2022-05-04 0:36 ` Srivatsa, Anusha
2022-05-03 23:36 Anusha Srivatsa
2022-05-03 22:04 Anusha Srivatsa
2022-04-05 17:14 Anusha Srivatsa
2022-04-05 18:02 ` Lucas De Marchi
2022-04-06 17:16 ` Srivatsa, Anusha
2022-04-06 17:46 ` Lucas De Marchi
2022-04-06 20:53 ` Srivatsa, Anusha
2022-04-25 18:16 ` Lucas De Marchi
2022-04-05 23:18 ` kernel test robot
2022-04-06 6:58 ` kernel test robot
2022-04-05 0:35 Anusha Srivatsa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.