[Intel-xe] [PATCH 1/5] drm/xe: Introduce new selftests framework
Lucas De Marchi
lucas.demarchi at intel.com
Fri Mar 17 16:26:15 UTC 2023
On Fri, Mar 17, 2023 at 03:29:36PM +0100, Mauro Carvalho Chehab wrote:
>On Fri, 17 Mar 2023 08:20:46 +0100
>Thomas Hellström <thomas.hellstrom at linux.intel.com> wrote:
>
>> Hi,
>>
>> On 3/16/23 07:46, Lucas De Marchi wrote:
>> > On Mon, Mar 13, 2023 at 07:18:34PM +0100, Michal Wajdeczko wrote:
>> >>
>> >>
>> >> On 13.03.2023 17:23, Lucas De Marchi wrote:
>> >>> On Mon, Mar 13, 2023 at 09:13:12AM +0100, Michal Wajdeczko wrote:
>> >>>> Our initial attempt to run Kunit based tests on live devices is hard
>> >>>> to use and will not work well with multiple devices attached.
>> >>>>
>> >>>> Instead of relying on separate test module and duplicating support
>> >>>> for multiple live devices in every test entry point, create new set
>> >>>> of helpers and use test->priv to pass reference of the device under
>> >>>> test (DUT) directly to every test function.
>> >>>>
>> >>>> The same use pattern is supported for both mock and live selftests.
>> >>>>
>> >>>> By using bus_notifier we can start testing each new bound device
>> >>>> separatelly, subject to filtering using dut_filter modparam.
>> >>>>
>> >>>> All test cases can be included directly from .c file, allowing test
>> >>>> code to access and use any private functions.
>> >>>>
>> >>>> As Kunit will attempt to run all suites on module load, including
>> >>>> our new live test suites, we may want to allow to execute them on
>> >>>> any available device. This is controlled by anonymous_dut modparam.
>> >>>>
>> >>>> Support for either mock or live selftests, only one at time, is
>> >>>> controlled by new Kconfig options.
>> >>>>
>> >>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> >>>> ---
>> >>>> drivers/gpu/drm/xe/Kconfig.debug | 22 ++++
>> >>>> drivers/gpu/drm/xe/Makefile | 11 ++
>> >>>> drivers/gpu/drm/xe/selftests/xe_selftests.h | 35 ++++++
>> >>>> .../gpu/drm/xe/selftests/xe_selftests_auto.c | 46 +++++++
>> >>>> .../drm/xe/selftests/xe_selftests_executor.c | 118 ++++++++++++++++++
>> >>>> .../drm/xe/selftests/xe_selftests_helpers.c | 101 +++++++++++++++
>> >>>> .../drm/xe/selftests/xe_selftests_internal.h | 22 ++++
>> >>>> .../gpu/drm/xe/selftests/xe_selftests_mock.c | 32 +++++
>> >>>> .../drm/xe/selftests/xe_selftests_params.c | 23 ++++
>> >>>> drivers/gpu/drm/xe/xe_module.c | 2 +
>> >>>> 10 files changed, 412 insertions(+)
>> >>>> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests.h
>> >>>> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_auto.c
>> >>>> create mode 100644
>> >>>> drivers/gpu/drm/xe/selftests/xe_selftests_executor.c
>> >>>> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_helpers.c
>> >>>> create mode 100644
>> >>>> drivers/gpu/drm/xe/selftests/xe_selftests_internal.h
>> >>>> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_mock.c
>> >>>> create mode 100644 drivers/gpu/drm/xe/selftests/xe_selftests_params.c
>> >>>>
>> >>>> diff --git a/drivers/gpu/drm/xe/Kconfig.debug
>> >>>> b/drivers/gpu/drm/xe/Kconfig.debug
>> >>>> index 93b284cdd0a2..aeba43977dca 100644
>> >>>> --- a/drivers/gpu/drm/xe/Kconfig.debug
>> >>>> +++ b/drivers/gpu/drm/xe/Kconfig.debug
>> >>>> @@ -61,6 +61,28 @@ config DRM_XE_SIMPLE_ERROR_CAPTURE
>> >>>>
>> >>>> If in doubt, say "N".
>> >>>>
>> >>>> +config DRM_XE_SELFTESTS_MOCK
>> >>>> + bool "Mock selftests for Xe" if !KUNIT_ALL_TESTS
>> >>>> + depends on DRM_XE && KUNIT
>> >>>> + default KUNIT_ALL_TESTS
>> >>>> + help
>> >>>> + Choose this option to allow the driver to perform mock
>> >>>> + selftests based on the Kunit framework.
>> >>>> +
>> >>>> + Recommended for driver developers only.
>> >>>> + If in doubt, say "N".
>> >>>> +
>> >>>> +config DRM_XE_SELFTESTS_LIVE
>> >>>> + bool "Live selftests for Xe" if !DRM_XE_SELFTESTS_MOCK
>> >>>
>> >>> why? I think we should aim to run live tests in *addition* to
>> >>> mock ones. The mock tests are more testing our SW infra and can be
>> >>> used
>> >>> in a true "unit test" sense. While the live ones are testing the
>> >>> integration with the HW and making sure they work together.
>> >>
>> >> I've added that just to simplify the code logic as I assumed we will be
>> >> running mock tests only in "true kunit" fashion as part of the build.
>> >>
>> >> So then we could avoid mixing mock/live "kunit suites" in single driver
>> >> module, especially that as of today there is no nice support to filter
>> >> out undesired suites from the execution. Thus adding compile time flag
>> >> to only have one category of selftests at time was looking promising.
>> >
>> > I'm using this in the patches I'm sending to unit test rtp and wa parts:
>> >
>> > ./tools/testing/kunit/kunit.py run --raw_output \
>> > --kunitconfig drivers/gpu/drm/xe/.kunitconfig xe_rtp
>> >
>> >>
>> >> So I was using .kunitconfig with
>> >> CONFIG_DRM_XE_SELFTESTS_MOCK=y
>> >> and normal .config with
>> >> CONFIG_DRM_XE_SELFTESTS_LIVE=y
>> >>
>> >>>
>> >>> I have changes pending to test some WA and RTP infra we added,
>> >>> regardless of the HW. But I don't want to have to choose either one or
>> >>> the other.
>> >>
>> >> And I guess you still can do that, just it wont be automatically
>> >> compiled out, nor you be able to benefit from the helpers, but they will
>> >> be executed
>> >>
>> >>>
>> >>> Also Cc Thomas as original author of the kunit integration.
>> >>>
>> >>> We also have some overloaded terms that need better explanation:
>> >>> mock, selftest, live.
>> >>
>> >> I reused terms from i915, assumed everyone is familiar with that.
>> >>
>> >> "mock selftest" - a kunit test suite to be run on abstract xe device
>> >> "live selftest" - a kunit test suite to be executed on real xe device
>> >
>> > Things I like on the current kunit integration we have in xe:
>> >
>> > 1) The ability to move unit test code out to a separate module
>> > 2) Not having a big infra that is xe-only
>> > 3) Share terminology with the rest of drm subsystem
>> >
>> > This patch series is basically killing (1) and (3). And for (2) it seems
>> > to be heading another direction. Looking at the current available
>> > drivers, I think we should be more like the drm tests and the vc4 tests
>> > rather than the i915 ones. If we need more helpers we may probably
>> > extend the ones in DRM_KUNIT_TEST_HELPERS
>> >
>> > I'd love to hear others' opinions though.
>>
>> I basically agree with Lucas here. Some points:
>>
>> * For mock selftests IMO we don't need anything on top of what we
>> already have, except perhaps consolidate initialization of a mock
>> device as a helper each test can optionally use. I think, though,
>> that Mauro's suggestion of coalescing multiple mock tests together
>> in a single separate kernel module with multiple test suites to
>> avoid too many separate kernel module might be worth considering if
>> needed.
>
>IMO there's one important feature here that it is missing upstream:
>support for filtering out KUnit tests.
>
>Yet, the patch is adding it inside Xe. The right solution is, IMO, to
>add support as much as possible inside the KUnit core and having
>additional variables inside the module(s) implementing KUnit to setup
>such filters.
>
>This is more important for live selftests, but having this also for
>mock won't hurt
I'm always filtering the tests I run with:
./tools/testing/kunit/kunit.py run \
--kunitconfig drivers/gpu/drm/xe/.kunitconfig \
xe_rtp
so it only runs the xe_rtp tests I added. What am I missing?
Missing the filter for the device, I'd agree we'd need to add, but I
don't understand what support for filtering kunit tests you 2 are
talking about. From the script's help:
positional arguments:
filter_glob Filter which KUnit test suites/tests run
at boot-time, e.g. list* or list*.*del_test
there is also this that might be useful:
--run_isolated {suite,test} If set, boot the kernel for each
individual suite/test. This is can be useful for debugging a
on-hermetic test, one that might pass/fail based on what ran before
it.
... particularly if we integrate running the kunit tests, even for
"live" tests under qemu. That has a higher setup cost, but could save
us some headache with tests crashing the kernel.
linux-modules recently started using kdevops
(https://github.com/mcgrof/kdevops) for testing. I didn't have time yet
to give it a deeper look yet, but it's something we could think about.
Adding Luis here. Maybe he has more input on this. Or a demo recorded he
can point to.
thanks
Lucas De Marchi
>
>> * For live selftests it seems like where the big disconnect is. I
>> still believe that we should have separate modules, and depending on
>> how the IGT runner end up working have enough information in that
>> module for IGT to execute the tests with the --run-subtest and
>> --dynamic-subtest options.
>
>KUnit patch series is currently in rev. 5. It sounds almost ready for
>being merged. In any case, to properly support dynamic subtests, we
>need filtering capabilities. Once the filters get added, having multiple
>test suites on the same module or on different modules won't make any
>difference for IGT.
>
>Regards,
>Mauro
More information about the Intel-xe
mailing list