Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/panthor: missing output handle field in PERF_SESSION_CREATE UAPI --
commit 94c846a97bea171800cf5afb9d2812fd54ce5a02 Author: Lukas Zapolskas <[email protected]> drm/panthor: Add performance counter uAPI This commit extends the DEV_QUERY ioctl and introduces the new DRM_PANTHOR_PERF_SESSION ioctls to allow for the sampling of performance counters. It adds several structures to map user buffers and configure the counter sessions. > diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h > --- a/include/uapi/drm/panthor_drm.h > +++ b/include/uapi/drm/panthor_drm.h [ ... ] > +/** > + * struct drm_panthor_perf_session_create - Arguments passed to > DRM_PANTHOR_PERF_SESSION_CREATE. > + */ > +struct drm_panthor_perf_session_create { > + /** > + * @block_set: Set of performance counter blocks, member of > + * enum drm_panthor_perf_counter_set. > + * > + * This is a global configuration and only one set can be active at a > time. If > + * another client has already requested a counter set, any further > requests > + * for a different counter set will fail and return an -EBUSY. > + * > + * If the requested set does not exist, the request will fail and > return an -EINVAL. > + * > + * Some sets have additional requirements to be enabled, and the setup > request will > + * fail with an -EACCES if these requirements are not satisfied. > + */ > + __u8 block_set; > + > + /** @pad: MBZ. */ > + __u8 pad[7]; > + > + /** @fd: eventfd for signalling the availability of a new sample. */ > + __u32 fd; > + > + /** @ringbuf_handle: Handle to the BO to write perf counter sample to. > */ > + __u32 ringbuf_handle; > + > + /** > + * @control_handle: Handle to the BO containing a contiguous 16 byte > range, used for the > + * insert and extract indices for the ringbuffer. > + */ > + __u32 control_handle; [Severity: High] Does this structure need a __u32 handle output field to return the created session identifier to userspace? It appears that this UAPI returns the newly allocated session ID directly via the ioctl's integer return value rather than writing it back to an output field in this struct. Returning positive values from ioctls can cause standard DRM wrappers, such as libdrm's drmIoctl(), to misinterpret the positive return value as an error code, which might render the API unusable for those clients. Additionally, the core drm_ioctl() function prints a debug warning for any non-zero ioctl return value. Could this cause every successful session creation to spam the kernel debug log with false errors? > + > + /** > + * @sample_slots: The number of slots available in the > userspace-provided BO. Must be > + * a power of 2. [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
