Re: [PATCH v2,1/2] media: mediatek: vcodec: adding lock to protect decoder context list

[Yunfei Dong (董云飞)]

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@xxxxxxxxxxxx>
> > ---
> >   .../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(-)
> > 
> 
>