* [Intel-gfx] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini @ 2022-03-07 8:26 ` Janusz Krzysztofik 0 siblings, 0 replies; 10+ messages in thread From: Janusz Krzysztofik @ 2022-03-07 8:26 UTC (permalink / raw) To: igt-dev; +Cc: intel-gfx Commit 5f3cfa485eb4 ("lib: Use safe wrappers around libpciaccess initialization functions") took care of not leaking memory allocated by pci_system_init() but didn't take care of users potentially attempting to reinitialize global data maintained by libpciaccess. For example, intel_register_access_init() mmaps device's PCI BAR0 resource with pci_device_map_range() but intel_register_access_fini() doesn't unmap it and next call to intel_register_access_init() fails on attempt to mmap it again. Fix it, and also provide intel_mmio_unmap_*() counterparts to public functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file(). v2: apply last minute fixes, cached but unfortunately not committed before sending v3: use .pci_device_id field content as an indicator of arg initialization via intel_register_access_init(), - improve checks of argument initialization status, - shorten warning messages (Kamil), - don't fill .mmio_size field until initialization succeeds (Kamil) Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> --- lib/intel_io.h | 4 +++ lib/intel_mmio.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/lib/intel_io.h b/lib/intel_io.h index 1cfe4fb6b9..ea2649d9bc 100644 --- a/lib/intel_io.h +++ b/lib/intel_io.h @@ -49,6 +49,8 @@ struct intel_register_map { struct intel_mmio_data { void *igt_mmio; + size_t mmio_size; + struct pci_device *dev; struct intel_register_map map; uint32_t pci_device_id; int key; @@ -57,7 +59,9 @@ struct intel_mmio_data { void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev); +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data); void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file); +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data); int intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd); diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c index 667a69f5aa..d6ce0ee3ea 100644 --- a/lib/intel_mmio.c +++ b/lib/intel_mmio.c @@ -82,6 +82,8 @@ void *igt_global_mmio; * Sets also up mmio_data->igt_mmio to point at the data contained * in @file. This allows the same code to get reused for dumping and decoding * from running hardware as from register dumps. + * + * Users are expected to call intel_mmio_unmap_dump_file() after use. */ void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) @@ -99,11 +101,32 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) igt_fail_on_f(mmio_data->igt_mmio == MAP_FAILED, "Couldn't mmap %s\n", file); + mmio_data->mmio_size = st.st_size; igt_global_mmio = mmio_data->igt_mmio; close(fd); } +/** + * intel_mmio_unmap_dump_file: + * @mmio_data: mmio structure for IO operations + * + * Unmaps a dump file mmapped with intel_mmio_use_dump_file() + */ +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data) +{ + if (igt_warn_on_f(mmio_data->dev, + "test bug: arg initialized with intel_mmio_use_pci_bar()\n")) + return; + if (igt_warn_on_f(!mmio_data->mmio_size, + "test bug: arg not initialized\n")) + return; + + igt_global_mmio = NULL; + igt_debug_on(munmap(mmio_data->igt_mmio, mmio_data->mmio_size) < 0); + mmio_data->mmio_size = 0; +} + /** * intel_mmio_use_pci_bar: * @mmio_data: mmio structure for IO operations @@ -112,6 +135,8 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) * Fill a mmio_data stucture with igt_mmio to point at the mmio bar. * * @pci_dev can be obtained from intel_get_pci_device(). + * + * Users are expected to call intel_mmio_unmap_pci_bar() after use. */ void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev) @@ -141,10 +166,34 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci PCI_DEV_MAP_FLAG_WRITABLE, &mmio_data->igt_mmio); - igt_global_mmio = mmio_data->igt_mmio; - igt_fail_on_f(error != 0, "Couldn't map MMIO region\n"); + + mmio_data->mmio_size = mmio_size; + mmio_data->dev = pci_dev; + igt_global_mmio = mmio_data->igt_mmio; +} + +/** + * intel_mmio_unmap_pci_bar: + * @mmio_data: mmio structure for IO operations + * + * Unmaps a PCI BAR region mmapped with intel_mmio_use_pci_bar() + */ +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data) +{ + if (igt_warn_on_f(mmio_data->pci_device_id, + "test bug: arg initialized with intel_register_access_init()\n")) + return; + if (igt_warn_on_f(!mmio_data->dev, + "test bug: arg not initialized with intel_mmio_use_pci_bar()\n")) + return; + + igt_global_mmio = NULL; + igt_debug_on(pci_device_unmap_range(mmio_data->dev, + mmio_data->igt_mmio, mmio_data->mmio_size) < 0); + mmio_data->dev = NULL; + mmio_data->mmio_size = 0; } static void @@ -166,6 +215,8 @@ release_forcewake_lock(int fd) * It also initializes mmio_data->igt_mmio like intel_mmio_use_pci_bar(). * * @pci_dev can be obtained from intel_get_pci_device(). + * + * Users are expected to call intel_register_access_fini() after use. */ int intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd) @@ -222,8 +273,15 @@ int intel_register_access_needs_fakewake(struct intel_mmio_data *mmio_data) void intel_register_access_fini(struct intel_mmio_data *mmio_data) { - if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) + if (igt_warn_on_f(!mmio_data->pci_device_id, + "test bug: arg not initialized with intel_register_access_init()\n")) + return; + + if (intel_register_access_needs_wake(mmio_data)) release_forcewake_lock(mmio_data->key); + + mmio_data->pci_device_id = 0; + intel_mmio_unmap_pci_bar(mmio_data); } /** -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [igt-dev] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini @ 2022-03-07 8:26 ` Janusz Krzysztofik 0 siblings, 0 replies; 10+ messages in thread From: Janusz Krzysztofik @ 2022-03-07 8:26 UTC (permalink / raw) To: igt-dev; +Cc: intel-gfx Commit 5f3cfa485eb4 ("lib: Use safe wrappers around libpciaccess initialization functions") took care of not leaking memory allocated by pci_system_init() but didn't take care of users potentially attempting to reinitialize global data maintained by libpciaccess. For example, intel_register_access_init() mmaps device's PCI BAR0 resource with pci_device_map_range() but intel_register_access_fini() doesn't unmap it and next call to intel_register_access_init() fails on attempt to mmap it again. Fix it, and also provide intel_mmio_unmap_*() counterparts to public functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file(). v2: apply last minute fixes, cached but unfortunately not committed before sending v3: use .pci_device_id field content as an indicator of arg initialization via intel_register_access_init(), - improve checks of argument initialization status, - shorten warning messages (Kamil), - don't fill .mmio_size field until initialization succeeds (Kamil) Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> --- lib/intel_io.h | 4 +++ lib/intel_mmio.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/lib/intel_io.h b/lib/intel_io.h index 1cfe4fb6b9..ea2649d9bc 100644 --- a/lib/intel_io.h +++ b/lib/intel_io.h @@ -49,6 +49,8 @@ struct intel_register_map { struct intel_mmio_data { void *igt_mmio; + size_t mmio_size; + struct pci_device *dev; struct intel_register_map map; uint32_t pci_device_id; int key; @@ -57,7 +59,9 @@ struct intel_mmio_data { void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev); +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data); void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file); +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data); int intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd); diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c index 667a69f5aa..d6ce0ee3ea 100644 --- a/lib/intel_mmio.c +++ b/lib/intel_mmio.c @@ -82,6 +82,8 @@ void *igt_global_mmio; * Sets also up mmio_data->igt_mmio to point at the data contained * in @file. This allows the same code to get reused for dumping and decoding * from running hardware as from register dumps. + * + * Users are expected to call intel_mmio_unmap_dump_file() after use. */ void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) @@ -99,11 +101,32 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) igt_fail_on_f(mmio_data->igt_mmio == MAP_FAILED, "Couldn't mmap %s\n", file); + mmio_data->mmio_size = st.st_size; igt_global_mmio = mmio_data->igt_mmio; close(fd); } +/** + * intel_mmio_unmap_dump_file: + * @mmio_data: mmio structure for IO operations + * + * Unmaps a dump file mmapped with intel_mmio_use_dump_file() + */ +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data) +{ + if (igt_warn_on_f(mmio_data->dev, + "test bug: arg initialized with intel_mmio_use_pci_bar()\n")) + return; + if (igt_warn_on_f(!mmio_data->mmio_size, + "test bug: arg not initialized\n")) + return; + + igt_global_mmio = NULL; + igt_debug_on(munmap(mmio_data->igt_mmio, mmio_data->mmio_size) < 0); + mmio_data->mmio_size = 0; +} + /** * intel_mmio_use_pci_bar: * @mmio_data: mmio structure for IO operations @@ -112,6 +135,8 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) * Fill a mmio_data stucture with igt_mmio to point at the mmio bar. * * @pci_dev can be obtained from intel_get_pci_device(). + * + * Users are expected to call intel_mmio_unmap_pci_bar() after use. */ void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev) @@ -141,10 +166,34 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci PCI_DEV_MAP_FLAG_WRITABLE, &mmio_data->igt_mmio); - igt_global_mmio = mmio_data->igt_mmio; - igt_fail_on_f(error != 0, "Couldn't map MMIO region\n"); + + mmio_data->mmio_size = mmio_size; + mmio_data->dev = pci_dev; + igt_global_mmio = mmio_data->igt_mmio; +} + +/** + * intel_mmio_unmap_pci_bar: + * @mmio_data: mmio structure for IO operations + * + * Unmaps a PCI BAR region mmapped with intel_mmio_use_pci_bar() + */ +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data) +{ + if (igt_warn_on_f(mmio_data->pci_device_id, + "test bug: arg initialized with intel_register_access_init()\n")) + return; + if (igt_warn_on_f(!mmio_data->dev, + "test bug: arg not initialized with intel_mmio_use_pci_bar()\n")) + return; + + igt_global_mmio = NULL; + igt_debug_on(pci_device_unmap_range(mmio_data->dev, + mmio_data->igt_mmio, mmio_data->mmio_size) < 0); + mmio_data->dev = NULL; + mmio_data->mmio_size = 0; } static void @@ -166,6 +215,8 @@ release_forcewake_lock(int fd) * It also initializes mmio_data->igt_mmio like intel_mmio_use_pci_bar(). * * @pci_dev can be obtained from intel_get_pci_device(). + * + * Users are expected to call intel_register_access_fini() after use. */ int intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd) @@ -222,8 +273,15 @@ int intel_register_access_needs_fakewake(struct intel_mmio_data *mmio_data) void intel_register_access_fini(struct intel_mmio_data *mmio_data) { - if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) + if (igt_warn_on_f(!mmio_data->pci_device_id, + "test bug: arg not initialized with intel_register_access_init()\n")) + return; + + if (intel_register_access_needs_wake(mmio_data)) release_forcewake_lock(mmio_data->key); + + mmio_data->pci_device_id = 0; + intel_mmio_unmap_pci_bar(mmio_data); } /** -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev3) 2022-03-07 8:26 ` [igt-dev] " Janusz Krzysztofik (?) @ 2022-03-07 9:44 ` Patchwork -1 siblings, 0 replies; 10+ messages in thread From: Patchwork @ 2022-03-07 9:44 UTC (permalink / raw) To: Janusz Krzysztofik; +Cc: igt-dev [-- Attachment #1: Type: text/plain, Size: 10396 bytes --] == Series Details == Series: lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev3) URL : https://patchwork.freedesktop.org/series/100882/ State : success == Summary == CI Bug Log - changes from IGT_6365 -> IGTPW_6743 ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/index.html Participating hosts (43 -> 42) ------------------------------ Additional (1): fi-icl-u2 Missing (2): fi-bsw-cyan fi-bdw-samus Possible new issues ------------------- Here are the unknown changes that may have been introduced in IGTPW_6743: ### IGT changes ### #### Suppressed #### The following results come from untrusted machines, tests, or statuses. They do not affect the overall result. * igt@i915_selftest@live@requests: - {bat-rpls-2}: [DMESG-FAIL][1] ([i915#5087]) -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/bat-rpls-2/igt@i915_selftest@live@requests.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/bat-rpls-2/igt@i915_selftest@live@requests.html Known issues ------------ Here are the changes found in IGTPW_6743 that come from known issues: ### IGT changes ### #### Issues hit #### * igt@amdgpu/amd_basic@cs-multi-fence: - fi-blb-e6850: NOTRUN -> [SKIP][3] ([fdo#109271]) +17 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-blb-e6850/igt@amdgpu/amd_basic@cs-multi-fence.html * igt@amdgpu/amd_cs_nop@fork-gfx0: - fi-icl-u2: NOTRUN -> [SKIP][4] ([fdo#109315]) +17 similar issues [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-icl-u2/igt@amdgpu/amd_cs_nop@fork-gfx0.html * igt@amdgpu/amd_cs_nop@sync-fork-gfx0: - fi-skl-6600u: NOTRUN -> [SKIP][5] ([fdo#109271]) +21 similar issues [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-skl-6600u/igt@amdgpu/amd_cs_nop@sync-fork-gfx0.html * igt@core_hotunplug@unbind-rebind: - fi-tgl-1115g4: [PASS][6] -> [DMESG-WARN][7] ([i915#4002]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/fi-tgl-1115g4/igt@core_hotunplug@unbind-rebind.html [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-tgl-1115g4/igt@core_hotunplug@unbind-rebind.html * igt@gem_huc_copy@huc-copy: - fi-skl-6600u: NOTRUN -> [SKIP][8] ([fdo#109271] / [i915#2190]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html - fi-icl-u2: NOTRUN -> [SKIP][9] ([i915#2190]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-icl-u2/igt@gem_huc_copy@huc-copy.html * igt@gem_lmem_swapping@parallel-random-engines: - fi-icl-u2: NOTRUN -> [SKIP][10] ([i915#4613]) +3 similar issues [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-icl-u2/igt@gem_lmem_swapping@parallel-random-engines.html * igt@gem_lmem_swapping@verify-random: - fi-skl-6600u: NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#4613]) +3 similar issues [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-skl-6600u/igt@gem_lmem_swapping@verify-random.html * igt@i915_selftest@live@hangcheck: - fi-hsw-4770: [PASS][12] -> [INCOMPLETE][13] ([i915#4785]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html - fi-snb-2600: [PASS][14] -> [INCOMPLETE][15] ([i915#3921]) [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/fi-snb-2600/igt@i915_selftest@live@hangcheck.html [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-snb-2600/igt@i915_selftest@live@hangcheck.html * igt@kms_chamelium@hdmi-hpd-fast: - fi-icl-u2: NOTRUN -> [SKIP][16] ([fdo#111827]) +8 similar issues [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html * igt@kms_chamelium@vga-edid-read: - fi-skl-6600u: NOTRUN -> [SKIP][17] ([fdo#109271] / [fdo#111827]) +8 similar issues [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-skl-6600u/igt@kms_chamelium@vga-edid-read.html * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy: - fi-icl-u2: NOTRUN -> [SKIP][18] ([fdo#109278]) +2 similar issues [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html * igt@kms_force_connector_basic@force-load-detect: - fi-icl-u2: NOTRUN -> [SKIP][19] ([fdo#109285]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-icl-u2/igt@kms_force_connector_basic@force-load-detect.html * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d: - fi-skl-6600u: NOTRUN -> [SKIP][20] ([fdo#109271] / [i915#533]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html * igt@prime_vgem@basic-userptr: - fi-icl-u2: NOTRUN -> [SKIP][21] ([i915#3301]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-icl-u2/igt@prime_vgem@basic-userptr.html * igt@runner@aborted: - fi-hsw-4770: NOTRUN -> [FAIL][22] ([fdo#109271] / [i915#1436] / [i915#4312]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-hsw-4770/igt@runner@aborted.html #### Possible fixes #### * igt@gem_exec_suspend@basic-s3@smem: - fi-skl-6600u: [INCOMPLETE][23] ([i915#4547]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html - {fi-rkl-11600}: [INCOMPLETE][25] ([i915#5127]) -> [PASS][26] [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html * igt@gem_sync@basic-each: - {bat-dg2-9}: [DMESG-WARN][27] -> [PASS][28] [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/bat-dg2-9/igt@gem_sync@basic-each.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/bat-dg2-9/igt@gem_sync@basic-each.html * igt@i915_selftest@live@hangcheck: - bat-dg1-5: [DMESG-FAIL][29] ([i915#4957]) -> [PASS][30] [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/bat-dg1-5/igt@i915_selftest@live@hangcheck.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/bat-dg1-5/igt@i915_selftest@live@hangcheck.html * igt@i915_selftest@live@requests: - fi-blb-e6850: [DMESG-FAIL][31] ([i915#5026]) -> [PASS][32] [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/fi-blb-e6850/igt@i915_selftest@live@requests.html [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/fi-blb-e6850/igt@i915_selftest@live@requests.html * igt@kms_busy@basic@flip: - {bat-adlp-6}: [DMESG-WARN][33] ([i915#3576]) -> [PASS][34] +2 similar issues [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/bat-adlp-6/igt@kms_busy@basic@flip.html [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/bat-adlp-6/igt@kms_busy@basic@flip.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#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278 [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285 [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315 [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827 [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072 [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436 [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190 [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575 [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#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576 [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708 [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921 [i915#4002]: https://gitlab.freedesktop.org/drm/intel/issues/4002 [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077 [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079 [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098 [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103 [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212 [i915#4215]: https://gitlab.freedesktop.org/drm/intel/issues/4215 [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312 [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547 [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613 [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785 [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957 [i915#5026]: https://gitlab.freedesktop.org/drm/intel/issues/5026 [i915#5087]: https://gitlab.freedesktop.org/drm/intel/issues/5087 [i915#5127]: https://gitlab.freedesktop.org/drm/intel/issues/5127 [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190 [i915#5244]: https://gitlab.freedesktop.org/drm/intel/issues/5244 [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533 Build changes ------------- * CI: CI-20190529 -> None * IGT: IGT_6365 -> IGTPW_6743 CI-20190529: 20190529 CI_DRM_11331: a9440d1fadbcc8dcf632c60dbbd2476963a9cd21 @ git://anongit.freedesktop.org/gfx-ci/linux IGTPW_6743: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/index.html IGT_6365: 06de04209b6aa8232694382556a7b3f21103c45c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/index.html [-- Attachment #2: Type: text/html, Size: 11231 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev3) 2022-03-07 8:26 ` [igt-dev] " Janusz Krzysztofik (?) (?) @ 2022-03-07 11:23 ` Patchwork -1 siblings, 0 replies; 10+ messages in thread From: Patchwork @ 2022-03-07 11:23 UTC (permalink / raw) To: Janusz Krzysztofik; +Cc: igt-dev [-- Attachment #1: Type: text/plain, Size: 30279 bytes --] == Series Details == Series: lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev3) URL : https://patchwork.freedesktop.org/series/100882/ State : success == Summary == CI Bug Log - changes from IGT_6365_full -> IGTPW_6743_full ==================================================== Summary ------- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/index.html Participating hosts (8 -> 8) ------------------------------ No changes in participating hosts Known issues ------------ Here are the changes found in IGTPW_6743_full that come from known issues: ### IGT changes ### #### Issues hit #### * igt@feature_discovery@display-2x: - shard-tglb: NOTRUN -> [SKIP][1] ([i915#1839]) [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb2/igt@feature_discovery@display-2x.html * igt@feature_discovery@display-3x: - shard-glk: NOTRUN -> [SKIP][2] ([fdo#109271]) +52 similar issues [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk2/igt@feature_discovery@display-3x.html - shard-iclb: NOTRUN -> [SKIP][3] ([i915#1839]) +1 similar issue [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb8/igt@feature_discovery@display-3x.html * igt@gem_create@create-massive: - shard-iclb: NOTRUN -> [DMESG-WARN][4] ([i915#4991]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb5/igt@gem_create@create-massive.html - shard-snb: NOTRUN -> [DMESG-WARN][5] ([i915#4991]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-snb5/igt@gem_create@create-massive.html - shard-kbl: NOTRUN -> [DMESG-WARN][6] ([i915#4991]) [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl6/igt@gem_create@create-massive.html - shard-tglb: NOTRUN -> [DMESG-WARN][7] ([i915#4991]) [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@gem_create@create-massive.html - shard-glk: NOTRUN -> [DMESG-WARN][8] ([i915#4991]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk2/igt@gem_create@create-massive.html - shard-apl: NOTRUN -> [DMESG-WARN][9] ([i915#4991]) [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl2/igt@gem_create@create-massive.html * igt@gem_ctx_persistence@many-contexts: - shard-tglb: NOTRUN -> [FAIL][10] ([i915#2410]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@gem_ctx_persistence@many-contexts.html * igt@gem_ctx_persistence@process: - shard-snb: NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#1099]) +1 similar issue [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-snb2/igt@gem_ctx_persistence@process.html * igt@gem_exec_balancer@parallel-contexts: - shard-kbl: NOTRUN -> [DMESG-WARN][12] ([i915#5076]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl6/igt@gem_exec_balancer@parallel-contexts.html * igt@gem_exec_fair@basic-flow@rcs0: - shard-tglb: [PASS][13] -> [FAIL][14] ([i915#2842]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-tglb2/igt@gem_exec_fair@basic-flow@rcs0.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@gem_exec_fair@basic-flow@rcs0.html * igt@gem_exec_fair@basic-none-solo@rcs0: - shard-kbl: NOTRUN -> [FAIL][15] ([i915#2842]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl6/igt@gem_exec_fair@basic-none-solo@rcs0.html * igt@gem_exec_fair@basic-pace-share@rcs0: - shard-glk: [PASS][16] -> [FAIL][17] ([i915#2842]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-glk2/igt@gem_exec_fair@basic-pace-share@rcs0.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk2/igt@gem_exec_fair@basic-pace-share@rcs0.html * igt@gem_exec_fair@basic-pace@vecs0: - shard-kbl: [PASS][18] -> [FAIL][19] ([i915#2842]) [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-kbl3/igt@gem_exec_fair@basic-pace@vecs0.html [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl1/igt@gem_exec_fair@basic-pace@vecs0.html * igt@gem_exec_fair@basic-throttle@rcs0: - shard-tglb: NOTRUN -> [FAIL][20] ([i915#2842]) +1 similar issue [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb3/igt@gem_exec_fair@basic-throttle@rcs0.html * igt@gem_exec_params@secure-non-master: - shard-tglb: NOTRUN -> [SKIP][21] ([fdo#112283]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb6/igt@gem_exec_params@secure-non-master.html - shard-iclb: NOTRUN -> [SKIP][22] ([fdo#112283]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb3/igt@gem_exec_params@secure-non-master.html * igt@gem_lmem_swapping@heavy-verify-multi: - shard-iclb: NOTRUN -> [SKIP][23] ([i915#4613]) +1 similar issue [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb2/igt@gem_lmem_swapping@heavy-verify-multi.html * igt@gem_lmem_swapping@parallel-random-engines: - shard-glk: NOTRUN -> [SKIP][24] ([fdo#109271] / [i915#4613]) [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk7/igt@gem_lmem_swapping@parallel-random-engines.html - shard-apl: NOTRUN -> [SKIP][25] ([fdo#109271] / [i915#4613]) [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl2/igt@gem_lmem_swapping@parallel-random-engines.html - shard-tglb: NOTRUN -> [SKIP][26] ([i915#4613]) +4 similar issues [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb3/igt@gem_lmem_swapping@parallel-random-engines.html * igt@gem_lmem_swapping@smem-oom: - shard-kbl: NOTRUN -> [SKIP][27] ([fdo#109271] / [i915#4613]) +4 similar issues [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl3/igt@gem_lmem_swapping@smem-oom.html * igt@gem_pread@exhaustion: - shard-apl: NOTRUN -> [WARN][28] ([i915#2658]) [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl7/igt@gem_pread@exhaustion.html * igt@gem_pwrite@basic-exhaustion: - shard-kbl: NOTRUN -> [WARN][29] ([i915#2658]) [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl6/igt@gem_pwrite@basic-exhaustion.html - shard-tglb: NOTRUN -> [WARN][30] ([i915#2658]) [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb1/igt@gem_pwrite@basic-exhaustion.html * igt@gem_pxp@dmabuf-shared-protected-dst-is-context-refcounted: - shard-iclb: NOTRUN -> [SKIP][31] ([i915#4270]) +1 similar issue [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb8/igt@gem_pxp@dmabuf-shared-protected-dst-is-context-refcounted.html * igt@gem_pxp@reject-modify-context-protection-on: - shard-tglb: NOTRUN -> [SKIP][32] ([i915#4270]) +3 similar issues [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb1/igt@gem_pxp@reject-modify-context-protection-on.html * igt@gem_render_copy@linear-to-vebox-yf-tiled: - shard-iclb: NOTRUN -> [SKIP][33] ([i915#768]) [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb8/igt@gem_render_copy@linear-to-vebox-yf-tiled.html * igt@gem_userptr_blits@readonly-unsync: - shard-iclb: NOTRUN -> [SKIP][34] ([i915#3297]) +1 similar issue [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb4/igt@gem_userptr_blits@readonly-unsync.html * igt@gem_userptr_blits@unsync-unmap-cycles: - shard-tglb: NOTRUN -> [SKIP][35] ([i915#3297]) +2 similar issues [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb6/igt@gem_userptr_blits@unsync-unmap-cycles.html * igt@gem_workarounds@suspend-resume: - shard-apl: [PASS][36] -> [DMESG-WARN][37] ([i915#180]) +2 similar issues [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-apl8/igt@gem_workarounds@suspend-resume.html [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl6/igt@gem_workarounds@suspend-resume.html * igt@gen3_render_linear_blits: - shard-tglb: NOTRUN -> [SKIP][38] ([fdo#109289]) +5 similar issues [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb3/igt@gen3_render_linear_blits.html - shard-iclb: NOTRUN -> [SKIP][39] ([fdo#109289]) [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb6/igt@gen3_render_linear_blits.html * igt@gen9_exec_parse@allowed-all: - shard-apl: [PASS][40] -> [DMESG-WARN][41] ([i915#1436] / [i915#716]) [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-apl7/igt@gen9_exec_parse@allowed-all.html [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl8/igt@gen9_exec_parse@allowed-all.html * igt@gen9_exec_parse@bb-secure: - shard-tglb: NOTRUN -> [SKIP][42] ([i915#2527] / [i915#2856]) +4 similar issues [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb3/igt@gen9_exec_parse@bb-secure.html * igt@gen9_exec_parse@unaligned-access: - shard-iclb: NOTRUN -> [SKIP][43] ([i915#2856]) +2 similar issues [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb3/igt@gen9_exec_parse@unaligned-access.html * igt@i915_pm_dc@dc6-dpms: - shard-tglb: NOTRUN -> [FAIL][44] ([i915#454]) [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb2/igt@i915_pm_dc@dc6-dpms.html * igt@i915_pm_dc@dc6-psr: - shard-iclb: [PASS][45] -> [FAIL][46] ([i915#454]) [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-iclb7/igt@i915_pm_dc@dc6-psr.html [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb2/igt@i915_pm_dc@dc6-psr.html * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp: - shard-kbl: NOTRUN -> [SKIP][47] ([fdo#109271] / [i915#1937]) [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl1/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp.html * igt@i915_pm_rpm@dpms-mode-unset-non-lpsp: - shard-tglb: NOTRUN -> [SKIP][48] ([fdo#111644] / [i915#1397] / [i915#2411]) +4 similar issues [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb1/igt@i915_pm_rpm@dpms-mode-unset-non-lpsp.html * igt@i915_pm_rpm@modeset-non-lpsp: - shard-iclb: NOTRUN -> [SKIP][49] ([fdo#110892]) +1 similar issue [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb3/igt@i915_pm_rpm@modeset-non-lpsp.html * igt@i915_pm_rpm@modeset-pc8-residency-stress: - shard-tglb: NOTRUN -> [SKIP][50] ([fdo#109506] / [i915#2411]) +1 similar issue [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@i915_pm_rpm@modeset-pc8-residency-stress.html * igt@i915_query@query-topology-unsupported: - shard-tglb: NOTRUN -> [SKIP][51] ([fdo#109302]) [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@i915_query@query-topology-unsupported.html * igt@kms_big_fb@linear-16bpp-rotate-90: - shard-iclb: NOTRUN -> [SKIP][52] ([fdo#110725] / [fdo#111614]) [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb8/igt@kms_big_fb@linear-16bpp-rotate-90.html * igt@kms_big_fb@linear-32bpp-rotate-0: - shard-glk: [PASS][53] -> [DMESG-WARN][54] ([i915#118]) [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-glk4/igt@kms_big_fb@linear-32bpp-rotate-0.html [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk2/igt@kms_big_fb@linear-32bpp-rotate-0.html * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip: - shard-apl: NOTRUN -> [SKIP][55] ([fdo#109271] / [i915#3777]) +2 similar issues [55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl6/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip.html - shard-glk: NOTRUN -> [SKIP][56] ([fdo#109271] / [i915#3777]) [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk6/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip.html - shard-kbl: NOTRUN -> [SKIP][57] ([fdo#109271] / [i915#3777]) [57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl3/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip.html * igt@kms_big_fb@y-tiled-64bpp-rotate-270: - shard-tglb: NOTRUN -> [SKIP][58] ([fdo#111614]) +1 similar issue [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb6/igt@kms_big_fb@y-tiled-64bpp-rotate-270.html * igt@kms_big_fb@yf-tiled-8bpp-rotate-0: - shard-iclb: NOTRUN -> [SKIP][59] ([fdo#110723]) +1 similar issue [59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb1/igt@kms_big_fb@yf-tiled-8bpp-rotate-0.html * igt@kms_big_fb@yf-tiled-addfb-size-overflow: - shard-tglb: NOTRUN -> [SKIP][60] ([fdo#111615]) +6 similar issues [60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb7/igt@kms_big_fb@yf-tiled-addfb-size-overflow.html * igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_ccs: - shard-tglb: NOTRUN -> [SKIP][61] ([i915#3689]) +7 similar issues [61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb1/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_ccs.html * igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc: - shard-apl: NOTRUN -> [SKIP][62] ([fdo#109271] / [i915#3886]) +4 similar issues [62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl3/igt@kms_ccs@pipe-b-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html * igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_mc_ccs: - shard-kbl: NOTRUN -> [SKIP][63] ([fdo#109271] / [i915#3886]) +5 similar issues [63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl6/igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_mc_ccs.html * igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_gen12_rc_ccs_cc: - shard-iclb: NOTRUN -> [SKIP][64] ([fdo#109278] / [i915#3886]) +1 similar issue [64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb7/igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_gen12_rc_ccs_cc.html * igt@kms_ccs@pipe-c-missing-ccs-buffer-yf_tiled_ccs: - shard-tglb: NOTRUN -> [SKIP][65] ([fdo#111615] / [i915#3689]) +7 similar issues [65]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb7/igt@kms_ccs@pipe-c-missing-ccs-buffer-yf_tiled_ccs.html * igt@kms_ccs@pipe-c-random-ccs-data-y_tiled_gen12_mc_ccs: - shard-tglb: NOTRUN -> [SKIP][66] ([i915#3689] / [i915#3886]) +4 similar issues [66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@kms_ccs@pipe-c-random-ccs-data-y_tiled_gen12_mc_ccs.html * igt@kms_cdclk@plane-scaling: - shard-iclb: NOTRUN -> [SKIP][67] ([i915#3742]) [67]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb8/igt@kms_cdclk@plane-scaling.html - shard-tglb: NOTRUN -> [SKIP][68] ([i915#3742]) [68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb6/igt@kms_cdclk@plane-scaling.html * igt@kms_chamelium@hdmi-hpd-for-each-pipe: - shard-kbl: NOTRUN -> [SKIP][69] ([fdo#109271] / [fdo#111827]) +9 similar issues [69]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl1/igt@kms_chamelium@hdmi-hpd-for-each-pipe.html * igt@kms_chamelium@vga-hpd-after-suspend: - shard-glk: NOTRUN -> [SKIP][70] ([fdo#109271] / [fdo#111827]) +5 similar issues [70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk6/igt@kms_chamelium@vga-hpd-after-suspend.html * igt@kms_chamelium@vga-hpd-with-enabled-mode: - shard-iclb: NOTRUN -> [SKIP][71] ([fdo#109284] / [fdo#111827]) +7 similar issues [71]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb1/igt@kms_chamelium@vga-hpd-with-enabled-mode.html * igt@kms_color_chamelium@pipe-a-ctm-0-25: - shard-snb: NOTRUN -> [SKIP][72] ([fdo#109271] / [fdo#111827]) +5 similar issues [72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-snb5/igt@kms_color_chamelium@pipe-a-ctm-0-25.html * igt@kms_color_chamelium@pipe-c-ctm-max: - shard-apl: NOTRUN -> [SKIP][73] ([fdo#109271] / [fdo#111827]) +9 similar issues [73]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl8/igt@kms_color_chamelium@pipe-c-ctm-max.html * igt@kms_color_chamelium@pipe-d-ctm-red-to-blue: - shard-tglb: NOTRUN -> [SKIP][74] ([fdo#109284] / [fdo#111827]) +14 similar issues [74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb6/igt@kms_color_chamelium@pipe-d-ctm-red-to-blue.html * igt@kms_content_protection@dp-mst-type-0: - shard-tglb: NOTRUN -> [SKIP][75] ([i915#3116] / [i915#3299]) [75]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb2/igt@kms_content_protection@dp-mst-type-0.html - shard-iclb: NOTRUN -> [SKIP][76] ([i915#3116]) [76]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb4/igt@kms_content_protection@dp-mst-type-0.html * igt@kms_content_protection@legacy: - shard-kbl: NOTRUN -> [TIMEOUT][77] ([i915#1319]) +1 similar issue [77]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl6/igt@kms_content_protection@legacy.html * igt@kms_content_protection@lic: - shard-iclb: NOTRUN -> [SKIP][78] ([fdo#109300] / [fdo#111066]) [78]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb6/igt@kms_content_protection@lic.html * igt@kms_content_protection@uevent: - shard-kbl: NOTRUN -> [FAIL][79] ([i915#2105]) [79]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl1/igt@kms_content_protection@uevent.html - shard-tglb: NOTRUN -> [SKIP][80] ([i915#1063]) +2 similar issues [80]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@kms_content_protection@uevent.html * igt@kms_cursor_crc@pipe-b-cursor-32x32-sliding: - shard-tglb: NOTRUN -> [SKIP][81] ([i915#3319]) +5 similar issues [81]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb5/igt@kms_cursor_crc@pipe-b-cursor-32x32-sliding.html * igt@kms_cursor_crc@pipe-c-cursor-512x512-random: - shard-iclb: NOTRUN -> [SKIP][82] ([fdo#109278] / [fdo#109279]) +6 similar issues [82]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb5/igt@kms_cursor_crc@pipe-c-cursor-512x512-random.html * igt@kms_cursor_crc@pipe-d-cursor-512x170-offscreen: - shard-tglb: NOTRUN -> [SKIP][83] ([fdo#109279] / [i915#3359]) +8 similar issues [83]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb2/igt@kms_cursor_crc@pipe-d-cursor-512x170-offscreen.html * igt@kms_cursor_crc@pipe-d-cursor-64x64-random: - shard-iclb: NOTRUN -> [SKIP][84] ([fdo#109278]) +22 similar issues [84]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb6/igt@kms_cursor_crc@pipe-d-cursor-64x64-random.html * igt@kms_cursor_crc@pipe-d-cursor-max-size-rapid-movement: - shard-tglb: NOTRUN -> [SKIP][85] ([i915#3359]) +6 similar issues [85]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb2/igt@kms_cursor_crc@pipe-d-cursor-max-size-rapid-movement.html * igt@kms_cursor_legacy@2x-cursor-vs-flip-atomic: - shard-snb: NOTRUN -> [SKIP][86] ([fdo#109271]) +164 similar issues [86]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-snb4/igt@kms_cursor_legacy@2x-cursor-vs-flip-atomic.html * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic: - shard-iclb: NOTRUN -> [SKIP][87] ([fdo#109274] / [fdo#109278]) +5 similar issues [87]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb5/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html * igt@kms_dsc@xrgb8888-dsc-compression: - shard-tglb: NOTRUN -> [SKIP][88] ([i915#3828]) [88]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb3/igt@kms_dsc@xrgb8888-dsc-compression.html * igt@kms_fbcon_fbt@fbc-suspend: - shard-apl: [PASS][89] -> [INCOMPLETE][90] ([i915#180] / [i915#1982]) [89]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-apl1/igt@kms_fbcon_fbt@fbc-suspend.html [90]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl3/igt@kms_fbcon_fbt@fbc-suspend.html * igt@kms_flip@2x-absolute-wf_vblank: - shard-tglb: NOTRUN -> [SKIP][91] ([fdo#109274] / [fdo#111825] / [i915#3966]) [91]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@kms_flip@2x-absolute-wf_vblank.html * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2: - shard-glk: [PASS][92] -> [FAIL][93] ([i915#79]) [92]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-glk6/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2.html [93]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible@ab-hdmi-a1-hdmi-a2.html * igt@kms_flip@2x-modeset-vs-vblank-race-interruptible: - shard-iclb: NOTRUN -> [SKIP][94] ([fdo#109274]) +6 similar issues [94]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb5/igt@kms_flip@2x-modeset-vs-vblank-race-interruptible.html * igt@kms_flip@2x-plain-flip-ts-check: - shard-tglb: NOTRUN -> [SKIP][95] ([fdo#109274] / [fdo#111825]) +22 similar issues [95]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb1/igt@kms_flip@2x-plain-flip-ts-check.html * igt@kms_flip@flip-vs-suspend@c-dp1: - shard-kbl: [PASS][96] -> [DMESG-WARN][97] ([i915#180]) +4 similar issues [96]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-kbl6/igt@kms_flip@flip-vs-suspend@c-dp1.html [97]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl7/igt@kms_flip@flip-vs-suspend@c-dp1.html * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-upscaling: - shard-tglb: NOTRUN -> [SKIP][98] ([i915#2587]) +1 similar issue [98]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-upscaling.html * igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-downscaling: - shard-iclb: [PASS][99] -> [SKIP][100] ([i915#3701]) [99]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-iclb7/igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-downscaling.html [100]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-downscaling.html * igt@kms_frontbuffer_tracking@fbc-2p-shrfb-fliptrack-mmap-gtt: - shard-iclb: NOTRUN -> [SKIP][101] ([fdo#109280]) +20 similar issues [101]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-2p-shrfb-fliptrack-mmap-gtt.html * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-cpu: - shard-iclb: [PASS][102] -> [FAIL][103] ([i915#1888] / [i915#2546]) [102]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-cpu.html [103]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-cpu.html * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-gtt: - shard-tglb: NOTRUN -> [SKIP][104] ([fdo#109280] / [fdo#111825]) +48 similar issues [104]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-gtt.html * igt@kms_hdr@static-toggle: - shard-tglb: NOTRUN -> [SKIP][105] ([i915#1187]) [105]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb5/igt@kms_hdr@static-toggle.html * igt@kms_invalid_mode@clock-too-high: - shard-tglb: NOTRUN -> [SKIP][106] ([i915#4278]) [106]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@kms_invalid_mode@clock-too-high.html - shard-iclb: NOTRUN -> [SKIP][107] ([i915#4278]) [107]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb7/igt@kms_invalid_mode@clock-too-high.html * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc: - shard-kbl: NOTRUN -> [FAIL][108] ([fdo#108145] / [i915#265]) +1 similar issue [108]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl1/igt@kms_plane_alpha_blend@pipe-a-alpha-7efc.html * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb: - shard-apl: NOTRUN -> [FAIL][109] ([fdo#108145] / [i915#265]) [109]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl8/igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb.html * igt@kms_plane_lowres@pipe-a-tiling-x: - shard-iclb: NOTRUN -> [SKIP][110] ([i915#3536]) [110]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-x.html - shard-tglb: NOTRUN -> [SKIP][111] ([i915#3536]) [111]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb7/igt@kms_plane_lowres@pipe-a-tiling-x.html * igt@kms_plane_lowres@pipe-d-tiling-yf: - shard-tglb: NOTRUN -> [SKIP][112] ([fdo#111615] / [fdo#112054]) +1 similar issue [112]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb3/igt@kms_plane_lowres@pipe-d-tiling-yf.html * igt@kms_psr2_sf@cursor-plane-update-sf: - shard-tglb: NOTRUN -> [SKIP][113] ([i915#2920]) +1 similar issue [113]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb1/igt@kms_psr2_sf@cursor-plane-update-sf.html * igt@kms_psr2_sf@overlay-plane-update-continuous-sf: - shard-kbl: NOTRUN -> [SKIP][114] ([fdo#109271] / [i915#658]) +2 similar issues [114]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl1/igt@kms_psr2_sf@overlay-plane-update-continuous-sf.html * igt@kms_psr2_su@page_flip-nv12: - shard-tglb: NOTRUN -> [SKIP][115] ([i915#1911]) [115]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@kms_psr2_su@page_flip-nv12.html - shard-apl: NOTRUN -> [SKIP][116] ([fdo#109271] / [i915#658]) [116]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl6/igt@kms_psr2_su@page_flip-nv12.html - shard-glk: NOTRUN -> [SKIP][117] ([fdo#109271] / [i915#658]) [117]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk7/igt@kms_psr2_su@page_flip-nv12.html - shard-iclb: NOTRUN -> [SKIP][118] ([fdo#109642] / [fdo#111068] / [i915#658]) [118]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb8/igt@kms_psr2_su@page_flip-nv12.html * igt@kms_psr@psr2_cursor_plane_onoff: - shard-tglb: NOTRUN -> [FAIL][119] ([i915#132] / [i915#3467]) +2 similar issues [119]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb5/igt@kms_psr@psr2_cursor_plane_onoff.html * igt@kms_psr@psr2_sprite_mmap_gtt: - shard-iclb: NOTRUN -> [SKIP][120] ([fdo#109441]) [120]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb3/igt@kms_psr@psr2_sprite_mmap_gtt.html * igt@kms_psr@psr2_sprite_plane_move: - shard-iclb: [PASS][121] -> [SKIP][122] ([fdo#109441]) +2 similar issues [121]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_6365/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html [122]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb8/igt@kms_psr@psr2_sprite_plane_move.html * igt@kms_scaling_modes@scaling-mode-full-aspect: - shard-kbl: NOTRUN -> [SKIP][123] ([fdo#109271]) +197 similar issues [123]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl6/igt@kms_scaling_modes@scaling-mode-full-aspect.html * igt@kms_tv_load_detect@load-detect: - shard-tglb: NOTRUN -> [SKIP][124] ([fdo#109309]) [124]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb2/igt@kms_tv_load_detect@load-detect.html * igt@kms_vblank@pipe-d-wait-idle: - shard-kbl: NOTRUN -> [SKIP][125] ([fdo#109271] / [i915#533]) +3 similar issues [125]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-kbl6/igt@kms_vblank@pipe-d-wait-idle.html - shard-apl: NOTRUN -> [SKIP][126] ([fdo#109271] / [i915#533]) +1 similar issue [126]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl2/igt@kms_vblank@pipe-d-wait-idle.html - shard-glk: NOTRUN -> [SKIP][127] ([fdo#109271] / [i915#533]) +1 similar issue [127]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-glk2/igt@kms_vblank@pipe-d-wait-idle.html * igt@kms_vrr@flipline: - shard-tglb: NOTRUN -> [SKIP][128] ([fdo#109502]) [128]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb1/igt@kms_vrr@flipline.html * igt@kms_writeback@writeback-invalid-parameters: - shard-apl: NOTRUN -> [SKIP][129] ([fdo#109271] / [i915#2437]) [129]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-apl2/igt@kms_writeback@writeback-invalid-parameters.html * igt@nouveau_crc@pipe-b-source-outp-complete: - shard-iclb: NOTRUN -> [SKIP][130] ([i915#2530]) +3 similar issues [130]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb8/igt@nouveau_crc@pipe-b-source-outp-complete.html * igt@nouveau_crc@pipe-d-ctx-flip-detection: - shard-tglb: NOTRUN -> [SKIP][131] ([i915#2530]) +8 similar issues [131]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-tglb8/igt@nouveau_crc@pipe-d-ctx-flip-detection.html - shard-iclb: NOTRUN -> [SKIP][132] ([fdo#109278] / [i915#2530]) [132]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/shard-iclb5/igt@nouveau_crc@pipe-d-ctx-flip-detection.html * igt@prime_nv_api@i915_nv_import_twice_check_flink_name: == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6743/index.html [-- Attachment #2: Type: text/html, Size: 33986 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini 2022-03-07 8:26 ` [igt-dev] " Janusz Krzysztofik @ 2022-03-07 13:23 ` Kamil Konieczny -1 siblings, 0 replies; 10+ messages in thread From: Kamil Konieczny @ 2022-03-07 13:23 UTC (permalink / raw) To: igt-dev; +Cc: intel-gfx Hi Janusz, Dnia 2022-03-07 at 09:26:43 +0100, Janusz Krzysztofik napisał(a): > Commit 5f3cfa485eb4 ("lib: Use safe wrappers around libpciaccess > initialization functions") took care of not leaking memory allocated by > pci_system_init() but didn't take care of users potentially attempting to > reinitialize global data maintained by libpciaccess. For example, > intel_register_access_init() mmaps device's PCI BAR0 resource with > pci_device_map_range() but intel_register_access_fini() doesn't unmap it > and next call to intel_register_access_init() fails on attempt to mmap it > again. > > Fix it, and also provide intel_mmio_unmap_*() counterparts to public > functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file(). > > v2: apply last minute fixes, cached but unfortunately not committed before > sending > v3: use .pci_device_id field content as an indicator of arg initialization > via intel_register_access_init(), > - improve checks of argument initialization status, > - shorten warning messages (Kamil), > - don't fill .mmio_size field until initialization succeeds (Kamil) > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > --- > lib/intel_io.h | 4 +++ > lib/intel_mmio.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 65 insertions(+), 3 deletions(-) > > diff --git a/lib/intel_io.h b/lib/intel_io.h > index 1cfe4fb6b9..ea2649d9bc 100644 > --- a/lib/intel_io.h > +++ b/lib/intel_io.h > @@ -49,6 +49,8 @@ struct intel_register_map { > > struct intel_mmio_data { > void *igt_mmio; > + size_t mmio_size; > + struct pci_device *dev; > struct intel_register_map map; > uint32_t pci_device_id; > int key; > @@ -57,7 +59,9 @@ struct intel_mmio_data { > > void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, > struct pci_device *pci_dev); > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data); > void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file); > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data); > > int intel_register_access_init(struct intel_mmio_data *mmio_data, > struct pci_device *pci_dev, int safe, int fd); > diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c > index 667a69f5aa..d6ce0ee3ea 100644 > --- a/lib/intel_mmio.c > +++ b/lib/intel_mmio.c > @@ -82,6 +82,8 @@ void *igt_global_mmio; > * Sets also up mmio_data->igt_mmio to point at the data contained > * in @file. This allows the same code to get reused for dumping and decoding > * from running hardware as from register dumps. > + * > + * Users are expected to call intel_mmio_unmap_dump_file() after use. > */ > void > intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > @@ -99,11 +101,32 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > igt_fail_on_f(mmio_data->igt_mmio == MAP_FAILED, > "Couldn't mmap %s\n", file); > > + mmio_data->mmio_size = st.st_size; > igt_global_mmio = mmio_data->igt_mmio; > > close(fd); > } > > +/** > + * intel_mmio_unmap_dump_file: > + * @mmio_data: mmio structure for IO operations > + * > + * Unmaps a dump file mmapped with intel_mmio_use_dump_file() > + */ > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data) > +{ > + if (igt_warn_on_f(mmio_data->dev, > + "test bug: arg initialized with intel_mmio_use_pci_bar()\n")) > + return; Please add a global description about this kind of errors, this one is for using unmap when mmio was mmap-ed from other mmap type. > + if (igt_warn_on_f(!mmio_data->mmio_size, > + "test bug: arg not initialized\n")) > + return; Can we replace this with one check igt_global_mmio != NULL ? Something like: if (igt_warn_on_f(!igt_global_mmio, "mmio regs not mmap-ed\n")) return; Or should we add this before all other checks in unmap functions and keep this additional check ? > + > + igt_global_mmio = NULL; > + igt_debug_on(munmap(mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > + mmio_data->mmio_size = 0; > +} > + > /** > * intel_mmio_use_pci_bar: > * @mmio_data: mmio structure for IO operations > @@ -112,6 +135,8 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > * Fill a mmio_data stucture with igt_mmio to point at the mmio bar. > * > * @pci_dev can be obtained from intel_get_pci_device(). > + * > + * Users are expected to call intel_mmio_unmap_pci_bar() after use. > */ > void > intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev) > @@ -141,10 +166,34 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci > PCI_DEV_MAP_FLAG_WRITABLE, > &mmio_data->igt_mmio); > > - igt_global_mmio = mmio_data->igt_mmio; > - > igt_fail_on_f(error != 0, > "Couldn't map MMIO region\n"); > + > + mmio_data->mmio_size = mmio_size; > + mmio_data->dev = pci_dev; > + igt_global_mmio = mmio_data->igt_mmio; > +} > + > +/** > + * intel_mmio_unmap_pci_bar: > + * @mmio_data: mmio structure for IO operations > + * > + * Unmaps a PCI BAR region mmapped with intel_mmio_use_pci_bar() > + */ > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data) > +{ > + if (igt_warn_on_f(mmio_data->pci_device_id, > + "test bug: arg initialized with intel_register_access_init()\n")) > + return; > + if (igt_warn_on_f(!mmio_data->dev, > + "test bug: arg not initialized with intel_mmio_use_pci_bar()\n")) > + return; > + > + igt_global_mmio = NULL; > + igt_debug_on(pci_device_unmap_range(mmio_data->dev, > + mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > + mmio_data->dev = NULL; > + mmio_data->mmio_size = 0; > } > > static void > @@ -166,6 +215,8 @@ release_forcewake_lock(int fd) > * It also initializes mmio_data->igt_mmio like intel_mmio_use_pci_bar(). > * > * @pci_dev can be obtained from intel_get_pci_device(). > + * > + * Users are expected to call intel_register_access_fini() after use. > */ > int > intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd) > @@ -222,8 +273,15 @@ int intel_register_access_needs_fakewake(struct intel_mmio_data *mmio_data) > void > intel_register_access_fini(struct intel_mmio_data *mmio_data) > { > - if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) > + if (igt_warn_on_f(!mmio_data->pci_device_id, > + "test bug: arg not initialized with intel_register_access_init()\n")) > + return; > + > + if (intel_register_access_needs_wake(mmio_data)) > release_forcewake_lock(mmio_data->key); > + > + mmio_data->pci_device_id = 0; Here we should check other conditions so no warn triggers in unmap_pci_bar or make the messages generic (and document it in comments at beggining) or maybe make helper with no checks for unmap_pci_bar. > + intel_mmio_unmap_pci_bar(mmio_data); > } > > /** > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [igt-dev] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini @ 2022-03-07 13:23 ` Kamil Konieczny 0 siblings, 0 replies; 10+ messages in thread From: Kamil Konieczny @ 2022-03-07 13:23 UTC (permalink / raw) To: igt-dev; +Cc: intel-gfx Hi Janusz, Dnia 2022-03-07 at 09:26:43 +0100, Janusz Krzysztofik napisał(a): > Commit 5f3cfa485eb4 ("lib: Use safe wrappers around libpciaccess > initialization functions") took care of not leaking memory allocated by > pci_system_init() but didn't take care of users potentially attempting to > reinitialize global data maintained by libpciaccess. For example, > intel_register_access_init() mmaps device's PCI BAR0 resource with > pci_device_map_range() but intel_register_access_fini() doesn't unmap it > and next call to intel_register_access_init() fails on attempt to mmap it > again. > > Fix it, and also provide intel_mmio_unmap_*() counterparts to public > functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file(). > > v2: apply last minute fixes, cached but unfortunately not committed before > sending > v3: use .pci_device_id field content as an indicator of arg initialization > via intel_register_access_init(), > - improve checks of argument initialization status, > - shorten warning messages (Kamil), > - don't fill .mmio_size field until initialization succeeds (Kamil) > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > --- > lib/intel_io.h | 4 +++ > lib/intel_mmio.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--- > 2 files changed, 65 insertions(+), 3 deletions(-) > > diff --git a/lib/intel_io.h b/lib/intel_io.h > index 1cfe4fb6b9..ea2649d9bc 100644 > --- a/lib/intel_io.h > +++ b/lib/intel_io.h > @@ -49,6 +49,8 @@ struct intel_register_map { > > struct intel_mmio_data { > void *igt_mmio; > + size_t mmio_size; > + struct pci_device *dev; > struct intel_register_map map; > uint32_t pci_device_id; > int key; > @@ -57,7 +59,9 @@ struct intel_mmio_data { > > void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, > struct pci_device *pci_dev); > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data); > void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file); > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data); > > int intel_register_access_init(struct intel_mmio_data *mmio_data, > struct pci_device *pci_dev, int safe, int fd); > diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c > index 667a69f5aa..d6ce0ee3ea 100644 > --- a/lib/intel_mmio.c > +++ b/lib/intel_mmio.c > @@ -82,6 +82,8 @@ void *igt_global_mmio; > * Sets also up mmio_data->igt_mmio to point at the data contained > * in @file. This allows the same code to get reused for dumping and decoding > * from running hardware as from register dumps. > + * > + * Users are expected to call intel_mmio_unmap_dump_file() after use. > */ > void > intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > @@ -99,11 +101,32 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > igt_fail_on_f(mmio_data->igt_mmio == MAP_FAILED, > "Couldn't mmap %s\n", file); > > + mmio_data->mmio_size = st.st_size; > igt_global_mmio = mmio_data->igt_mmio; > > close(fd); > } > > +/** > + * intel_mmio_unmap_dump_file: > + * @mmio_data: mmio structure for IO operations > + * > + * Unmaps a dump file mmapped with intel_mmio_use_dump_file() > + */ > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data) > +{ > + if (igt_warn_on_f(mmio_data->dev, > + "test bug: arg initialized with intel_mmio_use_pci_bar()\n")) > + return; Please add a global description about this kind of errors, this one is for using unmap when mmio was mmap-ed from other mmap type. > + if (igt_warn_on_f(!mmio_data->mmio_size, > + "test bug: arg not initialized\n")) > + return; Can we replace this with one check igt_global_mmio != NULL ? Something like: if (igt_warn_on_f(!igt_global_mmio, "mmio regs not mmap-ed\n")) return; Or should we add this before all other checks in unmap functions and keep this additional check ? > + > + igt_global_mmio = NULL; > + igt_debug_on(munmap(mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > + mmio_data->mmio_size = 0; > +} > + > /** > * intel_mmio_use_pci_bar: > * @mmio_data: mmio structure for IO operations > @@ -112,6 +135,8 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > * Fill a mmio_data stucture with igt_mmio to point at the mmio bar. > * > * @pci_dev can be obtained from intel_get_pci_device(). > + * > + * Users are expected to call intel_mmio_unmap_pci_bar() after use. > */ > void > intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev) > @@ -141,10 +166,34 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci > PCI_DEV_MAP_FLAG_WRITABLE, > &mmio_data->igt_mmio); > > - igt_global_mmio = mmio_data->igt_mmio; > - > igt_fail_on_f(error != 0, > "Couldn't map MMIO region\n"); > + > + mmio_data->mmio_size = mmio_size; > + mmio_data->dev = pci_dev; > + igt_global_mmio = mmio_data->igt_mmio; > +} > + > +/** > + * intel_mmio_unmap_pci_bar: > + * @mmio_data: mmio structure for IO operations > + * > + * Unmaps a PCI BAR region mmapped with intel_mmio_use_pci_bar() > + */ > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data) > +{ > + if (igt_warn_on_f(mmio_data->pci_device_id, > + "test bug: arg initialized with intel_register_access_init()\n")) > + return; > + if (igt_warn_on_f(!mmio_data->dev, > + "test bug: arg not initialized with intel_mmio_use_pci_bar()\n")) > + return; > + > + igt_global_mmio = NULL; > + igt_debug_on(pci_device_unmap_range(mmio_data->dev, > + mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > + mmio_data->dev = NULL; > + mmio_data->mmio_size = 0; > } > > static void > @@ -166,6 +215,8 @@ release_forcewake_lock(int fd) > * It also initializes mmio_data->igt_mmio like intel_mmio_use_pci_bar(). > * > * @pci_dev can be obtained from intel_get_pci_device(). > + * > + * Users are expected to call intel_register_access_fini() after use. > */ > int > intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd) > @@ -222,8 +273,15 @@ int intel_register_access_needs_fakewake(struct intel_mmio_data *mmio_data) > void > intel_register_access_fini(struct intel_mmio_data *mmio_data) > { > - if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) > + if (igt_warn_on_f(!mmio_data->pci_device_id, > + "test bug: arg not initialized with intel_register_access_init()\n")) > + return; > + > + if (intel_register_access_needs_wake(mmio_data)) > release_forcewake_lock(mmio_data->key); > + > + mmio_data->pci_device_id = 0; Here we should check other conditions so no warn triggers in unmap_pci_bar or make the messages generic (and document it in comments at beggining) or maybe make helper with no checks for unmap_pci_bar. > + intel_mmio_unmap_pci_bar(mmio_data); > } > > /** > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini 2022-03-07 13:23 ` [igt-dev] " Kamil Konieczny @ 2022-03-07 15:06 ` Janusz Krzysztofik -1 siblings, 0 replies; 10+ messages in thread From: Janusz Krzysztofik @ 2022-03-07 15:06 UTC (permalink / raw) To: Kamil Konieczny, igt-dev, intel-gfx, Janusz Krzysztofik Hi Kamil, On Monday, 7 March 2022 14:23:30 CET Kamil Konieczny wrote: > Hi Janusz, > > Dnia 2022-03-07 at 09:26:43 +0100, Janusz Krzysztofik napisał(a): > > Commit 5f3cfa485eb4 ("lib: Use safe wrappers around libpciaccess > > initialization functions") took care of not leaking memory allocated by > > pci_system_init() but didn't take care of users potentially attempting to > > reinitialize global data maintained by libpciaccess. For example, > > intel_register_access_init() mmaps device's PCI BAR0 resource with > > pci_device_map_range() but intel_register_access_fini() doesn't unmap it > > and next call to intel_register_access_init() fails on attempt to mmap it > > again. > > > > Fix it, and also provide intel_mmio_unmap_*() counterparts to public > > functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file(). > > > > v2: apply last minute fixes, cached but unfortunately not committed before > > sending > > v3: use .pci_device_id field content as an indicator of arg initialization > > via intel_register_access_init(), > > - improve checks of argument initialization status, > > - shorten warning messages (Kamil), > > - don't fill .mmio_size field until initialization succeeds (Kamil) > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > > --- > > lib/intel_io.h | 4 +++ > > lib/intel_mmio.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--- > > 2 files changed, 65 insertions(+), 3 deletions(-) > > > > diff --git a/lib/intel_io.h b/lib/intel_io.h > > index 1cfe4fb6b9..ea2649d9bc 100644 > > --- a/lib/intel_io.h > > +++ b/lib/intel_io.h > > @@ -49,6 +49,8 @@ struct intel_register_map { > > > > struct intel_mmio_data { > > void *igt_mmio; > > + size_t mmio_size; > > + struct pci_device *dev; > > struct intel_register_map map; > > uint32_t pci_device_id; > > int key; > > @@ -57,7 +59,9 @@ struct intel_mmio_data { > > > > void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, > > struct pci_device *pci_dev); > > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data); > > void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file); > > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data); > > > > int intel_register_access_init(struct intel_mmio_data *mmio_data, > > struct pci_device *pci_dev, int safe, int fd); > > diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c > > index 667a69f5aa..d6ce0ee3ea 100644 > > --- a/lib/intel_mmio.c > > +++ b/lib/intel_mmio.c > > @@ -82,6 +82,8 @@ void *igt_global_mmio; > > * Sets also up mmio_data->igt_mmio to point at the data contained > > * in @file. This allows the same code to get reused for dumping and decoding > > * from running hardware as from register dumps. > > + * > > + * Users are expected to call intel_mmio_unmap_dump_file() after use. > > */ > > void > > intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > @@ -99,11 +101,32 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > igt_fail_on_f(mmio_data->igt_mmio == MAP_FAILED, > > "Couldn't mmap %s\n", file); > > > > + mmio_data->mmio_size = st.st_size; > > igt_global_mmio = mmio_data->igt_mmio; > > > > close(fd); > > } > > > > +/** > > + * intel_mmio_unmap_dump_file: > > + * @mmio_data: mmio structure for IO operations > > + * > > + * Unmaps a dump file mmapped with intel_mmio_use_dump_file() > > + */ > > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data) > > +{ > > + if (igt_warn_on_f(mmio_data->dev, > > + "test bug: arg initialized with intel_mmio_use_pci_bar()\n")) > > + return; > > Please add a global description about this kind of errors, this > one is for using unmap when mmio was mmap-ed from other mmap > type. Can you please be more specific in what you mean by "global description of this kind of errors"? A more detailed warning? A comment? If the latter then how would you like me to make it global? If you just don't like the reference to intel_mmio_use_pci_bar() here then would you be satisfied with something like "test bug: arg initialized by a method other than intel_mmio_use_dump_file()\n"? > > + if (igt_warn_on_f(!mmio_data->mmio_size, > > + "test bug: arg not initialized\n")) > > + return; > > Can we replace this with one check igt_global_mmio != NULL ? > Something like: > > if (igt_warn_on_f(!igt_global_mmio, > "mmio regs not mmap-ed\n")) > return; > > Or should we add this before all other checks in unmap functions > and keep this additional check ? Why igt_global_mmio again? I still think this global variable is broken and users should just use the structure they pass to intel_mmio_use_*() or intel_register_access_init(), then introducing another a dependency on it with this patch doesn't make sense to me. If you think the opposite then please explain why. > > + > > + igt_global_mmio = NULL; > > + igt_debug_on(munmap(mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > > + mmio_data->mmio_size = 0; > > +} > > + > > /** > > * intel_mmio_use_pci_bar: > > * @mmio_data: mmio structure for IO operations > > @@ -112,6 +135,8 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > * Fill a mmio_data stucture with igt_mmio to point at the mmio bar. > > * > > * @pci_dev can be obtained from intel_get_pci_device(). > > + * > > + * Users are expected to call intel_mmio_unmap_pci_bar() after use. > > */ > > void > > intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev) > > @@ -141,10 +166,34 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci > > PCI_DEV_MAP_FLAG_WRITABLE, > > &mmio_data->igt_mmio); > > > > - igt_global_mmio = mmio_data->igt_mmio; > > - > > igt_fail_on_f(error != 0, > > "Couldn't map MMIO region\n"); > > + > > + mmio_data->mmio_size = mmio_size; > > + mmio_data->dev = pci_dev; > > + igt_global_mmio = mmio_data->igt_mmio; To be consequent with what I said above, in next version of the patch I'm going to not touch the assignment of mmio_data->igt_mmio to the out of scope and depreciated igt_global_mmio, leaving it as broken as it already is. > > +} > > + > > +/** > > + * intel_mmio_unmap_pci_bar: > > + * @mmio_data: mmio structure for IO operations > > + * > > + * Unmaps a PCI BAR region mmapped with intel_mmio_use_pci_bar() > > + */ > > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data) > > +{ > > + if (igt_warn_on_f(mmio_data->pci_device_id, > > + "test bug: arg initialized with intel_register_access_init()\n")) Similarly to the case of intel_mmio_unmap_dump_file(), I can change the message to "test bug: arg initialized with a method other than intel_mmio_use_pci_bar()\n" if that's what you prefer. > > + return; > > + if (igt_warn_on_f(!mmio_data->dev, > > + "test bug: arg not initialized with intel_mmio_use_pci_bar()\n")) > > + return; > > + > > + igt_global_mmio = NULL; > > + igt_debug_on(pci_device_unmap_range(mmio_data->dev, > > + mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > > + mmio_data->dev = NULL; > > + mmio_data->mmio_size = 0; > > } > > > > static void > > @@ -166,6 +215,8 @@ release_forcewake_lock(int fd) > > * It also initializes mmio_data->igt_mmio like intel_mmio_use_pci_bar(). > > * > > * @pci_dev can be obtained from intel_get_pci_device(). > > + * > > + * Users are expected to call intel_register_access_fini() after use. > > */ > > int > > intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd) > > @@ -222,8 +273,15 @@ int intel_register_access_needs_fakewake(struct intel_mmio_data *mmio_data) > > void > > intel_register_access_fini(struct intel_mmio_data *mmio_data) > > { > > - if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) > > + if (igt_warn_on_f(!mmio_data->pci_device_id, > > + "test bug: arg not initialized with intel_register_access_init()\n")) > > + return; > > + > > + if (intel_register_access_needs_wake(mmio_data)) As mmio_data->key is no longer used since v3 as an indication of mmio_data having been initialized by intel_register_access_init() or not, the original (potentially broken) condition: if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) should be not touched by this patch as out of scope but its original form preserved. I'm going to restore it in next version of the patch. > > release_forcewake_lock(mmio_data->key); > > + > > + mmio_data->pci_device_id = 0; > > Here we should check other conditions so no warn triggers in unmap_pci_bar > or make the messages generic (and document it in comments at beggining) > or maybe make helper with no checks for unmap_pci_bar. Why? I can't see any potential scenario where mmio_data->pci_device_id is not 0 but mmio_data->dev is NULL. If you can see one then please tell me more about it. Thanks, Janusz > > + intel_mmio_unmap_pci_bar(mmio_data); > > } > > > > /** > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [igt-dev] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini @ 2022-03-07 15:06 ` Janusz Krzysztofik 0 siblings, 0 replies; 10+ messages in thread From: Janusz Krzysztofik @ 2022-03-07 15:06 UTC (permalink / raw) To: Kamil Konieczny, igt-dev, intel-gfx, Janusz Krzysztofik Hi Kamil, On Monday, 7 March 2022 14:23:30 CET Kamil Konieczny wrote: > Hi Janusz, > > Dnia 2022-03-07 at 09:26:43 +0100, Janusz Krzysztofik napisał(a): > > Commit 5f3cfa485eb4 ("lib: Use safe wrappers around libpciaccess > > initialization functions") took care of not leaking memory allocated by > > pci_system_init() but didn't take care of users potentially attempting to > > reinitialize global data maintained by libpciaccess. For example, > > intel_register_access_init() mmaps device's PCI BAR0 resource with > > pci_device_map_range() but intel_register_access_fini() doesn't unmap it > > and next call to intel_register_access_init() fails on attempt to mmap it > > again. > > > > Fix it, and also provide intel_mmio_unmap_*() counterparts to public > > functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file(). > > > > v2: apply last minute fixes, cached but unfortunately not committed before > > sending > > v3: use .pci_device_id field content as an indicator of arg initialization > > via intel_register_access_init(), > > - improve checks of argument initialization status, > > - shorten warning messages (Kamil), > > - don't fill .mmio_size field until initialization succeeds (Kamil) > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > > --- > > lib/intel_io.h | 4 +++ > > lib/intel_mmio.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--- > > 2 files changed, 65 insertions(+), 3 deletions(-) > > > > diff --git a/lib/intel_io.h b/lib/intel_io.h > > index 1cfe4fb6b9..ea2649d9bc 100644 > > --- a/lib/intel_io.h > > +++ b/lib/intel_io.h > > @@ -49,6 +49,8 @@ struct intel_register_map { > > > > struct intel_mmio_data { > > void *igt_mmio; > > + size_t mmio_size; > > + struct pci_device *dev; > > struct intel_register_map map; > > uint32_t pci_device_id; > > int key; > > @@ -57,7 +59,9 @@ struct intel_mmio_data { > > > > void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, > > struct pci_device *pci_dev); > > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data); > > void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file); > > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data); > > > > int intel_register_access_init(struct intel_mmio_data *mmio_data, > > struct pci_device *pci_dev, int safe, int fd); > > diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c > > index 667a69f5aa..d6ce0ee3ea 100644 > > --- a/lib/intel_mmio.c > > +++ b/lib/intel_mmio.c > > @@ -82,6 +82,8 @@ void *igt_global_mmio; > > * Sets also up mmio_data->igt_mmio to point at the data contained > > * in @file. This allows the same code to get reused for dumping and decoding > > * from running hardware as from register dumps. > > + * > > + * Users are expected to call intel_mmio_unmap_dump_file() after use. > > */ > > void > > intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > @@ -99,11 +101,32 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > igt_fail_on_f(mmio_data->igt_mmio == MAP_FAILED, > > "Couldn't mmap %s\n", file); > > > > + mmio_data->mmio_size = st.st_size; > > igt_global_mmio = mmio_data->igt_mmio; > > > > close(fd); > > } > > > > +/** > > + * intel_mmio_unmap_dump_file: > > + * @mmio_data: mmio structure for IO operations > > + * > > + * Unmaps a dump file mmapped with intel_mmio_use_dump_file() > > + */ > > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data) > > +{ > > + if (igt_warn_on_f(mmio_data->dev, > > + "test bug: arg initialized with intel_mmio_use_pci_bar()\n")) > > + return; > > Please add a global description about this kind of errors, this > one is for using unmap when mmio was mmap-ed from other mmap > type. Can you please be more specific in what you mean by "global description of this kind of errors"? A more detailed warning? A comment? If the latter then how would you like me to make it global? If you just don't like the reference to intel_mmio_use_pci_bar() here then would you be satisfied with something like "test bug: arg initialized by a method other than intel_mmio_use_dump_file()\n"? > > + if (igt_warn_on_f(!mmio_data->mmio_size, > > + "test bug: arg not initialized\n")) > > + return; > > Can we replace this with one check igt_global_mmio != NULL ? > Something like: > > if (igt_warn_on_f(!igt_global_mmio, > "mmio regs not mmap-ed\n")) > return; > > Or should we add this before all other checks in unmap functions > and keep this additional check ? Why igt_global_mmio again? I still think this global variable is broken and users should just use the structure they pass to intel_mmio_use_*() or intel_register_access_init(), then introducing another a dependency on it with this patch doesn't make sense to me. If you think the opposite then please explain why. > > + > > + igt_global_mmio = NULL; > > + igt_debug_on(munmap(mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > > + mmio_data->mmio_size = 0; > > +} > > + > > /** > > * intel_mmio_use_pci_bar: > > * @mmio_data: mmio structure for IO operations > > @@ -112,6 +135,8 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > * Fill a mmio_data stucture with igt_mmio to point at the mmio bar. > > * > > * @pci_dev can be obtained from intel_get_pci_device(). > > + * > > + * Users are expected to call intel_mmio_unmap_pci_bar() after use. > > */ > > void > > intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev) > > @@ -141,10 +166,34 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci > > PCI_DEV_MAP_FLAG_WRITABLE, > > &mmio_data->igt_mmio); > > > > - igt_global_mmio = mmio_data->igt_mmio; > > - > > igt_fail_on_f(error != 0, > > "Couldn't map MMIO region\n"); > > + > > + mmio_data->mmio_size = mmio_size; > > + mmio_data->dev = pci_dev; > > + igt_global_mmio = mmio_data->igt_mmio; To be consequent with what I said above, in next version of the patch I'm going to not touch the assignment of mmio_data->igt_mmio to the out of scope and depreciated igt_global_mmio, leaving it as broken as it already is. > > +} > > + > > +/** > > + * intel_mmio_unmap_pci_bar: > > + * @mmio_data: mmio structure for IO operations > > + * > > + * Unmaps a PCI BAR region mmapped with intel_mmio_use_pci_bar() > > + */ > > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data) > > +{ > > + if (igt_warn_on_f(mmio_data->pci_device_id, > > + "test bug: arg initialized with intel_register_access_init()\n")) Similarly to the case of intel_mmio_unmap_dump_file(), I can change the message to "test bug: arg initialized with a method other than intel_mmio_use_pci_bar()\n" if that's what you prefer. > > + return; > > + if (igt_warn_on_f(!mmio_data->dev, > > + "test bug: arg not initialized with intel_mmio_use_pci_bar()\n")) > > + return; > > + > > + igt_global_mmio = NULL; > > + igt_debug_on(pci_device_unmap_range(mmio_data->dev, > > + mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > > + mmio_data->dev = NULL; > > + mmio_data->mmio_size = 0; > > } > > > > static void > > @@ -166,6 +215,8 @@ release_forcewake_lock(int fd) > > * It also initializes mmio_data->igt_mmio like intel_mmio_use_pci_bar(). > > * > > * @pci_dev can be obtained from intel_get_pci_device(). > > + * > > + * Users are expected to call intel_register_access_fini() after use. > > */ > > int > > intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd) > > @@ -222,8 +273,15 @@ int intel_register_access_needs_fakewake(struct intel_mmio_data *mmio_data) > > void > > intel_register_access_fini(struct intel_mmio_data *mmio_data) > > { > > - if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) > > + if (igt_warn_on_f(!mmio_data->pci_device_id, > > + "test bug: arg not initialized with intel_register_access_init()\n")) > > + return; > > + > > + if (intel_register_access_needs_wake(mmio_data)) As mmio_data->key is no longer used since v3 as an indication of mmio_data having been initialized by intel_register_access_init() or not, the original (potentially broken) condition: if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) should be not touched by this patch as out of scope but its original form preserved. I'm going to restore it in next version of the patch. > > release_forcewake_lock(mmio_data->key); > > + > > + mmio_data->pci_device_id = 0; > > Here we should check other conditions so no warn triggers in unmap_pci_bar > or make the messages generic (and document it in comments at beggining) > or maybe make helper with no checks for unmap_pci_bar. Why? I can't see any potential scenario where mmio_data->pci_device_id is not 0 but mmio_data->dev is NULL. If you can see one then please tell me more about it. Thanks, Janusz > > + intel_mmio_unmap_pci_bar(mmio_data); > > } > > > > /** > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini 2022-03-07 15:06 ` [igt-dev] " Janusz Krzysztofik @ 2022-03-07 17:04 ` Kamil Konieczny -1 siblings, 0 replies; 10+ messages in thread From: Kamil Konieczny @ 2022-03-07 17:04 UTC (permalink / raw) To: igt-dev; +Cc: intel-gfx Hi Janusz, Dnia 2022-03-07 at 16:06:10 +0100, Janusz Krzysztofik napisał(a): > Hi Kamil, > > On Monday, 7 March 2022 14:23:30 CET Kamil Konieczny wrote: > > Hi Janusz, > > > > Dnia 2022-03-07 at 09:26:43 +0100, Janusz Krzysztofik napisał(a): > > > Commit 5f3cfa485eb4 ("lib: Use safe wrappers around libpciaccess > > > initialization functions") took care of not leaking memory allocated by > > > pci_system_init() but didn't take care of users potentially attempting to > > > reinitialize global data maintained by libpciaccess. For example, > > > intel_register_access_init() mmaps device's PCI BAR0 resource with > > > pci_device_map_range() but intel_register_access_fini() doesn't unmap it > > > and next call to intel_register_access_init() fails on attempt to mmap it > > > again. > > > > > > Fix it, and also provide intel_mmio_unmap_*() counterparts to public > > > functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file(). > > > > > > v2: apply last minute fixes, cached but unfortunately not committed before > > > sending > > > v3: use .pci_device_id field content as an indicator of arg initialization > > > via intel_register_access_init(), > > > - improve checks of argument initialization status, > > > - shorten warning messages (Kamil), > > > - don't fill .mmio_size field until initialization succeeds (Kamil) > > > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > > > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > > > --- > > > lib/intel_io.h | 4 +++ > > > lib/intel_mmio.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--- > > > 2 files changed, 65 insertions(+), 3 deletions(-) > > > > > > diff --git a/lib/intel_io.h b/lib/intel_io.h > > > index 1cfe4fb6b9..ea2649d9bc 100644 > > > --- a/lib/intel_io.h > > > +++ b/lib/intel_io.h > > > @@ -49,6 +49,8 @@ struct intel_register_map { > > > > > > struct intel_mmio_data { > > > void *igt_mmio; > > > + size_t mmio_size; > > > + struct pci_device *dev; > > > struct intel_register_map map; > > > uint32_t pci_device_id; > > > int key; > > > @@ -57,7 +59,9 @@ struct intel_mmio_data { > > > > > > void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, > > > struct pci_device *pci_dev); > > > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data); > > > void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file); > > > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data); > > > > > > int intel_register_access_init(struct intel_mmio_data *mmio_data, > > > struct pci_device *pci_dev, int safe, int fd); > > > diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c > > > index 667a69f5aa..d6ce0ee3ea 100644 > > > --- a/lib/intel_mmio.c > > > +++ b/lib/intel_mmio.c > > > @@ -82,6 +82,8 @@ void *igt_global_mmio; > > > * Sets also up mmio_data->igt_mmio to point at the data contained > > > * in @file. This allows the same code to get reused for dumping and decoding > > > * from running hardware as from register dumps. > > > + * > > > + * Users are expected to call intel_mmio_unmap_dump_file() after use. > > > */ > > > void > > > intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > > @@ -99,11 +101,32 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > > igt_fail_on_f(mmio_data->igt_mmio == MAP_FAILED, > > > "Couldn't mmap %s\n", file); > > > > > > + mmio_data->mmio_size = st.st_size; > > > igt_global_mmio = mmio_data->igt_mmio; > > > > > > close(fd); > > > } > > > > > > +/** > > > + * intel_mmio_unmap_dump_file: > > > + * @mmio_data: mmio structure for IO operations > > > + * > > > + * Unmaps a dump file mmapped with intel_mmio_use_dump_file() > > > + */ > > > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data) > > > +{ > > > + if (igt_warn_on_f(mmio_data->dev, > > > + "test bug: arg initialized with intel_mmio_use_pci_bar()\n")) > > > + return; > > > > Please add a global description about this kind of errors, this > > one is for using unmap when mmio was mmap-ed from other mmap > > type. > > Can you please be more specific in what you mean by "global description of > this kind of errors"? A more detailed warning? A comment? If the latter > then how would you like me to make it global? Yes, I was thinking about comment at begin of file, but maybe it is better to change warning message like below. > > If you just don't like the reference to intel_mmio_use_pci_bar() here then > would you be satisfied with something like "test bug: arg initialized by a > method other than intel_mmio_use_dump_file()\n"? Yes, this sounds good. > > > > + if (igt_warn_on_f(!mmio_data->mmio_size, > > > + "test bug: arg not initialized\n")) > > > + return; > > > > Can we replace this with one check igt_global_mmio != NULL ? > > Something like: > > > > if (igt_warn_on_f(!igt_global_mmio, > > "mmio regs not mmap-ed\n")) > > return; > > > > Or should we add this before all other checks in unmap functions > > and keep this additional check ? > > Why igt_global_mmio again? I still think this global variable is broken and > users should just use the structure they pass to intel_mmio_use_*() or > intel_register_access_init(), then introducing another a dependency on it with > this patch doesn't make sense to me. If you think the opposite then please > explain why. Good point, maybe in next series you will remove this global var ? > > > > + > > > + igt_global_mmio = NULL; > > > + igt_debug_on(munmap(mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > > > + mmio_data->mmio_size = 0; > > > +} > > > + > > > /** > > > * intel_mmio_use_pci_bar: > > > * @mmio_data: mmio structure for IO operations > > > @@ -112,6 +135,8 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > > * Fill a mmio_data stucture with igt_mmio to point at the mmio bar. > > > * > > > * @pci_dev can be obtained from intel_get_pci_device(). > > > + * > > > + * Users are expected to call intel_mmio_unmap_pci_bar() after use. > > > */ > > > void > > > intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev) > > > @@ -141,10 +166,34 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci > > > PCI_DEV_MAP_FLAG_WRITABLE, > > > &mmio_data->igt_mmio); > > > > > > - igt_global_mmio = mmio_data->igt_mmio; > > > - > > > igt_fail_on_f(error != 0, > > > "Couldn't map MMIO region\n"); > > > + > > > + mmio_data->mmio_size = mmio_size; > > > + mmio_data->dev = pci_dev; > > > + igt_global_mmio = mmio_data->igt_mmio; > > To be consequent with what I said above, in next version of the patch I'm > going to not touch the assignment of mmio_data->igt_mmio to the out of scope > and depreciated igt_global_mmio, leaving it as broken as it already is. If it is not used anywhere then ok. > > > +} > > > + > > > +/** > > > + * intel_mmio_unmap_pci_bar: > > > + * @mmio_data: mmio structure for IO operations > > > + * > > > + * Unmaps a PCI BAR region mmapped with intel_mmio_use_pci_bar() > > > + */ > > > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data) > > > +{ > > > + if (igt_warn_on_f(mmio_data->pci_device_id, > > > + "test bug: arg initialized with intel_register_access_init()\n")) > > Similarly to the case of intel_mmio_unmap_dump_file(), I can change the > message to "test bug: arg initialized with a method other than > intel_mmio_use_pci_bar()\n" if that's what you prefer. > > > > + return; > > > + if (igt_warn_on_f(!mmio_data->dev, > > > + "test bug: arg not initialized with intel_mmio_use_pci_bar()\n")) > > > + return; > > > + > > > + igt_global_mmio = NULL; > > > + igt_debug_on(pci_device_unmap_range(mmio_data->dev, > > > + mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > > > + mmio_data->dev = NULL; > > > + mmio_data->mmio_size = 0; > > > } > > > > > > static void > > > @@ -166,6 +215,8 @@ release_forcewake_lock(int fd) > > > * It also initializes mmio_data->igt_mmio like intel_mmio_use_pci_bar(). > > > * > > > * @pci_dev can be obtained from intel_get_pci_device(). > > > + * > > > + * Users are expected to call intel_register_access_fini() after use. > > > */ > > > int > > > intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd) > > > @@ -222,8 +273,15 @@ int intel_register_access_needs_fakewake(struct intel_mmio_data *mmio_data) > > > void > > > intel_register_access_fini(struct intel_mmio_data *mmio_data) > > > { > > > - if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) > > > + if (igt_warn_on_f(!mmio_data->pci_device_id, > > > + "test bug: arg not initialized with intel_register_access_init()\n")) > > > + return; > > > + > > > + if (intel_register_access_needs_wake(mmio_data)) > > As mmio_data->key is no longer used since v3 as an indication of mmio_data > having been initialized by intel_register_access_init() or not, the original > (potentially broken) condition: > > if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) > > should be not touched by this patch as out of scope but its original form > preserved. I'm going to restore it in next version of the patch. ok > > > > release_forcewake_lock(mmio_data->key); > > > + > > > + mmio_data->pci_device_id = 0; > > > > Here we should check other conditions so no warn triggers in unmap_pci_bar > > or make the messages generic (and document it in comments at beggining) > > or maybe make helper with no checks for unmap_pci_bar. > > Why? I can't see any potential scenario where mmio_data->pci_device_id is not > 0 but mmio_data->dev is NULL. If you can see one then please tell me more > about it. Yes, you are right, I overlooked that. > > Thanks, > Janusz > > > > > + intel_mmio_unmap_pci_bar(mmio_data); > > > } > > > > > > /** Regards, Kamil ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [igt-dev] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini @ 2022-03-07 17:04 ` Kamil Konieczny 0 siblings, 0 replies; 10+ messages in thread From: Kamil Konieczny @ 2022-03-07 17:04 UTC (permalink / raw) To: igt-dev; +Cc: intel-gfx Hi Janusz, Dnia 2022-03-07 at 16:06:10 +0100, Janusz Krzysztofik napisał(a): > Hi Kamil, > > On Monday, 7 March 2022 14:23:30 CET Kamil Konieczny wrote: > > Hi Janusz, > > > > Dnia 2022-03-07 at 09:26:43 +0100, Janusz Krzysztofik napisał(a): > > > Commit 5f3cfa485eb4 ("lib: Use safe wrappers around libpciaccess > > > initialization functions") took care of not leaking memory allocated by > > > pci_system_init() but didn't take care of users potentially attempting to > > > reinitialize global data maintained by libpciaccess. For example, > > > intel_register_access_init() mmaps device's PCI BAR0 resource with > > > pci_device_map_range() but intel_register_access_fini() doesn't unmap it > > > and next call to intel_register_access_init() fails on attempt to mmap it > > > again. > > > > > > Fix it, and also provide intel_mmio_unmap_*() counterparts to public > > > functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file(). > > > > > > v2: apply last minute fixes, cached but unfortunately not committed before > > > sending > > > v3: use .pci_device_id field content as an indicator of arg initialization > > > via intel_register_access_init(), > > > - improve checks of argument initialization status, > > > - shorten warning messages (Kamil), > > > - don't fill .mmio_size field until initialization succeeds (Kamil) > > > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > > > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com> > > > --- > > > lib/intel_io.h | 4 +++ > > > lib/intel_mmio.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--- > > > 2 files changed, 65 insertions(+), 3 deletions(-) > > > > > > diff --git a/lib/intel_io.h b/lib/intel_io.h > > > index 1cfe4fb6b9..ea2649d9bc 100644 > > > --- a/lib/intel_io.h > > > +++ b/lib/intel_io.h > > > @@ -49,6 +49,8 @@ struct intel_register_map { > > > > > > struct intel_mmio_data { > > > void *igt_mmio; > > > + size_t mmio_size; > > > + struct pci_device *dev; > > > struct intel_register_map map; > > > uint32_t pci_device_id; > > > int key; > > > @@ -57,7 +59,9 @@ struct intel_mmio_data { > > > > > > void intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, > > > struct pci_device *pci_dev); > > > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data); > > > void intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file); > > > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data); > > > > > > int intel_register_access_init(struct intel_mmio_data *mmio_data, > > > struct pci_device *pci_dev, int safe, int fd); > > > diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c > > > index 667a69f5aa..d6ce0ee3ea 100644 > > > --- a/lib/intel_mmio.c > > > +++ b/lib/intel_mmio.c > > > @@ -82,6 +82,8 @@ void *igt_global_mmio; > > > * Sets also up mmio_data->igt_mmio to point at the data contained > > > * in @file. This allows the same code to get reused for dumping and decoding > > > * from running hardware as from register dumps. > > > + * > > > + * Users are expected to call intel_mmio_unmap_dump_file() after use. > > > */ > > > void > > > intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > > @@ -99,11 +101,32 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > > igt_fail_on_f(mmio_data->igt_mmio == MAP_FAILED, > > > "Couldn't mmap %s\n", file); > > > > > > + mmio_data->mmio_size = st.st_size; > > > igt_global_mmio = mmio_data->igt_mmio; > > > > > > close(fd); > > > } > > > > > > +/** > > > + * intel_mmio_unmap_dump_file: > > > + * @mmio_data: mmio structure for IO operations > > > + * > > > + * Unmaps a dump file mmapped with intel_mmio_use_dump_file() > > > + */ > > > +void intel_mmio_unmap_dump_file(struct intel_mmio_data *mmio_data) > > > +{ > > > + if (igt_warn_on_f(mmio_data->dev, > > > + "test bug: arg initialized with intel_mmio_use_pci_bar()\n")) > > > + return; > > > > Please add a global description about this kind of errors, this > > one is for using unmap when mmio was mmap-ed from other mmap > > type. > > Can you please be more specific in what you mean by "global description of > this kind of errors"? A more detailed warning? A comment? If the latter > then how would you like me to make it global? Yes, I was thinking about comment at begin of file, but maybe it is better to change warning message like below. > > If you just don't like the reference to intel_mmio_use_pci_bar() here then > would you be satisfied with something like "test bug: arg initialized by a > method other than intel_mmio_use_dump_file()\n"? Yes, this sounds good. > > > > + if (igt_warn_on_f(!mmio_data->mmio_size, > > > + "test bug: arg not initialized\n")) > > > + return; > > > > Can we replace this with one check igt_global_mmio != NULL ? > > Something like: > > > > if (igt_warn_on_f(!igt_global_mmio, > > "mmio regs not mmap-ed\n")) > > return; > > > > Or should we add this before all other checks in unmap functions > > and keep this additional check ? > > Why igt_global_mmio again? I still think this global variable is broken and > users should just use the structure they pass to intel_mmio_use_*() or > intel_register_access_init(), then introducing another a dependency on it with > this patch doesn't make sense to me. If you think the opposite then please > explain why. Good point, maybe in next series you will remove this global var ? > > > > + > > > + igt_global_mmio = NULL; > > > + igt_debug_on(munmap(mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > > > + mmio_data->mmio_size = 0; > > > +} > > > + > > > /** > > > * intel_mmio_use_pci_bar: > > > * @mmio_data: mmio structure for IO operations > > > @@ -112,6 +135,8 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file) > > > * Fill a mmio_data stucture with igt_mmio to point at the mmio bar. > > > * > > > * @pci_dev can be obtained from intel_get_pci_device(). > > > + * > > > + * Users are expected to call intel_mmio_unmap_pci_bar() after use. > > > */ > > > void > > > intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev) > > > @@ -141,10 +166,34 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci > > > PCI_DEV_MAP_FLAG_WRITABLE, > > > &mmio_data->igt_mmio); > > > > > > - igt_global_mmio = mmio_data->igt_mmio; > > > - > > > igt_fail_on_f(error != 0, > > > "Couldn't map MMIO region\n"); > > > + > > > + mmio_data->mmio_size = mmio_size; > > > + mmio_data->dev = pci_dev; > > > + igt_global_mmio = mmio_data->igt_mmio; > > To be consequent with what I said above, in next version of the patch I'm > going to not touch the assignment of mmio_data->igt_mmio to the out of scope > and depreciated igt_global_mmio, leaving it as broken as it already is. If it is not used anywhere then ok. > > > +} > > > + > > > +/** > > > + * intel_mmio_unmap_pci_bar: > > > + * @mmio_data: mmio structure for IO operations > > > + * > > > + * Unmaps a PCI BAR region mmapped with intel_mmio_use_pci_bar() > > > + */ > > > +void intel_mmio_unmap_pci_bar(struct intel_mmio_data *mmio_data) > > > +{ > > > + if (igt_warn_on_f(mmio_data->pci_device_id, > > > + "test bug: arg initialized with intel_register_access_init()\n")) > > Similarly to the case of intel_mmio_unmap_dump_file(), I can change the > message to "test bug: arg initialized with a method other than > intel_mmio_use_pci_bar()\n" if that's what you prefer. > > > > + return; > > > + if (igt_warn_on_f(!mmio_data->dev, > > > + "test bug: arg not initialized with intel_mmio_use_pci_bar()\n")) > > > + return; > > > + > > > + igt_global_mmio = NULL; > > > + igt_debug_on(pci_device_unmap_range(mmio_data->dev, > > > + mmio_data->igt_mmio, mmio_data->mmio_size) < 0); > > > + mmio_data->dev = NULL; > > > + mmio_data->mmio_size = 0; > > > } > > > > > > static void > > > @@ -166,6 +215,8 @@ release_forcewake_lock(int fd) > > > * It also initializes mmio_data->igt_mmio like intel_mmio_use_pci_bar(). > > > * > > > * @pci_dev can be obtained from intel_get_pci_device(). > > > + * > > > + * Users are expected to call intel_register_access_fini() after use. > > > */ > > > int > > > intel_register_access_init(struct intel_mmio_data *mmio_data, struct pci_device *pci_dev, int safe, int fd) > > > @@ -222,8 +273,15 @@ int intel_register_access_needs_fakewake(struct intel_mmio_data *mmio_data) > > > void > > > intel_register_access_fini(struct intel_mmio_data *mmio_data) > > > { > > > - if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) > > > + if (igt_warn_on_f(!mmio_data->pci_device_id, > > > + "test bug: arg not initialized with intel_register_access_init()\n")) > > > + return; > > > + > > > + if (intel_register_access_needs_wake(mmio_data)) > > As mmio_data->key is no longer used since v3 as an indication of mmio_data > having been initialized by intel_register_access_init() or not, the original > (potentially broken) condition: > > if (mmio_data->key && intel_register_access_needs_wake(mmio_data)) > > should be not touched by this patch as out of scope but its original form > preserved. I'm going to restore it in next version of the patch. ok > > > > release_forcewake_lock(mmio_data->key); > > > + > > > + mmio_data->pci_device_id = 0; > > > > Here we should check other conditions so no warn triggers in unmap_pci_bar > > or make the messages generic (and document it in comments at beggining) > > or maybe make helper with no checks for unmap_pci_bar. > > Why? I can't see any potential scenario where mmio_data->pci_device_id is not > 0 but mmio_data->dev is NULL. If you can see one then please tell me more > about it. Yes, you are right, I overlooked that. > > Thanks, > Janusz > > > > > + intel_mmio_unmap_pci_bar(mmio_data); > > > } > > > > > > /** Regards, Kamil ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-03-07 17:04 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-07 8:26 [Intel-gfx] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini Janusz Krzysztofik 2022-03-07 8:26 ` [igt-dev] " Janusz Krzysztofik 2022-03-07 9:44 ` [igt-dev] ✓ Fi.CI.BAT: success for lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev3) Patchwork 2022-03-07 11:23 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork 2022-03-07 13:23 ` [Intel-gfx] [PATCH v3 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini Kamil Konieczny 2022-03-07 13:23 ` [igt-dev] " Kamil Konieczny 2022-03-07 15:06 ` [Intel-gfx] " Janusz Krzysztofik 2022-03-07 15:06 ` [igt-dev] " Janusz Krzysztofik 2022-03-07 17:04 ` [Intel-gfx] " Kamil Konieczny 2022-03-07 17:04 ` [igt-dev] " Kamil Konieczny
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.