media: mediatek: vcodec: Use spinlock for context list protection lock

[ Upstream commit a5844227e0f030d2af2d85d4aed10c5eca6ca176 ]

Previously a mutex was added to protect the encoder and decoder context
lists from unexpected changes originating from the SCP IP block, causing
the context pointer to go invalid, resulting in a NULL pointer
dereference in the IPI handler.

Turns out on the MT8173, the VPU IPI handler is called from hard IRQ
context. This causes a big warning from the scheduler. This was first
reported downstream on the ChromeOS kernels, but is also reproducible
on mainline using Fluster with the FFmpeg v4l2m2m decoders. Even though
the actual capture format is not supported, the affected code paths
are triggered.

Since this lock just protects the context list and operations on it are
very fast, it should be OK to switch to a spinlock.

Fixes: 6467cda18c9f ("media: mediatek: vcodec: adding lock to protect decoder context list")
Fixes: afaaf3a0f647 ("media: mediatek: vcodec: adding lock to protect encoder context list")
Cc: Yunfei Dong <yunfei.dong@mediatek.com>
Cc: stable@vger.kernel.org
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Fei Shao <fshao@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
[ adapted file_to_dec_ctx() and file_to_enc_ctx() helper calls to equivalent fh_to_dec_ctx(file->private_data) and fh_to_enc_ctx(file->private_data) pattern ]
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
index 1c94316f2d7d95ca87992e8bf59cf0222a1e77be..3632037f78f5055264adec971935e9588f6dac2b 100644 (file)
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
@@ -47,30 +47,32 @@ static void mtk_vcodec_vpu_reset_dec_handler(void *priv)
 {
        struct mtk_vcodec_dec_dev *dev = priv;
        struct mtk_vcodec_dec_ctx *ctx;
+       unsigned long flags;
 
        dev_err(&dev->plat_dev->dev, "Watchdog timeout!!");
 
-       mutex_lock(&dev->dev_ctx_lock);
+       spin_lock_irqsave(&dev->dev_ctx_lock, flags);
        list_for_each_entry(ctx, &dev->ctx_list, list) {
                ctx->state = MTK_STATE_ABORT;
                mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->id);
        }
-       mutex_unlock(&dev->dev_ctx_lock);
+       spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
 }
 
 static void mtk_vcodec_vpu_reset_enc_handler(void *priv)
 {
        struct mtk_vcodec_enc_dev *dev = priv;
        struct mtk_vcodec_enc_ctx *ctx;
+       unsigned long flags;
 
        dev_err(&dev->plat_dev->dev, "Watchdog timeout!!");
 
-       mutex_lock(&dev->dev_ctx_lock);
+       spin_lock_irqsave(&dev->dev_ctx_lock, flags);
        list_for_each_entry(ctx, &dev->ctx_list, list) {
                ctx->state = MTK_STATE_ABORT;
                mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->id);
        }
