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

[Michal Wajdeczko]

On 29.04.2025 16:22, 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.
>>
>> 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

maybe also mention 59654 here?

>> 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
> 
> Maybe change the file name to xe_guc_pagefault_desc.h ,
> since this is currently guc specific.

IMO 'guc' tag is not applicable here

my understanding was that GuC sends data as it was read from the HW
regs, so the origin of the struct layout is HW, hence we put it in regs/
where we have all the HW defs, not in the abi/ folder which is for FW defs.

unless we want to make this struct part of the stable GuC pagefault ABI,
and then even if HW definition/layout would change, we will stick with
current layout. Then agree, we should move all this to abi/ and also
drop the Bspec reference as n/a

> 
> Also, the define 'PF_MSG_LEN_DW    4' relates to the
> length of this struct so should move here.

any G2H message related definitions should be in the abi/ GuC files, not
here in regs/ where we keep HW definitions.

and please don't start define name with "PF", use "GUC" instead

and btw, in many places by message length we assume also HXG header
length, so the actual length of the G2H 6002 message is 5 as it includes
1 DW of header with DATA0 and 4 DWs of payload with DATA1..4

#define GUC2HOST_NOTIFY_PAGE_FAULT_REQ_DESC_MSG_LEN \
	(GUC_HXG_REQUEST_MSG_MIN_LEN + 4u)

> 
> Thanks,
> Dafna
>