Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 1 Jun 2022 14:39:56 -0500
From:   Rob Herring <robh@kernel.org>
To:     Oleksii Moisieiev <Oleksii_Moisieiev@epam.com>
Cc:     "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Cristian Marussi <cristian.marussi@arm.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 0/1] dt-bindings: Add device-perms property description
Message-ID: <20220601193956.GA234900-robh@kernel.org>
References: <cover.1651749565.git.oleksii_moisieiev@epam.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <cover.1651749565.git.oleksii_moisieiev@epam.com>
Precedence: bulk

On Thu, May 05, 2022 at 11:23:50AM +0000, Oleksii Moisieiev wrote:
> Introduce device-perms property which is intended to set the device
> permissions for the System Management interfaces.
> An example of this interface is SCMI (System Control and Management
> Interface) which controls clocks/power-domains/resets etc from the
> Firmware. This property sets the device_id to set the device permissions
> for the Fimware using BASE_SET_DEVICE_PERMISSIONS message (see 4.2.2.10 of [0]).

Is that an exhaustive list of controls? Seems like there would be a 
GET_DEVICE_PERMISSIONS.

> Device permissions management described in DEN 0056, Section 4.2.2.10 [0].
> Given parameter should set the device_id, needed to set device
> permissions in the Firmware.
> This property is used by trusted Agent to set permissions for the devices,
> passed-through to the non-trusted Agents. Trusted Agent will use device-perms 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].

As I said on the call discussing this, this looks very similar to other 
proposals wanting to control or check permissions on devices handled by 
some provider. While the consumer of the binding is different in various 
proposals, that doesn't really matter from a DT perspective. DT is just 
describing some type of connection between nodes. So I'm looking for 
collaboration here with folks that have made prior proposals. To put it 
another way, for a new common binding like this, I want to see more than 
one user. 

> 
> device-perms in Device-tree node example:
> usb@e6590000
> {
>     device-perms = <&scmi 19>;

Please follow typical design patterns. For example, all of these:

>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
>     power-domains = <&scmi_power 0>;

The provider is what determines the number of cells and their meaning. 
That's certainly the case here.

> };
> 
> Given example shows the configuration of the hsusb node, which is using
> scmi to contol clocks, resets and power-domains. device-perms links to
> &scmi phandle and set the permission parameter 19, which should match
> defined id for usb in the Firmware.

See, the provider is what determines the meaning.

> Current implementation defines Xen hypervisor as trusted Agent and OS
> (Linux or other) as non-trusted Agent.
> Trusted Agent will use device-perms to set the device permissions for
> the Agents. Non-trusted Agent (OS) should not have an access to the permissions
> settings, so no code to process device-perms 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 device-perms 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
> 
> ---
> Changes V1 -> V2:
>    - update parameter name, made it xen-specific
>    - add xen vendor bindings
> 
> Changes V2 -> V3:
>    - update parameter name, make it generic
>    - update parameter format, add link to controller
>    - do not include xen vendor bindings as already upstreamed
> 
> Oleksii Moisieiev (1):
>   dt-bindings: Add device-perms property description
> 
>  .../bindings/firmware/device-perms.yaml       | 43 +++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/device-perms.yaml
> 
> -- 
> 2.27.0
>