[RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Introducing new parameter called scmi_devid to the device-tree bindings.
This parameter should be set for the device nodes, which has
clocks/power-domains/resets working through SCMI.
Given parameter should set the device_id, needed to set device
permissions in the Firmware. This feature will be extremely useful for
the virtualized systems, which has more that one Guests running on the
system at the same time or for the syestems, which require several
agents with different permissions. Trusted agent will use scmi_devid to
set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
for details).
Agents concept is described in Section 4.2.1 [0].

scmi_devid in Device-tree node example:
usb@e6590000
{
    scmi_devid = <19>;
    clocks = <&scmi_clock 3>, <&scmi_clock 2>;
    resets = <&scmi_reset 10>, <&scmi_reset 9>;
    power-domains = <&scmi_power 0>;
};

Given example shows the configuration of the hsusb node, which is using
scmi to contol clocks, resets and power-domains. scmi_devid is set
equals to 19, which should match defined id for usb in the Firmware.

Trusted agent will use scmi_devid to set the device permissions for
the Agents. Guest OS should not have an access to the permissions
settings, so no code to process scmi_devid was presented in Linux
kernel.

We are currently contributing changes to Xen, which are intended to
mediate SCMI access from Guests to the Firmware. Xen uses scmi_devid to set
the permissions for the devices. See [1] thread for details.

[0] https://developer.arm.com/documentation/den0056/latest
[1] https://xen.markmail.org/message/mmi4fpb4qr6e3kad

---
Posting this patch as RFC because I'm not sure if I set maintainers
correctly. Hope community will help me so I could set the maintainers in
v2.

Oleksii Moisieiev (1):
  dt-bindings: arm: Add scmi_devid property description for SCMI

 .../bindings/firmware/arm,scmi-devid.yaml     | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml

-- 
2.27.0

[RFC PATCH 1/1] dt-bindings: arm: Add scmi_devid property description for SCMI

[Oleksii Moisieiev]

Document scmi_devid property for the devices, using SCMI protocol
to work with clocks/resets/power-domains etc. This property is intended
to set the device id, which should be used to manage device permissions
in the firmware. Device permissions management is descibed in DEN 0056,
Section 4.2.2.10 [0].

This property is useful for the virtualized systems, when several agents
are running on the same platform. Agent term is descibed in Section
4.1.1 [0].

[0] https://developer.arm.com/documentation/den0056/latest

Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
---
 .../bindings/firmware/arm,scmi-devid.yaml     | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml
new file mode 100644
index 000000000000..b0949319e00b
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2022 EPAM Systems.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/arm,scmi-devid.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: System Control and Management Interface (SCMI) Device ID for permissions
+    handling
+
+maintainers:
+  - Oleksii Moisieiev <oleksii_moisieiev@epam.com>
+
+description: |
+  SCMI device id property is intended to set the device id, needed to manage
+  the device permissions via SCMI protocol in the firmware. The device Id
+  should match device ids, defined in the firmware so the device permissions
+  can be requested by sending BASE_SET_DEVICE_PERMISSIONS (see 4.2.2.10 of [0]).
+
+  This property is useful on the virtualized systems, when several agents are
+  running on the same time and permissions should be set for the devices,
+  related to different agents.
+
+  [0] https://developer.arm.com/documentation/den0056/latest
+
+properties:
+  scmi_devid:
+    description: Identifier of the device, matching device id, defined in
+      the firmware.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+additionalProperties: false
+
+examples:
+  - |
+    ohci1: usb@ee0a0000 {
+        /* ... */
+        reg = <0 0xee0a0000 0 0x100>;
+        scmi_devid = <11>;
+        clocks = <&scmi_clock 4>;
+    };
-- 
2.27.0

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Krzysztof Kozlowski]

On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> Introducing new parameter called scmi_devid to the device-tree bindings.
> This parameter should be set for the device nodes, which has
> clocks/power-domains/resets working through SCMI.
> Given parameter should set the device_id, needed to set device
> permissions in the Firmware. This feature will be extremely useful for
> the virtualized systems, which has more that one Guests running on the
> system at the same time or for the syestems, which require several
> agents with different permissions. Trusted agent will use scmi_devid to
> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> for details).
> Agents concept is described in Section 4.2.1 [0].
> 
> scmi_devid in Device-tree node example:
> usb@e6590000
> {
>     scmi_devid = <19>;
>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
>     power-domains = <&scmi_power 0>;
> };

And how do you prevent DT overlay adding such devid to any other node
thus allowing any other device to send requests with given devid?

Plus few technicalities:
1. Hyphen, not underscore in property name, so scmi-devid.
2. Your schema does is not selected by anything. How is it intended to
be used? Nothing is including it, either...

Best regards,
Krzysztof

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Hi Krzysztof,

On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> > Introducing new parameter called scmi_devid to the device-tree bindings.
> > This parameter should be set for the device nodes, which has
> > clocks/power-domains/resets working through SCMI.
> > Given parameter should set the device_id, needed to set device
> > permissions in the Firmware. This feature will be extremely useful for
> > the virtualized systems, which has more that one Guests running on the
> > system at the same time or for the syestems, which require several
> > agents with different permissions. Trusted agent will use scmi_devid to
> > set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > for details).
> > Agents concept is described in Section 4.2.1 [0].
> > 
> > scmi_devid in Device-tree node example:
> > usb@e6590000
> > {
> >     scmi_devid = <19>;
> >     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> >     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> >     power-domains = <&scmi_power 0>;
> > };
> 
> And how do you prevent DT overlay adding such devid to any other node
> thus allowing any other device to send requests with given devid?
>
Thank you for the quick response.
scmi_devid value will be used only by Trusted Agent when the device
permissions are set. Non-trusted agents, which in our case are
represented as Guest OS are using scmi drivers, already present in linux
kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
nodes to access to SCMI protocol.

> Plus few technicalities:
> 1. Hyphen, not underscore in property name, so scmi-devid.

Thanks for the tip, I will change that in v2.

> 2. Your schema does is not selected by anything. How is it intended to
> be used? Nothing is including it, either...
> 

The idea is to use this parameter to set the device_id for the device in
the device-tree, which matches to the device mapping in the Firmware, so
Trusted Agent can use it to the device permissions.
Please see Sections 4.2.2.10 and 4.2.1 [0] (Link was provided in the
cover letter).

I'm currently propose the new feature, called SCI mediator to Xen-devel
community. Please see link [1] from cover letter for the details.
In this feature - Xen is the Trusted Agent, which uses scmi_devid
parameter to set the device permissions.
We think that this parameter will be useful for other possible SCMI
implementations, such as other hypervisor or SCMI backend server etc.

Best regards,
Oleksii.

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Krzysztof Kozlowski]

On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> Hi Krzysztof,
> 
> On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
>> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
>>> Introducing new parameter called scmi_devid to the device-tree bindings.
>>> This parameter should be set for the device nodes, which has
>>> clocks/power-domains/resets working through SCMI.
>>> Given parameter should set the device_id, needed to set device
>>> permissions in the Firmware. This feature will be extremely useful for
>>> the virtualized systems, which has more that one Guests running on the
>>> system at the same time or for the syestems, which require several
>>> agents with different permissions. Trusted agent will use scmi_devid to
>>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
>>> for details).
>>> Agents concept is described in Section 4.2.1 [0].
>>>
>>> scmi_devid in Device-tree node example:
>>> usb@e6590000
>>> {
>>>     scmi_devid = <19>;
>>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
>>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
>>>     power-domains = <&scmi_power 0>;
>>> };
>>
>> And how do you prevent DT overlay adding such devid to any other node
>> thus allowing any other device to send requests with given devid?
>>
> Thank you for the quick response.
> scmi_devid value will be used only by Trusted Agent when the device
> permissions are set. Non-trusted agents, which in our case are
> represented as Guest OS are using scmi drivers, already present in linux
> kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> nodes to access to SCMI protocol.

Ah, ok.

> 
>> Plus few technicalities:
>> 1. Hyphen, not underscore in property name, so scmi-devid.
> 
> Thanks for the tip, I will change that in v2.

Few more thoughts:
1. This looks specific to ARM SCMI, so you also need vendor prefix, so
something like:
arm,scmi-devid
arm,scmi-device-id

2. Does your example work properly? Passes dt_binding_check? Reg looks
different than unit-address.


> 
>> 2. Your schema does is not selected by anything. How is it intended to
>> be used? Nothing is including it, either...
>>
> 
> The idea is to use this parameter to set the device_id for the device in
> the device-tree, which matches to the device mapping in the Firmware, so
> Trusted Agent can use it to the device permissions.
> Please see Sections 4.2.2.10 and 4.2.1 [0] (Link was provided in the
> cover letter).
> 
> I'm currently propose the new feature, called SCI mediator to Xen-devel
> community. Please see link [1] from cover letter for the details.
> In this feature - Xen is the Trusted Agent, which uses scmi_devid
> parameter to set the device permissions.
> We think that this parameter will be useful for other possible SCMI
> implementations, such as other hypervisor or SCMI backend server etc.

We talk about different things, I think. I was asking how is this schema
selected?

I gave it a fast try (dtbs_check) and it confirmed - schema does not
have an effect. It's a noop. You need something like "select: true", see:
Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml
or this schema should be included by other schemas... but then I would
be happy to see actual usage in this patchset (more commits...).


Best regards,
Krzysztof

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Hi Krzysztof,

On Tue, Feb 22, 2022 at 09:06:25AM +0100, Krzysztof Kozlowski wrote:
> On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> > Hi Krzysztof,
> > 
> > On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> >> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> >>> Introducing new parameter called scmi_devid to the device-tree bindings.
> >>> This parameter should be set for the device nodes, which has
> >>> clocks/power-domains/resets working through SCMI.
> >>> Given parameter should set the device_id, needed to set device
> >>> permissions in the Firmware. This feature will be extremely useful for
> >>> the virtualized systems, which has more that one Guests running on the
> >>> system at the same time or for the syestems, which require several
> >>> agents with different permissions. Trusted agent will use scmi_devid to
> >>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> >>> for details).
> >>> Agents concept is described in Section 4.2.1 [0].
> >>>
> >>> scmi_devid in Device-tree node example:
> >>> usb@e6590000
> >>> {
> >>>     scmi_devid = <19>;
> >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> >>>     power-domains = <&scmi_power 0>;
> >>> };
> >>
> >> And how do you prevent DT overlay adding such devid to any other node
> >> thus allowing any other device to send requests with given devid?
> >>
> > Thank you for the quick response.
> > scmi_devid value will be used only by Trusted Agent when the device
> > permissions are set. Non-trusted agents, which in our case are
> > represented as Guest OS are using scmi drivers, already present in linux
> > kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> > nodes to access to SCMI protocol.
> 
> Ah, ok.
> 
> > 
> >> Plus few technicalities:
> >> 1. Hyphen, not underscore in property name, so scmi-devid.
> > 
> > Thanks for the tip, I will change that in v2.
> 
> Few more thoughts:
> 1. This looks specific to ARM SCMI, so you also need vendor prefix, so
> something like:
> arm,scmi-devid
> arm,scmi-device-id

That's a good advice. arm,scmi-devid looks good for me.

> 
> 2. Does your example work properly? Passes dt_binding_check? Reg looks
> different than unit-address.
> 
dt_bindings_check passes without errors. Also I've checked this file
explicitly by using command:
yamllint -c Documentation/devicetree/bindings/.yamllint Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml

Reg value, if you mean reg parameter from an Example, was taken from
r8a77961.dtsi file.

> 
> > 
> >> 2. Your schema does is not selected by anything. How is it intended to
> >> be used? Nothing is including it, either...
> >>
> > 
> > The idea is to use this parameter to set the device_id for the device in
> > the device-tree, which matches to the device mapping in the Firmware, so
> > Trusted Agent can use it to the device permissions.
> > Please see Sections 4.2.2.10 and 4.2.1 [0] (Link was provided in the
> > cover letter).
> > 
> > I'm currently propose the new feature, called SCI mediator to Xen-devel
> > community. Please see link [1] from cover letter for the details.
> > In this feature - Xen is the Trusted Agent, which uses scmi_devid
> > parameter to set the device permissions.
> > We think that this parameter will be useful for other possible SCMI
> > implementations, such as other hypervisor or SCMI backend server etc.
> 
> We talk about different things, I think. I was asking how is this schema
> selected?
> 
> I gave it a fast try (dtbs_check) and it confirmed - schema does not
> have an effect. It's a noop. You need something like "select: true", see:
> Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml
> or this schema should be included by other schemas... but then I would
> be happy to see actual usage in this patchset (more commits...).
> 

I think select: true will work for me. I'll do dtbs_check and
dt_bindings_check after making all changes and prepare v2 if there will
be no further comments.

Also what do you think about maintainers: field? Is it correct? I'm not
sure if I used it correctly.

> 
> Best regards,
> Krzysztof

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Krzysztof Kozlowski]

On 22/02/2022 09:55, Oleksii Moisieiev wrote:
> 
>>
>> 2. Does your example work properly? Passes dt_binding_check? Reg looks
>> different than unit-address.
>>
> dt_bindings_check passes without errors. Also I've checked this file
> explicitly by using command:
> yamllint -c Documentation/devicetree/bindings/.yamllint Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml
> 
> Reg value, if you mean reg parameter from an Example, was taken from
> r8a77961.dtsi file.
The check does not pass. You have an error there:

Documentation/devicetree/bindings/firmware/arm,scmi-devid.example.dt.yaml:
example-0: usb@ee0a0000:reg:0: [0, 3993632768, 0, 256] is too long



> 
>>
>>>
>>>> 2. Your schema does is not selected by anything. How is it intended to
>>>> be used? Nothing is including it, either...
>>>>
>>>
>>> The idea is to use this parameter to set the device_id for the device in
>>> the device-tree, which matches to the device mapping in the Firmware, so
>>> Trusted Agent can use it to the device permissions.
>>> Please see Sections 4.2.2.10 and 4.2.1 [0] (Link was provided in the
>>> cover letter).
>>>
>>> I'm currently propose the new feature, called SCI mediator to Xen-devel
>>> community. Please see link [1] from cover letter for the details.
>>> In this feature - Xen is the Trusted Agent, which uses scmi_devid
>>> parameter to set the device permissions.
>>> We think that this parameter will be useful for other possible SCMI
>>> implementations, such as other hypervisor or SCMI backend server etc.
>>
>> We talk about different things, I think. I was asking how is this schema
>> selected?
>>
>> I gave it a fast try (dtbs_check) and it confirmed - schema does not
>> have an effect. It's a noop. You need something like "select: true", see:
>> Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml
>> or this schema should be included by other schemas... but then I would
>> be happy to see actual usage in this patchset (more commits...).
>>
> 
> I think select: true will work for me. I'll do dtbs_check and
> dt_bindings_check after making all changes and prepare v2 if there will
> be no further comments.
> 
> Also what do you think about maintainers: field? Is it correct? I'm not
> sure if I used it correctly.


I think you should add arm,scmi maintainer next to you.


Best regards,
Krzysztof

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Hi Krzysztof,

On Tue, Feb 22, 2022 at 10:27:10AM +0100, Krzysztof Kozlowski wrote:
> On 22/02/2022 09:55, Oleksii Moisieiev wrote:
> > 
> >>
> >> 2. Does your example work properly? Passes dt_binding_check? Reg looks
> >> different than unit-address.
> >>
> > dt_bindings_check passes without errors. Also I've checked this file
> > explicitly by using command:
> > yamllint -c Documentation/devicetree/bindings/.yamllint Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml
> > 
> > Reg value, if you mean reg parameter from an Example, was taken from
> > r8a77961.dtsi file.
> The check does not pass. You have an error there:
> 
> Documentation/devicetree/bindings/firmware/arm,scmi-devid.example.dt.yaml:
> example-0: usb@ee0a0000:reg:0: [0, 3993632768, 0, 256] is too long
> 
> 
Probably I've missed that. I'll doublecheck.
> 
> > 
> >>
> >>>
> >>>> 2. Your schema does is not selected by anything. How is it intended to
> >>>> be used? Nothing is including it, either...
> >>>>
> >>>
> >>> The idea is to use this parameter to set the device_id for the device in
> >>> the device-tree, which matches to the device mapping in the Firmware, so
> >>> Trusted Agent can use it to the device permissions.
> >>> Please see Sections 4.2.2.10 and 4.2.1 [0] (Link was provided in the
> >>> cover letter).
> >>>
> >>> I'm currently propose the new feature, called SCI mediator to Xen-devel
> >>> community. Please see link [1] from cover letter for the details.
> >>> In this feature - Xen is the Trusted Agent, which uses scmi_devid
> >>> parameter to set the device permissions.
> >>> We think that this parameter will be useful for other possible SCMI
> >>> implementations, such as other hypervisor or SCMI backend server etc.
> >>
> >> We talk about different things, I think. I was asking how is this schema
> >> selected?
> >>
> >> I gave it a fast try (dtbs_check) and it confirmed - schema does not
> >> have an effect. It's a noop. You need something like "select: true", see:
> >> Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml
> >> or this schema should be included by other schemas... but then I would
> >> be happy to see actual usage in this patchset (more commits...).
> >>
> > 
> > I think select: true will work for me. I'll do dtbs_check and
> > dt_bindings_check after making all changes and prepare v2 if there will
> > be no further comments.
> > 
> > Also what do you think about maintainers: field? Is it correct? I'm not
> > sure if I used it correctly.
> 
> 
> I think you should add arm,scmi maintainer next to you.
> 

Should I update MAINTAINERS file as well?

Best regards,
Oleksii.

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Cristian Marussi]

