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 <[email protected]> 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/[email protected]?part=2
