Re: [PATCH 06/11] drm/xe: Do not claim flat CCS without a graphics IP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] drm/xe: clearing has_flat_ccs in xe_info_init() enables AuxCCS
  on media GT
- [Medium] drm/xe: xe_info_init() leaves has_flat_ccs set when primary
  GT is disabled
--

commit f555718484d7a834cd43df271a384a71adacc98b
Author: Stuart Summers <stuart.summers@intel.com>

drm/xe: Do not claim flat CCS without a graphics IP

This commit clears xe->info.has_flat_ccs in xe_info_init() when the
device has no graphics IP descriptor.

> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index b3e5c025677cf..e4220cb0cc298 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -1106,6 +1106,14 @@ static int xe_info_init(struct xe_device *xe,
>  		xe->info.has_ctx_tlb_inval = graphics_desc->has_ctx_tlb_inval;
>  		xe->info.has_usm = graphics_desc->has_usm;
>  		xe->info.has_64bit_timestamp = graphics_desc->has_64bit_timestamp;
> +	} else {
> +		/*
> +		 * Flat CCS comes from the device descriptor rather than from
> +		 * the graphics IP, but it is still a graphics feature: its
> +		 * layout is read back through the primary GT, which is not
> +		 * there to be read.
> +		 */
> +		xe->info.has_flat_ccs = 0;
>  	}
>  
>  	if (xe->info.platform != XE_PVC)

[Severity: High]
Does clearing xe->info.has_flat_ccs when graphics_desc is NULL cause
has_aux_ccs() to return true on Xe2+ platforms without a graphics IP?

When graphics_ip is absent, GRAPHICS_VERx100(xe) is 0, so has_aux_ccs()
falls through to return !xe->info.has_flat_ccs:

drivers/gpu/drm/xe/xe_ring_ops.c:has_aux_ccs() {
	if (GRAPHICS_VERx100(xe) >= 1270 || xe->info.platform == XE_PVC)
		return false;

	return !xe->info.has_flat_ccs;
}

During media GT initialization on Xe2+ platforms without a graphics IP,
xe_ring_ops_get() then selects auxccs_ring_ops_gen12_video_decode and
auxccs_ring_ops_gen12_video_enhance, emitting unsupported Gen12 AuxCCS
register invalidations and semaphore waits (VD0_AUX_INV and VE0_AUX_INV)
on every video and page-table migration job.

[Severity: Medium]
What happens when the hardware has a graphics IP (so graphics_desc is
non-NULL), but the primary GT is disabled via configfs
(gt_types_allowed=media, as enabled by commit "drm/xe: Allow probe
without a primary GT" and commit "drm/xe: Document that
gt_types_allowed=media now probes")?

In that configuration, alloc_primary_gt() returns NULL while
xe->info.has_flat_ccs remains 1.  On discrete platforms with Flat CCS,
xe_vram_probe() -> tile_vram_size() then calls
get_flat_ccs_offset(tile->primary_gt, ...) with a NULL gt pointer and
dereferences it in gt_to_xe() and gt_to_fw().  On integrated platforms
with Flat CCS, leaving has_flat_ccs set without a primary GT keeps
xe_bo_needs_ccs_pages() and handle_system_ccs active in xe_bo_move() even
though xe_migrate_can_blit() is false.

Also, the commit message attributes the primary GT layout read to
probe_has_flat_ccs(), which only checks XE2_FLAT_CCS_ENABLE and returns 0
early when xe_root_mmio_gt(xe) is NULL; the layout read is in
tile_vram_size() -> get_flat_ccs_offset().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110231442.218721-13-stuart.summers@intel.com?part=6