Re: [PATCH v15 6/6] docs: arm: add SCI SCMI SMC multi-agent driver docs

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi,


I'm sorry, I postponed documentation review till we finished with code
part. This was my mistake. I didn't expected that it require serious
proofreading. Anyway, I tried to highlight most glaring mistakes, but it
would be great if you proofread it by yourself once again.

Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx> writes:

> From: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
>
> Add SCI SCMI SMC multi-agent driver documentation.
> It includes a detailed description of the SCMI multi-agent driver.
> This document explains the driver's functionality, configuration,
> and the compilation process. The Xen SCMI multi-agent driver is
> designed to provide SCMI access to system resources from different
> domains.
>
> Signed-off-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx>
> ---
>
> (no changes since v14)
>
> Changes in v14:
> - describe secondary agents as xen,scmi-agent nodes under agents
> - update host, discovery and dom0less examples for the child-node format
> - clarify shared-memory placement and platform RAM reservations
>
> Changes in v10:
> - rephrase section about /firmware/scmi. Mentioned that this node is
> taken from Host DT and copied unmodified.
> - fix xen,reg address for secondary domains for Dom0less configuration
>
> Changes in v8:
> - update documentation to match the last DT format
> - fixed RST: "... code-block:: dts" -> ".. code-block:: dts"
> - update documentation with dom0less configuration example
> - update documentation with new param xen,dom0-sci-agent-id
> instead of the command line parameter
>
> Changes in v7:
> - update documentation in section of the xen_scmi configuration which
> is matched by "xen,sci" compatible instead of the direct path.
>
> Changes in v6:
> - remove all HVC mentions from the multi-agent doc
> - update sci-agent-id parameter description in the documentation
> - add missing Sign-of
> - minor fixes across the document
>
> Changes in v5:
> - rework multi-agent driver to leave Host Device-tree unmodified
>
>  .../arm/firmware/arm-scmi.rst                 | 405 ++++++++++++++++++
>  1 file changed, 405 insertions(+)
>
> diff --git a/docs/hypervisor-guide/arm/firmware/arm-scmi.rst 
> b/docs/hypervisor-guide/arm/firmware/arm-scmi.rst
> index d9698f4e4b..25459d22dc 100644
> --- a/docs/hypervisor-guide/arm/firmware/arm-scmi.rst
> +++ b/docs/hypervisor-guide/arm/firmware/arm-scmi.rst
> @@ -36,6 +36,8 @@ The below sections describe SCMI support options available 
> for Xen.
>
>  | [1] `Arm SCMI
>  | <https://developer.arm.com/documentation/den0056/latest/>`_
>  | [2] `System Control and Management Interface (SCMI) bindings
>  | 
> <https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/firmware/arm,scmi.yaml>`_
> +| [3] `Generic Domain Access Controllers bindings 
> <https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/access-controllers/access-controllers.yaml>`_
> +
>
>  Simple SCMI over SMC calls forwarding driver (EL3)
>  ------------------------------------------------------
> @@ -189,3 +191,406 @@ except explicitly enabling SCMI with "arm_sci" xl.cfg 
> option.
>      ->        xen,reg = <0x0 0x47ff0000 0x0 0x1000 0x0 0x22001000>;
>      ->        xen,force-assign-without-iommu;
>        };
> +
> +SCMI SMC multi-agent driver (EL3)
> +-------------------------------------
> +
> +The SCMI SMC multi-agent driver enables support for ARM EL3 Trusted 
> Firmware-A (TF-A) which
> +provides SCMI interface with multi-agent support, as shown below.
> +
> +::
> +
> +      +-----------------------------------------+
> +      |                                         |
> +      | EL3 TF-A SCMI                           |
> +      +-------+--+-------+--+-------+--+-------++
> +      |shmem1 |  |shmem0 |  |shmem2 |  |shmemX |
> +      +-----+-+  +---+---+  +--+----+  +---+---+
> +    smc-id1 |        |         |           |
> +    agent1  |        |         |           |
> +      +-----v--------+---------+-----------+----+
> +      |              |         |           |    |

Xen, I believe?

