[PATCH i-g-t 0/3] Improvements for xe_sriov_flr test and xe_mmio lib

[PATCH i-g-t 0/3] Improvements for xe_sriov_flr test and xe_mmio lib

[Marcin Bernatowicz]

Enhance code maintainability, improve error handling, and provide clearer
debugging information.

lib/xe/xe_mmio: Replace open-coded init/cleanup with existing functions

Use intel_register_access_init and intel_register_access_fini to handle
initialization and cleanup, reducing code duplication. This improves
maintainability and ensures consistency, including handling forcewake
for Xe.

lib/xe/xe_sriov_provisioning: Refactor range handling and logging

Introduce an append_range helper to reduce code duplication in handling
provisioned PTE ranges. Limit debug logs to the first 70 entries for
clarity and improved reasoning. Enhance error handling by propagating
realloc failures.

tests/xe/xe_sriov_flr: Improve clear-ggtt subcheck initialization

Mark the ggtt-clear subcheck as SKIP when prerequisites, such as scanning
GGTT provisioned offsets, are not met. Asserting in these cases falsely
implies that FLR is broken, while the test has not even started.

Marcin Bernatowicz (3):
  lib/xe/xe_mmio: Replace open-coded init/cleanup with existing
    functions
  lib/xe/xe_sriov_provisioning: Refactor range handling and logging
  tests/xe/xe_sriov_flr: Improve clear-ggtt subcheck initialization

 lib/xe/xe_mmio.c               | 10 +----
 lib/xe/xe_sriov_provisioning.c | 71 ++++++++++++++++++++++------------
 tests/intel/xe_sriov_flr.c     | 11 ++++--
 3 files changed, 57 insertions(+), 35 deletions(-)

-- 
2.31.1

[PATCH i-g-t 1/3] lib/xe/xe_mmio: Replace open-coded init/cleanup with existing functions

[Marcin Bernatowicz]

Use intel_register_access_init and intel_register_access_fini to handle
initialization and cleanup, reducing code duplication. This improves
maintainability and ensures consistency, including handling forcewake
for Xe.

Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
Cc: Adam Miszczak <adam.miszczak@linux.intel.com>
Cc: Jakub Kolakowski <jakub1.kolakowski@intel.com>
Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Cc: Lukasz Laguna <lukasz.laguna@intel.com>
Cc: Piotr Piórkowski <piotr.piorkowski@intel.com>
---
 lib/xe/xe_mmio.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/lib/xe/xe_mmio.c b/lib/xe/xe_mmio.c
index 0817c12f6..834816133 100644
--- a/lib/xe/xe_mmio.c
+++ b/lib/xe/xe_mmio.c
@@ -23,13 +23,8 @@ void xe_mmio_vf_access_init(int pf_fd, int vf_id, struct xe_mmio *mmio)
 
 	igt_assert_f(pci_dev, "No PCI device found for VF%u\n", vf_id);
 
-	intel_mmio_use_pci_bar(&mmio->intel_mmio, pci_dev);
-
-	igt_assert(mmio->intel_mmio.igt_mmio);
-
+	intel_register_access_init(&mmio->intel_mmio, pci_dev, false);
 	mmio->fd = pf_fd;
-	mmio->intel_mmio.safe = false;
-	mmio->intel_mmio.pci_device_id = pci_dev->device_id;
 }
 
 /**
@@ -53,8 +48,7 @@ void xe_mmio_access_init(int pf_fd, struct xe_mmio *mmio)
  */
 void xe_mmio_access_fini(struct xe_mmio *mmio)
 {
-	mmio->intel_mmio.pci_device_id = 0;
-	intel_mmio_unmap_pci_bar(&mmio->intel_mmio);
+	intel_register_access_fini(&mmio->intel_mmio);
 }
 
 /**
-- 
2.31.1

[PATCH i-g-t 2/3] lib/xe/xe_sriov_provisioning: Refactor range handling and logging

[Marcin Bernatowicz]

Introduce an append_range helper to reduce code duplication in handling
provisioned PTE ranges. Limit debug logs to the first 70 entries for
clarity and improved reasoning. Enhance error handling by propagating
realloc failures.

Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
Cc: Adam Miszczak <adam.miszczak@linux.intel.com>
Cc: C V Narasimha <narasimha.c.v@intel.com>
Cc: Jakub Kolakowski <jakub1.kolakowski@intel.com>
Cc: K V P Satyanarayana <satyanarayana.k.v.p@intel.com>
Cc: Lukasz Laguna <lukasz.laguna@intel.com>
Cc: Michał Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Piotr Piórkowski <piotr.piorkowski@intel.com>
Cc: Tomasz Lis <tomasz.lis@intel.com>
---
 lib/xe/xe_sriov_provisioning.c | 71 ++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 24 deletions(-)

diff --git a/lib/xe/xe_sriov_provisioning.c b/lib/xe/xe_sriov_provisioning.c
index 7cde2c240..66c0576aa 100644
--- a/lib/xe/xe_sriov_provisioning.c
+++ b/lib/xe/xe_sriov_provisioning.c
@@ -3,7 +3,7 @@
  * Copyright(c) 2024 Intel Corporation. All rights reserved.
  */
 