On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> Introducing new parameter called scmi_devid to the device-tree bindings.
> This parameter should be set for the device nodes, which has
> clocks/power-domains/resets working through SCMI.
> Given parameter should set the device_id, needed to set device
> permissions in the Firmware. This feature will be extremely useful for
> the virtualized systems, which has more that one Guests running on the
> system at the same time or for the syestems, which require several
> agents with different permissions. Trusted agent will use scmi_devid to
> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> for details).
> Agents concept is described in Section 4.2.1 [0].
> 

Hi Oleksii,

I had a look at this patch and the related XEN series and I'd have a few
questions/doubts. (adding to the loop in CC Souvik from ATG and Vincent
from Linaro since he's working on similar SCMI virtualization stuff)

> scmi_devid in Device-tree node example:
> usb@e6590000
> {
>     scmi_devid = <19>;
>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
>     power-domains = <&scmi_power 0>;
> };
> 

So this SCMI device ID is meant to identify an SCMI device, viewed as a
grouping of SCMI resources (clock/power/...etc) so that a Trusted Agent
can issue a BASE_SET_DEVICE_PERMISSIONS telling to the SCMI Server
platform backend (SCP sw sitting somewhere) which SCMI agents on the
system can access which (SCMI) devices (in the Normal nonSecure world):
basically, if I got it right from the Xen series, your hypervisor acting
as Trusted Agent (and recognized as trusted agent by by the SCP) tells
the SCMI platform server SCP (via SMC in your case) how to configure the
access to the devices for all the other (non trusted) agents in the system
(other Guest OS/Domains instances).

The SCMI spec does not indeed cover the discovery of such devices and
the related associated resources: it indeed delegates such description
to FDT/ACPI as of now.

AFAIU in this scenario I imagine:

- SCMI Server platform (SCP) knows via its own methods (builtin_config
  FDT...etc) the list of defined SCMI devices and related associated
  resources like:
  
  deviceNNN -->> clock_X / power_Q
  deviceYYY ---> clock_Z / power_W
  ..etc


- Trusted Agent (XEN hypervisor) in turn:

  + is configured/recognized by the SCMI Server as a Trusted Agent (based
    on the channel it uses to talk to the server) and as such it is allowed
    to issue BASE_SET_DEVICE_PERMISSIONS (by the SCMI server)

  + has knowledge of the same set of devices/resources allocations (via
    its own FDT) as the SCMI server

  + can issue a proper set of BASE_SET_DEVICE_PERMISSIONS telling the
    SCMI server backend which devices can be used by which non-trusted
    agents (GuestOS) ... even dynamically I suppose when guests come and
    go.

    Xen:
	    BASE_SET_DEVICE_PERMISSIONS(dev_NNN, agent_3)
	    BASE_SET_DEVICE_PERMISSIONS(dev_YYY, agent_2)
	    BASE_SET_DEVICE_PERMISSIONS(dev_NNN, agent_4)

    and in this scenario the same dev_NNN could be made accessible to
    two different agents, it will be anyway up to the SCMI Server
    backend to armonize or reject such requests from 2 different
    agents around the same shared resources


- Other non-trusted agents on the system (GuestOSes or other non
  virtualized subsystems...e.g. WiFi/Modem...etc), described in their
  DTs (for Linux GuestOS) as using SCMI resources as usual (without SCMI
  device id) just issue SCMI request on the basic resource and those are
  routed to the SCMI Server backend by the Hypervisor UNMODIFIED:

  example for a shared resource:

   - Agent_2 set power_Q ON --->>> SCMI Server - OK - powerQ TURNED ON
    				  (allowed as configured by Trusted Agent,
				   powerQ hw was OFF turn it ON)

   - Agent_3 set power_Q OFF --->>> SCMI Server - DENIED
    				  (disallowed as configured by Trusted Agent)

   - Agent_4 set power_Q ON --->>> SCMI Server - OK - powerQ ALREADY ON
    				  (allowed as configured by Trusted Agent,
				   powerQ hw was ON nothing to be done)

   - Agent_2 set power_Q OFF --->>> SCMI Server - OK - powerQ UNCHANGED (SHARED with Agent_2)
    				  (allowed as configured by Trusted Agent but shared with
				   another agent)

   - Agent_4 set power_Q OFF --->>> SCMI Server - OK - powerQ OFF
    				  (allowed as configured by Trusted Agent but shared with
				   another agent

So in all of this, I don't get why you need this DT definition aggregating SCMI
resources to SCMI device IDs in the Guest OS, which is an SCMI agent that does not
need to now anything about SCMI device IDs (at least with the current spec): this
would make sense only if the Linux Kernel was the TrustedAgent in charge of
configuring the devices permissions via BASE_SET_DEVICE_PERMISSIONS.
(in fact you said you won't provide any code to manage this scmi_devid
in the kernel since those guests are not trusted agents and the won't be
allowed to set device permissions...)

The only tricky part I can see in all of the above is agent identification, since
the agents are assigned an ID by the SCMI platform (which can be queried) and they
have a set of dedicated channels to use, so basically the platform really identifies
the Agents looking at the channel from which a request is coming from and AgentID is
not carried inside the message as a source and cannot be spoofed.

> Given example shows the configuration of the hsusb node, which is using
> scmi to contol clocks, resets and power-domains. scmi_devid is set
> equals to 19, which should match defined id for usb in the Firmware.
> 
> Trusted agent will use scmi_devid to set the device permissions for
> the Agents. Guest OS should not have an access to the permissions
> settings, so no code to process scmi_devid was presented in Linux
> kernel.
> 
> We are currently contributing changes to Xen, which are intended to
> mediate SCMI access from Guests to the Firmware. Xen uses scmi_devid to set
> the permissions for the devices. See [1] thread for details.
> 
> [0] https://developer.arm.com/documentation/den0056/latest
> [1] https://xen.markmail.org/message/mmi4fpb4qr6e3kad

IMHO, but I could be wrong, looking at the current SCMI spec you cannot just
gather messages from a set of GuestOs talking via different SCMI channels and
then pipe/route them through a single channel to the backend server,
attaching/spoofing some sort of Agent source ID to each message like you seem to
be doing in the Xen series

" Also XEN is the mediator which redirects SCMI requests, adding agentID so
  firmware should know the sender."

I may missing something though, not really a Xen expert here, or maybe this
agentID identification trick could be considered something not covered
by the spec and strictly part of the transport layer...not sure... the guys
in CC may have different/opposite opinions so feel free to redirect my
blabbing to /dev/null at the end :D

Thanks,
Cristian

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Krzysztof Kozlowski]

On 22/02/2022 11:30, Oleksii Moisieiev wrote:
> Hi Krzysztof,
> 
>>>
>>> Also what do you think about maintainers: field? Is it correct? I'm not
>>> sure if I used it correctly.
>>
>>
>> I think you should add arm,scmi maintainer next to you.
>>
> 
> Should I update MAINTAINERS file as well?

Yes, the bindings file should be added to the SCPI/SCMI entry. You can
just add a wildcard * to cover all files.


Best regards,
Krzysztof

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Hi Cristian,

On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > Introducing new parameter called scmi_devid to the device-tree bindings.
> > This parameter should be set for the device nodes, which has
> > clocks/power-domains/resets working through SCMI.
> > Given parameter should set the device_id, needed to set device
> > permissions in the Firmware. This feature will be extremely useful for
> > the virtualized systems, which has more that one Guests running on the
> > system at the same time or for the syestems, which require several
> > agents with different permissions. Trusted agent will use scmi_devid to
> > set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > for details).
> > Agents concept is described in Section 4.2.1 [0].
> > 
> 
> Hi Oleksii,
> 
> I had a look at this patch and the related XEN series and I'd have a few
> questions/doubts. (adding to the loop in CC Souvik from ATG and Vincent
> from Linaro since he's working on similar SCMI virtualization stuff)
> 

I'm aware of what Vincent is working on.

> > scmi_devid in Device-tree node example:
> > usb@e6590000
> > {
> >     scmi_devid = <19>;
> >     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> >     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> >     power-domains = <&scmi_power 0>;
> > };
> > 
> 
> So this SCMI device ID is meant to identify an SCMI device, viewed as a
> grouping of SCMI resources (clock/power/...etc) so that a Trusted Agent
> can issue a BASE_SET_DEVICE_PERMISSIONS telling to the SCMI Server
> platform backend (SCP sw sitting somewhere) which SCMI agents on the
> system can access which (SCMI) devices (in the Normal nonSecure world):
> basically, if I got it right from the Xen series, your hypervisor acting
> as Trusted Agent (and recognized as trusted agent by by the SCP) tells
> the SCMI platform server SCP (via SMC in your case) how to configure the
> access to the devices for all the other (non trusted) agents in the system
> (other Guest OS/Domains instances).

That's correct.

> 
> The SCMI spec does not indeed cover the discovery of such devices and
> the related associated resources: it indeed delegates such description
> to FDT/ACPI as of now.
> 
> AFAIU in this scenario I imagine:
> 
> - SCMI Server platform (SCP) knows via its own methods (builtin_config
>   FDT...etc) the list of defined SCMI devices and related associated
>   resources like:
>   
>   deviceNNN -->> clock_X / power_Q
>   deviceYYY ---> clock_Z / power_W
>   ..etc
> 
> 
> - Trusted Agent (XEN hypervisor) in turn:
> 
>   + is configured/recognized by the SCMI Server as a Trusted Agent (based
>     on the channel it uses to talk to the server) and as such it is allowed
>     to issue BASE_SET_DEVICE_PERMISSIONS (by the SCMI server)
> 
>   + has knowledge of the same set of devices/resources allocations (via
>     its own FDT) as the SCMI server
> 
>   + can issue a proper set of BASE_SET_DEVICE_PERMISSIONS telling the
>     SCMI server backend which devices can be used by which non-trusted
>     agents (GuestOS) ... even dynamically I suppose when guests come and
>     go.
> 
>     Xen:
> 	    BASE_SET_DEVICE_PERMISSIONS(dev_NNN, agent_3)
> 	    BASE_SET_DEVICE_PERMISSIONS(dev_YYY, agent_2)
> 	    BASE_SET_DEVICE_PERMISSIONS(dev_NNN, agent_4)
> 
>     and in this scenario the same dev_NNN could be made accessible to
>     two different agents, it will be anyway up to the SCMI Server
>     backend to armonize or reject such requests from 2 different
>     agents around the same shared resources
> 
> 
> - Other non-trusted agents on the system (GuestOSes or other non
>   virtualized subsystems...e.g. WiFi/Modem...etc), described in their
>   DTs (for Linux GuestOS) as using SCMI resources as usual (without SCMI
>   device id) just issue SCMI request on the basic resource and those are
>   routed to the SCMI Server backend by the Hypervisor UNMODIFIED:
> 
>   example for a shared resource:
> 
>    - Agent_2 set power_Q ON --->>> SCMI Server - OK - powerQ TURNED ON
>     				  (allowed as configured by Trusted Agent,
> 				   powerQ hw was OFF turn it ON)
> 
>    - Agent_3 set power_Q OFF --->>> SCMI Server - DENIED
>     				  (disallowed as configured by Trusted Agent)
> 
>    - Agent_4 set power_Q ON --->>> SCMI Server - OK - powerQ ALREADY ON
>     				  (allowed as configured by Trusted Agent,
> 				   powerQ hw was ON nothing to be done)
> 
>    - Agent_2 set power_Q OFF --->>> SCMI Server - OK - powerQ UNCHANGED (SHARED with Agent_2)
>     				  (allowed as configured by Trusted Agent but shared with
> 				   another agent)
> 
>    - Agent_4 set power_Q OFF --->>> SCMI Server - OK - powerQ OFF
>     				  (allowed as configured by Trusted Agent but shared with
> 				   another agent
> 
> So in all of this, I don't get why you need this DT definition aggregating SCMI
> resources to SCMI device IDs in the Guest OS, which is an SCMI agent that does not
> need to now anything about SCMI device IDs (at least with the current spec): this
> would make sense only if the Linux Kernel was the TrustedAgent in charge of
> configuring the devices permissions via BASE_SET_DEVICE_PERMISSIONS.
> (in fact you said you won't provide any code to manage this scmi_devid
> in the kernel since those guests are not trusted agents and the won't be
> allowed to set device permissions...)

You are right. The idea is that only Xen in our case will use
scmi-devid. I thought that providing this parameter in DT-bindings will
be useful for the future usage, let's say for some sort of the scmi
backend, running in the different OS or for the different hypervisor.
That's why I've decided to open discussion here.

> 
> The only tricky part I can see in all of the above is agent identification, since
> the agents are assigned an ID by the SCMI platform (which can be queried) and they
> have a set of dedicated channels to use, so basically the platform really identifies
> the Agents looking at the channel from which a request is coming from and AgentID is
> not carried inside the message as a source and cannot be spoofed.

That's a very good question. I've raised it in Xen mailing list: 
https://xen.markmail.org/message/jofia74b2s4jprqa

The main question for me is how Firmware will know from which channel it
should read the message.

In current implementataion we use SMC Client_ID (reg7) of SMC message to
pass channel_id from Xen to Firmware.

I'm also thinking about the alternative way to generate unique Func_ID
for each channel, so Firmware can get information about channel_id from
the Func_ID. 
In any case - I will be happy if you join the discussion about agent_id
in Xen mailing list. I can CC you if you intend to take part.

> 
> > Given example shows the configuration of the hsusb node, which is using
> > scmi to contol clocks, resets and power-domains. scmi_devid is set
> > equals to 19, which should match defined id for usb in the Firmware.
> > 
> > Trusted agent will use scmi_devid to set the device permissions for
> > the Agents. Guest OS should not have an access to the permissions
> > settings, so no code to process scmi_devid was presented in Linux
> > kernel.
> > 
> > We are currently contributing changes to Xen, which are intended to
> > mediate SCMI access from Guests to the Firmware. Xen uses scmi_devid to set
> > the permissions for the devices. See [1] thread for details.
> > 
> > [0] https://urldefense.com/v3/__https://developer.arm.com/documentation/den0056/latest__;!!GF_29dbcQIUBPA!ipwkZidQn94BuaF7b6AopvUjQJGLSkgQm6QnAqOaevFBqCYEYEQZVOq8CGFd5j8_JWEz$ [developer[.]arm[.]com]
> > [1] https://urldefense.com/v3/__https://xen.markmail.org/message/mmi4fpb4qr6e3kad__;!!GF_29dbcQIUBPA!ipwkZidQn94BuaF7b6AopvUjQJGLSkgQm6QnAqOaevFBqCYEYEQZVOq8CGFd5seDaFDv$ [xen[.]markmail[.]org]
> 
> IMHO, but I could be wrong, looking at the current SCMI spec you cannot just
> gather messages from a set of GuestOs talking via different SCMI channels and
> then pipe/route them through a single channel to the backend server,
> attaching/spoofing some sort of Agent source ID to each message like you seem to
> be doing in the Xen series

I think you misunderstood my approach. We're not gathering messages from
GuestOs. Each GuestOs uses it's own channel_id to communicate with
firmware. What Xen does in current implementation is intercept SMC
messages and set channel_id to SMC Client_ID parameter (reg7) and
redirects it to the Firmware. Firmware parses reg7 and knows which
channed it should use to get the request.

In the new approach, which I suggest in Xen mailing list (I will be very
happy if you share your thoughts there) doesn't require SMC messages to
be intercepted by Xen, so each GuestOs will have it's own channel with
the Firmware.

> 
> " Also XEN is the mediator which redirects SCMI requests, adding agentID so
>   firmware should know the sender."
> 
> I may missing something though, not really a Xen expert here, or maybe this
> agentID identification trick could be considered something not covered
> by the spec and strictly part of the transport layer...not sure... the guys
> in CC may have different/opposite opinions so feel free to redirect my
> blabbing to /dev/null at the end :D
> 

SCMI spec describes how agent_id should be passed only for Message type
3 which is Notification (See 4.1.2 of DEN0056D). SMC spec say that
Client_ID is reg7. This Client_id is what I've used originally.
The new approach I suggest gives ability to get rid of Client_ID and use
Func_ID to determine channel.

Best regards,
Oleksii

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Hi Krzysztof,

On Tue, Feb 22, 2022 at 12:16:40PM +0100, Krzysztof Kozlowski wrote:
> On 22/02/2022 11:30, Oleksii Moisieiev wrote:
> > Hi Krzysztof,
> > 
> >>>
> >>> Also what do you think about maintainers: field? Is it correct? I'm not
> >>> sure if I used it correctly.
> >>
> >>
> >> I think you should add arm,scmi maintainer next to you.
> >>
> > 
> > Should I update MAINTAINERS file as well?
> 
> Yes, the bindings file should be added to the SCPI/SCMI entry. You can
> just add a wildcard * to cover all files.
> 

Thank you for the tip, I will add this to the MAINTAINERS.
I think I would wait some time before making v2 because I don't want to
interrupt or split the discussion with Cristian.

Best regards,
Oleksii

Re: [RFC PATCH 1/1] dt-bindings: arm: Add scmi_devid property description for SCMI

[Rob Herring]

On Mon, 21 Feb 2022 17:26:47 +0000, Oleksii Moisieiev wrote:
> Document scmi_devid property for the devices, using SCMI protocol
> to work with clocks/resets/power-domains etc. This property is intended
> to set the device id, which should be used to manage device permissions
> in the firmware. Device permissions management is descibed in DEN 0056,
> Section 4.2.2.10 [0].
> 
> This property is useful for the virtualized systems, when several agents
> are running on the same platform. Agent term is descibed in Section
> 4.1.1 [0].
> 
> [0] https://developer.arm.com/documentation/den0056/latest
> 
> Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> ---
>  .../bindings/firmware/arm,scmi-devid.yaml     | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/firmware/arm,scmi-devid.example.dt.yaml: example-0: usb@ee0a0000:reg:0: [0, 3993632768, 0, 256] is too long
	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1595720

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [RFC PATCH 1/1] dt-bindings: arm: Add scmi_devid property description for SCMI

[Oleksii Moisieiev]

On Tue, Feb 22, 2022 at 08:22:43AM -0600, Rob Herring wrote:
> On Mon, 21 Feb 2022 17:26:47 +0000, Oleksii Moisieiev wrote:
> > Document scmi_devid property for the devices, using SCMI protocol
> > to work with clocks/resets/power-domains etc. This property is intended
> > to set the device id, which should be used to manage device permissions
> > in the firmware. Device permissions management is descibed in DEN 0056,
> > Section 4.2.2.10 [0].
> > 
> > This property is useful for the virtualized systems, when several agents
> > are running on the same platform. Agent term is descibed in Section
> > 4.1.1 [0].
> > 
> > [0] https://urldefense.com/v3/__https://developer.arm.com/documentation/den0056/latest__;!!GF_29dbcQIUBPA!m_dudHGSVg6mys2GtScvkfHNbQ1mv2kCGeU2GEdcV_C0cqHoWuknIkJPdLTaKiq6Ed9f$ [developer[.]arm[.]com]
> > 
> > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> > ---
> >  .../bindings/firmware/arm,scmi-devid.yaml     | 41 +++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/firmware/arm,scmi-devid.example.dt.yaml: example-0: usb@ee0a0000:reg:0: [0, 3993632768, 0, 256] is too long
> 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> 
> doc reference errors (make refcheckdocs):
> 
> See https://urldefense.com/v3/__https://patchwork.ozlabs.org/patch/1595720__;!!GF_29dbcQIUBPA!m_dudHGSVg6mys2GtScvkfHNbQ1mv2kCGeU2GEdcV_C0cqHoWuknIkJPdLTaKsRHUfCS$ [patchwork[.]ozlabs[.]org]
> 
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit.
> 

Hi Rob,

Thank you. I will recheck on my side.

Best regards,
Oleksii.

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Sudeep Holla]

Hi Oleksii,

My initial feedback on this. And thanks Cristian for making it so easy as
you have covered most of the things in depth(which I might have not done
myself that well)

On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > Introducing new parameter called scmi_devid to the device-tree bindings.
> > This parameter should be set for the device nodes, which has
> > clocks/power-domains/resets working through SCMI.

I prefer you had given more details on your usage model here instead of
pointing to the other Xen thread as it helps for someone without much
background on Xen or your use-case to review this.

> > Given parameter should set the device_id, needed to set device
> > permissions in the Firmware. This feature will be extremely useful for
> > the virtualized systems, which has more that one Guests running on the
> > system at the same time or for the syestems, which require several
> > agents with different permissions. Trusted agent will use scmi_devid to
> > set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > for details).

