linux-kernel - Re: [PATCH] firmware: arm_scmi: Make scmi core independent of transport type

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]

Date:   Tue, 10 Dec 2019 18:46:33 +0000
From:   Sudeep Holla <sudeep.holla@....com>
To:     Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Cristian Marussi <cristian.marussi@....com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        Jassi Brar <jassisinghbrar@...il.com>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Sudeep Holla <sudeep.holla@....com>
Subject: Re: [PATCH] firmware: arm_scmi: Make scmi core independent of
 transport type

On Tue, Dec 10, 2019 at 11:04:48AM +0530, Viresh Kumar wrote:
> On 09-12-19, 18:13, Cristian Marussi wrote:
> > On 29/11/2019 09:31, Viresh Kumar wrote:

[...]

> > >  	desc = of_device_get_match_data(dev);
> > >  	if (!desc)
> > >  		return -EINVAL;
> >
> > This scmi_desc struct descriptor is retrieved from  of_match_table .data and points to
> > the driver-provided scmi_generic_desc
> >
> > static const struct scmi_desc scmi_generic_desc = {
> >         .max_rx_timeout_ms = 30,        /* We may increase this if required */
> >         .max_msg = 20,          /* Limited by MBOX_TX_QUEUE_LEN */
> >         .max_msg_size = 128,
> > };
> >
> > Is not this kind of information possibly (maybe partially) related to the selected
> > transport, and as such it should be also provided dynamically by the chosen transport
> > layer at probe time, like the transport_ops, instead of being hard-coded in
> > this driver ?
>
> I had my doubts about this thing and I missed checking it out.
>
> @Sudeep: Is this information completely mailbox specific ? Should I move it to
> mailbox.c here ?
>

May be to some/small extent.

1. max_rx_timeout_ms is firmware dependent, maximum time it expects to
   complete a synchronous request or acknowledge async request(worst case value).
2. max_msg_size is maximum size of the buffer we need to allocate, mostly
   based on the specification and we don't have any more that 0x80. But
   the custom/vendor specific protocols may wary and hence I thought of
   keeping it configurable for platforms.
3. max_msg is the maximum number of messages the transport can support.
   This is currently based on the mailbox layer. For SMC/HVC we can have
   upto nr_cpus, something different for spci/optee. We must be able to
   make it transport independent if required.

   This is mainly used to pre-allocate number of (tx/rx) buffers.

--
Regards,
Sudeep