Re: [RFC] firmware: arm_scmi: clock: add fixed clock attribute support

[Sudeep Holla]

On Tue, Oct 10, 2023 at 08:08:01AM +0000, Peng Fan wrote:
> > On Tue, Oct 10, 2023 at 08:49:33AM +0100, Cristian Marussi wrote:
> > 
> > (CCed souvik.chakravarty@xxxxxxx)
> > 
> > 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.
>

OK, let me try to understand this better. The SCMI platform provides all the
permissions to change to the Linux(agent) but it is just that you need to
enforce this policy of fixed rate in the kernel ? Or the SCMI platform
restricts the Linux agent to do anything other than fetch the rate ?

> > 
> > 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.

+1, that is exactly why I asked the above question.

>
> 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.
>

Interesting, if you can make the driver work with fixed clock, what can't
you make it work with failure to adjust the clock rates ?

> 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.
>

I need to study the clock layer in details, but would this happen if the
clock is present with just one fixed clock rate ?

> >
> > 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.
> >

Indeed.

> > 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.
> >

Unless I am missing something, I already replied to Peng with reasons why
I don't see the need for change in the specification to represent this case.

> > 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.

Agree.

>
> 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.

I would like to check why would clk framework ask to recalculate the clock
rate if it is fixed clock.

Or you are trying to paper over the issue that it is not a fixed clock. The
other agents can change but not Linux agent but it must always read back the
set clock rate. It is sort of read only clock. That may be an attribute you
want to obtain from the firmware and see if that can be utilised in the clk
framework. But the way you have presented it as fixed is simply wrong to me.

--
Regards,
Sudeep