Re: [PATCH 3/3] drm/xe/vf: Set submission version in xe_uc_fw struct

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

On 2/27/2025 3:22 AM, Laguna, Lukasz wrote:
>
> On 2/25/2025 23:30, Daniele Ceraolo Spurio wrote:
>>
>>
>> On 2/25/2025 5:23 AM, Lukasz Laguna wrote:
>>> The VF driver has already negotiated the ABI version with GuC. What
>>> remains is to populate the generic xe_uc_fw struct with the version
>>> before initializing submission.
>>>
>>> Signed-off-by: Lukasz Laguna <lukasz.laguna@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_guc.c | 9 ++++++++-
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>>> index bc1ff0a4e1e7..7b38447d902c 100644
>>> --- a/drivers/gpu/drm/xe/xe_guc.c
>>> +++ b/drivers/gpu/drm/xe/xe_guc.c
>>> @@ -703,9 +703,16 @@ int xe_guc_init(struct xe_guc *guc)
>>>     static int vf_guc_init_post_hwconfig(struct xe_guc *guc)
>>>   {
>>> +    struct xe_gt *gt = guc_to_gt(guc);
>>> +    struct xe_uc_fw_version ver;
>>>       int err;
>>>   -    err = xe_guc_submit_init(guc, 
>>> xe_gt_sriov_vf_guc_ids(guc_to_gt(guc)));
>>> +    err = xe_gt_sriov_vf_get_guc_ver(gt, &ver);
>>> +    xe_gt_assert(gt, !err);
>>> +
>>> +    xe_uc_fw_set_compatibility_ver(&guc->fw, &ver);
>>
>> Why not just pass &gt->sriov.vf.guc_version here, instead of copying 
>> it into another variable first?
>
> By using xe_gt_sriov_vf_get_guc_ver(), we additionally verify whether 
> the handshake completed successfully (major > 0). Only in that case we 
> set the version in xe_uc_fw.

But it's impossible to get here with a wrong major value, because 
xe_gt_sriov_vf_bootstrap() would fail. If you really want to have an 
emergency check then IMO it'd be better to just have:

xe_assert(xe, gt->sriov.vf.guc_version.major);

which you can have in this function without the need for an helper (the 
same assert exists in other SRIOV functions like xe_gt_sriov_vf_lmem).

Daniele

>
> Lukasz
>
>>
>> Daniele
>>
>>> +
>>> +    err = xe_guc_submit_init(guc, xe_gt_sriov_vf_guc_ids(gt));
>>>       if (err)
>>>           return err;
>>