> +      |              |         |           |    |
> +      +--------------+---------+-----------+----+
> +             smc-id0 |  smc-id2|    smc-idX|
> +             agent0  |  agent2 |    agentX |
> +                     |         |           |
> +                +----v---+  +--v-----+  +--v-----+
> +                |        |  |        |  |        |
> +                | Dom0   |  | Dom1   |  | DomX   |
> +                |        |  |        |  |        |
> +                |        |  |        |  |        |
> +                +--------+  +--------+  +--------+
> +
> +The EL3 SCMI multi-agent firmware is expected to provide SCMI SMC 
> shared-memory transport
> +for every Agent in the system. The SCMI Agent transport channel defined by 
> pair:
> +
> +- smc-id: SMC function id used for Doorbell
> +- shmem: shared memory for messages transfer, **Xen page aligned**.
> +  Shared memory is mapped with the following flags: MT_DEVICE_nGnRE and 
> _PAGE_DEVICE, indicating that this

Do we have any line length limitation for .rst files? This one is 114
characters long.

> +  memory is mapped as device memory.
> +
> +The following SCMI Agents are expected to be defined by SCMI FW to enable 
> SCMI multi-agent functionality
> +under Xen:
> +
> +- Xen management agent: trusted agents that accesses to the Base Protocol 
> commands to configure

"trusted agent", not "agents"

also, "that accesses the Base Protocol commands to configure..." or "that has
access to the Base Protocol commands to configure..."

> +  agent specific permissions

"agent-specific"

> +- OSPM VM agents: non-trusted agent, one for each Guest domain which is  
> allowed direct HW access.

"non-trusted agents". Also, double space between "is" and "allowed"

> +  At least one OSPM VM agent has to be provided by FW if HW is handled only 
> by Dom0 or Driver Domain.

"Hardware Domain", probably

> +
> +The EL3 SCMI FW is expected to implement following Base protocol messages:
> +
> +- BASE_DISCOVER_AGENT (required for Xen; required for secondary agents 
> without ``reg``)
> +- BASE_RESET_AGENT_CONFIGURATION (optional)

How Xen can guarantee that SCMI state is sane, if it can't reset agent
during domain destruction?

> +- BASE_SET_DEVICE_PERMISSIONS (optional)

If this is optional, does it mean that each agent will have access to
all devices in the system?

> +
> +The number of supported SCMI agents and their transport specifications are 
> SCMI FW implementation
> +specific.
> +
> +Compiling with multi-agent support
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +To build with the SCMI SMC multi-agent driver support, enable Kconfig option:
> +
> +::
> +
> +    CONFIG_SCMI_SMC_MA
> +
> +
> +Driver functionality
> +^^^^^^^^^^^^^^^^^^^^
> +
> +The SCI SCMI SMC multi-agent driver implements following functionality:
> +
> +- The driver is initialized from the Xen SCMI container ``xen_scmi_config``

Why "container"? It is called "node" in Device Tree lingo.

> +  under ``/chosen/xen`` (for example ``/chosen/xen/xen_scmi_config/scmi``).
> +  Only one SCMI interface is supported. The SCMI configuration must live 
> under
> +  the Xen SCMI container ``xen,sci`` beneath ``/chosen``.
> +  The Xen SCMI mediator will bind only to the "arm,scmi-smc" node that is a 
> child of
> +  this "xen,sci" container; any other "arm,scmi-smc" nodes (for example under
> +  "/firmware") are ignored to avoid stealing the host's SCMI OSPM instance.
> +
> +.. code-block:: dts
> +
> +        scmi_shm_1: sram@47ff1000 {
> +            compatible = "arm,scmi-shmem";
> +            reg = <0x0 0x47ff1000 0x0 0x1000>;
> +        };
> +        scmi_xen: scmi {
> +          compatible = "arm,scmi-smc";
> +          arm,smc-id = <0x82000003>; /* Xen management agent */
> +          #address-cells = < 1>;
> +          #size-cells = < 0>;
> +          #access-controller-cells = < 1>;
> +          shmem = <&scmi_shm_1>; /* Xen management shared memory */
> +        };
> +
> +.. note::
> +   This layout keeps the Host DT unchanged for Dom0 and baremetal Linux by
> +   using func_id 0x82000002 / shmem 0x47ff0000 for Dom0, while Xen uses a
> +   separate privileged channel func_id 0x82000003 / shmem 0x47ff1000. EL3
> +   firmware enforces permissions per agent_id, so there is no conflict 
> between
> +   Dom0 and Xen channels.
> +
> +- The driver reads secondary agents from an ``agents`` child of the

