Re: [PATCH v6 06/12] drm/xe: Improve xe_gt_sriov_pf_config GGTT handling

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

On 10/14/2025 11:20 PM, Maarten Lankhorst wrote:
> Do not directly dereference xe_ggtt_node, and add
> a member to store the GGTT size.
> 
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c       | 15 ++++++++-------
>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config_types.h |  8 ++++++--
>  2 files changed, 14 insertions(+), 9 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 2289756761636..c0dfffd5c553b 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> @@ -264,7 +264,7 @@ static u32 encode_config_ggtt(u32 *cfg, const struct xe_gt_sriov_config *config,
>  	if (!xe_ggtt_node_allocated(node))
>  		return 0;
>  
> -	return encode_ggtt(cfg, node->base.start, node->base.size, details);
> +	return encode_ggtt(cfg, xe_ggtt_node_addr(node), config->ggtt_size, details);
>  }
>  
>  /* Return: number of configuration dwords written */
> @@ -495,13 +495,14 @@ static int pf_provision_vf_ggtt(struct xe_gt *gt, unsigned int vfid, u64 size)
>  
>  	xe_ggtt_assign(node, vfid);
>  	xe_gt_sriov_dbg_verbose(gt, "VF%u assigned GGTT %llx-%llx\n",
> -				vfid, node->base.start, node->base.start + node->base.size - 1);
> +				vfid, xe_ggtt_node_addr(node), xe_ggtt_node_addr(node) + size - 1);
>  
> -	err = pf_distribute_config_ggtt(gt->tile, vfid, node->base.start, node->base.size);
> +	err = pf_distribute_config_ggtt(gt->tile, vfid, xe_ggtt_node_addr(node), size);
>  	if (unlikely(err))
>  		goto err;
>  
>  	config->ggtt_region = node;
> +	config->ggtt_size = size;
>  	return 0;
>  err:
>  	pf_release_ggtt(tile, node);
> @@ -514,7 +515,7 @@ static u64 pf_get_vf_config_ggtt(struct xe_gt *gt, unsigned int vfid)
>  	struct xe_ggtt_node *node = config->ggtt_region;
>  
>  	xe_gt_assert(gt, xe_gt_is_main_type(gt));
> -	return xe_ggtt_node_allocated(node) ? node->base.size : 0;
> +	return xe_ggtt_node_allocated(node) ? config->ggtt_size : 0;
>  }
>  
>  /**
> @@ -2516,11 +2517,11 @@ int xe_gt_sriov_pf_config_print_ggtt(struct xe_gt *gt, struct drm_printer *p)
>  		if (!xe_ggtt_node_allocated(config->ggtt_region))
>  			continue;
>  
> -		string_get_size(config->ggtt_region->base.size, 1, STRING_UNITS_2,
> +		string_get_size(config->ggtt_size, 1, STRING_UNITS_2,
>  				buf, sizeof(buf));
>  		drm_printf(p, "VF%u:\t%#0llx-%#llx\t(%s)\n",
> -			   n, config->ggtt_region->base.start,
> -			   config->ggtt_region->base.start + config->ggtt_region->base.size - 1,
> +			   n, xe_ggtt_node_addr(config->ggtt_region),
> +			   xe_ggtt_node_addr(config->ggtt_region) + config->ggtt_size - 1,
>  			   buf);
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config_types.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config_types.h
> index 686c7b3b6d7a5..9a8e66c8b539f 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config_types.h
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config_types.h
> @@ -17,10 +17,14 @@ struct xe_bo;
>   * Used by the PF driver to maintain per-VF provisioning data.
>   */
>  struct xe_gt_sriov_config {
> -	/** @ggtt_region: GGTT region assigned to the VF. */
> -	struct xe_ggtt_node *ggtt_region;
>  	/** @lmem_obj: LMEM allocation for use by the VF. */
>  	struct xe_bo *lmem_obj;
> +
> +	/** @ggtt_region: GGTT region assigned to the VF. */
> +	struct xe_ggtt_node *ggtt_region;
> +	/** @ggtt_size: Size of GGTT region */
> +	u64 ggtt_size;

can't we just add another helper on GGTT side to get the size of the node?

	u64 xe_ggtt_node_size(node);

there is one for xe_bo, why can't we have another for xe_ggtt_node ?
> +
>  	/** @num_ctxs: number of GuC contexts IDs.  */
>  	u16 num_ctxs;
>  	/** @begin_ctx: start index of GuC context ID range. */