Claude review: drm/msm: Add PERFCNTR_CONFIG ioctl

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

Patch Review

**Status: Several issues**

This is the core patch with the UAPI and main ioctl implementation.

**Issue 1: Missing bufsz_shift validation**

```c
stream->fifo_size = 1 << args->bufsz_shift;
```
If `bufsz_shift` is >= 31 (or >= 63 on 64-bit), this is undefined behavior. The later check `stream->fifo_size > SZ_128M` would catch large values, but by then the shift has already been performed. The v4 changelog says "Add upper limit to userspace controlled FIFO size [Claude]" was added, and the `> SZ_128M` check is there, but the shift itself should be validated first:
```c
if (args->bufsz_shift > 27)  /* SZ_128M = 1 << 27 */
    return -EINVAL;
```

**Issue 2: copy_from_user with args->group_stride could under-read**

```c
if (copy_from_user(&g, userptr, args->group_stride))
```
If `args->group_stride < sizeof(struct drm_msm_perfcntr_group)`, this copies fewer bytes than the struct size, leaving parts of `g` at their zero-initialized values. If `args->group_stride > sizeof(struct drm_msm_perfcntr_group)`, this overflows the stack variable `g`. There's no validation that `group_stride` matches `sizeof(struct drm_msm_perfcntr_group)`. This is presumably intentional for extensibility, but the overflow case needs guarding:
```c
if (args->group_stride > sizeof(g))
    return -EINVAL;
```
Or use `min(args->group_stride, sizeof(g))` in the copy.

**Issue 3: State not rolled back on E2BIG for global stream**

When processing groups with `MSM_PERFCNTR_STREAM`, the code writes `allocated_counters` and `countables` into `perfcntrs->groups[idx]` before checking all groups:
```c
perfcntrs->groups[idx]->allocated_counters = g.nr_countables;
...
if (copy_from_user(perfcntrs->groups[idx]->countables, userptr, sz))
    return -EFAULT;
```
If a later group triggers E2BIG, the function returns the error but has already modified `perfcntrs->groups[idx]->allocated_counters` for earlier groups. Since no stream is actually installed (`perfcntrs->stream` remains NULL), this seems harmless in practice -- those `allocated_counters` values are only meaningful when `perfcntrs->stream` is set. But it's messy; a subsequent call with different groups could see stale `allocated_counters` values affecting `get_available_counters()`. Consider clearing the group state on error.

**Issue 4: FIFO read may return 0 bytes without -EAGAIN**

In `msm_perfcntrs_stream_read()`, after waking from the wait for blocking mode:
```c
count = min_t(size_t, count, fifo_count_to_end(stream));
```
If `fifo_count_to_end()` returns 0 (possible if another reader consumed the data, though `read_lock` prevents that for the single-consumer case), the function returns 0, which means EOF to userspace. For a streaming fd, this would be unexpected. Since `read_lock` serializes consumers, this shouldn't happen in practice, but it's a subtle edge case.

**Issue 5: Missing `O_CLOEXEC` on anon_inode_getfd**

```c
stream_fd = anon_inode_getfd("[msm_perfcntrs]", &stream_fops, stream, 0);
```
The flags arg is 0. Most modern kernel code passes `O_CLOEXEC` to prevent fd leaks across exec. Consider using `O_CLOEXEC` by default, or accepting flags from userspace.

**Issue 6: `guard(pm_runtime_active_auto)` scope**

```c
guard(pm_runtime_active_auto)(&gpu->pdev->dev);
guard(mutex)(&gpu->perfcntr_lock);
```
The pm_runtime guard holds a reference for the entire ioctl scope, but it's only needed for the stream case (to ensure the GPU is powered for SEL programming). For the local reservation case, pm_runtime is unnecessary.

**UAPI comment nit:**
```c
 * The data read from the has the following format
```
Missing word: "The data read from the **fd** has the following format".

---
Generated by Claude Code Patch Reviewer