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@xxxxxxxxx> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> Cc: Kamil Konieczny <kamil.konieczny@xxxxxxxxxxxxxxx> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx> --- 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