Claude review: accel/ivpu: Limit number of maximum contexts and doorbells per user

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

Patch Review

**Off-by-one in IVPU_USER_CONTEXT_MAX_SSID (medium)**

The commit message states "Root users can allocate all available contexts (128)" but the new define yields 129 contexts:

```c
#define IVPU_USER_CONTEXT_MAX_SSID     (IVPU_USER_CONTEXT_MIN_SSID + 128)
```

With `IVPU_USER_CONTEXT_MIN_SSID = 2`, MAX_SSID becomes 130. Then `ivpu_get_context_count()` returns `130 - 2 + 1 = 129`. The original code had `+ 63` which gave exactly 64 contexts. To get exactly 128, this should be `+ 127`. The v1->v2 changelog says "Fixed off-by-one error" but it appears the fix was applied in the wrong direction, or the commit message is wrong about the intended count.

**UID==0 check instead of capability check (medium)**

```c
if (uid == 0) {
    limits->max_ctx_count = ivpu_get_context_count(vdev);
    limits->max_db_count = ivpu_get_doorbell_count(vdev);
}
```

Direct UID comparisons are discouraged in the kernel. This bypasses the `kuid_t` type-safety system and doesn't integrate with capability-based access control. The standard kernel pattern would be to use `capable(CAP_SYS_ADMIN)` (or a more specific capability) at the point where limits are being set, or at minimum `uid_eq(current_uid(), GLOBAL_ROOT_UID)`. Using `capable()` would also correctly handle containerized workloads.

Additionally, the UID is captured at allocation time and cached in the `ivpu_user_limits` struct. If `capable()` were used instead, this check would need to happen at lookup time rather than allocation time, since different processes with the same UID could have different capabilities.

**Using kref_read() for control-flow decisions (minor)**

```c
if (kref_read(&limits->ref) >= limits->max_ctx_count) {
    ivpu_dbg(vdev, IOCTL, "User %u exceeded max ctx count %u\n", uid,
             limits->max_ctx_count);
    return ERR_PTR(-EMFILE);
}
```

`kref_read()` is documented as being primarily for debugging purposes, not for making control-flow decisions. While the mutex protection makes this functionally safe, a dedicated `u32 ctx_count` counter (mirroring the `atomic_t db_count` pattern) would be more idiomatic and consistent within the patch itself. It's also somewhat confusing that context counting uses kref while doorbell counting uses an atomic - the two resources are tracked with different mechanisms for no obvious reason.

**UAPI behavior change (medium)**

```c
case DRM_IVPU_PARAM_NUM_CONTEXTS:
-   args->value = ivpu_get_context_count(vdev);
+   args->value = file_priv->user_limits->max_ctx_count;
```

This changes what `DRM_IVPU_PARAM_NUM_CONTEXTS` reports from the total device context count to the per-user maximum. Existing userspace that queries this parameter will now see a different (potentially halved) value. This could break applications that use this value for sizing or configuration. If this is intentional, it should be explicitly noted in the commit message as a UAPI behavior change. Consider whether a separate parameter should be added instead.

**Unrelated formatting changes mixed in (minor)**

The patch includes several unrelated whitespace changes:
- `ivpu_wait_for_ready`: line-wrapping change for `ivpu_err`
- `ivpu_fops`: `.owner\t\t= THIS_MODULE` changed to `.owner = THIS_MODULE`
- `ivpu_pci_ids`: `{ }` changed to `{}`

These should be in a separate cleanup patch to keep the functional change reviewable.

**No doorbell limit reported to userspace (minor)**

While the context limit is now exposed via `DRM_IVPU_PARAM_NUM_CONTEXTS`, there is no corresponding mechanism for userspace to query its doorbell limit. If userspace needs to know how many doorbells it can allocate, there's no way to discover this.

**lockdep_assert_held added to ivpu_cmdq_destroy (good)**

```c
static void ivpu_cmdq_destroy(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
{
+   lockdep_assert_held(&file_priv->lock);
```

Good addition - all callers already hold `file_priv->lock`, and this makes the locking requirement explicit.

**ivpu_cmdq_reset doorbell check (good)**

```c
xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq) {
-   xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
-   cmdq->db_id = 0;
+   if (cmdq->db_id) {
+       xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
+       atomic_dec(&file_priv->user_limits->db_count);
+       cmdq->db_id = 0;
+   }
}
```

The added `if (cmdq->db_id)` guard is a correct improvement - the original code would have called `xa_erase` with id 0 for unregistered command queues, which is harmless but wasteful. With the new atomic decrement, the guard is now necessary to avoid decrementing below zero.

**Removal of unused macros (fine)**

`IVPU_NUM_PRIORITIES` and `IVPU_NUM_CMDQS_PER_CTX` are defined but never referenced anywhere in the codebase, so removing them is correct, though it would be cleaner in a separate patch.

---
Generated by Claude Code Patch Reviewer