"child node"

> +  ``xen,sci`` container. This child has ``#address-cells = <1>`` and
> +  ``#size-cells = <0>``. Each enabled ``xen,scmi-agent`` child specifies its
> +  doorbell in ``arm,smc-id`` and a single shared-memory phandle in ``shmem``.
> +  The optional single-cell ``reg`` property supplies the SCMI agent ID
> +  (0..254). IDs must be unique and distinct from Xen's management agent.
> +  At least one enabled secondary agent is required.
> +
> +  The shared-memory nodes remain under ``xen_scmi_config``; the host OSPM

I don't think that "remain" is a correct word here. Maybe "stay?". Or
reword it altogether: "The SCMI-specific ``shared-memory`` nodes should
be placed in ``xen_scmi_config`` node"

> +  shared-memory node and ``/firmware/scmi`` remain in the platform DT.
> +  A shared-memory node describes a channel location and does not reserve RAM.
> +  If buffers occupy system RAM, the platform must reserve the entire
> +  firmware-owned range in ``/reserved-memory`` before Xen boots. A single
> +  reservation may cover multiple channels, including unused ones. SRAM
> +  outside system RAM does not require a RAM reservation. The channel nodes
> +  do not have to be moved into ``/reserved-memory``.
> +
> +.. code-block:: dts
> +
> +    / {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        scmi_shm_0: sram@47ff0000 {
> +            compatible = "arm,scmi-shmem";
> +            reg = <0x0 0x47ff0000 0x0 0x1000>;
> +        };
> +
> +        firmware {
> +            scmi: scmi {
> +                compatible = "arm,scmi-smc";
> +                arm,smc-id = <0x82000002>; /* Host OSPM agent smc-id */
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                shmem = <&scmi_shm_0>; /* Host OSPM agent shmem */
> +            };
> +        };
> +
> +        chosen {
> +            #address-cells = <2>;
> +            #size-cells = <2>;
> +            ranges;
> +
> +            xen {
> +                #address-cells = <2>;
> +                #size-cells = <2>;
> +                ranges;
> +
> +                xen_scmi_config {
> +                    compatible = "xen,sci";
> +                    #address-cells = <2>;
> +                    #size-cells = <2>;
> +                    ranges;
> +                    xen,dom0-sci-agent-id = <0>;
> +
> +                    /* Xen management channel */
> +                    scmi_shm_1: sram@47ff1000 {
> +                        compatible = "arm,scmi-shmem";
> +                        reg = <0x0 0x47ff1000 0x0 0x1000>;
> +                    };
> +
> +                    scmi_shm_2: sram@47ff2000 {
> +                        compatible = "arm,scmi-shmem";
> +                        reg = <0x0 0x47ff2000 0x0 0x1000>;
> +                    };
> +
> +                    scmi_shm_3: sram@47ff3000 {
> +                        compatible = "arm,scmi-shmem";
> +                        reg = <0x0 0x47ff3000 0x0 0x1000>;
> +                    };
> +
> +                    scmi_xen: scmi {
> +                        compatible = "arm,scmi-smc";
> +                        arm,smc-id = <0x82000003>; /* Xen management smc-id 
> */
> +                        #address-cells = <1>;
> +                        #size-cells = <0>;
> +                        #access-controller-cells = <1>;
> +                        shmem = <&scmi_shm_1>; /* Xen management agent shmem 
> */
> +                    };
> +
> +                    agents {
> +                        #address-cells = <1>;
> +                        #size-cells = <0>;
> +
> +                        agent@0 {
> +                            compatible = "xen,scmi-agent";
> +                            reg = <0>;
> +                            arm,smc-id = <0x82000002>;
> +                            shmem = <&scmi_shm_0>;
> +                        };
> +
> +                        agent@2 {
> +                            compatible = "xen,scmi-agent";
> +                            reg = <2>;
> +                            arm,smc-id = <0x82000004>;
> +                            shmem = <&scmi_shm_2>;
> +                        };
> +
> +                        agent@3 {
> +                            compatible = "xen,scmi-agent";
> +                            reg = <3>;
> +                            arm,smc-id = <0x82000005>;
> +                            shmem = <&scmi_shm_3>;
> +                        };
> +                    };
> +                };
> +            };
> +        };
> +    };
> +
> +The ``xen,sci`` subtree is omitted from Dom0's DT. The host OSPM channel is
> +copied to Dom0 when SCMI is enabled for it. Xen uses its separate privileged
> +management channel to configure the other agents.
> +
> +To discover a secondary agent ID through ``BASE_DISCOVER_AGENT``, omit
> +``reg`` and use a node name without a unit address. For example, replace
> +only ``agent@2`` above with:
> +
> +.. code-block:: dts
> +
> +    agent-domu {
> +        compatible = "xen,scmi-agent";
> +        arm,smc-id = <0x82000004>;
> +        shmem = <&scmi_shm_2>;
> +    };
> +
> +Providing ``reg`` skips mapping and probing that secondary channel during
> +initialization. A malformed ``reg`` is rejected. Xen always discovers its
> +own management agent ID. Thus ``BASE_DISCOVER_AGENT`` is mandatory for the
> +management channel and for secondary channels whose ``reg`` is omitted.
> +The former flat secondary-agent properties are replaced by these nodes.
> +
> +.. note::
> +
> +    Note that Xen is the only one entry in the system which need to know 
> about SCMI multi-agent support.
> +
> +- The driver implements the SCI subsystem interface required for configuring 
> and enabling SCMI
> +  functionality for Dom0/hwdom and Guest domains. To enable SCMI 
> functionality for guest domain

