Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Shekhar Chauhan <shekhar.chauhan@intel.com>
To: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>,
	<igt-dev@lists.freedesktop.org>, <harish.chegondi@intel.com>
Cc: <riana.tauro@intel.com>
Subject: Re: [PATCH] tests/intel/xe_oa: Add unbind/rebind test with an open OA stream
Date: Fri, 30 Oct 2026 13:00:37 +0530	[thread overview]
Message-ID: <90bc9be0-75bf-4cd7-8095-e6c5c7f97c3a@intel.com> (raw)
In-Reply-To: <20261026233036.1589204-2-umesh.nerlige.ramappa@intel.com>


On 10/27/2026 5:00, Umesh Nerlige Ramappa wrote:
> Open an OA stream, unbind and rebind the xe device, and log the state
> of the stream fd (poll/read/ioctls) after removal and after probe.
> Close stream either before or after the rebind.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>

The test, in itself seems fine to me but there are certain suggestions:
     1. After close/rebind of the GPU, we poke/check/probe the IOCTLs 
from probe_stream_after_removal() but we never defined what the expected 
behaviour is.  I mean, how do we know that this is what is supposed to 
be the case, at the end of this subtests execution.  Can we have some 
asserts?  Although, since this is a new test being added, I do not know 
that the kernel is even addressing something like this or not, like, how 
should the XeKMD respond when we re-poke/probe a stream after it's 
device is gone.

     2. Because we have a random decide between close and open in 
main(), I feel not everything will be tested everytime CI runs this.  
Maybe we can have both of them run definitely.

     3. (AI-Suggestion) If we unbind the device, we did not update the 
primary set-up of the device for this test in igt_main(). Right now, 
it's the last subtest, so no one uses the GPU after this, but tomorrow, 
if a new test is added after this test, then that new subtest might fail 
without cause.

-shekhar

