Re: [PATCH v12 5/5] drm/xe/xe_vm: Implement xe_vm_get_property_ioctl

[Raag Jadav <raag.jadav@intel.com>]

On Mon, Mar 24, 2025 at 11:09:28PM +0000, Jonathan Cavitt wrote:
> Add support for userspace to request a list of observed faults
> from a specified VM.

...

> v10:
> - Remove unnecessary switch case logic (Raag)

This is usually "changes present in version" and not "comments received
in version" but I guess this must be one of those drm things.

...

> +static int xe_vm_get_property_helper(struct xe_vm *vm,
> +				     struct drm_xe_vm_get_property *args)
> +{
> +	int size;
> +
> +	switch (args->property) {
> +	case DRM_XE_VM_GET_PROPERTY_FAULTS:
> +		spin_lock(&vm->faults.lock);
> +		size = size_mul(sizeof(struct xe_vm_fault), vm->faults.len);
> +		spin_unlock(&vm->faults.lock);
> +
> +		if (args->size)
> +			/*
> +			 * Number of faults may increase between calls to
> +			 * xe_vm_get_property_ioctl, so just report the
> +			 * number of faults the user requests if it's less
> +			 * than or equal to the number of faults in the VM
> +			 * fault array.
> +			 */
> +			return args->size <= size ? fill_faults(vm, args) : -EINVAL;

You're comparing an int with u32 and I'm not sure how this plays out.
The usual practice is to use size_t (even in the struct) but I'm not aware
of its userspace counterpart.

Raag