* [PATCH] arm/scmi: fix base agent discover response
@ 2021-11-16 17:32 Vincent Guittot
2021-11-16 18:27 ` Cristian Marussi
0 siblings, 1 reply; 3+ messages in thread
From: Vincent Guittot @ 2021-11-16 17:32 UTC (permalink / raw)
To: sudeep.holla, cristian.marussi, linux-arm-kernel, linux-kernel
Cc: Vincent Guittot
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
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
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
drivers/firmware/arm_scmi/base.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index de416f9e7921..f5219334fd3a 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -34,6 +34,12 @@ struct scmi_msg_resp_base_attributes {
__le16 reserved;
};
+struct scmi_msg_resp_base_discover_agent {
+ __le32 agent_id;
+ u8 name[SCMI_MAX_STR_SIZE];
+};
+
+
struct scmi_msg_base_error_notify {
__le32 event_control;
#define BASE_TP_NOTIFY_ALL BIT(0)
@@ -225,18 +231,21 @@ static int scmi_base_discover_agent_get(const struct scmi_protocol_handle *ph,
int id, char *name)
{
int ret;
+ struct scmi_msg_resp_base_discover_agent *agent_info;
struct scmi_xfer *t;
ret = ph->xops->xfer_get_init(ph, BASE_DISCOVER_AGENT,
- sizeof(__le32), SCMI_MAX_STR_SIZE, &t);
+ sizeof(__le32), sizeof(*agent_info), &t);
if (ret)
return ret;
put_unaligned_le32(id, t->tx.buf);
ret = ph->xops->do_xfer(ph, t);
- if (!ret)
- strlcpy(name, t->rx.buf, SCMI_MAX_STR_SIZE);
+ if (!ret) {
+ agent_info = t->rx.buf;
+ strlcpy(name, agent_info->name, SCMI_MAX_STR_SIZE);
+ }
ph->xops->xfer_put(ph, t);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] arm/scmi: fix base agent discover response
2021-11-16 17:32 [PATCH] arm/scmi: fix base agent discover response Vincent Guittot
@ 2021-11-16 18:27 ` Cristian Marussi
2021-11-17 8:13 ` Vincent Guittot
0 siblings, 1 reply; 3+ messages in thread
From: Cristian Marussi @ 2021-11-16 18:27 UTC (permalink / raw)
To: Vincent Guittot; +Cc: sudeep.holla, linux-arm-kernel, linux-kernel
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...
The earlier where this was introduced seems:
Fixes: b6f20ff8bd94 ("firmware: arm_scmi: add common infrastructure and support for base protocol")
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@arm.com>
Tested-by: Cristian Marussi <cristian.marussi@arm.com>
Thanks,
Cristian
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] arm/scmi: fix base agent discover response
2021-11-16 18:27 ` Cristian Marussi
@ 2021-11-17 8:13 ` Vincent Guittot
0 siblings, 0 replies; 3+ messages in thread
From: Vincent Guittot @ 2021-11-17 8:13 UTC (permalink / raw)
To: Cristian Marussi; +Cc: sudeep.holla, linux-arm-kernel, linux-kernel
Hi Cristian,
On Tue, 16 Nov 2021 at 19:28, Cristian Marussi <cristian.marussi@arm.com> 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@arm.com>
> Tested-by: Cristian Marussi <cristian.marussi@arm.com>
Thanks
>
> Thanks,
> Cristian
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-17 8:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16 17:32 [PATCH] arm/scmi: fix base agent discover response Vincent Guittot
2021-11-16 18:27 ` Cristian Marussi
2021-11-17 8:13 ` Vincent Guittot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).