public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v3] accel/ivpu: Limit number of maximum contexts and doorbells per user
@ 2026-03-02 20:22 Maciej Falkowski
  2026-03-02 20:45 ` Lizhi Hou
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Maciej Falkowski @ 2026-03-02 20:22 UTC (permalink / raw)
  To: dri-devel
  Cc: oded.gabbay, jeff.hugo, karol.wachowski, lizhi.hou,
	Maciej Falkowski

From: Karol Wachowski <karol.wachowski@linux.intel.com>

Implement per-user resource limits to prevent resource exhaustion.

Root users can allocate all available contexts (128) and doorbells
(255), while non-root users are limited to half of the available
resources (64 contexts and 127 doorbells respectively).

This prevents scenarios where a single user could monopolize NPU
resources and starve other users on multi-user systems.

Change doorbell ID and command queue ID allocation errors to debug
messages as those are user triggered.

Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
---
v2 -> v3:
  - Refactor to use kzalloc_obj*() due to treewide rework.
v1 -> v2:
  - Fixed off-by-one error (Lizhi)
---
 drivers/accel/ivpu/ivpu_drv.c | 94 ++++++++++++++++++++++++++++++++---
 drivers/accel/ivpu/ivpu_drv.h | 26 ++++++++--
 drivers/accel/ivpu/ivpu_job.c | 36 ++++++++++----
 3 files changed, 136 insertions(+), 20 deletions(-)

diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
index 5900a40c7a78..dd3a486df5f1 100644
--- a/drivers/accel/ivpu/ivpu_drv.c
+++ b/drivers/accel/ivpu/ivpu_drv.c
@@ -67,6 +67,73 @@ bool ivpu_force_snoop;
 module_param_named(force_snoop, ivpu_force_snoop, bool, 0444);
 MODULE_PARM_DESC(force_snoop, "Force snooping for NPU host memory access");
 
+static struct ivpu_user_limits *ivpu_user_limits_alloc(struct ivpu_device *vdev, uid_t uid)
+{
+	struct ivpu_user_limits *limits;
+
+	limits = kzalloc_obj(*limits);
+	if (!limits)
+		return ERR_PTR(-ENOMEM);
+
+	kref_init(&limits->ref);
+	atomic_set(&limits->db_count, 0);
+	limits->vdev = vdev;
+	limits->uid = uid;
+
+	/* Allow root user to allocate all contexts */
+	if (uid == 0) {
+		limits->max_ctx_count = ivpu_get_context_count(vdev);
+		limits->max_db_count = ivpu_get_doorbell_count(vdev);
+	} else {
+		limits->max_ctx_count = ivpu_get_context_count(vdev) / 2;
+		limits->max_db_count = ivpu_get_doorbell_count(vdev) / 2;
+	}
+
+	hash_add(vdev->user_limits, &limits->hash_node, uid);
+
+	return limits;
+}
+
+static struct ivpu_user_limits *ivpu_user_limits_get(struct ivpu_device *vdev)
+{
+	struct ivpu_user_limits *limits;
+	uid_t uid = current_uid().val;
+
+	guard(mutex)(&vdev->user_limits_lock);
+
+	hash_for_each_possible(vdev->user_limits, limits, hash_node, uid) {
+		if (limits->uid == uid) {
+			if (kref_read(&limits->ref) >= limits->max_ctx_count) {
+				ivpu_dbg(vdev, IOCTL, "User %u exceeded max ctx count %u\n", uid,
+					 limits->max_ctx_count);
+				return ERR_PTR(-EMFILE);
+			}
+
+			kref_get(&limits->ref);
+			return limits;
+		}
+	}
+
+	return ivpu_user_limits_alloc(vdev, uid);
+}
+
+static void ivpu_user_limits_release(struct kref *ref)
+{
+	struct ivpu_user_limits *limits = container_of(ref, struct ivpu_user_limits, ref);
+	struct ivpu_device *vdev = limits->vdev;
+
+	lockdep_assert_held(&vdev->user_limits_lock);
+	drm_WARN_ON(&vdev->drm, atomic_read(&limits->db_count));
+	hash_del(&limits->hash_node);
+	kfree(limits);
+}
+
+static void ivpu_user_limits_put(struct ivpu_device *vdev, struct ivpu_user_limits *limits)
+{
+	guard(mutex)(&vdev->user_limits_lock);
+	kref_put(&limits->ref, ivpu_user_limits_release);
+}
+
 struct ivpu_file_priv *ivpu_file_priv_get(struct ivpu_file_priv *file_priv)
 {
 	struct ivpu_device *vdev = file_priv->vdev;
@@ -110,6 +177,7 @@ static void file_priv_release(struct kref *ref)
 	mutex_unlock(&vdev->context_list_lock);
 	pm_runtime_put_autosuspend(vdev->drm.dev);
 
+	ivpu_user_limits_put(vdev, file_priv->user_limits);
 	mutex_destroy(&file_priv->ms_lock);
 	mutex_destroy(&file_priv->lock);
 	kfree(file_priv);
@@ -169,7 +237,7 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_f
 		args->value = ivpu_hw_dpu_max_freq_get(vdev);
 		break;
 	case DRM_IVPU_PARAM_NUM_CONTEXTS:
-		args->value = ivpu_get_context_count(vdev);
+		args->value = file_priv->user_limits->max_ctx_count;
 		break;
 	case DRM_IVPU_PARAM_CONTEXT_BASE_ADDRESS:
 		args->value = vdev->hw->ranges.user.start;
@@ -231,22 +299,30 @@ static int ivpu_open(struct drm_device *dev, struct drm_file *file)
 {
 	struct ivpu_device *vdev = to_ivpu_device(dev);
 	struct ivpu_file_priv *file_priv;
+	struct ivpu_user_limits *limits;
 	u32 ctx_id;
 	int idx, ret;
 
 	if (!drm_dev_enter(dev, &idx))
 		return -ENODEV;
 
+	limits = ivpu_user_limits_get(vdev);
+	if (IS_ERR(limits)) {
+		ret = PTR_ERR(limits);
+		goto err_dev_exit;
+	}
+
 	file_priv = kzalloc_obj(*file_priv);
 	if (!file_priv) {
 		ret = -ENOMEM;
-		goto err_dev_exit;
+		goto err_user_limits_put;
 	}
 
 	INIT_LIST_HEAD(&file_priv->ms_instance_list);
 
 	file_priv->vdev = vdev;
 	file_priv->bound = true;
+	file_priv->user_limits = limits;
 	kref_init(&file_priv->ref);
 	mutex_init(&file_priv->lock);
 	mutex_init(&file_priv->ms_lock);
@@ -284,6 +360,8 @@ static int ivpu_open(struct drm_device *dev, struct drm_file *file)
 	mutex_destroy(&file_priv->ms_lock);
 	mutex_destroy(&file_priv->lock);
 	kfree(file_priv);
+err_user_limits_put:
+	ivpu_user_limits_put(vdev, limits);
 err_dev_exit:
 	drm_dev_exit(idx);
 	return ret;
@@ -343,8 +421,7 @@ static int ivpu_wait_for_ready(struct ivpu_device *vdev)
 	ivpu_ipc_consumer_del(vdev, &cons);
 
 	if (!ret && ipc_hdr.data_addr != IVPU_IPC_BOOT_MSG_DATA_ADDR) {
-		ivpu_err(vdev, "Invalid NPU ready message: 0x%x\n",
-			 ipc_hdr.data_addr);
+		ivpu_err(vdev, "Invalid NPU ready message: 0x%x\n", ipc_hdr.data_addr);
 		return -EIO;
 	}
 
@@ -453,7 +530,7 @@ int ivpu_shutdown(struct ivpu_device *vdev)
 }
 
 static const struct file_operations ivpu_fops = {
-	.owner		= THIS_MODULE,
+	.owner = THIS_MODULE,
 	DRM_ACCEL_FOPS,
 #ifdef CONFIG_PROC_FS
 	.show_fdinfo = drm_show_fdinfo,
@@ -592,6 +669,7 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
 	xa_init_flags(&vdev->submitted_jobs_xa, XA_FLAGS_ALLOC1);
 	xa_init_flags(&vdev->db_xa, XA_FLAGS_ALLOC1);
 	INIT_LIST_HEAD(&vdev->bo_list);
+	hash_init(vdev->user_limits);
 
 	vdev->db_limit.min = IVPU_MIN_DB;
 	vdev->db_limit.max = IVPU_MAX_DB;
@@ -600,6 +678,10 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
 	if (ret)
 		goto err_xa_destroy;
 
+	ret = drmm_mutex_init(&vdev->drm, &vdev->user_limits_lock);
+	if (ret)
+		goto err_xa_destroy;
+
 	ret = drmm_mutex_init(&vdev->drm, &vdev->submitted_jobs_lock);
 	if (ret)
 		goto err_xa_destroy;
@@ -717,7 +799,7 @@ static struct pci_device_id ivpu_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_PTL_P) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_WCL) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_NVL) },
-	{ }
+	{}
 };
 MODULE_DEVICE_TABLE(pci, ivpu_pci_ids);
 
diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
index 5b34b6f50e69..6378e23e0c97 100644
--- a/drivers/accel/ivpu/ivpu_drv.h
+++ b/drivers/accel/ivpu/ivpu_drv.h
@@ -12,6 +12,7 @@
 #include <drm/drm_mm.h>
 #include <drm/drm_print.h>
 
+#include <linux/hashtable.h>
 #include <linux/pci.h>
 #include <linux/xarray.h>
 #include <uapi/drm/ivpu_accel.h>
@@ -43,7 +44,7 @@
 /* SSID 1 is used by the VPU to represent reserved context */
 #define IVPU_RESERVED_CONTEXT_MMU_SSID 1
 #define IVPU_USER_CONTEXT_MIN_SSID     2
-#define IVPU_USER_CONTEXT_MAX_SSID     (IVPU_USER_CONTEXT_MIN_SSID + 63)
+#define IVPU_USER_CONTEXT_MAX_SSID     (IVPU_USER_CONTEXT_MIN_SSID + 128)
 
 #define IVPU_MIN_DB 1
 #define IVPU_MAX_DB 255
@@ -51,9 +52,6 @@
 #define IVPU_JOB_ID_JOB_MASK		GENMASK(7, 0)
 #define IVPU_JOB_ID_CONTEXT_MASK	GENMASK(31, 8)
 
-#define IVPU_NUM_PRIORITIES    4
-#define IVPU_NUM_CMDQS_PER_CTX (IVPU_NUM_PRIORITIES)
-
 #define IVPU_CMDQ_MIN_ID 1
 #define IVPU_CMDQ_MAX_ID 255
 
@@ -123,6 +121,16 @@ struct ivpu_fw_info;
 struct ivpu_ipc_info;
 struct ivpu_pm_info;
 
+struct ivpu_user_limits {
+	struct hlist_node hash_node;
+	struct ivpu_device *vdev;
+	struct kref ref;
+	u32 max_ctx_count;
+	u32 max_db_count;
+	u32 uid;
+	atomic_t db_count;
+};
+
 struct ivpu_device {
 	struct drm_device drm;
 	void __iomem *regb;
@@ -142,6 +150,8 @@ struct ivpu_device {
 	struct mutex context_list_lock; /* Protects user context addition/removal */
 	struct xarray context_xa;
 	struct xa_limit context_xa_limit;
+	DECLARE_HASHTABLE(user_limits, 8);
+	struct mutex user_limits_lock; /* Protects user_limits */
 
 	struct xarray db_xa;
 	struct xa_limit db_limit;
@@ -189,6 +199,7 @@ struct ivpu_file_priv {
 	struct list_head ms_instance_list;
 	struct ivpu_bo *ms_info_bo;
 	struct xa_limit job_limit;
+	struct ivpu_user_limits *user_limits;
 	u32 job_id_next;
 	struct xa_limit cmdq_limit;
 	u32 cmdq_id_next;
@@ -286,6 +297,13 @@ static inline u32 ivpu_get_context_count(struct ivpu_device *vdev)
 	return (ctx_limit.max - ctx_limit.min + 1);
 }
 
+static inline u32 ivpu_get_doorbell_count(struct ivpu_device *vdev)
+{
+	struct xa_limit db_limit = vdev->db_limit;
+
+	return (db_limit.max - db_limit.min + 1);
+}
+
 static inline u32 ivpu_get_platform(struct ivpu_device *vdev)
 {
 	WARN_ON_ONCE(vdev->platform == IVPU_PLATFORM_INVALID);
diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
index fe02b7bd465b..f0154dfa6ddc 100644
--- a/drivers/accel/ivpu/ivpu_job.c
+++ b/drivers/accel/ivpu/ivpu_job.c
@@ -173,7 +173,7 @@ static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 p
 	ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
 			      &file_priv->cmdq_id_next, GFP_KERNEL);
 	if (ret < 0) {
-		ivpu_err(vdev, "Failed to allocate command queue ID: %d\n", ret);
+		ivpu_dbg(vdev, IOCTL, "Failed to allocate command queue ID: %d\n", ret);
 		goto err_free_cmdq;
 	}
 
@@ -215,14 +215,22 @@ static int ivpu_hws_cmdq_init(struct ivpu_file_priv *file_priv, struct ivpu_cmdq
 
 static int ivpu_register_db(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
 {
+	struct ivpu_user_limits *limits = file_priv->user_limits;
 	struct ivpu_device *vdev = file_priv->vdev;
 	int ret;
 
+	if (atomic_inc_return(&limits->db_count) > limits->max_db_count) {
+		ivpu_dbg(vdev, IOCTL, "Maximum number of %u doorbells for uid %u reached\n",
+			 limits->max_db_count, limits->uid);
+		ret = -EBUSY;
+		goto err_dec_db_count;
+	}
+
 	ret = xa_alloc_cyclic(&vdev->db_xa, &cmdq->db_id, NULL, vdev->db_limit, &vdev->db_next,
 			      GFP_KERNEL);
 	if (ret < 0) {
-		ivpu_err(vdev, "Failed to allocate doorbell ID: %d\n", ret);
-		return ret;
+		ivpu_dbg(vdev, IOCTL, "Failed to allocate doorbell ID: %d\n", ret);
+		goto err_dec_db_count;
 	}
 
 	if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW)
@@ -231,15 +239,18 @@ static int ivpu_register_db(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *
 	else
 		ret = ivpu_jsm_register_db(vdev, file_priv->ctx.id, cmdq->db_id,
 					   cmdq->mem->vpu_addr, ivpu_bo_size(cmdq->mem));
-
-	if (!ret) {
-		ivpu_dbg(vdev, JOB, "DB %d registered to cmdq %d ctx %d priority %d\n",
-			 cmdq->db_id, cmdq->id, file_priv->ctx.id, cmdq->priority);
-	} else {
+	if (ret) {
 		xa_erase(&vdev->db_xa, cmdq->db_id);
 		cmdq->db_id = 0;
+		goto err_dec_db_count;
 	}
 
+	ivpu_dbg(vdev, JOB, "DB %d registered to cmdq %d ctx %d priority %d\n",
+		 cmdq->db_id, cmdq->id, file_priv->ctx.id, cmdq->priority);
+	return 0;
+
+err_dec_db_count:
+	atomic_dec(&limits->db_count);
 	return ret;
 }
 
@@ -298,6 +309,7 @@ static int ivpu_cmdq_unregister(struct ivpu_file_priv *file_priv, struct ivpu_cm
 	}
 
 	xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
+	atomic_dec(&file_priv->user_limits->db_count);
 	cmdq->db_id = 0;
 
 	return 0;
@@ -313,6 +325,7 @@ static inline u8 ivpu_job_to_jsm_priority(u8 priority)
 
 static void ivpu_cmdq_destroy(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
 {
+	lockdep_assert_held(&file_priv->lock);
 	ivpu_cmdq_unregister(file_priv, cmdq);
 	xa_erase(&file_priv->cmdq_xa, cmdq->id);
 	ivpu_cmdq_free(file_priv, cmdq);
@@ -380,8 +393,11 @@ static void ivpu_cmdq_reset(struct ivpu_file_priv *file_priv)
 	mutex_lock(&file_priv->lock);
 
 	xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq) {
-		xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
-		cmdq->db_id = 0;
+		if (cmdq->db_id) {
+			xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
+			atomic_dec(&file_priv->user_limits->db_count);
+			cmdq->db_id = 0;
+		}
 	}
 
 	mutex_unlock(&file_priv->lock);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] accel/ivpu: Limit number of maximum contexts and doorbells per user
  2026-03-02 20:22 [PATCH v3] accel/ivpu: Limit number of maximum contexts and doorbells per user Maciej Falkowski
@ 2026-03-02 20:45 ` Lizhi Hou
  2026-03-03  2:50 ` Claude review: " Claude Code Review Bot
  2026-03-03  2:50 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Lizhi Hou @ 2026-03-02 20:45 UTC (permalink / raw)
  To: Maciej Falkowski, dri-devel; +Cc: oded.gabbay, jeff.hugo, karol.wachowski


On 3/2/26 12:22, Maciej Falkowski wrote:
> From: Karol Wachowski <karol.wachowski@linux.intel.com>
>
> Implement per-user resource limits to prevent resource exhaustion.
>
> Root users can allocate all available contexts (128) and doorbells
> (255), while non-root users are limited to half of the available
> resources (64 contexts and 127 doorbells respectively).
>
> This prevents scenarios where a single user could monopolize NPU
> resources and starve other users on multi-user systems.
>
> Change doorbell ID and command queue ID allocation errors to debug
> messages as those are user triggered.
>
> Signed-off-by: Karol Wachowski <karol.wachowski@linux.intel.com>
> Signed-off-by: Maciej Falkowski <maciej.falkowski@linux.intel.com>
> ---
> v2 -> v3:
>    - Refactor to use kzalloc_obj*() due to treewide rework.
> v1 -> v2:
>    - Fixed off-by-one error (Lizhi)
> ---
>   drivers/accel/ivpu/ivpu_drv.c | 94 ++++++++++++++++++++++++++++++++---
>   drivers/accel/ivpu/ivpu_drv.h | 26 ++++++++--
>   drivers/accel/ivpu/ivpu_job.c | 36 ++++++++++----
>   3 files changed, 136 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index 5900a40c7a78..dd3a486df5f1 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
> @@ -67,6 +67,73 @@ bool ivpu_force_snoop;
>   module_param_named(force_snoop, ivpu_force_snoop, bool, 0444);
>   MODULE_PARM_DESC(force_snoop, "Force snooping for NPU host memory access");
>   
> +static struct ivpu_user_limits *ivpu_user_limits_alloc(struct ivpu_device *vdev, uid_t uid)
> +{
> +	struct ivpu_user_limits *limits;
> +
> +	limits = kzalloc_obj(*limits);
Reviewed-by: Lizhi Hou <lizhi.hou@amd.com>
> +	if (!limits)
> +		return ERR_PTR(-ENOMEM);
> +
> +	kref_init(&limits->ref);
> +	atomic_set(&limits->db_count, 0);
> +	limits->vdev = vdev;
> +	limits->uid = uid;
> +
> +	/* Allow root user to allocate all contexts */
> +	if (uid == 0) {
> +		limits->max_ctx_count = ivpu_get_context_count(vdev);
> +		limits->max_db_count = ivpu_get_doorbell_count(vdev);
> +	} else {
> +		limits->max_ctx_count = ivpu_get_context_count(vdev) / 2;
> +		limits->max_db_count = ivpu_get_doorbell_count(vdev) / 2;
> +	}
> +
> +	hash_add(vdev->user_limits, &limits->hash_node, uid);
> +
> +	return limits;
> +}
> +
> +static struct ivpu_user_limits *ivpu_user_limits_get(struct ivpu_device *vdev)
> +{
> +	struct ivpu_user_limits *limits;
> +	uid_t uid = current_uid().val;
> +
> +	guard(mutex)(&vdev->user_limits_lock);
> +
> +	hash_for_each_possible(vdev->user_limits, limits, hash_node, uid) {
> +		if (limits->uid == uid) {
> +			if (kref_read(&limits->ref) >= limits->max_ctx_count) {
> +				ivpu_dbg(vdev, IOCTL, "User %u exceeded max ctx count %u\n", uid,
> +					 limits->max_ctx_count);
> +				return ERR_PTR(-EMFILE);
> +			}
> +
> +			kref_get(&limits->ref);
> +			return limits;
> +		}
> +	}
> +
> +	return ivpu_user_limits_alloc(vdev, uid);
> +}
> +
> +static void ivpu_user_limits_release(struct kref *ref)
> +{
> +	struct ivpu_user_limits *limits = container_of(ref, struct ivpu_user_limits, ref);
> +	struct ivpu_device *vdev = limits->vdev;
> +
> +	lockdep_assert_held(&vdev->user_limits_lock);
> +	drm_WARN_ON(&vdev->drm, atomic_read(&limits->db_count));
> +	hash_del(&limits->hash_node);
> +	kfree(limits);
> +}
> +
> +static void ivpu_user_limits_put(struct ivpu_device *vdev, struct ivpu_user_limits *limits)
> +{
> +	guard(mutex)(&vdev->user_limits_lock);
> +	kref_put(&limits->ref, ivpu_user_limits_release);
> +}
> +
>   struct ivpu_file_priv *ivpu_file_priv_get(struct ivpu_file_priv *file_priv)
>   {
>   	struct ivpu_device *vdev = file_priv->vdev;
> @@ -110,6 +177,7 @@ static void file_priv_release(struct kref *ref)
>   	mutex_unlock(&vdev->context_list_lock);
>   	pm_runtime_put_autosuspend(vdev->drm.dev);
>   
> +	ivpu_user_limits_put(vdev, file_priv->user_limits);
>   	mutex_destroy(&file_priv->ms_lock);
>   	mutex_destroy(&file_priv->lock);
>   	kfree(file_priv);
> @@ -169,7 +237,7 @@ static int ivpu_get_param_ioctl(struct drm_device *dev, void *data, struct drm_f
>   		args->value = ivpu_hw_dpu_max_freq_get(vdev);
>   		break;
>   	case DRM_IVPU_PARAM_NUM_CONTEXTS:
> -		args->value = ivpu_get_context_count(vdev);
> +		args->value = file_priv->user_limits->max_ctx_count;
>   		break;
>   	case DRM_IVPU_PARAM_CONTEXT_BASE_ADDRESS:
>   		args->value = vdev->hw->ranges.user.start;
> @@ -231,22 +299,30 @@ static int ivpu_open(struct drm_device *dev, struct drm_file *file)
>   {
>   	struct ivpu_device *vdev = to_ivpu_device(dev);
>   	struct ivpu_file_priv *file_priv;
> +	struct ivpu_user_limits *limits;
>   	u32 ctx_id;
>   	int idx, ret;
>   
>   	if (!drm_dev_enter(dev, &idx))
>   		return -ENODEV;
>   
> +	limits = ivpu_user_limits_get(vdev);
> +	if (IS_ERR(limits)) {
> +		ret = PTR_ERR(limits);
> +		goto err_dev_exit;
> +	}
> +
>   	file_priv = kzalloc_obj(*file_priv);
>   	if (!file_priv) {
>   		ret = -ENOMEM;
> -		goto err_dev_exit;
> +		goto err_user_limits_put;
>   	}
>   
>   	INIT_LIST_HEAD(&file_priv->ms_instance_list);
>   
>   	file_priv->vdev = vdev;
>   	file_priv->bound = true;
> +	file_priv->user_limits = limits;
>   	kref_init(&file_priv->ref);
>   	mutex_init(&file_priv->lock);
>   	mutex_init(&file_priv->ms_lock);
> @@ -284,6 +360,8 @@ static int ivpu_open(struct drm_device *dev, struct drm_file *file)
>   	mutex_destroy(&file_priv->ms_lock);
>   	mutex_destroy(&file_priv->lock);
>   	kfree(file_priv);
> +err_user_limits_put:
> +	ivpu_user_limits_put(vdev, limits);
>   err_dev_exit:
>   	drm_dev_exit(idx);
>   	return ret;
> @@ -343,8 +421,7 @@ static int ivpu_wait_for_ready(struct ivpu_device *vdev)
>   	ivpu_ipc_consumer_del(vdev, &cons);
>   
>   	if (!ret && ipc_hdr.data_addr != IVPU_IPC_BOOT_MSG_DATA_ADDR) {
> -		ivpu_err(vdev, "Invalid NPU ready message: 0x%x\n",
> -			 ipc_hdr.data_addr);
> +		ivpu_err(vdev, "Invalid NPU ready message: 0x%x\n", ipc_hdr.data_addr);
>   		return -EIO;
>   	}
>   
> @@ -453,7 +530,7 @@ int ivpu_shutdown(struct ivpu_device *vdev)
>   }
>   
>   static const struct file_operations ivpu_fops = {
> -	.owner		= THIS_MODULE,
> +	.owner = THIS_MODULE,
>   	DRM_ACCEL_FOPS,
>   #ifdef CONFIG_PROC_FS
>   	.show_fdinfo = drm_show_fdinfo,
> @@ -592,6 +669,7 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
>   	xa_init_flags(&vdev->submitted_jobs_xa, XA_FLAGS_ALLOC1);
>   	xa_init_flags(&vdev->db_xa, XA_FLAGS_ALLOC1);
>   	INIT_LIST_HEAD(&vdev->bo_list);
> +	hash_init(vdev->user_limits);
>   
>   	vdev->db_limit.min = IVPU_MIN_DB;
>   	vdev->db_limit.max = IVPU_MAX_DB;
> @@ -600,6 +678,10 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
>   	if (ret)
>   		goto err_xa_destroy;
>   
> +	ret = drmm_mutex_init(&vdev->drm, &vdev->user_limits_lock);
> +	if (ret)
> +		goto err_xa_destroy;
> +
>   	ret = drmm_mutex_init(&vdev->drm, &vdev->submitted_jobs_lock);
>   	if (ret)
>   		goto err_xa_destroy;
> @@ -717,7 +799,7 @@ static struct pci_device_id ivpu_pci_ids[] = {
>   	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_PTL_P) },
>   	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_WCL) },
>   	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_NVL) },
> -	{ }
> +	{}
>   };
>   MODULE_DEVICE_TABLE(pci, ivpu_pci_ids);
>   
> diff --git a/drivers/accel/ivpu/ivpu_drv.h b/drivers/accel/ivpu/ivpu_drv.h
> index 5b34b6f50e69..6378e23e0c97 100644
> --- a/drivers/accel/ivpu/ivpu_drv.h
> +++ b/drivers/accel/ivpu/ivpu_drv.h
> @@ -12,6 +12,7 @@
>   #include <drm/drm_mm.h>
>   #include <drm/drm_print.h>
>   
> +#include <linux/hashtable.h>
>   #include <linux/pci.h>
>   #include <linux/xarray.h>
>   #include <uapi/drm/ivpu_accel.h>
> @@ -43,7 +44,7 @@
>   /* SSID 1 is used by the VPU to represent reserved context */
>   #define IVPU_RESERVED_CONTEXT_MMU_SSID 1
>   #define IVPU_USER_CONTEXT_MIN_SSID     2
> -#define IVPU_USER_CONTEXT_MAX_SSID     (IVPU_USER_CONTEXT_MIN_SSID + 63)
> +#define IVPU_USER_CONTEXT_MAX_SSID     (IVPU_USER_CONTEXT_MIN_SSID + 128)
>   
>   #define IVPU_MIN_DB 1
>   #define IVPU_MAX_DB 255
> @@ -51,9 +52,6 @@
>   #define IVPU_JOB_ID_JOB_MASK		GENMASK(7, 0)
>   #define IVPU_JOB_ID_CONTEXT_MASK	GENMASK(31, 8)
>   
> -#define IVPU_NUM_PRIORITIES    4
> -#define IVPU_NUM_CMDQS_PER_CTX (IVPU_NUM_PRIORITIES)
> -
>   #define IVPU_CMDQ_MIN_ID 1
>   #define IVPU_CMDQ_MAX_ID 255
>   
> @@ -123,6 +121,16 @@ struct ivpu_fw_info;
>   struct ivpu_ipc_info;
>   struct ivpu_pm_info;
>   
> +struct ivpu_user_limits {
> +	struct hlist_node hash_node;
> +	struct ivpu_device *vdev;
> +	struct kref ref;
> +	u32 max_ctx_count;
> +	u32 max_db_count;
> +	u32 uid;
> +	atomic_t db_count;
> +};
> +
>   struct ivpu_device {
>   	struct drm_device drm;
>   	void __iomem *regb;
> @@ -142,6 +150,8 @@ struct ivpu_device {
>   	struct mutex context_list_lock; /* Protects user context addition/removal */
>   	struct xarray context_xa;
>   	struct xa_limit context_xa_limit;
> +	DECLARE_HASHTABLE(user_limits, 8);
> +	struct mutex user_limits_lock; /* Protects user_limits */
>   
>   	struct xarray db_xa;
>   	struct xa_limit db_limit;
> @@ -189,6 +199,7 @@ struct ivpu_file_priv {
>   	struct list_head ms_instance_list;
>   	struct ivpu_bo *ms_info_bo;
>   	struct xa_limit job_limit;
> +	struct ivpu_user_limits *user_limits;
>   	u32 job_id_next;
>   	struct xa_limit cmdq_limit;
>   	u32 cmdq_id_next;
> @@ -286,6 +297,13 @@ static inline u32 ivpu_get_context_count(struct ivpu_device *vdev)
>   	return (ctx_limit.max - ctx_limit.min + 1);
>   }
>   
> +static inline u32 ivpu_get_doorbell_count(struct ivpu_device *vdev)
> +{
> +	struct xa_limit db_limit = vdev->db_limit;
> +
> +	return (db_limit.max - db_limit.min + 1);
> +}
> +
>   static inline u32 ivpu_get_platform(struct ivpu_device *vdev)
>   {
>   	WARN_ON_ONCE(vdev->platform == IVPU_PLATFORM_INVALID);
> diff --git a/drivers/accel/ivpu/ivpu_job.c b/drivers/accel/ivpu/ivpu_job.c
> index fe02b7bd465b..f0154dfa6ddc 100644
> --- a/drivers/accel/ivpu/ivpu_job.c
> +++ b/drivers/accel/ivpu/ivpu_job.c
> @@ -173,7 +173,7 @@ static struct ivpu_cmdq *ivpu_cmdq_create(struct ivpu_file_priv *file_priv, u8 p
>   	ret = xa_alloc_cyclic(&file_priv->cmdq_xa, &cmdq->id, cmdq, file_priv->cmdq_limit,
>   			      &file_priv->cmdq_id_next, GFP_KERNEL);
>   	if (ret < 0) {
> -		ivpu_err(vdev, "Failed to allocate command queue ID: %d\n", ret);
> +		ivpu_dbg(vdev, IOCTL, "Failed to allocate command queue ID: %d\n", ret);
>   		goto err_free_cmdq;
>   	}
>   
> @@ -215,14 +215,22 @@ static int ivpu_hws_cmdq_init(struct ivpu_file_priv *file_priv, struct ivpu_cmdq
>   
>   static int ivpu_register_db(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
>   {
> +	struct ivpu_user_limits *limits = file_priv->user_limits;
>   	struct ivpu_device *vdev = file_priv->vdev;
>   	int ret;
>   
> +	if (atomic_inc_return(&limits->db_count) > limits->max_db_count) {
> +		ivpu_dbg(vdev, IOCTL, "Maximum number of %u doorbells for uid %u reached\n",
> +			 limits->max_db_count, limits->uid);
> +		ret = -EBUSY;
> +		goto err_dec_db_count;
> +	}
> +
>   	ret = xa_alloc_cyclic(&vdev->db_xa, &cmdq->db_id, NULL, vdev->db_limit, &vdev->db_next,
>   			      GFP_KERNEL);
>   	if (ret < 0) {
> -		ivpu_err(vdev, "Failed to allocate doorbell ID: %d\n", ret);
> -		return ret;
> +		ivpu_dbg(vdev, IOCTL, "Failed to allocate doorbell ID: %d\n", ret);
> +		goto err_dec_db_count;
>   	}
>   
>   	if (vdev->fw->sched_mode == VPU_SCHEDULING_MODE_HW)
> @@ -231,15 +239,18 @@ static int ivpu_register_db(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *
>   	else
>   		ret = ivpu_jsm_register_db(vdev, file_priv->ctx.id, cmdq->db_id,
>   					   cmdq->mem->vpu_addr, ivpu_bo_size(cmdq->mem));
> -
> -	if (!ret) {
> -		ivpu_dbg(vdev, JOB, "DB %d registered to cmdq %d ctx %d priority %d\n",
> -			 cmdq->db_id, cmdq->id, file_priv->ctx.id, cmdq->priority);
> -	} else {
> +	if (ret) {
>   		xa_erase(&vdev->db_xa, cmdq->db_id);
>   		cmdq->db_id = 0;
> +		goto err_dec_db_count;
>   	}
>   
> +	ivpu_dbg(vdev, JOB, "DB %d registered to cmdq %d ctx %d priority %d\n",
> +		 cmdq->db_id, cmdq->id, file_priv->ctx.id, cmdq->priority);
> +	return 0;
> +
> +err_dec_db_count:
> +	atomic_dec(&limits->db_count);
>   	return ret;
>   }
>   
> @@ -298,6 +309,7 @@ static int ivpu_cmdq_unregister(struct ivpu_file_priv *file_priv, struct ivpu_cm
>   	}
>   
>   	xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
> +	atomic_dec(&file_priv->user_limits->db_count);
>   	cmdq->db_id = 0;
>   
>   	return 0;
> @@ -313,6 +325,7 @@ static inline u8 ivpu_job_to_jsm_priority(u8 priority)
>   
>   static void ivpu_cmdq_destroy(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
>   {
> +	lockdep_assert_held(&file_priv->lock);
>   	ivpu_cmdq_unregister(file_priv, cmdq);
>   	xa_erase(&file_priv->cmdq_xa, cmdq->id);
>   	ivpu_cmdq_free(file_priv, cmdq);
> @@ -380,8 +393,11 @@ static void ivpu_cmdq_reset(struct ivpu_file_priv *file_priv)
>   	mutex_lock(&file_priv->lock);
>   
>   	xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq) {
> -		xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
> -		cmdq->db_id = 0;
> +		if (cmdq->db_id) {
> +			xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
> +			atomic_dec(&file_priv->user_limits->db_count);
> +			cmdq->db_id = 0;
> +		}
>   	}
>   
>   	mutex_unlock(&file_priv->lock);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Claude review: accel/ivpu: Limit number of maximum contexts and doorbells per user
  2026-03-02 20:22 [PATCH v3] accel/ivpu: Limit number of maximum contexts and doorbells per user Maciej Falkowski
  2026-03-02 20:45 ` Lizhi Hou
