Received-SPF: pass (google.com: domain of linux-kernel+bounces-110219-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 21 Mar 2024 09:25:23 -0600
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: andersson@kernel.org, matthias.bgg@gmail.com, tzungbi@kernel.org,
	tinghan.shen@mediatek.com, linux-remoteproc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org, wenst@chromium.org,
	kernel@collabora.com
Subject: Re: [PATCH 1/2] remoteproc: mediatek: Make sure IPI buffer fits in
 L2TCM
Message-ID: <ZfxRY475SKaRYVTj@p14s>
References: <20240321084614.45253-1-angelogioacchino.delregno@collabora.com>
 <20240321084614.45253-2-angelogioacchino.delregno@collabora.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240321084614.45253-2-angelogioacchino.delregno@collabora.com>

Good day,

On Thu, Mar 21, 2024 at 09:46:13AM +0100, AngeloGioacchino Del Regno wrote:
> The IPI buffer location is read from the firmware that we load to the
> System Companion Processor, and it's not granted that both the SRAM
> (L2TCM) size that is defined in the devicetree node is large enough
> for that, and while this is especially true for multi-core SCP, it's
> still useful to check on single-core variants as well.
> 
> Failing to perform this check may make this driver perform R/W
> oeprations out of the L2TCM boundary, resulting (at best) in a

s/oeprations/operations

I will fix that when I apply the patch.

> kernel panic.
> 
> To fix that, check that the IPI buffer fits, otherwise return a
> failure and refuse to boot the relevant SCP core (or the SCP at
> all, if this is single core).
> 
> Fixes: 3efa0ea743b7 ("remoteproc/mediatek: read IPI buffer offset from FW")
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/remoteproc/mtk_scp.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index a35409eda0cf..67518291a8ad 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -132,7 +132,7 @@ static int scp_elf_read_ipi_buf_addr(struct mtk_scp *scp,
>  static int scp_ipi_init(struct mtk_scp *scp, const struct firmware *fw)
>  {
>  	int ret;
> -	size_t offset;
> +	size_t buf_sz, offset;
>  
>  	/* read the ipi buf addr from FW itself first */
>  	ret = scp_elf_read_ipi_buf_addr(scp, fw, &offset);
> @@ -144,6 +144,14 @@ static int scp_ipi_init(struct mtk_scp *scp, const struct firmware *fw)
>  	}
>  	dev_info(scp->dev, "IPI buf addr %#010zx\n", offset);
>  
> +	/* Make sure IPI buffer fits in the L2TCM range assigned to this core */
> +	buf_sz = sizeof(*scp->recv_buf) + sizeof(*scp->send_buf);
> +
> +	if (scp->sram_size < buf_sz + offset) {
> +		dev_err(scp->dev, "IPI buffer does not fit in SRAM.\n");
> +		return -EOVERFLOW;
> +	}
> +
>  	scp->recv_buf = (struct mtk_share_obj __iomem *)
>  			(scp->sram_base + offset);
>  	scp->send_buf = (struct mtk_share_obj __iomem *)
> -- 
> 2.44.0
>