* [RFC PATCH v2 01/13] firmware: arm_scmi: Add receive buffer support for notifications
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 02/13] firmware: arm_scmi: Update protocol commands and notification list Cristian Marussi
` (12 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
From: Sudeep Holla <sudeep.holla@arm.com>
With all the plumbing in place, let's just add the separate dedicated
receive buffers to handle notifications that can arrive asynchronously
from the platform firmware to OS.
Also add one check to see if the platform supports any receive channels
before allocating the receive buffers: since those buffers are optionally
supported though, the whole xfer initialization is also postponed to be
able to check for their existence in advance.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
[Changed parameters in __scmi_xfer_info_init()]
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2:
- reviewed commit message
- reviewed parameters of __scmi_xfer_info_init()
---
drivers/firmware/arm_scmi/driver.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index dbec767222e9..efb660c34b57 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -76,6 +76,7 @@ struct scmi_xfers_info {
* implementation version and (sub-)vendor identification.
* @handle: Instance of SCMI handle to send to clients
* @tx_minfo: Universal Transmit Message management info
+ * @rx_minfo: Universal Receive Message management info
* @tx_idr: IDR object to map protocol id to Tx channel info pointer
* @rx_idr: IDR object to map protocol id to Rx channel info pointer
* @protocols_imp: List of protocols implemented, currently maximum of
@@ -89,6 +90,7 @@ struct scmi_info {
struct scmi_revision_info version;
struct scmi_handle handle;
struct scmi_xfers_info tx_minfo;
+ struct scmi_xfers_info rx_minfo;
struct idr tx_idr;
struct idr rx_idr;
u8 *protocols_imp;
@@ -525,13 +527,13 @@ int scmi_handle_put(const struct scmi_handle *handle)
return 0;
}
-static int scmi_xfer_info_init(struct scmi_info *sinfo)
+static int __scmi_xfer_info_init(struct scmi_info *sinfo,
+ struct scmi_xfers_info *info)
{
int i;
struct scmi_xfer *xfer;
struct device *dev = sinfo->dev;
const struct scmi_desc *desc = sinfo->desc;
- struct scmi_xfers_info *info = &sinfo->tx_minfo;
/* Pre-allocated messages, no more than what hdr.seq can support */
if (WARN_ON(desc->max_msg >= MSG_TOKEN_MAX)) {
@@ -566,6 +568,16 @@ static int scmi_xfer_info_init(struct scmi_info *sinfo)
return 0;
}
+static int scmi_xfer_info_init(struct scmi_info *sinfo)
+{
+ int ret = __scmi_xfer_info_init(sinfo, &sinfo->tx_minfo);
+
+ if (!ret && idr_find(&sinfo->rx_idr, SCMI_PROTOCOL_BASE))
+ ret = __scmi_xfer_info_init(sinfo, &sinfo->rx_minfo);
+
+ return ret;
+}
+
static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
int prot_id, bool tx)
{
@@ -699,10 +711,6 @@ static int scmi_probe(struct platform_device *pdev)
info->desc = desc;
INIT_LIST_HEAD(&info->node);
- ret = scmi_xfer_info_init(info);
- if (ret)
- return ret;
-
platform_set_drvdata(pdev, info);
idr_init(&info->tx_idr);
idr_init(&info->rx_idr);
@@ -715,6 +723,10 @@ static int scmi_probe(struct platform_device *pdev)
if (ret)
return ret;
+ ret = scmi_xfer_info_init(info);
+ if (ret)
+ return ret;
+
ret = scmi_base_protocol_init(handle);
if (ret) {
dev_err(dev, "unable to communicate with SCMI(%d)\n", ret);
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 02/13] firmware: arm_scmi: Update protocol commands and notification list
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 01/13] firmware: arm_scmi: Add receive buffer support for notifications Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 03/13] firmware: arm_scmi: Add notifications support in transport layer Cristian Marussi
` (11 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
From: Sudeep Holla <sudeep.holla@arm.com>
Add commands' enumerations and messages definitions for all existing
notify-enable commands across all protocols.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/base.c | 7 +++++++
drivers/firmware/arm_scmi/perf.c | 5 +++++
drivers/firmware/arm_scmi/power.c | 6 ++++++
drivers/firmware/arm_scmi/sensors.c | 4 ++++
4 files changed, 22 insertions(+)
diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index f804e8af6521..ce7d9203e41b 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -14,6 +14,13 @@ enum scmi_base_protocol_cmd {
BASE_DISCOVER_LIST_PROTOCOLS = 0x6,
BASE_DISCOVER_AGENT = 0x7,
BASE_NOTIFY_ERRORS = 0x8,
+ BASE_SET_DEVICE_PERMISSIONS = 0x9,
+ BASE_SET_PROTOCOL_PERMISSIONS = 0xa,
+ BASE_RESET_AGENT_CONFIGURATION = 0xb,
+};
+
+enum scmi_base_protocol_notify {
+ BASE_ERROR_EVENT = 0x0,
};
struct scmi_msg_resp_base_attributes {
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index ec81e6f7e7a4..88509ec637d0 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -27,6 +27,11 @@ enum scmi_performance_protocol_cmd {
PERF_DESCRIBE_FASTCHANNEL = 0xb,
};
+enum scmi_performance_protocol_notify {
+ PERFORMANCE_LIMITS_CHANGED = 0x0,
+ PERFORMANCE_LEVEL_CHANGED = 0x1,
+};
+
struct scmi_opp {
u32 perf;
u32 power;
diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c
index 214886ce84f1..cf7f0312381b 100644
--- a/drivers/firmware/arm_scmi/power.c
+++ b/drivers/firmware/arm_scmi/power.c
@@ -12,6 +12,12 @@ enum scmi_power_protocol_cmd {
POWER_STATE_SET = 0x4,
POWER_STATE_GET = 0x5,
POWER_STATE_NOTIFY = 0x6,
+ POWER_STATE_CHANGE_REQUESTED_NOTIFY = 0x7,
+};
+
+enum scmi_power_protocol_notify {
+ POWER_STATE_CHANGED = 0x0,
+ POWER_STATE_CHANGE_REQUESTED = 0x1,
};
struct scmi_msg_resp_power_attributes {
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index eba61b9c1f53..db1b1ab303da 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -14,6 +14,10 @@ enum scmi_sensor_protocol_cmd {
SENSOR_READING_GET = 0x6,
};
+enum scmi_sensor_protocol_notify {
+ SENSOR_TRIP_POINT_EVENT = 0x0,
+};
+
struct scmi_msg_resp_sensor_attributes {
__le16 num_sensors;
u8 max_requests;
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 03/13] firmware: arm_scmi: Add notifications support in transport layer
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 01/13] firmware: arm_scmi: Add receive buffer support for notifications Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 02/13] firmware: arm_scmi: Update protocol commands and notification list Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-17 10:49 ` Viresh Kumar
2020-02-14 15:35 ` [RFC PATCH v2 04/13] firmware: arm_scmi: Add support for notifications message processing Cristian Marussi
` (10 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Add common transport-layer methods to:
- fetch a notification instead of a response
- clear a pending notification
Add also all the needed support in mailbox/shmem transports.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
drivers/firmware/arm_scmi/common.h | 8 ++++++++
drivers/firmware/arm_scmi/mailbox.c | 17 +++++++++++++++++
drivers/firmware/arm_scmi/shmem.c | 15 +++++++++++++++
3 files changed, 40 insertions(+)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 5ac06469b01c..3c2e5d0d7b68 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -178,6 +178,8 @@ struct scmi_chan_info {
* @send_message: Callback to send a message
* @mark_txdone: Callback to mark tx as done
* @fetch_response: Callback to fetch response
+ * @fetch_notification: Callback to fetch notification
+ * @clear_notification: Callback to clear a pending notification
* @poll_done: Callback to poll transfer status
*/
struct scmi_transport_ops {
@@ -190,6 +192,9 @@ struct scmi_transport_ops {
void (*mark_txdone)(struct scmi_chan_info *cinfo, int ret);
void (*fetch_response)(struct scmi_chan_info *cinfo,
struct scmi_xfer *xfer);
+ void (*fetch_notification)(struct scmi_chan_info *cinfo,
+ size_t max_len, struct scmi_xfer *xfer);
+ void (*clear_notification)(struct scmi_chan_info *cinfo);
bool (*poll_done)(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer);
};
@@ -222,5 +227,8 @@ void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem,
u32 shmem_read_header(struct scmi_shared_mem __iomem *shmem);
void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem,
struct scmi_xfer *xfer);
+void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,
+ size_t max_len, struct scmi_xfer *xfer);
+void shmem_clear_notification(struct scmi_shared_mem __iomem *shmem);
bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
struct scmi_xfer *xfer);
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index 73077bbc4ad9..19ee058f9f44 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -158,6 +158,21 @@ static void mailbox_fetch_response(struct scmi_chan_info *cinfo,
shmem_fetch_response(smbox->shmem, xfer);
}
+static void mailbox_fetch_notification(struct scmi_chan_info *cinfo,
+ size_t max_len, struct scmi_xfer *xfer)
+{
+ struct scmi_mailbox *smbox = cinfo->transport_info;
+
+ shmem_fetch_notification(smbox->shmem, max_len, xfer);
+}
+
+static void mailbox_clear_notification(struct scmi_chan_info *cinfo)
+{
+ struct scmi_mailbox *smbox = cinfo->transport_info;
+
+ shmem_clear_notification(smbox->shmem);
+}
+
static bool
mailbox_poll_done(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer)
{
@@ -173,6 +188,8 @@ static struct scmi_transport_ops scmi_mailbox_ops = {
.send_message = mailbox_send_message,
.mark_txdone = mailbox_mark_txdone,
.fetch_response = mailbox_fetch_response,
+ .fetch_notification = mailbox_fetch_notification,
+ .clear_notification = mailbox_clear_notification,
.poll_done = mailbox_poll_done,
};
diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
index ca0ffd302ea2..e1ab05be90e3 100644
--- a/drivers/firmware/arm_scmi/shmem.c
+++ b/drivers/firmware/arm_scmi/shmem.c
@@ -67,6 +67,21 @@ void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem,
memcpy_fromio(xfer->rx.buf, shmem->msg_payload + 4, xfer->rx.len);
}
+void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,
+ size_t max_len, struct scmi_xfer *xfer)
+{
+ /* Skip only the length of header in shmem area i.e 4 bytes */
+ xfer->rx.len = min_t(size_t, max_len, ioread32(&shmem->length) - 4);
+
+ /* Take a copy to the rx buffer.. */
+ memcpy_fromio(xfer->rx.buf, shmem->msg_payload, xfer->rx.len);
+}
+
+void shmem_clear_notification(struct scmi_shared_mem __iomem *shmem)
+{
+ iowrite32(SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE, &shmem->channel_status);
+}
+
bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
struct scmi_xfer *xfer)
{
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC PATCH v2 03/13] firmware: arm_scmi: Add notifications support in transport layer
2020-02-14 15:35 ` [RFC PATCH v2 03/13] firmware: arm_scmi: Add notifications support in transport layer Cristian Marussi
@ 2020-02-17 10:49 ` Viresh Kumar
0 siblings, 0 replies; 23+ messages in thread
From: Viresh Kumar @ 2020-02-17 10:49 UTC (permalink / raw)
To: Cristian Marussi
Cc: Linux Kernel Mailing List, Linux ARM, Sudeep Holla, lukasz.luba,
james.quinlan, Jonathan.Cameron
On Fri, Feb 14, 2020 at 9:07 PM Cristian Marussi
<cristian.marussi@arm.com> wrote:
>
> Add common transport-layer methods to:
> - fetch a notification instead of a response
> - clear a pending notification
>
> Add also all the needed support in mailbox/shmem transports.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> drivers/firmware/arm_scmi/common.h | 8 ++++++++
> drivers/firmware/arm_scmi/mailbox.c | 17 +++++++++++++++++
> drivers/firmware/arm_scmi/shmem.c | 15 +++++++++++++++
> 3 files changed, 40 insertions(+)
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH v2 04/13] firmware: arm_scmi: Add support for notifications message processing
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (2 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 03/13] firmware: arm_scmi: Add notifications support in transport layer Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 05/13] firmware: arm_scmi: Add notification protocol-registration Cristian Marussi
` (9 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
From: Sudeep Holla <sudeep.holla@arm.com>
Add the mechanisms to distinguish notifications from delayed responses and
to properly fetch notification messages upon reception: notifications
processing does not continue further after the fetch phase.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
[Reworked/renamed scmi_handle_xfer_delayed_resp()]
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- switch the notif/delayed_resp message processing logic to use new
transport independent layer methods
- reviewed logic of scmi_handle_xfer_delayed_resp() while renaming it as
scmi_handle_response()
- properly relocated tracer points
---
drivers/firmware/arm_scmi/driver.c | 84 +++++++++++++++++++++++-------
1 file changed, 64 insertions(+), 20 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index efb660c34b57..868cc36a07c9 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -202,29 +202,42 @@ __scmi_xfer_put(struct scmi_xfers_info *minfo, struct scmi_xfer *xfer)
spin_unlock_irqrestore(&minfo->xfer_lock, flags);
}
-/**
- * scmi_rx_callback() - callback for receiving messages
- *
- * @cinfo: SCMI channel info
- * @msg_hdr: Message header
- *
- * Processes one received message to appropriate transfer information and
- * signals completion of the transfer.
- *
- * NOTE: This function will be invoked in IRQ context, hence should be
- * as optimal as possible.
- */
-void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr)
+static void scmi_handle_notification(struct scmi_chan_info *cinfo, u32 msg_hdr)
{
- struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
- struct scmi_xfers_info *minfo = &info->tx_minfo;
- u16 xfer_id = MSG_XTRACT_TOKEN(msg_hdr);
- u8 msg_type = MSG_XTRACT_TYPE(msg_hdr);
- struct device *dev = cinfo->dev;
struct scmi_xfer *xfer;
+ struct device *dev = cinfo->dev;
+ struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
+ struct scmi_xfers_info *minfo = &info->rx_minfo;
+
+ xfer = scmi_xfer_get(cinfo->handle, minfo);
+ if (IS_ERR(xfer)) {
+ dev_err(dev, "failed to get free message slot (%ld)\n",
+ PTR_ERR(xfer));
+ info->desc->ops->clear_notification(cinfo);
+ return;
+ }
+
+ unpack_scmi_header(msg_hdr, &xfer->hdr);
+ scmi_dump_header_dbg(dev, &xfer->hdr);
+ info->desc->ops->fetch_notification(cinfo, info->desc->max_msg_size,
+ xfer);
+
+ trace_scmi_rx_done(xfer->transfer_id, xfer->hdr.id,
+ xfer->hdr.protocol_id, xfer->hdr.seq,
+ MSG_TYPE_NOTIFICATION);
- if (msg_type == MSG_TYPE_NOTIFICATION)
- return; /* Notifications not yet supported */
+ __scmi_xfer_put(minfo, xfer);
+
+ info->desc->ops->clear_notification(cinfo);
+}
+
+static void scmi_handle_response(struct scmi_chan_info *cinfo,
+ u16 xfer_id, u8 msg_type)
+{
+ struct scmi_xfer *xfer;
+ struct device *dev = cinfo->dev;
+ struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
+ struct scmi_xfers_info *minfo = &info->tx_minfo;
/* Are we even expecting this? */
if (!test_bit(xfer_id, minfo->xfer_alloc_table)) {
@@ -248,6 +261,37 @@ void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr)
complete(&xfer->done);
}
+/**
+ * scmi_rx_callback() - callback for receiving messages
+ *
+ * @cinfo: SCMI channel info
+ * @msg_hdr: Message header
+ *
+ * Processes one received message to appropriate transfer information and
+ * signals completion of the transfer.
+ *
+ * NOTE: This function will be invoked in IRQ context, hence should be
+ * as optimal as possible.
+ */
+void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr)
+{
+ u16 xfer_id = MSG_XTRACT_TOKEN(msg_hdr);
+ u8 msg_type = MSG_XTRACT_TYPE(msg_hdr);
+
+ switch (msg_type) {
+ case MSG_TYPE_NOTIFICATION:
+ scmi_handle_notification(cinfo, msg_hdr);
+ break;
+ case MSG_TYPE_COMMAND:
+ case MSG_TYPE_DELAYED_RESP:
+ scmi_handle_response(cinfo, xfer_id, msg_type);
+ break;
+ default:
+ WARN_ONCE(1, "received unknown msg_type:%d\n", msg_type);
+ break;
+ }
+}
+
/**
* scmi_xfer_put() - Release a transmit message
*
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 05/13] firmware: arm_scmi: Add notification protocol-registration
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (3 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 04/13] firmware: arm_scmi: Add support for notifications message processing Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 06/13] firmware: arm_scmi: Add notification callbacks-registration Cristian Marussi
` (8 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Add core SCMI Notifications protocol-registration support: allow protocols
to register their own set of supported events, during their initialization
phase.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- splitted out of V1 patch 04
- moved from IDR maps to real HashTables to store events
- scmi_notifications_initialized is now an atomic_t
- reviewed protocol registration/unregistration to use devres
- fixed:
drivers/firmware/arm_scmi/notify.c:483:18-23: ERROR:
reference preceded by free on line 482
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
---
drivers/firmware/arm_scmi/Makefile | 2 +-
drivers/firmware/arm_scmi/common.h | 4 +
drivers/firmware/arm_scmi/notify.c | 425 +++++++++++++++++++++++++++++
drivers/firmware/arm_scmi/notify.h | 56 ++++
4 files changed, 486 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/arm_scmi/notify.c
create mode 100644 drivers/firmware/arm_scmi/notify.h
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 6694d0d908d6..24a03a36aee4 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-y = scmi-bus.o scmi-driver.o scmi-protocols.o scmi-transport.o
scmi-bus-y = bus.o
-scmi-driver-y = driver.o
+scmi-driver-y = driver.o notify.o
scmi-transport-y = mailbox.o shmem.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o
obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 3c2e5d0d7b68..2106c35195ce 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -6,6 +6,8 @@
*
* Copyright (C) 2018 ARM Ltd.
*/
+#ifndef _SCMI_COMMON_H
+#define _SCMI_COMMON_H
#include <linux/bitfield.h>
#include <linux/completion.h>
@@ -232,3 +234,5 @@ void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,
void shmem_clear_notification(struct scmi_shared_mem __iomem *shmem);
bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
struct scmi_xfer *xfer);
+
+#endif /* _SCMI_COMMON_H */
diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
new file mode 100644
index 000000000000..d496522dea72
--- /dev/null
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -0,0 +1,425 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) Notification support
+ *
+ * Copyright (C) 2019 ARM Ltd.
+ *
+ * SCMI Protocol specification allows the platform to signal events to
+ * interested agents via notification messages: this in an implementation
+ * of the dispatch and delivery of such notifications to the interested users
+ * inside the Linux kernel.
+ *
+ * Each SCMI Protocol implementation, during its initialization, registers with
+ * this core notification framework its set of supported events using
+ * @scmi_register_protocol_events(): all these protocols and events descriptors
+ * are stored in the @scmi_registered_protocols and @scmi_registered_events
+ * hashtables.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/atomic.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/hashtable.h>
+#include <linux/kernel.h>
+#include <linux/kfifo.h>
+#include <linux/mutex.h>
+#include <linux/refcount.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+#include "notify.h"
+
+#define SCMI_ALL_SRC_IDS 0xffffUL
+
+/*
+ * Builds an unsigned 32bit key from the given input tuple to be used
+ * as a key in hashtables.
+ */
+#define MAKE_HASH_KEY(p, e, s) \
+ ((u32)(((p) << 24) | ((e) << 16) | ((s) & SCMI_ALL_SRC_IDS)))
+
+/**
+ * Assumes that the stored obj includes its own hash-key in a field named 'key':
+ * with this simplification this macro can be equally used for all the objects'
+ * types hashed by this implementation.
+ *
+ * @__ht: The hashtable name
+ * @__obj: A pointer to the object type to be retrieved from the hashtable;
+ * it will be used as a cursor while scanning the hastable and it will
+ * be possibly left as NULL when @__k is not found
+ * @__k: The key to search for
+ */
+#define KEY_FIND(__ht, __obj, __k) \
+({ \
+ hash_for_each_possible((__ht), (__obj), hash, (__k)) \
+ if (likely((__obj)->key == (__k))) \
+ break; \
+ __obj; \
+})
+
+/**
+ * events_queue - Describes a queue and its associated worker
+ *
+ * Each protocol has its own dedicated events_queue descriptor.
+ *
+ * @sz: Size in bytes of the related kfifo
+ * @qbuf: Pre-allocated buffer of @sz bytes to be used by the kfifo
+ * @kfifo: A dedicated Kernel kfifo
+ */
+struct events_queue {
+ size_t sz;
+ u8 *qbuf;
+ struct kfifo kfifo;
+};
+
+/**
+ * scmi_registered_protocol_events_desc - Protocol Specific information
+ *
+ * All protocols that registers at least one event have their protocol-specific
+ * information stored here, together with a ref to the allocated events_queue.
+ * These descriptors are stored in the @scmi_registered_protocols hashtable.
+ *
+ * @key: The used hashkey
+ * @id: Protocol ID
+ * @ops: Protocol specific and event-related operations
+ * @equeue: A reference to the associated per-protocol events_queue
+ * @hash: The hash list_node used for collision handling
+ * @gid: The associated devres group id, for cleanup purposes
+ */
+struct scmi_registered_protocol_events_desc {
+ u32 key;
+ u8 id;
+ const struct scmi_protocol_event_ops *ops;
+ struct events_queue *equeue;
+ struct hlist_node hash;
+ void *gid;
+};
+
+/**
+ * scmi_registered_event - Event Specific Information
+ *
+ * All registered events are represented by one of these structures.
+ * These descriptors are stored in the @scmi_registered_events hashtable.
+ *
+ * @key: The used hashkey
+ * @proto: A reference to the associated protocol descriptor
+ * @evt: A reference to the associated event descriptor (as provided at
+ * registration time)
+ * @scratch_isr: A pre-allocated buffer to be used as a scratch area by ISR
+ * @scratch_isr: A pre-allocated buffer to be used as a scratch area by the
+ * deferred worker
+ * @hash: The hash list_node used for collision handling
+ * @num_sources: The number of possible sources for this event as stated at
+ * events' registration time
+ * @sources: A reference to a dynamically allocated array used to refcount the
+ * events' enables for all the existing sources.
+ * @sources_mtx: A mutex to serialize the access to @sources
+ */
+struct scmi_registered_event {
+ u32 key;
+ struct scmi_registered_protocol_events_desc *proto;
+ const struct scmi_event *evt;
+ void *scratch_isr;
+ void *scratch_bh;
+ struct hlist_node hash;
+ u32 num_sources;
+ refcount_t *sources;
+ struct mutex sources_mtx;
+};
+
+/**
+ * scmi_event_header - A utility header
+ *
+ * This header is prepended to each received event message payload before
+ * queueing it on the related events_queue: it carries ancillary information
+ * for the attached event message payload.
+ *
+ * @timestamp: The timestamp, in nanoseconds (boottime), which was associated
+ * to this event as soon as it entered the SCMI RX ISR
+ * @proto_id: Protocol ID
+ * @evt_id: Event ID (corresponds to the Event MsgID for this Protocol)
+ * @payld_sz: Effective size of the attached message payload which follows
+ * @payld: A reference to the embedded event payload
+ */
+struct scmi_event_header {
+ u64 timestamp;
+ u8 proto_id;
+ u8 evt_id;
+ size_t payld_sz;
+ u8 payld[];
+} __packed;
+
+/*
+ * A few hashtables, of various size, to track:
+ *
+ * - @scmi_registered_protocols: All protocol-level specific information related
+ * to events' handling
+ * - @scmi_registered_events: All events' descriptors registered by the
+ * protocols, together with their ancillary data
+ */
+static DECLARE_HASHTABLE(scmi_registered_protocols, 4);
+static DECLARE_HASHTABLE(scmi_registered_events, 5);
+
+static atomic_t scmi_notifications_initialized = ATOMIC_INIT(0);
+
+/**
+ * scmi_allocate_events_queue - Allocate/Initialize an events_queue descriptor
+ *
+ * Allocate an events_queue, a kfifo of the requested size and initialize the
+ * associated worker.
+ *
+ * @dev: Device to use for devres
+ * @sz: Size of the kfifo to initialize
+ *
+ * Return: A valid pointer to the allocated events_queue on Success
+ */
+static struct events_queue *scmi_allocate_events_queue(struct device *dev,
+ size_t sz)
+{
+ int ret;
+ struct events_queue *equeue;
+
+ equeue = devm_kzalloc(dev, sizeof(*equeue), GFP_KERNEL);
+ if (!equeue)
+ return ERR_PTR(-ENOMEM);
+
+ equeue->qbuf = devm_kzalloc(dev, sz, GFP_KERNEL);
+ if (!equeue->qbuf)
+ return ERR_PTR(-ENOMEM);
+ equeue->sz = sz;
+
+ ret = kfifo_init(&equeue->kfifo, equeue->qbuf, equeue->sz);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return equeue;
+}
+
+/**
+ * scmi_allocate_registered_protocol_desc - Allocate a registered protocol
+ * events' descriptor
+ *
+ * Used to keep protocol specific information related to events handling for any
+ * protocol which has registered at least one event.
+ *
+ * @dev: Device to use for devres
+ * @gid: Devres group id to be stored
+ * @proto_id: Protocol ID
+ * @queue_sz: Size of the associated queue to allocate
+ * @ops: Pointer to a struct holding references to protocol specific helpers
+ * needed during events handling
+ */
+static struct scmi_registered_protocol_events_desc *
+scmi_allocate_registered_protocol_desc(struct device *dev, void *gid,
+ u8 proto_id, size_t queue_sz,
+ const struct scmi_protocol_event_ops *ops)
+{
+ struct scmi_registered_protocol_events_desc *pdesc;
+ u32 proto_key = MAKE_HASH_KEY(proto_id, 0, SCMI_ALL_SRC_IDS);
+
+ pdesc = KEY_FIND(scmi_registered_protocols, pdesc, proto_key);
+ if (pdesc)
+ return pdesc;
+
+ pdesc = devm_kzalloc(dev, sizeof(*pdesc), GFP_KERNEL);
+ if (!pdesc)
+ return ERR_PTR(-ENOMEM);
+ pdesc->key = proto_key;
+ pdesc->id = proto_id;
+ pdesc->ops = ops;
+ pdesc->gid = gid;
+
+ pdesc->equeue = scmi_allocate_events_queue(dev, queue_sz);
+ if (IS_ERR(pdesc->equeue))
+ return ERR_PTR(PTR_ERR(pdesc->equeue));
+
+ hash_add(scmi_registered_protocols, &pdesc->hash, pdesc->key);
+
+ return pdesc;
+}
+
+/**
+ * scmi_register_protocol_events - Register Protocol Events with the core
+ *
+ * Used by SCMI Protocols initialization code to register with the notification
+ * core the list of supported events and their descriptors: takes care to
+ * pre-allocate amd store all needed descriptors, scratch buffers and event
+ * queues.
+ *
+ * @dev: Device to use for devres
+ * @proto_id: Protocol ID
+ * @queue_sz: Size in bytes of the associated queue to be allocated
+ * @ops: Protocol specific event-related operations
+ * @evt: Event descriptor array
+ * @num_events: Number of events in @evt array
+ * @num_sources: Number of possible sources for this protocol on this
+ * platform.
+ *
+ * Return: 0 on Success
+ */
+int scmi_register_protocol_events(struct device *dev,
+ u8 proto_id, size_t queue_sz,
+ const struct scmi_protocol_event_ops *ops,
+ const struct scmi_event *evt, int num_events,
+ int num_sources)
+{
+ int i;
+ void *gid;
+ struct scmi_registered_protocol_events_desc *pdesc;
+
+ /* Ensure atomic value is updated */
+ smp_mb__before_atomic();
+ if (!ops || !evt ||
+ unlikely(!atomic_read(&scmi_notifications_initialized)))
+ return -EINVAL;
+
+ gid = devres_open_group(dev, NULL, GFP_KERNEL);
+ if (!gid)
+ return -ENOMEM;
+
+ pdesc = scmi_allocate_registered_protocol_desc(dev, gid, proto_id,
+ queue_sz, ops);
+ if (IS_ERR(pdesc))
+ goto err;
+
+ for (i = 0; i < num_events; i++, evt++) {
+ struct scmi_registered_event *r_evt;
+
+ r_evt = devm_kzalloc(dev, sizeof(*r_evt), GFP_KERNEL);
+ if (!r_evt)
+ goto err;
+ r_evt->proto = pdesc;
+ r_evt->evt = evt;
+
+ r_evt->sources = devm_kcalloc(dev, num_sources,
+ sizeof(refcount_t), GFP_KERNEL);
+ if (!r_evt->sources)
+ goto err;
+ r_evt->num_sources = num_sources;
+ mutex_init(&r_evt->sources_mtx);
+
+ r_evt->scratch_isr = devm_kzalloc(dev,
+ sizeof(struct scmi_event_header) +
+ evt->max_payld_sz, GFP_KERNEL);
+ if (!r_evt->scratch_isr)
+ goto err;
+
+ r_evt->scratch_bh = devm_kzalloc(dev, evt->max_payld_sz,
+ GFP_KERNEL);
+ if (!r_evt->scratch_bh)
+ goto err;
+
+ r_evt->key = MAKE_HASH_KEY(r_evt->proto->id,
+ r_evt->evt->evt_id,
+ SCMI_ALL_SRC_IDS);
+ hash_add(scmi_registered_events, &r_evt->hash, r_evt->key);
+
+ pr_info("SCMI Notifications: registered event - key:%X\n",
+ r_evt->key);
+ }
+ devres_close_group(dev, NULL);
+
+ return 0;
+
+err:
+ pr_warn("SCMI Notifications - Proto:%X - Registration Failed !\n",
+ proto_id);
+ devres_release_group(dev, NULL);
+ return -ENOMEM;
+}
+
+/**
+ * scmi_unregister_protocol_events - Unregister protocol-event resources
+ *
+ * Removes all registered events related to this protocol descriptor and
+ * frees all the underlying resources associated with this protocol devres
+ * group id.
+ *
+ * Assumes that the caller has already taken care to stop events dispatching
+ * and to flush the related queue. (via scmi_stop_and_flush_protocol_events)
+ *
+ * @dev: Device to use for devres
+ * @proto_id: The protocol to act upon
+ *
+ * Return: The number of released non-group resources
+ */
+int scmi_unregister_protocol_events(struct device *dev, u8 proto_id)
+{
+ int bkt;
+ struct scmi_registered_event *r_evt;
+ struct scmi_registered_protocol_events_desc *pdesc;
+
+ pdesc = KEY_FIND(scmi_registered_protocols, pdesc,
+ MAKE_HASH_KEY(proto_id, 0, SCMI_ALL_SRC_IDS));
+ if (!pdesc)
+ return 0;
+
+ /* Remove all registered events for pdesc and pdesc itself */
+ hash_for_each(scmi_registered_events, bkt, r_evt, hash)
+ if (r_evt->proto == pdesc)
+ hash_del(&r_evt->hash);
+ hash_del(&pdesc->hash);
+
+ /* Free all underlying resources */
+ return devres_release_group(dev, pdesc->gid);
+}
+
+/**
+ * scmi_stop_and_flush_protocol_events - Stop events processing
+ *
+ * Stop ISR dispatching and flush protocol queue: after this point no more
+ * events will be queued for this protocol and so, most importantly lookups
+ * on scmi_registered_events cannot happen anymore.
+ * Note that here we want to address also the possibility that a faulty
+ * platform keeps on emitting notification message even after having being
+ * asked to stop after the last user has gone.
+ *
+ * @proto_id: The protocol to act upon
+ *
+ * Return: False if protocol not found, True otherwise
+ */
+bool scmi_stop_and_flush_protocol_events(u8 proto_id)
+{
+ struct scmi_registered_protocol_events_desc *pdesc;
+
+ pdesc = KEY_FIND(scmi_registered_protocols, pdesc,
+ MAKE_HASH_KEY(proto_id, 0, SCMI_ALL_SRC_IDS));
+ if (!pdesc)
+ return false;
+
+ atomic_set(&scmi_notifications_initialized, 0);
+ /* Ensure atomic value is updated */
+ smp_mb__after_atomic();
+
+ return true;
+}
+
+/**
+ * scmi_notification_init - Initializes Notification Core Support
+ *
+ * Return: 0 on Success
+ */
+int __init scmi_notification_init(void)
+{
+ hash_init(scmi_registered_protocols);
+ hash_init(scmi_registered_events);
+
+ atomic_set(&scmi_notifications_initialized, 1);
+ /* Ensure atomic value is updated */
+ smp_mb__after_atomic();
+
+ pr_info("SCMI Notifications Core enabled.\n");
+
+ return 0;
+}
+
+/**
+ * scmi_notification_exit - Shutdown and clean Notification core
+ */
+void __exit scmi_notification_exit(void)
+{
+ pr_info("SCMI Notifications Core disabled.\n");
+}
diff --git a/drivers/firmware/arm_scmi/notify.h b/drivers/firmware/arm_scmi/notify.h
new file mode 100644
index 000000000000..3f8a69c85a36
--- /dev/null
+++ b/drivers/firmware/arm_scmi/notify.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * System Control and Management Interface (SCMI) Message Protocol
+ * notification header file containing some definitions, structures
+ * and function prototypes related to SCMI Notification handling.
+ *
+ * Copyright (C) 2019 ARM Ltd.
+ */
+#ifndef _SCMI_NOTIFY_H
+#define _SCMI_NOTIFY_H
+
+#include <linux/device.h>
+#include <linux/types.h>
+
+/**
+ * scmi_event - Describes an event to be supported
+ *
+ * Each SCMI protocol, during its initialization phase, can describe the events
+ * it wishes to support in a few struct scmi_event and pass them to the core
+ * using scmi_register_protocol_events().
+ *
+ * @evt_id: Event ID
+ * @max_payld_sz: Max possible size for the payload of a notif msg of this kind
+ * @max_report_sz: Max possible size for the report of a notif msg of this kind
+ */
+struct scmi_event {
+ u8 evt_id;
+ size_t max_payld_sz;
+
+};
+
+/**
+ * scmi_protocol_event_ops - Helpers called by notification core.
+ *
+ * These are called only in process context.
+ *
+ * @set_notify_enabled: Enable/disable the required evt_id/src_id notifications
+ * using the proper custom protocol commands.
+ * Return true if at least one the required src_id
+ * has been successfully enabled/disabled
+ */
+struct scmi_protocol_event_ops {
+ bool (*set_notify_enabled)(u8 evt_id, u32 src_id, bool enabled);
+};
+
+int scmi_notification_init(void);
+void scmi_notification_exit(void);
+int scmi_register_protocol_events(struct device *dev,
+ u8 proto_id, size_t queue_sz,
+ const struct scmi_protocol_event_ops *ops,
+ const struct scmi_event *evt, int num_events,
+ int num_sources);
+int scmi_unregister_protocol_events(struct device *dev, u8 proto_id);
+bool scmi_stop_and_flush_protocol_events(u8 proto_id);
+
+#endif /* _SCMI_NOTIFY_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 06/13] firmware: arm_scmi: Add notification callbacks-registration
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (4 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 05/13] firmware: arm_scmi: Add notification protocol-registration Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 07/13] firmware: arm_scmi: Add notification dispatch and delivery Cristian Marussi
` (7 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Add core SCMI Notifications callbacks-registration support: allow users
to register their own callbacks against the desired events.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- splitted out of V1 patch 04
- moved from IDR maps to real HashTables to store event_handlers
- added proper enable_events refcounting via __scmi_enable_evt()
[was broken in V1 when using ALL_SRCIDs notification chains]
- reviewed hashtable cleanup strategy in scmi_notifications_exit()
- added scmi_register_event_notifier()/scmi_unregister_event_notifier()
to include/linux/scmi_protocol.h as a candidate user API
[no EXPORTs still]
- added notify_ops to handle during initialization as an additional
internal API for scmi_drivers
---
drivers/firmware/arm_scmi/notify.c | 443 ++++++++++++++++++++++++++++-
drivers/firmware/arm_scmi/notify.h | 2 +-
include/linux/scmi_protocol.h | 63 ++++
3 files changed, 505 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
index d496522dea72..1339b6de0a4c 100644
--- a/drivers/firmware/arm_scmi/notify.c
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -14,18 +14,56 @@
* @scmi_register_protocol_events(): all these protocols and events descriptors
* are stored in the @scmi_registered_protocols and @scmi_registered_events
* hashtables.
+ *
+ * Kernel users interested in some specific event can register their associated
+ * callbacks providing the usual notifier_block descriptor, since this
+ * notification system implements events' delivery using the standard Kernel
+ * notification chains machinery. All users provided callbacks are instead
+ * stored in the @scmi_registered_events_handlers hashtable.
+ *
+ * Given the number of possible events defined by SCMI and the extensibility
+ * of the SCMI Protocol itself, the underlying notification chains are created
+ * and destroyed dynamically on demand depending on the number of users
+ * effectively registered for an event, so that no support structures or chains
+ * are allocated until at least one user has registered a notifier_block for
+ * such event. Similarly, events' generation itself is enabled at the platform
+ * level only after at least one user has registered, and it is shutdown after
+ * the last user for that event has gone.
+ *
+ * An event is identified univocally by the tuple (proto_id, evt_id, src_id)
+ * and is served by its own dedicated notification chain: given that such chain
+ * is indeed dynamically created, the registration API simply represents a mean
+ * for the users to associate their callbacks with the above tuple descriptor
+ * and possibly trigger the allocation and initialization of all the required
+ * resources.
+ *
+ * The information contained in such tuples is used, in a few different ways,
+ * to generate the final keys used to store/retrieve descriptors from the above
+ * hastables.
+ *
+ * Here proto_id and evt_id are simply the protocol_id and message_id numbers
+ * as described in the SCMI Protocol specification, while src_id represents an
+ * optional, protocol dependent, source identifier (like domain_id, perf_id
+ * or sensor_id and so forth), so that a user can register its callback for a
+ * particular event coming only from a well defined source (like CPU vs GPU).
+ * When the source is not specified the user callback will be registered for
+ * all existing sources for that event (if any).
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/atomic.h>
#include <linux/device.h>
+#include <linux/bitfield.h>
#include <linux/err.h>
#include <linux/hashtable.h>
#include <linux/kernel.h>
#include <linux/kfifo.h>
+#include <linux/module.h>
#include <linux/mutex.h>
+#include <linux/notifier.h>
#include <linux/refcount.h>
+#include <linux/scmi_protocol.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/workqueue.h>
@@ -60,6 +98,17 @@
__obj; \
})
+#define PROTO_ID_MASK GENMASK(31, 24)
+#define EVT_ID_MASK GENMASK(23, 16)
+#define SRC_ID_MASK GENMASK(15, 0)
+#define KEY_XTRACT_PROTO_ID(key) FIELD_GET(PROTO_ID_MASK, (key))
+#define KEY_XTRACT_EVT_ID(key) FIELD_GET(EVT_ID_MASK, (key))
+#define KEY_XTRACT_SRC_ID(key) FIELD_GET(SRC_ID_MASK, (key))
+
+/* A couple of utility macros to limit cruft when calling protocols' helpers */
+#define REVT_NOTIFY_ENABLE(revt, ...) \
+ ((revt)->proto->ops->set_notify_enabled(__VA_ARGS__))
+
/**
* events_queue - Describes a queue and its associated worker
*
@@ -130,6 +179,32 @@ struct scmi_registered_event {
struct mutex sources_mtx;
};
+/**
+ * scmi_event_handler - Event handler information
+ *
+ * This structure collects all the information needed to process a received
+ * event described by the tuple (proto_id, evt_id, src_id) invoking all the
+ * currently registered users' callbacks.
+ * These descriptors are stored in the @scmi_registered_events_handlers
+ * hashtable using as a key a value derived from that tuple.
+ *
+ * @key: The used hashkey
+ * @users: A reference count for number of active users for this handler
+ * @r_evt: A reference to the underlying registered event
+ * @chain: The notification chain dedicated to this specific event tuple
+ * @hash: The hash list_node used for collision handling
+ * @enabled: A boolean which records if event's generation has been already
+ * enabled for this handler as a whole
+ */
+struct scmi_event_handler {
+ u32 key;
+ refcount_t users;
+ struct scmi_registered_event *r_evt;
+ struct blocking_notifier_head chain;
+ struct hlist_node hash;
+ bool enabled;
+};
+
/**
* scmi_event_header - A utility header
*
@@ -159,9 +234,22 @@ struct scmi_event_header {
* to events' handling
* - @scmi_registered_events: All events' descriptors registered by the
* protocols, together with their ancillary data
+ * - @scmi_registered_events_handlers: All events' handlers descriptors, created
+ * to collect all the users' notifier_block
+ * callbacks and related notification chains
*/
static DECLARE_HASHTABLE(scmi_registered_protocols, 4);
static DECLARE_HASHTABLE(scmi_registered_events, 5);
+static DECLARE_HASHTABLE(scmi_registered_events_handlers, 8);
+
+/*
+ * A mutex to coordinate access to @scmi_registered_events_handlers which is the
+ * only hashtable which can potentially have multiple concurrent readers and
+ * writers at run-time; all other hashtables are fully populated at init time
+ * and then subsequently only read till cleanup-time when all notifications'
+ * processing would have been anyway already being stopped.
+ */
+static DEFINE_MUTEX(scmi_registered_events_handler_mtx);
static atomic_t scmi_notifications_initialized = ATOMIC_INIT(0);
@@ -352,6 +440,18 @@ int scmi_unregister_protocol_events(struct device *dev, u8 proto_id)
struct scmi_registered_event *r_evt;
struct scmi_registered_protocol_events_desc *pdesc;
+ /*
+ * We should get here only after all users have unregistered their
+ * handlers using the API...if that's not the case give up.
+ */
+ mutex_lock(&scmi_registered_events_handler_mtx);
+ if (!hash_empty(scmi_registered_events_handlers)) {
+ pr_err("SCMI Notifications: active users found at exit.\n");
+ mutex_unlock(&scmi_registered_events_handler_mtx);
+ return 0;
+ }
+ mutex_unlock(&scmi_registered_events_handler_mtx);
+
pdesc = KEY_FIND(scmi_registered_protocols, pdesc,
MAKE_HASH_KEY(proto_id, 0, SCMI_ALL_SRC_IDS));
if (!pdesc)
@@ -397,15 +497,354 @@ bool scmi_stop_and_flush_protocol_events(u8 proto_id)
return true;
}
+/**
+ * scmi_allocate_event_handler - Allocate and Register an Event handler
+ *
+ * Allocate an event handler and related notification chain associated with
+ * the event identified by the provided event key. Fails if the associated
+ * event is unknown to the core (i.e. it had not been previously registered
+ * as supported by some SCMI protocol)
+ *
+ * @evt_key: 32bit key uniquely bind to the event identified by the tuple
+ * (proto_id, evt_id, src_id)
+ *
+ * Return: the freshly allocated structure on Success
+ */
+static struct scmi_event_handler *scmi_allocate_event_handler(u32 evt_key)
+{
+ u8 proto_id, evt_id;
+ struct scmi_registered_event *r_evt;
+ struct scmi_event_handler *hndl;
+
+ proto_id = KEY_XTRACT_PROTO_ID(evt_key);
+ evt_id = KEY_XTRACT_EVT_ID(evt_key);
+ r_evt = KEY_FIND(scmi_registered_events, r_evt,
+ MAKE_HASH_KEY(proto_id, evt_id, SCMI_ALL_SRC_IDS));
+ if (!r_evt)
+ return ERR_PTR(-EINVAL);
+
+ hndl = kzalloc(sizeof(*hndl), GFP_KERNEL);
+ if (!hndl)
+ return ERR_PTR(-ENOMEM);
+ hndl->r_evt = r_evt;
+ hndl->key = evt_key;
+ BLOCKING_INIT_NOTIFIER_HEAD(&hndl->chain);
+ refcount_set(&hndl->users, 1);
+
+ /* Register freshly allocated event handler */
+ hash_add(scmi_registered_events_handlers, &hndl->hash, hndl->key);
+
+ return hndl;
+}
+
+/**
+ * scmi_free_event_handler - Free the provided Event handler
+ *
+ * @hndl: The event handler structure to free
+ */
+static void scmi_free_event_handler(struct scmi_event_handler *hndl)
+{
+ hash_del(&hndl->hash);
+ kfree(hndl);
+}
+
+/**
+ * __scmi_event_handler_get_ops - Utility to get or create an event handler
+ *
+ * After having got exclusive access to the registered events hashtable,
+ * search for the desired handler matching the key:
+ * - if found adjust users refcount
+ * - if not found and @create is true, create and register a new handler
+ *
+ * Events generation is NOT enabled on create within this routine since at
+ * creation time we usually want to have all setup and registered before events
+ * really start flowing.
+ *
+ * @evt_key: The event key to use
+ * @create: A boolean flag to specify if a handler must be created when
+ * not already existent
+ *
+ * Return: the freshly allocated structure on Success
+ */
+static inline struct scmi_event_handler *
+__scmi_event_handler_get_ops(u32 evt_key, bool create)
+{
+ struct scmi_event_handler *hndl = NULL;
+
+ mutex_lock(&scmi_registered_events_handler_mtx);
+ hndl = KEY_FIND(scmi_registered_events_handlers, hndl, evt_key);
+ if (hndl)
+ refcount_inc(&hndl->users);
+ else if (create)
+ hndl = scmi_allocate_event_handler(evt_key);
+ mutex_unlock(&scmi_registered_events_handler_mtx);
+
+ return hndl;
+}
+
+static struct scmi_event_handler *scmi_get_event_handler(u32 evt_key)
+{
+ return __scmi_event_handler_get_ops(evt_key, false);
+}
+
+static struct scmi_event_handler *scmi_get_or_create_event_handler(u32 evt_key)
+{
+ return __scmi_event_handler_get_ops(evt_key, true);
+}
+
+/**
+ * __scmi_enable_evt - Enable/disable events generation
+ *
+ * Takes care of proper refcounting while performing enable/disable:
+ * handles the special case of ALL sources requests.
+ *
+ * @r_evt: The registered event to act upon
+ * @src_id: The src_id to act upon
+ * @enable: The action to perform: true->Enable, false->Disable
+ *
+ * Return: True when the required @action has been successfully executed
+ */
+static inline bool __scmi_enable_evt(struct scmi_registered_event *r_evt,
+ u32 src_id, bool enable)
+{
+ int ret = 0;
+ u32 num_sources;
+ refcount_t *sid;
+
+ if (src_id == SCMI_ALL_SRC_IDS) {
+ src_id = 0;
+ num_sources = r_evt->num_sources;
+ } else if (src_id < r_evt->num_sources) {
+ num_sources = 1;
+ } else {
+ return ret;
+ }
+
+ mutex_lock(&r_evt->sources_mtx);
+ if (enable) {
+ for (; num_sources; src_id++, num_sources--) {
+ bool r;
+
+ sid = &r_evt->sources[src_id];
+ if (refcount_read(sid) == 0) {
+ r = REVT_NOTIFY_ENABLE(r_evt,
+ r_evt->evt->evt_id,
+ src_id, enable);
+ if (r)
+ refcount_set(sid, 1);
+ } else {
+ refcount_inc(sid);
+ r = true;
+ }
+ ret += r;
+ }
+ } else {
+ for (; num_sources; src_id++, num_sources--) {
+ sid = &r_evt->sources[src_id];
+ if (refcount_dec_and_test(sid))
+ REVT_NOTIFY_ENABLE(r_evt,
+ r_evt->evt->evt_id,
+ src_id, enable);
+ }
+ ret = 1;
+ }
+ mutex_unlock(&r_evt->sources_mtx);
+
+ return ret;
+}
+
+static bool scmi_enable_events(struct scmi_event_handler *hndl)
+{
+ if (!hndl->enabled)
+ hndl->enabled = __scmi_enable_evt(hndl->r_evt,
+ KEY_XTRACT_SRC_ID(hndl->key),
+ true);
+ return hndl->enabled;
+}
+
+static bool scmi_disable_events(struct scmi_event_handler *hndl)
+{
+ if (hndl->enabled)
+ hndl->enabled = !__scmi_enable_evt(hndl->r_evt,
+ KEY_XTRACT_SRC_ID(hndl->key),
+ false);
+ return !hndl->enabled;
+}
+
+static inline
+void scmi_put_event_handler_unlocked(struct scmi_event_handler *hndl)
+{
+ if (refcount_dec_and_test(&hndl->users)) {
+ scmi_disable_events(hndl);
+ scmi_free_event_handler(hndl);
+ }
+}
+
+/**
+ * scmi_put_event_handler - Put an event handler
+ *
+ * After having got exclusive access to the registered events hashtable, update
+ * the refcount and if @hndl is no more in use by anyone:
+ *
+ * - ask for events' generation disabling
+ * - unregister and free the handler itself
+ *
+ * @hndl: The handler to act upon
+ */
+static void scmi_put_event_handler(struct scmi_event_handler *hndl)
+{
+ mutex_lock(&scmi_registered_events_handler_mtx);
+ scmi_put_event_handler_unlocked(hndl);
+ mutex_unlock(&scmi_registered_events_handler_mtx);
+}
+
+static int scmi_register_notifier(u8 proto_id, u8 evt_id, u32 *src_id,
+ struct notifier_block *nb)
+{
+ u32 evt_key;
+ struct scmi_event_handler *hndl;
+
+ /* Ensure atomic value is updated */
+ smp_mb__before_atomic();
+ if (unlikely(!atomic_read(&scmi_notifications_initialized)))
+ return 0;
+
+ /* Account for handle->notify_ops access */
+ try_module_get(THIS_MODULE);
+
+ evt_key = MAKE_HASH_KEY(proto_id, evt_id,
+ src_id ? *src_id : SCMI_ALL_SRC_IDS);
+ hndl = scmi_get_or_create_event_handler(evt_key);
+ if (IS_ERR_OR_NULL(hndl))
+ return PTR_ERR(hndl);
+
+ blocking_notifier_chain_register(&hndl->chain, nb);
+
+ /*
+ * Ask platform to enable events': effective requests are emitted
+ * only when required by the current state of the underlying sources.
+ */
+ if (!scmi_enable_events(hndl)) {
+ pr_err("SCMI Notifications: Failed to ENABLE events for key:%X !\n",
+ evt_key);
+ blocking_notifier_chain_unregister(&hndl->chain, nb);
+ scmi_put_event_handler(hndl);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/**
+ * scmi_register_event_notifier - Register a notifier_block for an event
+ *
+ * Generic helper to register a notifier_block attached to a protocol event.
+ *
+ * A notifier_block @nb will be registered for each distinct event identified
+ * by the tuple (proto_id, evt_id, src_id) on a dedicated notification chain
+ * so that:
+ *
+ * (proto_X, evt_Y, src_Z) --> chain_X_Y_Z
+ *
+ * @src_id meaning is protocol specific and identifies the origin of the event
+ * (like domain_id, sensor_id and os forth).
+ *
+ * @src_id can be NULL to signify that the caller is interested in receiving
+ * notifications from ALL the available sources for that protocol OR simply that
+ * the protocol does not support distinct sources: in these cases @nb will be
+ * attached to a generic notification chain defined for ALL src_id of that
+ * proto_id/evt_id pair like:
+ *
+ * (proto_X, evt_Y, NULL) --> chain_X_Y_ALL
+ *
+ * Any received event will be then dispatched to both such chains if at least
+ * one user had registered an @nb on them.
+ *
+ * @proto_id: Protocol ID
+ * @evt_id: Event ID
+ * @src_id: Source ID, when NULL register for events coming form ALL possible
+ * sources
+ * @nb: A standard notifier block to register for the specified event
+ *
+ * Return: Return 0 on Success
+ */
+int scmi_register_event_notifier(u8 proto_id, u8 evt_id, u32 *src_id,
+ struct notifier_block *nb)
+{
+ return scmi_register_notifier(proto_id, evt_id, src_id, nb);
+}
+
+static int scmi_unregister_notifier(u8 proto_id, u8 evt_id, u32 *src_id,
+ struct notifier_block *nb)
+{
+ u32 evt_key;
+ struct scmi_event_handler *hndl;
+
+ /* Account for handle->notify_ops access */
+ module_put(THIS_MODULE);
+
+ evt_key = MAKE_HASH_KEY(proto_id, evt_id,
+ src_id ? *src_id : SCMI_ALL_SRC_IDS);
+ hndl = scmi_get_event_handler(evt_key);
+ if (IS_ERR_OR_NULL(hndl))
+ return -EINVAL;
+
+ blocking_notifier_chain_unregister(&hndl->chain, nb);
+ scmi_put_event_handler(hndl);
+ /*
+ * If this was the last user callback for this handler, this last
+ * additional put will force the handler to be freed.
+ * Note that if a call_chain walk is ongoing it will be instead the
+ * call_chain's put_request which will finally free the handler;
+ * note also that any operation on the inner notifier_block chain
+ * is already protected on its own.
+ */
+ scmi_put_event_handler(hndl);
+
+ return 0;
+}
+
+/**
+ * scmi_unregister_event_notifier - Unregister a notifier_block for an event
+ *
+ * Takes care to unregister the provided @nb from the notification chain
+ * associated to the specified event and, if there are no more users for the
+ * event handler, frees also the associated event handler structures.
+ *
+ * @proto_id: Protocol ID
+ * @evt_id: Event ID
+ * @src_id: Source ID
+ * @nb: The notifier_block to unregister
+ *
+ * Return: 0 on Success
+ */
+int scmi_unregister_event_notifier(u8 proto_id, u8 evt_id, u32 *src_id,
+ struct notifier_block *nb)
+{
+ return scmi_unregister_notifier(proto_id, evt_id, src_id, nb);
+}
+
+/*
+ * notify_ops are also to the attached to the handle so that can be accessed
+ * directly from an scmi_driver to register its own notifiers.
+ */
+static struct scmi_notify_ops notify_ops = {
+ .register_event_notifier = scmi_register_notifier,
+ .unregister_event_notifier = scmi_unregister_notifier,
+};
+
/**
* scmi_notification_init - Initializes Notification Core Support
*
* Return: 0 on Success
*/
-int __init scmi_notification_init(void)
+int scmi_notification_init(struct scmi_handle *handle)
{
hash_init(scmi_registered_protocols);
hash_init(scmi_registered_events);
+ hash_init(scmi_registered_events_handlers);
+
+ handle->notify_ops = ¬ify_ops;
atomic_set(&scmi_notifications_initialized, 1);
/* Ensure atomic value is updated */
@@ -419,7 +858,7 @@ int __init scmi_notification_init(void)
/**
* scmi_notification_exit - Shutdown and clean Notification core
*/
-void __exit scmi_notification_exit(void)
+void scmi_notification_exit(void)
{
pr_info("SCMI Notifications Core disabled.\n");
}
diff --git a/drivers/firmware/arm_scmi/notify.h b/drivers/firmware/arm_scmi/notify.h
index 3f8a69c85a36..438181147fc8 100644
--- a/drivers/firmware/arm_scmi/notify.h
+++ b/drivers/firmware/arm_scmi/notify.h
@@ -43,7 +43,7 @@ struct scmi_protocol_event_ops {
bool (*set_notify_enabled)(u8 evt_id, u32 src_id, bool enabled);
};
-int scmi_notification_init(void);
+int scmi_notification_init(struct scmi_handle *handle);
void scmi_notification_exit(void);
int scmi_register_protocol_events(struct device *dev,
u8 proto_id, size_t queue_sz,
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 5c873a59b387..f675f2aa7d87 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -4,7 +4,12 @@
*
* Copyright (C) 2018 ARM Ltd.
*/
+
+#ifndef _LINUX_SCMI_PROTOCOL_H
+#define _LINUX_SCMI_PROTOCOL_H
+
#include <linux/device.h>
+#include <linux/notifier.h>
#include <linux/types.h>
#define SCMI_MAX_STR_SIZE 16
@@ -207,6 +212,24 @@ struct scmi_reset_ops {
int (*deassert)(const struct scmi_handle *handle, u32 domain);
};
+/**
+ * scmi_notify_ops - represents notifications' operations provided by SCMI core
+ *
+ * Further details below within documentation for @scmi_register_event_notifier
+ * and @scmi_unregister_event_notifier(). The functionality is the same, they
+ * are just exposed in a different way.
+ *
+ * @register_event_notifier: Register a notifier_block for the requested event
+ * @unregister_event_notifier: Unregister a notifier_block for the requested
+ * event
+ */
+struct scmi_notify_ops {
+ int (*register_event_notifier)(u8 proto_id, u8 evt_id, u32 *src_id,
+ struct notifier_block *nb);
+ int (*unregister_event_notifier)(u8 proto_id, u8 evt_id, u32 *src_id,
+ struct notifier_block *nb);
+};
+
/**
* struct scmi_handle - Handle returned to ARM SCMI clients for usage.
*
@@ -236,6 +259,7 @@ struct scmi_handle {
struct scmi_power_ops *power_ops;
struct scmi_sensor_ops *sensor_ops;
struct scmi_reset_ops *reset_ops;
+ struct scmi_notify_ops *notify_ops;
/* for protocol internal use */
void *perf_priv;
void *clk_priv;
@@ -319,3 +343,42 @@ static inline void scmi_driver_unregister(struct scmi_driver *driver) {}
typedef int (*scmi_prot_init_fn_t)(struct scmi_handle *);
int scmi_protocol_register(int protocol_id, scmi_prot_init_fn_t fn);
void scmi_protocol_unregister(int protocol_id);
+
+/*
+ * SCMI Notification User API
+ *
+ * A user can register/unregister its own notifier_block against the
+ * desired event identified by the tuple: (proto_id, evt_id, src_id)
+ *
+ * using the following
+ * - scmi_register_event_notifier() / scmi_unregister_event_notifier()
+ *
+ * where:
+ *
+ * @proto_id: The protocol ID as in SCMI Specification
+ * @evt_id: The message ID of the desired event as in SCMI Specification
+ * @src_id: A pointer to the desired source ID if different sources are
+ * possible for the protocol (like domain_id, sensor_id...etc)
+ *
+ * @src_id can be provided as NULL if it simply does NOT make sense for
+ * the protocol at hand, OR if the user is explicitly interested in
+ * receiving notifications from ANY existent source associated to the
+ * specified proto_id / evt_id.
+ *
+ * Received notifications are finally delivered to the registered users,
+ * invoking the callback provided with the notifier_block *nb as follows:
+ *
+ * int user_cb(nb, evt_id, report)
+ *
+ * where:
+ *
+ * @nb: The notifier block provided by the user
+ * @evt_id: The message ID of the delivered event
+ * @report: A custom struct describing the specific event delivered
+ */
+int scmi_register_event_notifier(u8 proto_id, u8 evt_id, u32 *src_id,
+ struct notifier_block *nb);
+int scmi_unregister_event_notifier(u8 proto_id, u8 evt_id, u32 *src_id,
+ struct notifier_block *nb);
+
+#endif /* _LINUX_SCMI_PROTOCOL_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 07/13] firmware: arm_scmi: Add notification dispatch and delivery
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (5 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 06/13] firmware: arm_scmi: Add notification callbacks-registration Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-21 13:25 ` Lukasz Luba
2020-02-14 15:35 ` [RFC PATCH v2 08/13] firmware: arm_scmi: Enable notification core Cristian Marussi
` (6 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Add core SCMI Notifications dispatch and delivery support logic which is
able, at first, to dispatch well-known received events from the RX ISR to
the dedicated deferred worker, and then, from there, to final deliver the
events to the registered users' callbacks.
Dispatch and delivery is just added here, still not enabled.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- splitted out of V1 patch 04
- moved from IDR maps to real HashTables to store event_handlers
- simplified delivery logic
---
drivers/firmware/arm_scmi/notify.c | 242 ++++++++++++++++++++++++++++-
drivers/firmware/arm_scmi/notify.h | 22 +++
2 files changed, 262 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
index 1339b6de0a4c..c2341c5304cf 100644
--- a/drivers/firmware/arm_scmi/notify.c
+++ b/drivers/firmware/arm_scmi/notify.c
@@ -48,13 +48,44 @@
* particular event coming only from a well defined source (like CPU vs GPU).
* When the source is not specified the user callback will be registered for
* all existing sources for that event (if any).
+ *
+ * Upon reception of a notification message from the platform the SCMI RX ISR
+ * passes the received message payload and some ancillary information (including
+ * an arrival timestamp in nanoseconds) to the core via @scmi_notify(), which,
+ * in turn, after having looked up the event in @scmi_registered_events, pushes
+ * the event-data itself on a protocol dedicated kfifo queue for further
+ * deferred processing.
+ *
+ * Such dedicated protocols' queues are allocated once for all at initialization
+ * time, together with a dedicated work_item running the common delivery logic
+ * of @scmi_events_dispatcher(), so that each protocol has it own dedicated
+ * work_item and worker which, once kicked by the ISR, takes care to empty its
+ * own dedicated queue deliverying the queued items into the proper chain.
+ *
+ * Note that since the underlying cmwq workers run one distinct work_item per
+ * protocol and we allow for multiple concurrent workers (max_active != 1),
+ * notifications processing can proceed concurrently on distinct workers only
+ * between events belonging to different protocols: delivery of events within
+ * the same protocol is still strictly sequentially ordered by time of arrival;
+ * this separation of queues by protocol effectively avoids the possibility that
+ * one particularly verbose protocol, flooding the queues with events, can cause
+ * other protocols' events to be lost or their processing starved.
+ *
+ * Events' information is then extracted from SCMI Notification messages and
+ * conveyed, converted into a custom per-event report struct, as the void *data
+ * param to the user callback provided by the registered notifier_block, so that
+ * from the user perspective his callback will look invoked like:
+ *
+ * int user_cb(struct notifier_block *nb, unsigned long event_id, void *report)
+ *
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/atomic.h>
-#include <linux/device.h>
#include <linux/bitfield.h>
+#include <linux/bug.h>
+#include <linux/device.h>
#include <linux/err.h>
#include <linux/hashtable.h>
#include <linux/kernel.h>
@@ -108,6 +139,8 @@
/* A couple of utility macros to limit cruft when calling protocols' helpers */
#define REVT_NOTIFY_ENABLE(revt, ...) \
((revt)->proto->ops->set_notify_enabled(__VA_ARGS__))
+#define REVT_FILL_REPORT(revt, ...) \
+ ((revt)->proto->ops->fill_custom_report(__VA_ARGS__))
/**
* events_queue - Describes a queue and its associated worker
@@ -117,11 +150,15 @@
* @sz: Size in bytes of the related kfifo
* @qbuf: Pre-allocated buffer of @sz bytes to be used by the kfifo
* @kfifo: A dedicated Kernel kfifo
+ * @notify_work: A custom work item bound to this queue
+ * @wq: A reference to the related workqueue
*/
struct events_queue {
size_t sz;
u8 *qbuf;
struct kfifo kfifo;
+ struct work_struct notify_work;
+ struct workqueue_struct *wq;
};
/**
@@ -160,6 +197,8 @@ struct scmi_registered_protocol_events_desc {
* @scratch_isr: A pre-allocated buffer to be used as a scratch area by ISR
* @scratch_isr: A pre-allocated buffer to be used as a scratch area by the
* deferred worker
+ * @report: A pre-allocated buffer used by the deferred worker to fill a
+ * customized event report
* @hash: The hash list_node used for collision handling
* @num_sources: The number of possible sources for this event as stated at
* events' registration time
@@ -173,6 +212,7 @@ struct scmi_registered_event {
const struct scmi_event *evt;
void *scratch_isr;
void *scratch_bh;
+ void *report;
struct hlist_node hash;
u32 num_sources;
refcount_t *sources;
@@ -251,8 +291,191 @@ static DECLARE_HASHTABLE(scmi_registered_events_handlers, 8);
*/
static DEFINE_MUTEX(scmi_registered_events_handler_mtx);
+/* Common Kernel cmwq workqueue used by notifications core */
+static struct workqueue_struct *scmi_notify_wq;
+
static atomic_t scmi_notifications_initialized = ATOMIC_INIT(0);
+static struct scmi_event_handler *scmi_get_event_handler(u32 evt_key);
+static void scmi_put_event_handler(struct scmi_event_handler *hndl);
+
+/**
+ * scmi_discard_bad_evt_payload() - Utility to discard data from a kfifo
+ *
+ * @kq: The kfifo to act on
+ * @count: Number of bytes to flush
+ */
+static inline void scmi_discard_bad_evt_payload(struct kfifo *kq,
+ const unsigned int count)
+{
+ int i = 0;
+
+ pr_warn("SCMI Notifications: WQ skipping bad EVT Payload - %d bytes\n",
+ count);
+ /* Discard stale pending queued payload. */
+ for (i = 0; i < count; i++)
+ kfifo_skip(kq);
+}
+
+/**
+ * scmi_lookup_and_call_event_chain - Lookup the proper chain and call it
+ *
+ * @evt_key: The key to use to lookup the related notification chain
+ * @report: The customized event-specific report to pass down to the callbacks
+ * as their *data parameter.
+ */
+static inline void scmi_lookup_and_call_event_chain(u32 evt_key, void *report)
+{
+ int ret;
+ struct scmi_event_handler *hndl;
+
+ /* Here ensure the event handler cannot vanish while using it */
+ hndl = scmi_get_event_handler(evt_key);
+ if (IS_ERR_OR_NULL(hndl))
+ return;
+
+ ret = blocking_notifier_call_chain(&hndl->chain,
+ KEY_XTRACT_EVT_ID(evt_key),
+ report);
+ /* Notifiers are NOT supposed to cut the chain */
+ WARN_ON_ONCE(ret & NOTIFY_STOP_MASK);
+
+ scmi_put_event_handler(hndl);
+}
+
+/**
+ * scmi_events_dispatcher - Common worker logic for all work items.
+ *
+ * In turn:
+ * 1. dequeue one pending RX notification (queued in SCMI RX ISR context)
+ * 2. generate a custom event report from the received event message
+ * 3. lookup for any registered ALL_SRC_IDs handler
+ * - > call the related notification chain passing in the report
+ * 4. lookup for any registered specific SRC_ID handler
+ * - > call the related notification chain passing in the report
+ *
+ * Note that:
+ * - a dedicated per-protocol kfifo queue is used: in this way an anomalous
+ * flood of events cannot saturate other protocols' queues.
+ *
+ * - each per-protocol queue is associated to a distinct work_item, which
+ * means, in turn, that:
+ * + all protocols can process their dedicated queues concurrently
+ * (since scmi_notify_wq:max_active != 1)
+ * + anyway at most one worker instance is allowed to run on the same queue
+ * concurrently: this ensures that we can have only one concurrent
+ * reader/writer on the associated kfifo, and enables us to use the kfifo
+ * in a lock-less manner
+ *
+ * @work: The work item to use, which is associated to a dedicated events_queue
+ */
+static void scmi_events_dispatcher(struct work_struct *work)
+{
+ struct events_queue *equeue;
+ struct scmi_event_header eh;
+
+ equeue = container_of(work, struct events_queue, notify_work);
+ /* Read the scmi_event_header at first since it contains payld_sz ... */
+ while (kfifo_out(&equeue->kfifo, &eh, sizeof(eh))) {
+ u32 src_id, key;
+ unsigned int outs;
+ struct scmi_registered_event *r_evt;
+ void *report = NULL;
+
+ key = MAKE_HASH_KEY(eh.proto_id, eh.evt_id, SCMI_ALL_SRC_IDS);
+ r_evt = KEY_FIND(scmi_registered_events, r_evt, key);
+ if (!r_evt) {
+ scmi_discard_bad_evt_payload(&equeue->kfifo,
+ eh.payld_sz);
+ continue;
+ }
+
+ /* ... then read payld_sz bytes holding the evt-msg payload */
+ outs = kfifo_out(&equeue->kfifo, r_evt->scratch_bh,
+ eh.payld_sz);
+ if (outs != eh.payld_sz) {
+ pr_warn("SCMI Notifications: WQ SKIP corrupted EVT Payload.\n");
+ continue;
+ }
+
+
+ /* Reset and fill custom report, and obtain src_id */
+ memset(r_evt->report, 0x00, r_evt->evt->max_report_sz);
+ report = REVT_FILL_REPORT(r_evt, eh.evt_id, eh.timestamp,
+ r_evt->scratch_bh, eh.payld_sz,
+ r_evt->report, &src_id);
+ if (!report)
+ continue;
+
+ /* At first search for a generic ALL src_ids handler... */
+ scmi_lookup_and_call_event_chain(key, report);
+ /* ...then search for any specific src_id */
+ key = MAKE_HASH_KEY(eh.proto_id, eh.evt_id, src_id);
+ scmi_lookup_and_call_event_chain(key, report);
+ }
+}
+
+/**
+ * scmi_notify - Queues a notification for further deferred processing
+ *
+ * This is called in interrupt context to queue a received event for
+ * deferred processing.
+ *
+ * @proto_id: Protocol ID
+ * @evt_id: Event ID (msgID)
+ * @buf: Event Message Payload (without the header)
+ * @len: Event Message Payload size
+ * @ts: RX Timestamp in nanoseconds (boottime)
+ *
+ * Return: 0 on Success
+ */
+int scmi_notify(u8 proto_id, u8 evt_id, const void *buf, size_t len, u64 ts)
+{
+ struct scmi_registered_event *r_evt;
+ struct scmi_event_header *eh;
+
+
+ /* Ensure atomic value is updated */
+ smp_mb__before_atomic();
+ if (unlikely(!atomic_read(&scmi_notifications_initialized)))
+ return 0;
+
+ r_evt = KEY_FIND(scmi_registered_events, r_evt,
+ MAKE_HASH_KEY(proto_id, evt_id, SCMI_ALL_SRC_IDS));
+ if (unlikely(!r_evt))
+ return -EINVAL;
+
+ if (unlikely(len > r_evt->evt->max_payld_sz)) {
+ pr_err("SCMI Notifications: discard badly sized message\n");
+ return -EINVAL;
+ }
+ if (unlikely(kfifo_avail(&r_evt->proto->equeue->kfifo) <
+ sizeof(*eh) + len)) {
+ pr_warn("SCMI Notifications: queue full dropping proto_id:%d evt_id:%d ts:%lld\n",
+ proto_id, evt_id, ts);
+ return -ENOMEM;
+ }
+
+ /*
+ * Note that we can use the per protocol kfifo in a lock-less manner
+ * since we have only one concurrent reader and writer but, in order
+ * to avoid any trouble on the reader side, here we ensure to perform
+ * one single kfifo write, so we have to collate in advance the event
+ * header and payload in a scratch area at first.
+ */
+ eh = r_evt->scratch_isr;
+ eh->timestamp = ts;
+ eh->proto_id = proto_id;
+ eh->evt_id = evt_id;
+ eh->payld_sz = len;
+ memcpy(eh->payld, buf, eh->payld_sz);
+ kfifo_in(&r_evt->proto->equeue->kfifo, eh, sizeof(*eh) + eh->payld_sz);
+ queue_work(r_evt->proto->equeue->wq,
+ &r_evt->proto->equeue->notify_work);
+
+ return 0;
+}
+
/**
* scmi_allocate_events_queue - Allocate/Initialize an events_queue descriptor
*
@@ -278,10 +501,11 @@ static struct events_queue *scmi_allocate_events_queue(struct device *dev,
if (!equeue->qbuf)
return ERR_PTR(-ENOMEM);
equeue->sz = sz;
-
ret = kfifo_init(&equeue->kfifo, equeue->qbuf, equeue->sz);
if (ret)
return ERR_PTR(ret);
+ INIT_WORK(&equeue->notify_work, scmi_events_dispatcher);
+ equeue->wq = scmi_notify_wq;
return equeue;
}
@@ -400,6 +624,11 @@ int scmi_register_protocol_events(struct device *dev,
if (!r_evt->scratch_bh)
goto err;
+ r_evt->report = devm_kzalloc(dev, evt->max_report_sz,
+ GFP_KERNEL);
+ if (!r_evt->report)
+ goto err;
+
r_evt->key = MAKE_HASH_KEY(r_evt->proto->id,
r_evt->evt->evt_id,
SCMI_ALL_SRC_IDS);
@@ -494,6 +723,8 @@ bool scmi_stop_and_flush_protocol_events(u8 proto_id)
/* Ensure atomic value is updated */
smp_mb__after_atomic();
+ cancel_work_sync(&pdesc->equeue->notify_work);
+
return true;
}
@@ -840,6 +1071,11 @@ static struct scmi_notify_ops notify_ops = {
*/
int scmi_notification_init(struct scmi_handle *handle)
{
+ scmi_notify_wq = alloc_workqueue("scmi_notify",
+ WQ_UNBOUND | WQ_FREEZABLE, 0);
+ if (!scmi_notify_wq)
+ return -ENOMEM;
+
hash_init(scmi_registered_protocols);
hash_init(scmi_registered_events);
hash_init(scmi_registered_events_handlers);
@@ -861,4 +1097,6 @@ int scmi_notification_init(struct scmi_handle *handle)
void scmi_notification_exit(void)
{
pr_info("SCMI Notifications Core disabled.\n");
+ /* Destroy while letting pending work complete */
+ destroy_workqueue(scmi_notify_wq);
}
diff --git a/drivers/firmware/arm_scmi/notify.h b/drivers/firmware/arm_scmi/notify.h
index 438181147fc8..bcc599822bf5 100644
--- a/drivers/firmware/arm_scmi/notify.h
+++ b/drivers/firmware/arm_scmi/notify.h
@@ -9,9 +9,22 @@
#ifndef _SCMI_NOTIFY_H
#define _SCMI_NOTIFY_H
+#include <linux/bug.h>
#include <linux/device.h>
+#include <linux/notifier.h>
#include <linux/types.h>
+#define MAP_EVT_TO_ENABLE_CMD(id, map) \
+({ \
+ int ret = -1; \
+ \
+ if (likely((id) < ARRAY_SIZE((map)))) \
+ ret = (map)[(id)]; \
+ else \
+ WARN(1, "UN-KNOWN evt_id:%d\n", (id)); \
+ ret; \
+})
+
/**
* scmi_event - Describes an event to be supported
*
@@ -26,6 +39,7 @@
struct scmi_event {
u8 evt_id;
size_t max_payld_sz;
+ size_t max_report_sz;
};
@@ -38,9 +52,16 @@ struct scmi_event {
* using the proper custom protocol commands.
* Return true if at least one the required src_id
* has been successfully enabled/disabled
+ * @fill_custom_report: fills a custom event report from the provided
+ * event message payld identifying the event
+ * specific src_id.
+ * Return NULL on failure otherwise @report now fully
+ * populated
*/
struct scmi_protocol_event_ops {
bool (*set_notify_enabled)(u8 evt_id, u32 src_id, bool enabled);
+ void *(*fill_custom_report)(u8 evt_id, u64 timestamp, const void *payld,
+ size_t payld_sz, void *report, u32 *src_id);
};
int scmi_notification_init(struct scmi_handle *handle);
@@ -52,5 +73,6 @@ int scmi_register_protocol_events(struct device *dev,
int num_sources);
int scmi_unregister_protocol_events(struct device *dev, u8 proto_id);
bool scmi_stop_and_flush_protocol_events(u8 proto_id);
+int scmi_notify(u8 proto_id, u8 evt_id, const void *buf, size_t len, u64 ts);
#endif /* _SCMI_NOTIFY_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC PATCH v2 07/13] firmware: arm_scmi: Add notification dispatch and delivery
2020-02-14 15:35 ` [RFC PATCH v2 07/13] firmware: arm_scmi: Add notification dispatch and delivery Cristian Marussi
@ 2020-02-21 13:25 ` Lukasz Luba
2020-02-21 19:01 ` Cristian Marussi
0 siblings, 1 reply; 23+ messages in thread
From: Lukasz Luba @ 2020-02-21 13:25 UTC (permalink / raw)
To: Cristian Marussi, linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, Jonathan.Cameron
Hi Cristian,
I didn't want to jump into your discussion with Jim in other broader
thread with this small thought, so I added a comment below.
On 2/14/20 3:35 PM, Cristian Marussi wrote:
> Add core SCMI Notifications dispatch and delivery support logic which is
> able, at first, to dispatch well-known received events from the RX ISR to
> the dedicated deferred worker, and then, from there, to final deliver the
> events to the registered users' callbacks.
>
> Dispatch and delivery is just added here, still not enabled.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> V1 --> V2
> - splitted out of V1 patch 04
> - moved from IDR maps to real HashTables to store event_handlers
> - simplified delivery logic
> ---
> drivers/firmware/arm_scmi/notify.c | 242 ++++++++++++++++++++++++++++-
> drivers/firmware/arm_scmi/notify.h | 22 +++
> 2 files changed, 262 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
> index 1339b6de0a4c..c2341c5304cf 100644
> --- a/drivers/firmware/arm_scmi/notify.c
> +++ b/drivers/firmware/arm_scmi/notify.c
> @@ -48,13 +48,44 @@
> * particular event coming only from a well defined source (like CPU vs GPU).
> * When the source is not specified the user callback will be registered for
> * all existing sources for that event (if any).
[snip]
>
> @@ -840,6 +1071,11 @@ static struct scmi_notify_ops notify_ops = {
> */
> int scmi_notification_init(struct scmi_handle *handle)
> {
> + scmi_notify_wq = alloc_workqueue("scmi_notify",
> + WQ_UNBOUND | WQ_FREEZABLE, 0);
I think it might limit some platforms. It depends on their workload.
If they have some high priority workloads which rely on this mechanisms,
they might need a RT task here. The workqueues would be scheduled in
CFS, so it depends on workload in there (we might even see 10s ms delays
in scheduling-up them). If we use RT we would grab the CPU from CFS.
It would be good if it is a customization option: which mechanism
to use based on some a parameter. Then we could create:
a) workqueue with the flags above
b) workqueue with WQ_HIGHPRI (limited by minimum nice)
c) kthread_create_worker() with RT/DL/FIFO sched policy
(with also a parameterized priority)
In default clients might use a) but when they want to tune their
platform, they might change only a parameter in their scmi code,
not maintaining a patch for the RT function out of tree.
Regards,
Lukasz
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH v2 07/13] firmware: arm_scmi: Add notification dispatch and delivery
2020-02-21 13:25 ` Lukasz Luba
@ 2020-02-21 19:01 ` Cristian Marussi
2020-02-21 19:11 ` Lukasz Luba
0 siblings, 1 reply; 23+ messages in thread
From: Cristian Marussi @ 2020-02-21 19:01 UTC (permalink / raw)
To: Lukasz Luba, linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, Jonathan.Cameron
Hi Lukasz
Thanks for your feedback !
On 21/02/2020 13:25, Lukasz Luba wrote:
> Hi Cristian,
>
> I didn't want to jump into your discussion with Jim in other broader
> thread with this small thought, so I added a comment below.
>
> On 2/14/20 3:35 PM, Cristian Marussi wrote:
>> Add core SCMI Notifications dispatch and delivery support logic which is
> [snip]
>
>>
>> @@ -840,6 +1071,11 @@ static struct scmi_notify_ops notify_ops = {
>> */
>> int scmi_notification_init(struct scmi_handle *handle)
>> {
>> + scmi_notify_wq = alloc_workqueue("scmi_notify",
>> + WQ_UNBOUND | WQ_FREEZABLE, 0);
>
> I think it might limit some platforms. It depends on their workload.
> If they have some high priority workloads which rely on this mechanisms,
> they might need a RT task here. The workqueues would be scheduled in
> CFS, so it depends on workload in there (we might even see 10s ms delays
> in scheduling-up them). If we use RT we would grab the CPU from CFS.
>
> It would be good if it is a customization option: which mechanism
> to use based on some a parameter. Then we could create:
> a) workqueue with the flags above
> b) workqueue with WQ_HIGHPRI (limited by minimum nice)
> c) kthread_create_worker() with RT/DL/FIFO sched policy
> (with also a parameterized priority)
> In default clients might use a) but when they want to tune their
> platform, they might change only a parameter in their scmi code,
> not maintaining a patch for the RT function out of tree.
In this series, I have not addressed configurability issues at all (as noted in the cover):
in fact I was thinking that stuff like WQ_HIGHPRI flags and per-protocol queue sizes could
be beneficial to be customizable depending on the specific platform, but I had not gone to
the extreme of thinking of adopting a dedicated RT kthread as a worker...good point...it
makes surely sense to have this configurable option to try to reduce the latency where possible.
I think it's important to give the user the possibility to configure the deferred worker
as you suggested, if the user decides to rely on Linux to handle a critical notification,
but I'd prefer queuing up this work you suggested on a different series on top of this one.
(which is starting to be a little to much voluminous...for being just the core support)
Regards
Cristian
>
> Regards,
> Lukasz
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH v2 07/13] firmware: arm_scmi: Add notification dispatch and delivery
2020-02-21 19:01 ` Cristian Marussi
@ 2020-02-21 19:11 ` Lukasz Luba
2020-02-24 9:59 ` Lukasz Luba
0 siblings, 1 reply; 23+ messages in thread
From: Lukasz Luba @ 2020-02-21 19:11 UTC (permalink / raw)
To: Cristian Marussi, linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, Jonathan.Cameron
On 2/21/20 7:01 PM, Cristian Marussi wrote:
> Hi Lukasz
>
> Thanks for your feedback !
>
> On 21/02/2020 13:25, Lukasz Luba wrote:
>> Hi Cristian,
>>
>> I didn't want to jump into your discussion with Jim in other broader
>> thread with this small thought, so I added a comment below.
>>
>> On 2/14/20 3:35 PM, Cristian Marussi wrote:
>>> Add core SCMI Notifications dispatch and delivery support logic which is
>> [snip]
>>
>>>
>>> @@ -840,6 +1071,11 @@ static struct scmi_notify_ops notify_ops = {
>>> */
>>> int scmi_notification_init(struct scmi_handle *handle)
>>> {
>>> + scmi_notify_wq = alloc_workqueue("scmi_notify",
>>> + WQ_UNBOUND | WQ_FREEZABLE, 0);
>>
>> I think it might limit some platforms. It depends on their workload.
>> If they have some high priority workloads which rely on this mechanisms,
>> they might need a RT task here. The workqueues would be scheduled in
>> CFS, so it depends on workload in there (we might even see 10s ms delays
>> in scheduling-up them). If we use RT we would grab the CPU from CFS.
>>
>> It would be good if it is a customization option: which mechanism
>> to use based on some a parameter. Then we could create:
>> a) workqueue with the flags above
>> b) workqueue with WQ_HIGHPRI (limited by minimum nice)
>> c) kthread_create_worker() with RT/DL/FIFO sched policy
>> (with also a parameterized priority)
>> In default clients might use a) but when they want to tune their
>> platform, they might change only a parameter in their scmi code,
>> not maintaining a patch for the RT function out of tree.
>
> In this series, I have not addressed configurability issues at all (as noted in the cover):
> in fact I was thinking that stuff like WQ_HIGHPRI flags and per-protocol queue sizes could
> be beneficial to be customizable depending on the specific platform, but I had not gone to
> the extreme of thinking of adopting a dedicated RT kthread as a worker...good point...it
> makes surely sense to have this configurable option to try to reduce the latency where possible.
>
> I think it's important to give the user the possibility to configure the deferred worker
> as you suggested, if the user decides to rely on Linux to handle a critical notification,
> but I'd prefer queuing up this work you suggested on a different series on top of this one.
> (which is starting to be a little to much voluminous...for being just the core support)
Agree, you can build these features incrementally.
Regards,
Lukasz
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH v2 07/13] firmware: arm_scmi: Add notification dispatch and delivery
2020-02-21 19:11 ` Lukasz Luba
@ 2020-02-24 9:59 ` Lukasz Luba
2020-02-24 14:33 ` Cristian Marussi
0 siblings, 1 reply; 23+ messages in thread
From: Lukasz Luba @ 2020-02-24 9:59 UTC (permalink / raw)
To: Cristian Marussi, linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, Jonathan.Cameron
On 2/21/20 7:11 PM, Lukasz Luba wrote:
>
>
> On 2/21/20 7:01 PM, Cristian Marussi wrote:
>> Hi Lukasz
>>
>> Thanks for your feedback !
>>
>> On 21/02/2020 13:25, Lukasz Luba wrote:
>>> Hi Cristian,
>>>
>>> I didn't want to jump into your discussion with Jim in other broader
>>> thread with this small thought, so I added a comment below.
>>>
>>> On 2/14/20 3:35 PM, Cristian Marussi wrote:
>>>> Add core SCMI Notifications dispatch and delivery support logic
>>>> which is
>>> [snip]
>>>
>>>> @@ -840,6 +1071,11 @@ static struct scmi_notify_ops notify_ops = {
>>>> */
>>>> int scmi_notification_init(struct scmi_handle *handle)
>>>> {
>>>> + scmi_notify_wq = alloc_workqueue("scmi_notify",
>>>> + WQ_UNBOUND | WQ_FREEZABLE, 0);
>>>
>>> I think it might limit some platforms. It depends on their workload.
>>> If they have some high priority workloads which rely on this mechanisms,
>>> they might need a RT task here. The workqueues would be scheduled in
>>> CFS, so it depends on workload in there (we might even see 10s ms delays
>>> in scheduling-up them). If we use RT we would grab the CPU from CFS.
>>>
>>> It would be good if it is a customization option: which mechanism
>>> to use based on some a parameter. Then we could create:
>>> a) workqueue with the flags above
>>> b) workqueue with WQ_HIGHPRI (limited by minimum nice)
>>> c) kthread_create_worker() with RT/DL/FIFO sched policy
>>> (with also a parameterized priority)
>>> In default clients might use a) but when they want to tune their
>>> platform, they might change only a parameter in their scmi code,
>>> not maintaining a patch for the RT function out of tree.
>>
>> In this series, I have not addressed configurability issues at all (as
>> noted in the cover):
>> in fact I was thinking that stuff like WQ_HIGHPRI flags and
>> per-protocol queue sizes could
>> be beneficial to be customizable depending on the specific platform,
>> but I had not gone to
>> the extreme of thinking of adopting a dedicated RT kthread as a
>> worker...good point...it
>> makes surely sense to have this configurable option to try to reduce
>> the latency where possible.
>>
>> I think it's important to give the user the possibility to configure
>> the deferred worker
>> as you suggested, if the user decides to rely on Linux to handle a
>> critical notification,
>> but I'd prefer queuing up this work you suggested on a different
>> series on top of this one.
>> (which is starting to be a little to much voluminous...for being just
>> the core support)
>
> Agree, you can build these features incrementally.
Although, a WQ_SYSFS flag wouldn't harm too much this version and might
give possibility to tune/experiment with it.
>
> Regards,
> Lukasz
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH v2 07/13] firmware: arm_scmi: Add notification dispatch and delivery
2020-02-24 9:59 ` Lukasz Luba
@ 2020-02-24 14:33 ` Cristian Marussi
0 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-24 14:33 UTC (permalink / raw)
To: Lukasz Luba, linux-kernel, linux-arm-kernel
Cc: sudeep.holla, james.quinlan, Jonathan.Cameron
On 24/02/2020 09:59, Lukasz Luba wrote:
>
>
> On 2/21/20 7:11 PM, Lukasz Luba wrote:
>>
>>
>> On 2/21/20 7:01 PM, Cristian Marussi wrote:
>>> Hi Lukasz
>>>
>>> Thanks for your feedback !
>>>
>>> On 21/02/2020 13:25, Lukasz Luba wrote:
>>>> Hi Cristian,
>>>>
>>>> I didn't want to jump into your discussion with Jim in other broader
>>>> thread with this small thought, so I added a comment below.
>>>>
>>>> On 2/14/20 3:35 PM, Cristian Marussi wrote:
>>>>> Add core SCMI Notifications dispatch and delivery support logic
>>>>> which is
>>>> [snip]
>>>>
>>>>> @@ -840,6 +1071,11 @@ static struct scmi_notify_ops notify_ops = {
>>>>> */
>>>>> int scmi_notification_init(struct scmi_handle *handle)
>>>>> {
>>>>> + scmi_notify_wq = alloc_workqueue("scmi_notify",
>>>>> + WQ_UNBOUND | WQ_FREEZABLE, 0);
>>>>
>>>> I think it might limit some platforms. It depends on their workload.
>>>> If they have some high priority workloads which rely on this mechanisms,
>>>> they might need a RT task here. The workqueues would be scheduled in
>>>> CFS, so it depends on workload in there (we might even see 10s ms delays
>>>> in scheduling-up them). If we use RT we would grab the CPU from CFS.
>>>>
>>>> It would be good if it is a customization option: which mechanism
>>>> to use based on some a parameter. Then we could create:
>>>> a) workqueue with the flags above
>>>> b) workqueue with WQ_HIGHPRI (limited by minimum nice)
>>>> c) kthread_create_worker() with RT/DL/FIFO sched policy
>>>> (with also a parameterized priority)
>>>> In default clients might use a) but when they want to tune their
>>>> platform, they might change only a parameter in their scmi code,
>>>> not maintaining a patch for the RT function out of tree.
>>>
>>> In this series, I have not addressed configurability issues at all (as
>>> noted in the cover):
>>> in fact I was thinking that stuff like WQ_HIGHPRI flags and
>>> per-protocol queue sizes could
>>> be beneficial to be customizable depending on the specific platform,
>>> but I had not gone to
>>> the extreme of thinking of adopting a dedicated RT kthread as a
>>> worker...good point...it
>>> makes surely sense to have this configurable option to try to reduce
>>> the latency where possible.
>>>
>>> I think it's important to give the user the possibility to configure
>>> the deferred worker
>>> as you suggested, if the user decides to rely on Linux to handle a
>>> critical notification,
>>> but I'd prefer queuing up this work you suggested on a different
>>> series on top of this one.
>>> (which is starting to be a little to much voluminous...for being just
>>> the core support)
>>
>> Agree, you can build these features incrementally.
>
> Although, a WQ_SYSFS flag wouldn't harm too much this version and might
> give possibility to tune/experiment with it.
>
True. Added in v3.
Thanks
Cristian
>>
>> Regards,
>> Lukasz
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [RFC PATCH v2 08/13] firmware: arm_scmi: Enable notification core
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (6 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 07/13] firmware: arm_scmi: Add notification dispatch and delivery Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 09/13] firmware: arm_scmi: Add Power notifications support Cristian Marussi
` (5 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Initialize and enable SCMI Notifications core support during bus/driver
init and probe phases, so that protocols can start registering their
supported events during their initialization, and later users can start
enrolling their callbacks for the subset of events their interested in.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- added timestamping
- moved notification init/exit and using devres
---
drivers/firmware/arm_scmi/bus.c | 11 +++++++++++
drivers/firmware/arm_scmi/driver.c | 10 ++++++++++
2 files changed, 21 insertions(+)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index db55c43a2cbd..0761f306982f 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -14,6 +14,7 @@
#include <linux/device.h>
#include "common.h"
+#include "notify.h"
static DEFINE_IDA(scmi_bus_id);
static DEFINE_IDR(scmi_protocols);
@@ -90,11 +91,21 @@ static int scmi_dev_probe(struct device *dev)
return scmi_drv->probe(scmi_dev);
}
+static inline void scmi_protocol_events_cleanup(struct scmi_device *scmi_dev)
+{
+ if (scmi_stop_and_flush_protocol_events(scmi_dev->protocol_id))
+ scmi_unregister_protocol_events(scmi_dev->handle->dev,
+ scmi_dev->protocol_id);
+}
+
static int scmi_dev_remove(struct device *dev)
{
struct scmi_driver *scmi_drv = to_scmi_driver(dev->driver);
struct scmi_device *scmi_dev = to_scmi_dev(dev);
+ /* Release events resources allocated by scmi_protocol_init() */
+ scmi_protocol_events_cleanup(scmi_dev);
+
if (scmi_drv->remove)
scmi_drv->remove(scmi_dev);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 868cc36a07c9..dc93d608c43c 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -26,6 +26,7 @@
#include <linux/slab.h>
#include "common.h"
+#include "notify.h"
#define CREATE_TRACE_POINTS
#include <trace/events/scmi.h>
@@ -204,11 +205,13 @@ __scmi_xfer_put(struct scmi_xfers_info *minfo, struct scmi_xfer *xfer)
static void scmi_handle_notification(struct scmi_chan_info *cinfo, u32 msg_hdr)
{
+ u64 ts;
struct scmi_xfer *xfer;
struct device *dev = cinfo->dev;
struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
struct scmi_xfers_info *minfo = &info->rx_minfo;
+ ts = ktime_get_boottime_ns();
xfer = scmi_xfer_get(cinfo->handle, minfo);
if (IS_ERR(xfer)) {
dev_err(dev, "failed to get free message slot (%ld)\n",
@@ -221,6 +224,8 @@ static void scmi_handle_notification(struct scmi_chan_info *cinfo, u32 msg_hdr)
scmi_dump_header_dbg(dev, &xfer->hdr);
info->desc->ops->fetch_notification(cinfo, info->desc->max_msg_size,
xfer);
+ scmi_notify(xfer->hdr.protocol_id, xfer->hdr.id, xfer->rx.buf,
+ xfer->rx.len, ts);
trace_scmi_rx_done(xfer->transfer_id, xfer->hdr.id,
xfer->hdr.protocol_id, xfer->hdr.seq,
@@ -771,6 +776,9 @@ static int scmi_probe(struct platform_device *pdev)
if (ret)
return ret;
+ if (scmi_notification_init(handle))
+ dev_err(dev, "SCMI Notifications NOT available.\n");
+
ret = scmi_base_protocol_init(handle);
if (ret) {
dev_err(dev, "unable to communicate with SCMI(%d)\n", ret);
@@ -813,6 +821,8 @@ static int scmi_remove(struct platform_device *pdev)
struct scmi_info *info = platform_get_drvdata(pdev);
struct idr *idr = &info->tx_idr;
+ scmi_notification_exit();
+
mutex_lock(&scmi_list_mutex);
if (info->users)
ret = -EBUSY;
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 09/13] firmware: arm_scmi: Add Power notifications support
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (7 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 08/13] firmware: arm_scmi: Enable notification core Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 10/13] firmware: arm_scmi: Add Perf " Cristian Marussi
` (4 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Make SCMI Power protocol register with the notification core.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/power.c | 127 +++++++++++++++++++++++++++++-
include/linux/scmi_protocol.h | 15 ++++
2 files changed, 141 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c
index cf7f0312381b..8bd073bcdf1c 100644
--- a/drivers/firmware/arm_scmi/power.c
+++ b/drivers/firmware/arm_scmi/power.c
@@ -6,6 +6,7 @@
*/
#include "common.h"
+#include "notify.h"
enum scmi_power_protocol_cmd {
POWER_DOMAIN_ATTRIBUTES = 0x3,
@@ -48,6 +49,12 @@ struct scmi_power_state_notify {
__le32 notify_enable;
};
+struct scmi_power_state_notify_payld {
+ __le32 agent_id;
+ __le32 domain_id;
+ __le32 power_state;
+};
+
struct power_dom_info {
bool state_set_sync;
bool state_set_async;
@@ -61,6 +68,14 @@ struct scmi_power_info {
u64 stats_addr;
u32 stats_size;
struct power_dom_info *dom_info;
+ const struct scmi_handle *handle;
+};
+
+static struct scmi_power_info *pinfo;
+
+static enum scmi_power_protocol_cmd evt_2_cmd[] = {
+ POWER_STATE_NOTIFY,
+ POWER_STATE_CHANGE_REQUESTED_NOTIFY,
};
static int scmi_power_attributes_get(const struct scmi_handle *handle,
@@ -186,11 +201,114 @@ static struct scmi_power_ops power_ops = {
.state_get = scmi_power_state_get,
};
+static int scmi_power_request_notify(const struct scmi_handle *handle,
+ u32 domain, int message_id, bool enable)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_power_state_notify *notify;
+
+ ret = scmi_xfer_get_init(handle, message_id, SCMI_PROTOCOL_POWER,
+ sizeof(*notify), 0, &t);
+ if (ret)
+ return ret;
+
+ notify = t->tx.buf;
+ notify->domain = cpu_to_le32(domain);
+ notify->notify_enable = enable ? cpu_to_le32(BIT(0)) : 0;
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static bool scmi_power_set_notify_enabled(u8 evt_id, u32 src_id, bool enable)
+{
+ int ret, cmd_id;
+
+ cmd_id = MAP_EVT_TO_ENABLE_CMD(evt_id, evt_2_cmd);
+ if (cmd_id < 0)
+ return false;
+
+ ret = scmi_power_request_notify(pinfo->handle, src_id, cmd_id, enable);
+ if (ret)
+ pr_warn("SCMI Notifications - Proto:%X - FAIL_ENABLE - evt[%X] dom[%d] - ret:%d\n",
+ SCMI_PROTOCOL_POWER, evt_id, src_id, ret);
+
+ return !ret ? true : false;
+}
+
+static void *scmi_power_fill_custom_report(u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ void *rep = NULL;
+
+ switch (evt_id) {
+ case POWER_STATE_CHANGED:
+ {
+ const struct scmi_power_state_notify_payld *p = payld;
+ struct scmi_power_state_changed_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ break;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->domain_id = le32_to_cpu(p->domain_id);
+ r->power_state = le32_to_cpu(p->power_state);
+ *src_id = r->domain_id;
+ rep = r;
+ break;
+ }
+ case POWER_STATE_CHANGE_REQUESTED:
+ {
+ const struct scmi_power_state_notify_payld *p = payld;
+ struct scmi_power_state_change_requested_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ break;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->domain_id = le32_to_cpu(p->domain_id);
+ r->power_state = le32_to_cpu(p->power_state);
+ *src_id = r->domain_id;
+ rep = r;
+ break;
+ }
+ default:
+ break;
+ }
+
+ return rep;
+}
+
+static const struct scmi_event power_events[] = {
+ {
+ .evt_id = POWER_STATE_CHANGED,
+ .max_payld_sz = 12,
+ .max_report_sz =
+ sizeof(struct scmi_power_state_changed_report),
+ },
+ {
+ .evt_id = POWER_STATE_CHANGE_REQUESTED,
+ .max_payld_sz = 12,
+ .max_report_sz =
+ sizeof(struct scmi_power_state_change_requested_report),
+ },
+};
+
+static const struct scmi_protocol_event_ops power_event_ops = {
+ .set_notify_enabled = scmi_power_set_notify_enabled,
+ .fill_custom_report = scmi_power_fill_custom_report,
+};
+
static int scmi_power_protocol_init(struct scmi_handle *handle)
{
int domain;
u32 version;
- struct scmi_power_info *pinfo;
scmi_version_get(handle, SCMI_PROTOCOL_POWER, &version);
@@ -214,7 +332,14 @@ static int scmi_power_protocol_init(struct scmi_handle *handle)
scmi_power_domain_attributes_get(handle, domain, dom);
}
+ scmi_register_protocol_events(handle->dev,
+ SCMI_PROTOCOL_POWER, PAGE_SIZE,
+ &power_event_ops, power_events,
+ ARRAY_SIZE(power_events),
+ pinfo->num_domains);
+
pinfo->version = version;
+ pinfo->handle = handle;
handle->power_ops = &power_ops;
handle->power_priv = pinfo;
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index f675f2aa7d87..3ec9f1ffa1bf 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -9,6 +9,7 @@
#define _LINUX_SCMI_PROTOCOL_H
#include <linux/device.h>
+#include <linux/ktime.h>
#include <linux/notifier.h>
#include <linux/types.h>
@@ -381,4 +382,18 @@ int scmi_register_event_notifier(u8 proto_id, u8 evt_id, u32 *src_id,
int scmi_unregister_event_notifier(u8 proto_id, u8 evt_id, u32 *src_id,
struct notifier_block *nb);
+struct scmi_power_state_changed_report {
+ ktime_t timestamp;
+ u32 agent_id;
+ u32 domain_id;
+ u32 power_state;
+};
+
+struct scmi_power_state_change_requested_report {
+ ktime_t timestamp;
+ u32 agent_id;
+ u32 domain_id;
+ u32 power_state;
+};
+
#endif /* _LINUX_SCMI_PROTOCOL_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 10/13] firmware: arm_scmi: Add Perf notifications support
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (8 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 09/13] firmware: arm_scmi: Add Power notifications support Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 11/13] firmware: arm_scmi: Add Sensor " Cristian Marussi
` (3 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Make SCMI Perf protocol register with the notification core.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/perf.c | 135 ++++++++++++++++++++++++++++++-
include/linux/scmi_protocol.h | 15 ++++
2 files changed, 149 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
index 88509ec637d0..c427dcfb99e4 100644
--- a/drivers/firmware/arm_scmi/perf.c
+++ b/drivers/firmware/arm_scmi/perf.c
@@ -14,6 +14,7 @@
#include <linux/sort.h>
#include "common.h"
+#include "notify.h"
enum scmi_performance_protocol_cmd {
PERF_DOMAIN_ATTRIBUTES = 0x3,
@@ -86,6 +87,19 @@ struct scmi_perf_notify_level_or_limits {
__le32 notify_enable;
};
+struct scmi_perf_limits_notify_payld {
+ __le32 agent_id;
+ __le32 domain_id;
+ __le32 range_max;
+ __le32 range_min;
+};
+
+struct scmi_perf_level_notify_payld {
+ __le32 agent_id;
+ __le32 domain_id;
+ __le32 performance_level;
+};
+
struct scmi_msg_resp_perf_describe_levels {
__le16 num_returned;
__le16 num_remaining;
@@ -156,6 +170,14 @@ struct scmi_perf_info {
u64 stats_addr;
u32 stats_size;
struct perf_dom_info *dom_info;
+ const struct scmi_handle *handle;
+};
+
+static struct scmi_perf_info *pinfo;
+
+static enum scmi_performance_protocol_cmd evt_2_cmd[] = {
+ PERF_NOTIFY_LIMITS,
+ PERF_NOTIFY_LEVEL,
};
static int scmi_perf_attributes_get(const struct scmi_handle *handle,
@@ -488,6 +510,29 @@ static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain,
return scmi_perf_mb_level_get(handle, domain, level, poll);
}
+static int scmi_perf_level_limits_notify(const struct scmi_handle *handle,
+ u32 domain, int message_id,
+ bool enable)
+{
+ int ret;
+ struct scmi_xfer *t;
+ struct scmi_perf_notify_level_or_limits *notify;
+
+ ret = scmi_xfer_get_init(handle, message_id, SCMI_PROTOCOL_PERF,
+ sizeof(*notify), 0, &t);
+ if (ret)
+ return ret;
+
+ notify = t->tx.buf;
+ notify->domain = cpu_to_le32(domain);
+ notify->notify_enable = enable ? cpu_to_le32(BIT(0)) : 0;
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
static bool scmi_perf_fc_size_is_valid(u32 msg, u32 size)
{
if ((msg == PERF_LEVEL_GET || msg == PERF_LEVEL_SET) && size == 4)
@@ -710,11 +755,92 @@ static struct scmi_perf_ops perf_ops = {
.est_power_get = scmi_dvfs_est_power_get,
};
+static bool scmi_perf_set_notify_enabled(u8 evt_id, u32 src_id, bool enable)
+{
+ int ret, cmd_id;
+
+ cmd_id = MAP_EVT_TO_ENABLE_CMD(evt_id, evt_2_cmd);
+ if (cmd_id < 0)
+ return false;
+
+ ret = scmi_perf_level_limits_notify(pinfo->handle,
+ src_id, cmd_id, enable);
+ if (ret)
+ pr_warn("SCMI Notifications - Proto:%X - FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
+ SCMI_PROTOCOL_PERF, evt_id, src_id, ret);
+
+ return !ret ? true : false;
+}
+
+static void *scmi_perf_fill_custom_report(u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ void *rep = NULL;
+
+ switch (evt_id) {
+ case PERFORMANCE_LIMITS_CHANGED:
+ {
+ const struct scmi_perf_limits_notify_payld *p = payld;
+ struct scmi_perf_limits_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ break;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->domain_id = le32_to_cpu(p->domain_id);
+ r->range_max = le32_to_cpu(p->range_max);
+ r->range_min = le32_to_cpu(p->range_min);
+ *src_id = r->domain_id;
+ rep = r;
+ break;
+ }
+ case PERFORMANCE_LEVEL_CHANGED:
+ {
+ const struct scmi_perf_level_notify_payld *p = payld;
+ struct scmi_perf_level_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ break;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->domain_id = le32_to_cpu(p->domain_id);
+ r->performance_level = le32_to_cpu(p->performance_level);
+ *src_id = r->domain_id;
+ rep = r;
+ break;
+ }
+ default:
+ break;
+ }
+
+ return rep;
+}
+
+static const struct scmi_event perf_events[] = {
+ {
+ .evt_id = PERFORMANCE_LIMITS_CHANGED,
+ .max_payld_sz = 16,
+ .max_report_sz = sizeof(struct scmi_perf_limits_report),
+ },
+ {
+ .evt_id = PERFORMANCE_LEVEL_CHANGED,
+ .max_payld_sz = 12,
+ .max_report_sz = sizeof(struct scmi_perf_level_report),
+ },
+};
+
+static const struct scmi_protocol_event_ops perf_event_ops = {
+ .set_notify_enabled = scmi_perf_set_notify_enabled,
+ .fill_custom_report = scmi_perf_fill_custom_report,
+};
+
static int scmi_perf_protocol_init(struct scmi_handle *handle)
{
int domain;
u32 version;
- struct scmi_perf_info *pinfo;
scmi_version_get(handle, SCMI_PROTOCOL_PERF, &version);
@@ -742,7 +868,14 @@ static int scmi_perf_protocol_init(struct scmi_handle *handle)
scmi_perf_domain_init_fc(handle, domain, &dom->fc_info);
}
+ scmi_register_protocol_events(handle->dev,
+ SCMI_PROTOCOL_PERF, PAGE_SIZE,
+ &perf_event_ops, perf_events,
+ ARRAY_SIZE(perf_events),
+ pinfo->num_domains);
+
pinfo->version = version;
+ pinfo->handle = handle;
handle->perf_ops = &perf_ops;
handle->perf_priv = pinfo;
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 3ec9f1ffa1bf..da4f8bfda168 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -396,4 +396,19 @@ struct scmi_power_state_change_requested_report {
u32 power_state;
};
+struct scmi_perf_limits_report {
+ ktime_t timestamp;
+ u32 agent_id;
+ u32 domain_id;
+ u32 range_max;
+ u32 range_min;
+};
+
+struct scmi_perf_level_report {
+ ktime_t timestamp;
+ u32 agent_id;
+ u32 domain_id;
+ u32 performance_level;
+};
+
#endif /* _LINUX_SCMI_PROTOCOL_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 11/13] firmware: arm_scmi: Add Sensor notifications support
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (9 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 10/13] firmware: arm_scmi: Add Perf " Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 12/13] firmware: arm_scmi: Add Reset " Cristian Marussi
` (2 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Make SCMI Sensor protocol register with the notification core.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/sensors.c | 73 ++++++++++++++++++++++++++++-
include/linux/scmi_protocol.h | 7 +++
2 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c
index db1b1ab303da..99c5c67a9b6d 100644
--- a/drivers/firmware/arm_scmi/sensors.c
+++ b/drivers/firmware/arm_scmi/sensors.c
@@ -6,6 +6,7 @@
*/
#include "common.h"
+#include "notify.h"
enum scmi_sensor_protocol_cmd {
SENSOR_DESCRIPTION_GET = 0x3,
@@ -71,6 +72,12 @@ struct scmi_msg_sensor_reading_get {
#define SENSOR_READ_ASYNC BIT(0)
};
+struct scmi_sensor_trip_notify_payld {
+ __le32 agent_id;
+ __le32 sensor_id;
+ __le32 trip_point_desc;
+};
+
struct sensors_info {
u32 version;
int num_sensors;
@@ -78,8 +85,11 @@ struct sensors_info {
u64 reg_addr;
u32 reg_size;
struct scmi_sensor_info *sensors;
+ const struct scmi_handle *handle;
};
+static struct sensors_info *sinfo;
+
static int scmi_sensor_attributes_get(const struct scmi_handle *handle,
struct sensors_info *si)
{
@@ -276,10 +286,64 @@ static struct scmi_sensor_ops sensor_ops = {
.reading_get = scmi_sensor_reading_get,
};
+static bool scmi_sensor_set_notify_enabled(u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ ret = scmi_sensor_trip_point_notify(sinfo->handle, src_id, enable);
+ if (ret)
+ pr_warn("SCMI Notifications - Proto:%X - FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
+ SCMI_PROTOCOL_SENSOR, evt_id, src_id, ret);
+
+ return !ret ? true : false;
+}
+
+static void *scmi_sensor_fill_custom_report(u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ void *rep = NULL;
+
+ switch (evt_id) {
+ case SENSOR_TRIP_POINT_EVENT:
+ {
+ const struct scmi_sensor_trip_notify_payld *p = payld;
+ struct scmi_sensor_trip_point_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ break;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->sensor_id = le32_to_cpu(p->sensor_id);
+ r->trip_point_desc = le32_to_cpu(p->trip_point_desc);
+ *src_id = r->sensor_id;
+ rep = r;
+ break;
+ }
+ default:
+ break;
+ }
+
+ return rep;
+}
+
+static const struct scmi_event sensor_events[] = {
+ {
+ .evt_id = SENSOR_TRIP_POINT_EVENT,
+ .max_payld_sz = 12,
+ .max_report_sz = sizeof(struct scmi_sensor_trip_point_report),
+ },
+};
+
+static const struct scmi_protocol_event_ops sensor_event_ops = {
+ .set_notify_enabled = scmi_sensor_set_notify_enabled,
+ .fill_custom_report = scmi_sensor_fill_custom_report,
+};
+
static int scmi_sensors_protocol_init(struct scmi_handle *handle)
{
u32 version;
- struct sensors_info *sinfo;
scmi_version_get(handle, SCMI_PROTOCOL_SENSOR, &version);
@@ -299,7 +363,14 @@ static int scmi_sensors_protocol_init(struct scmi_handle *handle)
scmi_sensor_description_get(handle, sinfo);
+ scmi_register_protocol_events(handle->dev,
+ SCMI_PROTOCOL_SENSOR, PAGE_SIZE,
+ &sensor_event_ops, sensor_events,
+ ARRAY_SIZE(sensor_events),
+ sinfo->num_sensors);
+
sinfo->version = version;
+ sinfo->handle = handle;
handle->sensor_ops = &sensor_ops;
handle->sensor_priv = sinfo;
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index da4f8bfda168..77cf107a7391 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -411,4 +411,11 @@ struct scmi_perf_level_report {
u32 performance_level;
};
+struct scmi_sensor_trip_point_report {
+ ktime_t timestamp;
+ u32 agent_id;
+ u32 sensor_id;
+ u32 trip_point_desc;
+};
+
#endif /* _LINUX_SCMI_PROTOCOL_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 12/13] firmware: arm_scmi: Add Reset notifications support
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (10 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 11/13] firmware: arm_scmi: Add Sensor " Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-14 15:35 ` [RFC PATCH v2 13/13] firmware: arm_scmi: Add Base " Cristian Marussi
2020-02-18 20:19 ` [RFC PATCH v2 00/13] SCMI Notifications Core Support Jim Quinlan
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Make SCMI Reset protocol register with the notification core.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/reset.c | 100 +++++++++++++++++++++++++++++-
include/linux/scmi_protocol.h | 6 ++
2 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/reset.c b/drivers/firmware/arm_scmi/reset.c
index de73054554f3..4316d772a0cf 100644
--- a/drivers/firmware/arm_scmi/reset.c
+++ b/drivers/firmware/arm_scmi/reset.c
@@ -6,6 +6,7 @@
*/
#include "common.h"
+#include "notify.h"
enum scmi_reset_protocol_cmd {
RESET_DOMAIN_ATTRIBUTES = 0x3,
@@ -40,6 +41,17 @@ struct scmi_msg_reset_domain_reset {
#define ARCH_COLD_RESET (ARCH_RESET_TYPE | COLD_RESET_STATE)
};
+struct scmi_msg_reset_notify {
+ __le32 id;
+ __le32 event_control;
+#define RESET_TP_NOTIFY_ALL BIT(0)
+};
+
+struct scmi_reset_issued_notify_payld {
+ __le32 domain_id;
+ __le32 reset_state;
+};
+
struct reset_dom_info {
bool async_reset;
bool reset_notify;
@@ -51,8 +63,11 @@ struct scmi_reset_info {
u32 version;
int num_domains;
struct reset_dom_info *dom_info;
+ const struct scmi_handle *handle;
};
+static struct scmi_reset_info *pinfo;
+
static int scmi_reset_attributes_get(const struct scmi_handle *handle,
struct scmi_reset_info *pi)
{
@@ -190,11 +205,87 @@ static struct scmi_reset_ops reset_ops = {
.deassert = scmi_reset_domain_deassert,
};
+static int scmi_reset_notify(const struct scmi_handle *handle, u32 domain_id,
+ bool enable)
+{
+ int ret;
+ u32 evt_cntl = enable ? RESET_TP_NOTIFY_ALL : 0;
+ struct scmi_xfer *t;
+ struct scmi_msg_reset_notify *cfg;
+
+ ret = scmi_xfer_get_init(handle, RESET_NOTIFY,
+ SCMI_PROTOCOL_RESET, sizeof(*cfg), 0, &t);
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->id = cpu_to_le32(domain_id);
+ cfg->event_control = cpu_to_le32(evt_cntl);
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static bool scmi_reset_set_notify_enabled(u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ ret = scmi_reset_notify(pinfo->handle, src_id, enable);
+ if (ret)
+ pr_warn("SCMI Notifications - Proto:%X - FAIL_ENABLED - evt[%X] dom[%d] - ret:%d\n",
+ SCMI_PROTOCOL_RESET, evt_id, src_id, ret);
+
+ return !ret ? true : false;
+}
+
+static void *scmi_reset_fill_custom_report(u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ void *rep = NULL;
+
+ switch (evt_id) {
+ case RESET_ISSUED:
+ {
+ const struct scmi_reset_issued_notify_payld *p = payld;
+ struct scmi_reset_issued_report *r = report;
+
+ if (sizeof(*p) != payld_sz)
+ break;
+
+ r->timestamp = timestamp;
+ r->domain_id = le32_to_cpu(p->domain_id);
+ r->reset_state = le32_to_cpu(p->reset_state);
+ *src_id = r->domain_id;
+ rep = r;
+ break;
+ }
+ default:
+ break;
+ }
+
+ return rep;
+}
+
+static const struct scmi_event reset_events[] = {
+ {
+ .evt_id = RESET_NOTIFY,
+ .max_payld_sz = 8,
+ .max_report_sz = sizeof(struct scmi_reset_issued_report),
+ },
+};
+
+static const struct scmi_protocol_event_ops reset_event_ops = {
+ .set_notify_enabled = scmi_reset_set_notify_enabled,
+ .fill_custom_report = scmi_reset_fill_custom_report,
+};
+
static int scmi_reset_protocol_init(struct scmi_handle *handle)
{
int domain;
u32 version;
- struct scmi_reset_info *pinfo;
scmi_version_get(handle, SCMI_PROTOCOL_RESET, &version);
@@ -218,7 +309,14 @@ static int scmi_reset_protocol_init(struct scmi_handle *handle)
scmi_reset_domain_attributes_get(handle, domain, dom);
}
+ scmi_register_protocol_events(handle->dev,
+ SCMI_PROTOCOL_RESET, PAGE_SIZE,
+ &reset_event_ops, reset_events,
+ ARRAY_SIZE(reset_events),
+ pinfo->num_domains);
+
pinfo->version = version;
+ pinfo->handle = handle;
handle->reset_ops = &reset_ops;
handle->reset_priv = pinfo;
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 77cf107a7391..e6d4fc01d560 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -418,4 +418,10 @@ struct scmi_sensor_trip_point_report {
u32 trip_point_desc;
};
+struct scmi_reset_issued_report {
+ ktime_t timestamp;
+ u32 domain_id;
+ u32 reset_state;
+};
+
#endif /* _LINUX_SCMI_PROTOCOL_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [RFC PATCH v2 13/13] firmware: arm_scmi: Add Base notifications support
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (11 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 12/13] firmware: arm_scmi: Add Reset " Cristian Marussi
@ 2020-02-14 15:35 ` Cristian Marussi
2020-02-18 20:19 ` [RFC PATCH v2 00/13] SCMI Notifications Core Support Jim Quinlan
13 siblings, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-14 15:35 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel
Cc: sudeep.holla, lukasz.luba, james.quinlan, Jonathan.Cameron,
cristian.marussi
Make SCMI Base protocol register with the notification core.
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
V1 --> V2
- simplified .set_notify_enabled() implementation moving the ALL_SRCIDs
logic out of protocol. ALL_SRCIDs logic is now in charge of the
notification core, together with proper reference counting of enables
- switched to devres protocol-registration
---
drivers/firmware/arm_scmi/base.c | 114 +++++++++++++++++++++++++++++++
include/linux/scmi_protocol.h | 8 +++
2 files changed, 122 insertions(+)
diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c
index ce7d9203e41b..df9fae9a0cff 100644
--- a/drivers/firmware/arm_scmi/base.c
+++ b/drivers/firmware/arm_scmi/base.c
@@ -6,6 +6,9 @@
*/
#include "common.h"
+#include "notify.h"
+
+#define SCMI_BASE_NUM_SOURCES 1
enum scmi_base_protocol_cmd {
BASE_DISCOVER_VENDOR = 0x3,
@@ -29,6 +32,21 @@ struct scmi_msg_resp_base_attributes {
__le16 reserved;
};
+struct scmi_msg_base_error_notify {
+ __le32 event_control;
+#define BASE_TP_NOTIFY_ALL BIT(0)
+};
+
+struct scmi_base_error_notify_payld {
+ __le32 agent_id;
+ __le32 error_status;
+#define IS_FATAL_ERROR(x) ((x) & BIT(31))
+#define ERROR_CMD_COUNT(x) FIELD_GET(GENMASK(9, 0), (x))
+ __le64 msg_reports[8192];
+};
+
+static const struct scmi_handle *protocol_handle;
+
/**
* scmi_base_attributes_get() - gets the implementation details
* that are associated with the base protocol.
@@ -222,6 +240,95 @@ static int scmi_base_discover_agent_get(const struct scmi_handle *handle,
return ret;
}
+static int scmi_base_error_notify(const struct scmi_handle *handle, bool enable)
+{
+ int ret;
+ u32 evt_cntl = enable ? BASE_TP_NOTIFY_ALL : 0;
+ struct scmi_xfer *t;
+ struct scmi_msg_base_error_notify *cfg;
+
+ ret = scmi_xfer_get_init(handle, BASE_NOTIFY_ERRORS,
+ SCMI_PROTOCOL_BASE, sizeof(*cfg), 0, &t);
+ if (ret)
+ return ret;
+
+ cfg = t->tx.buf;
+ cfg->event_control = cpu_to_le32(evt_cntl);
+
+ ret = scmi_do_xfer(handle, t);
+
+ scmi_xfer_put(handle, t);
+ return ret;
+}
+
+static bool scmi_base_set_notify_enabled(u8 evt_id, u32 src_id, bool enable)
+{
+ int ret;
+
+ if (!protocol_handle)
+ return false;
+
+ ret = scmi_base_error_notify(protocol_handle, enable);
+ if (ret)
+ pr_warn("SCMI Notifications - Proto:%X - FAIL_ENABLED - evt[%X] ret:%d\n",
+ SCMI_PROTOCOL_BASE, evt_id, ret);
+
+ return !ret ? true : false;
+}
+
+static void *scmi_base_fill_custom_report(u8 evt_id, u64 timestamp,
+ const void *payld, size_t payld_sz,
+ void *report, u32 *src_id)
+{
+ void *rep = NULL;
+
+ switch (evt_id) {
+ case BASE_ERROR_EVENT:
+ {
+ int i;
+ const struct scmi_base_error_notify_payld *p = payld;
+ struct scmi_base_error_report *r = report;
+
+ /*
+ * BaseError notification payload is variable in size but
+ * up to a maximum length determined by the struct ponted by p.
+ * Instead payld_sz is the effective length of this notification
+ * payload so cannot be greater of the maximum allowed size as
+ * pointed by p.
+ */
+ if (sizeof(*p) < payld_sz)
+ break;
+
+ r->timestamp = timestamp;
+ r->agent_id = le32_to_cpu(p->agent_id);
+ r->fatal = IS_FATAL_ERROR(le32_to_cpu(p->error_status));
+ r->cmd_count = ERROR_CMD_COUNT(le32_to_cpu(p->error_status));
+ for (i = 0; i < r->cmd_count; i++)
+ r->reports[i] = le64_to_cpu(p->msg_reports[i]);
+ *src_id = 0;
+ rep = r;
+ break;
+ }
+ default:
+ break;
+ }
+
+ return rep;
+}
+
+static const struct scmi_event base_events[] = {
+ {
+ .evt_id = BASE_ERROR_EVENT,
+ .max_payld_sz = 8192,
+ .max_report_sz = sizeof(struct scmi_base_error_report),
+ },
+};
+
+static const struct scmi_protocol_event_ops base_event_ops = {
+ .set_notify_enabled = scmi_base_set_notify_enabled,
+ .fill_custom_report = scmi_base_fill_custom_report,
+};
+
int scmi_base_protocol_init(struct scmi_handle *h)
{
int id, ret;
@@ -256,10 +363,17 @@ int scmi_base_protocol_init(struct scmi_handle *h)
dev_dbg(dev, "Found %d protocol(s) %d agent(s)\n", rev->num_protocols,
rev->num_agents);
+ scmi_register_protocol_events(handle->dev,
+ SCMI_PROTOCOL_BASE, (4 * PAGE_SIZE),
+ &base_event_ops, base_events,
+ ARRAY_SIZE(base_events),
+ SCMI_BASE_NUM_SOURCES);
+
for (id = 0; id < rev->num_agents; id++) {
scmi_base_discover_agent_get(handle, id, name);
dev_dbg(dev, "Agent %d: %s\n", id, name);
}
+ protocol_handle = handle;
return 0;
}
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index e6d4fc01d560..16b184a36c22 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -424,4 +424,12 @@ struct scmi_reset_issued_report {
u32 reset_state;
};
+struct scmi_base_error_report {
+ ktime_t timestamp;
+ u32 agent_id;
+ bool fatal;
+ u16 cmd_count;
+ u64 reports[8192];
+};
+
#endif /* _LINUX_SCMI_PROTOCOL_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [RFC PATCH v2 00/13] SCMI Notifications Core Support
2020-02-14 15:35 [RFC PATCH v2 00/13] SCMI Notifications Core Support Cristian Marussi
` (12 preceding siblings ...)
2020-02-14 15:35 ` [RFC PATCH v2 13/13] firmware: arm_scmi: Add Base " Cristian Marussi
@ 2020-02-18 20:19 ` Jim Quinlan
2020-02-19 17:43 ` Cristian Marussi
2020-02-24 14:40 ` Cristian Marussi
13 siblings, 2 replies; 23+ messages in thread
From: Jim Quinlan @ 2020-02-18 20:19 UTC (permalink / raw)
To: Cristian Marussi
Cc: linux-kernel, linux-arm-kernel, Sudeep Holla, Lukasz Luba,
Jonathan.Cameron
Hi Cristian,
This looks very robust and general but I'm wondering about the
"notification -> user" latency. It appears there are at least five or
so memcpy()s of the notification message as it works its way to the
user, and added to that is the work queue deferral latency. Is there
a concern on your end for low latency notifications, perhaps for
emergency notifications or do you consider this to be quick enough?
Note that we (BrcmSTB) have implemented our own SCMI notification
system for a proprietary protocol and we designed it to deliver
notifications as quickly as possible. Our system is more or less a
disposable hack and would not stand up to heavy or general usage. We
fully intend to move to the approved Linux notification system. On
that note, I'm just wondering if you had any comment on the
possibility of "slimming down" your RFC, e.g. perhaps somehow
collapsing some memcpy()s.
Thanks,
Jim
PS I'm going on vacation so I won't be able to email for a week.
On Fri, Feb 14, 2020 at 10:36 AM Cristian Marussi
<cristian.marussi@arm.com> wrote:
>
> Hi all,
>
> this series wants to introduce SCMI Notification Support, built on top of
> the standard Kernel notification chain subsystem.
>
> At initialization time each SCMI Protocol takes care to register with the
> new SCMI notification core the set of its own events which it intends to
> support.
>
> Using a possibly proposed API in include/linux/scmi_protocol.h (not
> finalized though, NO EXPORTs_) a Kernel user can register its own
> notifier_t callback (via a notifier_block as usual) against any registered
> event as identified by the tuple:
>
> (proto_id, event_id, src_id)
>
> where src_id represents a generic source identifier which is protocol
> dependent like domain_id, performance_id, sensor_id and so forth.
> (users can anyway do NOT provide any src_id, and subscribe instead to ALL
> the existing (if any) src_id sources for that proto_id/evt_id combination)
>
> Each of the above tuple-specified event will be served on its own dedicated
> blocking notification chain.
>
> Upon a notification delivery all the users' registered notifier_t callbacks
> will be in turn invoked and fed with the event_id as @action param and a
> generated custom per-event struct _report as @data param.
> (as in include/linux/scmi_protocol.h)
>
> The final step of notification delivery via users' callback invocation is
> instead delegated to a pool of deferred workers (Kernel cmwq): each
> SCMI protocol has its own dedicated worker and dedicated queue to push
> events from the rx ISR to the worker.
>
> The series is marked as RFC mainly because:
>
> - the API as said is tentative and not EXPORTed; currently consisting of a
> generic interface like:
>
> scmi_register_event_notifier(proto_id, evt_id, *src_id, *nb)
>
> as found in scmi_protocol.h, or using the equivalent 'handle' operations
> in scmi_notify_ops if used by an scmi_driver.
>
> It's open for discussion.
>
> - no Event priorization has been considered: each protocol has its own
> queue and deferred worker instance, so as to avoid that one protocol
> flood can overrun a single queue and influence other protocols'
> notifications' delivery.
> But that's it, all the workers are unbound, low_pri cmwq workers.
>
> Should we enforce some sort of built-in prio amongst the events ?
> Should this priority instead be compile time configurable ?
>
> Again, open for discussion.
>
> - no configuration is possible: it can be imagined that on a real platform
> events' priority (if any) and events queues' depth could be something
> somehow compile-time configurable, but this is not addressed by this
> series at all.
>
> Based on scmi-next 5.6 [1], on top of:
>
> commit 5c8a47a5a91d ("firmware: arm_scmi: Make scmi core independent of
> the transport type")
>
> This series has been tested on JUNO with an experimental firmware only
> supporting Perf Notifications.
>
> Any thoughts ?
>
> Thanks
>
> Cristian
> ----
>
> v1 --> v2:
> - dropped anti-tampering patch
> - rebased on top of scmi-for-next-5.6, which includes Viresh series that
> make SCMI core independent of transport (5c8a47a5a91d)
> - add a few new SCMI transport methods on top of Viresh patch to address
> needs of SCMI Notifications
> - reviewed/renamed scmi_handle_xfer_delayed_resp()
> - split main SCMI Notification core patch (~1k lines) into three chunks:
> protocol-registration / callbacks-registration / dispatch-and-delivery
> - removed awkward usage of IDR maps in favour of pure hashtables
> - added enable/disable refcounting in notification core (was broken in v1)
> - removed per-protocol candidate API: a single generic API is now proposed
> instead of scmi_register_<proto>_event_notifier(evt_id, *src_id, *nb)
> - added handle->notify_ops as an alternative notification API
> for scmi_driver
> - moved ALL_SRCIDs enabled handling from protocol code to core code
> - reviewed protocol registration/unregistration logic to use devres
> - reviewed cleanup phase on shutdown
> - fixed ERROR: reference preceded by free as reported by kbuild test robot
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
>
> Cristian Marussi (10):
> firmware: arm_scmi: Add notifications support in transport layer
> firmware: arm_scmi: Add notification protocol-registration
> firmware: arm_scmi: Add notification callbacks-registration
> firmware: arm_scmi: Add notification dispatch and delivery
> firmware: arm_scmi: Enable notification core
> firmware: arm_scmi: Add Power notifications support
> firmware: arm_scmi: Add Perf notifications support
> firmware: arm_scmi: Add Sensor notifications support
> firmware: arm_scmi: Add Reset notifications support
> firmware: arm_scmi: Add Base notifications support
>
> Sudeep Holla (3):
> firmware: arm_scmi: Add receive buffer support for notifications
> firmware: arm_scmi: Update protocol commands and notification list
> firmware: arm_scmi: Add support for notifications message processing
>
> drivers/firmware/arm_scmi/Makefile | 2 +-
> drivers/firmware/arm_scmi/base.c | 121 +++
> drivers/firmware/arm_scmi/bus.c | 11 +
> drivers/firmware/arm_scmi/common.h | 12 +
> drivers/firmware/arm_scmi/driver.c | 118 ++-
> drivers/firmware/arm_scmi/mailbox.c | 17 +
> drivers/firmware/arm_scmi/notify.c | 1102 +++++++++++++++++++++++++++
> drivers/firmware/arm_scmi/notify.h | 78 ++
> drivers/firmware/arm_scmi/perf.c | 140 +++-
> drivers/firmware/arm_scmi/power.c | 133 +++-
> drivers/firmware/arm_scmi/reset.c | 100 ++-
> drivers/firmware/arm_scmi/sensors.c | 77 +-
> drivers/firmware/arm_scmi/shmem.c | 15 +
> include/linux/scmi_protocol.h | 114 +++
> 14 files changed, 2009 insertions(+), 31 deletions(-)
> create mode 100644 drivers/firmware/arm_scmi/notify.c
> create mode 100644 drivers/firmware/arm_scmi/notify.h
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH v2 00/13] SCMI Notifications Core Support
2020-02-18 20:19 ` [RFC PATCH v2 00/13] SCMI Notifications Core Support Jim Quinlan
@ 2020-02-19 17:43 ` Cristian Marussi
2020-02-24 14:40 ` Cristian Marussi
1 sibling, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-19 17:43 UTC (permalink / raw)
To: Jim Quinlan
Cc: linux-kernel, linux-arm-kernel, Sudeep Holla, Lukasz Luba,
Jonathan.Cameron
Hi Jim
and thanks for the feedback first of all.
On 18/02/2020 20:19, Jim Quinlan wrote:
> Hi Cristian,
>
> This looks very robust and general but I'm wondering about the
> "notification -> user" latency. It appears there are at least five or
> so memcpy()s of the notification message as it works its way to the
> user, and added to that is the work queue deferral latency. Is there
> a concern on your end for low latency notifications, perhaps for
> emergency notifications or do you consider this to be quick enough?
>
I have to say user-latency was a primary concern when I first started
looking into this, so that at first I had thought about exposing a
dedicated additional method to let the users register callbacks which
would have been called straight away from the RX ISR using an
atomic notification chain.
Reasoning about that, though:
- me being really nervous about letting users register something that I'm
going to call on behalf of them in atomic context (...not bold enough I know :D)
since can potentially be misused and cause slowdowns to the notification
delivery or to the system as a whole
- if something like that must be allowed it must be policed somehow from the
core....what if dozens of callbacks are allowed to be registered in this way
instead of the blocking mode ? they will slow down each other anyway..and
what about relative priorities amongst callbacks inside the same notification
chain ? (potentially registered from different users)
- let's say we allow this and you can register your own atomic callback (which it would
be policed), what you can do with that at the end ? What are the benefits of this
scenario ? AICS there are two possible scenarios (but if you can propose more scenarios
that I've missed you're more than welcome):
1. you can do the smallest-amount-possible-of-work-in-irq-as-usual and defer,
probably attaching a timestamp of arrival to the stuff you push for
deferred processing (and this is what I'm doing already for you..even if
with too much memcpying around probably :D....more on this later...)
OR
2. you can immediately signal the condition just received to an external
processing unit...say like raising a line which will trigger an interrupt
in those agent...and this is a scenario in which I can imagine the atomic
notification chain can benefit (in fact that's the reason I'm still thinking
about it as a controlled/policed alternative)...but, anyway, in this scenario,
you are going to route as quickly as possible a critical SCMI notification to
some external processor for immediate processing...and so you want it to be
propagated as fast as possible...fine..but why passing through the richOS at all ?
if it's so critical shouldn't have been already handled by the platform to the
processing entity bypassing the richOS as a whole and all its inherent latencies ?
(beside my own memcpys :D)
So for this last consideration I gave up thinking about some more performant
atomic delivery of notifications ? But if you can make a case where some sort of
alternative quick atomic (policed) delivery makes sense, I'll be happy to think
about it again.
> Note that we (BrcmSTB) have implemented our own SCMI notification
> system for a proprietary protocol and we designed it to deliver
> notifications as quickly as possible. Our system is more or less a
> disposable hack and would not stand up to heavy or general usage. We
> fully intend to move to the approved Linux notification system. On
> that note, I'm just wondering if you had any comment on the
> possibility of "slimming down" your RFC, e.g. perhaps somehow
> collapsing some memcpy()s.
>
Having said all of the above, I'll certainly review the memcopying to see if I can
redesign an shrink a bit their number. (the only thing that I want to avoid is placing
a spinlock into the ISR side of the dispatching...since now is all lock-free on that side
thanks to the kfifo, the 1-reader-1-writer scnario, and an additional memcpy)
In these regards what I had started to do (but no more), it was to sort of "profiling" a bit
the latency from event RX up to the point immediately before the report is delivered to the users
callbacks, and see if I can improve a bit this path in term of performance reshuffling the code.
In V3 I'm reviewing a lot notification init/exit logic (which was a bit dirty and faulty ...)
and handling of possible different platform instances (which were not handled), I'll see if I
can stick some meaningful fix about memcopies (if not it will be V4)
Moreover on this theme, given your low latencies requirements: as of now I'm using as a backend
the normal Kernel notification chains, which means though, that:
- users are in control of callbacks relative priorities, so that registering a high-pri slow one
could waste any gain and slow-down some other user registered callbacks on the same notification
chain (no matter if atomic or blocking)
- users (maliciously ?) could simply register a callback that return NOTIFY_STOP and to cut the chain
- users can (maliciously or not) mangle the event report fed to your callback along the way,
so that one user can trash potentially the event seen by the next user on the same chain
Do you think the above issues are worth considering to be addressed (for performance/security issues) ?
Because in v1 I had a patch for anti-tampering (now dropped to simplify the code) that could address
all of the above, but I did not know if such policying/anti-tampering attempt was worth an valid.
(even though, I think that if I'd end up adding some emergency/critical fast support, that would need
some sort of callbacks priority policying at least)
Thanks
Regards
Cristian
> Thanks,
> Jim
> PS I'm going on vacation so I won't be able to email for a week.
>
>
> On Fri, Feb 14, 2020 at 10:36 AM Cristian Marussi
> <cristian.marussi@arm.com> wrote:
>>
>> Hi all,
>>
>> this series wants to introduce SCMI Notification Support, built on top of
>> the standard Kernel notification chain subsystem.
>>
>> At initialization time each SCMI Protocol takes care to register with the
>> new SCMI notification core the set of its own events which it intends to
>> support.
>>
>> Using a possibly proposed API in include/linux/scmi_protocol.h (not
>> finalized though, NO EXPORTs_) a Kernel user can register its own
>> notifier_t callback (via a notifier_block as usual) against any registered
>> event as identified by the tuple:
>>
>> (proto_id, event_id, src_id)
>>
>> where src_id represents a generic source identifier which is protocol
>> dependent like domain_id, performance_id, sensor_id and so forth.
>> (users can anyway do NOT provide any src_id, and subscribe instead to ALL
>> the existing (if any) src_id sources for that proto_id/evt_id combination)
>>
>> Each of the above tuple-specified event will be served on its own dedicated
>> blocking notification chain.
>>
>> Upon a notification delivery all the users' registered notifier_t callbacks
>> will be in turn invoked and fed with the event_id as @action param and a
>> generated custom per-event struct _report as @data param.
>> (as in include/linux/scmi_protocol.h)
>>
>> The final step of notification delivery via users' callback invocation is
>> instead delegated to a pool of deferred workers (Kernel cmwq): each
>> SCMI protocol has its own dedicated worker and dedicated queue to push
>> events from the rx ISR to the worker.
>>
>> The series is marked as RFC mainly because:
>>
>> - the API as said is tentative and not EXPORTed; currently consisting of a
>> generic interface like:
>>
>> scmi_register_event_notifier(proto_id, evt_id, *src_id, *nb)
>>
>> as found in scmi_protocol.h, or using the equivalent 'handle' operations
>> in scmi_notify_ops if used by an scmi_driver.
>>
>> It's open for discussion.
>>
>> - no Event priorization has been considered: each protocol has its own
>> queue and deferred worker instance, so as to avoid that one protocol
>> flood can overrun a single queue and influence other protocols'
>> notifications' delivery.
>> But that's it, all the workers are unbound, low_pri cmwq workers.
>>
>> Should we enforce some sort of built-in prio amongst the events ?
>> Should this priority instead be compile time configurable ?
>>
>> Again, open for discussion.
>>
>> - no configuration is possible: it can be imagined that on a real platform
>> events' priority (if any) and events queues' depth could be something
>> somehow compile-time configurable, but this is not addressed by this
>> series at all.
>>
>> Based on scmi-next 5.6 [1], on top of:
>>
>> commit 5c8a47a5a91d ("firmware: arm_scmi: Make scmi core independent of
>> the transport type")
>>
>> This series has been tested on JUNO with an experimental firmware only
>> supporting Perf Notifications.
>>
>> Any thoughts ?
>>
>> Thanks
>>
>> Cristian
>> ----
>>
>> v1 --> v2:
>> - dropped anti-tampering patch
>> - rebased on top of scmi-for-next-5.6, which includes Viresh series that
>> make SCMI core independent of transport (5c8a47a5a91d)
>> - add a few new SCMI transport methods on top of Viresh patch to address
>> needs of SCMI Notifications
>> - reviewed/renamed scmi_handle_xfer_delayed_resp()
>> - split main SCMI Notification core patch (~1k lines) into three chunks:
>> protocol-registration / callbacks-registration / dispatch-and-delivery
>> - removed awkward usage of IDR maps in favour of pure hashtables
>> - added enable/disable refcounting in notification core (was broken in v1)
>> - removed per-protocol candidate API: a single generic API is now proposed
>> instead of scmi_register_<proto>_event_notifier(evt_id, *src_id, *nb)
>> - added handle->notify_ops as an alternative notification API
>> for scmi_driver
>> - moved ALL_SRCIDs enabled handling from protocol code to core code
>> - reviewed protocol registration/unregistration logic to use devres
>> - reviewed cleanup phase on shutdown
>> - fixed ERROR: reference preceded by free as reported by kbuild test robot
>>
>> [1] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
>>
>> Cristian Marussi (10):
>> firmware: arm_scmi: Add notifications support in transport layer
>> firmware: arm_scmi: Add notification protocol-registration
>> firmware: arm_scmi: Add notification callbacks-registration
>> firmware: arm_scmi: Add notification dispatch and delivery
>> firmware: arm_scmi: Enable notification core
>> firmware: arm_scmi: Add Power notifications support
>> firmware: arm_scmi: Add Perf notifications support
>> firmware: arm_scmi: Add Sensor notifications support
>> firmware: arm_scmi: Add Reset notifications support
>> firmware: arm_scmi: Add Base notifications support
>>
>> Sudeep Holla (3):
>> firmware: arm_scmi: Add receive buffer support for notifications
>> firmware: arm_scmi: Update protocol commands and notification list
>> firmware: arm_scmi: Add support for notifications message processing
>>
>> drivers/firmware/arm_scmi/Makefile | 2 +-
>> drivers/firmware/arm_scmi/base.c | 121 +++
>> drivers/firmware/arm_scmi/bus.c | 11 +
>> drivers/firmware/arm_scmi/common.h | 12 +
>> drivers/firmware/arm_scmi/driver.c | 118 ++-
>> drivers/firmware/arm_scmi/mailbox.c | 17 +
>> drivers/firmware/arm_scmi/notify.c | 1102 +++++++++++++++++++++++++++
>> drivers/firmware/arm_scmi/notify.h | 78 ++
>> drivers/firmware/arm_scmi/perf.c | 140 +++-
>> drivers/firmware/arm_scmi/power.c | 133 +++-
>> drivers/firmware/arm_scmi/reset.c | 100 ++-
>> drivers/firmware/arm_scmi/sensors.c | 77 +-
>> drivers/firmware/arm_scmi/shmem.c | 15 +
>> include/linux/scmi_protocol.h | 114 +++
>> 14 files changed, 2009 insertions(+), 31 deletions(-)
>> create mode 100644 drivers/firmware/arm_scmi/notify.c
>> create mode 100644 drivers/firmware/arm_scmi/notify.h
>>
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [RFC PATCH v2 00/13] SCMI Notifications Core Support
2020-02-18 20:19 ` [RFC PATCH v2 00/13] SCMI Notifications Core Support Jim Quinlan
2020-02-19 17:43 ` Cristian Marussi
@ 2020-02-24 14:40 ` Cristian Marussi
1 sibling, 0 replies; 23+ messages in thread
From: Cristian Marussi @ 2020-02-24 14:40 UTC (permalink / raw)
To: Jim Quinlan
Cc: linux-kernel, linux-arm-kernel, Sudeep Holla, Lukasz Luba,
Jonathan.Cameron
Hi Jim
On 18/02/2020 20:19, Jim Quinlan wrote:
> Hi Cristian,
>
> This looks very robust and general but I'm wondering about the
> "notification -> user" latency. It appears there are at least five or
> so memcpy()s of the notification message as it works its way to the
> user, and added to that is the work queue deferral latency. Is there
> a concern on your end for low latency notifications, perhaps for
> emergency notifications or do you consider this to be quick enough?
>
> Note that we (BrcmSTB) have implemented our own SCMI notification
> system for a proprietary protocol and we designed it to deliver
> notifications as quickly as possible. Our system is more or less a
> disposable hack and would not stand up to heavy or general usage. We
> fully intend to move to the approved Linux notification system. On
> that note, I'm just wondering if you had any comment on the
> possibility of "slimming down" your RFC, e.g. perhaps somehow
> collapsing some memcpy()s.
Besides other still open considerations on latency, I spotted one certainly
not strictly needed event-payload-sized memcpy that I'm going to remove so as
to have only the bare minimum bytes copied in and out the queues. (it will
complicate a bit the code in the worker but not dramatically)
I'll do that not in today v3 but in the next v4 (due anyway to solve some
residual initialization corner cases).
Thanks
Regards
Cristian
>
> Thanks,
> Jim
> PS I'm going on vacation so I won't be able to email for a week.
>
>
> On Fri, Feb 14, 2020 at 10:36 AM Cristian Marussi
> <cristian.marussi@arm.com> wrote:
>>
>> Hi all,
>>
>> this series wants to introduce SCMI Notification Support, built on top of
>> the standard Kernel notification chain subsystem.
>>
>> At initialization time each SCMI Protocol takes care to register with the
>> new SCMI notification core the set of its own events which it intends to
>> support.
>>
>> Using a possibly proposed API in include/linux/scmi_protocol.h (not
>> finalized though, NO EXPORTs_) a Kernel user can register its own
>> notifier_t callback (via a notifier_block as usual) against any registered
>> event as identified by the tuple:
>>
>> (proto_id, event_id, src_id)
>>
>> where src_id represents a generic source identifier which is protocol
>> dependent like domain_id, performance_id, sensor_id and so forth.
>> (users can anyway do NOT provide any src_id, and subscribe instead to ALL
>> the existing (if any) src_id sources for that proto_id/evt_id combination)
>>
>> Each of the above tuple-specified event will be served on its own dedicated
>> blocking notification chain.
>>
>> Upon a notification delivery all the users' registered notifier_t callbacks
>> will be in turn invoked and fed with the event_id as @action param and a
>> generated custom per-event struct _report as @data param.
>> (as in include/linux/scmi_protocol.h)
>>
>> The final step of notification delivery via users' callback invocation is
>> instead delegated to a pool of deferred workers (Kernel cmwq): each
>> SCMI protocol has its own dedicated worker and dedicated queue to push
>> events from the rx ISR to the worker.
>>
>> The series is marked as RFC mainly because:
>>
>> - the API as said is tentative and not EXPORTed; currently consisting of a
>> generic interface like:
>>
>> scmi_register_event_notifier(proto_id, evt_id, *src_id, *nb)
>>
>> as found in scmi_protocol.h, or using the equivalent 'handle' operations
>> in scmi_notify_ops if used by an scmi_driver.
>>
>> It's open for discussion.
>>
>> - no Event priorization has been considered: each protocol has its own
>> queue and deferred worker instance, so as to avoid that one protocol
>> flood can overrun a single queue and influence other protocols'
>> notifications' delivery.
>> But that's it, all the workers are unbound, low_pri cmwq workers.
>>
>> Should we enforce some sort of built-in prio amongst the events ?
>> Should this priority instead be compile time configurable ?#git send-email --dry-run --suppress-cc=all --to=linux-kernel@vger.kernel.org,linux-arm-kernel@lists.infradead.org --cc=sudeep.holla@arm.com,lukasz.luba@arm.com,james.quinlan@broadcom.com,Jonathan.Cameron@Huawei.com,cristian.marussi@arm.com patch_scmi_notif/ext_V1/final/
>>
>> Again, open for discussion.
>>
>> - no configuration is possible: it can be imagined that on a real platform
>> events' priority (if any) and events queues' depth could be something
>> somehow compile-time configurable, but this is not addressed by this
>> series at all.
>>
>> Based on scmi-next 5.6 [1], on top of:
>>
>> commit 5c8a47a5a91d ("firmware: arm_scmi: Make scmi core independent of
>> the transport type")
>>
>> This series has been tested on JUNO with an experimental firmware only
>> supporting Perf Notifications.
>>
>> Any thoughts ?
>>
>> Thanks
>>
>> Cristian
>> ----
>>
>> v1 --> v2:
>> - dropped anti-tampering patch
>> - rebased on top of scmi-for-next-5.6, which includes Viresh series that
>> make SCMI core independent of transport (5c8a47a5a91d)
>> - add a few new SCMI transport methods on top of Viresh patch to address
>> needs of SCMI Notifications
>> - reviewed/renamed scmi_handle_xfer_delayed_resp()
>> - split main SCMI Notification core patch (~1k lines) into three chunks:
>> protocol-registration / callbacks-registration / dispatch-and-delivery
>> - removed awkward usage of IDR maps in favour of pure hashtables
>> - added enable/disable refcounting in notification core (was broken in v1)
>> - removed per-protocol candidate API: a single generic API is now proposed
>> instead of scmi_register_<proto>_event_notifier(evt_id, *src_id, *nb)
>> - added handle->notify_ops as an alternative notification API
>> for scmi_driver
>> - moved ALL_SRCIDs enabled handling from protocol code to core code
>> - reviewed protocol registration/unregistration logic to use devres
>> - reviewed cleanup phase on shutdown
>> - fixed ERROR: reference preceded by free as reported by kbuild test robot
>>
>> [1] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
>>
>> Cristian Marussi (10):
>> firmware: arm_scmi: Add notifications support in transport layer
>> firmware: arm_scmi: Add notification protocol-registration
>> firmware: arm_scmi: Add notification callbacks-registration
>> firmware: arm_scmi: Add notification dispatch and delivery
>> firmware: arm_scmi: Enable notification core
>> firmware: arm_scmi: Add Power notifications support
>> firmware: arm_scmi: Add Perf notifications support
>> firmware: arm_scmi: Add Sensor notifications support
>> firmware: arm_scmi: Add Reset notifications support
>> firmware: arm_scmi: Add Base notifications support
>>
>> Sudeep Holla (3):
>> firmware: arm_scmi: Add receive buffer support for notifications
>> firmware: arm_scmi: Update protocol commands and notification list
>> firmware: arm_scmi: Add support for notifications message processing
>>
>> drivers/firmware/arm_scmi/Makefile | 2 +-
>> drivers/firmware/arm_scmi/base.c | 121 +++
>> drivers/firmware/arm_scmi/bus.c | 11 +
>> drivers/firmware/arm_scmi/common.h | 12 +
>> drivers/firmware/arm_scmi/driver.c | 118 ++-
>> drivers/firmware/arm_scmi/mailbox.c | 17 +
>> drivers/firmware/arm_scmi/notify.c | 1102 +++++++++++++++++++++++++++
>> drivers/firmware/arm_scmi/notify.h | 78 ++
>> drivers/firmware/arm_scmi/perf.c | 140 +++-
>> drivers/firmware/arm_scmi/power.c | 133 +++-
>> drivers/firmware/arm_scmi/reset.c | 100 ++-
>> drivers/firmware/arm_scmi/sensors.c | 77 +-
>> drivers/firmware/arm_scmi/shmem.c | 15 +
>> include/linux/scmi_protocol.h | 114 +++
>> 14 files changed, 2009 insertions(+), 31 deletions(-)
>> create mode 100644 drivers/firmware/arm_scmi/notify.c
>> create mode 100644 drivers/firmware/arm_scmi/notify.h
>>
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 23+ messages in thread