Claude review: drm/xe/xe_vm: Implement xe_vm_get_property_ioctl

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

Patch Review

**Missing `pad` field validation:** The ioctl validates `args->reserved[0..2]` but does NOT validate `args->pad`. All other xe ioctls validate their pad fields (as seen by `args->pad` checks in `xe_vm_bind_ioctl` etc.). This is a uAPI bug — once shipped, a non-zero pad value will silently be accepted, preventing future use of this field for extensions.

```c
if (XE_IOCTL_DBG(xe, (args->reserved[0] || args->reserved[1] ||
              args->reserved[2])))
    return -EINVAL;
// Missing: args->pad check
```

**Missing `extensions` field validation:** `args->extensions` is not checked for being 0. If extensions aren't supported yet, it should be validated as MBZ.

**TOCTOU race in size validation:** In `xe_vm_get_property_helper`, the fault count is read under the lock:
```c
spin_lock(&vm->faults.lock);
size = size_mul(sizeof(struct xe_vm_fault), vm->faults.len);
spin_unlock(&vm->faults.lock);
```
Then `args->size > size` is checked. But `vm->faults.len` can increase between this check and the actual `fill_faults` call, which does its own iteration. The comment acknowledges this ("Number of faults may increase between calls"), but the validation is backwards — it allows `args->size <= size` at check time, but by the time `fill_faults` runs, there could be more faults. The `fill_faults` function itself has `count = args->size / entry_size` and iterates `min(count, actual_list_len)`, so this is safe in practice, but the size validation and the comment are misleading. The check `args->size > size` could reject valid requests if faults were added between the size query and the fill call.

**`copy_to_user` copies more than populated:** In `fill_faults`:
```c
ret = copy_to_user(usr_ptr, fault_list, args->size);
```
This copies `args->size` bytes, but only `i` entries were actually filled. If `args->size` corresponds to, say, 10 entries but only 5 faults exist, it copies 10 entries worth of data — the last 5 are zero-initialized (from `kcalloc`), so it's not a data leak, but it's wasteful and confusing. Consider copying only `i * entry_size` bytes and updating `args->size` to reflect the actual amount returned.

**Trivial wrappers `xe_to_user_fault_type` and `xe_to_user_fault_level`:** These are identity functions that just return their input. While the comment says the mapping is "approximately 1-to-1", having no-op wrappers adds code without value. If the mapping changes in the future, these can be added then.

**`fault_entry` memcpy unnecessary:** In `fill_faults`, a local `fault_entry` is populated field-by-field, then `memcpy`'d into `fault_list[i]`. Just assign directly to `fault_list[i]` — the memcpy adds nothing since the types are the same.

---
Generated by Claude Code Patch Reviewer