[v9,2/6] drm/xe/xe_gt_pagefault: Move pagefault struct to header

Commit Message

Jonathan Cavitt March 18, 2025, 5:11 p.m. UTC

Move the pagefault struct from xe_gt_pagefault.c to the
xe_gt_pagefault_types.h header file, along with the associated enum values.

v2:
- Normalize names for common header (Matt Brost)

v3:
- s/Migrate/Move (Michal W)
- s/xe_pagefault/xe_gt_pagefault (Michal W)
- Create new header file, xe_gt_pagefault_types.h (Michal W)
- Add kernel docs (Michal W)

v4:
- Fix includes usage (Michal W)
- Reference Bspec (Michal W)

Bspec: 77412

Signed-off-by: Jonathan Cavitt <[e-mail]>
Cc: Michal Wajdeczko <[e-mail]>
---
 drivers/gpu/drm/xe/xe_gt_pagefault.c       | 42 +++------------
 drivers/gpu/drm/xe/xe_gt_pagefault_types.h | 61 ++++++++++++++++++++++
 2 files changed, 69 insertions(+), 34 deletions(-)
 create mode 100644 drivers/gpu/drm/xe/xe_gt_pagefault_types.h

Comments

Michal Wajdeczko March 18, 2025, 6:37 p.m. UTC | #1

On 18.03.2025 18:11, Jonathan Cavitt wrote:
> Move the pagefault struct from xe_gt_pagefault.c to the
> xe_gt_pagefault_types.h header file, along with the associated enum values.
> 
> v2:
> - Normalize names for common header (Matt Brost)
> 
> v3:
> - s/Migrate/Move (Michal W)
> - s/xe_pagefault/xe_gt_pagefault (Michal W)
> - Create new header file, xe_gt_pagefault_types.h (Michal W)
> - Add kernel docs (Michal W)
> 
> v4:
> - Fix includes usage (Michal W)
> - Reference Bspec (Michal W)
> 
> Bspec: 77412
> 

keep all tags together

> Signed-off-by: Jonathan Cavitt <[e-mail]>
> Cc: Michal Wajdeczko <[e-mail]>
> ---
>  drivers/gpu/drm/xe/xe_gt_pagefault.c       | 42 +++------------
>  drivers/gpu/drm/xe/xe_gt_pagefault_types.h | 61 ++++++++++++++++++++++
>  2 files changed, 69 insertions(+), 34 deletions(-)
>  create mode 100644 drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> index 3240890aac07..37b4ab5135a8 100644
> --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> @@ -14,6 +14,7 @@
>  #include "abi/guc_actions_abi.h"
>  #include "xe_bo.h"
>  #include "xe_gt.h"
> +#include "xe_gt_pagefault_types.h"
>  #include "xe_gt_stats.h"
>  #include "xe_gt_tlb_invalidation.h"
>  #include "xe_guc.h"
> @@ -23,33 +24,6 @@
>  #include "xe_trace_bo.h"
>  #include "xe_vm.h"
>  
> -struct pagefault {
> -	u64 page_addr;
> -	u32 asid;
> -	u16 pdata;
> -	u8 vfid;
> -	u8 access_type;
> -	u8 fault_type;
> -	u8 fault_level;
> -	u8 engine_class;
> -	u8 engine_instance;
> -	u8 fault_unsuccessful;
> -	bool trva_fault;
> -};
> -
> -enum access_type {
> -	ACCESS_TYPE_READ = 0,
> -	ACCESS_TYPE_WRITE = 1,
> -	ACCESS_TYPE_ATOMIC = 2,
> -	ACCESS_TYPE_RESERVED = 3,
> -};
> -
> -enum fault_type {
> -	NOT_PRESENT = 0,
> -	WRITE_ACCESS_VIOLATION = 1,
> -	ATOMIC_ACCESS_VIOLATION = 2,
> -};
> -
>  struct acc {
>  	u64 va_range_base;
>  	u32 asid;
> @@ -61,9 +35,9 @@ struct acc {
>  	u8 engine_instance;
>  };
>  
> -static bool access_is_atomic(enum access_type access_type)
> +static bool access_is_atomic(enum xe_gt_pagefault_access_type access_type)
>  {
> -	return access_type == ACCESS_TYPE_ATOMIC;
> +	return access_type == XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC;
>  }
>  
>  static bool vma_is_valid(struct xe_tile *tile, struct xe_vma *vma)
> @@ -205,7 +179,7 @@ static struct xe_vm *asid_to_vm(struct xe_device *xe, u32 asid)
>  	return vm;
>  }
>  
> -static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
> +static int handle_pagefault(struct xe_gt *gt, struct xe_gt_pagefault *pf)
>  {
>  	struct xe_device *xe = gt_to_xe(gt);
>  	struct xe_vm *vm;
> @@ -237,7 +211,7 @@ static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
>  		goto unlock_vm;
>  	}
>  
> -	if (xe_vma_read_only(vma) && pf->access_type != ACCESS_TYPE_READ) {
> +	if (xe_vma_read_only(vma) && pf->access_type != XE_GT_PAGEFAULT_ACCESS_TYPE_READ) {
>  		err = -EPERM;
>  		goto unlock_vm;
>  	}
> @@ -271,7 +245,7 @@ static int send_pagefault_reply(struct xe_guc *guc,
>  	return xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action), 0, 0);
>  }
>  
> -static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
> +static void print_pagefault(struct xe_device *xe, struct xe_gt_pagefault *pf)
>  {
>  	drm_dbg(&xe->drm, "\n\tASID: %d\n"
>  		 "\tVFID: %d\n"
> @@ -291,7 +265,7 @@ static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
>  
>  #define PF_MSG_LEN_DW	4
>  
> -static bool get_pagefault(struct pf_queue *pf_queue, struct pagefault *pf)
> +static bool get_pagefault(struct pf_queue *pf_queue, struct xe_gt_pagefault *pf)
>  {
>  	const struct xe_guc_pagefault_desc *desc;
>  	bool ret = false;
> @@ -378,7 +352,7 @@ static void pf_queue_work_func(struct work_struct *w)
>  	struct xe_gt *gt = pf_queue->gt;
>  	struct xe_device *xe = gt_to_xe(gt);
>  	struct xe_guc_pagefault_reply reply = {};
> -	struct pagefault pf = {};
> +	struct xe_gt_pagefault pf = {};
>  	unsigned long threshold;
>  	int ret;
>  
> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault_types.h b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> new file mode 100644
> index 000000000000..55c598c4a1bc
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022-2025 Intel Corporation
> + */
> +
> +#ifndef _XE_GT_PAGEFAULT_TYPES_H_
> +#define _XE_GT_PAGEFAULT_TYPES_H_
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct xe_gt_pagefault - Structure of pagefaults returned by the
> + * pagefault handler
> + */
> +struct xe_gt_pagefault {
> +	/** @page_addr: faulted address of this pagefault */
> +	u64 page_addr;
> +	/** @asid: ASID of this pagefault */
> +	u32 asid;
> +	/** @pdata: PDATA of this pagefault */
> +	u16 pdata;
> +	/** @vfid: VFID of this pagefault */
> +	u8 vfid;
> +	/**
> +	 * @access_type: access type of this pagefault, as a value
> +	 * from xe_gt_pagefault_access_type
> +	 */
> +	u8 access_type;
> +	/**
> +	 * @fault_type: fault type of this pagefault, as a value
> +	 * from xe_gt_pagefault_fault_type
> +	 */
> +	u8 fault_type;
> +	/** @fault_level: fault level of this pagefault */
> +	u8 fault_level;
> +	/** @engine_class: engine class this pagefault was reported on */
> +	u8 engine_class;
> +	/** @engine_instance: engine instance this pagefault was reported on */
> +	u8 engine_instance;
> +	/** @fault_unsuccessful: flag for if the pagefault recovered or not */
> +	u8 fault_unsuccessful;
> +	/** @prefetch: unused */
> +	bool prefetch;
> +	/** @trva_fault: is set if this is a TRTT fault */
> +	bool trva_fault;
> +};
> +
> +enum xe_gt_pagefault_access_type {
> +	XE_GT_PAGEFAULT_ACCESS_TYPE_READ = 0,
> +	XE_GT_PAGEFAULT_ACCESS_TYPE_WRITE = 1,
> +	XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC = 2,
> +	XE_GT_PAGEFAULT_ACCESS_TYPE_RESERVED = 3,
> +};
> +
> +enum xe_gt_pagefault_fault_type {
> +	XE_GT_PAGEFAULT_TYPE_NOT_PRESENT = 0,
> +	XE_GT_PAGEFAULT_TYPE_WRITE_ACCESS_VIOLATION = 1,
> +	XE_GT_PAGEFAULT_TYPE_ATOMIC_ACCESS_VIOLATION = 2,
> +};

as those two enums seems to be reflecting HW definitions, IMO they
should be defined separately from the SW definitions

most of the HW definitions we keep in the regs/ folder, even if not
being pure register definitions, see:
 - xe_bars.h
 - xe_lrc_layout.h
 - xe_gtt_defs.h

we can add page faults defs to new regs/xe_pagefault_desc.h *) file,
additionally we can define there format of the page fault descriptor,
and remove it from the xe_guc_fwif.h, as GuC seems to pass data in HW
format anyway

or maybe @MattR has a better idea where these defs should go


*) please don't be tempted to use regs/xe_pf_desc.h since we want to use
"PF" term only to mean "Physical Function", not "page fault" ;)

