Re: [PATCH 5/9] soc: apple: Add RTKit IPC library

[Sven Peter]

On Wed, Mar 23, 2022, at 12:19, Krzysztof Kozlowski wrote:
> On 21/03/2022 17:50, Sven Peter wrote:
>> Apple SoCs such as the M1 come with multiple embedded co-processors
>> running proprietary firmware. Communication with those is established
>> over a simple mailbox using the RTKit IPC protocol.
>> 
>> Signed-off-by: Sven Peter <sven@xxxxxxxxxxxxx>
>> ---
>>  drivers/soc/apple/Kconfig          |  13 +
>>  drivers/soc/apple/Makefile         |   3 +
>>  drivers/soc/apple/rtkit-crashlog.c | 147 +++++
>>  drivers/soc/apple/rtkit-internal.h |  76 +++
>>  drivers/soc/apple/rtkit.c          | 842 +++++++++++++++++++++++++++++
>>  include/linux/soc/apple/rtkit.h    | 203 +++++++
>>  6 files changed, 1284 insertions(+)
>
> Isn't this some implementation of a mailbox? If so, it should be in
> drivers/mailbox. Please don't put all stuff in soc/apple, that's not how
> Linux is organized. To drivers/soc usually we put drivers which do not
> fit regular subsystems.
>

I put this into soc/apple because I don't think it fits within the mailbox
framework very well.
(It actually uses the mailbox framework for the actual communication
with the hardware with a driver that's already upstream.)

Essentially, the mailbox subsystem provides a common API to send and
receive messages over indepedent hardware channels and devicetree bindings
to describe the relationship between those channels and other drivers.

One of the features that doesn't really fit is that we need to be able
to start, shutdown and re-start these co-processors. The NVMe driver
actually doesn't need to send/receive any messages except those required
to setup the common syslog/crashlog/etc. interfaces.
The mailbox framework would have to be extended to support these specific
use cases.

Another thing that doesn't fit is the memory management: These co-processors
sometimes need shared memory buffers to e.g. send syslog messages.
They always request these buffers with an IPC message but then there are
different possibilities:

	- For some processor the DMA API can just be used and an IOVA must be
	  sent back. For NVMe these buffers must additionally be allowed in this
	  SART address filter.
	- At least one other processor (SMC) does not request such buffers but
	  instead just sends a pointer into MMIO space and the buffer must be
	  accessed using readl/writel. This MMIO memory region is used for
	  both the common buffers (syslog etc.) and for the actual shared buffers
	  used for communication, such that the resource would have to be shared
	  across drivers.
	- And yet another coprocessor (for the display controller) requests some
	  buffers with an already existing IOVA that than need to be mapped
	  specifically inside the IOMMU.

Each of these co-processors also provides a single function and most
of them don't even have different endpoints. And even those that do (DCP) will
just become a single driver since all those endpoints are very much related.

While it's not impossible to do all that by extending and forcing this into the
mailbox framework at lest I think that it doesn't fit very well and would just
create unneccesarry impedance.


Best,


Sven