[RFC] [PATCH 0/5] Extend IGT to support Android

[RFC] [PATCH 0/5] Extend IGT to support Android

[Jeevaka Prabu Badrappan]

The primary objective of this proposal is to extend the functionality of
IGT to Android environments.

Enabling IGT on Android will allow developers to perform comprehensive
testing of graphics drivers, ensuring robustness and reliability across
different platforms.

By providing stub implementations for libraries that are not natively
supported on Android, we will bridge the gap between Android and Linux
environments, promoting code reuse and reducing development time.

Changes done:
- Create stub implementations of GLib, libunwind, libprocps, and
other necessary libraries to satisfy IGT dependencies on Android.
- Replace glib hash table with generic hash table implementation

Note:
- Incase of linux systems, system libraries will be used whereas for
Android stubs will be used with reduced test coverage.
- Support will be provided to maintain Android support.


Jeevaka Prabu Badrappan (4):
  Replace program_invocation_short_name with prog_name from command line
  Avoid use of pthread_cancel by introducing an exit flag
  Replace glib hash table with c specific implementation
  igt-gpu-tools: Changes to compile for Android

Sapna (1):
  Add stub for libunwind, procps and glib

 include/stub/glib.h      |  43 ++++++++++++++
 include/stub/libunwind.h |  20 +++++++
 include/stub/pids.h      |  41 ++++++++++++++
 lib/igt_aux.c            |   2 +-
 lib/igt_core.c           |   4 ++
 lib/igt_core.h           |   1 +
 lib/igt_device_scan.c    | 118 +++++++++++++++++++++++++++++----------
 lib/igt_dummyload.c      |   6 +-
 lib/igt_dummyload.h      |   2 +
 lib/igt_kmod.c           |  10 ++++
 lib/xe/xe_spin.c         |   2 +-
 tests/intel/xe_create.c  |   1 +
 tools/gputop.c           |   6 +-
 tools/intel_gpu_top.c    |   2 +-
 14 files changed, 221 insertions(+), 37 deletions(-)
 create mode 100644 include/stub/glib.h
 create mode 100644 include/stub/libunwind.h
 create mode 100644 include/stub/pids.h

-- 
2.49.0

[PATCH 1/5] Add stub for libunwind, procps and glib

[Jeevaka Prabu Badrappan]

From: Sapna <sapna1.singh@intel.com>

As libunwind, procps, glib are not available in Android, not able to
build igt for Android.

Provided stub implementations so igt can be compiled for both Android
and linux from same code base. Incase of linux systems, system libraries
will be used whereas for Android stubs will be used with reduced
test coverage.

Signed-off-by: Sapna <sapna1.singh@intel.com>
---
 include/stub/glib.h      | 43 ++++++++++++++++++++++++++++++++++++++++
 include/stub/libunwind.h | 20 +++++++++++++++++++
 include/stub/pids.h      | 41 ++++++++++++++++++++++++++++++++++++++
 lib/igt_aux.c            |  2 +-
 4 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100644 include/stub/glib.h
 create mode 100644 include/stub/libunwind.h
 create mode 100644 include/stub/pids.h

