Re: [PATCH v5 0/8] Add SCMI Virtio & Clock atomic support

[Cristian Marussi]

On Fri, Mar 04, 2022 at 11:13:47AM -0500, Michael S. Tsirkin wrote:
> On Thu, Feb 17, 2022 at 01:12:26PM +0000, Cristian Marussi wrote:
> > Hi,
> > 
> > This small series is the tail-subset of the previous V8 series about atomic
> > support in SCMI [1], whose 8-patches head-subset has now been queued on
> > [2]; as such, it is based on [2] on top of tag scmi-updates-5.17:
> > 
> > commit 94d0cd1da14a ("firmware: arm_scmi: Add new parameter to
> > 		     mark_txdone")
> > 
> > Patch [1/8] substitute virtio-scmi ready flag and lock with a reference
> > counter to keep track of vio channels lifetime while removing the need of
> > a wide spinlocked section (that would cause issues with introduction of
> > virtio polling support)
> > 
> > Patch [2/8] adds a few helpers to handle the TX free_list and a dedicated
> > spinlock to reduce the reliance on the main one.
> > 
> > Patch [3/8] adds polling mode to SCMI VirtIO transport in order to support
> > atomic operations on such transport.
> > 
> > Patches [4,5/8] introduce a new optional SCMI binding, atomic-threshold-us,
> > to configure a platform specific time threshold used in the following
> > patches to select with a finer grain which SCMI resources should be
> > eligible for atomic operations when requested.
> > 
> > Patch [6/8] exposes new SCMI Clock protocol operations to allow an SCMI
> > user to request atomic mode on clock enable commands.
> > 
> > Patch [7/8] adds support to SCMI Clock protocol for a new clock attributes
> > field which advertises typical enable latency for a specific resource.
> > 
> > Finally patch [8/8] add support for atomic operations to the SCMI clock
> > driver; the underlying logic here is that we register with the Clock
> > framework atomic-capable clock resources if and only if the backing SCMI
> > transport is capable of atomic operations AND the specific clock resource
> > has been advertised by the SCMI platform as having:
> > 
> > 	clock_enable_latency <= atomic-threshold-us
> > 
> > The idea is to avoid costly atomic busy-waiting for resources that have
> > been advertised as 'slow' to operate upon. (i.e. a PLL vs a gating clock)
> > 
> > To ease testing the whole series can be find at [3].
> > 
> > Any feedback/testing welcome as usual.
> > 
> > Thanks,
> > Cristian
> 
> 
> SCMI specific stuff so I don't have anything to add here.
> 
> By the way, it does not look like anything regarding SCMI atomic support
> is needed in the virtio spec - is it true the interface with the device
> is unaffected?
> 

Yes SCMI atomic uses the existing VirtIO SCMI Device specification and the
existing  VirtIO common API for polling.

The only addition on the implementation side is the polling ABA problem
mitigation that I proposed as an addiional mechanism (wrap counters) in
the Virtio core with:

https://lore.kernel.org/linux-arm-kernel/20220208152520.52983-1-cristian.marussi@xxxxxxx/#r

I'll repost soon this latter series about wrap counters on top of the merged
SCMI atomic series so as to make use of this mitigation from SCMI while in
polling mode.

Thanks for the feedback,
Cristian