[PATCH v4 2/8] drm/xe/vf: Finish RESFIX by reset if CTB not enabled

[Lis, Tomasz]

On 09.06.2025 13:00, Michal Wajdeczko wrote:
> On 06.06.2025 02:18, Tomasz Lis wrote:
>> The RESFIX state should be achievable only when CTB communication is
> I'm not sure this is a valid statement, we can't make assumptions about
> the VF driver state, and CTB is enabled only when driver is actively
> submitting workloads, which might not be its 100% life time

Right. I probably remembered from early spec that GuC decides whether a 
VF KMD is working by checking CTB registration.

In the current spec I see the decision is based on whether "the GuC 
interface version is 0", as I understand that refers to the version 
remembered by GuC from handshake.

Since this is now the indication of whether KMD is loaded, I hope GuC is 
clearing that version on `rmmod`.

Also that changes the way migration interacts with suspend/resume.

But back to this patch - it looks to me that there should be no reset 
added, then.

We can get `vf.guc_version.major == 0` only in case we receive the 
MIGRATED IRQ just after enabling IRQs.

And if this happens, VF2GUC_VF_RESET message will be sent to GuC anyway, 
as initial part of `vf_botstrap`.

Of course the bootstrap happens before we get provisioning (before 
`vf_query_config`), so all VF KMD should

do is check whether the config was queried, and if not - skip the 
migration completely.

Will replace this patch with a solution which takes the above into account.

>> enabled. If CTB was disabled and we still got it, then either we're
>> dealing with unclean initial state, or the driver is not currently
>> functional. In these cases, exit the RESFIX state by reset.
>>
>> Signed-off-by: Tomasz Lis<tomasz.lis at intel.com>
>> Cc: Michal Wajdeczko<michal.wajdeczko at intel.com>
>> Cc: Michal Winiarski<michal.winiarski at intel.com>
>> Reviewed-by: Satyanarayana K V P<satyanarayana.k.v.p at intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 11 +++++++++++
>>   drivers/gpu/drm/xe/xe_sriov_vf.c    | 16 ++++++++++++++++
>>   drivers/gpu/drm/xe/xe_sriov_vf.h    |  1 +
>>   3 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>> index 792523cfa6e6..8fa210c0ef1a 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>> @@ -23,6 +23,7 @@
>>   #include "xe_gt_sriov_vf.h"
>>   #include "xe_gt_sriov_vf_types.h"
>>   #include "xe_guc.h"
>> +#include "xe_guc_ct.h"
>>   #include "xe_guc_hxg_helpers.h"
>>   #include "xe_guc_relay.h"
>>   #include "xe_mmio.h"
>> @@ -721,6 +722,16 @@ void xe_gt_sriov_vf_migrated_event_handler(struct xe_gt *gt)
>>   
>>   	xe_gt_assert(gt, IS_SRIOV_VF(xe));
>>   
>> +	if (!xe_guc_ct_enabled(&gt->uc.guc.ct)) {
>> +		/*
>> +		 * If driver initialization is running in parallel to this handler,
>> +		 * ignore the migration which happened before the driver was loaded.
> CT could be temporarily disabled during reset or suspend
> shouldn't we attempt to do fixups in those cases?
>
>> +		 * Force GuC to take the VF out of RESFIX state without any fixups.
>> +		 */
>> +		xe_sriov_vf_post_migration_reset_guc_state(xe);
>> +		return;
> if we exit early here, wouldn't that impact logic in the
> vf_ready_to_recovery_on_all_gts() which looks for bits in gt_flags?

Yes. Even without these skips, the per-device vf recovery with per-gt 
MIGRATED interrupt always brings

  the possibility that some flags will not be set.

The way to avoid that would be to make vf recovery per-gt. I don't think 
the way xe works prevents us from

that currently, but I am still uncertain whether that would be a good 
idea. That would mean first GuC exits

the RESFIX state before the 2nd even started the fixups procedure.

Another possibility would be to have a timeout - if some of the per-gt 
flags were set, but no further flags

come in ie. 5 seconds, just run the post-migration recovery.

-Tomasz

>> +	}
>> +
>>   	set_bit(gt->info.id, &xe->sriov.vf.migration.gt_flags);
>>   	/*
>>   	 * We need to be certain that if all flags were set, at least one
>> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf.c b/drivers/gpu/drm/xe/xe_sriov_vf.c
>> index 6526fe450e55..eff6c7b96f25 100644
>> --- a/drivers/gpu/drm/xe/xe_sriov_vf.c
>> +++ b/drivers/gpu/drm/xe/xe_sriov_vf.c
>> @@ -147,6 +147,22 @@ void xe_sriov_vf_init_early(struct xe_device *xe)
>>   		xe_sriov_info(xe, "migration not supported by this module version\n");
>>   }
>>   
>> +/**
>> + * xe_sriov_vf_post_migration_reset_guc_state - Reset VF state in all GuCs.
>> + * @xe: the &xe_device struct instance
>> + *
>> + * This function sends VF state reset to GuC, as a way of exiting RESFIX
> "sends VF state reset to GuC"
>
> well, this function does much more than that
>
>> + * state if a proper post-migration recovery procedure has failed.
>> + */
>> +void xe_sriov_vf_post_migration_reset_guc_state(struct xe_device *xe)
>> +{
>> +	struct xe_gt *gt;
>> +	unsigned int id;
>> +
>> +	for_each_gt(gt, xe, id)
>> +		xe_gt_reset_async(gt);
>> +}
>> +
>>   /**
>>    * vf_post_migration_requery_guc - Re-query GuC for current VF provisioning.
>>    * @xe: the &xe_device struct instance
>> diff --git a/drivers/gpu/drm/xe/xe_sriov_vf.h b/drivers/gpu/drm/xe/xe_sriov_vf.h
>> index 7b8622cff2b7..ba846af34a13 100644
>> --- a/drivers/gpu/drm/xe/xe_sriov_vf.h
>> +++ b/drivers/gpu/drm/xe/xe_sriov_vf.h
>> @@ -10,5 +10,6 @@ struct xe_device;
>>   
>>   void xe_sriov_vf_init_early(struct xe_device *xe);
>>   void xe_sriov_vf_start_migration_recovery(struct xe_device *xe);
>> +void xe_sriov_vf_post_migration_reset_guc_state(struct xe_device *xe);
>>   
>>   #endif