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

[John Harrison <john.c.harrison@intel.com>]

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? 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.

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;