Re: [PATCH v5 1/2] drm/xe/pagefault: Add SRCID to pagefault struct

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The format specifier `%ld` is used for an `unsigned long` argument in `xe_pagefault_print()`, causing a signedness mismatch.
--

commit a84658007b56ca870c768c6042faa49f53ab109e
Author: Jonathan Cavitt <jonathan.cavitt@intel.com>
Subject: drm/xe/pagefault: Add SRCID to pagefault struct

This commit adds SRCID information to the pagefault struct to report the
hardware unit that triggered the pagefault. It folds the SRCID into the
existing ASID field to maintain a compact structure.

> diff --git a/drivers/gpu/drm/xe/xe_pagefault.c b/drivers/gpu/drm/xe/xe_pagefault.c
> index dd3c068e1a399..3b0510f2a72e6 100644
> --- a/drivers/gpu/drm/xe/xe_pagefault.c
> +++ b/drivers/gpu/drm/xe/xe_pagefault.c

[ ... ]

> @@ -242,14 +243,16 @@ static bool xe_pagefault_queue_pop(struct xe_pagefault_queue *pf_queue,
>  
>  static void xe_pagefault_print(struct xe_pagefault *pf)
>  {
> -	xe_gt_info(pf->gt, "\n\tASID: %d\n"
> +	xe_gt_info(pf->gt, "\n\tASID: %ld\n"

[Severity: Low]
Does this code introduce a format specifier mismatch? FIELD_GET() returns an
unsigned long, but the format string uses %ld which expects a signed long.
Should this use %lu instead to maintain format string type safety?

>  		   "\tFaulted Address: 0x%08x%08x\n"
>  		   "\tFaultType: %lu\n"
>  		   "\tAccessType: %lu\n"
>  		   "\tFaultLevel: %lu\n"
>  		   "\tEngineClass: %d %s\n"
> -		   "\tEngineInstance: %d\n",
> -		   pf->consumer.asid,
> +		   "\tEngineInstance: %d\n"
> +		   "\tSRCID: 0x%02lx\n",
> +		   FIELD_GET(XE_PAGEFAULT_ASID_MASK,
> +			     pf->consumer.id),
>  		   upper_32_bits(pf->consumer.page_addr),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623164610.1019512-1-jonathan.cavitt@intel.com?part=1