Hi, On Wed, Jun 02, 2021 at 10:25:36AM +0200, Peter Hilber wrote: > On 01.06.21 16:53, Vincent Guittot wrote: > > On Tue, 11 May 2021 at 02:28, Peter Hilber <peter.hilber@xxxxxxxxxxxxxxx> wrote: > >> > >> From: Igor Skalkin <igor.skalkin@xxxxxxxxxxxxxxx> > >> > >> This transport enables accessing an SCMI platform as a virtio device. > >> > >> Implement an SCMI virtio driver according to the virtio SCMI device spec > >> [1]. Virtio device id 32 has been reserved for the SCMI device [2]. > >> > >> The virtio transport has one Tx channel (virtio cmdq, A2P channel) and > >> at most one Rx channel (virtio eventq, P2A channel). > >> > >> The following feature bit defined in [1] is not implemented: > >> VIRTIO_SCMI_F_SHARED_MEMORY. > >> > >> After the preparatory patches, this implements the virtio transport as > >> paraphrased: > >> > >> Only support a single arm-scmi device (which is consistent with the SCMI > >> spec). scmi-virtio init is called from arm-scmi module init. During the > >> arm-scmi probing, link to the first probed scmi-virtio device. Defer > >> arm-scmi probing if no scmi-virtio device is bound yet. > >> > >> For simplicity, restrict the number of messages which can be pending > >> simultaneously according to the virtqueue capacity. (The virtqueue sizes > >> are negotiated with the virtio device.) > >> > >> As soon as Rx channel message buffers are allocated or have been read > >> out by the arm-scmi driver, feed them to the virtio device. > >> > >> Since some virtio devices may not have the short response time exhibited > >> by SCMI platforms using other transports, set a generous response > >> timeout. > >> > >> Limitations: > >> > >> - Polling is not supported. > >> > >> - The timeout for delayed responses has not been adjusted. > >> > >> [1] https://github.com/oasis-tcs/virtio-spec/blob/master/virtio-scmi.tex > >> [2] https://www.oasis-open.org/committees/ballot.php?id=3496 > >> > >> Signed-off-by: Igor Skalkin <igor.skalkin@xxxxxxxxxxxxxxx> > >> [ Peter: Adapted patch for submission to upstream. ] > >> Co-developed-by: Peter Hilber <peter.hilber@xxxxxxxxxxxxxxx> > >> Signed-off-by: Peter Hilber <peter.hilber@xxxxxxxxxxxxxxx> [snip] > >> + > >> +static unsigned int virtio_get_max_msg(bool tx, > >> + 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) { > > > > __scmi_xfer_info_init() returns error for info->max_msg >= > > MSG_TOKEN_MAX so it should be (ret >= MSG_TOKEN_MAX > > > > I think it would be better to use the > comparison in > __scmi_xfer_info_init(), too. That should work in my understanding. I > think that change was originally in my branch, but I lost it somehow. > I'll include this proposed fixes on my temp rework for V4. Thanks, Cristian > > > >> + 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; > > > > should be ret = MSG_TOKEN_MAX-1; > > > >> + } > >> + > >> + return ret; > >> +} > >> + > >> +static int scmi_vio_match_any_dev(struct device *dev, const void *data) > >> +{ > >> + return 1; > >> +} > >> + > >> +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); > >> + > >> + if (!vdev) { > >> + dev_notice_once( > >> + dev, > >> + "Deferring probe after not finding a bound scmi-virtio device\n"); > >> + return -EPROBE_DEFER; > >> + } > >> + > >> + /* Add device link for remove order and sysfs link. */ > >> + if (!device_link_add(dev, vdev, DL_FLAG_AUTOREMOVE_CONSUMER)) { > >> + put_device(vdev); > >> + dev_err(dev, "Adding link to supplier virtio device failed\n"); > >> + return -ECANCELED; > >> + } > >> + > >> + put_device(vdev); > >> + return scmi_set_transport_info(dev, dev_to_virtio(vdev)); > >> +} > >> + > >> +static bool virtio_chan_available(struct device *dev, int idx) > >> +{ > >> + struct virtio_device *vdev; > >> + > >> + /* scmi-virtio doesn't support per-protocol channels */ > >> + if (is_scmi_protocol_device(dev)) > >> + return false; > >> + > >> + vdev = scmi_get_transport_info(dev); > >> + if (!vdev) > >> + return false; > >> + > >> + switch (idx) { > >> + case VIRTIO_SCMI_VQ_TX: > >> + return true; > >> + case VIRTIO_SCMI_VQ_RX: > >> + return scmi_vio_have_vq_rx(vdev); > >> + default: > >> + return false; > >> + } > >> +} > >> + > >> +static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, > >> + bool tx) > >> +{ > >> + unsigned long flags; > >> + 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]; > >> + > >> + spin_lock_irqsave(&vioch->lock, flags); > >> + cinfo->transport_info = vioch; > >> + vioch->cinfo = cinfo; > >> + spin_unlock_irqrestore(&vioch->lock, flags); > >> + > >> + max_msg = virtio_get_max_msg(tx, cinfo); > >> + > >> + for (i = 0; i < max_msg; i++) { > >> + struct scmi_vio_msg *msg; > >> + > >> + msg = devm_kzalloc(cinfo->dev, sizeof(*msg), GFP_KERNEL); > >> + if (!msg) > >> + return -ENOMEM; > >> + > >> + if (tx) { > >> + msg->request = devm_kzalloc(cinfo->dev, > >> + VIRTIO_SCMI_MAX_PDU_SIZE, > >> + GFP_KERNEL); > >> + if (!msg->request) > >> + return -ENOMEM; > >> + } > >> + > >> + msg->input = devm_kzalloc(cinfo->dev, VIRTIO_SCMI_MAX_PDU_SIZE, > >> + GFP_KERNEL); > >> + if (!msg->input) > >> + return -ENOMEM; > >> + > >> + if (tx) { > >> + spin_lock_irqsave(&vioch->lock, flags); > >> + list_add_tail(&msg->list, &vioch->free_list); > >> + spin_unlock_irqrestore(&vioch->lock, flags); > >> + } else { > >> + scmi_vio_feed_vq_rx(vioch, msg); > >> + } > >> + } > >> + > >> + spin_lock_irqsave(&vioch->ready_lock, flags); > >> + vioch->ready = true; > >> + spin_unlock_irqrestore(&vioch->ready_lock, flags); > >> + > >> + return 0; > >> +} > >> + > >> +static int virtio_chan_free(int id, void *p, void *data) > >> +{ > >> + unsigned long flags; > >> + struct scmi_chan_info *cinfo = p; > >> + struct scmi_vio_channel *vioch = cinfo->transport_info; > >> + > >> + spin_lock_irqsave(&vioch->ready_lock, flags); > >> + vioch->ready = false; > >> + spin_unlock_irqrestore(&vioch->ready_lock, flags); > >> + > >> + scmi_free_channel(cinfo, data, id); > >> + return 0; > >> +} > >> + > >> +static int virtio_send_message(struct scmi_chan_info *cinfo, > >> + struct scmi_xfer *xfer) > >> +{ > >> + struct scmi_vio_channel *vioch = cinfo->transport_info; > >> + struct scatterlist sg_out; > >> + struct scatterlist sg_in; > >> + struct scatterlist *sgs[DESCRIPTORS_PER_TX_MSG] = { &sg_out, &sg_in }; > >> + unsigned long flags; > >> + 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)) { > >> + spin_unlock_irqrestore(&vioch->lock, flags); > >> + return -EBUSY; > >> + } > >> + > >> + msg = list_first_entry(&vioch->free_list, typeof(*msg), list); > >> + list_del(&msg->list); > >> + > >> + msg_tx_prepare(msg->request, xfer); > >> + > >> + sg_init_one(&sg_out, msg->request, msg_command_size(xfer)); > >> + sg_init_one(&sg_in, msg->input, msg_response_size(xfer)); > >> + > >> + rc = virtqueue_add_sgs(vioch->vqueue, sgs, 1, 1, msg, GFP_ATOMIC); > >> + if (rc) { > >> + list_add(&msg->list, &vioch->free_list); > >> + dev_err_once(vioch->cinfo->dev, > >> + "%s() failed to add to virtqueue (%d)\n", __func__, > >> + rc); > >> + } else { > >> + virtqueue_kick(vioch->vqueue); > >> + } > >> + > >> + spin_unlock_irqrestore(&vioch->lock, flags); > >> + > >> + return rc; > >> +} > >> + > >> +static void virtio_fetch_response(struct scmi_chan_info *cinfo, > >> + struct scmi_xfer *xfer, void *msg_handle) > >> +{ > >> + 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); > >> +} > >> + > >> +static void virtio_fetch_notification(struct scmi_chan_info *cinfo, > >> + size_t max_len, struct scmi_xfer *xfer, > >> + void *msg_handle) > >> +{ > >> + 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); > >> +} > >> + > >> +static void dummy_clear_channel(struct scmi_chan_info *cinfo) > >> +{ > >> +} > >> + > >> +static bool dummy_poll_done(struct scmi_chan_info *cinfo, > >> + struct scmi_xfer *xfer) > >> +{ > >> + 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, > >> + .chan_setup = virtio_chan_setup, > >> + .chan_free = virtio_chan_free, > >> + .get_max_msg = virtio_get_max_msg, > >> + .send_message = virtio_send_message, > >> + .fetch_response = virtio_fetch_response, > >> + .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) > >> +{ > >> + struct device *dev = &vdev->dev; > >> + struct scmi_vio_channel *channels; > >> + bool have_vq_rx; > >> + int vq_cnt; > >> + int i; > >> + int ret; > >> + struct virtqueue *vqs[VIRTIO_SCMI_VQ_MAX_CNT]; > >> + > >> + have_vq_rx = scmi_vio_have_vq_rx(vdev); > >> + vq_cnt = have_vq_rx ? VIRTIO_SCMI_VQ_MAX_CNT : 1; > >> + > >> + channels = devm_kcalloc(dev, vq_cnt, sizeof(*channels), GFP_KERNEL); > >> + if (!channels) > >> + return -ENOMEM; > >> + > >> + if (have_vq_rx) > >> + channels[VIRTIO_SCMI_VQ_RX].is_rx = true; > >> + > >> + ret = virtio_find_vqs(vdev, vq_cnt, vqs, scmi_vio_complete_callbacks, > >> + scmi_vio_vqueue_names, NULL); > >> + if (ret) { > >> + dev_err(dev, "Failed to get %d virtqueue(s)\n", vq_cnt); > >> + return ret; > >> + } > >> + dev_info(dev, "Found %d virtqueue(s)\n", vq_cnt); > >> + > >> + for (i = 0; i < vq_cnt; i++) { > >> + 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]; > >> + } > >> + > >> + vdev->priv = channels; > >> + > >> + return 0; > >> +} > >> + > >> +static void scmi_vio_remove(struct virtio_device *vdev) > >> +{ > >> + vdev->config->reset(vdev); > >> + vdev->config->del_vqs(vdev); > >> +} > >> + > >> +static unsigned int features[] = { > >> + VIRTIO_SCMI_F_P2A_CHANNELS, > >> +}; > >> + > >> +static const struct virtio_device_id id_table[] = { > >> + { VIRTIO_ID_SCMI, VIRTIO_DEV_ANY_ID }, > >> + { 0 } > >> +}; > >> + > >> +static struct virtio_driver virtio_scmi_driver = { > >> + .driver.name = "scmi-virtio", > >> + .driver.owner = THIS_MODULE, > >> + .feature_table = features, > >> + .feature_table_size = ARRAY_SIZE(features), > >> + .id_table = id_table, > >> + .probe = scmi_vio_probe, > >> + .remove = scmi_vio_remove, > >> +}; > >> + > >> +static int __init virtio_scmi_init(void) > >> +{ > >> + return register_virtio_driver(&virtio_scmi_driver); > >> +} > >> + > >> +static void __exit virtio_scmi_exit(void) > >> +{ > >> + unregister_virtio_driver(&virtio_scmi_driver); > >> +} > >> + > >> +const struct scmi_desc scmi_virtio_desc = { > >> + .init = virtio_scmi_init, > >> + .exit = virtio_scmi_exit, > >> + .ops = &scmi_virtio_ops, > >> + .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, > >> +}; > >> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h > >> index f0c35ce8628c..c146fe30e589 100644 > >> --- a/include/uapi/linux/virtio_ids.h > >> +++ b/include/uapi/linux/virtio_ids.h > >> @@ -56,5 +56,6 @@ > >> #define VIRTIO_ID_PMEM 27 /* virtio pmem */ > >> #define VIRTIO_ID_BT 28 /* virtio bluetooth */ > >> #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */ > >> +#define VIRTIO_ID_SCMI 32 /* virtio SCMI */ > >> > >> #endif /* _LINUX_VIRTIO_IDS_H */ > >> diff --git a/include/uapi/linux/virtio_scmi.h b/include/uapi/linux/virtio_scmi.h > >> new file mode 100644 > >> index 000000000000..732b01504c35 > >> --- /dev/null > >> +++ b/include/uapi/linux/virtio_scmi.h > >> @@ -0,0 +1,25 @@ > >> +/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */ > >> +/* > >> + * Copyright (C) 2020 OpenSynergy GmbH > >> + */ > >> + > >> +#ifndef _UAPI_LINUX_VIRTIO_SCMI_H > >> +#define _UAPI_LINUX_VIRTIO_SCMI_H > >> + > >> +#include <linux/virtio_types.h> > >> + > >> +/* Feature bits */ > >> + > >> +/* Device implements some SCMI notifications, or delayed responses. */ > >> +#define VIRTIO_SCMI_F_P2A_CHANNELS 0 > >> + > >> +/* Device implements any SCMI statistics shared memory region */ > >> +#define VIRTIO_SCMI_F_SHARED_MEMORY 1 > >> + > >> +/* Virtqueues */ > >> + > >> +#define VIRTIO_SCMI_VQ_TX 0 /* cmdq */ > >> +#define VIRTIO_SCMI_VQ_RX 1 /* eventq */ > >> +#define VIRTIO_SCMI_VQ_MAX_CNT 2 > >> + > >> +#endif /* _UAPI_LINUX_VIRTIO_SCMI_H */ > >> -- > >> 2.25.1 > >> > >> > > > >
