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: Wed, 25 Mar 2026 06:46:43 +1000 Message-ID: In-Reply-To: <20260324152935.72444-8-jonathan.cavitt@intel.com> References: <20260324152935.72444-6-jonathan.cavitt@intel.com> <20260324152935.72444-8-jonathan.cavitt@intel.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **uAPI naming concern:** The `struct xe_vm_fault` does not follow the `drm_= xe_` prefix convention used by all other uAPI structs in this header (e.g.,= `drm_xe_vm_create`, `drm_xe_vm_bind`). Similarly, the `FAULT_ACCESS_TYPE_*= `, `FAULT_TYPE_*`, and `FAULT_LEVEL_*` defines lack a `DRM_XE_` prefix, whi= ch risks namespace collisions. Every other define in this header uses `DRM_= XE_` prefixing. This is a uAPI that will be set in stone once merged =E2=80= =94 the naming should be consistent. **Struct padding/alignment:** `struct xe_vm_fault` has `__u64 address`, the= n `__u32 address_precision`, then three `__u8` fields, then one `__u8 pad`,= then `__u64 reserved[4]`. Between `pad` and `reserved` there are 0 bytes o= f implicit padding since the three u8 + pad =3D 4 bytes, and address_precis= ion (u32) + 4 bytes of u8s =3D 8 bytes after the u64, so alignment is fine.= Good. **`data`/`value` union:** The union of `data` and `value` in `drm_xe_vm_get= _property` is reasonable for supporting both pointer and scalar property ty= pes, though currently only `data` (pointer) is used. **Size field is `__u32`:** The `size` field is `__u32`, which limits the ma= ximum reportable faults. With `sizeof(struct xe_vm_fault)` being 48 bytes a= nd MAX_FAULTS_SAVED_PER_VM=3D50, the max is 2400 bytes =E2=80=94 well withi= n `__u32` range. Fine, but somewhat inconsistent with other ioctls that use= `__u64` for sizes. --- Generated by Claude Code Patch Reviewer