I am bit confused here, so you expecting a non-secure/non-trusted entity
to supply this device-id to the Trusted agent ? Is that not the breach of
trust as any non-trusted entity can supply any agent-id and get the permission
to access the associated resource in this way ? Or am I missing something
totally here.

> > Agents concept is described in Section 4.2.1 [0].
> >
>
> Hi Oleksii,
>
> I had a look at this patch and the related XEN series and I'd have a few
> questions/doubts. (adding to the loop in CC Souvik from ATG and Vincent
> from Linaro since he's working on similar SCMI virtualization stuff)
>
> > scmi_devid in Device-tree node example:
> > usb@e6590000
> > {
> >     scmi_devid = <19>;
> >     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> >     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> >     power-domains = <&scmi_power 0>;
> > };
> >
>
> So this SCMI device ID is meant to identify an SCMI device, viewed as a
> grouping of SCMI resources (clock/power/...etc) so that a Trusted Agent
> can issue a BASE_SET_DEVICE_PERMISSIONS telling to the SCMI Server
> platform backend (SCP sw sitting somewhere) which SCMI agents on the
> system can access which (SCMI) devices (in the Normal nonSecure world):
> basically, if I got it right from the Xen series, your hypervisor acting
> as Trusted Agent (and recognized as trusted agent by by the SCP) tells
> the SCMI platform server SCP (via SMC in your case) how to configure the
> access to the devices for all the other (non trusted) agents in the system
> (other Guest OS/Domains instances).
>

To add to the above, getting the device-id from GuestOS/DomX(X!=0) is main
issue.

> The SCMI spec does not indeed cover the discovery of such devices and
> the related associated resources: it indeed delegates such description
> to FDT/ACPI as of now.
>
> AFAIU in this scenario I imagine:
>
> - SCMI Server platform (SCP) knows via its own methods (builtin_config
>   FDT...etc) the list of defined SCMI devices and related associated
>   resources like:
>
>   deviceNNN -->> clock_X / power_Q
>   deviceYYY ---> clock_Z / power_W
>   ..etc
>
>
> - Trusted Agent (XEN hypervisor) in turn:
>
>   + is configured/recognized by the SCMI Server as a Trusted Agent (based
>     on the channel it uses to talk to the server) and as such it is allowed
>     to issue BASE_SET_DEVICE_PERMISSIONS (by the SCMI server)
>
>   + has knowledge of the same set of devices/resources allocations (via
>     its own FDT) as the SCMI server
>
>   + can issue a proper set of BASE_SET_DEVICE_PERMISSIONS telling the
>     SCMI server backend which devices can be used by which non-trusted
>     agents (GuestOS) ... even dynamically I suppose when guests come and
>     go.
>
>     Xen:
> 	    BASE_SET_DEVICE_PERMISSIONS(dev_NNN, agent_3)
> 	    BASE_SET_DEVICE_PERMISSIONS(dev_YYY, agent_2)
> 	    BASE_SET_DEVICE_PERMISSIONS(dev_NNN, agent_4)
>
>     and in this scenario the same dev_NNN could be made accessible to
>     two different agents, it will be anyway up to the SCMI Server
>     backend to armonize or reject such requests from 2 different
>     agents around the same shared resources
>

Agreed with all the details above.

>
> - Other non-trusted agents on the system (GuestOSes or other non
>   virtualized subsystems...e.g. WiFi/Modem...etc), described in their
>   DTs (for Linux GuestOS) as using SCMI resources as usual (without SCMI
>   device id) just issue SCMI request on the basic resource and those are
>   routed to the SCMI Server backend by the Hypervisor UNMODIFIED:
>

+1(specifically mentioning it to emphasize how much I agree on no need
for device-id in guest OS(VMs)

>   example for a shared resource:
>
>    - Agent_2 set power_Q ON --->>> SCMI Server - OK - powerQ TURNED ON
>     				  (allowed as configured by Trusted Agent,
> 				   powerQ hw was OFF turn it ON)
>
>    - Agent_3 set power_Q OFF --->>> SCMI Server - DENIED
>     				  (disallowed as configured by Trusted Agent)
>
>    - Agent_4 set power_Q ON --->>> SCMI Server - OK - powerQ ALREADY ON
>     				  (allowed as configured by Trusted Agent,
> 				   powerQ hw was ON nothing to be done)
>
>    - Agent_2 set power_Q OFF --->>> SCMI Server - OK - powerQ UNCHANGED (SHARED with Agent_2)
>     				  (allowed as configured by Trusted Agent but shared with
> 				   another agent)
>
>    - Agent_4 set power_Q OFF --->>> SCMI Server - OK - powerQ OFF
>     				  (allowed as configured by Trusted Agent but shared with
> 				   another agent
>
> So in all of this, I don't get why you need this DT definition aggregating SCMI
> resources to SCMI device IDs in the Guest OS, which is an SCMI agent that does not
> need to now anything about SCMI device IDs (at least with the current spec): this
> would make sense only if the Linux Kernel was the TrustedAgent in charge of
> configuring the devices permissions via BASE_SET_DEVICE_PERMISSIONS.
> (in fact you said you won't provide any code to manage this scmi_devid
> in the kernel since those guests are not trusted agents and the won't be
> allowed to set device permissions...)
>

+1 (again)

> The only tricky part I can see in all of the above is agent identification, since
> the agents are assigned an ID by the SCMI platform (which can be queried) and they
> have a set of dedicated channels to use, so basically the platform really identifies
> the Agents looking at the channel from which a request is coming from and AgentID is
> not carried inside the message as a source and cannot be spoofed.
>

IIUC, the physical/virtual transport and associated transport chosen
identifies the agent for the SCMI platform.

> > Given example shows the configuration of the hsusb node, which is using
> > scmi to contol clocks, resets and power-domains. scmi_devid is set
> > equals to 19, which should match defined id for usb in the Firmware.
> >
> > Trusted agent will use scmi_devid to set the device permissions for
> > the Agents. Guest OS should not have an access to the permissions
> > settings, so no code to process scmi_devid was presented in Linux
> > kernel.
> >
> > We are currently contributing changes to Xen, which are intended to
> > mediate SCMI access from Guests to the Firmware. Xen uses scmi_devid to set
> > the permissions for the devices. See [1] thread for details.
> >
> > [0] https://developer.arm.com/documentation/den0056/latest
> > [1] https://xen.markmail.org/message/mmi4fpb4qr6e3kad
>
> IMHO, but I could be wrong, looking at the current SCMI spec you cannot just
> gather messages from a set of GuestOs talking via different SCMI channels and
> then pipe/route them through a single channel to the backend server,
> attaching/spoofing some sort of Agent source ID to each message like you seem to
> be doing in the Xen series
>

I haven't looked at the other series, but it is hard to say the spec prohibits
this. I don't understand that spoofing part, but Xen hyp can arbitrate the
requests across guests I believe. But the devil is in details so I can't
comment on what is done. What I can say is this Agent ID is in each message is
not compliant to spec.

--
Regards,
Sudeep

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Sudeep Holla]

On Tue, Feb 22, 2022 at 09:06:25AM +0100, Krzysztof Kozlowski wrote:
> On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> > Hi Krzysztof,
> > 
> > On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> >> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> >>> Introducing new parameter called scmi_devid to the device-tree bindings.
> >>> This parameter should be set for the device nodes, which has
> >>> clocks/power-domains/resets working through SCMI.
> >>> Given parameter should set the device_id, needed to set device
> >>> permissions in the Firmware. This feature will be extremely useful for
> >>> the virtualized systems, which has more that one Guests running on the
> >>> system at the same time or for the syestems, which require several
> >>> agents with different permissions. Trusted agent will use scmi_devid to
> >>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> >>> for details).
> >>> Agents concept is described in Section 4.2.1 [0].
> >>>
> >>> scmi_devid in Device-tree node example:
> >>> usb@e6590000
> >>> {
> >>>     scmi_devid = <19>;
> >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> >>>     power-domains = <&scmi_power 0>;
> >>> };
> >>
> >> And how do you prevent DT overlay adding such devid to any other node
> >> thus allowing any other device to send requests with given devid?
> >>
> > Thank you for the quick response.
> > scmi_devid value will be used only by Trusted Agent when the device
> > permissions are set. Non-trusted agents, which in our case are
> > represented as Guest OS are using scmi drivers, already present in linux
> > kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> > nodes to access to SCMI protocol.
> 
> Ah, ok.
> 
> > 
> >> Plus few technicalities:
> >> 1. Hyphen, not underscore in property name, so scmi-devid.
> > 
> > Thanks for the tip, I will change that in v2.
> 
> Few more thoughts:
> 1. This looks specific to ARM SCMI, so you also need vendor prefix, so
> something like:
> arm,scmi-devid
> arm,scmi-device-id
> 

Keeping the other discussion separate, I wanted to comment on this.
I agree with Krzysztof on having vendor specific prefix if we decide to add
this device id thing. However, I prefer not to use "arm,scmi-" here.
It can be "xen,scmi-" as we had plans to introduce some concepts in SCMI
spec that may use looks like this device-id. I would just like to avoid
conflicting with that in the future. It may happen to be same in the future
(i.e. this xen device-id matches 100% with definition of device-id we might
introduce in the spec, but I want to make assumption otherwise and leave
scope for divergence however small/little it can be). No issues even if
they converge and match 100% later in the far future.

-- 
Regards,
Sudeep

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Hi Sudeep,

On Tue, Feb 22, 2022 at 04:06:37PM +0000, Sudeep Holla wrote:
> Hi Oleksii,
> 
> My initial feedback on this. And thanks Cristian for making it so easy as
> you have covered most of the things in depth(which I might have not done
> myself that well)
> 
> On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> > On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > > Introducing new parameter called scmi_devid to the device-tree bindings.
> > > This parameter should be set for the device nodes, which has
> > > clocks/power-domains/resets working through SCMI.
> 
> I prefer you had given more details on your usage model here instead of
> pointing to the other Xen thread as it helps for someone without much
> background on Xen or your use-case to review this.
> 
Let me describe the process in few words:
We implemented a new feature, called SCI-mediator in Xen.
The proposed implementation allows Guests to communicate with the Firmware using SCMI
protocol with SMC as a transport. Other implementation are also
possible, such as SCMI-Mailbox, SCPI-mailbox etc.

In this feature Xen is the Trusted Agent, which receives the following
information in Xen device-tree:
1) All channels should be described, each channel defined as
arm,scmi-shmem node;
2) Scmi node arm,scmi-smc with protocols description;
3) scmi-devid should be set in nodes, which works through SCMI.

On start Xen inits itself as trusted agent and requests agent
configuration by using BASE_DISCOVER_AGENT message. This message is sent
to each configured channel to get agent_id

On Domain creation stage Xen will do the following steps:
1) Assign channel to the Guest and map channel address to the Domain
address. For the Domain this address should be the same;
2) Generate arm,scmi-shmem and arm,scmi-smc nodes if needed for Guest
device-tree (the device-tree which should be passed to the Guest);
3) Process devices, which are passed through to this Guest and set
BASE_SET_DEVICE_PERMISSIONS for the scmi-devid, received from the
device-node;

Guest OS will receive non-trusted channel and ignore scmi-devid fields
in the device-nodes.

IMPORTANT: Guest OS is non-trusted Agent. Xen is the only trusted agent
in the system. Guest OS uses standart scmi drivers without any xen
related changes. So Guest OS doesn't know it works through mediator.

The main question is - how Firmware will know what agent sent SMC
message and what channel_id should be used? I couldn't find clear
explanation in spec.
That's why I end up with the following approaches:
1) Current implemenation: Guest OS send SMC request, Xen intercept this
request and set channel ID to SMC Client_ID field (reg7), then resend
SMC message to Firmware. Firmware parses SMC Client ID to get channel_id
to work with.
2) Another approach is to generate unique FuncID for each GuestOS. In
this case no interception from Xen is needed - Guest OS can work
directly with Firmware.

I hope you'll be able to help me with that.

> > > Given parameter should set the device_id, needed to set device
> > > permissions in the Firmware. This feature will be extremely useful for
> > > the virtualized systems, which has more that one Guests running on the
> > > system at the same time or for the syestems, which require several
> > > agents with different permissions. Trusted agent will use scmi_devid to
> > > set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > > for details).
> 
> I am bit confused here, so you expecting a non-secure/non-trusted entity
> to supply this device-id to the Trusted agent ? Is that not the breach of
> trust as any non-trusted entity can supply any agent-id and get the permission
> to access the associated resource in this way ? Or am I missing something
> totally here.
> 

No, Device-id will be used only by trusted agents, which is Xen in our
case. Please see above.

