Re: [PATCH i-g-t v5 13/17] tests/xe_eudebug: Test eudebug resource tracking and manipulation

["Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>]

On Thu, Aug 29, 2024 at 04:45:43PM +0200, Christoph Manszewski wrote:
> From: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
> 
> For typical debugging under gdb one can specify two main usecases:
> accessing and manupulating resources created by the application and
> manipulating thread execution (interrupting and setting breakpoints).
> 
> This test adds coverage for the former by checking that:
> - the debugger reports the expected events for Xe resources created
> by the debugged client,
> - the debugger is able to read and write the vm of the debugged client.
> 
> Signed-off-by: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Christoph Manszewski <christoph.manszewski@intel.com>
> Signed-off-by: Karolina Stolarek <karolina.stolarek@intel.com>
> Signed-off-by: Maciej Patelczyk <maciej.patelczyk@intel.com>
> Signed-off-by: Pawel Sikora <pawel.sikora@intel.com>
> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> ---
>  docs/testplan/meson.build |   20 +-
>  meson_options.txt         |    2 +-
>  tests/intel/xe_eudebug.c  | 2715 +++++++++++++++++++++++++++++++++++++
>  tests/meson.build         |    6 +
>  4 files changed, 2741 insertions(+), 2 deletions(-)
>  create mode 100644 tests/intel/xe_eudebug.c
> 
> diff --git a/docs/testplan/meson.build b/docs/testplan/meson.build
> index 5560347f1..3898a994a 100644
> --- a/docs/testplan/meson.build
> +++ b/docs/testplan/meson.build
> @@ -33,11 +33,29 @@ else
>  	doc_dependencies = []
>  endif
>  
> +xe_excluded_tests = []
> +
> +if not build_xe_eudebug
> +	xe_excluded_tests += [
> +		'xe_eudebug.c',
> +	]
> +endif
> +
> +tmp = []
> +foreach test : xe_excluded_tests
> +	tmp += meson.current_source_dir() + '/../../tests/intel/' + test
> +endforeach
> +xe_excluded_tests = tmp
> +
> +if xe_excluded_tests.length() > 0
> +	xe_excluded_tests = ['--exclude-files'] + xe_excluded_tests
> +endif

Here I'm going to review only meson related part.

According to our offline discussion and experiments please use common
list defined in tests/meson.build and used conditionally regarding
xe_eudebug value. This way we may avoid defining test names twice.

> +
>  if build_xe
>  	test_dict = {
>  		'i915_tests': { 'input': i915_test_config, 'extra_args': check_testlist },
>  		'kms_tests': { 'input': kms_test_config, 'extra_args': kms_check_testlist },
> -		'xe_tests': { 'input': xe_test_config, 'extra_args': check_testlist }
> +		'xe_tests': { 'input': xe_test_config, 'extra_args': check_testlist + xe_excluded_tests }
>  	    }
>  else
>  	test_dict = {
> diff --git a/meson_options.txt b/meson_options.txt
> index 11922523b..c410f9b77 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -45,7 +45,7 @@ option('xe_driver',
>  option('xe_eudebug',
>         type : 'feature',
>         value : 'disabled',
> -       description : 'Build library for Xe EU debugger')
> +       description : 'Build library and tests for Xe EU debugger')

<cut>

> diff --git a/tests/meson.build b/tests/meson.build
> index 00556c9d6..02d91567c 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -318,6 +318,12 @@ intel_xe_progs = [
>  	'xe_sysfs_scheduler',
>  ]
>  
> +if build_xe_eudebug
> +	intel_xe_progs += [
> +		'xe_eudebug',
> +	]
> +endif
> +

Here I mean you should define xe_eudebug_progs = [] and conditionally
add tests. This array may be used then in testplan meson.build.

>  chamelium_progs = [
>  	'kms_chamelium_audio',
>  	'kms_chamelium_color',
> -- 
> 2.34.1
>

Tests part I'm going to address in separate review.

--
Zbigniew