Claude review: drm/msm: restore GEM-related IOCTLs for KMS devices

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

Patch Review

**Correctness of ioctl table indexing: OK**

The `DRM_IOCTL_DEF_DRV` macro uses designated initializers:
```c
[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = { ... }
```
So entries in `msm_kms_ioctls[]` will be placed at their correct indices (e.g., `MSM_GEM_NEW` at index 2, `MSM_GEM_MADVISE` at index 8) with zero-filled gaps. `ARRAY_SIZE()` will correctly yield 9. This is fine.

**Concern 1: `MSM_INFO_GET_IOVA` and `MSM_INFO_SET_IOVA` on KMS-only devices**

The `msm_ioctl_gem_info` handler is exposed via `MSM_GEM_INFO` in the new table. Some of its sub-commands (`MSM_INFO_GET_IOVA`, `MSM_INFO_SET_IOVA`) call `msm_ioctl_gem_info_iova()` and `msm_ioctl_gem_info_set_iova()` respectively, which check `if (!priv->gpu) return -EINVAL` (lines 428 and 452). On a KMS-only device, `priv->gpu` will be NULL, so these sub-commands will return `-EINVAL`, which is safe but could be confusing. This is probably fine since the other sub-commands (`MSM_INFO_GET_OFFSET`, `MSM_INFO_GET_FLAGS`, metadata, name) all work without a GPU. Worth confirming this matches the intended use case though.

**Concern 2: Missing `gem_prime_import` in `msm_kms_driver`**

The combined `msm_driver` has `.gem_prime_import = msm_gem_prime_import` but `msm_kms_driver` does not. If userspace allocates a GEM buffer via `MSM_GEM_NEW` on the KMS device and then exports/re-imports it, the default `drm_gem_prime_import` will be used instead of `msm_gem_prime_import`. This may or may not matter depending on the GBM backend workflow (which exports from KMS and imports into the GPU driver, not the reverse). But it's worth noting that there's an asymmetry here.

**Concern 3: Commit message notes raise a question**

The author's note:
> Note, here I assume that dumb buffers generally should not be used for
> rendering. That doesn't seem to be complete truth as Mesa kmsro on MSM
> devices uses DRM_IOCTL_MODE_CREATE_DUMB to create buffers for resources.

This is an honest acknowledgment that weakens the justification somewhat. If kmsro already uses dumb buffers successfully for rendering on MSM, then the original commit 98f11fd1cf92 may have been intentional in pushing towards that path. The patch could benefit from clarifying which specific use case is broken — is it specifically the MSM GBM backend (as opposed to kmsro), and on which platforms/configurations?

**Minor: No `MSM_GET_PARAM` / `MSM_SET_PARAM`**

The patch deliberately omits `MSM_GET_PARAM` and `MSM_SET_PARAM`. This seems intentional since those are GPU-related queries, but userspace that calls `MSM_GEM_NEW` might also query params to determine supported flags or GPU capabilities. If the GBM backend needs param queries too, this will need a follow-up.

**Summary:** The patch is technically sound and addresses a real regression. The main open question is whether this is the right set of ioctls and whether the commit message should more precisely identify the broken use case. The `!priv->gpu` guards in `gem_info` sub-commands provide adequate safety for the KMS-only case.

---
Generated by Claude Code Patch Reviewer