> +
> +#endif

Jonathan Cavitt March 18, 2025, 8:12 p.m. UTC | #2

-----Original Message-----
From: Wajdeczko, Michal <[e-mail]> 
Sent: Tuesday, March 18, 2025 11:38 AM
To: Cavitt, Jonathan <[e-mail]>; [e-mail]; Roper, Matthew D <[e-mail]>
Cc: Gupta, saurabhg <[e-mail]>; Zuo, Alex <[e-mail]>; [e-mail]; Brost, Matthew <[e-mail]>; Zhang, Jianxun <[e-mail]>; Lin, Shuicheng <[e-mail]>; [e-mail]; Mrozek, Michal <[e-mail]>
Subject: Re: [PATCH v9 2/6] drm/xe/xe_gt_pagefault: Move pagefault struct to header
> 
> On 18.03.2025 18:11, Jonathan Cavitt wrote:
> > Move the pagefault struct from xe_gt_pagefault.c to the
> > xe_gt_pagefault_types.h header file, along with the associated enum values.
> > 
> > v2:
> > - Normalize names for common header (Matt Brost)
> > 
> > v3:
> > - s/Migrate/Move (Michal W)
> > - s/xe_pagefault/xe_gt_pagefault (Michal W)
> > - Create new header file, xe_gt_pagefault_types.h (Michal W)
> > - Add kernel docs (Michal W)
> > 
> > v4:
> > - Fix includes usage (Michal W)
> > - Reference Bspec (Michal W)
> > 
> > Bspec: 77412
> > 
> 
> keep all tags together
> 
> > Signed-off-by: Jonathan Cavitt <[e-mail]>
> > Cc: Michal Wajdeczko <[e-mail]>
> > ---
> >  drivers/gpu/drm/xe/xe_gt_pagefault.c       | 42 +++------------
> >  drivers/gpu/drm/xe/xe_gt_pagefault_types.h | 61 ++++++++++++++++++++++
> >  2 files changed, 69 insertions(+), 34 deletions(-)
> >  create mode 100644 drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > index 3240890aac07..37b4ab5135a8 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > @@ -14,6 +14,7 @@
> >  #include "abi/guc_actions_abi.h"
> >  #include "xe_bo.h"
> >  #include "xe_gt.h"
> > +#include "xe_gt_pagefault_types.h"
> >  #include "xe_gt_stats.h"
> >  #include "xe_gt_tlb_invalidation.h"
> >  #include "xe_guc.h"
> > @@ -23,33 +24,6 @@
> >  #include "xe_trace_bo.h"
> >  #include "xe_vm.h"
> >  
> > -struct pagefault {
> > -	u64 page_addr;
> > -	u32 asid;
> > -	u16 pdata;
> > -	u8 vfid;
> > -	u8 access_type;
> > -	u8 fault_type;
> > -	u8 fault_level;
> > -	u8 engine_class;
> > -	u8 engine_instance;
> > -	u8 fault_unsuccessful;
> > -	bool trva_fault;
> > -};
> > -
> > -enum access_type {
> > -	ACCESS_TYPE_READ = 0,
> > -	ACCESS_TYPE_WRITE = 1,
> > -	ACCESS_TYPE_ATOMIC = 2,
> > -	ACCESS_TYPE_RESERVED = 3,
> > -};
> > -
> > -enum fault_type {
> > -	NOT_PRESENT = 0,
> > -	WRITE_ACCESS_VIOLATION = 1,
> > -	ATOMIC_ACCESS_VIOLATION = 2,
> > -};
> > -
> >  struct acc {
> >  	u64 va_range_base;
> >  	u32 asid;
> > @@ -61,9 +35,9 @@ struct acc {
> >  	u8 engine_instance;
> >  };
> >  
> > -static bool access_is_atomic(enum access_type access_type)
> > +static bool access_is_atomic(enum xe_gt_pagefault_access_type access_type)
> >  {
> > -	return access_type == ACCESS_TYPE_ATOMIC;
> > +	return access_type == XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC;
> >  }
> >  
> >  static bool vma_is_valid(struct xe_tile *tile, struct xe_vma *vma)
> > @@ -205,7 +179,7 @@ static struct xe_vm *asid_to_vm(struct xe_device *xe, u32 asid)
> >  	return vm;
> >  }
> >  
> > -static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
> > +static int handle_pagefault(struct xe_gt *gt, struct xe_gt_pagefault *pf)
> >  {
> >  	struct xe_device *xe = gt_to_xe(gt);
> >  	struct xe_vm *vm;
> > @@ -237,7 +211,7 @@ static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
> >  		goto unlock_vm;
> >  	}
> >  
> > -	if (xe_vma_read_only(vma) && pf->access_type != ACCESS_TYPE_READ) {
> > +	if (xe_vma_read_only(vma) && pf->access_type != XE_GT_PAGEFAULT_ACCESS_TYPE_READ) {
> >  		err = -EPERM;
> >  		goto unlock_vm;
> >  	}
> > @@ -271,7 +245,7 @@ static int send_pagefault_reply(struct xe_guc *guc,
> >  	return xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action), 0, 0);
> >  }
> >  
> > -static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
> > +static void print_pagefault(struct xe_device *xe, struct xe_gt_pagefault *pf)
> >  {
> >  	drm_dbg(&xe->drm, "\n\tASID: %d\n"
> >  		 "\tVFID: %d\n"
> > @@ -291,7 +265,7 @@ static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
> >  
> >  #define PF_MSG_LEN_DW	4
> >  
> > -static bool get_pagefault(struct pf_queue *pf_queue, struct pagefault *pf)
> > +static bool get_pagefault(struct pf_queue *pf_queue, struct xe_gt_pagefault *pf)
> >  {
> >  	const struct xe_guc_pagefault_desc *desc;
> >  	bool ret = false;
> > @@ -378,7 +352,7 @@ static void pf_queue_work_func(struct work_struct *w)
> >  	struct xe_gt *gt = pf_queue->gt;
> >  	struct xe_device *xe = gt_to_xe(gt);
> >  	struct xe_guc_pagefault_reply reply = {};
> > -	struct pagefault pf = {};
> > +	struct xe_gt_pagefault pf = {};
> >  	unsigned long threshold;
> >  	int ret;
> >  
> > diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault_types.h b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> > new file mode 100644
> > index 000000000000..55c598c4a1bc
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> > @@ -0,0 +1,61 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2022-2025 Intel Corporation
> > + */
> > +
> > +#ifndef _XE_GT_PAGEFAULT_TYPES_H_
> > +#define _XE_GT_PAGEFAULT_TYPES_H_
> > +
> > +#include <linux/types.h>
> > +
> > +/**
> > + * struct xe_gt_pagefault - Structure of pagefaults returned by the
> > + * pagefault handler
> > + */
> > +struct xe_gt_pagefault {
> > +	/** @page_addr: faulted address of this pagefault */
> > +	u64 page_addr;
> > +	/** @asid: ASID of this pagefault */
> > +	u32 asid;
> > +	/** @pdata: PDATA of this pagefault */
> > +	u16 pdata;
> > +	/** @vfid: VFID of this pagefault */
> > +	u8 vfid;
> > +	/**
> > +	 * @access_type: access type of this pagefault, as a value
> > +	 * from xe_gt_pagefault_access_type
> > +	 */
> > +	u8 access_type;
> > +	/**
> > +	 * @fault_type: fault type of this pagefault, as a value
> > +	 * from xe_gt_pagefault_fault_type
> > +	 */
> > +	u8 fault_type;
> > +	/** @fault_level: fault level of this pagefault */
> > +	u8 fault_level;
> > +	/** @engine_class: engine class this pagefault was reported on */
> > +	u8 engine_class;
> > +	/** @engine_instance: engine instance this pagefault was reported on */
> > +	u8 engine_instance;
> > +	/** @fault_unsuccessful: flag for if the pagefault recovered or not */
> > +	u8 fault_unsuccessful;
> > +	/** @prefetch: unused */
> > +	bool prefetch;
> > +	/** @trva_fault: is set if this is a TRTT fault */
> > +	bool trva_fault;
> > +};
> > +
> > +enum xe_gt_pagefault_access_type {
> > +	XE_GT_PAGEFAULT_ACCESS_TYPE_READ = 0,
> > +	XE_GT_PAGEFAULT_ACCESS_TYPE_WRITE = 1,
> > +	XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC = 2,
> > +	XE_GT_PAGEFAULT_ACCESS_TYPE_RESERVED = 3,
> > +};
> > +
> > +enum xe_gt_pagefault_fault_type {
> > +	XE_GT_PAGEFAULT_TYPE_NOT_PRESENT = 0,
> > +	XE_GT_PAGEFAULT_TYPE_WRITE_ACCESS_VIOLATION = 1,
> > +	XE_GT_PAGEFAULT_TYPE_ATOMIC_ACCESS_VIOLATION = 2,
> > +};
> 
> as those two enums seems to be reflecting HW definitions, IMO they
> should be defined separately from the SW definitions
> 
> most of the HW definitions we keep in the regs/ folder, even if not
> being pure register definitions, see:
>  - xe_bars.h
>  - xe_lrc_layout.h
>  - xe_gtt_defs.h
> 
> we can add page faults defs to new regs/xe_pagefault_desc.h *) file,
> additionally we can define there format of the page fault descriptor,
> and remove it from the xe_guc_fwif.h, as GuC seems to pass data in HW
> format anyway

For clarification, when I move these enum definitions to the regs folder,
I'm going to assume you want me to include the created header file as
an includes in the relevant .c files, and not just staple it into
xe_gt_pagefault.h or xe_gt_pagefault_types.h.  Is this correct?

-Jonathan Cavitt

> 
> or maybe @MattR has a better idea where these defs should go
> 
> 
> *) please don't be tempted to use regs/xe_pf_desc.h since we want to use
> "PF" term only to mean "Physical Function", not "page fault" ;)
> 
> > +
> > +#endif
> 
>