diff --git a/include/stub/glib.h b/include/stub/glib.h
new file mode 100644
index 000000000..0bd862fa2
--- /dev/null
+++ b/include/stub/glib.h
@@ -0,0 +1,43 @@
+#pragma once
+#include <stdbool.h>
+#include <stdlib.h>
+#define G_KEY_FILE_NONE 0
+#define G_REGEX_OPTIMIZE 0
+
+typedef struct _GError {
+    int code;
+    char *message;
+} GError;
+
+typedef struct _GKeyFile {
+    char *key;
+    char *value;
+} GKeyFile;
+
+typedef int gint;
+typedef size_t gsize;
+typedef char gchar;
+typedef unsigned char guchar;
+typedef void GRegex;
+
+static inline void g_clear_error(GError **error) { }
+static inline void g_error_free(GError *error) { }
+static inline const char *g_get_home_dir(void) { return "/data/local/tmp/igt"; }
+static inline void g_key_file_free(GKeyFile *file) { }
+static inline GKeyFile *g_key_file_new(void) { return NULL; }
+static inline int g_key_file_get_integer(GKeyFile *key_file,
+    const char *group_name, const char *key, GError **error) { return 0; }
+static inline char *g_key_file_get_string(GKeyFile *key_file,
+    const char *group_name, const char *key, GError **error) { return NULL; }
+static inline bool g_key_file_load_from_file(GKeyFile *key_file,
+    const char *file, int flags, GError **error) { return false; }
+static inline GRegex* g_regex_new(const char *pattern, int compile_options,
+    int match_options, GError **error) { return NULL; }
+static inline void g_regex_unref(GRegex *pattern) { };
+static gchar* g_base64_encode(const guchar* data, gsize len) {
+    gchar* str = (gchar*)malloc(1);  // Allocate memory for an empty string
+    if (str) {
+        str[0] = '\0';  // Set the string to be empty
+    }
+    return str;
+}
diff --git a/include/stub/libunwind.h b/include/stub/libunwind.h
new file mode 100644
index 000000000..168c31c14
--- /dev/null
+++ b/include/stub/libunwind.h
@@ -0,0 +1,20 @@
+#define UNW_TDEP_CURSOR_LEN     127
+typedef struct unw_cursor
+{
+   int opaque[UNW_TDEP_CURSOR_LEN];
+}
+unw_cursor_t;
+
+
+typedef struct unw_context
+{
+   int ctx;
+}
+unw_context_t;
+
+typedef uint32_t unw_word_t;
+
+int unw_getcontext(unw_context_t *uc) { return 0; }
+int unw_init_local (unw_cursor_t *cur, unw_context_t *uc) { return 0; }
+int unw_step (unw_cursor_t *cur) { return 0; }
+int unw_get_proc_name (unw_cursor_t *cur, char *ch, size_t size, unw_word_t *word) { return 0; }
diff --git a/include/stub/pids.h b/include/stub/pids.h
new file mode 100644
index 000000000..bcdd095e0
--- /dev/null
+++ b/include/stub/pids.h
@@ -0,0 +1,41 @@
+enum pids_item {
+    PIDS_CMD     = 13,           //      str        stat: comm or status: Name
+    PIDS_ID_EGID = 29,           //    u_int        status: Gid
+    PIDS_ID_EUID = 31,           //    u_int        status: Uid
+    PIDS_ID_PID  = 39,           //    s_int        from /proc/<pid>
+};
+enum pids_fetch_type {
+    PIDS_FETCH_TASKS_ONLY,
+    PIDS_FETCH_THREADS_TOO
+};
+
+struct pids_result {
+    enum pids_item item;
+    union {
+        signed char         s_ch;
+        signed int          s_int;
+        unsigned int        u_int;
+        unsigned long       ul_int;
+        unsigned long long  ull_int;
+        char               *str;
+        char              **strv;
+        double              real;
+    } result;
+};
+
+struct pids_stack {
+    struct pids_result *head;
+};
+
+struct pids_info;
+
+#define PIDS_VAL( relative_enum, type, stack, info) \
+    stack -> head [ relative_enum ] . result . type
+
+int procps_pids_new   (struct pids_info **info, enum pids_item *items, int numitems) { return 0; }
+int procps_pids_unref (struct pids_info **info) { return 0;}
+
+struct pids_stack *procps_pids_get(struct pids_info *info, enum pids_fetch_type which) {
+    struct pids_stack *ps = NULL;
+    return ps;
+}
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index f5bf48da6..78bae7542 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -56,7 +56,7 @@
 #ifdef HAVE_LIBPROCPS
 #  include <proc/readproc.h>
 #else
-#  include <libproc2/pids.h>
+#include "pids.h"
 #endif
 
 #include <dirent.h>
-- 
2.49.0

[PATCH 2/5] Replace program_invocation_short_name with prog_name from command line

[Jeevaka Prabu Badrappan]

program_invocation_short_name is global variable provided by glibc.

Inorder to get the igt built for Android(without glibc), replaced the
program_invocation_short_name with the prog_name.

Signed-off-by: Jeevaka Prabu Badrappan <jeevaka.badrappan@intel.com>
---
 tools/gputop.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/gputop.c b/tools/gputop.c
index 43b01f566..ef868f670 100644
--- a/tools/gputop.c
+++ b/tools/gputop.c
@@ -335,7 +335,7 @@ struct gputop_args {
 	unsigned long delay_usec;
 };
 
-static void help(void)
+static void help(char* prog_name)
 {
 	printf("Usage:\n"
 	       "\t%s [options]\n\n"
@@ -343,7 +343,7 @@ static void help(void)
 	       "\t-h, --help                show this help\n"
 	       "\t-d, --delay =SEC[.TENTHS] iterative delay as SECS [.TENTHS]\n"
 	       "\t-n, --iterations =NUMBER  number of executions\n"
-	       , program_invocation_short_name);
+	       , prog_name);
 }
 
 static int parse_args(int argc, char * const argv[], struct gputop_args *args)
@@ -384,7 +384,7 @@ static int parse_args(int argc, char * const argv[], struct gputop_args *args)
 			}
 			break;
 		case 'h':
-			help();
+			help(argv[0]);
 			return 0;
 		default:
 			fprintf(stderr, "Unkonwn option '%c'.\n", c);
-- 
2.49.0

[PATCH 3/5] Avoid use of pthread_cancel by introducing an exit flag

[Jeevaka Prabu Badrappan]

Android build fails due to pthread_cancel unavailability.
Replace pthread_cancel with thread exit based on an exit flag.

Signed-off-by: Jeevaka Prabu Badrappan <jeevaka.badrappan@intel.com>
---
 lib/igt_dummyload.c | 6 +++---
 lib/igt_dummyload.h | 2 ++
 lib/xe/xe_spin.c    | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 3cf80b762..38249afb7 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -530,7 +530,7 @@ static void *timer_thread(void *data)
 	/* Wait until we see the timer fire, or we get cancelled */
 	do {
 		read(spin->timerfd, &overruns, sizeof(overruns));
-	} while (!overruns);
+	} while (!overruns && !spin->exit_thread);
 
 	igt_spin_end(spin);
 	return NULL;
