Re: [PATCH 6/7] drm/xe: Improve xe_gt_sriov_pf_config GGTT handling

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

On 10/21/2025 2:18 PM, Maarten Lankhorst wrote:
> Do not directly dereference xe_ggtt_node, and add
> a function to retrieve the allocated GGTT size.
> 
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> Reviewed-by: <Matthew.brost@intel.com>
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>

double s-o-b

> ---
>  drivers/gpu/drm/xe/xe_ggtt.c               | 11 +++++++++++
>  drivers/gpu/drm/xe/xe_ggtt.h               |  1 +
>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 15 ++++++++-------
>  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 05515037e2d6a..81038d94b497d 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -1090,3 +1090,14 @@ u64 xe_ggtt_node_addr(const struct xe_ggtt_node *node)
>  {
>  	return node->base.start;
>  }
> +
> +/**
> + * xe_ggtt_node_size - Get @node allocation size.
> + * @node: &xe_ggtt_node
> + *
> + * Get the allocated node's size.

    * Return: ...

> + */
> +u64 xe_ggtt_node_size(const struct xe_ggtt_node *node)
> +{
> +	return node->base.size;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
> index 4a8ef1b824156..b69f05a6adbf8 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.h
> +++ b/drivers/gpu/drm/xe/xe_ggtt.h
> @@ -59,5 +59,6 @@ u64 xe_ggtt_encode_pte_flags(struct xe_ggtt *ggtt, struct xe_bo *bo, u16 pat_ind
>  u64 xe_ggtt_read_pte(struct xe_ggtt *ggtt, u64 offset);
>  
>  u64 xe_ggtt_node_addr(const struct xe_ggtt_node *node);
> +u64 xe_ggtt_node_size(const struct xe_ggtt_node *node);
>  
>  #endif
> 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 f96f96425c2ec..19d1013d0c4c8 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), xe_ggtt_node_size(node), details);
>  }
>  
>  /* Return: number of configuration dwords written */
> @@ -495,9 +495,9 @@ 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;
>  
> @@ -514,7 +514,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) ? xe_ggtt_node_size(node) : 0;
>  }
>  
>  /**
> @@ -2517,11 +2517,12 @@ 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(xe_ggtt_node_size(config->ggtt_region), 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) +
> +			   xe_ggtt_node_size(config->ggtt_region) - 1,
>  			   buf);
>  	}
>  

just nits, so

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>