Messages in this thread | | | Date | Tue, 10 Oct 2023 09:29:39 +0100 | From | Cristian Marussi <> | Subject | Re: [RFC] firmware: arm_scmi: clock: add fixed clock attribute support |
| |
On Tue, Oct 10, 2023 at 08:08:01AM +0000, Peng Fan wrote: > > Subject: Re: [RFC] firmware: arm_scmi: clock: add fixed clock attribute > > support > > > > On Tue, Oct 10, 2023 at 10:29:11AM +0800, Peng Fan (OSS) wrote: > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > There are clocks: > > > system critical, not allow linux to disable, change rate allow linux > > > to get rate, because some periphals will use the frequency to > > > configure periphals. > > > > > > So introduce an attribute to indicated FIXED clock > > > > > > > Hi, > > > > (CCed souvik.chakravarty@arm.com) > > > > so AFAIU here you are describing a clock that really is NOT fixed in general, it > > is just that the Linux SCMI Agent cannot touch it, but other SCMI agents on > > the system CAN change it and so, on one side, you keep the ability for the > > Linux agent to read back the current rate with > > recalc_rate() and remove all the Clk frameworks callbacks needed to modify > > its state, am I right ? > > Right. > > > > > In this scenario, it is really the SCMI platform fw (server) that has to > > implement the checks and simply DENY the requests coming from an agent > > that is not supposed to touch that clock, while allowing the current rate to be > > retrieved. > > Linux will try to enable, get rate, runtime disable the clock. > But the server does not allow enable/disable the clock, so the driver probe > will fail. >
Which driver probe ? I have just double checked and when clk-scmi driver is loaded there are a bunch of SCMI queries to the server BUT no *_SET command is issued during the clk-scmi probe; indeed JUNO had never had any issue despite having access to a bunch of CLKs which are visibile BUT not modifiable from Linux.
> The SCMI server could bypass enable/disable and only allow get rate, > But this introduces heavy RPC, so just wanna whether it is ok to register > fixed clock and avoid RPC. > > > > > JUNO/SCP is an example of how the CPUs clocks are visible to Linux BUT > > cannot be touched directly via Clock protocol by Linux since in the SCMI > > world you are supposed to use the Perf protocol instead to change the OPPs > > when you want to modify the performance level of the runnning CPU. > > > > This kind of server-side permissions checks, meant to filter access to resources > > based on the requesting agent, are part of the SCMI declared aim to push the > > responsibility of such controls out of the kernel into the platform fw in order > > to avoid attacks like CLOCK_SCREW by letting the SCMI firmware be the one > > and only final arbiter on the requests coming from the agents; you can ask > > teh server whatever you like as an agent but your request can be DENIED or > > silently ignored (in case of shared resources) at the will of the platform which > > has the final say and it is implemented in a physically distinct code-base. > > > > It seems to me that this patch and the possible associated SCMI specification > > change would give back the control to the Linux agent and could allow the > > implementation of an SCMI Server that does NOT perform any of these > > permission checks. > > > > So, IMO, while this change, on one side, could be certainly useful by removing > > a bunch of unused/uneeded callbacks from the CLK SCMI driver when a fixed > > clock is identified, it could open the door to a bad implementation like the > > one mentioned above which does NOT perform any agent-based permission > > check. > > Thanks for detailed information, let me check whether our SCMI firmware > could do more on the permission side. But if RPC could be removed, > it could save some time. >
Avoiding to issue SCMI requests destined to fail would be certainly good, even though it could lead to just quietly implementing a server with no-checks at all, but why these unneeded calls happen in first place ?
I have never observed anything like that in my setups.
Thanks, Cristian
| |