From c644f737666a710038411c74f6a555bb0865b6e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ADra=20Canal?= Date: Mon, 7 Oct 2024 19:34:52 -0300 Subject: [PATCH] v3d: Don't use performance counters names array with an older kernel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting with Linux v6.11+, performance counter information is no longer duplicated in both the kernel and userspace. Instead, an IOCTL retrieves this information, allowing userspace to maintain a local array for reuse, thus avoiding redundant kernel queries. However, support for older kernels without these new IOCTLs remains. To distinguish between versions, we check `devinfo->max_perfcnt` - which is non-zero on Linux v6.11+ and zero on older kernels. Currently, applications using performance queries on platforms with older kernels encounter a SEGFAULT, as we don't validate `devinfo->max_perfcnt` before accessing the userspace array for performance counter information. This commit makes sure that, if `devinfo->max_perfcnt` is zero, `screen->perfcnt_names` will be NULL. This way, we can check if `screen->perfcnt_names` is different than NULL before attempting to use the userspace array. Fixes: 017dde0d1ca ("v3d: Use DRM_IOCTL_V3D_GET_COUNTER to get perfcnt information") Signed-off-by: MaĆ­ra Canal Reviewed-by: Juan A. Suarez Part-of: (cherry picked from commit 47a78614eaece530244a23ec630e57b24654db46) --- .pick_status.json | 2 +- src/gallium/drivers/v3d/v3d_screen.c | 11 ++++--- src/gallium/drivers/v3d/v3dx_query_perfcnt.c | 32 +++++++++++--------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 1fbc110325f..68bf1b31de1 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1634,7 +1634,7 @@ "description": "v3d: Don't use performance counters names array with an older kernel", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "017dde0d1caa981560c5e6c54491337e4e06d497", "notes": null diff --git a/src/gallium/drivers/v3d/v3d_screen.c b/src/gallium/drivers/v3d/v3d_screen.c index 91c5c2a1aa0..f966d813a44 100644 --- a/src/gallium/drivers/v3d/v3d_screen.c +++ b/src/gallium/drivers/v3d/v3d_screen.c @@ -929,10 +929,13 @@ v3d_screen_create(int fd, const struct pipe_screen_config *config, if (!v3d_get_device_info(screen->fd, &screen->devinfo, &v3d_ioctl)) goto fail; - screen->perfcnt_names = rzalloc_array(screen, char*, screen->devinfo.max_perfcnt); - if (!screen->perfcnt_names) { - fprintf(stderr, "Error allocating performance counters names"); - goto fail; + const uint8_t max_perfcnt = screen->devinfo.max_perfcnt; + if (max_perfcnt) { + screen->perfcnt_names = rzalloc_array(screen, char*, max_perfcnt); + if (!screen->perfcnt_names) { + fprintf(stderr, "Error allocating performance counters names"); + goto fail; + } } driParseConfigFiles(config->options, config->options_info, 0, "v3d", diff --git a/src/gallium/drivers/v3d/v3dx_query_perfcnt.c b/src/gallium/drivers/v3d/v3dx_query_perfcnt.c index 84acbb769da..d627b54d8ce 100644 --- a/src/gallium/drivers/v3d/v3dx_query_perfcnt.c +++ b/src/gallium/drivers/v3d/v3dx_query_perfcnt.c @@ -91,22 +91,24 @@ v3dX(get_driver_query_info_perfcnt)(struct v3d_screen *screen, unsigned index, if (index >= max_perfcnt) return 0; - if (screen->perfcnt_names[index]) { - info->name = screen->perfcnt_names[index]; - } else if (devinfo->max_perfcnt) { - struct drm_v3d_perfmon_get_counter counter = { - .counter = index, - }; - int ret = v3d_ioctl(screen->fd, DRM_IOCTL_V3D_PERFMON_GET_COUNTER, &counter); - if (ret != 0) { - fprintf(stderr, "Failed to get performance counter %d: %s\n", - index, strerror(errno)); - return 0; - } + if (screen->perfcnt_names) { + if (screen->perfcnt_names[index]) { + info->name = screen->perfcnt_names[index]; + } else { + struct drm_v3d_perfmon_get_counter counter = { + .counter = index, + }; + int ret = v3d_ioctl(screen->fd, DRM_IOCTL_V3D_PERFMON_GET_COUNTER, &counter); + if (ret != 0) { + fprintf(stderr, "Failed to get performance counter %d: %s\n", + index, strerror(errno)); + return 0; + } - screen->perfcnt_names[index] = ralloc_strdup(screen->perfcnt_names, - (const char *) counter.name); - info->name = screen->perfcnt_names[index]; + screen->perfcnt_names[index] = ralloc_strdup(screen->perfcnt_names, + (const char *) counter.name); + info->name = screen->perfcnt_names[index]; + } } else { info->name = v3d_performance_counters[index][V3D_PERFCNT_NAME]; }