On Mon, Aug 17, 2026 at 10:17:42AM +0530, Ekansh Gupta wrote:
> Introduce the QDA memory manager (qda_memory_manager) to track the
> IOMMU devices that back each compute context bank (CB).
>
> Each CB device registered on the qda-compute-cb bus is wrapped in a
> qda_iommu_device descriptor recording the device pointer and its stream
> ID, and stored in the memory manager's registry. Later patches use this
> registry to resolve the IOMMU device a session should allocate from.
>
> The registry is a plain array sized to the number of
> "qcom,fastrpc-compute-cb" nodes present in the device tree: the RPMsg
> probe counts those nodes and passes the count to qda_init_device(),
> which allocates the array in qda_memory_manager_init(). The memory
> manager is created before CB devices are populated and destroyed after
> they are torn down, so no dangling descriptors remain.
>
> qda_cb_setup_device() is called immediately after a CB device is
> registered on the bus: it allocates the descriptor, registers it with
> the memory manager, and stores it as the CB device's driver data so
> that qda_destroy_cb_device() can unregister and free it during teardown.
>
> Assisted-by: Claude:claude-sonnet-5
> Signed-off-by: Ekansh Gupta <[email protected]>
> ---
> Changes in v2:
> - Replace the XArray with a plain array sized to the device tree's CB
> node count instead of a fixed 16-entry table (Dmitry Baryshkov)
> - Fold the init_memory_manager()/cleanup_memory_manager() wrappers into
> qda_init_device()/qda_deinit_device() (Dmitry Baryshkov)
> - Drop the pr_debug() calls (Dmitry Baryshkov)
> - Use goto labels to unwind probe failures instead of open-coding the
> cleanup at each error site (Dmitry Baryshkov)
> ---
> drivers/accel/qda/Makefile | 1 +
> drivers/accel/qda/qda_cb.c | 39 ++++++++++++++
> drivers/accel/qda/qda_drv.c | 26 +++++++++
> drivers/accel/qda/qda_drv.h | 5 ++
> drivers/accel/qda/qda_memory_manager.c | 98
> ++++++++++++++++++++++++++++++++++
> drivers/accel/qda/qda_memory_manager.h | 55 +++++++++++++++++++
> drivers/accel/qda/qda_rpmsg.c | 23 +++++++-
> 7 files changed, 246 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/qda/Makefile b/drivers/accel/qda/Makefile
> index 143c9e4e789e..701fad5ffb50 100644
> --- a/drivers/accel/qda/Makefile
> +++ b/drivers/accel/qda/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_ACCEL_QDA) := qda.o
> qda-y := \
> qda_cb.o \
> qda_drv.o \
> + qda_memory_manager.o \
> qda_rpmsg.o
>
> obj-$(CONFIG_DRM_ACCEL_QDA_COMPUTE_BUS) += qda_compute_bus.o
> diff --git a/drivers/accel/qda/qda_cb.c b/drivers/accel/qda/qda_cb.c
> index 9c5ef32f351e..cd32d6df6318 100644
> --- a/drivers/accel/qda/qda_cb.c
> +++ b/drivers/accel/qda/qda_cb.c
> @@ -8,13 +8,40 @@
> #include <linux/slab.h>
> #include <drm/drm_print.h>
> #include "qda_drv.h"
> +#include "qda_memory_manager.h"
> #include "qda_cb.h"
>
> +static int qda_cb_setup_device(struct qda_dev *qdev, struct device *cb_dev,
> u32 sid)
> +{
> + struct qda_iommu_device *iommu_dev;
> + int ret;
> +
> + iommu_dev = kzalloc_obj(*iommu_dev);
> + if (!iommu_dev)
> + return -ENOMEM;
> +
> + iommu_dev->dev = cb_dev;
> + iommu_dev->qdev = qdev;
> + iommu_dev->sid = sid;
> +
> + ret = qda_memory_manager_register_device(qdev->iommu_mgr, iommu_dev);
> + if (ret) {
> + drm_err(&qdev->drm_dev, "Failed to register IOMMU device:
> %d\n", ret);
> + kfree(iommu_dev);
> + return ret;
> + }
> +
> + dev_set_drvdata(cb_dev, iommu_dev);
> +
> + return 0;
> +}
> +
> int qda_create_cb_device(struct qda_dev *qdev, struct device_node *cb_node)
> {
> struct qda_cb_dev *entry;
> struct device *cb_dev;
> u32 sid = 0;
> + int ret;
>
> if (of_property_read_u32(cb_node, "reg", &sid))
> drm_info(&qdev->drm_dev, "No stream ID in DT node %pOFn, using
> 0\n", cb_node);
> @@ -27,6 +54,12 @@ int qda_create_cb_device(struct qda_dev *qdev, struct
> device_node *cb_node)
> return PTR_ERR(cb_dev);
> }
>
> + ret = qda_cb_setup_device(qdev, cb_dev, sid);
> + if (ret) {
> + device_unregister(cb_dev);
> + return ret;
> + }
> +
> entry = kzalloc_obj(*entry);
> if (!entry) {
> qda_destroy_cb_device(cb_dev);
> @@ -74,8 +107,14 @@ int qda_cb_populate(struct qda_dev *qdev, struct
> device_node *parent_node)
>
> void qda_destroy_cb_device(struct device *cb_dev)
> {
> + struct qda_iommu_device *iommu_dev;
> struct iommu_group *group;
>
> + iommu_dev = dev_get_drvdata(cb_dev);
> + if (iommu_dev && iommu_dev->qdev->iommu_mgr)
Can either of them be NULL?
> + qda_memory_manager_unregister_device(iommu_dev->qdev->iommu_mgr,
> + iommu_dev);
> +
> group = iommu_group_get(cb_dev);
> if (group) {
> iommu_group_remove_device(cb_dev);
> diff --git a/drivers/accel/qda/qda_drv.c b/drivers/accel/qda/qda_drv.c
> index 09b6d5ac390b..fdc909facd95 100644
> --- a/drivers/accel/qda/qda_drv.c
> +++ b/drivers/accel/qda/qda_drv.c
> @@ -56,6 +56,32 @@ struct qda_dev *qda_alloc_device(struct device *dev)
> return qdev;
> }
>
> +void qda_deinit_device(struct qda_dev *qdev)
> +{
> + if (qdev->iommu_mgr) {
Why?
> + qda_memory_manager_exit(qdev->iommu_mgr);
> + kfree(qdev->iommu_mgr);
> + qdev->iommu_mgr = NULL;
What for? If it's a teardown path, qdev will be destroyed soon.
> + }
> +}
> +
> +int qda_init_device(struct qda_dev *qdev, int num_cbs)
> +{
> + int ret;
> +
> + qdev->iommu_mgr = kzalloc_obj(*qdev->iommu_mgr);
devm_kzalloc_obj()?
> + if (!qdev->iommu_mgr)
> + return -ENOMEM;
> + ret = qda_memory_manager_init(qdev->iommu_mgr, num_cbs);
> + if (ret) {
> + drm_err(&qdev->drm_dev, "Failed to initialize memory manager:
> %d\n", ret);
> + kfree(qdev->iommu_mgr);
> + qdev->iommu_mgr = NULL;
And what is going to read this NULL?
> + }
> +
> + return ret;
> +}
> +
> int qda_register_device(struct qda_dev *qdev)
> {
> int ret;
> diff --git a/drivers/accel/qda/qda_drv.h b/drivers/accel/qda/qda_drv.h
> index 3bd41b9593fa..690a833d732b 100644
> --- a/drivers/accel/qda/qda_drv.h
> +++ b/drivers/accel/qda/qda_drv.h
> @@ -13,6 +13,7 @@
> #include <drm/drm_device.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_file.h>
> +#include "qda_memory_manager.h"
>
> /* Driver identification */
> #define QDA_DRIVER_NAME "qda"
> @@ -40,6 +41,8 @@ struct qda_dev {
> struct device *dev;
> /** @cb_devs: Compute context-bank (CB) child devices */
> struct list_head cb_devs;
> + /** @iommu_mgr: IOMMU/memory manager instance */
> + struct qda_memory_manager *iommu_mgr;
> /** @dsp_name: Name of the DSP domain (e.g. "cdsp", "adsp") */
> const char *dsp_name;
> };
> @@ -59,6 +62,8 @@ static inline struct qda_dev *qda_dev_from_drm(struct
> drm_device *dev)
> struct qda_dev *qda_alloc_device(struct device *dev);
>
> /* Core device lifecycle */
> +int qda_init_device(struct qda_dev *qdev, int num_cbs);
> +void qda_deinit_device(struct qda_dev *qdev);
> int qda_register_device(struct qda_dev *qdev);
>
> #endif /* __QDA_DRV_H__ */
> diff --git a/drivers/accel/qda/qda_memory_manager.c
> b/drivers/accel/qda/qda_memory_manager.c
> new file mode 100644
> index 000000000000..b1a80ee77c35
> --- /dev/null
> +++ b/drivers/accel/qda/qda_memory_manager.c
> @@ -0,0 +1,98 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> +
> +#include <linux/slab.h>
> +#include "qda_drv.h"
> +#include "qda_memory_manager.h"
> +
> +static void cleanup_all_memory_devices(struct qda_memory_manager *mem_mgr)
> +{
> + int i;
> +
> + for (i = 0; i < mem_mgr->num_devices; i++) {
> + struct qda_iommu_device *iommu_dev = mem_mgr->devices[i];
> +
> + if (!iommu_dev)
> + continue;
> +
> + mem_mgr->devices[i] = NULL;
> + kfree(iommu_dev);
> + }
> + mem_mgr->num_devices = 0;
What for?
> +}
> +
> +/**
> + * qda_memory_manager_register_device() - Register an IOMMU device
> + * @mem_mgr: Pointer to memory manager
> + * @iommu_dev: Pointer to IOMMU device to register
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +int qda_memory_manager_register_device(struct qda_memory_manager *mem_mgr,
> + struct qda_iommu_device *iommu_dev)
> +{
> + if (mem_mgr->num_devices >= mem_mgr->max_devices)
> + return -ENOSPC;
> +
> + iommu_dev->id = mem_mgr->num_devices;
> + mem_mgr->devices[mem_mgr->num_devices++] = iommu_dev;
> +
> + return 0;
> +}
> +
> +/**
> + * qda_memory_manager_unregister_device() - Unregister an IOMMU device
> + * @mem_mgr: Pointer to memory manager
> + * @iommu_dev: Pointer to IOMMU device to unregister
> + *
> + * Removes the device from the registry and frees it.
> + */
> +void qda_memory_manager_unregister_device(struct qda_memory_manager *mem_mgr,
> + struct qda_iommu_device *iommu_dev)
> +{
> + int i;
> +
> + for (i = 0; i < mem_mgr->num_devices; i++) {
> + if (mem_mgr->devices[i] == iommu_dev) {
> + mem_mgr->devices[i] = NULL;
> + break;
> + }
> + }
> +
> + kfree(iommu_dev);
> +}
> +
> +/**
> + * qda_memory_manager_init() - Initialize the memory manager
> + * @mem_mgr: Pointer to memory manager structure to initialize
> + * @max_devices: Number of IOMMU context bank devices to make room for
> + *
> + * Return: 0 on success, negative error code on failure
> + */
> +int qda_memory_manager_init(struct qda_memory_manager *mem_mgr, int
> max_devices)
> +{
> + if (max_devices <= 0)
> + return -EINVAL;
> +
> + mem_mgr->devices = kcalloc(max_devices, sizeof(*mem_mgr->devices),
> GFP_KERNEL);
> + if (!mem_mgr->devices)
> + return -ENOMEM;
> +
> + mem_mgr->num_devices = 0;
> + mem_mgr->max_devices = max_devices;
> +
> + return 0;
> +}
> +
> +/**
> + * qda_memory_manager_exit() - Clean up the memory manager
> + * @mem_mgr: Pointer to memory manager structure to clean up
> + */
> +void qda_memory_manager_exit(struct qda_memory_manager *mem_mgr)
> +{
> + cleanup_all_memory_devices(mem_mgr);
> +
> + kfree(mem_mgr->devices);
> + mem_mgr->devices = NULL;
> + mem_mgr->max_devices = 0;
> +}
> diff --git a/drivers/accel/qda/qda_memory_manager.h
> b/drivers/accel/qda/qda_memory_manager.h
> new file mode 100644
> index 000000000000..7e38c8a18284
> --- /dev/null
> +++ b/drivers/accel/qda/qda_memory_manager.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef __QDA_MEMORY_MANAGER_H__
> +#define __QDA_MEMORY_MANAGER_H__
> +
> +#include <linux/device.h>
> +
> +/* Forward declarations */
> +struct qda_dev;
> +
> +/**
> + * struct qda_iommu_device - IOMMU device instance for memory management
> + *
> + * Represents a single IOMMU-enabled device managed by the memory manager.
> + * Each device can be assigned to a specific process session.
> + */
> +struct qda_iommu_device {
> + /** @dev: Pointer to the underlying device */
> + struct device *dev;
> + /** @qdev: Back-pointer to the parent QDA device */
> + struct qda_dev *qdev;
> + /** @id: Unique identifier assigned by the memory manager */
> + u32 id;
What kind of identifier? What is going to use it? I don't see any
readers in this patch, please don't add useless data.
> + /** @sid: Stream ID for IOMMU transactions */
> + u32 sid;
Do you need to store it?
> +};
> +
> +/**
> + * struct qda_memory_manager - Central memory management coordinator
> + *
> + * Coordinates memory management across multiple IOMMU devices. Maintains
> + * a registry of devices in an array sized to the number of context banks
> + * described in the device tree.
> + */
> +struct qda_memory_manager {
> + /** @devices: Array storing registered IOMMU devices */
> + struct qda_iommu_device **devices;
> + /** @num_devices: Number of registered IOMMU devices */
> + int num_devices;
What for? Is devices array to be looped up to num_devices or
max_devices?
> + /** @max_devices: Capacity of the @devices array */
> + int max_devices;
> +};
> +
> +int qda_memory_manager_init(struct qda_memory_manager *mem_mgr, int
> max_devices);
> +void qda_memory_manager_exit(struct qda_memory_manager *mem_mgr);
> +
> +int qda_memory_manager_register_device(struct qda_memory_manager *mem_mgr,
> + struct qda_iommu_device *iommu_dev);
> +void qda_memory_manager_unregister_device(struct qda_memory_manager *mem_mgr,
> + struct qda_iommu_device *iommu_dev);
> +
> +#endif /* __QDA_MEMORY_MANAGER_H__ */
> diff --git a/drivers/accel/qda/qda_rpmsg.c b/drivers/accel/qda/qda_rpmsg.c
> index 723b3e61bf0a..64bf503106d9 100644
> --- a/drivers/accel/qda/qda_rpmsg.c
> +++ b/drivers/accel/qda/qda_rpmsg.c
> @@ -42,11 +42,25 @@ static void qda_rpmsg_remove(struct rpmsg_device *rpdev)
> drm_dev_unplug(&qdev->drm_dev);
> qdev->rpdev = NULL;
> qda_cb_unpopulate(qdev);
> + qda_deinit_device(qdev);
> +}
> +
> +static int qda_count_cb_nodes(struct device_node *parent_node)
> +{
> + struct device_node *child;
> + int count = 0;
> +
> + for_each_child_of_node(parent_node, child)
> + if (of_device_is_compatible(child, "qcom,fastrpc-compute-cb"))
> + count++;
> +
> + return count;
> }
>
> static int qda_rpmsg_probe(struct rpmsg_device *rpdev)
> {
> struct qda_dev *qdev;
> + int num_cbs;
> int ret;
>
> qdev = qda_rpmsg_alloc_and_init_qdev(rpdev);
> @@ -59,10 +73,15 @@ static int qda_rpmsg_probe(struct rpmsg_device *rpdev)
> return ret;
> }
>
> - ret = qda_cb_populate(qdev, rpdev->dev.of_node);
> + num_cbs = qda_count_cb_nodes(rpdev->dev.of_node);
> + ret = qda_init_device(qdev, num_cbs);
> if (ret)
> return ret;
>
> + ret = qda_cb_populate(qdev, rpdev->dev.of_node);
> + if (ret)
> + goto err_deinit;
> +
> ret = qda_register_device(qdev);
> if (ret)
> goto err_unpopulate;
> @@ -71,6 +90,8 @@ static int qda_rpmsg_probe(struct rpmsg_device *rpdev)
>
> err_unpopulate:
> qda_cb_unpopulate(qdev);
> +err_deinit:
> + qda_deinit_device(qdev);
> return ret;
> }
>
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
