Re: [RFC 11/15] drm/xe: Add xe_guc_access_counter layer

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Mar 18, 2026 at 01:14:52PM +0530, Himal Prasad Ghimiray wrote:
> Add GuC to host (G2H) access counter notification handler to parse
> GuC firmware messages into struct xe_access_counter and forward to
> xe_access_counter_handler for processing.
> 
> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/xe/Makefile                |  1 +
>  drivers/gpu/drm/xe/xe_guc_access_counter.c | 62 ++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_guc_access_counter.h | 15 ++++++
>  drivers/gpu/drm/xe/xe_guc_ct.c             |  4 ++
>  4 files changed, 82 insertions(+)
>  create mode 100644 drivers/gpu/drm/xe/xe_guc_access_counter.c
>  create mode 100644 drivers/gpu/drm/xe/xe_guc_access_counter.h
> 
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index 92d8d6e4a447..296b3cba0b89 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -74,6 +74,7 @@ xe-y += xe_access_counter.o \
>  	xe_guc_id_mgr.o \
>  	xe_guc_klv_helpers.o \
>  	xe_guc_log.o \
> +	xe_guc_access_counter.o \
>  	xe_guc_pagefault.o \
>  	xe_guc_pc.o \
>  	xe_guc_rc.o \
> diff --git a/drivers/gpu/drm/xe/xe_guc_access_counter.c b/drivers/gpu/drm/xe/xe_guc_access_counter.c
> new file mode 100644
> index 000000000000..2158400bc50a
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_guc_access_counter.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#include "xe_guc_access_counter.h"
> +
> +#include "xe_access_counter.h"
> +#include "xe_device.h"
> +#include "xe_gt.h"
> +#include "xe_guc.h"
> +#include "xe_guc_fwif.h"
> +
> +/**
> + * xe_guc_access_counter_handler() - G2H access counter handler
> + * @guc: GuC object
> + * @msg: G2H message
> + * @len: Length of G2H message
> + *
> + * Parse GuC to host (G2H) message into a struct xe_access_counter and forward
> + * onto the Xe access counter layer.
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +int xe_guc_access_counter_handler(struct xe_guc *guc, u32 *msg, u32 len)
> +{
> +	struct xe_access_counter ac;
> +	struct xe_device *xe = guc_to_xe(guc);
> +	int i;
> +
> +#define GUC_ACC_MSG_LEN_DW	\
> +	(sizeof(struct xe_guc_acc_desc) / sizeof(u32))
> +
> +	BUILD_BUG_ON(GUC_ACC_MSG_LEN_DW > XE_ACCESS_COUNTER_PRODUCER_MSG_LEN_DW);
> +
> +	if (len != GUC_ACC_MSG_LEN_DW)
> +		return -EPROTO;
> +
> +	ac.gt = guc_to_gt(guc);
> +
> +	/* Parse access counter descriptor */
> +	ac.consumer.granularity = FIELD_GET(ACC_GRANULARITY, msg[2]);
> +	ac.consumer.sub_granularity = FIELD_GET(ACC_SUBG_HI, msg[1]) << 31 |
> +				      FIELD_GET(ACC_SUBG_LO, msg[0]);

Would it be better to move the math in xe_access_counter_get_vma() for
calculating page_va into this layer and simply pass in page_va (and
perhaps a generic length)? That would better isolate the details of the
GuC access counter interface. It would also ensure that, if we wire up a
different hardware access counter interface in the future, the common
access counter layer can remain unchanged.

> +	ac.consumer.counter_type = FIELD_GET(ACC_TYPE, msg[0]);

Should we reject bad count types here? I think that makes more sense.
The only reason we don’t reject bad page faults in the G2H handler is
because page faults issue an ACK, whereas we don’t ACK access counters.

> +	ac.consumer.va_range_base = ((u64)(msg[3] & ACC_VIRTUAL_ADDR_RANGE_HI) << 32) |
> +				    (msg[2] & ACC_VIRTUAL_ADDR_RANGE_LO);
> +	/* xe3: Use ASID and engine info */
> +	ac.consumer.xe3.asid = FIELD_GET(ACC_ASID, msg[1]);
> +	ac.consumer.xe3.engine_class = FIELD_GET(ACC_ENG_CLASS, msg[1]);
> +	ac.consumer.xe3.engine_instance = FIELD_GET(ACC_ENG_INSTANCE, msg[1]);
> +	ac.consumer.xe3.vfid = FIELD_GET(ACC_VFID, msg[2]);
> +
> +	/* Store producer message for potential acknowledgment */
> +	ac.producer.private = guc;
> +	for (i = 0; i < GUC_ACC_MSG_LEN_DW; ++i)
> +		ac.producer.msg[i] = msg[i];

Do we need this? That is, we don’t currently ACK access counters—are
there plans to change this? If there no immediate plans, I'd drop this
for now.

Matt

> +
> +#undef GUC_ACC_MSG_LEN_DW
> +
> +	return xe_access_counter_handler(xe, &ac);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_guc_access_counter.h b/drivers/gpu/drm/xe/xe_guc_access_counter.h
> new file mode 100644
> index 000000000000..1ac8e76398d2
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_guc_access_counter.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#ifndef _XE_GUC_ACCESS_COUNTER_H_
> +#define _XE_GUC_ACCESS_COUNTER_H_
> +
> +#include <linux/types.h>
> +
> +struct xe_guc;
> +
> +int xe_guc_access_counter_handler(struct xe_guc *guc, u32 *msg, u32 len);
> +
> +#endif
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index a11cff7a20be..8ac0938f7a28 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -26,6 +26,7 @@
>  #include "xe_gt_sriov_pf_monitor.h"
>  #include "xe_guc.h"
>  #include "xe_guc_log.h"
> +#include "xe_guc_access_counter.h"
>  #include "xe_guc_pagefault.h"
>  #include "xe_guc_relay.h"
>  #include "xe_guc_submit.h"
> @@ -1630,6 +1631,9 @@ static int process_g2h_msg(struct xe_guc_ct *ct, u32 *msg, u32 len)
>  	case XE_GUC_ACTION_REPORT_PAGE_FAULT_REQ_DESC:
>  		ret = xe_guc_pagefault_handler(guc, payload, adj_len);
>  		break;
> +	case XE_GUC_ACTION_ACCESS_COUNTER_NOTIFY:
> +		ret = xe_guc_access_counter_handler(guc, payload, adj_len);
> +		break;
>  	case XE_GUC_ACTION_TLB_INVALIDATION_DONE:
>  		ret = xe_guc_tlb_inval_done_handler(guc, payload, adj_len);
>  		break;
> -- 
> 2.34.1
>