From dfa25e9f0f9a41bc7dae42e1f57e7bbab10d8cc0 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Thu, 16 Sep 2021 11:33:34 +0100 Subject: firmware: arm_scmi: Review some virtio log messages Be more verbose avoiding to use _once flavour of dev_info/_err/_notice. Remove usage of __func_ to identify which vqueue is referred in some error messages and explicitly name the TX/RX vqueue. Link: https://lore.kernel.org/r/20210916103336.7243-1-cristian.marussi@arm.com Cc: "Michael S. Tsirkin" Cc: Sudeep Holla Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/virtio.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c index 87039c5c03fd..c30f82cc59ac 100644 --- a/drivers/firmware/arm_scmi/virtio.c +++ b/drivers/firmware/arm_scmi/virtio.c @@ -95,7 +95,7 @@ static int scmi_vio_feed_vq_rx(struct scmi_vio_channel *vioch, rc = virtqueue_add_inbuf(vioch->vqueue, &sg_in, 1, msg, GFP_ATOMIC); if (rc) - dev_err_once(dev, "failed to add to virtqueue (%d)\n", rc); + dev_err(dev, "failed to add to RX virtqueue (%d)\n", rc); else virtqueue_kick(vioch->vqueue); @@ -193,8 +193,8 @@ static unsigned int virtio_get_max_msg(struct scmi_chan_info *base_cinfo) static int virtio_link_supplier(struct device *dev) { if (!scmi_vdev) { - dev_notice_once(dev, - "Deferring probe after not finding a bound scmi-virtio device\n"); + dev_notice(dev, + "Deferring probe after not finding a bound scmi-virtio device\n"); return -EPROBE_DEFER; } @@ -334,9 +334,8 @@ static int virtio_send_message(struct scmi_chan_info *cinfo, 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); + dev_err(vioch->cinfo->dev, + "failed to add to TX virtqueue (%d)\n", rc); } else { virtqueue_kick(vioch->vqueue); } @@ -427,10 +426,10 @@ static int scmi_vio_probe(struct virtio_device *vdev) 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); + dev_info(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; -- cgit v1.2.3 From 5f90f189a052f6fc46048f6ce29a37b709548b81 Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Thu, 28 Oct 2021 16:00:09 +0200 Subject: firmware: arm_scmi: Add optee transport Add a new transport channel to the SCMI firmware interface driver for SCMI message exchange based on optee transport channel. The optee transport is realized by connecting and invoking OP-TEE SCMI service interface PTA. Optee transport support (CONFIG_ARM_SCMI_TRANSPORT_OPTEE) is default enabled when optee driver (CONFIG_OPTEE) is enabled. Effective optee transport is setup upon OP-TEE SCMI service discovery at optee device initialization. For this SCMI UUID is registered to the optee bus for probing. This is done from the link_supplier operator of the SCMI optee transport. The optee transport can use a statically defined shared memory in which case SCMI device tree node defines it using an "arm,scmi-shmem" compatible phandle through property shmem. Alternatively, optee transport allocates the shared memory buffer from the optee driver when no shmem property is defined. The protocol used to exchange SCMI message over that shared memory is negotiated between optee transport driver and the OP-TEE service through capabilities exchange. OP-TEE SCMI service is integrated in OP-TEE since its release tag 3.13.0. The service interface is published in [1]. Link: [1] https://github.com/OP-TEE/optee_os/blob/3.13.0/lib/libutee/include/pta_scmi_client.h Link: https://lore.kernel.org/r/20211028140009.23331-2-etienne.carriere@linaro.org Cc: Cristian Marussi Cc: Sudeep Holla Reviewed-by: Cristian Marussi Signed-off-by: Etienne Carriere Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/Kconfig | 12 + drivers/firmware/arm_scmi/Makefile | 1 + drivers/firmware/arm_scmi/common.h | 3 + drivers/firmware/arm_scmi/driver.c | 3 + drivers/firmware/arm_scmi/optee.c | 581 +++++++++++++++++++++++++++++++++++++ 5 files changed, 600 insertions(+) create mode 100644 drivers/firmware/arm_scmi/optee.c (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig index 3d7081e84853..da1daa593204 100644 --- a/drivers/firmware/arm_scmi/Kconfig +++ b/drivers/firmware/arm_scmi/Kconfig @@ -54,6 +54,18 @@ config ARM_SCMI_TRANSPORT_MAILBOX If you want the ARM SCMI PROTOCOL stack to include support for a transport based on mailboxes, answer Y. +config ARM_SCMI_TRANSPORT_OPTEE + bool "SCMI transport based on OP-TEE service" + depends on OPTEE=y || OPTEE=ARM_SCMI_PROTOCOL + select ARM_SCMI_HAVE_TRANSPORT + select ARM_SCMI_HAVE_SHMEM + default y + help + This enables the OP-TEE service based transport for SCMI. + + If you want the ARM SCMI PROTOCOL stack to include support for a + transport based on OP-TEE SCMI service, answer Y. + config ARM_SCMI_TRANSPORT_SMC bool "SCMI transport based on SMC" depends on HAVE_ARM_SMCCC_DISCOVERY diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile index 1dcf123d64ab..ef66ec8ca917 100644 --- a/drivers/firmware/arm_scmi/Makefile +++ b/drivers/firmware/arm_scmi/Makefile @@ -6,6 +6,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_MAILBOX) += mailbox.o scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o +scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \ $(scmi-transport-y) diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index dea1bfbe1052..6438b5248c24 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -421,6 +421,9 @@ extern const struct scmi_desc scmi_smc_desc; #ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO extern const struct scmi_desc scmi_virtio_desc; #endif +#ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE +extern const struct scmi_desc scmi_optee_desc; +#endif void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv); void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id); diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index b406b3f78f46..768926a77f5d 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -1994,6 +1994,9 @@ static const struct of_device_id scmi_of_match[] = { #ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX { .compatible = "arm,scmi", .data = &scmi_mailbox_desc }, #endif +#ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE + { .compatible = "linaro,scmi-optee", .data = &scmi_optee_desc }, +#endif #ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC { .compatible = "arm,scmi-smc", .data = &scmi_smc_desc}, #endif diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c new file mode 100644 index 000000000000..d9819b0197ec --- /dev/null +++ b/drivers/firmware/arm_scmi/optee.c @@ -0,0 +1,581 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019-2021 Linaro Ltd. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "common.h" + +#define SCMI_OPTEE_MAX_MSG_SIZE 128 + +enum scmi_optee_pta_cmd { + /* + * PTA_SCMI_CMD_CAPABILITIES - Get channel capabilities + * + * [out] value[0].a: Capability bit mask (enum pta_scmi_caps) + * [out] value[0].b: Extended capabilities or 0 + */ + PTA_SCMI_CMD_CAPABILITIES = 0, + + /* + * PTA_SCMI_CMD_PROCESS_SMT_CHANNEL - Process SCMI message in SMT buffer + * + * [in] value[0].a: Channel handle + * + * Shared memory used for SCMI message/response exhange is expected + * already identified and bound to channel handle in both SCMI agent + * and SCMI server (OP-TEE) parts. + * The memory uses SMT header to carry SCMI meta-data (protocol ID and + * protocol message ID). + */ + PTA_SCMI_CMD_PROCESS_SMT_CHANNEL = 1, + + /* + * PTA_SCMI_CMD_PROCESS_SMT_CHANNEL_MESSAGE - Process SMT/SCMI message + * + * [in] value[0].a: Channel handle + * [in/out] memref[1]: Message/response buffer (SMT and SCMI payload) + * + * Shared memory used for SCMI message/response is a SMT buffer + * referenced by param[1]. It shall be 128 bytes large to fit response + * payload whatever message playload size. + * The memory uses SMT header to carry SCMI meta-data (protocol ID and + * protocol message ID). + */ + PTA_SCMI_CMD_PROCESS_SMT_CHANNEL_MESSAGE = 2, + + /* + * PTA_SCMI_CMD_GET_CHANNEL - Get channel handle + * + * SCMI shm information are 0 if agent expects to use OP-TEE regular SHM + * + * [in] value[0].a: Channel identifier + * [out] value[0].a: Returned channel handle + * [in] value[0].b: Requested capabilities mask (enum pta_scmi_caps) + */ + PTA_SCMI_CMD_GET_CHANNEL = 3, +}; + +/* + * OP-TEE SCMI service capabilities bit flags (32bit) + * + * PTA_SCMI_CAPS_SMT_HEADER + * When set, OP-TEE supports command using SMT header protocol (SCMI shmem) in + * shared memory buffers to carry SCMI protocol synchronisation information. + */ +#define PTA_SCMI_CAPS_NONE 0 +#define PTA_SCMI_CAPS_SMT_HEADER BIT(0) + +/** + * struct scmi_optee_channel - Description of an OP-TEE SCMI channel + * + * @channel_id: OP-TEE channel ID used for this transport + * @tee_session: TEE session identifier + * @caps: OP-TEE SCMI channel capabilities + * @mu: Mutex protection on channel access + * @cinfo: SCMI channel information + * @shmem: Virtual base address of the shared memory + * @tee_shm: Reference to TEE shared memory or NULL if using static shmem + * @link: Reference in agent's channel list + */ +struct scmi_optee_channel { + u32 channel_id; + u32 tee_session; + u32 caps; + struct mutex mu; + struct scmi_chan_info *cinfo; + struct scmi_shared_mem __iomem *shmem; + struct tee_shm *tee_shm; + struct list_head link; +}; + +/** + * struct scmi_optee_agent - OP-TEE transport private data + * + * @dev: Device used for communication with TEE + * @tee_ctx: TEE context used for communication + * @caps: Supported channel capabilities + * @mu: Mutex for protection of @channel_list + * @channel_list: List of all created channels for the agent + */ +struct scmi_optee_agent { + struct device *dev; + struct tee_context *tee_ctx; + u32 caps; + struct mutex mu; + struct list_head channel_list; +}; + +/* There can be only 1 SCMI service in OP-TEE we connect to */ +static struct scmi_optee_agent *scmi_optee_private; + +/* Forward reference to scmi_optee transport initialization */ +static int scmi_optee_init(void); + +/* Open a session toward SCMI OP-TEE service with REE_KERNEL identity */ +static int open_session(struct scmi_optee_agent *agent, u32 *tee_session) +{ + struct device *dev = agent->dev; + struct tee_client_device *scmi_pta = to_tee_client_device(dev); + struct tee_ioctl_open_session_arg arg = { }; + int ret; + + memcpy(arg.uuid, scmi_pta->id.uuid.b, TEE_IOCTL_UUID_LEN); + arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL; + + ret = tee_client_open_session(agent->tee_ctx, &arg, NULL); + if (ret < 0 || arg.ret) { + dev_err(dev, "Can't open tee session: %d / %#x\n", ret, arg.ret); + return -EOPNOTSUPP; + } + + *tee_session = arg.session; + + return 0; +} + +static void close_session(struct scmi_optee_agent *agent, u32 tee_session) +{ + tee_client_close_session(agent->tee_ctx, tee_session); +} + +static int get_capabilities(struct scmi_optee_agent *agent) +{ + struct tee_ioctl_invoke_arg arg = { }; + struct tee_param param[1] = { }; + u32 caps; + u32 tee_session; + int ret; + + ret = open_session(agent, &tee_session); + if (ret) + return ret; + + arg.func = PTA_SCMI_CMD_CAPABILITIES; + arg.session = tee_session; + arg.num_params = 1; + + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT; + + ret = tee_client_invoke_func(agent->tee_ctx, &arg, param); + + close_session(agent, tee_session); + + if (ret < 0 || arg.ret) { + dev_err(agent->dev, "Can't get capabilities: %d / %#x\n", ret, arg.ret); + return -EOPNOTSUPP; + } + + caps = param[0].u.value.a; + + if (!(caps & PTA_SCMI_CAPS_SMT_HEADER)) { + dev_err(agent->dev, "OP-TEE SCMI PTA doesn't support SMT\n"); + return -EOPNOTSUPP; + } + + agent->caps = caps; + + return 0; +} + +static int get_channel(struct scmi_optee_channel *channel) +{ + struct device *dev = scmi_optee_private->dev; + struct tee_ioctl_invoke_arg arg = { }; + struct tee_param param[1] = { }; + unsigned int caps = PTA_SCMI_CAPS_SMT_HEADER; + int ret; + + arg.func = PTA_SCMI_CMD_GET_CHANNEL; + arg.session = channel->tee_session; + arg.num_params = 1; + + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT; + param[0].u.value.a = channel->channel_id; + param[0].u.value.b = caps; + + ret = tee_client_invoke_func(scmi_optee_private->tee_ctx, &arg, param); + + if (ret || arg.ret) { + dev_err(dev, "Can't get channel with caps %#x: %d / %#x\n", caps, ret, arg.ret); + return -EOPNOTSUPP; + } + + /* From now on use channel identifer provided by OP-TEE SCMI service */ + channel->channel_id = param[0].u.value.a; + channel->caps = caps; + + return 0; +} + +static int invoke_process_smt_channel(struct scmi_optee_channel *channel) +{ + struct tee_ioctl_invoke_arg arg = { }; + struct tee_param param[2] = { }; + int ret; + + arg.session = channel->tee_session; + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT; + param[0].u.value.a = channel->channel_id; + + if (channel->tee_shm) { + param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT; + param[1].u.memref.shm = channel->tee_shm; + param[1].u.memref.size = SCMI_OPTEE_MAX_MSG_SIZE; + arg.num_params = 2; + arg.func = PTA_SCMI_CMD_PROCESS_SMT_CHANNEL_MESSAGE; + } else { + arg.num_params = 1; + arg.func = PTA_SCMI_CMD_PROCESS_SMT_CHANNEL; + } + + ret = tee_client_invoke_func(scmi_optee_private->tee_ctx, &arg, param); + if (ret < 0 || arg.ret) { + dev_err(scmi_optee_private->dev, "Can't invoke channel %u: %d / %#x\n", + channel->channel_id, ret, arg.ret); + return -EIO; + } + + return 0; +} + +static int scmi_optee_link_supplier(struct device *dev) +{ + if (!scmi_optee_private) { + if (scmi_optee_init()) + dev_dbg(dev, "Optee bus not yet ready\n"); + + /* Wait for optee bus */ + return -EPROBE_DEFER; + } + + if (!device_link_add(dev, scmi_optee_private->dev, DL_FLAG_AUTOREMOVE_CONSUMER)) { + dev_err(dev, "Adding link to supplier optee device failed\n"); + return -ECANCELED; + } + + return 0; +} + +static bool scmi_optee_chan_available(struct device *dev, int idx) +{ + u32 channel_id; + + return !of_property_read_u32_index(dev->of_node, "linaro,optee-channel-id", + idx, &channel_id); +} + +static void scmi_optee_clear_channel(struct scmi_chan_info *cinfo) +{ + struct scmi_optee_channel *channel = cinfo->transport_info; + + shmem_clear_channel(channel->shmem); +} + +static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *channel) +{ + const size_t msg_size = SCMI_OPTEE_MAX_MSG_SIZE; + + channel->tee_shm = tee_shm_alloc_kernel_buf(scmi_optee_private->tee_ctx, msg_size); + if (IS_ERR(channel->tee_shm)) { + dev_err(channel->cinfo->dev, "shmem allocation failed\n"); + return -ENOMEM; + } + + channel->shmem = (void *)tee_shm_get_va(channel->tee_shm, 0); + memset(channel->shmem, 0, msg_size); + shmem_clear_channel(channel->shmem); + + return 0; +} + +static int setup_static_shmem(struct device *dev, struct scmi_chan_info *cinfo, + struct scmi_optee_channel *channel) +{ + struct device_node *np; + resource_size_t size; + struct resource res; + int ret; + + np = of_parse_phandle(cinfo->dev->of_node, "shmem", 0); + if (!of_device_is_compatible(np, "arm,scmi-shmem")) { + ret = -ENXIO; + goto out; + } + + ret = of_address_to_resource(np, 0, &res); + if (ret) { + dev_err(dev, "Failed to get SCMI Tx shared memory\n"); + goto out; + } + + size = resource_size(&res); + + channel->shmem = devm_ioremap(dev, res.start, size); + if (!channel->shmem) { + dev_err(dev, "Failed to ioremap SCMI Tx shared memory\n"); + ret = -EADDRNOTAVAIL; + goto out; + } + + ret = 0; + +out: + of_node_put(np); + + return ret; +} + +static int setup_shmem(struct device *dev, struct scmi_chan_info *cinfo, + struct scmi_optee_channel *channel) +{ + if (of_find_property(cinfo->dev->of_node, "shmem", NULL)) + return setup_static_shmem(dev, cinfo, channel); + else + return setup_dynamic_shmem(dev, channel); +} + +static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, bool tx) +{ + struct scmi_optee_channel *channel; + uint32_t channel_id; + int ret; + + if (!tx) + return -ENODEV; + + channel = devm_kzalloc(dev, sizeof(*channel), GFP_KERNEL); + if (!channel) + return -ENOMEM; + + ret = of_property_read_u32_index(cinfo->dev->of_node, "linaro,optee-channel-id", + 0, &channel_id); + if (ret) + return ret; + + cinfo->transport_info = channel; + channel->cinfo = cinfo; + channel->channel_id = channel_id; + mutex_init(&channel->mu); + + ret = setup_shmem(dev, cinfo, channel); + if (ret) + return ret; + + ret = open_session(scmi_optee_private, &channel->tee_session); + if (ret) + goto err_free_shm; + + ret = get_channel(channel); + if (ret) + goto err_close_sess; + + mutex_lock(&scmi_optee_private->mu); + list_add(&channel->link, &scmi_optee_private->channel_list); + mutex_unlock(&scmi_optee_private->mu); + + return 0; + +err_close_sess: + close_session(scmi_optee_private, channel->tee_session); +err_free_shm: + if (channel->tee_shm) + tee_shm_free(channel->tee_shm); + + return ret; +} + +static int scmi_optee_chan_free(int id, void *p, void *data) +{ + struct scmi_chan_info *cinfo = p; + struct scmi_optee_channel *channel = cinfo->transport_info; + + mutex_lock(&scmi_optee_private->mu); + list_del(&channel->link); + mutex_unlock(&scmi_optee_private->mu); + + close_session(scmi_optee_private, channel->tee_session); + + if (channel->tee_shm) { + tee_shm_free(channel->tee_shm); + channel->tee_shm = NULL; + } + + cinfo->transport_info = NULL; + channel->cinfo = NULL; + + scmi_free_channel(cinfo, data, id); + + return 0; +} + +static struct scmi_shared_mem *get_channel_shm(struct scmi_optee_channel *chan, + struct scmi_xfer *xfer) +{ + if (!chan) + return NULL; + + return chan->shmem; +} + + +static int scmi_optee_send_message(struct scmi_chan_info *cinfo, + struct scmi_xfer *xfer) +{ + struct scmi_optee_channel *channel = cinfo->transport_info; + struct scmi_shared_mem *shmem = get_channel_shm(channel, xfer); + int ret; + + mutex_lock(&channel->mu); + shmem_tx_prepare(shmem, xfer); + + ret = invoke_process_smt_channel(channel); + + scmi_rx_callback(cinfo, shmem_read_header(shmem), NULL); + mutex_unlock(&channel->mu); + + return ret; +} + +static void scmi_optee_fetch_response(struct scmi_chan_info *cinfo, + struct scmi_xfer *xfer) +{ + struct scmi_optee_channel *channel = cinfo->transport_info; + struct scmi_shared_mem *shmem = get_channel_shm(channel, xfer); + + shmem_fetch_response(shmem, xfer); +} + +static bool scmi_optee_poll_done(struct scmi_chan_info *cinfo, + struct scmi_xfer *xfer) +{ + struct scmi_optee_channel *channel = cinfo->transport_info; + struct scmi_shared_mem *shmem = get_channel_shm(channel, xfer); + + return shmem_poll_done(shmem, xfer); +} + +static struct scmi_transport_ops scmi_optee_ops = { + .link_supplier = scmi_optee_link_supplier, + .chan_available = scmi_optee_chan_available, + .chan_setup = scmi_optee_chan_setup, + .chan_free = scmi_optee_chan_free, + .send_message = scmi_optee_send_message, + .fetch_response = scmi_optee_fetch_response, + .clear_channel = scmi_optee_clear_channel, + .poll_done = scmi_optee_poll_done, +}; + +static int scmi_optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) +{ + return ver->impl_id == TEE_IMPL_ID_OPTEE; +} + +static int scmi_optee_service_probe(struct device *dev) +{ + struct scmi_optee_agent *agent; + struct tee_context *tee_ctx; + int ret; + + /* Only one SCMI OP-TEE device allowed */ + if (scmi_optee_private) { + dev_err(dev, "An SCMI OP-TEE device was already initialized: only one allowed\n"); + return -EBUSY; + } + + tee_ctx = tee_client_open_context(NULL, scmi_optee_ctx_match, NULL, NULL); + if (IS_ERR(tee_ctx)) + return -ENODEV; + + agent = devm_kzalloc(dev, sizeof(*agent), GFP_KERNEL); + if (!agent) { + ret = -ENOMEM; + goto err; + } + + agent->dev = dev; + agent->tee_ctx = tee_ctx; + INIT_LIST_HEAD(&agent->channel_list); + + ret = get_capabilities(agent); + if (ret) + goto err; + + /* Ensure agent resources are all visible before scmi_optee_private is */ + smp_mb(); + scmi_optee_private = agent; + + return 0; + +err: + tee_client_close_context(tee_ctx); + + return ret; +} + +static int scmi_optee_service_remove(struct device *dev) +{ + struct scmi_optee_agent *agent = scmi_optee_private; + + if (!scmi_optee_private) + return -EINVAL; + + if (!list_empty(&scmi_optee_private->channel_list)) + return -EBUSY; + + /* Ensure cleared reference is visible before resources are released */ + smp_store_mb(scmi_optee_private, NULL); + + tee_client_close_context(agent->tee_ctx); + + return 0; +} + +static const struct tee_client_device_id scmi_optee_service_id[] = { + { + UUID_INIT(0xa8cfe406, 0xd4f5, 0x4a2e, + 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99) + }, + { } +}; + +MODULE_DEVICE_TABLE(tee, scmi_optee_service_id); + +static struct tee_client_driver scmi_optee_driver = { + .id_table = scmi_optee_service_id, + .driver = { + .name = "scmi-optee", + .bus = &tee_bus_type, + .probe = scmi_optee_service_probe, + .remove = scmi_optee_service_remove, + }, +}; + +static int scmi_optee_init(void) +{ + return driver_register(&scmi_optee_driver.driver); +} + +static void scmi_optee_exit(void) +{ + if (scmi_optee_private) + driver_unregister(&scmi_optee_driver.driver); +} + +const struct scmi_desc scmi_optee_desc = { + .transport_exit = scmi_optee_exit, + .ops = &scmi_optee_ops, + .max_rx_timeout_ms = 30, + .max_msg = 20, + .max_msg_size = SCMI_OPTEE_MAX_MSG_SIZE, +}; -- cgit v1.2.3 From 530897ecdb3d69d71757c353a003e7138a791bcc Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 15 Nov 2021 10:29:10 +0000 Subject: firmware: arm_scmi: Make virtio Version_1 compliance optional Introduce a compilation option to disable strict enforcement of compliance against VirtIO Version_1 backends, so as to allow to support also Legacy VirtIO devices implementations. Link: https://lore.kernel.org/r/20211115102910.7639-1-cristian.marussi@arm.com Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/Kconfig | 15 +++++++++++++++ drivers/firmware/arm_scmi/virtio.c | 3 ++- 2 files changed, 17 insertions(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig index da1daa593204..638ecec89ff1 100644 --- a/drivers/firmware/arm_scmi/Kconfig +++ b/drivers/firmware/arm_scmi/Kconfig @@ -89,6 +89,21 @@ config ARM_SCMI_TRANSPORT_VIRTIO If you want the ARM SCMI PROTOCOL stack to include support for a transport based on VirtIO, answer Y. +config ARM_SCMI_TRANSPORT_VIRTIO_VERSION1_COMPLIANCE + bool "SCMI VirtIO transport Version 1 compliance" + depends on ARM_SCMI_TRANSPORT_VIRTIO + default y + help + This enforces strict compliance with VirtIO Version 1 specification. + + If you want the ARM SCMI VirtIO transport layer to refuse to work + with Legacy VirtIO backends and instead support only VirtIO Version 1 + devices (or above), answer Y. + + If you want instead to support also old Legacy VirtIO backends (like + the ones implemented by kvmtool) and let the core Kernel VirtIO layer + take care of the needed conversions, 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 c30f82cc59ac..fd0f6f91fc0b 100644 --- a/drivers/firmware/arm_scmi/virtio.c +++ b/drivers/firmware/arm_scmi/virtio.c @@ -459,12 +459,13 @@ static void scmi_vio_remove(struct virtio_device *vdev) static int scmi_vio_validate(struct virtio_device *vdev) { +#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO_VERSION1_COMPLIANCE if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { dev_err(&vdev->dev, "device does not comply with spec version 1.x\n"); return -EINVAL; } - +#endif return 0; } -- cgit v1.2.3 From 61bc76be367e9928c2c49fbde9783f4821446482 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Tue, 23 Nov 2021 08:36:20 +0000 Subject: firmware: arm_scmi: optee: Fix missing mutex_init() The driver allocates the mutex but not initialize it. Use mutex_init() on it to initialize it correctly. Link: https://lore.kernel.org/r/20211123083620.2366860-1-weiyongjun1@huawei.com Fixes: 5f90f189a052 ("firmware: arm_scmi: Add optee transport") Reported-by: Hulk Robot Reviewed-by: Etienne Carriere Signed-off-by: Wei Yongjun Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/optee.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c index d9819b0197ec..901737c9f5f8 100644 --- a/drivers/firmware/arm_scmi/optee.c +++ b/drivers/firmware/arm_scmi/optee.c @@ -506,6 +506,7 @@ static int scmi_optee_service_probe(struct device *dev) agent->dev = dev; agent->tee_ctx = tee_ctx; INIT_LIST_HEAD(&agent->channel_list); + mutex_init(&agent->mu); ret = get_capabilities(agent); if (ret) -- cgit v1.2.3 From afc9c1e26bc7d3145bd1112d74bbe8d0152da934 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Thu, 25 Nov 2021 15:07:30 +0000 Subject: firmware: arm_scmi: optee: Drop the support for the OPTEE shared dynamic buffer The shared memory buffer allocated by the optee driver is normal cached memory and can't be used with IOMEM APIs used in shmem_*. We currently support only IO memory for shared memory and supporting normal cached memory needs more changes and needs to be thought through properly. So for now, let us drop the support for this OPTEE shared buffer. Link: https://lore.kernel.org/r/20211125150730.188487-1-sudeep.holla@arm.com Cc: Cristian Marussi Cc: Etienne Carriere Reviewed-by: Etienne Carriere Reviewed-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/optee.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c index 901737c9f5f8..175b39bcd470 100644 --- a/drivers/firmware/arm_scmi/optee.c +++ b/drivers/firmware/arm_scmi/optee.c @@ -282,23 +282,6 @@ static void scmi_optee_clear_channel(struct scmi_chan_info *cinfo) shmem_clear_channel(channel->shmem); } -static int setup_dynamic_shmem(struct device *dev, struct scmi_optee_channel *channel) -{ - const size_t msg_size = SCMI_OPTEE_MAX_MSG_SIZE; - - channel->tee_shm = tee_shm_alloc_kernel_buf(scmi_optee_private->tee_ctx, msg_size); - if (IS_ERR(channel->tee_shm)) { - dev_err(channel->cinfo->dev, "shmem allocation failed\n"); - return -ENOMEM; - } - - channel->shmem = (void *)tee_shm_get_va(channel->tee_shm, 0); - memset(channel->shmem, 0, msg_size); - shmem_clear_channel(channel->shmem); - - return 0; -} - static int setup_static_shmem(struct device *dev, struct scmi_chan_info *cinfo, struct scmi_optee_channel *channel) { @@ -342,7 +325,7 @@ static int setup_shmem(struct device *dev, struct scmi_chan_info *cinfo, if (of_find_property(cinfo->dev->of_node, "shmem", NULL)) return setup_static_shmem(dev, cinfo, channel); else - return setup_dynamic_shmem(dev, channel); + return -ENOMEM; } static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, bool tx) -- cgit v1.2.3 From d211ddeb511af5998dbd3e555be0fbe6033459d9 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 29 Nov 2021 19:11:41 +0000 Subject: firmware: arm_scmi: Perform earlier cinfo lookup call in do_xfer Lookup cinfo data early in do_xfer so as to avoid any further init work on xfer structure in case of error. No functional change. Link: https://lore.kernel.org/r/20211129191156.29322-2-cristian.marussi@arm.com Reviewed-by: Florian Fainelli Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 768926a77f5d..3cf161f3bcc7 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -766,6 +766,10 @@ static int do_xfer(const struct scmi_protocol_handle *ph, return -EINVAL; } + cinfo = idr_find(&info->tx_idr, pi->proto->id); + if (unlikely(!cinfo)) + return -EINVAL; + /* * Initialise protocol id now from protocol handle to avoid it being * overridden by mistake (or malice) by the protocol code mangling with @@ -774,10 +778,6 @@ static int do_xfer(const struct scmi_protocol_handle *ph, xfer->hdr.protocol_id = pi->proto->id; reinit_completion(&xfer->done); - cinfo = idr_find(&info->tx_idr, xfer->hdr.protocol_id); - if (unlikely(!cinfo)) - return -EINVAL; - trace_scmi_xfer_begin(xfer->transfer_id, xfer->hdr.id, xfer->hdr.protocol_id, xfer->hdr.seq, xfer->hdr.poll_completion); -- cgit v1.2.3 From 582730b9cbcc534a39beaf3aa9078e2c431ff39f Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 29 Nov 2021 19:11:42 +0000 Subject: firmware: arm_scmi: Set polling timeout to max_rx_timeout_ms Use transport specific transmission timeout (max_rx_timeout_ms) also for polling transactions. Initially when polling mode was added, it was intended to be used only in scheduler context and hence the choice of 100us for the polling timeout. However the only user for that was dropped for other SCMI concurrency issues, so it shouldn't cause any issue to increase this timeout value now. Link: https://lore.kernel.org/r/20211129191156.29322-3-cristian.marussi@arm.com Reviewed-by: Florian Fainelli Signed-off-by: Cristian Marussi [sudeep.holla: Updated commit message with historical facts about 100us timeout] Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 3cf161f3bcc7..568562121f64 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -724,8 +724,6 @@ static void xfer_put(const struct scmi_protocol_handle *ph, __scmi_xfer_put(&info->tx_minfo, xfer); } -#define SCMI_MAX_POLL_TO_NS (100 * NSEC_PER_USEC) - static bool scmi_xfer_done_no_timeout(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer, ktime_t stop) { @@ -799,7 +797,8 @@ static int do_xfer(const struct scmi_protocol_handle *ph, } if (xfer->hdr.poll_completion) { - ktime_t stop = ktime_add_ns(ktime_get(), SCMI_MAX_POLL_TO_NS); + ktime_t stop = ktime_add_ms(ktime_get(), + info->desc->max_rx_timeout_ms); spin_until_cond(scmi_xfer_done_no_timeout(cinfo, xfer, stop)); if (ktime_before(ktime_get(), stop)) { -- cgit v1.2.3 From 5a731aebd31bf840a93deae12bdfd831513e7211 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 29 Nov 2021 19:11:43 +0000 Subject: firmware: arm_scmi: Refactor message response path Refactor code path waiting for message responses into a dedicated helper function. No functional change. Link: https://lore.kernel.org/r/20211129191156.29322-4-cristian.marussi@arm.com Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 88 ++++++++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 32 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 568562121f64..9a8d6bfd4ebb 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -738,6 +738,61 @@ static bool scmi_xfer_done_no_timeout(struct scmi_chan_info *cinfo, ktime_after(ktime_get(), stop); } +/** + * scmi_wait_for_message_response - An helper to group all the possible ways of + * waiting for a synchronous message response. + * + * @cinfo: SCMI channel info + * @xfer: Reference to the transfer being waited for. + * + * Chooses waiting strategy (sleep-waiting vs busy-waiting) depending on + * configuration flags like xfer->hdr.poll_completion. + * + * Return: 0 on Success, error otherwise. + */ +static int scmi_wait_for_message_response(struct scmi_chan_info *cinfo, + struct scmi_xfer *xfer) +{ + struct scmi_info *info = handle_to_scmi_info(cinfo->handle); + struct device *dev = info->dev; + int ret = 0, timeout_ms = info->desc->max_rx_timeout_ms; + + if (xfer->hdr.poll_completion) { + ktime_t stop = ktime_add_ms(ktime_get(), timeout_ms); + + spin_until_cond(scmi_xfer_done_no_timeout(cinfo, xfer, stop)); + if (ktime_before(ktime_get(), stop)) { + unsigned long flags; + + /* + * Do not fetch_response if an out-of-order delayed + * response is being processed. + */ + spin_lock_irqsave(&xfer->lock, flags); + if (xfer->state == SCMI_XFER_SENT_OK) { + info->desc->ops->fetch_response(cinfo, xfer); + xfer->state = SCMI_XFER_RESP_OK; + } + spin_unlock_irqrestore(&xfer->lock, flags); + } else { + dev_err(dev, + "timed out in resp(caller: %pS) - polling\n", + (void *)_RET_IP_); + ret = -ETIMEDOUT; + } + } else { + /* And we wait for the response. */ + if (!wait_for_completion_timeout(&xfer->done, + msecs_to_jiffies(timeout_ms))) { + dev_err(dev, "timed out in resp(caller: %pS)\n", + (void *)_RET_IP_); + ret = -ETIMEDOUT; + } + } + + return ret; +} + /** * do_xfer() - Do one transfer * @@ -752,7 +807,6 @@ static int do_xfer(const struct scmi_protocol_handle *ph, struct scmi_xfer *xfer) { int ret; - int timeout; const struct scmi_protocol_instance *pi = ph_to_pi(ph); struct scmi_info *info = handle_to_scmi_info(pi->handle); struct device *dev = info->dev; @@ -796,37 +850,7 @@ static int do_xfer(const struct scmi_protocol_handle *ph, return ret; } - if (xfer->hdr.poll_completion) { - ktime_t stop = ktime_add_ms(ktime_get(), - info->desc->max_rx_timeout_ms); - - spin_until_cond(scmi_xfer_done_no_timeout(cinfo, xfer, stop)); - if (ktime_before(ktime_get(), stop)) { - unsigned long flags; - - /* - * Do not fetch_response if an out-of-order delayed - * response is being processed. - */ - spin_lock_irqsave(&xfer->lock, flags); - if (xfer->state == SCMI_XFER_SENT_OK) { - info->desc->ops->fetch_response(cinfo, xfer); - xfer->state = SCMI_XFER_RESP_OK; - } - spin_unlock_irqrestore(&xfer->lock, flags); - } else { - ret = -ETIMEDOUT; - } - } else { - /* And we wait for the response. */ - timeout = msecs_to_jiffies(info->desc->max_rx_timeout_ms); - if (!wait_for_completion_timeout(&xfer->done, timeout)) { - dev_err(dev, "timed out in resp(caller: %pS)\n", - (void *)_RET_IP_); - ret = -ETIMEDOUT; - } - } - + ret = scmi_wait_for_message_response(cinfo, xfer); if (!ret && xfer->hdr.status) ret = scmi_to_linux_errno(xfer->hdr.status); -- cgit v1.2.3 From f872af09094c042ac46e64d030f223b63ead5967 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 29 Nov 2021 19:11:45 +0000 Subject: firmware: arm_scmi: Use new trace event scmi_xfer_response_wait Use new trace event to mark start of waiting for response section. Link: https://lore.kernel.org/r/20211129191156.29322-6-cristian.marussi@arm.com Reviewed-by: Florian Fainelli Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 9a8d6bfd4ebb..476b91845e40 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -757,6 +757,11 @@ static int scmi_wait_for_message_response(struct scmi_chan_info *cinfo, struct device *dev = info->dev; int ret = 0, timeout_ms = info->desc->max_rx_timeout_ms; + trace_scmi_xfer_response_wait(xfer->transfer_id, xfer->hdr.id, + xfer->hdr.protocol_id, xfer->hdr.seq, + timeout_ms, + xfer->hdr.poll_completion); + if (xfer->hdr.poll_completion) { ktime_t stop = ktime_add_ms(ktime_get(), timeout_ms); -- cgit v1.2.3 From a690b7e6e774b7c43fed37d5bd3b6e037f3b3db9 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 20 Dec 2021 19:56:36 +0000 Subject: firmware: arm_scmi: Add configurable polling mode for transports SCMI communications along TX channels can optionally be provided of a completion interrupt; when such interrupt is not available, command transactions should rely on polling, where the SCMI core takes care to repeatedly evaluate the transport-specific .poll_done() function, if available, to determine if and when a request was fully completed or timed out. Such mechanism is already present and working on a single transfer base: SCMI protocols can indeed enable hdr.poll_completion on specific commands ahead of each transfer and cause that transaction to be handled with polling. Introduce a couple of flags to be able to enforce such polling behaviour globally at will: - scmi_desc.force_polling: to statically switch the whole transport to polling mode. - scmi_chan_info.no_completion_irq: to switch a single channel dynamically to polling mode if, at runtime, is determined that no completion interrupt was available for such channel. Link: https://lore.kernel.org/r/20211220195646.44498-2-cristian.marussi@arm.com Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/common.h | 8 ++++++++ drivers/firmware/arm_scmi/driver.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 6438b5248c24..652e5d95ee65 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -339,11 +339,16 @@ void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id); * @dev: Reference to device in the SCMI hierarchy corresponding to this * channel * @handle: Pointer to SCMI entity handle + * @no_completion_irq: Flag to indicate that this channel has no completion + * interrupt mechanism for synchronous commands. + * This can be dynamically set by transports at run-time + * inside their provided .chan_setup(). * @transport_info: Transport layer related information */ struct scmi_chan_info { struct device *dev; struct scmi_handle *handle; + bool no_completion_irq; void *transport_info; }; @@ -402,6 +407,8 @@ struct scmi_device *scmi_child_dev_find(struct device *parent, * be pending simultaneously in the system. May be overridden by the * get_max_msg op. * @max_msg_size: Maximum size of data per message that can be handled. + * @force_polling: Flag to force this whole transport to use SCMI core polling + * mechanism instead of completion interrupts even if available. */ struct scmi_desc { int (*transport_init)(void); @@ -410,6 +417,7 @@ struct scmi_desc { int max_rx_timeout_ms; int max_msg; int max_msg_size; + const bool force_polling; }; #ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 476b91845e40..7579f54b0047 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -609,6 +609,24 @@ static inline void scmi_clear_channel(struct scmi_info *info, info->desc->ops->clear_channel(cinfo); } +static inline bool is_polling_required(struct scmi_chan_info *cinfo, + struct scmi_info *info) +{ + return cinfo->no_completion_irq || info->desc->force_polling; +} + +static inline bool is_transport_polling_capable(struct scmi_info *info) +{ + return info->desc->ops->poll_done; +} + +static inline bool is_polling_enabled(struct scmi_chan_info *cinfo, + struct scmi_info *info) +{ + return is_polling_required(cinfo, info) && + is_transport_polling_capable(info); +} + static void scmi_handle_notification(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv) { @@ -817,6 +835,7 @@ static int do_xfer(const struct scmi_protocol_handle *ph, struct device *dev = info->dev; struct scmi_chan_info *cinfo; + /* Check for polling request on custom command xfers at first */ if (xfer->hdr.poll_completion && !info->desc->ops->poll_done) { dev_warn_once(dev, "Polling mode is not supported by transport.\n"); @@ -827,6 +846,10 @@ static int do_xfer(const struct scmi_protocol_handle *ph, if (unlikely(!cinfo)) return -EINVAL; + /* True ONLY if also supported by transport. */ + if (is_polling_enabled(cinfo, info)) + xfer->hdr.poll_completion = true; + /* * Initialise protocol id now from protocol handle to avoid it being * overridden by mistake (or malice) by the protocol code mangling with @@ -1527,6 +1550,16 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev, if (ret) return ret; + if (tx && is_polling_required(cinfo, info)) { + if (is_transport_polling_capable(info)) + dev_info(dev, + "Enabled polling mode TX channel - prot_id:%d\n", + prot_id); + else + dev_warn(dev, + "Polling mode NOT supported by transport.\n"); + } + idr_alloc: ret = idr_alloc(idr, cinfo, prot_id, prot_id + 1, GFP_KERNEL); if (ret != prot_id) { -- cgit v1.2.3 From f716cbd33f038af87824c30e165b3b70e4c6be1e Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 20 Dec 2021 19:56:37 +0000 Subject: firmware: arm_scmi: Make smc transport use common completions When a completion irq is available use it and delegate command completion handling to the core SCMI completion mechanism. If no completion irq is available revert to polling, using the core common polling machinery. Link: https://lore.kernel.org/r/20211220195646.44498-3-cristian.marussi@arm.com Reviewed-by: Florian Fainelli Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/smc.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c index 4effecc3bb46..d6c6ad9f6bab 100644 --- a/drivers/firmware/arm_scmi/smc.c +++ b/drivers/firmware/arm_scmi/smc.c @@ -25,8 +25,6 @@ * @shmem: Transmit/Receive shared memory area * @shmem_lock: Lock to protect access to Tx/Rx shared memory area * @func_id: smc/hvc call function id - * @irq: Optional; employed when platforms indicates msg completion by intr. - * @tx_complete: Optional, employed only when irq is valid. */ struct scmi_smc { @@ -34,15 +32,14 @@ struct scmi_smc { struct scmi_shared_mem __iomem *shmem; struct mutex shmem_lock; u32 func_id; - int irq; - struct completion tx_complete; }; static irqreturn_t smc_msg_done_isr(int irq, void *data) { struct scmi_smc *scmi_info = data; - complete(&scmi_info->tx_complete); + scmi_rx_callback(scmi_info->cinfo, + shmem_read_header(scmi_info->shmem), NULL); return IRQ_HANDLED; } @@ -111,8 +108,8 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, dev_err(dev, "failed to setup SCMI smc irq\n"); return ret; } - init_completion(&scmi_info->tx_complete); - scmi_info->irq = irq; + } else { + cinfo->no_completion_irq = true; } scmi_info->func_id = func_id; @@ -142,26 +139,22 @@ static int smc_send_message(struct scmi_chan_info *cinfo, struct scmi_smc *scmi_info = cinfo->transport_info; struct arm_smccc_res res; + /* + * Channel lock will be released only once response has been + * surely fully retrieved, so after .mark_txdone() + */ mutex_lock(&scmi_info->shmem_lock); shmem_tx_prepare(scmi_info->shmem, xfer); - if (scmi_info->irq) - reinit_completion(&scmi_info->tx_complete); - arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res); - if (scmi_info->irq) - wait_for_completion(&scmi_info->tx_complete); - - scmi_rx_callback(scmi_info->cinfo, - shmem_read_header(scmi_info->shmem), NULL); - - mutex_unlock(&scmi_info->shmem_lock); - /* Only SMCCC_RET_NOT_SUPPORTED is valid error code */ - if (res.a0) + if (res.a0) { + mutex_unlock(&scmi_info->shmem_lock); return -EOPNOTSUPP; + } + return 0; } @@ -173,6 +166,13 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo, shmem_fetch_response(scmi_info->shmem, xfer); } +static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret) +{ + struct scmi_smc *scmi_info = cinfo->transport_info; + + mutex_unlock(&scmi_info->shmem_lock); +} + static bool smc_poll_done(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer) { @@ -186,6 +186,7 @@ static const struct scmi_transport_ops scmi_smc_ops = { .chan_setup = smc_chan_setup, .chan_free = smc_chan_free, .send_message = smc_send_message, + .mark_txdone = smc_mark_txdone, .fetch_response = smc_fetch_response, .poll_done = smc_poll_done, }; -- cgit v1.2.3 From 31d2f803c19c1a7ad8d05c20b6cd83e8a647fb5c Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 20 Dec 2021 19:56:38 +0000 Subject: firmware: arm_scmi: Add sync_cmds_completed_on_ret transport flag Add a flag to let the transport signal to the core if its handling of sync command implies that, after .send_message has returned successfully, the requested command can be assumed to be fully and completely executed on SCMI platform side so that any possible response value is already immediately available to be retrieved by a .fetch_response: in other words the polling phase can be skipped in such a case and the response values accessed straight away. Note that all of the above applies only when polling mode of operation was selected by the core: if instead a completion IRQ was found to be available the normal response processing path based on completions will still be followed. Link: https://lore.kernel.org/r/20211220195646.44498-4-cristian.marussi@arm.com Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/common.h | 8 ++++++++ drivers/firmware/arm_scmi/driver.c | 34 ++++++++++++++++++++++++---------- 2 files changed, 32 insertions(+), 10 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 652e5d95ee65..24b1d1ac5f12 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -409,6 +409,13 @@ struct scmi_device *scmi_child_dev_find(struct device *parent, * @max_msg_size: Maximum size of data per message that can be handled. * @force_polling: Flag to force this whole transport to use SCMI core polling * mechanism instead of completion interrupts even if available. + * @sync_cmds_completed_on_ret: Flag to indicate that the transport assures + * synchronous-command messages are atomically + * completed on .send_message: no need to poll + * actively waiting for a response. + * Used by core internally only when polling is + * selected as a waiting for reply method: i.e. + * if a completion irq was found use that anyway. */ struct scmi_desc { int (*transport_init)(void); @@ -418,6 +425,7 @@ struct scmi_desc { int max_msg; int max_msg_size; const bool force_polling; + const bool sync_cmds_completed_on_ret; }; #ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 7579f54b0047..a1c33d36800b 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -617,7 +617,8 @@ static inline bool is_polling_required(struct scmi_chan_info *cinfo, static inline bool is_transport_polling_capable(struct scmi_info *info) { - return info->desc->ops->poll_done; + return info->desc->ops->poll_done || + info->desc->sync_cmds_completed_on_ret; } static inline bool is_polling_enabled(struct scmi_chan_info *cinfo, @@ -781,10 +782,28 @@ static int scmi_wait_for_message_response(struct scmi_chan_info *cinfo, xfer->hdr.poll_completion); if (xfer->hdr.poll_completion) { - ktime_t stop = ktime_add_ms(ktime_get(), timeout_ms); + /* + * Real polling is needed only if transport has NOT declared + * itself to support synchronous commands replies. + */ + if (!info->desc->sync_cmds_completed_on_ret) { + /* + * Poll on xfer using transport provided .poll_done(); + * assumes no completion interrupt was available. + */ + ktime_t stop = ktime_add_ms(ktime_get(), timeout_ms); + + spin_until_cond(scmi_xfer_done_no_timeout(cinfo, + xfer, stop)); + if (ktime_after(ktime_get(), stop)) { + dev_err(dev, + "timed out in resp(caller: %pS) - polling\n", + (void *)_RET_IP_); + ret = -ETIMEDOUT; + } + } - spin_until_cond(scmi_xfer_done_no_timeout(cinfo, xfer, stop)); - if (ktime_before(ktime_get(), stop)) { + if (!ret) { unsigned long flags; /* @@ -797,11 +816,6 @@ static int scmi_wait_for_message_response(struct scmi_chan_info *cinfo, xfer->state = SCMI_XFER_RESP_OK; } spin_unlock_irqrestore(&xfer->lock, flags); - } else { - dev_err(dev, - "timed out in resp(caller: %pS) - polling\n", - (void *)_RET_IP_); - ret = -ETIMEDOUT; } } else { /* And we wait for the response. */ @@ -836,7 +850,7 @@ static int do_xfer(const struct scmi_protocol_handle *ph, struct scmi_chan_info *cinfo; /* Check for polling request on custom command xfers at first */ - if (xfer->hdr.poll_completion && !info->desc->ops->poll_done) { + if (xfer->hdr.poll_completion && !is_transport_polling_capable(info)) { dev_warn_once(dev, "Polling mode is not supported by transport.\n"); return -EINVAL; -- cgit v1.2.3 From 117542b81fe7b12002afc0cfdcf6cdd3ebfc0f18 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 20 Dec 2021 19:56:39 +0000 Subject: firmware: arm_scmi: Make smc support sync_cmds_completed_on_ret Enable sync_cmds_completed_on_ret in the SMC transport descriptor and remove SMC specific .poll_done callback support since polling is bypassed when sync_cmds_completed_on_ret is set. Link: https://lore.kernel.org/r/20211220195646.44498-5-cristian.marussi@arm.com Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/smc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c index d6c6ad9f6bab..df0defd9f8bb 100644 --- a/drivers/firmware/arm_scmi/smc.c +++ b/drivers/firmware/arm_scmi/smc.c @@ -173,14 +173,6 @@ static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret) mutex_unlock(&scmi_info->shmem_lock); } -static bool -smc_poll_done(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer) -{ - struct scmi_smc *scmi_info = cinfo->transport_info; - - return shmem_poll_done(scmi_info->shmem, xfer); -} - static const struct scmi_transport_ops scmi_smc_ops = { .chan_available = smc_chan_available, .chan_setup = smc_chan_setup, @@ -188,7 +180,6 @@ static const struct scmi_transport_ops scmi_smc_ops = { .send_message = smc_send_message, .mark_txdone = smc_mark_txdone, .fetch_response = smc_fetch_response, - .poll_done = smc_poll_done, }; const struct scmi_desc scmi_smc_desc = { @@ -196,4 +187,13 @@ const struct scmi_desc scmi_smc_desc = { .max_rx_timeout_ms = 30, .max_msg = 20, .max_msg_size = 128, + /* + * Setting .sync_cmds_atomic_replies to true for SMC assumes that, + * once the SMC instruction has completed successfully, the issued + * SCMI command would have been already fully processed by the SCMI + * platform firmware and so any possible response value expected + * for the issued command will be immmediately ready to be fetched + * from the shared memory area. + */ + .sync_cmds_completed_on_ret = true, }; -- cgit v1.2.3 From bf322084fec30b92423911db0169a3610008fc15 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 20 Dec 2021 19:56:40 +0000 Subject: firmware: arm_scmi: Make optee support sync_cmds_completed_on_ret Declare each OPTEE SCMI channel as not having a completion_irq so as to enable polling mode and then enable also .sync_cmds_completed_on_ret flag in the OPTEE transport descriptor so that real polling is itself effectively bypassed on the rx path: once the optee command invocation has successfully returned the core will directly fetch the response from the shared memory area. Remove OPTEE SCMI transport specific .poll_done callback support since real polling is effectively bypassed when .sync_cmds_completed_on_ret is set. Add OPTEE SCMI transport specific .mark_txdone callback support in order to properly handle channel locking along the tx path. Link: https://lore.kernel.org/r/20211220195646.44498-6-cristian.marussi@arm.com Cc: Etienne Carriere Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/optee.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c index 175b39bcd470..f460e12be4ea 100644 --- a/drivers/firmware/arm_scmi/optee.c +++ b/drivers/firmware/arm_scmi/optee.c @@ -363,6 +363,9 @@ static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *de if (ret) goto err_close_sess; + /* Enable polling */ + cinfo->no_completion_irq = true; + mutex_lock(&scmi_optee_private->mu); list_add(&channel->link, &scmi_optee_private->channel_list); mutex_unlock(&scmi_optee_private->mu); @@ -423,9 +426,8 @@ static int scmi_optee_send_message(struct scmi_chan_info *cinfo, shmem_tx_prepare(shmem, xfer); ret = invoke_process_smt_channel(channel); - - scmi_rx_callback(cinfo, shmem_read_header(shmem), NULL); - mutex_unlock(&channel->mu); + if (ret) + mutex_unlock(&channel->mu); return ret; } @@ -439,13 +441,11 @@ static void scmi_optee_fetch_response(struct scmi_chan_info *cinfo, shmem_fetch_response(shmem, xfer); } -static bool scmi_optee_poll_done(struct scmi_chan_info *cinfo, - struct scmi_xfer *xfer) +static void scmi_optee_mark_txdone(struct scmi_chan_info *cinfo, int ret) { struct scmi_optee_channel *channel = cinfo->transport_info; - struct scmi_shared_mem *shmem = get_channel_shm(channel, xfer); - return shmem_poll_done(shmem, xfer); + mutex_unlock(&channel->mu); } static struct scmi_transport_ops scmi_optee_ops = { @@ -454,9 +454,9 @@ static struct scmi_transport_ops scmi_optee_ops = { .chan_setup = scmi_optee_chan_setup, .chan_free = scmi_optee_chan_free, .send_message = scmi_optee_send_message, + .mark_txdone = scmi_optee_mark_txdone, .fetch_response = scmi_optee_fetch_response, .clear_channel = scmi_optee_clear_channel, - .poll_done = scmi_optee_poll_done, }; static int scmi_optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) @@ -562,4 +562,5 @@ const struct scmi_desc scmi_optee_desc = { .max_rx_timeout_ms = 30, .max_msg = 20, .max_msg_size = SCMI_OPTEE_MAX_MSG_SIZE, + .sync_cmds_completed_on_ret = true, }; -- cgit v1.2.3 From 69255e746890274e887ba36a403019380cde0b48 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 20 Dec 2021 19:56:41 +0000 Subject: firmware: arm_scmi: Add support for atomic transports An SCMI transport can be configured as .atomic_enabled in order to signal to the SCMI core that all its TX path is executed in atomic context and that, when requested, polling mode should be used while waiting for command responses. When a specific platform configuration had properly configured such a transport as .atomic_enabled, the SCMI core will also take care not to sleep in the corresponding RX path while waiting for a response if that specific command transaction was requested as atomic using polling mode. Asynchronous commands should not be used in an atomic context and so a warning is emitted if polling was requested for an asynchronous command. Add also a method to check, from the SCMI drivers, if the underlying SCMI transport is currently configured to support atomic transactions: this will be used by upper layers to determine if atomic requests can be supported at all on this SCMI instance. Link: https://lore.kernel.org/r/20211220195646.44498-7-cristian.marussi@arm.com Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/common.h | 4 +++ drivers/firmware/arm_scmi/driver.c | 51 ++++++++++++++++++++++++++++++++++++-- include/linux/scmi_protocol.h | 8 ++++++ 3 files changed, 61 insertions(+), 2 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 24b1d1ac5f12..01d42c2069d4 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -416,6 +416,9 @@ struct scmi_device *scmi_child_dev_find(struct device *parent, * Used by core internally only when polling is * selected as a waiting for reply method: i.e. * if a completion irq was found use that anyway. + * @atomic_enabled: Flag to indicate that this transport, which is assured not + * to sleep anywhere on the TX path, can be used in atomic mode + * when requested. */ struct scmi_desc { int (*transport_init)(void); @@ -426,6 +429,7 @@ struct scmi_desc { int max_msg_size; const bool force_polling; const bool sync_cmds_completed_on_ret; + const bool atomic_enabled; }; #ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index a1c33d36800b..78924db59290 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -923,6 +923,20 @@ static void reset_rx_to_maxsz(const struct scmi_protocol_handle *ph, * @ph: Pointer to SCMI protocol handle * @xfer: Transfer to initiate and wait for response * + * Using asynchronous commands in atomic/polling mode should be avoided since + * it could cause long busy-waiting here, so ignore polling for the delayed + * response and WARN if it was requested for this command transaction since + * upper layers should refrain from issuing such kind of requests. + * + * The only other option would have been to refrain from using any asynchronous + * command even if made available, when an atomic transport is detected, and + * instead forcibly use the synchronous version (thing that can be easily + * attained at the protocol layer), but this would also have led to longer + * stalls of the channel for synchronous commands and possibly timeouts. + * (in other words there is usually a good reason if a platform provides an + * asynchronous version of a command and we should prefer to use it...just not + * when using atomic/polling mode) + * * Return: -ETIMEDOUT in case of no delayed response, if transmit error, * return corresponding error, else if all goes well, return 0. */ @@ -934,12 +948,24 @@ static int do_xfer_with_response(const struct scmi_protocol_handle *ph, xfer->async_done = &async_response; + /* + * Delayed responses should not be polled, so an async command should + * not have been used when requiring an atomic/poll context; WARN and + * perform instead a sleeping wait. + * (Note Async + IgnoreDelayedResponses are sent via do_xfer) + */ + WARN_ON_ONCE(xfer->hdr.poll_completion); + ret = do_xfer(ph, xfer); if (!ret) { - if (!wait_for_completion_timeout(xfer->async_done, timeout)) + if (!wait_for_completion_timeout(xfer->async_done, timeout)) { + dev_err(ph->dev, + "timed out in delayed resp(caller: %pS)\n", + (void *)_RET_IP_); ret = -ETIMEDOUT; - else if (xfer->hdr.status) + } else if (xfer->hdr.status) { ret = scmi_to_linux_errno(xfer->hdr.status); + } } xfer->async_done = NULL; @@ -1373,6 +1399,22 @@ static void scmi_devm_protocol_put(struct scmi_device *sdev, u8 protocol_id) WARN_ON(ret); } +/** + * scmi_is_transport_atomic - Method to check if underlying transport for an + * SCMI instance is configured as atomic. + * + * @handle: A reference to the SCMI platform instance. + * + * Return: True if transport is configured as atomic + */ +static bool scmi_is_transport_atomic(const struct scmi_handle *handle) +{ + struct scmi_info *info = handle_to_scmi_info(handle); + + return info->desc->atomic_enabled && + is_transport_polling_capable(info); +} + static inline struct scmi_handle *scmi_handle_get_from_info_unlocked(struct scmi_info *info) { @@ -1910,6 +1952,7 @@ static int scmi_probe(struct platform_device *pdev) handle->version = &info->version; handle->devm_protocol_get = scmi_devm_protocol_get; handle->devm_protocol_put = scmi_devm_protocol_put; + handle->is_transport_atomic = scmi_is_transport_atomic; if (desc->ops->link_supplier) { ret = desc->ops->link_supplier(dev); @@ -1928,6 +1971,10 @@ static int scmi_probe(struct platform_device *pdev) if (scmi_notification_init(handle)) dev_err(dev, "SCMI Notifications NOT available.\n"); + if (info->desc->atomic_enabled && !is_transport_polling_capable(info)) + dev_err(dev, + "Transport is not polling capable. Atomic mode not supported.\n"); + /* * Trigger SCMI Base protocol initialization. * It's mandatory and won't be ever released/deinit until the diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index 80e781c51ddc..9f895cb81818 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -612,6 +612,13 @@ struct scmi_notify_ops { * @devm_protocol_get: devres managed method to acquire a protocol and get specific * operations and a dedicated protocol handler * @devm_protocol_put: devres managed method to release a protocol + * @is_transport_atomic: method to check if the underlying transport for this + * instance handle is configured to support atomic + * transactions for commands. + * Some users of the SCMI stack in the upper layers could + * be interested to know if they can assume SCMI + * command transactions associated to this handle will + * never sleep and act accordingly. * @notify_ops: pointer to set of notifications related operations */ struct scmi_handle { @@ -622,6 +629,7 @@ struct scmi_handle { (*devm_protocol_get)(struct scmi_device *sdev, u8 proto, struct scmi_protocol_handle **ph); void (*devm_protocol_put)(struct scmi_device *sdev, u8 proto); + bool (*is_transport_atomic)(const struct scmi_handle *handle); const struct scmi_notify_ops *notify_ops; }; -- cgit v1.2.3 From 0bfdca8a8661aaa2433b3f7b74d83e3520aa56e5 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 20 Dec 2021 19:56:42 +0000 Subject: firmware: arm_scmi: Add atomic mode support to smc transport Add a Kernel configuration option to enable SCMI SMC transport atomic mode operation for selected SCMI transactions and leave it as default disabled. Substitute mutex usages with busy-waiting and declare smc transport as .atomic_enabled if such Kernel configuration option is enabled. Link: https://lore.kernel.org/r/20211220195646.44498-8-cristian.marussi@arm.com Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/Kconfig | 14 ++++++++++ drivers/firmware/arm_scmi/smc.c | 56 ++++++++++++++++++++++++++++++++++----- 2 files changed, 64 insertions(+), 6 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig index 638ecec89ff1..d429326433d1 100644 --- a/drivers/firmware/arm_scmi/Kconfig +++ b/drivers/firmware/arm_scmi/Kconfig @@ -78,6 +78,20 @@ config ARM_SCMI_TRANSPORT_SMC If you want the ARM SCMI PROTOCOL stack to include support for a transport based on SMC, answer Y. +config ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE + bool "Enable atomic mode support for SCMI SMC transport" + depends on ARM_SCMI_TRANSPORT_SMC + help + Enable support of atomic operation for SCMI SMC based transport. + + If you want the SCMI SMC 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. + config ARM_SCMI_TRANSPORT_VIRTIO bool "SCMI transport based on VirtIO" depends on VIRTIO=y || VIRTIO=ARM_SCMI_PROTOCOL diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c index df0defd9f8bb..6c7871a40611 100644 --- a/drivers/firmware/arm_scmi/smc.c +++ b/drivers/firmware/arm_scmi/smc.c @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -14,6 +15,7 @@ #include #include #include +#include #include #include "common.h" @@ -23,14 +25,20 @@ * * @cinfo: SCMI channel info * @shmem: Transmit/Receive shared memory area - * @shmem_lock: Lock to protect access to Tx/Rx shared memory area + * @shmem_lock: Lock to protect access to Tx/Rx shared memory area. + * Used when NOT operating in atomic mode. + * @inflight: Atomic flag to protect access to Tx/Rx shared memory area. + * Used when operating in atomic mode. * @func_id: smc/hvc call function id */ struct scmi_smc { struct scmi_chan_info *cinfo; struct scmi_shared_mem __iomem *shmem; + /* Protect access to shmem area */ struct mutex shmem_lock; +#define INFLIGHT_NONE MSG_TOKEN_MAX + atomic_t inflight; u32 func_id; }; @@ -54,6 +62,41 @@ static bool smc_chan_available(struct device *dev, int idx) return true; } +static inline void smc_channel_lock_init(struct scmi_smc *scmi_info) +{ + if (IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE)) + atomic_set(&scmi_info->inflight, INFLIGHT_NONE); + else + mutex_init(&scmi_info->shmem_lock); +} + +static bool smc_xfer_inflight(struct scmi_xfer *xfer, atomic_t *inflight) +{ + int ret; + + ret = atomic_cmpxchg(inflight, INFLIGHT_NONE, xfer->hdr.seq); + + return ret == INFLIGHT_NONE; +} + +static inline void +smc_channel_lock_acquire(struct scmi_smc *scmi_info, + struct scmi_xfer *xfer __maybe_unused) +{ + if (IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE)) + spin_until_cond(smc_xfer_inflight(xfer, &scmi_info->inflight)); + else + mutex_lock(&scmi_info->shmem_lock); +} + +static inline void smc_channel_lock_release(struct scmi_smc *scmi_info) +{ + if (IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE)) + atomic_set(&scmi_info->inflight, INFLIGHT_NONE); + else + mutex_unlock(&scmi_info->shmem_lock); +} + static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, bool tx) { @@ -114,7 +157,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, scmi_info->func_id = func_id; scmi_info->cinfo = cinfo; - mutex_init(&scmi_info->shmem_lock); + smc_channel_lock_init(scmi_info); cinfo->transport_info = scmi_info; return 0; @@ -140,10 +183,10 @@ static int smc_send_message(struct scmi_chan_info *cinfo, struct arm_smccc_res res; /* - * Channel lock will be released only once response has been + * Channel will be released only once response has been * surely fully retrieved, so after .mark_txdone() */ - mutex_lock(&scmi_info->shmem_lock); + smc_channel_lock_acquire(scmi_info, xfer); shmem_tx_prepare(scmi_info->shmem, xfer); @@ -151,7 +194,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo, /* Only SMCCC_RET_NOT_SUPPORTED is valid error code */ if (res.a0) { - mutex_unlock(&scmi_info->shmem_lock); + smc_channel_lock_release(scmi_info); return -EOPNOTSUPP; } @@ -170,7 +213,7 @@ static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret) { struct scmi_smc *scmi_info = cinfo->transport_info; - mutex_unlock(&scmi_info->shmem_lock); + smc_channel_lock_release(scmi_info); } static const struct scmi_transport_ops scmi_smc_ops = { @@ -196,4 +239,5 @@ const struct scmi_desc scmi_smc_desc = { * from the shared memory area. */ .sync_cmds_completed_on_ret = true, + .atomic_enabled = IS_ENABLED(CONFIG_ARM_SCMI_TRANSPORT_SMC_ATOMIC_ENABLE), }; -- cgit v1.2.3 From 94d0cd1da14af0042c8ee7c2cf401dfc321c575c Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Mon, 20 Dec 2021 19:56:43 +0000 Subject: firmware: arm_scmi: Add new parameter to mark_txdone Add a new xfer parameter to mark_txdone transport operation which enables the SCMI core to optionally pass back into the transport layer a reference to the xfer descriptor that is being handled. Link: https://lore.kernel.org/r/20211220195646.44498-9-cristian.marussi@arm.com Reviewed-by: Florian Fainelli Signed-off-by: Cristian Marussi Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/common.h | 3 ++- drivers/firmware/arm_scmi/driver.c | 2 +- drivers/firmware/arm_scmi/mailbox.c | 3 ++- drivers/firmware/arm_scmi/optee.c | 3 ++- drivers/firmware/arm_scmi/smc.c | 3 ++- 5 files changed, 9 insertions(+), 5 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 01d42c2069d4..4fda84bfab42 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -378,7 +378,8 @@ struct scmi_transport_ops { unsigned int (*get_max_msg)(struct scmi_chan_info *base_cinfo); int (*send_message)(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer); - void (*mark_txdone)(struct scmi_chan_info *cinfo, int ret); + void (*mark_txdone)(struct scmi_chan_info *cinfo, int ret, + struct scmi_xfer *xfer); void (*fetch_response)(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer); void (*fetch_notification)(struct scmi_chan_info *cinfo, diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 78924db59290..c2e7897ff56e 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -897,7 +897,7 @@ static int do_xfer(const struct scmi_protocol_handle *ph, ret = scmi_to_linux_errno(xfer->hdr.status); if (info->desc->ops->mark_txdone) - info->desc->ops->mark_txdone(cinfo, ret); + info->desc->ops->mark_txdone(cinfo, ret, xfer); trace_scmi_xfer_end(xfer->transfer_id, xfer->hdr.id, xfer->hdr.protocol_id, xfer->hdr.seq, ret); diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c index e09eb12bf421..08ff4d110beb 100644 --- a/drivers/firmware/arm_scmi/mailbox.c +++ b/drivers/firmware/arm_scmi/mailbox.c @@ -140,7 +140,8 @@ static int mailbox_send_message(struct scmi_chan_info *cinfo, return ret; } -static void mailbox_mark_txdone(struct scmi_chan_info *cinfo, int ret) +static void mailbox_mark_txdone(struct scmi_chan_info *cinfo, int ret, + struct scmi_xfer *__unused) { struct scmi_mailbox *smbox = cinfo->transport_info; diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c index f460e12be4ea..734f1eeee161 100644 --- a/drivers/firmware/arm_scmi/optee.c +++ b/drivers/firmware/arm_scmi/optee.c @@ -441,7 +441,8 @@ static void scmi_optee_fetch_response(struct scmi_chan_info *cinfo, shmem_fetch_response(shmem, xfer); } -static void scmi_optee_mark_txdone(struct scmi_chan_info *cinfo, int ret) +static void scmi_optee_mark_txdone(struct scmi_chan_info *cinfo, int ret, + struct scmi_xfer *__unused) { struct scmi_optee_channel *channel = cinfo->transport_info; diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c index 6c7871a40611..745acfdd0b3d 100644 --- a/drivers/firmware/arm_scmi/smc.c +++ b/drivers/firmware/arm_scmi/smc.c @@ -209,7 +209,8 @@ static void smc_fetch_response(struct scmi_chan_info *cinfo, shmem_fetch_response(scmi_info->shmem, xfer); } -static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret) +static void smc_mark_txdone(struct scmi_chan_info *cinfo, int ret, + struct scmi_xfer *__unused) { struct scmi_smc *scmi_info = cinfo->transport_info; -- cgit v1.2.3