[PATCH v23 2/5] drm/xe/xe_gt_pagefault: Move pagefault struct to header

Matthew Brost matthew.brost at intel.com
Mon Apr 28 02:14:29 UTC 2025


On Sun, Apr 27, 2025 at 02:57:22PM +0300, Dafna Hirschfeld wrote:
> On 24.04.2025 20:49, Jonathan Cavitt wrote:
> > Move the pagefault struct from xe_gt_pagefault.c to the
> > xe_gt_pagefault_types.h header file, and move the associated enum values
> > into the regs folder under xe_pagefault_desc.h
> > 
> > Since xe_pagefault_desc.h is being initialized here, also move the
> > xe_guc_pagefault_desc hardware formats to the new file.
> 
> Hi, currently page fault is assumed to be handled by guc only.

Yea, this is layered incorrectly - this is my bad. So in our TLB
invalidation code, also my bad.

> In order to make the descriptor more general, I sent two patches moving
> the descriptor to xe_uc_fw_types.h. [1]
> Maybe my patches could be used instead of this one?
> [1] https://lore.kernel.org/intel-xe/20250209121217.1604458-1-dafna.hirschfeld@intel.com/
>

I think this patch is merged, or will be very shortly. I don't this is
worth holding up, but in general I think the flow should be adjusted to
roughly...

- GuC (or any other PF client) parses PF into HW agnostic format and
  passes it to GT page fault component - I think we drop any fields from
  the current GuC format that are unused by the GT page fault component
- A vfunc is attached too the HW agnostic format which is used to signal
  PF done, in the case of the GuC this issues H2G
- All GuC specific code is removed from GT page fault component

This should allow other firmwares to hook into the generic page fault
code.

Matt
 