> ---
>   tests/intel/xe_oa.c | 127 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 127 insertions(+)
>
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index b9f62c1154fe..8a56efa43dfd 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -22,6 +22,7 @@
>   #include "drm.h"
>   #include "igt.h"
>   #include "igt_device.h"
> +#include "igt_pci.h"
>   #include "igt_syncobj.h"
>   #include "igt_sysfs.h"
>   #include "intel_wa.h"
> @@ -2725,6 +2726,125 @@ test_enable_disable(const struct drm_xe_oa_unit *oau)
>   	__perf_close(stream_fd);
>   }
>   
> +static void probe_stream_after_removal(const char *when)
> +{
> +	struct drm_xe_oa_stream_info info = {};
> +	uint8_t buf[4096];
> +	struct pollfd pfd = { .fd = stream_fd, .events = POLLIN };
> +	int ret;
> +
> +	ret = poll(&pfd, 1, 1000);
> +	igt_info("[%s] poll: ret=%d revents=0x%x errno=%s\n", when, ret, pfd.revents,
> +		 ret < 0 ? strerror(errno) : "-");
> +
> +	errno = 0;
> +	ret = read(stream_fd, buf, sizeof(buf));
> +	igt_info("[%s] read: ret=%d errno=%s\n", when, ret, ret < 0 ? strerror(errno) : "-");
> +
> +	errno = 0;
> +	ret = igt_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_INFO, &info);
> +	igt_info("[%s] ioctl INFO: ret=%d errno=%s oa_buf_size=%" PRIu64 "\n", when, ret,
> +		 ret < 0 ? strerror(errno) : "-", (uint64_t)info.oa_buf_size);
> +
> +	errno = 0;
> +	ret = igt_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_DISABLE, 0);
> +	igt_info("[%s] ioctl DISABLE: ret=%d errno=%s\n", when, ret,
> +		 ret < 0 ? strerror(errno) : "-");
> +
> +	errno = 0;
> +	ret = igt_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0);
> +	igt_info("[%s] ioctl ENABLE: ret=%d errno=%s\n", when, ret,
> +		 ret < 0 ? strerror(errno) : "-");
> +}
> +
> +/**
> + * SUBTEST: unbind-rebind-stream
> + * Description: Open an OA stream and test the effects of unbind and rebind
> + */
> +static void
> +test_unbind_rebind(const struct drm_xe_oa_unit *oau, bool close_before_rebind)
> +{
> +	struct intel_xe_perf_metric_set *test_set = oa_unit_metric_set(oau);
> +	uint64_t fmt = test_set->perf_oa_format;
> +	uint64_t properties[] = {
> +		DRM_XE_OA_PROPERTY_OA_UNIT_ID, oau->oa_unit_id,
> +		DRM_XE_OA_PROPERTY_SAMPLE_OA, true,
> +		DRM_XE_OA_PROPERTY_OA_METRIC_SET, test_set->perf_oa_metrics_set,
> +		DRM_XE_OA_PROPERTY_OA_FORMAT, __ff(fmt),
> +		DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT, oa_exponent_default,
> +	};
> +	struct intel_xe_oa_open_prop param = {
> +		.num_properties = ARRAY_SIZE(properties) / 2,
> +		.properties_ptr = to_user_pointer(properties),
> +	};
> +	size_t format_size = get_oa_format(fmt).size;
> +	uint8_t buf[format_size * 4];
> +	char pci_slot[NAME_MAX];
> +	int ret;
> +
> +	igt_device_get_pci_slot_name(drm_fd, pci_slot);
> +	igt_info("Device %s\n", pci_slot);
> +
> +	stream_fd = __perf_open(drm_fd, &param, false);
> +	set_fd_flags(stream_fd, O_CLOEXEC | O_NONBLOCK);
> +	do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_ENABLE, 0);
> +
> +	/* Make sure the stream is live before pulling the device */
> +	{
> +		struct pollfd pfd = { .fd = stream_fd, .events = POLLIN };
> +
> +		igt_assert_lt(0, poll(&pfd, 1, 2000));
> +	}
> +	ret = read(stream_fd, buf, sizeof(buf));
> +	igt_info("Pre-unbind read: ret=%d\n", ret);
> +	igt_assert_lt(0, ret);
> +
> +	/* Guard against a wedged unbind/bind */
> +	igt_set_timeout(60, "xe unbind timed out (wedged?)");
> +	igt_info("Unbinding xe from %s with stream open\n", pci_slot);
> +	ret = igt_pci_driver_unbind("xe", pci_slot);
> +	igt_reset_timeout();
> +	igt_assert_f(ret == 0, "xe unbind failed: %s\n", strerror(-ret));
> +	igt_info("Unbind complete\n");
> +
> +	probe_stream_after_removal("after-unbind");
> +
> +	if (close_before_rebind) {
> +		igt_info("Closing stream fd while device is unbound\n");
> +		igt_set_timeout(60, "stream close timed out (wedged?)");
> +		__perf_close(stream_fd);
> +		igt_reset_timeout();
> +		igt_info("Stream close complete\n");
> +	}
> +
> +	/* The drm fd also refers to the removed device */
> +	xe_device_put(drm_fd);
> +	drm_close_driver(drm_fd);
> +	drm_fd = -1;
> +
> +	igt_set_timeout(60, "xe bind timed out (wedged?)");
> +	igt_info("Rebinding xe to %s\n", pci_slot);
> +	ret = igt_pci_driver_bind("xe", pci_slot);
> +	igt_reset_timeout();
> +	igt_assert_f(ret == 0, "xe bind failed: %s\n", strerror(-ret));
> +	igt_info("Bind complete\n");
> +
> +	if (!close_before_rebind) {
> +		probe_stream_after_removal("after-rebind");
> +
> +		igt_info("Closing stream fd after rebind\n");
> +		igt_set_timeout(60, "stream close timed out (wedged?)");
> +		__perf_close(stream_fd);
> +		igt_reset_timeout();
> +		igt_info("Stream close complete\n");
> +	}
> +
> +	/* Restore state for the remaining subtests/fixtures */
> +	drm_fd = drm_open_driver(DRIVER_XE);
> +	xe_device_get(drm_fd);
> +	igt_info("Reopened xe device after rebind\n");
> +}
> +
>   /**
>    * SUBTEST: short-reads
>    * Description: Test behavior for short reads
> @@ -5221,6 +5341,13 @@ int igt_main_args("b:td", long_options, help_str, opt_handler, NULL)
>   		}
>   	}
>   
> +	igt_subtest_with_dynamic("unbind-rebind-stream") {
> +		bool close_before_rebind = random() & 0x1;
> +
> +		igt_dynamic_f("%s", close_before_rebind ? "close" : "open")
> +			test_unbind_rebind(oau0, close_before_rebind);
> +	}
> +
>   	igt_fixture() {
>   		/* leave sysctl options in their default state... */
>   		write_u64_file("/proc/sys/dev/xe/observation_paranoid", 1);

-- 
Shekhar Chauhan
Linux Graphics Software Engineer
Intel Corporation


  parent reply	other threads:[~2026-10-30  7:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-26 23:30 [PATCH] tests/intel/xe_oa: Add unbind/rebind test with an open OA stream Umesh Nerlige Ramappa
2026-10-27  0:31 ` ✓ Xe.CI.BAT: success for " Patchwork
2026-10-27  0:45 ` ✓ i915.CI.BAT: " Patchwork
2026-10-27  6:26 ` ✗ i915.CI.Full: failure " Patchwork
2026-10-27  7:04 ` ✗ Xe.CI.FULL: " Patchwork
2026-10-30  7:30 ` Shekhar Chauhan [this message]
2026-10-30 17:56   ` [PATCH] " Umesh Nerlige Ramappa

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=90bc9be0-75bf-4cd7-8095-e6c5c7f97c3a@intel.com \
    --to=shekhar.chauhan@intel.com \
    --cc=harish.chegondi@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=riana.tauro@intel.com \
    --cc=umesh.nerlige.ramappa@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