Michal Wajdeczko March 18, 2025, 10:01 p.m. UTC | #3

On 18.03.2025 21:12, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Wajdeczko, Michal <[e-mail]> 
> Sent: Tuesday, March 18, 2025 11:38 AM
> To: Cavitt, Jonathan <[e-mail]>; [e-mail]; Roper, Matthew D <[e-mail]>
> Cc: Gupta, saurabhg <[e-mail]>; Zuo, Alex <[e-mail]>; [e-mail]; Brost, Matthew <[e-mail]>; Zhang, Jianxun <[e-mail]>; Lin, Shuicheng <[e-mail]>; [e-mail]; Mrozek, Michal <[e-mail]>
> Subject: Re: [PATCH v9 2/6] drm/xe/xe_gt_pagefault: Move pagefault struct to header
>>
>> On 18.03.2025 18:11, Jonathan Cavitt wrote:
>>> Move the pagefault struct from xe_gt_pagefault.c to the
>>> xe_gt_pagefault_types.h header file, along with the associated enum values.
>>>
>>> v2:
>>> - Normalize names for common header (Matt Brost)
>>>
>>> v3:
>>> - s/Migrate/Move (Michal W)
>>> - s/xe_pagefault/xe_gt_pagefault (Michal W)
>>> - Create new header file, xe_gt_pagefault_types.h (Michal W)
>>> - Add kernel docs (Michal W)
>>>
>>> v4:
>>> - Fix includes usage (Michal W)
>>> - Reference Bspec (Michal W)
>>>
>>> Bspec: 77412
>>>
>>
>> keep all tags together
>>
>>> Signed-off-by: Jonathan Cavitt <[e-mail]>
>>> Cc: Michal Wajdeczko <[e-mail]>
>>> ---
>>>  drivers/gpu/drm/xe/xe_gt_pagefault.c       | 42 +++------------
>>>  drivers/gpu/drm/xe/xe_gt_pagefault_types.h | 61 ++++++++++++++++++++++
>>>  2 files changed, 69 insertions(+), 34 deletions(-)
>>>  create mode 100644 drivers/gpu/drm/xe/xe_gt_pagefault_types.h
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
>>> index 3240890aac07..37b4ab5135a8 100644
>>> --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
>>> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
>>> @@ -14,6 +14,7 @@
>>>  #include "abi/guc_actions_abi.h"
>>>  #include "xe_bo.h"
>>>  #include "xe_gt.h"
>>> +#include "xe_gt_pagefault_types.h"
>>>  #include "xe_gt_stats.h"
>>>  #include "xe_gt_tlb_invalidation.h"
>>>  #include "xe_guc.h"
>>> @@ -23,33 +24,6 @@
>>>  #include "xe_trace_bo.h"
>>>  #include "xe_vm.h"
>>>  
>>> -struct pagefault {
>>> -	u64 page_addr;
>>> -	u32 asid;
>>> -	u16 pdata;
>>> -	u8 vfid;
>>> -	u8 access_type;
>>> -	u8 fault_type;
>>> -	u8 fault_level;
>>> -	u8 engine_class;
>>> -	u8 engine_instance;
>>> -	u8 fault_unsuccessful;
>>> -	bool trva_fault;
>>> -};
>>> -
>>> -enum access_type {
>>> -	ACCESS_TYPE_READ = 0,
>>> -	ACCESS_TYPE_WRITE = 1,
>>> -	ACCESS_TYPE_ATOMIC = 2,
>>> -	ACCESS_TYPE_RESERVED = 3,
>>> -};
>>> -
>>> -enum fault_type {
>>> -	NOT_PRESENT = 0,
>>> -	WRITE_ACCESS_VIOLATION = 1,
>>> -	ATOMIC_ACCESS_VIOLATION = 2,
>>> -};
>>> -
>>>  struct acc {
>>>  	u64 va_range_base;
>>>  	u32 asid;
>>> @@ -61,9 +35,9 @@ struct acc {
>>>  	u8 engine_instance;
>>>  };
>>>  
>>> -static bool access_is_atomic(enum access_type access_type)
>>> +static bool access_is_atomic(enum xe_gt_pagefault_access_type access_type)
>>>  {
>>> -	return access_type == ACCESS_TYPE_ATOMIC;
>>> +	return access_type == XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC;
>>>  }
>>>  
>>>  static bool vma_is_valid(struct xe_tile *tile, struct xe_vma *vma)
>>> @@ -205,7 +179,7 @@ static struct xe_vm *asid_to_vm(struct xe_device *xe, u32 asid)
>>>  	return vm;
>>>  }
>>>  
>>> -static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
>>> +static int handle_pagefault(struct xe_gt *gt, struct xe_gt_pagefault *pf)
>>>  {
>>>  	struct xe_device *xe = gt_to_xe(gt);
>>>  	struct xe_vm *vm;
>>> @@ -237,7 +211,7 @@ static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
>>>  		goto unlock_vm;
>>>  	}
>>>  
>>> -	if (xe_vma_read_only(vma) && pf->access_type != ACCESS_TYPE_READ) {
>>> +	if (xe_vma_read_only(vma) && pf->access_type != XE_GT_PAGEFAULT_ACCESS_TYPE_READ) {
>>>  		err = -EPERM;
>>>  		goto unlock_vm;
>>>  	}
>>> @@ -271,7 +245,7 @@ static int send_pagefault_reply(struct xe_guc *guc,
>>>  	return xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action), 0, 0);
>>>  }
>>>  
>>> -static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
>>> +static void print_pagefault(struct xe_device *xe, struct xe_gt_pagefault *pf)
>>>  {
>>>  	drm_dbg(&xe->drm, "\n\tASID: %d\n"
>>>  		 "\tVFID: %d\n"
>>> @@ -291,7 +265,7 @@ static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
>>>  
>>>  #define PF_MSG_LEN_DW	4
>>>  
>>> -static bool get_pagefault(struct pf_queue *pf_queue, struct pagefault *pf)
>>> +static bool get_pagefault(struct pf_queue *pf_queue, struct xe_gt_pagefault *pf)
>>>  {
>>>  	const struct xe_guc_pagefault_desc *desc;
>>>  	bool ret = false;
>>> @@ -378,7 +352,7 @@ static void pf_queue_work_func(struct work_struct *w)
>>>  	struct xe_gt *gt = pf_queue->gt;
>>>  	struct xe_device *xe = gt_to_xe(gt);
>>>  	struct xe_guc_pagefault_reply reply = {};
>>> -	struct pagefault pf = {};
>>> +	struct xe_gt_pagefault pf = {};
>>>  	unsigned long threshold;
>>>  	int ret;
>>>  
>>> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault_types.h b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
>>> new file mode 100644
>>> index 000000000000..55c598c4a1bc
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
>>> @@ -0,0 +1,61 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2022-2025 Intel Corporation
>>> + */
>>> +
>>> +#ifndef _XE_GT_PAGEFAULT_TYPES_H_
>>> +#define _XE_GT_PAGEFAULT_TYPES_H_
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +/**
>>> + * struct xe_gt_pagefault - Structure of pagefaults returned by the
>>> + * pagefault handler
>>> + */
>>> +struct xe_gt_pagefault {
>>> +	/** @page_addr: faulted address of this pagefault */
>>> +	u64 page_addr;
>>> +	/** @asid: ASID of this pagefault */
>>> +	u32 asid;
>>> +	/** @pdata: PDATA of this pagefault */
>>> +	u16 pdata;
>>> +	/** @vfid: VFID of this pagefault */
>>> +	u8 vfid;
>>> +	/**
>>> +	 * @access_type: access type of this pagefault, as a value
>>> +	 * from xe_gt_pagefault_access_type
>>> +	 */
>>> +	u8 access_type;
>>> +	/**
>>> +	 * @fault_type: fault type of this pagefault, as a value
>>> +	 * from xe_gt_pagefault_fault_type
>>> +	 */
>>> +	u8 fault_type;
>>> +	/** @fault_level: fault level of this pagefault */
>>> +	u8 fault_level;
>>> +	/** @engine_class: engine class this pagefault was reported on */
>>> +	u8 engine_class;
>>> +	/** @engine_instance: engine instance this pagefault was reported on */
>>> +	u8 engine_instance;
>>> +	/** @fault_unsuccessful: flag for if the pagefault recovered or not */
>>> +	u8 fault_unsuccessful;
>>> +	/** @prefetch: unused */
>>> +	bool prefetch;
>>> +	/** @trva_fault: is set if this is a TRTT fault */
>>> +	bool trva_fault;
>>> +};
>>> +
>>> +enum xe_gt_pagefault_access_type {
>>> +	XE_GT_PAGEFAULT_ACCESS_TYPE_READ = 0,
>>> +	XE_GT_PAGEFAULT_ACCESS_TYPE_WRITE = 1,
>>> +	XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC = 2,
>>> +	XE_GT_PAGEFAULT_ACCESS_TYPE_RESERVED = 3,
>>> +};
>>> +
>>> +enum xe_gt_pagefault_fault_type {
>>> +	XE_GT_PAGEFAULT_TYPE_NOT_PRESENT = 0,
>>> +	XE_GT_PAGEFAULT_TYPE_WRITE_ACCESS_VIOLATION = 1,
>>> +	XE_GT_PAGEFAULT_TYPE_ATOMIC_ACCESS_VIOLATION = 2,
>>> +};
>>
>> as those two enums seems to be reflecting HW definitions, IMO they
>> should be defined separately from the SW definitions
>>
>> most of the HW definitions we keep in the regs/ folder, even if not
>> being pure register definitions, see:
>>  - xe_bars.h
>>  - xe_lrc_layout.h
>>  - xe_gtt_defs.h
>>
>> we can add page faults defs to new regs/xe_pagefault_desc.h *) file,
>> additionally we can define there format of the page fault descriptor,
>> and remove it from the xe_guc_fwif.h, as GuC seems to pass data in HW
>> format anyway
> 
> For clarification, when I move these enum definitions to the regs folder,

I'm not sure that "move as-is" would be ok

right now most (all?) hardware definitions are using #define not the
non-anonymous enums, also I would try to follow names used in the Bspec:

#define FLT_TYPE_NOT_PRESENT_FAULT		0u
#define FLT_TYPE_WRITE_ACCESS_VIOLATION		1u
#define FLT_TYPE_ATOMIC_ACCESS_VIOLATION	2u

#define ACC_TYPE_READ_ACCESS			0u
...

so the "Normalize names for common header" request does not apply for
them, only name of struct xe_gt_pagefault must be aligned

> I'm going to assume you want me to include the created header file as
> an includes in the relevant .c files, 

you likely need only few of these HW defs in xe_gt_pagefault.c

in other places only struct definition is needed (which does not have
any of these "enums" on board)

> and not just staple it into
> xe_gt_pagefault.h or xe_gt_pagefault_types.h.  Is this correct?

the rule is:
if you don't need any of these defs in .h, don't include them there

> 
> -Jonathan Cavitt
> 
>>
>> or maybe @MattR has a better idea where these defs should go
>>
>>
>> *) please don't be tempted to use regs/xe_pf_desc.h since we want to use
>> "PF" term only to mean "Physical Function", not "page fault" ;)
>>
>>> +
>>> +#endif
>>
>>