> Thanks,
> Dafna
> 
> 
> > 
> > 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)
> > 
> > v5:
> > - Convert enums to defines in regs folder (Michal W)
> > - Move xe_guc_pagefault_desc to regs folder (Michal W)
> > 
> > Bspec: 77412
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> > Reviewed-by: Shuicheng Lin <shuicheng.lin at intel.com>
> > Acked-by: Matthew Brost <matthew.brost at intel.com>
> > Cc: Michal Wajdeczko <Michal.Wajdeczko at intel.com>
> > ---
> > drivers/gpu/drm/xe/regs/xe_pagefault_desc.h | 49 +++++++++++++++++++++
> > drivers/gpu/drm/xe/xe_gt_pagefault.c        | 43 ++++--------------
> > drivers/gpu/drm/xe/xe_gt_pagefault_types.h  | 42 ++++++++++++++++++
> > drivers/gpu/drm/xe/xe_guc_fwif.h            | 28 ------------
> > 4 files changed, 100 insertions(+), 62 deletions(-)
> > create mode 100644 drivers/gpu/drm/xe/regs/xe_pagefault_desc.h
> > create mode 100644 drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> > 
> > diff --git a/drivers/gpu/drm/xe/regs/xe_pagefault_desc.h b/drivers/gpu/drm/xe/regs/xe_pagefault_desc.h
> > new file mode 100644
> > index 000000000000..a169ac274e14
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/regs/xe_pagefault_desc.h
> > @@ -0,0 +1,49 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2025 Intel Corporation
> > + */
> > +
> > +#ifndef _XE_PAGEFAULT_DESC_H_
> > +#define _XE_PAGEFAULT_DESC_H_
> > +
> > +#include <linux/bits.h>
> > +#include <linux/types.h>
> > +
> > +struct xe_guc_pagefault_desc {
> > +	u32 dw0;
> > +#define PFD_FAULT_LEVEL		GENMASK(2, 0)
> > +#define PFD_SRC_ID		GENMASK(10, 3)
> > +#define PFD_RSVD_0		GENMASK(17, 11)
> > +#define XE2_PFD_TRVA_FAULT	BIT(18)
> > +#define PFD_ENG_INSTANCE	GENMASK(24, 19)
> > +#define PFD_ENG_CLASS		GENMASK(27, 25)
> > +#define PFD_PDATA_LO		GENMASK(31, 28)
> > +
> > +	u32 dw1;
> > +#define PFD_PDATA_HI		GENMASK(11, 0)
> > +#define PFD_PDATA_HI_SHIFT	4
> > +#define PFD_ASID		GENMASK(31, 12)
> > +
> > +	u32 dw2;
> > +#define PFD_ACCESS_TYPE		GENMASK(1, 0)
> > +#define PFD_FAULT_TYPE		GENMASK(3, 2)
> > +#define PFD_VFID		GENMASK(9, 4)
> > +#define PFD_RSVD_1		GENMASK(11, 10)
> > +#define PFD_VIRTUAL_ADDR_LO	GENMASK(31, 12)
> > +#define PFD_VIRTUAL_ADDR_LO_SHIFT 12
> > +
> > +	u32 dw3;
> > +#define PFD_VIRTUAL_ADDR_HI	GENMASK(31, 0)
> > +#define PFD_VIRTUAL_ADDR_HI_SHIFT 32
> > +} __packed;
> > +
> > +#define FLT_ACCESS_TYPE_READ		0u
> > +#define FLT_ACCESS_TYPE_WRITE		1u
> > +#define FLT_ACCESS_TYPE_ATOMIC		2u
> > +#define FLT_ACCESS_TYPE_RESERVED	3u
> > +
> > +#define FLT_TYPE_NOT_PRESENT_FAULT		0u
> > +#define FLT_TYPE_WRITE_ACCESS_VIOLATION		1u
> > +#define FLT_TYPE_ATOMIC_ACCESS_VIOLATION	2u
> > +
> > +#endif
> > diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > index d4e3b7eb165a..93afa54c8780 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c
> > @@ -12,8 +12,10 @@
> > #include <drm/drm_managed.h>
> > 
> > #include "abi/guc_actions_abi.h"
> > +#include "regs/xe_pagefault_desc.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 +25,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 +36,9 @@ struct acc {
> > 	u8 engine_instance;
> > };
> > 
> > -static bool access_is_atomic(enum access_type access_type)
> > +static bool access_is_atomic(u32 access_type)
> > {
> > -	return access_type == ACCESS_TYPE_ATOMIC;
> > +	return access_type == FLT_ACCESS_TYPE_ATOMIC;
> > }
> > 
> > static bool vma_is_valid(struct xe_tile *tile, struct xe_vma *vma)
> > @@ -205,7 +180,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 +212,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 != FLT_ACCESS_TYPE_READ) {
> > 		err = -EPERM;
> > 		goto unlock_vm;
> > 	}
> > @@ -271,7 +246,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 +266,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 +353,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..b7d41b558de3
> > --- /dev/null
> > +++ b/drivers/gpu/drm/xe/xe_gt_pagefault_types.h
> > @@ -0,0 +1,42 @@
> > +/* 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 */
> > +	u8 access_type;
> > +	/** @fault_type: fault type of this pagefault */
> > +	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;
> > +};
> > +
> > +#endif
> > diff --git a/drivers/gpu/drm/xe/xe_guc_fwif.h b/drivers/gpu/drm/xe/xe_guc_fwif.h
> > index 6f57578b07cb..30ac21bb4f15 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_fwif.h
> > +++ b/drivers/gpu/drm/xe/xe_guc_fwif.h
> > @@ -290,34 +290,6 @@ enum xe_guc_response_desc_type {
> > 	FAULT_RESPONSE_DESC
> > };
> > 
> > -struct xe_guc_pagefault_desc {
> > -	u32 dw0;
> > -#define PFD_FAULT_LEVEL		GENMASK(2, 0)
> > -#define PFD_SRC_ID		GENMASK(10, 3)
> > -#define PFD_RSVD_0		GENMASK(17, 11)
> > -#define XE2_PFD_TRVA_FAULT	BIT(18)
> > -#define PFD_ENG_INSTANCE	GENMASK(24, 19)
> > -#define PFD_ENG_CLASS		GENMASK(27, 25)
> > -#define PFD_PDATA_LO		GENMASK(31, 28)
> > -
> > -	u32 dw1;
> > -#define PFD_PDATA_HI		GENMASK(11, 0)
> > -#define PFD_PDATA_HI_SHIFT	4
> > -#define PFD_ASID		GENMASK(31, 12)
> > -
> > -	u32 dw2;
> > -#define PFD_ACCESS_TYPE		GENMASK(1, 0)
> > -#define PFD_FAULT_TYPE		GENMASK(3, 2)
> > -#define PFD_VFID		GENMASK(9, 4)
> > -#define PFD_RSVD_1		GENMASK(11, 10)
> > -#define PFD_VIRTUAL_ADDR_LO	GENMASK(31, 12)
> > -#define PFD_VIRTUAL_ADDR_LO_SHIFT 12
> > -
> > -	u32 dw3;
> > -#define PFD_VIRTUAL_ADDR_HI	GENMASK(31, 0)
> > -#define PFD_VIRTUAL_ADDR_HI_SHIFT 32
> > -} __packed;
> > -
> > struct xe_guc_pagefault_reply {
> > 	u32 dw0;
> > #define PFR_VALID		BIT(0)
> > -- 
> > 2.43.0
> > 


More information about the Intel-xe mailing list