[PATCH] drm/xe/pf: Fix VF config validation on multi-GT platforms

[Cavitt, Jonathan]

-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Michal Wajdeczko
Sent: Tuesday, August 6, 2024 11:05 AM
To: intel-xe at lists.freedesktop.org
Cc: Wajdeczko, Michal <Michal.Wajdeczko at intel.com>; Piorkowski, Piotr <piotr.piorkowski at intel.com>
Subject: [PATCH] drm/xe/pf: Fix VF config validation on multi-GT platforms
> 
> When validating VF config on the media GT, we may wrongly report
> that VF is already partially configured on it, as we consider GGTT
> and LMEM provisioning done on the primary GT (since both GGTT and
> LMEM are tile-level resources, not a GT-level).
> 
> This will cause skipping a VF auto-provisioning on the media-GT and
> in result will block a VF from successfully initialize that GT.
> 
> Fix that by considering GGTT and LMEM configurations only when
> checking if a VF provisioning is complete, and omit GGTT and LMEM
> when reporting empty/partial provisioning.
> 
> Fixes: 234670cea9a2 ("drm/xe/pf: Skip fair VFs provisioning if already provisioned")
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Piotr Piórkowski <piotr.piorkowski at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> index 52c7277d243d..227527785afd 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> @@ -1928,6 +1928,7 @@ static int pf_validate_vf_config(struct xe_gt *gt, unsigned int vfid)
>  {
>  	struct xe_gt *primary_gt = gt_to_tile(gt)->primary_gt;
>  	struct xe_device *xe = gt_to_xe(gt);
> +	bool is_primary = !xe_gt_is_media_type(gt);
>  	bool valid_ggtt, valid_ctxs, valid_dbs;
>  	bool valid_any, valid_all;
>  
> @@ -1936,13 +1937,17 @@ static int pf_validate_vf_config(struct xe_gt *gt, unsigned int vfid)
>  	valid_dbs = pf_get_vf_config_dbs(gt, vfid);
>  
>  	/* note that GuC doorbells are optional */
> -	valid_any = valid_ggtt || valid_ctxs || valid_dbs;
> -	valid_all = valid_ggtt && valid_ctxs;
> +	valid_any = valid_ctxs || valid_dbs;
> +	valid_all = valid_ctxs;
> +
> +	/* and GGTT/LMEM is configured on primary GT only */
> +	valid_all = valid_all && valid_ggtt;
> +	valid_any = valid_any || (valid_ggtt && is_primary);

I can see why you would want to keep the ggtt configuration separate from
the rest of the config, though it might be cleaner to just keep it all collected
together like it was before.  Not blocking on this, though:
Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt

>  
>  	if (IS_DGFX(xe)) {
>  		bool valid_lmem = pf_get_vf_config_ggtt(primary_gt, vfid);
>  
> -		valid_any = valid_any || valid_lmem;
> +		valid_any = valid_any || (valid_lmem && is_primary);
>  		valid_all = valid_all && valid_lmem;
>  	}
>  
> -- 
> 2.43.0
> 
>