"for a guest domain"

> +  it has to be configured with unique supported SCMI Agent_id and use 
> corresponding SCMI SMC

sometimes you write "agent_id" and sometimes "Agent_id"...

> +  shared-memory transport ``[smc-id, shmem]`` defined for this SCMI Agent_id.
> +
> +- Once Xen domain is configured it can communicate with EL3 SCMI FW:
> +
> +  - zero-copy, the guest domain puts/gets SCMI message in/from shmem;

"to/from shmem". Also, "zero-copy: ", maybe?

> +  - the guest triggers SMC exception with agent "smc-id" (doorbell);
> +  - the Xen driver catches exception, do checks and synchronously forwards 
> it to EL3 FW.

not "do checks". "Perform checks", maybe?


> +
> +- the Xen driver sends BASE_RESET_AGENT_CONFIGURATION message to Xen 
> management agent channel on
> +  domain destroy event. This allows to reset resources used by domain and so 
> implement use-case

s/so//

> +  like domain reboot.
> +
> +
> +Configure SCMI for Dom0

"How to configure..." or "Configuring..."

> +^^^^^^^^^^^^^^^^^^^^^^^
> +Set the Dom0 SCMI agent ID in the device tree using the Xen SCMI container 
> under ``/chosen``.
> +Add ``xen,dom0-sci-agent-id`` to the ``xen,sci`` node. If the property is 
> absent, SCMI stays
> +disabled for Dom0 and the SCMI nodes are removed from Dom0 DT.
> +
> +.. code-block:: dts
> +
> +  chosen {
> +    xen {
> +      ranges;
> +      xen_scmi_config {
> +        compatible = "xen,sci";
> +        xen,dom0-sci-agent-id = <0>;  /* Dom0 agent ID */
> +        /* agents, shared memory and scmi_xen as shown above */
> +      };
> +    };
> +  };
> +
> +The Host DT ``/firmware/scmi`` node is copied to the Dom0 DT unmodified. 
> However, for Dom0 SCMI
> +configuration, Xen relies on the ``agents`` child and the 
> ``xen,dom0-sci-agent-id``

"child node"

> +property from the ``xen,sci`` container under ``/chosen``. If the 
> ``/firmware/scmi`` node is
> +missing or disabled, or if ``xen,dom0-sci-agent-id`` is not provided, the 
> Dom0 SCMI agent will not
> +be configured.

> +
> +.. note::
> +
> +  The ``xen,dom0-sci-agent-id`` value must match the ``func_id`` and 
> ``shmem`` pairing provided by
> +  the EL3 firmware for Dom0 (for example in the ``/firmware/scmi`` node).

Well, maybe better to use phandle then? Like

xen,dom0-sci-agent-id=<&agent@0>

> +
> +Configure SCMI for for guest domain with toolstack

"How to configure..." or "Configuring..."

> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +* In domain's xl.cfg file add **"arm_sci"** option as below
> +
> +::
> +
> +    arm_sci = "type=scmi_smc_multiagent,agent_id=2"
> +
> +* In domain's xl.cfg file enable access to the "arm,scmi-shmem" which should 
> correspond
> +  assigned "agent_id" for the domain, for example:
> +
> +::
> +
> +    iomem = [
> +        "47ff2,1@22001",
> +    ]
> +
> +.. note:: It's up to the user to select guest IPA for mapping SCMI 
> shared-memory.
> +
> +* Add SCMI nodes to the Driver domain partial device tree as in the below 
> example.
> +  The "arm,smc-id" should correspond assigned agent_id for the domain:
> +
> +.. code::
> +
> +    passthrough {
> +       scmi_shm_0: sram@22001000 {
> +           compatible = "arm,scmi-shmem";
> +           reg = <0x0 0x22001000 0x0 0x1000>;
> +       };
> +
> +       firmware {
> +            compatible = "simple-bus";
> +                scmi: scmi {
> +                    compatible = "arm,scmi-smc";
> +                    arm,smc-id = <0x82000004>;  <--- smc-id for agent_id=2
> +                    shmem = <&scmi_shm_0>;
> +                    ...
> +                }
> +        }
> +    }
> +
> +**Device specific access control**
> +
> +The XEN SCMI SMC multi-agent driver performs "access-controller" provider 
> function in case
> +EL3 SCMI FW implements SCMI "4.2.1.1 Device specific access control" and 
> provides the
> +BASE_SET_DEVICE_PERMISSIONS command to configure the devices that an agents 
> have access to.
> +The Host DT SCMI node should have "#access-controller-cells=<1>" property 
> and DT devices should
> +be bound to the SCMI node using Access Controllers bindings [3].
> +
> +For example:
> +
> +.. code-block:: dts
> +
> +    &i2c1 {
> +            access-controllers = <&scmi 0>;
> +    };
> +
> +Use domain's xl.cfg file **"dtdev"** property to assign SCMI devices from 
> toolstack to the guest:
> +
> +::
> +
> +    dtdev = [
> +        "/soc/i2c@e6508000",
> +    ]
> +
> +.. note::
> +
> +    xl.cfg:"dtdev" need contain all nodes which are under SCMI management 
> (not only those which are

"need to contain"

> +    behind IOMMU) and passed-through to the guest domain.
> +
> +Configure SCMI for predefined domains (dom0less)

(the same as for previous section header)

> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +* add "xen,sci_type" and "xen,sci-agent-id" properties for required DomU 
> ("xen,domain") node
> +
> +::
> +
> +    xen,sci_type="scmi_smc_multiagent"
> +    xen,sci-agent-id=2
> +
> +* add scmi nodes to the Driver domain partial device tree the same way as 
> above (toolstack case) and
> +  enable access to the "arm,scmi-shmem" according to the dom0less 
> documentation. For example:
> +
> +.. code-block:: dts
> +
> +      scmi_shm_0: sram@22001000 {
> +            compatible = "arm,scmi-shmem";
> +            reg = <0x00 0x22001000 0x00 0x1000>;
> +    ->        xen,reg = <0x0 0x47ff2000 0x0 0x1000 0x0 0x22001000>;
> +    ->        xen,force-assign-without-iommu;
> +      };
> +
> +* For SCMI device access control configure pass-through devices in the guest 
> partial DT according to
> +  the dom0less documentation and ensure that devices SCMI management has 
> "xen,path" property set:
> +
> +Use the complete Xen SCMI configuration above. Remove
> +``xen,dom0-sci-agent-id`` if Dom0 does not use SCMI. The guest selects the
> +secondary channel described by ``agent@2``:
> +
> +.. code-block:: dts
> +
> +    chosen {
> +        xen,domain@1 {
> +            compatible = "xen,domain";
> +            xen,sci_type = "scmi_smc_multiagent";
> +            xen,sci-agent-id = <2>;
> +            /* other domain properties here */
> +        };
> +    };
> +
> +.. code-block:: dts
> +
> +             i2c@e6508000 {
> +            ...
> +                     reg = <0x00 0xe6508000 0x00 0x1000>;
> +    ->        xen,path = "/soc/i2c@e6508000"
> +    ->        xen,reg = <0x0 0xe6508000 0x0 0x1000 0x0 0xe6508000>;
> +    ->        xen,force-assign-without-iommu;
> +        };

--
WBR, Volodymyr