linux-kernel - RE: [EXT] Re: [PATCH] media: amphion: return error if format is unsupported by vpu

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DB8PR04MB634682E34667F804608E3018E7D39@DB8PR04MB6346.eurprd04.prod.outlook.com>
Date:   Fri, 20 May 2022 01:25:31 +0000
From:   Ming Qian <ming.qian@....com>
To:     Nicolas Dufresne <nicolas@...fresne.ca>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "hverkuil-cisco@...all.nl" <hverkuil-cisco@...all.nl>
CC:     "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [EXT] Re: [PATCH] media: amphion: return error if format is
 unsupported by vpu

> From: Nicolas Dufresne [mailto:nicolas@...fresne.ca]
> Sent: Friday, May 20, 2022 2:13 AM
> To: Ming Qian <ming.qian@....com>; mchehab@...nel.org;
> hverkuil-cisco@...all.nl
> Cc: shawnguo@...nel.org; robh+dt@...nel.org; s.hauer@...gutronix.de;
> kernel@...gutronix.de; festevam@...il.com; dl-linux-imx
> <linux-imx@....com>; linux-media@...r.kernel.org;
> linux-kernel@...r.kernel.org; linux-arm-kernel@...ts.infradead.org
> Subject: [EXT] Re: [PATCH] media: amphion: return error if format is
> unsupported by vpu
> 
> Caution: EXT Email
> 
> Hi Ming Qian,
> 
> Le jeudi 19 mai 2022 à 15:28 +0800, Ming Qian a écrit :
> > return error if format is unsupported by vpu, otherwise the vpu will
> > be stalled at decoding
> 
> I have a reasonable doubt about this patch. I don't think such a case should be
> reachable by users. Normally, calls to S_FMT should ensure the driver format
> state is valid on both ends but modifying the relevant structures. As an
> example, for decoders, setting the CODEC (OUTPUT queue) format, may
> change the raw format (CAPTURE queue) implicitly to prevent this situation.
> Are we certain this change isn't papering around some missing format
> propagation ?
> 
> regards,
> Nicolas
> 

Hi Nicolas,
    You're right, it's not reachable currently.
    And there are some formats supported by VPU, but I didn't add support in driver, as they are not defined in kernel yet.
So if someone wants to enable them in future, and if he only adds a format into vdec_formats[] without modifying the vpu_malone part , then he can enum_fmt and set_fmt successfully, but meet vpu hang without any error message.
	I think driver should report an error in case of the new format is not implemented fully.

Ming

> >
> > Signed-off-by: Ming Qian <ming.qian@....com>
> > ---
> >  drivers/media/platform/amphion/vpu_malone.c | 2 ++
> >  drivers/media/platform/amphion/vpu_v4l2.c   | 4 ++--
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/amphion/vpu_malone.c
> > b/drivers/media/platform/amphion/vpu_malone.c
> > index f29c223eefce..0930b6ba8c42 100644
> > --- a/drivers/media/platform/amphion/vpu_malone.c
> > +++ b/drivers/media/platform/amphion/vpu_malone.c
> > @@ -610,6 +610,8 @@ static int vpu_malone_set_params(struct
> vpu_shared_addr *shared,
> >       enum vpu_malone_format malone_format;
> >
> >       malone_format =
> vpu_malone_format_remap(params->codec_format);
> > +     if (malone_format == MALONE_FMT_NULL)
> > +             return -EINVAL;
> >       iface->udata_buffer[instance].base = params->udata.base;
> >       iface->udata_buffer[instance].slot_size = params->udata.size;
> >
> > diff --git a/drivers/media/platform/amphion/vpu_v4l2.c
> > b/drivers/media/platform/amphion/vpu_v4l2.c
> > index 446f07d09d0b..89b88e063e45 100644
> > --- a/drivers/media/platform/amphion/vpu_v4l2.c
> > +++ b/drivers/media/platform/amphion/vpu_v4l2.c
> > @@ -500,10 +500,10 @@ static int vpu_vb2_start_streaming(struct
> vb2_queue *q, unsigned int count)
> >                 fmt->sizeimage[1], fmt->bytesperline[1],
> >                 fmt->sizeimage[2], fmt->bytesperline[2],
> >                 q->num_buffers);
> > -     call_void_vop(inst, start, q->type);
> > +     ret = call_vop(inst, start, q->type);
> >       vb2_clear_last_buffer_dequeued(q);
> >
> > -     return 0;
> > +     return ret;
> >  }
> >
> >  static void vpu_vb2_stop_streaming(struct vb2_queue *q)