[igt-dev] [PATCH v2 2/2] tests/debugfs_test: Add subtest guc_log_relay

[Chris Wilson]

Quoting Piotr Piorkowski (2018-04-24 08:50:04)
> From: Piotr Piórkowski <piotr.piorkowski at intel.com>
> 
> We doesn't have any tests to verify functionality of i915_guc_log_relay.
> I think we should test this component.
> 
> Let's add simple subtest to debugfs_test, which will test if guc
> is enabled:
> - opening the i915_guc_log_relay and creating a file guc_log0,
> - flushing GuC log buffer,
> - closing i915_guc_log_relay with removing from debugfs guc_log0.
> Otherwise, the test will check if the attempt to open the guc_log_relay
> returns an error ENODEV.
> 
> v2:
> - guc isn't required for run the subtest (Chris)
> - rewrite subtest as function (Michal Winiarski)
> - add case when the guc is not enabled
> 
> Signed-off-by: Piotr Piórkowski <piotr.piorkowski at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Lukasz Fiedorowicz <lukasz.fiedorowicz at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  tests/debugfs_test.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 93 insertions(+)
> 
> diff --git a/tests/debugfs_test.c b/tests/debugfs_test.c
> index 2e87e442..c7a953b1 100644
> --- a/tests/debugfs_test.c
> +++ b/tests/debugfs_test.c
> @@ -26,6 +26,7 @@
>  #include <fcntl.h>
>  #include <sys/types.h>
>  #include <dirent.h>
> +#include <errno.h>
>  
>  static void read_and_discard_sysfs_entries(int path_fd, int indent)
>  {
> @@ -87,6 +88,95 @@ static void read_and_discard_sysfs_entries(int path_fd, int indent)
>         closedir(dir);
>  }
>  
> +static bool is_guc_loaded(int gfx_fd)
> +{
> +       bool load;
> +
> +       load = igt_debugfs_search(gfx_fd, "i915_guc_load_status",
> +                               "    status: fetch SUCCESS, load SUCCESS");
> +       igt_debug("GuC %s loaded\n", load ? "is" : "is not");
> +
> +       return load;
> +}
> +
> +#define I915_GUC_LOG_RELAY_FILE  "i915_guc_log_relay"
> +#define GUC_LOG_FILE  "guc_log0"
> +
> +static void check_guc_log_relay(int gfx_fd)
> +{
> +
> +       int page_size = getpagesize();
> +       int fd_relay = -1, fd_log = -1;
> +       ssize_t read_size;
> +       uint8_t *buffer;
> +       bool guc_is_loaded;
> +       int debugfs;
> +
> +       guc_is_loaded = is_guc_loaded(gfx_fd);
> +
> +       debugfs = igt_debugfs_dir(gfx_fd);
> +       fd_relay = openat(debugfs, I915_GUC_LOG_RELAY_FILE, O_RDWR);
> +
> +       if (!guc_is_loaded) {
> +               igt_debug("Case when GuC is disabled\n");

Debugging left over? Lots of fluff messages.

> +               igt_debug("Attempt open file %s return value: %d ( %m )\n",
> +                         I915_GUC_LOG_RELAY_FILE, errno);

Push this into the failure message.

> +
> +               igt_assert(errno == ENODEV);

errno may have been scribbled over many times, and would have only been
valid if fd_relay is -1.

Use igt_assert_eq, but you really want igt_assert_f() if you want
informative error messages rather than igt_debug spam.

But is this the interface you really want to enforce? If the guc log
relay is accessible is up to the kernel, not you. If the kernel tells
you it is not available, skip and move on.
-Chris