Re: [PATCH v4 2/3] drm/xe/ggtt: Initialize GGTT pools by SR-IOV mode

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

On 9/1/2026 6:44 PM, Piórkowski, Piotr wrote:
> From: Piotr Piórkowski <piotr.piorkowski@intel.com>
> 
> GGTT initialization currently uses the full available range for both the
> usable and shareable pools, regardless of the SR-IOV mode. The range is
> read from hardware on native and PF devices and assigned by GuC on VFs.
> 
> Let's separate range discovery from pool setup and initialize the pools
> based on SR-IOV mode. Native and VF modes will use only the usable pool.
> Shared PF mode will use both pools over the same full range, so they fully
> overlap.
> 
> Assisted-by: Claude:claude-5-sonnet
> Signed-off-by: Piotr Piórkowski <piotr.piorkowski@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <dev@lankhorst.se>
> ---
>  drivers/gpu/drm/xe/xe_ggtt.c | 132 +++++++++++++++++++++++++----------
>  1 file changed, 97 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 1f0bd876cb35..7fcd686f9729 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -366,8 +366,8 @@ static const struct xe_ggtt_pt_ops xelpg_pt_wa_ops = {
>  	.ggtt_get_pte = xe_ggtt_get_pte,
>  };
>  
> -static void __xe_ggtt_init_early(struct xe_ggtt *ggtt, u64 start, u64 usable_size,
> -				 u64 shareable_size)
> +static void ggtt_init_ranges(struct xe_ggtt *ggtt, u64 start, u64 usable_size,
> +			     u64 shareable_size)

maybe this function rename could be done in patch 1/3 when we've changed its signature?

