Re: [PATCH v29 2/5] drm/xe/xe_pagefault: Track address precision per pagefault

[Matthew Brost <matthew.brost@intel.com>]

On Tue, Dec 02, 2025 at 01:45:10PM -0800, Matthew Brost wrote:
> On Tue, Dec 02, 2025 at 06:46:39PM +0000, Jonathan Cavitt wrote:
> > Add an address precision field to the pagefault consumer.  This captures
> > the fact that pagefaults are reported on a SZ_4K granularity by GuC,
> > meaning the reported pagefault address is only the address of the page
> > where the faulting access occurred rather than the exact address of the
> > fault.  This field is necessary in case more reporters are added where
> > the granularity can be different.
> > 
> > Suggested-by: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_guc_pagefault.c   | 1 +
> >  drivers/gpu/drm/xe/xe_pagefault.c       | 2 ++
> >  drivers/gpu/drm/xe/xe_pagefault_types.h | 8 +++++++-
> >  3 files changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_pagefault.c b/drivers/gpu/drm/xe/xe_guc_pagefault.c
> > index 719a18187a31..79b790fedda8 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_pagefault.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_pagefault.c
> > @@ -74,6 +74,7 @@ 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.addr_precision = 12;
> >  	pf.consumer.asid = FIELD_GET(PFD_ASID, msg[1]);
> >  	pf.consumer.access_type = FIELD_GET(PFD_ACCESS_TYPE, msg[2]);
> >  	pf.consumer.fault_type = FIELD_GET(PFD_FAULT_TYPE, msg[2]);
> > diff --git a/drivers/gpu/drm/xe/xe_pagefault.c b/drivers/gpu/drm/xe/xe_pagefault.c
> > index 0b625a52a598..47dec46515b5 100644
> > --- a/drivers/gpu/drm/xe/xe_pagefault.c
> > +++ b/drivers/gpu/drm/xe/xe_pagefault.c
> > @@ -231,6 +231,7 @@ static void xe_pagefault_print(struct xe_pagefault *pf)
> >  {
> >  	xe_gt_dbg(pf->gt, "\n\tASID: %d\n"
> >  		  "\tFaulted Address: 0x%08x%08x\n"
> > +		  "\tAddress Precision: %lu\n"
> >  		  "\tFaultType: %d\n"
> >  		  "\tAccessType: %d\n"
> >  		  "\tFaultLevel: %d\n"
> > @@ -239,6 +240,7 @@ static void xe_pagefault_print(struct xe_pagefault *pf)
> >  		  pf->consumer.asid,
> >  		  upper_32_bits(pf->consumer.page_addr),
> >  		  lower_32_bits(pf->consumer.page_addr),
> > +		  BIT(pf->consumer.addr_precision),
> >  		  pf->consumer.fault_type,
> >  		  pf->consumer.access_type,
> >  		  pf->consumer.fault_level,
> > diff --git a/drivers/gpu/drm/xe/xe_pagefault_types.h b/drivers/gpu/drm/xe/xe_pagefault_types.h
> > index d3b516407d60..2cf439658466 100644
> > --- a/drivers/gpu/drm/xe/xe_pagefault_types.h
> > +++ b/drivers/gpu/drm/xe/xe_pagefault_types.h
> > @@ -65,6 +65,12 @@ struct xe_pagefault {
> >  	struct {
> >  		/** @consumer.page_addr: address of page fault */
> >  		u64 page_addr;
> > +		/**
> > +		 * @consumer.addr_precision: precision of the page fault address.
> > +		 * u8 rather than u32 to keep compact - actual precision is
> > +		 * BIT(consumer.addr_precision).  Currently only 12
> > +		 */
> > +		u8 addr_precision;
> 
> This is going to make this structure bigger, notice this carefully
> packed to to be 64 bytes. I believe the structure will be larger than
> that after this change cause the PF queue to view this as 128 bytes (it
> aligns size to a pow2). This should be placed by the other u8 in this
> structure.
> 
> Matt
> 
> >  		/** @consumer.asid: address space ID */
> >  		u32 asid;
> >  		/**
> > @@ -85,7 +91,7 @@ struct xe_pagefault {
> >  		/** @consumer.engine_instance: engine instance */
> >  		u8 engine_instance;
> >  		/** consumer.reserved: reserved bits for future expansion */
> > -		u8 reserved[7];
> > +		u8 reserved[6];
> >  	} consumer;
> >  	/**
> >  	 * @producer: State for the producer (i.e., HW/FW interface). Populated
> > -- 
> > 2.43.0
> >