> > > Agents concept is described in Section 4.2.1 [0].
> > >
> >
> > Hi Oleksii,
> >
> > I had a look at this patch and the related XEN series and I'd have a few
> > questions/doubts. (adding to the loop in CC Souvik from ATG and Vincent
> > from Linaro since he's working on similar SCMI virtualization stuff)
> >
> > > scmi_devid in Device-tree node example:
> > > usb@e6590000
> > > {
> > >     scmi_devid = <19>;
> > >     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> > >     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> > >     power-domains = <&scmi_power 0>;
> > > };
> > >
> >
> > So this SCMI device ID is meant to identify an SCMI device, viewed as a
> > grouping of SCMI resources (clock/power/...etc) so that a Trusted Agent
> > can issue a BASE_SET_DEVICE_PERMISSIONS telling to the SCMI Server
> > platform backend (SCP sw sitting somewhere) which SCMI agents on the
> > system can access which (SCMI) devices (in the Normal nonSecure world):
> > basically, if I got it right from the Xen series, your hypervisor acting
> > as Trusted Agent (and recognized as trusted agent by by the SCP) tells
> > the SCMI platform server SCP (via SMC in your case) how to configure the
> > access to the devices for all the other (non trusted) agents in the system
> > (other Guest OS/Domains instances).
> >
> 
> To add to the above, getting the device-id from GuestOS/DomX(X!=0) is main
> issue.
> 

please see above. We will not either use not provice scmi-devid from
non-trusted agent.

> > The SCMI spec does not indeed cover the discovery of such devices and
> > the related associated resources: it indeed delegates such description
> > to FDT/ACPI as of now.
> >
> > AFAIU in this scenario I imagine:
> >
> > - SCMI Server platform (SCP) knows via its own methods (builtin_config
> >   FDT...etc) the list of defined SCMI devices and related associated
> >   resources like:
> >
> >   deviceNNN -->> clock_X / power_Q
> >   deviceYYY ---> clock_Z / power_W
> >   ..etc
> >
> >
> > - Trusted Agent (XEN hypervisor) in turn:
> >
> >   + is configured/recognized by the SCMI Server as a Trusted Agent (based
> >     on the channel it uses to talk to the server) and as such it is allowed
> >     to issue BASE_SET_DEVICE_PERMISSIONS (by the SCMI server)
> >
> >   + has knowledge of the same set of devices/resources allocations (via
> >     its own FDT) as the SCMI server
> >
> >   + can issue a proper set of BASE_SET_DEVICE_PERMISSIONS telling the
> >     SCMI server backend which devices can be used by which non-trusted
> >     agents (GuestOS) ... even dynamically I suppose when guests come and
> >     go.
> >
> >     Xen:
> > 	    BASE_SET_DEVICE_PERMISSIONS(dev_NNN, agent_3)
> > 	    BASE_SET_DEVICE_PERMISSIONS(dev_YYY, agent_2)
> > 	    BASE_SET_DEVICE_PERMISSIONS(dev_NNN, agent_4)
> >
> >     and in this scenario the same dev_NNN could be made accessible to
> >     two different agents, it will be anyway up to the SCMI Server
> >     backend to armonize or reject such requests from 2 different
> >     agents around the same shared resources
> >
> 
> Agreed with all the details above.
> 
> >
> > - Other non-trusted agents on the system (GuestOSes or other non
> >   virtualized subsystems...e.g. WiFi/Modem...etc), described in their
> >   DTs (for Linux GuestOS) as using SCMI resources as usual (without SCMI
> >   device id) just issue SCMI request on the basic resource and those are
> >   routed to the SCMI Server backend by the Hypervisor UNMODIFIED:
> >
> 
> +1(specifically mentioning it to emphasize how much I agree on no need
> for device-id in guest OS(VMs)

I agree with this too.
> 
> >   example for a shared resource:
> >
> >    - Agent_2 set power_Q ON --->>> SCMI Server - OK - powerQ TURNED ON
> >     				  (allowed as configured by Trusted Agent,
> > 				   powerQ hw was OFF turn it ON)
> >
> >    - Agent_3 set power_Q OFF --->>> SCMI Server - DENIED
> >     				  (disallowed as configured by Trusted Agent)
> >
> >    - Agent_4 set power_Q ON --->>> SCMI Server - OK - powerQ ALREADY ON
> >     				  (allowed as configured by Trusted Agent,
> > 				   powerQ hw was ON nothing to be done)
> >
> >    - Agent_2 set power_Q OFF --->>> SCMI Server - OK - powerQ UNCHANGED (SHARED with Agent_2)
> >     				  (allowed as configured by Trusted Agent but shared with
> > 				   another agent)
> >
> >    - Agent_4 set power_Q OFF --->>> SCMI Server - OK - powerQ OFF
> >     				  (allowed as configured by Trusted Agent but shared with
> > 				   another agent
> >
> > So in all of this, I don't get why you need this DT definition aggregating SCMI
> > resources to SCMI device IDs in the Guest OS, which is an SCMI agent that does not
> > need to now anything about SCMI device IDs (at least with the current spec): this
> > would make sense only if the Linux Kernel was the TrustedAgent in charge of
> > configuring the devices permissions via BASE_SET_DEVICE_PERMISSIONS.
> > (in fact you said you won't provide any code to manage this scmi_devid
> > in the kernel since those guests are not trusted agents and the won't be
> > allowed to set device permissions...)
> >
> 
> +1 (again)
> 
> > The only tricky part I can see in all of the above is agent identification, since
> > the agents are assigned an ID by the SCMI platform (which can be queried) and they
> > have a set of dedicated channels to use, so basically the platform really identifies
> > the Agents looking at the channel from which a request is coming from and AgentID is
> > not carried inside the message as a source and cannot be spoofed.
> >
> 
> IIUC, the physical/virtual transport and associated transport chosen
> identifies the agent for the SCMI platform.

Could you please clarify what do you mean under "physical/virtual
transport"?
For now yes - Firmware should get information for the channel from
transport.
> 
> > > Given example shows the configuration of the hsusb node, which is using
> > > scmi to contol clocks, resets and power-domains. scmi_devid is set
> > > equals to 19, which should match defined id for usb in the Firmware.
> > >
> > > Trusted agent will use scmi_devid to set the device permissions for
> > > the Agents. Guest OS should not have an access to the permissions
> > > settings, so no code to process scmi_devid was presented in Linux
> > > kernel.
> > >
> > > We are currently contributing changes to Xen, which are intended to
> > > mediate SCMI access from Guests to the Firmware. Xen uses scmi_devid to set
> > > the permissions for the devices. See [1] thread for details.
> > >
> > > [0] https://urldefense.com/v3/__https://developer.arm.com/documentation/den0056/latest__;!!GF_29dbcQIUBPA!mGggDzmp0B8cSdGJdH4utz6sx7g5PMXq05mXf91dU8XgkJaCuEpHdARZCdl-g1BnrduL$ [developer[.]arm[.]com]
> > > [1] https://urldefense.com/v3/__https://xen.markmail.org/message/mmi4fpb4qr6e3kad__;!!GF_29dbcQIUBPA!mGggDzmp0B8cSdGJdH4utz6sx7g5PMXq05mXf91dU8XgkJaCuEpHdARZCdl-g-bWzzb5$ [xen[.]markmail[.]org]
> >
> > IMHO, but I could be wrong, looking at the current SCMI spec you cannot just
> > gather messages from a set of GuestOs talking via different SCMI channels and
> > then pipe/route them through a single channel to the backend server,
> > attaching/spoofing some sort of Agent source ID to each message like you seem to
> > be doing in the Xen series
> >
> 
> I haven't looked at the other series, but it is hard to say the spec prohibits
> this. I don't understand that spoofing part, but Xen hyp can arbitrate the
> requests across guests I believe. But the devil is in details so I can't
> comment on what is done. What I can say is this Agent ID is in each message is
> not compliant to spec.
> 

In our implementation XEN do not copy any data from shared memory. The
only thing it does is letting Firmware know which channel it should use.

Best regards,
Oleksii.

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

On Tue, Feb 22, 2022 at 04:14:40PM +0000, Sudeep Holla wrote:
> On Tue, Feb 22, 2022 at 09:06:25AM +0100, Krzysztof Kozlowski wrote:
> > On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> > > Hi Krzysztof,
> > > 
> > > On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> > >> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> > >>> Introducing new parameter called scmi_devid to the device-tree bindings.
> > >>> This parameter should be set for the device nodes, which has
> > >>> clocks/power-domains/resets working through SCMI.
> > >>> Given parameter should set the device_id, needed to set device
> > >>> permissions in the Firmware. This feature will be extremely useful for
> > >>> the virtualized systems, which has more that one Guests running on the
> > >>> system at the same time or for the syestems, which require several
> > >>> agents with different permissions. Trusted agent will use scmi_devid to
> > >>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > >>> for details).
> > >>> Agents concept is described in Section 4.2.1 [0].
> > >>>
> > >>> scmi_devid in Device-tree node example:
> > >>> usb@e6590000
> > >>> {
> > >>>     scmi_devid = <19>;
> > >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> > >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> > >>>     power-domains = <&scmi_power 0>;
> > >>> };
> > >>
> > >> And how do you prevent DT overlay adding such devid to any other node
> > >> thus allowing any other device to send requests with given devid?
> > >>
> > > Thank you for the quick response.
> > > scmi_devid value will be used only by Trusted Agent when the device
> > > permissions are set. Non-trusted agents, which in our case are
> > > represented as Guest OS are using scmi drivers, already present in linux
> > > kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> > > nodes to access to SCMI protocol.
> > 
> > Ah, ok.
> > 
> > > 
> > >> Plus few technicalities:
> > >> 1. Hyphen, not underscore in property name, so scmi-devid.
> > > 
> > > Thanks for the tip, I will change that in v2.
> > 
> > Few more thoughts:
> > 1. This looks specific to ARM SCMI, so you also need vendor prefix, so
> > something like:
> > arm,scmi-devid
> > arm,scmi-device-id
> > 
> 
> Keeping the other discussion separate, I wanted to comment on this.
> I agree with Krzysztof on having vendor specific prefix if we decide to add
> this device id thing. However, I prefer not to use "arm,scmi-" here.
> It can be "xen,scmi-" as we had plans to introduce some concepts in SCMI
> spec that may use looks like this device-id. I would just like to avoid
> conflicting with that in the future. It may happen to be same in the future
> (i.e. this xen device-id matches 100% with definition of device-id we might
> introduce in the spec, but I want to make assumption otherwise and leave
> scope for divergence however small/little it can be). No issues even if
> they converge and match 100% later in the far future.
> 

xem,scmi- works for me. What do other thinks?

Re: [RFC PATCH 1/1] dt-bindings: arm: Add scmi_devid property description for SCMI

[Rob Herring]

On Tue, Feb 22, 2022 at 02:51:16PM +0000, Oleksii Moisieiev wrote:
> On Tue, Feb 22, 2022 at 08:22:43AM -0600, Rob Herring wrote:
> > On Mon, 21 Feb 2022 17:26:47 +0000, Oleksii Moisieiev wrote:
> > > Document scmi_devid property for the devices, using SCMI protocol
> > > to work with clocks/resets/power-domains etc. This property is intended
> > > to set the device id, which should be used to manage device permissions
> > > in the firmware. Device permissions management is descibed in DEN 0056,
> > > Section 4.2.2.10 [0].
> > > 
> > > This property is useful for the virtualized systems, when several agents
> > > are running on the same platform. Agent term is descibed in Section
> > > 4.1.1 [0].
> > > 
> > > [0] https://urldefense.com/v3/__https://developer.arm.com/documentation/den0056/latest__;!!GF_29dbcQIUBPA!m_dudHGSVg6mys2GtScvkfHNbQ1mv2kCGeU2GEdcV_C0cqHoWuknIkJPdLTaKiq6Ed9f$ [developer[.]arm[.]com]
> > > 
> > > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@epam.com>
> > > ---
> > >  .../bindings/firmware/arm,scmi-devid.yaml     | 41 +++++++++++++++++++
> > >  1 file changed, 41 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/firmware/arm,scmi-devid.yaml
> > > 
> > 
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > 
> > yamllint warnings/errors:
> > 
> > dtschema/dtc warnings/errors:
> > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/firmware/arm,scmi-devid.example.dt.yaml: example-0: usb@ee0a0000:reg:0: [0, 3993632768, 0, 256] is too long
> > 	From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
> > 
> > doc reference errors (make refcheckdocs):
> > 
> > See https://urldefense.com/v3/__https://patchwork.ozlabs.org/patch/1595720__;!!GF_29dbcQIUBPA!m_dudHGSVg6mys2GtScvkfHNbQ1mv2kCGeU2GEdcV_C0cqHoWuknIkJPdLTaKsRHUfCS$ [patchwork[.]ozlabs[.]org]
> > 
> > This check can fail if there are any dependencies. The base for a patch
> > series is generally the most recent rc1.
> > 
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure 'yamllint' is installed and dt-schema is up to
> > date:
> > 
> > pip3 install dtschema --upgrade
> > 
> > Please check and re-submit.
> > 
> 
> Hi Rob,
> 
> Thank you. I will recheck on my side.

The default address and size cell size for examples are 1 cell.

Rob

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Stefano Stabellini]

On Tue, 22 Feb 2022, Oleksii Moisieiev wrote:
> On Tue, Feb 22, 2022 at 04:14:40PM +0000, Sudeep Holla wrote:
> > On Tue, Feb 22, 2022 at 09:06:25AM +0100, Krzysztof Kozlowski wrote:
> > > On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> > > > Hi Krzysztof,
> > > > 
> > > > On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> > > >> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> > > >>> Introducing new parameter called scmi_devid to the device-tree bindings.
> > > >>> This parameter should be set for the device nodes, which has
> > > >>> clocks/power-domains/resets working through SCMI.
> > > >>> Given parameter should set the device_id, needed to set device
> > > >>> permissions in the Firmware. This feature will be extremely useful for
> > > >>> the virtualized systems, which has more that one Guests running on the
> > > >>> system at the same time or for the syestems, which require several
> > > >>> agents with different permissions. Trusted agent will use scmi_devid to
> > > >>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > > >>> for details).
> > > >>> Agents concept is described in Section 4.2.1 [0].
> > > >>>
> > > >>> scmi_devid in Device-tree node example:
> > > >>> usb@e6590000
> > > >>> {
> > > >>>     scmi_devid = <19>;
> > > >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> > > >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> > > >>>     power-domains = <&scmi_power 0>;
> > > >>> };
> > > >>
> > > >> And how do you prevent DT overlay adding such devid to any other node
> > > >> thus allowing any other device to send requests with given devid?
> > > >>
> > > > Thank you for the quick response.
> > > > scmi_devid value will be used only by Trusted Agent when the device
> > > > permissions are set. Non-trusted agents, which in our case are
> > > > represented as Guest OS are using scmi drivers, already present in linux
> > > > kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> > > > nodes to access to SCMI protocol.
> > > 
> > > Ah, ok.
> > > 
> > > > 
> > > >> Plus few technicalities:
> > > >> 1. Hyphen, not underscore in property name, so scmi-devid.
> > > > 
> > > > Thanks for the tip, I will change that in v2.
> > > 
> > > Few more thoughts:
> > > 1. This looks specific to ARM SCMI, so you also need vendor prefix, so
> > > something like:
> > > arm,scmi-devid
> > > arm,scmi-device-id
> > > 
> > 
> > Keeping the other discussion separate, I wanted to comment on this.
> > I agree with Krzysztof on having vendor specific prefix if we decide to add
> > this device id thing. However, I prefer not to use "arm,scmi-" here.
> > It can be "xen,scmi-" as we had plans to introduce some concepts in SCMI
> > spec that may use looks like this device-id. I would just like to avoid
> > conflicting with that in the future. It may happen to be same in the future
> > (i.e. this xen device-id matches 100% with definition of device-id we might
> > introduce in the spec, but I want to make assumption otherwise and leave
> > scope for divergence however small/little it can be). No issues even if
> > they converge and match 100% later in the far future.
> > 
> 
> xem,scmi- works for me. What do other thinks?
  ^ xen,scmi-

If this problem was Xen specific, then it would be fine to use xen,scmi-
As Xen developer, it solves my problem and I am fine with it.

However, from a device tree and SCMI point of view, it looks like this
problem is generic and it just happens that Xen is the first
implementation to encounter it.

Cristian wrote: "The SCMI spec does not indeed cover the discovery of
such devices and the related associated resources: it indeed delegates
such description to FDT/ACPI as of now." How is that supposed to happen
today with the current DT definitions, regardless of Xen? Is it a gap in
the current device tree binding?

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Cristian Marussi]

On Wed, Feb 23, 2022 at 06:51:59PM -0800, Stefano Stabellini wrote:
> On Tue, 22 Feb 2022, Oleksii Moisieiev wrote:
> > On Tue, Feb 22, 2022 at 04:14:40PM +0000, Sudeep Holla wrote:
> > > On Tue, Feb 22, 2022 at 09:06:25AM +0100, Krzysztof Kozlowski wrote:
> > > > On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> > > > > Hi Krzysztof,
> > > > > 

Hi Stefano,

> > > > > On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> > > > >> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> > > > >>> Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > >>> This parameter should be set for the device nodes, which has
> > > > >>> clocks/power-domains/resets working through SCMI.
> > > > >>> Given parameter should set the device_id, needed to set device
> > > > >>> permissions in the Firmware. This feature will be extremely useful for
> > > > >>> the virtualized systems, which has more that one Guests running on the
> > > > >>> system at the same time or for the syestems, which require several
> > > > >>> agents with different permissions. Trusted agent will use scmi_devid to
> > > > >>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > > > >>> for details).
> > > > >>> Agents concept is described in Section 4.2.1 [0].
> > > > >>>
> > > > >>> scmi_devid in Device-tree node example:
> > > > >>> usb@e6590000
> > > > >>> {
> > > > >>>     scmi_devid = <19>;
> > > > >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> > > > >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> > > > >>>     power-domains = <&scmi_power 0>;
> > > > >>> };
> > > > >>
> > > > >> And how do you prevent DT overlay adding such devid to any other node
> > > > >> thus allowing any other device to send requests with given devid?
> > > > >>
> > > > > Thank you for the quick response.
> > > > > scmi_devid value will be used only by Trusted Agent when the device
> > > > > permissions are set. Non-trusted agents, which in our case are
> > > > > represented as Guest OS are using scmi drivers, already present in linux
> > > > > kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> > > > > nodes to access to SCMI protocol.
> > > > 
> > > > Ah, ok.
> > > > 
> > > > > 
> > > > >> Plus few technicalities:
> > > > >> 1. Hyphen, not underscore in property name, so scmi-devid.
> > > > > 
> > > > > Thanks for the tip, I will change that in v2.
> > > > 
> > > > Few more thoughts:
> > > > 1. This looks specific to ARM SCMI, so you also need vendor prefix, so
> > > > something like:
> > > > arm,scmi-devid
> > > > arm,scmi-device-id
> > > > 
> > > 
> > > Keeping the other discussion separate, I wanted to comment on this.
> > > I agree with Krzysztof on having vendor specific prefix if we decide to add
> > > this device id thing. However, I prefer not to use "arm,scmi-" here.
> > > It can be "xen,scmi-" as we had plans to introduce some concepts in SCMI
> > > spec that may use looks like this device-id. I would just like to avoid
> > > conflicting with that in the future. It may happen to be same in the future
> > > (i.e. this xen device-id matches 100% with definition of device-id we might
> > > introduce in the spec, but I want to make assumption otherwise and leave
> > > scope for divergence however small/little it can be). No issues even if
> > > they converge and match 100% later in the far future.
> > > 
> > 
> > xem,scmi- works for me. What do other thinks?
>   ^ xen,scmi-
> 
> If this problem was Xen specific, then it would be fine to use xen,scmi-
> As Xen developer, it solves my problem and I am fine with it.
> 
> However, from a device tree and SCMI point of view, it looks like this
> problem is generic and it just happens that Xen is the first
> implementation to encounter it.
> 
> Cristian wrote: "The SCMI spec does not indeed cover the discovery of
> such devices and the related associated resources: it indeed delegates
> such description to FDT/ACPI as of now." How is that supposed to happen
> today with the current DT definitions, regardless of Xen? Is it a gap in
> the current device tree binding?

