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: Tue, 24 Feb 2026 10:00:00 +1000 Message-ID: In-Reply-To: <20260223172120.98961-8-jonathan.cavitt@intel.com> References: <20260223172120.98961-6-jonathan.cavitt@intel.com> <20260223172120.98961-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: **Naming convention violation.** Every other UAPI struct in xe_drm.h uses the `drm_xe_` prefix: `drm_xe_vm_create`, `drm_xe_vm_bind`, `drm_xe_exec_queue_create`, etc. The fault struct is named just `xe_vm_fault`: > +/** struct xe_vm_fault - Describes faults for %DRM_XE_VM_GET_PROPERTY_FAULTS */ > +struct xe_vm_fault { This should be `drm_xe_vm_fault` to be consistent with the rest of the header. Similarly, all defines in xe_drm.h use `DRM_XE_` prefixes, but the fault-related defines use bare `FAULT_` prefixes: > +#define FAULT_ACCESS_TYPE_READ 0 > +#define FAULT_ACCESS_TYPE_WRITE 1 > +#define FAULT_ACCESS_TYPE_ATOMIC 2 > ... > +#define FAULT_TYPE_NOT_PRESENT 0 > +#define FAULT_LEVEL_PTE 0 These are in the global UAPI namespace. They should be `DRM_XE_FAULT_ACCESS_TYPE_READ`, `DRM_XE_FAULT_TYPE_NOT_PRESENT`, etc., to avoid namespace collisions with other subsystems. **Documentation says `-EINVAL` for copy failures.** The doc says "...or -EINVAL if the IOCTL fails for any other reason, such as... if the property data could not be copied to the memory allocated to the data member." But the implementation returns `-EFAULT` for copy_to_user failures (which is the correct return code). The documentation should be updated to match. --- Generated by Claude Code Patch Reviewer