Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Incorrect error handling of pm_runtime_get_if_active() leads to unsafe hardware access when runtime PM is disabled. --
commit 8d990169095e4c032a10ab3f7f2aa29eb0345717 Author: Adrián Larumbe <[email protected]> drm/panfrost: Add debugfs knob for manually triggering a GPU reset This commit introduces a new debugfs file to manually schedule a GPU reset. It is intended to help test potential race conditions between the reset sequence and other hardware register accesses. > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c > b/drivers/gpu/drm/panfrost/panfrost_device.c > index f0fae6fa1e248..f171abe26f80d 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_device.c > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c [ ... ] > +static int reset_set(void *data, u64 val) > +{ > + struct panfrost_device *pfdev = > + container_of(data, struct panfrost_device, base); > + int ret = pm_runtime_get_if_active(pfdev->base.dev); > + > + if (ret) { [Severity: High] Can this cause a kernel panic if runtime PM is disabled? pm_runtime_get_if_active() returns a negative error code (like -EINVAL) when runtime PM is disabled, such as during system suspend. Because this condition checks if (ret) instead of if (ret > 0), a negative error return evaluates to true. This would cause panfrost_device_schedule_reset() to execute and flush the work queue while the device is potentially powered down, which may lead to unclocked register accesses and a synchronous external abort (bus fault). Should this condition be if (ret > 0) to ensure the device is genuinely active before proceeding with the hardware reset? > + panfrost_device_schedule_reset(pfdev); > + flush_work(&pfdev->reset.work); > + if (ret > 0) > + pm_runtime_put(pfdev->base.dev); > + } > + > + return 0; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10
