* [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder context list
@ 2024-01-29 2:31 Yunfei Dong
2024-01-29 2:31 ` [PATCH v2,2/2] media: mediatek: vcodec: adding lock to protect encoder " Yunfei Dong
2024-01-29 11:19 ` [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder " AngeloGioacchino Del Regno
0 siblings, 2 replies; 5+ messages in thread
From: Yunfei Dong @ 2024-01-29 2:31 UTC (permalink / raw)
To: Nícolas F . R . A . Prado, Nicolas Dufresne, Hans Verkuil,
AngeloGioacchino Del Regno, Benjamin Gaignard, Nathan Hebert,
Irui Wang
Cc: Hsin-Yi Wang, Fritz Koenig, Daniel Vetter, Steve Cho,
Yunfei Dong, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek,
Project_Global_Chrome_Upstream_Group
The ctx_list will be deleted when scp getting unexpected behavior, then the
ctx_list->next will be NULL, the kernel driver maybe access NULL pointer in
function vpu_dec_ipi_handler when going through each context, then reboot.
Need to add lock to protect the ctx_list to make sure the ctx_list->next isn't
NULL pointer.
Hardware name: Google juniper sku16 board (DT)
pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
pc : vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec]
lr : scp_ipi_handler+0xd0/0x194 [mtk_scp]
sp : ffffffc0131dbbd0
x29: ffffffc0131dbbd0 x28: 0000000000000000
x27: ffffff9bb277f348 x26: ffffff9bb242ad00
x25: ffffffd2d440d3b8 x24: ffffffd2a13ff1d4
x23: ffffff9bb7fe85a0 x22: ffffffc0133fbdb0
x21: 0000000000000010 x20: ffffff9b050ea328
x19: ffffffc0131dbc08 x18: 0000000000001000
x17: 0000000000000000 x16: ffffffd2d461c6e0
x15: 0000000000000242 x14: 000000000000018f
x13: 000000000000004d x12: 0000000000000000
x11: 0000000000000001 x10: fffffffffffffff0
x9 : ffffff9bb6e793a8 x8 : 0000000000000000
x7 : 0000000000000000 x6 : 000000000000003f
x5 : 0000000000000040 x4 : fffffffffffffff0
x3 : 0000000000000020 x2 : ffffff9bb6e79080
x1 : 0000000000000010 x0 : ffffffc0131dbc08
Call trace:
vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec (HASH:6c3f 2)]
scp_ipi_handler+0xd0/0x194 [mtk_scp (HASH:7046 3)]
mt8183_scp_irq_handler+0x44/0x88 [mtk_scp (HASH:7046 3)]
scp_irq_handler+0x48/0x90 [mtk_scp (HASH:7046 3)]
irq_thread_fn+0x38/0x94
irq_thread+0x100/0x1c0
kthread+0x140/0x1fc
ret_from_fork+0x10/0x30
Code: 54000088 f94ca50a eb14015f 54000060 (f9400108)
---[ end trace ace43ce36cbd5c93 ]---
Kernel panic - not syncing: Oops: Fatal exception
SMP: stopping secondary CPUs
Kernel Offset: 0x12c4000000 from 0xffffffc010000000
PHYS_OFFSET: 0xffffffe580000000
CPU features: 0x08240002,2188200c
Memory Limit: none
'Fixes: 655b86e52eac ("media: mediatek: vcodec: Fix possible invalid memory access for decoder")'
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c | 4 ++--
.../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 5 +++++
.../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h | 2 ++
drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c | 2 ++
4 files changed, 11 insertions(+), 2 deletions(-)
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 9f6e4b59455d..9a11a2c24804 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
@@ -58,12 +58,12 @@ static void mtk_vcodec_vpu_reset_dec_handler(void *priv)
dev_err(&dev->plat_dev->dev, "Watchdog timeout!!");
- mutex_lock(&dev->dev_mutex);
+ mutex_lock(&dev->dev_ctx_lock);
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_mutex);
+ mutex_unlock(&dev->dev_ctx_lock);
}
static void mtk_vcodec_vpu_reset_enc_handler(void *priv)
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 f47c98faf068..2073781ccadb 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
@@ -268,7 +268,9 @@ static int fops_vcodec_open(struct file *file)
ctx->dev->vdec_pdata->init_vdec_params(ctx);
+ mutex_lock(&dev->dev_ctx_lock);
list_add(&ctx->list, &dev->ctx_list);
+ mutex_unlock(&dev->dev_ctx_lock);
mtk_vcodec_dbgfs_create(ctx);
mutex_unlock(&dev->dev_mutex);
@@ -311,7 +313,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);
list_del_init(&ctx->list);
+ mutex_unlock(&dev->dev_ctx_lock);
kfree(ctx);
mutex_unlock(&dev->dev_mutex);
return 0;
@@ -404,6 +408,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->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 849b89dd205c..85b2c0d3d8bc 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
@@ -241,6 +241,7 @@ struct mtk_vcodec_dec_ctx {
*
* @dec_mutex: decoder hardware lock
* @dev_mutex: video_device lock
+ * @dev_ctx_lock: the lock of context list
* @decode_workqueue: decode work queue
*
* @irqlock: protect data access by irq handler and work thread
@@ -282,6 +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;
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 82e57ae983d5..da6be556727b 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c
@@ -77,12 +77,14 @@ static bool vpu_dec_check_ap_inst(struct mtk_vcodec_dec_dev *dec_dev, struct vde
struct mtk_vcodec_dec_ctx *ctx;
int ret = false;
+ mutex_lock(&dec_dev->dev_ctx_lock);
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);
return ret;
}
--
2.18.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2,2/2] media: mediatek: vcodec: adding lock to protect encoder context list
2024-01-29 2:31 [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder context list Yunfei Dong
@ 2024-01-29 2:31 ` Yunfei Dong
2024-01-29 11:19 ` [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder " AngeloGioacchino Del Regno
1 sibling, 0 replies; 5+ messages in thread
From: Yunfei Dong @ 2024-01-29 2:31 UTC (permalink / raw)
To: Nícolas F . R . A . Prado, Nicolas Dufresne, Hans Verkuil,
AngeloGioacchino Del Regno, Benjamin Gaignard, Nathan Hebert,
Irui Wang
Cc: Hsin-Yi Wang, Fritz Koenig, Daniel Vetter, Steve Cho,
Yunfei Dong, linux-media, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek,
Project_Global_Chrome_Upstream_Group
The ctx_list will be deleted when scp getting unexpected behavior, then the
ctx_list->next will be NULL, the kernel driver maybe access NULL pointer in
function vpu_enc_ipi_handler when going through each context, then reboot.
Need to add lock to protect the ctx_list to make sure the ctx_list->next isn't
NULL pointer.
'Fixes: 1972e32431ed ("media: mediatek: vcodec: Fix possible invalid memory access for encoder")'
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
change in v2:
- fix unlock issue.
---
.../platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c | 4 ++--
.../platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c | 5 +++++
.../platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h | 2 ++
drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c | 2 ++
4 files changed, 11 insertions(+), 2 deletions(-)
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 9a11a2c24804..8d578b690214 100644
--- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
+++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c
@@ -73,12 +73,12 @@ static void mtk_vcodec_vpu_reset_enc_handler(void *priv)
dev_err(&dev->plat_dev->dev, "Watchdog timeout!!");
- mutex_lock(&dev->dev_mutex);
+ mutex_lock(&dev->dev_ctx_lock);
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_mutex);
+ mutex_unlock(&dev->dev_ctx_lock);
}
static const struct mtk_vcodec_fw_ops mtk_vcodec_vpu_msg = {
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 6319f24bc714..3cb8a1622222 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c
@@ -177,7 +177,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);
list_add(&ctx->list, &dev->ctx_list);
+ mutex_unlock(&dev->dev_ctx_lock);
mutex_unlock(&dev->dev_mutex);
mtk_v4l2_venc_dbg(0, ctx, "%s encoder [%d]", dev_name(&dev->plat_dev->dev),
@@ -212,7 +214,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);
list_del_init(&ctx->list);
+ mutex_unlock(&dev->dev_ctx_lock);
kfree(ctx);
mutex_unlock(&dev->dev_mutex);
return 0;
@@ -294,6 +298,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->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 a042f607ed8d..0bd85d0fb379 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h
@@ -178,6 +178,7 @@ struct mtk_vcodec_enc_ctx {
*
* @enc_mutex: encoder hardware lock.
* @dev_mutex: video_device lock
+ * @dev_ctx_lock: the lock of context list
* @encode_workqueue: encode work queue
*
* @enc_irq: h264 encoder irq resource
@@ -205,6 +206,7 @@ struct mtk_vcodec_enc_dev {
/* encoder hardware mutex lock */
struct mutex enc_mutex;
struct mutex dev_mutex;
+ struct mutex 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 84ad1cc6ad17..51bb7ee141b9 100644
--- a/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
+++ b/drivers/media/platform/mediatek/vcodec/encoder/venc_vpu_if.c
@@ -47,12 +47,14 @@ static bool vpu_enc_check_ap_inst(struct mtk_vcodec_enc_dev *enc_dev, struct ven
struct mtk_vcodec_enc_ctx *ctx;
int ret = false;
+ mutex_lock(&enc_dev->dev_ctx_lock);
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);
return ret;
}
--
2.18.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder context list
2024-01-29 2:31 [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder context list Yunfei Dong
2024-01-29 2:31 ` [PATCH v2,2/2] media: mediatek: vcodec: adding lock to protect encoder " Yunfei Dong
@ 2024-01-29 11:19 ` AngeloGioacchino Del Regno
2024-01-30 6:29 ` Yunfei Dong (董云飞)
1 sibling, 1 reply; 5+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-29 11:19 UTC (permalink / raw)
To: Yunfei Dong, Nícolas F . R . A . Prado, Nicolas Dufresne,
Hans Verkuil, Benjamin Gaignard, Nathan Hebert, Irui Wang
Cc: Hsin-Yi Wang, Fritz Koenig, Daniel Vetter, Steve Cho,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Project_Global_Chrome_Upstream_Group
Il 29/01/24 03:31, Yunfei Dong ha scritto:
> The ctx_list will be deleted when scp getting unexpected behavior, then the
> ctx_list->next will be NULL, the kernel driver maybe access NULL pointer in
> function vpu_dec_ipi_handler when going through each context, then reboot.
>
> Need to add lock to protect the ctx_list to make sure the ctx_list->next isn't
> NULL pointer.
>
> Hardware name: Google juniper sku16 board (DT)
> pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
> pc : vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec]
> lr : scp_ipi_handler+0xd0/0x194 [mtk_scp]
> sp : ffffffc0131dbbd0
> x29: ffffffc0131dbbd0 x28: 0000000000000000
> x27: ffffff9bb277f348 x26: ffffff9bb242ad00
> x25: ffffffd2d440d3b8 x24: ffffffd2a13ff1d4
> x23: ffffff9bb7fe85a0 x22: ffffffc0133fbdb0
> x21: 0000000000000010 x20: ffffff9b050ea328
> x19: ffffffc0131dbc08 x18: 0000000000001000
> x17: 0000000000000000 x16: ffffffd2d461c6e0
> x15: 0000000000000242 x14: 000000000000018f
> x13: 000000000000004d x12: 0000000000000000
> x11: 0000000000000001 x10: fffffffffffffff0
> x9 : ffffff9bb6e793a8 x8 : 0000000000000000
> x7 : 0000000000000000 x6 : 000000000000003f
> x5 : 0000000000000040 x4 : fffffffffffffff0
> x3 : 0000000000000020 x2 : ffffff9bb6e79080
> x1 : 0000000000000010 x0 : ffffffc0131dbc08
> Call trace:
> vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec (HASH:6c3f 2)]
> scp_ipi_handler+0xd0/0x194 [mtk_scp (HASH:7046 3)]
> mt8183_scp_irq_handler+0x44/0x88 [mtk_scp (HASH:7046 3)]
> scp_irq_handler+0x48/0x90 [mtk_scp (HASH:7046 3)]
> irq_thread_fn+0x38/0x94
> irq_thread+0x100/0x1c0
> kthread+0x140/0x1fc
> ret_from_fork+0x10/0x30
> Code: 54000088 f94ca50a eb14015f 54000060 (f9400108)
> ---[ end trace ace43ce36cbd5c93 ]---
> Kernel panic - not syncing: Oops: Fatal exception
> SMP: stopping secondary CPUs
> Kernel Offset: 0x12c4000000 from 0xffffffc010000000
> PHYS_OFFSET: 0xffffffe580000000
> CPU features: 0x08240002,2188200c
> Memory Limit: none
>
> 'Fixes: 655b86e52eac ("media: mediatek: vcodec: Fix possible invalid memory access for decoder")'
Hello Yunfei,
You've sent two patches as a v2, but:
- The two patches are identical (!) apart from the commit message?!
- It's Fixes: xxxx , not 'Fixes: xxxx' (please remove the quotes!)
- There's no changelog from v1, so, what changed in v2?!
Cheers,
Angelo
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
> .../platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c | 4 ++--
> .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 5 +++++
> .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h | 2 ++
> drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c | 2 ++
> 4 files changed, 11 insertions(+), 2 deletions(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder context list
2024-01-29 11:19 ` [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder " AngeloGioacchino Del Regno
@ 2024-01-30 6:29 ` Yunfei Dong (董云飞)
2024-01-30 8:50 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 5+ messages in thread
From: Yunfei Dong (董云飞) @ 2024-01-30 6:29 UTC (permalink / raw)
To: nhebert, benjamin.gaignard, nfraprado, angelogioacchino.delregno,
nicolas.dufresne, hverkuil-cisco, Irui Wang (王瑞)
Cc: linux-kernel, linux-mediatek, frkoenig, stevecho, linux-media,
devicetree, daniel, Project_Global_Chrome_Upstream_Group, hsinyi,
linux-arm-kernel
Hi AngeloGioacchino,
Thanks for your reviewing.
On Mon, 2024-01-29 at 12:19 +0100, AngeloGioacchino Del Regno wrote:
> Il 29/01/24 03:31, Yunfei Dong ha scritto:
> > The ctx_list will be deleted when scp getting unexpected behavior,
> > then the
> > ctx_list->next will be NULL, the kernel driver maybe access NULL
> > pointer in
> > function vpu_dec_ipi_handler when going through each context, then
> > reboot.
> >
> > Need to add lock to protect the ctx_list to make sure the ctx_list-
> > >next isn't
> > NULL pointer.
> >
> > Hardware name: Google juniper sku16 board (DT)
> > pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
> > pc : vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec]
> > lr : scp_ipi_handler+0xd0/0x194 [mtk_scp]
> > sp : ffffffc0131dbbd0
> > x29: ffffffc0131dbbd0 x28: 0000000000000000
> > x27: ffffff9bb277f348 x26: ffffff9bb242ad00
> > x25: ffffffd2d440d3b8 x24: ffffffd2a13ff1d4
> > x23: ffffff9bb7fe85a0 x22: ffffffc0133fbdb0
> > x21: 0000000000000010 x20: ffffff9b050ea328
> > x19: ffffffc0131dbc08 x18: 0000000000001000
> > x17: 0000000000000000 x16: ffffffd2d461c6e0
> > x15: 0000000000000242 x14: 000000000000018f
> > x13: 000000000000004d x12: 0000000000000000
> > x11: 0000000000000001 x10: fffffffffffffff0
> > x9 : ffffff9bb6e793a8 x8 : 0000000000000000
> > x7 : 0000000000000000 x6 : 000000000000003f
> > x5 : 0000000000000040 x4 : fffffffffffffff0
> > x3 : 0000000000000020 x2 : ffffff9bb6e79080
> > x1 : 0000000000000010 x0 : ffffffc0131dbc08
> > Call trace:
> > vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec (HASH:6c3f 2)]
> > scp_ipi_handler+0xd0/0x194 [mtk_scp (HASH:7046 3)]
> > mt8183_scp_irq_handler+0x44/0x88 [mtk_scp (HASH:7046 3)]
> > scp_irq_handler+0x48/0x90 [mtk_scp (HASH:7046 3)]
> > irq_thread_fn+0x38/0x94
> > irq_thread+0x100/0x1c0
> > kthread+0x140/0x1fc
> > ret_from_fork+0x10/0x30
> > Code: 54000088 f94ca50a eb14015f 54000060 (f9400108)
> > ---[ end trace ace43ce36cbd5c93 ]---
> > Kernel panic - not syncing: Oops: Fatal exception
> > SMP: stopping secondary CPUs
> > Kernel Offset: 0x12c4000000 from 0xffffffc010000000
> > PHYS_OFFSET: 0xffffffe580000000
> > CPU features: 0x08240002,2188200c
> > Memory Limit: none
> >
> > 'Fixes: 655b86e52eac ("media: mediatek: vcodec: Fix possible
> > invalid memory access for decoder")'
>
> Hello Yunfei,
>
> You've sent two patches as a v2, but:
> - The two patches are identical (!) apart from the commit message?!
> - It's Fixes: xxxx , not 'Fixes: xxxx' (please remove the quotes!)
> - There's no changelog from v1, so, what changed in v2?!
>
1> These two patch used to fix the same issue, just used to separate
encoder with decoder;
2> Will fix in next patch;
3> patch 1 are the same for v1 and v2, just the patch 2 (encoder)
change something.
Best Regards,
Yunfei Dong
> Cheers,
> Angelo
>
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> > .../platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c | 4
> > ++--
> > .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 5
> > +++++
> > .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h | 2
> > ++
> > drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c | 2
> > ++
> > 4 files changed, 11 insertions(+), 2 deletions(-)
> >
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder context list
2024-01-30 6:29 ` Yunfei Dong (董云飞)
@ 2024-01-30 8:50 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 5+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-01-30 8:50 UTC (permalink / raw)
To: Yunfei Dong (董云飞),
nhebert, benjamin.gaignard, nfraprado, nicolas.dufresne,
hverkuil-cisco, Irui Wang (王瑞)
Cc: linux-kernel, linux-mediatek, frkoenig, stevecho, linux-media,
devicetree, daniel, Project_Global_Chrome_Upstream_Group, hsinyi,
linux-arm-kernel
Il 30/01/24 07:29, Yunfei Dong (董云飞) ha scritto:
> Hi AngeloGioacchino,
>
> Thanks for your reviewing.
> On Mon, 2024-01-29 at 12:19 +0100, AngeloGioacchino Del Regno wrote:
>> Il 29/01/24 03:31, Yunfei Dong ha scritto:
>>> The ctx_list will be deleted when scp getting unexpected behavior,
>>> then the
>>> ctx_list->next will be NULL, the kernel driver maybe access NULL
>>> pointer in
>>> function vpu_dec_ipi_handler when going through each context, then
>>> reboot.
>>>
>>> Need to add lock to protect the ctx_list to make sure the ctx_list-
>>>> next isn't
>>> NULL pointer.
>>>
>>> Hardware name: Google juniper sku16 board (DT)
>>> pstate: 20400005 (nzCv daif +PAN -UAO -TCO BTYPE=--)
>>> pc : vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec]
>>> lr : scp_ipi_handler+0xd0/0x194 [mtk_scp]
>>> sp : ffffffc0131dbbd0
>>> x29: ffffffc0131dbbd0 x28: 0000000000000000
>>> x27: ffffff9bb277f348 x26: ffffff9bb242ad00
>>> x25: ffffffd2d440d3b8 x24: ffffffd2a13ff1d4
>>> x23: ffffff9bb7fe85a0 x22: ffffffc0133fbdb0
>>> x21: 0000000000000010 x20: ffffff9b050ea328
>>> x19: ffffffc0131dbc08 x18: 0000000000001000
>>> x17: 0000000000000000 x16: ffffffd2d461c6e0
>>> x15: 0000000000000242 x14: 000000000000018f
>>> x13: 000000000000004d x12: 0000000000000000
>>> x11: 0000000000000001 x10: fffffffffffffff0
>>> x9 : ffffff9bb6e793a8 x8 : 0000000000000000
>>> x7 : 0000000000000000 x6 : 000000000000003f
>>> x5 : 0000000000000040 x4 : fffffffffffffff0
>>> x3 : 0000000000000020 x2 : ffffff9bb6e79080
>>> x1 : 0000000000000010 x0 : ffffffc0131dbc08
>>> Call trace:
>>> vpu_dec_ipi_handler+0x58/0x1f8 [mtk_vcodec_dec (HASH:6c3f 2)]
>>> scp_ipi_handler+0xd0/0x194 [mtk_scp (HASH:7046 3)]
>>> mt8183_scp_irq_handler+0x44/0x88 [mtk_scp (HASH:7046 3)]
>>> scp_irq_handler+0x48/0x90 [mtk_scp (HASH:7046 3)]
>>> irq_thread_fn+0x38/0x94
>>> irq_thread+0x100/0x1c0
>>> kthread+0x140/0x1fc
>>> ret_from_fork+0x10/0x30
>>> Code: 54000088 f94ca50a eb14015f 54000060 (f9400108)
>>> ---[ end trace ace43ce36cbd5c93 ]---
>>> Kernel panic - not syncing: Oops: Fatal exception
>>> SMP: stopping secondary CPUs
>>> Kernel Offset: 0x12c4000000 from 0xffffffc010000000
>>> PHYS_OFFSET: 0xffffffe580000000
>>> CPU features: 0x08240002,2188200c
>>> Memory Limit: none
>>>
>>> 'Fixes: 655b86e52eac ("media: mediatek: vcodec: Fix possible
>>> invalid memory access for decoder")'
>>
>> Hello Yunfei,
>>
>> You've sent two patches as a v2, but:
>> - The two patches are identical (!) apart from the commit message?!
>> - It's Fixes: xxxx , not 'Fixes: xxxx' (please remove the quotes!)
>> - There's no changelog from v1, so, what changed in v2?!
>>
> 1> These two patch used to fix the same issue, just used to separate
> encoder with decoder;
I just noticed that - I'm sorry.
> 2> Will fix in next patch;
> 3> patch 1 are the same for v1 and v2, just the patch 2 (encoder)
> change something.
>
Next time, can you please add a cover letter to your series?
I think it would be easier for people to see what changed in the entire series,
even if it is just two or three patches, as you'd be writing the changelog in
there instead of writing it in each patch :-)
> Best Regards,
> Yunfei Dong
>> Cheers,
>> Angelo
>>
>>> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>>> ---
>>> .../platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c | 4
>>> ++--
>>> .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 5
>>> +++++
>>> .../platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h | 2
>>> ++
>>> drivers/media/platform/mediatek/vcodec/decoder/vdec_vpu_if.c | 2
>>> ++
>>> 4 files changed, 11 insertions(+), 2 deletions(-)
>>>
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-30 8:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-29 2:31 [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder context list Yunfei Dong
2024-01-29 2:31 ` [PATCH v2,2/2] media: mediatek: vcodec: adding lock to protect encoder " Yunfei Dong
2024-01-29 11:19 ` [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder " AngeloGioacchino Del Regno
2024-01-30 6:29 ` Yunfei Dong (董云飞)
2024-01-30 8:50 ` AngeloGioacchino Del Regno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).