Jonathan Cavitt March 19, 2025, 3:26 p.m. UTC | #4

-----Original Message-----
From: Wajdeczko, Michal <[e-mail]> 
Sent: Tuesday, March 18, 2025 3:02 PM
To: Cavitt, Jonathan <[e-mail]>; [e-mail]
Cc: Gupta, saurabhg <[e-mail]>; Zuo, Alex <[e-mail]>; [e-mail]; Brost, Matthew <[e-mail]>; Zhang, Jianxun <[e-mail]>; Lin, Shuicheng <[e-mail]>; [e-mail]; Mrozek, Michal <[e-mail]>; Roper, Matthew D <[e-mail]>
Subject: Re: [PATCH v9 2/6] drm/xe/xe_gt_pagefault: Move pagefault struct to header
> 
> On 18.03.2025 21:12, Cavitt, Jonathan wrote:
> > -----Original Message-----
> > From: Wajdeczko, Michal <[e-mail]> 
> > Sent: Tuesday, March 18, 2025 11:38 AM
> > To: Cavitt, Jonathan <[e-mail]>; [e-mail]; Roper, Matthew D <[e-mail]>
> > Cc: Gupta, saurabhg <[e-mail]>; Zuo, Alex <[e-mail]>; [e-mail]; Brost, Matthew <[e-mail]>; Zhang, Jianxun <[e-mail]>; Lin, Shuicheng <[e-mail]>; [e-mail]; Mrozek, Michal <[e-mail]>
> > Subject: Re: [PATCH v9 2/6] drm/xe/xe_gt_pagefault: Move pagefault struct to header
> >>
> >> On 18.03.2025 18:11, Jonathan Cavitt wrote:
> >>> Move the pagefault struct from xe_gt_pagefault.c to the
> >>> xe_gt_pagefault_types.h header file, along with the associated enum values.
> >>>
> >>> v2:
> >>> - Normalize names for common header (Matt Brost)
> >>>
> >>> v3:
> >>> - s/Migrate/Move (Michal W)
> >>> - s/xe_pagefault/xe_gt_pagefault (Michal W)
> >>> - Create new header file, xe_gt_pagefault_types.h (Michal W)
> >>> - Add kernel docs (Michal W)
> >>>
> >>> v4:
> >>> - Fix includes usage (Michal W)
> >>> - Reference Bspec (Michal W)
> >>>
> >>> Bspec: 77412
> >>>
> >>
> >> keep all tags together
> >>
> >>> Signed-off-by: Jonathan Cavitt <[e-mail]>
> >>> Cc: Michal Wajdeczko <[e-mail]>
> >>> ---
> >>>  drivers/gpu/drm/xe/xe_gt_pagefault.c       | 42 +++------------
> >>>  drivers/gpu/drm/xe/xe_gt_pagefault_types.h | 61 ++++++++++++++++++++++
> >>>  2 files changed, 69 insertions(+), 34 deletions(-)
> >>>  create mode 100644 drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> >>>
> >>> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> >>> index 3240890aac07..37b4ab5135a8 100644
> >>> --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
> >>> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> >>> @@ -14,6 +14,7 @@
> >>>  #include "abi/guc_actions_abi.h"
> >>>  #include "xe_bo.h"
> >>>  #include "xe_gt.h"
> >>> +#include "xe_gt_pagefault_types.h"
> >>>  #include "xe_gt_stats.h"
> >>>  #include "xe_gt_tlb_invalidation.h"
> >>>  #include "xe_guc.h"
> >>> @@ -23,33 +24,6 @@
> >>>  #include "xe_trace_bo.h"
> >>>  #include "xe_vm.h"
> >>>  
> >>> -struct pagefault {
> >>> -	u64 page_addr;
> >>> -	u32 asid;
> >>> -	u16 pdata;
> >>> -	u8 vfid;
> >>> -	u8 access_type;
> >>> -	u8 fault_type;
> >>> -	u8 fault_level;
> >>> -	u8 engine_class;
> >>> -	u8 engine_instance;
> >>> -	u8 fault_unsuccessful;
> >>> -	bool trva_fault;
> >>> -};
> >>> -
> >>> -enum access_type {
> >>> -	ACCESS_TYPE_READ = 0,
> >>> -	ACCESS_TYPE_WRITE = 1,
> >>> -	ACCESS_TYPE_ATOMIC = 2,
> >>> -	ACCESS_TYPE_RESERVED = 3,
> >>> -};
> >>> -
> >>> -enum fault_type {
> >>> -	NOT_PRESENT = 0,
> >>> -	WRITE_ACCESS_VIOLATION = 1,
> >>> -	ATOMIC_ACCESS_VIOLATION = 2,
> >>> -};
> >>> -
> >>>  struct acc {
> >>>  	u64 va_range_base;
> >>>  	u32 asid;
> >>> @@ -61,9 +35,9 @@ struct acc {
> >>>  	u8 engine_instance;
> >>>  };
> >>>  
> >>> -static bool access_is_atomic(enum access_type access_type)
> >>> +static bool access_is_atomic(enum xe_gt_pagefault_access_type access_type)
> >>>  {
> >>> -	return access_type == ACCESS_TYPE_ATOMIC;
> >>> +	return access_type == XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC;
> >>>  }
> >>>  
> >>>  static bool vma_is_valid(struct xe_tile *tile, struct xe_vma *vma)
> >>> @@ -205,7 +179,7 @@ static struct xe_vm *asid_to_vm(struct xe_device *xe, u32 asid)
> >>>  	return vm;
> >>>  }
> >>>  
> >>> -static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
> >>> +static int handle_pagefault(struct xe_gt *gt, struct xe_gt_pagefault *pf)
> >>>  {
> >>>  	struct xe_device *xe = gt_to_xe(gt);
> >>>  	struct xe_vm *vm;
> >>> @@ -237,7 +211,7 @@ static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
> >>>  		goto unlock_vm;
> >>>  	}
> >>>  
> >>> -	if (xe_vma_read_only(vma) && pf->access_type != ACCESS_TYPE_READ) {
> >>> +	if (xe_vma_read_only(vma) && pf->access_type != XE_GT_PAGEFAULT_ACCESS_TYPE_READ) {
> >>>  		err = -EPERM;
> >>>  		goto unlock_vm;
> >>>  	}
> >>> @@ -271,7 +245,7 @@ static int send_pagefault_reply(struct xe_guc *guc,
> >>>  	return xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action), 0, 0);
> >>>  }
> >>>  
> >>> -static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
> >>> +static void print_pagefault(struct xe_device *xe, struct xe_gt_pagefault *pf)
> >>>  {
> >>>  	drm_dbg(&xe->drm, "\n\tASID: %d\n"
> >>>  		 "\tVFID: %d\n"
> >>> @@ -291,7 +265,7 @@ static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
> >>>  
> >>>  #define PF_MSG_LEN_DW	4
> >>>  
> >>> -static bool get_pagefault(struct pf_queue *pf_queue, struct pagefault *pf)
> >>> +static bool get_pagefault(struct pf_queue *pf_queue, struct xe_gt_pagefault *pf)
> >>>  {
> >>>  	const struct xe_guc_pagefault_desc *desc;
> >>>  	bool ret = false;
> >>> @@ -378,7 +352,7 @@ static void pf_queue_work_func(struct work_struct *w)
> >>>  	struct xe_gt *gt = pf_queue->gt;
> >>>  	struct xe_device *xe = gt_to_xe(gt);
> >>>  	struct xe_guc_pagefault_reply reply = {};
> >>> -	struct pagefault pf = {};
> >>> +	struct xe_gt_pagefault pf = {};
> >>>  	unsigned long threshold;
> >>>  	int ret;
> >>>  
> >>> diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault_types.h b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> >>> new file mode 100644
> >>> index 000000000000..55c598c4a1bc
> >>> --- /dev/null
> >>> +++ b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> >>> @@ -0,0 +1,61 @@
> >>> +/* SPDX-License-Identifier: MIT */
> >>> +/*
> >>> + * Copyright © 2022-2025 Intel Corporation
> >>> + */
> >>> +
> >>> +#ifndef _XE_GT_PAGEFAULT_TYPES_H_
> >>> +#define _XE_GT_PAGEFAULT_TYPES_H_
> >>> +
> >>> +#include <linux/types.h>
> >>> +
> >>> +/**
> >>> + * struct xe_gt_pagefault - Structure of pagefaults returned by the
> >>> + * pagefault handler
> >>> + */
> >>> +struct xe_gt_pagefault {
> >>> +	/** @page_addr: faulted address of this pagefault */
> >>> +	u64 page_addr;
> >>> +	/** @asid: ASID of this pagefault */
> >>> +	u32 asid;
> >>> +	/** @pdata: PDATA of this pagefault */
> >>> +	u16 pdata;
> >>> +	/** @vfid: VFID of this pagefault */
> >>> +	u8 vfid;
> >>> +	/**
> >>> +	 * @access_type: access type of this pagefault, as a value
> >>> +	 * from xe_gt_pagefault_access_type
> >>> +	 */
> >>> +	u8 access_type;
> >>> +	/**
> >>> +	 * @fault_type: fault type of this pagefault, as a value
> >>> +	 * from xe_gt_pagefault_fault_type
> >>> +	 */
> >>> +	u8 fault_type;
> >>> +	/** @fault_level: fault level of this pagefault */
> >>> +	u8 fault_level;
> >>> +	/** @engine_class: engine class this pagefault was reported on */
> >>> +	u8 engine_class;
> >>> +	/** @engine_instance: engine instance this pagefault was reported on */
> >>> +	u8 engine_instance;
> >>> +	/** @fault_unsuccessful: flag for if the pagefault recovered or not */
> >>> +	u8 fault_unsuccessful;
> >>> +	/** @prefetch: unused */
> >>> +	bool prefetch;
> >>> +	/** @trva_fault: is set if this is a TRTT fault */
> >>> +	bool trva_fault;
> >>> +};
> >>> +
> >>> +enum xe_gt_pagefault_access_type {
> >>> +	XE_GT_PAGEFAULT_ACCESS_TYPE_READ = 0,
> >>> +	XE_GT_PAGEFAULT_ACCESS_TYPE_WRITE = 1,
> >>> +	XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC = 2,
> >>> +	XE_GT_PAGEFAULT_ACCESS_TYPE_RESERVED = 3,
> >>> +};
> >>> +
> >>> +enum xe_gt_pagefault_fault_type {
> >>> +	XE_GT_PAGEFAULT_TYPE_NOT_PRESENT = 0,
> >>> +	XE_GT_PAGEFAULT_TYPE_WRITE_ACCESS_VIOLATION = 1,
> >>> +	XE_GT_PAGEFAULT_TYPE_ATOMIC_ACCESS_VIOLATION = 2,
> >>> +};
> >>
> >> as those two enums seems to be reflecting HW definitions, IMO they
> >> should be defined separately from the SW definitions
> >>
> >> most of the HW definitions we keep in the regs/ folder, even if not
> >> being pure register definitions, see:
> >>  - xe_bars.h
> >>  - xe_lrc_layout.h
> >>  - xe_gtt_defs.h
> >>
> >> we can add page faults defs to new regs/xe_pagefault_desc.h *) file,
> >> additionally we can define there format of the page fault descriptor,
> >> and remove it from the xe_guc_fwif.h, as GuC seems to pass data in HW
> >> format anyway
> > 
> > For clarification, when I move these enum definitions to the regs folder,
> 
> I'm not sure that "move as-is" would be ok
> 
> right now most (all?) hardware definitions are using #define not the
> non-anonymous enums, also I would try to follow names used in the Bspec:
> 
> #define FLT_TYPE_NOT_PRESENT_FAULT		0u
> #define FLT_TYPE_WRITE_ACCESS_VIOLATION		1u
> #define FLT_TYPE_ATOMIC_ACCESS_VIOLATION	2u
> 
> #define ACC_TYPE_READ_ACCESS			0u
> ...
> 
> so the "Normalize names for common header" request does not apply for
> them, only name of struct xe_gt_pagefault must be aligned
> 
> > I'm going to assume you want me to include the created header file as
> > an includes in the relevant .c files, 
> 
> you likely need only few of these HW defs in xe_gt_pagefault.c
> 
> in other places only struct definition is needed (which does not have
> any of these "enums" on board)
> 
> > and not just staple it into
> > xe_gt_pagefault.h or xe_gt_pagefault_types.h.  Is this correct?
> 
> the rule is:
> if you don't need any of these defs in .h, don't include them there

These revision notes have been applied.  I'll wait for 6 hours before posting in
case I get additional revision notes, though.
-Jonathan Cavitt

> 
> > 
> > -Jonathan Cavitt
> > 
> >>
> >> or maybe @MattR has a better idea where these defs should go
> >>
> >>
> >> *) please don't be tempted to use regs/xe_pf_desc.h since we want to use
> >> "PF" term only to mean "Physical Function", not "page fault" ;)
> >>
> >>> +
> >>> +#endif
> >>
> >>
> 
>

