All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cristian Marussi <cristian.marussi@arm.com>
To: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	virtualization@lists.linux-foundation.org,
	virtio-dev@lists.oasis-open.org
Cc: sudeep.holla@arm.com, james.quinlan@broadcom.com,
	Jonathan.Cameron@Huawei.com, f.fainelli@gmail.com,
	etienne.carriere@linaro.org, vincent.guittot@linaro.org,
	souvik.chakravarty@arm.com, cristian.marussi@arm.com,
	igor.skalkin@opensynergy.com, peter.hilber@opensynergy.com,
	alex.bennee@linaro.org, jean-philippe@linaro.org,
	mikhail.golubev@opensynergy.com, anton.yakovlev@opensynergy.com,
	Vasyl.Vavrychuk@opensynergy.com,
	Andriy.Tryshnivskyy@opensynergy.com
Subject: [PATCH v4 15/16] [RFC][REWORK] firmware: arm_scmi: make virtio-scmi use delegated xfers
Date: Fri, 11 Jun 2021 17:59:36 +0100	[thread overview]
Message-ID: <20210611165937.701-16-cristian.marussi@arm.com> (raw)
In-Reply-To: <20210611165937.701-1-cristian.marussi@arm.com>

Draft changes to virtio-scmi to use new support for core delegated xfers
in an attempt to simplify the interactions between virtio-scmi transport
and the SCMI core transport layer.

TODO:
 - Polling is still not supported.
 - Probe/remove sequence still to be reviewed.
 - Concurrent or inverted reception of related responses and delayed
   responses is still not addressed.
   (it will be addressed in the SCMI core anyway most probably)

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/common.h |   5 +
 drivers/firmware/arm_scmi/msg.c    |  35 +++++
 drivers/firmware/arm_scmi/virtio.c | 212 +++++++++++++++--------------
 3 files changed, 152 insertions(+), 100 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index c143a449d278..22e5532fc698 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -428,6 +428,11 @@ void msg_fetch_response(struct scmi_msg_payld *msg, size_t len,
 void msg_fetch_notification(struct scmi_msg_payld *msg, size_t len,
 			    size_t max_len, struct scmi_xfer *xfer);
 
+void msg_fetch_raw_payload(struct scmi_msg_payld *msg, size_t msg_len,
+			   size_t max_len, struct scmi_xfer *xfer);
+void msg_fetch_raw_response(struct scmi_xfer *xfer);
+void msg_fetch_raw_notification(struct scmi_xfer *xfer);
+
 void scmi_notification_instance_data_set(const struct scmi_handle *handle,
 					 void *priv);
 void *scmi_notification_instance_data_get(const struct scmi_handle *handle);
diff --git a/drivers/firmware/arm_scmi/msg.c b/drivers/firmware/arm_scmi/msg.c
index 8a2d3303d281..3ed3ad0961ae 100644
--- a/drivers/firmware/arm_scmi/msg.c
+++ b/drivers/firmware/arm_scmi/msg.c
@@ -74,6 +74,17 @@ u32 msg_read_header(struct scmi_msg_payld *msg)
 	return le32_to_cpu(msg->msg_header);
 }
 
+void msg_fetch_raw_payload(struct scmi_msg_payld *msg, size_t msg_len,
+			   size_t max_len, struct scmi_xfer *xfer)
+{
+	xfer->rx_raw_len = min_t(size_t, max_len,
+				 msg_len >= sizeof(*msg) ?
+				 msg_len - sizeof(*msg) : 0);
+
+	/* Take a copy to the rx buffer.. */
+	memcpy(xfer->rx.buf, msg->msg_payload, xfer->rx_raw_len);
+}
+
 /**
  * msg_fetch_response() - Fetch response SCMI payload from transport SDU.
  *
@@ -94,6 +105,25 @@ void msg_fetch_response(struct scmi_msg_payld *msg, size_t len,
 	memcpy(xfer->rx.buf, &msg->msg_payload[1], xfer->rx.len);
 }
 
+void msg_fetch_raw_response(struct scmi_xfer *xfer)
+{
+	__le32 *msg_payload = xfer->rx.buf;
+
+	if (xfer->rx_raw_len < sizeof(xfer->hdr.status)) {
+		xfer->rx.len = 0;
+		return;
+	}
+
+	xfer->hdr.status = le32_to_cpu(msg_payload[0]);
+	/*
+	 * rx.len has been already pre-calculated by fetch_raw
+	 * for the whole payload including status, so shrink it
+	 */
+	xfer->rx.len = xfer->rx_raw_len - sizeof(xfer->hdr.status);
+	/* Carveout status 4-byte field */
+	memmove(xfer->rx.buf, &msg_payload[1], xfer->rx.len);
+}
+
 /**
  * msg_fetch_notification() - Fetch notification payload from transport SDU.
  *
@@ -111,3 +141,8 @@ void msg_fetch_notification(struct scmi_msg_payld *msg, size_t len,
 	/* Take a copy to the rx buffer.. */
 	memcpy(xfer->rx.buf, msg->msg_payload, xfer->rx.len);
 }
