Re: [PATCH i-g-t 3/4] eudebug: Add eudebug pagefault event declarations

["Hajda, Andrzej" <andrzej.hajda@intel.com>]

W dniu 15.11.2024 o 15:11, Gwan-gyeong Mun pisze:
> From: Jonathan Cavitt <jonathan.cavitt@intel.com>
> 
> Update the xe_eudebug drm to include pagefault events.
> 
> Co-developed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> ---
>   include/drm-uapi-experimental/xe_drm_eudebug.h | 13 +++++++++++++

I am lost between prelim/eudebug/experimental header placements. So just 
out of curiosity what is the role of drm-uapi-experimental comparing to 
prelim or just drm-uapi?


>   lib/xe/xe_eudebug.c                            | 11 +++++++++++
>   2 files changed, 24 insertions(+)
> 
> diff --git a/include/drm-uapi-experimental/xe_drm_eudebug.h b/include/drm-uapi-experimental/xe_drm_eudebug.h
> index f50051e9a..ed2e728c2 100644
> --- a/include/drm-uapi-experimental/xe_drm_eudebug.h
> +++ b/include/drm-uapi-experimental/xe_drm_eudebug.h
> @@ -154,6 +154,7 @@ struct drm_xe_eudebug_event {
>   #define DRM_XE_EUDEBUG_EVENT_VM_BIND_UFENCE	9
>   #define DRM_XE_EUDEBUG_EVENT_METADATA		10
>   #define DRM_XE_EUDEBUG_EVENT_VM_BIND_OP_METADATA 11
> +#define DRM_XE_EUDEBUG_EVENT_PAGEFAULT		12
>   
>   	__u16 flags;
>   #define DRM_XE_EUDEBUG_EVENT_CREATE		(1 << 0)
> @@ -352,6 +353,18 @@ struct drm_xe_eudebug_event_vm_bind_op_metadata {
>   	__u64 metadata_cookie;
>   };
>   
> +struct drm_xe_eudebug_event_pagefault {
> +	struct drm_xe_eudebug_event base;
> +
> +	__u64 client_handle;
> +	__u64 exec_queue_handle;
> +	__u64 lrc_handle;
> +	__u32 flags;
> +	__u32 bitmask_size;
> +	__u64 pagefault_address;
> +	__u8 bitmask[];
> +};
> +
>   #if defined(__cplusplus)
>   }
>   #endif
> diff --git a/lib/xe/xe_eudebug.c b/lib/xe/xe_eudebug.c
> index c5814421b..de01e6b49 100644
> --- a/lib/xe/xe_eudebug.c
> +++ b/lib/xe/xe_eudebug.c
> @@ -91,6 +91,8 @@ static const char *type_to_str(unsigned int type)
>   		return "metadata";
>   	case DRM_XE_EUDEBUG_EVENT_VM_BIND_OP_METADATA:
>   		return "vm_bind_op_metadata";
> +	case DRM_XE_EUDEBUG_EVENT_PAGEFAULT:
> +		return "pagefault";
>   	}
>   
>   	return "UNKNOWN";
> @@ -222,6 +224,15 @@ static const char *event_members_to_str(struct drm_xe_eudebug_event *e, char *bu
>   			op->vm_bind_op_ref_seqno, op->metadata_handle, op->metadata_cookie);
>   		break;
>   	}
> +	case DRM_XE_EUDEBUG_EVENT_PAGEFAULT: {
> +		struct drm_xe_eudebug_event_pagefault *pf = (void *)e;

maybe better: pf = container_of(e, pf, base);

Beside this:

Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>


Regards
Andrzej

> +
> +		sprintf(buf, "client_handle=%llu, exec_queue_handle=%llu, "
> +			"lrc_handle=%llu, bitmask_size=%d, pagefault_address=0x%llx",
> +			pf->client_handle, pf->exec_queue_handle, pf->lrc_handle,
> +			pf->bitmask_size, pf->pagefault_address);
> +		break;
> +	}
>   	default:
>   		strcpy(buf, "<...>");
>   	}