Re: [PATCH V18 10/14] drm/xe/ras: Cache bad_page_reservation policy at init

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The commit message claims caching happens in xe_drm_ras_init(), but it is actually implemented in xe_ras_init().

Pre-existing issues:
- [High] Ignored return value from `xe_drm_ras_init()` leads to potential NULL pointer dereference on error paths.
--

commit 5b627dce853b33e2e2e98dd02e3de8fcd0a8ae35
Author: Tejas Upadhyay <tejas.upadhyay@intel.com>

drm/xe/ras: Cache bad_page_reservation policy at init

This commit caches the configfs bad_page_reservation attribute during driver
initialization to prevent repeated configfs lookups on every fault.

> drm/xe/ras: Cache bad_page_reservation policy at init
> 
> The configfs bad_page_reservation attribute can only be set before
> device bind, so its value is immutable at runtime. Cache it in
> struct xe_drm_ras during xe_drm_ras_init() to avoid repeated configfs
> lookups on every fault.

[Severity: Medium]
The commit message states that caching happens "during xe_drm_ras_init()", but
the patch actually implements the configfs lookup and caching in xe_ras_init()
instead. Should the commit message be updated to reflect the correct function?

> diff --git a/drivers/gpu/drm/xe/xe_ras.c b/drivers/gpu/drm/xe/xe_ras.c
> index d25d25f775310..c7e86ae0e8adb 100644
> --- a/drivers/gpu/drm/xe/xe_ras.c
> +++ b/drivers/gpu/drm/xe/xe_ras.c
[ ... ]
> @@ -804,6 +805,10 @@ void xe_ras_init(struct xe_device *xe)
>  {
>  	int ret;
>  
> +	if (xe->info.platform == XE_CRESCENTISLAND)
> +		xe->ras.bad_page_reservation =
> +			xe_configfs_get_bad_page_reservation(to_pci_dev(xe->drm.dev));
> +
>  	xe_drm_ras_init(xe);

[Severity: High]
This isn't a bug introduced by this patch, but the return value of
xe_drm_ras_init() is completely ignored here. 

If xe_drm_ras_init() fails (e.g., if drmm_kcalloc() returns NULL),
xe->ras.node remains NULL. Because the error is ignored, device initialization
continues and DRM RAS is considered enabled (has_drm_ras remains true).

Later, if a hardware error triggers xe_drm_ras_event(), the code calculates:

    node = &ras->node[severity];

If severity > 0, this results in a non-NULL but invalid pointer (computed as a
small offset from NULL). drm_ras_nl_error_event() receives this pointer, its
!node check passes, and it dereferences node->id, causing a kernel panic.

Should we check the return value of xe_drm_ras_init() and handle the failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826135136.204044-16-tejas.upadhyay@intel.com?part=10