+
+void msg_fetch_raw_notification(struct scmi_xfer *xfer)
+{
+	xfer->rx.len = xfer->rx_raw_len;
+}
diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c
index 20972adf6dc7..4412bc590ca7 100644
--- a/drivers/firmware/arm_scmi/virtio.c
+++ b/drivers/firmware/arm_scmi/virtio.c
@@ -37,23 +37,24 @@
 /**
  * struct scmi_vio_channel - Transport channel information
  *
- * @lock: Protects access to all members except ready.
- * @ready_lock: Protects access to ready. If required, it must be taken before
- *              lock.
  * @vqueue: Associated virtqueue
  * @cinfo: SCMI Tx or Rx channel
  * @free_list: List of unused scmi_vio_msg, maintained for Tx channels only
  * @is_rx: Whether channel is an Rx channel
  * @ready: Whether transport user is ready to hear about channel
+ * @lock: Protects access to all members except ready.
+ * @ready_lock: Protects access to ready. If required, it must be taken before
+ *              lock.
  */
 struct scmi_vio_channel {
-	spinlock_t lock;
-	spinlock_t ready_lock;
 	struct virtqueue *vqueue;
 	struct scmi_chan_info *cinfo;
 	struct list_head free_list;
-	u8 is_rx;
-	u8 ready;
+	bool is_rx;
+	bool ready;
+	unsigned int max_msg;
+	spinlock_t lock;
+	spinlock_t ready_lock;
 };
 
 /**
@@ -100,6 +101,73 @@ static int scmi_vio_feed_vq_rx(struct scmi_vio_channel *vioch,
 	return rc;
 }
 
+static void scmi_finalize_message(struct scmi_vio_channel *vioch,
+				  struct scmi_vio_msg *msg)
+{
+	unsigned long flags;
+
+	if (vioch->is_rx) {
+		scmi_vio_feed_vq_rx(vioch, msg);
+	} else {
+		spin_lock_irqsave(&vioch->lock, flags);
+		list_add(&msg->list, &vioch->free_list);
+		spin_unlock_irqrestore(&vioch->lock, flags);
+	}
+}
+
+static void scmi_process_vqueue_input(struct scmi_vio_channel *vioch,
+				      struct scmi_vio_msg *msg)
+{
+	u32 msg_hdr;
+	int ret;
+	struct scmi_xfer *xfer = NULL;
+
+	msg_hdr = msg_read_header(msg->input);
+	/*
+	 * Acquire from the core transport layer a currently valid xfer
+	 * descriptor associated to the received msg_hdr: this could be a
+	 * previously allocated xfer for responses and delayed responses to
+	 * in-flight commands, or a freshly allocated new xfer for a just
+	 * received notification.
+	 *
+	 * In case of responses and delayed_responses the acquired xfer, at
+	 * the time scmi_transfer_acquire() succcessfully returns is guaranteed
+	 * to be still associated with a valid (not timed-out nor stale)
+	 * descriptor and proper refcounting is kept in the core along this xfer
+	 * so that should the core time out the xfer concurrently to this receive
+	 * path the xfer will be properly deallocated only once the last user is
+	 * done with it. (and this code path will terminate normally even though
+	 * all the processing related to the timed out xfer will be discarded).
+	 */
+	ret = scmi_transfer_acquire(vioch->cinfo, &msg_hdr, &xfer);
+	if (ret) {
+		dev_err(vioch->cinfo->dev,
+			"Cannot find matching xfer for hdr:0x%X\n", msg_hdr);
+		scmi_finalize_message(vioch, msg);
+		return;
+	}
+
+	dev_dbg(vioch->cinfo->dev,
+		"VQUEUE[%d] - INPUT MSG_RX_LEN:%d - HDR:0x%X  TYPE:%d  XFER_ID:%d  XFER:%px\n",
+		vioch->vqueue->index, msg->rx_len, msg_hdr, xfer->hdr.type,
+		xfer->hdr.seq, xfer);
+
+	msg_fetch_raw_payload(msg->input, msg->rx_len,
+			      scmi_virtio_desc.max_msg_size, xfer);
+
+	/* Drop processed virtio message anyway */
+	scmi_finalize_message(vioch, msg);
+
+	if (vioch->is_rx || !xfer->hdr.poll_completion)
+		scmi_rx_callback(vioch->cinfo, msg_hdr);
+	else
+		dev_warn(vioch->cinfo->dev,
+			 "Polling mode NOT supported. Dropped hdr:0X%X\n",
+			 msg_hdr);
+
+	scmi_transfer_release(vioch->cinfo, xfer);
+}
+
 static void scmi_vio_complete_cb(struct virtqueue *vqueue)
 {
 	unsigned long ready_flags;
@@ -138,15 +206,9 @@ static void scmi_vio_complete_cb(struct virtqueue *vqueue)
 
 		if (msg) {
 			msg->rx_len = length;
-
-			/*
-			 * Hold the ready_lock during the callback to avoid
-			 * races when the arm-scmi driver is unbinding while
-			 * the virtio device is not quiesced yet.
-			 */
-			scmi_rx_callback(vioch->cinfo,
-					 msg_read_header(msg->input), msg);
+			scmi_process_vqueue_input(vioch, msg);
 		}
+
 		spin_unlock_irqrestore(&vioch->ready_lock, ready_flags);
 	}
 
@@ -163,27 +225,11 @@ static vq_callback_t *scmi_vio_complete_callbacks[] = {
 	scmi_vio_complete_cb
 };
 
-static unsigned int virtio_get_max_msg(bool tx,
-				       struct scmi_chan_info *base_cinfo)
+static unsigned int virtio_get_max_msg(struct scmi_chan_info *base_cinfo)
 {
 	struct scmi_vio_channel *vioch = base_cinfo->transport_info;
-	unsigned int ret;
 
-	ret = virtqueue_get_vring_size(vioch->vqueue);
-
-	/* Tx messages need multiple descriptors. */
-	if (tx)
-		ret /= DESCRIPTORS_PER_TX_MSG;
-
-	if (ret > MSG_TOKEN_MAX) {
-		dev_info_once(
-			base_cinfo->dev,
-			"Only %ld messages can be pending simultaneously, while the %s virtqueue could hold %d\n",
-			MSG_TOKEN_MAX, tx ? "tx" : "rx", ret);
-		ret = MSG_TOKEN_MAX;
-	}
-
-	return ret;
+	return vioch->max_msg;
 }
 
 static int scmi_vio_match_any_dev(struct device *dev, const void *data)
@@ -195,13 +241,14 @@ static struct virtio_driver virtio_scmi_driver; /* Forward declaration */
 
 static int virtio_link_supplier(struct device *dev)
 {
-	struct device *vdev = driver_find_device(
-		&virtio_scmi_driver.driver, NULL, NULL, scmi_vio_match_any_dev);
+	struct device *vdev;
+
+	vdev = driver_find_device(&virtio_scmi_driver.driver,
+				  NULL, NULL, scmi_vio_match_any_dev);
 
 	if (!vdev) {
-		dev_notice_once(
-			dev,
-			"Deferring probe after not finding a bound scmi-virtio device\n");
+		dev_notice_once(dev,
+				"Deferring probe after not finding a bound scmi-virtio device\n");
 		return -EPROBE_DEFER;
 	}
 
@@ -245,12 +292,8 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	struct virtio_device *vdev;
 	struct scmi_vio_channel *vioch;
 	int index = tx ? VIRTIO_SCMI_VQ_TX : VIRTIO_SCMI_VQ_RX;
-	int max_msg;
 	int i;
 
-	if (!virtio_chan_available(dev, index))
-		return -ENODEV;
-
 	vdev = scmi_get_transport_info(dev);
 	vioch = &((struct scmi_vio_channel *)vdev->priv)[index];
 
@@ -259,9 +302,7 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	vioch->cinfo = cinfo;
 	spin_unlock_irqrestore(&vioch->lock, flags);
 
-	max_msg = virtio_get_max_msg(tx, cinfo);
-
-	for (i = 0; i < max_msg; i++) {
+	for (i = 0; i < vioch->max_msg; i++) {
 		struct scmi_vio_msg *msg;
 
 		msg = devm_kzalloc(cinfo->dev, sizeof(*msg), GFP_KERNEL);
@@ -322,13 +363,6 @@ static int virtio_send_message(struct scmi_chan_info *cinfo,
 	int rc;
 	struct scmi_vio_msg *msg;
 
-	/*
-	 * TODO: For now, we don't support polling. But it should not be
-	 * difficult to add support.
-	 */
-	if (xfer->hdr.poll_completion)
-		return -EINVAL;
-
 	spin_lock_irqsave(&vioch->lock, flags);
 
 	if (list_empty(&vioch->free_list)) {
@@ -351,6 +385,11 @@ static int virtio_send_message(struct scmi_chan_info *cinfo,
 			     "%s() failed to add to virtqueue (%d)\n", __func__,
 			     rc);
 	} else {
+		dev_dbg(vioch->cinfo->dev,
+			"VQUEUE[%d] - REQUEST - PROTO:0x%X  ID:0x%X  XFER_ID:%d  XFER:%px  RX_LEN:%zd\n",
+		 vioch->vqueue->index, xfer->hdr.protocol_id,
+		 xfer->hdr.id, xfer->hdr.seq, xfer, xfer->rx.len);
+
 		virtqueue_kick(vioch->vqueue);
 	}
 
@@ -360,36 +399,15 @@ static int virtio_send_message(struct scmi_chan_info *cinfo,
 }
 
 static void virtio_fetch_response(struct scmi_chan_info *cinfo,
-				  struct scmi_xfer *xfer, void *msg_handle)
+				  struct scmi_xfer *xfer)
 {
-	struct scmi_vio_msg *msg = msg_handle;
-	struct scmi_vio_channel *vioch = cinfo->transport_info;
-
-	if (!msg) {
-		dev_dbg_once(&vioch->vqueue->vdev->dev,
-			     "Ignoring %s() call with NULL msg_handle\n",
-			     __func__);
-		return;
-	}
-
-	msg_fetch_response(msg->input, msg->rx_len, xfer);
+	msg_fetch_raw_response(xfer);
 }
 
 static void virtio_fetch_notification(struct scmi_chan_info *cinfo,
-				      size_t max_len, struct scmi_xfer *xfer,
-				      void *msg_handle)
+				      size_t max_len, struct scmi_xfer *xfer)
 {
-	struct scmi_vio_msg *msg = msg_handle;
-	struct scmi_vio_channel *vioch = cinfo->transport_info;
-
-	if (!msg) {
-		dev_dbg_once(&vioch->vqueue->vdev->dev,
-			     "Ignoring %s() call with NULL msg_handle\n",
-			     __func__);
-		return;
-	}
-
-	msg_fetch_notification(msg->input, msg->rx_len, max_len, xfer);
+	msg_fetch_raw_notification(xfer);
 }
 
 static void dummy_clear_channel(struct scmi_chan_info *cinfo)
@@ -402,28 +420,6 @@ static bool dummy_poll_done(struct scmi_chan_info *cinfo,
 	return false;
 }
 
-static void virtio_drop_message(struct scmi_chan_info *cinfo, void *msg_handle)
-{
-	unsigned long flags;
-	struct scmi_vio_channel *vioch = cinfo->transport_info;
-	struct scmi_vio_msg *msg = msg_handle;
-
-	if (!msg) {
-		dev_dbg_once(&vioch->vqueue->vdev->dev,
-			     "Ignoring %s() call with NULL msg_handle\n",
-			     __func__);
-		return;
-	}
-
-	if (vioch->is_rx) {
-		scmi_vio_feed_vq_rx(vioch, msg);
-	} else {
-		spin_lock_irqsave(&vioch->lock, flags);
-		list_add(&msg->list, &vioch->free_list);
-		spin_unlock_irqrestore(&vioch->lock, flags);
-	}
-}
-
 static const struct scmi_transport_ops scmi_virtio_ops = {
 	.link_supplier = virtio_link_supplier,
 	.chan_available = virtio_chan_available,
@@ -435,7 +431,6 @@ static const struct scmi_transport_ops scmi_virtio_ops = {
 	.fetch_notification = virtio_fetch_notification,
 	.clear_channel = dummy_clear_channel,
 	.poll_done = dummy_poll_done,
-	.drop_message = virtio_drop_message,
 };
 
 static int scmi_vio_probe(struct virtio_device *vdev)
@@ -467,10 +462,26 @@ static int scmi_vio_probe(struct virtio_device *vdev)
 	dev_info(dev, "Found %d virtqueue(s)\n", vq_cnt);
 
 	for (i = 0; i < vq_cnt; i++) {
+		unsigned int sz;
+
 		spin_lock_init(&channels[i].lock);
 		spin_lock_init(&channels[i].ready_lock);
 		INIT_LIST_HEAD(&channels[i].free_list);
 		channels[i].vqueue = vqs[i];
+
+		sz = virtqueue_get_vring_size(channels[i].vqueue);
+		/* Tx messages need multiple descriptors. */
+		if (!channels[i].is_rx)
+			sz /= DESCRIPTORS_PER_TX_MSG;
+
+		if (sz > MSG_TOKEN_MAX) {
+			dev_info_once(dev,
+				      "%s virtqueue could hold %d messages. Only %ld allowed to be pending.\n",
+				      channels[i].is_rx ? "rx" : "tx",
+				      sz, MSG_TOKEN_MAX);
+			sz = MSG_TOKEN_MAX;
+		}
+		channels[i].max_msg = sz;
 	}
 
 	vdev->priv = channels;
@@ -520,4 +531,5 @@ const struct scmi_desc scmi_virtio_desc = {
 	.max_rx_timeout_ms = 60000, /* for non-realtime virtio devices */
 	.max_msg = 0, /* overridden by virtio_get_max_msg() */
 	.max_msg_size = VIRTIO_SCMI_MAX_MSG_SIZE,
+	.support_xfers_delegation = true,
 };
-- 
2.17.1


WARNING: multiple messages have this Message-ID (diff)
From: Cristian Marussi <cristian.marussi@arm.com>
To: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	virtualization@lists.linux-foundation.org,
	virtio-dev@lists.oasis-open.org
Cc: sudeep.holla@arm.com, james.quinlan@broadcom.com,
	Jonathan.Cameron@Huawei.com, f.fainelli@gmail.com,
	etienne.carriere@linaro.org, vincent.guittot@linaro.org,
	souvik.chakravarty@arm.com, cristian.marussi@arm.com,
	igor.skalkin@opensynergy.com, peter.hilber@opensynergy.com,
	alex.bennee@linaro.org, jean-philippe@linaro.org,
	mikhail.golubev@opensynergy.com, anton.yakovlev@opensynergy.com,
	Vasyl.Vavrychuk@opensynergy.com,
	Andriy.Tryshnivskyy@opensynergy.com
Subject: [PATCH v4 15/16] [RFC][REWORK] firmware: arm_scmi: make virtio-scmi use delegated xfers
Date: Fri, 11 Jun 2021 17:59:36 +0100	[thread overview]
Message-ID: <20210611165937.701-16-cristian.marussi@arm.com> (raw)
In-Reply-To: <20210611165937.701-1-cristian.marussi@arm.com>

Draft changes to virtio-scmi to use new support for core delegated xfers
in an attempt to simplify the interactions between virtio-scmi transport
and the SCMI core transport layer.

TODO:
 - Polling is still not supported.
 - Probe/remove sequence still to be reviewed.
 - Concurrent or inverted reception of related responses and delayed
   responses is still not addressed.
   (it will be addressed in the SCMI core anyway most probably)

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/common.h |   5 +
 drivers/firmware/arm_scmi/msg.c    |  35 +++++
 drivers/firmware/arm_scmi/virtio.c | 212 +++++++++++++++--------------
 3 files changed, 152 insertions(+), 100 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index c143a449d278..22e5532fc698 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -428,6 +428,11 @@ void msg_fetch_response(struct scmi_msg_payld *msg, size_t len,
 void msg_fetch_notification(struct scmi_msg_payld *msg, size_t len,
 			    size_t max_len, struct scmi_xfer *xfer);
 
+void msg_fetch_raw_payload(struct scmi_msg_payld *msg, size_t msg_len,
+			   size_t max_len, struct scmi_xfer *xfer);
+void msg_fetch_raw_response(struct scmi_xfer *xfer);
+void msg_fetch_raw_notification(struct scmi_xfer *xfer);
+
 void scmi_notification_instance_data_set(const struct scmi_handle *handle,
 					 void *priv);
 void *scmi_notification_instance_data_get(const struct scmi_handle *handle);
diff --git a/drivers/firmware/arm_scmi/msg.c b/drivers/firmware/arm_scmi/msg.c
index 8a2d3303d281..3ed3ad0961ae 100644
--- a/drivers/firmware/arm_scmi/msg.c
+++ b/drivers/firmware/arm_scmi/msg.c
@@ -74,6 +74,17 @@ u32 msg_read_header(struct scmi_msg_payld *msg)
 	return le32_to_cpu(msg->msg_header);
 }
 
+void msg_fetch_raw_payload(struct scmi_msg_payld *msg, size_t msg_len,
+			   size_t max_len, struct scmi_xfer *xfer)
+{
+	xfer->rx_raw_len = min_t(size_t, max_len,
+				 msg_len >= sizeof(*msg) ?
+				 msg_len - sizeof(*msg) : 0);
+
+	/* Take a copy to the rx buffer.. */
+	memcpy(xfer->rx.buf, msg->msg_payload, xfer->rx_raw_len);
+}
+
 /**
  * msg_fetch_response() - Fetch response SCMI payload from transport SDU.
  *
@@ -94,6 +105,25 @@ void msg_fetch_response(struct scmi_msg_payld *msg, size_t len,
 	memcpy(xfer->rx.buf, &msg->msg_payload[1], xfer->rx.len);
 }
 
+void msg_fetch_raw_response(struct scmi_xfer *xfer)
+{
+	__le32 *msg_payload = xfer->rx.buf;
+
+	if (xfer->rx_raw_len < sizeof(xfer->hdr.status)) {
+		xfer->rx.len = 0;
+		return;
+	}
+
+	xfer->hdr.status = le32_to_cpu(msg_payload[0]);
+	/*
+	 * rx.len has been already pre-calculated by fetch_raw
+	 * for the whole payload including status, so shrink it
+	 */
+	xfer->rx.len = xfer->rx_raw_len - sizeof(xfer->hdr.status);
+	/* Carveout status 4-byte field */
+	memmove(xfer->rx.buf, &msg_payload[1], xfer->rx.len);
+}
+
 /**
  * msg_fetch_notification() - Fetch notification payload from transport SDU.
  *
@@ -111,3 +141,8 @@ void msg_fetch_notification(struct scmi_msg_payld *msg, size_t len,
 	/* Take a copy to the rx buffer.. */
 	memcpy(xfer->rx.buf, msg->msg_payload, xfer->rx.len);
 }
+
+void msg_fetch_raw_notification(struct scmi_xfer *xfer)
+{
+	xfer->rx.len = xfer->rx_raw_len;
+}
diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c
index 20972adf6dc7..4412bc590ca7 100644
--- a/drivers/firmware/arm_scmi/virtio.c
+++ b/drivers/firmware/arm_scmi/virtio.c
@@ -37,23 +37,24 @@
 /**
  * struct scmi_vio_channel - Transport channel information
  *
- * @lock: Protects access to all members except ready.
- * @ready_lock: Protects access to ready. If required, it must be taken before
- *              lock.
  * @vqueue: Associated virtqueue
  * @cinfo: SCMI Tx or Rx channel
  * @free_list: List of unused scmi_vio_msg, maintained for Tx channels only
  * @is_rx: Whether channel is an Rx channel
  * @ready: Whether transport user is ready to hear about channel
+ * @lock: Protects access to all members except ready.
+ * @ready_lock: Protects access to ready. If required, it must be taken before
+ *              lock.
  */
 struct scmi_vio_channel {
-	spinlock_t lock;
-	spinlock_t ready_lock;
 	struct virtqueue *vqueue;
 	struct scmi_chan_info *cinfo;
 	struct list_head free_list;
-	u8 is_rx;
-	u8 ready;
+	bool is_rx;
+	bool ready;
+	unsigned int max_msg;
+	spinlock_t lock;
+	spinlock_t ready_lock;
 };
 
 /**
@@ -100,6 +101,73 @@ static int scmi_vio_feed_vq_rx(struct scmi_vio_channel *vioch,
 	return rc;
 }
 
+static void scmi_finalize_message(struct scmi_vio_channel *vioch,
+				  struct scmi_vio_msg *msg)
+{
+	unsigned long flags;
+
+	if (vioch->is_rx) {
+		scmi_vio_feed_vq_rx(vioch, msg);
+	} else {
+		spin_lock_irqsave(&vioch->lock, flags);
+		list_add(&msg->list, &vioch->free_list);
+		spin_unlock_irqrestore(&vioch->lock, flags);
+	}
+}
+
+static void scmi_process_vqueue_input(struct scmi_vio_channel *vioch,
+				      struct scmi_vio_msg *msg)
+{
+	u32 msg_hdr;
+	int ret;
+	struct scmi_xfer *xfer = NULL;
+
+	msg_hdr = msg_read_header(msg->input);
+	/*
+	 * Acquire from the core transport layer a currently valid xfer
+	 * descriptor associated to the received msg_hdr: this could be a
+	 * previously allocated xfer for responses and delayed responses to
+	 * in-flight commands, or a freshly allocated new xfer for a just
+	 * received notification.
+	 *
+	 * In case of responses and delayed_responses the acquired xfer, at
+	 * the time scmi_transfer_acquire() succcessfully returns is guaranteed
+	 * to be still associated with a valid (not timed-out nor stale)
+	 * descriptor and proper refcounting is kept in the core along this xfer
+	 * so that should the core time out the xfer concurrently to this receive
+	 * path the xfer will be properly deallocated only once the last user is
+	 * done with it. (and this code path will terminate normally even though
+	 * all the processing related to the timed out xfer will be discarded).
+	 */
+	ret = scmi_transfer_acquire(vioch->cinfo, &msg_hdr, &xfer);
+	if (ret) {
+		dev_err(vioch->cinfo->dev,
+			"Cannot find matching xfer for hdr:0x%X\n", msg_hdr);
+		scmi_finalize_message(vioch, msg);
+		return;
+	}
+
+	dev_dbg(vioch->cinfo->dev,
+		"VQUEUE[%d] - INPUT MSG_RX_LEN:%d - HDR:0x%X  TYPE:%d  XFER_ID:%d  XFER:%px\n",
+		vioch->vqueue->index, msg->rx_len, msg_hdr, xfer->hdr.type,
+		xfer->hdr.seq, xfer);
+
+	msg_fetch_raw_payload(msg->input, msg->rx_len,
+			      scmi_virtio_desc.max_msg_size, xfer);
+
+	/* Drop processed virtio message anyway */
+	scmi_finalize_message(vioch, msg);
+
+	if (vioch->is_rx || !xfer->hdr.poll_completion)
+		scmi_rx_callback(vioch->cinfo, msg_hdr);
+	else
+		dev_warn(vioch->cinfo->dev,
+			 "Polling mode NOT supported. Dropped hdr:0X%X\n",
+			 msg_hdr);
+
+	scmi_transfer_release(vioch->cinfo, xfer);
+}
+
 static void scmi_vio_complete_cb(struct virtqueue *vqueue)
 {
 	unsigned long ready_flags;
@@ -138,15 +206,9 @@ static void scmi_vio_complete_cb(struct virtqueue *vqueue)
 
 		if (msg) {
 			msg->rx_len = length;
-
-			/*
-			 * Hold the ready_lock during the callback to avoid
-			 * races when the arm-scmi driver is unbinding while
-			 * the virtio device is not quiesced yet.
-			 */
-			scmi_rx_callback(vioch->cinfo,
-					 msg_read_header(msg->input), msg);
+			scmi_process_vqueue_input(vioch, msg);
 		}
+
 		spin_unlock_irqrestore(&vioch->ready_lock, ready_flags);
 	}
 
