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

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi,

Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx> writes:

> On 04/11/2026 14:27, Volodymyr Babchuk wrote:
>> Hi Oleksii,
>>
>> Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx> writes:
>>
>>> Hi Volodymyr,
>>>
>>> Thank you for the review. please see below:
>>>
>>> On 04/11/2026 06:28, Volodymyr Babchuk wrote:
>>>> 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?
>>> It is. But in terms of SCMI this is privileged agent. Haven't mentioned
>>> Xen here for the sake of
>>>
>>> diagram simplicity (just don't see a good place whre I can place "Xen"
>>> in this diagram. The main idea is
>> I think you can place "Xen" in the centre. There is free space for it fit.
>> Just add a new line, so it is nice and centered, like this:
>>
>> +      |              |   Xen   |           |    |
>>
>>> that driver leaves scmi privileged agent for itself (agent1 in our case)
>>> regardless where this driver runs.
>>>
>>> This format was taken since driver itself is avare about the domains and
>>> channels.
>>>
>>>>> +      |              |         |           |    |
>>>>> +      +--------------+---------+-----------+----+
>>>>> +             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.
>>> rstcheck gives 0 errors, but I can reformat it with maxline 120.
>>>>> +  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?
>>> If this command is not implemented by the SCP firmware, but the firmware
>>> provides agent_num > 1, then it is the responsibility of the SCP
>>> firmware. We made this optional to cover exactly this case — where we
>>> don't have RESET_AGENT_CONFIGURATION but still have agent_num > 1, which
>>> could be applicable to a dom0less configuration (for example) and should
>>> be used carefully by developers.
>> Okay, I see... On one hand it is not worse than the current state of
>> affairs, when we just destroying a domain and leaving its devices in
>> undetermined state. On other hand, SCMI is meant to resolve this
>> particular problem, so it will be strange to have SCMI but not to have
>> this particular feature. Maybe it is worth to write a huge warning if a
>> SCMI-enabled domain is destroyed, but we can't call RESET for it?
>>
>> Looks like I overlooked this case when reviewed "xen/arm: scmi:
>>   introduce SCI SCMI SMC multi-agent driver"
> this could be done. Will add huge warning.
>>>
>>>>> +- BASE_SET_DEVICE_PERMISSIONS (optional)
>>>> If this is optional, does it mean that each agent will have access to
>>>> all devices in the system?
>>> Probably. But that doesn't mean it's the only case. For example, a
>>> resource could be acquired on first access (not a real-world case, but
>>> it's possible). So we can't make this decision on behalf of the SCP
>>> firmware.
>> Again, this might be not what user wants or expects. And it is even
>> worse then the previous case, because without SCMI I am sure that if I
>> didn't gave a domain access to a peripheral, then it can't affects its
>> state. If you really want to support such case, then you should ensure
>> that only HWDOM can access SCMI of SCP does not support
>> SET_DEVICE_PERMISSIONS.
>>
>
> When only the HWDOM has access to the SCMI of the SCP without
> SET_DEVICE_PERMISSIONS support, there is no point in using
> scmi-multiagent at all. In this case, scmi-single-agent (which has
> already been merged) should be used instead.
>
> So I think we should probably make SET_DEVICE_PERMISSIONS support
> mandatory in this case and fail if it is not supported. We should also
> mention this in the documentation.
>
> What do you think?

Well, this certainly is an option. In other words, I am fine with that.

>> [...]
>>
>>
>>>>> +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>
>>> It is used to set arm_sci_agent_id in the config and works the same way
>>> as libxl, so we maintain consistent approaches for setting dom_id in
>>> both libxl and the device-tree.
>>>
>>> Additionally, the agent_id is used when acquiring the scmi_channel.
>>>
>>> I don't see any particular benefits to using a phandle here, but perhaps
>>> I've missed something. Could you clarify?
>> Having less room for user error. Having phandle, you can extract both
>> func_id and shmem from it. But, thinking of it, you can extract shmem
>> just by looking at func_id... On other hand, it is better to move checks
>> to the compilation time. 'dtc' will tell you if it can't find a phandle,
>> but it will not tell you if numeric agent_id does not match any entry in
>> that agents array.
>>
>> [...]
> Moving the checks to 'dtc' compilation time sounds reasonable.
>
> I can reorganize this in the following way:
> 1) Make it look like:
>    xen,dom0-sci-agent = <&agent@0>;
>
> 2) For dom0less configuration:
>    change:  xen,sci-agent-id = <2>;
>    to look like xen,sci-agent = <&agent@X>;
>
> 3) for xen-scmi-agent node add xen,sci-agent-id optional property.
>
> Does this match what you had in mind?

Yes.

Just a small note: it will not work exactly like that, because agent@x
is a node name, not a node label. You'll encounter an error if you'll try
to compile device tree with this exact code.

[...]

--
WBR, Volodymyr