Re: [PATCH 04/10] drm/xe/sriov: Scheduler groups are incompatible with multi-lrc

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 12/2/2025 10:25 PM, Daniele Ceraolo Spurio wrote:
> 
> 
> On 12/2/2025 1:17 PM, Michal Wajdeczko wrote:
>>
>> On 12/2/2025 6:57 PM, Daniele Ceraolo Spurio wrote:
>>>
>>> On 12/2/2025 5:32 AM, Michal Wajdeczko wrote:
>>>> On 11/27/2025 2:45 AM, Daniele Ceraolo Spurio wrote:
>>>>> Since engines in the same class can be divided across multiple groups,
>>>>> the GuC does not allow scheduler groups to be active if there are
>>>>> multi-lrc contexts. This means that:
>>>>>
>>>>> 1) if a mlrc context is registered when we enable scheduler groups, the
>>>>>      GuC will silently ignore the configuration
>>>>> 2) if a mlrc context is registered after scheduler groups are enabled,
>>>>>      the GuC will disable the groups and generate an adverse event.
>>>>>
>>>>> We therefore need to block mlrc context creation when scheduler groups
>>>>> are enabled.
>>>> s/mlrc/MLRC
>>>>
>>>>> An adverse event threshold is available for the new adverse event.
>>>> changes related to introduction of new threshold deserves separate patch
>>>> (as we need to handle GuC FW version checks)
>>> ok
>>>
>>>>> 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         | 14 +++++
>>>>>    drivers/gpu/drm/xe/xe_exec_queue.c            | 19 ++++++
>>>>>    drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c    | 30 ++++++++++
>>>>>    drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h    |  1 +
>>>>>    drivers/gpu/drm/xe/xe_gt_sriov_vf.c           | 59 +++++++++++++++++++
>>>>>    drivers/gpu/drm/xe/xe_gt_sriov_vf.h           |  3 +
>>>>>    drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h     |  2 +
>>>>>    .../drm/xe/xe_guc_klv_thresholds_set_types.h  |  1 +
>>>>>    8 files changed, 129 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
>>>>> index 274f1b1ec37f..a6dce9da339f 100644
>>>>> --- a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
>>>>> +++ b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h
>>>>> @@ -46,11 +46,18 @@
>>>>>     *      Refers to 32 bit architecture version as reported by the HW IP.
>>>>>     *      This key is supported on MTL+ platforms only.
>>>>>     *      Requires GuC ABI 1.2+.
>>>>> + *
>>>>> + * _`GUC_KLV_GLOBAL_CFG_GROUP_SCHEDULING_AVAILABLE` : 0x3001
>>>>> + *      Tells the driver whether scheduler groups are enabled or not.
>>>>> + *      Requres GuC ABI 1.26+
>>>> typo: Requires
>>>>
>>>> and don't forget to update xe_guc_klv_key_to_string() with new KEY
>>> ok
>>>
>>>>>     */
>>>>>      #define GUC_KLV_GLOBAL_CFG_GMD_ID_KEY            0x3000u
>>>>>    #define GUC_KLV_GLOBAL_CFG_GMD_ID_LEN            1u
>>>>>    +#define GUC_KLV_GLOBAL_CFG_GROUP_SCHEDULING_AVAILABLE_KEY    0x3001u
>>>>> +#define GUC_KLV_GLOBAL_CFG_GROUP_SCHEDULING_AVAILABLE_LEN    1u
>>>>> +
>>>>>    /**
>>>>>     * DOC: GuC Self Config KLVs
>>>>>     *
>>>>> @@ -369,6 +376,10 @@ 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
>>>>> + *      This config sets the threshold for LRCA context registration when SRIOV
>>>> ... threshold for _Multi_ LRCA context registrations ...
>>> ooops :)
>>>
>>>>> + *      scheduler groups are enabled.
>>>> "This is allows PF to monitor VFs' behavior when EGS is enabled."
>>>>
>>>>>     */
>>>>>      #define GUC_KLV_VF_CFG_GGTT_START_KEY        0x0001
>>>>> @@ -427,6 +438,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_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
>>>>> index 8724f8de67e2..e59c41c913b4 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
>>>>> @@ -16,6 +16,7 @@
>>>>>    #include "xe_dep_scheduler.h"
>>>>>    #include "xe_device.h"
>>>>>    #include "xe_gt.h"
>>>>> +#include "xe_gt_sriov_pf_policy.h"
>>>>>    #include "xe_gt_sriov_vf.h"
>>>>>    #include "xe_hw_engine_class_sysfs.h"
>>>>>    #include "xe_hw_engine_group.h"
>>>>> @@ -698,6 +699,17 @@ static u32 calc_validate_logical_mask(struct xe_device *xe,
>>>>>        return return_mask;
>>>>>    }
>>>>>    +static bool has_sched_groups(struct xe_gt *gt)
>>>>> +{
>>>>> +    if (IS_SRIOV_PF(gt_to_xe(gt)) && xe_gt_sriov_pf_policy_sched_groups_enabled(gt))
>>>> hmm, usually we don't want core code to look so deeply into PF subcomponent
>>> I thought about that, but I didn't know which sriov file would be the right one, since almost all of them are either pf-only or vf-only. should I just move it to xe_sriov.c?
>> hmm, but xe_sriov.c is device level function
>>
>> we might add helper to xe_gt_sriov_pf.h where we can also provide stub there:
>>
>> #ifdef CONFIG_PCI_IOV
>> bool xe_gt_sriov_pf_sched_groups_enabled(gt);
>> #else
>> static inline bool xe_gt_sriov_pf_sched_groups_enabled(gt) { return false; }
>> #endif
> 
> Do you mean to check for both PF and VF from that function, and just have it in the PF file for convenience?

this is PF file and only PF functions might require stubs (when PCI_IOV=n)
so it is just to hide PF internals

VF function is already exposed at GT level

> I could also just create a new xe_gt_sriov.h file and place it there, making the function an inline.

maybe later ;) we already have many files

