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

[Gustavo Sousa <gustavo.sousa@intel.com>]

Ashutosh Dixit <ashutosh.dixit@intel.com> writes:

> 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>

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

> ---
>  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;
>  	}
> -- 
> 2.54.0