Message-ID: <20220222171549.GA2194063@EPUAKYIW015D>
Date:   Tue, 22 Feb 2022 17:15:49 +0000
From:   Oleksii Moisieiev <Oleksii_Moisieiev@...m.com>
To:     Sudeep Holla <sudeep.holla@....com>
CC:     Cristian Marussi <cristian.marussi@....com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Souvik Chakravarty <Souvik.Chakravarty@....com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 0/1] dt-bindings: arm: Add scmi_devid paramter for

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@...90000
> > > {
> > >     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.

Powered by blists - more mailing lists