Re: [PATCH v3 2/4] media: mali-c55: Implement CCM block validation

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hi Vincenzo

On Mon, Jun 29, 2026 at 12:08:08PM +0100, Vincenzo Frascino wrote:
> Hello Laurent,
>
> On 29/06/2026 10:57, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > On Sat, Jun 27, 2026 at 04:29:14PM +0200, Jacopo Mondi wrote:
> >> From: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> >>
> >> Implement validation of CCM block parameters.
> >>
> >> CCM coefficients are expressed as 13 bits signed Q4.8 format and their
> >> raw value cannot be higher than 8191 (BIT(13) - 1).
> >>
> >> CCM gains are expressed as unsigned 12 bits Q4.8 format and their raw
> >> value cannot be higher than 4095 (BIT(12) - 1).
> >>
> >> CCM offsets are 12 bits unsigned integers and their value cannot be
> >> higher than 4095 (BIT(12) - 1).
> >>
> >> Validate the parameters provided by userspace using the .block_validate
> >> callback of struct v4l2_isp_params_block_type_info.
> > I don't think this is needed.
> >
> > We need to validate parameters that can cause the ISP to malfunction in
> > ways that requires a system reset, or in ways that cause malfunction of
> > other system components (e.g. buffer overflows, memory bus lock ups,
> > ...). The rest doesn't need to be validated.
> >
> > If you want to be cautious, you can just mask the value when writing to
> > registers, which I think you're doing in patch 1/4.
>
> According to me here is not a matter of being cautious, but of honouring the
> contract with the userspace.
>
> If the userspace is doing something wrong it should be notified. The only
> reasonable argument against this would be if this code is on a critical path and
> the validations have a performance impact.
>
> @Jacopo, can you please confirm if this is the case?

validation happens in the qbuf ioctl handler call path (and in the
prepare_buf handler too).

I don't think it's strictly an hot path but we're in ioctl context and
I think it makes sense to minimize the time it takes to complete the
ioctl call.

Thing is, if we go down the path of validating everything, then why
would you validate Gamma LUT tables of 129 entries but not LSC tables
of 3k values ?

I feel it's hard and quite subjective to draw a line on when it's too
costly to perform validation or not, and I think the severity of the
potential issue caused by a wrong parameter is a more suitable metric
to decide what to validate ?

>
>
> --
> Regards,
> Vincenzo
>