linux-kernel - Re: [PATCH 1/5] firmware: arm_scmi: always initialize protocols

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201009162916.GA1105@e120937-lin>
Date:   Fri, 9 Oct 2020 17:31:13 +0100
From:   Cristian Marussi <cristian.marussi@....com>
To:     Etienne Carriere <etienne.carriere@...aro.org>
Cc:     Sudeep Holla <sudeep.holla@....com>, linux-kernel@...r.kernel.org,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Souvik Chakravarty <Souvik.Chakravarty@....com>,
        cristian.marussi@....com
Subject: Re: [PATCH 1/5] firmware: arm_scmi: always initialize protocols

Hi Etienne,

On Fri, Oct 09, 2020 at 02:31:55PM +0200, Etienne Carriere wrote:
> On Thu, 8 Oct 2020 at 21:17, Sudeep Holla <sudeep.holla@....com> wrote:
> >
> > On Thu, Oct 08, 2020 at 04:37:18PM +0200, Etienne Carriere wrote:
> > > Remove the IDR replacement that prevent initializing an SCMI protocol
> > > when it has already been initialized. This is needed when there are
> > > several SCMI agents that do implement a given SCMI protocol unless
> > > what only the related SCMI protocol communication is initialized only
> > > for first probed agent.
> > >
> >
> > Can you please elaborate on your usecase please. What do you mean by several
> > SCMI agents here. OSPM is the only agent we are interested here. What
> > other agents is this driver supposed to handle here. We allocate memory
> > in init and calling init multiple times messes up the allocated and
> > initialised structures.
> >
> > So NACK for this patch as it needs more work if we need this at all.
> >
> 
> Hello Sudeep,
> 
> Considering a device with several SCMI servers spread over several co-processor
> and possibly also in the Arm TZ secure world, each of these servers
> uses a specific
> SCMI channel. Without this change, each SCMI protocol gets initialized
> only for the
> first agent device that is probed.
> 
> My setup is also a bit specific. My device has several secure configuration
> features that can individually be enabled or not. For example, configuring
> domain X as secure makes some clocks reachable by Linux only through SCMI,
> and configuring domain Y as secure makes other clocks reachable by Linux
> only through SCMI. For flexibility, I expose domain X resources (here clocks)
> to an Linux agent whereas domain Y resources (here clocks also) are
> exposed to another agent, each agent with its specific transport/channel.
> Enabling each agent node in the Linux FDT allows to define which SCMI clocks
> get exposed and hence registered in the kernel.
> Without the change proposed here, I cannot get the clocks exposed to both
> agents when enabled as the SCMI clock protocol is initialized only for the 1st
> probbed agent device.
> 

Just a heads up that I have a series to be posted soon(ish) that is
meant to address vendor protocols support and more in general protocols
modularization by simplifying and reorganizing 'a bit' the whole
initialization process and the handle interface itself: in that context
I've got rid as a whole of the above protocol initialization code inside
device probing and moved it at the time of first usage of the protocol,
i.e. when the first SCMI driver, inside its probe, attempts to get hold
of a protocol issuing something like:

	perf_ops = handle->get_ops(handle, SCMI_PROTOCOL_PERF);

This will effectively trigger a run of the usual protocol initialization
code if the protocol was still NOT initialized (no previous get_ops) for
that SCMI instance (server): so if your SCMI driver/device gets instantiated
multiple times against multiple different servers, the needed protocol(s)
will be initialized multiple times one for each instance (handle) upon
first usage, while any other further instance of another SCMI driver
requesting the same, already initialized, protocol will find it ready to go.

Cleanup/deinitialization is delegated to a corresponding handle->put_ops(),
while the general notifications will still be available using the current
.notify_ops() interface, but those requests will be now tracked
internally against the relevant protocol in order to avoid premature
deinitialization or removal of still in-use protocols.

Thanks

Cristian

> Regards,
> Etienne
> 
> > --
> > Regards,
> > Sudeep