Patch

diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
index 3240890aac07..37b4ab5135a8 100644
--- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
@@ -14,6 +14,7 @@ 
 #include "abi/guc_actions_abi.h"
 #include "xe_bo.h"
 #include "xe_gt.h"
+#include "xe_gt_pagefault_types.h"
 #include "xe_gt_stats.h"
 #include "xe_gt_tlb_invalidation.h"
 #include "xe_guc.h"
@@ -23,33 +24,6 @@ 
 #include "xe_trace_bo.h"
 #include "xe_vm.h"
 
-struct pagefault {
-	u64 page_addr;
-	u32 asid;
-	u16 pdata;
-	u8 vfid;
-	u8 access_type;
-	u8 fault_type;
-	u8 fault_level;
-	u8 engine_class;
-	u8 engine_instance;
-	u8 fault_unsuccessful;
-	bool trva_fault;
-};
-
-enum access_type {
-	ACCESS_TYPE_READ = 0,
-	ACCESS_TYPE_WRITE = 1,
-	ACCESS_TYPE_ATOMIC = 2,
-	ACCESS_TYPE_RESERVED = 3,
-};
-
-enum fault_type {
-	NOT_PRESENT = 0,
-	WRITE_ACCESS_VIOLATION = 1,
-	ATOMIC_ACCESS_VIOLATION = 2,
-};
-
 struct acc {
 	u64 va_range_base;
 	u32 asid;
@@ -61,9 +35,9 @@  struct acc {
 	u8 engine_instance;
 };
 
-static bool access_is_atomic(enum access_type access_type)
+static bool access_is_atomic(enum xe_gt_pagefault_access_type access_type)
 {
-	return access_type == ACCESS_TYPE_ATOMIC;
+	return access_type == XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC;
 }
 
 static bool vma_is_valid(struct xe_tile *tile, struct xe_vma *vma)
@@ -205,7 +179,7 @@  static struct xe_vm *asid_to_vm(struct xe_device *xe, u32 asid)
 	return vm;
 }
 
