| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1f1207d0-c1ab-4d9a-8369-b8c6eadb289e@oss.nxp.com>
Date: Tue, 6 May 2025 14:28:35 +0800
From: "Ming Qian(OSS)" <ming.qian@....nxp.com>
To: Nicolas Dufresne <nicolas@...fresne.ca>, mchehab@...nel.org,
hverkuil-cisco@...all.nl, mirela.rabulea@....nxp.com
Cc: shawnguo@...nel.org, s.hauer@...gutronix.de, kernel@...gutronix.de,
festevam@...il.com, xiahong.bao@....com, eagle.zhou@....com,
linux-imx@....com, imx@...ts.linux.dev, linux-media@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] media: imx-jpeg: Account for data_offset when getting
image address
Hi Nicolas,
On 2025/5/2 1:02, Nicolas Dufresne wrote:
> Le mercredi 30 avril 2025 à 13:39 +0800, ming.qian@....nxp.com a écrit :
>> From: Ming Qian <ming.qian@....com>
>>
>> Applications may set data_offset when it refers to an output queue. So
>> driver need to account for it when getting the start address of input
>> image in the plane.
>>
>> Meanwhile data_offset is included in bytesused. So the data_offset
>> should be subtracted from the payload of input image.
>
> I think you should revisit this commit message a little in the next version.
> While the overall patch looks good, I believe you forgot to add code to verify
> that addr + data_offset still falls within the HW needed alignment. I don't
> have the HW documentation for that chip, but I have never seen HW capapble of
> handlign random alignment.
>
> Without the data_offset, the data is always page align, so we don't usually
> have to validate that.
>
> regards,
> Nicolas
>
Thanks for the reminder, the mxc-jpeg codec requires addresses to be
16-aligned. I'll fix it in v2.
Regards,
Ming
>>
>> Signed-off-by: Ming Qian <ming.qian@....com>
>> ---
>> .../media/platform/nxp/imx-jpeg/mxc-jpeg.c | 42 ++++++++++++++-----
>> 1 file changed, 31 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index 1221b309a916..035368d65913 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -587,6 +587,27 @@ static void _bswap16(u16 *a)
>> *a = ((*a & 0x00FF) << 8) | ((*a & 0xFF00) >> 8);
>> }
>>
>> +static dma_addr_t mxc_jpeg_get_plane_dma_addr(struct vb2_buffer *buf, unsigned int plane_no)
>> +{
>> + if (plane_no >= buf->num_planes)
>> + return 0;
>> + return vb2_dma_contig_plane_dma_addr(buf, plane_no) + buf->planes[plane_no].data_offset;
>> +}
>> +
>> +static void *mxc_jpeg_get_plane_vaddr(struct vb2_buffer *buf, unsigned int plane_no)
>> +{
>> + if (plane_no >= buf->num_planes)
>> + return NULL;
>> + return vb2_plane_vaddr(buf, plane_no) + buf->planes[plane_no].data_offset;
>> +}
>> +
>> +static unsigned long mxc_jpeg_get_plane_payload(struct vb2_buffer *buf, unsigned int plane_no)
>> +{
>> + if (plane_no >= buf->num_planes)
>> + return 0;
>> + return vb2_get_plane_payload(buf, plane_no) - buf->planes[plane_no].data_offset;
>> +}
>> +
>> static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
>> unsigned long len)
>> {
>> @@ -599,11 +620,11 @@ static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
>> return;
>>
>> for (plane_no = 0; plane_no < buf->num_planes; plane_no++) {
>> - payload = vb2_get_plane_payload(buf, plane_no);
>> + payload = mxc_jpeg_get_plane_payload(buf, plane_no);
>> if (len == 0)
>> len = payload;
>> - dma_addr = vb2_dma_contig_plane_dma_addr(buf, plane_no);
>> - vaddr = vb2_plane_vaddr(buf, plane_no);
>> + dma_addr = mxc_jpeg_get_plane_dma_addr(buf, plane_no);
>> + vaddr = mxc_jpeg_get_plane_vaddr(buf, plane_no);
>> v4l2_dbg(3, debug, &jpeg->v4l2_dev,
>> "plane %d (vaddr=%p dma_addr=%x payload=%ld):",
>> plane_no, vaddr, dma_addr, payload);
>> @@ -701,16 +722,15 @@ static void mxc_jpeg_addrs(struct mxc_jpeg_desc *desc,
>> struct mxc_jpeg_q_data *q_data;
>>
>> q_data = mxc_jpeg_get_q_data(ctx, raw_buf->type);
>> - desc->buf_base0 = vb2_dma_contig_plane_dma_addr(raw_buf, 0);
>> + desc->buf_base0 = mxc_jpeg_get_plane_dma_addr(raw_buf, 0);
>> desc->buf_base1 = 0;
>> if (img_fmt == STM_CTRL_IMAGE_FORMAT(MXC_JPEG_YUV420)) {
>> if (raw_buf->num_planes == 2)
>> - desc->buf_base1 = vb2_dma_contig_plane_dma_addr(raw_buf, 1);
>> + desc->buf_base1 = mxc_jpeg_get_plane_dma_addr(raw_buf, 1);
>> else
>> desc->buf_base1 = desc->buf_base0 + q_data->sizeimage[0];
>> }
>> - desc->stm_bufbase = vb2_dma_contig_plane_dma_addr(jpeg_buf, 0) +
>> - offset;
>> + desc->stm_bufbase = mxc_jpeg_get_plane_dma_addr(jpeg_buf, 0) + offset;
>> }
>>
>> static bool mxc_jpeg_is_extended_sequential(const struct mxc_jpeg_fmt *fmt)
>> @@ -967,8 +987,8 @@ static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
>> vb2_set_plane_payload(&dst_buf->vb2_buf, 1, payload);
>> }
>> dev_dbg(dev, "Decoding finished, payload size: %ld + %ld\n",
>> - vb2_get_plane_payload(&dst_buf->vb2_buf, 0),
>> - vb2_get_plane_payload(&dst_buf->vb2_buf, 1));
>> + mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 0),
>> + mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 1));
>> }
>>
>> /* short preview of the results */
>> @@ -1827,8 +1847,8 @@ static int mxc_jpeg_parse(struct mxc_jpeg_ctx *ctx, struct vb2_buffer *vb)
>> struct mxc_jpeg_sof *psof = NULL;
>> struct mxc_jpeg_sos *psos = NULL;
>> struct mxc_jpeg_src_buf *jpeg_src_buf = vb2_to_mxc_buf(vb);
>> - u8 *src_addr = (u8 *)vb2_plane_vaddr(vb, 0);
>> - u32 size = vb2_get_plane_payload(vb, 0);
>> + u8 *src_addr = (u8 *)mxc_jpeg_get_plane_vaddr(vb, 0);
>> + u32 size = mxc_jpeg_get_plane_payload(vb, 0);
>> int ret;
>>
>> memset(&header, 0, sizeof(header));
>>
>> base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
>> prerequisite-patch-id: 0000000000000000000000000000000000000000
Powered by blists - more mailing lists