Messages in this thread Patch in this message | | | Date | Tue, 23 Nov 2021 12:11:36 +0000 | From | Cristian Marussi <> | Subject | Re: [PATCH v6 14/16] firmware: arm_scmi: Add atomic mode support to virtio transport |
| |
On Mon, Nov 22, 2021 at 11:06:38PM +0000, Cristian Marussi wrote: > Add support for .mark_txdone and .poll_done transport operations to SCMI > VirtIO transport as pre-requisites to enable atomic operations. > > Add a Kernel configuration option to enable SCMI VirtIO transport polling > and atomic mode for selected SCMI transactions while leaving it default > disabled. > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Igor Skalkin <igor.skalkin@opensynergy.com> > Cc: Peter Hilber <peter.hilber@opensynergy.com> > Cc: virtualization@lists.linux-foundation.org > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
Hi,
> --- > drivers/firmware/arm_scmi/Kconfig | 15 ++ > drivers/firmware/arm_scmi/virtio.c | 212 +++++++++++++++++++++++++++-- > 2 files changed, 214 insertions(+), 13 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig > index d429326433d1..7794bd41eaa0 100644 > --- a/drivers/firmware/arm_scmi/Kconfig > +++ b/drivers/firmware/arm_scmi/Kconfig > @@ -118,6 +118,21 @@ config ARM_SCMI_TRANSPORT_VIRTIO_VERSION1_COMPLIANCE > the ones implemented by kvmtool) and let the core Kernel VirtIO layer > take care of the needed conversions, say N. > > +config ARM_SCMI_TRANSPORT_VIRTIO_ATOMIC_ENABLE > + bool "Enable atomic mode for SCMI VirtIO transport" > + depends on ARM_SCMI_TRANSPORT_VIRTIO > + help > + Enable support of atomic operation for SCMI VirtIO based transport. > + > + If you want the SCMI VirtIO based transport to operate in atomic > + mode, avoiding any kind of sleeping behaviour for selected > + transactions on the TX path, answer Y. > + > + Enabling atomic mode operations allows any SCMI driver using this > + transport to optionally ask for atomic SCMI transactions and operate > + in atomic context too, at the price of using a number of busy-waiting > + primitives all over instead. If unsure say N. > + > endif #ARM_SCMI_PROTOCOL >
[snip]
> static void scmi_vio_complete_cb(struct virtqueue *vqueue) > @@ -138,12 +157,29 @@ static void scmi_vio_complete_cb(struct virtqueue *vqueue) > goto unlock_ready_out; > } > > +#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO_ATOMIC_ENABLE > + /* At first loop on pending_cmd_list of buffered msg if any */ > + if (!vioch->is_rx) { > + struct scmi_vio_msg *tmp; > + > + list_for_each_entry_safe(msg, tmp, > + &vioch->pending_cmds_list, > + list) { > + scmi_rx_callback(vioch->cinfo, > + msg_read_header(msg->input), > + msg); > + scmi_vio_feed_vq_tx(vioch, msg); > + } > + } > +#endif > +
doing further testing I spotted a trivial bug on the limit condition where polling and non-polling requests are interleaved on the cmd vqueue. (hard to reproduce ... but it's there and it's broken)
This one-liner fixes the issue with the above snippet:
diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c index 3101024751b0..cf247093ef48 100644 --- a/drivers/firmware/arm_scmi/virtio.c +++ b/drivers/firmware/arm_scmi/virtio.c @@ -168,6 +168,7 @@ static void scmi_vio_complete_cb(struct virtqueue *vqueue) scmi_rx_callback(vioch->cinfo, msg_read_header(msg->input), msg); + list_del(&msg->list); scmi_vio_feed_vq_tx(vioch, msg); } } It seems there will be a V7 ...
Thanks, Cristian
| |