-#include <stdlib.h>
+#include <errno.h>
 
 #include "igt_core.h"
 #include "intel_chipset.h"
@@ -48,6 +48,37 @@ static uint64_t get_vfid_mask(int fd)
 		GGTT_PTE_VFID_MASK : PRE_1250_IP_VER_GGTT_PTE_VFID_MASK;
 }
 
+#define MAX_DEBUG_ENTRIES 70
+
+static int append_range(struct xe_sriov_provisioned_range **ranges,
+			unsigned int *nr_ranges, unsigned int vf_id,
+			uint32_t start, uint32_t end)
+{
+	struct xe_sriov_provisioned_range *new_ranges;
+
+	new_ranges = realloc(*ranges,
+			     (*nr_ranges + 1) * sizeof(struct xe_sriov_provisioned_range));
+	if (!new_ranges) {
+		free(*ranges);
+		*ranges = NULL;
+		*nr_ranges = 0;
+		return -ENOMEM;
+	}
+
+	*ranges = new_ranges;
+	if (*nr_ranges < MAX_DEBUG_ENTRIES)
+		igt_debug("Found VF%u GGTT range [%#x-%#x] num_ptes=%ld\n",
+			  vf_id, start, end,
+			  (end - start + sizeof(xe_ggtt_pte_t)) /
+			  sizeof(xe_ggtt_pte_t));
+	(*ranges)[*nr_ranges].vf_id = vf_id;
+	(*ranges)[*nr_ranges].start = start;
+	(*ranges)[*nr_ranges].end = end;
+	(*nr_ranges)++;
+
+	return 0;
+}
+
 /**
  * xe_sriov_find_ggtt_provisioned_pte_offsets - Find GGTT provisioned PTE offsets
  * @pf_fd: File descriptor for the Physical Function
@@ -76,6 +107,7 @@ int xe_sriov_find_ggtt_provisioned_pte_offsets(int pf_fd, int gt, struct xe_mmio
 	uint32_t current_start = 0;
 	uint32_t current_end = 0;
 	xe_ggtt_pte_t pte;
+	int ret;
 
 	*ranges = NULL;
 	*nr_ranges = 0;
@@ -86,18 +118,11 @@ int xe_sriov_find_ggtt_provisioned_pte_offsets(int pf_fd, int gt, struct xe_mmio
 
 		if (vf_id != current_vf_id) {
 			if (current_vf_id != -1) {
-				/* End the current range */
-				*ranges = realloc(*ranges, (*nr_ranges + 1) *
-						  sizeof(struct xe_sriov_provisioned_range));
-				igt_assert(*ranges);
-				igt_debug("Found VF%u ggtt range [%#x-%#x] num_ptes=%ld\n",
-					  current_vf_id, current_start, current_end,
-					  (current_end - current_start + sizeof(xe_ggtt_pte_t)) /
-					  sizeof(xe_ggtt_pte_t));
-				(*ranges)[*nr_ranges].vf_id = current_vf_id;
-				(*ranges)[*nr_ranges].start = current_start;
-				(*ranges)[*nr_ranges].end = current_end;
-				(*nr_ranges)++;
+				/* End the current range and append it */
+				ret = append_range(ranges, nr_ranges, current_vf_id,
+						   current_start, current_end);
+				if (ret < 0)
+					return ret;
 			}
 			/* Start a new range */
 			current_vf_id = vf_id;
