linux-kernel - Re: [PATCH] firmware: arm_scmi: add timeout in do_xfer_with

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z_zJbRH7vQ0TswGg@pluto>
Date: Mon, 14 Apr 2025 09:38:23 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: Matthew Bystrin <dev.mbstr@...il.com>
Cc: Sudeep Holla <sudeep.holla@....com>,
	Cristian Marussi <cristian.marussi@....com>,
	arm-scmi@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	Philipp Zabel <p.zabel@...gutronix.de>, Peng Fan <peng.fan@....com>
Subject: Re: [PATCH] firmware: arm_scmi: add timeout in
 do_xfer_with_response()

On Sat, Apr 12, 2025 at 01:39:45PM +0300, Matthew Bystrin wrote:
> Sudeep,
> 

Hi Matthew,

> Thanks for taking your time.
> 
> Sudeep Holla, Apr 09, 2025 at 14:12:
> > The start update should retain as soon as Platform uC acks the request.
> > And 2 notifications can be sent out for update procedure started and
> > completed. I don't see any issue there. What is the semantics you are
> > talking about ?
> 
> I'm going to refer to section 4.1.1 from the spec, where stated following about
> delayed responses,
> 
> "Messages sent to indicate completion of the work that is associated with an
> asynchronous command" 
> 
> Compared to notifications,
> 
> "These messages provide notifications of events taking place in the platform.
> Events might include changes in power state, performance state, or other
> platform status"
> 
> So before I implemented mentioned driver I had red this two and had chosen
> delayed responses, because it had seemed more appropriate. Details below.
> 
> > Even delayed response as some timeout so I would rather prefer to use
> > notifications
> 
> Hmm, I see.
> 
> > in your usecase as it is completely async.
> 
> Just to emphasize, according to the spec I don't think that delayed responses
> and events have different degree of asynchrony. The difference is in the
> initiator of 'messaging'. Events are sent by platform to indicate its' state and
> delayed responses are sent to indicate status of previously requested operation.
> 

Delayed reponses are certainly better than notification for completion
of agent initiated actions BUT this does not exclude the usage instead
of a sync-command to start the operation and a notification to signal
its completion...depends really on the case.

The classic example of a needed async-cmd is reading a sensor that takes
a long time due to its own physical nature...

AFAIU, in this case you have an async operation whose completion time is
considerably longer (so you aim to configure a specific timeout for that
specific command) BUT it is also bound to the payload itself that you
are trying to load AND/OR to other platform specific HW charactristics
(like how slow are your flashes in this HW releases...): this means that
while the sensor slowness is stable and predictable, and the timeout can
be fixed a-priori, in this case you risk to have in the future anyway to
have to refine and tune this ad-hoc custom timeout....while you'd have
none of this issue by simply waiting for a notification (ofc you could
have to set a large timeout on your side anyway while waiting for
notifs...)

...unless you plan to dynamically tune the async-cmd timeout at runtime
based on the known payload size (that means more commands to query the
soon-to-be-flahsed payload) but anyway this does NOT solve the fact that
the platform characteristics can influence the length of the operation.

Thanks,
Cristian