Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220624074549.3298-1-vincent.guittot@linaro.org> <YrWCctNSyKcqSHE9@e120937-lin>
In-Reply-To: <YrWCctNSyKcqSHE9@e120937-lin>
From:   Etienne Carriere <etienne.carriere@linaro.org>
Date:   Mon, 27 Jun 2022 12:47:22 +0200
Message-ID: <CAN5uoS_v+WBRJjNkOnDDjtavQAnyPVMHEOrYx6GPo0HJ8a2Z_w@mail.gmail.com>
Subject: Re: [PATCH] scmi/optee: fix response size warning
To:     Cristian Marussi <cristian.marussi@arm.com>
Cc:     Vincent Guittot <vincent.guittot@linaro.org>, sudeep.holla@arm.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hello Vincent,

On Fri, 24 Jun 2022 at 11:23, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> On Fri, Jun 24, 2022 at 09:45:49AM +0200, Vincent Guittot wrote:
> > Some protocols check the response size with the expected value but optee
> > shared memory doesn't return such size whereas it is available in the
> > optee output buffer.
> >
> > As an example, the base protocol compares the response size with the
> > expected result when requesting the list of protocol which triggers a
> > warning with optee shared memory:
> >
> > [    1.260306] arm-scmi firmware:scmi0: Malformed reply - real_sz:116  calc_sz:4  (loop_num_ret:4)
> >
> > Save the output buffer length and use it when fetching the answer.
> >
> > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
>
> Hi Vincent,
>
> > ---
> >
> > Tested on sudeep's for-next/scmi branch
> >
> >  drivers/firmware/arm_scmi/optee.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
> > index b503c22cfd32..8abace56b958 100644
> > --- a/drivers/firmware/arm_scmi/optee.c
> > +++ b/drivers/firmware/arm_scmi/optee.c
> > @@ -117,6 +117,7 @@ struct scmi_optee_channel {
> >       u32 channel_id;
> >       u32 tee_session;
> >       u32 caps;
> > +     u32 rx_len;
> >       struct mutex mu;
> >       struct scmi_chan_info *cinfo;
> >       union {
> > @@ -302,6 +303,9 @@ static int invoke_process_msg_channel(struct scmi_optee_channel *channel, size_t
> >               return -EIO;
> >       }
> >
> > +     /* Save response size */
> > +     channel->rx_len = param[2].u.memref.size;
> > +
> >       return 0;
> >  }
> >
> > @@ -353,6 +357,7 @@ static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *ch
> >       shbuf = tee_shm_get_va(channel->tee_shm, 0);
> >       memset(shbuf, 0, msg_size);
> >       channel->req.msg = shbuf;
> > +     channel->rx_len = msg_size;
> >
> >       return 0;
> >  }
> > @@ -508,7 +513,7 @@ static void scmi_optee_fetch_response(struct scmi_chan_info *cinfo,
> >       struct scmi_optee_channel *channel = cinfo->transport_info;
> >
> >       if (channel->tee_shm)
> > -             msg_fetch_response(channel->req.msg, SCMI_OPTEE_MAX_MSG_SIZE, xfer);
> > +             msg_fetch_response(channel->req.msg, channel->rx_len, xfer);
> >       else
> >               shmem_fetch_response(channel->req.shmem, xfer);
> >  }

Thanks for the fix.
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>


>
> Looks good to me.
>
> Reviewed-by: Cristian Marussi <cristian.marussi@arm.com>
>
> Thanks,
> Cristian
>