Re: [PATCH v2] drm/xe/xe_guc: Dynamically decide g2g buffer owner

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

On 10/28/2025 5:00 PM, Jonathan Cavitt wrote:
> On today's driver, xe_device_get_gt(xe, 0); can never return NULL.
> Hardware-wise there's always at least one tile, and every tilie has a
> primary GT.  If something went wrong during init of tile or GT and we
> couldn't create/initialize the structures, then we already aborted the
> device probe immediately and we'll never get further on to places in the
> code that would be chasing a NULL pointer.
> 
> However, there's currently ongoing work to allow the primary GT to be
> disabled via configfs for debugging purposes.  Once that lands, it will
> be possible for this query to return a NULL pointer.  This can cause
> problems in guc_g2g_alloc, as this process currently relies on the
> primary GT always being present.

in such case, maybe we should just skip the G2G tests instead of trying
to fix something that probably will not work anyway, nor is a goal of
this test suite

below diff should be sufficient to survive broken (for other debug) xe:

@@ -683,6 +683,9 @@ static void g2g_check_skip(struct kunit *test)
        if (xe->info.gt_count <= 1)
                kunit_skip(test, "not enough GTs");

+       if (!xe_device_get_gt(xe, 0))
+               kunit_skip(test, "no GT0");
+
        for_each_gt(gt, xe, i) {
                struct xe_guc *guc = &gt->uc.guc;



> 
> Instead of making the primary GT the g2g buffer owner, make the first
> GuC passed to guc_g2g_alloc the g2g buffer owner.  This requires keeping
> track of the g2g buffer owner in the xe device so each GuC can know if
> it's the owner or not during initialization.
> 
> v2:
> - Update the kunit tests to clear and reset the g2g_owner variable as
>   needed (Daniele)
> 
> Suggested-by: Matt Roper <matthew.d.roper@intel.com>
> Suggested-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  drivers/gpu/drm/xe/tests/xe_guc_g2g_test.c | 2 ++
>  drivers/gpu/drm/xe/xe_device_types.h       | 4 ++++
>  drivers/gpu/drm/xe/xe_guc.c                | 9 ++++++---
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/tests/xe_guc_g2g_test.c b/drivers/gpu/drm/xe/tests/xe_guc_g2g_test.c
> index 3b213fcae916..6446232422e2 100644
> --- a/drivers/gpu/drm/xe/tests/xe_guc_g2g_test.c
> +++ b/drivers/gpu/drm/xe/tests/xe_guc_g2g_test.c
> @@ -358,6 +358,7 @@ static void g2g_distribute(struct kunit *test, struct xe_device *xe, struct xe_b
>  	root_gt = xe_device_get_gt(xe, 0);
>  	root_gt->uc.guc.g2g.bo = bo;
>  	root_gt->uc.guc.g2g.owned = true;
> +	xe->g2g_owner = &root_gt->uc.guc;
>  	kunit_info(test, "[%d.%d] Assigned 0x%p\n", gt_to_tile(root_gt)->id, root_gt->info.id, bo);
>  
>  	for_each_gt(gt, xe, i) {
> @@ -447,6 +448,7 @@ static void g2g_free(struct kunit *test, struct xe_device *xe)
>  
>  		gt->uc.guc.g2g.bo = NULL;
>  	}
> +	xe->g2g_owner = NULL;
>  }
>  
>  static void g2g_stop(struct kunit *test, struct xe_device *xe)
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index af0ce275b032..91ec9a295226 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -38,6 +38,7 @@ struct dram_info;
>  struct intel_display;
>  struct intel_dg_nvm_dev;
>  struct xe_ggtt;
> +struct xe_guc;
>  struct xe_i2c;
>  struct xe_pat_ops;
>  struct xe_pxp;
> @@ -628,6 +629,9 @@ struct xe_device {
>  	atomic_t g2g_test_count;
>  #endif
>  
> +	/** @g2g_owner: Pointer to the GuC that is the owner of the g2g buffer */
> +	struct xe_guc *g2g_owner;
> +
>  	/* private: */
>  
>  #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
> diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c
> index ecc3e091b89e..a3a0961456d0 100644
> --- a/drivers/gpu/drm/xe/xe_guc.c
> +++ b/drivers/gpu/drm/xe/xe_guc.c
> @@ -468,9 +468,8 @@ static int guc_g2g_alloc(struct xe_guc *guc)
>  	if (guc->g2g.bo)
>  		return 0;
>  
> -	if (gt->info.id != 0) {
> -		struct xe_gt *root_gt = xe_device_get_gt(xe, 0);
> -		struct xe_guc *root_guc = &root_gt->uc.guc;
> +	if (xe->g2g_owner) {
> +		struct xe_guc *root_guc = xe->g2g_owner;
>  		struct xe_bo *bo;
>  
>  		bo = xe_bo_get(root_guc->g2g.bo);
> @@ -495,6 +494,7 @@ static int guc_g2g_alloc(struct xe_guc *guc)
>  	xe_map_memset(xe, &bo->vmap, 0, 0, g2g_size);
>  	guc->g2g.bo = bo;
>  	guc->g2g.owned = true;
> +	xe->g2g_owner = guc;
>  
>  	return 0;
>  }
> @@ -507,6 +507,9 @@ static void guc_g2g_fini(struct xe_guc *guc)
>  	/* Unpinning the owned object is handled by generic shutdown */
>  	if (!guc->g2g.owned)
>  		xe_bo_put(guc->g2g.bo);
> +	/* g2g owner is no longer valid.  Mark as NULL in xe device */
> +	else
> +		guc_to_xe(guc)->g2g_owner = NULL;
>  
>  	guc->g2g.bo = NULL;
>  }