>  {
>  	struct xe_gt *gt = ggtt->tile->primary_gt;
>  
> @@ -387,10 +387,87 @@ static void __xe_ggtt_init_early(struct xe_ggtt *ggtt, u64 start, u64 usable_siz
>  	drm_mm_init(&ggtt->mm, 0, ggtt->hw_size);
>  }
>  
> +static int ggtt_get_range_from_hw(struct xe_ggtt *ggtt, u64 *start, u64 *size)

ggtt_probe() ?

> +{
> +	struct xe_device *xe = tile_to_xe(ggtt->tile);
> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> +	unsigned int gsm_size;
> +
> +	if (GRAPHICS_VERx100(xe) >= 1250)
> +		gsm_size = SZ_8M; /* GGTT is expected to be 4GiB */
> +	else
> +		gsm_size = probe_gsm_size(pdev);
> +
> +	if (!gsm_size) {
> +		xe_tile_err(ggtt->tile, "Hardware reported no preallocated GSM\n");
> +		return -ENOMEM;
> +	}
> +
> +	*start = xe_wopcm_size(xe);
> +	*size = (gsm_size / 8) * (u64)XE_PAGE_SIZE - *start;
> +
> +	return 0;
> +}
> +
> +static int ggtt_get_range_from_guc(struct xe_ggtt *ggtt, u64 *start, u64 *size)
> +{
> +	struct xe_device *xe = tile_to_xe(ggtt->tile);
> +	u64 wopcm = xe_wopcm_size(xe);
> +
> +	*start = xe_tile_sriov_vf_ggtt_base(ggtt->tile);
> +	*size = xe_tile_sriov_vf_ggtt(ggtt->tile);
> +
> +	if (*start < wopcm || *start + *size > GUC_GGTT_TOP) {
> +		xe_tile_err(ggtt->tile, "Invalid GGTT configuration: %#llx-%#llx\n",
> +			    *start, *start + *size - 1);
> +		return -ERANGE;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ggtt_read_available_range(struct xe_ggtt *ggtt, u64 *start, u64 *size)
> +{
> +	struct xe_device *xe = tile_to_xe(ggtt->tile);
> +	int err;
> +
> +	if (IS_SRIOV_VF(xe))
> +		err = ggtt_get_range_from_guc(ggtt, start, size);
> +	else
> +		err = ggtt_get_range_from_hw(ggtt, start, size);
> +	if (err)
> +		return err;
> +
> +	if (*start + *size > GUC_GGTT_TOP)
> +		*size = GUC_GGTT_TOP - *start;

we can make such adjustment only on native/PF

on VF, this should be treated as a fatal bug (like start < wopcm)
> +
> +	return 0;
> +}
> +
> +static void ggtt_init_native(struct xe_ggtt *ggtt, u64 start, u64 size)
> +{
> +	ggtt_init_ranges(ggtt, start, size, 0);
> +}
> +
> +static void ggtt_init_shared(struct xe_ggtt *ggtt, u64 start, u64 size)
> +{
> +	ggtt_init_ranges(ggtt, start, size, size);
> +}
> +
> +static void ggtt_init_generic(struct xe_ggtt *ggtt, u64 start, u64 size)
> +{
> +	struct xe_device *xe = tile_to_xe(ggtt->tile);
> +
> +	if (!IS_SRIOV_PF(xe))
> +		ggtt_init_native(ggtt, start, size);
> +	else
> +		ggtt_init_shared(ggtt, start, size);
> +}
> +
>  int xe_ggtt_init_kunit(struct xe_ggtt *ggtt, u32 start, u32 size)
>  {
>  	ggtt->hw_size = size;
> -	__xe_ggtt_init_early(ggtt, start, size, 0);
> +	ggtt_init_ranges(ggtt, start, size, 0);
>  	return 0;
>  }
>  EXPORT_SYMBOL_IF_KUNIT(xe_ggtt_init_kunit);
> @@ -418,43 +495,19 @@ static void dev_fini_ggtt(void *arg)
>  int xe_ggtt_init_early(struct xe_ggtt *ggtt)
>  {
>  	struct xe_device *xe = tile_to_xe(ggtt->tile);
> -	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> -	unsigned int gsm_size;
> -	u64 ggtt_start, wopcm = xe_wopcm_size(xe), ggtt_size;
> +	u64 ggtt_start, ggtt_size;
>  	int err;
>  
> -	if (!IS_SRIOV_VF(xe)) {
> -		if (GRAPHICS_VERx100(xe) >= 1250)
> -			gsm_size = SZ_8M; /* GGTT is expected to be 4GiB */
> -		else
> -			gsm_size = probe_gsm_size(pdev);
> -		if (gsm_size == 0) {
> -			xe_tile_err(ggtt->tile, "Hardware reported no preallocated GSM\n");
> -			return -ENOMEM;
> -		}
> -		ggtt_start = wopcm;
> -		ggtt_size = (gsm_size / 8) * (u64)XE_PAGE_SIZE - ggtt_start;
> -	} else {
> -		ggtt_start = xe_tile_sriov_vf_ggtt_base(ggtt->tile);
> -		ggtt_size = xe_tile_sriov_vf_ggtt(ggtt->tile);
> -
> -		if (ggtt_start < wopcm ||
> -		    ggtt_start + ggtt_size > GUC_GGTT_TOP) {
> -			xe_tile_err(ggtt->tile, "Invalid GGTT configuration: %#llx-%#llx\n",
> -				    ggtt_start, ggtt_start + ggtt_size - 1);
> -			return -ERANGE;
> -		}
> -	}
> +	err = ggtt_read_available_range(ggtt, &ggtt_start, &ggtt_size);
> +	if (err)
> +		return err;
> +
> +	ggtt->hw_size = ggtt_size;
>  
>  	ggtt->gsm = ggtt->tile->mmio.regs + SZ_8M;
>  	if (IS_DGFX(xe) && xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K)
>  		ggtt->flags |= XE_GGTT_FLAGS_64K;
>  
> -	if (ggtt_size + ggtt_start > GUC_GGTT_TOP)
> -		ggtt_size = GUC_GGTT_TOP - ggtt_start;
> -
> -	ggtt->hw_size = ggtt_size;
> -
>  	if (GRAPHICS_VERx100(xe) >= 1270)
>  		ggtt->pt_ops =
>  			(ggtt->tile->media_gt && XE_GT_WA(ggtt->tile->media_gt, 22019338487)) ||
> @@ -467,8 +520,17 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt)
>  	if (!ggtt->wq)
>  		return -ENOMEM;
>  
> -	__xe_ggtt_init_early(ggtt, ggtt_start, ggtt_size,
> -			     IS_SRIOV_PF(xe) ? ggtt_size : 0);
> +	ggtt_init_generic(ggtt, ggtt_start, ggtt_size);
> +	xe_tile_info(ggtt->tile, "GGTT usable %#llx-%#llx = %lluK\n",
> +		     ggtt->start, ggtt->start + ggtt->size - 1,
> +		     ggtt->size / SZ_1K);

both info() could go to init_generic(), no?

> +#ifdef CONFIG_PCI_IOV
> +	if (IS_SRIOV_PF(xe))
> +		xe_tile_info(ggtt->tile, "GGTT shareable %#llx-%#llx = %lluK\n",
> +			     ggtt->start + ggtt->shareable.start,
> +			     ggtt->start + ggtt->shareable.start + ggtt->shareable.size - 1,
> +			     ggtt->shareable.size / SZ_1K);
> +#endif
>  
>  	err = drmm_add_action_or_reset(&xe->drm, ggtt_fini_early, ggtt);
>  	if (err)