[PATCH i-g-t v6 1/2] lib/igt_perf: Add utils to extract PMU event info

[Lucas De Marchi]

On Mon, Jan 27, 2025 at 02:33:00PM -0800, Vinay Belgaumkar wrote:
>Functions to parse event ID and GT bit shift for PMU events.
>
>v2: Review comments (Riana)
>
>Cc: Riana Tauro <riana.tauro at intel.com>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>---
> lib/igt_perf.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/igt_perf.h |  2 ++
> 2 files changed, 72 insertions(+)
>
>diff --git a/lib/igt_perf.c b/lib/igt_perf.c
>index 3866c6d77..7e81d5516 100644
>--- a/lib/igt_perf.c
>+++ b/lib/igt_perf.c
>@@ -92,6 +92,76 @@ const char *xe_perf_device(int xe, char *buf, int buflen)
> 	return buf;
> }
>
>+/**
>+ * perf_event_format: Returns the start/end positions of an event format param
>+ * @device: Device string in driver:pci format

driver:pci seems wrong and is not true neither for i915 or xe.
This is actually the pmu_device:

/sys/bus/event_source/devices/{pmu_device}/events/{event_name}

and

/sys/bus/event_source/devices/{pmu_device}/format/{field}



>+ * @param: Parameter for which you need the format start/end bits
>+ * Returns: Start/end bit positions for a event parameter format

	Returns 0 on success, or a negative error code on failure. 

would be more accurate to the int return of this function


>+ */
>+int perf_event_format(const char *device, const char *param, uint32_t *start, uint32_t *end)
>+{
>+	char buf[NAME_MAX];

it seems this was part of a previous review, but I don't understand how
NAME_MAX is related to the buffer size here. You use it for the entire
**path** size and then re-use it for the buffer content.

Well... don´t really care much: as long as we don't overflow and when we
do we fail accordingly, should be good enough **for IGT**

>+	ssize_t bytes;
>+	int ret;
>+	int fd;
>+
>+	snprintf(buf, sizeof(buf),
>+		 "/sys/bus/event_source/devices/%s/format/%s",
>+		 device, param);
>+
>+	fd = open(buf, O_RDONLY);

O_CLOEXEC

we have igt_sysfs_read() that could be used here (note dirfd is ignored
when path is absolute).

not really blocking this as is though.

Lucas De Marchi

>+	if (fd < 0)
>+		return -EINVAL;
>+
>+	bytes = read(fd, buf, sizeof(buf) - 1);
>+	close(fd);
>+	if (bytes < 1)
>+		return -EINVAL;
>+
>+	buf[bytes] = '\0';
>+	ret = sscanf(buf, "config:%u-%u", start, end);
>+	if (ret != 2)
>+		return -EINVAL;
>+
>+	return ret;
>+}
>+
>+/**
>+ * perf_event_config:
>+ * @device: Device string in driver:pci format
>+ * @event: The event name
>+ * @config: Pointer to the config
>+ * Returns: 0 for success, negative value on error
>+ */
>+int perf_event_config(const char *device, const char *event, uint64_t *config)
>+{
>+	char buf[NAME_MAX];
>+	ssize_t bytes;
>+	int ret;
>+	int fd;
>+
>+	snprintf(buf, sizeof(buf),
>+		 "/sys/bus/event_source/devices/%s/events/%s",
>+		 device,
>+		 event);
>+
>+	fd = open(buf, O_RDONLY);
>+	if (fd < 0)
>+		return -EINVAL;
>+
>+	bytes = read(fd, buf, sizeof(buf) - 1);
>+	close(fd);
>+	if (bytes < 1)
>+		return ret;
>+
>+	buf[bytes] = '\0';
>+	ret = sscanf(buf, "event=0x%lx", config);
>+	if (ret != 1)
>+		return -EINVAL;
>+
>+	return 0;
>+}
>+
> uint64_t xe_perf_type_id(int xe)
> {
> 	char buf[80];
>diff --git a/lib/igt_perf.h b/lib/igt_perf.h
>index 3d9ba2917..69f7a3d74 100644
>--- a/lib/igt_perf.h
>+++ b/lib/igt_perf.h
>@@ -71,5 +71,7 @@ int perf_i915_open(int i915, uint64_t config);
> int perf_i915_open_group(int i915, uint64_t config, int group);
>
> int perf_xe_open(int xe, uint64_t config);
>+int perf_event_config(const char *device, const char *event, uint64_t *config);
>+int perf_event_format(const char *device, const char *param, uint32_t *start, uint32_t *end);
>
> #endif /* I915_PERF_H */
>-- 
>2.38.1
>