On Mon, April 21, 2025 1:34 PM Cavitt, Jonathan 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.
> 
> 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.cav...@intel.com>
> Cc: Michal Wajdeczko <michal.wajdec...@intel.com>

LGTM.
Reviewed-by: Shuicheng Lin <shuicheng....@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)

we generally define bits/fields in descending order rather than ascending. 
But here, as PFD_PDATA_LO is combined with PFD_PDATA_HI in dw1, I am OK with 
current order.

Shuicheng

> +
> +     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

Reply via email to