From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/xe/uapi: Define drm_xe_vm_get_property Date: Mon, 09 Mar 2026 08:42:06 +1000 Message-ID: In-Reply-To: <20260306155556.67500-8-jonathan.cavitt@intel.com> References: <20260306155556.67500-6-jonathan.cavitt@intel.com> <20260306155556.67500-8-jonathan.cavitt@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review Several uAPI concerns: 1. **Namespace violation for `struct xe_vm_fault`:** The struct is in the DRM uAPI header but uses the `xe_vm_fault` name without the `drm_` prefix. Every other uAPI struct in `xe_drm.h` uses `drm_xe_*` naming. This should be `struct drm_xe_vm_fault`. 2. **Non-namespaced defines:** The defines `FAULT_ACCESS_TYPE_*`, `FAULT_TYPE_*`, and `FAULT_LEVEL_*` are very generic names in a uAPI header. They should be namespaced, e.g., `DRM_XE_FAULT_ACCESS_TYPE_READ`. These are likely to collide with other kernel or userspace defines. 3. **`address_precision` semantics unclear:** The field is documented as "Precision of faulted address" but the meaning isn't specified. From patch 3, it's always `SZ_4K`. The doc should clarify what the value represents (e.g., "the granularity in bytes of the faulted address"). 4. **The data/value union:** The union of `data` and `value` shares the same 8 bytes. When `size == 0`, the ioctl fills in `size` but also potentially returns a `value`. This dual-use is documented but could be confusing. Currently only `DRM_XE_VM_GET_PROPERTY_FAULTS` exists which never uses `value`, so the `value` field is dead code. 5. **Missing `pad` in `struct xe_vm_fault`:** There's a single `__u8 pad` after three `__u8` fields. The struct layout is: `__u64 address` (8), `__u32 address_precision` (4), three `__u8` fields (3), `__u8 pad` (1), `__u64 reserved[4]` (32) = 48 bytes total. This is correct for alignment but the `reserved` array at 4 entries of `__u64` seems very generous for future expansion. --- Generated by Claude Code Patch Reviewer