* [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
@ 2022-10-13 17:56 Jonathan Cavitt
2022-10-13 19:28 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Jonathan Cavitt @ 2022-10-13 17:56 UTC (permalink / raw)
To: intel-gfx; +Cc: jonathan.cavitt, matthew.auld, andrzej.hajda, nirmoy.das
i915_ttm_to_gem can return a NULL pointer, which is
dereferenced in i915_ttm_access_memory without first
checking if it is NULL. Inspecting
i915_ttm_io_mem_reserve, it appears the correct
behavior in this case is to return -EINVAL.
Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
Suggested-by: John C Harrison <John.C.Harrison@intel.com>
CC: Matthew Auld <matthew.auld@intel.com>
CC: Andrzej Hajda <andrzej.hajda@intel.com>
CC: Nirmoy Das <nirmoy.das@intel.com>
CC: Andi Shyti <andi.shyti@linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index d63f30efd631..b569624f2ed9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct ttm_buffer_object *bo,
int len, int write)
{
struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
- resource_size_t iomap = obj->mm.region->iomap.base -
- obj->mm.region->region.start;
+ resource_size_t iomap;
unsigned long page = offset >> PAGE_SHIFT;
unsigned long bytes_left = len;
+ if (!obj)
+ return -EINVAL;
+
+ iomap = obj->mm.region->iomap.base -
+ obj->mm.region->region.start;
+
/*
* TODO: For now just let it fail if the resource is non-mappable,
* otherwise we need to perform the memcpy from the gpu here, without
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/ttm: Fix access_memory null pointer exception
2022-10-13 17:56 [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception Jonathan Cavitt
@ 2022-10-13 19:28 ` Patchwork
2022-10-13 23:07 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2022-10-13 19:28 UTC (permalink / raw)
To: Jonathan Cavitt; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 6925 bytes --]
== Series Details ==
Series: drm/i915/ttm: Fix access_memory null pointer exception
URL : https://patchwork.freedesktop.org/series/109677/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12242 -> Patchwork_109677v1
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/index.html
Participating hosts (45 -> 34)
------------------------------
Additional (3): fi-tgl-u2 bat-atsm-1 fi-pnv-d510
Missing (14): fi-ilk-m540 fi-hsw-4200u bat-dg2-8 bat-adlm-1 bat-dg2-9 bat-adlp-6 bat-adlp-4 bat-adln-1 bat-jsl-3 bat-rplp-1 bat-rpls-1 bat-rpls-2 bat-dg2-11 bat-jsl-1
Known issues
------------
Here are the changes found in Patchwork_109677v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_huc_copy@huc-copy:
- fi-tgl-u2: NOTRUN -> [SKIP][1] ([i915#2190])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-tgl-u2/igt@gem_huc_copy@huc-copy.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-bxt-dsi: [PASS][2] -> [DMESG-FAIL][3] ([i915#5334])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/fi-bxt-dsi/igt@i915_selftest@live@gt_heartbeat.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-bxt-dsi/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@hangcheck:
- fi-hsw-4770: [PASS][4] -> [INCOMPLETE][5] ([i915#4785])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
* igt@i915_suspend@basic-s2idle-without-i915:
- fi-apl-guc: [PASS][6] -> [DMESG-WARN][7] ([i915#180] / [i915#5904] / [i915#62])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/fi-apl-guc/igt@i915_suspend@basic-s2idle-without-i915.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-apl-guc/igt@i915_suspend@basic-s2idle-without-i915.html
* igt@kms_chamelium@hdmi-edid-read:
- fi-tgl-u2: NOTRUN -> [SKIP][8] ([fdo#109284] / [fdo#111827]) +7 similar issues
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-tgl-u2/igt@kms_chamelium@hdmi-edid-read.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
- fi-tgl-u2: NOTRUN -> [SKIP][9] ([i915#4103])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-tgl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html
* igt@kms_force_connector_basic@force-load-detect:
- fi-tgl-u2: NOTRUN -> [SKIP][10] ([fdo#109285])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-tgl-u2/igt@kms_force_connector_basic@force-load-detect.html
* igt@kms_psr@primary_page_flip:
- fi-pnv-d510: NOTRUN -> [SKIP][11] ([fdo#109271]) +43 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-pnv-d510/igt@kms_psr@primary_page_flip.html
* igt@kms_setmode@basic-clone-single-crtc:
- fi-tgl-u2: NOTRUN -> [SKIP][12] ([i915#3555])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-tgl-u2/igt@kms_setmode@basic-clone-single-crtc.html
* igt@runner@aborted:
- fi-hsw-4770: NOTRUN -> [FAIL][13] ([fdo#109271] / [i915#4312] / [i915#5594])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-hsw-4770/igt@runner@aborted.html
#### Possible fixes ####
* igt@i915_pm_rpm@module-reload:
- {fi-tgl-mst}: [DMESG-WARN][14] ([i915#5537]) -> [PASS][15]
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/fi-tgl-mst/igt@i915_pm_rpm@module-reload.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-tgl-mst/igt@i915_pm_rpm@module-reload.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions:
- fi-bsw-kefka: [FAIL][16] ([i915#6298]) -> [PASS][17]
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.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#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
[i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
[i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#5537]: https://gitlab.freedesktop.org/drm/intel/issues/5537
[i915#5594]: https://gitlab.freedesktop.org/drm/intel/issues/5594
[i915#5904]: https://gitlab.freedesktop.org/drm/intel/issues/5904
[i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
[i915#6298]: https://gitlab.freedesktop.org/drm/intel/issues/6298
[i915#6596]: https://gitlab.freedesktop.org/drm/intel/issues/6596
[i915#7029]: https://gitlab.freedesktop.org/drm/intel/issues/7029
[i915#7030]: https://gitlab.freedesktop.org/drm/intel/issues/7030
[i915#7031]: https://gitlab.freedesktop.org/drm/intel/issues/7031
Build changes
-------------
* Linux: CI_DRM_12242 -> Patchwork_109677v1
CI-20190529: 20190529
CI_DRM_12242: 075a81b1efd29300194bdf7877e08b6dbe3079d9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7012: ca6f5bdd537d26692c4b1ca011b8c4f227d95703 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_109677v1: 075a81b1efd29300194bdf7877e08b6dbe3079d9 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
62b5b4cc3e7d drm/i915/ttm: Fix access_memory null pointer exception
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/index.html
[-- Attachment #2: Type: text/html, Size: 7218 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/ttm: Fix access_memory null pointer exception
2022-10-13 17:56 [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception Jonathan Cavitt
2022-10-13 19:28 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
@ 2022-10-13 23:07 ` Patchwork
2022-10-14 8:39 ` [Intel-gfx] [PATCH] " Matthew Auld
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2022-10-13 23:07 UTC (permalink / raw)
To: Jonathan Cavitt; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 33432 bytes --]
== Series Details ==
Series: drm/i915/ttm: Fix access_memory null pointer exception
URL : https://patchwork.freedesktop.org/series/109677/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12242_full -> Patchwork_109677v1_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (9 -> 9)
------------------------------
No changes in participating hosts
Known issues
------------
Here are the changes found in Patchwork_109677v1_full that come from known issues:
### CI changes ###
#### Issues hit ####
* boot:
- shard-apl: ([PASS][1], [PASS][2], [PASS][3], [PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25]) -> ([PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [FAIL][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50]) ([i915#4386])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl8/boot.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl8/boot.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl8/boot.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl8/boot.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl7/boot.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl7/boot.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl7/boot.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl7/boot.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl6/boot.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl6/boot.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl6/boot.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl6/boot.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/boot.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/boot.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/boot.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/boot.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/boot.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl2/boot.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl2/boot.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl2/boot.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl2/boot.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl1/boot.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl1/boot.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl1/boot.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl1/boot.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/boot.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl8/boot.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl8/boot.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl8/boot.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl8/boot.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl1/boot.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl1/boot.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl7/boot.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl7/boot.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl7/boot.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl1/boot.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl1/boot.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl2/boot.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl2/boot.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl7/boot.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl6/boot.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl6/boot.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl2/boot.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl6/boot.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl6/boot.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/boot.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl2/boot.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/boot.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/boot.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/boot.html
#### Possible fixes ####
* boot:
- shard-skl: ([PASS][51], [PASS][52], [PASS][53], [PASS][54], [PASS][55], [PASS][56], [PASS][57], [PASS][58], [PASS][59], [PASS][60], [PASS][61], [FAIL][62], [PASS][63], [FAIL][64], [PASS][65], [PASS][66], [PASS][67], [PASS][68], [PASS][69], [PASS][70], [PASS][71], [PASS][72], [PASS][73], [PASS][74], [PASS][75]) ([i915#5032]) -> ([PASS][76], [PASS][77], [PASS][78], [PASS][79], [PASS][80], [PASS][81], [PASS][82], [PASS][83], [PASS][84], [PASS][85], [PASS][86], [PASS][87], [PASS][88], [PASS][89], [PASS][90], [PASS][91], [PASS][92], [PASS][93], [PASS][94], [PASS][95], [PASS][96], [PASS][97], [PASS][98])
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl9/boot.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl9/boot.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl9/boot.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl9/boot.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl7/boot.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl7/boot.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl7/boot.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/boot.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/boot.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/boot.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl5/boot.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl5/boot.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl5/boot.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl5/boot.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl4/boot.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl4/boot.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl4/boot.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl4/boot.html
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl1/boot.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl1/boot.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl1/boot.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl10/boot.html
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl10/boot.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl10/boot.html
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl10/boot.html
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl9/boot.html
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl9/boot.html
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl9/boot.html
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl7/boot.html
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl7/boot.html
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl7/boot.html
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl7/boot.html
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl6/boot.html
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl6/boot.html
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl6/boot.html
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl5/boot.html
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl5/boot.html
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl4/boot.html
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl4/boot.html
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl4/boot.html
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/boot.html
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/boot.html
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/boot.html
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/boot.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl10/boot.html
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl10/boot.html
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl10/boot.html
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl10/boot.html
### IGT changes ###
#### Issues hit ####
* igt@gem_eio@reset-stress:
- shard-tglb: [PASS][99] -> [FAIL][100] ([i915#5784])
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-tglb5/igt@gem_eio@reset-stress.html
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-tglb5/igt@gem_eio@reset-stress.html
* igt@gem_exec_balancer@parallel-keep-in-fence:
- shard-iclb: [PASS][101] -> [SKIP][102] ([i915#4525]) +2 similar issues
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@gem_exec_balancer@parallel-keep-in-fence.html
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb3/igt@gem_exec_balancer@parallel-keep-in-fence.html
* igt@gem_exec_endless@dispatch@vcs1:
- shard-tglb: [PASS][103] -> [INCOMPLETE][104] ([i915#3778])
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-tglb7/igt@gem_exec_endless@dispatch@vcs1.html
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-tglb7/igt@gem_exec_endless@dispatch@vcs1.html
* igt@gem_exec_fair@basic-pace@vecs0:
- shard-iclb: [PASS][105] -> [FAIL][106] ([i915#2842])
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb8/igt@gem_exec_fair@basic-pace@vecs0.html
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb6/igt@gem_exec_fair@basic-pace@vecs0.html
* igt@gem_lmem_swapping@heavy-random:
- shard-skl: NOTRUN -> [SKIP][107] ([fdo#109271] / [i915#4613]) +1 similar issue
[107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/igt@gem_lmem_swapping@heavy-random.html
* igt@gem_userptr_blits@dmabuf-sync:
- shard-skl: NOTRUN -> [SKIP][108] ([fdo#109271] / [i915#3323])
[108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl9/igt@gem_userptr_blits@dmabuf-sync.html
* igt@i915_pipe_stress@stress-xrgb8888-untiled:
- shard-apl: NOTRUN -> [FAIL][109] ([i915#7036])
[109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/igt@i915_pipe_stress@stress-xrgb8888-untiled.html
- shard-skl: NOTRUN -> [FAIL][110] ([i915#7036])
[110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl4/igt@i915_pipe_stress@stress-xrgb8888-untiled.html
* igt@kms_addfb_basic@legacy-format:
- shard-tglb: [PASS][111] -> [INCOMPLETE][112] ([i915#6987])
[111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-tglb5/igt@kms_addfb_basic@legacy-format.html
[112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-tglb5/igt@kms_addfb_basic@legacy-format.html
* igt@kms_async_flips@alternate-sync-async-flip@pipe-b-edp-1:
- shard-skl: [PASS][113] -> [FAIL][114] ([i915#2521])
[113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl10/igt@kms_async_flips@alternate-sync-async-flip@pipe-b-edp-1.html
[114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl10/igt@kms_async_flips@alternate-sync-async-flip@pipe-b-edp-1.html
* igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
- shard-skl: NOTRUN -> [SKIP][115] ([fdo#109271] / [i915#3886]) +2 similar issues
[115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html
* igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_gen12_rc_ccs_cc:
- shard-apl: NOTRUN -> [SKIP][116] ([fdo#109271] / [i915#3886]) +2 similar issues
[116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl7/igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_gen12_rc_ccs_cc.html
- shard-glk: NOTRUN -> [SKIP][117] ([fdo#109271] / [i915#3886])
[117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-glk1/igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_gen12_rc_ccs_cc.html
* igt@kms_ccs@pipe-b-bad-rotation-90-4_tiled_dg2_rc_ccs_cc:
- shard-apl: NOTRUN -> [SKIP][118] ([fdo#109271]) +52 similar issues
[118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/igt@kms_ccs@pipe-b-bad-rotation-90-4_tiled_dg2_rc_ccs_cc.html
* igt@kms_ccs@pipe-c-crc-sprite-planes-basic-4_tiled_dg2_rc_ccs:
- shard-glk: NOTRUN -> [SKIP][119] ([fdo#109271]) +27 similar issues
[119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-glk1/igt@kms_ccs@pipe-c-crc-sprite-planes-basic-4_tiled_dg2_rc_ccs.html
* igt@kms_chamelium@hdmi-hpd-storm:
- shard-apl: NOTRUN -> [SKIP][120] ([fdo#109271] / [fdo#111827]) +3 similar issues
[120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl7/igt@kms_chamelium@hdmi-hpd-storm.html
- shard-glk: NOTRUN -> [SKIP][121] ([fdo#109271] / [fdo#111827])
[121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-glk1/igt@kms_chamelium@hdmi-hpd-storm.html
* igt@kms_chamelium@hdmi-hpd-with-enabled-mode:
- shard-skl: NOTRUN -> [SKIP][122] ([fdo#109271] / [fdo#111827]) +6 similar issues
[122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl4/igt@kms_chamelium@hdmi-hpd-with-enabled-mode.html
* igt@kms_color@ctm-0-25:
- shard-skl: NOTRUN -> [SKIP][123] ([fdo#109271] / [i915#3546])
[123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/igt@kms_color@ctm-0-25.html
* igt@kms_cursor_legacy@flip-vs-cursor@toggle:
- shard-iclb: [PASS][124] -> [FAIL][125] ([i915#2346]) +3 similar issues
[124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb6/igt@kms_cursor_legacy@flip-vs-cursor@toggle.html
[125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb7/igt@kms_cursor_legacy@flip-vs-cursor@toggle.html
* igt@kms_fbcon_fbt@psr-suspend:
- shard-skl: NOTRUN -> [INCOMPLETE][126] ([i915#1982] / [i915#4939])
[126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl9/igt@kms_fbcon_fbt@psr-suspend.html
* igt@kms_flip@flip-vs-expired-vblank@b-edp1:
- shard-skl: [PASS][127] -> [FAIL][128] ([i915#79])
[127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl1/igt@kms_flip@flip-vs-expired-vblank@b-edp1.html
[128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl5/igt@kms_flip@flip-vs-expired-vblank@b-edp1.html
* igt@kms_flip@flip-vs-expired-vblank@c-edp1:
- shard-skl: [PASS][129] -> [FAIL][130] ([i915#2122]) +2 similar issues
[129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl1/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
[130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl5/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
* igt@kms_flip@flip-vs-suspend@b-dp1:
- shard-apl: [PASS][131] -> [DMESG-WARN][132] ([i915#180]) +1 similar issue
[131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl1/igt@kms_flip@flip-vs-suspend@b-dp1.html
[132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl1/igt@kms_flip@flip-vs-suspend@b-dp1.html
* igt@kms_flip@plain-flip-ts-check-interruptible@b-hdmi-a1:
- shard-glk: [PASS][133] -> [FAIL][134] ([i915#2122])
[133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk6/igt@kms_flip@plain-flip-ts-check-interruptible@b-hdmi-a1.html
[134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-glk5/igt@kms_flip@plain-flip-ts-check-interruptible@b-hdmi-a1.html
* igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-upscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][135] ([i915#2672]) +3 similar issues
[135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-upscaling@pipe-a-default-mode.html
* igt@kms_frontbuffer_tracking@fbcpsr-suspend:
- shard-skl: NOTRUN -> [SKIP][136] ([fdo#109271]) +145 similar issues
[136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt:
- shard-skl: [PASS][137] -> [DMESG-WARN][138] ([i915#1982]) +1 similar issue
[137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl4/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt.html
[138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt.html
* igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-a-dp-1:
- shard-apl: NOTRUN -> [FAIL][139] ([i915#4573]) +2 similar issues
[139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-a-dp-1.html
* igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-b-edp-1:
- shard-skl: NOTRUN -> [FAIL][140] ([i915#4573]) +2 similar issues
[140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl4/igt@kms_plane_alpha_blend@alpha-opaque-fb@pipe-b-edp-1.html
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-5@pipe-a-edp-1:
- shard-iclb: [PASS][141] -> [SKIP][142] ([i915#5235]) +2 similar issues
[141]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb3/igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-5@pipe-a-edp-1.html
[142]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb2/igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-5@pipe-a-edp-1.html
* igt@kms_psr2_su@page_flip-xrgb8888:
- shard-skl: NOTRUN -> [SKIP][143] ([fdo#109271] / [i915#658]) +2 similar issues
[143]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl9/igt@kms_psr2_su@page_flip-xrgb8888.html
* igt@kms_psr@psr2_primary_mmap_cpu:
- shard-iclb: [PASS][144] -> [SKIP][145] ([fdo#109441]) +3 similar issues
[144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
[145]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb5/igt@kms_psr@psr2_primary_mmap_cpu.html
* igt@kms_psr@suspend:
- shard-skl: [PASS][146] -> [INCOMPLETE][147] ([i915#4939])
[146]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl1/igt@kms_psr@suspend.html
[147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/igt@kms_psr@suspend.html
* igt@kms_writeback@writeback-invalid-parameters:
- shard-skl: NOTRUN -> [SKIP][148] ([fdo#109271] / [i915#2437])
[148]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/igt@kms_writeback@writeback-invalid-parameters.html
* igt@sysfs_clients@create:
- shard-apl: NOTRUN -> [SKIP][149] ([fdo#109271] / [i915#2994])
[149]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/igt@sysfs_clients@create.html
- shard-skl: NOTRUN -> [SKIP][150] ([fdo#109271] / [i915#2994])
[150]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl4/igt@sysfs_clients@create.html
#### Possible fixes ####
* igt@gen9_exec_parse@allowed-all:
- shard-skl: [DMESG-WARN][151] ([i915#5566] / [i915#716]) -> [PASS][152]
[151]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl10/igt@gen9_exec_parse@allowed-all.html
[152]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl4/igt@gen9_exec_parse@allowed-all.html
- shard-apl: [DMESG-WARN][153] ([i915#5566] / [i915#716]) -> [PASS][154]
[153]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl2/igt@gen9_exec_parse@allowed-all.html
[154]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/igt@gen9_exec_parse@allowed-all.html
* igt@i915_pm_dc@dc9-dpms:
- shard-iclb: [SKIP][155] ([i915#4281]) -> [PASS][156]
[155]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb3/igt@i915_pm_dc@dc9-dpms.html
[156]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb1/igt@i915_pm_dc@dc9-dpms.html
* igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions:
- shard-glk: [FAIL][157] ([i915#2346]) -> [PASS][158]
[157]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk9/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html
[158]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-glk6/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html
* igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2:
- shard-glk: [FAIL][159] ([i915#2122]) -> [PASS][160]
[159]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk8/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html
[160]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-glk3/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible@ab-hdmi-a1-hdmi-a2.html
* igt@kms_flip@busy-flip@c-edp1:
- shard-skl: [FAIL][161] -> [PASS][162] +1 similar issue
[161]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl5/igt@kms_flip@busy-flip@c-edp1.html
[162]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/igt@kms_flip@busy-flip@c-edp1.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1:
- shard-skl: [FAIL][163] ([i915#79]) -> [PASS][164]
[163]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
[164]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
* igt@kms_flip_scaled_crc@flip-64bpp-linear-to-16bpp-linear-downscaling@pipe-a-default-mode:
- shard-iclb: [SKIP][165] ([i915#3555]) -> [PASS][166] +1 similar issue
[165]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-linear-to-16bpp-linear-downscaling@pipe-a-default-mode.html
[166]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb3/igt@kms_flip_scaled_crc@flip-64bpp-linear-to-16bpp-linear-downscaling@pipe-a-default-mode.html
* igt@kms_hdr@bpc-switch-suspend@pipe-a-dp-1:
- shard-apl: [DMESG-WARN][167] ([i915#180]) -> [PASS][168] +1 similar issue
[167]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/igt@kms_hdr@bpc-switch-suspend@pipe-a-dp-1.html
[168]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl7/igt@kms_hdr@bpc-switch-suspend@pipe-a-dp-1.html
* igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-c-edp-1:
- shard-iclb: [SKIP][169] ([i915#5235]) -> [PASS][170] +2 similar issues
[169]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-c-edp-1.html
[170]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb3/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-c-edp-1.html
* igt@kms_psr@psr2_primary_render:
- shard-iclb: [SKIP][171] ([fdo#109441]) -> [PASS][172] +1 similar issue
[171]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb7/igt@kms_psr@psr2_primary_render.html
[172]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb2/igt@kms_psr@psr2_primary_render.html
* igt@perf@stress-open-close:
- shard-glk: [INCOMPLETE][173] ([i915#5213]) -> [PASS][174]
[173]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-glk1/igt@perf@stress-open-close.html
[174]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-glk1/igt@perf@stress-open-close.html
#### Warnings ####
* igt@gem_exec_balancer@parallel-ordering:
- shard-iclb: [SKIP][175] ([i915#4525]) -> [FAIL][176] ([i915#6117])
[175]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb3/igt@gem_exec_balancer@parallel-ordering.html
[176]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb1/igt@gem_exec_balancer@parallel-ordering.html
* igt@i915_pm_sseu@full-enable:
- shard-skl: [FAIL][177] ([i915#3524]) -> [FAIL][178] ([i915#7084])
[177]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-skl5/igt@i915_pm_sseu@full-enable.html
[178]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-skl1/igt@i915_pm_sseu@full-enable.html
* igt@kms_psr2_sf@cursor-plane-move-continuous-sf:
- shard-iclb: [SKIP][179] ([i915#658]) -> [SKIP][180] ([i915#2920])
[179]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb1/igt@kms_psr2_sf@cursor-plane-move-continuous-sf.html
[180]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb2/igt@kms_psr2_sf@cursor-plane-move-continuous-sf.html
* igt@kms_psr2_sf@cursor-plane-update-sf:
- shard-iclb: [SKIP][181] ([fdo#111068] / [i915#658]) -> [SKIP][182] ([i915#2920])
[181]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb7/igt@kms_psr2_sf@cursor-plane-update-sf.html
[182]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb2/igt@kms_psr2_sf@cursor-plane-update-sf.html
* igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf:
- shard-iclb: [SKIP][183] ([i915#2920]) -> [SKIP][184] ([i915#658]) +1 similar issue
[183]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html
[184]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb3/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html
* igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area:
- shard-iclb: [SKIP][185] ([i915#2920]) -> [SKIP][186] ([fdo#111068] / [i915#658])
[185]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-iclb2/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area.html
[186]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-iclb3/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area.html
* igt@runner@aborted:
- shard-apl: ([FAIL][187], [FAIL][188], [FAIL][189], [FAIL][190], [FAIL][191]) ([fdo#109271] / [i915#3002] / [i915#4312]) -> ([FAIL][192], [FAIL][193], [FAIL][194], [FAIL][195]) ([i915#180] / [i915#3002] / [i915#4312])
[187]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/igt@runner@aborted.html
[188]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl3/igt@runner@aborted.html
[189]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl6/igt@runner@aborted.html
[190]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl2/igt@runner@aborted.html
[191]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12242/shard-apl8/igt@runner@aborted.html
[192]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl6/igt@runner@aborted.html
[193]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl3/igt@runner@aborted.html
[194]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl1/igt@runner@aborted.html
[195]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/shard-apl8/igt@runner@aborted.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
[i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
[i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
[i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
[i915#3323]: https://gitlab.freedesktop.org/drm/intel/issues/3323
[i915#3524]: https://gitlab.freedesktop.org/drm/intel/issues/3524
[i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3778]: https://gitlab.freedesktop.org/drm/intel/issues/3778
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4386]: https://gitlab.freedesktop.org/drm/intel/issues/4386
[i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
[i915#4573]: https://gitlab.freedesktop.org/drm/intel/issues/4573
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4939]: https://gitlab.freedesktop.org/drm/intel/issues/4939
[i915#5032]: https://gitlab.freedesktop.org/drm/intel/issues/5032
[i915#5213]: https://gitlab.freedesktop.org/drm/intel/issues/5213
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
[i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
[i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#6987]: https://gitlab.freedesktop.org/drm/intel/issues/6987
[i915#7036]: https://gitlab.freedesktop.org/drm/intel/issues/7036
[i915#7084]: https://gitlab.freedesktop.org/drm/intel/issues/7084
[i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
Build changes
-------------
* Linux: CI_DRM_12242 -> Patchwork_109677v1
CI-20190529: 20190529
CI_DRM_12242: 075a81b1efd29300194bdf7877e08b6dbe3079d9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7012: ca6f5bdd537d26692c4b1ca011b8c4f227d95703 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_109677v1: 075a81b1efd29300194bdf7877e08b6dbe3079d9 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109677v1/index.html
[-- Attachment #2: Type: text/html, Size: 38584 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-13 17:56 [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception Jonathan Cavitt
2022-10-13 19:28 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2022-10-13 23:07 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
@ 2022-10-14 8:39 ` Matthew Auld
2022-10-14 8:56 ` Andi Shyti
2022-10-14 9:27 ` Das, Nirmoy
2022-10-14 8:47 ` Andi Shyti
` (2 subsequent siblings)
5 siblings, 2 replies; 15+ messages in thread
From: Matthew Auld @ 2022-10-14 8:39 UTC (permalink / raw)
To: Jonathan Cavitt, intel-gfx; +Cc: andrzej.hajda, nirmoy.das
On 13/10/2022 18:56, Jonathan Cavitt wrote:
> i915_ttm_to_gem can return a NULL pointer, which is
> dereferenced in i915_ttm_access_memory without first
> checking if it is NULL. Inspecting
> i915_ttm_io_mem_reserve, it appears the correct
> behavior in this case is to return -EINVAL.
The GEM object has already been dereferenced before this point, if you
look at the caller (vm_access_ttm). The NULL obj thing is to identify
"ttm ghost objects", and I don't think a normal userpace object can
suddenly become one (access_memory comes from ptrace). AFAIK ghost
objects are just for temporarily hanging on to some memory/state, while
the dma-resv is busy. In the places where ttm is the one giving us the
object, then it might be possible to see these types of objects, since
ttm could in theory pass one in (like during eviction).
>
> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
> CC: Matthew Auld <matthew.auld@intel.com>
> CC: Andrzej Hajda <andrzej.hajda@intel.com>
> CC: Nirmoy Das <nirmoy.das@intel.com>
> CC: Andi Shyti <andi.shyti@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index d63f30efd631..b569624f2ed9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct ttm_buffer_object *bo,
> int len, int write)
> {
> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> - resource_size_t iomap = obj->mm.region->iomap.base -
> - obj->mm.region->region.start;
> + resource_size_t iomap;
> unsigned long page = offset >> PAGE_SHIFT;
> unsigned long bytes_left = len;
>
> + if (!obj)
> + return -EINVAL;
> +
> + iomap = obj->mm.region->iomap.base -
> + obj->mm.region->region.start;
> +
> /*
> * TODO: For now just let it fail if the resource is non-mappable,
> * otherwise we need to perform the memcpy from the gpu here, without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-13 17:56 [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception Jonathan Cavitt
` (2 preceding siblings ...)
2022-10-14 8:39 ` [Intel-gfx] [PATCH] " Matthew Auld
@ 2022-10-14 8:47 ` Andi Shyti
2022-10-14 9:02 ` Andrzej Hajda
2022-10-14 9:52 ` Tvrtko Ursulin
5 siblings, 0 replies; 15+ messages in thread
From: Andi Shyti @ 2022-10-14 8:47 UTC (permalink / raw)
To: Jonathan Cavitt; +Cc: intel-gfx, matthew.auld, andrzej.hajda, nirmoy.das
Hi Jonathan,
On Thu, Oct 13, 2022 at 10:56:50AM -0700, Jonathan Cavitt wrote:
> i915_ttm_to_gem can return a NULL pointer, which is
> dereferenced in i915_ttm_access_memory without first
> checking if it is NULL. Inspecting
> i915_ttm_io_mem_reserve, it appears the correct
> behavior in this case is to return -EINVAL.
>
> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
> CC: Matthew Auld <matthew.auld@intel.com>
> CC: Andrzej Hajda <andrzej.hajda@intel.com>
> CC: Nirmoy Das <nirmoy.das@intel.com>
> CC: Andi Shyti <andi.shyti@linux.intel.com>
Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>
Thanks,
Andi
> ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index d63f30efd631..b569624f2ed9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct ttm_buffer_object *bo,
> int len, int write)
> {
> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> - resource_size_t iomap = obj->mm.region->iomap.base -
> - obj->mm.region->region.start;
> + resource_size_t iomap;
> unsigned long page = offset >> PAGE_SHIFT;
> unsigned long bytes_left = len;
>
> + if (!obj)
> + return -EINVAL;
> +
> + iomap = obj->mm.region->iomap.base -
> + obj->mm.region->region.start;
> +
> /*
> * TODO: For now just let it fail if the resource is non-mappable,
> * otherwise we need to perform the memcpy from the gpu here, without
> --
> 2.25.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-14 8:39 ` [Intel-gfx] [PATCH] " Matthew Auld
@ 2022-10-14 8:56 ` Andi Shyti
2022-10-14 9:44 ` Matthew Auld
2022-10-14 9:27 ` Das, Nirmoy
1 sibling, 1 reply; 15+ messages in thread
From: Andi Shyti @ 2022-10-14 8:56 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx, Jonathan Cavitt, andrzej.hajda, nirmoy.das
On Fri, Oct 14, 2022 at 09:39:52AM +0100, Matthew Auld wrote:
> On 13/10/2022 18:56, Jonathan Cavitt wrote:
> > i915_ttm_to_gem can return a NULL pointer, which is
> > dereferenced in i915_ttm_access_memory without first
> > checking if it is NULL. Inspecting
> > i915_ttm_io_mem_reserve, it appears the correct
> > behavior in this case is to return -EINVAL.
>
> The GEM object has already been dereferenced before this point, if you look
> at the caller (vm_access_ttm). The NULL obj thing is to identify "ttm ghost
> objects", and I don't think a normal userpace object can suddenly become one
> (access_memory comes from ptrace). AFAIK ghost objects are just for
> temporarily hanging on to some memory/state, while the dma-resv is busy. In
> the places where ttm is the one giving us the object, then it might be
> possible to see these types of objects, since ttm could in theory pass one
> in (like during eviction).
True that, but because from a code persepctive we can still receive
NULL, I think the check is correct, perhaps we could:
if (unlikely(!obj))
return -EINVAL;
Andi
> > Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > Suggested-by: John C Harrison <John.C.Harrison@intel.com>
> > CC: Matthew Auld <matthew.auld@intel.com>
> > CC: Andrzej Hajda <andrzej.hajda@intel.com>
> > CC: Nirmoy Das <nirmoy.das@intel.com>
> > CC: Andi Shyti <andi.shyti@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > index d63f30efd631..b569624f2ed9 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> > @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct ttm_buffer_object *bo,
> > int len, int write)
> > {
> > struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> > - resource_size_t iomap = obj->mm.region->iomap.base -
> > - obj->mm.region->region.start;
> > + resource_size_t iomap;
> > unsigned long page = offset >> PAGE_SHIFT;
> > unsigned long bytes_left = len;
> > + if (!obj)
> > + return -EINVAL;
> > +
> > + iomap = obj->mm.region->iomap.base -
> > + obj->mm.region->region.start;
> > +
> > /*
> > * TODO: For now just let it fail if the resource is non-mappable,
> > * otherwise we need to perform the memcpy from the gpu here, without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-13 17:56 [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception Jonathan Cavitt
` (3 preceding siblings ...)
2022-10-14 8:47 ` Andi Shyti
@ 2022-10-14 9:02 ` Andrzej Hajda
2022-10-14 9:52 ` Tvrtko Ursulin
5 siblings, 0 replies; 15+ messages in thread
From: Andrzej Hajda @ 2022-10-14 9:02 UTC (permalink / raw)
To: Jonathan Cavitt, intel-gfx; +Cc: matthew.auld, nirmoy.das
On 13.10.2022 19:56, Jonathan Cavitt wrote:
> i915_ttm_to_gem can return a NULL pointer, which is
> dereferenced in i915_ttm_access_memory without first
> checking if it is NULL. Inspecting
> i915_ttm_io_mem_reserve, it appears the correct
> behavior in this case is to return -EINVAL.
>
> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
> CC: Matthew Auld <matthew.auld@intel.com>
> CC: Andrzej Hajda <andrzej.hajda@intel.com>
> CC: Nirmoy Das <nirmoy.das@intel.com>
> CC: Andi Shyti <andi.shyti@linux.intel.com> > ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index d63f30efd631..b569624f2ed9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct ttm_buffer_object *bo,
> int len, int write)
> {
> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> - resource_size_t iomap = obj->mm.region->iomap.base -
> - obj->mm.region->region.start;
> + resource_size_t iomap;
> unsigned long page = offset >> PAGE_SHIFT;
> unsigned long bytes_left = len;
>
> + if (!obj)
> + return -EINVAL;
> +
> + iomap = obj->mm.region->iomap.base -
> + obj->mm.region->region.start;
> +
There are 4 occurrences of this subtraction in the code, enough for
helper :) ?
Anyway:
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Regards
Andrzej
> /*
> * TODO: For now just let it fail if the resource is non-mappable,
> * otherwise we need to perform the memcpy from the gpu here, without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-14 8:39 ` [Intel-gfx] [PATCH] " Matthew Auld
2022-10-14 8:56 ` Andi Shyti
@ 2022-10-14 9:27 ` Das, Nirmoy
2022-10-14 10:13 ` Matthew Auld
1 sibling, 1 reply; 15+ messages in thread
From: Das, Nirmoy @ 2022-10-14 9:27 UTC (permalink / raw)
To: Matthew Auld, Jonathan Cavitt, intel-gfx; +Cc: andrzej.hajda
Hi Matt
On 10/14/2022 10:39 AM, Matthew Auld wrote:
> On 13/10/2022 18:56, Jonathan Cavitt wrote:
>> i915_ttm_to_gem can return a NULL pointer, which is
>> dereferenced in i915_ttm_access_memory without first
>> checking if it is NULL. Inspecting
>> i915_ttm_io_mem_reserve, it appears the correct
>> behavior in this case is to return -EINVAL.
>
> The GEM object has already been dereferenced before this point, if you
> look at the caller (vm_access_ttm). The NULL obj thing is to identify
> "ttm ghost objects", and I don't think a normal userpace object can
> suddenly become one (access_memory comes from ptrace). AFAIK ghost
> objects are just for temporarily hanging on to some memory/state,
> while the dma-resv is busy. In the places where ttm is the one giving
> us the object, then it might be possible to see these types of
> objects, since ttm could in theory pass one in (like during eviction).
Yes, we should not hit this. Thanks for the nice "ttm ghost objects"
reminder :)
I think we can still have this check to avoid code analysis tool
warnings, what do you think ?
Thanks,
Nirmoy
>
>>
>> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
>> CC: Matthew Auld <matthew.auld@intel.com>
>> CC: Andrzej Hajda <andrzej.hajda@intel.com>
>> CC: Nirmoy Das <nirmoy.das@intel.com>
>> CC: Andi Shyti <andi.shyti@linux.intel.com>
>> ---
>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> index d63f30efd631..b569624f2ed9 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct
>> ttm_buffer_object *bo,
>> int len, int write)
>> {
>> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
>> - resource_size_t iomap = obj->mm.region->iomap.base -
>> - obj->mm.region->region.start;
>> + resource_size_t iomap;
>> unsigned long page = offset >> PAGE_SHIFT;
>> unsigned long bytes_left = len;
>> + if (!obj)
>> + return -EINVAL;
>> +
>> + iomap = obj->mm.region->iomap.base -
>> + obj->mm.region->region.start;
>> +
>> /*
>> * TODO: For now just let it fail if the resource is non-mappable,
>> * otherwise we need to perform the memcpy from the gpu here,
>> without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-14 8:56 ` Andi Shyti
@ 2022-10-14 9:44 ` Matthew Auld
2022-10-14 14:49 ` Andi Shyti
0 siblings, 1 reply; 15+ messages in thread
From: Matthew Auld @ 2022-10-14 9:44 UTC (permalink / raw)
To: Andi Shyti; +Cc: intel-gfx, Jonathan Cavitt, andrzej.hajda, nirmoy.das
On 14/10/2022 09:56, Andi Shyti wrote:
> On Fri, Oct 14, 2022 at 09:39:52AM +0100, Matthew Auld wrote:
>> On 13/10/2022 18:56, Jonathan Cavitt wrote:
>>> i915_ttm_to_gem can return a NULL pointer, which is
>>> dereferenced in i915_ttm_access_memory without first
>>> checking if it is NULL. Inspecting
>>> i915_ttm_io_mem_reserve, it appears the correct
>>> behavior in this case is to return -EINVAL.
>>
>> The GEM object has already been dereferenced before this point, if you look
>> at the caller (vm_access_ttm). The NULL obj thing is to identify "ttm ghost
>> objects", and I don't think a normal userpace object can suddenly become one
>> (access_memory comes from ptrace). AFAIK ghost objects are just for
>> temporarily hanging on to some memory/state, while the dma-resv is busy. In
>> the places where ttm is the one giving us the object, then it might be
>> possible to see these types of objects, since ttm could in theory pass one
>> in (like during eviction).
>
> True that, but because from a code persepctive we can still receive
> NULL, I think the check is correct, perhaps we could:
>
> if (unlikely(!obj))
> return -EINVAL;
Hmm, so that will dereference some pointer, and then later check if it
is NULL here? Or do you mean to move this into vm_access()? If we are
given a "ghost object" for ptrace this would likely mean we have a very
nasty bug somewhere (unless I'm misunderstanding something), and so
returning a normal user error here doesn't seem right to me (maybe this
just hides the issue)? Letting it crash seems fine to me tbh. It also
makes the code harder to understand IMO, because looking at this it now
suggests that it is somehow possible to have a "ghost object" here. Also
there are a fair few places calling i915_ttm_to_gem() which already
don't check for NULL, since it should be impossible, like it should be here.
>
> Andi
>
>>> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
>>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
>>> CC: Matthew Auld <matthew.auld@intel.com>
>>> CC: Andrzej Hajda <andrzej.hajda@intel.com>
>>> CC: Nirmoy Das <nirmoy.das@intel.com>
>>> CC: Andi Shyti <andi.shyti@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> index d63f30efd631..b569624f2ed9 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct ttm_buffer_object *bo,
>>> int len, int write)
>>> {
>>> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
>>> - resource_size_t iomap = obj->mm.region->iomap.base -
>>> - obj->mm.region->region.start;
>>> + resource_size_t iomap;
>>> unsigned long page = offset >> PAGE_SHIFT;
>>> unsigned long bytes_left = len;
>>> + if (!obj)
>>> + return -EINVAL;
>>> +
>>> + iomap = obj->mm.region->iomap.base -
>>> + obj->mm.region->region.start;
>>> +
>>> /*
>>> * TODO: For now just let it fail if the resource is non-mappable,
>>> * otherwise we need to perform the memcpy from the gpu here, without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-13 17:56 [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception Jonathan Cavitt
` (4 preceding siblings ...)
2022-10-14 9:02 ` Andrzej Hajda
@ 2022-10-14 9:52 ` Tvrtko Ursulin
5 siblings, 0 replies; 15+ messages in thread
From: Tvrtko Ursulin @ 2022-10-14 9:52 UTC (permalink / raw)
To: Jonathan Cavitt, intel-gfx; +Cc: matthew.auld, andrzej.hajda, nirmoy.das
Hi,
A couple nit picks for the benefit of coding style.
On 13/10/2022 18:56, Jonathan Cavitt wrote:
> i915_ttm_to_gem can return a NULL pointer, which is
> dereferenced in i915_ttm_access_memory without first
> checking if it is NULL. Inspecting
> i915_ttm_io_mem_reserve, it appears the correct
> behavior in this case is to return -EINVAL.
Too narrow wrap - see kernel's submitting-patches.rst:
- The body of the explanation, line wrapped at 75 columns, which will
be copied to the permanent changelog to describe this patch.
>
> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
> CC: Matthew Auld <matthew.auld@intel.com>
> CC: Andrzej Hajda <andrzej.hajda@intel.com>
> CC: Nirmoy Das <nirmoy.das@intel.com>
> CC: Andi Shyti <andi.shyti@linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> index d63f30efd631..b569624f2ed9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct ttm_buffer_object *bo,
> int len, int write)
> {
> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
> - resource_size_t iomap = obj->mm.region->iomap.base -
> - obj->mm.region->region.start;
> + resource_size_t iomap;
> unsigned long page = offset >> PAGE_SHIFT;
> unsigned long bytes_left = len;
We tend to re-order the declarations from long to narrow, where not too cumbersome, just for that extra readability.
>
> + if (!obj)
> + return -EINVAL;
Someone perhaps can update the vfunc kerneldoc since it appears to be overly strict in allowed return codes.
include/drm/ttm/ttm_device.h:
/**
* Read/write memory buffers for ptrace access
*
* @bo: the BO to access
* @offset: the offset from the start of the BO
* @buf: pointer to source/destination buffer
* @len: number of bytes to copy
* @write: whether to read (0) from or write (non-0) to BO
*
* If successful, this function should return the number of
* bytes copied, -EIO otherwise. If the number of bytes
* returned is < len, the function may be called again with
* the remainder of the buffer to copy.
*/
int (*access_memory)(struct ttm_buffer_object *bo, unsigned long offset,
void *buf, int len, int write);
Regards,
Tvrtko
> +
> + iomap = obj->mm.region->iomap.base -
> + obj->mm.region->region.start;
> +
> /*
> * TODO: For now just let it fail if the resource is non-mappable,
> * otherwise we need to perform the memcpy from the gpu here, without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-14 9:27 ` Das, Nirmoy
@ 2022-10-14 10:13 ` Matthew Auld
2022-10-14 10:38 ` Das, Nirmoy
0 siblings, 1 reply; 15+ messages in thread
From: Matthew Auld @ 2022-10-14 10:13 UTC (permalink / raw)
To: Das, Nirmoy, Jonathan Cavitt, intel-gfx; +Cc: andrzej.hajda
On 14/10/2022 10:27, Das, Nirmoy wrote:
> Hi Matt
>
> On 10/14/2022 10:39 AM, Matthew Auld wrote:
>> On 13/10/2022 18:56, Jonathan Cavitt wrote:
>>> i915_ttm_to_gem can return a NULL pointer, which is
>>> dereferenced in i915_ttm_access_memory without first
>>> checking if it is NULL. Inspecting
>>> i915_ttm_io_mem_reserve, it appears the correct
>>> behavior in this case is to return -EINVAL.
>>
>> The GEM object has already been dereferenced before this point, if you
>> look at the caller (vm_access_ttm). The NULL obj thing is to identify
>> "ttm ghost objects", and I don't think a normal userpace object can
>> suddenly become one (access_memory comes from ptrace). AFAIK ghost
>> objects are just for temporarily hanging on to some memory/state,
>> while the dma-resv is busy. In the places where ttm is the one giving
>> us the object, then it might be possible to see these types of
>> objects, since ttm could in theory pass one in (like during eviction).
>
>
> Yes, we should not hit this. Thanks for the nice "ttm ghost objects"
> reminder :)
>
>
> I think we can still have this check to avoid code analysis tool
> warnings, what do you think ?
IMHO I think it just makes it harder to understand the code, since
conceptually it should be impossible, given how "ghost objects" actually
work. Adding such a check gives the impression that it is somehow now
possible to be given one here (like with eviction etc). AFAIK just
letting it crash is fine, instead of littering the code with NULL checks
for stuff that is never meant to be NULL and would be a driver bug. Also
there are a bunch of other places not checking that i915_ttm_to_gem()
returns NULL, so why just here? Did the code analysis tool find
something? Also why doesn't it complain about vm_access_ttm(), which is
the one actually calling access_memory() and is itself also doing
i915_ttm_to_gem() and also not checking for NULL?
>
>
> Thanks,
>
> Nirmoy
>
>>
>>>
>>> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
>>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
>>> CC: Matthew Auld <matthew.auld@intel.com>
>>> CC: Andrzej Hajda <andrzej.hajda@intel.com>
>>> CC: Nirmoy Das <nirmoy.das@intel.com>
>>> CC: Andi Shyti <andi.shyti@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> index d63f30efd631..b569624f2ed9 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct
>>> ttm_buffer_object *bo,
>>> int len, int write)
>>> {
>>> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
>>> - resource_size_t iomap = obj->mm.region->iomap.base -
>>> - obj->mm.region->region.start;
>>> + resource_size_t iomap;
>>> unsigned long page = offset >> PAGE_SHIFT;
>>> unsigned long bytes_left = len;
>>> + if (!obj)
>>> + return -EINVAL;
>>> +
>>> + iomap = obj->mm.region->iomap.base -
>>> + obj->mm.region->region.start;
>>> +
>>> /*
>>> * TODO: For now just let it fail if the resource is non-mappable,
>>> * otherwise we need to perform the memcpy from the gpu here,
>>> without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-14 10:13 ` Matthew Auld
@ 2022-10-14 10:38 ` Das, Nirmoy
2022-10-14 10:52 ` Matthew Auld
0 siblings, 1 reply; 15+ messages in thread
From: Das, Nirmoy @ 2022-10-14 10:38 UTC (permalink / raw)
To: Matthew Auld, Jonathan Cavitt, intel-gfx; +Cc: andrzej.hajda
Hi Matt,
On 10/14/2022 12:13 PM, Matthew Auld wrote:
> On 14/10/2022 10:27, Das, Nirmoy wrote:
>> Hi Matt
>>
>> On 10/14/2022 10:39 AM, Matthew Auld wrote:
>>> On 13/10/2022 18:56, Jonathan Cavitt wrote:
>>>> i915_ttm_to_gem can return a NULL pointer, which is
>>>> dereferenced in i915_ttm_access_memory without first
>>>> checking if it is NULL. Inspecting
>>>> i915_ttm_io_mem_reserve, it appears the correct
>>>> behavior in this case is to return -EINVAL.
>>>
>>> The GEM object has already been dereferenced before this point, if
>>> you look at the caller (vm_access_ttm). The NULL obj thing is to
>>> identify "ttm ghost objects", and I don't think a normal userpace
>>> object can suddenly become one (access_memory comes from ptrace).
>>> AFAIK ghost objects are just for temporarily hanging on to some
>>> memory/state, while the dma-resv is busy. In the places where ttm is
>>> the one giving us the object, then it might be possible to see these
>>> types of objects, since ttm could in theory pass one in (like during
>>> eviction).
>>
>>
>> Yes, we should not hit this. Thanks for the nice "ttm ghost objects"
>> reminder :)
>>
>>
>> I think we can still have this check to avoid code analysis tool
>> warnings, what do you think ?
>
> IMHO I think it just makes it harder to understand the code, since
> conceptually it should be impossible, given how "ghost objects"
> actually work. Adding such a check gives the impression that it is
> somehow now possible to be given one here (like with eviction etc).
> AFAIK just letting it crash is fine, instead of littering the code
> with NULL checks for stuff that is never meant to be NULL and would be
> a driver bug. Also there are a bunch of other places not checking that
> i915_ttm_to_gem() returns NULL, so why just here?
This is tricky because some place we might receive NULL and some other
places we might not(from i915_ttm_to_gem). I also don't like the idea of
sprinkling NULL check everywhere.
I think the issue is i915_ttm_to_gem returns NULL for non-i915 BO. We
should move "if (bo->destroy != i915_ttm_bo_destroy)" check to the
respective function where we
expect ghost object. That should make the static code analyzer happy and
also makes it very clear which function expect ghost objects.
> Did the code analysis tool find something? Also why doesn't it
> complain about vm_access_ttm(), which is the one actually calling
> access_memory() and is itself also doing i915_ttm_to_gem() and also
> not checking for NULL?
Yes, I think the patch idea came from our static code analyzer warning
but I can't seem to open the URL. I am also not sure why it doesn't
complain for other cases.
Thanks,
Nirmoy
>
>>
>>
>> Thanks,
>>
>> Nirmoy
>>
>>>
>>>>
>>>> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
>>>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>>> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
>>>> CC: Matthew Auld <matthew.auld@intel.com>
>>>> CC: Andrzej Hajda <andrzej.hajda@intel.com>
>>>> CC: Nirmoy Das <nirmoy.das@intel.com>
>>>> CC: Andi Shyti <andi.shyti@linux.intel.com>
>>>> ---
>>>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
>>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>> index d63f30efd631..b569624f2ed9 100644
>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct
>>>> ttm_buffer_object *bo,
>>>> int len, int write)
>>>> {
>>>> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
>>>> - resource_size_t iomap = obj->mm.region->iomap.base -
>>>> - obj->mm.region->region.start;
>>>> + resource_size_t iomap;
>>>> unsigned long page = offset >> PAGE_SHIFT;
>>>> unsigned long bytes_left = len;
>>>> + if (!obj)
>>>> + return -EINVAL;
>>>> +
>>>> + iomap = obj->mm.region->iomap.base -
>>>> + obj->mm.region->region.start;
>>>> +
>>>> /*
>>>> * TODO: For now just let it fail if the resource is
>>>> non-mappable,
>>>> * otherwise we need to perform the memcpy from the gpu here,
>>>> without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-14 10:38 ` Das, Nirmoy
@ 2022-10-14 10:52 ` Matthew Auld
2022-10-14 10:56 ` Das, Nirmoy
0 siblings, 1 reply; 15+ messages in thread
From: Matthew Auld @ 2022-10-14 10:52 UTC (permalink / raw)
To: Das, Nirmoy, Jonathan Cavitt, intel-gfx; +Cc: andrzej.hajda
On 14/10/2022 11:38, Das, Nirmoy wrote:
> Hi Matt,
>
> On 10/14/2022 12:13 PM, Matthew Auld wrote:
>> On 14/10/2022 10:27, Das, Nirmoy wrote:
>>> Hi Matt
>>>
>>> On 10/14/2022 10:39 AM, Matthew Auld wrote:
>>>> On 13/10/2022 18:56, Jonathan Cavitt wrote:
>>>>> i915_ttm_to_gem can return a NULL pointer, which is
>>>>> dereferenced in i915_ttm_access_memory without first
>>>>> checking if it is NULL. Inspecting
>>>>> i915_ttm_io_mem_reserve, it appears the correct
>>>>> behavior in this case is to return -EINVAL.
>>>>
>>>> The GEM object has already been dereferenced before this point, if
>>>> you look at the caller (vm_access_ttm). The NULL obj thing is to
>>>> identify "ttm ghost objects", and I don't think a normal userpace
>>>> object can suddenly become one (access_memory comes from ptrace).
>>>> AFAIK ghost objects are just for temporarily hanging on to some
>>>> memory/state, while the dma-resv is busy. In the places where ttm is
>>>> the one giving us the object, then it might be possible to see these
>>>> types of objects, since ttm could in theory pass one in (like during
>>>> eviction).
>>>
>>>
>>> Yes, we should not hit this. Thanks for the nice "ttm ghost objects"
>>> reminder :)
>>>
>>>
>>> I think we can still have this check to avoid code analysis tool
>>> warnings, what do you think ?
>>
>> IMHO I think it just makes it harder to understand the code, since
>> conceptually it should be impossible, given how "ghost objects"
>> actually work. Adding such a check gives the impression that it is
>> somehow now possible to be given one here (like with eviction etc).
>> AFAIK just letting it crash is fine, instead of littering the code
>> with NULL checks for stuff that is never meant to be NULL and would be
>> a driver bug. Also there are a bunch of other places not checking that
>> i915_ttm_to_gem() returns NULL, so why just here?
>
> This is tricky because some place we might receive NULL and some other
> places we might not(from i915_ttm_to_gem). I also don't like the idea of
> sprinkling NULL check everywhere.
>
> I think the issue is i915_ttm_to_gem returns NULL for non-i915 BO. We
> should move "if (bo->destroy != i915_ttm_bo_destroy)" check to the
> respective function where we
>
> expect ghost object. That should make the static code analyzer happy and
> also makes it very clear which function expect ghost objects.
Yeah, that sounds like a really nice idea to me. amdgpu looks to have
something like amdgpu_bo_is_amdgpu_bo() for the spots that might be
"ghost objects". Maybe we can add something like i915_ttm_is_ghost_bo()
or similar for our needs.
>
>
>> Did the code analysis tool find something? Also why doesn't it
>> complain about vm_access_ttm(), which is the one actually calling
>> access_memory() and is itself also doing i915_ttm_to_gem() and also
>> not checking for NULL?
>
>
> Yes, I think the patch idea came from our static code analyzer warning
> but I can't seem to open the URL. I am also not sure why it doesn't
> complain for other cases.
>
>
> Thanks,
>
> Nirmoy
>
>>
>>>
>>>
>>> Thanks,
>>>
>>> Nirmoy
>>>
>>>>
>>>>>
>>>>> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
>>>>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>>>> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
>>>>> CC: Matthew Auld <matthew.auld@intel.com>
>>>>> CC: Andrzej Hajda <andrzej.hajda@intel.com>
>>>>> CC: Nirmoy Das <nirmoy.das@intel.com>
>>>>> CC: Andi Shyti <andi.shyti@linux.intel.com>
>>>>> ---
>>>>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
>>>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>>> index d63f30efd631..b569624f2ed9 100644
>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>>> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct
>>>>> ttm_buffer_object *bo,
>>>>> int len, int write)
>>>>> {
>>>>> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
>>>>> - resource_size_t iomap = obj->mm.region->iomap.base -
>>>>> - obj->mm.region->region.start;
>>>>> + resource_size_t iomap;
>>>>> unsigned long page = offset >> PAGE_SHIFT;
>>>>> unsigned long bytes_left = len;
>>>>> + if (!obj)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + iomap = obj->mm.region->iomap.base -
>>>>> + obj->mm.region->region.start;
>>>>> +
>>>>> /*
>>>>> * TODO: For now just let it fail if the resource is
>>>>> non-mappable,
>>>>> * otherwise we need to perform the memcpy from the gpu here,
>>>>> without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-14 10:52 ` Matthew Auld
@ 2022-10-14 10:56 ` Das, Nirmoy
0 siblings, 0 replies; 15+ messages in thread
From: Das, Nirmoy @ 2022-10-14 10:56 UTC (permalink / raw)
To: Matthew Auld, Jonathan Cavitt, intel-gfx; +Cc: andrzej.hajda
On 10/14/2022 12:52 PM, Matthew Auld wrote:
> On 14/10/2022 11:38, Das, Nirmoy wrote:
>> Hi Matt,
>>
>> On 10/14/2022 12:13 PM, Matthew Auld wrote:
>>> On 14/10/2022 10:27, Das, Nirmoy wrote:
>>>> Hi Matt
>>>>
>>>> On 10/14/2022 10:39 AM, Matthew Auld wrote:
>>>>> On 13/10/2022 18:56, Jonathan Cavitt wrote:
>>>>>> i915_ttm_to_gem can return a NULL pointer, which is
>>>>>> dereferenced in i915_ttm_access_memory without first
>>>>>> checking if it is NULL. Inspecting
>>>>>> i915_ttm_io_mem_reserve, it appears the correct
>>>>>> behavior in this case is to return -EINVAL.
>>>>>
>>>>> The GEM object has already been dereferenced before this point, if
>>>>> you look at the caller (vm_access_ttm). The NULL obj thing is to
>>>>> identify "ttm ghost objects", and I don't think a normal userpace
>>>>> object can suddenly become one (access_memory comes from ptrace).
>>>>> AFAIK ghost objects are just for temporarily hanging on to some
>>>>> memory/state, while the dma-resv is busy. In the places where ttm
>>>>> is the one giving us the object, then it might be possible to see
>>>>> these types of objects, since ttm could in theory pass one in
>>>>> (like during eviction).
>>>>
>>>>
>>>> Yes, we should not hit this. Thanks for the nice "ttm ghost
>>>> objects" reminder :)
>>>>
>>>>
>>>> I think we can still have this check to avoid code analysis tool
>>>> warnings, what do you think ?
>>>
>>> IMHO I think it just makes it harder to understand the code, since
>>> conceptually it should be impossible, given how "ghost objects"
>>> actually work. Adding such a check gives the impression that it is
>>> somehow now possible to be given one here (like with eviction etc).
>>> AFAIK just letting it crash is fine, instead of littering the code
>>> with NULL checks for stuff that is never meant to be NULL and would
>>> be a driver bug. Also there are a bunch of other places not checking
>>> that i915_ttm_to_gem() returns NULL, so why just here?
>>
>> This is tricky because some place we might receive NULL and some
>> other places we might not(from i915_ttm_to_gem). I also don't like
>> the idea of sprinkling NULL check everywhere.
>>
>> I think the issue is i915_ttm_to_gem returns NULL for non-i915 BO.
>> We should move "if (bo->destroy != i915_ttm_bo_destroy)" check to the
>> respective function where we
>>
>> expect ghost object. That should make the static code analyzer happy
>> and also makes it very clear which function expect ghost objects.
>
> Yeah, that sounds like a really nice idea to me. amdgpu looks to have
> something like amdgpu_bo_is_amdgpu_bo() for the spots that might be
> "ghost objects". Maybe we can add something like
> i915_ttm_is_ghost_bo() or similar for our needs.
I will prepare patch for that then.
Thanks,
Nirmoy
>
>>
>>
>>> Did the code analysis tool find something? Also why doesn't it
>>> complain about vm_access_ttm(), which is the one actually calling
>>> access_memory() and is itself also doing i915_ttm_to_gem() and also
>>> not checking for NULL?
>>
>>
>> Yes, I think the patch idea came from our static code analyzer
>> warning but I can't seem to open the URL. I am also not sure why it
>> doesn't complain for other cases.
>>
>>
>> Thanks,
>>
>> Nirmoy
>>
>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Nirmoy
>>>>
>>>>>
>>>>>>
>>>>>> Fixes: 26b15eb0 ("drm/i915/ttm: implement access_memory")
>>>>>> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>>>>> Suggested-by: John C Harrison <John.C.Harrison@intel.com>
>>>>>> CC: Matthew Auld <matthew.auld@intel.com>
>>>>>> CC: Andrzej Hajda <andrzej.hajda@intel.com>
>>>>>> CC: Nirmoy Das <nirmoy.das@intel.com>
>>>>>> CC: Andi Shyti <andi.shyti@linux.intel.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 +++++++--
>>>>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>>>> b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>>>> index d63f30efd631..b569624f2ed9 100644
>>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
>>>>>> @@ -704,11 +704,16 @@ static int i915_ttm_access_memory(struct
>>>>>> ttm_buffer_object *bo,
>>>>>> int len, int write)
>>>>>> {
>>>>>> struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
>>>>>> - resource_size_t iomap = obj->mm.region->iomap.base -
>>>>>> - obj->mm.region->region.start;
>>>>>> + resource_size_t iomap;
>>>>>> unsigned long page = offset >> PAGE_SHIFT;
>>>>>> unsigned long bytes_left = len;
>>>>>> + if (!obj)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + iomap = obj->mm.region->iomap.base -
>>>>>> + obj->mm.region->region.start;
>>>>>> +
>>>>>> /*
>>>>>> * TODO: For now just let it fail if the resource is
>>>>>> non-mappable,
>>>>>> * otherwise we need to perform the memcpy from the gpu
>>>>>> here, without
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception
2022-10-14 9:44 ` Matthew Auld
@ 2022-10-14 14:49 ` Andi Shyti
0 siblings, 0 replies; 15+ messages in thread
From: Andi Shyti @ 2022-10-14 14:49 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx, Jonathan Cavitt, andrzej.hajda, nirmoy.das
Hi Matt,
On Fri, Oct 14, 2022 at 10:44:11AM +0100, Matthew Auld wrote:
> On 14/10/2022 09:56, Andi Shyti wrote:
> > On Fri, Oct 14, 2022 at 09:39:52AM +0100, Matthew Auld wrote:
> > > On 13/10/2022 18:56, Jonathan Cavitt wrote:
> > > > i915_ttm_to_gem can return a NULL pointer, which is
> > > > dereferenced in i915_ttm_access_memory without first
> > > > checking if it is NULL. Inspecting
> > > > i915_ttm_io_mem_reserve, it appears the correct
> > > > behavior in this case is to return -EINVAL.
> > >
> > > The GEM object has already been dereferenced before this point, if you look
> > > at the caller (vm_access_ttm). The NULL obj thing is to identify "ttm ghost
> > > objects", and I don't think a normal userpace object can suddenly become one
> > > (access_memory comes from ptrace). AFAIK ghost objects are just for
> > > temporarily hanging on to some memory/state, while the dma-resv is busy. In
> > > the places where ttm is the one giving us the object, then it might be
> > > possible to see these types of objects, since ttm could in theory pass one
> > > in (like during eviction).
> >
> > True that, but because from a code persepctive we can still receive
> > NULL, I think the check is correct, perhaps we could:
> >
> > if (unlikely(!obj))
> > return -EINVAL;
>
> Hmm, so that will dereference some pointer, and then later check if it is
> NULL here? Or do you mean to move this into vm_access()? If we are given a
> "ghost object" for ptrace this would likely mean we have a very nasty bug
> somewhere (unless I'm misunderstanding something), and so returning a normal
> user error here doesn't seem right to me (maybe this just hides the issue)?
> Letting it crash seems fine to me tbh. It also makes the code harder to
> understand IMO, because looking at this it now suggests that it is somehow
> possible to have a "ghost object" here. Also there are a fair few places
> calling i915_ttm_to_gem() which already don't check for NULL, since it
> should be impossible, like it should be here.
By just analyzing the code, getting NULL is not impossible. In
that case even a GEM_BUG_ON would have worked. But the NULL
pointer, as it is, needs to be checked.
Anyway, I see that an agreement has been reached with Nirmoy, so
that it doesn't matter anymore :)
Andi
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-10-14 14:53 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-13 17:56 [Intel-gfx] [PATCH] drm/i915/ttm: Fix access_memory null pointer exception Jonathan Cavitt
2022-10-13 19:28 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2022-10-13 23:07 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2022-10-14 8:39 ` [Intel-gfx] [PATCH] " Matthew Auld
2022-10-14 8:56 ` Andi Shyti
2022-10-14 9:44 ` Matthew Auld
2022-10-14 14:49 ` Andi Shyti
2022-10-14 9:27 ` Das, Nirmoy
2022-10-14 10:13 ` Matthew Auld
2022-10-14 10:38 ` Das, Nirmoy
2022-10-14 10:52 ` Matthew Auld
2022-10-14 10:56 ` Das, Nirmoy
2022-10-14 8:47 ` Andi Shyti
2022-10-14 9:02 ` Andrzej Hajda
2022-10-14 9:52 ` Tvrtko Ursulin
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.