@@ -107,18 +132,16 @@ int xe_sriov_find_ggtt_provisioned_pte_offsets(int pf_fd, int gt, struct xe_mmio
 	}
 
 	if (current_vf_id != -1) {
-		*ranges = realloc(*ranges, (*nr_ranges + 1) *
-				  sizeof(struct xe_sriov_provisioned_range));
-		igt_assert(*ranges);
-		igt_debug("Found VF%u ggtt range [%#x-%#x] num_ptes=%ld\n",
-			  current_vf_id, current_start, current_end,
-			  (current_end - current_start + sizeof(xe_ggtt_pte_t)) /
-			  sizeof(xe_ggtt_pte_t));
-		(*ranges)[*nr_ranges].vf_id = current_vf_id;
-		(*ranges)[*nr_ranges].start = current_start;
-		(*ranges)[*nr_ranges].end = current_end;
-		(*nr_ranges)++;
+		/* Append the last range */
+		ret = append_range(ranges, nr_ranges, current_vf_id,
+				   current_start, current_end);
+		if (ret < 0)
+			return ret;
 	}
 
+	if (*nr_ranges > MAX_DEBUG_ENTRIES)
+		igt_debug("Ranges output trimmed to first %u entries out of %u",
+			  MAX_DEBUG_ENTRIES, *nr_ranges);
+
 	return 0;
 }
-- 
2.31.1

[PATCH i-g-t 3/3] tests/xe/xe_sriov_flr: Improve clear-ggtt subcheck initialization

[Marcin Bernatowicz]

Mark the ggtt-clear subcheck as SKIP when prerequisites, such as scanning
GGTT provisioned offsets, are not met. Asserting in these cases falsely
implies that FLR is broken, while the test has not even started.

Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
Cc: Adam Miszczak <adam.miszczak@linux.intel.com>
Cc: C V Narasimha <narasimha.c.v@intel.com>
Cc: Jakub Kolakowski <jakub1.kolakowski@intel.com>
Cc: K V P Satyanarayana <satyanarayana.k.v.p@intel.com>
Cc: Lukasz Laguna <lukasz.laguna@intel.com>
Cc: Michał Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Piotr Piórkowski <piotr.piorkowski@intel.com>
Cc: Tomasz Lis <tomasz.lis@intel.com>
---
 tests/intel/xe_sriov_flr.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tests/intel/xe_sriov_flr.c b/tests/intel/xe_sriov_flr.c
index 1049cffec..502691765 100644
--- a/tests/intel/xe_sriov_flr.c
+++ b/tests/intel/xe_sriov_flr.c
@@ -393,7 +393,12 @@ static int populate_ggtt_pte_offsets(struct ggtt_data *gdata)
 		if (vf_id == 0)
 			continue;
 
