* [Intel-gfx] [PATCH v2 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini
@ 2022-03-01 14:07 ` Janusz Krzysztofik
0 siblings, 0 replies; 8+ messages in thread
From: Janusz Krzysztofik @ 2022-03-01 14:07 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 with pci_device_map_range().
Fix it, and also provide intel_mmio_umap_*() 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
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
lib/intel_io.h | 4 +++
lib/intel_mmio.c | 67 ++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 63 insertions(+), 8 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..cb8f9db2e5 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,29 @@ 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->mmio_size || mmio_data->dev,
+ "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_dump_file()\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,12 +132,14 @@ 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)
{
uint32_t devid, gen;
- int mmio_bar, mmio_size;
+ int mmio_bar;
int error;
memset(mmio_data, 0, sizeof(struct intel_mmio_data));
@@ -129,22 +151,42 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci
gen = intel_gen(devid);
if (gen < 3)
- mmio_size = 512*1024;
+ mmio_data->mmio_size = 512*1024;
else if (gen < 5)
- mmio_size = 512*1024;
+ mmio_data->mmio_size = 512*1024;
else
- mmio_size = 2*1024*1024;
+ mmio_data->mmio_size = 2*1024*1024;
error = pci_device_map_range (pci_dev,
pci_dev->regions[mmio_bar].base_addr,
- mmio_size,
+ mmio_data->mmio_size,
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->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->dev,
+ "test bug: argument doesn't point to struct intel_mmio_data 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 +208,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 +266,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->key,
+ "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_register_access_init()\n"))
+ return;
+
+ if (intel_register_access_needs_wake(mmio_data))
release_forcewake_lock(mmio_data->key);
+ mmio_data->key = 0;
+
+ intel_mmio_unmap_pci_bar(mmio_data);
}
/**
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [igt-dev] [PATCH v2 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini
@ 2022-03-01 14:07 ` Janusz Krzysztofik
0 siblings, 0 replies; 8+ messages in thread
From: Janusz Krzysztofik @ 2022-03-01 14:07 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 with pci_device_map_range().
Fix it, and also provide intel_mmio_umap_*() 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
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
lib/intel_io.h | 4 +++
lib/intel_mmio.c | 67 ++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 63 insertions(+), 8 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..cb8f9db2e5 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,29 @@ 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->mmio_size || mmio_data->dev,
+ "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_dump_file()\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,12 +132,14 @@ 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)
{
uint32_t devid, gen;
- int mmio_bar, mmio_size;
+ int mmio_bar;
int error;
memset(mmio_data, 0, sizeof(struct intel_mmio_data));
@@ -129,22 +151,42 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci
gen = intel_gen(devid);
if (gen < 3)
- mmio_size = 512*1024;
+ mmio_data->mmio_size = 512*1024;
else if (gen < 5)
- mmio_size = 512*1024;
+ mmio_data->mmio_size = 512*1024;
else
- mmio_size = 2*1024*1024;
+ mmio_data->mmio_size = 2*1024*1024;
error = pci_device_map_range (pci_dev,
pci_dev->regions[mmio_bar].base_addr,
- mmio_size,
+ mmio_data->mmio_size,
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->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->dev,
+ "test bug: argument doesn't point to struct intel_mmio_data 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 +208,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 +266,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->key,
+ "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_register_access_init()\n"))
+ return;
+
+ if (intel_register_access_needs_wake(mmio_data))
release_forcewake_lock(mmio_data->key);
+ mmio_data->key = 0;
+
+ intel_mmio_unmap_pci_bar(mmio_data);
}
/**
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev2)
2022-03-01 14:07 ` [igt-dev] " Janusz Krzysztofik
(?)
@ 2022-03-01 14:52 ` Patchwork
-1 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2022-03-01 14:52 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: igt-dev
[-- Attachment #1: Type: text/plain, Size: 6014 bytes --]
== Series Details ==
Series: lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev2)
URL : https://patchwork.freedesktop.org/series/100882/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11303 -> IGTPW_6724
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/index.html
Participating hosts (47 -> 35)
------------------------------
Additional (2): fi-icl-u2 fi-tgl-dsi
Missing (14): fi-cml-u2 fi-bdw-samus shard-tglu bat-dg1-6 bat-dg1-5 fi-hsw-4200u bat-dg2-9 fi-bsw-cyan bat-adlp-6 bat-adlp-4 fi-ctg-p8600 bat-rpls-2 bat-jsl-2 bat-jsl-1
Known issues
------------
Here are the changes found in IGTPW_6724 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@amdgpu/amd_basic@cs-gfx:
- fi-hsw-4770: NOTRUN -> [SKIP][1] ([fdo#109271] / [fdo#109315]) +17 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-hsw-4770/igt@amdgpu/amd_basic@cs-gfx.html
* igt@amdgpu/amd_cs_nop@fork-gfx0:
- fi-icl-u2: NOTRUN -> [SKIP][2] ([fdo#109315]) +17 similar issues
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-icl-u2/igt@amdgpu/amd_cs_nop@fork-gfx0.html
* igt@gem_huc_copy@huc-copy:
- fi-icl-u2: NOTRUN -> [SKIP][3] ([i915#2190])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-icl-u2/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@parallel-random-engines:
- fi-icl-u2: NOTRUN -> [SKIP][4] ([i915#4613]) +3 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-icl-u2/igt@gem_lmem_swapping@parallel-random-engines.html
* igt@i915_selftest@live@hangcheck:
- fi-snb-2600: [PASS][5] -> [INCOMPLETE][6] ([i915#3921])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-icl-u2: NOTRUN -> [SKIP][7] ([fdo#111827]) +8 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- fi-icl-u2: NOTRUN -> [SKIP][8] ([fdo#109278]) +2 similar issues
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/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][9] ([fdo#109285])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-icl-u2/igt@kms_force_connector_basic@force-load-detect.html
* igt@prime_vgem@basic-userptr:
- fi-icl-u2: NOTRUN -> [SKIP][10] ([i915#3301])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-icl-u2/igt@prime_vgem@basic-userptr.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s3@smem:
- {fi-rkl-11600}: [INCOMPLETE][11] ([i915#5127]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-rkl-11600/igt@gem_exec_suspend@basic-s3@smem.html
* igt@i915_selftest@live@hangcheck:
- fi-hsw-4770: [INCOMPLETE][13] ([i915#3303]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/fi-hsw-4770/igt@i915_selftest@live@hangcheck.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#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#1759]: https://gitlab.freedesktop.org/drm/intel/issues/1759
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2373]: https://gitlab.freedesktop.org/drm/intel/issues/2373
[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#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
[i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#5127]: https://gitlab.freedesktop.org/drm/intel/issues/5127
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_6361 -> IGTPW_6724
CI-20190529: 20190529
CI_DRM_11303: 329182aa4c789d378e58b87b5f283116d87278a7 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_6724: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/index.html
IGT_6361: 2372a4beb6a33c5f0799a4a8ccbb93794f52dbca @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/index.html
[-- Attachment #2: Type: text/html, Size: 6080 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev2)
2022-03-01 14:07 ` [igt-dev] " Janusz Krzysztofik
(?)
(?)
@ 2022-03-01 21:33 ` Patchwork
-1 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2022-03-01 21:33 UTC (permalink / raw)
To: Janusz Krzysztofik; +Cc: igt-dev
[-- Attachment #1: Type: text/plain, Size: 30284 bytes --]
== Series Details ==
Series: lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev2)
URL : https://patchwork.freedesktop.org/series/100882/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11303_full -> IGTPW_6724_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/index.html
Participating hosts (13 -> 8)
------------------------------
Missing (5): pig-kbl-iris pig-glk-j5005 pig-skl-6260u shard-rkl shard-dg1
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_6724_full:
### IGT changes ###
#### Possible regressions ####
* {igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-25@pipe-d-edp-1-downscale-with-pixel-format} (NEW):
- shard-tglb: NOTRUN -> [SKIP][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb6/igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-25@pipe-d-edp-1-downscale-with-pixel-format.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* {igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-25@pipe-b-edp-1-downscale-with-pixel-format}:
- shard-tglb: NOTRUN -> [SKIP][2] +2 similar issues
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb6/igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-25@pipe-b-edp-1-downscale-with-pixel-format.html
* {igt@kms_plane_scaling@planes-downscale-factor-0-25@pipe-a-hdmi-a-1-planes-downscale}:
- {shard-tglu}: NOTRUN -> [SKIP][3] +7 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglu-6/igt@kms_plane_scaling@planes-downscale-factor-0-25@pipe-a-hdmi-a-1-planes-downscale.html
* {igt@kms_plane_scaling@planes-downscale-factor-0-5@pipe-a-edp-1-planes-downscale}:
- shard-iclb: [PASS][4] -> [SKIP][5] +5 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-iclb4/igt@kms_plane_scaling@planes-downscale-factor-0-5@pipe-a-edp-1-planes-downscale.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb2/igt@kms_plane_scaling@planes-downscale-factor-0-5@pipe-a-edp-1-planes-downscale.html
New tests
---------
New tests have been introduced between CI_DRM_11303_full and IGTPW_6724_full:
### New IGT tests (37) ###
* igt@gem_ctx_persistence@legacy-engines-queued@blt:
- Statuses : 2 pass(s)
- Exec time: [0.13, 0.17] s
* igt@gem_ctx_persistence@legacy-engines-queued@bsd:
- Statuses : 2 pass(s)
- Exec time: [0.03, 0.04] s
* igt@gem_ctx_persistence@legacy-engines-queued@render:
- Statuses : 2 pass(s)
- Exec time: [0.03, 0.04] s
* igt@gem_ctx_persistence@legacy-engines-queued@vebox:
- Statuses : 2 pass(s)
- Exec time: [0.04] s
* igt@gem_exec_create@forked:
- Statuses :
- Exec time: [None] s
* igt@gem_exec_suspend@basic:
- Statuses :
- Exec time: [None] s
* igt@gem_render_copy@mixed-tiled-to-y-tiled-ccs:
- Statuses :
- Exec time: [None] s
* igt@gem_render_copy@mixed-tiled-to-yf-tiled-ccs:
- Statuses :
- Exec time: [None] s
* igt@gem_render_copy@y-tiled-ccs-to-x-tiled:
- Statuses :
- Exec time: [None] s
* igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
- Statuses :
- Exec time: [None] s
* igt@kms_atomic_interruptible@legacy-dpms:
- Statuses :
- Exec time: [None] s
* igt@kms_atomic_interruptible@legacy-pageflip:
- Statuses :
- Exec time: [None] s
* igt@kms_flip@blocking-absolute-wf_vblank:
- Statuses :
- Exec time: [None] s
* igt@kms_flip@dpms-off-confusion-interruptible:
- Statuses :
- Exec time: [None] s
* igt@kms_flip@flip-vs-panning-vs-hang:
- Statuses :
- Exec time: [None] s
* igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-25@pipe-d-edp-1-downscale-with-pixel-format:
- Statuses : 1 skip(s)
- Exec time: [0.02] s
* igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-75@pipe-b-hdmi-a-1-downscale-with-pixel-format:
- Statuses : 1 pass(s)
- Exec time: [22.73] s
* igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-75@pipe-d-hdmi-a-1-downscale-with-pixel-format:
- Statuses : 1 pass(s)
- Exec time: [0.27] s
* igt@kms_plane_scaling@invalid-num-scalers@pipe-d-edp-1-invalid-num-scalers:
- Statuses : 1 pass(s)
- Exec time: [0.02] s
* igt@kms_plane_scaling@invalid-num-scalers@pipe-d-hdmi-a-1-invalid-num-scalers:
- Statuses : 1 pass(s)
- Exec time: [0.02] s
* igt@kms_plane_scaling@planes-scaling-unity-scaling@pipe-d-edp-1-planes-unity-scaling:
- Statuses : 1 pass(s)
- Exec time: [1.29] s
* igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-75@pipe-d-edp-1-planes-upscale-downscale:
- Statuses : 1 pass(s)
- Exec time: [1.28] s
* igt@kms_plane_scaling@planes-upscale-20x20@pipe-b-hdmi-a-1-planes-upscale:
- Statuses : 1 pass(s)
- Exec time: [0.10] s
* igt@kms_plane_scaling@planes-upscale-20x20@pipe-d-edp-1-planes-upscale:
- Statuses : 1 pass(s)
- Exec time: [1.21] s
* igt@kms_plane_scaling@planes-upscale-20x20@pipe-d-hdmi-a-1-planes-upscale:
- Statuses : 1 pass(s)
- Exec time: [0.12] s
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-75@pipe-a-edp-1-planes-upscale-downscale:
- Statuses : 2 pass(s)
- Exec time: [0.14, 0.18] s
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-75@pipe-a-hdmi-a-1-planes-upscale-downscale:
- Statuses : 1 pass(s)
- Exec time: [0.37] s
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-75@pipe-a-vga-1-planes-upscale-downscale:
- Statuses : 1 skip(s)
- Exec time: [0.05] s
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-75@pipe-b-edp-1-planes-upscale-downscale:
- Statuses : 2 pass(s)
- Exec time: [1.24, 1.32] s
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-75@pipe-b-hdmi-a-2-planes-upscale-downscale:
- Statuses : 1 pass(s)
- Exec time: [0.35] s
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-75@pipe-b-vga-1-planes-upscale-downscale:
- Statuses : 1 skip(s)
- Exec time: [0.03] s
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-75@pipe-c-edp-1-planes-upscale-downscale:
- Statuses : 2 pass(s)
- Exec time: [1.28, 1.29] s
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-75@pipe-c-hdmi-a-1-planes-upscale-downscale:
- Statuses : 1 skip(s)
- Exec time: [0.13] s
* igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-75@pipe-d-edp-1-planes-upscale-downscale:
- Statuses : 1 pass(s)
- Exec time: [1.28] s
* igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-b-hdmi-a-1-scaler-with-clipping-clamping:
- Statuses : 1 pass(s)
- Exec time: [22.76] s
* igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-d-hdmi-a-1-scaler-with-clipping-clamping:
- Statuses : 1 pass(s)
- Exec time: [0.28] s
* igt@prime_mmap@test_userptr:
- Statuses :
- Exec time: [None] s
Known issues
------------
Here are the changes found in IGTPW_6724_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@feature_discovery@display-2x:
- shard-tglb: NOTRUN -> [SKIP][6] ([i915#1839])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb7/igt@feature_discovery@display-2x.html
* igt@feature_discovery@display-4x:
- shard-iclb: NOTRUN -> [SKIP][7] ([i915#1839]) +1 similar issue
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb1/igt@feature_discovery@display-4x.html
* igt@feature_discovery@psr2:
- shard-iclb: [PASS][8] -> [SKIP][9] ([i915#658])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-iclb2/igt@feature_discovery@psr2.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb8/igt@feature_discovery@psr2.html
* igt@gem_ctx_isolation@preservation-s3@bcs0:
- shard-apl: [PASS][10] -> [DMESG-WARN][11] ([i915#180]) +2 similar issues
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-apl7/igt@gem_ctx_isolation@preservation-s3@bcs0.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl2/igt@gem_ctx_isolation@preservation-s3@bcs0.html
* igt@gem_ctx_param@set-priority-not-supported:
- shard-tglb: NOTRUN -> [SKIP][12] ([fdo#109314])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb1/igt@gem_ctx_param@set-priority-not-supported.html
* igt@gem_ctx_persistence@legacy-engines-queued:
- shard-snb: NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#1099]) +1 similar issue
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-snb4/igt@gem_ctx_persistence@legacy-engines-queued.html
* igt@gem_ctx_sseu@mmap-args:
- shard-tglb: NOTRUN -> [SKIP][14] ([i915#280])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb2/igt@gem_ctx_sseu@mmap-args.html
* igt@gem_eio@unwedge-stress:
- shard-iclb: [PASS][15] -> [TIMEOUT][16] ([i915#2481] / [i915#3070])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-iclb2/igt@gem_eio@unwedge-stress.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb3/igt@gem_eio@unwedge-stress.html
* igt@gem_exec_balancer@parallel-out-fence:
- shard-kbl: NOTRUN -> [DMESG-WARN][17] ([i915#5076])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl6/igt@gem_exec_balancer@parallel-out-fence.html
* igt@gem_exec_fair@basic-flow@rcs0:
- shard-tglb: NOTRUN -> [FAIL][18] ([i915#2842]) +6 similar issues
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb7/igt@gem_exec_fair@basic-flow@rcs0.html
* igt@gem_exec_fair@basic-none-solo@rcs0:
- shard-kbl: NOTRUN -> [FAIL][19] ([i915#2842]) +2 similar issues
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl4/igt@gem_exec_fair@basic-none-solo@rcs0.html
* igt@gem_exec_fair@basic-none-vip@rcs0:
- shard-kbl: [PASS][20] -> [FAIL][21] ([i915#2842])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-kbl3/igt@gem_exec_fair@basic-none-vip@rcs0.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl1/igt@gem_exec_fair@basic-none-vip@rcs0.html
* igt@gem_exec_fair@basic-pace@vcs0:
- shard-iclb: [PASS][22] -> [FAIL][23] ([i915#2842])
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-iclb2/igt@gem_exec_fair@basic-pace@vcs0.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb7/igt@gem_exec_fair@basic-pace@vcs0.html
* igt@gem_exec_fair@basic-throttle@rcs0:
- shard-iclb: NOTRUN -> [FAIL][24] ([i915#2842])
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb4/igt@gem_exec_fair@basic-throttle@rcs0.html
* igt@gem_exec_params@secure-non-root:
- shard-tglb: NOTRUN -> [SKIP][25] ([fdo#112283])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb3/igt@gem_exec_params@secure-non-root.html
* igt@gem_lmem_swapping@heavy-random:
- shard-iclb: NOTRUN -> [SKIP][26] ([i915#4613]) +1 similar issue
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb3/igt@gem_lmem_swapping@heavy-random.html
- shard-apl: NOTRUN -> [SKIP][27] ([fdo#109271] / [i915#4613])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl2/igt@gem_lmem_swapping@heavy-random.html
- shard-glk: NOTRUN -> [SKIP][28] ([fdo#109271] / [i915#4613])
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-glk5/igt@gem_lmem_swapping@heavy-random.html
* igt@gem_lmem_swapping@heavy-verify-random:
- shard-kbl: NOTRUN -> [SKIP][29] ([fdo#109271] / [i915#4613]) +2 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl7/igt@gem_lmem_swapping@heavy-verify-random.html
- shard-tglb: NOTRUN -> [SKIP][30] ([i915#4613]) +2 similar issues
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb1/igt@gem_lmem_swapping@heavy-verify-random.html
* igt@gem_ppgtt@flink-and-close-vma-leak:
- shard-glk: [PASS][31] -> [FAIL][32] ([i915#644])
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-glk5/igt@gem_ppgtt@flink-and-close-vma-leak.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-glk7/igt@gem_ppgtt@flink-and-close-vma-leak.html
* igt@gem_pxp@dmabuf-shared-protected-dst-is-context-refcounted:
- shard-iclb: NOTRUN -> [SKIP][33] ([i915#4270]) +3 similar issues
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb3/igt@gem_pxp@dmabuf-shared-protected-dst-is-context-refcounted.html
* igt@gem_pxp@verify-pxp-execution-after-suspend-resume:
- shard-tglb: NOTRUN -> [SKIP][34] ([i915#4270]) +4 similar issues
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb7/igt@gem_pxp@verify-pxp-execution-after-suspend-resume.html
* igt@gem_render_copy@x-tiled-to-vebox-yf-tiled:
- shard-kbl: NOTRUN -> [SKIP][35] ([fdo#109271]) +245 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl7/igt@gem_render_copy@x-tiled-to-vebox-yf-tiled.html
* igt@gem_render_copy@y-tiled-to-vebox-linear:
- shard-iclb: NOTRUN -> [SKIP][36] ([i915#768]) +2 similar issues
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb1/igt@gem_render_copy@y-tiled-to-vebox-linear.html
* igt@gem_softpin@evict-snoop:
- shard-iclb: NOTRUN -> [SKIP][37] ([fdo#109312])
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb1/igt@gem_softpin@evict-snoop.html
* igt@gem_userptr_blits@unsync-unmap-cycles:
- shard-tglb: NOTRUN -> [SKIP][38] ([i915#3297])
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb6/igt@gem_userptr_blits@unsync-unmap-cycles.html
* igt@gen3_render_tiledy_blits:
- shard-tglb: NOTRUN -> [SKIP][39] ([fdo#109289]) +5 similar issues
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb8/igt@gen3_render_tiledy_blits.html
* igt@gen7_exec_parse@oacontrol-tracking:
- shard-iclb: NOTRUN -> [SKIP][40] ([fdo#109289]) +3 similar issues
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb6/igt@gen7_exec_parse@oacontrol-tracking.html
* igt@gen9_exec_parse@shadow-peek:
- shard-tglb: NOTRUN -> [SKIP][41] ([i915#2527] / [i915#2856]) +3 similar issues
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb6/igt@gen9_exec_parse@shadow-peek.html
* igt@gen9_exec_parse@unaligned-access:
- shard-iclb: NOTRUN -> [SKIP][42] ([i915#2856]) +1 similar issue
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb4/igt@gen9_exec_parse@unaligned-access.html
* igt@i915_pm_dc@dc6-dpms:
- shard-iclb: [PASS][43] -> [FAIL][44] ([i915#454])
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-iclb2/igt@i915_pm_dc@dc6-dpms.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb3/igt@i915_pm_dc@dc6-dpms.html
* igt@i915_pm_dc@dc9-dpms:
- shard-iclb: NOTRUN -> [SKIP][45] ([i915#4281])
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb3/igt@i915_pm_dc@dc9-dpms.html
* igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp:
- shard-apl: NOTRUN -> [SKIP][46] ([fdo#109271] / [i915#1937])
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl4/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp.html
* igt@i915_pm_rc6_residency@media-rc6-accuracy:
- shard-tglb: NOTRUN -> [SKIP][47] ([fdo#109289] / [fdo#111719])
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb8/igt@i915_pm_rc6_residency@media-rc6-accuracy.html
* igt@i915_pm_rc6_residency@rc6-idle:
- shard-tglb: NOTRUN -> [WARN][48] ([i915#2681] / [i915#2684])
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb1/igt@i915_pm_rc6_residency@rc6-idle.html
- shard-iclb: NOTRUN -> [WARN][49] ([i915#1804] / [i915#2684])
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb6/igt@i915_pm_rc6_residency@rc6-idle.html
* igt@i915_pm_rpm@gem-execbuf-stress-pc8:
- shard-tglb: NOTRUN -> [SKIP][50] ([fdo#109506] / [i915#2411])
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb8/igt@i915_pm_rpm@gem-execbuf-stress-pc8.html
* igt@i915_pm_sseu@full-enable:
- shard-tglb: NOTRUN -> [SKIP][51] ([i915#4387])
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb6/igt@i915_pm_sseu@full-enable.html
- shard-iclb: NOTRUN -> [SKIP][52] ([i915#4387])
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb8/igt@i915_pm_sseu@full-enable.html
* igt@i915_selftest@live@hangcheck:
- shard-snb: [PASS][53] -> [INCOMPLETE][54] ([i915#3921])
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-snb5/igt@i915_selftest@live@hangcheck.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-snb7/igt@i915_selftest@live@hangcheck.html
* igt@i915_suspend@fence-restore-untiled:
- shard-kbl: [PASS][55] -> [DMESG-WARN][56] ([i915#180]) +1 similar issue
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-kbl1/igt@i915_suspend@fence-restore-untiled.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl6/igt@i915_suspend@fence-restore-untiled.html
* igt@kms_atomic_transition@plane-all-modeset-transition:
- shard-iclb: NOTRUN -> [SKIP][57] ([i915#1769])
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb7/igt@kms_atomic_transition@plane-all-modeset-transition.html
* igt@kms_big_fb@linear-32bpp-rotate-180:
- shard-glk: [PASS][58] -> [DMESG-WARN][59] ([i915#118]) +2 similar issues
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-glk1/igt@kms_big_fb@linear-32bpp-rotate-180.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-glk3/igt@kms_big_fb@linear-32bpp-rotate-180.html
* igt@kms_big_fb@x-tiled-64bpp-rotate-270:
- shard-iclb: NOTRUN -> [SKIP][60] ([fdo#110725] / [fdo#111614]) +1 similar issue
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb7/igt@kms_big_fb@x-tiled-64bpp-rotate-270.html
* igt@kms_big_fb@x-tiled-8bpp-rotate-90:
- shard-tglb: NOTRUN -> [SKIP][61] ([fdo#111614]) +3 similar issues
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb5/igt@kms_big_fb@x-tiled-8bpp-rotate-90.html
* igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-hflip-async-flip:
- shard-kbl: NOTRUN -> [SKIP][62] ([fdo#109271] / [i915#3777])
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl4/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-0-hflip-async-flip.html
* igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0-hflip-async-flip:
- shard-apl: NOTRUN -> [SKIP][63] ([fdo#109271] / [i915#3777]) +1 similar issue
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl7/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-0-hflip-async-flip.html
* igt@kms_big_fb@yf-tiled-64bpp-rotate-270:
- shard-tglb: NOTRUN -> [SKIP][64] ([fdo#111615]) +7 similar issues
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb8/igt@kms_big_fb@yf-tiled-64bpp-rotate-270.html
* igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0:
- shard-apl: NOTRUN -> [SKIP][65] ([fdo#109271]) +137 similar issues
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl3/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0.html
- shard-iclb: NOTRUN -> [SKIP][66] ([fdo#110723]) +1 similar issue
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb3/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0.html
* igt@kms_big_joiner@basic:
- shard-tglb: NOTRUN -> [SKIP][67] ([i915#2705])
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb7/igt@kms_big_joiner@basic.html
- shard-iclb: NOTRUN -> [SKIP][68] ([i915#2705])
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb2/igt@kms_big_joiner@basic.html
* igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_mc_ccs:
- shard-tglb: NOTRUN -> [SKIP][69] ([i915#3689] / [i915#3886]) +6 similar issues
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb6/igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_mc_ccs.html
* igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_ccs:
- shard-tglb: NOTRUN -> [SKIP][70] ([i915#3689]) +5 similar issues
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb2/igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_ccs.html
* igt@kms_ccs@pipe-b-bad-pixel-format-y_tiled_ccs:
- shard-snb: NOTRUN -> [SKIP][71] ([fdo#109271]) +142 similar issues
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-snb5/igt@kms_ccs@pipe-b-bad-pixel-format-y_tiled_ccs.html
* igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_mc_ccs:
- shard-iclb: NOTRUN -> [SKIP][72] ([fdo#109278] / [i915#3886]) +8 similar issues
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb8/igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_mc_ccs.html
- shard-kbl: NOTRUN -> [SKIP][73] ([fdo#109271] / [i915#3886]) +11 similar issues
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl4/igt@kms_ccs@pipe-b-random-ccs-data-y_tiled_gen12_mc_ccs.html
* igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_mc_ccs:
- shard-apl: NOTRUN -> [SKIP][74] ([fdo#109271] / [i915#3886]) +7 similar issues
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl8/igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_mc_ccs.html
* igt@kms_ccs@pipe-c-random-ccs-data-y_tiled_gen12_rc_ccs_cc:
- shard-glk: NOTRUN -> [SKIP][75] ([fdo#109271] / [i915#3886]) +4 similar issues
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-glk5/igt@kms_ccs@pipe-c-random-ccs-data-y_tiled_gen12_rc_ccs_cc.html
* igt@kms_ccs@pipe-d-missing-ccs-buffer-yf_tiled_ccs:
- shard-tglb: NOTRUN -> [SKIP][76] ([fdo#111615] / [i915#3689]) +6 similar issues
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb7/igt@kms_ccs@pipe-d-missing-ccs-buffer-yf_tiled_ccs.html
* igt@kms_chamelium@dp-hpd-with-enabled-mode:
- shard-iclb: NOTRUN -> [SKIP][77] ([fdo#109284] / [fdo#111827]) +12 similar issues
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb8/igt@kms_chamelium@dp-hpd-with-enabled-mode.html
* igt@kms_chamelium@dp-mode-timings:
- shard-apl: NOTRUN -> [SKIP][78] ([fdo#109271] / [fdo#111827]) +17 similar issues
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl2/igt@kms_chamelium@dp-mode-timings.html
* igt@kms_chamelium@hdmi-hpd:
- shard-glk: NOTRUN -> [SKIP][79] ([fdo#109271] / [fdo#111827]) +10 similar issues
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-glk8/igt@kms_chamelium@hdmi-hpd.html
* igt@kms_chamelium@vga-hpd-for-each-pipe:
- shard-kbl: NOTRUN -> [SKIP][80] ([fdo#109271] / [fdo#111827]) +19 similar issues
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl4/igt@kms_chamelium@vga-hpd-for-each-pipe.html
* igt@kms_color_chamelium@pipe-c-ctm-green-to-red:
- shard-snb: NOTRUN -> [SKIP][81] ([fdo#109271] / [fdo#111827]) +10 similar issues
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-snb5/igt@kms_color_chamelium@pipe-c-ctm-green-to-red.html
* igt@kms_color_chamelium@pipe-d-ctm-limited-range:
- shard-tglb: NOTRUN -> [SKIP][82] ([fdo#109284] / [fdo#111827]) +20 similar issues
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb5/igt@kms_color_chamelium@pipe-d-ctm-limited-range.html
- shard-iclb: NOTRUN -> [SKIP][83] ([fdo#109278] / [fdo#109284] / [fdo#111827])
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb1/igt@kms_color_chamelium@pipe-d-ctm-limited-range.html
* igt@kms_content_protection@atomic:
- shard-kbl: NOTRUN -> [TIMEOUT][84] ([i915#1319])
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl6/igt@kms_content_protection@atomic.html
- shard-apl: NOTRUN -> [TIMEOUT][85] ([i915#1319])
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl3/igt@kms_content_protection@atomic.html
* igt@kms_content_protection@atomic-dpms:
- shard-tglb: NOTRUN -> [SKIP][86] ([i915#1063])
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb5/igt@kms_content_protection@atomic-dpms.html
- shard-iclb: NOTRUN -> [SKIP][87] ([fdo#109300] / [fdo#111066])
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb2/igt@kms_content_protection@atomic-dpms.html
* igt@kms_content_protection@uevent:
- shard-kbl: NOTRUN -> [FAIL][88] ([i915#2105])
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl4/igt@kms_content_protection@uevent.html
* igt@kms_cursor_crc@pipe-a-cursor-32x10-offscreen:
- shard-tglb: NOTRUN -> [SKIP][89] ([i915#3359]) +7 similar issues
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb1/igt@kms_cursor_crc@pipe-a-cursor-32x10-offscreen.html
* igt@kms_cursor_crc@pipe-a-cursor-512x170-random:
- shard-iclb: NOTRUN -> [SKIP][90] ([fdo#109278] / [fdo#109279]) +1 similar issue
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb3/igt@kms_cursor_crc@pipe-a-cursor-512x170-random.html
* igt@kms_cursor_crc@pipe-a-cursor-suspend:
- shard-kbl: NOTRUN -> [DMESG-WARN][91] ([i915#180])
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
- shard-apl: NOTRUN -> [DMESG-WARN][92] ([i915#180])
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
* igt@kms_cursor_crc@pipe-b-cursor-32x32-sliding:
- shard-tglb: NOTRUN -> [SKIP][93] ([i915#3319]) +3 similar issues
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb5/igt@kms_cursor_crc@pipe-b-cursor-32x32-sliding.html
* igt@kms_cursor_crc@pipe-d-cursor-512x512-offscreen:
- shard-tglb: NOTRUN -> [SKIP][94] ([fdo#109279] / [i915#3359]) +3 similar issues
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb1/igt@kms_cursor_crc@pipe-d-cursor-512x512-offscreen.html
* igt@kms_cursor_edge_walk@pipe-a-64x64-right-edge:
- shard-snb: [PASS][95] -> [SKIP][96] ([fdo#109271]) +1 similar issue
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-snb2/igt@kms_cursor_edge_walk@pipe-a-64x64-right-edge.html
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-snb4/igt@kms_cursor_edge_walk@pipe-a-64x64-right-edge.html
* igt@kms_cursor_legacy@cursora-vs-flipb-atomic-transitions-varying-size:
- shard-iclb: NOTRUN -> [SKIP][97] ([fdo#109274] / [fdo#109278]) +5 similar issues
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb5/igt@kms_cursor_legacy@cursora-vs-flipb-atomic-transitions-varying-size.html
* igt@kms_cursor_legacy@pipe-d-single-bo:
- shard-kbl: NOTRUN -> [SKIP][98] ([fdo#109271] / [i915#533])
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl6/igt@kms_cursor_legacy@pipe-d-single-bo.html
- shard-apl: NOTRUN -> [SKIP][99] ([fdo#109271] / [i915#533])
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-apl2/igt@kms_cursor_legacy@pipe-d-single-bo.html
* igt@kms_cursor_legacy@pipe-d-single-move:
- shard-iclb: NOTRUN -> [SKIP][100] ([fdo#109278]) +37 similar issues
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb4/igt@kms_cursor_legacy@pipe-d-single-move.html
* igt@kms_cursor_legacy@short-busy-flip-before-cursor-atomic-transitions-varying-size:
- shard-tglb: NOTRUN -> [SKIP][101] ([i915#4103]) +1 similar issue
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb3/igt@kms_cursor_legacy@short-busy-flip-before-cursor-atomic-transitions-varying-size.html
* igt@kms_flip@2x-absolute-wf_vblank:
- shard-tglb: NOTRUN -> [SKIP][102] ([fdo#109274] / [fdo#111825] / [i915#3966])
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb6/igt@kms_flip@2x-absolute-wf_vblank.html
* igt@kms_flip@2x-modeset-vs-vblank-race-interruptible:
- shard-iclb: NOTRUN -> [SKIP][103] ([fdo#109274]) +4 similar issues
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-iclb3/igt@kms_flip@2x-modeset-vs-vblank-race-interruptible.html
* igt@kms_flip@2x-plain-flip-fb-recreate:
- shard-tglb: NOTRUN -> [SKIP][104] ([fdo#109274] / [fdo#111825]) +9 similar issues
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-tglb5/igt@kms_flip@2x-plain-flip-fb-recreate.html
* igt@kms_flip@flip-vs-suspend@c-dp1:
- shard-kbl: [PASS][105] -> [INCOMPLETE][106] ([i915#636])
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11303/shard-kbl4/igt@kms_flip@flip-vs-suspend@c-dp1.html
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/shard-kbl3/igt@kms_
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_6724/index.html
[-- Attachment #2: Type: text/html, Size: 34538 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH v2 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini
2022-03-01 14:07 ` [igt-dev] " Janusz Krzysztofik
@ 2022-03-04 15:14 ` Kamil Konieczny
-1 siblings, 0 replies; 8+ messages in thread
From: Kamil Konieczny @ 2022-03-04 15:14 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx
Hi Janusz,
Dnia 2022-03-01 at 15:07:55 +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 with pci_device_map_range().
------ ^
imho you can cut here, no need to repeat it twice.
>
> Fix it, and also provide intel_mmio_umap_*() counterparts to public
-------------------------------------- ^
s/umap/unmap/
> functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file().
>
> v2: apply last minute fixes, cached but unfortunately not committed before
> sending
>
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
> lib/intel_io.h | 4 +++
> lib/intel_mmio.c | 67 ++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 63 insertions(+), 8 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..cb8f9db2e5 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,29 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file)
imho at beginning of this function there should be check
that igt_global_mmio == NULL, and the same check should be at
other init functions.
Looks like we cannot mmap two different pcie cards at the same
time with this lib.
> 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->mmio_size || mmio_data->dev,
> + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_dump_file()\n"))
Please shorten text for warning, something like: arg was not
inialized with ...
Please also add check for null at global var.
> + 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,12 +132,14 @@ 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)
> {
> uint32_t devid, gen;
> - int mmio_bar, mmio_size;
> + int mmio_bar;
Please use this local var and assign it to struct only after
succesfull initialization.
> int error;
>
> memset(mmio_data, 0, sizeof(struct intel_mmio_data));
> @@ -129,22 +151,42 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci
>
> gen = intel_gen(devid);
> if (gen < 3)
> - mmio_size = 512*1024;
> + mmio_data->mmio_size = 512*1024;
> else if (gen < 5)
> - mmio_size = 512*1024;
> + mmio_data->mmio_size = 512*1024;
Both places uses the same number 512*1024, please make it one
if check: if (gen < 5)
Or maybe it is an error for gen < 3 ?
> else
> - mmio_size = 2*1024*1024;
> + mmio_data->mmio_size = 2*1024*1024;
>
> error = pci_device_map_range (pci_dev,
> pci_dev->regions[mmio_bar].base_addr,
> - mmio_size,
> + mmio_data->mmio_size,
> 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->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->dev,
> + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_pci_bar()\n"))
Same here, please shorten this warn.
> + 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 +208,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 +266,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->key,
> + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_register_access_init()\n"))
Same here, please shorten this warn.
Btw, in this lib error condition for key is -1, so maybe this
should also be cheked ?
> + return;
> +
> + if (intel_register_access_needs_wake(mmio_data))
> release_forcewake_lock(mmio_data->key);
> + mmio_data->key = 0;
> +
> + intel_mmio_unmap_pci_bar(mmio_data);
> }
>
> /**
> --
> 2.25.1
>
Please correct desciption of global var igt_global_mmio, there
is one more method for initialize it: intel_mmio_use_pci_bar
as you wrote on irc.
Regards,
Kamil Konieczny
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [igt-dev] [PATCH v2 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini
@ 2022-03-04 15:14 ` Kamil Konieczny
0 siblings, 0 replies; 8+ messages in thread
From: Kamil Konieczny @ 2022-03-04 15:14 UTC (permalink / raw)
To: igt-dev; +Cc: intel-gfx
Hi Janusz,
Dnia 2022-03-01 at 15:07:55 +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 with pci_device_map_range().
------ ^
imho you can cut here, no need to repeat it twice.
>
> Fix it, and also provide intel_mmio_umap_*() counterparts to public
-------------------------------------- ^
s/umap/unmap/
> functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file().
>
> v2: apply last minute fixes, cached but unfortunately not committed before
> sending
>
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
> lib/intel_io.h | 4 +++
> lib/intel_mmio.c | 67 ++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 63 insertions(+), 8 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..cb8f9db2e5 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,29 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file)
imho at beginning of this function there should be check
that igt_global_mmio == NULL, and the same check should be at
other init functions.
Looks like we cannot mmap two different pcie cards at the same
time with this lib.
> 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->mmio_size || mmio_data->dev,
> + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_dump_file()\n"))
Please shorten text for warning, something like: arg was not
inialized with ...
Please also add check for null at global var.
> + 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,12 +132,14 @@ 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)
> {
> uint32_t devid, gen;
> - int mmio_bar, mmio_size;
> + int mmio_bar;
Please use this local var and assign it to struct only after
succesfull initialization.
> int error;
>
> memset(mmio_data, 0, sizeof(struct intel_mmio_data));
> @@ -129,22 +151,42 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci
>
> gen = intel_gen(devid);
> if (gen < 3)
> - mmio_size = 512*1024;
> + mmio_data->mmio_size = 512*1024;
> else if (gen < 5)
> - mmio_size = 512*1024;
> + mmio_data->mmio_size = 512*1024;
Both places uses the same number 512*1024, please make it one
if check: if (gen < 5)
Or maybe it is an error for gen < 3 ?
> else
> - mmio_size = 2*1024*1024;
> + mmio_data->mmio_size = 2*1024*1024;
>
> error = pci_device_map_range (pci_dev,
> pci_dev->regions[mmio_bar].base_addr,
> - mmio_size,
> + mmio_data->mmio_size,
> 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->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->dev,
> + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_pci_bar()\n"))
Same here, please shorten this warn.
> + 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 +208,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 +266,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->key,
> + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_register_access_init()\n"))
Same here, please shorten this warn.
Btw, in this lib error condition for key is -1, so maybe this
should also be cheked ?
> + return;
> +
> + if (intel_register_access_needs_wake(mmio_data))
> release_forcewake_lock(mmio_data->key);
> + mmio_data->key = 0;
> +
> + intel_mmio_unmap_pci_bar(mmio_data);
> }
>
> /**
> --
> 2.25.1
>
Please correct desciption of global var igt_global_mmio, there
is one more method for initialize it: intel_mmio_use_pci_bar
as you wrote on irc.
Regards,
Kamil Konieczny
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH v2 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini
2022-03-04 15:14 ` Kamil Konieczny
@ 2022-03-04 16:20 ` Janusz Krzysztofik
-1 siblings, 0 replies; 8+ messages in thread
From: Janusz Krzysztofik @ 2022-03-04 16:20 UTC (permalink / raw)
To: Kamil Konieczny, igt-dev, intel-gfx, Janusz Krzysztofik
Hi Kamil,
Thanks for review.
On Friday, 4 March 2022 16:14:05 CET Kamil Konieczny wrote:
> Hi Janusz,
>
> Dnia 2022-03-01 at 15:07:55 +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 with pci_device_map_range().
> ------ ^
> imho you can cut here, no need to repeat it twice.
OK
> >
> > Fix it, and also provide intel_mmio_umap_*() counterparts to public
> -------------------------------------- ^
> s/umap/unmap/
Thanks.
> > functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file().
> >
> > v2: apply last minute fixes, cached but unfortunately not committed before
> > sending
> >
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> > lib/intel_io.h | 4 +++
> > lib/intel_mmio.c | 67 ++++++++++++++++++++++++++++++++++++++++++------
> > 2 files changed, 63 insertions(+), 8 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..cb8f9db2e5 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,29 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file)
>
> imho at beginning of this function there should be check
> that igt_global_mmio == NULL, and the same check should be at
> other init functions.
No, what I think needs to be fixed are users who are still using
igt_global_mmio while they should use the value they passed as the mmio
argument to intel_mmio_use_*() or intel_register_access_init(), and that
global variable should be dropped. But first of all, that's not related to
the issue this patch is trying to fix, then out of scope of this patch.
> Looks like we cannot mmap two different pcie cards at the same
> time with this lib.
We can, if we just ignore that depreciated global variable, I believe.
> > 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->mmio_size || mmio_data->dev,
> > + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_dump_file()\n"))
>
> Please shorten text for warning, something like: arg was not
> inialized with ...
OK
> Please also add check for null at global var.
>
> > + 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,12 +132,14 @@ 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)
> > {
> > uint32_t devid, gen;
> > - int mmio_bar, mmio_size;
> > + int mmio_bar;
>
> Please use this local var and assign it to struct only after
> succesfull initialization.
OK
>
> > int error;
> >
> > memset(mmio_data, 0, sizeof(struct intel_mmio_data));
> > @@ -129,22 +151,42 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci
> >
> > gen = intel_gen(devid);
> > if (gen < 3)
> > - mmio_size = 512*1024;
> > + mmio_data->mmio_size = 512*1024;
> > else if (gen < 5)
> > - mmio_size = 512*1024;
> > + mmio_data->mmio_size = 512*1024;
>
> Both places uses the same number 512*1024, please make it one
> if check: if (gen < 5)
>
> Or maybe it is an error for gen < 3 ?
Out of scope.
>
> > else
> > - mmio_size = 2*1024*1024;
> > + mmio_data->mmio_size = 2*1024*1024;
> >
> > error = pci_device_map_range (pci_dev,
> > pci_dev->regions[mmio_bar].base_addr,
> > - mmio_size,
> > + mmio_data->mmio_size,
> > 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->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->dev,
> > + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_pci_bar()\n"))
>
> Same here, please shorten this warn.
>
> > + 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 +208,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 +266,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->key,
> > + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_register_access_init()\n"))
>
> Same here, please shorten this warn.
>
> Btw, in this lib error condition for key is -1, so maybe this
> should also be cheked ?
No, key == -1 means forcewake is not supported and shouldn't be used but it
still means the structure was initialized by intel_register_access_init().
However, thanks for this question, since now I can see 0, though unlikely, can
be a valid key.
AFAICT, 0000 is not a valid PCI device ID, then mmio->device_id is a much
better candidate than ->key for this check -- I'll switch to it in v2 of this
patch.
> > + return;
> > +
> > + if (intel_register_access_needs_wake(mmio_data))
> > release_forcewake_lock(mmio_data->key);
> > + mmio_data->key = 0;
> > +
> > + intel_mmio_unmap_pci_bar(mmio_data);
> > }
> >
> > /**
> >
>
> Please correct desciption of global var igt_global_mmio, there
> is one more method for initialize it: intel_mmio_use_pci_bar
> as you wrote on irc.
Out of scope.
Thanks,
Janusz
>
> Regards,
> Kamil Konieczny
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [igt-dev] [PATCH v2 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini
@ 2022-03-04 16:20 ` Janusz Krzysztofik
0 siblings, 0 replies; 8+ messages in thread
From: Janusz Krzysztofik @ 2022-03-04 16:20 UTC (permalink / raw)
To: Kamil Konieczny, igt-dev, intel-gfx, Janusz Krzysztofik
Hi Kamil,
Thanks for review.
On Friday, 4 March 2022 16:14:05 CET Kamil Konieczny wrote:
> Hi Janusz,
>
> Dnia 2022-03-01 at 15:07:55 +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 with pci_device_map_range().
> ------ ^
> imho you can cut here, no need to repeat it twice.
OK
> >
> > Fix it, and also provide intel_mmio_umap_*() counterparts to public
> -------------------------------------- ^
> s/umap/unmap/
Thanks.
> > functions intel_mmio_use_pci_bar() and intel_mmio_use_dump_file().
> >
> > v2: apply last minute fixes, cached but unfortunately not committed before
> > sending
> >
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> > lib/intel_io.h | 4 +++
> > lib/intel_mmio.c | 67 ++++++++++++++++++++++++++++++++++++++++++------
> > 2 files changed, 63 insertions(+), 8 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..cb8f9db2e5 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,29 @@ intel_mmio_use_dump_file(struct intel_mmio_data *mmio_data, char *file)
>
> imho at beginning of this function there should be check
> that igt_global_mmio == NULL, and the same check should be at
> other init functions.
No, what I think needs to be fixed are users who are still using
igt_global_mmio while they should use the value they passed as the mmio
argument to intel_mmio_use_*() or intel_register_access_init(), and that
global variable should be dropped. But first of all, that's not related to
the issue this patch is trying to fix, then out of scope of this patch.
> Looks like we cannot mmap two different pcie cards at the same
> time with this lib.
We can, if we just ignore that depreciated global variable, I believe.
> > 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->mmio_size || mmio_data->dev,
> > + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_dump_file()\n"))
>
> Please shorten text for warning, something like: arg was not
> inialized with ...
OK
> Please also add check for null at global var.
>
> > + 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,12 +132,14 @@ 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)
> > {
> > uint32_t devid, gen;
> > - int mmio_bar, mmio_size;
> > + int mmio_bar;
>
> Please use this local var and assign it to struct only after
> succesfull initialization.
OK
>
> > int error;
> >
> > memset(mmio_data, 0, sizeof(struct intel_mmio_data));
> > @@ -129,22 +151,42 @@ intel_mmio_use_pci_bar(struct intel_mmio_data *mmio_data, struct pci_device *pci
> >
> > gen = intel_gen(devid);
> > if (gen < 3)
> > - mmio_size = 512*1024;
> > + mmio_data->mmio_size = 512*1024;
> > else if (gen < 5)
> > - mmio_size = 512*1024;
> > + mmio_data->mmio_size = 512*1024;
>
> Both places uses the same number 512*1024, please make it one
> if check: if (gen < 5)
>
> Or maybe it is an error for gen < 3 ?
Out of scope.
>
> > else
> > - mmio_size = 2*1024*1024;
> > + mmio_data->mmio_size = 2*1024*1024;
> >
> > error = pci_device_map_range (pci_dev,
> > pci_dev->regions[mmio_bar].base_addr,
> > - mmio_size,
> > + mmio_data->mmio_size,
> > 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->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->dev,
> > + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_mmio_use_pci_bar()\n"))
>
> Same here, please shorten this warn.
>
> > + 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 +208,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 +266,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->key,
> > + "test bug: argument doesn't point to struct intel_mmio_data initialized with intel_register_access_init()\n"))
>
> Same here, please shorten this warn.
>
> Btw, in this lib error condition for key is -1, so maybe this
> should also be cheked ?
No, key == -1 means forcewake is not supported and shouldn't be used but it
still means the structure was initialized by intel_register_access_init().
However, thanks for this question, since now I can see 0, though unlikely, can
be a valid key.
AFAICT, 0000 is not a valid PCI device ID, then mmio->device_id is a much
better candidate than ->key for this check -- I'll switch to it in v2 of this
patch.
> > + return;
> > +
> > + if (intel_register_access_needs_wake(mmio_data))
> > release_forcewake_lock(mmio_data->key);
> > + mmio_data->key = 0;
> > +
> > + intel_mmio_unmap_pci_bar(mmio_data);
> > }
> >
> > /**
> >
>
> Please correct desciption of global var igt_global_mmio, there
> is one more method for initialize it: intel_mmio_use_pci_bar
> as you wrote on irc.
Out of scope.
Thanks,
Janusz
>
> Regards,
> Kamil Konieczny
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-03-04 16:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01 14:07 [Intel-gfx] [PATCH v2 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini Janusz Krzysztofik
2022-03-01 14:07 ` [igt-dev] " Janusz Krzysztofik
2022-03-01 14:52 ` [igt-dev] ✓ Fi.CI.BAT: success for lib/intel_mmio: Fix mmapped resources not unmapped on fini (rev2) Patchwork
2022-03-01 21:33 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2022-03-04 15:14 ` [Intel-gfx] [igt-dev] [PATCH v2 i-g-t] lib/intel_mmio: Fix mmapped resources not unmapped on fini Kamil Konieczny
2022-03-04 15:14 ` Kamil Konieczny
2022-03-04 16:20 ` [Intel-gfx] " Janusz Krzysztofik
2022-03-04 16:20 ` Janusz Krzysztofik
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.