mbox series

[v2,0/11] drm: Use devm_of_reserved_mem_device_init()

Message ID 20260902201640.2024648-1-mukesh.ojha@oss.qualcomm.com (mailing list archive)
Headers
Series drm: Use devm_of_reserved_mem_device_init() |

Message

Mukesh Ojha Sept. 2, 2026, 8:16 p.m. UTC 
Several DRM drivers manually manage reserved memory lifetime by pairing
of_reserved_mem_device_init() with an explicit of_reserved_mem_device_release()
in the remove or unbind path. Missing the release on any error path between
the two calls leaks the region for the lifetime of the driver.

devm_of_reserved_mem_device_init() was recently introduced to tie the
release to the device's devres lifetime automatically. Convert the affected
DRM drivers to use it.

Note for drivers using the component framework (hdlcd, malidp, sun4i): the
previous code called of_reserved_mem_device_release() explicitly in the
component unbind callback. After conversion, the release happens at device
removal (devm teardown) rather than at component unbind. For standard CMA
or system heap reserved memory, this is benign. Drivers relying on custom
rmem->ops with meaningful release-time side effects may need to retain an
explicit release in unbind; those cases are not known to exist for the
drivers converted here.

Changes since v1: https://lore.kernel.org/lkml/20260703193855.110619-2-mukesh.ojha@oss.qualcomm.com/
 - Carried R-b tag if any.
 - Rebased it.

Mukesh Ojha (11):
  drm: logicvc: Use devm_of_reserved_mem_device_init()
  drm: hdlcd: Use devm_of_reserved_mem_device_init()
  drm: pl111: Use devm_of_reserved_mem_device_init()
  drm: komeda: Use devm_of_reserved_mem_device_init()
  drm: malidp: Use devm_of_reserved_mem_device_init()
  drm: ingenic: Use devm_of_reserved_mem_device_init()
  drm: kmb: Use devm_of_reserved_mem_device_init()
  drm: sun4i: Use devm_of_reserved_mem_device_init()
  drm: xlnx: zynqmp_dpsub: Use devm_of_reserved_mem_device_init()
  drm: aspeed: Use devm_of_reserved_mem_device_init()
  drm: arcpgu: Use devm_of_reserved_mem_device_init()

 drivers/gpu/drm/logicvc/logicvc_drm.c             | 21 ++++++++-------------
 drivers/gpu/drm/arm/hdlcd_drv.c                   |  6 +-----
 drivers/gpu/drm/pl111/pl111_drv.c                 |  4 +---
 drivers/gpu/drm/arm/display/komeda/komeda_dev.c   |  4 +---
 drivers/gpu/drm/arm/malidp_drv.c                  |  4 +---
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c         | 13 +------------
 drivers/gpu/drm/kmb/kmb_drv.c                     | 12 ++----------
 drivers/gpu/drm/sun4i/sun4i_drv.c                 |  4 +---
 drivers/gpu/drm/xlnx/zynqmp_dpsub.c               |  4 +---
 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c           |  2 +-
 drivers/gpu/drm/tiny/arcpgu.c                     |  2 +-
 11 files changed, 14 insertions(+), 76 deletions(-)

--
2.34.1

Comments

sashiko-bot@kernel.org Sept. 2, 2026, 8:35 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Permanent CMA capacity leak due to clearing dev->cma_area before GEM objects are freed during device unbind.
--

commit a1f947cbe50ede0e9b650dcb9e31cc36cb85952d
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

drm: malidp: Use devm_of_reserved_mem_device_init()

This commit simplifies the driver by transitioning to the devres-managed
devm_of_reserved_mem_device_init(), removing manual cleanup calls during
bind failures and unbind.

> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 23fa942ae4bbe..3e17513c31147 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -758,7 +758,7 @@ static int malidp_bind(struct device *dev)
>  		return PTR_ERR(hwdev->pxlclk);
>  
>  	/* Get the optional framebuffer memory resource */
> -	ret = of_reserved_mem_device_init(dev);
> +	ret = devm_of_reserved_mem_device_init(dev);

[Severity: High]
This isn't a bug introduced by this patch, but does this devres conversion
preserve a pre-existing CMA memory capacity leak during device unbind?

In malidp_bind(), devm_drm_dev_alloc() is called before this initialization
function. Because devres actions execute in reverse order of allocation, the
cleanup action registered here will execute before the drm_dev_put() action.

When malidp_unbind() returns, the component framework immediately releases
the component's devres group, clearing dev->cma_area. Later, when the DRM
device is freed (either immediately or when userspace eventually closes its
file descriptors), drm_gem_object_free() calls dma_free_contiguous().

Because dev->cma_area is already NULL at that point, it bypasses
cma_release() and falls back to the buddy allocator, permanently leaking
the CMA region's bitmap capacity.

The previous code suffered from this exact same ordering issue since the
manual of_reserved_mem_device_release() call happened before drm_dev_put()
ran. Is there a way to adjust the cleanup order so the CMA area outlives
the GEM objects?

>  	if (ret && ret != -ENODEV)
>  		return ret;
>  

[ ... ]
> @@ -913,7 +912,6 @@ static void malidp_unbind(struct device *dev)
>  	else
>  		malidp_runtime_pm_suspend(dev);
>  	dev_set_drvdata(dev, NULL);
> -	of_reserved_mem_device_release(dev);
>  }
>  
>  static const struct component_master_ops malidp_master_ops = {