Re: [RFC PATCH 01/10] firmware: arm_scmi: Reset properly xfer SCMI status
From: Sudeep Holla <hidden>
Date: 2021-06-07 18:28:18
Also in:
lkml
On Mon, Jun 07, 2021 at 07:01:37PM +0100, Cristian Marussi wrote:
On Mon, Jun 07, 2021 at 06:38:09PM +0100, Sudeep Holla wrote:quoted
On Sun, Jun 06, 2021 at 11:12:23PM +0100, Cristian Marussi wrote:quoted
When an SCMI command transfer fails due to some protocol issue an SCMI error code is reported inside the SCMI message payload itself and it is then retrieved and transcribed by the specific transport layer into the xfer.hdr.status field by transport specific .fetch_response(). The core SCMI transport layer never explicitly reset xfer.hdr.status, so when an xfer is reused, if a transport misbehaved in handling such status field, we risk to see an invalid ghost error code. Reset xfer.hdr.status to SCMI_SUCCESS right before each transfer is started.Any particular reason why it can't be part of xfer_get_init which has other initialisations ? If none, please move it there.Well it was there initially then I moved it here. The reason is mostly the same as the reason for the other patch in this series that adds a reinit_completion() in this same point: the core does not forbid to reuse an xfer multiple times, once obtained with xfer_get() or xfer_get_init(), and indeed some protocols do such a thing: they implements such do_xfer looping and bails out on error.
Makes sense. But it is okay to retain xfer->transfer_id for every transfer in such a loop ?
In the way that it is implemented now in protocols poses no problem indeed because the do_xfer loop bails out on error and the xfer is put, but as soon as some protocol is implemented that violates this common practice and it just keeps on reuse an xfer after an error fo other do_xfers() this breaks...so it seemed more defensive to just reinit the completion and the status before each send.
Fair enough. But they use it to send same message I guess, may be if it gave error or something ? I would like to really know such a sequence instead of assisting that 😉. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel