Re: [PATCH v3 0/9] Introduce a unified API for SCMI Server testing

[Vincent Guittot]

Hi Cristian,

On Sat, 3 Sept 2022 at 20:31, Cristian Marussi <cristian.marussi@xxxxxxx> 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.
>

...

>
> In V2 the runtime enable/disable switching capability has been removed
> (for now) since still not deemed to be stable/reliable enough: as a
> consequence when SCMI Raw support is compiled in, the regular SCMI stack
> drivers are now inhibited permanently for that Kernel.
>
> A quick and trivial example from the shell...reading from a sensor
> injecting a properly crafted packet in raw mode:
>
>         # 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_raw/message

I have tried your patchset with an SCMI server using an optee-os
transport channel but I have a timed out error when trying your
example above to read sensor1

#  echo -e -n \\x06\\x54\\x00\\x00\\x01\\x00\\x00\\x00\\x00\\x00\\x00\\x00
> /sys/kernel/debug/scmi_raw/message
# [   93.306690] arm-scmi firmware:scmi0: timed out in RAW response -
HDR:00005406

and there no response available when trying to read it with
# cat /sys/kernel/debug/scmi_raw/message


The sensor 1 can be successfully read in normal mode:
# cat /sys/class/hwmon/hwmon0/temp1_input
25000
#

In both case, the SCMI server received the requests and replied successfully

Could it be that you process the answer differently in case of raw mode ?

>
>         # READING BACK THE REPLY...
>         root@deb-buster-arm64:~# cat /sys/kernel/debug/scmi_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: 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: 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_raw/message_async
>
>               bash-329     [000] ..... 16415.938739: scmi_msg_dump: 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: 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_raw/message | od --endian=little -t x4
>         0000000 00005406 ffffffff
>         0000010
>
> Note that this 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_raw/
> ├── errors
> ├── message
> ├── message_async
> ├── notification
> ├── reset
> ├── transport_max_msg_size
> ├── transport_rx_timeout_ms
> └── transport_tx_max_msg
>
> where:
>
>  - 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)
>  - transport*: a bunch of configuration useful to setup the user
>    application expectations in terms of timeouts and message
>    characteristics.
>
> 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
> ...certainly not up for review as of now...it is just a mean for me to
> test the testing API ... O_o)
>
> The series is based on sudeep/for-next/scmi [3] on top of:
>
> commit 40d30cf680cb ("firmware: arm_scmi: Harmonize SCMI tracing message format")
>
> Still todo:
>
> - needs more complete testing, ideally running to completion at least the full
>   SCMI compliance suite at [2] to use it as a reference
> - more feedback on the API
>
> Having said that (in such a concise and brief way :P) ...
>
> ...any feedback/comments are welcome !
>
> Thanks,
> Cristian
>
> ---
> 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/linux-arm/scmi-tests-cm/-/commits/raw_mode_support_V3/
> [3]: https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/log/?h=for-next/scmi
>
> Cristian Marussi (9):
>   firmware: arm_scmi: Refactor xfer in-flight registration routines
>   firmware: arm_scmi: Simplify chan_available transport operation
>   firmware: arm_scmi: Use dedicated devices to initialize channels
>   firmware: arm_scmi: Add xfer raw helpers
>   firmware: arm_scmi: Move errors defs and code to common.h
>   firmware: arm_scmi: Add 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
>
>  Documentation/ABI/testing/debugfs-scmi-raw |   88 ++
>  drivers/firmware/arm_scmi/Kconfig          |   13 +
>  drivers/firmware/arm_scmi/Makefile         |    1 +
>  drivers/firmware/arm_scmi/common.h         |   51 +-
>  drivers/firmware/arm_scmi/driver.c         |  389 ++++--
>  drivers/firmware/arm_scmi/mailbox.c        |    4 +-
>  drivers/firmware/arm_scmi/optee.c          |    4 +-
>  drivers/firmware/arm_scmi/raw_mode.c       | 1235 ++++++++++++++++++++
>  drivers/firmware/arm_scmi/raw_mode.h       |   29 +
>  drivers/firmware/arm_scmi/smc.c            |    4 +-
>  drivers/firmware/arm_scmi/virtio.c         |    2 +-
>  11 files changed, 1714 insertions(+), 106 deletions(-)
>  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.32.0
>