Re: [PATCH 2/2] drm/xe/vf: Custom GuC initialization if VF

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

On 20.05.2024 23:30, John Harrison wrote:
> On 5/20/2024 14:23, Michal Wajdeczko wrote:
>> The GuC firmware is loaded and initialized by the PF driver. Make
>> sure VF drivers only perform permitted operations. For submission
>> initialization, use number of GuC context IDs from self config.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 17 ++++++++++++
>>   drivers/gpu/drm/xe/xe_gt_sriov_vf.h |  2 ++
>>   drivers/gpu/drm/xe/xe_guc.c         | 40 +++++++++++++++++++++++++++++
>>   3 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>> b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>> index 378dde5ad4f9..bf4dfe301a71 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
>> @@ -408,6 +408,23 @@ int xe_gt_sriov_vf_query_config(struct xe_gt *gt)
>>       return 0;
>>   }
>>   +/**
>> + * xe_gt_sriov_vf_guc_ids - VF GuC context IDs configuration.
>> + * @gt: the &xe_gt
>> + *
>> + * This function is for VF use only.
>> + *
>> + * Return: number of GuC context IDs assigned to VF.
>> + */
>> +u16 xe_gt_sriov_vf_guc_ids(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 ||
>> gt->sriov.vf.guc_version.major);
> I may be going blind but this says 'X || X'? Is the second term supposed
> to say minor? 

oops, it's me blind (again)

> Is there any need to check past the major version anyway?
> Xe does not support legacy GuCs from before VF version 1.0.0. So if
> major is zero, there is a problem.

this is to make sure that we have completed the GuC handshake (and have
negotiated major.minor != 0)

> 
> John.
> 
>> +    xe_gt_assert(gt, gt->sriov.vf.self_config.num_ctxs);
>> +
>> +    return gt->sriov.vf.self_config.num_ctxs;
>> +}
>> +
>>   static int relay_action_handshake(struct xe_gt *gt, u32 *major, u32
>> *minor)
>>   {
>>       u32 request[VF2PF_HANDSHAKE_REQUEST_MSG_LEN] = {
>> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>> b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>> index 997cb7541036..d6d37b193d17 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.h
>> @@ -16,6 +16,8 @@ int xe_gt_sriov_vf_query_config(struct xe_gt *gt);
>>   int xe_gt_sriov_vf_connect(struct xe_gt *gt);
>>   int xe_gt_sriov_vf_query_runtime(struct xe_gt *gt);
>>   +u16 xe_gt_sriov_vf_guc_ids(struct xe_gt *gt);
>> +
>>   void xe_gt_sriov_vf_print_config(struct xe_gt *gt, struct
>> drm_printer *p);
>>   void xe_gt_sriov_vf_print_runtime(struct xe_gt *gt, struct
>> drm_printer *p);
>>   void xe_gt_sriov_vf_print_version(struct xe_gt *gt, struct
>> drm_printer *p);
>> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
>> index 807ad53449e4..cec709a908c3 100644
>> --- a/drivers/gpu/drm/xe/xe_guc.c
>> +++ b/drivers/gpu/drm/xe/xe_guc.c
>> @@ -295,6 +295,23 @@ static int xe_guc_realloc_post_hwconfig(struct
>> xe_guc *guc)
>>       return 0;
>>   }
>>   +static int vf_guc_init(struct xe_guc *guc)
>> +{
>> +    int err;
>> +
>> +    xe_guc_comm_init_early(guc);
>> +
>> +    err = xe_guc_ct_init(&guc->ct);
>> +    if (err)
>> +        return err;
>> +
>> +    err = xe_guc_relay_init(&guc->relay);
>> +    if (err)
>> +        return err;
>> +
>> +    return 0;
>> +}
>> +
>>   int xe_guc_init(struct xe_guc *guc)
>>   {
>>       struct xe_device *xe = guc_to_xe(guc);
>> @@ -309,6 +326,13 @@ int xe_guc_init(struct xe_guc *guc)
>>       if (!xe_uc_fw_is_enabled(&guc->fw))
>>           return 0;
>>   +    if (IS_SRIOV_VF(xe)) {
>> +        ret = vf_guc_init(guc);
>> +        if (ret)
>> +            goto out;
>> +        return 0;
>> +    }
>> +
>>       ret = xe_guc_log_init(&guc->log);
>>       if (ret)
>>           goto out;
>> @@ -342,6 +366,19 @@ int xe_guc_init(struct xe_guc *guc)
>>       return ret;
>>   }
>>   +static int vf_guc_init_post_hwconfig(struct xe_guc *guc)
>> +{
>> +    int err;
>> +
>> +    err = xe_guc_submit_init(guc,
>> xe_gt_sriov_vf_guc_ids(guc_to_gt(guc)));
>> +    if (err)
>> +        return err;
>> +
>> +    /* XXX xe_guc_db_mgr_init not needed for now */
>> +
>> +    return 0;
>> +}
>> +
>>   /**
>>    * xe_guc_init_post_hwconfig - initialize GuC post hwconfig load
>>    * @guc: The GuC object
>> @@ -352,6 +389,9 @@ int xe_guc_init_post_hwconfig(struct xe_guc *guc)
>>   {
>>       int ret;
>>   +    if (IS_SRIOV_VF(guc_to_xe(guc)))
>> +        return vf_guc_init_post_hwconfig(guc);
>> +
>>       ret = xe_guc_realloc_post_hwconfig(guc);
>>       if (ret)
>>           return ret;
>