Re: [PATCH 2/5] drm/xe: Guard against NULL GT in xe_pmu.c

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

On 9/26/2025 5:59 PM, Jonathan Cavitt wrote:
> Static analysis reveals the following issue:
> xe_device_get_gt is theoretically able to return NULL in some cases, but
> several use cases don't check the return value before performing a
> dereference, resulting in a NULL pointer dereference.
> 
> Add guards against this in xe_pmu.c:
> - Use xe_root_mmio_gt instead of xe_device_get_gt for the tile 0 case.

it's about GT ID case, not tile, right?

> - Check the return value of xe_device_get_gt when this is not possible.
> - Return early from xe_pmu_event_destroy if GT is NULL.

> 
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pmu.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pmu.c b/drivers/gpu/drm/xe/xe_pmu.c
> index cab51d826345..531022d80731 100644
> --- a/drivers/gpu/drm/xe/xe_pmu.c
> +++ b/drivers/gpu/drm/xe/xe_pmu.c
> @@ -142,6 +142,9 @@ static bool event_gt_forcewake(struct perf_event *event)
>  
>  	gt = xe_device_get_gt(xe, config_to_gt_id(config));
>  
> +	if (!gt)
> +		return false;

this function is only called from xe_pmu_event_init() which already checks that GT is valid

so maybe instead of adding redundant check, just pass gt to event_gt_forcewake() ?

or just teach the tool about this false-positive?

> +
>  	fw_ref = kzalloc(sizeof(*fw_ref), GFP_KERNEL);
>  	if (!fw_ref)
>  		return false;
> @@ -219,6 +222,10 @@ static void xe_pmu_event_destroy(struct perf_event *event)
>  
>  	if (fw_ref) {
>  		gt = xe_device_get_gt(xe, config_to_gt_id(event->attr.config));
> +
> +		if (!gt)
> +			return;

now you may leak fw_ref itself and its allocation

> +
>  		xe_force_wake_put(gt_to_fw(gt), *fw_ref);
>  		kfree(fw_ref);
>  		event->pmu_private = NULL;
> @@ -497,7 +504,7 @@ static const struct attribute_group *pmu_events_attr_update[] = {
>  static void set_supported_events(struct xe_pmu *pmu)
>  {
>  	struct xe_device *xe = container_of(pmu, typeof(*xe), pmu);
> -	struct xe_gt *gt = xe_device_get_gt(xe, 0);
> +	struct xe_gt *gt = xe_root_mmio_gt(xe);
>  
>  	if (!xe->info.skip_guc_pc) {
>  		pmu->supported_events |= BIT_ULL(XE_PMU_EVENT_GT_C6_RESIDENCY);