From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 96CF2C433F5 for ; Tue, 14 Dec 2021 10:58:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=CgjuoIlXZ/q7sOyj2LG6l36PklO6J8ttMhHnh9qWZFE=; b=N1RbdcvxJdgs89 Beh58Ddvt79lVSZgflt5FHxWzkYTqlZc3C9nXKHRi9hFwPcvbv7dWJ1wJjo0i4G9YHSlVI0WpWVl2 1WRbX4EqmXwnqkf7VgcPpJvxqI6mE6Goabz4zAr+VXNnuSjhdlsA4xyYTUenkI9637pObp06nrL9l pDACb2F6mc1dKr4eTkqMZCj8071lCTiC/Bn1CilOeX7B5fEtKKQ0vCVqvVXmEmVNE9muENhxtxaKj qFdHBoe0qNQ4cNsJwMSG45kjFRUr5l1iaMgIH4nuSFZEQE+6NBiuMT58U+FAgt38X1xskll81eUZa Kk4V2e3FF8bgW4bjzI/w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mx5UW-00DaJU-9e; Tue, 14 Dec 2021 10:57:04 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mx5UP-00DaH7-Fy for linux-arm-kernel@lists.infradead.org; Tue, 14 Dec 2021 10:56:59 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8D3DC6D; Tue, 14 Dec 2021 02:56:56 -0800 (PST) Received: from e120937-lin (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A41823F5A1; Tue, 14 Dec 2021 02:56:54 -0800 (PST) Date: Tue, 14 Dec 2021 10:56:43 +0000 From: Cristian Marussi To: Sudeep Holla Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, james.quinlan@broadcom.com, Jonathan.Cameron@Huawei.com, f.fainelli@gmail.com, etienne.carriere@linaro.org, vincent.guittot@linaro.org, souvik.chakravarty@arm.com, "Michael S. Tsirkin" , Igor Skalkin , Peter Hilber , virtualization@lists.linux-foundation.org Subject: Re: [PATCH v7 14/16] firmware: arm_scmi: Add atomic mode support to virtio transport Message-ID: <20211214105643.GH6207@e120937-lin> References: <20211129191156.29322-1-cristian.marussi@arm.com> <20211129191156.29322-15-cristian.marussi@arm.com> <20211213113456.neztrurf3xxcraow@bogus> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211213113456.neztrurf3xxcraow@bogus> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211214_025657_670847_C1CA35BA X-CRM114-Status: GOOD ( 32.27 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Dec 13, 2021 at 11:34:56AM +0000, Sudeep Holla wrote: > On Mon, Nov 29, 2021 at 07:11:54PM +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" > > Cc: Igor Skalkin > > Cc: Peter Hilber > > Cc: virtualization@lists.linux-foundation.org > > Signed-off-by: Cristian Marussi > > --- > > V6 --> V7 > > - added a few comments about virtio polling internals > > - fixed missing list_del on pending_cmds_list processing > > - shrinked spinlocked areas in virtio_poll_done > > - added proper spinlocking to scmi_vio_complete_cb while scanning list > > of pending cmds > > --- > > drivers/firmware/arm_scmi/Kconfig | 15 ++ > > drivers/firmware/arm_scmi/virtio.c | 241 +++++++++++++++++++++++++++-- > > 2 files changed, 243 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 > > > > config ARM_SCMI_POWER_DOMAIN > > diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c > > index fd0f6f91fc0b..0598e185a786 100644 > > --- a/drivers/firmware/arm_scmi/virtio.c > > +++ b/drivers/firmware/arm_scmi/virtio.c > > @@ -38,6 +38,7 @@ > > * @vqueue: Associated virtqueue > > * @cinfo: SCMI Tx or Rx channel > > * @free_list: List of unused scmi_vio_msg, maintained for Tx channels only > > + * @pending_cmds_list: List of pre-fetched commands queueud for later processing > > * @is_rx: Whether channel is an Rx channel > > * @ready: Whether transport user is ready to hear about channel > > * @max_msg: Maximum number of pending messages for this channel. > > @@ -49,6 +50,9 @@ struct scmi_vio_channel { > > struct virtqueue *vqueue; > > struct scmi_chan_info *cinfo; > > struct list_head free_list; > > +#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO_ATOMIC_ENABLE > > + struct list_head pending_cmds_list; > > +#endif > > bool is_rx; > > bool ready; > > unsigned int max_msg; > > @@ -65,12 +69,22 @@ struct scmi_vio_channel { > > * @input: SDU used for (delayed) responses and notifications > > * @list: List which scmi_vio_msg may be part of > > * @rx_len: Input SDU size in bytes, once input has been received > > + * @poll_idx: Last used index registered for polling purposes if this message > > + * transaction reply was configured for polling. > > + * Note that virtqueue used index is an unsigned 16-bit. > > + * @poll_lock: Protect access to @poll_idx. > > */ > > struct scmi_vio_msg { > > struct scmi_msg_payld *request; > > struct scmi_msg_payld *input; > > struct list_head list; > > unsigned int rx_len; > > +#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO_ATOMIC_ENABLE > > Do we really need the #ifdefery for struct definition ? TBH I don't like > the way it is. I would avoid it as much as possible. I assume some are > added to avoid build warnings ? > > Doesn't __maybe_unused help to remove some of them like the functions > mark_txdone and poll_done. I haven't tried but thought of checking. > Ok, I'll remove ifdeffery while addressing Peter concerns. Thanks, Cristian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel