lkml.org 
[lkml]   [2021]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2021-11-23 13:12    [W:0.112 / U:0.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site