Re: [PATCH v8 03/10] arm_ffa: introduce Arm FF-A low-level driver

Simon Glass
2023-01-19T18:04:36Z

Hi Tom,

On Thu, 19 Jan 2023 at 10:24, Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jan 19, 2023 at 10:21:07AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 19 Jan 2023 at 09:57, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Jan 19, 2023 at 09:54:29AM -0700, Simon Glass wrote:
> > > > Hi Sudeep,
> > > >
> > > > On Thu, 19 Jan 2023 at 09:46, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > >
> > > > > Hi Abdellatif,
> > > > >
> > > > > On Thu, Jan 19, 2023 at 04:31:57PM +0000, Abdellatif El Khlifi wrote:
> > > > > >
> > > > > > Hi Simon, Tom,
> > > > > >
> > > > > > The FF-A transport is a SW bus and is not associated to any HW peripheral or
> > > > > > undiscoverable base address.
> > > > > >
> > > > > > There is only 1 way of discovering the FF-A bus and it's through the FF-A SW
> > > > > > interfaces. The FF-A spec [1] describes this in details.
> > > > > >
> > > > > > Discovering means gathering information about the FF-A framework such as:
> > > > > > the FF-A version, supported features, secure partitions number and attributes.
> > > > > >
> > > > > > Please refer to the following paragraphs for more details: [2], [3], [4], [5]
> > > > > >
> > > > > > The core driver provided by this patchset implements the Setup and discovery interfaces
> > > > > > in addition to direct messaging.
> > > > > >
> > > > > > The driver provides ffa_bus_discover() API that allows to discover the FF-A bus
> > > > > > as described by the spec and in the FF-A driver readme [6].
> > > > > >
> > > > > > We expect and highly recommend FF-A users to always discover the FF-A bus using ffa_bus_discover() API.
> > > > > >
> > > > >
> > > > > Thanks for the details. But IIRC this discussion is not about the FF-A bus
> > > > > and device(partitions) discovery, but the support for FF-A itself. The
> > > > > discussion is about where to have a device node to represent the existence of
> > > > > FF-A support on a platform. If we are talking about individual partitions
> > > > > (devices) in the device tree, then that is pure stupidity as it goes out
> > > > > of since with the firmware the moment a partition is added or removed in
> > > > > the firmware.
> > > > >
> > > > > IIUC, the whole discussion was around whether to use FFA_VERSION as the
> > > > > discovery mechanism for existence of FF-A support on a platform or you
> > > > > have a device node to specify the same.
> > > >
> > > > No, with respect, that is not quite the situation here.
> > > >
> > > > >
> > > > > Just to be clear, even if it is decided to add a device node, the
> > > > > FFA_VERSION must be used to detect the presence of FF-A support and
> > > > > return error otherwise. DT node presence is just to satisfy the design
> > > > > and must be treated as no auto-confirmation for the presence of FF-A
> > > > > support. We are just arguing the device node presence is just redundant,
> > > > > but as mentioned before it is up to U-Boot community to make a call on
> > > > > what is best.
> > > >
> > > > U-Boot driver model design already supports this. You can have a
> > > > device that binds (from DT) but will not probe because it is not
> > > > present / wrong version. Perhaps this was missed in the conversion to
> > > > Linux:
> > > >
> > > > https://u-boot.readthedocs.io/en/latest/develop/driver-model/design.html#driver-lifecycle
> > > >
> > > > So there is nothing clever needed here at all and anything you do just
> > > > adds confusion and bad precedent.
> > >
> > > But it's also true that at run-time, within U-Boot, we can modify the
> > > device tree we have, with live tree yes? So, the whole series in
> > > question here can be done without modifying the base DT and getting in
> > > to the further discussions that doing so entails. The assertion is that
> > > the software discoverable bus here is sufficient to not need DT, so, OK,
> > > lets go.
> >
> > One of the reasons that I find this all so frustrating is that it is
> > circular logic:
> >
> > 1. Device-tree bindings are controlled by Linux; U-Boot cannot upstream bindings
> > 2. We can only have upstreamed bindings in any device tree
> >
> > We have invented a whole u-boot.dtsi feature in U-Boot to hold
> > modifications from Linux. Board vendors have been suffering with this
> > for years.
> >
> > It is not fair and it really needs to stop. I am doing what I can to
> > upstream some basic U-Boot bindings and I hope that will work and can
> > lead to a healthier relationship here.
>
> Yes, but this is a problem outside of that scope. The argument here is
> that one does not need a device tree node to work. So lets see just how
> clean and nice the code can be without what you and I have been
> insisting would lead to the cleanest result.

Well we already have the code, right...?

The entire device tree is optional. We could just use platform data or
even C function calls to set up devices. We could call a C function
for each board which builds a device tree early in boot. Etc... As I
remember it, no one wanted to use DT and it was only Linus Torvalds'
refusal to accept another ARM pull request that made people move away
from platform data and other ad-hoc mechanisms. I'll note that U-Boot
adopted DT on ARM in 2011, the same year as Linux [1] [2]. Since then
huge strides have been made in many ways, e.g. loads of useful
bindings and Rob Herring's validation stuff.

IMO the problem here is exactly because of the point I mentioned. I
suspect the reason no one wants to add a compatible string for the
top-level FF-A device is that it will be rejected by Linux and they
don't want another case of PTSD (I am willing to take that on if it
helps). I'm sorry that we are in this situation, but it is not going
to be fixed by ignoring the problem. It is causing all sorts of
work-arounds [3], is bad for project interoperability (and therefore
the open source industry as a whole), wastes a huge amount of time in
discussion and gives device tree a bad name[4]. It really, really
needs to stop.

Can we all agree to work together on this and see device tree as a
shared technology and resource, across firmware and OS? Could this be
the last thread on this topic?

Regards,
Simon

[1] U-Boot 45ba8077f3e ("fdt: ARM: Add device tree control of U-Boot
(CONFIG_OF_CONTROL)")
[2] Linux 9eb8f6743b076b ("arm/dt: Allow CONFIG_OF on ARM")
[3] I won't attempt to list them but I could probably find >100
separate U-Boot mailing-list threads trying to work around the
inability to have U-Boot bindings upstream
[4] I suspect it is a major reason why ARM adopted ACPI on servers