# [PATCH v6] Add virtio SCMI device specification

Cristian Marussi cristian.marussi at arm.com
Tue Feb 16 11:23:07 EST 2021

Hi Peter

On Tue, Feb 16, 2021 at 04:11:42PM +0100, Peter Hilber wrote:
> On 15.02.21 14:20, Cristian Marussi wrote:
> > Hi Peter,
> >
> > just one remark down below.
> >
>
> Hi Cristian,
>
>
> [snip]
>
> >
> > [snip]
> >
> >> +The cmdq is used by the driver to send commands to the device. The
> >> +device replies with responses (not delayed responses) over the cmdq.
> >> +
> >> +The eventq is used by the device to send notifications and delayed
> >> +responses. The eventq only exists if VIRTIO_SCMI_F_P2A_CHANNELS was
> >> +negotiated.
> >> +
> >> +\subsection{Feature bits}\label{sec:Device Types / SCMI Device / Feature bits}
> >> +
> >> +\begin{description}
> >> +\item[VIRTIO_SCMI_F_P2A_CHANNELS (0)] Device implements some SCMI
> >> +notifications, or delayed responses.
> >> +\item[VIRTIO_SCMI_F_SHARED_MEMORY (1)] Device implements any SCMI
> >> +statistics shared memory region.
> >> +\end{description}
> >> +
> >> +VIRTIO_SCMI_F_P2A_CHANNELS is used to determine the existence of the
> >> +eventq. The eventq is required for SCMI notifications and delayed
> >> +responses.
> >> +
> >> +VIRTIO_SCMI_F_SHARED_MEMORY is used to determine whether the device
> >> +provides any SCMI statistics shared memory region. SCMI statistics
> >> +shared memory regions are defined by some SCMI protocols.
> >> +
> >> +The SCMI protocols provide the PROTOCOL_MESSAGE_ATTRIBUTES commands to
> >> +implemented by the device. The SCMI protocols provide additional
> >> +commands to detect other features implemented by the device.
> >> +
> >> +\devicenormative{\subsubsection}{Feature bits}{Device Types / SCMI Device / Feature bits}
> >> +
> >> +The device MUST offer VIRTIO_SCMI_F_P2A_CHANNELS if the device can
> >> +implement at least one SCMI notification, or delayed response.
> >> +
> >> +The device MUST offer VIRTIO_SCMI_F_SHARED_MEMORY if the device can
> >> +implement at least one SCMI statistics shared memory region.
> >> +
> >> +\subsection{Device configuration layout}\label{sec:Device Types / SCMI Device / Device configuration layout}
> >> +
> >> +There is no configuration data for the device.
> >> +
> >
> > I could be missing something here, so I apologize upfront if this has
> > already been discussed elsewhere, but AFAIU the fact that no device
> > configuration is exposed here (like a device ID) implies that SCMI devices
> > are not "identifiable" from within the VirtIO subsystem, so not
> > distinguishable in other words.
> >
> > At the same time in the companion RFC driver series it has been already
> > also ruled out (as of now at least) by the DT maintainer the possibility
> > to establish a link at the DT level between an SCMI virtio-mmio device
> > definition and the SCMI related DT descriptions (because, AFAIU, it's not
> > something needed by any of all the other existent VirtIO devices that are
> > indeed usually somehow 'identifiable')
> >
> > These 2 things combined together rule out any future possibility to have
> > multiple SCMI devices defined on a system (at least not identifiable/usable)
> > in order to support multiple distinct transport channels against the same
> > platform (or, less desirable, multiple platforms), and in fact the SCMI virtio
> > transport RFC driver as of now states upfront that only one device/once channel
> > configuration is supported.
> >
> > Even if not the norm probably, I would say that could be useful to leave
> > open for the future the possibility to redirect some protocols down through
> > a specific distinct VirtIO channel (other than the main one), at least for
> > debug/testing purposes if not to support multiple platform residing in
> > distinct VMs.
> > Another use case could be separating the transport channel used by some
> > possibly chatty protocol like Sensors (possibly flooding the system with
> > lots of notifications) from other SCMI protocols, that could benefit from
> > having a dedicated quieter virtio-mmio SCMI device channel (with dedicated
> > IRQs and friends...)
> >
> > Beside that, also the general fact that this SCMI transport does not support
> > multiple channels is a bit at odd with all the other SCMI transports
> > (like mailbox/svc that, it's to be said, are indeed based instead on
> > shared memory)
> >
> > So, what I'm saying here is if it's not the case of having some sort of
> > device_id in the configuration space, so that a platform could expose,
> > if it wants, multiple identifiable devices and the agents could pick the
> > device they want and associate with a specific protocol.
> > (via some DT transport reference to the desired SCMI device_id to use,
> >  similarly to what's possible with shared mems transports)
> >
> > But, as said, it could have been already discussed and ruled out so feel
> > free to ignore this and point me at the related pre-existing discussion
> >
>
> This has not been discussed on the mailing list so far. I think both
> features, multiple devices and dedicated transport channels/virtqueues
> (similar to the SCMI SHM transport), do make sense, but we at
> OpenSynergy do so far not have a use case which requires them. The
> multiple devices feature would be straightforward to add in my
> understanding (just add an id' device configuration field). From what
> you wrote I understand the dedicated transport channels could just mimic
> the SCMI SHM channels scheme from the Linux kernel, with optional
> per-protocol channels.

Yes, I suppose that kind of DT reference, i.e. attaching some transport-id
to a protocol node to identify the SCMI configuration device-id to use as
a channel could have more chances to be accepted since it's not a cross
reference at the DT level between SCMI and virtio...but that's more a hope
on my side than a certainty as of now :D

>
> Virtio is extensible through feature bits in a backwards-compatible way,
> so I think both features could also be introduced as extensions once
> there is a need for them. (This is actually done very often for other
> virtio devices.) I guess it would even be very straightforward to
> backport these features to older Linux kernels then. Would this option
>

Sure, mine remarks were more to understand there were some previous
discussion I missed and raise the point, but I think it makes perfectly
sense for now to leave the spec as it is given you don't have such a use
case in your config, especially if you tell me that VirtIO spec is easily
extensible (both from the technical and procedural point of view)...I was
not sure about that not being so familiar with it so I raised a flag
fearing that this could have become something set in stone as of current
spec.

Thanks for your feedback, unless Souvik has anything specific to add I'm
fine with it.

Regards

Cristian

> Best regards,
>
> Peter
>
>
> > Thanks
> >
> > Cristian
> >
> >
>

`