Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
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
> 
> -- 

-- 

  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