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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 18 Oct 2021 10:16:54 +0900 From: Alexandre Courbot <acourbot@...omium.org> To: Hans Verkuil <hverkuil@...all.nl> Cc: Dafna Hirschfeld <dafna.hirschfeld@...labora.com>, Linux Media Mailing List <linux-media@...r.kernel.org>, "moderated list:ARM/Mediatek SoC support" <linux-mediatek@...ts.infradead.org>, LKML <linux-kernel@...r.kernel.org>, kernel@...labora.com, Enric Balletbo i Serra <enric.balletbo@...labora.com>, Dafna Hirschfeld <dafna3@...il.com>, Tiffany Lin <tiffany.lin@...iatek.com>, Andrew-CT Chen <andrew-ct.chen@...iatek.com>, minghsiu.tsai@...iatek.com, houlong.wei@...iatek.com, Mauro Carvalho Chehab <mchehab@...nel.org>, Matthias Brugger <matthias.bgg@...il.com> Subject: Re: [PATCH v4] media: mtk-vpu: Ensure alignment of 8 for DTCM buffer Hi Hans! On Mon, Oct 4, 2021 at 6:37 PM Hans Verkuil <hverkuil@...all.nl> wrote: > > On 20/09/2021 19:04, Dafna Hirschfeld wrote: > > From: Alexandre Courbot <acourbot@...omium.org> > > > > When running memcpy_toio: > > memcpy_toio(send_obj->share_buf, buf, len); > > it was found that errors appear if len is not a multiple of 8: > > > > [58.350841] mtk-mdp 14001000.rdma: processing failed: -22 > > Why do errors appear? Is that due to a HW bug? Some other reason? MTK folks would be the best placed to answer this, but since the failure is reported by the firmware I'd suspect either a firmware or hardware limitation. > > > > > This patch ensures the copy of a multiple of 8 size by calling > > round_up(len, 8) when copying > > > > Fixes: e6599adfad30 ("media: mtk-vpu: avoid unaligned access to DTCM buffer.") > > Signed-off-by: Alexandre Courbot <acourbot@...omium.org> > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@...labora.com> > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@...labora.com> > > Reviewed-by: Houlong Wei <houlong.wei@...iatek.com> > > --- > > changes since v3: > > 1. multile -> multiple > > 2. add inline doc > > > > changes since v2: > > 1. do the extra copy only if len is not multiple of 8 > > > > changes since v1: > > 1. change sign-off-by tags > > 2. change values to memset > > > > drivers/media/platform/mtk-vpu/mtk_vpu.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/platform/mtk-vpu/mtk_vpu.c b/drivers/media/platform/mtk-vpu/mtk_vpu.c > > index ec290dde59cf..1df031716c8f 100644 > > --- a/drivers/media/platform/mtk-vpu/mtk_vpu.c > > +++ b/drivers/media/platform/mtk-vpu/mtk_vpu.c > > @@ -349,7 +349,20 @@ int vpu_ipi_send(struct platform_device *pdev, > > } > > } while (vpu_cfg_readl(vpu, HOST_TO_VPU)); > > > > - memcpy_toio(send_obj->share_buf, buf, len); > > + /* > > + * when copying data to the vpu hardware, the memcpy_toio operation must copy > > + * a multiple of 8. Otherwise the processing fails > > Same here: it needs to explain why the processing fails. > > > + */ > > + if (len % 8 != 0) { > > + unsigned char data[SHARE_BUF_SIZE]; > > Wouldn't it be more robust if you say: > > unsigned char data[sizeof(send_obj->share_buf)]; Definitely yes. > > I also think that the SHARE_BUF_SIZE define needs a comment stating that it must be a > multiple of 8, otherwise unexpected things can happen. > > You also noticed that the current SHARE_BUF_SIZE define is too low, but I saw > no patch correcting this. Shouldn't that be fixed as well? AFAICT the firmware expects this exact size on its end, so I don't believe it can be changed that easily. But maybe someone from MTK can prove me wrong. Cheers, Alex.
Powered by blists - more mailing lists