From: Claude Code Review Bot <claude-review@example.com>
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 [thread overview]
Message-ID: <review-patch2-20260223172120.98961-8-jonathan.cavitt@intel.com> (raw)
In-Reply-To: <20260223172120.98961-8-jonathan.cavitt@intel.com>
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
next prev parent reply other threads:[~2026-02-24 0:00 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-23 17:21 [PATCH v35 0/4] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
2026-02-23 17:21 ` [PATCH v35 1/4] drm/xe/xe_pagefault: Disallow writes to read-only VMAs Jonathan Cavitt
2026-02-24 0:00 ` Claude review: " Claude Code Review Bot
2026-02-23 17:21 ` [PATCH v35 2/4] drm/xe/uapi: Define drm_xe_vm_get_property Jonathan Cavitt
2026-02-24 0:00 ` Claude Code Review Bot [this message]
2026-02-23 17:21 ` [PATCH v35 3/4] drm/xe/xe_vm: Add per VM fault info Jonathan Cavitt
2026-02-24 0:00 ` Claude review: " Claude Code Review Bot
2026-02-23 17:21 ` [PATCH v35 4/4] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
2026-02-24 0:00 ` Claude review: " Claude Code Review Bot
2026-02-23 23:59 ` Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-03-06 15:55 [PATCH v36 0/4] " Jonathan Cavitt
2026-03-06 15:55 ` [PATCH v36 2/4] drm/xe/uapi: Define drm_xe_vm_get_property Jonathan Cavitt
2026-03-08 22:42 ` Claude review: " Claude Code Review Bot
2026-03-10 14:49 [PATCH v37 0/4] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
2026-03-10 14:49 ` [PATCH v37 2/4] drm/xe/uapi: Define drm_xe_vm_get_property Jonathan Cavitt
2026-03-11 3:12 ` Claude review: " Claude Code Review Bot
2026-03-20 18:35 [PATCH v38 0/4] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
2026-03-20 18:35 ` [PATCH v38 2/4] drm/xe/uapi: Define drm_xe_vm_get_property Jonathan Cavitt
2026-03-21 17:27 ` Claude review: " Claude Code Review Bot
2026-03-24 15:29 [PATCH v39 0/4] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl Jonathan Cavitt
2026-03-24 15:29 ` [PATCH v39 2/4] drm/xe/uapi: Define drm_xe_vm_get_property Jonathan Cavitt
2026-03-24 20:46 ` Claude review: " Claude Code Review Bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=review-patch2-20260223172120.98961-8-jonathan.cavitt@intel.com \
--to=claude-review@example.com \
--cc=dri-devel-reviews@example.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox