On Wed Nov 19 15:25:47 2025 +0900, Jackson Lee wrote:
> When multi instances are created/destroyed, many interrupts happens
> and structures for decoder are removed.
> "struct vpu_instance" this structure is shared for all flow in the decoder,
> so if the structure is not protected by lock, Null dereference
> could happens sometimes.
> IRQ Handler was spilt to two phases and Lock was added as well.
>
> Fixes: 9707a6254a8a ("media: chips-media: wave5: Add the v4l2 layer")
> Cc: [email protected]
> Signed-off-by: Jackson Lee <[email protected]>
> Signed-off-by: Nas Chung <[email protected]>
> Tested-by: Brandon Brnich <[email protected]>
> Signed-off-by: Nicolas Dufresne <[email protected]>
> Signed-off-by: Hans Verkuil <[email protected]>
Patch committed.
Thanks,
Hans Verkuil
.../platform/chips-media/wave5/wave5-helper.c | 28 ++++++-
.../platform/chips-media/wave5/wave5-helper.h | 1 +
.../platform/chips-media/wave5/wave5-vpu-dec.c | 5 ++
.../platform/chips-media/wave5/wave5-vpu-enc.c | 5 ++
.../media/platform/chips-media/wave5/wave5-vpu.c | 97 +++++++++++++++++++---
.../platform/chips-media/wave5/wave5-vpuapi.h | 6 ++
6 files changed, 131 insertions(+), 11 deletions(-)
---
diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c
b/drivers/media/platform/chips-media/wave5/wave5-helper.c
index f03ad9c0de22..53a0ac068c2e 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
@@ -27,6 +27,11 @@ const char *state_to_str(enum vpu_instance_state state)
}
}
+int wave5_kfifo_alloc(struct vpu_instance *inst)
+{
+ return kfifo_alloc(&inst->irq_status, 16 * sizeof(int), GFP_KERNEL);
+}
+
void wave5_cleanup_instance(struct vpu_instance *inst, struct file *filp)
{
int i;
@@ -49,7 +54,7 @@ void wave5_cleanup_instance(struct vpu_instance *inst, struct
file *filp)
v4l2_fh_del(&inst->v4l2_fh, filp);
v4l2_fh_exit(&inst->v4l2_fh);
}
- list_del_init(&inst->list);
+ kfifo_free(&inst->irq_status);
ida_free(&inst->dev->inst_ida, inst->id);
kfree(inst->codec_info);
kfree(inst);
@@ -61,8 +66,29 @@ int wave5_vpu_release_device(struct file *filp,
{
struct vpu_instance *inst = file_to_vpu_inst(filp);
int ret = 0;
+ unsigned long flags;
v4l2_m2m_ctx_release(inst->v4l2_fh.m2m_ctx);
+ /*
+ * To prevent Null reference exception, the existing irq handler were
+ * separated to two modules.
+ * One is to queue interrupt reason into the irq handler,
+ * the other is irq_thread to call the wave5_vpu_dec_finish_decode
+ * to get decoded frame.
+ * The list of instances should be protected between all flow of the
+ * decoding process, but to protect the list in the irq_handler, spin
lock
+ * should be used, and mutex should be used in the irq_thread because
spin lock
+ * is not able to be used because mutex is already being used
+ * in the wave5_vpu_dec_finish_decode.
+ * So the spin lock and mutex were used to protect the list in the
release function.
+ */
+ ret = mutex_lock_interruptible(&inst->dev->irq_lock);
+ if (ret)
+ return ret;
+ spin_lock_irqsave(&inst->dev->irq_spinlock, flags);
+ list_del_init(&inst->list);
+ spin_unlock_irqrestore(&inst->dev->irq_spinlock, flags);
+ mutex_unlock(&inst->dev->irq_lock);
if (inst->state != VPU_INST_STATE_NONE) {
u32 fail_res;
diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.h
b/drivers/media/platform/chips-media/wave5/wave5-helper.h
index 976a402e426f..d61fdbda359d 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-helper.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-helper.h
@@ -33,4 +33,5 @@ void wave5_update_pix_fmt(struct v4l2_pix_format_mplane
*pix_mp,
unsigned int width,
unsigned int height,
const struct v4l2_frmsize_stepwise *frmsize);
+int wave5_kfifo_alloc(struct vpu_instance *inst);
#endif
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index 528387c6ecf6..b10396fa2379 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -1808,6 +1808,11 @@ static int wave5_vpu_open_dec(struct file *filp)
inst->xfer_func = V4L2_XFER_FUNC_DEFAULT;
init_completion(&inst->irq_done);
+ ret = wave5_kfifo_alloc(inst);
+ if (ret) {
+ dev_err(inst->dev->dev, "failed to allocate fifo\n");
+ goto cleanup_inst;
+ }
inst->id = ida_alloc(&inst->dev->inst_ida, GFP_KERNEL);
if (inst->id < 0) {
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
index 3be79dda3a2e..e69bef5d1b52 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
@@ -1757,6 +1757,11 @@ static int wave5_vpu_open_enc(struct file *filp)
inst->frame_rate = 30;
init_completion(&inst->irq_done);
+ ret = wave5_kfifo_alloc(inst);
+ if (ret) {
+ dev_err(inst->dev->dev, "failed to allocate fifo\n");
+ goto cleanup_inst;
+ }
inst->id = ida_alloc(&inst->dev->inst_ida, GFP_KERNEL);
if (inst->id < 0) {
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
index 0026f5840362..3216b4997644 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
@@ -51,8 +51,11 @@ static void wave5_vpu_handle_irq(void *dev_id)
u32 seq_done;
u32 cmd_done;
u32 irq_reason;
- struct vpu_instance *inst;
+ u32 irq_subreason;
+ struct vpu_instance *inst, *tmp;
struct vpu_device *dev = dev_id;
+ int val;
+ unsigned long flags;
irq_reason = wave5_vdi_read_register(dev, W5_VPU_VINT_REASON);
seq_done = wave5_vdi_read_register(dev, W5_RET_SEQ_DONE_INSTANCE_INFO);
@@ -60,7 +63,8 @@ static void wave5_vpu_handle_irq(void *dev_id)
wave5_vdi_write_register(dev, W5_VPU_VINT_REASON_CLR, irq_reason);
wave5_vdi_write_register(dev, W5_VPU_VINT_CLEAR, 0x1);
- list_for_each_entry(inst, &dev->instances, list) {
+ spin_lock_irqsave(&dev->irq_spinlock, flags);
+ list_for_each_entry_safe(inst, tmp, &dev->instances, list) {
if (irq_reason & BIT(INT_WAVE5_INIT_SEQ) ||
irq_reason & BIT(INT_WAVE5_ENC_SET_PARAM)) {
@@ -82,22 +86,54 @@ static void wave5_vpu_handle_irq(void *dev_id)
irq_reason & BIT(INT_WAVE5_ENC_PIC)) {
if (cmd_done & BIT(inst->id)) {
cmd_done &= ~BIT(inst->id);
- wave5_vdi_write_register(dev,
W5_RET_QUEUE_CMD_DONE_INST,
- cmd_done);
- inst->ops->finish_process(inst);
+ if (dev->irq >= 0) {
+ irq_subreason =
+ wave5_vdi_read_register(dev,
W5_VPU_VINT_REASON);
+ if (!(irq_subreason &
BIT(INT_WAVE5_DEC_PIC)))
+ wave5_vdi_write_register(dev,
+
W5_RET_QUEUE_CMD_DONE_INST,
+
cmd_done);
+ }
+ val = BIT(INT_WAVE5_DEC_PIC);
+ kfifo_in(&inst->irq_status, &val, sizeof(int));
}
}
+ }
+ spin_unlock_irqrestore(&dev->irq_spinlock, flags);
+
+ if (dev->irq < 0)
+ up(&dev->irq_sem);
+}
+
+static irqreturn_t wave5_vpu_irq(int irq, void *dev_id)
+{
+ struct vpu_device *dev = dev_id;
- wave5_vpu_clear_interrupt(inst, irq_reason);
+ if (wave5_vdi_read_register(dev, W5_VPU_VPU_INT_STS)) {
+ wave5_vpu_handle_irq(dev);
+ return IRQ_WAKE_THREAD;
}
+
+ return IRQ_HANDLED;
}
static irqreturn_t wave5_vpu_irq_thread(int irq, void *dev_id)
{
struct vpu_device *dev = dev_id;
+ struct vpu_instance *inst, *tmp;
+ int irq_status, ret;
- if (wave5_vdi_read_register(dev, W5_VPU_VPU_INT_STS))
- wave5_vpu_handle_irq(dev);
+ mutex_lock(&dev->irq_lock);
+ list_for_each_entry_safe(inst, tmp, &dev->instances, list) {
+ while (kfifo_len(&inst->irq_status)) {
+ ret = kfifo_out(&inst->irq_status, &irq_status,
sizeof(int));
+ if (!ret)
+ break;
+
+ inst->ops->finish_process(inst);
+ }
+ }
+ mutex_unlock(&dev->irq_lock);
return IRQ_HANDLED;
}
@@ -121,6 +157,35 @@ static enum hrtimer_restart
wave5_vpu_timer_callback(struct hrtimer *timer)
return HRTIMER_RESTART;
}
+static int irq_thread(void *data)
+{
+ struct vpu_device *dev = (struct vpu_device *)data;
+ struct vpu_instance *inst, *tmp;
+ int irq_status, ret;
+
+ while (!kthread_should_stop()) {
+ if (down_interruptible(&dev->irq_sem))
+ continue;
+
+ if (kthread_should_stop())
+ break;
+
+ mutex_lock(&dev->irq_lock);
+ list_for_each_entry_safe(inst, tmp, &dev->instances, list) {
+ while (kfifo_len(&inst->irq_status)) {
+ ret = kfifo_out(&inst->irq_status, &irq_status,
sizeof(int));
+ if (!ret)
+ break;
+
+ inst->ops->finish_process(inst);
+ }
+ }
+ mutex_unlock(&dev->irq_lock);
+ }
+
+ return 0;
+}
+
static int wave5_vpu_load_firmware(struct device *dev, const char *fw_name,
u32 *revision)
{
@@ -224,6 +289,8 @@ static int wave5_vpu_probe(struct platform_device *pdev)
mutex_init(&dev->dev_lock);
mutex_init(&dev->hw_lock);
+ mutex_init(&dev->irq_lock);
+ spin_lock_init(&dev->irq_spinlock);
dev_set_drvdata(&pdev->dev, dev);
dev->dev = &pdev->dev;
@@ -266,9 +333,13 @@ static int wave5_vpu_probe(struct platform_device *pdev)
}
dev->product = wave5_vpu_get_product_id(dev);
+ INIT_LIST_HEAD(&dev->instances);
+
dev->irq = platform_get_irq(pdev, 0);
if (dev->irq < 0) {
dev_err(&pdev->dev, "failed to get irq resource, falling back
to polling\n");
+ sema_init(&dev->irq_sem, 1);
+ dev->irq_thread = kthread_run(irq_thread, dev, "irq thread");
hrtimer_setup(&dev->hrtimer, &wave5_vpu_timer_callback,
CLOCK_MONOTONIC,
HRTIMER_MODE_REL_PINNED);
dev->worker = kthread_run_worker(0, "vpu_irq_thread");
@@ -280,7 +351,7 @@ static int wave5_vpu_probe(struct platform_device *pdev)
dev->vpu_poll_interval = vpu_poll_interval;
kthread_init_work(&dev->work, wave5_vpu_irq_work_fn);
} else {
- ret = devm_request_threaded_irq(&pdev->dev, dev->irq, NULL,
+ ret = devm_request_threaded_irq(&pdev->dev, dev->irq,
wave5_vpu_irq,
wave5_vpu_irq_thread,
IRQF_ONESHOT, "vpu_irq", dev);
if (ret) {
dev_err(&pdev->dev, "Register interrupt handler, fail:
%d\n", ret);
@@ -288,7 +359,6 @@ static int wave5_vpu_probe(struct platform_device *pdev)
}
}
- INIT_LIST_HEAD(&dev->instances);
ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
if (ret) {
dev_err(&pdev->dev, "v4l2_device_register, fail: %d\n", ret);
@@ -356,6 +426,12 @@ static void wave5_vpu_remove(struct platform_device *pdev)
v4l2_device_unregister(&dev->v4l2_dev);
if (dev->irq < 0) {
+ if (dev->irq_thread) {
+ kthread_stop(dev->irq_thread);
+ up(&dev->irq_sem);
+ dev->irq_thread = NULL;
+ }
+
hrtimer_cancel(&dev->hrtimer);
kthread_cancel_work_sync(&dev->work);
kthread_destroy_worker(dev->worker);
@@ -366,6 +442,7 @@ static void wave5_vpu_remove(struct platform_device *pdev)
mutex_destroy(&dev->dev_lock);
mutex_destroy(&dev->hw_lock);
+ mutex_destroy(&dev->irq_lock);
reset_control_assert(dev->resets);
clk_bulk_disable_unprepare(dev->num_clks, dev->clks);
wave5_vdi_release(&pdev->dev);
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
index 45615c15beca..bc101397204d 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
@@ -8,6 +8,7 @@
#ifndef VPUAPI_H_INCLUDED
#define VPUAPI_H_INCLUDED
+#include <linux/kfifo.h>
#include <linux/idr.h>
#include <linux/genalloc.h>
#include <media/v4l2-device.h>
@@ -747,6 +748,7 @@ struct vpu_device {
struct video_device *video_dev_enc;
struct mutex dev_lock; /* lock for the src, dst v4l2 queues */
struct mutex hw_lock; /* lock hw configurations */
+ struct mutex irq_lock;
int irq;
enum product_id product;
struct vpu_attr attr;
@@ -764,7 +766,10 @@ struct vpu_device {
struct kthread_worker *worker;
int vpu_poll_interval;
int num_clks;
+ struct task_struct *irq_thread;
+ struct semaphore irq_sem; /* signal to irq_thread when interrupt
happens*/
struct reset_control *resets;
+ spinlock_t irq_spinlock; /* protect instances list */
};
struct vpu_instance;
@@ -788,6 +793,7 @@ struct vpu_instance {
enum v4l2_ycbcr_encoding ycbcr_enc;
enum v4l2_quantization quantization;
+ struct kfifo irq_status;
enum vpu_instance_state state;
enum vpu_instance_type type;
const struct vpu_instance_ops *ops;
_______________________________________________
linuxtv-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
