All of 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: 6+ 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]

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.