Re: [PATCH v10 07/10] drm/xe/vf: Add bounds checking for queried GGTT base and size

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

On 9/21/2026 11:21 AM, Satyanarayana K V P wrote:
> Add explicit bounds checks for GGTT base and size which can detect
> and reject invalid configuration data from a misconfigured or
> malfunctioning PF, preventing protocol violations and protecting VF
> initialization.
> 
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
> V9 -> V10:
> - Updated vf_get_ggtt_alignment() function (Sashiko).
> 
> V8 -> V9:
> - Align GGTT base and size to XE_PAGE_SIZE (Sashiko).
> 
> V7 -> V8:
> - Align GGTT base and size to PAGE_SIZE (Michal W).
> 
> V6 -> V7:
> - Added new helper to get ggtt alignment.
> - Rearranged the base and sizes checks (Sashiko, Michal W).
> 
> V5 -> V6:
> - Updated error codes for unaligned GGTT base and size (Michal W).
> - Fixed review comments (Michal W).
> 
> V4 -> V5:
> - New commit.
> ---
>  drivers/gpu/drm/xe/xe_gt_sriov_vf.c | 45 ++++++++++++++++++++++++++---
>  1 file changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> index 715dfb04a1f3..1d79bde2ad73 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_vf.c
> @@ -15,6 +15,7 @@
>  #include "abi/guc_klvs_abi.h"
>  #include "abi/guc_relay_actions_abi.h"
>  #include "regs/xe_gt_regs.h"
> +#include "regs/xe_gtt_defs.h"
>  #include "regs/xe_guc_regs.h"
>  
>  #include "xe_assert.h"
> @@ -486,26 +487,62 @@ u32 xe_gt_sriov_vf_gmdid(struct xe_gt *gt)
>  	return value;
>  }
>  
> +static u64 vf_get_ggtt_alignment(struct xe_gt *gt)
> +{
> +	return xe_sriov_ggtt_alignment(gt_to_xe(gt));
> +}
> +
>  static int vf_get_ggtt_info(struct xe_gt *gt)
>  {
> +	u64 alignment = vf_get_ggtt_alignment(gt);
>  	struct xe_tile *tile = gt_to_tile(gt);
>  	struct xe_guc *guc = &gt->uc.guc;
> -	u64 start, size, ggtt_size;
> +	u64 start, size, ggtt_size, end;
> +	u64 start_query, size_query;
>  	int err;
>  
>  	xe_gt_assert(gt, IS_SRIOV_VF(gt_to_xe(gt)));
>  
> -	err = guc_action_query_single_klv64(guc, GUC_KLV_VF_CFG_GGTT_START_KEY, &start);
> +	err = guc_action_query_single_klv64(guc, GUC_KLV_VF_CFG_GGTT_START_KEY, &start_query);

do we really need/want to rename existing vars?

>  	if (unlikely(err))
>  		return err;

shouldn't we immediately check here for 4K misalignment?
it is a clear VF provisioning violation, see [1]:

	if (!IS_ALIGNED(start, XE_PAGE_SIZE))
		return -EINVAL; // or -EUCLEAN or -EDOM or -ERANGE

[1] https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/gpu/drm/xe/abi/guc_klvs_abi.h#L280

>  
> -	err = guc_action_query_single_klv64(guc, GUC_KLV_VF_CFG_GGTT_SIZE_KEY, &size);
> +	err = guc_action_query_single_klv64(guc, GUC_KLV_VF_CFG_GGTT_SIZE_KEY, &size_query);
>  	if (unlikely(err))
>  		return err;

ditto

>  
> -	if (!size)
> +	if (!size_query)
>  		return -ENODATA;
>  
> +	start = ALIGN(start_query, alignment);

not needed, as it looks any 64K alignment is handled on the per-object basis
by the xe_ggtt layer

> +	if (check_add_overflow(start_query, size_query, &end)) {
> +		xe_gt_sriov_err(gt, "GGTT range overflow: base %#llx, size %#llx\n",
> +				start_query, size_query);

we are not so verbose for other error cases,
maybe demote to dbg/dbg_verbose or drop completely?

> +		return -ERANGE;
> +	}
> +
> +	end = ALIGN_DOWN(end, alignment);

not needed, see above

> +	if (end <= start) {

impossible now, as at this point:
 both start & size should be already 4K aligned
 end = start + size
 and size can't be 0

> +		xe_gt_sriov_err(gt, "GGTT range too small: base %#llx, size %#llx\n",
> +				start_query, size_query);
> +		return -ERANGE;
> +	}
> +
> +	size = end - start;
> +
> +	if (start != start_query)
> +		xe_gt_sriov_notice(gt, "Unaligned GGTT base %#llx aligned to %#llx\n",
> +				   start_query, start);
> +	if (size != size_query)
> +		xe_gt_sriov_notice(gt, "Unaligned GGTT size %#llx aligned to %#llx\n",
> +				   size_query, size);

will not be needed after adding above checks for 4K alignment right after query

> +
> +	if (start < xe_wopcm_size(gt_to_xe(gt)) || end > GUC_GGTT_TOP) {

nit: we can check "start < wopcm" right after query
nit: we can check "end > GUC_TOP" right after query

> +		xe_gt_sriov_err(gt, "Out of bounds GGTT configuration: base %#llx, size %#llx\n",
> +				start, size);
> +		return -ERANGE;
> +	}
> +
>  	xe_tile_sriov_vf_ggtt_base_store(tile, start);
>  	ggtt_size = xe_tile_sriov_vf_ggtt(tile);
>  	if (!ggtt_size) {