-		igt_assert(vf_id >= 1 && vf_id <= num_vfs);
+		if (vf_id < 1 || vf_id > num_vfs) {
+			set_skip_reason(&gdata->base, "Unexpected VF%u at range entry %u [%#lx-%#lx], num_vfs=%u\n",
+					vf_id, i, ranges[i].start, ranges[i].end, num_vfs);
+			free(ranges);
+			return -1;
+		}
 
 		if (gdata->pte_offsets[vf_id].end) {
 			set_skip_reason(&gdata->base, "Duplicate GGTT PTE offset range for VF%u\n",
@@ -410,7 +415,7 @@ static int populate_ggtt_pte_offsets(struct ggtt_data *gdata)
 
 	for (int vf_id = 1; vf_id <= num_vfs; ++vf_id)
 		if (!gdata->pte_offsets[vf_id].end) {
-			set_fail_reason(&gdata->base,
+			set_skip_reason(&gdata->base,
 					"Failed to find VF%u provisioned GGTT PTE offset range\n",
 					vf_id);
 			return -1;
@@ -440,7 +445,7 @@ static void ggtt_subcheck_init(struct subcheck_data *data)
 
 		populate_ggtt_pte_offsets(gdata);
 	} else {
-		set_fail_reason(data, "xe_mmio is NULL\n");
+		set_skip_reason(data, "xe_mmio is NULL\n");
 	}
 }
 
-- 
2.31.1

✗ CI.xeBAT: failure for Improvements for xe_sriov_flr test and xe_mmio lib

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2995 bytes --]

== Series Details ==

Series: Improvements for xe_sriov_flr test and xe_mmio lib
URL   : https://patchwork.freedesktop.org/series/141556/
State : failure

== Summary ==

CI Bug Log - changes from XEIGT_8118_BAT -> XEIGTPW_12143_BAT
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with XEIGTPW_12143_BAT absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in XEIGTPW_12143_BAT, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Participating hosts (9 -> 9)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in XEIGTPW_12143_BAT:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_psr@psr-primary-page-flip@edp-1:
    - bat-adlp-7:         [PASS][1] -> [DMESG-WARN][2] +1 other test dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_8118/bat-adlp-7/igt@kms_psr@psr-primary-page-flip@edp-1.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_12143/bat-adlp-7/igt@kms_psr@psr-primary-page-flip@edp-1.html

  
Known issues
------------

  Here are the changes found in XEIGTPW_12143_BAT that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_hotunplug@unbind-rebind:
    - bat-adlp-7:         [PASS][3] -> [DMESG-WARN][4] ([Intel XE#3517])
   [3]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_8118/bat-adlp-7/igt@core_hotunplug@unbind-rebind.html
   [4]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_12143/bat-adlp-7/igt@core_hotunplug@unbind-rebind.html

  * igt@kms_psr@psr-cursor-plane-move:
    - bat-adlp-7:         [PASS][5] -> [SKIP][6] ([Intel XE#3443] / [Intel XE#455]) +3 other tests skip
   [5]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_8118/bat-adlp-7/igt@kms_psr@psr-cursor-plane-move.html
   [6]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_12143/bat-adlp-7/igt@kms_psr@psr-cursor-plane-move.html

  
  [Intel XE#3443]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/3443
  [Intel XE#3517]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/3517
  [Intel XE#455]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/455


Build changes
-------------

  * IGT: IGT_8118 -> IGTPW_12143
  * Linux: xe-2252-f8f85a38f6c75e091805f01ceff4041ac2fdf3fd -> xe-2254-ccf64319402f90f68549e2047a7f985a46436e41

  IGTPW_12143: 12143
  IGT_8118: 17707095f1e5d3c30f463b43022f01c0160579b6 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  xe-2252-f8f85a38f6c75e091805f01ceff4041ac2fdf3fd: f8f85a38f6c75e091805f01ceff4041ac2fdf3fd
  xe-2254-ccf64319402f90f68549e2047a7f985a46436e41: ccf64319402f90f68549e2047a7f985a46436e41

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_12143/index.html

[-- Attachment #2: Type: text/html, Size: 3640 bytes --]

✗ Fi.CI.BAT: failure for Improvements for xe_sriov_flr test and xe_mmio lib

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 4877 bytes --]

== Series Details ==

Series: Improvements for xe_sriov_flr test and xe_mmio lib
URL   : https://patchwork.freedesktop.org/series/141556/
State : failure

== Summary ==

CI Bug Log - changes from IGT_8118 -> IGTPW_12143
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_12143 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_12143, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/index.html

Participating hosts (46 -> 45)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_12143:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@nonblocking-crc:
    - fi-kbl-7567u:       [PASS][1] -> [DMESG-WARN][2] +5 other tests dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8118/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/fi-kbl-7567u/igt@kms_pipe_crc_basic@nonblocking-crc.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@vgem_basic@unload:
    - {bat-arls-6}:       [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8118/bat-arls-6/igt@vgem_basic@unload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/bat-arls-6/igt@vgem_basic@unload.html

  
Known issues
------------

  Here are the changes found in IGTPW_12143 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live:
    - bat-arls-5:         [PASS][5] -> [ABORT][6] ([i915#12829])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8118/bat-arls-5/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/bat-arls-5/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-arls-5:         [PASS][7] -> [ABORT][8] ([i915#12061])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8118/bat-arls-5/igt@i915_selftest@live@workarounds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/bat-arls-5/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - bat-dg1-7:          [FAIL][9] ([i915#12903]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8118/bat-dg1-7/igt@i915_pm_rpm@module-reload.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/bat-dg1-7/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [ABORT][11] ([i915#12829]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8118/bat-mtlp-8/igt@i915_selftest@live.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/bat-mtlp-8/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-8:         [ABORT][13] -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8118/bat-mtlp-8/igt@i915_selftest@live@workarounds.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/bat-mtlp-8/igt@i915_selftest@live@workarounds.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-apl-1:          [DMESG-WARN][15] -> [PASS][16] +2 other tests pass
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8118/bat-apl-1/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/bat-apl-1/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12829]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12829
  [i915#12903]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12903


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_8118 -> IGTPW_12143
  * Linux: CI_DRM_15720 -> CI_DRM_15722

  CI-20190529: 20190529
  CI_DRM_15720: f8f85a38f6c75e091805f01ceff4041ac2fdf3fd @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_15722: ccf64319402f90f68549e2047a7f985a46436e41 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_12143: 12143
  IGT_8118: 17707095f1e5d3c30f463b43022f01c0160579b6 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_12143/index.html

[-- Attachment #2: Type: text/html, Size: 5794 bytes --]

Re: [PATCH i-g-t 2/3] lib/xe/xe_sriov_provisioning: Refactor range handling and logging

[Laguna, Lukasz]

[-- Attachment #1: Type: text/plain, Size: 4863 bytes --]

On 11/19/2024 16:55, Marcin Bernatowicz wrote:
> Introduce an append_range helper to reduce code duplication in handling
> provisioned PTE ranges. Limit debug logs to the first 70 entries for
> clarity and improved reasoning. Enhance error handling by propagating
> realloc failures.
>
> Signed-off-by: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
> Cc: Adam Miszczak<adam.miszczak@linux.intel.com>
> Cc: C V Narasimha<narasimha.c.v@intel.com>
> Cc: Jakub Kolakowski<jakub1.kolakowski@intel.com>
> Cc: K V P Satyanarayana<satyanarayana.k.v.p@intel.com>
> Cc: Lukasz Laguna<lukasz.laguna@intel.com>
> Cc: Michał Wajdeczko<michal.wajdeczko@intel.com>
> Cc: Michał Winiarski<michal.winiarski@intel.com>
> Cc: Piotr Piórkowski<piotr.piorkowski@intel.com>
> Cc: Tomasz Lis<tomasz.lis@intel.com>
> ---
>   lib/xe/xe_sriov_provisioning.c | 71 ++++++++++++++++++++++------------
>   1 file changed, 47 insertions(+), 24 deletions(-)
>
> diff --git a/lib/xe/xe_sriov_provisioning.c b/lib/xe/xe_sriov_provisioning.c
> index 7cde2c240..66c0576aa 100644
> --- a/lib/xe/xe_sriov_provisioning.c
> +++ b/lib/xe/xe_sriov_provisioning.c
> @@ -3,7 +3,7 @@
>    * Copyright(c) 2024 Intel Corporation. All rights reserved.
>    */
>   
> -#include <stdlib.h>
> +#include <errno.h>
>   
>   #include "igt_core.h"
>   #include "intel_chipset.h"
> @@ -48,6 +48,37 @@ static uint64_t get_vfid_mask(int fd)
>   		GGTT_PTE_VFID_MASK : PRE_1250_IP_VER_GGTT_PTE_VFID_MASK;
>   }
>   
> +#define MAX_DEBUG_ENTRIES 70
> +
> +static int append_range(struct xe_sriov_provisioned_range **ranges,
> +			unsigned int *nr_ranges, unsigned int vf_id,
> +			uint32_t start, uint32_t end)
> +{
> +	struct xe_sriov_provisioned_range *new_ranges;
> +
> +	new_ranges = realloc(*ranges,
> +			     (*nr_ranges + 1) * sizeof(struct xe_sriov_provisioned_range));
> +	if (!new_ranges) {
> +		free(*ranges);
> +		*ranges = NULL;
> +		*nr_ranges = 0;
> +		return -ENOMEM;
> +	}
> +
> +	*ranges = new_ranges;
> +	if (*nr_ranges < MAX_DEBUG_ENTRIES)
> +		igt_debug("Found VF%u GGTT range [%#x-%#x] num_ptes=%ld\n",
> +			  vf_id, start, end,
> +			  (end - start + sizeof(xe_ggtt_pte_t)) /
> +			  sizeof(xe_ggtt_pte_t));
> +	(*ranges)[*nr_ranges].vf_id = vf_id;
> +	(*ranges)[*nr_ranges].start = start;
> +	(*ranges)[*nr_ranges].end = end;
> +	(*nr_ranges)++;
> +
> +	return 0;
> +}
> +
>   /**
>    * xe_sriov_find_ggtt_provisioned_pte_offsets - Find GGTT provisioned PTE offsets
>    * @pf_fd: File descriptor for the Physical Function
> @@ -76,6 +107,7 @@ int xe_sriov_find_ggtt_provisioned_pte_offsets(int pf_fd, int gt, struct xe_mmio
>   	uint32_t current_start = 0;
>   	uint32_t current_end = 0;
>   	xe_ggtt_pte_t pte;
> +	int ret;
>   
>   	*ranges = NULL;
>   	*nr_ranges = 0;
> @@ -86,18 +118,11 @@ int xe_sriov_find_ggtt_provisioned_pte_offsets(int pf_fd, int gt, struct xe_mmio
>   
>   		if (vf_id != current_vf_id) {
>   			if (current_vf_id != -1) {
> -				/* End the current range */
> -				*ranges = realloc(*ranges, (*nr_ranges + 1) *
> -						  sizeof(struct xe_sriov_provisioned_range));
> -				igt_assert(*ranges);
> -				igt_debug("Found VF%u ggtt range [%#x-%#x] num_ptes=%ld\n",
> -					  current_vf_id, current_start, current_end,
> -					  (current_end - current_start + sizeof(xe_ggtt_pte_t)) /
> -					  sizeof(xe_ggtt_pte_t));
> -				(*ranges)[*nr_ranges].vf_id = current_vf_id;
> -				(*ranges)[*nr_ranges].start = current_start;
> -				(*ranges)[*nr_ranges].end = current_end;
> -				(*nr_ranges)++;
> +				/* End the current range and append it */
> +				ret = append_range(ranges, nr_ranges, current_vf_id,
> +						   current_start, current_end);
> +				if (ret < 0)
> +					return ret;
>   			}
>   			/* Start a new range */
>   			current_vf_id = vf_id;
> @@ -107,18 +132,16 @@ int xe_sriov_find_ggtt_provisioned_pte_offsets(int pf_fd, int gt, struct xe_mmio
>   	}
>   
>   	if (current_vf_id != -1) {
> -		*ranges = realloc(*ranges, (*nr_ranges + 1) *
> -				  sizeof(struct xe_sriov_provisioned_range));
> -		igt_assert(*ranges);
> -		igt_debug("Found VF%u ggtt range [%#x-%#x] num_ptes=%ld\n",
> -			  current_vf_id, current_start, current_end,
> -			  (current_end - current_start + sizeof(xe_ggtt_pte_t)) /
> -			  sizeof(xe_ggtt_pte_t));
> -		(*ranges)[*nr_ranges].vf_id = current_vf_id;
> -		(*ranges)[*nr_ranges].start = current_start;
> -		(*ranges)[*nr_ranges].end = current_end;
> -		(*nr_ranges)++;
> +		/* Append the last range */
> +		ret = append_range(ranges, nr_ranges, current_vf_id,
> +				   current_start, current_end);
> +		if (ret < 0)
> +			return ret;
>   	}
>   
> +	if (*nr_ranges > MAX_DEBUG_ENTRIES)
> +		igt_debug("Ranges output trimmed to first %u entries out of %u",
> +			  MAX_DEBUG_ENTRIES, *nr_ranges);
> +
>   	return 0;
>   }

LGTM,
Reviewed-by: Lukasz Laguna <lukasz.laguna@intel.com 
<mailto:lukasz.laguna@intel.com>>


[-- Attachment #2: Type: text/html, Size: 6147 bytes --]

Re: [PATCH i-g-t 3/3] tests/xe/xe_sriov_flr: Improve clear-ggtt subcheck initialization

[Laguna, Lukasz]

[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]

On 11/19/2024 16:55, Marcin Bernatowicz wrote:
> Mark the ggtt-clear subcheck as SKIP when prerequisites, such as scanning
> GGTT provisioned offsets, are not met. Asserting in these cases falsely
> implies that FLR is broken, while the test has not even started.
>
> Signed-off-by: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
> Cc: Adam Miszczak<adam.miszczak@linux.intel.com>
> Cc: C V Narasimha<narasimha.c.v@intel.com>
> Cc: Jakub Kolakowski<jakub1.kolakowski@intel.com>
> Cc: K V P Satyanarayana<satyanarayana.k.v.p@intel.com>
> Cc: Lukasz Laguna<lukasz.laguna@intel.com>
> Cc: Michał Wajdeczko<michal.wajdeczko@intel.com>
> Cc: Michał Winiarski<michal.winiarski@intel.com>
> Cc: Piotr Piórkowski<piotr.piorkowski@intel.com>
> Cc: Tomasz Lis<tomasz.lis@intel.com>
> ---
>   tests/intel/xe_sriov_flr.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/tests/intel/xe_sriov_flr.c b/tests/intel/xe_sriov_flr.c
> index 1049cffec..502691765 100644
> --- a/tests/intel/xe_sriov_flr.c
> +++ b/tests/intel/xe_sriov_flr.c
> @@ -393,7 +393,12 @@ static int populate_ggtt_pte_offsets(struct ggtt_data *gdata)
>   		if (vf_id == 0)
>   			continue;
>   
> -		igt_assert(vf_id >= 1 && vf_id <= num_vfs);
> +		if (vf_id < 1 || vf_id > num_vfs) {
> +			set_skip_reason(&gdata->base, "Unexpected VF%u at range entry %u [%#lx-%#lx], num_vfs=%u\n",

nit: Unexpected VF ID (%u)...

> +					vf_id, i, ranges[i].start, ranges[i].end, num_vfs);
> +			free(ranges);
> +			return -1;
> +		}
>   
>   		if (gdata->pte_offsets[vf_id].end) {
>   			set_skip_reason(&gdata->base, "Duplicate GGTT PTE offset range for VF%u\n",
> @@ -410,7 +415,7 @@ static int populate_ggtt_pte_offsets(struct ggtt_data *gdata)
>   
>   	for (int vf_id = 1; vf_id <= num_vfs; ++vf_id)
>   		if (!gdata->pte_offsets[vf_id].end) {
> -			set_fail_reason(&gdata->base,
> +			set_skip_reason(&gdata->base,
>   					"Failed to find VF%u provisioned GGTT PTE offset range\n",
>   					vf_id);
>   			return -1;
> @@ -440,7 +445,7 @@ static void ggtt_subcheck_init(struct subcheck_data *data)
>   
>   		populate_ggtt_pte_offsets(gdata);
>   	} else {
> -		set_fail_reason(data, "xe_mmio is NULL\n");
> +		set_skip_reason(data, "xe_mmio is NULL\n");
>   	}
>   }
>   

Looks good overall,but I see that we still fail in case of other 
prerequisites not met (LMEM, GGTT write/read, no access to MMIO, etc). 
Maybe we should skip in these cases too?

Lukasz


[-- Attachment #2: Type: text/html, Size: 3975 bytes --]

Re: [PATCH i-g-t 1/3] lib/xe/xe_mmio: Replace open-coded init/cleanup with existing functions

[Laguna, Lukasz]

[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]

On 11/19/2024 16:55, Marcin Bernatowicz wrote:
> Use intel_register_access_init and intel_register_access_fini to handle
> initialization and cleanup, reducing code duplication. This improves
> maintainability and ensures consistency, including handling forcewake
> for Xe.
>
> Signed-off-by: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
> Cc: Adam Miszczak<adam.miszczak@linux.intel.com>
> Cc: Jakub Kolakowski<jakub1.kolakowski@intel.com>
> Cc: Kamil Konieczny<kamil.konieczny@linux.intel.com>
> Cc: Lukasz Laguna<lukasz.laguna@intel.com>
> Cc: Piotr Piórkowski<piotr.piorkowski@intel.com>
> ---
>   lib/xe/xe_mmio.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/lib/xe/xe_mmio.c b/lib/xe/xe_mmio.c
> index 0817c12f6..834816133 100644
> --- a/lib/xe/xe_mmio.c
> +++ b/lib/xe/xe_mmio.c
> @@ -23,13 +23,8 @@ void xe_mmio_vf_access_init(int pf_fd, int vf_id, struct xe_mmio *mmio)
>   
>   	igt_assert_f(pci_dev, "No PCI device found for VF%u\n", vf_id);
>   
> -	intel_mmio_use_pci_bar(&mmio->intel_mmio, pci_dev);
> -
> -	igt_assert(mmio->intel_mmio.igt_mmio);
> -
> +	intel_register_access_init(&mmio->intel_mmio, pci_dev, false);
>   	mmio->fd = pf_fd;
> -	mmio->intel_mmio.safe = false;
> -	mmio->intel_mmio.pci_device_id = pci_dev->device_id;
>   }
>   
>   /**
> @@ -53,8 +48,7 @@ void xe_mmio_access_init(int pf_fd, struct xe_mmio *mmio)
>    */
>   void xe_mmio_access_fini(struct xe_mmio *mmio)
>   {
> -	mmio->intel_mmio.pci_device_id = 0;
> -	intel_mmio_unmap_pci_bar(&mmio->intel_mmio);
> +	intel_register_access_fini(&mmio->intel_mmio);
>   }
>   
>   /**

Reviewed-by: Lukasz Laguna <lukasz.laguna@intel.com 
<mailto:lukasz.laguna@intel.com>>


[-- Attachment #2: Type: text/html, Size: 2817 bytes --]

Re: [PATCH i-g-t 3/3] tests/xe/xe_sriov_flr: Improve clear-ggtt subcheck initialization

[Bernatowicz, Marcin]

On 11/20/2024 11:53 AM, Laguna, Lukasz wrote:
> On 11/19/2024 16:55, Marcin Bernatowicz wrote:
>> Mark the ggtt-clear subcheck as SKIP when prerequisites, such as scanning
>> GGTT provisioned offsets, are not met. Asserting in these cases falsely
>> implies that FLR is broken, while the test has not even started.
>>
>> Signed-off-by: Marcin Bernatowicz<marcin.bernatowicz@linux.intel.com>
>> Cc: Adam Miszczak<adam.miszczak@linux.intel.com>
>> Cc: C V Narasimha<narasimha.c.v@intel.com>
>> Cc: Jakub Kolakowski<jakub1.kolakowski@intel.com>
>> Cc: K V P Satyanarayana<satyanarayana.k.v.p@intel.com>
>> Cc: Lukasz Laguna<lukasz.laguna@intel.com>
>> Cc: Michał Wajdeczko<michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski<michal.winiarski@intel.com>
>> Cc: Piotr Piórkowski<piotr.piorkowski@intel.com>
>> Cc: Tomasz Lis<tomasz.lis@intel.com>
>> ---
>>   tests/intel/xe_sriov_flr.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/intel/xe_sriov_flr.c b/tests/intel/xe_sriov_flr.c
>> index 1049cffec..502691765 100644
>> --- a/tests/intel/xe_sriov_flr.c
>> +++ b/tests/intel/xe_sriov_flr.c
>> @@ -393,7 +393,12 @@ static int populate_ggtt_pte_offsets(struct ggtt_data *gdata)
>>   		if (vf_id == 0)
>>   			continue;
>>   
>> -		igt_assert(vf_id >= 1 && vf_id <= num_vfs);
>> +		if (vf_id < 1 || vf_id > num_vfs) {
>> +			set_skip_reason(&gdata->base, "Unexpected VF%u at range entry %u [%#lx-%#lx], num_vfs=%u\n",
> 
> nit: Unexpected VF ID (%u)...
> 
>> +					vf_id, i, ranges[i].start, ranges[i].end, num_vfs);
>> +			free(ranges);
>> +			return -1;
>> +		}
>>   
>>   		if (gdata->pte_offsets[vf_id].end) {
>>   			set_skip_reason(&gdata->base, "Duplicate GGTT PTE offset range for VF%u\n",
>> @@ -410,7 +415,7 @@ static int populate_ggtt_pte_offsets(struct ggtt_data *gdata)
>>   
>>   	for (int vf_id = 1; vf_id <= num_vfs; ++vf_id)
>>   		if (!gdata->pte_offsets[vf_id].end) {
>> -			set_fail_reason(&gdata->base,
>> +			set_skip_reason(&gdata->base,
>>   					"Failed to find VF%u provisioned GGTT PTE offset range\n",
>>   					vf_id);
>>   			return -1;
>> @@ -440,7 +445,7 @@ static void ggtt_subcheck_init(struct subcheck_data *data)
>>   
>>   		populate_ggtt_pte_offsets(gdata);
>>   	} else {
>> -		set_fail_reason(data, "xe_mmio is NULL\n");
>> +		set_skip_reason(data, "xe_mmio is NULL\n");
>>   	}
>>   }
>>   
> 
> Looks good overall,but I see that we still fail in case of other 
> prerequisites not met (LMEM, GGTT write/read, no access to MMIO, etc). 
> Maybe we should skip in these cases too?

Makes sense, I'll extend the skips to the init/prepare phase for all checks.

Marcin

> 
> Lukasz
> 
>