Hi all,
This series aims to introduce a new SCMI unified userspace interface meant
to ease testing an SCMI Server implementation for compliance, fuzzing etc.,
from the perspective of the OSPM agent (non-secure world only ...)
It is proposed as a testing/development facility, it is NOT meant to be a
feature to use in production, but only enabled in Kconfig for test
deployments.
Currently an SCMI Compliance Suite like the one at [1] can only work by
injecting SCMI messages at the SCMI transport layer using the mailbox test
driver (CONFIG_MAILBOX_TEST) via its few debugfs entries and looking at
the related replies from the SCMI backend Server.
This approach has a few drawbacks:
- the SCMI Server under test MUST be reachable through a mailbox based
SCMI transport: any other SCMI Server placement is not possible (like in
a VM reachable via SCMI Virtio). In order to cover other placements in
the current scenario we should write some sort of test driver for each
and every existent SCMI transport and for any future additional transport
...this clearly does not scale.
- even in the mailbox case the userspace Compliance suite cannot simply
send and receive bare SCMI messages BUT it has to properly lay them out
into the shared memory exposed by the mailbox test driver as expected by
the transport definitions. In other words such a userspace test
application has to, not only use a proper transport driver for the system
at hand, but it also has to have a comprehensive knowledge of the
internals of the underlying transport in order to operate.
- last but not least, the system under test has to be specifically
configured and built, in terms of Kconfig and DT, to perform such kind of
testing, it cannot be used for anything else, which is unfortunate for
CI/CD deployments.
This series introduces a new SCMI Raw mode support feature that, when
configured and enabled exposes a new interface in debugfs through which:
- a userspace application can inject bare SCMI binary messages into the
SCMI core stack; such messages will be routed by the SCMI regular kernel
stack to the backend Server using the currently configured transport
transparently: in other words you can test the SCMI server, no matter
where it is placed, as long as it is reachable from the currently
configured SCMI stack.
Same goes the other way around on the reading path: any SCMI server reply
can be read as a bare SCMI binary message from the same debugfs path.
- as a direct consequence of this way of injecting bare messages in the
middle of the SCMI stack (instead of beneath it at the transport layer)
the user application has to handle only bare SCMI messages without having
to worry about the specific underlying transport internals that will be
taken care of by the SCMI core stack itself using its own machinery,
without duplicating such logic.
- a system under test, once configured with SCMI Raw support enabled in
Kconfig, can be booted without any particular DT change.
Latest V6 additions:
- improved scmi traces for msg dumps to include used channels
- added a new common SCMI debugfs root fs
- reworked SCMI Raw debugfs layout
- added support of a new additional per-channel API that allows a user to
select a specific egress channel for the message injection (when more
than one channel is available)
A quick and trivial example from the shell...reading from a sensor by
injecting a properly crafted packet in raw mode (letting the stack select
the channel):
# INJECT THE SENSOR_READING MESSAGE FOR SENSOR ID=1 (binary little endian)
root@deb-buster-arm64:~# echo -e -n \\x06\\x54\\x00\\x00\\x01\\x00\\x00\\x00\\x00\\x00\\x00\\x00 > /sys/kernel/debug/scmi/0/raw/message
# READING BACK THE REPLY...
root@deb-buster-arm64:~# cat /sys/kernel/debug/scmi/0/raw/message | od --endian=little -t x4
0000000 00005406 00000000 00000335 00000000
0000020
while doing that, since Raw mode makes (partial) use of the regular SCMI
stack, you can observe the messages going through the SCMI stack with the
usual traces:
bash-329 [000] ..... 14183.446808: scmi_msg_dump: id=0 ch=10 pt=15 t=cmnd msg_id=06 seq=0000 s=0 pyld=0100000000000000
irq/35-mhu_db_l-81 [000] ..... 14183.447809: scmi_msg_dump: id=0 ch=10 pt=15 t=resp msg_id=06 seq=0000 s=0 pyld=3503000000000000
..trying to read in async when the backend server does NOT supports asyncs:
# AN ASYNC SENSOR READING REQUEST...
root@deb-buster-arm64:~# echo -e -n \\x06\\x54\\x00\\x00\\x01\\x00\\x00\\x00\\x01\\x00\\x00\\x00 > /sys/kernel/debug/scmi/0/raw/message_async
bash-329 [000] ..... 16415.938739: scmi_msg_dump: id=0 ch=10 pt=15 t=cmnd msg_id=06 seq=0000 s=0 pyld=0100000001000000
irq/35-mhu_db_l-81 [000] ..... 16415.944129: scmi_msg_dump: id=0 ch=10 pt=15 t=resp msg_id=06 seq=0000 s=-1 pyld=
# RETURNS A STATUS -1 FROM THE SERVER NOT SUPPORTING IT
root@deb-buster-arm64:~# cat /sys/kernel/debug/scmi/0/raw/message | od --endian=little -t x4
0000000 00005406 ffffffff
0000010
Note that the above example was on a JUNO, BUT exactly the same steps can
be used to reach an SCMI Server living on a VM reachable via virtio as
long as the system under test if properly configured to work with a
virtio transport.
In a nutshell the exposed API is as follows:
/sys/kernel/debug/scmi/
`-- 0
|-- atomic_threshold_us
|-- instance_name
|-- raw
| |-- channels
| | |-- 0x10
| | | |-- message
| | | `-- message_async
| | `-- 0x13
| | |-- message
| | `-- message_async
| |-- errors
| |-- message
| |-- message_async
| |-- notification
| `-- reset
`-- transport
|-- is_atomic
|-- max_msg_size
|-- max_rx_timeout_ms
|-- rx_max_msg
|-- tx_max_msg
`-- type
... where at the top level:
- <N>: a progressive sequence number identifying this SCMI instance, in
case there are multiple SCMI instance defined
- instance_name: can be used (by CI) to identify the SCMI instance <N>
that you are using through this Raw accessors: it corresponds to the SCMI
DT top node full name of the underlying SCMI instance
... rooted under /transport:
- a bunch of configuration info useful to setup the user application
expectations in terms of timeouts and message characteristics.
... rooted at /raw (the real SCMI Raw interface :D):
- message*: used to send sync/async commands and read back immediate and
delayed responses (if any)
- errors: used to report timeout and unexpected replies
- reset: used to reset the SCMI Raw stack, flushing all queues from
received messages still pending to be read out (useful to be sure to
cleanup between test suite runs...)
- notification: used to read any notification being spit by the system
(if previously enabled by the user app)
... rooted at /raw/channels/<M>/:
- message*: used to send sync/async commands and read back immediate and
delayed responses (if any), using a SPECIFIC transport channel <M>
(instead of letting the system choose for you based on transport config
and the rotocol embedded in the injected message)
NOTE THAT these entries are optional, created only if there is more than
transport channel defined on the system.
Each write corresponds to one command request and the replies or delayed
response are read back one message at time (receiving an EOF at each
message boundary).
The user application running the test is in charge of handling timeouts
and properly choosing SCMI sequence numbers for the outgoing requests: note
that the same fixed number can be re-used (...though discouraged...) as
long as the suite does NOT expect to send multiple in-flight commands
concurrently.
Since the SCMI core regular stack is partially used to deliver and collect
the messages, late replies after timeouts and any other sort of unexpected
message sent by the SCMI server platform back can be identified by the SCMI
core as usual and it will be reported under /errors for later analysis.
(a userspace test-app will have anyway properly detected the timeout on
/message* ...)
All of the above has been roughly tested against a standard JUNO SCP SCMI
Server (mailbox trans) and an emulated SCMI Server living in a VM (virtio
trans) using a custom experimental version of the scmi-tests Compliance
suite patched to support Raw mode and posted at [2]. (still in development
...merge requests are in progress...for now it is just a mean for me to
test the testing API ... O_o)
This V8 series is based on v6.2-rc1 PLUS another series which reworked a
bit the SCMI core stack init/probe [3]; to ease testing a V8 properly
based can be picked up from [4].
Having said that (in such a concise and brief way :P) ...
...any feedback/comments are welcome !
Thanks,
Cristian
---
v7 --> v8
- fixed a few bad handling on error path
- ignoring debugfs_ retvals as supposed to
- using XArray for per-channel queus instead of IDRs
- refactored debugfs setup calls
v6 --> v7
- fixed one sparse error
- removed redundant info.num_chans/channels fields: enumerate them dyamically
once needed.
v5 --> v6
- exported symbol debugfs_create_str
- rebased on top of v6.2-rc1 plus series at [3]
- redesigned SCMI debugfs layout with a bunch of common entries
enabled by implicit CONFIG_ARM_SCMI_NEED_DEBUGFS
- refactored SCMI Raw internal queues handling
- added SCMI Raw per-channel injection support
- added channels info on SCMI msg_dump traces
- fix debugfs multiple writers cases
- added DEBUG_FS dependency to Raw mode
- select CONFIG_ARM_SCMI_NEED_DEBUGFS when Raw mode is compiled
v4 --> v5
- rebased on sudeep/for-next/scmi
- added multiple SCMI instances support
- added optional Raw full-cohexistence mode
- use custom tags to distinguish Raw msg_dump traces
- add circular handling of raw buffers queues for errors and notifications
V3 --> v4
- rebased on v6.1-rc1
- addedd missing support for 'polled' transports and transport lacking a
completion_irq (like smc/optee)
- removed a few inlines
- refactored SCMI Raw RX patch to make use more extensively of the regular
non-Raw RX path
- fix handling of O_NONBLOCK raw_mode read requests
v2 --> v3
- fixed some sparse warning on LE and __poll_t
- reworked and simplified deferred worker in charge of xfer delayed waiting
- allow for injection of DT-unknown protocols messages when in Raw mode
(needed for any kind of fuzzing...)
v1 --> v2
- added comments and debugfs docs
- added dedicated transport devices for channels initialization
- better channels handling in Raw mode
- removed runtime enable, moved to static compile time exclusion
of SCMI regular stack
[1]: https://gitlab.arm.com/tests/scmi-tests
[2]: https://gitlab.arm.com/tests/scmi-tests/-/commits/raw_mode_support_devel/
[3]: https://lore.kernel.org/all/[email protected]/
[4]: https://gitlab.arm.com/linux-arm/linux-cm/-/commits/scmi_raw_mode_V8/
Cristian Marussi (17):
firmware: arm_scmi: Refactor xfer in-flight registration routines
firmware: arm_scmi: Refactor polling helpers
firmware: arm_scmi: Refactor scmi_wait_for_message_response
firmware: arm_scmi: Add flags field to xfer
firmware: arm_scmi: Add xfer Raw helpers
firmware: arm_scmi: Move errors defs and code to common.h
firmware: arm_scmi: Add internal platform/channel IDs
include: trace: Add platform and channel instance references
debugfs: Export debugfs_create_str symbol
firmware: arm_scmi: Populate a common SCMI debugsfs root
firmware: arm_scmi: Add debugfs ABI documentation for common entries
firmware: arm_scmi: Add core Raw transmission support
firmware: arm_scmi: Add debugfs ABI documentation for Raw mode
firmware: arm_scmi: Reject SCMI drivers while in Raw mode
firmware: arm_scmi: Call Raw mode hooks from the core stack
firmware: arm_scmi: Add Raw mode coexistence support
firmware: arm_scmi: Add per-channel Raw injection support
Documentation/ABI/testing/debugfs-scmi | 70 +
Documentation/ABI/testing/debugfs-scmi-raw | 109 ++
drivers/firmware/arm_scmi/Kconfig | 32 +
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/bus.c | 7 +
drivers/firmware/arm_scmi/common.h | 75 +
drivers/firmware/arm_scmi/driver.c | 638 +++++++--
drivers/firmware/arm_scmi/protocols.h | 7 +
drivers/firmware/arm_scmi/raw_mode.c | 1443 ++++++++++++++++++++
drivers/firmware/arm_scmi/raw_mode.h | 31 +
fs/debugfs/file.c | 1 +
include/trace/events/scmi.h | 18 +-
12 files changed, 2292 insertions(+), 140 deletions(-)
create mode 100644 Documentation/ABI/testing/debugfs-scmi
create mode 100644 Documentation/ABI/testing/debugfs-scmi-raw
create mode 100644 drivers/firmware/arm_scmi/raw_mode.c
create mode 100644 drivers/firmware/arm_scmi/raw_mode.h
--
2.34.1
Create a common SCMI debugfs root-directory and populate it with some
common SCMI data for each discovered SCMI platform instance if SCMI debugfs
is needed by any configured SCMI facility.
Signed-off-by: Cristian Marussi <[email protected]>
---
v7 --> v8
- Fixed scmi_debugfs_common_setup return values
Reported-by: Dan Carpenter <[email protected]>
v6 --> v7
- Fixed uninitialized value in scmi_debugfs_common_setup
as reported by:
---
drivers/firmware/arm_scmi/Kconfig | 7 ++
drivers/firmware/arm_scmi/driver.c | 120 +++++++++++++++++++++++++++++
2 files changed, 127 insertions(+)
diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index a14f65444b35..f36f89a22740 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -23,6 +23,13 @@ config ARM_SCMI_PROTOCOL
if ARM_SCMI_PROTOCOL
+config ARM_SCMI_NEED_DEBUGFS
+ bool
+ help
+ This declares whether at least one SCMI facility is configured
+ which needs debugfs support. When selected causess the creation
+ of a common SCMI debugfs root directory.
+
config ARM_SCMI_HAVE_TRANSPORT
bool
help
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index c765d0c51dc5..5d5defd92403 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -17,6 +17,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/bitmap.h>
+#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/export.h>
#include <linux/idr.h>
@@ -51,6 +52,8 @@ static DEFINE_MUTEX(scmi_list_mutex);
/* Track the unique id for the transfers for debug & profiling purpose */
static atomic_t transfer_last_id;
+static struct dentry *scmi_top_dentry;
+
/**
* struct scmi_xfers_info - Structure to manage transfer information
*
@@ -97,6 +100,20 @@ struct scmi_protocol_instance {
#define ph_to_pi(h) container_of(h, struct scmi_protocol_instance, ph)
+/**
+ * struct scmi_debug_info - Debug common info
+ * @top_dentry: A reference to the top debugfs dentry
+ * @name: Name of this SCMI instance
+ * @type: Type of this SCMI instance
+ * @is_atomic: Flag to state if the transport of this instance is atomic
+ */
+struct scmi_debug_info {
+ struct dentry *top_dentry;
+ const char *name;
+ const char *type;
+ bool is_atomic;
+};
+
/**
* struct scmi_info - Structure representing a SCMI instance
*
@@ -132,6 +149,7 @@ struct scmi_protocol_instance {
* @dev_req_nb: A notifier to listen for device request/unrequest on the scmi
* bus
* @devreq_mtx: A mutex to serialize device creation for this SCMI instance
+ * @dbg: A pointer to debugfs related data (if any)
*/
struct scmi_info {
int id;
@@ -156,6 +174,7 @@ struct scmi_info {
struct notifier_block dev_req_nb;
/* Serialize device creation process for this instance */
struct mutex devreq_mtx;
+ struct scmi_debug_info *dbg;
};
#define handle_to_scmi_info(h) container_of(h, struct scmi_info, handle)
@@ -2478,6 +2497,83 @@ static int scmi_device_request_notifier(struct notifier_block *nb,
return NOTIFY_OK;
}
+static void scmi_debugfs_common_cleanup(void *d)
+{
+ struct scmi_debug_info *dbg = d;
+
+ if (!dbg)
+ return;
+
+ debugfs_remove_recursive(dbg->top_dentry);
+ kfree(dbg->name);
+ kfree(dbg->type);
+}
+
+static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info)
+{
+ char top_dir[16];
+ struct dentry *trans, *top_dentry;
+ struct scmi_debug_info *dbg;
+ const char *c_ptr = NULL;
+
+ dbg = devm_kzalloc(info->dev, sizeof(*dbg), GFP_KERNEL);
+ if (!dbg)
+ return NULL;
+
+ dbg->name = kstrdup(of_node_full_name(info->dev->of_node), GFP_KERNEL);
+ if (!dbg->name) {
+ devm_kfree(info->dev, dbg);
+ return NULL;
+ }
+
+ of_property_read_string(info->dev->of_node, "compatible", &c_ptr);
+ dbg->type = kstrdup(c_ptr, GFP_KERNEL);
+ if (!dbg->type) {
+ kfree(dbg->name);
+ devm_kfree(info->dev, dbg);
+ return NULL;
+ }
+
+ snprintf(top_dir, 16, "%d", info->id);
+ top_dentry = debugfs_create_dir(top_dir, scmi_top_dentry);
+ trans = debugfs_create_dir("transport", top_dentry);
+
+ dbg->is_atomic = info->desc->atomic_enabled &&
+ is_transport_polling_capable(info->desc);
+
+ debugfs_create_str("instance_name", 0400, top_dentry,
+ (char **)&dbg->name);
+
+ debugfs_create_u32("atomic_threshold_us", 0400, top_dentry,
+ &info->atomic_threshold);
+
+ debugfs_create_str("type", 0400, trans, (char **)&dbg->type);
+
+ debugfs_create_bool("is_atomic", 0400, trans, &dbg->is_atomic);
+
+ debugfs_create_u32("max_rx_timeout_ms", 0400, trans,
+ (u32 *)&info->desc->max_rx_timeout_ms);
+
+ debugfs_create_u32("max_msg_size", 0400, trans,
+ (u32 *)&info->desc->max_msg_size);
+
+ debugfs_create_u32("tx_max_msg", 0400, trans,
+ (u32 *)&info->tx_minfo.max_msg);
+
+ debugfs_create_u32("rx_max_msg", 0400, trans,
+ (u32 *)&info->rx_minfo.max_msg);
+
+ dbg->top_dentry = top_dentry;
+
+ if (devm_add_action_or_reset(info->dev,
+ scmi_debugfs_common_cleanup, dbg)) {
+ scmi_debugfs_common_cleanup(dbg);
+ return NULL;
+ }
+
+ return dbg;
+}
+
static int scmi_probe(struct platform_device *pdev)
{
int ret;
@@ -2552,6 +2648,12 @@ static int scmi_probe(struct platform_device *pdev)
if (ret)
goto clear_dev_req_notifier;
+ if (scmi_top_dentry) {
+ info->dbg = scmi_debugfs_common_setup(info);
+ if (!info->dbg)
+ dev_warn(dev, "Failed to setup SCMI debugfs.\n");
+ }
+
if (scmi_notification_init(handle))
dev_err(dev, "SCMI Notifications NOT available.\n");
@@ -2781,6 +2883,19 @@ static void __exit scmi_transports_exit(void)
__scmi_transports_setup(false);
}
+static struct dentry *scmi_debugfs_init(void)
+{
+ struct dentry *d;
+
+ d = debugfs_create_dir("scmi", NULL);
+ if (IS_ERR(d)) {
+ pr_err("Could NOT create SCMI top dentry.\n");
+ return NULL;
+ }
+
+ return d;
+}
+
static int __init scmi_driver_init(void)
{
int ret;
@@ -2794,6 +2909,9 @@ static int __init scmi_driver_init(void)
if (ret)
return ret;
+ if (IS_ENABLED(CONFIG_ARM_SCMI_NEED_DEBUGFS))
+ scmi_top_dentry = scmi_debugfs_init();
+
scmi_base_register();
scmi_clock_register();
@@ -2825,6 +2943,8 @@ static void __exit scmi_driver_exit(void)
scmi_transports_exit();
platform_driver_unregister(&scmi_driver);
+
+ debugfs_remove_recursive(scmi_top_dentry);
}
module_exit(scmi_driver_exit);
--
2.34.1
On a system configured with multiple transport channels, expose a few
additional debugfs per-channel entries to allow a user to explicitly select
which transport channel to use for the SCMI message injection.
Signed-off-by: Cristian Marussi <[email protected]>
---
v7 --> v8
- use Xarray instead of per-channel IDR
- refactor to fit new scmi_debugfs_raw_mode_setup helper
- Fixed error path while handling channels queues
Reported-by: Dan Carpenter <[email protected]>
v6 -> v7
- remove info channels/num_chans fields since uneeded, gather
such data dynamically
---
Documentation/ABI/testing/debugfs-scmi-raw | 43 ++++++
drivers/firmware/arm_scmi/common.h | 2 +
drivers/firmware/arm_scmi/driver.c | 38 +++++-
drivers/firmware/arm_scmi/protocols.h | 3 +
drivers/firmware/arm_scmi/raw_mode.c | 152 ++++++++++++++++++---
drivers/firmware/arm_scmi/raw_mode.h | 3 +-
6 files changed, 217 insertions(+), 24 deletions(-)
diff --git a/Documentation/ABI/testing/debugfs-scmi-raw b/Documentation/ABI/testing/debugfs-scmi-raw
index 45817d465b1f..5468ec08c084 100644
--- a/Documentation/ABI/testing/debugfs-scmi-raw
+++ b/Documentation/ABI/testing/debugfs-scmi-raw
@@ -64,3 +64,46 @@ Description: SCMI Raw stack reset facility; writing a value to this entry
Can be used to reset and clean the SCMI Raw stack between to
different test-run.
Users: Debugging, any userspace test suite
+
+What: /sys/kernel/debug/scmi/<n>/raw/channels/<m>/message
+Date: March 2023
+KernelVersion: 6.3
+Contact: [email protected]
+Description: SCMI Raw synchronous message injection/snooping facility; write
+ a complete SCMI synchronous command message (header included)
+ in little-endian binary format to have it sent to the configured
+ backend SCMI server for instance <n> through the <m> transport
+ channel.
+ Any subsequently received response can be read from this same
+ entry if it arrived on channel <m> within the configured
+ timeout.
+ Each write to the entry causes one command request to be built
+ and sent while the replies are read back one message at time
+ (receiving an EOF at each message boundary).
+ Note that these per-channel entries rooted at <..>/channels
+ exist only if the transport is configured to have more than
+ one channel.
+Users: Debugging, any userspace test suite
+
+What: /sys/kernel/debug/scmi/<n>/raw/channels/<m>/message_async
+Date: March 2023
+KernelVersion: 6.3
+Contact: [email protected]
+Description: SCMI Raw asynchronous message injection/snooping facility; write
+ a complete SCMI asynchronous command message (header included)
+ in little-endian binary format to have it sent to the configured
+ backend SCMI server for instance <n> through the <m> transport
+ channel.
+ Any subsequently received response can be read from this same
+ entry if it arrived on channel <m> within the configured
+ timeout.
+ Any additional delayed response received afterwards can be read
+ from this same entry too if it arrived within the configured
+ timeout.
+ Each write to the entry causes one command request to be built
+ and sent while the replies are read back one message at time
+ (receiving an EOF at each message boundary).
+ Note that these per-channel entries rooted at <..>/channels
+ exist only if the transport is configured to have more than
+ one channel.
+Users: Debugging, any userspace test suite
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index bf3883f169e3..c46dc5215af7 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -27,6 +27,8 @@
#include "protocols.h"
#include "notify.h"
+#define SCMI_MAX_CHANNELS 256
+
#define SCMI_MAX_RESPONSE_TIMEOUT (2 * MSEC_PER_SEC)
enum scmi_error_codes {
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 282d7737cb8f..d21c7eafd641 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -660,6 +660,7 @@ void scmi_xfer_raw_put(const struct scmi_handle *handle, struct scmi_xfer *xfer)
struct scmi_info *info = handle_to_scmi_info(handle);
xfer->flags &= ~SCMI_XFER_FLAG_IS_RAW;
+ xfer->flags &= ~SCMI_XFER_FLAG_CHAN_SET;
return __scmi_xfer_put(&info->tx_minfo, xfer);
}
@@ -896,7 +897,8 @@ static void scmi_handle_notification(struct scmi_chan_info *cinfo,
if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
xfer->hdr.seq = MSG_XTRACT_TOKEN(msg_hdr);
- scmi_raw_message_report(info->raw, xfer, SCMI_RAW_NOTIF_QUEUE);
+ scmi_raw_message_report(info->raw, xfer, SCMI_RAW_NOTIF_QUEUE,
+ cinfo->id);
}
__scmi_xfer_put(minfo, xfer);
@@ -955,7 +957,9 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
* poll loop.
*/
if (!xfer->hdr.poll_completion)
- scmi_raw_message_report(info->raw, xfer, SCMI_RAW_REPLY_QUEUE);
+ scmi_raw_message_report(info->raw, xfer,
+ SCMI_RAW_REPLY_QUEUE,
+ cinfo->id);
}
scmi_xfer_command_release(info, xfer);
@@ -1078,7 +1082,8 @@ static int scmi_wait_for_reply(struct device *dev, const struct scmi_desc *desc,
handle_to_scmi_info(cinfo->handle);
scmi_raw_message_report(info->raw, xfer,
- SCMI_RAW_REPLY_QUEUE);
+ SCMI_RAW_REPLY_QUEUE,
+ cinfo->id);
}
}
} else {
@@ -2608,14 +2613,35 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info)
static int scmi_debugfs_raw_mode_setup(struct scmi_info *info)
{
- int ret = 0;
+ int id, num_chans = 0, ret = 0;
+ struct scmi_chan_info *cinfo;
+ u8 channels[SCMI_MAX_CHANNELS] = {};
+ DECLARE_BITMAP(protos, SCMI_MAX_CHANNELS) = {};
if (!info->dbg)
return -EINVAL;
+ /* Enumerate all channels to collect their ids */
+ idr_for_each_entry(&info->tx_idr, cinfo, id) {
+ /*
+ * Cannot happen, but be defensive.
+ * Zero as num_chans is ok, warn and carry on.
+ */
+ if (num_chans >= SCMI_MAX_CHANNELS || !cinfo) {
+ dev_warn(info->dev,
+ "SCMI RAW - Error enumerating channels\n");
+ break;
+ }
+
+ if (!test_bit(cinfo->id, protos)) {
+ channels[num_chans++] = cinfo->id;
+ set_bit(cinfo->id, protos);
+ }
+ }
+
info->raw = scmi_raw_mode_init(&info->handle, info->dbg->top_dentry,
- info->id, info->desc,
- info->tx_minfo.max_msg);
+ info->id, channels, num_chans,
+ info->desc, info->tx_minfo.max_msg);
if (IS_ERR(info->raw)) {
dev_err(info->dev, "Failed to initialize SCMI RAW Mode !\n");
ret = PTR_ERR(info->raw);
diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index b5941beb8b03..78e1a01eb656 100644
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -138,6 +138,9 @@ struct scmi_xfer {
int state;
#define SCMI_XFER_FLAG_IS_RAW BIT(0)
#define SCMI_XFER_IS_RAW(x) ((x)->flags & SCMI_XFER_FLAG_IS_RAW)
+#define SCMI_XFER_FLAG_CHAN_SET BIT(1)
+#define SCMI_XFER_IS_CHAN_SET(x) \
+ ((x)->flags & SCMI_XFER_FLAG_CHAN_SET)
int flags;
/* A lock to protect state and busy fields */
spinlock_t lock;
diff --git a/drivers/firmware/arm_scmi/raw_mode.c b/drivers/firmware/arm_scmi/raw_mode.c
index 037ef9c14fae..d40df099fd51 100644
--- a/drivers/firmware/arm_scmi/raw_mode.c
+++ b/drivers/firmware/arm_scmi/raw_mode.c
@@ -37,10 +37,17 @@
* which in turn is rooted under the corresponding underlying SCMI instance.
*
* /sys/kernel/debug/scmi/
- * |-- 0
+ * `-- 0
* |-- atomic_threshold_us
* |-- instance_name
* |-- raw
+ * | |-- channels
+ * | | |-- 0x10
+ * | | | |-- message
+ * | | | `-- message_async
+ * | | `-- 0x13
+ * | | |-- message
+ * | | `-- message_async
* | |-- errors
* | |-- message
* | |-- message_async
@@ -65,6 +72,15 @@
* to be read; this is useful at test-suite start/stop to get
* rid of any unread messages from the previous run.
*
+ * with the per-channel entries rooted at /channels being present only on a
+ * system where multiple transport channels have been configured.
+ *
+ * Such per-channel entries can be used to explicitly choose a specific channel
+ * for SCMI bare message injection, in contrast with the general entries above
+ * where, instead, the selection of the proper channel to use is automatically
+ * performed based the protocol embedded in the injected message and on how the
+ * transport is configured on the system.
+ *
* Note that other common general entries are available under transport/ to let
* the user applications properly make up their expectations in terms of
* timeouts and message characteristics.
@@ -95,7 +111,6 @@
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/export.h>
-#include <linux/idr.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/fs.h>
@@ -104,6 +119,7 @@
#include <linux/poll.h>
#include <linux/of.h>
#include <linux/slab.h>
+#include <linux/xarray.h>
#include "common.h"
@@ -140,6 +156,7 @@ struct scmi_raw_queue {
* @desc: Pointer to the transport descriptor to use
* @tx_max_msg: Maximum number of concurrent TX in-flight messages
* @q: An array of Raw queue descriptors
+ * @chans_q: An XArray mapping optional additional per-channel queues
* @free_waiters: Head of freelist for unused waiters
* @free_mtx: A mutex to protect the waiters freelist
* @active_waiters: Head of list for currently active and used waiters
@@ -159,6 +176,7 @@ struct scmi_raw_mode_info {
const struct scmi_desc *desc;
int tx_max_msg;
struct scmi_raw_queue *q[SCMI_RAW_MAX_QUEUE];
+ struct xarray chans_q;
struct list_head free_waiters;
/* Protect free_waiters list */
struct mutex free_mtx;
@@ -208,6 +226,8 @@ struct scmi_raw_buffer {
* struct scmi_dbg_raw_data - Structure holding data needed by the debugfs
* layer
*
+ * @chan_id: The preferred channel to use: if zero the channel is automatically
+ * selected based on protocol.
* @raw: A reference to the Raw instance.
* @tx: A message buffer used to collect TX message on write.
* @tx_size: The effective size of the TX message.
@@ -216,6 +236,7 @@ struct scmi_raw_buffer {
* @rx_size: The effective size of the RX message.
*/
struct scmi_dbg_raw_data {
+ u8 chan_id;
struct scmi_raw_mode_info *raw;
struct scmi_msg tx;
size_t tx_size;
@@ -224,6 +245,16 @@ struct scmi_dbg_raw_data {
size_t rx_size;
};
+static struct scmi_raw_queue *
+scmi_raw_queue_select(struct scmi_raw_mode_info *raw, unsigned int idx,
+ unsigned int chan_id)
+{
+ if (!chan_id)
+ return raw->q[idx];
+
+ return xa_load(&raw->chans_q, chan_id);
+}
+
static struct scmi_raw_buffer *scmi_raw_buffer_get(struct scmi_raw_queue *q)
{
unsigned long flags;
@@ -563,6 +594,8 @@ static int scmi_xfer_raw_get_init(struct scmi_raw_mode_info *raw, void *buf,
*
* @raw: A reference to the Raw instance.
* @xfer: The xfer to send
+ * @chan_id: The channel ID to use, if zero the channels is automatically
+ * selected based on the protocol used.
* @async: A flag stating if an asynchronous command is required.
*
* This function send a previously built raw xfer using an appropriate channel
@@ -576,14 +609,20 @@ static int scmi_xfer_raw_get_init(struct scmi_raw_mode_info *raw, void *buf,
* Return: 0 on Success
*/
static int scmi_do_xfer_raw_start(struct scmi_raw_mode_info *raw,
- struct scmi_xfer *xfer, bool async)
+ struct scmi_xfer *xfer, u8 chan_id,
+ bool async)
{
int ret;
struct scmi_chan_info *cinfo;
struct scmi_xfer_raw_waiter *rw;
struct device *dev = raw->handle->dev;
- cinfo = scmi_xfer_raw_channel_get(raw->handle, xfer->hdr.protocol_id);
+ if (!chan_id)
+ chan_id = xfer->hdr.protocol_id;
+ else
+ xfer->flags |= SCMI_XFER_FLAG_CHAN_SET;
+
+ cinfo = scmi_xfer_raw_channel_get(raw->handle, chan_id);
if (IS_ERR(cinfo))
return PTR_ERR(cinfo);
@@ -630,12 +669,13 @@ static int scmi_do_xfer_raw_start(struct scmi_raw_mode_info *raw,
* @buf: A buffer containing the whole SCMI message to send (including the
* header) in little-endian binary format.
* @len: Length of the message in @buf.
+ * @chan_id: The channel ID to use.
* @async: A flag stating if an asynchronous command is required.
*
* Return: 0 on Success
*/
static int scmi_raw_message_send(struct scmi_raw_mode_info *raw,
- void *buf, size_t len, bool async)
+ void *buf, size_t len, u8 chan_id, bool async)
{
int ret;
struct scmi_xfer *xfer;
@@ -644,7 +684,7 @@ static int scmi_raw_message_send(struct scmi_raw_mode_info *raw,
if (ret)
return ret;
- ret = scmi_do_xfer_raw_start(raw, xfer, async);
+ ret = scmi_do_xfer_raw_start(raw, xfer, chan_id, async);
if (ret)
scmi_xfer_raw_put(raw->handle, xfer);
@@ -687,18 +727,23 @@ scmi_raw_message_dequeue(struct scmi_raw_queue *q, bool o_nonblock)
* @len: Length of @buf.
* @size: The effective size of the message copied into @buf
* @idx: The index of the queue to pick the next queued message from.
+ * @chan_id: The channel ID to use.
* @o_nonblock: A flag to request a non-blocking message dequeue.
*
* Return: 0 on Success
*/
static int scmi_raw_message_receive(struct scmi_raw_mode_info *raw,
void *buf, size_t len, size_t *size,
- unsigned int idx,
+ unsigned int idx, unsigned int chan_id,
bool o_nonblock)
{
int ret = 0;
struct scmi_raw_buffer *rb;
- struct scmi_raw_queue *q = raw->q[idx];
+ struct scmi_raw_queue *q;
+
+ q = scmi_raw_queue_select(raw, idx, chan_id);
+ if (!q)
+ return -ENODEV;
rb = scmi_raw_message_dequeue(q, o_nonblock);
if (IS_ERR(rb)) {
@@ -732,7 +777,7 @@ static ssize_t scmi_dbg_raw_mode_common_read(struct file *filp,
int ret;
ret = scmi_raw_message_receive(rd->raw, rd->rx.buf, rd->rx.len,
- &rd->rx_size, idx,
+ &rd->rx_size, idx, rd->chan_id,
filp->f_flags & O_NONBLOCK);
if (ret) {
rd->rx_size = 0;
@@ -782,7 +827,8 @@ static ssize_t scmi_dbg_raw_mode_common_write(struct file *filp,
return cnt;
}
- ret = scmi_raw_message_send(rd->raw, rd->tx.buf, rd->tx_size, async);
+ ret = scmi_raw_message_send(rd->raw, rd->tx.buf, rd->tx_size,
+ rd->chan_id, async);
/* Reset ppos for next message ... */
rd->tx_size = 0;
@@ -797,9 +843,13 @@ static __poll_t scmi_test_dbg_raw_common_poll(struct file *filp,
{
unsigned long flags;
struct scmi_dbg_raw_data *rd = filp->private_data;
- struct scmi_raw_queue *q = rd->raw->q[idx];
+ struct scmi_raw_queue *q;
__poll_t mask = 0;
+ q = scmi_raw_queue_select(rd->raw, idx, rd->chan_id);
+ if (!q)
+ return mask;
+
poll_wait(filp, &q->wq, wait);
spin_lock_irqsave(&q->msg_q_lock, flags);
@@ -833,8 +883,10 @@ static __poll_t scmi_dbg_raw_mode_message_poll(struct file *filp,
static int scmi_dbg_raw_mode_open(struct inode *inode, struct file *filp)
{
+ u8 id;
struct scmi_raw_mode_info *raw;
struct scmi_dbg_raw_data *rd;
+ const char *id_str = filp->f_path.dentry->d_parent->d_name.name;
if (!inode->i_private)
return -ENODEV;
@@ -859,6 +911,10 @@ static int scmi_dbg_raw_mode_open(struct inode *inode, struct file *filp)
return -ENOMEM;
}
+ /* Grab channel ID from debugfs entry naming if any */
+ if (!kstrtou8(id_str, 16, &id))
+ rd->chan_id = id;
+
rd->raw = raw;
filp->private_data = rd;
@@ -1028,7 +1084,8 @@ static int scmi_xfer_raw_worker_init(struct scmi_raw_mode_info *raw)
return 0;
}
-static int scmi_raw_mode_setup(struct scmi_raw_mode_info *raw)
+static int scmi_raw_mode_setup(struct scmi_raw_mode_info *raw,
+ u8 *channels, int num_chans)
{
int ret, idx;
void *gid;
@@ -1046,15 +1103,43 @@ static int scmi_raw_mode_setup(struct scmi_raw_mode_info *raw)
}
}
+ xa_init(&raw->chans_q);
+ if (num_chans > 1) {
+ int i;
+
+ for (i = 0; i < num_chans; i++) {
+ void *xret;
+ struct scmi_raw_queue *q;
+
+ q = scmi_raw_queue_init(raw);
+ if (IS_ERR(q)) {
+ ret = PTR_ERR(q);
+ goto err_xa;
+ }
+
+ xret = xa_store(&raw->chans_q, channels[i], q,
+ GFP_KERNEL);
+ if (xa_err(xret)) {
+ dev_err(dev,
+ "Fail to allocate Raw queue 0x%02X\n",
+ channels[i]);
+ ret = xa_err(xret);
+ goto err_xa;
+ }
+ }
+ }
+
ret = scmi_xfer_raw_worker_init(raw);
if (ret)
- goto err;
+ goto err_xa;
devres_close_group(dev, gid);
raw->gid = gid;
return 0;
+err_xa:
+ xa_destroy(&raw->chans_q);
err:
devres_release_group(dev, gid);
return ret;
@@ -1067,6 +1152,8 @@ static int scmi_raw_mode_setup(struct scmi_raw_mode_info *raw)
* @top_dentry: A reference to the top Raw debugfs dentry
* @instance_id: The ID of the underlying SCMI platform instance represented by
* this Raw instance
+ * @channels: The list of the existing channels
+ * @num_chans: The number of entries in @channels
* @desc: Reference to the transport operations
* @tx_max_msg: Max number of in-flight messages allowed by the transport
*
@@ -1076,6 +1163,7 @@ static int scmi_raw_mode_setup(struct scmi_raw_mode_info *raw)
*/
void *scmi_raw_mode_init(const struct scmi_handle *handle,
struct dentry *top_dentry, int instance_id,
+ u8 *channels, int num_chans,
const struct scmi_desc *desc, int tx_max_msg)
{
int ret;
@@ -1095,7 +1183,7 @@ void *scmi_raw_mode_init(const struct scmi_handle *handle,
raw->tx_max_msg = tx_max_msg;
raw->id = instance_id;
- ret = scmi_raw_mode_setup(raw);
+ ret = scmi_raw_mode_setup(raw, channels, num_chans);
if (ret) {
devm_kfree(dev, raw);
return ERR_PTR(ret);
@@ -1118,6 +1206,32 @@ void *scmi_raw_mode_init(const struct scmi_handle *handle,
debugfs_create_file("errors", 0400, raw->dentry, raw,
&scmi_dbg_raw_mode_errors_fops);
+ /*
+ * Expose per-channel entries if multiple channels available.
+ * Just ignore errors while setting up these interfaces since we
+ * have anyway already a working core Raw support.
+ */
+ if (num_chans > 1) {
+ int i;
+ struct dentry *top_chans;
+
+ top_chans = debugfs_create_dir("channels", raw->dentry);
+
+ for (i = 0; i < num_chans; i++) {
+ char cdir[8];
+ struct dentry *chd;
+
+ snprintf(cdir, 8, "0x%02X", channels[i]);
+ chd = debugfs_create_dir(cdir, top_chans);
+
+ debugfs_create_file("message", 0600, chd, raw,
+ &scmi_dbg_raw_mode_message_fops);
+
+ debugfs_create_file("message_async", 0600, chd, raw,
+ &scmi_dbg_raw_mode_message_async_fops);
+ }
+ }
+
dev_info(dev, "SCMI RAW Mode initialized for instance %d\n", raw->id);
return raw;
@@ -1139,6 +1253,7 @@ void scmi_raw_mode_cleanup(void *r)
cancel_work_sync(&raw->waiters_work);
destroy_workqueue(raw->wait_wq);
+ xa_destroy(&raw->chans_q);
}
static int scmi_xfer_raw_collect(void *msg, size_t *msg_len,
@@ -1178,6 +1293,7 @@ static int scmi_xfer_raw_collect(void *msg, size_t *msg_len,
* @r: An opaque reference to the raw instance configuration
* @xfer: The xfer containing the message to be reported
* @idx: The index of the queue.
+ * @chan_id: The channel ID to use.
*
* If Raw mode is enabled, this is called from the SCMI core on the regular RX
* path to save and enqueue the response/notification payload carried by this
@@ -1187,7 +1303,8 @@ static int scmi_xfer_raw_collect(void *msg, size_t *msg_len,
* user can read back the raw message payload at its own pace (if ever) without
* holding an xfer for too long.
*/
-void scmi_raw_message_report(void *r, struct scmi_xfer *xfer, unsigned int idx)
+void scmi_raw_message_report(void *r, struct scmi_xfer *xfer,
+ unsigned int idx, unsigned int chan_id)
{
int ret;
unsigned long flags;
@@ -1200,7 +1317,8 @@ void scmi_raw_message_report(void *r, struct scmi_xfer *xfer, unsigned int idx)
return;
dev = raw->handle->dev;
- q = raw->q[idx];
+ q = scmi_raw_queue_select(raw, idx,
+ SCMI_XFER_IS_CHAN_SET(xfer) ? chan_id : 0);
/*
* Grab the msg_q_lock upfront to avoid a possible race between
@@ -1319,7 +1437,7 @@ void scmi_raw_error_report(void *r, struct scmi_chan_info *cinfo,
smp_store_mb(xfer.priv, priv);
scmi_xfer_raw_fill(raw, cinfo, &xfer, msg_hdr);
- scmi_raw_message_report(raw, &xfer, SCMI_RAW_ERRS_QUEUE);
+ scmi_raw_message_report(raw, &xfer, SCMI_RAW_ERRS_QUEUE, 0);
kfree(xfer.rx.buf);
}
diff --git a/drivers/firmware/arm_scmi/raw_mode.h b/drivers/firmware/arm_scmi/raw_mode.h
index 1e3d1660b0e4..8af756a83fd1 100644
--- a/drivers/firmware/arm_scmi/raw_mode.h
+++ b/drivers/firmware/arm_scmi/raw_mode.h
@@ -19,11 +19,12 @@ enum {
void *scmi_raw_mode_init(const struct scmi_handle *handle,
struct dentry *top_dentry, int instance_id,
+ u8 *channels, int num_chans,
const struct scmi_desc *desc, int tx_max_msg);
void scmi_raw_mode_cleanup(void *raw);
void scmi_raw_message_report(void *raw, struct scmi_xfer *xfer,
- unsigned int idx);
+ unsigned int idx, unsigned int chan_id);
void scmi_raw_error_report(void *raw, struct scmi_chan_info *cinfo,
u32 msg_hdr, void *priv);
--
2.34.1
Refactor scmi_wait_for_message_response() to use a internal helper to
carry out its main duties; while doing that make it accept directly an
scmi_desc parameter to interact with the configured transport.
No functional change.
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_scmi/driver.c | 57 +++++++++++++++++-------------
1 file changed, 33 insertions(+), 24 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index d7f21e81bd11..ce204a77c7f3 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -891,36 +891,18 @@ static bool scmi_xfer_done_no_timeout(struct scmi_chan_info *cinfo,
ktime_after(ktime_get(), stop);
}
-/**
- * scmi_wait_for_message_response - An helper to group all the possible ways of
- * waiting for a synchronous message response.
- *
- * @cinfo: SCMI channel info
- * @xfer: Reference to the transfer being waited for.
- *
- * Chooses waiting strategy (sleep-waiting vs busy-waiting) depending on
- * configuration flags like xfer->hdr.poll_completion.
- *
- * Return: 0 on Success, error otherwise.
- */
-static int scmi_wait_for_message_response(struct scmi_chan_info *cinfo,
- struct scmi_xfer *xfer)
+static int scmi_wait_for_reply(struct device *dev, const struct scmi_desc *desc,
+ struct scmi_chan_info *cinfo,
+ struct scmi_xfer *xfer, unsigned int timeout_ms)
{
- struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
- struct device *dev = info->dev;
- int ret = 0, timeout_ms = info->desc->max_rx_timeout_ms;
-
- trace_scmi_xfer_response_wait(xfer->transfer_id, xfer->hdr.id,
- xfer->hdr.protocol_id, xfer->hdr.seq,
- timeout_ms,
- xfer->hdr.poll_completion);
+ int ret = 0;
if (xfer->hdr.poll_completion) {
/*
* Real polling is needed only if transport has NOT declared
* itself to support synchronous commands replies.
*/
- if (!info->desc->sync_cmds_completed_on_ret) {
+ if (!desc->sync_cmds_completed_on_ret) {
/*
* Poll on xfer using transport provided .poll_done();
* assumes no completion interrupt was available.
@@ -946,7 +928,7 @@ static int scmi_wait_for_message_response(struct scmi_chan_info *cinfo,
*/
spin_lock_irqsave(&xfer->lock, flags);
if (xfer->state == SCMI_XFER_SENT_OK) {
- info->desc->ops->fetch_response(cinfo, xfer);
+ desc->ops->fetch_response(cinfo, xfer);
xfer->state = SCMI_XFER_RESP_OK;
}
spin_unlock_irqrestore(&xfer->lock, flags);
@@ -970,6 +952,33 @@ static int scmi_wait_for_message_response(struct scmi_chan_info *cinfo,
return ret;
}
+/**
+ * scmi_wait_for_message_response - An helper to group all the possible ways of
+ * waiting for a synchronous message response.
+ *
+ * @cinfo: SCMI channel info
+ * @xfer: Reference to the transfer being waited for.
+ *
+ * Chooses waiting strategy (sleep-waiting vs busy-waiting) depending on
+ * configuration flags like xfer->hdr.poll_completion.
+ *
+ * Return: 0 on Success, error otherwise.
+ */
+static int scmi_wait_for_message_response(struct scmi_chan_info *cinfo,
+ struct scmi_xfer *xfer)
+{
+ struct scmi_info *info = handle_to_scmi_info(cinfo->handle);
+ struct device *dev = info->dev;
+
+ trace_scmi_xfer_response_wait(xfer->transfer_id, xfer->hdr.id,
+ xfer->hdr.protocol_id, xfer->hdr.seq,
+ info->desc->max_rx_timeout_ms,
+ xfer->hdr.poll_completion);
+
+ return scmi_wait_for_reply(dev, info->desc, cinfo, xfer,
+ info->desc->max_rx_timeout_ms);
+}
+
/**
* do_xfer() - Do one transfer
*
--
2.34.1
Add a few call sites where, if SCMI Raw mode access had been enabled in
Kconfig, the needed SCMI Raw initialization and hooks are called.
Signed-off-by: Cristian Marussi <[email protected]>
---
v7 --> v8
- refactor RAw mode initialization
v5 --> v6
- fix error path for RAW mode in scmi_probe
- use new debugfs Raw rootdir
v4 --> v5
- rework to use multiple SCMI instances
v3 --> v4
- add call hooks to support polled transports
v1 --> v2
- fixes need to use multiple cinfo if available
---
drivers/firmware/arm_scmi/driver.c | 63 ++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 2b96d9c3489d..ad4e53f0c342 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -37,6 +37,8 @@
#include "common.h"
#include "notify.h"
+#include "raw_mode.h"
+
#define CREATE_TRACE_POINTS
#include <trace/events/scmi.h>
@@ -150,6 +152,7 @@ struct scmi_debug_info {
* bus
* @devreq_mtx: A mutex to serialize device creation for this SCMI instance
* @dbg: A pointer to debugfs related data (if any)
+ * @raw: An opaque reference handle used by SCMI Raw mode.
*/
struct scmi_info {
int id;
@@ -175,6 +178,7 @@ struct scmi_info {
/* Serialize device creation process for this instance */
struct mutex devreq_mtx;
struct scmi_debug_info *dbg;
+ void *raw;
};
#define handle_to_scmi_info(h) container_of(h, struct scmi_info, handle)
@@ -890,6 +894,11 @@ static void scmi_handle_notification(struct scmi_chan_info *cinfo,
xfer->hdr.protocol_id, xfer->hdr.seq,
MSG_TYPE_NOTIFICATION);
+ if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
+ xfer->hdr.seq = MSG_XTRACT_TOKEN(msg_hdr);
+ scmi_raw_message_report(info->raw, xfer, SCMI_RAW_NOTIF_QUEUE);
+ }
+
__scmi_xfer_put(minfo, xfer);
scmi_clear_channel(info, cinfo);
@@ -903,6 +912,9 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
xfer = scmi_xfer_command_acquire(cinfo, msg_hdr);
if (IS_ERR(xfer)) {
+ if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT))
+ scmi_raw_error_report(info->raw, cinfo, msg_hdr, priv);
+
if (MSG_XTRACT_TYPE(msg_hdr) == MSG_TYPE_DELAYED_RESP)
scmi_clear_channel(info, cinfo);
return;
@@ -936,6 +948,16 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
complete(&xfer->done);
}
+ if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
+ /*
+ * When in polling mode avoid to queue the Raw xfer on the IRQ
+ * RX path since it will be already queued at the end of the TX
+ * poll loop.
+ */
+ if (!xfer->hdr.poll_completion)
+ scmi_raw_message_report(info->raw, xfer, SCMI_RAW_REPLY_QUEUE);
+ }
+
scmi_xfer_command_release(info, xfer);
}
@@ -1050,6 +1072,14 @@ static int scmi_wait_for_reply(struct device *dev, const struct scmi_desc *desc,
"RESP" : "resp",
xfer->hdr.seq, xfer->hdr.status,
xfer->rx.buf, xfer->rx.len);
+
+ if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
+ struct scmi_info *info =
+ handle_to_scmi_info(cinfo->handle);
+
+ scmi_raw_message_report(info->raw, xfer,
+ SCMI_RAW_REPLY_QUEUE);
+ }
}
} else {
/* And we wait for the response. */
@@ -2576,6 +2606,26 @@ static struct scmi_debug_info *scmi_debugfs_common_setup(struct scmi_info *info)
return dbg;
}
+static int scmi_debugfs_raw_mode_setup(struct scmi_info *info)
+{
+ int ret = 0;
+
+ if (!info->dbg)
+ return -EINVAL;
+
+ info->raw = scmi_raw_mode_init(&info->handle, info->dbg->top_dentry,
+ info->id, info->desc,
+ info->tx_minfo.max_msg);
+
+ if (IS_ERR(info->raw)) {
+ dev_err(info->dev, "Failed to initialize SCMI RAW Mode !\n");
+ ret = PTR_ERR(info->raw);
+ info->raw = NULL;
+ }
+
+ return ret;
+}
+
static int scmi_probe(struct platform_device *pdev)
{
int ret;
@@ -2654,6 +2704,14 @@ static int scmi_probe(struct platform_device *pdev)
info->dbg = scmi_debugfs_common_setup(info);
if (!info->dbg)
dev_warn(dev, "Failed to setup SCMI debugfs.\n");
+
+ if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
+ ret = scmi_debugfs_raw_mode_setup(info);
+ if (ret)
+ goto clear_dev_req_notifier;
+
+ return 0;
+ }
}
if (scmi_notification_init(handle))
@@ -2713,6 +2771,8 @@ static int scmi_probe(struct platform_device *pdev)
return 0;
notification_exit:
+ if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT))
+ scmi_raw_mode_cleanup(info->raw);
scmi_notification_exit(&info->handle);
clear_dev_req_notifier:
blocking_notifier_chain_unregister(&scmi_requested_devices_nh,
@@ -2732,6 +2792,9 @@ static int scmi_remove(struct platform_device *pdev)
struct scmi_info *info = platform_get_drvdata(pdev);
struct device_node *child;
+ if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT))
+ scmi_raw_mode_cleanup(info->raw);
+
mutex_lock(&scmi_list_mutex);
if (info->users)
dev_warn(&pdev->dev,
--
2.34.1
Refactor polling helpers to receive scmi_desc directly as a parameter and
move all of them to common.h.
No functional change.
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_scmi/common.h | 18 ++++++++++++++++
drivers/firmware/arm_scmi/driver.c | 34 ++++++++----------------------
2 files changed, 27 insertions(+), 25 deletions(-)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 80f63fc8ca14..f785f0ff2090 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -211,6 +211,24 @@ struct scmi_desc {
const bool atomic_enabled;
};
+static inline bool is_polling_required(struct scmi_chan_info *cinfo,
+ const struct scmi_desc *desc)
+{
+ return cinfo->no_completion_irq || desc->force_polling;
+}
+
+static inline bool is_transport_polling_capable(const struct scmi_desc *desc)
+{
+ return desc->ops->poll_done || desc->sync_cmds_completed_on_ret;
+}
+
+static inline bool is_polling_enabled(struct scmi_chan_info *cinfo,
+ const struct scmi_desc *desc)
+{
+ return is_polling_required(cinfo, desc) &&
+ is_transport_polling_capable(desc);
+}
+
#ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX
extern const struct scmi_desc scmi_mailbox_desc;
#endif
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 3400bd124a38..d7f21e81bd11 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -748,25 +748,6 @@ static inline void scmi_clear_channel(struct scmi_info *info,
info->desc->ops->clear_channel(cinfo);
}
-static inline bool is_polling_required(struct scmi_chan_info *cinfo,
- struct scmi_info *info)
-{
- return cinfo->no_completion_irq || info->desc->force_polling;
-}
-
-static inline bool is_transport_polling_capable(struct scmi_info *info)
-{
- return info->desc->ops->poll_done ||
- info->desc->sync_cmds_completed_on_ret;
-}
-
-static inline bool is_polling_enabled(struct scmi_chan_info *cinfo,
- struct scmi_info *info)
-{
- return is_polling_required(cinfo, info) &&
- is_transport_polling_capable(info);
-}
-
static void scmi_handle_notification(struct scmi_chan_info *cinfo,
u32 msg_hdr, void *priv)
{
@@ -1009,7 +990,8 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
struct scmi_chan_info *cinfo;
/* Check for polling request on custom command xfers at first */
- if (xfer->hdr.poll_completion && !is_transport_polling_capable(info)) {
+ if (xfer->hdr.poll_completion &&
+ !is_transport_polling_capable(info->desc)) {
dev_warn_once(dev,
"Polling mode is not supported by transport.\n");
return -EINVAL;
@@ -1020,7 +1002,7 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
return -EINVAL;
/* True ONLY if also supported by transport. */
- if (is_polling_enabled(cinfo, info))
+ if (is_polling_enabled(cinfo, info->desc))
xfer->hdr.poll_completion = true;
/*
@@ -1956,7 +1938,8 @@ static bool scmi_is_transport_atomic(const struct scmi_handle *handle,
bool ret;
struct scmi_info *info = handle_to_scmi_info(handle);
- ret = info->desc->atomic_enabled && is_transport_polling_capable(info);
+ ret = info->desc->atomic_enabled &&
+ is_transport_polling_capable(info->desc);
if (ret && atomic_threshold)
*atomic_threshold = info->atomic_threshold;
@@ -2180,8 +2163,8 @@ static int scmi_chan_setup(struct scmi_info *info, struct device_node *of_node,
return ret;
}
- if (tx && is_polling_required(cinfo, info)) {
- if (is_transport_polling_capable(info))
+ if (tx && is_polling_required(cinfo, info->desc)) {
+ if (is_transport_polling_capable(info->desc))
dev_info(&tdev->dev,
"Enabled polling mode TX channel - prot_id:%d\n",
prot_id);
@@ -2443,7 +2426,8 @@ static int scmi_probe(struct platform_device *pdev)
if (scmi_notification_init(handle))
dev_err(dev, "SCMI Notifications NOT available.\n");
- if (info->desc->atomic_enabled && !is_transport_polling_capable(info))
+ if (info->desc->atomic_enabled &&
+ !is_transport_polling_capable(info->desc))
dev_err(dev,
"Transport is not polling capable. Atomic mode not supported.\n");
--
2.34.1
Add a flags field to xfer and define a flagbit and related macro to easily
identify xfers originated from Raw transmissions.
Signed-off-by: Cristian Marussi <[email protected]>
---
v5 --> v6
- convert is_raw boolean to a bitflag
---
drivers/firmware/arm_scmi/protocols.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h
index 2f3bf691db7c..b5941beb8b03 100644
--- a/drivers/firmware/arm_scmi/protocols.h
+++ b/drivers/firmware/arm_scmi/protocols.h
@@ -115,6 +115,7 @@ struct scmi_msg_hdr {
* - SCMI_XFER_SENT_OK -> SCMI_XFER_RESP_OK [ -> SCMI_XFER_DRESP_OK ]
* - SCMI_XFER_SENT_OK -> SCMI_XFER_DRESP_OK
* (Missing synchronous response is assumed OK and ignored)
+ * @flags: Optional flags associated to this xfer.
* @lock: A spinlock to protect state and busy fields.
* @priv: A pointer for transport private usage.
*/
@@ -135,6 +136,9 @@ struct scmi_xfer {
#define SCMI_XFER_RESP_OK 1
#define SCMI_XFER_DRESP_OK 2
int state;
+#define SCMI_XFER_FLAG_IS_RAW BIT(0)
+#define SCMI_XFER_IS_RAW(x) ((x)->flags & SCMI_XFER_FLAG_IS_RAW)
+ int flags;
/* A lock to protect state and busy fields */
spinlock_t lock;
void *priv;
--
2.34.1
Add channel and platform instance indentifier to SCMI message dump traces
in order to easily associate message flows to specific transport channels.
Signed-off-by: Cristian Marussi <[email protected]>
---
drivers/firmware/arm_scmi/driver.c | 20 ++++++++++++--------
include/trace/events/scmi.h | 18 ++++++++++++------
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index af34324e923b..c765d0c51dc5 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -860,9 +860,9 @@ static void scmi_handle_notification(struct scmi_chan_info *cinfo,
info->desc->ops->fetch_notification(cinfo, info->desc->max_msg_size,
xfer);
- trace_scmi_msg_dump(xfer->hdr.protocol_id, xfer->hdr.id, "NOTI",
- xfer->hdr.seq, xfer->hdr.status,
- xfer->rx.buf, xfer->rx.len);
+ trace_scmi_msg_dump(info->id, cinfo->id, xfer->hdr.protocol_id,
+ xfer->hdr.id, "NOTI", xfer->hdr.seq,
+ xfer->hdr.status, xfer->rx.buf, xfer->rx.len);
scmi_notify(cinfo->handle, xfer->hdr.protocol_id,
xfer->hdr.id, xfer->rx.buf, xfer->rx.len, ts);
@@ -898,7 +898,8 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
smp_store_mb(xfer->priv, priv);
info->desc->ops->fetch_response(cinfo, xfer);
- trace_scmi_msg_dump(xfer->hdr.protocol_id, xfer->hdr.id,
+ trace_scmi_msg_dump(info->id, cinfo->id, xfer->hdr.protocol_id,
+ xfer->hdr.id,
xfer->hdr.type == MSG_TYPE_DELAYED_RESP ?
"DLYD" : "RESP",
xfer->hdr.seq, xfer->hdr.status,
@@ -1008,6 +1009,8 @@ static int scmi_wait_for_reply(struct device *dev, const struct scmi_desc *desc,
if (!ret) {
unsigned long flags;
+ struct scmi_info *info =
+ handle_to_scmi_info(cinfo->handle);
/*
* Do not fetch_response if an out-of-order delayed
@@ -1021,7 +1024,8 @@ static int scmi_wait_for_reply(struct device *dev, const struct scmi_desc *desc,
spin_unlock_irqrestore(&xfer->lock, flags);
/* Trace polled replies. */
- trace_scmi_msg_dump(xfer->hdr.protocol_id, xfer->hdr.id,
+ trace_scmi_msg_dump(info->id, cinfo->id,
+ xfer->hdr.protocol_id, xfer->hdr.id,
"RESP",
xfer->hdr.seq, xfer->hdr.status,
xfer->rx.buf, xfer->rx.len);
@@ -1157,9 +1161,9 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
return ret;
}
- trace_scmi_msg_dump(xfer->hdr.protocol_id, xfer->hdr.id, "CMND",
- xfer->hdr.seq, xfer->hdr.status,
- xfer->tx.buf, xfer->tx.len);
+ trace_scmi_msg_dump(info->id, cinfo->id, xfer->hdr.protocol_id,
+ xfer->hdr.id, "CMND", xfer->hdr.seq,
+ xfer->hdr.status, xfer->tx.buf, xfer->tx.len);
ret = scmi_wait_for_message_response(cinfo, xfer);
if (!ret && xfer->hdr.status)
diff --git a/include/trace/events/scmi.h b/include/trace/events/scmi.h
index f160d68f961d..422c1ad9484d 100644
--- a/include/trace/events/scmi.h
+++ b/include/trace/events/scmi.h
@@ -139,11 +139,15 @@ TRACE_EVENT(scmi_rx_done,
);
TRACE_EVENT(scmi_msg_dump,
- TP_PROTO(u8 protocol_id, u8 msg_id, unsigned char *tag, u16 seq,
- int status, void *buf, size_t len),
- TP_ARGS(protocol_id, msg_id, tag, seq, status, buf, len),
+ TP_PROTO(int id, u8 channel_id, u8 protocol_id, u8 msg_id,
+ unsigned char *tag, u16 seq, int status,
+ void *buf, size_t len),
+ TP_ARGS(id, channel_id, protocol_id, msg_id, tag, seq, status,
+ buf, len),
TP_STRUCT__entry(
+ __field(int, id)
+ __field(u8, channel_id)
__field(u8, protocol_id)
__field(u8, msg_id)
__array(char, tag, 5)
@@ -154,6 +158,8 @@ TRACE_EVENT(scmi_msg_dump,
),
TP_fast_assign(
+ __entry->id = id;
+ __entry->channel_id = channel_id;
__entry->protocol_id = protocol_id;
__entry->msg_id = msg_id;
strscpy(__entry->tag, tag, 5);
@@ -163,9 +169,9 @@ TRACE_EVENT(scmi_msg_dump,
memcpy(__get_dynamic_array(cmd), buf, __entry->len);
),
- TP_printk("pt=%02X t=%s msg_id=%02X seq=%04X s=%d pyld=%s",
- __entry->protocol_id, __entry->tag, __entry->msg_id,
- __entry->seq, __entry->status,
+ TP_printk("id=%d ch=%02X pt=%02X t=%s msg_id=%02X seq=%04X s=%d pyld=%s",
+ __entry->id, __entry->channel_id, __entry->protocol_id,
+ __entry->tag, __entry->msg_id, __entry->seq, __entry->status,
__print_hex_str(__get_dynamic_array(cmd), __entry->len))
);
#endif /* _TRACE_SCMI_H */
--
2.34.1
On Wed, 18 Jan 2023 12:14:09 +0000, Cristian Marussi wrote:
> This series aims to introduce a new SCMI unified userspace interface meant
> to ease testing an SCMI Server implementation for compliance, fuzzing etc.,
> from the perspective of the OSPM agent (non-secure world only ...)
>
> It is proposed as a testing/development facility, it is NOT meant to be a
> feature to use in production, but only enabled in Kconfig for test
> deployments.
>
> [...]
Applied to sudeep.holla/linux (for-next/scmi), thanks!
[01/17] firmware: arm_scmi: Refactor xfer in-flight registration routines
https://git.kernel.org/sudeep.holla/c/9374324e8d78
[02/17] firmware: arm_scmi: Refactor polling helpers
https://git.kernel.org/sudeep.holla/c/d5552a45c828
[03/17] firmware: arm_scmi: Refactor scmi_wait_for_message_response
https://git.kernel.org/sudeep.holla/c/ac49e46c453a
[04/17] firmware: arm_scmi: Add flags field to xfer
https://git.kernel.org/sudeep.holla/c/0eb24e5a0ead
[05/17] firmware: arm_scmi: Add xfer Raw helpers
https://git.kernel.org/sudeep.holla/c/aaa9d521b01a
[06/17] firmware: arm_scmi: Move errors defs and code to common.h
https://git.kernel.org/sudeep.holla/c/5e15c2197fc7
[07/17] firmware: arm_scmi: Add internal platform/channel IDs
https://git.kernel.org/sudeep.holla/c/5b6034808c96
[08/17] include: trace: Add platform and channel instance references
https://git.kernel.org/sudeep.holla/c/bd752cc79916
[09/17] debugfs: Export debugfs_create_str symbol
https://git.kernel.org/sudeep.holla/c/cade53b60a04
[10/17] firmware: arm_scmi: Populate a common SCMI debugsfs root
https://git.kernel.org/sudeep.holla/c/f5bc54a5041b
[11/17] firmware: arm_scmi: Add debugfs ABI documentation for common entries
https://git.kernel.org/sudeep.holla/c/b5dc013d98b4
[12/17] firmware: arm_scmi: Add core Raw transmission support
https://git.kernel.org/sudeep.holla/c/e789a8ab15b0
[13/17] firmware: arm_scmi: Add debugfs ABI documentation for Raw mode
https://git.kernel.org/sudeep.holla/c/25376d813405
[14/17] firmware: arm_scmi: Reject SCMI drivers while in Raw mode
https://git.kernel.org/sudeep.holla/c/9e834da670ce
[15/17] firmware: arm_scmi: Call Raw mode hooks from the core stack
https://git.kernel.org/sudeep.holla/c/a6fb9dc05c8c
[16/17] firmware: arm_scmi: Add Raw mode coexistence support
https://git.kernel.org/sudeep.holla/c/145f03626257
[17/17] firmware: arm_scmi: Add per-channel Raw injection support
https://git.kernel.org/sudeep.holla/c/eeb087fa365a
--
Regards,
Sudeep
Hi Christian,
On 1/18/23 04:14, Cristian Marussi wrote:
> Hi all,
>
> This series aims to introduce a new SCMI unified userspace interface meant
> to ease testing an SCMI Server implementation for compliance, fuzzing etc.,
> from the perspective of the OSPM agent (non-secure world only ...)
>
> It is proposed as a testing/development facility, it is NOT meant to be a
> feature to use in production, but only enabled in Kconfig for test
> deployments.
>
> Currently an SCMI Compliance Suite like the one at [1] can only work by
> injecting SCMI messages at the SCMI transport layer using the mailbox test
> driver (CONFIG_MAILBOX_TEST) via its few debugfs entries and looking at
> the related replies from the SCMI backend Server.
Took a while but finally:
Tested-by: Florian Fainelli <[email protected]>
Any idea when the raw_mode_support or the acs_raw_mode_support will hit
your master branch in the scmi-tests repository?
--
Florian
On Wed, 18 Jan 2023 at 13:15, Cristian Marussi <[email protected]> wrote:
>
> Hi all,
>
> This series aims to introduce a new SCMI unified userspace interface meant
> to ease testing an SCMI Server implementation for compliance, fuzzing etc.,
> from the perspective of the OSPM agent (non-secure world only ...)
>
> It is proposed as a testing/development facility, it is NOT meant to be a
> feature to use in production, but only enabled in Kconfig for test
> deployments.
>
> Currently an SCMI Compliance Suite like the one at [1] can only work by
> injecting SCMI messages at the SCMI transport layer using the mailbox test
> driver (CONFIG_MAILBOX_TEST) via its few debugfs entries and looking at
> the related replies from the SCMI backend Server.
>
> This approach has a few drawbacks:
>
> - the SCMI Server under test MUST be reachable through a mailbox based
> SCMI transport: any other SCMI Server placement is not possible (like in
> a VM reachable via SCMI Virtio). In order to cover other placements in
> the current scenario we should write some sort of test driver for each
> and every existent SCMI transport and for any future additional transport
> ...this clearly does not scale.
>
> - even in the mailbox case the userspace Compliance suite cannot simply
> send and receive bare SCMI messages BUT it has to properly lay them out
> into the shared memory exposed by the mailbox test driver as expected by
> the transport definitions. In other words such a userspace test
> application has to, not only use a proper transport driver for the system
> at hand, but it also has to have a comprehensive knowledge of the
> internals of the underlying transport in order to operate.
>
> - last but not least, the system under test has to be specifically
> configured and built, in terms of Kconfig and DT, to perform such kind of
> testing, it cannot be used for anything else, which is unfortunate for
> CI/CD deployments.
>
> This series introduces a new SCMI Raw mode support feature that, when
> configured and enabled exposes a new interface in debugfs through which:
>
> - a userspace application can inject bare SCMI binary messages into the
> SCMI core stack; such messages will be routed by the SCMI regular kernel
> stack to the backend Server using the currently configured transport
> transparently: in other words you can test the SCMI server, no matter
> where it is placed, as long as it is reachable from the currently
> configured SCMI stack.
> Same goes the other way around on the reading path: any SCMI server reply
> can be read as a bare SCMI binary message from the same debugfs path.
>
> - as a direct consequence of this way of injecting bare messages in the
> middle of the SCMI stack (instead of beneath it at the transport layer)
> the user application has to handle only bare SCMI messages without having
> to worry about the specific underlying transport internals that will be
> taken care of by the SCMI core stack itself using its own machinery,
> without duplicating such logic.
>
> - a system under test, once configured with SCMI Raw support enabled in
> Kconfig, can be booted without any particular DT change.
>
> Latest V6 additions:
>
> - improved scmi traces for msg dumps to include used channels
> - added a new common SCMI debugfs root fs
> - reworked SCMI Raw debugfs layout
> - added support of a new additional per-channel API that allows a user to
> select a specific egress channel for the message injection (when more
> than one channel is available)
>
> A quick and trivial example from the shell...reading from a sensor by
> injecting a properly crafted packet in raw mode (letting the stack select
> the channel):
>
> # INJECT THE SENSOR_READING MESSAGE FOR SENSOR ID=1 (binary little endian)
> root@deb-buster-arm64:~# echo -e -n \\x06\\x54\\x00\\x00\\x01\\x00\\x00\\x00\\x00\\x00\\x00\\x00 > /sys/kernel/debug/scmi/0/raw/message
>
> # READING BACK THE REPLY...
> root@deb-buster-arm64:~# cat /sys/kernel/debug/scmi/0/raw/message | od --endian=little -t x4
> 0000000 00005406 00000000 00000335 00000000
> 0000020
>
> while doing that, since Raw mode makes (partial) use of the regular SCMI
> stack, you can observe the messages going through the SCMI stack with the
> usual traces:
>
> bash-329 [000] ..... 14183.446808: scmi_msg_dump: id=0 ch=10 pt=15 t=cmnd msg_id=06 seq=0000 s=0 pyld=0100000000000000
> irq/35-mhu_db_l-81 [000] ..... 14183.447809: scmi_msg_dump: id=0 ch=10 pt=15 t=resp msg_id=06 seq=0000 s=0 pyld=3503000000000000
>
> ..trying to read in async when the backend server does NOT supports asyncs:
>
> # AN ASYNC SENSOR READING REQUEST...
> root@deb-buster-arm64:~# echo -e -n \\x06\\x54\\x00\\x00\\x01\\x00\\x00\\x00\\x01\\x00\\x00\\x00 > /sys/kernel/debug/scmi/0/raw/message_async
>
> bash-329 [000] ..... 16415.938739: scmi_msg_dump: id=0 ch=10 pt=15 t=cmnd msg_id=06 seq=0000 s=0 pyld=0100000001000000
> irq/35-mhu_db_l-81 [000] ..... 16415.944129: scmi_msg_dump: id=0 ch=10 pt=15 t=resp msg_id=06 seq=0000 s=-1 pyld=
>
> # RETURNS A STATUS -1 FROM THE SERVER NOT SUPPORTING IT
> root@deb-buster-arm64:~# cat /sys/kernel/debug/scmi/0/raw/message | od --endian=little -t x4
> 0000000 00005406 ffffffff
> 0000010
>
> Note that the above example was on a JUNO, BUT exactly the same steps can
> be used to reach an SCMI Server living on a VM reachable via virtio as
> long as the system under test if properly configured to work with a
> virtio transport.
>
> In a nutshell the exposed API is as follows:
>
> /sys/kernel/debug/scmi/
> `-- 0
> |-- atomic_threshold_us
> |-- instance_name
> |-- raw
> | |-- channels
> | | |-- 0x10
> | | | |-- message
> | | | `-- message_async
> | | `-- 0x13
> | | |-- message
> | | `-- message_async
> | |-- errors
> | |-- message
> | |-- message_async
> | |-- notification
> | `-- reset
> `-- transport
> |-- is_atomic
> |-- max_msg_size
> |-- max_rx_timeout_ms
> |-- rx_max_msg
> |-- tx_max_msg
> `-- type
>
> ... where at the top level:
>
> - <N>: a progressive sequence number identifying this SCMI instance, in
> case there are multiple SCMI instance defined
>
> - instance_name: can be used (by CI) to identify the SCMI instance <N>
> that you are using through this Raw accessors: it corresponds to the SCMI
> DT top node full name of the underlying SCMI instance
>
>
> ... rooted under /transport:
>
> - a bunch of configuration info useful to setup the user application
> expectations in terms of timeouts and message characteristics.
>
>
> ... rooted at /raw (the real SCMI Raw interface :D):
>
> - message*: used to send sync/async commands and read back immediate and
> delayed responses (if any)
> - errors: used to report timeout and unexpected replies
> - reset: used to reset the SCMI Raw stack, flushing all queues from
> received messages still pending to be read out (useful to be sure to
> cleanup between test suite runs...)
> - notification: used to read any notification being spit by the system
> (if previously enabled by the user app)
>
>
> ... rooted at /raw/channels/<M>/:
I haven't seen a description of the <M>. I figured out that this is
the protocol id to which the channel was associated in DT while
testing it but it could be good to describe this somewhere.
Apart from this minor thing, I have tested it with an scmi server
embedded in OPTEE and run the scmi compliance tests. Everything works
fine using the default mode or using one specific channel.
FWIW
Tested-by: Vincent Guittot <[email protected]>
Thanks
>
> - message*: used to send sync/async commands and read back immediate and
> delayed responses (if any), using a SPECIFIC transport channel <M>
> (instead of letting the system choose for you based on transport config
> and the rotocol embedded in the injected message)
> NOTE THAT these entries are optional, created only if there is more than
> transport channel defined on the system.
>
> Each write corresponds to one command request and the replies or delayed
> response are read back one message at time (receiving an EOF at each
> message boundary).
>
> The user application running the test is in charge of handling timeouts
> and properly choosing SCMI sequence numbers for the outgoing requests: note
> that the same fixed number can be re-used (...though discouraged...) as
> long as the suite does NOT expect to send multiple in-flight commands
> concurrently.
>
> Since the SCMI core regular stack is partially used to deliver and collect
> the messages, late replies after timeouts and any other sort of unexpected
> message sent by the SCMI server platform back can be identified by the SCMI
> core as usual and it will be reported under /errors for later analysis.
> (a userspace test-app will have anyway properly detected the timeout on
> /message* ...)
>
> All of the above has been roughly tested against a standard JUNO SCP SCMI
> Server (mailbox trans) and an emulated SCMI Server living in a VM (virtio
> trans) using a custom experimental version of the scmi-tests Compliance
> suite patched to support Raw mode and posted at [2]. (still in development
> ...merge requests are in progress...for now it is just a mean for me to
> test the testing API ... O_o)
>
> This V8 series is based on v6.2-rc1 PLUS another series which reworked a
> bit the SCMI core stack init/probe [3]; to ease testing a V8 properly
> based can be picked up from [4].
>
> Having said that (in such a concise and brief way :P) ...
>
> ...any feedback/comments are welcome !
>
> Thanks,
> Cristian
>
> ---
> v7 --> v8
> - fixed a few bad handling on error path
> - ignoring debugfs_ retvals as supposed to
> - using XArray for per-channel queus instead of IDRs
> - refactored debugfs setup calls
>
> v6 --> v7
> - fixed one sparse error
> - removed redundant info.num_chans/channels fields: enumerate them dyamically
> once needed.
>
> v5 --> v6
> - exported symbol debugfs_create_str
> - rebased on top of v6.2-rc1 plus series at [3]
> - redesigned SCMI debugfs layout with a bunch of common entries
> enabled by implicit CONFIG_ARM_SCMI_NEED_DEBUGFS
> - refactored SCMI Raw internal queues handling
> - added SCMI Raw per-channel injection support
> - added channels info on SCMI msg_dump traces
> - fix debugfs multiple writers cases
> - added DEBUG_FS dependency to Raw mode
> - select CONFIG_ARM_SCMI_NEED_DEBUGFS when Raw mode is compiled
>
> v4 --> v5
> - rebased on sudeep/for-next/scmi
> - added multiple SCMI instances support
> - added optional Raw full-cohexistence mode
> - use custom tags to distinguish Raw msg_dump traces
> - add circular handling of raw buffers queues for errors and notifications
>
> V3 --> v4
> - rebased on v6.1-rc1
> - addedd missing support for 'polled' transports and transport lacking a
> completion_irq (like smc/optee)
> - removed a few inlines
> - refactored SCMI Raw RX patch to make use more extensively of the regular
> non-Raw RX path
> - fix handling of O_NONBLOCK raw_mode read requests
>
> v2 --> v3
> - fixed some sparse warning on LE and __poll_t
> - reworked and simplified deferred worker in charge of xfer delayed waiting
> - allow for injection of DT-unknown protocols messages when in Raw mode
> (needed for any kind of fuzzing...)
>
> v1 --> v2
> - added comments and debugfs docs
> - added dedicated transport devices for channels initialization
> - better channels handling in Raw mode
> - removed runtime enable, moved to static compile time exclusion
> of SCMI regular stack
>
> [1]: https://gitlab.arm.com/tests/scmi-tests
> [2]: https://gitlab.arm.com/tests/scmi-tests/-/commits/raw_mode_support_devel/
> [3]: https://lore.kernel.org/all/[email protected]/
> [4]: https://gitlab.arm.com/linux-arm/linux-cm/-/commits/scmi_raw_mode_V8/
>
>
> Cristian Marussi (17):
> firmware: arm_scmi: Refactor xfer in-flight registration routines
> firmware: arm_scmi: Refactor polling helpers
> firmware: arm_scmi: Refactor scmi_wait_for_message_response
> firmware: arm_scmi: Add flags field to xfer
> firmware: arm_scmi: Add xfer Raw helpers
> firmware: arm_scmi: Move errors defs and code to common.h
> firmware: arm_scmi: Add internal platform/channel IDs
> include: trace: Add platform and channel instance references
> debugfs: Export debugfs_create_str symbol
> firmware: arm_scmi: Populate a common SCMI debugsfs root
> firmware: arm_scmi: Add debugfs ABI documentation for common entries
> firmware: arm_scmi: Add core Raw transmission support
> firmware: arm_scmi: Add debugfs ABI documentation for Raw mode
> firmware: arm_scmi: Reject SCMI drivers while in Raw mode
> firmware: arm_scmi: Call Raw mode hooks from the core stack
> firmware: arm_scmi: Add Raw mode coexistence support
> firmware: arm_scmi: Add per-channel Raw injection support
>
> Documentation/ABI/testing/debugfs-scmi | 70 +
> Documentation/ABI/testing/debugfs-scmi-raw | 109 ++
> drivers/firmware/arm_scmi/Kconfig | 32 +
> drivers/firmware/arm_scmi/Makefile | 1 +
> drivers/firmware/arm_scmi/bus.c | 7 +
> drivers/firmware/arm_scmi/common.h | 75 +
> drivers/firmware/arm_scmi/driver.c | 638 +++++++--
> drivers/firmware/arm_scmi/protocols.h | 7 +
> drivers/firmware/arm_scmi/raw_mode.c | 1443 ++++++++++++++++++++
> drivers/firmware/arm_scmi/raw_mode.h | 31 +
> fs/debugfs/file.c | 1 +
> include/trace/events/scmi.h | 18 +-
> 12 files changed, 2292 insertions(+), 140 deletions(-)
> create mode 100644 Documentation/ABI/testing/debugfs-scmi
> create mode 100644 Documentation/ABI/testing/debugfs-scmi-raw
> create mode 100644 drivers/firmware/arm_scmi/raw_mode.c
> create mode 100644 drivers/firmware/arm_scmi/raw_mode.h
>
> --
> 2.34.1
>
On Fri, Jan 20, 2023 at 09:11:24AM +0100, Vincent Guittot wrote:
> On Wed, 18 Jan 2023 at 13:15, Cristian Marussi <[email protected]> wrote:
> >
> > Hi all,
> >
> > This series aims to introduce a new SCMI unified userspace interface meant
> > to ease testing an SCMI Server implementation for compliance, fuzzing etc.,
> > from the perspective of the OSPM agent (non-secure world only ...)
> >
> > It is proposed as a testing/development facility, it is NOT meant to be a
> > feature to use in production, but only enabled in Kconfig for test
> > deployments.
> >
> > Currently an SCMI Compliance Suite like the one at [1] can only work by
> > injecting SCMI messages at the SCMI transport layer using the mailbox test
> > driver (CONFIG_MAILBOX_TEST) via its few debugfs entries and looking at
> > the related replies from the SCMI backend Server.
> >
> > This approach has a few drawbacks:
> >
> > - the SCMI Server under test MUST be reachable through a mailbox based
> > SCMI transport: any other SCMI Server placement is not possible (like in
> > a VM reachable via SCMI Virtio). In order to cover other placements in
> > the current scenario we should write some sort of test driver for each
> > and every existent SCMI transport and for any future additional transport
> > ...this clearly does not scale.
> >
> > - even in the mailbox case the userspace Compliance suite cannot simply
> > send and receive bare SCMI messages BUT it has to properly lay them out
> > into the shared memory exposed by the mailbox test driver as expected by
> > the transport definitions. In other words such a userspace test
> > application has to, not only use a proper transport driver for the system
> > at hand, but it also has to have a comprehensive knowledge of the
> > internals of the underlying transport in order to operate.
> >
> > - last but not least, the system under test has to be specifically
> > configured and built, in terms of Kconfig and DT, to perform such kind of
> > testing, it cannot be used for anything else, which is unfortunate for
> > CI/CD deployments.
> >
> > This series introduces a new SCMI Raw mode support feature that, when
> > configured and enabled exposes a new interface in debugfs through which:
> >
> > - a userspace application can inject bare SCMI binary messages into the
> > SCMI core stack; such messages will be routed by the SCMI regular kernel
> > stack to the backend Server using the currently configured transport
> > transparently: in other words you can test the SCMI server, no matter
> > where it is placed, as long as it is reachable from the currently
> > configured SCMI stack.
> > Same goes the other way around on the reading path: any SCMI server reply
> > can be read as a bare SCMI binary message from the same debugfs path.
> >
> > - as a direct consequence of this way of injecting bare messages in the
> > middle of the SCMI stack (instead of beneath it at the transport layer)
> > the user application has to handle only bare SCMI messages without having
> > to worry about the specific underlying transport internals that will be
> > taken care of by the SCMI core stack itself using its own machinery,
> > without duplicating such logic.
> >
> > - a system under test, once configured with SCMI Raw support enabled in
> > Kconfig, can be booted without any particular DT change.
> >
> > Latest V6 additions:
> >
> > - improved scmi traces for msg dumps to include used channels
> > - added a new common SCMI debugfs root fs
> > - reworked SCMI Raw debugfs layout
> > - added support of a new additional per-channel API that allows a user to
> > select a specific egress channel for the message injection (when more
> > than one channel is available)
> >
> > A quick and trivial example from the shell...reading from a sensor by
> > injecting a properly crafted packet in raw mode (letting the stack select
> > the channel):
> >
> > # INJECT THE SENSOR_READING MESSAGE FOR SENSOR ID=1 (binary little endian)
> > root@deb-buster-arm64:~# echo -e -n \\x06\\x54\\x00\\x00\\x01\\x00\\x00\\x00\\x00\\x00\\x00\\x00 > /sys/kernel/debug/scmi/0/raw/message
> >
> > # READING BACK THE REPLY...
> > root@deb-buster-arm64:~# cat /sys/kernel/debug/scmi/0/raw/message | od --endian=little -t x4
> > 0000000 00005406 00000000 00000335 00000000
> > 0000020
> >
> > while doing that, since Raw mode makes (partial) use of the regular SCMI
> > stack, you can observe the messages going through the SCMI stack with the
> > usual traces:
> >
> > bash-329 [000] ..... 14183.446808: scmi_msg_dump: id=0 ch=10 pt=15 t=cmnd msg_id=06 seq=0000 s=0 pyld=0100000000000000
> > irq/35-mhu_db_l-81 [000] ..... 14183.447809: scmi_msg_dump: id=0 ch=10 pt=15 t=resp msg_id=06 seq=0000 s=0 pyld=3503000000000000
> >
> > ..trying to read in async when the backend server does NOT supports asyncs:
> >
> > # AN ASYNC SENSOR READING REQUEST...
> > root@deb-buster-arm64:~# echo -e -n \\x06\\x54\\x00\\x00\\x01\\x00\\x00\\x00\\x01\\x00\\x00\\x00 > /sys/kernel/debug/scmi/0/raw/message_async
> >
> > bash-329 [000] ..... 16415.938739: scmi_msg_dump: id=0 ch=10 pt=15 t=cmnd msg_id=06 seq=0000 s=0 pyld=0100000001000000
> > irq/35-mhu_db_l-81 [000] ..... 16415.944129: scmi_msg_dump: id=0 ch=10 pt=15 t=resp msg_id=06 seq=0000 s=-1 pyld=
> >
> > # RETURNS A STATUS -1 FROM THE SERVER NOT SUPPORTING IT
> > root@deb-buster-arm64:~# cat /sys/kernel/debug/scmi/0/raw/message | od --endian=little -t x4
> > 0000000 00005406 ffffffff
> > 0000010
> >
> > Note that the above example was on a JUNO, BUT exactly the same steps can
> > be used to reach an SCMI Server living on a VM reachable via virtio as
> > long as the system under test if properly configured to work with a
> > virtio transport.
> >
> > In a nutshell the exposed API is as follows:
> >
> > /sys/kernel/debug/scmi/
> > `-- 0
> > |-- atomic_threshold_us
> > |-- instance_name
> > |-- raw
> > | |-- channels
> > | | |-- 0x10
> > | | | |-- message
> > | | | `-- message_async
> > | | `-- 0x13
> > | | |-- message
> > | | `-- message_async
> > | |-- errors
> > | |-- message
> > | |-- message_async
> > | |-- notification
> > | `-- reset
> > `-- transport
> > |-- is_atomic
> > |-- max_msg_size
> > |-- max_rx_timeout_ms
> > |-- rx_max_msg
> > |-- tx_max_msg
> > `-- type
> >
> > ... where at the top level:
> >
> > - <N>: a progressive sequence number identifying this SCMI instance, in
> > case there are multiple SCMI instance defined
> >
> > - instance_name: can be used (by CI) to identify the SCMI instance <N>
> > that you are using through this Raw accessors: it corresponds to the SCMI
> > DT top node full name of the underlying SCMI instance
> >
> >
> > ... rooted under /transport:
> >
> > - a bunch of configuration info useful to setup the user application
> > expectations in terms of timeouts and message characteristics.
> >
> >
> > ... rooted at /raw (the real SCMI Raw interface :D):
> >
> > - message*: used to send sync/async commands and read back immediate and
> > delayed responses (if any)
> > - errors: used to report timeout and unexpected replies
> > - reset: used to reset the SCMI Raw stack, flushing all queues from
> > received messages still pending to be read out (useful to be sure to
> > cleanup between test suite runs...)
> > - notification: used to read any notification being spit by the system
> > (if previously enabled by the user app)
> >
> >
> > ... rooted at /raw/channels/<M>/:
>
Hi Vincent,
thanks for trying this out.
> I haven't seen a description of the <M>. I figured out that this is
> the protocol id to which the channel was associated in DT while
> testing it but it could be good to describe this somewhere.
Ah, damn yes, I had made a note to myself to add an explicit description
of how the channel number IDs are chosen, then I forgot :P
I'll add a follow-on patch once is queued.
> Apart from this minor thing, I have tested it with an scmi server
> embedded in OPTEE and run the scmi compliance tests. Everything works
> fine using the default mode or using one specific channel.
>
> FWIW
> Tested-by: Vincent Guittot <[email protected]>
>
Thanks,
Cristian
On Thu, Jan 19, 2023 at 01:47:04PM -0800, Florian Fainelli wrote:
> Hi Christian,
>
Hi Florian,
> On 1/18/23 04:14, Cristian Marussi wrote:
> > Hi all,
> >
> > This series aims to introduce a new SCMI unified userspace interface meant
> > to ease testing an SCMI Server implementation for compliance, fuzzing etc.,
> > from the perspective of the OSPM agent (non-secure world only ...)
> >
> > It is proposed as a testing/development facility, it is NOT meant to be a
> > feature to use in production, but only enabled in Kconfig for test
> > deployments.
> >
> > Currently an SCMI Compliance Suite like the one at [1] can only work by
> > injecting SCMI messages at the SCMI transport layer using the mailbox test
> > driver (CONFIG_MAILBOX_TEST) via its few debugfs entries and looking at
> > the related replies from the SCMI backend Server.
>
> Took a while but finally:
>
> Tested-by: Florian Fainelli <[email protected]>
>
Thanks for giving it a go.
> Any idea when the raw_mode_support or the acs_raw_mode_support will hit your
> master branch in the scmi-tests repository?
I was holding off waiting for this series to be merged to be sure the
debugfs ABI was not changing anymore, so now, soon-ish, I'll move to request
merge the Raw support in the ACS too; the only further delay regarding this,
that I can think of, could be that I know QA is also adding v3.1 tests to the
ACS and they are too going for the MR in these days, so this could delay a bit
further the Raw support merge.
Thanks,
Cristian
On Fri, Jan 20, 2023 at 09:56:32AM +0000, Cristian Marussi wrote:
> On Fri, Jan 20, 2023 at 09:11:24AM +0100, Vincent Guittot wrote:
>
> Hi Vincent,
>
> thanks for trying this out.
>
> > I haven't seen a description of the <M>. I figured out that this is
> > the protocol id to which the channel was associated in DT while
> > testing it but it could be good to describe this somewhere.
>
> Ah, damn yes, I had made a note to myself to add an explicit description
> of how the channel number IDs are chosen, then I forgot :P
>
> I'll add a follow-on patch once is queued.
>
Please post it ASAP so that I can include it before I tag, just trying
to avoid missing it now and sending it as fix later :).
> > Apart from this minor thing, I have tested it with an scmi server
> > embedded in OPTEE and run the scmi compliance tests. Everything works
> > fine using the default mode or using one specific channel.
> >
> > FWIW
> > Tested-by: Vincent Guittot <[email protected]>
Thanks Vincent and Florian. I just pulled and merged this last night, will
update to include your Tested-by tags.
--
Regards,
Sudeep
On Wed, 18 Jan 2023 12:14:09 +0000, Cristian Marussi wrote:
> This series aims to introduce a new SCMI unified userspace interface meant
> to ease testing an SCMI Server implementation for compliance, fuzzing etc.,
> from the perspective of the OSPM agent (non-secure world only ...)
>
> It is proposed as a testing/development facility, it is NOT meant to be a
> feature to use in production, but only enabled in Kconfig for test
> deployments.
>
> [...]
Re-applied to sudeep.holla/linux (for-next/scmi) with Tested-by tags now, thanks!
[01/17] firmware: arm_scmi: Refactor xfer in-flight registration routines
https://git.kernel.org/sudeep.holla/c/b0e924a955cb
[02/17] firmware: arm_scmi: Refactor polling helpers
https://git.kernel.org/sudeep.holla/c/f21c2b0ba8f3
[03/17] firmware: arm_scmi: Refactor scmi_wait_for_message_response
https://git.kernel.org/sudeep.holla/c/07cdfc44f1a4
[04/17] firmware: arm_scmi: Add flags field to xfer
https://git.kernel.org/sudeep.holla/c/37057bf2b509
[05/17] firmware: arm_scmi: Add xfer Raw helpers
https://git.kernel.org/sudeep.holla/c/3095a3e25d8f
[06/17] firmware: arm_scmi: Move errors defs and code to common.h
https://git.kernel.org/sudeep.holla/c/936a2b91c2e8
[07/17] firmware: arm_scmi: Add internal platform/channel IDs
https://git.kernel.org/sudeep.holla/c/75c86dc72dc8
[08/17] include: trace: Add platform and channel instance references
https://git.kernel.org/sudeep.holla/c/8b2bd71119dd
[09/17] debugfs: Export debugfs_create_str symbol
https://git.kernel.org/sudeep.holla/c/d60b59b96795
[10/17] firmware: arm_scmi: Populate a common SCMI debugsfs root
https://git.kernel.org/sudeep.holla/c/c3d4aed763ce
[11/17] firmware: arm_scmi: Add debugfs ABI documentation for common entries
https://git.kernel.org/sudeep.holla/c/0f62ed0092ec
[12/17] firmware: arm_scmi: Add core Raw transmission support
https://git.kernel.org/sudeep.holla/c/3c3d818a9317
[13/17] firmware: arm_scmi: Add debugfs ABI documentation for Raw mode
https://git.kernel.org/sudeep.holla/c/74225707b334
[14/17] firmware: arm_scmi: Reject SCMI drivers while in Raw mode
https://git.kernel.org/sudeep.holla/c/ba80acb0dfca
[15/17] firmware: arm_scmi: Call Raw mode hooks from the core stack
https://git.kernel.org/sudeep.holla/c/7063887b5386
[16/17] firmware: arm_scmi: Add Raw mode coexistence support
https://git.kernel.org/sudeep.holla/c/9c54633e4e3d
[17/17] firmware: arm_scmi: Add per-channel Raw injection support
https://git.kernel.org/sudeep.holla/c/7860701d1e6e
--
Regards,
Sudeep