Messages in this thread | | | Date | Mon, 5 Aug 2019 13:52:45 +0100 | From | Sudeep Holla <> | Subject | Re: [PATCH v2 6/6] firmware: arm_scmi: Check if platform has released shmem before using |
| |
On Mon, Aug 05, 2019 at 02:33:53PM +0200, Etienne Carriere wrote: > Hello Sudeep, > > On Fri, 26 Jul 2019 at 15:46, Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > Sometimes platfom may take too long to respond to the command and OS > > might timeout before platform transfer the ownership of the shared > > memory region to the OS with the response. > > > > Since the mailbox channel associated with the channel is freed and new > > commands are dispatch on the same channel, OS needs to wait until it > > gets back the ownership. If not, either OS may end up overwriting the > > platform response for the last command(which is fine as OS timed out > > that command) or platform might overwrite the payload for the next > > command with the response for the old. > > > > The latter is problematic as platform may end up interpretting the > > response as the payload. In order to avoid such race, let's wait until > > the OS gets back the ownership before we prepare the shared memory with > > the payload for the next command. > > > > Reported-by: Jim Quinlan <james.quinlan@broadcom.com> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > --- > > drivers/firmware/arm_scmi/driver.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c > > index 69bf85fea967..765573756987 100644 > > --- a/drivers/firmware/arm_scmi/driver.c > > +++ b/drivers/firmware/arm_scmi/driver.c > > @@ -265,6 +265,14 @@ static void scmi_tx_prepare(struct mbox_client *cl, void *m) > > struct scmi_chan_info *cinfo = client_to_scmi_chan_info(cl); > > struct scmi_shared_mem __iomem *mem = cinfo->payload; > > > > + /* > > + * Ideally channel must be free by now unless OS timeout last > > + * request and platform continued to process the same, wait > > + * until it releases the shared memory, otherwise we may endup > > + * overwriting it's response with new command payload or vice-versa > > minor typo: s/it's/its/ > maybe also s/command/message/ >
Thanks for taking a look at this, both are fixed locally now.
-- Regards, Sudeep
| |