Re: [PATCH v2 05/11] drm/xe/sriov: Add handling for MLRC adverse event threshold

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

On 12/8/2025 9:52 AM, Daniele Ceraolo Spurio wrote:
>
>
> On 12/7/2025 2:03 PM, Michal Wajdeczko wrote:
>>
>> On 12/7/2025 12:04 AM, Daniele Ceraolo Spurio wrote:
>>> Since it is illegal to register a MLRC context when scheduler groups 
>>> are
>>> enabled, the GuC consider the VF doing so as an adverse event. Like for
>>> other adverse event, there is a threshold for how many times the event
>>> can happen before the GuC throws an error, which we need to add support
>>> for.
>>>
>>> Since this is the first threshold that we have that has a minimum GuC
>>> version requirement, support for checking that has been added to the
>>> generic threshold handling. As part of it, some of the version code has
>>> been moved to its own file and with the occasion some SRIOV
>>> documentation has been added.
>>>
>>> v2: split from previous patch, add GuC version checking
>>>
>>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/abi/guc_klvs_abi.h         |  9 +++++
>>>   drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c    | 19 ++++++----
>>>   drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c   |  9 +++--
>>>   drivers/gpu/drm/xe/xe_guc.h                   |  7 +---
>>>   .../drm/xe/xe_guc_klv_thresholds_set_types.h  | 18 +++++-----
>>>   drivers/gpu/drm/xe/xe_guc_version.h           | 36 
>>> +++++++++++++++++++
>>>   6 files changed, 74 insertions(+), 24 deletions(-)
>>>   create mode 100644 drivers/gpu/drm/xe/xe_guc_version.h
>>>
>>> diff --git a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h 
>>> b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
>>> index edb0546fb163..30a051a0b4ee 100644
>>> --- a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
>>> +++ b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
>>> @@ -376,6 +376,12 @@ enum  {
>>>    *      :1: NORMAL = schedule VF always, irrespective of whether 
>>> it has work or not
>>>    *      :2: HIGH = schedule VF in the next time-slice after 
>>> current active
>>>    *          time-slice completes if it has active work
>>> + *
>>> + * _`GUC_KLV_VF_CFG_THRESHOLD_MULTI_LRC_COUNT` : 0x8A0D
>>> + *      Given that multi-LRC contexts are incompatible with SRIOV 
>>> scheduler
>>> + *      groups and cause the latter to be turned off when 
>>> registered with the
>>> + *      GuC, this config allows the PF to set a threshold for 
>>> multi-LRC context
>>> + *      registrations by VFs to monitor their behavior.
>>>    */
>>>     #define GUC_KLV_VF_CFG_GGTT_START_KEY        0x0001
>>> @@ -434,6 +440,9 @@ enum  {
>>>   #define   GUC_SCHED_PRIORITY_NORMAL        1u
>>>   #define   GUC_SCHED_PRIORITY_HIGH        2u
>>>   +#define GUC_KLV_VF_CFG_THRESHOLD_MULTI_LRC_COUNT_KEY 0x8a0d
>>> +#define GUC_KLV_VF_CFG_THRESHOLD_MULTI_LRC_COUNT_LEN    1u
>>> +
>>>   /*
>>>    * Workaround keys:
>>>    */
>>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c 
>>> b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
>>> index 59c5c6b4d994..dda671d05b89 100644
>>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
>>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
>>> @@ -269,7 +269,8 @@ static u32 encode_config_ggtt(u32 *cfg, const 
>>> struct xe_gt_sriov_config *config,
>>>   }
>>>     /* Return: number of configuration dwords written */
>>> -static u32 encode_config(u32 *cfg, const struct xe_gt_sriov_config 
>>> *config, bool details)
>>> +static u32 encode_config(struct xe_gt *gt, u32 *cfg,
>>> +             const struct xe_gt_sriov_config *config, bool details)
>>>   {
>>>       u32 n = 0;
>>>   @@ -303,9 +304,11 @@ static u32 encode_config(u32 *cfg, const 
>>> struct xe_gt_sriov_config *config, bool
>>>       cfg[n++] = PREP_GUC_KLV_TAG(VF_CFG_PREEMPT_TIMEOUT);
>>>       cfg[n++] = config->preempt_timeout;
>>>   -#define encode_threshold_config(TAG, ...) ({                    \
>>> -    cfg[n++] = PREP_GUC_KLV_TAG(VF_CFG_THRESHOLD_##TAG);            \
>>> -    cfg[n++] = 
>>> config->thresholds[MAKE_XE_GUC_KLV_THRESHOLD_INDEX(TAG)]; \
>>> +#define encode_threshold_config(TAG, NAME, MIN_GUC_VER) 
>>> ({                \
>>> +    if (!MIN_GUC_VER || GUC_FIRMWARE_VER(&gt->uc.guc) >= 
>>> MIN_GUC_VER) {        \
>>> +        cfg[n++] = 
>>> PREP_GUC_KLV_TAG(VF_CFG_THRESHOLD_##TAG);            \
>>> +        cfg[n++] = 
>>> config->thresholds[MAKE_XE_GUC_KLV_THRESHOLD_INDEX(TAG)]; \
>>> +    }                                        \
>>>   });
>>> MAKE_XE_GUC_KLV_THRESHOLDS_SET(encode_threshold_config);
>>> @@ -328,7 +331,7 @@ static int pf_push_full_vf_config(struct xe_gt 
>>> *gt, unsigned int vfid)
>>>           return -ENOBUFS;
>>>         cfg = xe_guc_buf_cpu_ptr(buf);
>>> -    num_dwords = encode_config(cfg, config, true);
>>> +    num_dwords = encode_config(gt, cfg, config, true);
>>>       xe_gt_assert(gt, num_dwords <= max_cfg_dwords);
>>>         if (xe_gt_is_media_type(gt)) {
>>> @@ -2518,7 +2521,7 @@ ssize_t xe_gt_sriov_pf_config_save(struct 
>>> xe_gt *gt, unsigned int vfid, void *bu
>>>               ret = -ENOBUFS;
>>>           } else {
>>>               config = pf_pick_vf_config(gt, vfid);
>>> -            ret = encode_config(buf, config, false) * sizeof(u32);
>>> +            ret = encode_config(gt, buf, config, false) * sizeof(u32);
>>>           }
>>>       }
>>>       mutex_unlock(xe_gt_sriov_pf_master_mutex(gt));
>>> @@ -2551,9 +2554,11 @@ static int pf_restore_vf_config_klv(struct 
>>> xe_gt *gt, unsigned int vfid,
>>>           return pf_provision_preempt_timeout(gt, vfid, value[0]);
>>>         /* auto-generate case statements */
>>> -#define define_threshold_key_to_provision_case(TAG, 
>>> ...)                \
>>> +#define define_threshold_key_to_provision_case(TAG, NAME, 
>>> MIN_GUC_VER)            \
>>>       case MAKE_GUC_KLV_VF_CFG_THRESHOLD_KEY(TAG):                    \
>>>           BUILD_BUG_ON(MAKE_GUC_KLV_VF_CFG_THRESHOLD_LEN(TAG) != 
>>> 1u);        \
>>> +        if (MIN_GUC_VER && GUC_FIRMWARE_VER(&gt->uc.guc) < 
>>> MIN_GUC_VER) \
>>> +            return -ENOKEY;                            \
>>>           if (len != 
>>> MAKE_GUC_KLV_VF_CFG_THRESHOLD_LEN(TAG))            \
>>>               return -EBADMSG;                        \
>>>           return pf_provision_threshold(gt, vfid,                    \
>>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c 
>>> b/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
>>> index 0fd863609848..5123ff1fb116 100644
>>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
>>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c
>>> @@ -21,6 +21,7 @@
>>>   #include "xe_gt_sriov_pf_monitor.h"
>>>   #include "xe_gt_sriov_pf_policy.h"
>>>   #include "xe_gt_sriov_pf_service.h"
>>> +#include "xe_guc.h"
>>>   #include "xe_pm.h"
>>>   #include "xe_sriov_pf.h"
>>>   #include "xe_sriov_pf_provision.h"
>>> @@ -301,9 +302,11 @@ static void pf_add_config_attrs(struct xe_gt 
>>> *gt, struct dentry *parent, unsigne
>>>                      &sched_priority_fops);
>>>         /* register all threshold attributes */
>>> -#define register_threshold_attribute(TAG, NAME, ...) \
>>> -    debugfs_create_file_unsafe("threshold_" #NAME, 0644, parent, 
>>> parent, \
>>> -                   &NAME##_fops);
>>> +#define register_threshold_attribute(TAG, NAME, MIN_GUC_VER) 
>>> ({                \
>>> +    if (!MIN_GUC_VER || GUC_FIRMWARE_VER(&gt->uc.guc) >= 
>>> MIN_GUC_VER)        \
>>> +        debugfs_create_file_unsafe("threshold_" #NAME, 0644, 
>>> parent, parent,    \
>>> +                       &NAME##_fops);                \
>>> +});
>>> MAKE_XE_GUC_KLV_THRESHOLDS_SET(register_threshold_attribute)
>>>   #undef register_threshold_attribute
>>>   }
>>> diff --git a/drivers/gpu/drm/xe/xe_guc.h b/drivers/gpu/drm/xe/xe_guc.h
>>> index fdb08658d05a..9028718189ed 100644
>>> --- a/drivers/gpu/drm/xe/xe_guc.h
>>> +++ b/drivers/gpu/drm/xe/xe_guc.h
>>> @@ -8,15 +8,10 @@
>>>     #include "xe_gt.h"
>>>   #include "xe_guc_types.h"
>>> +#include "xe_guc_version.h"
>>>   #include "xe_hw_engine_types.h"
>>>   #include "xe_macros.h"
>>>   -/*
>>> - * GuC version number components are defined to be only 8-bit size,
>>> - * so converting to a 32bit 8.8.8 integer allows simple (and safe)
>>> - * numerical comparisons.
>>> - */
>>> -#define MAKE_GUC_VER(maj, min, pat)    (((maj) << 16) | ((min) << 
>>> 8) | (pat))
>>>   #define MAKE_GUC_VER_STRUCT(ver) MAKE_GUC_VER((ver).major, 
>>> (ver).minor, (ver).patch)
>> I guess this macro can also be moved
>
> I purposely didn't move this as MAKE_GUC_VER_STRUCT is specific to how 
> we code xe_uc_fw_version, while MAKE_GUC_VER is based on what the GuC 
> interface define.
>
>>
>>>   #define GUC_SUBMIT_VER(guc) \
>>> MAKE_GUC_VER_STRUCT((guc)->fw.versions.found[XE_UC_FW_VER_COMPATIBILITY])
>>> diff --git a/drivers/gpu/drm/xe/xe_guc_klv_thresholds_set_types.h 
>>> b/drivers/gpu/drm/xe/xe_guc_klv_thresholds_set_types.h
>>> index 0a028c94756d..f7ed32244c6b 100644
>>> --- a/drivers/gpu/drm/xe/xe_guc_klv_thresholds_set_types.h
>>> +++ b/drivers/gpu/drm/xe/xe_guc_klv_thresholds_set_types.h
>>> @@ -7,6 +7,7 @@
>>>   #define _XE_GUC_KLV_THRESHOLDS_SET_TYPES_H_
>>>     #include "xe_args.h"
>>> +#include "xe_guc_version.h"
>>>     /**
>>>    * MAKE_XE_GUC_KLV_THRESHOLDS_SET - Generate various GuC 
>>> thresholds definitions.
>>> @@ -23,15 +24,16 @@
>>>    * with the &TAG, that corresponds to the GuC threshold KLV key 
>>> name defined by
>>>    * ABI and the associated &NAME, that may be used in code or 
>>> debugfs/sysfs::
>>>    *
>>> - *    define(TAG, NAME)
>>> + *    define(TAG, NAME, MIN_GUC_VER)
>>>    */
>>> -#define MAKE_XE_GUC_KLV_THRESHOLDS_SET(define)        \
>>> -    define(CAT_ERR, cat_error_count)        \
>>> -    define(ENGINE_RESET, engine_reset_count)    \
>>> -    define(PAGE_FAULT, page_fault_count)        \
>>> -    define(H2G_STORM, guc_time_us)            \
>>> -    define(IRQ_STORM, irq_time_us)            \
>>> -    define(DOORBELL_STORM, doorbell_time_us)    \
>>> +#define MAKE_XE_GUC_KLV_THRESHOLDS_SET(define)                    \
>>> +    define(CAT_ERR, cat_error_count, 0)                    \
>>> +    define(ENGINE_RESET, engine_reset_count, 0)                \
>>> +    define(PAGE_FAULT, page_fault_count, 0)                    \
>>> +    define(H2G_STORM, guc_time_us, 0)                    \
>>> +    define(IRQ_STORM, irq_time_us, 0)                    \
>>> +    define(DOORBELL_STORM, doorbell_time_us, 0)                \
>>> +    define(MULTI_LRC_COUNT, multi_lrc_count, MAKE_GUC_VER(70, 53, 
>>> 0))    \
>>>       /* end */
>>>     /**
>>> diff --git a/drivers/gpu/drm/xe/xe_guc_version.h 
>>> b/drivers/gpu/drm/xe/xe_guc_version.h
>>> new file mode 100644
>> introduction of this new ver.h file is self-contained so maybe it 
>> should be in its own patch?
>
> IMO it is simple enough to keep here, to avoid too many small patches 
> in the series.
>
>>
>>> index 000000000000..e6f80abd2f05
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/xe/xe_guc_version.h
>>> @@ -0,0 +1,36 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2025 Intel Corporation
>>> + */
>>> +
>>> +#ifndef _XE_GUC_VERSION_H_
>>> +#define _XE_GUC_VERSION_H_
>>> +
>>> +/*
>> this should be regular kernel-doc
>>
>>> + * GuC version number components are defined to be only 8-bit size,
>>> + * so converting to a 32bit 8.8.8 integer allows simple (and safe)
>>> + * numerical comparisons.
>>> + */
>>> +#define MAKE_GUC_VER(maj, min, pat)    (((maj) << 16) | ((min) << 
>>> 8) | (pat))
>>> +
>>> +/**
>>> + * DOC: SRIOV-changes
>>     DOC: SR-IOV Changes
>>
>>> + *
>>> + * We record SRIOV-specific changes here as those need to be 
>>> tracked carefully.
>>> + *
>> what about 1.23.0 (CCS) ?
>
> If you tell me exactly what to write I'll add it in, because I don't 
> know the specifics.
>
>>
>>> + * GuC 70.53.0 (VF interface 1.26.0):
>>> + *
>>> + * Added support for EGS. See:
>> probably we need extra line here to render correctly
>>
>>> + *  * GUC_KLV_VGT_POLICY_ENGINE_GROUP_CONFIG
>>> + *  * GUC_KLV_VF_CFG_THRESHOLD_MULTI_LRC_COUNT
>>> + *
>>> + * GuC 70.54.0 (VF interface 1.27.0):
>>> + *
>>> + * Updated VF migration support. See RESFIX actions
>> maybe we can list those actions:
>>
>>     * VF2GUC_RESFIX_START
>>     * VF2GUC_RESFIX_DONE
>
> Those don't seem to yet be in the driver. Should I list them anyway?

Ignore this question, I was looking in an older tree.

Daniele

>
> Daniele
>
>>> + *
>>> + * GuC 70.55.1 (VF interface 1.28.1):
>>> + *
>>> + * Fixes for EGS.
>>> + */
>>> +
>>> +#endif
>