@@ -565,7 +565,7 @@ void igt_spin_set_timeout(igt_spin_t *spin, int64_t ns)
 	timerfd = timerfd_create(CLOCK_MONOTONIC, 0);
 	igt_assert(timerfd >= 0);
 	spin->timerfd = timerfd;
-
+	spin->exit_thread = false;
 	pthread_attr_init(&attr);
 	pthread_attr_setinheritsched(&attr, PTHREAD_EXPLICIT_SCHED);
 	pthread_attr_setschedpolicy(&attr, SCHED_FIFO);
@@ -631,7 +631,7 @@ void igt_spin_end(igt_spin_t *spin)
 static void __igt_spin_free(int fd, igt_spin_t *spin)
 {
 	if (spin->timerfd >= 0) {
-		pthread_cancel(spin->timer_thread);
+		spin->exit_thread = true;
 		igt_assert(pthread_join(spin->timer_thread, NULL) == 0);
 		close(spin->timerfd);
 	}
diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
index b771011af..07b21227d 100644
--- a/lib/igt_dummyload.h
+++ b/lib/igt_dummyload.h
@@ -26,6 +26,7 @@
 #define __IGT_DUMMYLOAD_H__
 
 #include <stdint.h>
+#include <stdatomic.h>
 #include <time.h>
 
 #include "drmtest.h"
@@ -75,6 +76,7 @@ typedef struct igt_spin {
 
 	struct timespec last_signal;
 	pthread_t timer_thread;
+	atomic_bool exit_thread;
 	int timerfd;
 
 	int out_fence;
diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
index a92903b6b..2fb688179 100644
--- a/lib/xe/xe_spin.c
+++ b/lib/xe/xe_spin.c
@@ -264,7 +264,7 @@ void xe_spin_free(int fd, struct igt_spin *spin)
 	igt_assert(spin->driver == INTEL_DRIVER_XE);
 
 	if (spin->timerfd >= 0) {
-		pthread_cancel(spin->timer_thread);
+		spin->exit_thread = true;
 		igt_assert(pthread_join(spin->timer_thread, NULL) == 0);
 		close(spin->timerfd);
 	}
-- 
2.49.0

Re: [PATCH 3/5] Avoid use of pthread_cancel by introducing an exit flag

[Dixit, Ashutosh]

On Tue, 29 Apr 2025 12:57:43 -0700, Jeevaka Prabu Badrappan wrote:
>
> Android build fails due to pthread_cancel unavailability.
> Replace pthread_cancel with thread exit based on an exit flag.

How about adding the definition of pthread_cancel() in an android specific
library and using that, rather than making changes here?

I would think that approach might be better in general, calling into the
android specific library, rather than using '#ifdef ANDROID' as seems to be
done in other patches here?

Thoughts?

>
> Signed-off-by: Jeevaka Prabu Badrappan <jeevaka.badrappan@intel.com>
> ---
>  lib/igt_dummyload.c | 6 +++---
>  lib/igt_dummyload.h | 2 ++
>  lib/xe/xe_spin.c    | 2 +-
>  3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> index 3cf80b762..38249afb7 100644
> --- a/lib/igt_dummyload.c
> +++ b/lib/igt_dummyload.c
> @@ -530,7 +530,7 @@ static void *timer_thread(void *data)
>	/* Wait until we see the timer fire, or we get cancelled */
>	do {
>		read(spin->timerfd, &overruns, sizeof(overruns));
> -	} while (!overruns);
> +	} while (!overruns && !spin->exit_thread);
>
>	igt_spin_end(spin);
>	return NULL;
> @@ -565,7 +565,7 @@ void igt_spin_set_timeout(igt_spin_t *spin, int64_t ns)
>	timerfd = timerfd_create(CLOCK_MONOTONIC, 0);
>	igt_assert(timerfd >= 0);
>	spin->timerfd = timerfd;
> -
> +	spin->exit_thread = false;
>	pthread_attr_init(&attr);
>	pthread_attr_setinheritsched(&attr, PTHREAD_EXPLICIT_SCHED);
>	pthread_attr_setschedpolicy(&attr, SCHED_FIFO);
> @@ -631,7 +631,7 @@ void igt_spin_end(igt_spin_t *spin)
>  static void __igt_spin_free(int fd, igt_spin_t *spin)
>  {
>	if (spin->timerfd >= 0) {
> -		pthread_cancel(spin->timer_thread);
> +		spin->exit_thread = true;
>		igt_assert(pthread_join(spin->timer_thread, NULL) == 0);
>		close(spin->timerfd);
>	}
> diff --git a/lib/igt_dummyload.h b/lib/igt_dummyload.h
> index b771011af..07b21227d 100644
> --- a/lib/igt_dummyload.h
> +++ b/lib/igt_dummyload.h
> @@ -26,6 +26,7 @@
>  #define __IGT_DUMMYLOAD_H__
>
>  #include <stdint.h>
> +#include <stdatomic.h>
>  #include <time.h>
>
>  #include "drmtest.h"
> @@ -75,6 +76,7 @@ typedef struct igt_spin {
>
>	struct timespec last_signal;
>	pthread_t timer_thread;
> +	atomic_bool exit_thread;
>	int timerfd;
>
>	int out_fence;
> diff --git a/lib/xe/xe_spin.c b/lib/xe/xe_spin.c
> index a92903b6b..2fb688179 100644
> --- a/lib/xe/xe_spin.c
> +++ b/lib/xe/xe_spin.c
> @@ -264,7 +264,7 @@ void xe_spin_free(int fd, struct igt_spin *spin)
>	igt_assert(spin->driver == INTEL_DRIVER_XE);
>
>	if (spin->timerfd >= 0) {
> -		pthread_cancel(spin->timer_thread);
> +		spin->exit_thread = true;
>		igt_assert(pthread_join(spin->timer_thread, NULL) == 0);
>		close(spin->timerfd);
>	}
> --
> 2.49.0
>

[PATCH 4/5] Replace glib hash table with c specific implementation

[Jeevaka Prabu Badrappan]

Inorder to support igt build for Android(without glib), replaced
the glib hashtable function with generic hash table implementation.

Signed-off-by: Jeevaka Prabu Badrappan <jeevaka.badrappan@intel.com>
---
 lib/igt_device_scan.c | 118 ++++++++++++++++++++++++++++++++----------
 1 file changed, 90 insertions(+), 28 deletions(-)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 3f26a1737..089777374 100644
--- a/lib/igt_device_scan.c
+++ b/lib/igt_device_scan.c
@@ -28,10 +28,10 @@
 #include "igt_list.h"
 #include "intel_chipset.h"
 
+#include <string.h>
 #include <ctype.h>
 #include <dirent.h>
 #include <fcntl.h>
-#include <glib.h>
 #include <libudev.h>
 #ifdef __linux__
 #include <linux/limits.h>
@@ -217,6 +217,18 @@ static inline bool strequal(const char *a, const char *b)
 	return strcmp(a, b) == 0;
 }
 
+#define TABLE_SIZE 100
+
+typedef struct KeyValue {
+	char *key;
+	char *value;
+	struct KeyValue *next;
+} KeyValue;
+
+typedef struct {
+	KeyValue *table[TABLE_SIZE];
+} HashTable;
+
 struct igt_device {
 	/* Filled for drm devices */
 	struct igt_device *parent;
@@ -224,8 +236,8 @@ struct igt_device {
 	/* Point to vendor spec if can be found */
 
 	/* Properties / sysattrs rewriten from udev lists */
-	GHashTable *props_ht;
-	GHashTable *attrs_ht;
+	HashTable *props_ht;
+	HashTable *attrs_ht;
 
 	/* Most usable variables from udev device */
 	char *subsystem;
@@ -261,6 +273,61 @@ static void igt_device_free(struct igt_device *dev);
 typedef char *(*devname_fn)(uint16_t, uint16_t);
 typedef enum dev_type (*devtype_fn)(uint16_t, uint16_t, const char *);
 
+unsigned int hash(const char *key)
+{
+	unsigned int hash = 0;
+	while (*key) {
+		hash = (hash << 5) + *key++;
+	}
+	return hash % TABLE_SIZE;
+}
+
+HashTable *create_table()
+{
+	HashTable *table = malloc(sizeof(HashTable));
+	for (int i = 0; i < TABLE_SIZE; i++) {
+		table->table[i] = NULL;
+	}
+	return table;
+}
+
+void insert_table(HashTable *table, const char *key, const char *value)
+{
+	unsigned int index = hash(key);
+	KeyValue *new_pair = malloc(sizeof(KeyValue));
+	new_pair->key = strdup(key);
+	new_pair->value = strdup(value);
+	new_pair->next = table->table[index];
+	table->table[index] = new_pair;
+}
+
+char *search_table(HashTable *table, const char *key)
+{
+	unsigned int index = hash(key);
+	KeyValue *pair = table->table[index];
+	while (pair) {
+		if (strcmp(pair->key, key) == 0) {
+			return pair->value;
+		}
+		pair = pair->next;
+	}
+	return NULL;
+}
+
+void free_table(HashTable *table) {
+	for (int i = 0; i < TABLE_SIZE; i++) {
+		KeyValue *pair = table->table[i];
+		while (pair) {
+			KeyValue *temp = pair;
+			pair = pair->next;
+			free(temp->key);
+			free(temp->value);
+			free(temp);
+		}
+	}
+	free(table);
+}
+
 static char *devname_hex(uint16_t vendor, uint16_t device)
 {
 	char *s;
@@ -472,10 +539,8 @@ static struct igt_device *igt_device_new(void)
 	if (!dev)
 		return NULL;
 
-	dev->attrs_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
-					      free, free);
-	dev->props_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
-					      free, free);
+	dev->attrs_ht = create_table();
+	dev->props_ht = create_table();
 
 	if (dev->attrs_ht && dev->props_ht)
 		return dev;
@@ -491,7 +556,7 @@ static void igt_device_add_prop(struct igt_device *dev,
 	if (!key || !value)
 		return;
 
-	g_hash_table_insert(dev->props_ht, strdup(key), strdup(value));
+	insert_table(dev->props_ht, key, value);
 }
 
 static void igt_device_add_attr(struct igt_device *dev,
@@ -525,7 +590,7 @@ static void igt_device_add_attr(struct igt_device *dev,
 		v++;
 	}
 
-	g_hash_table_insert(dev->attrs_ht, strdup(key), strdup(v));
+	insert_table(dev->attrs_ht, key, v);
 }
 
 /* Iterate over udev properties list and rewrite it to igt_device properties
@@ -585,13 +650,13 @@ static void get_attrs_limited(struct udev_device *dev, struct igt_device *idev)
 	}
 }
 
-#define get_prop(dev, prop) ((char *) g_hash_table_lookup(dev->props_ht, prop))
-#define get_attr(dev, attr) ((char *) g_hash_table_lookup(dev->attrs_ht, attr))
+#define get_prop(dev, prop) ((char *) search_table(dev->props_ht, prop))
+#define get_attr(dev, attr) ((char *) search_table(dev->attrs_ht, attr))
 #define get_prop_subsystem(dev) get_prop(dev, "SUBSYSTEM")
 #define is_drm_subsystem(dev)  (strequal(get_prop_subsystem(dev), "drm"))
 #define is_pci_subsystem(dev)  (strequal(get_prop_subsystem(dev), "pci"))
 
-static void print_ht(GHashTable *ht);
+static void print_ht(HashTable *ht);
 static void dump_props_and_attrs(const struct igt_device *dev)
 {
 	printf("\n[properties]\n");
@@ -949,7 +1014,7 @@ static struct igt_device *duplicate_device(struct igt_device *dev) {
 	return dup;
 }
 
-static gint devs_compare(const void *a, const void *b)
+static int devs_compare(const void *a, const void *b)
 {
 	struct igt_device *dev1, *dev2;
 	int ret;
@@ -1088,8 +1153,8 @@ static void igt_device_free(struct igt_device *dev)
 	free(dev->device);
 	free(dev->driver);
 	free(dev->pci_slot_name);
-	g_hash_table_destroy(dev->attrs_ht);
-	g_hash_table_destroy(dev->props_ht);
+	free_table(dev->attrs_ht);
+	free_table(dev->props_ht);
 }
 
 void igt_devices_free(void)
@@ -1258,7 +1323,7 @@ igt_devs_print_user(struct igt_list_head *view,
 		if (!dev->drm_card || dev->drm_render)
 			continue;
 
-		drm_name = rindex(dev->drm_card, '/');
+		drm_name = strrchr(dev->drm_card, '/');
 		if (!drm_name || !*++drm_name)
 			continue;
 
@@ -1306,7 +1371,7 @@ igt_devs_print_user(struct igt_list_head *view,
 			if (strcmp(dev2->parent->syspath, dev->parent->syspath))
 				continue;
 
-			drm_name = rindex(dev2->drm_render, '/');
+			drm_name = strrchr(dev2->drm_render, '/');
 			if (!drm_name || !*++drm_name)
 				continue;
 
@@ -1328,19 +1393,16 @@ static inline void _print_key_value(const char* k, const char *v)
 	printf("%-32s: %s\n", k, v);
 }
 
-static void print_ht(GHashTable *ht)
+static void print_ht(HashTable *table)
 {
-	GList *keys = g_hash_table_get_keys(ht);
-
-	keys = g_list_sort(keys, (GCompareFunc) strcmp);
-	while (keys) {
-		char *k = (char *) keys->data;
-		char *v = g_hash_table_lookup(ht, k);
-
-		_print_key_value(k, v);
-		keys = g_list_next(keys);
+	for (int i = 0; i < TABLE_SIZE; i++) {
+		KeyValue *pair = table->table[i];
+		printf("Index %d:\n", i);
+		while (pair) {
+			printf("  Key: %s, Value: %s\n", pair->key, pair->value);
+			pair = pair->next;
+		}
 	}
-	g_list_free(keys);
 }
 
 static void
-- 
2.49.0

Re: [PATCH 4/5] Replace glib hash table with c specific implementation

[Dixit, Ashutosh]

On Tue, 29 Apr 2025 12:57:44 -0700, Jeevaka Prabu Badrappan wrote:
>
> Inorder to support igt build for Android(without glib), replaced
> the glib hashtable function with generic hash table implementation.

Once again, instead of changing this, how about providing these glibc
provided data structs and functions in an android specific library, to
contain changes to android only?

Looking for feedback from other people on the mailing list too. Thanks.

>
> Signed-off-by: Jeevaka Prabu Badrappan <jeevaka.badrappan@intel.com>
> ---
>  lib/igt_device_scan.c | 118 ++++++++++++++++++++++++++++++++----------
>  1 file changed, 90 insertions(+), 28 deletions(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index 3f26a1737..089777374 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -28,10 +28,10 @@
>  #include "igt_list.h"
>  #include "intel_chipset.h"
>
> +#include <string.h>
>  #include <ctype.h>
>  #include <dirent.h>
>  #include <fcntl.h>
> -#include <glib.h>
>  #include <libudev.h>
>  #ifdef __linux__
>  #include <linux/limits.h>
> @@ -217,6 +217,18 @@ static inline bool strequal(const char *a, const char *b)
>	return strcmp(a, b) == 0;
>  }
>
> +#define TABLE_SIZE 100
> +
> +typedef struct KeyValue {
> +	char *key;
> +	char *value;
> +	struct KeyValue *next;
> +} KeyValue;
> +
> +typedef struct {
> +	KeyValue *table[TABLE_SIZE];
> +} HashTable;
> +
>  struct igt_device {
>	/* Filled for drm devices */
>	struct igt_device *parent;
> @@ -224,8 +236,8 @@ struct igt_device {
>	/* Point to vendor spec if can be found */
>
>	/* Properties / sysattrs rewriten from udev lists */
> -	GHashTable *props_ht;
> -	GHashTable *attrs_ht;
> +	HashTable *props_ht;
> +	HashTable *attrs_ht;
>
>	/* Most usable variables from udev device */
>	char *subsystem;
> @@ -261,6 +273,61 @@ static void igt_device_free(struct igt_device *dev);
>  typedef char *(*devname_fn)(uint16_t, uint16_t);
>  typedef enum dev_type (*devtype_fn)(uint16_t, uint16_t, const char *);
>
> +unsigned int hash(const char *key)
> +{
> +	unsigned int hash = 0;
> +	while (*key) {
> +		hash = (hash << 5) + *key++;
> +	}
> +	return hash % TABLE_SIZE;
> +}
> +
> +HashTable *create_table()
> +{
> +	HashTable *table = malloc(sizeof(HashTable));
> +	for (int i = 0; i < TABLE_SIZE; i++) {
> +		table->table[i] = NULL;
> +	}
> +	return table;
> +}
> +
> +void insert_table(HashTable *table, const char *key, const char *value)
> +{
> +	unsigned int index = hash(key);
> +	KeyValue *new_pair = malloc(sizeof(KeyValue));
> +	new_pair->key = strdup(key);
> +	new_pair->value = strdup(value);
> +	new_pair->next = table->table[index];
> +	table->table[index] = new_pair;
> +}
> +
> +char *search_table(HashTable *table, const char *key)
> +{
> +	unsigned int index = hash(key);
> +	KeyValue *pair = table->table[index];
> +	while (pair) {
> +		if (strcmp(pair->key, key) == 0) {
> +			return pair->value;
> +		}
> +		pair = pair->next;
> +	}
> +	return NULL;
> +}
> +
> +void free_table(HashTable *table) {
> +	for (int i = 0; i < TABLE_SIZE; i++) {
> +		KeyValue *pair = table->table[i];
> +		while (pair) {
> +			KeyValue *temp = pair;
> +			pair = pair->next;
> +			free(temp->key);
> +			free(temp->value);
> +			free(temp);
> +		}
> +	}
> +	free(table);
> +}
> +
>  static char *devname_hex(uint16_t vendor, uint16_t device)
>  {
>	char *s;
> @@ -472,10 +539,8 @@ static struct igt_device *igt_device_new(void)
>	if (!dev)
>		return NULL;
>
> -	dev->attrs_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
> -					      free, free);
> -	dev->props_ht = g_hash_table_new_full(g_str_hash, g_str_equal,
> -					      free, free);
> +	dev->attrs_ht = create_table();
> +	dev->props_ht = create_table();
>
>	if (dev->attrs_ht && dev->props_ht)
>		return dev;
> @@ -491,7 +556,7 @@ static void igt_device_add_prop(struct igt_device *dev,
>	if (!key || !value)
>		return;
>
> -	g_hash_table_insert(dev->props_ht, strdup(key), strdup(value));
> +	insert_table(dev->props_ht, key, value);
>  }
>
>  static void igt_device_add_attr(struct igt_device *dev,
> @@ -525,7 +590,7 @@ static void igt_device_add_attr(struct igt_device *dev,
>		v++;
>	}
>
> -	g_hash_table_insert(dev->attrs_ht, strdup(key), strdup(v));
> +	insert_table(dev->attrs_ht, key, v);
>  }
>
>  /* Iterate over udev properties list and rewrite it to igt_device properties
> @@ -585,13 +650,13 @@ static void get_attrs_limited(struct udev_device *dev, struct igt_device *idev)
>	}
>  }
>
> -#define get_prop(dev, prop) ((char *) g_hash_table_lookup(dev->props_ht, prop))
> -#define get_attr(dev, attr) ((char *) g_hash_table_lookup(dev->attrs_ht, attr))
> +#define get_prop(dev, prop) ((char *) search_table(dev->props_ht, prop))
> +#define get_attr(dev, attr) ((char *) search_table(dev->attrs_ht, attr))
>  #define get_prop_subsystem(dev) get_prop(dev, "SUBSYSTEM")
>  #define is_drm_subsystem(dev)  (strequal(get_prop_subsystem(dev), "drm"))
>  #define is_pci_subsystem(dev)  (strequal(get_prop_subsystem(dev), "pci"))
>
> -static void print_ht(GHashTable *ht);
> +static void print_ht(HashTable *ht);
>  static void dump_props_and_attrs(const struct igt_device *dev)
>  {
>	printf("\n[properties]\n");
> @@ -949,7 +1014,7 @@ static struct igt_device *duplicate_device(struct igt_device *dev) {
>	return dup;
>  }
>
> -static gint devs_compare(const void *a, const void *b)
> +static int devs_compare(const void *a, const void *b)
>  {
>	struct igt_device *dev1, *dev2;
>	int ret;
> @@ -1088,8 +1153,8 @@ static void igt_device_free(struct igt_device *dev)
>	free(dev->device);
>	free(dev->driver);
>	free(dev->pci_slot_name);
> -	g_hash_table_destroy(dev->attrs_ht);
> -	g_hash_table_destroy(dev->props_ht);
> +	free_table(dev->attrs_ht);
> +	free_table(dev->props_ht);
>  }
>
>  void igt_devices_free(void)
> @@ -1258,7 +1323,7 @@ igt_devs_print_user(struct igt_list_head *view,
>		if (!dev->drm_card || dev->drm_render)
>			continue;
>
> -		drm_name = rindex(dev->drm_card, '/');
> +		drm_name = strrchr(dev->drm_card, '/');
>		if (!drm_name || !*++drm_name)
>			continue;
>
> @@ -1306,7 +1371,7 @@ igt_devs_print_user(struct igt_list_head *view,
>			if (strcmp(dev2->parent->syspath, dev->parent->syspath))
>				continue;
>
> -			drm_name = rindex(dev2->drm_render, '/');
> +			drm_name = strrchr(dev2->drm_render, '/');
>			if (!drm_name || !*++drm_name)
>				continue;
>
> @@ -1328,19 +1393,16 @@ static inline void _print_key_value(const char* k, const char *v)
>	printf("%-32s: %s\n", k, v);
>  }
>
> -static void print_ht(GHashTable *ht)
> +static void print_ht(HashTable *table)
>  {
> -	GList *keys = g_hash_table_get_keys(ht);
> -
> -	keys = g_list_sort(keys, (GCompareFunc) strcmp);
> -	while (keys) {
> -		char *k = (char *) keys->data;
> -		char *v = g_hash_table_lookup(ht, k);
> -
> -		_print_key_value(k, v);
> -		keys = g_list_next(keys);
> +	for (int i = 0; i < TABLE_SIZE; i++) {
> +		KeyValue *pair = table->table[i];
> +		printf("Index %d:\n", i);
> +		while (pair) {
> +			printf("  Key: %s, Value: %s\n", pair->key, pair->value);
> +			pair = pair->next;
> +		}
>	}
> -	g_list_free(keys);
>  }
>
>  static void
> --
> 2.49.0
>

[PATCH 5/5] igt-gpu-tools: Changes to compile for Android

[Jeevaka Prabu Badrappan]

Changes done to compile for android:
-  __noreturn and PAGE_SIZE are already defined in Android,
undef and then define it.
- define strchrnul for Android
- Replace index with strchr

Signed-off-by: Jeevaka Prabu Badrappan <jeevaka.badrappan@intel.com>
---
 lib/igt_core.c          |  4 ++++
 lib/igt_core.h          |  1 +
 lib/igt_kmod.c          | 10 ++++++++++
 tests/intel/xe_create.c |  1 +
 tools/intel_gpu_top.c   |  2 +-
 5 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index e1061a2ed..519600dfa 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -373,6 +373,9 @@ static bool stderr_needs_sentinel = false;
 
 static int _igt_dynamic_tests_executed = -1;
 
+#ifdef ANDROID
+static void print_backtrace(void) {}
+#else
 static void print_backtrace(void)
 {
 	unw_cursor_t cursor;
@@ -433,6 +436,7 @@ static void print_backtrace(void)
 	if (dwfl)
 		dwfl_end(dwfl);
 }
+#endif // ANDROID
 
 __attribute__((format(printf, 2, 3)))
 static void internal_assert(bool cond, const char *format, ...)
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 45170e215..7add65938 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -86,6 +86,7 @@
 #define igt_assume(e) BUILD_BUG_ON_INVALID(e)
 #endif
 
+#undef __noreturn
 #define __noreturn __attribute__((noreturn))
 #define __maybe_unused __attribute__((unused))
 
diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 14b6b957a..d0a7372b8 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -765,6 +765,16 @@ igt_amdgpu_driver_unload(void)
 	return IGT_EXIT_SUCCESS;
 }
 
+#ifdef ANDROID
+char *strchrnul(const char *s, int c)
+{
+	while (*s && *s != (char)c) {
+		s++;
+	}
+	return (char *)s;
+}
+#endif
+
 static void kmsg_dump(int fd)
 {
 	char record[4096 + 1];
diff --git a/tests/intel/xe_create.c b/tests/intel/xe_create.c
index b22084c84..264b2ae37 100644
--- a/tests/intel/xe_create.c
+++ b/tests/intel/xe_create.c
@@ -18,6 +18,7 @@
 #include "xe/xe_ioctl.h"
 #include "xe/xe_query.h"
 
+#undef PAGE_SIZE
 #define PAGE_SIZE 0x1000
 
 static struct param {
diff --git a/tools/intel_gpu_top.c b/tools/intel_gpu_top.c
index 68d27089b..54e71e559 100644
--- a/tools/intel_gpu_top.c
+++ b/tools/intel_gpu_top.c
@@ -290,7 +290,7 @@ get_pmu_config(int dirfd, const char *name, const char *counter)
 	if (ret <= 0)
 		return -1;
 
-	p = index(buf, '0');
+	p = strchr(buf, '0');
 	if (!p)
 		return -1;
 
-- 
2.49.0

Re: [RFC] [PATCH 0/5] Extend IGT to support Android

[Dixit, Ashutosh]

On Tue, 29 Apr 2025 12:57:40 -0700, Jeevaka Prabu Badrappan wrote:
>
> The primary objective of this proposal is to extend the functionality of
> IGT to Android environments.
>
> Enabling IGT on Android will allow developers to perform comprehensive
> testing of graphics drivers, ensuring robustness and reliability across
> different platforms.
>
> By providing stub implementations for libraries that are not natively
> supported on Android, we will bridge the gap between Android and Linux
> environments, promoting code reuse and reducing development time.
>
> Changes done:
> - Create stub implementations of GLib, libunwind, libprocps, and
> other necessary libraries to satisfy IGT dependencies on Android.
> - Replace glib hash table with generic hash table implementation
>
> Note:
> - Incase of linux systems, system libraries will be used whereas for
> Android stubs will be used with reduced test coverage.
> - Support will be provided to maintain Android support.

I applied this series and immediately see these compile errors on regular
Linux. Please fix these and resend and then we can start looking at
this. The series should compile without errors or warnings.

$ ninja -C build
ninja: Entering directory `build'
[62/1691] Compiling C object lib/libigt-igt_aux_c.a.p/igt_aux.c.o
FAILED: lib/libigt-igt_aux_c.a.p/igt_aux.c.o
ccache cc -Ilib/libigt-igt_aux_c.a.p -Ilib -I../lib -I../include -I../include/drm-uapi -I../include/drm-uapi-experimental -I../include/linux-uapi -I../lib/stubs/syscalls -I. -I.. -I/usr/include/cairo -I/usr/include/libxml2 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-6 -I/usr/include/pixman-1 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/valgrind -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=dangling-pointer -Werror=implicit -Werror=init-self -Werror=int-conversion -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -D_LARGEFILE64_SOURCE=1 -fPIC -pthread -DWITH_GZFILEOP '-DIGT_DATADIR="/usr/local/share/igt-gpu-tools"' '-DIGT_SRCDIR="/home/adixit/git/igt-gpu-tools/tests"' '-DIGT_LOG_DOMAIN="igt_aux"' -MD -MQ lib/libigt-igt_aux_c.a.p/igt_aux.c.o -MF lib/libigt-igt_aux_c.a.p/igt_aux.c.o.d -o lib/libigt-igt_aux_c.a.p/igt_aux.c.o -c ../lib/igt_aux.c
../lib/igt_aux.c:59:10: fatal error: pids.h: No such file or directory
   59 | #include "pids.h"
      |          ^~~~~~~~
compilation terminated.
[67/1691] Generating lib/iga64_generated_codes.c with a custom command
iga64 assemblies not changed, reusing pre-compiled file ../lib/iga64_generated_codes.c.
[125/1691] Compiling C object lib/libigt-igt_device_scan_c.a.p/igt_device_scan.c.o
../lib/igt_device_scan.c:276:14: warning: no previous prototype for ‘hash’ [-Wmissing-prototypes]
  276 | unsigned int hash(const char *key)
      |              ^~~~
../lib/igt_device_scan.c:285:12: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
  285 | HashTable *create_table()
      |            ^~~~~~~~~~~~
../lib/igt_device_scan.c: In function ‘create_table’:
../lib/igt_device_scan.c:285:12: warning: old-style function definition [-Wold-style-definition]
../lib/igt_device_scan.c: At top level:
../lib/igt_device_scan.c:294:6: warning: no previous prototype for ‘insert_table’ [-Wmissing-prototypes]
  294 | void insert_table(HashTable *table, const char *key, const char *value)
      |      ^~~~~~~~~~~~
../lib/igt_device_scan.c:304:7: warning: no previous prototype for ‘search_table’ [-Wmissing-prototypes]
  304 | char *search_table(HashTable *table, const char *key)
      |       ^~~~~~~~~~~~
../lib/igt_device_scan.c:317:6: warning: no previous prototype for ‘free_table’ [-Wmissing-prototypes]
  317 | void free_table(HashTable *table) {
      |      ^~~~~~~~~~
[135/1691] Compiling C object lib/libigt-i915_intel_decode_c.a.p/i915_intel_decode.c.o
ninja: build stopped: subcommand failed.

Thanks.
--
Ashutosh