From: "Piotr Piórkowski" <piotr.piorkowski@intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-xe@lists.freedesktop.org,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Maarten Lankhorst" <dev@lankhorst.se>
Subject: Re: [PATCH v7 5/5] drm/xe/ggtt: Add KUnit tests for usable and shareable pools
Date: Mon, 9 Nov 2026 09:54:16 +0100 [thread overview]
Message-ID: <20261109085416.4bf46msdbavgb3dl@intel.com> (raw)
In-Reply-To: <20261029121233.pkbpd6sbbjwdemh6@intel.com>
Piotr Piórkowski <piotr.piorkowski@intel.com> wrote on czw [2026-paź-29 13:12:33 +0100]:
> Jani Nikula <jani.nikula@linux.intel.com> wrote on wto [2026-paź-27 18:31:53 +0200]:
> > On Wed, 21 Oct 2026, Piórkowski, Piotr <piotr.piorkowski@intel.com> wrote:
> > > diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> > > index b7e3d626e8cb8..3dd029bc76dfc 100644
> > > --- a/drivers/gpu/drm/xe/xe_ggtt.c
> > > +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> > > @@ -1395,3 +1395,7 @@ u64 xe_ggtt_node_size(const struct xe_ggtt_node *node)
> > > {
> > > return node->base.size;
> > > }
> > > +
> > > +#if IS_BUILTIN(CONFIG_DRM_XE_KUNIT_TEST)
> > > +#include "tests/xe_ggtt_kunit.c"
> > > +#endif
> >
> > Why IS_BUILTIN()?
> >
> > It smells like a mistake.
> >
> >
> > BR,
> > Jani.
> Hello Jani,
>
> I simply used a pattern that had already been used earlier in the other files — to be honest,
> I didn't give it any further thought.
>
> List of files with #if IS_BUILTIN(CONFIG_DRM_XE_KUNIT_TEST)
> xe_lmtt.c
> xe_guc_db_mgr.c
> xe_guc_relay.c
> xe_guc_id_mgr.c
> xe_sriov_pf_service.c
> xe_guc_buf.c
> xe_gt_sriov_pf_config.c
> xe_guc_klv_helpers.c
> xe_sriov_packet.c
> xe_log.c
>
> If we have to use IS_ENABLED In that case, I guess we'll have to fix that
> in the other places as well.
>
> Thanks,
> Piotr
Hi Jani,
Sorry, my previous answer was too short. I looked into it more closely,
and I believe IS_BUILTIN() is the right choice for this kind of test.
xe_ggtt_kunit.c calls kunit_test_suites() directly. Because the file is
#included into xe_ggtt.c, that puts the suite array into the
.kunit_test_suites section of xe.ko itself. The module loader picks up
that section for every module, and the KUnit module notifier runs those
suites on MODULE_STATE_LIVE. In other words, they run on module load,
as documented in include/kunit/test.h.
So with CONFIG_DRM_XE=m and CONFIG_DRM_XE_KUNIT_TEST=m:
IS_ENABLED(): every "modprobe xe", even on real hardware, would also
run the xe_ggtt/xe_ggtt_iov suites and taint the kernel with
TAINT_TEST.
IS_BUILTIN(): these tests are only built with =y, which is the
configuration used by kunit.py with drivers/gpu/drm/xe/.kunitconfig.
With =y, both variants behave the same.
The xe files that use IS_ENABLED() (xe_bo.c, xe_mocs.c, xe_migrate.c,
...) follow a different pattern. Their included test file does not call
kunit_test_suites(). It only exports the suite with
EXPORT_SYMBOL_IF_KUNIT(), and the separate xe_live_test.ko module
registers it. Nothing runs on xe.ko load, so =m is safe there.
So IIUC the rule is:
kunit_test_suites() in a file #included into xe.ko -> IS_BUILTIN()
suite exported and registered from a separate test module -> IS_ENABLED()
The other files I listed (xe_lmtt.c, xe_guc_id_mgr.c, xe_guc_relay.c,
...) follow the first rule.
Thanks,
Piotr
> >
> >
> > --
> > Jani Nikula, Intel
>
> --
--
next prev parent reply other threads:[~2026-11-09 8:54 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-21 8:57 [PATCH v7 0/5] Separate GGTT pools for submissions and VFs provisioning Piórkowski, Piotr
2026-10-21 8:57 ` [PATCH v7 1/5] drm/xe/ggtt: Assert lower bound of GGTT PTE address Piórkowski, Piotr
2026-10-28 15:50 ` Lis, Tomasz
2026-10-21 8:57 ` [PATCH v7 2/5] drm/xe/ggtt: Introduce full GGTT range size Piórkowski, Piotr
2026-10-28 15:56 ` Lis, Tomasz
2026-10-21 8:57 ` [PATCH v7 3/5] drm/xe/ggtt: Split GGTT into usable and shareable pools Piórkowski, Piotr
2026-10-28 23:59 ` Lis, Tomasz
2026-10-21 8:57 ` [PATCH v7 4/5] drm/xe/ggtt: Initialize GGTT pools by SR-IOV mode Piórkowski, Piotr
2026-10-28 15:23 ` Lis, Tomasz
2026-10-21 8:57 ` [PATCH v7 5/5] drm/xe/ggtt: Add KUnit tests for usable and shareable pools Piórkowski, Piotr
2026-10-27 16:31 ` Jani Nikula
2026-10-29 12:12 ` Piotr Piórkowski
2026-11-09 8:54 ` Piotr Piórkowski [this message]
2026-11-10 15:31 ` [v7,5/5] " Laguna, Lukasz
2026-11-12 11:00 ` Piotr Piórkowski
2026-10-21 9:04 ` ✗ CI.checkpatch: warning for Separate GGTT pools for submissions and VFs provisioning (rev7) Patchwork
2026-10-21 9:06 ` ✓ CI.KUnit: success " Patchwork
2026-10-21 9:44 ` ✓ Xe.CI.BAT: " Patchwork
2026-10-21 12:03 ` ✗ Xe.CI.FULL: failure " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261109085416.4bf46msdbavgb3dl@intel.com \
--to=piotr.piorkowski@intel.com \
--cc=dev@lankhorst.se \
--cc=intel-xe@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox