Re: [PATCH i-g-t] Revert "lib/intel_wa: Assert on error instead of returning -1"

[Ngai-Mint Kwan <ngai-mint.kwan@linux.intel.com>]

Hi Ashutosh,
On 2026-05-27 10:08, Ashutosh Dixit wrote:
> This reverts commit b360115af75ac702b4f776aa3db0ac8a2a620657.
>
> There are cases (e.g. igt@core_setmaster@master-drop-set-user) which drop
> root before accessing debugfs, where we don't want to assert.
>
> Also fix the igt_has_intel_wa() call in tests/intel/xe_oa.c.
>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
This looks good to me.

Reviewed-by: Ngai-Mint Kwan <ngai-mint.kwan@linux.intel.com>

Thanks,
Ngai-Mint Kwan
> ---
>   lib/intel_wa.c      | 19 ++++++++++---------
>   lib/intel_wa.h      |  2 +-
>   tests/intel/xe_oa.c |  2 +-
>   3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/lib/intel_wa.c b/lib/intel_wa.c
> index a16ea62a4c..727dd6c988 100644
> --- a/lib/intel_wa.c
> +++ b/lib/intel_wa.c
> @@ -7,18 +7,18 @@
>   #include <stdint.h>
>   #include <stdio.h>
>   
> -#include "igt_core.h"
>   #include "igt_debugfs.h"
>   #include "igt_sysfs.h"
>   #include "intel_wa.h"
>   #include "xe/xe_query.h"
>   
> -static bool debugfs_file_has_wa(int drm_fd, int debugfs_fd,
> +static int debugfs_file_has_wa(int drm_fd, int debugfs_fd,
>   			       const char *debugfs_name, const char *wa)
>   {
>   	char *debugfs_dump;
>   
> -	igt_assert(igt_debugfs_exists(drm_fd, debugfs_name, O_RDONLY));
> +	if (!igt_debugfs_exists(drm_fd, debugfs_name, O_RDONLY))
> +		return -1;
>   
>   	debugfs_dump = igt_sysfs_get(debugfs_fd, debugfs_name);
>   	if (debugfs_dump) {
> @@ -27,10 +27,10 @@ static bool debugfs_file_has_wa(int drm_fd, int debugfs_fd,
>   		free(debugfs_dump);
>   
>   		if (has_wa)
> -			return true;
> +			return 1;
>   	}
>   
> -	return false;
> +	return 0;
>   }
>   
>   /**
> @@ -38,17 +38,18 @@ static bool debugfs_file_has_wa(int drm_fd, int debugfs_fd,
>    * @drm_fd:	A drm file descriptor
>    * @check_wa:	Workaround to be checked
>    *
> - * Returns:	true if WA present, false otherwise
> + * Returns:	0 if no WA, 1 if WA present, -1 on error
>    */
> -bool igt_has_intel_wa(int drm_fd, const char *check_wa)
> +int igt_has_intel_wa(int drm_fd, const char *check_wa)
>   {
> -	bool ret = false;
> +	int ret = 0;
>   	int debugfs_fd;
>   	unsigned int xe;
>   	char name[256];
>   
>   	debugfs_fd = igt_debugfs_dir(drm_fd);
> -	igt_assert(debugfs_fd >= 0);
> +	if (debugfs_fd == -1)
> +		return -1;
>   
>   	xe_for_each_gt(drm_fd, xe) {
>   		sprintf(name, "gt%d/workarounds", xe);
> diff --git a/lib/intel_wa.h b/lib/intel_wa.h
> index 34cafecc41..765a5948ef 100644
> --- a/lib/intel_wa.h
> +++ b/lib/intel_wa.h
> @@ -6,6 +6,6 @@
>   #ifndef __INTEL_WA_H__
>   #define __INTEL_WA_H__
>   
> -bool igt_has_intel_wa(int drm_fd, const char *check_wa);
> +int igt_has_intel_wa(int drm_fd, const char *check_wa);
>   
>   #endif /* __INTEL_WA_H__ */
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index 56d818d860..e59fe953b0 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -2678,7 +2678,7 @@ test_non_zero_reason(const struct drm_xe_oa_unit *oau, size_t oa_buffer_size)
>   	 * can result in buffer overflows.
>   	 */
>   	if (oau->oa_unit_type == DRM_XE_OA_UNIT_TYPE_MERT &&
> -		igt_has_intel_wa(drm_fd, "14026633728")) {
> +		igt_has_intel_wa(drm_fd, "14026633728") > 0) {
>   		oa_exponent = max(oa_exponent, 8);
>   		properties[9] = oa_exponent;
>   	}