[PATCH v2] drm/imagination: Improve logging of unknown freelist IDs

[PATCH v2] drm/imagination: Improve logging of unknown freelist IDs

[Alexandru Dadu]

From: Alessio Belle <alessio.belle@imgtec.com>

Log freelist IDs not recognised by the driver when processing freelist
grow or reconstruction requests from the firmware.

In the freelist grow request path, remove a WARN_ON() which is not
particularly useful since it is known where the request came from.

Signed-off-by: Alessio Belle <alessio.belle@imgtec.com>
Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
Changes in v2:
- Fixed an issue pointed by Sashiko by replacing drm_warn() with drm_warn_ratelimited().
- Link to v1: https://patch.msgid.link/20261006-b4-improve-logging-of-unknown-freelist-ids-v1-1-96ebcb132b80@imgtec.com

To: Alessio Belle <alessio.belle@imgtec.com>
To: Luigi Santivetti <luigi.santivetti@imgtec.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
Cc: imagination@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/gpu/drm/imagination/pvr_free_list.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_free_list.c b/drivers/gpu/drm/imagination/pvr_free_list.c
index e85cac83834c..ec02fbbe0168 100644
--- a/drivers/gpu/drm/imagination/pvr_free_list.c
+++ b/drivers/gpu/drm/imagination/pvr_free_list.c
@@ -8,6 +8,7 @@
 #include "pvr_vm.h"
 
 #include <drm/drm_gem.h>
+#include <drm/drm_print.h>
 #include <linux/slab.h>
 #include <linux/xarray.h>
 #include <uapi/drm/pvr_drm.h>
@@ -367,8 +368,12 @@ void pvr_free_list_process_grow_req(struct pvr_device *pvr_dev,
 	u32 grow_pages = 0;
 
 	/* If we don't have a freelist registered for this ID, we can't do much. */
-	if (WARN_ON(!free_list))
+	if (!free_list) {
+		drm_warn_ratelimited(from_pvr_device(pvr_dev),
+				     "Received freelist grow request for unknown ID 0x%x\n",
+				     req->freelist_id);
 		return;
+	}
 
 	/* Since the FW made the request, it has already consumed the ready pages,
 	 * update the host struct.
@@ -564,8 +569,12 @@ pvr_free_list_reconstruct(struct pvr_device *pvr_dev, u32 freelist_id)
 	struct rogue_fwif_freelist *fw_data;
 	struct pvr_hwrt_data *hwrt_data;
 
-	if (!free_list)
+	if (!free_list) {
+		drm_warn_ratelimited(from_pvr_device(pvr_dev),
+				     "Received freelist reconstruction request for unknown ID 0x%x\n",
+				     freelist_id);
 		return;
+	}
 
 	mutex_lock(&free_list->lock);
 

---
base-commit: 6e375de99d0c420169481fcd36064177ab55b09d
change-id: 20261006-b4-improve-logging-of-unknown-freelist-ids-8a69777fd61e

Best regards,
--  
Alexandru Dadu <alexandru.dadu@imgtec.com>

Re: [PATCH v2] drm/imagination: Improve logging of unknown freelist IDs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/imagination: missing lock around free_list page accounting
--

commit 26a8dc61d661134bbaa586ea04b036fc6a817608
Author: Alessio Belle <alessio.belle@imgtec.com>
drm/imagination: Improve logging of unknown freelist IDs

This commit improves logging when the driver receives freelist grow or
reconstruction requests from the firmware for unrecognised freelist IDs.
It replaces a WARN_ON() with a ratelimited warning.

Link: https://patch.msgid.link/20261006-b4-improve-logging-of-unknown-freelist-ids-v1-1-96ebcb132b80@imgtec.com

> diff --git a/drivers/gpu/drm/imagination/pvr_free_list.c b/drivers/gpu/drm/imagination/pvr_free_list.c
> index e85cac83834c6..ec02fbbe0168b 100644
> --- a/drivers/gpu/drm/imagination/pvr_free_list.c
> +++ b/drivers/gpu/drm/imagination/pvr_free_list.c
[ ... ]
> @@ -367,8 +368,12 @@ void pvr_free_list_process_grow_req(struct pvr_device *pvr_dev,
>  	u32 grow_pages = 0;
>  
>  	/* If we don't have a freelist registered for this ID, we can't do much. */
> -	if (WARN_ON(!free_list))
> +	if (!free_list) {
> +		drm_warn_ratelimited(from_pvr_device(pvr_dev),
> +				     "Received freelist grow request for unknown ID 0x%x\n",
> +				     req->freelist_id);
>  		return;
> +	}
>  
>  	/* Since the FW made the request, it has already consumed the ready pages,
>  	 * update the host struct.

[Severity: High]
Are there missing locks around the free_list page accounting in
pvr_free_list_process_grow_req()?

Right below this change, free_list->current_pages and free_list->ready_pages are
modified without holding free_list->lock:

	free_list->current_pages += free_list->ready_pages;
	free_list->ready_pages = 0;

The FWCCB processing loop (pvr_fwccb_process) drops pvr_dev->fwccb.lock before
calling process_fwccb_command().

Since pvr_fwccb_process() is called by both the IRQ thread
(pvr_device_irq_thread_handler) and the release path (pvr_free_list_release),
could multiple threads concurrently pull commands from the global FWCCB queue?

If pvr_free_list_release() runs concurrently with the IRQ thread, it appears
they could execute pvr_free_list_process_grow_req() or
pvr_free_list_reconstruct() for the same freelist concurrently.

Could this result in a data race and corrupted page tracking, causing the
firmware to access unmapped or unallocated memory (GPU MMU fault)?

Additionally, after pvr_free_list_grow() returns, the fields are read again
without holding the lock:

	resp->new_pages = free_list->current_pages + free_list->ready_pages;
	resp->ready_pages = free_list->ready_pages;

Could this create a time-of-check to time-of-use vulnerability where the values
are modified by another thread before being used to populate the response?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-b4-improve-logging-of-unknown-freelist-ids-v2-1-5e6c3f63370e@imgtec.com?part=1