What I meant is that in fact SCMI device IDs are NOT needed in the Linux
Kernel DT, in fact also this series does not add any code using it and
there is no code as of now in Kernel to issue BASE_SET_DEVICE_PERMISSIONS
commands; Linux Guest OS in the above scenario is a Non-Trusted agent and
doesn't need to know SCMI DevIDs and must NOT have access to those IDs for
security reasons too (as Sudeep was saying): the Trusted Agent (XEN here)
and the SCMI platform server are the only ones required to share the
knowledge of such Device IDs (and how the related resources are grouped)
via some HW description scheme as you are doing indeed in XEN.

So, while on one side such device IDs discovery is delegated by the spec
to the HW description mechanisms, it seems just not needed in Kernel DT
given the kind or role it has as an SCMI agent in this context: as said
in fact there won't be any use as of now in Linux of such DT entries as
of now.

Thanks,
Cristian

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Sudeep Holla]

On Tue, Feb 22, 2022 at 05:15:49PM +0000, Oleksii Moisieiev wrote:
> Hi Sudeep,
> 
> On Tue, Feb 22, 2022 at 04:06:37PM +0000, Sudeep Holla wrote:
> > Hi Oleksii,
> > 
> > My initial feedback on this. And thanks Cristian for making it so easy as
> > you have covered most of the things in depth(which I might have not done
> > myself that well)
> > 
> > On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> > > On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > > > Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > This parameter should be set for the device nodes, which has
> > > > clocks/power-domains/resets working through SCMI.
> > 
> > I prefer you had given more details on your usage model here instead of
> > pointing to the other Xen thread as it helps for someone without much
> > background on Xen or your use-case to review this.
> > 
> Let me describe the process in few words:
> We implemented a new feature, called SCI-mediator in Xen.
> The proposed implementation allows Guests to communicate with the Firmware using SCMI
> protocol with SMC as a transport. Other implementation are also
> possible, such as SCMI-Mailbox, SCPI-mailbox etc.
> 
> In this feature Xen is the Trusted Agent, which receives the following
> information in Xen device-tree:
> 1) All channels should be described, each channel defined as
> arm,scmi-shmem node;
> 2) Scmi node arm,scmi-smc with protocols description;

Sounds good so far.

> 3) scmi-devid should be set in nodes, which works through SCMI.
>

Why is this needed for Guest OS, you need not populate this if Guest OS
is not required to use it, right ? If it is needed just by Xen hypervisor,
lets talk about that and why it is bad idea to mix that with general
SCMI bindings.

> On start Xen inits itself as trusted agent and requests agent
> configuration by using BASE_DISCOVER_AGENT message. This message is sent
> to each configured channel to get agent_id
> 
> On Domain creation stage Xen will do the following steps:
> 1) Assign channel to the Guest and map channel address to the Domain
> address. For the Domain this address should be the same;
> 2) Generate arm,scmi-shmem and arm,scmi-smc nodes if needed for Guest
> device-tree (the device-tree which should be passed to the Guest);
> 3) Process devices, which are passed through to this Guest and set
> BASE_SET_DEVICE_PERMISSIONS for the scmi-devid, received from the
> device-node;
>

I am confused here. So the Xen knows which devices are assigned to each
Guest OS but doesn't know device ID for them, but relies on the device
tree node ?

> Guest OS will receive non-trusted channel and ignore scmi-devid fields
> in the device-nodes.
>

Then no need to pass it. It keeps the SCMI agent binding clean.

> IMPORTANT: Guest OS is non-trusted Agent. Xen is the only trusted agent
> in the system. Guest OS uses standart scmi drivers without any xen
> related changes. So Guest OS doesn't know it works through mediator.
>

Good.

> The main question is - how Firmware will know what agent sent SMC
> message and what channel_id should be used? I couldn't find clear
> explanation in spec.

1. So the hypervisor forwards all the messages from different guests without
   any marshalling ?
2. If Xen is just acting as pass through, why does firmware care about the
   origin of the message.

> That's why I end up with the following approaches:
> 1) Current implemenation: Guest OS send SMC request, Xen intercept this
> request and set channel ID to SMC Client_ID field (reg7), then resend
> SMC message to Firmware. Firmware parses SMC Client ID to get channel_id
> to work with.

As asked above, why is this information important to the firmware.

> 2) Another approach is to generate unique FuncID for each GuestOS. In
> this case no interception from Xen is needed - Guest OS can work
> directly with Firmware.
>

I think that is the only way today to support multiple channels with
SMC/HVC. The reason for that is since the SMC FID is custom, we can't take
custom parameters and write a generic SCMI smc transport driver. This was
discussed and we decided to go for different FID, otherwise we would have
to standardise parameters to the custom FID which is insane IMO.

> I hope you'll be able to help me with that.
>

Thanks for details. It definitely provided more information though not yet
complete as you can guess with my questions here.

> > > > Given parameter should set the device_id, needed to set device
> > > > permissions in the Firmware. This feature will be extremely useful for
> > > > the virtualized systems, which has more that one Guests running on the
> > > > system at the same time or for the syestems, which require several
> > > > agents with different permissions. Trusted agent will use scmi_devid to
> > > > set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > > > for details).
> > 
> > I am bit confused here, so you expecting a non-secure/non-trusted entity
> > to supply this device-id to the Trusted agent ? Is that not the breach of
> > trust as any non-trusted entity can supply any agent-id and get the permission
> > to access the associated resource in this way ? Or am I missing something
> > totally here.
> > 
> 
> No, Device-id will be used only by trusted agents, which is Xen in our
> case. Please see above.
>

Understood now, and I will assert guest OS must not have it in its DT.


[...]

> > >
> > > So in all of this, I don't get why you need this DT definition aggregating SCMI
> > > resources to SCMI device IDs in the Guest OS, which is an SCMI agent that does not
> > > need to now anything about SCMI device IDs (at least with the current spec): this
> > > would make sense only if the Linux Kernel was the TrustedAgent in charge of
> > > configuring the devices permissions via BASE_SET_DEVICE_PERMISSIONS.
> > > (in fact you said you won't provide any code to manage this scmi_devid
> > > in the kernel since those guests are not trusted agents and the won't be
> > > allowed to set device permissions...)
> > >
> > 
> > +1 (again)
> > 
> > > The only tricky part I can see in all of the above is agent identification, since
> > > the agents are assigned an ID by the SCMI platform (which can be queried) and they
> > > have a set of dedicated channels to use, so basically the platform really identifies
> > > the Agents looking at the channel from which a request is coming from and AgentID is
> > > not carried inside the message as a source and cannot be spoofed.
> > >
> > 
> > IIUC, the physical/virtual transport and associated transport chosen
> > identifies the agent for the SCMI platform.
> 
> Could you please clarify what do you mean under "physical/virtual
> transport"?

I was speculating some design in Xen on how it present virtual channels to
guests. Ignore that as I now understand you are using SMC.

> For now yes - Firmware should get information for the channel from
> transport.

Indeed, with SMC/HVC, you will need different FID for reasons stated above.

> > 
> > > > Given example shows the configuration of the hsusb node, which is using
> > > > scmi to contol clocks, resets and power-domains. scmi_devid is set
> > > > equals to 19, which should match defined id for usb in the Firmware.
> > > >
> > > > Trusted agent will use scmi_devid to set the device permissions for
> > > > the Agents. Guest OS should not have an access to the permissions
> > > > settings, so no code to process scmi_devid was presented in Linux
> > > > kernel.
> > > >
> > > > We are currently contributing changes to Xen, which are intended to
> > > > mediate SCMI access from Guests to the Firmware. Xen uses scmi_devid to set
> > > > the permissions for the devices. See [1] thread for details.
> > > >
> > > > [0] https://urldefense.com/v3/__https://developer.arm.com/documentation/den0056/latest__;!!GF_29dbcQIUBPA!mGggDzmp0B8cSdGJdH4utz6sx7g5PMXq05mXf91dU8XgkJaCuEpHdARZCdl-g1BnrduL$ [developer[.]arm[.]com]
> > > > [1] https://urldefense.com/v3/__https://xen.markmail.org/message/mmi4fpb4qr6e3kad__;!!GF_29dbcQIUBPA!mGggDzmp0B8cSdGJdH4utz6sx7g5PMXq05mXf91dU8XgkJaCuEpHdARZCdl-g-bWzzb5$ [xen[.]markmail[.]org]
> > >
> > > IMHO, but I could be wrong, looking at the current SCMI spec you cannot just
> > > gather messages from a set of GuestOs talking via different SCMI channels and
> > > then pipe/route them through a single channel to the backend server,
> > > attaching/spoofing some sort of Agent source ID to each message like you seem to
> > > be doing in the Xen series
> > >
> > 
> > I haven't looked at the other series, but it is hard to say the spec prohibits
> > this. I don't understand that spoofing part, but Xen hyp can arbitrate the
> > requests across guests I believe. But the devil is in details so I can't
> > comment on what is done. What I can say is this Agent ID is in each message is
> > not compliant to spec.
> > 
> 
> In our implementation XEN do not copy any data from shared memory. The
> only thing it does is letting Firmware know which channel it should use.

OK.

-- 
Regards,
Sudeep

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Stefano Stabellini]

On Thu, 24 Feb 2022, Cristian Marussi wrote:
> On Wed, Feb 23, 2022 at 06:51:59PM -0800, Stefano Stabellini wrote:
> > On Tue, 22 Feb 2022, Oleksii Moisieiev wrote:
> > > On Tue, Feb 22, 2022 at 04:14:40PM +0000, Sudeep Holla wrote:
> > > > On Tue, Feb 22, 2022 at 09:06:25AM +0100, Krzysztof Kozlowski wrote:
> > > > > On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> > > > > > Hi Krzysztof,
> > > > > > 
> 
> Hi Stefano,
> 
> > > > > > On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> > > > > >> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> > > > > >>> Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > >>> This parameter should be set for the device nodes, which has
> > > > > >>> clocks/power-domains/resets working through SCMI.
> > > > > >>> Given parameter should set the device_id, needed to set device
> > > > > >>> permissions in the Firmware. This feature will be extremely useful for
> > > > > >>> the virtualized systems, which has more that one Guests running on the
> > > > > >>> system at the same time or for the syestems, which require several
> > > > > >>> agents with different permissions. Trusted agent will use scmi_devid to
> > > > > >>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > > > > >>> for details).
> > > > > >>> Agents concept is described in Section 4.2.1 [0].
> > > > > >>>
> > > > > >>> scmi_devid in Device-tree node example:
> > > > > >>> usb@e6590000
> > > > > >>> {
> > > > > >>>     scmi_devid = <19>;
> > > > > >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> > > > > >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> > > > > >>>     power-domains = <&scmi_power 0>;
> > > > > >>> };
> > > > > >>
> > > > > >> And how do you prevent DT overlay adding such devid to any other node
> > > > > >> thus allowing any other device to send requests with given devid?
> > > > > >>
> > > > > > Thank you for the quick response.
> > > > > > scmi_devid value will be used only by Trusted Agent when the device
> > > > > > permissions are set. Non-trusted agents, which in our case are
> > > > > > represented as Guest OS are using scmi drivers, already present in linux
> > > > > > kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> > > > > > nodes to access to SCMI protocol.
> > > > > 
> > > > > Ah, ok.
> > > > > 
> > > > > > 
> > > > > >> Plus few technicalities:
> > > > > >> 1. Hyphen, not underscore in property name, so scmi-devid.
> > > > > > 
> > > > > > Thanks for the tip, I will change that in v2.
> > > > > 
> > > > > Few more thoughts:
> > > > > 1. This looks specific to ARM SCMI, so you also need vendor prefix, so
> > > > > something like:
> > > > > arm,scmi-devid
> > > > > arm,scmi-device-id
> > > > > 
> > > > 
> > > > Keeping the other discussion separate, I wanted to comment on this.
> > > > I agree with Krzysztof on having vendor specific prefix if we decide to add
> > > > this device id thing. However, I prefer not to use "arm,scmi-" here.
> > > > It can be "xen,scmi-" as we had plans to introduce some concepts in SCMI
> > > > spec that may use looks like this device-id. I would just like to avoid
> > > > conflicting with that in the future. It may happen to be same in the future
> > > > (i.e. this xen device-id matches 100% with definition of device-id we might
> > > > introduce in the spec, but I want to make assumption otherwise and leave
> > > > scope for divergence however small/little it can be). No issues even if
> > > > they converge and match 100% later in the far future.
> > > > 
> > > 
> > > xem,scmi- works for me. What do other thinks?
> >   ^ xen,scmi-
> > 
> > If this problem was Xen specific, then it would be fine to use xen,scmi-
> > As Xen developer, it solves my problem and I am fine with it.
> > 
> > However, from a device tree and SCMI point of view, it looks like this
> > problem is generic and it just happens that Xen is the first
> > implementation to encounter it.
> > 
> > Cristian wrote: "The SCMI spec does not indeed cover the discovery of
> > such devices and the related associated resources: it indeed delegates
> > such description to FDT/ACPI as of now." How is that supposed to happen
> > today with the current DT definitions, regardless of Xen? Is it a gap in
> > the current device tree binding?
> 
> What I meant is that in fact SCMI device IDs are NOT needed in the Linux
> Kernel DT, in fact also this series does not add any code using it and
> there is no code as of now in Kernel to issue BASE_SET_DEVICE_PERMISSIONS
> commands; Linux Guest OS in the above scenario is a Non-Trusted agent and
> doesn't need to know SCMI DevIDs and must NOT have access to those IDs for
> security reasons too (as Sudeep was saying): the Trusted Agent (XEN here)
> and the SCMI platform server are the only ones required to share the
> knowledge of such Device IDs (and how the related resources are grouped)
> via some HW description scheme as you are doing indeed in XEN.
> 
> So, while on one side such device IDs discovery is delegated by the spec
> to the HW description mechanisms, it seems just not needed in Kernel DT
> given the kind or role it has as an SCMI agent in this context: as said
> in fact there won't be any use as of now in Linux of such DT entries as
> of now.

I see. This is one of those "difficult" cases where the device tree
binding is needed but it is not used by the Linux kernel. (We have a few
cases like this in system device tree and the more device tree gets
adopted by other projects the more will see instances like this one.)

In that case, I think it is OK to call it "xen,scmi-" if Rob also
agrees.

After all, we are currently using in Xen a property called
"linux,pci-domain". We might as well have Linux in the future use a
property called "xen,scmi-devid" to even things out :-)

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Stefano Stabellini]

On Thu, 24 Feb 2022, Sudeep Holla wrote:
> On Tue, Feb 22, 2022 at 05:15:49PM +0000, Oleksii Moisieiev wrote:
> > Hi Sudeep,
> > 
> > On Tue, Feb 22, 2022 at 04:06:37PM +0000, Sudeep Holla wrote:
> > > Hi Oleksii,
> > > 
> > > My initial feedback on this. And thanks Cristian for making it so easy as
> > > you have covered most of the things in depth(which I might have not done
> > > myself that well)
> > > 
> > > On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> > > > On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > > > > Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > This parameter should be set for the device nodes, which has
> > > > > clocks/power-domains/resets working through SCMI.
> > > 
> > > I prefer you had given more details on your usage model here instead of
> > > pointing to the other Xen thread as it helps for someone without much
> > > background on Xen or your use-case to review this.
> > > 
> > Let me describe the process in few words:
> > We implemented a new feature, called SCI-mediator in Xen.
> > The proposed implementation allows Guests to communicate with the Firmware using SCMI
> > protocol with SMC as a transport. Other implementation are also
> > possible, such as SCMI-Mailbox, SCPI-mailbox etc.
> > 
> > In this feature Xen is the Trusted Agent, which receives the following
> > information in Xen device-tree:
> > 1) All channels should be described, each channel defined as
> > arm,scmi-shmem node;
> > 2) Scmi node arm,scmi-smc with protocols description;
> 
> Sounds good so far.
> 
> > 3) scmi-devid should be set in nodes, which works through SCMI.
> >
> 
> Why is this needed for Guest OS, you need not populate this if Guest OS
> is not required to use it, right ? If it is needed just by Xen hypervisor,
> lets talk about that and why it is bad idea to mix that with general
> SCMI bindings.

I'll try to help Oleksii by answering here, I hope I am not off the mark
:-)

I think Sudeep is right, scmi-devid is not needed by the guest OS.

The host device tree is a more interesting discussion. As the host
device tree is meant to be generic and not tied to a specific version of
Linux, it should fully describe the SCMI interface available. If the
device tree is provided to a Trusted Agent, then it should also have the
scmi-devid information, right?


> > On start Xen inits itself as trusted agent and requests agent
> > configuration by using BASE_DISCOVER_AGENT message. This message is sent
> > to each configured channel to get agent_id
> > 
> > On Domain creation stage Xen will do the following steps:
> > 1) Assign channel to the Guest and map channel address to the Domain
> > address. For the Domain this address should be the same;
> > 2) Generate arm,scmi-shmem and arm,scmi-smc nodes if needed for Guest
> > device-tree (the device-tree which should be passed to the Guest);
> > 3) Process devices, which are passed through to this Guest and set
> > BASE_SET_DEVICE_PERMISSIONS for the scmi-devid, received from the
> > device-node;
> >
> 
> I am confused here. So the Xen knows which devices are assigned to each
> Guest OS but doesn't know device ID for them, but relies on the device
> tree node ?

Which devices go to which guest OS is a user-provided configuration. For
instance, a user can say: "assing /amba/ethernet@ff0e0000 to dom1". This
is normal and not related to SCMI: when a user configures a static
partitioning system, they decide which resources belong to which domain.

So Xen is told that /amba/ethernet@ff0e0000 is supposed to go to dom1.
Xen proceeds to map memory and interrupts corresponding to
/amba/ethernet@ff0e0000 to dom1. So far so good. What about SCMI?

