-----Original Message-----
From: Brost, Matthew <[email protected]>
Sent: Monday, June 22, 2026 10:13 PM
To: Cavitt, Jonathan <[email protected]>
Cc: [email protected]; Gupta, Saurabhg
<[email protected]>; Zuo, Alex <[email protected]>; [email protected];
[email protected]; [email protected]; [email protected];
[email protected]; Vivi, Rodrigo <[email protected]>;
[email protected]; [email protected];
[email protected]
Subject: Re: [PATCH v4 1/2] drm/xe/pagefault: Add SRCID to pagefault struct
>
> On Thu, Jun 18, 2026 at 11:33:11PM +0800, Jonathan Cavitt wrote:
> > Add SRCID information to pagefault struct for the purpose of reporting
> > the hardware unit that resulted in the pagefault.
> >
> > v2:
> > - Squash SRCID with ASID to keep the struct compact (Matthew)
> >
> > Signed-off-by: Jonathan Cavitt <[email protected]>
> > Cc: Matthew Brost <[email protected]>
> > Cc: Thomas Hellström <[email protected]>
> > Cc: Rodrigo Vivi <[email protected]>
> > Cc: David Airlie <[email protected]>
> > Cc: Simona Vetter <[email protected]>
> > ---
> > drivers/gpu/drm/xe/xe_guc_pagefault.c | 5 ++++-
> > drivers/gpu/drm/xe/xe_pagefault.c | 23 +++++++++++++++--------
> > drivers/gpu/drm/xe/xe_pagefault_types.h | 9 +++++++--
> > 3 files changed, 26 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_guc_pagefault.c
> > b/drivers/gpu/drm/xe/xe_guc_pagefault.c
> > index 607e32392f46..7d24bd828e5b 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_pagefault.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_pagefault.c
> > @@ -78,7 +78,10 @@ int xe_guc_pagefault_handler(struct xe_guc *guc, u32
> > *msg, u32 len)
> > << PFD_VIRTUAL_ADDR_HI_SHIFT) |
> > (FIELD_GET(PFD_VIRTUAL_ADDR_LO, msg[2]) <<
> > PFD_VIRTUAL_ADDR_LO_SHIFT);
> > - pf.consumer.asid = FIELD_GET(PFD_ASID, msg[1]);
>
> Maybe a BUILD_ON_BUG(XE_MAX_ASID > XE_PAGEFAULT_ASID_MASK);
>
> > + 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]));
> > pf.consumer.access_type = FIELD_GET(PFD_ACCESS_TYPE, msg[2]) |
> > (FIELD_GET(PFD_PREFETCH, msg[2]) ? XE_PAGEFAULT_ACCESS_PREFETCH
> > : 0);
> > if (FIELD_GET(XE2_PFD_TRVA_FAULT, msg[0]))
> > diff --git a/drivers/gpu/drm/xe/xe_pagefault.c
> > b/drivers/gpu/drm/xe/xe_pagefault.c
> > index dd3c068e1a39..2a21203b1a2a 100644
> > --- a/drivers/gpu/drm/xe/xe_pagefault.c
> > +++ b/drivers/gpu/drm/xe/xe_pagefault.c
> > @@ -169,6 +169,7 @@ static struct xe_vm *xe_pagefault_asid_to_vm(struct
> > xe_device *xe, u32 asid)
> >
> > static int xe_pagefault_service(struct xe_pagefault *pf)
> > {
> > + u32 asid = FIELD_GET(XE_PAGEFAULT_ASID_MASK, pf->consumer.id);
>
> Nit: can you move this below the struct stack variables.
>
> > struct xe_gt *gt = pf->gt;
> > struct xe_device *xe = gt_to_xe(gt);
> > struct xe_vm *vm;
> > @@ -180,7 +181,7 @@ static int xe_pagefault_service(struct xe_pagefault *pf)
> > if (pf->consumer.fault_type_level == XE_PAGEFAULT_TYPE_LEVEL_NACK)
> > return -EFAULT;
> >
> > - vm = xe_pagefault_asid_to_vm(xe, pf->consumer.asid);
> > + vm = xe_pagefault_asid_to_vm(xe, asid);
> > if (IS_ERR(vm))
> > return PTR_ERR(vm);
> >
> > @@ -242,14 +243,16 @@ static bool xe_pagefault_queue_pop(struct
> > xe_pagefault_queue *pf_queue,
> >
> > static void xe_pagefault_print(struct xe_pagefault *pf)
> > {
> > - xe_gt_info(pf->gt, "\n\tASID: %d\n"
> > + xe_gt_info(pf->gt, "\n\tASID: %ld\n"
> > "\tFaulted Address: 0x%08x%08x\n"
> > "\tFaultType: %lu\n"
> > "\tAccessType: %lu\n"
> > "\tFaultLevel: %lu\n"
> > "\tEngineClass: %d %s\n"
> > - "\tEngineInstance: %d\n",
> > - pf->consumer.asid,
> > + "\tEngineInstance: %d\n"
> > + "\tSRCID: 0x%02lx\n",
> > + FIELD_GET(XE_PAGEFAULT_ASID_MASK,
> > + pf->consumer.id),
> > upper_32_bits(pf->consumer.page_addr),
> > lower_32_bits(pf->consumer.page_addr),
> > FIELD_GET(XE_PAGEFAULT_TYPE_MASK,
> > @@ -260,7 +263,9 @@ static void xe_pagefault_print(struct xe_pagefault *pf)
> > pf->consumer.fault_type_level),
> > pf->consumer.engine_class,
> > xe_hw_engine_class_to_str(pf->consumer.engine_class),
> > - pf->consumer.engine_instance);
> > + pf->consumer.engine_instance,
> > + FIELD_GET(XE_PAGEFAULT_SRCID_MASK,
> > + pf->consumer.id));
> > }
> >
> > static void xe_pagefault_save_to_vm(struct xe_device *xe, struct
> > xe_pagefault *pf)
> > @@ -273,7 +278,8 @@ static void xe_pagefault_save_to_vm(struct xe_device
> > *xe, struct xe_pagefault *p
> > * mode, return VM anyways.
> > */
> > down_read(&xe->usm.lock);
> > - vm = xa_load(&xe->usm.asid_to_vm, pf->consumer.asid);
> > + vm = xa_load(&xe->usm.asid_to_vm,
> > + FIELD_GET(XE_PAGEFAULT_ASID_MASK, pf->consumer.id));
> > if (vm)
> > xe_vm_get(vm);
> > else
> > @@ -474,8 +480,9 @@ static bool xe_pagefault_queue_full(struct
> > xe_pagefault_queue *pf_queue)
> > */
> > int xe_pagefault_handler(struct xe_device *xe, struct xe_pagefault *pf)
> > {
> > + u32 asid = FIELD_GET(XE_PAGEFAULT_ASID_MASK, pf->consumer.id);
>
> Same nit as above.
The ASID value is necessary to get the initial starting position of the
xe_pagefault_queue
pf_queue below, so defining it later is unviable.
I can apply basically every other suggestion, though.
-Jonathan Cavitt
>
> Otherwise LGTM.
>
> Matt
>
> > struct xe_pagefault_queue *pf_queue = xe->usm.pf_queue +
> > - (pf->consumer.asid % XE_PAGEFAULT_QUEUE_COUNT);
> > + (asid % XE_PAGEFAULT_QUEUE_COUNT);
> > unsigned long flags;
> > bool full;
> >
> > @@ -489,7 +496,7 @@ int xe_pagefault_handler(struct xe_device *xe, struct
> > xe_pagefault *pf)
> > } else {
> > drm_warn(&xe->drm,
> > "PageFault Queue (%d) full, shouldn't be possible\n",
> > - pf->consumer.asid % XE_PAGEFAULT_QUEUE_COUNT);
> > + asid % XE_PAGEFAULT_QUEUE_COUNT);
> > }
> > spin_unlock_irqrestore(&pf_queue->lock, flags);
> >
> > diff --git a/drivers/gpu/drm/xe/xe_pagefault_types.h
> > b/drivers/gpu/drm/xe/xe_pagefault_types.h
> > index c4ee625b93dd..2213cea886c7 100644
> > --- a/drivers/gpu/drm/xe/xe_pagefault_types.h
> > +++ b/drivers/gpu/drm/xe/xe_pagefault_types.h
> > @@ -65,8 +65,13 @@ struct xe_pagefault {
> > struct {
> > /** @consumer.page_addr: address of page fault */
> > u64 page_addr;
> > - /** @consumer.asid: address space ID */
> > - u32 asid;
> > +#define XE_PAGEFAULT_ASID_MASK GENMASK(23, 0)
> > +#define XE_PAGEFAULT_SRCID_MASK GENMASK(31, 24)
> > + /**
> > + * @consumer.id: address space ID and SRCID, folded into one
> > + * to keep size compact
> > + */
> > + u32 id;
> > /**
> > * @consumer.access_type: access type and prefetch flag packed
> > * into a u8.
> > --
> > 2.53.0
> >
>