@ 2026-03-03  2:50 ` Claude Code Review Bot
  2026-03-03  2:50 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  2:50 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: accel/ivpu: Limit number of maximum contexts and doorbells per user
Author: Maciej Falkowski <maciej.falkowski@linux.intel.com>
Patches: 2
Reviewed: 2026-03-03T12:50:56.418884

---

This is a single patch (v3) that adds per-user resource limits for contexts and doorbells in the Intel IVPU (NPU) accelerator driver. The goal is to prevent a single user from monopolizing hardware resources on multi-user systems, with root getting full access and non-root users limited to half.

The overall design is reasonable: a hash table keyed by UID tracks per-user limits, kref-counted to the number of open file descriptors. The doorbell limit uses an atomic increment-then-check pattern that is correct. However, there are several issues ranging from a likely off-by-one to a security anti-pattern and mixed-in unrelated formatting changes.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Claude review: accel/ivpu: Limit number of maximum contexts and doorbells per user
  2026-03-02 20:22 [PATCH v3] accel/ivpu: Limit number of maximum contexts and doorbells per user Maciej Falkowski
  2026-03-02 20:45 ` Lizhi Hou
  2026-03-03  2:50 ` Claude review: " Claude Code Review Bot
@ 2026-03-03  2:50 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  2:50 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Off-by-one in IVPU_USER_CONTEXT_MAX_SSID (medium)**

The commit message states "Root users can allocate all available contexts (128)" but the new define yields 129 contexts:

```c
#define IVPU_USER_CONTEXT_MAX_SSID     (IVPU_USER_CONTEXT_MIN_SSID + 128)
```

With `IVPU_USER_CONTEXT_MIN_SSID = 2`, MAX_SSID becomes 130. Then `ivpu_get_context_count()` returns `130 - 2 + 1 = 129`. The original code had `+ 63` which gave exactly 64 contexts. To get exactly 128, this should be `+ 127`. The v1->v2 changelog says "Fixed off-by-one error" but it appears the fix was applied in the wrong direction, or the commit message is wrong about the intended count.

**UID==0 check instead of capability check (medium)**

```c
if (uid == 0) {
    limits->max_ctx_count = ivpu_get_context_count(vdev);
    limits->max_db_count = ivpu_get_doorbell_count(vdev);
}
```

Direct UID comparisons are discouraged in the kernel. This bypasses the `kuid_t` type-safety system and doesn't integrate with capability-based access control. The standard kernel pattern would be to use `capable(CAP_SYS_ADMIN)` (or a more specific capability) at the point where limits are being set, or at minimum `uid_eq(current_uid(), GLOBAL_ROOT_UID)`. Using `capable()` would also correctly handle containerized workloads.

Additionally, the UID is captured at allocation time and cached in the `ivpu_user_limits` struct. If `capable()` were used instead, this check would need to happen at lookup time rather than allocation time, since different processes with the same UID could have different capabilities.

**Using kref_read() for control-flow decisions (minor)**

```c
if (kref_read(&limits->ref) >= limits->max_ctx_count) {
    ivpu_dbg(vdev, IOCTL, "User %u exceeded max ctx count %u\n", uid,
             limits->max_ctx_count);
    return ERR_PTR(-EMFILE);
}
```

`kref_read()` is documented as being primarily for debugging purposes, not for making control-flow decisions. While the mutex protection makes this functionally safe, a dedicated `u32 ctx_count` counter (mirroring the `atomic_t db_count` pattern) would be more idiomatic and consistent within the patch itself. It's also somewhat confusing that context counting uses kref while doorbell counting uses an atomic - the two resources are tracked with different mechanisms for no obvious reason.

**UAPI behavior change (medium)**

```c
case DRM_IVPU_PARAM_NUM_CONTEXTS:
-   args->value = ivpu_get_context_count(vdev);
+   args->value = file_priv->user_limits->max_ctx_count;
```

This changes what `DRM_IVPU_PARAM_NUM_CONTEXTS` reports from the total device context count to the per-user maximum. Existing userspace that queries this parameter will now see a different (potentially halved) value. This could break applications that use this value for sizing or configuration. If this is intentional, it should be explicitly noted in the commit message as a UAPI behavior change. Consider whether a separate parameter should be added instead.

**Unrelated formatting changes mixed in (minor)**

The patch includes several unrelated whitespace changes:
- `ivpu_wait_for_ready`: line-wrapping change for `ivpu_err`
- `ivpu_fops`: `.owner\t\t= THIS_MODULE` changed to `.owner = THIS_MODULE`
- `ivpu_pci_ids`: `{ }` changed to `{}`

These should be in a separate cleanup patch to keep the functional change reviewable.

**No doorbell limit reported to userspace (minor)**

While the context limit is now exposed via `DRM_IVPU_PARAM_NUM_CONTEXTS`, there is no corresponding mechanism for userspace to query its doorbell limit. If userspace needs to know how many doorbells it can allocate, there's no way to discover this.

**lockdep_assert_held added to ivpu_cmdq_destroy (good)**

```c
static void ivpu_cmdq_destroy(struct ivpu_file_priv *file_priv, struct ivpu_cmdq *cmdq)
{
+   lockdep_assert_held(&file_priv->lock);
```

Good addition - all callers already hold `file_priv->lock`, and this makes the locking requirement explicit.

**ivpu_cmdq_reset doorbell check (good)**

```c
xa_for_each(&file_priv->cmdq_xa, cmdq_id, cmdq) {
-   xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
-   cmdq->db_id = 0;
+   if (cmdq->db_id) {
+       xa_erase(&file_priv->vdev->db_xa, cmdq->db_id);
+       atomic_dec(&file_priv->user_limits->db_count);
+       cmdq->db_id = 0;
+   }
}
```

The added `if (cmdq->db_id)` guard is a correct improvement - the original code would have called `xa_erase` with id 0 for unregistered command queues, which is harmless but wasteful. With the new atomic decrement, the guard is now necessary to avoid decrementing below zero.

**Removal of unused macros (fine)**

`IVPU_NUM_PRIORITIES` and `IVPU_NUM_CMDQS_PER_CTX` are defined but never referenced anywhere in the codebase, so removing them is correct, though it would be cleaner in a separate patch.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-03-03  2:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-02 20:22 [PATCH v3] accel/ivpu: Limit number of maximum contexts and doorbells per user Maciej Falkowski
2026-03-02 20:45 ` Lizhi Hou
2026-03-03  2:50 ` Claude review: " Claude Code Review Bot
2026-03-03  2:50 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox