[igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Lucas De Marchi]

https://patchwork.freedesktop.org/series/126786/ groups all the live
tests into a single kernel module. Use that module to run any of the
tests triggered by igt.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 tests/intel/xe_live_ktest.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/tests/intel/xe_live_ktest.c b/tests/intel/xe_live_ktest.c
index fe7b2e69f..c8bd68928 100644
--- a/tests/intel/xe_live_ktest.c
+++ b/tests/intel/xe_live_ktest.c
@@ -19,15 +19,10 @@
  * Functionality: migrate
  */
 
-struct kunit_tests {
-	const char *kunit;
-	const char *name;
-};
-
-static const struct kunit_tests live_tests[] = {
-	{ "xe_bo_test",		"bo" },
-	{ "xe_dma_buf_test",	"dmabuf" },
-	{ "xe_migrate_test",	"migrate" },
+static const char *live_tests[] = {
+	"bo",
+	"dmabuf",
+	"migrate",
 };
 
 igt_main
@@ -35,5 +30,5 @@ igt_main
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(live_tests); i++)
-		igt_kunit(live_tests[i].kunit, live_tests[i].name, NULL);
+		igt_kunit("xe_live_test", live_tests[i], NULL);
 }
-- 
2.40.1

[igt-dev] ✗ Fi.CI.BAT: failure for xe_live_ktest: Use xe_live_test kernel module

[Patchwork]

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

== Series Details ==

Series: xe_live_ktest: Use xe_live_test kernel module
URL   : https://patchwork.freedesktop.org/series/127396/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13983 -> IGTPW_10347
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_10347 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_10347, 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_10347/index.html

Participating hosts (35 -> 21)
------------------------------

  Missing    (14): fi-rkl-11600 bat-dg2-14 bat-adls-5 fi-bsw-n3050 bat-dg2-9 fi-cfl-8700k fi-snb-2520m fi-cfl-guc fi-kbl-guc bat-atsm-1 bat-jsl-3 bat-rplp-1 fi-bsw-nick bat-jsl-1 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-adlp-9:         NOTRUN -> [SKIP][1] +2 other tests skip
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10347/bat-adlp-9/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_flip@basic-flip-vs-dpms@b-dp5:
    - bat-adlp-11:        [PASS][2] -> [DMESG-FAIL][3] ([i915#6868])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13983/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@b-dp5.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10347/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@b-dp5.html

  * igt@kms_flip@basic-flip-vs-dpms@d-dp6:
    - bat-adlp-11:        [PASS][4] -> [FAIL][5] ([i915#6121]) +5 other tests fail
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13983/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10347/bat-adlp-11/igt@kms_flip@basic-flip-vs-dpms@d-dp6.html

  * igt@kms_flip@basic-flip-vs-modeset@b-dp5:
    - bat-adlp-11:        [PASS][6] -> [DMESG-WARN][7] ([i915#6868])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13983/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@b-dp5.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10347/bat-adlp-11/igt@kms_flip@basic-flip-vs-modeset@b-dp5.html

  
  [i915#6121]: https://gitlab.freedesktop.org/drm/intel/issues/6121
  [i915#6868]: https://gitlab.freedesktop.org/drm/intel/issues/6868


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7622 -> IGTPW_10347

  CI-20190529: 20190529
  CI_DRM_13983: a9d99261a978835b02e248fe18af3026416af3e8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_10347: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10347/index.html
  IGT_7622: 48a47d91b7727215b965690c69d84159c8fb1aa2 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


Testlist changes
----------------

+igt@xe_guc_pc@freq_basic_api
+igt@xe_guc_pc@freq_fixed_idle
+igt@xe_guc_pc@freq_low_max
+igt@xe_guc_pc@freq_range_idle
+igt@xe_guc_pc@freq_reset
+igt@xe_guc_pc@freq_reset_multiple
+igt@xe_guc_pc@freq_suspend
-igt@xe_gt_freq@freq_basic_api
-igt@xe_gt_freq@freq_fixed_idle
-igt@xe_gt_freq@freq_low_max
-igt@xe_gt_freq@freq_range_idle
-igt@xe_gt_freq@freq_reset
-igt@xe_gt_freq@freq_reset_multiple
-igt@xe_gt_freq@freq_suspend
-igt@xe_gt_freq@throttle_basic_api

== Logs ==

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

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

[igt-dev] ✗ CI.xeBAT: failure for xe_live_ktest: Use xe_live_test kernel module

[Patchwork]

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

== Series Details ==

Series: xe_live_ktest: Use xe_live_test kernel module
URL   : https://patchwork.freedesktop.org/series/127396/
State : failure

== Summary ==

CI Bug Log - changes from XEIGT_7622_BAT -> XEIGTPW_10347_BAT
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with XEIGTPW_10347_BAT absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in XEIGTPW_10347_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 (4 -> 4)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@xe_live_ktest@bo:
    - bat-pvc-2:          [PASS][1] -> [SKIP][2] +2 other tests skip
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7622/bat-pvc-2/igt@xe_live_ktest@bo.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10347/bat-pvc-2/igt@xe_live_ktest@bo.html
    - bat-adlp-7:         [PASS][3] -> [SKIP][4] +2 other tests skip
   [3]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7622/bat-adlp-7/igt@xe_live_ktest@bo.html
   [4]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10347/bat-adlp-7/igt@xe_live_ktest@bo.html

  * igt@xe_live_ktest@migrate:
    - bat-dg2-oem2:       [PASS][5] -> [SKIP][6] +2 other tests skip
   [5]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7622/bat-dg2-oem2/igt@xe_live_ktest@migrate.html
   [6]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10347/bat-dg2-oem2/igt@xe_live_ktest@migrate.html
    - bat-atsm-2:         [PASS][7] -> [SKIP][8] +2 other tests skip
   [7]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7622/bat-atsm-2/igt@xe_live_ktest@migrate.html
   [8]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10347/bat-atsm-2/igt@xe_live_ktest@migrate.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - bat-adlp-7:         [PASS][9] -> [FAIL][10] ([i915#2346])
   [9]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7622/bat-adlp-7/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [10]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10347/bat-adlp-7/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3:
    - bat-dg2-oem2:       [PASS][11] -> [FAIL][12] ([Intel XE#480])
   [11]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7622/bat-dg2-oem2/igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3.html
   [12]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10347/bat-dg2-oem2/igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3.html

  
#### Possible fixes ####

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-dp3:
    - bat-dg2-oem2:       [FAIL][13] ([Intel XE#480]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7622/bat-dg2-oem2/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp3.html
   [14]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10347/bat-dg2-oem2/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp3.html

  
  [Intel XE#480]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/480
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346


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

  * IGT: IGT_7622 -> IGTPW_10347
  * Linux: xe-550-ac7b89571d802765762e7c15d78a6dcd5d92c41b -> xe-552-83cacef2403d947b26b12b6abb11240d626a9b75

  IGTPW_10347: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10347/index.html
  IGT_7622: 48a47d91b7727215b965690c69d84159c8fb1aa2 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  xe-550-ac7b89571d802765762e7c15d78a6dcd5d92c41b: ac7b89571d802765762e7c15d78a6dcd5d92c41b
  xe-552-83cacef2403d947b26b12b6abb11240d626a9b75: 83cacef2403d947b26b12b6abb11240d626a9b75

== Logs ==

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

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

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Mauro Carvalho Chehab]

On Tue,  5 Dec 2023 14:49:26 -0800
Lucas De Marchi <lucas.demarchi@intel.com> wrote:

> https://patchwork.freedesktop.org/series/126786/ groups all the live
> tests into a single kernel module. Use that module to run any of the
> tests triggered by igt.
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  tests/intel/xe_live_ktest.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/intel/xe_live_ktest.c b/tests/intel/xe_live_ktest.c
> index fe7b2e69f..c8bd68928 100644
> --- a/tests/intel/xe_live_ktest.c
> +++ b/tests/intel/xe_live_ktest.c
> @@ -19,15 +19,10 @@
>   * Functionality: migrate
>   */
>  
> -struct kunit_tests {
> -	const char *kunit;
> -	const char *name;
> -};
> -
> -static const struct kunit_tests live_tests[] = {
> -	{ "xe_bo_test",		"bo" },
> -	{ "xe_dma_buf_test",	"dmabuf" },
> -	{ "xe_migrate_test",	"migrate" },
> +static const char *live_tests[] = {
> +	"bo",
> +	"dmabuf",
> +	"migrate",
>  };
>  
>  igt_main
> @@ -35,5 +30,5 @@ igt_main
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(live_tests); i++)
> -		igt_kunit(live_tests[i].kunit, live_tests[i].name, NULL);
> +		igt_kunit("xe_live_test", live_tests[i], NULL);

This loop will modprobe xe_live_test the times. Just do something
like:

	igt_kunit("xe_live_test", "live_tests", NULL);

>  }

Regards,
Mauro

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Lucas De Marchi]

On Thu, Dec 21, 2023 at 04:15:23PM -0600, Lucas De Marchi wrote:
>On Fri, Dec 08, 2023 at 12:13:36PM -0600, Lucas De Marchi wrote:
>>On Thu, Dec 07, 2023 at 04:41:52PM +0100, Mauro Carvalho Chehab wrote:
>>>On Tue,  5 Dec 2023 14:49:26 -0800
>>>Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>>>
>>>>https://patchwork.freedesktop.org/series/126786/ groups all the live
>>>>tests into a single kernel module. Use that module to run any of the
>>>>tests triggered by igt.
>>>>
>>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>>>---
>>>>tests/intel/xe_live_ktest.c | 15 +++++----------
>>>>1 file changed, 5 insertions(+), 10 deletions(-)
>>>>
>>>>diff --git a/tests/intel/xe_live_ktest.c b/tests/intel/xe_live_ktest.c
>>>>index fe7b2e69f..c8bd68928 100644
>>>>--- a/tests/intel/xe_live_ktest.c
>>>>+++ b/tests/intel/xe_live_ktest.c
>>>>@@ -19,15 +19,10 @@
>>>> * Functionality: migrate
>>>> */
>>>>
>>>>-struct kunit_tests {
>>>>-	const char *kunit;
>>>>-	const char *name;
>>>>-};
>>>>-
>>>>-static const struct kunit_tests live_tests[] = {
>>>>-	{ "xe_bo_test",		"bo" },
>>>>-	{ "xe_dma_buf_test",	"dmabuf" },
>>>>-	{ "xe_migrate_test",	"migrate" },
>>>>+static const char *live_tests[] = {
>>>>+	"bo",
>>>>+	"dmabuf",
>>>>+	"migrate",
>>>>};
>>>>
>>>>igt_main
>>>>@@ -35,5 +30,5 @@ igt_main
>>>>	int i;
>>>>
>>>>	for (i = 0; i < ARRAY_SIZE(live_tests); i++)
>>>>-		igt_kunit(live_tests[i].kunit, live_tests[i].name, NULL);
>>>>+		igt_kunit("xe_live_test", live_tests[i], NULL);
>>>
>>>This loop will modprobe xe_live_test the times. Just do something
>>>like:
>>>
>>>	igt_kunit("xe_live_test", "live_tests", NULL);
>>
>>but there's no such "live_tests" suite and we want to run each of
>>them individually, not together.  How would you select the subtest
>>otherwise?
>
>gentle ping as I want to merge the kernel patch that is blocked on this
>one.

I did some tests today and... current situation is not very good. The
kunit module has the param filter_glob to filter the testsuite, but it's
not used at all by igt.  live_tests[i].name is more informational than
to really do any filtering.

Another issue I see is that since we have the dep xe.ko -> kunit.ko, as
soon as we load xe, kunit comes as a dependency. Since kunit's param are
read-only, we can't really change the testsuite that will run without
first remove xe. Having to reload xe multiple times doesn't seem a good
option.

Some manual tests I did here. First a kernel patch:

	diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
	index 1236b3cd2fbb..30ed9d321c19 100644
	--- a/lib/kunit/executor.c
	+++ b/lib/kunit/executor.c
	@@ -31,7 +31,7 @@ static char *filter_glob_param;
	 static char *filter_param;
	 static char *filter_action_param;

	-module_param_named(filter_glob, filter_glob_param, charp, 0400);
	+module_param_named(filter_glob, filter_glob_param, charp, 0600);
	 MODULE_PARM_DESC(filter_glob,
			"Filter which KUnit test suites/tests run at boot-time, e.g. list* or list*.*del_test");
	 module_param_named(filter, filter_param, charp, 0400);

Now we can filter the testsuite at will:

	# modprobe xe
	
Run only xe_bo tests:

	# echo -n xe_bo > /sys/modules/kunit/parameters/filter_glob
	# dmesg -C
	# modprobe xe_live_test
	# dmesg
	[ 4064.402190] KTAP version 1
	[ 4064.402195] 1..1
	[ 4064.403336]     KTAP version 1
	[ 4064.403339]     # Subtest: xe_bo
	[ 4064.403342]     # module: xe_live_test
	[ 4064.403410]     1..2
	...
	[ 4065.712622]     ok 2 xe_bo_evict_kunit
	[ 4065.712646] # xe_bo: pass:2 fail:0 skip:0 total:2
	[ 4065.712668] # Totals: pass:2 fail:0 skip:0 total:2
	[ 4065.712700] ok 1 xe_bo

Execute all live tests:

	# modprobe -r xe_live_test
	# dmesg -C
	# echo -n \* > /sys/modules/kunit/parameters/filter_glob
	# modprobe xe_live_test
	# dmesg
	[ 5010.536719] KTAP version 1
	[ 5010.536725] 1..4
	[ 5010.537618]     KTAP version 1
	[ 5010.537620]     # Subtest: xe_bo
	[ 5010.537623]     # module: xe_live_test
	...
	[ 5011.831597]     KTAP version 1
	[ 5011.831599]     # Subtest: xe_dma_buf
	[ 5011.831601]     # module: xe_live_test
	...
	[ 5011.867751]     KTAP version 1
	[ 5011.867753]     # Subtest: xe_migrate
	[ 5011.867755]     # module: xe_live_test
	...
	[ 5011.882481]     KTAP version 1
	[ 5011.882483]     # Subtest: xe_mocs
	[ 5011.882485]     # module: xe_live_test

To simplify our kunit lib, we could also rely on kunit_debugfs to list
tests.

I will prepare a patch for the kunit part in the kernel.

Lucas De Marchi

>
>Lucas De Marchi
>
>>
>>Lucas De Marchi
>>
>>>
>>>>}
>>>
>>>Regards,
>>>Mauro

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Mauro Carvalho Chehab]

On Thu, 11 Jan 2024 17:45:37 -0600
Lucas De Marchi <lucas.demarchi@intel.com> wrote:

> On Thu, Dec 21, 2023 at 04:15:23PM -0600, Lucas De Marchi wrote:
> >On Fri, Dec 08, 2023 at 12:13:36PM -0600, Lucas De Marchi wrote:  
> >>On Thu, Dec 07, 2023 at 04:41:52PM +0100, Mauro Carvalho Chehab wrote:  
> >>>On Tue,  5 Dec 2023 14:49:26 -0800
> >>>Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> >>>  
> >>>>https://patchwork.freedesktop.org/series/126786/ groups all the live
> >>>>tests into a single kernel module. Use that module to run any of the
> >>>>tests triggered by igt.
> >>>>
> >>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >>>>---
> >>>>tests/intel/xe_live_ktest.c | 15 +++++----------
> >>>>1 file changed, 5 insertions(+), 10 deletions(-)
> >>>>
> >>>>diff --git a/tests/intel/xe_live_ktest.c b/tests/intel/xe_live_ktest.c
> >>>>index fe7b2e69f..c8bd68928 100644
> >>>>--- a/tests/intel/xe_live_ktest.c
> >>>>+++ b/tests/intel/xe_live_ktest.c
> >>>>@@ -19,15 +19,10 @@
> >>>> * Functionality: migrate
> >>>> */
> >>>>
> >>>>-struct kunit_tests {
> >>>>-	const char *kunit;
> >>>>-	const char *name;
> >>>>-};
> >>>>-
> >>>>-static const struct kunit_tests live_tests[] = {
> >>>>-	{ "xe_bo_test",		"bo" },
> >>>>-	{ "xe_dma_buf_test",	"dmabuf" },
> >>>>-	{ "xe_migrate_test",	"migrate" },
> >>>>+static const char *live_tests[] = {
> >>>>+	"bo",
> >>>>+	"dmabuf",
> >>>>+	"migrate",
> >>>>};
> >>>>
> >>>>igt_main
> >>>>@@ -35,5 +30,5 @@ igt_main
> >>>>	int i;
> >>>>
> >>>>	for (i = 0; i < ARRAY_SIZE(live_tests); i++)
> >>>>-		igt_kunit(live_tests[i].kunit, live_tests[i].name, NULL);
> >>>>+		igt_kunit("xe_live_test", live_tests[i], NULL);  
> >>>
> >>>This loop will modprobe xe_live_test the times. Just do something
> >>>like:
> >>>
> >>>	igt_kunit("xe_live_test", "live_tests", NULL);  
> >>
> >>but there's no such "live_tests" suite and we want to run each of
> >>them individually, not together.  How would you select the subtest
> >>otherwise?  
> >
> >gentle ping as I want to merge the kernel patch that is blocked on this
> >one.  
> 
> I did some tests today and... current situation is not very good. The
> kunit module has the param filter_glob to filter the testsuite, but it's
> not used at all by igt.  live_tests[i].name is more informational than
> to really do any filtering.

Yeah, that's what I would be expecting when I commented this patch. 
Just didn't have the time yet for doing the tests. Btw, before Janusz Kernel
patches, filter_glob were actually completely ignored with modprobe. It used
to work only at Kernel boot time and when compiled with Kunit modules builtin.
In practice, it was originally designed to work with kunit.py.

> Another issue I see is that since we have the dep xe.ko -> kunit.ko, as
> soon as we load xe, kunit comes as a dependency. Since kunit's param are
> read-only, we can't really change the testsuite that will run without
> first remove xe. Having to reload xe multiple times doesn't seem a good
> option.

The dependency between xe and kunit can be removed, but will require some
redesign, moving all the logic calling kunit kAPI symbols out of xe.ko,
moving them to xe_live_test & friends. This will likely require using
some namespaced export symbol logic, to let them to call functions that
aren't part of kAPI. Doable, but not trivial.

> Some manual tests I did here. First a kernel patch:
> 
> 	diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> 	index 1236b3cd2fbb..30ed9d321c19 100644
> 	--- a/lib/kunit/executor.c
> 	+++ b/lib/kunit/executor.c
> 	@@ -31,7 +31,7 @@ static char *filter_glob_param;
> 	 static char *filter_param;
> 	 static char *filter_action_param;
> 
> 	-module_param_named(filter_glob, filter_glob_param, charp, 0400);
> 	+module_param_named(filter_glob, filter_glob_param, charp, 0600);
> 	 MODULE_PARM_DESC(filter_glob,
> 			"Filter which KUnit test suites/tests run at boot-time, e.g. list* or list*.*del_test");
> 	 module_param_named(filter, filter_param, charp, 0400);

Such patch needs to be reviewed by Kunit maintainers. IMO, it only
makes sense upstream if one could do something similar to:

	modprobe kunit filter_glob=none
	modprobe -r xe_live_test
	echo -n xe_bo > /sys/modules/kunit/parameters/filter_glob
	<xe_bo tests executed>
	echo -n xe_dmabuf > /sys/modules/kunit/parameters/filter_glob
	<xe_dmabuf tests executed>

e.g. if changing filter_glob would actually trigger tests, and, when
kunit itself is loaded, the filter_glob parameter is set in a way that
no tests will be executed.

In practice, lib/kunit should use module_param_cb() to register a 
callback that will be used to <re->trigger the tests.

> 
> Now we can filter the testsuite at will:
> 
> 	# modprobe xe
> 	
> Run only xe_bo tests:
> 
> 	# echo -n xe_bo > /sys/modules/kunit/parameters/filter_glob
> 	# dmesg -C
> 	# modprobe xe_live_test
> 	# dmesg
> 	[ 4064.402190] KTAP version 1
> 	[ 4064.402195] 1..1
> 	[ 4064.403336]     KTAP version 1
> 	[ 4064.403339]     # Subtest: xe_bo
> 	[ 4064.403342]     # module: xe_live_test
> 	[ 4064.403410]     1..2
> 	...
> 	[ 4065.712622]     ok 2 xe_bo_evict_kunit
> 	[ 4065.712646] # xe_bo: pass:2 fail:0 skip:0 total:2
> 	[ 4065.712668] # Totals: pass:2 fail:0 skip:0 total:2
> 	[ 4065.712700] ok 1 xe_bo
> 
> Execute all live tests:
> 
> 	# modprobe -r xe_live_test
> 	# dmesg -C
> 	# echo -n \* > /sys/modules/kunit/parameters/filter_glob
> 	# modprobe xe_live_test

See, here, filter_glob is used R/O by xe_live_test. You are
changing it to 0600 just because xe.o (IMO incorrectly) depends on
kunit.o.

> 	# dmesg
> 	[ 5010.536719] KTAP version 1
> 	[ 5010.536725] 1..4
> 	[ 5010.537618]     KTAP version 1
> 	[ 5010.537620]     # Subtest: xe_bo
> 	[ 5010.537623]     # module: xe_live_test
> 	...
> 	[ 5011.831597]     KTAP version 1
> 	[ 5011.831599]     # Subtest: xe_dma_buf
> 	[ 5011.831601]     # module: xe_live_test
> 	...
> 	[ 5011.867751]     KTAP version 1
> 	[ 5011.867753]     # Subtest: xe_migrate
> 	[ 5011.867755]     # module: xe_live_test
> 	...
> 	[ 5011.882481]     KTAP version 1
> 	[ 5011.882483]     # Subtest: xe_mocs
> 	[ 5011.882485]     # module: xe_live_test
> 
> To simplify our kunit lib, we could also rely on kunit_debugfs to list
> tests.

On such case, you should document kunit_debugfs at Documentation/ABI.

> 
> I will prepare a patch for the kunit part in the kernel.
> 
> Lucas De Marchi

Regards,
Mauro

> 
> >
> >Lucas De Marchi
> >  
> >>
> >>Lucas De Marchi
> >>  
> >>>  
> >>>>}  
> >>>
> >>>Regards,
> >>>Mauro  

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Lucas De Marchi]

On Fri, Jan 12, 2024 at 09:32:52AM +0100, Mauro Carvalho Chehab wrote:
>On Thu, 11 Jan 2024 17:45:37 -0600
>Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>
>> On Thu, Dec 21, 2023 at 04:15:23PM -0600, Lucas De Marchi wrote:
>> >On Fri, Dec 08, 2023 at 12:13:36PM -0600, Lucas De Marchi wrote:
>> >>On Thu, Dec 07, 2023 at 04:41:52PM +0100, Mauro Carvalho Chehab wrote:
>> >>>On Tue,  5 Dec 2023 14:49:26 -0800
>> >>>Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> >>>
>> >>>>https://patchwork.freedesktop.org/series/126786/ groups all the live
>> >>>>tests into a single kernel module. Use that module to run any of the
>> >>>>tests triggered by igt.
>> >>>>
>> >>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> >>>>---
>> >>>>tests/intel/xe_live_ktest.c | 15 +++++----------
>> >>>>1 file changed, 5 insertions(+), 10 deletions(-)
>> >>>>
>> >>>>diff --git a/tests/intel/xe_live_ktest.c b/tests/intel/xe_live_ktest.c
>> >>>>index fe7b2e69f..c8bd68928 100644
>> >>>>--- a/tests/intel/xe_live_ktest.c
>> >>>>+++ b/tests/intel/xe_live_ktest.c
>> >>>>@@ -19,15 +19,10 @@
>> >>>> * Functionality: migrate
>> >>>> */
>> >>>>
>> >>>>-struct kunit_tests {
>> >>>>-	const char *kunit;
>> >>>>-	const char *name;
>> >>>>-};
>> >>>>-
>> >>>>-static const struct kunit_tests live_tests[] = {
>> >>>>-	{ "xe_bo_test",		"bo" },
>> >>>>-	{ "xe_dma_buf_test",	"dmabuf" },
>> >>>>-	{ "xe_migrate_test",	"migrate" },
>> >>>>+static const char *live_tests[] = {
>> >>>>+	"bo",
>> >>>>+	"dmabuf",
>> >>>>+	"migrate",
>> >>>>};
>> >>>>
>> >>>>igt_main
>> >>>>@@ -35,5 +30,5 @@ igt_main
>> >>>>	int i;
>> >>>>
>> >>>>	for (i = 0; i < ARRAY_SIZE(live_tests); i++)
>> >>>>-		igt_kunit(live_tests[i].kunit, live_tests[i].name, NULL);
>> >>>>+		igt_kunit("xe_live_test", live_tests[i], NULL);
>> >>>
>> >>>This loop will modprobe xe_live_test the times. Just do something
>> >>>like:
>> >>>
>> >>>	igt_kunit("xe_live_test", "live_tests", NULL);
>> >>
>> >>but there's no such "live_tests" suite and we want to run each of
>> >>them individually, not together.  How would you select the subtest
>> >>otherwise?
>> >
>> >gentle ping as I want to merge the kernel patch that is blocked on this
>> >one.
>>
>> I did some tests today and... current situation is not very good. The
>> kunit module has the param filter_glob to filter the testsuite, but it's
>> not used at all by igt.  live_tests[i].name is more informational than
>> to really do any filtering.
>
>Yeah, that's what I would be expecting when I commented this patch.
>Just didn't have the time yet for doing the tests. Btw, before Janusz Kernel
>patches, filter_glob were actually completely ignored with modprobe. It used
>to work only at Kernel boot time and when compiled with Kunit modules builtin.
>In practice, it was originally designed to work with kunit.py.

great improvement and this builds on top. It works with kunit as builtin
or as a module.

>
>> Another issue I see is that since we have the dep xe.ko -> kunit.ko, as
>> soon as we load xe, kunit comes as a dependency. Since kunit's param are
>> read-only, we can't really change the testsuite that will run without
>> first remove xe. Having to reload xe multiple times doesn't seem a good
>> option.
>
>The dependency between xe and kunit can be removed, but will require some
>redesign, moving all the logic calling kunit kAPI symbols out of xe.ko,
>moving them to xe_live_test & friends. This will likely require using
>some namespaced export symbol logic, to let them to call functions that
>aren't part of kAPI. Doable, but not trivial.

That's not a goal and afaik never was. As we add complexity to the tests
it's counter productive to require everything to be inside the _test.ko,
which limits things we can do. It may also come as an indirect
dependency as we share helpers with drm.

>
>> Some manual tests I did here. First a kernel patch:
>>
>> 	diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
>> 	index 1236b3cd2fbb..30ed9d321c19 100644
>> 	--- a/lib/kunit/executor.c
>> 	+++ b/lib/kunit/executor.c
>> 	@@ -31,7 +31,7 @@ static char *filter_glob_param;
>> 	 static char *filter_param;
>> 	 static char *filter_action_param;
>>
>> 	-module_param_named(filter_glob, filter_glob_param, charp, 0400);
>> 	+module_param_named(filter_glob, filter_glob_param, charp, 0600);
>> 	 MODULE_PARM_DESC(filter_glob,
>> 			"Filter which KUnit test suites/tests run at boot-time, e.g. list* or list*.*del_test");
>> 	 module_param_named(filter, filter_param, charp, 0400);
>
>Such patch needs to be reviewed by Kunit maintainers. IMO, it only
>makes sense upstream if one could do something similar to:

already sent upstream

>
>	modprobe kunit filter_glob=none
>	modprobe -r xe_live_test
>	echo -n xe_bo > /sys/modules/kunit/parameters/filter_glob
>	<xe_bo tests executed>
>	echo -n xe_dmabuf > /sys/modules/kunit/parameters/filter_glob
>	<xe_dmabuf tests executed>

No, kunit tests execution is triggered by loading a module
(https://docs.kernel.org/dev-tools/kunit/run_manual.html

	( Once we have built our kernel (and/or modules), it is simple
	  to run the tests. If the tests are built-in, they will run
	  automatically on the kernel boot. The results will be written to
	  the kernel log (dmesg) in TAP format. )

and I kept that design. We don't really need to re-run it if we can simply
remove the *_test.ko. See https://lore.kernel.org/intel-xe/20240112001240.1710962-1-lucas.demarchi@intel.com/
and we make the _test.ko module trivial so it's more a
test trigger && check.

>
>e.g. if changing filter_glob would actually trigger tests, and, when
>kunit itself is loaded, the filter_glob parameter is set in a way that
>no tests will be executed.
>
>In practice, lib/kunit should use module_param_cb() to register a
>callback that will be used to <re->trigger the tests.

disagree as it's not how tests are normally triggered in kunit.

>> To simplify our kunit lib, we could also rely on kunit_debugfs to list
>> tests.
>
>On such case, you should document kunit_debugfs at Documentation/ABI.

debugfs is not part of ABI. What I'm saying is documented at

https://docs.kernel.org/dev-tools/kunit/running_tips.html#retrieving-per-suite-results

Lucas De Marchi

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Mauro Carvalho Chehab]

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

On 1/12/24 15:29, Lucas De Marchi wrote:
>> On such case, you should document kunit_debugfs at Documentation/ABI.
>
> debugfs is not part of ABI.

That's arguable. Check Documentation/ABI and you'll see several debugfs 
nodes documented there.

$ ./scripts/get_abi.pl rest|grep debugfs|wc -l
162

Regards,
Mauro


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

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Jani Nikula]

On Mon, 15 Jan 2024, Mauro Carvalho Chehab <mauro.chehab@linux.intel.com> wrote:
> On 1/12/24 15:29, Lucas De Marchi wrote:
>>> On such case, you should document kunit_debugfs at Documentation/ABI.
>>
>> debugfs is not part of ABI.
>
> That's arguable. Check Documentation/ABI and you'll see several debugfs 
> nodes documented there.

I guess you could argue documenting your debugfs under Documentation/ABI
makes it ABI, but I'd avoid doing that. Usually we use debugfs precisely
to avoid the burden of ABI and all the baggage that brings.


BR,
Jani.


-- 
Jani Nikula, Intel

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Mauro Carvalho Chehab]

On Mon, 15 Jan 2024 09:21:35 +0200
Jani Nikula <jani.nikula@linux.intel.com> wrote:

> On Mon, 15 Jan 2024, Mauro Carvalho Chehab <mauro.chehab@linux.intel.com> wrote:
> > On 1/12/24 15:29, Lucas De Marchi wrote:  
> >>> On such case, you should document kunit_debugfs at Documentation/ABI.  
> >>
> >> debugfs is not part of ABI.  
> >
> > That's arguable. Check Documentation/ABI and you'll see several debugfs 
> > nodes documented there.  
> 
> I guess you could argue documenting your debugfs under Documentation/ABI
> makes it ABI, but I'd avoid doing that. Usually we use debugfs precisely
> to avoid the burden of ABI and all the baggage that brings.

Placing things at debugfs "precisely to avoid the burden of ABI and all
the baggage that brings" sounds a bad practice to me, but you can ask 
others at LKML and see if this is a acceptable.

My understanding is that, if userspace tools rely on what's there at
debugfs, and a change break such tools, there is a high chance that
such change will end being reverted - at best, it will surely provide
some flame wars.

I remember we had some troubles related to trace logic in the past,
due to something similar: even not originally meant to provide a stable
ABI, and being under debugfs, there were some changes breaking userspace 
which caused lots of discussions. Can't remember anymore if the changes
were reverted or not, but at the end ftrace ABI was considered as stable.
I have an userspace program which depends on it (rasdaemon).

Also, having it documented or not doesn't mean it is not ABI. See, for
instance, this article: https://lwn.net/Articles/309298/.

Regards,
Mauro

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Lucas De Marchi]

On Mon, Jan 15, 2024 at 08:52:07PM +0100, Mauro Carvalho Chehab wrote:
>On Mon, 15 Jan 2024 09:21:35 +0200
>Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
>> On Mon, 15 Jan 2024, Mauro Carvalho Chehab <mauro.chehab@linux.intel.com> wrote:
>> > On 1/12/24 15:29, Lucas De Marchi wrote:
>> >>> On such case, you should document kunit_debugfs at Documentation/ABI.
>> >>
>> >> debugfs is not part of ABI.
>> >
>> > That's arguable. Check Documentation/ABI and you'll see several debugfs
>> > nodes documented there.
>>
>> I guess you could argue documenting your debugfs under Documentation/ABI
>> makes it ABI, but I'd avoid doing that. Usually we use debugfs precisely
>> to avoid the burden of ABI and all the baggage that brings.
>
>Placing things at debugfs "precisely to avoid the burden of ABI and all
>the baggage that brings" sounds a bad practice to me, but you can ask
>others at LKML and see if this is a acceptable.
>
>My understanding is that, if userspace tools rely on what's there at
>debugfs, and a change break such tools, there is a high chance that
>such change will end being reverted - at best, it will surely provide
>some flame wars.
>
>I remember we had some troubles related to trace logic in the past,
>due to something similar: even not originally meant to provide a stable
>ABI, and being under debugfs, there were some changes breaking userspace
>which caused lots of discussions. Can't remember anymore if the changes
>were reverted or not, but at the end ftrace ABI was considered as stable.

ftrace provides a way for you to read the format of the tracepoint and
use it (API). If you did it otherwise and hardcoded the format (ABI),
you are on your own as tracepoints in the kernel keep changing.
Try grepping "tracepoint" in the last pull requests.

>I have an userspace program which depends on it (rasdaemon).

if there's a real tool that needs the info, then it should _not_ be in
debugfs.  I'm not sure what how kunit handles that, maybe ask them as
this is getting very off topic here.

Lucas De Marchi

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Mauro Carvalho Chehab]

On Tue, 16 Jan 2024 08:50:51 -0600
Lucas De Marchi <lucas.demarchi@intel.com> wrote:

> On Mon, Jan 15, 2024 at 08:52:07PM +0100, Mauro Carvalho Chehab wrote:
> >On Mon, 15 Jan 2024 09:21:35 +0200
> >Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >  
> >> On Mon, 15 Jan 2024, Mauro Carvalho Chehab <mauro.chehab@linux.intel.com> wrote:  
> >> > On 1/12/24 15:29, Lucas De Marchi wrote:  
> >> >>> On such case, you should document kunit_debugfs at Documentation/ABI.  
> >> >>
> >> >> debugfs is not part of ABI.  
> >> >
> >> > That's arguable. Check Documentation/ABI and you'll see several debugfs
> >> > nodes documented there.  
> >>
> >> I guess you could argue documenting your debugfs under Documentation/ABI
> >> makes it ABI, but I'd avoid doing that. Usually we use debugfs precisely
> >> to avoid the burden of ABI and all the baggage that brings.  
> >
> >Placing things at debugfs "precisely to avoid the burden of ABI and all
> >the baggage that brings" sounds a bad practice to me, but you can ask
> >others at LKML and see if this is a acceptable.
> >
> >My understanding is that, if userspace tools rely on what's there at
> >debugfs, and a change break such tools, there is a high chance that
> >such change will end being reverted - at best, it will surely provide
> >some flame wars.
> >
> >I remember we had some troubles related to trace logic in the past,
> >due to something similar: even not originally meant to provide a stable
> >ABI, and being under debugfs, there were some changes breaking userspace
> >which caused lots of discussions. Can't remember anymore if the changes
> >were reverted or not, but at the end ftrace ABI was considered as stable.  
> 
> ftrace provides a way for you to read the format of the tracepoint and
> use it (API). If you did it otherwise and hardcoded the format (ABI),
> you are on your own as tracepoints in the kernel keep changing.
> Try grepping "tracepoint" in the last pull requests.

Not quite true. Since when the EDAC subsystem was redesigned back
in 2013, RAS events have been using ftrace API to report hardware
problems to userspace. We did the design of such API together with the
ftrace maintainer, that developed an userspace counterpart (libtraceevent),
designed to have a stable uABI. 

The rasdaemon application uses it since then, originally on a fork, as 
it took a while for libtraceevent to become a separate tree and started
being shipped in distributions. Only last year we finally migrated to use
the current incarnation of libtraceevent, mostly to cleanup rasdaemon code.

During all this time (10+ years), the API remained the same, and we had
only once an incident on such interface - not directly related to uABI,
but to some behavioral change that happened at the logic to flush
ftrace ringbuffer, causing troubles for rasdaemon.

Regards,
Mauro

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Janusz Krzysztofik]

On Friday, 12 January 2024 00:45:37 CET Lucas De Marchi wrote:
> On Thu, Dec 21, 2023 at 04:15:23PM -0600, Lucas De Marchi wrote:
> >On Fri, Dec 08, 2023 at 12:13:36PM -0600, Lucas De Marchi wrote:
> >>On Thu, Dec 07, 2023 at 04:41:52PM +0100, Mauro Carvalho Chehab wrote:
> >>>On Tue,  5 Dec 2023 14:49:26 -0800
> >>>Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> >>>
> >>>>https://patchwork.freedesktop.org/series/126786/ groups all the live
> >>>>tests into a single kernel module. Use that module to run any of the
> >>>>tests triggered by igt.
> >>>>
> >>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >>>>---
> >>>>tests/intel/xe_live_ktest.c | 15 +++++----------
> >>>>1 file changed, 5 insertions(+), 10 deletions(-)
> >>>>
> >>>>diff --git a/tests/intel/xe_live_ktest.c b/tests/intel/xe_live_ktest.c
> >>>>index fe7b2e69f..c8bd68928 100644
> >>>>--- a/tests/intel/xe_live_ktest.c
> >>>>+++ b/tests/intel/xe_live_ktest.c
> >>>>@@ -19,15 +19,10 @@
> >>>> * Functionality: migrate
> >>>> */
> >>>>
> >>>>-struct kunit_tests {
> >>>>-	const char *kunit;
> >>>>-	const char *name;
> >>>>-};
> >>>>-
> >>>>-static const struct kunit_tests live_tests[] = {
> >>>>-	{ "xe_bo_test",		"bo" },
> >>>>-	{ "xe_dma_buf_test",	"dmabuf" },
> >>>>-	{ "xe_migrate_test",	"migrate" },
> >>>>+static const char *live_tests[] = {
> >>>>+	"bo",
> >>>>+	"dmabuf",
> >>>>+	"migrate",
> >>>>};
> >>>>
> >>>>igt_main
> >>>>@@ -35,5 +30,5 @@ igt_main
> >>>>	int i;
> >>>>
> >>>>	for (i = 0; i < ARRAY_SIZE(live_tests); i++)
> >>>>-		igt_kunit(live_tests[i].kunit, live_tests[i].name, NULL);
> >>>>+		igt_kunit("xe_live_test", live_tests[i], NULL);
> >>>
> >>>This loop will modprobe xe_live_test the times. Just do something
> >>>like:
> >>>
> >>>	igt_kunit("xe_live_test", "live_tests", NULL);
> >>
> >>but there's no such "live_tests" suite and we want to run each of
> >>them individually, not together.  How would you select the subtest
> >>otherwise?
> >
> >gentle ping as I want to merge the kernel patch that is blocked on this
> >one.
> 
> I did some tests today and... current situation is not very good. The
> kunit module has the param filter_glob to filter the testsuite, but it's
> not used at all by igt.  

Please see https://patchwork.freedesktop.org/series/126017/.  It has been 
blocked by the below mentioned xe -> kunit module dependency problem, but now 
as I'm back from my 2-months leave, I'll update it and resubmit as soon as we 
have that resolved with writable filter* module parameters.

> live_tests[i].name is more informational than
> to really do any filtering.
> 
> Another issue I see is that since we have the dep xe.ko -> kunit.ko, as
> soon as we load xe, kunit comes as a dependency. Since kunit's param are
> read-only, we can't really change the testsuite that will run without
> first remove xe. Having to reload xe multiple times doesn't seem a good
> option.
> 
> Some manual tests I did here. First a kernel patch:
> 
> 	diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> 	index 1236b3cd2fbb..30ed9d321c19 100644
> 	--- a/lib/kunit/executor.c
> 	+++ b/lib/kunit/executor.c
> 	@@ -31,7 +31,7 @@ static char *filter_glob_param;
> 	 static char *filter_param;
> 	 static char *filter_action_param;
> 
> 	-module_param_named(filter_glob, filter_glob_param, charp, 0400);
> 	+module_param_named(filter_glob, filter_glob_param, charp, 0600);
> 	 MODULE_PARM_DESC(filter_glob,
> 			"Filter which KUnit test suites/tests run at 
boot-time, e.g. list* or list*.*del_test");
> 	 module_param_named(filter, filter_param, charp, 0400);
> 
> Now we can filter the testsuite at will:
> 
> 	# modprobe xe
> 	
> Run only xe_bo tests:
> 
> 	# echo -n xe_bo > /sys/modules/kunit/parameters/filter_glob
> 	# dmesg -C
> 	# modprobe xe_live_test
> 	# dmesg
> 	[ 4064.402190] KTAP version 1
> 	[ 4064.402195] 1..1
> 	[ 4064.403336]     KTAP version 1
> 	[ 4064.403339]     # Subtest: xe_bo
> 	[ 4064.403342]     # module: xe_live_test
> 	[ 4064.403410]     1..2
> 	...
> 	[ 4065.712622]     ok 2 xe_bo_evict_kunit
> 	[ 4065.712646] # xe_bo: pass:2 fail:0 skip:0 total:2
> 	[ 4065.712668] # Totals: pass:2 fail:0 skip:0 total:2
> 	[ 4065.712700] ok 1 xe_bo
> 
> Execute all live tests:
> 
> 	# modprobe -r xe_live_test
> 	# dmesg -C
> 	# echo -n \* > /sys/modules/kunit/parameters/filter_glob
> 	# modprobe xe_live_test
> 	# dmesg
> 	[ 5010.536719] KTAP version 1
> 	[ 5010.536725] 1..4
> 	[ 5010.537618]     KTAP version 1
> 	[ 5010.537620]     # Subtest: xe_bo
> 	[ 5010.537623]     # module: xe_live_test
> 	...
> 	[ 5011.831597]     KTAP version 1
> 	[ 5011.831599]     # Subtest: xe_dma_buf
> 	[ 5011.831601]     # module: xe_live_test
> 	...
> 	[ 5011.867751]     KTAP version 1
> 	[ 5011.867753]     # Subtest: xe_migrate
> 	[ 5011.867755]     # module: xe_live_test
> 	...
> 	[ 5011.882481]     KTAP version 1
> 	[ 5011.882483]     # Subtest: xe_mocs
> 	[ 5011.882485]     # module: xe_live_test
> 
> To simplify our kunit lib, we could also rely on kunit_debugfs to list
> tests.

That option also sits in my IGT queue, though it requires an updated Xe CI 
kconfig (or at least it did two months ago).

Thanks,
Janusz

> 
> I will prepare a patch for the kunit part in the kernel.
> 
> Lucas De Marchi
> 
> >
> >Lucas De Marchi
> >
> >>
> >>Lucas De Marchi
> >>
> >>>
> >>>>}
> >>>
> >>>Regards,
> >>>Mauro
> 

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Janusz Krzysztofik]

On Friday, 12 January 2024 00:45:37 CET Lucas De Marchi wrote:
> On Thu, Dec 21, 2023 at 04:15:23PM -0600, Lucas De Marchi wrote:
> >On Fri, Dec 08, 2023 at 12:13:36PM -0600, Lucas De Marchi wrote:
> >>On Thu, Dec 07, 2023 at 04:41:52PM +0100, Mauro Carvalho Chehab wrote:
> >>>On Tue,  5 Dec 2023 14:49:26 -0800
> >>>Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> >>>
> >>>>https://patchwork.freedesktop.org/series/126786/ groups all the live
> >>>>tests into a single kernel module. Use that module to run any of the
> >>>>tests triggered by igt.
> >>>>
> >>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> >>>>---
> >>>>tests/intel/xe_live_ktest.c | 15 +++++----------
> >>>>1 file changed, 5 insertions(+), 10 deletions(-)
> >>>>
> >>>>diff --git a/tests/intel/xe_live_ktest.c b/tests/intel/xe_live_ktest.c
> >>>>index fe7b2e69f..c8bd68928 100644
> >>>>--- a/tests/intel/xe_live_ktest.c
> >>>>+++ b/tests/intel/xe_live_ktest.c
> >>>>@@ -19,15 +19,10 @@
> >>>> * Functionality: migrate
> >>>> */
> >>>>
> >>>>-struct kunit_tests {
> >>>>-	const char *kunit;
> >>>>-	const char *name;
> >>>>-};
> >>>>-
> >>>>-static const struct kunit_tests live_tests[] = {
> >>>>-	{ "xe_bo_test",		"bo" },
> >>>>-	{ "xe_dma_buf_test",	"dmabuf" },
> >>>>-	{ "xe_migrate_test",	"migrate" },
> >>>>+static const char *live_tests[] = {
> >>>>+	"bo",
> >>>>+	"dmabuf",
> >>>>+	"migrate",
> >>>>};
> >>>>
> >>>>igt_main
> >>>>@@ -35,5 +30,5 @@ igt_main
> >>>>	int i;
> >>>>
> >>>>	for (i = 0; i < ARRAY_SIZE(live_tests); i++)
> >>>>-		igt_kunit(live_tests[i].kunit, live_tests[i].name, NULL);
> >>>>+		igt_kunit("xe_live_test", live_tests[i], NULL);
> >>>
> >>>This loop will modprobe xe_live_test the times. Just do something
> >>>like:
> >>>
> >>>	igt_kunit("xe_live_test", "live_tests", NULL);
> >>
> >>but there's no such "live_tests" suite and we want to run each of
> >>them individually, not together.  How would you select the subtest
> >>otherwise?
> >
> >gentle ping as I want to merge the kernel patch that is blocked on this
> >one.
> 
> I did some tests today and... current situation is not very good. The
> kunit module has the param filter_glob to filter the testsuite, but it's
> not used at all by igt.  

Please see https://patchwork.freedesktop.org/series/126017/.  It has been 
blocked by the below mentioned xe -> kunit module dependency problem, but now 
as I'm back from my 2-months leave, I'll update it and resubmit as soon as we 
have that resolved with writable filter* module parameters.

> live_tests[i].name is more informational than
> to really do any filtering.
> 
> Another issue I see is that since we have the dep xe.ko -> kunit.ko, as
> soon as we load xe, kunit comes as a dependency. Since kunit's param are
> read-only, we can't really change the testsuite that will run without
> first remove xe. Having to reload xe multiple times doesn't seem a good
> option.
> 
> Some manual tests I did here. First a kernel patch:
> 
> 	diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> 	index 1236b3cd2fbb..30ed9d321c19 100644
> 	--- a/lib/kunit/executor.c
> 	+++ b/lib/kunit/executor.c
> 	@@ -31,7 +31,7 @@ static char *filter_glob_param;
> 	 static char *filter_param;
> 	 static char *filter_action_param;
> 
> 	-module_param_named(filter_glob, filter_glob_param, charp, 0400);
> 	+module_param_named(filter_glob, filter_glob_param, charp, 0600);
> 	 MODULE_PARM_DESC(filter_glob,
> 			"Filter which KUnit test suites/tests run at 
boot-time, e.g. list* or list*.*del_test");
> 	 module_param_named(filter, filter_param, charp, 0400);
> 
> Now we can filter the testsuite at will:
> 
> 	# modprobe xe
> 	
> Run only xe_bo tests:
> 
> 	# echo -n xe_bo > /sys/modules/kunit/parameters/filter_glob
> 	# dmesg -C
> 	# modprobe xe_live_test
> 	# dmesg
> 	[ 4064.402190] KTAP version 1
> 	[ 4064.402195] 1..1
> 	[ 4064.403336]     KTAP version 1
> 	[ 4064.403339]     # Subtest: xe_bo
> 	[ 4064.403342]     # module: xe_live_test
> 	[ 4064.403410]     1..2
> 	...
> 	[ 4065.712622]     ok 2 xe_bo_evict_kunit
> 	[ 4065.712646] # xe_bo: pass:2 fail:0 skip:0 total:2
> 	[ 4065.712668] # Totals: pass:2 fail:0 skip:0 total:2
> 	[ 4065.712700] ok 1 xe_bo
> 
> Execute all live tests:
> 
> 	# modprobe -r xe_live_test
> 	# dmesg -C
> 	# echo -n \* > /sys/modules/kunit/parameters/filter_glob
> 	# modprobe xe_live_test
> 	# dmesg
> 	[ 5010.536719] KTAP version 1
> 	[ 5010.536725] 1..4
> 	[ 5010.537618]     KTAP version 1
> 	[ 5010.537620]     # Subtest: xe_bo
> 	[ 5010.537623]     # module: xe_live_test
> 	...
> 	[ 5011.831597]     KTAP version 1
> 	[ 5011.831599]     # Subtest: xe_dma_buf
> 	[ 5011.831601]     # module: xe_live_test
> 	...
> 	[ 5011.867751]     KTAP version 1
> 	[ 5011.867753]     # Subtest: xe_migrate
> 	[ 5011.867755]     # module: xe_live_test
> 	...
> 	[ 5011.882481]     KTAP version 1
> 	[ 5011.882483]     # Subtest: xe_mocs
> 	[ 5011.882485]     # module: xe_live_test
> 
> To simplify our kunit lib, we could also rely on kunit_debugfs to list
> tests.

That option also sits in my IGT queue, though it requires an updated Xe CI 
kconfig (or at least it did two months ago).

Thanks,
Janusz

> 
> I will prepare a patch for the kunit part in the kernel.
> 
> Lucas De Marchi
> 
> >
> >Lucas De Marchi
> >
> >>
> >>Lucas De Marchi
> >>
> >>>
> >>>>}
> >>>
> >>>Regards,
> >>>Mauro
> 

Re: Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Lucas De Marchi]

On Mon, Jan 22, 2024 at 12:35:28PM +0100, Janusz Krzysztofik wrote:
>On Friday, 12 January 2024 00:45:37 CET Lucas De Marchi wrote:
>> On Thu, Dec 21, 2023 at 04:15:23PM -0600, Lucas De Marchi wrote:
>> >On Fri, Dec 08, 2023 at 12:13:36PM -0600, Lucas De Marchi wrote:
>> >>On Thu, Dec 07, 2023 at 04:41:52PM +0100, Mauro Carvalho Chehab wrote:
>> >>>On Tue,  5 Dec 2023 14:49:26 -0800
>> >>>Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>> >>>
>> >>>>https://patchwork.freedesktop.org/series/126786/ groups all the live
>> >>>>tests into a single kernel module. Use that module to run any of the
>> >>>>tests triggered by igt.
>> >>>>
>> >>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> >>>>---
>> >>>>tests/intel/xe_live_ktest.c | 15 +++++----------
>> >>>>1 file changed, 5 insertions(+), 10 deletions(-)
>> >>>>
>> >>>>diff --git a/tests/intel/xe_live_ktest.c b/tests/intel/xe_live_ktest.c
>> >>>>index fe7b2e69f..c8bd68928 100644
>> >>>>--- a/tests/intel/xe_live_ktest.c
>> >>>>+++ b/tests/intel/xe_live_ktest.c
>> >>>>@@ -19,15 +19,10 @@
>> >>>> * Functionality: migrate
>> >>>> */
>> >>>>
>> >>>>-struct kunit_tests {
>> >>>>-	const char *kunit;
>> >>>>-	const char *name;
>> >>>>-};
>> >>>>-
>> >>>>-static const struct kunit_tests live_tests[] = {
>> >>>>-	{ "xe_bo_test",		"bo" },
>> >>>>-	{ "xe_dma_buf_test",	"dmabuf" },
>> >>>>-	{ "xe_migrate_test",	"migrate" },
>> >>>>+static const char *live_tests[] = {
>> >>>>+	"bo",
>> >>>>+	"dmabuf",
>> >>>>+	"migrate",
>> >>>>};
>> >>>>
>> >>>>igt_main
>> >>>>@@ -35,5 +30,5 @@ igt_main
>> >>>>	int i;
>> >>>>
>> >>>>	for (i = 0; i < ARRAY_SIZE(live_tests); i++)
>> >>>>-		igt_kunit(live_tests[i].kunit, live_tests[i].name, NULL);
>> >>>>+		igt_kunit("xe_live_test", live_tests[i], NULL);
>> >>>
>> >>>This loop will modprobe xe_live_test the times. Just do something
>> >>>like:
>> >>>
>> >>>	igt_kunit("xe_live_test", "live_tests", NULL);
>> >>
>> >>but there's no such "live_tests" suite and we want to run each of
>> >>them individually, not together.  How would you select the subtest
>> >>otherwise?
>> >
>> >gentle ping as I want to merge the kernel patch that is blocked on this
>> >one.
>>
>> I did some tests today and... current situation is not very good. The
>> kunit module has the param filter_glob to filter the testsuite, but it's
>> not used at all by igt.
>
>Please see https://patchwork.freedesktop.org/series/126017/.  It has been
>blocked by the below mentioned xe -> kunit module dependency problem, but now

I don't think we will remove that dep so should plan accordingly.  There
are some kunit features that are only possible by depending on some
kunit symbols. That was intentional.


>as I'm back from my 2-months leave, I'll update it and resubmit as soon as we
>have that resolved with writable filter* module parameters.

I will update that series.

>
>> live_tests[i].name is more informational than
>> to really do any filtering.
>>
>> Another issue I see is that since we have the dep xe.ko -> kunit.ko, as
>> soon as we load xe, kunit comes as a dependency. Since kunit's param are
>> read-only, we can't really change the testsuite that will run without
>> first remove xe. Having to reload xe multiple times doesn't seem a good
>> option.
>>
>> Some manual tests I did here. First a kernel patch:
>>
>> 	diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
>> 	index 1236b3cd2fbb..30ed9d321c19 100644
>> 	--- a/lib/kunit/executor.c
>> 	+++ b/lib/kunit/executor.c
>> 	@@ -31,7 +31,7 @@ static char *filter_glob_param;
>> 	 static char *filter_param;
>> 	 static char *filter_action_param;
>>
>> 	-module_param_named(filter_glob, filter_glob_param, charp, 0400);
>> 	+module_param_named(filter_glob, filter_glob_param, charp, 0600);
>> 	 MODULE_PARM_DESC(filter_glob,
>> 			"Filter which KUnit test suites/tests run at
>boot-time, e.g. list* or list*.*del_test");
>> 	 module_param_named(filter, filter_param, charp, 0400);
>>
>> Now we can filter the testsuite at will:
>>
>> 	# modprobe xe
>> 	
>> Run only xe_bo tests:
>>
>> 	# echo -n xe_bo > /sys/modules/kunit/parameters/filter_glob
>> 	# dmesg -C
>> 	# modprobe xe_live_test
>> 	# dmesg
>> 	[ 4064.402190] KTAP version 1
>> 	[ 4064.402195] 1..1
>> 	[ 4064.403336]     KTAP version 1
>> 	[ 4064.403339]     # Subtest: xe_bo
>> 	[ 4064.403342]     # module: xe_live_test
>> 	[ 4064.403410]     1..2
>> 	...
>> 	[ 4065.712622]     ok 2 xe_bo_evict_kunit
>> 	[ 4065.712646] # xe_bo: pass:2 fail:0 skip:0 total:2
>> 	[ 4065.712668] # Totals: pass:2 fail:0 skip:0 total:2
>> 	[ 4065.712700] ok 1 xe_bo
>>
>> Execute all live tests:
>>
>> 	# modprobe -r xe_live_test
>> 	# dmesg -C
>> 	# echo -n \* > /sys/modules/kunit/parameters/filter_glob
>> 	# modprobe xe_live_test
>> 	# dmesg
>> 	[ 5010.536719] KTAP version 1
>> 	[ 5010.536725] 1..4
>> 	[ 5010.537618]     KTAP version 1
>> 	[ 5010.537620]     # Subtest: xe_bo
>> 	[ 5010.537623]     # module: xe_live_test
>> 	...
>> 	[ 5011.831597]     KTAP version 1
>> 	[ 5011.831599]     # Subtest: xe_dma_buf
>> 	[ 5011.831601]     # module: xe_live_test
>> 	...
>> 	[ 5011.867751]     KTAP version 1
>> 	[ 5011.867753]     # Subtest: xe_migrate
>> 	[ 5011.867755]     # module: xe_live_test
>> 	...
>> 	[ 5011.882481]     KTAP version 1
>> 	[ 5011.882483]     # Subtest: xe_mocs
>> 	[ 5011.882485]     # module: xe_live_test
>>
>> To simplify our kunit lib, we could also rely on kunit_debugfs to list
>> tests.
>
>That option also sits in my IGT queue, though it requires an updated Xe CI
>kconfig (or at least it did two months ago).

an MR to https://gitlab.freedesktop.org/drm/xe/ci would take care of
that. Can you prep one?


Lucas De Marchi

Re: [igt-dev] [PATCH i-g-t] xe_live_ktest: Use xe_live_test kernel module

[Janusz Krzysztofik]

On Monday, 22 January 2024 16:37:17 CET Lucas De Marchi wrote:
> On Mon, Jan 22, 2024 at 12:35:28PM +0100, Janusz Krzysztofik wrote:
> >On Friday, 12 January 2024 00:45:37 CET Lucas De Marchi wrote:
...
> >> To simplify our kunit lib, we could also rely on kunit_debugfs to list
> >> tests.
> >
> >That option also sits in my IGT queue, though it requires an updated Xe CI
> >kconfig (or at least it did two months ago).
> 
> an MR to https://gitlab.freedesktop.org/drm/xe/ci would take care of
> that. Can you prep one?

Done, but a merge conflict has been reported, please let me know if I should 
do that another way.

Thanks,
Janusz

> 
> 
> Lucas De Marchi
>