In Oleksii's design, Xen is going to assign one of the available SCMI
channels to dom1 and restrict its permission to only
/amba/ethernet@ff0e0000. To do that, Xen needs to know the scmi-devid of
/amba/ethernet@ff0e0000. As far as I can tell there is nothing
Xen-specific in this activitity, that's why I asked Oleksii to reach out
to the upstream device tree community to improve the generic bindings
for everyone's benefits.

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Stefano Stabellini]

On Thu, 24 Feb 2022, Stefano Stabellini wrote:
> On Thu, 24 Feb 2022, Sudeep Holla wrote:
> > On Tue, Feb 22, 2022 at 05:15:49PM +0000, Oleksii Moisieiev wrote:
> > > Hi Sudeep,
> > > 
> > > On Tue, Feb 22, 2022 at 04:06:37PM +0000, Sudeep Holla wrote:
> > > > Hi Oleksii,
> > > > 
> > > > My initial feedback on this. And thanks Cristian for making it so easy as
> > > > you have covered most of the things in depth(which I might have not done
> > > > myself that well)
> > > > 
> > > > On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> > > > > On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > > > > > Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > > This parameter should be set for the device nodes, which has
> > > > > > clocks/power-domains/resets working through SCMI.
> > > > 
> > > > I prefer you had given more details on your usage model here instead of
> > > > pointing to the other Xen thread as it helps for someone without much
> > > > background on Xen or your use-case to review this.
> > > > 
> > > Let me describe the process in few words:
> > > We implemented a new feature, called SCI-mediator in Xen.
> > > The proposed implementation allows Guests to communicate with the Firmware using SCMI
> > > protocol with SMC as a transport. Other implementation are also
> > > possible, such as SCMI-Mailbox, SCPI-mailbox etc.
> > > 
> > > In this feature Xen is the Trusted Agent, which receives the following
> > > information in Xen device-tree:
> > > 1) All channels should be described, each channel defined as
> > > arm,scmi-shmem node;
> > > 2) Scmi node arm,scmi-smc with protocols description;
> > 
> > Sounds good so far.
> > 
> > > 3) scmi-devid should be set in nodes, which works through SCMI.
> > >
> > 
> > Why is this needed for Guest OS, you need not populate this if Guest OS
> > is not required to use it, right ? If it is needed just by Xen hypervisor,
> > lets talk about that and why it is bad idea to mix that with general
> > SCMI bindings.
> 
> I'll try to help Oleksii by answering here, I hope I am not off the mark
> :-)
> 
> I think Sudeep is right, scmi-devid is not needed by the guest OS.
> 
> The host device tree is a more interesting discussion. As the host
> device tree is meant to be generic and not tied to a specific version of
> Linux, it should fully describe the SCMI interface available. If the
> device tree is provided to a Trusted Agent, then it should also have the
> scmi-devid information, right?
> 
> 
> > > On start Xen inits itself as trusted agent and requests agent
> > > configuration by using BASE_DISCOVER_AGENT message. This message is sent
> > > to each configured channel to get agent_id
> > > 
> > > On Domain creation stage Xen will do the following steps:
> > > 1) Assign channel to the Guest and map channel address to the Domain
> > > address. For the Domain this address should be the same;
> > > 2) Generate arm,scmi-shmem and arm,scmi-smc nodes if needed for Guest
> > > device-tree (the device-tree which should be passed to the Guest);
> > > 3) Process devices, which are passed through to this Guest and set
> > > BASE_SET_DEVICE_PERMISSIONS for the scmi-devid, received from the
> > > device-node;
> > >
> > 
> > I am confused here. So the Xen knows which devices are assigned to each
> > Guest OS but doesn't know device ID for them, but relies on the device
> > tree node ?
> 
> Which devices go to which guest OS is a user-provided configuration. For
> instance, a user can say: "assing /amba/ethernet@ff0e0000 to dom1". This
> is normal and not related to SCMI: when a user configures a static
> partitioning system, they decide which resources belong to which domain.
> 
> So Xen is told that /amba/ethernet@ff0e0000 is supposed to go to dom1.
> Xen proceeds to map memory and interrupts corresponding to
> /amba/ethernet@ff0e0000 to dom1. So far so good. What about SCMI?
> 
> In Oleksii's design, Xen is going to assign one of the available SCMI
> channels to dom1 and restrict its permission to only
> /amba/ethernet@ff0e0000. To do that, Xen needs to know the scmi-devid of
> /amba/ethernet@ff0e0000. As far as I can tell there is nothing
> Xen-specific in this activitity, that's why I asked Oleksii to reach out
> to the upstream device tree community to improve the generic bindings
> for everyone's benefits.

Let's leave Linux and Xen aside for the moment. What are other possible
Trusted Agents? (Maybe TF-A?) How do they get the scmi-devid? It looks
like it was supposed to come from device tree but nobody got around to
adding it to the binding because it is not used by Linux?

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Hi Stefano, Cristian,

On Thu, Feb 24, 2022 at 02:22:28PM -0800, Stefano Stabellini wrote:
> On Thu, 24 Feb 2022, Cristian Marussi wrote:
> > On Wed, Feb 23, 2022 at 06:51:59PM -0800, Stefano Stabellini wrote:
> > > On Tue, 22 Feb 2022, Oleksii Moisieiev wrote:
> > > > On Tue, Feb 22, 2022 at 04:14:40PM +0000, Sudeep Holla wrote:
> > > > > On Tue, Feb 22, 2022 at 09:06:25AM +0100, Krzysztof Kozlowski wrote:
> > > > > > On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> > > > > > > Hi Krzysztof,
> > > > > > > 
> > 
> > Hi Stefano,
> > 
> > > > > > > On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> > > > > > >> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> > > > > > >>> Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > > >>> This parameter should be set for the device nodes, which has
> > > > > > >>> clocks/power-domains/resets working through SCMI.
> > > > > > >>> Given parameter should set the device_id, needed to set device
> > > > > > >>> permissions in the Firmware. This feature will be extremely useful for
> > > > > > >>> the virtualized systems, which has more that one Guests running on the
> > > > > > >>> system at the same time or for the syestems, which require several
> > > > > > >>> agents with different permissions. Trusted agent will use scmi_devid to
> > > > > > >>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > > > > > >>> for details).
> > > > > > >>> Agents concept is described in Section 4.2.1 [0].
> > > > > > >>>
> > > > > > >>> scmi_devid in Device-tree node example:
> > > > > > >>> usb@e6590000
> > > > > > >>> {
> > > > > > >>>     scmi_devid = <19>;
> > > > > > >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> > > > > > >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> > > > > > >>>     power-domains = <&scmi_power 0>;
> > > > > > >>> };
> > > > > > >>
> > > > > > >> And how do you prevent DT overlay adding such devid to any other node
> > > > > > >> thus allowing any other device to send requests with given devid?
> > > > > > >>
> > > > > > > Thank you for the quick response.
> > > > > > > scmi_devid value will be used only by Trusted Agent when the device
> > > > > > > permissions are set. Non-trusted agents, which in our case are
> > > > > > > represented as Guest OS are using scmi drivers, already present in linux
> > > > > > > kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> > > > > > > nodes to access to SCMI protocol.
> > > > > > 
> > > > > > Ah, ok.
> > > > > > 
> > > > > > > 
> > > > > > >> Plus few technicalities:
> > > > > > >> 1. Hyphen, not underscore in property name, so scmi-devid.
> > > > > > > 
> > > > > > > Thanks for the tip, I will change that in v2.
> > > > > > 
> > > > > > Few more thoughts:
> > > > > > 1. This looks specific to ARM SCMI, so you also need vendor prefix, so
> > > > > > something like:
> > > > > > arm,scmi-devid
> > > > > > arm,scmi-device-id
> > > > > > 
> > > > > 
> > > > > Keeping the other discussion separate, I wanted to comment on this.
> > > > > I agree with Krzysztof on having vendor specific prefix if we decide to add
> > > > > this device id thing. However, I prefer not to use "arm,scmi-" here.
> > > > > It can be "xen,scmi-" as we had plans to introduce some concepts in SCMI
> > > > > spec that may use looks like this device-id. I would just like to avoid
> > > > > conflicting with that in the future. It may happen to be same in the future
> > > > > (i.e. this xen device-id matches 100% with definition of device-id we might
> > > > > introduce in the spec, but I want to make assumption otherwise and leave
> > > > > scope for divergence however small/little it can be). No issues even if
> > > > > they converge and match 100% later in the far future.
> > > > > 
> > > > 
> > > > xem,scmi- works for me. What do other thinks?
> > >   ^ xen,scmi-
> > > 
> > > If this problem was Xen specific, then it would be fine to use xen,scmi-
> > > As Xen developer, it solves my problem and I am fine with it.
> > > 
> > > However, from a device tree and SCMI point of view, it looks like this
> > > problem is generic and it just happens that Xen is the first
> > > implementation to encounter it.
> > > 
> > > Cristian wrote: "The SCMI spec does not indeed cover the discovery of
> > > such devices and the related associated resources: it indeed delegates
> > > such description to FDT/ACPI as of now." How is that supposed to happen
> > > today with the current DT definitions, regardless of Xen? Is it a gap in
> > > the current device tree binding?
> > 
> > What I meant is that in fact SCMI device IDs are NOT needed in the Linux
> > Kernel DT, in fact also this series does not add any code using it and
> > there is no code as of now in Kernel to issue BASE_SET_DEVICE_PERMISSIONS
> > commands; Linux Guest OS in the above scenario is a Non-Trusted agent and
> > doesn't need to know SCMI DevIDs and must NOT have access to those IDs for
> > security reasons too (as Sudeep was saying): the Trusted Agent (XEN here)
> > and the SCMI platform server are the only ones required to share the
> > knowledge of such Device IDs (and how the related resources are grouped)
> > via some HW description scheme as you are doing indeed in XEN.
> > 
> > So, while on one side such device IDs discovery is delegated by the spec
> > to the HW description mechanisms, it seems just not needed in Kernel DT
> > given the kind or role it has as an SCMI agent in this context: as said
> > in fact there won't be any use as of now in Linux of such DT entries as
> > of now.
> 
> I see. This is one of those "difficult" cases where the device tree
> binding is needed but it is not used by the Linux kernel. (We have a few
> cases like this in system device tree and the more device tree gets
> adopted by other projects the more will see instances like this one.)
> 
> In that case, I think it is OK to call it "xen,scmi-" if Rob also
> agrees.
> 
> After all, we are currently using in Xen a property called
> "linux,pci-domain". We might as well have Linux in the future use a
> property called "xen,scmi-devid" to even things out :-)

In this case, if Rob is ok with xen,scmi-devid then I'll rename binding.

Best regards,
Oleksii

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Hi Sudeep,

On Thu, Feb 24, 2022 at 11:54:43AM +0000, Sudeep Holla wrote:
> On Tue, Feb 22, 2022 at 05:15:49PM +0000, Oleksii Moisieiev wrote:
> > Hi Sudeep,
> > 
> > On Tue, Feb 22, 2022 at 04:06:37PM +0000, Sudeep Holla wrote:
> > > Hi Oleksii,
> > > 
> > > My initial feedback on this. And thanks Cristian for making it so easy as
> > > you have covered most of the things in depth(which I might have not done
> > > myself that well)
> > > 
> > > On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> > > > On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > > > > Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > This parameter should be set for the device nodes, which has
> > > > > clocks/power-domains/resets working through SCMI.
> > > 
> > > I prefer you had given more details on your usage model here instead of
> > > pointing to the other Xen thread as it helps for someone without much
> > > background on Xen or your use-case to review this.
> > > 
> > Let me describe the process in few words:
> > We implemented a new feature, called SCI-mediator in Xen.
> > The proposed implementation allows Guests to communicate with the Firmware using SCMI
> > protocol with SMC as a transport. Other implementation are also
> > possible, such as SCMI-Mailbox, SCPI-mailbox etc.
> > 
> > In this feature Xen is the Trusted Agent, which receives the following
> > information in Xen device-tree:
> > 1) All channels should be described, each channel defined as
> > arm,scmi-shmem node;
> > 2) Scmi node arm,scmi-smc with protocols description;
> 
> Sounds good so far.
> 
> > 3) scmi-devid should be set in nodes, which works through SCMI.
> >
> 
> Why is this needed for Guest OS, you need not populate this if Guest OS
> is not required to use it, right ? If it is needed just by Xen hypervisor,
> lets talk about that and why it is bad idea to mix that with general
> SCMI bindings.
> 
Yes, I agree that the device_id should not be populated to the Guest OS,
but trusted agent still require this information. I can't tell that it
is needed only by Xen hypervioer, I'd rather tell that it is needed only
for the Trusted Agent. This is only our case, when Trusted adent is Xen,
Different setup, where trusted agent is for example daemon, running in
the separate guest, or another hypervisor, such as KVM etc, still
requires the same device_ids. That's why the idea was to present generic
parameter.


> > On start Xen inits itself as trusted agent and requests agent
> > configuration by using BASE_DISCOVER_AGENT message. This message is sent
> > to each configured channel to get agent_id
> > 
> > On Domain creation stage Xen will do the following steps:
> > 1) Assign channel to the Guest and map channel address to the Domain
> > address. For the Domain this address should be the same;
> > 2) Generate arm,scmi-shmem and arm,scmi-smc nodes if needed for Guest
> > device-tree (the device-tree which should be passed to the Guest);
> > 3) Process devices, which are passed through to this Guest and set
> > BASE_SET_DEVICE_PERMISSIONS for the scmi-devid, received from the
> > device-node;
> >
> 
> I am confused here. So the Xen knows which devices are assigned to each
> Guest OS but doesn't know device ID for them, but relies on the device
> tree node ?

Xen knows the device-list that should be passed to the Domain. Each
device is represented by the device-tree path so Xen can access the
device-tree device node and get all infromation from it.

> 
> > Guest OS will receive non-trusted channel and ignore scmi-devid fields
> > in the device-nodes.
> >
> 
> Then no need to pass it. It keeps the SCMI agent binding clean.
> 

Ok, Agreed.

> > IMPORTANT: Guest OS is non-trusted Agent. Xen is the only trusted agent
> > in the system. Guest OS uses standart scmi drivers without any xen
> > related changes. So Guest OS doesn't know it works through mediator.
> >
> 
> Good.
> 
> > The main question is - how Firmware will know what agent sent SMC
> > message and what channel_id should be used? I couldn't find clear
> > explanation in spec.
> 
> 1. So the hypervisor forwards all the messages from different guests without
>    any marshalling ?

Correct.

> 2. If Xen is just acting as pass through, why does firmware care about the
>    origin of the message.
>

Because Firmware has, let's say, 15 agents with 15 shmem pages in memory.
It should know which page it should read.

> > That's why I end up with the following approaches:
> > 1) Current implemenation: Guest OS send SMC request, Xen intercept this
> > request and set channel ID to SMC Client_ID field (reg7), then resend
> > SMC message to Firmware. Firmware parses SMC Client ID to get channel_id
> > to work with.
> 
> As asked above, why is this information important to the firmware.

Please see above.

> 
> > 2) Another approach is to generate unique FuncID for each GuestOS. In
> > this case no interception from Xen is needed - Guest OS can work
> > directly with Firmware.
> >
> 
> I think that is the only way today to support multiple channels with
> SMC/HVC. The reason for that is since the SMC FID is custom, we can't take
> custom parameters and write a generic SCMI smc transport driver. This was
> discussed and we decided to go for different FID, otherwise we would have
> to standardise parameters to the custom FID which is insane IMO.
> 

Thank you for the clarification. I will refactor Xen code.
Do you mind if I mention you in the cover letter and say that this
approach was approved from Arm side?

