Re: [PATCH v6 2/2] drm/xe/vm: Add srcid to xe_vm_get_property_ioctl fault report

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Logical OR (`||`) is incorrectly used instead of bitwise OR (`|`) when squashing ASID and SRCID into `pf.consumer.id`, corrupting both values.
--

commit 307b4c2649f6f7d7b3a698b8e497542ed1a82f2f
Author: Jonathan Cavitt <jonathan.cavitt@intel.com>

drm/xe/vm: Add srcid to xe_vm_get_property_ioctl fault report

This commit adds the SRCID of the faulting hardware unit to the return
of the xe_vm_get_property_ioctl fault report.

> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index 19b3d0be79282..753a5fc55baa0 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -655,6 +655,7 @@ void xe_vm_add_fault_entry_pf(struct xe_vm *vm, struct xe_pagefault *pf)
>  				  pf->consumer.fault_type_level);
>  	e->fault_level = FIELD_GET(XE_PAGEFAULT_LEVEL_MASK,
>  				   pf->consumer.fault_type_level);
> +	e->srcid = FIELD_GET(XE_PAGEFAULT_SRCID_MASK, pf->consumer.id);

[Severity: High]
Will this always read a zero SRCID due to a logical OR in
xe_guc_pagefault_handler()?

When squashing ASID and SRCID into pf.consumer.id, the code uses a logical OR
(||) instead of a bitwise OR (|):

drivers/gpu/drm/xe/xe_guc_pagefault.c:xe_guc_pagefault_handler() {
    ...
	pf.consumer.id = FIELD_PREP(XE_PAGEFAULT_ASID_MASK,
				    FIELD_GET(PFD_ASID, msg[1])) ||
			 FIELD_PREP(XE_PAGEFAULT_SRCID_MASK,
				    FIELD_GET(PFD_SRC_ID, msg[0]));
    ...
}

Because the logical OR evaluates to a boolean (0 or 1), the actual bitmasks
are destroyed. This means FIELD_GET(XE_PAGEFAULT_SRCID_MASK, pf->consumer.id)
will always evaluate to 0 here. Additionally, this silently corrupts the ASID
to 0 or 1, which may cause xe_pagefault_asid_to_vm() to look up the wrong VM
or fail entirely.

Could the logical OR (||) in xe_guc_pagefault_handler() be replaced with a
bitwise OR (|) to prevent this data loss?

>  
>  	list_add_tail(&e->list, &vm->faults.list);
>  	vm->faults.len++;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902212549.347805-1-jonathan.cavitt@intel.com?part=2