-       mutex_unlock(&dev->dev_ctx_lock);
+       spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
 }
 
 static const struct mtk_vcodec_fw_ops mtk_vcodec_vpu_msg = {

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
index 2073781ccadb156116b1cbe86c49b3e06b7a93f3..fc920c4d6181afcf554a61b076a220e3faba3406 100644 (file)
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
@@ -198,6 +198,7 @@ static int fops_vcodec_open(struct file *file)
        struct mtk_vcodec_dec_ctx *ctx = NULL;
        int ret = 0, i, hw_count;
        struct vb2_queue *src_vq;
+       unsigned long flags;
 
        ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
        if (!ctx)
@@ -268,9 +269,9 @@ static int fops_vcodec_open(struct file *file)
 
        ctx->dev->vdec_pdata->init_vdec_params(ctx);
 
-       mutex_lock(&dev->dev_ctx_lock);
+       spin_lock_irqsave(&dev->dev_ctx_lock, flags);
        list_add(&ctx->list, &dev->ctx_list);
-       mutex_unlock(&dev->dev_ctx_lock);
+       spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
        mtk_vcodec_dbgfs_create(ctx);
 
        mutex_unlock(&dev->dev_mutex);
@@ -295,6 +296,7 @@ static int fops_vcodec_release(struct file *file)
 {
        struct mtk_vcodec_dec_dev *dev = video_drvdata(file);
        struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(file->private_data);
+       unsigned long flags;
 
        mtk_v4l2_vdec_dbg(0, ctx, "[%d] decoder", ctx->id);
        mutex_lock(&dev->dev_mutex);
@@ -313,9 +315,9 @@ static int fops_vcodec_release(struct file *file)
        v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
 
        mtk_vcodec_dbgfs_remove(dev, ctx->id);
-       mutex_lock(&dev->dev_ctx_lock);
+       spin_lock_irqsave(&dev->dev_ctx_lock, flags);
        list_del_init(&ctx->list);
-       mutex_unlock(&dev->dev_ctx_lock);
+       spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
        kfree(ctx);
        mutex_unlock(&dev->dev_mutex);
        return 0;
@@ -408,7 +410,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
        for (i = 0; i < MTK_VDEC_HW_MAX; i++)
                mutex_init(&dev->dec_mutex[i]);
        mutex_init(&dev->dev_mutex);
-       mutex_init(&dev->dev_ctx_lock);
+       spin_lock_init(&dev->dev_ctx_lock);
        spin_lock_init(&dev->irqlock);
 
        snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
index ac568ed14fa257d25b533b6fd6b3cd341227ecc2..309f0e5c9c6cb4c67ee9f1fdf27ce995a046cded 100644 (file)
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
@@ -283,7 +283,7 @@ struct mtk_vcodec_dec_dev {
        /* decoder hardware mutex lock */
        struct mutex dec_mutex[MTK_VDEC_HW_MAX];
        struct mutex dev_mutex;
-       struct mutex dev_ctx_lock;
+       spinlock_t dev_ctx_lock;
        struct workqueue_struct *decode_workqueue;
 
        spinlock_t irqlock;

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
index 145958206e38a4be1d8f4cccc331cde82f5f4e27..40b97f114cf6f02e4bc6317346b05eb4e660e90a 100644 (file)
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
@@ -75,16 +75,17 @@ static void handle_get_param_msg_ack(const struct vdec_vpu_ipi_get_param_ack *ms
 static bool vpu_dec_check_ap_inst(struct mtk_vcodec_dec_dev *dec_dev, struct vdec_vpu_inst *vpu)
 {
        struct mtk_vcodec_dec_ctx *ctx;
+       unsigned long flags;
        int ret = false;
 
-       mutex_lock(&dec_dev->dev_ctx_lock);
+       spin_lock_irqsave(&dec_dev->dev_ctx_lock, flags);
        list_for_each_entry(ctx, &dec_dev->ctx_list, list) {
                if (!IS_ERR_OR_NULL(ctx) && ctx->vpu_inst == vpu) {
                        ret = true;
                        break;
                }
        }
-       mutex_unlock(&dec_dev->dev_ctx_lock);
+       spin_unlock_irqrestore(&dec_dev->dev_ctx_lock, flags);
 
        return ret;
 }

diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
index 3cb8a16222220e2d5480b48b48879112a68fc11f..6dcd6e9047326d8cd107eef8e74a24c71c1ad174 100644 (file)
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
@@ -117,6 +117,7 @@ static int fops_vcodec_open(struct file *file)
        struct mtk_vcodec_enc_ctx *ctx = NULL;
        int ret = 0;
        struct vb2_queue *src_vq;
+       unsigned long flags;
 
        ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
        if (!ctx)
@@ -177,9 +178,9 @@ static int fops_vcodec_open(struct file *file)
        mtk_v4l2_venc_dbg(2, ctx, "Create instance [%d]@%p m2m_ctx=%p ",
                          ctx->id, ctx, ctx->m2m_ctx);
 
-       mutex_lock(&dev->dev_ctx_lock);
+       spin_lock_irqsave(&dev->dev_ctx_lock, flags);
        list_add(&ctx->list, &dev->ctx_list);
-       mutex_unlock(&dev->dev_ctx_lock);
+       spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
 
        mutex_unlock(&dev->dev_mutex);
        mtk_v4l2_venc_dbg(0, ctx, "%s encoder [%d]", dev_name(&dev->plat_dev->dev),
@@ -204,6 +205,7 @@ static int fops_vcodec_release(struct file *file)
 {
        struct mtk_vcodec_enc_dev *dev = video_drvdata(file);
        struct mtk_vcodec_enc_ctx *ctx = fh_to_enc_ctx(file->private_data);
+       unsigned long flags;
 
        mtk_v4l2_venc_dbg(1, ctx, "[%d] encoder", ctx->id);
        mutex_lock(&dev->dev_mutex);
@@ -214,9 +216,9 @@ static int fops_vcodec_release(struct file *file)
        v4l2_fh_exit(&ctx->fh);
        v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
 
-       mutex_lock(&dev->dev_ctx_lock);
+       spin_lock_irqsave(&dev->dev_ctx_lock, flags);
        list_del_init(&ctx->list);
-       mutex_unlock(&dev->dev_ctx_lock);
+       spin_unlock_irqrestore(&dev->dev_ctx_lock, flags);
        kfree(ctx);
        mutex_unlock(&dev->dev_mutex);
        return 0;
@@ -298,7 +300,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 
        mutex_init(&dev->enc_mutex);
        mutex_init(&dev->dev_mutex);
-       mutex_init(&dev->dev_ctx_lock);
+       spin_lock_init(&dev->dev_ctx_lock);
        spin_lock_init(&dev->irqlock);
 
        snprintf(dev->v4l2_dev.name, sizeof(dev->v4l2_dev.name), "%s",

diff --git a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
index 0bd85d0fb379acbba3ac07c01e780cf57bef0305..90b4f5ea0d6bcdf021cb45013377787e4ab25661 100644 (file)
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
@@ -206,7 +206,7 @@ struct mtk_vcodec_enc_dev {
        /* encoder hardware mutex lock */
        struct mutex enc_mutex;
        struct mutex dev_mutex;
-       struct mutex dev_ctx_lock;
+       spinlock_t dev_ctx_lock;
        struct workqueue_struct *encode_workqueue;
 
        int enc_irq;

diff --git a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
index 51bb7ee141b9e58ac98f940f5e419d9ef4df37ca..3c229b1f6b21f48f1824bdf00e0be8f05782125e 100644 (file)
--- a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
@@ -45,16 +45,17 @@ static void handle_enc_encode_msg(struct venc_vpu_inst *vpu, const void *data)
 static bool vpu_enc_check_ap_inst(struct mtk_vcodec_enc_dev *enc_dev, struct venc_vpu_inst *vpu)
 {
        struct mtk_vcodec_enc_ctx *ctx;
+       unsigned long flags;
        int ret = false;
 
-       mutex_lock(&enc_dev->dev_ctx_lock);
+       spin_lock_irqsave(&enc_dev->dev_ctx_lock, flags);
        list_for_each_entry(ctx, &enc_dev->ctx_list, list) {
                if (!IS_ERR_OR_NULL(ctx) && ctx->vpu_inst == vpu) {
                        ret = true;
                        break;
                }
        }
-       mutex_unlock(&enc_dev->dev_ctx_lock);
+       spin_unlock_irqrestore(&enc_dev->dev_ctx_lock, flags);
 
        return ret;
 }