[PATCH i-g-t] tests/core_hotunplug: Add exec-no-cleanup-hotunbind-rebind subtest

[Janusz Krzysztofik]

Hi Marcin,

On Thursday, 8 May 2025 11:51:04 CEST Marcin Bernatowicz wrote:
> Add a new subtest to verify driver behavior when unbinding an open
> device with active user handles and no explicit exec cleanup. The test
> opens a DRM device, submits a basic workload (currently Xe driver only),
> and unbinds the device while handles are still open. After cleanup, the
> device is rebound and checked for health.
> 
> This validates robustness of the driver across unbind/rebind cycles
> under realistic open-handle conditions.
> 
> Cc: Matthew Brost <matthew.brost at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Lukasz Laguna <lukasz.laguna at intel.com>
> Cc: Stuart Summers <stuart.summers at intel.com>
> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>
> ---
>  tests/core_hotunplug.c | 70 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 18e9dfc34..5d25602a4 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -39,6 +39,10 @@
>  #include "igt_kmod.h"
>  #include "igt_sysfs.h"
>  #include "sw_sync.h"
> +#include "xe_drm.h"
> +#include "xe/xe_ioctl.h"
> +#include "xe/xe_spin.h"
> +
>  /**
>   * TEST: core hotunplug
>   * Description: Examine behavior of a driver on device hot unplug
> @@ -69,6 +73,10 @@
>   * Description: Check if the driver can be cleanly unbound from an open device,
>   *		then released and rebound
>   *
> + * SUBTEST: exec-no-cleanup-hotunbind-rebind
> + * Description: Check if the driver can be cleanly unbound from an open device
> + *		with open handles, then released and rebound
> + *
>   * SUBTEST: hotunplug-rescan
>   * Description: Check if an open device can be cleanly unplugged, then released
>   *		and restored
> @@ -596,6 +604,56 @@ static void hotunbind_rebind(struct hotunplug *priv)
>  	igt_assert_f(healthcheck(priv, false), "%s\n", priv->failure);
>  }
>  
> +#define FLAG_SKIP_EXEC_CLEANUP 1
> +
> +static void
> +simple_xe_exec(int fd, struct drm_xe_engine_class_instance *eci, unsigned int test_flags)
> +{
> +	uint32_t vm;
> +	struct xe_cork *cork;
> +
> +	vm = xe_vm_create(fd, 0, 0);
> +	cork = xe_cork_create_opts(fd, eci, vm, 1, 1);
> +	xe_cork_sync_start(fd, cork);
> +	usleep(1000);
> +	xe_cork_sync_end(fd, cork);
> +
> +	if (test_flags & FLAG_SKIP_EXEC_CLEANUP)
> +		return;
> +
> +	if (cork)
> +		xe_cork_destroy(fd, cork);
> +
> +	xe_vm_destroy(fd, vm);
> +}
> +
> +static void simple_exec(struct hotunplug *priv, unsigned int test_flags)
> +{
> +	if (priv->chipset != DRIVER_XE)
> +		return;

If not providing i915 variant from the beginning then I would at least use a 
construct that is more easy to extend with other platforms, and also skip on 
not supported platforms, not just return if not Xe.

> +
> +	igt_assert_fd(priv->fd.drm);

That's not needed, already verified in local_drm_open_driver().

> +	simple_xe_exec(priv->fd.drm, &xe_engine(priv->fd.drm, 0)->instance, test_flags);
> +}
> +
> +static void exec_hotunbind_rebind(struct hotunplug *priv, unsigned int test_flags)
> +{
> +	pre_check(priv);
> +
> +	priv->fd.drm = local_drm_open_driver(false, "", " for exec and hot unbind");
> +
> +	simple_exec(priv, test_flags);
> +
> +	driver_unbind(priv, "hot ", 0);
> +
> +	priv->fd.drm = close_device(priv->fd.drm, "late ", "unbound ");
> +	igt_assert_eq(priv->fd.drm, -1);
> +
> +	driver_bind(priv, 0);
> +
> +	igt_assert_f(healthcheck(priv, false), "%s\n", priv->failure);

Why there are still no Xe specific health checks implemented?  Isn't your 
intended value added by the new subtest degraded somehow when there is no 
single health check for Xe available?

> +}
> +
>  static void hotunplug_rescan(struct hotunplug *priv)
>  {
>  	pre_check(priv);
> @@ -748,6 +806,18 @@ igt_main
>  			recover(&priv);
>  	}
>  
> +	igt_fixture
> +		post_healthcheck(&priv);
> +
> +	igt_subtest_group {
> +		igt_describe("Check if the driver can be cleanly unbound from an open device with open handles, then released and rebound");
> +		igt_subtest("exec-no-cleanup-hotunbind-rebind")
> +			exec_hotunbind_rebind(&priv, FLAG_SKIP_EXEC_CLEANUP);

Since you provide only one variant, the flag seems not needed for now.

Thanks,
Janusz

> +
> +		igt_fixture
> +			recover(&priv);
> +	}
> +
>  	igt_fixture
>  		post_healthcheck(&priv);
>  
>