> > I hope you'll be able to help me with that.
> >
> 
> Thanks for details. It definitely provided more information though not yet
> complete as you can guess with my questions here.
> 
> > > > > Given parameter should set the device_id, needed to set device
> > > > > permissions in the Firmware. This feature will be extremely useful for
> > > > > the virtualized systems, which has more that one Guests running on the
> > > > > system at the same time or for the syestems, which require several
> > > > > agents with different permissions. Trusted agent will use scmi_devid to
> > > > > set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > > > > for details).
> > > 
> > > I am bit confused here, so you expecting a non-secure/non-trusted entity
> > > to supply this device-id to the Trusted agent ? Is that not the breach of
> > > trust as any non-trusted entity can supply any agent-id and get the permission
> > > to access the associated resource in this way ? Or am I missing something
> > > totally here.
> > > 
> > 
> > No, Device-id will be used only by trusted agents, which is Xen in our
> > case. Please see above.
> >
> 
> Understood now, and I will assert guest OS must not have it in its DT.
> 
> 
> [...]
> 
> > > >
> > > > So in all of this, I don't get why you need this DT definition aggregating SCMI
> > > > resources to SCMI device IDs in the Guest OS, which is an SCMI agent that does not
> > > > need to now anything about SCMI device IDs (at least with the current spec): this
> > > > would make sense only if the Linux Kernel was the TrustedAgent in charge of
> > > > configuring the devices permissions via BASE_SET_DEVICE_PERMISSIONS.
> > > > (in fact you said you won't provide any code to manage this scmi_devid
> > > > in the kernel since those guests are not trusted agents and the won't be
> > > > allowed to set device permissions...)
> > > >
> > > 
> > > +1 (again)
> > > 
> > > > The only tricky part I can see in all of the above is agent identification, since
> > > > the agents are assigned an ID by the SCMI platform (which can be queried) and they
> > > > have a set of dedicated channels to use, so basically the platform really identifies
> > > > the Agents looking at the channel from which a request is coming from and AgentID is
> > > > not carried inside the message as a source and cannot be spoofed.
> > > >
> > > 
> > > IIUC, the physical/virtual transport and associated transport chosen
> > > identifies the agent for the SCMI platform.
> > 
> > Could you please clarify what do you mean under "physical/virtual
> > transport"?
> 
> I was speculating some design in Xen on how it present virtual channels to
> guests. Ignore that as I now understand you are using SMC.
> 
> > For now yes - Firmware should get information for the channel from
> > transport.
> 
> Indeed, with SMC/HVC, you will need different FID for reasons stated above.
> 
> > > 
> > > > > Given example shows the configuration of the hsusb node, which is using
> > > > > scmi to contol clocks, resets and power-domains. scmi_devid is set
> > > > > equals to 19, which should match defined id for usb in the Firmware.
> > > > >
> > > > > Trusted agent will use scmi_devid to set the device permissions for
> > > > > the Agents. Guest OS should not have an access to the permissions
> > > > > settings, so no code to process scmi_devid was presented in Linux
> > > > > kernel.
> > > > >
> > > > > We are currently contributing changes to Xen, which are intended to
> > > > > mediate SCMI access from Guests to the Firmware. Xen uses scmi_devid to set
> > > > > the permissions for the devices. See [1] thread for details.
> > > > >
> > > > > [0] https://urldefense.com/v3/__https://developer.arm.com/documentation/den0056/latest__;!!GF_29dbcQIUBPA!mGggDzmp0B8cSdGJdH4utz6sx7g5PMXq05mXf91dU8XgkJaCuEpHdARZCdl-g1BnrduL$ [developer[.]arm[.]com]
> > > > > [1] https://urldefense.com/v3/__https://xen.markmail.org/message/mmi4fpb4qr6e3kad__;!!GF_29dbcQIUBPA!mGggDzmp0B8cSdGJdH4utz6sx7g5PMXq05mXf91dU8XgkJaCuEpHdARZCdl-g-bWzzb5$ [xen[.]markmail[.]org]
> > > >
> > > > IMHO, but I could be wrong, looking at the current SCMI spec you cannot just
> > > > gather messages from a set of GuestOs talking via different SCMI channels and
> > > > then pipe/route them through a single channel to the backend server,
> > > > attaching/spoofing some sort of Agent source ID to each message like you seem to
> > > > be doing in the Xen series
> > > >
> > > 
> > > I haven't looked at the other series, but it is hard to say the spec prohibits
> > > this. I don't understand that spoofing part, but Xen hyp can arbitrate the
> > > requests across guests I believe. But the devil is in details so I can't
> > > comment on what is done. What I can say is this Agent ID is in each message is
> > > not compliant to spec.
> > > 
> > 
> > In our implementation XEN do not copy any data from shared memory. The
> > only thing it does is letting Firmware know which channel it should use.
> 
> OK.
> 
> -- 
> Regards,
> Sudeep

Best regards,
Oleksii.

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Oleksii Moisieiev]

Hi Stefano,

On Thu, Feb 24, 2022 at 02:51:42PM -0800, Stefano Stabellini wrote:
> On Thu, 24 Feb 2022, Sudeep Holla wrote:
> > On Tue, Feb 22, 2022 at 05:15:49PM +0000, Oleksii Moisieiev wrote:
> > > Hi Sudeep,
> > > 
> > > On Tue, Feb 22, 2022 at 04:06:37PM +0000, Sudeep Holla wrote:
> > > > Hi Oleksii,
> > > > 
> > > > My initial feedback on this. And thanks Cristian for making it so easy as
> > > > you have covered most of the things in depth(which I might have not done
> > > > myself that well)
> > > > 
> > > > On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> > > > > On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > > > > > Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > > This parameter should be set for the device nodes, which has
> > > > > > clocks/power-domains/resets working through SCMI.
> > > > 
> > > > I prefer you had given more details on your usage model here instead of
> > > > pointing to the other Xen thread as it helps for someone without much
> > > > background on Xen or your use-case to review this.
> > > > 
> > > Let me describe the process in few words:
> > > We implemented a new feature, called SCI-mediator in Xen.
> > > The proposed implementation allows Guests to communicate with the Firmware using SCMI
> > > protocol with SMC as a transport. Other implementation are also
> > > possible, such as SCMI-Mailbox, SCPI-mailbox etc.
> > > 
> > > In this feature Xen is the Trusted Agent, which receives the following
> > > information in Xen device-tree:
> > > 1) All channels should be described, each channel defined as
> > > arm,scmi-shmem node;
> > > 2) Scmi node arm,scmi-smc with protocols description;
> > 
> > Sounds good so far.
> > 
> > > 3) scmi-devid should be set in nodes, which works through SCMI.
> > >
> > 
> > Why is this needed for Guest OS, you need not populate this if Guest OS
> > is not required to use it, right ? If it is needed just by Xen hypervisor,
> > lets talk about that and why it is bad idea to mix that with general
> > SCMI bindings.
> 
> I'll try to help Oleksii by answering here, I hope I am not off the mark
> :-)

I appreciate it.

> 
> I think Sudeep is right, scmi-devid is not needed by the guest OS.
> 

I agree. I think this should be handled by Xen.

Best regards,
Oleksii.

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Sudeep Holla]

(sorry for the delay, had to move my email setup and some mails were
stuck in outbox and I missed to notice)

On Thu, Feb 24, 2022 at 03:34:01PM -0800, Stefano Stabellini wrote:
> On Thu, 24 Feb 2022, Stefano Stabellini wrote:
> > On Thu, 24 Feb 2022, Sudeep Holla wrote:
> > > On Tue, Feb 22, 2022 at 05:15:49PM +0000, Oleksii Moisieiev wrote:
> > > > Hi Sudeep,
> > > > 
> > > > On Tue, Feb 22, 2022 at 04:06:37PM +0000, Sudeep Holla wrote:
> > > > > Hi Oleksii,
> > > > > 
> > > > > My initial feedback on this. And thanks Cristian for making it so easy as
> > > > > you have covered most of the things in depth(which I might have not done
> > > > > myself that well)
> > > > > 
> > > > > On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> > > > > > On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > > > > > > Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > > > This parameter should be set for the device nodes, which has
> > > > > > > clocks/power-domains/resets working through SCMI.
> > > > > 
> > > > > I prefer you had given more details on your usage model here instead of
> > > > > pointing to the other Xen thread as it helps for someone without much
> > > > > background on Xen or your use-case to review this.
> > > > > 
> > > > Let me describe the process in few words:
> > > > We implemented a new feature, called SCI-mediator in Xen.
> > > > The proposed implementation allows Guests to communicate with the Firmware using SCMI
> > > > protocol with SMC as a transport. Other implementation are also
> > > > possible, such as SCMI-Mailbox, SCPI-mailbox etc.
> > > > 
> > > > In this feature Xen is the Trusted Agent, which receives the following
> > > > information in Xen device-tree:
> > > > 1) All channels should be described, each channel defined as
> > > > arm,scmi-shmem node;
> > > > 2) Scmi node arm,scmi-smc with protocols description;
> > > 
> > > Sounds good so far.
> > > 
> > > > 3) scmi-devid should be set in nodes, which works through SCMI.
> > > >
> > > 
> > > Why is this needed for Guest OS, you need not populate this if Guest OS
> > > is not required to use it, right ? If it is needed just by Xen hypervisor,
> > > lets talk about that and why it is bad idea to mix that with general
> > > SCMI bindings.
> > 
> > I'll try to help Oleksii by answering here, I hope I am not off the mark
> > :-)
> > 
> > I think Sudeep is right, scmi-devid is not needed by the guest OS.
> > 
> > The host device tree is a more interesting discussion. As the host
> > device tree is meant to be generic and not tied to a specific version of
> > Linux, it should fully describe the SCMI interface available. If the
> > device tree is provided to a Trusted Agent, then it should also have the
> > scmi-devid information, right?
> > 
> > 
> > > > On start Xen inits itself as trusted agent and requests agent
> > > > configuration by using BASE_DISCOVER_AGENT message. This message is sent
> > > > to each configured channel to get agent_id
> > > > 
> > > > On Domain creation stage Xen will do the following steps:
> > > > 1) Assign channel to the Guest and map channel address to the Domain
> > > > address. For the Domain this address should be the same;
> > > > 2) Generate arm,scmi-shmem and arm,scmi-smc nodes if needed for Guest
> > > > device-tree (the device-tree which should be passed to the Guest);
> > > > 3) Process devices, which are passed through to this Guest and set
> > > > BASE_SET_DEVICE_PERMISSIONS for the scmi-devid, received from the
> > > > device-node;
> > > >
> > > 
> > > I am confused here. So the Xen knows which devices are assigned to each
> > > Guest OS but doesn't know device ID for them, but relies on the device
> > > tree node ?
> > 
> > Which devices go to which guest OS is a user-provided configuration. For
> > instance, a user can say: "assing /amba/ethernet@ff0e0000 to dom1". This
> > is normal and not related to SCMI: when a user configures a static
> > partitioning system, they decide which resources belong to which domain.
> > 
> > So Xen is told that /amba/ethernet@ff0e0000 is supposed to go to dom1.
> > Xen proceeds to map memory and interrupts corresponding to
> > /amba/ethernet@ff0e0000 to dom1. So far so good. What about SCMI?
> > 
> > In Oleksii's design, Xen is going to assign one of the available SCMI
> > channels to dom1 and restrict its permission to only
> > /amba/ethernet@ff0e0000. To do that, Xen needs to know the scmi-devid of
> > /amba/ethernet@ff0e0000. As far as I can tell there is nothing
> > Xen-specific in this activitity, that's why I asked Oleksii to reach out
> > to the upstream device tree community to improve the generic bindings
> > for everyone's benefits.
> 
> Let's leave Linux and Xen aside for the moment. What are other possible
> Trusted Agents? (Maybe TF-A?) How do they get the scmi-devid? It looks
> like it was supposed to come from device tree but nobody got around to
> adding it to the binding because it is not used by Linux?

I do agree we need this info and probably device tree is the way. But what
I disagree here is that it needs to part of existing SCMI bindings which are
for the SCMI users only and not for one that may provide the interface(SCMI
platform/server/arbitrator/passthrough/..whatever). You can have bindings for
them as part of system device tree initiative and can be merged back to Linux
if that happens. Or we may even take the whole devicetree bindings out of
the Linux one day (when all the stars are aligned :) )

--
Regards,
Sudeep

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Sudeep Holla]

On Wed, Feb 23, 2022 at 06:51:59PM -0800, Stefano Stabellini wrote:
> On Tue, 22 Feb 2022, Oleksii Moisieiev wrote:
> > On Tue, Feb 22, 2022 at 04:14:40PM +0000, Sudeep Holla wrote:
> > > On Tue, Feb 22, 2022 at 09:06:25AM +0100, Krzysztof Kozlowski wrote:
> > > > On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> > > > > Hi Krzysztof,
> > > > > 
> > > > > On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> > > > >> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> > > > >>> Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > >>> This parameter should be set for the device nodes, which has
> > > > >>> clocks/power-domains/resets working through SCMI.
> > > > >>> Given parameter should set the device_id, needed to set device
> > > > >>> permissions in the Firmware. This feature will be extremely useful for
> > > > >>> the virtualized systems, which has more that one Guests running on the
> > > > >>> system at the same time or for the syestems, which require several
> > > > >>> agents with different permissions. Trusted agent will use scmi_devid to
> > > > >>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > > > >>> for details).
> > > > >>> Agents concept is described in Section 4.2.1 [0].
> > > > >>>
> > > > >>> scmi_devid in Device-tree node example:
> > > > >>> usb@e6590000
> > > > >>> {
> > > > >>>     scmi_devid = <19>;
> > > > >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> > > > >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> > > > >>>     power-domains = <&scmi_power 0>;
> > > > >>> };
> > > > >>
> > > > >> And how do you prevent DT overlay adding such devid to any other node
> > > > >> thus allowing any other device to send requests with given devid?
> > > > >>
> > > > > Thank you for the quick response.
> > > > > scmi_devid value will be used only by Trusted Agent when the device
> > > > > permissions are set. Non-trusted agents, which in our case are
> > > > > represented as Guest OS are using scmi drivers, already present in linux
> > > > > kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> > > > > nodes to access to SCMI protocol.
> > > > 
> > > > Ah, ok.
> > > > 
> > > > > 
> > > > >> Plus few technicalities:
> > > > >> 1. Hyphen, not underscore in property name, so scmi-devid.
> > > > > 
> > > > > Thanks for the tip, I will change that in v2.
> > > > 
> > > > Few more thoughts:
> > > > 1. This looks specific to ARM SCMI, so you also need vendor prefix, so
> > > > something like:
> > > > arm,scmi-devid
> > > > arm,scmi-device-id
> > > > 
> > > 
> > > Keeping the other discussion separate, I wanted to comment on this.
> > > I agree with Krzysztof on having vendor specific prefix if we decide to add
> > > this device id thing. However, I prefer not to use "arm,scmi-" here.
> > > It can be "xen,scmi-" as we had plans to introduce some concepts in SCMI
> > > spec that may use looks like this device-id. I would just like to avoid
> > > conflicting with that in the future. It may happen to be same in the future
> > > (i.e. this xen device-id matches 100% with definition of device-id we might
> > > introduce in the spec, but I want to make assumption otherwise and leave
> > > scope for divergence however small/little it can be). No issues even if
> > > they converge and match 100% later in the far future.
> > > 
> > 
> > xem,scmi- works for me. What do other thinks?
>   ^ xen,scmi-
> 
> If this problem was Xen specific, then it would be fine to use xen,scmi-
> As Xen developer, it solves my problem and I am fine with it.
> 
> However, from a device tree and SCMI point of view, it looks like this
> problem is generic and it just happens that Xen is the first
> implementation to encounter it.
> 
> Cristian wrote: "The SCMI spec does not indeed cover the discovery of
> such devices and the related associated resources: it indeed delegates
> such description to FDT/ACPI as of now." How is that supposed to happen
> today with the current DT definitions, regardless of Xen? Is it a gap in
> the current device tree binding?

The fact that we don't need this to be part of SCMI OSPM user bindings,
it is not addressed and can be considered as a gap.

+ The reason I want to keep it xen specific at the moment as we had some
plan to extended the device-id usage in the spec which hasn't progressed
a bit(I must admit that before you ask), and this addition should not be
obstruct that future development. If we align with what we define xen
specific as part of $subject work, we can always define generic binding
in the future and slowly make the other obsolete over the time.

-- 
Regards,
Sudeep

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Sudeep Holla]

On Thu, Feb 24, 2022 at 02:22:28PM -0800, Stefano Stabellini wrote:
> On Thu, 24 Feb 2022, Cristian Marussi wrote:
> > On Wed, Feb 23, 2022 at 06:51:59PM -0800, Stefano Stabellini wrote:
> > > On Tue, 22 Feb 2022, Oleksii Moisieiev wrote:
> > > > On Tue, Feb 22, 2022 at 04:14:40PM +0000, Sudeep Holla wrote:
> > > > > On Tue, Feb 22, 2022 at 09:06:25AM +0100, Krzysztof Kozlowski wrote:
> > > > > > On 21/02/2022 22:39, Oleksii Moisieiev wrote:
> > > > > > > Hi Krzysztof,
> > > > > > > 
> > 
> > Hi Stefano,
> > 
> > > > > > > On Mon, Feb 21, 2022 at 10:01:43PM +0100, Krzysztof Kozlowski wrote:
> > > > > > >> On 21/02/2022 18:26, Oleksii Moisieiev wrote:
> > > > > > >>> Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > > >>> This parameter should be set for the device nodes, which has
> > > > > > >>> clocks/power-domains/resets working through SCMI.
> > > > > > >>> Given parameter should set the device_id, needed to set device
> > > > > > >>> permissions in the Firmware. This feature will be extremely useful for
> > > > > > >>> the virtualized systems, which has more that one Guests running on the
> > > > > > >>> system at the same time or for the syestems, which require several
> > > > > > >>> agents with different permissions. Trusted agent will use scmi_devid to
> > > > > > >>> set the Device permissions for the Firmware (See Section 4.2.2.10 [0]
> > > > > > >>> for details).
> > > > > > >>> Agents concept is described in Section 4.2.1 [0].
> > > > > > >>>
> > > > > > >>> scmi_devid in Device-tree node example:
> > > > > > >>> usb@e6590000
> > > > > > >>> {
> > > > > > >>>     scmi_devid = <19>;
> > > > > > >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> > > > > > >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> > > > > > >>>     power-domains = <&scmi_power 0>;
> > > > > > >>> };
> > > > > > >>
> > > > > > >> And how do you prevent DT overlay adding such devid to any other node
> > > > > > >> thus allowing any other device to send requests with given devid?
> > > > > > >>
> > > > > > > Thank you for the quick response.
> > > > > > > scmi_devid value will be used only by Trusted Agent when the device
> > > > > > > permissions are set. Non-trusted agents, which in our case are
> > > > > > > represented as Guest OS are using scmi drivers, already present in linux
> > > > > > > kernel, ignores scmi_devid and uses scmi_clocks, scmi_power, scmi_reset
> > > > > > > nodes to access to SCMI protocol.
> > > > > > 
> > > > > > Ah, ok.
> > > > > > 
> > > > > > > 
> > > > > > >> Plus few technicalities:
> > > > > > >> 1. Hyphen, not underscore in property name, so scmi-devid.
> > > > > > > 
> > > > > > > Thanks for the tip, I will change that in v2.
> > > > > > 
> > > > > > Few more thoughts:
> > > > > > 1. This looks specific to ARM SCMI, so you also need vendor prefix, so
> > > > > > something like:
> > > > > > arm,scmi-devid
> > > > > > arm,scmi-device-id
> > > > > > 
> > > > > 
> > > > > Keeping the other discussion separate, I wanted to comment on this.
> > > > > I agree with Krzysztof on having vendor specific prefix if we decide to add
> > > > > this device id thing. However, I prefer not to use "arm,scmi-" here.
> > > > > It can be "xen,scmi-" as we had plans to introduce some concepts in SCMI
> > > > > spec that may use looks like this device-id. I would just like to avoid
> > > > > conflicting with that in the future. It may happen to be same in the future
> > > > > (i.e. this xen device-id matches 100% with definition of device-id we might
> > > > > introduce in the spec, but I want to make assumption otherwise and leave
> > > > > scope for divergence however small/little it can be). No issues even if
> > > > > they converge and match 100% later in the far future.
> > > > > 
> > > > 
> > > > xem,scmi- works for me. What do other thinks?
> > >   ^ xen,scmi-
> > > 
> > > If this problem was Xen specific, then it would be fine to use xen,scmi-
> > > As Xen developer, it solves my problem and I am fine with it.
> > > 
> > > However, from a device tree and SCMI point of view, it looks like this
> > > problem is generic and it just happens that Xen is the first
> > > implementation to encounter it.
> > > 
> > > Cristian wrote: "The SCMI spec does not indeed cover the discovery of
> > > such devices and the related associated resources: it indeed delegates
> > > such description to FDT/ACPI as of now." How is that supposed to happen
> > > today with the current DT definitions, regardless of Xen? Is it a gap in
> > > the current device tree binding?
> > 
> > What I meant is that in fact SCMI device IDs are NOT needed in the Linux
> > Kernel DT, in fact also this series does not add any code using it and
> > there is no code as of now in Kernel to issue BASE_SET_DEVICE_PERMISSIONS
> > commands; Linux Guest OS in the above scenario is a Non-Trusted agent and
> > doesn't need to know SCMI DevIDs and must NOT have access to those IDs for
> > security reasons too (as Sudeep was saying): the Trusted Agent (XEN here)
> > and the SCMI platform server are the only ones required to share the
> > knowledge of such Device IDs (and how the related resources are grouped)
> > via some HW description scheme as you are doing indeed in XEN.
> > 
> > So, while on one side such device IDs discovery is delegated by the spec
> > to the HW description mechanisms, it seems just not needed in Kernel DT
> > given the kind or role it has as an SCMI agent in this context: as said
> > in fact there won't be any use as of now in Linux of such DT entries as
> > of now.
> 
> I see. This is one of those "difficult" cases where the device tree
> binding is needed but it is not used by the Linux kernel. (We have a few
> cases like this in system device tree and the more device tree gets
> adopted by other projects the more will see instances like this one.)
>

