Claude review: drm/msm: Add PERFCNTR_CONFIG ioctl

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

This is the main ioctl patch. Several observations:

**Include style**: The includes use `"linux/anon_inodes.h"` (quotes) rather than `<linux/anon_inodes.h>` (angle brackets). Kernel convention is angle brackets for kernel headers:
```c
+#include "linux/anon_inodes.h"
+#include "linux/gfp_types.h"
+#include "linux/poll.h"
+#include "linux/slab.h"
```
Should be `<linux/...>`.

**`copy_from_user` with `group_stride`**: The ioctl copies `args->group_stride` bytes from each user group entry:
```c
if (copy_from_user(&g, userptr, args->group_stride))
```
There's no validation that `args->group_stride <= sizeof(struct drm_msm_perfcntr_group)`. If userspace passes a `group_stride` larger than the kernel's struct size, this overflows the stack-allocated `g`. The stride should be clamped to `min(args->group_stride, sizeof(g))`.

**`bufsz_shift` validation**: `stream->fifo_size = 1 << args->bufsz_shift`. If `bufsz_shift` is very large (e.g., 63), this is undefined behavior due to left-shifting past the type width. There's a later check `stream->fifo_size > SZ_128M`, but the UB happens first. Should validate `bufsz_shift` is reasonable (e.g., `<= 27` for 128M) before the shift.

**Non-blocking read returns 0**: In `msm_perfcntrs_stream_read()`, when `O_NONBLOCK` is set and the FIFO is empty, `fifo_count_to_end()` returns 0, so `count` becomes 0, `copy_to_user` copies nothing, and the function returns 0. A non-blocking read on an empty fd should return `-EAGAIN`, not 0. Returning 0 signals EOF to userspace.

**`fifo_space` in `sample_worker`**: The `fifo_space()` macro reads `stream->fifo.tail` without `smp_load_acquire`. This is the producer reading the consumer's tail — per the kernel circ_buf documentation, this should use `READ_ONCE()` at minimum (the macro uses `CIRC_SPACE` which doesn't include barriers). In practice, this likely works because the kthread worker is the only producer and the mutex-protected read path is the only consumer, but it would be more correct to use `READ_ONCE` on the tail read.

**Counter allocation not reverted on error**: In the ioctl, when processing groups with `MSM_PERFCNTR_STREAM`, the code sets `perfcntrs->groups[idx]->allocated_counters` and copies countables. If a later group fails validation (e.g., `-E2BIG` or `-EFAULT` from a subsequent `copy_from_user`), these already-modified group states are not rolled back. This could leave stale `allocated_counters` values since the `perfcntrs->groups[]` state is persistent (pre-allocated).

**UAPI comment typo**: "The data read from the has the following format" — missing noun after "from the".

---
Generated by Claude Code Patch Reviewer