From nobody Sat Oct 4 04:57:28 2025 Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F244D2C11D2 for ; Wed, 20 Aug 2025 07:54:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.169 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755676456; cv=none; b=uitlz5cqWkgVEv5BOipJPI8xtA24CS3p6mTkTsB5dAnDr6/Cf4w2D7oXmMZV11+EVXMnRBEDXUZbe7phlUeQwGNGPh/40LFn9k9xA8InjhFrgfMmazN+MKXsm6dDyOS2e7vEzRHOpjvah1J+RBpQpReewxlbjk3ZaGXJJXlTh3E= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755676456; c=relaxed/simple; bh=ntFIgaL02srWXrd27KTUirbWqr2GvntCST8bEnN1u10=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=P90rMeNpI1s18JCmmURBcdw/CfBIS9evSK5CI9NM9vxrTExUJ9a26Gjd3PTDmozaj4VYBf+Zfea7EI8OlD0becKjIQ+ZzoSqus3OMnq6OnPP8tm2OtyHDIcvvyxviPt9wZ3taBY92RgYcOOEnJkCWSpXpTC0BwvefawEwwY8wpc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=FSXw55u6; arc=none smtp.client-ip=209.85.210.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="FSXw55u6" Received: by mail-pf1-f169.google.com with SMTP id d2e1a72fcca58-76e92b3ded3so122216b3a.2 for ; Wed, 20 Aug 2025 00:54:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1755676454; x=1756281254; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=b6MrObmZF0W3Iz/7ouACW2WJ6KhbE+Mk18toMzH5Rik=; b=FSXw55u6dgG59+5EoNNnlEp2jy3afuJENKS0DjmLxM+tyGdBz1Vh0TY0Se5ZgIbLo0 ExAkdsk4BLdnxLmy58YMcw9gqDFf758PtEAnULa3xWq8Txun3dFi9IGz6QFj5Xu0VCav nvjw9M8Cw7OucuvjsOBTQUHHNA8lY+UWk2bmk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1755676454; x=1756281254; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=b6MrObmZF0W3Iz/7ouACW2WJ6KhbE+Mk18toMzH5Rik=; b=lqA5KfeNmCeM1wIPOBiILtws1AS0SbM2nUhaojxyuraYCnq+57cXyfYLxpYQ6i+hwa b5n0YaeLSUXjcdYsjrda9wlhvnNgow2MEm4/nlVnoI42vJLvqx0VtCD+sPrMJN2+mB2E 0Nh0yfOiYCPFzSKHGrsnPkHRS9ysRfEYtriyBIwb3OiVhzeVsxQtk/72VqJS4XhNgsAZ o83HjO4MSmmm4gH45IjgtpPYo86Uzdch1lXfLzeXUhbTfvX6goGygQObVdR/O0hMrR/f RzeN26tdhJggD+DiXpnUazAKTMJ2ajamAGTbXkLzgHDiM38css2XlzIAeL6HfoEuZgsG 3A+w== X-Forwarded-Encrypted: i=1; AJvYcCUAKjZCYdSVAEcMT8q9vWqiUC+t0thjxEojCzMslJoYYa9Hi/A8AfcXg+WkbZKCHJT4UQFUkmfF+ePY/A0=@vger.kernel.org X-Gm-Message-State: AOJu0Yz/93Uxjm+4ndLBm4mB+VV5YM85ttdgCUaKisBH2U7GO50L80PO Dr1P57w3uu4liX+6EUm3jYY0A0WuR5YRJudOce7ep6zJyLyWRy+/JIOwoXWQ+4c7fA== X-Gm-Gg: ASbGncuVu6JmVqKcPX3t4mQ0g/6rcJhxDYSwGHR5NvTlz2XJjBa96z3CYQEcfXB1DN3 qunl7oDdMBo2IcQWrFe8CX3vyQ+s4ZK7DBa+ZuOP6BoyHLwc5zF0RrEu2Gi3aHmpQyDf70zyXXO yQBd5lk7e8+tvdn8z3pTOhD7AC5gmlJBulLC95xG4f+tj/UZ90KWDYkvY6fLBUMCv5MMq8izgQm rBu3pYTP2Vnw1MHsv+Fkd/AprxpiFtq4OMrav+uTVloQ+fKEMdtUuLAOTXgutoohrDZ9b0I7TiO 8c6gAapk2cck3h4qzG9L0fXyij2r5vKZoOYDL7Qq1YAn73fhluSciPL+A4qcyZXZujH5WX2rQHc ODm2rYgndhTaXU4qBp3w6uMtLflJXlGmKw5sSInJS X-Google-Smtp-Source: AGHT+IHvJjrk2TtQqpsyJ1Ye0D3Ld8eJPj/J3yAc/mNjIWdy7fiHoTPy/AdpxRywfbk04Kizt/1zwQ== X-Received: by 2002:a05:6a20:389d:b0:23f:f431:9f64 with SMTP id adf61e73a8af0-2431b829560mr2427344637.25.1755676454152; Wed, 20 Aug 2025 00:54:14 -0700 (PDT) Received: from wenstp920.tpe.corp.google.com ([2401:fa00:1:10:53f4:ede0:803f:9a86]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-b4764090714sm1547298a12.32.2025.08.20.00.54.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 Aug 2025 00:54:13 -0700 (PDT) From: Chen-Yu Tsai To: Yunfei Dong , Mauro Carvalho Chehab , Hans Verkuil , Matthias Brugger , AngeloGioacchino Del Regno Cc: Chen-Yu Tsai , linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, Tomasz Figa Subject: [PATCH v2] media: mediatek: vcodec: Use spinlock for context list protection lock Date: Wed, 20 Aug 2025 15:54:05 +0800 Message-ID: <20250820075405.1041838-1-wenst@chromium.org> X-Mailer: git-send-email 2.51.0.rc1.167.g924127e9c0-goog Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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 decod= er context list") Fixes: afaaf3a0f647 ("media: mediatek: vcodec: adding lock to protect encod= er context list") Cc: Yunfei Dong Signed-off-by: Chen-Yu Tsai Reviewed-by: Fei Shao Reviewed-by: Tomasz Figa --- Changes since v1: - Switched to _irqsave / _irqrestore variants even in helper only called by IRQ handler (Tomasz) .../mediatek/vcodec/common/mtk_vcodec_fw_vpu.c | 10 ++++++---- .../mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 12 +++++++----- .../mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h | 2 +- .../platform/mediatek/vcodec/decoder/vdec_vpu_if.c | 5 +++-- .../mediatek/vcodec/encoder/mtk_vcodec_enc_drv.c | 12 +++++++----- .../mediatek/vcodec/encoder/mtk_vcodec_enc_drv.h | 2 +- .../platform/mediatek/vcodec/encoder/venc_vpu_if.c | 5 +++-- 7 files changed, 28 insertions(+), 20 deletions(-) diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vp= u.c b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_fw_vpu.c index d7027d600208..223fb2294894 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 @@ -47,30 +47,32 @@ static void mtk_vcodec_vpu_reset_dec_handler(void *priv) { struct mtk_vcodec_dec_dev *dev =3D priv; struct mtk_vcodec_dec_ctx *ctx; + unsigned long flags; =20 dev_err(&dev->plat_dev->dev, "Watchdog timeout!!"); =20 - 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 =3D MTK_STATE_ABORT; mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->i= d); } - mutex_unlock(&dev->dev_ctx_lock); + spin_unlock_irqrestore(&dev->dev_ctx_lock, flags); } =20 static void mtk_vcodec_vpu_reset_enc_handler(void *priv) { struct mtk_vcodec_enc_dev *dev =3D priv; struct mtk_vcodec_enc_ctx *ctx; + unsigned long flags; =20 dev_err(&dev->plat_dev->dev, "Watchdog timeout!!"); =20 - 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 =3D MTK_STATE_ABORT; mtk_v4l2_vdec_dbg(0, ctx, "[%d] Change to state MTK_STATE_ABORT", ctx->i= d); } - mutex_unlock(&dev->dev_ctx_lock); + spin_unlock_irqrestore(&dev->dev_ctx_lock, flags); } =20 static const struct mtk_vcodec_fw_ops mtk_vcodec_vpu_msg =3D { 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 46d176e6de63..3b81fae9f913 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 @@ -198,6 +198,7 @@ static int fops_vcodec_open(struct file *file) struct mtk_vcodec_dec_ctx *ctx =3D NULL; int ret =3D 0, i, hw_count; struct vb2_queue *src_vq; + unsigned long flags; =20 ctx =3D kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) @@ -267,9 +268,9 @@ static int fops_vcodec_open(struct file *file) =20 ctx->dev->vdec_pdata->init_vdec_params(ctx); =20 - 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); =20 mutex_unlock(&dev->dev_mutex); @@ -294,6 +295,7 @@ static int fops_vcodec_release(struct file *file) { struct mtk_vcodec_dec_dev *dev =3D video_drvdata(file); struct mtk_vcodec_dec_ctx *ctx =3D file_to_dec_ctx(file); + unsigned long flags; =20 mtk_v4l2_vdec_dbg(0, ctx, "[%d] decoder", ctx->id); mutex_lock(&dev->dev_mutex); @@ -312,9 +314,9 @@ static int fops_vcodec_release(struct file *file) v4l2_ctrl_handler_free(&ctx->ctrl_hdl); =20 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; @@ -407,7 +409,7 @@ static int mtk_vcodec_probe(struct platform_device *pde= v) for (i =3D 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); =20 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 d047d7c580fb..9d68808e8f9c 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 @@ -285,7 +285,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; =20 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 145958206e38..40b97f114cf6 100644 --- 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, stru= ct vdec_vpu_inst *vpu) { struct mtk_vcodec_dec_ctx *ctx; + unsigned long flags; int ret =3D false; =20 - 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 =3D=3D vpu) { ret =3D true; break; } } - mutex_unlock(&dec_dev->dev_ctx_lock); + spin_unlock_irqrestore(&dec_dev->dev_ctx_lock, flags); =20 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 fb1c3bdc2dae..82b8ff38e8f1 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 @@ -117,6 +117,7 @@ static int fops_vcodec_open(struct file *file) struct mtk_vcodec_enc_ctx *ctx =3D NULL; int ret =3D 0; struct vb2_queue *src_vq; + unsigned long flags; =20 ctx =3D kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) @@ -176,9 +177,9 @@ static int fops_vcodec_open(struct file *file) mtk_v4l2_venc_dbg(2, ctx, "Create instance [%d]@%p m2m_ctx=3D%p ", ctx->id, ctx, ctx->m2m_ctx); =20 - 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); =20 mutex_unlock(&dev->dev_mutex); mtk_v4l2_venc_dbg(0, ctx, "%s encoder [%d]", dev_name(&dev->plat_dev->dev= ), @@ -203,6 +204,7 @@ static int fops_vcodec_release(struct file *file) { struct mtk_vcodec_enc_dev *dev =3D video_drvdata(file); struct mtk_vcodec_enc_ctx *ctx =3D file_to_enc_ctx(file); + unsigned long flags; =20 mtk_v4l2_venc_dbg(1, ctx, "[%d] encoder", ctx->id); mutex_lock(&dev->dev_mutex); @@ -213,9 +215,9 @@ static int fops_vcodec_release(struct file *file) v4l2_fh_exit(&ctx->fh); v4l2_ctrl_handler_free(&ctx->ctrl_hdl); =20 - 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; @@ -297,7 +299,7 @@ static int mtk_vcodec_probe(struct platform_device *pde= v) =20 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); =20 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 5b304a551236..0cddfa13594f 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 @@ -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; =20 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 51bb7ee141b9..3c229b1f6b21 100644 --- 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, stru= ct venc_vpu_inst *vpu) { struct mtk_vcodec_enc_ctx *ctx; + unsigned long flags; int ret =3D false; =20 - 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 =3D=3D vpu) { ret =3D true; break; } } - mutex_unlock(&enc_dev->dev_ctx_lock); + spin_unlock_irqrestore(&enc_dev->dev_ctx_lock, flags); =20 return ret; } --=20 2.51.0.rc1.167.g924127e9c0-goog