> 
>>
>>>> also, do we want to work in hybrid mode where one GT is using MLRC and other is not?
>>> The GuCs allow it. I didn't want to block it in case we do get a scenario later where this is needed (e.g. we might get a split primary GT but still want to do mlrc on media)
>>>
>>>>> +        return true;
>>>>> +> +    if (IS_SRIOV_VF(gt_to_xe(gt)) && xe_gt_sriov_vf_sched_groups_enabled(gt))
>>>>> +        return true;
>>>>> +
>>>>> +    return false;
>>>>> +}
>>>>> +
>>>>>    int xe_exec_queue_create_ioctl(struct drm_device *dev, void *data,
>>>>>                       struct drm_file *file)
>>>>>    {
>>>>> @@ -790,6 +802,13 @@ int xe_exec_queue_create_ioctl(struct drm_device *dev, void *data,
>>>>>                return -ENOENT;
>>>>>            }
>>>>>    +        /* SRIOV sched groups are not compatible with multi-lrc */
>>>>> +        if (XE_IOCTL_DBG(xe, args->width > 1 && has_sched_groups(hwe->gt))) {
>>>>> +            up_read(&vm->lock);
>>>>> +            xe_vm_put(vm);
>>>>> +            return -EINVAL;
>>>>> +        }
>>>>> +
>>>>>            q = xe_exec_queue_create(xe, vm, logical_mask,
>>>>>                         args->width, hwe, flags,
>>>>>                         args->extensions);
>>>>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c
>>>>> index 48f250ae0d0d..c7f1ea8eb9c5 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.c
>>>>> @@ -8,6 +8,7 @@
>>>>>    #include <drm/drm_managed.h>
>>>>>      #include "xe_bo.h"
>>>>> +#include "xe_exec_queue_types.h"
>>>>>    #include "xe_gt.h"
>>>>>    #include "xe_gt_sriov_pf_helpers.h"
>>>>>    #include "xe_gt_sriov_pf_policy.h"
>>>>> @@ -527,6 +528,24 @@ static int __pf_provision_sched_groups(struct xe_gt *gt, u32 mode)
>>>>>                          masks, num_masks);
>>>>>    }
>>>>>    +static bool guc_has_mlrc_queue(struct xe_guc *guc)
>>>> this is all GuC stuff, so export it from xe_guc_submission.c
>>> ok
>>>
>>>>> +{
>>>>> +    struct xe_exec_queue *q;
>>>>> +    unsigned long index;
>>>>> +    bool found = false;
>>>>> +
>>>>> +    mutex_lock(&guc->submission_state.lock);
>>>> guard(mutex) ?
>>>>
>>>>> +    xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) {
>>>>> +        if (q->width > 1) {
>>>>> +            found = true;
>>>>> +            break;
>>>>> +        }
>>>>> +    }
>>>>> +    mutex_unlock(&guc->submission_state.lock);
>>>>> +
>>>>> +    return found;
>>>> what if new MLRC is created right now?
>>>>
>>>> maybe we should use xe_guard to lockdown one feature or the other?
>>> The idea is that the admin is responsible for enabling EGS when the system is in the correct state, which is why the GuC doesn't return an error and just ignore the KLV if the system is not in the correct state. This check is there to catch the case where the admin has closed all apps in preparation to enabling EGS but the driver still hasn't processed all the context de-registrations. Therefore, I don't really want to over-complicate it with extra locking. I'll add a comment to better explain.
>> so it doesn't have to be bullet-proof ?
> 
> Yup. If we had stricter requirements then the GuC would enforce them.
> 
> Daniele
> 
>>
>>>>> +}
>>>>> +
>>>>>    static int pf_provision_sched_groups(struct xe_gt *gt, u32 mode)
>>>>>    {
>>>>>        int err;
>>>>> @@ -548,6 +567,12 @@ static int pf_provision_sched_groups(struct xe_gt *gt, u32 mode)
>>>>>        if (xe_sriov_pf_num_vfs(gt_to_xe(gt)))
>>>>>            return -EPERM;
>>>>>    +    /* The GuC silently ignores the setting if any mlrc contexts are registered */
>>>>> +    if ((mode != XE_SRIOV_SCHED_GROUPS_NONE) && guc_has_mlrc_queue(&gt->uc.guc)) {
>>>>> +        xe_gt_sriov_notice(gt, "can't enable sched groups with active mlrc queues\n");
>>>>> +        return -EPERM;
>>>>> +    }
>>>>> +
>>>>>        err = __pf_provision_sched_groups(gt, mode);
>>>>>        if (err)
>>>>>            return err;
>>>>> @@ -600,6 +625,11 @@ int xe_gt_sriov_pf_policy_set_sched_groups_mode(struct xe_gt *gt, u32 value)
>>>>>        return err;
>>>>>    }
>>>>>    
>>>> every public function needs to have kernel-doc
>>> ok
>>>
>>>>> +bool xe_gt_sriov_pf_policy_sched_groups_enabled(struct xe_gt *gt)
>>>>> +{
>>>>> +    return gt->sriov.pf.policy.guc.sched_groups.current_mode != XE_SRIOV_SCHED_GROUPS_NONE;
>>>>> +}
>>>>> +
>>>>>    static void pf_sanitize_guc_policies(struct xe_gt *gt)
>>>>>    {
>>>>>        pf_sanitize_sched_if_idle(gt);
>>>>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h
>>>>> index 36680996f2bd..89aa3af6cc7d 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h
>>>>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_policy.h
>>>>> @@ -18,6 +18,7 @@ bool xe_gt_sriov_pf_policy_get_reset_engine(struct xe_gt *gt);
>>>>>    int xe_gt_sriov_pf_policy_set_sample_period(struct xe_gt *gt, u32 value);
>>>>>    u32 xe_gt_sriov_pf_policy_get_sample_period(struct xe_gt *gt);
>>>>>    int xe_gt_sriov_pf_policy_set_sched_groups_mode(struct xe_gt *gt, u32 value);
>>>>> +bool xe_gt_sriov_pf_policy_sched_groups_enabled(struct xe_gt *gt);
>>>>>      int xe_gt_sriov_pf_policy_init(struct xe_gt *gt);
>>>>>    void xe_gt_sriov_pf_policy_sanitize(struct xe_gt *gt);
>>>>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>>>>> index 4c73a077d314..7a180c947032 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>>>>> @@ -438,6 +438,30 @@ u32 xe_gt_sriov_vf_gmdid(struct xe_gt *gt)
>>>>>        return value;
>>>>>    }
>>>>>    +static int query_vf_sched_groups(struct xe_gt *gt)
>>>>> +{
>>>>> +    struct xe_guc *guc = &gt->uc.guc;
>>>>> +    u32 value = 0;
>>>>> +    int err;
>>>>> +
>>>>> +    xe_gt_assert(gt, IS_SRIOV_VF(gt_to_xe(gt)));
>>>>> +
>>>>> +    if (MAKE_GUC_VER_STRUCT(gt->sriov.vf.guc_version) < MAKE_GUC_VER(1, 26, 0))
>>>>> +        return 0;
>>>>> +
>>>>> +    err = guc_action_query_single_klv32(guc,
>>>>> +                        GUC_KLV_GLOBAL_CFG_GROUP_SCHEDULING_AVAILABLE_KEY,
>>>>> +                        &value);
>>>>> +    if (unlikely(err)) {
>>>>> +        xe_gt_sriov_err(gt, "Failed to obtain sched groups status (%pe)\n",
>>>>> +                ERR_PTR(err));
>>>>> +        return err;
>>>>> +    }
>>>>> +
>>>>> +    xe_gt_sriov_dbg(gt, "sched groups %s\n", value ? "enabled" : "disabled");
>>>> str_enabled_disabled(value)
>>>>
>>>>> +    return value;
>>>>> +}
>>>>> +
>>>>>    static int vf_get_ggtt_info(struct xe_gt *gt)
>>>>>    {
>>>>>        struct xe_tile *tile = gt_to_tile(gt);
>>>>> @@ -564,6 +588,21 @@ static void vf_cache_gmdid(struct xe_gt *gt)
>>>>>        gt->sriov.vf.runtime.gmdid = xe_gt_sriov_vf_gmdid(gt);
>>>>>    }
>>>>>    +static int vf_cache_sched_groups_status(struct xe_gt *gt)
>>>>> +{
>>>>> +    int ret;
>>>>> +
>>>>> +    xe_gt_assert(gt, IS_SRIOV_VF(gt_to_xe(gt)));
>>>>> +
>>>>> +    ret = query_vf_sched_groups(gt);
>>>>> +    if (ret < 0)
>>>>> +        return ret;
>>>>> +
>>>>> +    gt->sriov.vf.runtime.uses_sched_groups = ret;
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>>    /**
>>>>>     * xe_gt_sriov_vf_query_config - Query SR-IOV config data over MMIO.
>>>>>     * @gt: the &xe_gt
>>>>> @@ -593,12 +632,32 @@ int xe_gt_sriov_vf_query_config(struct xe_gt *gt)
>>>>>        if (unlikely(err))
>>>>>            return err;
>>>>>    +    err = vf_cache_sched_groups_status(gt);
>>>>> +    if (unlikely(err))
>>>>> +        return err;
>>>>> +
>>>>>        if (has_gmdid(xe))
>>>>>            vf_cache_gmdid(gt);
>>>>>          return 0;
>>>>>    }
>>>>>    +/**
>>>>> + * xe_gt_sriov_vf_sched_groups_enabled - Check if PF has enabled sched groups
>>>>       * xe_gt_sriov_vf_sched_groups_enabled() - Check ...
>>>>
>>>>
>>>>> + * @gt: the &xe_gt
>>>>> + *
>>>>> + * This function is for VF use only.
>>>>> + *
>>>>> + * Return: true if shed groups were enabled, false otherwise.
>>>> typo: s/shed/scheduler
>>>>
>>>>> + */
>>>>> +bool xe_gt_sriov_vf_sched_groups_enabled(struct xe_gt *gt)
>>>>> +{
>>>>> +    xe_gt_assert(gt, IS_SRIOV_VF(gt_to_xe(gt)));
>>>>> +    xe_gt_assert(gt, gt->sriov.vf.guc_version.major);
>>>>> +
>>>>> +    return gt->sriov.vf.runtime.uses_sched_groups;
>>>>> +}
>>>>> +
>>>>>    /**
>>>>>     * xe_gt_sriov_vf_guc_ids - VF GuC context IDs configuration.
>>>>>     * @gt: the &xe_gt
>>>>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>>>>> index af40276790fa..2e1d34c0397f 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>>>>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>>>>> @@ -23,11 +23,14 @@ int xe_gt_sriov_vf_connect(struct xe_gt *gt);
>>>>>    int xe_gt_sriov_vf_query_runtime(struct xe_gt *gt);
>>>>>    void xe_gt_sriov_vf_migrated_event_handler(struct xe_gt *gt);
>>>>>    +bool xe_gt_sriov_vf_sched_groups_enabled(struct xe_gt *gt);
>>>> move it there [1]
>>>>
>>>>> +
>>>>>    int xe_gt_sriov_vf_init_early(struct xe_gt *gt);
>>>>>    int xe_gt_sriov_vf_init(struct xe_gt *gt);
>>>>>    bool xe_gt_sriov_vf_recovery_pending(struct xe_gt *gt);
>>>>>      u32 xe_gt_sriov_vf_gmdid(struct xe_gt *gt);
>>>>> +u32 xe_gt_sriov_vf_sched_groups(struct xe_gt *gt);
>>>> unused ?
>>> oops, old function name that I forgot to remove.
>>>
>>>>>    u16 xe_gt_sriov_vf_guc_ids(struct xe_gt *gt);
>>>>>    u64 xe_gt_sriov_vf_lmem(struct xe_gt *gt);
>>>> [1] here
>>>>
>>>>>    diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h b/drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h
>>>>> index 420b0e6089de..5267c097ecd0 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h
>>>>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf_types.h
>>>>> @@ -27,6 +27,8 @@ struct xe_gt_sriov_vf_selfconfig {
>>>>>    struct xe_gt_sriov_vf_runtime {
>>>>>        /** @gmdid: cached value of the GDMID register. */
>>>>>        u32 gmdid;
>>>>> +    /** @uses_sched_groups: whether PF enabled sched groups or not. */
>>>>> +    bool uses_sched_groups;
>>>>>        /** @regs_size: size of runtime register array. */
>>>>>        u32 regs_size;
>>>>>        /** @num_regs: number of runtime registers in the array. */
>>>>> 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..3e55d9302855 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
>>>>> @@ -32,6 +32,7 @@
>>>>>        define(H2G_STORM, guc_time_us)            \
>>>>>        define(IRQ_STORM, irq_time_us)            \
>>>>>        define(DOORBELL_STORM, doorbell_time_us)    \
>>>>> +    define(MULTI_LRC_COUNT, multi_lrc_count)    \
>>>> this needs to be defined with some version info, maybe:
>>>>
>>>>      define(MULTI_LRC_COUNT, multi_lrc_count, (70, 53, 0))    \
>>>>
>>>> and then in encode_config() have two variants of code generators:
>>>>
>>>> #define encode_threshold_config2(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_config3(TAG, NAME, VERSION) ({                \
>>>>      if (GUC_FIRMWARE_VER(&gt->uc.guc) >= MAKE_GUC_VER VERSION) {        \
>>>>          encode_threshold_config2(TAG, NAME);                \
>>>>      }                                    \
>>>> });
>>>> #define encode_threshold_config(ARGS...) \
>>>>      CALL_ARGS(CONCATENATE(encode_threshold_config, COUNT_ARGS(ARGS)), ARGS)
>>>>
>>>> this should fix the issues already spotted by the CI on ADLP:
>>>>
>>>> <6> [133.603624] xe 0000:00:02.0: [drm] Tile0: GT0: { key 0x8a0d : 32b value 0 } # multi_lrc_count
>>>> <6> [133.603625] xe 0000:00:02.0: [drm] Tile0: GT0: { key 0x0001 : 64b value 0x200000 } # ggtt_start
>>>> <6> [133.603626] xe 0000:00:02.0: [drm] Tile0: GT0: { key 0x0002 : 64b value 0xfec00000 } # ggtt_size
>>>> <3> [133.603653] xe 0000:00:02.0: [drm] *ERROR* PF: Tile0: GT0: Failed to push self configuration (-ENOKEY)
>>> I think it works better if we add the version for all thresholds, so we don't have to define multiple macros every time we need to check the GuC version (which I believe needs to also be done at least in register_threshold_attribute() and define_threshold_key_to_provision_case().
>>> I can just use 70.29.2 (which is the minimum supported version for the Xe driver) for all the older defines.
>> but then the code for version check will be generated for all thresholds, even those that we are sure are supported
>>
>> I will try to find something simpler, or more reusable, but no promise
>>
>>> Daniele
>>>
>>>>
>>>>>        /* end */
>>>>>      /**
>