@@ -163,27 +225,11 @@ static vq_callback_t *scmi_vio_complete_callbacks[] = {
 	scmi_vio_complete_cb
 };
 
-static unsigned int virtio_get_max_msg(bool tx,
-				       struct scmi_chan_info *base_cinfo)
+static unsigned int virtio_get_max_msg(struct scmi_chan_info *base_cinfo)
 {
 	struct scmi_vio_channel *vioch = base_cinfo->transport_info;
-	unsigned int ret;
 
-	ret = virtqueue_get_vring_size(vioch->vqueue);
-
-	/* Tx messages need multiple descriptors. */
-	if (tx)
-		ret /= DESCRIPTORS_PER_TX_MSG;
-
-	if (ret > MSG_TOKEN_MAX) {
-		dev_info_once(
-			base_cinfo->dev,
-			"Only %ld messages can be pending simultaneously, while the %s virtqueue could hold %d\n",
-			MSG_TOKEN_MAX, tx ? "tx" : "rx", ret);
-		ret = MSG_TOKEN_MAX;
-	}
-
-	return ret;
+	return vioch->max_msg;
 }
 
 static int scmi_vio_match_any_dev(struct device *dev, const void *data)
@@ -195,13 +241,14 @@ static struct virtio_driver virtio_scmi_driver; /* Forward declaration */
 
 static int virtio_link_supplier(struct device *dev)
 {
-	struct device *vdev = driver_find_device(
-		&virtio_scmi_driver.driver, NULL, NULL, scmi_vio_match_any_dev);
+	struct device *vdev;
+
+	vdev = driver_find_device(&virtio_scmi_driver.driver,
+				  NULL, NULL, scmi_vio_match_any_dev);
 
 	if (!vdev) {
-		dev_notice_once(
-			dev,
-			"Deferring probe after not finding a bound scmi-virtio device\n");
+		dev_notice_once(dev,
+				"Deferring probe after not finding a bound scmi-virtio device\n");
 		return -EPROBE_DEFER;
 	}
 
@@ -245,12 +292,8 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	struct virtio_device *vdev;
 	struct scmi_vio_channel *vioch;
 	int index = tx ? VIRTIO_SCMI_VQ_TX : VIRTIO_SCMI_VQ_RX;
-	int max_msg;
 	int i;
 
-	if (!virtio_chan_available(dev, index))
-		return -ENODEV;
-
 	vdev = scmi_get_transport_info(dev);
 	vioch = &((struct scmi_vio_channel *)vdev->priv)[index];
 
@@ -259,9 +302,7 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
 	vioch->cinfo = cinfo;
 	spin_unlock_irqrestore(&vioch->lock, flags);
 
-	max_msg = virtio_get_max_msg(tx, cinfo);
-
-	for (i = 0; i < max_msg; i++) {
+	for (i = 0; i < vioch->max_msg; i++) {
 		struct scmi_vio_msg *msg;
 
 		msg = devm_kzalloc(cinfo->dev, sizeof(*msg), GFP_KERNEL);
@@ -322,13 +363,6 @@ static int virtio_send_message(struct scmi_chan_info *cinfo,
 	int rc;
 	struct scmi_vio_msg *msg;
 
-	/*
-	 * TODO: For now, we don't support polling. But it should not be
-	 * difficult to add support.
-	 */
-	if (xfer->hdr.poll_completion)
-		return -EINVAL;
-
 	spin_lock_irqsave(&vioch->lock, flags);
 
 	if (list_empty(&vioch->free_list)) {
@@ -351,6 +385,11 @@ static int virtio_send_message(struct scmi_chan_info *cinfo,
 			     "%s() failed to add to virtqueue (%d)\n", __func__,
 			     rc);
 	} else {
+		dev_dbg(vioch->cinfo->dev,
+			"VQUEUE[%d] - REQUEST - PROTO:0x%X  ID:0x%X  XFER_ID:%d  XFER:%px  RX_LEN:%zd\n",
+		 vioch->vqueue->index, xfer->hdr.protocol_id,
+		 xfer->hdr.id, xfer->hdr.seq, xfer, xfer->rx.len);
+
 		virtqueue_kick(vioch->vqueue);
 	}
 
@@ -360,36 +399,15 @@ static int virtio_send_message(struct scmi_chan_info *cinfo,
 }
 
 static void virtio_fetch_response(struct scmi_chan_info *cinfo,
-				  struct scmi_xfer *xfer, void *msg_handle)
+				  struct scmi_xfer *xfer)
 {
-	struct scmi_vio_msg *msg = msg_handle;
-	struct scmi_vio_channel *vioch = cinfo->transport_info;
-
-	if (!msg) {
-		dev_dbg_once(&vioch->vqueue->vdev->dev,
-			     "Ignoring %s() call with NULL msg_handle\n",
-			     __func__);
-		return;
-	}
-
-	msg_fetch_response(msg->input, msg->rx_len, xfer);
+	msg_fetch_raw_response(xfer);
 }
 
 static void virtio_fetch_notification(struct scmi_chan_info *cinfo,
-				      size_t max_len, struct scmi_xfer *xfer,
-				      void *msg_handle)
+				      size_t max_len, struct scmi_xfer *xfer)
 {
-	struct scmi_vio_msg *msg = msg_handle;
-	struct scmi_vio_channel *vioch = cinfo->transport_info;
-
-	if (!msg) {
-		dev_dbg_once(&vioch->vqueue->vdev->dev,
-			     "Ignoring %s() call with NULL msg_handle\n",
-			     __func__);
-		return;
-	}
-
-	msg_fetch_notification(msg->input, msg->rx_len, max_len, xfer);
+	msg_fetch_raw_notification(xfer);
 }
 
 static void dummy_clear_channel(struct scmi_chan_info *cinfo)
@@ -402,28 +420,6 @@ static bool dummy_poll_done(struct scmi_chan_info *cinfo,
 	return false;
 }
 
-static void virtio_drop_message(struct scmi_chan_info *cinfo, void *msg_handle)
-{
-	unsigned long flags;
-	struct scmi_vio_channel *vioch = cinfo->transport_info;
-	struct scmi_vio_msg *msg = msg_handle;
-
-	if (!msg) {
-		dev_dbg_once(&vioch->vqueue->vdev->dev,
-			     "Ignoring %s() call with NULL msg_handle\n",
-			     __func__);
-		return;
-	}
-
-	if (vioch->is_rx) {
-		scmi_vio_feed_vq_rx(vioch, msg);
-	} else {
-		spin_lock_irqsave(&vioch->lock, flags);
-		list_add(&msg->list, &vioch->free_list);
-		spin_unlock_irqrestore(&vioch->lock, flags);
-	}
-}
-
 static const struct scmi_transport_ops scmi_virtio_ops = {
 	.link_supplier = virtio_link_supplier,
 	.chan_available = virtio_chan_available,
@@ -435,7 +431,6 @@ static const struct scmi_transport_ops scmi_virtio_ops = {
 	.fetch_notification = virtio_fetch_notification,
 	.clear_channel = dummy_clear_channel,
 	.poll_done = dummy_poll_done,
-	.drop_message = virtio_drop_message,
 };
 
 static int scmi_vio_probe(struct virtio_device *vdev)
@@ -467,10 +462,26 @@ static int scmi_vio_probe(struct virtio_device *vdev)
 	dev_info(dev, "Found %d virtqueue(s)\n", vq_cnt);
 
 	for (i = 0; i < vq_cnt; i++) {
+		unsigned int sz;
+
 		spin_lock_init(&channels[i].lock);
 		spin_lock_init(&channels[i].ready_lock);
 		INIT_LIST_HEAD(&channels[i].free_list);
 		channels[i].vqueue = vqs[i];
+
+		sz = virtqueue_get_vring_size(channels[i].vqueue);
+		/* Tx messages need multiple descriptors. */
+		if (!channels[i].is_rx)
+			sz /= DESCRIPTORS_PER_TX_MSG;
+
+		if (sz > MSG_TOKEN_MAX) {
+			dev_info_once(dev,
+				      "%s virtqueue could hold %d messages. Only %ld allowed to be pending.\n",
+				      channels[i].is_rx ? "rx" : "tx",
+				      sz, MSG_TOKEN_MAX);
+			sz = MSG_TOKEN_MAX;
+		}
+		channels[i].max_msg = sz;
 	}
 
 	vdev->priv = channels;
@@ -520,4 +531,5 @@ const struct scmi_desc scmi_virtio_desc = {
 	.max_rx_timeout_ms = 60000, /* for non-realtime virtio devices */
 	.max_msg = 0, /* overridden by virtio_get_max_msg() */
 	.max_msg_size = VIRTIO_SCMI_MAX_MSG_SIZE,
+	.support_xfers_delegation = true,
 };
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2021-06-11 17:01 UTC|newest]

Thread overview: 97+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-11 16:59 [PATCH v4 00/16] Introduce SCMI VirtIO transport Cristian Marussi
2021-06-11 16:59 ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 01/16] firmware: arm_scmi: Fix max pending messages boundary check Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-07-01  8:42   ` Peter Hilber
2021-07-01  8:42     ` [virtio-dev] " Peter Hilber
2021-07-01  8:42     ` Peter Hilber
2021-07-01 10:04     ` Cristian Marussi
2021-07-01 10:04       ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 02/16] firmware: arm_scmi: Add support for type handling in common functions Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 03/16] firmware: arm_scmi: Add transport optional init/exit support Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-14 13:29   ` Jonathan Cameron
2021-06-14 13:29     ` Jonathan Cameron
2021-06-16  9:04     ` Cristian Marussi
2021-06-16  9:04       ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 04/16] firmware: arm_scmi: Introduce monotonically increasing tokens Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-14 13:53   ` Jonathan Cameron
2021-06-14 13:53     ` Jonathan Cameron
2021-06-16  9:11     ` Cristian Marussi
2021-06-16  9:11       ` Cristian Marussi
2021-07-01  8:42   ` Peter Hilber
2021-07-01  8:42     ` [virtio-dev] " Peter Hilber
2021-07-01  8:42     ` Peter Hilber
2021-07-01 10:16     ` Cristian Marussi
2021-07-01 10:16       ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 05/16] firmware: arm_scmi: Introduce delegated xfers support Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-07-01  8:42   ` Peter Hilber
2021-07-01  8:42     ` [virtio-dev] " Peter Hilber
2021-07-01  8:42     ` Peter Hilber
2021-07-01 10:24     ` Cristian Marussi
2021-07-01 10:24       ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 06/16] firmware: arm_scmi, smccc, mailbox: Make shmem based transports optional Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-07-01  8:42   ` Peter Hilber
2021-07-01  8:42     ` [virtio-dev] " Peter Hilber
2021-07-01  8:42     ` Peter Hilber
2021-07-01 10:27     ` Cristian Marussi
2021-07-01 10:27       ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 07/16] firmware: arm_scmi: Add op to override max message # Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-14 14:04   ` Jonathan Cameron
2021-06-14 14:04     ` Jonathan Cameron
2021-06-16  9:13     ` Cristian Marussi
2021-06-16  9:13       ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 08/16] [RFC][REWORK] " Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 09/16] firmware: arm_scmi: Add optional link_supplier() transport op Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 10/16] firmware: arm_scmi: Add per-device transport private info Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 11/16] firmware: arm_scmi: Add is_scmi_protocol_device() Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 12/16] firmware: arm_scmi: Add message passing abstractions for transports Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-14 14:10   ` Jonathan Cameron
2021-06-14 14:10     ` Jonathan Cameron
2021-06-16  9:14     ` Cristian Marussi
2021-06-16  9:14       ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 13/16] dt-bindings: arm: Add virtio transport for SCMI Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-24 19:22   ` Rob Herring
2021-06-24 19:22     ` Rob Herring
2021-06-24 19:22     ` Rob Herring
2021-07-01  8:43   ` Peter Hilber
2021-07-01  8:43     ` [virtio-dev] " Peter Hilber
2021-07-01  8:43     ` Peter Hilber
2021-07-01 10:31     ` Cristian Marussi
2021-07-01 10:31       ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 14/16] firmware: arm_scmi: Add virtio transport Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-14 14:23   ` Jonathan Cameron
2021-06-14 14:23     ` Jonathan Cameron
2021-06-16 10:18     ` Cristian Marussi
2021-06-16 10:18       ` Cristian Marussi
2021-07-01  8:43   ` Peter Hilber
2021-07-01  8:43     ` [virtio-dev] " Peter Hilber
2021-07-01  8:43     ` Peter Hilber
2021-07-01 10:34     ` Cristian Marussi
2021-07-01 10:34       ` Cristian Marussi
2021-06-11 16:59 ` Cristian Marussi [this message]
2021-06-11 16:59   ` [PATCH v4 15/16] [RFC][REWORK] firmware: arm_scmi: make virtio-scmi use delegated xfers Cristian Marussi
2021-07-01  8:43   ` Peter Hilber
2021-07-01  8:43     ` [virtio-dev] " Peter Hilber
2021-07-01  8:43     ` Peter Hilber
2021-07-01 11:26     ` Cristian Marussi
2021-07-01 11:26       ` Cristian Marussi
2021-06-11 16:59 ` [PATCH v4 16/16] firmware: arm_scmi: Add polling mode to virtio transport Cristian Marussi
2021-06-11 16:59   ` Cristian Marussi
2021-06-14 11:43 ` [PATCH v4 00/16] Introduce SCMI VirtIO transport Christoph Hellwig
2021-06-14 11:43   ` Christoph Hellwig
2021-06-14 11:43   ` Christoph Hellwig
2021-06-14 14:03   ` Cristian Marussi
2021-06-14 14:03     ` Cristian Marussi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210611165937.701-16-cristian.marussi@arm.com \
    --to=cristian.marussi@arm.com \
    --cc=Andriy.Tryshnivskyy@opensynergy.com \
    --cc=Jonathan.Cameron@Huawei.com \
    --cc=Vasyl.Vavrychuk@opensynergy.com \
    --cc=alex.bennee@linaro.org \
    --cc=anton.yakovlev@opensynergy.com \
    --cc=etienne.carriere@linaro.org \
    --cc=f.fainelli@gmail.com \
    --cc=igor.skalkin@opensynergy.com \
    --cc=james.quinlan@broadcom.com \
    --cc=jean-philippe@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikhail.golubev@opensynergy.com \
    --cc=peter.hilber@opensynergy.com \
    --cc=souvik.chakravarty@arm.com \
    --cc=sudeep.holla@arm.com \
    --cc=vincent.guittot@linaro.org \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.