-static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
+static int handle_pagefault(struct xe_gt *gt, struct xe_gt_pagefault *pf)
 {
 	struct xe_device *xe = gt_to_xe(gt);
 	struct xe_vm *vm;
@@ -237,7 +211,7 @@  static int handle_pagefault(struct xe_gt *gt, struct pagefault *pf)
 		goto unlock_vm;
 	}
 
-	if (xe_vma_read_only(vma) && pf->access_type != ACCESS_TYPE_READ) {
+	if (xe_vma_read_only(vma) && pf->access_type != XE_GT_PAGEFAULT_ACCESS_TYPE_READ) {
 		err = -EPERM;
 		goto unlock_vm;
 	}
@@ -271,7 +245,7 @@  static int send_pagefault_reply(struct xe_guc *guc,
 	return xe_guc_ct_send(&guc->ct, action, ARRAY_SIZE(action), 0, 0);
 }
 
-static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
+static void print_pagefault(struct xe_device *xe, struct xe_gt_pagefault *pf)
 {
 	drm_dbg(&xe->drm, "\n\tASID: %d\n"
 		 "\tVFID: %d\n"
@@ -291,7 +265,7 @@  static void print_pagefault(struct xe_device *xe, struct pagefault *pf)
 
 #define PF_MSG_LEN_DW	4
 
-static bool get_pagefault(struct pf_queue *pf_queue, struct pagefault *pf)
+static bool get_pagefault(struct pf_queue *pf_queue, struct xe_gt_pagefault *pf)
 {
 	const struct xe_guc_pagefault_desc *desc;
 	bool ret = false;
@@ -378,7 +352,7 @@  static void pf_queue_work_func(struct work_struct *w)
 	struct xe_gt *gt = pf_queue->gt;
 	struct xe_device *xe = gt_to_xe(gt);
 	struct xe_guc_pagefault_reply reply = {};
-	struct pagefault pf = {};
+	struct xe_gt_pagefault pf = {};
 	unsigned long threshold;
 	int ret;
 
diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault_types.h b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
new file mode 100644
index 000000000000..55c598c4a1bc
--- /dev/null
+++ b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
@@ -0,0 +1,61 @@ 
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022-2025 Intel Corporation
+ */
+
+#ifndef _XE_GT_PAGEFAULT_TYPES_H_
+#define _XE_GT_PAGEFAULT_TYPES_H_
+
+#include <linux/types.h>
+
+/**
+ * struct xe_gt_pagefault - Structure of pagefaults returned by the
+ * pagefault handler
+ */
+struct xe_gt_pagefault {
+	/** @page_addr: faulted address of this pagefault */
+	u64 page_addr;
+	/** @asid: ASID of this pagefault */
+	u32 asid;
+	/** @pdata: PDATA of this pagefault */
+	u16 pdata;
+	/** @vfid: VFID of this pagefault */
+	u8 vfid;
+	/**
+	 * @access_type: access type of this pagefault, as a value
+	 * from xe_gt_pagefault_access_type
+	 */
+	u8 access_type;
+	/**
+	 * @fault_type: fault type of this pagefault, as a value
+	 * from xe_gt_pagefault_fault_type
+	 */
+	u8 fault_type;
+	/** @fault_level: fault level of this pagefault */
+	u8 fault_level;
+	/** @engine_class: engine class this pagefault was reported on */
+	u8 engine_class;
+	/** @engine_instance: engine instance this pagefault was reported on */
+	u8 engine_instance;
+	/** @fault_unsuccessful: flag for if the pagefault recovered or not */
+	u8 fault_unsuccessful;
+	/** @prefetch: unused */
+	bool prefetch;
+	/** @trva_fault: is set if this is a TRTT fault */
+	bool trva_fault;
+};
+
+enum xe_gt_pagefault_access_type {
+	XE_GT_PAGEFAULT_ACCESS_TYPE_READ = 0,
+	XE_GT_PAGEFAULT_ACCESS_TYPE_WRITE = 1,
+	XE_GT_PAGEFAULT_ACCESS_TYPE_ATOMIC = 2,
+	XE_GT_PAGEFAULT_ACCESS_TYPE_RESERVED = 3,
+};
+
+enum xe_gt_pagefault_fault_type {
+	XE_GT_PAGEFAULT_TYPE_NOT_PRESENT = 0,
+	XE_GT_PAGEFAULT_TYPE_WRITE_ACCESS_VIOLATION = 1,
+	XE_GT_PAGEFAULT_TYPE_ATOMIC_ACCESS_VIOLATION = 2,
+};
+
+#endif