Indeed, I mentioned this in the other email. It clearly falls under the
larger umbrella of system device tree and not the one used by the Linux.

> In that case, I think it is OK to call it "xen,scmi-" if Rob also
> agrees.
>

Thanks.

> After all, we are currently using in Xen a property called
> "linux,pci-domain". We might as well have Linux in the future use a
> property called "xen,scmi-devid" to even things out :-)

Sure or we may add a generic one in the future as mentioned in the other
email for reasons mentioned there.

--
Regards,
Sudeep

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Stefano Stabellini]

On Fri, 4 Mar 2022, Sudeep Holla wrote:
> (sorry for the delay, had to move my email setup and some mails were
> stuck in outbox and I missed to notice)
> 
> On Thu, Feb 24, 2022 at 03:34:01PM -0800, Stefano Stabellini wrote:
> > On Thu, 24 Feb 2022, Stefano Stabellini wrote:
> > > On Thu, 24 Feb 2022, Sudeep Holla wrote:
> > > > On Tue, Feb 22, 2022 at 05:15:49PM +0000, Oleksii Moisieiev wrote:
> > > > > Hi Sudeep,
> > > > > 
> > > > > On Tue, Feb 22, 2022 at 04:06:37PM +0000, Sudeep Holla wrote:
> > > > > > Hi Oleksii,
> > > > > > 
> > > > > > My initial feedback on this. And thanks Cristian for making it so easy as
> > > > > > you have covered most of the things in depth(which I might have not done
> > > > > > myself that well)
> > > > > > 
> > > > > > On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> > > > > > > On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > > > > > > > Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > > > > This parameter should be set for the device nodes, which has
> > > > > > > > clocks/power-domains/resets working through SCMI.
> > > > > > 
> > > > > > I prefer you had given more details on your usage model here instead of
> > > > > > pointing to the other Xen thread as it helps for someone without much
> > > > > > background on Xen or your use-case to review this.
> > > > > > 
> > > > > Let me describe the process in few words:
> > > > > We implemented a new feature, called SCI-mediator in Xen.
> > > > > The proposed implementation allows Guests to communicate with the Firmware using SCMI
> > > > > protocol with SMC as a transport. Other implementation are also
> > > > > possible, such as SCMI-Mailbox, SCPI-mailbox etc.
> > > > > 
> > > > > In this feature Xen is the Trusted Agent, which receives the following
> > > > > information in Xen device-tree:
> > > > > 1) All channels should be described, each channel defined as
> > > > > arm,scmi-shmem node;
> > > > > 2) Scmi node arm,scmi-smc with protocols description;
> > > > 
> > > > Sounds good so far.
> > > > 
> > > > > 3) scmi-devid should be set in nodes, which works through SCMI.
> > > > >
> > > > 
> > > > Why is this needed for Guest OS, you need not populate this if Guest OS
> > > > is not required to use it, right ? If it is needed just by Xen hypervisor,
> > > > lets talk about that and why it is bad idea to mix that with general
> > > > SCMI bindings.
> > > 
> > > I'll try to help Oleksii by answering here, I hope I am not off the mark
> > > :-)
> > > 
> > > I think Sudeep is right, scmi-devid is not needed by the guest OS.
> > > 
> > > The host device tree is a more interesting discussion. As the host
> > > device tree is meant to be generic and not tied to a specific version of
> > > Linux, it should fully describe the SCMI interface available. If the
> > > device tree is provided to a Trusted Agent, then it should also have the
> > > scmi-devid information, right?
> > > 
> > > 
> > > > > On start Xen inits itself as trusted agent and requests agent
> > > > > configuration by using BASE_DISCOVER_AGENT message. This message is sent
> > > > > to each configured channel to get agent_id
> > > > > 
> > > > > On Domain creation stage Xen will do the following steps:
> > > > > 1) Assign channel to the Guest and map channel address to the Domain
> > > > > address. For the Domain this address should be the same;
> > > > > 2) Generate arm,scmi-shmem and arm,scmi-smc nodes if needed for Guest
> > > > > device-tree (the device-tree which should be passed to the Guest);
> > > > > 3) Process devices, which are passed through to this Guest and set
> > > > > BASE_SET_DEVICE_PERMISSIONS for the scmi-devid, received from the
> > > > > device-node;
> > > > >
> > > > 
> > > > I am confused here. So the Xen knows which devices are assigned to each
> > > > Guest OS but doesn't know device ID for them, but relies on the device
> > > > tree node ?
> > > 
> > > Which devices go to which guest OS is a user-provided configuration. For
> > > instance, a user can say: "assing /amba/ethernet@ff0e0000 to dom1". This
> > > is normal and not related to SCMI: when a user configures a static
> > > partitioning system, they decide which resources belong to which domain.
> > > 
> > > So Xen is told that /amba/ethernet@ff0e0000 is supposed to go to dom1.
> > > Xen proceeds to map memory and interrupts corresponding to
> > > /amba/ethernet@ff0e0000 to dom1. So far so good. What about SCMI?
> > > 
> > > In Oleksii's design, Xen is going to assign one of the available SCMI
> > > channels to dom1 and restrict its permission to only
> > > /amba/ethernet@ff0e0000. To do that, Xen needs to know the scmi-devid of
> > > /amba/ethernet@ff0e0000. As far as I can tell there is nothing
> > > Xen-specific in this activitity, that's why I asked Oleksii to reach out
> > > to the upstream device tree community to improve the generic bindings
> > > for everyone's benefits.
> > 
> > Let's leave Linux and Xen aside for the moment. What are other possible
> > Trusted Agents? (Maybe TF-A?) How do they get the scmi-devid? It looks
> > like it was supposed to come from device tree but nobody got around to
> > adding it to the binding because it is not used by Linux?
> 
> I do agree we need this info and probably device tree is the way. But what
> I disagree here is that it needs to part of existing SCMI bindings which are
> for the SCMI users only and not for one that may provide the interface(SCMI
> platform/server/arbitrator/passthrough/..whatever). You can have bindings for
> them as part of system device tree initiative and can be merged back to Linux
> if that happens. Or we may even take the whole devicetree bindings out of
> the Linux one day (when all the stars are aligned :) )

I would love to hear Rob's opinion on what I am about to write next on
the topic of whether the binding should be under linux.git.

I am not sure if the policy is that only device tree bindings actively
used by Linux are present under
linux.git/Documentation/devicetree/bindings/. There are a tons of other
projects using device tree and without a central point for keeping these
bindings the specification will shatter. Given that Linux prefers to
keep the bindings under linux.git, then the logic conclusion is that
linux.git/Documentation/devicetree/bindings/ should also hold bindings
not actively used by Linux right at the moment. Especially bindings that
could be used by Linux in the future. Otherwise we risk a new binding
being used by U-boot, Xen, Zephyr and others then Linux introduces an
incompatible version of it. Nobody would win in that situation.


> > After all, we are currently using in Xen a property called
> > "linux,pci-domain". We might as well have Linux in the future use a
> > property called "xen,scmi-devid" to even things out :-)
> 
> Sure or we may add a generic one in the future as mentioned in the other
> email for reasons mentioned there.

[...]

> The fact that we don't need this to be part of SCMI OSPM user bindings,
> it is not addressed and can be considered as a gap.
> 
> + The reason I want to keep it xen specific at the moment as we had some
> plan to extended the device-id usage in the spec which hasn't progressed
> a bit(I must admit that before you ask), and this addition should not be
> obstruct that future development. If we align with what we define xen
> specific as part of $subject work, we can always define generic binding
> in the future and slowly make the other obsolete over the time.

Keep in mind that device tree is supposed to be backward compatible,
with or without a "xen," prefix. The process of updating the binding and
making the older binding obsolete won't be any different whether the
property is called "super-official-device-id" or "xen,scmi-devid".

(Also, it is not Xen specific, but as we have enstablished, it is for any
Trusted Agents.)

Why not review the bindings now also considering a future Linux use of
it?

Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

[Rob Herring]

On Fri, Mar 04, 2022 at 04:45:31PM -0800, Stefano Stabellini wrote:
> On Fri, 4 Mar 2022, Sudeep Holla wrote:
> > (sorry for the delay, had to move my email setup and some mails were
> > stuck in outbox and I missed to notice)
> > 
> > On Thu, Feb 24, 2022 at 03:34:01PM -0800, Stefano Stabellini wrote:
> > > On Thu, 24 Feb 2022, Stefano Stabellini wrote:
> > > > On Thu, 24 Feb 2022, Sudeep Holla wrote:
> > > > > On Tue, Feb 22, 2022 at 05:15:49PM +0000, Oleksii Moisieiev wrote:
> > > > > > Hi Sudeep,
> > > > > > 
> > > > > > On Tue, Feb 22, 2022 at 04:06:37PM +0000, Sudeep Holla wrote:
> > > > > > > Hi Oleksii,
> > > > > > > 
> > > > > > > My initial feedback on this. And thanks Cristian for making it so easy as
> > > > > > > you have covered most of the things in depth(which I might have not done
> > > > > > > myself that well)
> > > > > > > 
> > > > > > > On Tue, Feb 22, 2022 at 11:00:03AM +0000, Cristian Marussi wrote:
> > > > > > > > On Mon, Feb 21, 2022 at 05:26:46PM +0000, Oleksii Moisieiev wrote:
> > > > > > > > > Introducing new parameter called scmi_devid to the device-tree bindings.
> > > > > > > > > This parameter should be set for the device nodes, which has
> > > > > > > > > clocks/power-domains/resets working through SCMI.
> > > > > > > 
> > > > > > > I prefer you had given more details on your usage model here instead of
> > > > > > > pointing to the other Xen thread as it helps for someone without much
> > > > > > > background on Xen or your use-case to review this.
> > > > > > > 
> > > > > > Let me describe the process in few words:
> > > > > > We implemented a new feature, called SCI-mediator in Xen.
> > > > > > The proposed implementation allows Guests to communicate with the Firmware using SCMI
> > > > > > protocol with SMC as a transport. Other implementation are also
> > > > > > possible, such as SCMI-Mailbox, SCPI-mailbox etc.
> > > > > > 
> > > > > > In this feature Xen is the Trusted Agent, which receives the following
> > > > > > information in Xen device-tree:
> > > > > > 1) All channels should be described, each channel defined as
> > > > > > arm,scmi-shmem node;
> > > > > > 2) Scmi node arm,scmi-smc with protocols description;
> > > > > 
> > > > > Sounds good so far.
> > > > > 
> > > > > > 3) scmi-devid should be set in nodes, which works through SCMI.
> > > > > >
> > > > > 
> > > > > Why is this needed for Guest OS, you need not populate this if Guest OS
> > > > > is not required to use it, right ? If it is needed just by Xen hypervisor,
> > > > > lets talk about that and why it is bad idea to mix that with general
> > > > > SCMI bindings.
> > > > 
> > > > I'll try to help Oleksii by answering here, I hope I am not off the mark
> > > > :-)
> > > > 
> > > > I think Sudeep is right, scmi-devid is not needed by the guest OS.
> > > > 
> > > > The host device tree is a more interesting discussion. As the host
> > > > device tree is meant to be generic and not tied to a specific version of
> > > > Linux, it should fully describe the SCMI interface available. If the
> > > > device tree is provided to a Trusted Agent, then it should also have the
> > > > scmi-devid information, right?
> > > > 
> > > > 
> > > > > > On start Xen inits itself as trusted agent and requests agent
> > > > > > configuration by using BASE_DISCOVER_AGENT message. This message is sent
> > > > > > to each configured channel to get agent_id
> > > > > > 
> > > > > > On Domain creation stage Xen will do the following steps:
> > > > > > 1) Assign channel to the Guest and map channel address to the Domain
> > > > > > address. For the Domain this address should be the same;
> > > > > > 2) Generate arm,scmi-shmem and arm,scmi-smc nodes if needed for Guest
> > > > > > device-tree (the device-tree which should be passed to the Guest);
> > > > > > 3) Process devices, which are passed through to this Guest and set
> > > > > > BASE_SET_DEVICE_PERMISSIONS for the scmi-devid, received from the
> > > > > > device-node;
> > > > > >
> > > > > 
> > > > > I am confused here. So the Xen knows which devices are assigned to each
> > > > > Guest OS but doesn't know device ID for them, but relies on the device
> > > > > tree node ?
> > > > 
> > > > Which devices go to which guest OS is a user-provided configuration. For
> > > > instance, a user can say: "assing /amba/ethernet@ff0e0000 to dom1". This
> > > > is normal and not related to SCMI: when a user configures a static
> > > > partitioning system, they decide which resources belong to which domain.
> > > > 
> > > > So Xen is told that /amba/ethernet@ff0e0000 is supposed to go to dom1.
> > > > Xen proceeds to map memory and interrupts corresponding to
> > > > /amba/ethernet@ff0e0000 to dom1. So far so good. What about SCMI?
> > > > 
> > > > In Oleksii's design, Xen is going to assign one of the available SCMI
> > > > channels to dom1 and restrict its permission to only
> > > > /amba/ethernet@ff0e0000. To do that, Xen needs to know the scmi-devid of
> > > > /amba/ethernet@ff0e0000. As far as I can tell there is nothing
> > > > Xen-specific in this activitity, that's why I asked Oleksii to reach out
> > > > to the upstream device tree community to improve the generic bindings
> > > > for everyone's benefits.
> > > 
> > > Let's leave Linux and Xen aside for the moment. What are other possible
> > > Trusted Agents? (Maybe TF-A?) How do they get the scmi-devid? It looks
> > > like it was supposed to come from device tree but nobody got around to
> > > adding it to the binding because it is not used by Linux?
> > 
> > I do agree we need this info and probably device tree is the way. But what
> > I disagree here is that it needs to part of existing SCMI bindings which are
> > for the SCMI users only and not for one that may provide the interface(SCMI
> > platform/server/arbitrator/passthrough/..whatever). You can have bindings for
> > them as part of system device tree initiative and can be merged back to Linux
> > if that happens. Or we may even take the whole devicetree bindings out of
> > the Linux one day (when all the stars are aligned :) )
> 
> I would love to hear Rob's opinion on what I am about to write next on
> the topic of whether the binding should be under linux.git.
> 
> I am not sure if the policy is that only device tree bindings actively
> used by Linux are present under
> linux.git/Documentation/devicetree/bindings/. There are a tons of other
> projects using device tree and without a central point for keeping these
> bindings the specification will shatter. Given that Linux prefers to
> keep the bindings under linux.git, then the logic conclusion is that
> linux.git/Documentation/devicetree/bindings/ should also hold bindings
> not actively used by Linux right at the moment. Especially bindings that
> could be used by Linux in the future. Otherwise we risk a new binding
> being used by U-boot, Xen, Zephyr and others then Linux introduces an
> incompatible version of it. Nobody would win in that situation.

There's 0 requirement that Linux use something to be hosted in 
linux.git/Documentation/devicetree/bindings/.

If you are not comfortable with that still, submit it to dt-schema. Have 
had several u-boot properties land there recently. The only thing I 
don't want there is any class of bindings where there's 10s to 100s of 
them (e.g. device specific bindings).

The advantage with the former is you get a wider audience reviewing it 
(or more people ignoring it).

> > > After all, we are currently using in Xen a property called
> > > "linux,pci-domain". We might as well have Linux in the future use a
> > > property called "xen,scmi-devid" to even things out :-)

FYI, I tend to reject 'linux,*' properties. They have a tendency to be 
either a poor design or expand to other environments. The same applies 
all $os,* properties, but I get asked to review fewer of those. 

Rob