|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v15 6/6] docs: arm: add SCI SCMI SMC multi-agent driver docs
On 05/11/2026 01:29, Volodymyr Babchuk wrote: > 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. > > [...] One note regarding this approach that I just noticed: If we move to the following format: xen,dom0-sci-agent = <&scmi_agent_dom0>; // &scmi_agent_dom0 is link to agent@0: we run into the following problem: scmi_agent_dom0: agent@0 { reg = <0>; /* optional field */ compatible = "xen,scmi-agent"; arm,smc-id = <0x82000002>; shmem = <&scmi_shm_0>; }; Here, we store the agent_id for agent@0 in the reg property. The issue is that reg is optional, and if it is omitted, we have to request the agent_ids using the SCMI_BASE_DISCOVER_AGENT SCMI call. This means that when we set arm_sci_agent_id in the create_dom0 function, we need to access the scmi_multiagent driver, which requires an additional callback to be added to the sci.h interface. I'm writing this to let you know that these changes will require modifications to the sci interface. Please reach out if you have any thoughts or questions. Thank you!
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |