Re: [PATCH] arm/scmi: fix base agent discover response

[Vincent Guittot]

Hi Cristian,

On Tue, 16 Nov 2021 at 19:28, Cristian Marussi <cristian.marussi@xxxxxxx> wrote:
>
> On Tue, Nov 16, 2021 at 06:32:55PM +0100, Vincent Guittot wrote:
> > According to scmi specification, the response of the discover agent request
> > is made of:
> > - int32 status
> > - uint32 agent_id
> > - uint8 name[16]
> >
> > but the current implementation doesn't take into account the agent_id field
> > and only allocates a rx buffer of SCMI_MAX_STR_SIZE length
> >
>
> Hi Vincent,
>
> > Allocate the correct length for rx buffer and copy the name from the
> > correct offset in the response.
> >
> > While no error were returned until v5.15, v5.16-rc1 fails with virtio_scmi
> > transport channel:
> >
> > [    1.093253] arm-scmi firmware:scmi0: SCMI Notifications - Core Enabled.
> > [    1.114776] arm-scmi firmware:scmi0: SCMI Protocol v2.0 'Linaro:PMWG' Firmware version 0x2090000
> > [    1.117544] scmi-virtio virtio0: tx:used len 28 is larger than in buflen 24
> >
>
> Good catch...I was (still) not testing BASE_DISCOVER_AGENT in my virtio setup
> given it's optional....
>
> Once implemented the command I could reproduce and test you fix.
>
> [    1.239629] arm-scmi firmware:scmi: SCMI Notifications - Core Enabled.
> [    1.266401] arm-scmi firmware:scmi: SCMI Protocol v2.0 'EMU-SCMI-VM:userland' Firmware version 0xdeadbeef
> [    1.280360] arm-scmi firmware:scmi: Found 3 protocol(s) 3 agent(s)
> [    1.286304] arm-scmi firmware:scmi: Agent 0: AGENT_00
> [    1.294115] arm-scmi firmware:scmi: Agent 1: AGENT_01
> [    1.301062] arm-scmi firmware:scmi: Agent 2: AGENT_02
>
> LGTM.
>
> Maybe it's worth also a Fixes...

I was not sure which commit to go back.

>
> The earlier where this was introduced seems:
>
> Fixes: b6f20ff8bd94 ("firmware: arm_scmi: add common infrastructure and support for base protocol")

I'm going to put this one to make sure that all LTS will get a fix

>
> and then it was carried on (:D) after heavily refactoring in:
>
> Fixes: 8d3581c2526f ("firmware: arm_scmi: Port base protocol to new interface")
>
> FWIW,
>
> Reviewed-by: Cristian Marussi <cristian.marussi@xxxxxxx>
> Tested-by: Cristian Marussi <cristian.marussi@xxxxxxx>

Thanks

>
> Thanks,
> Cristian
>