linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Achin Gupta <achin.gupta@arm.com>
To: Rob Herring <robh@kernel.org>
Cc: Sumit Garg <sumit.garg@linaro.org>,
	Sudeep Holla <sudeep.holla@arm.com>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Devicetree List <devicetree@vger.kernel.org>,
	Trilok Soni <tsoni@codeaurora.org>,
	arve@android.com, Andrew Walbran <qwandor@google.com>,
	David Hartley <dhh@qti.qualcomm.com>,
	Jens Wiklander <jens.wiklander@linaro.org>,
	Arunachalam Ganapathy <arunachalam.ganapathy@arm.com>,
	Marc Bonnici <marc.bonnici@arm.com>,
	nd@arm.com
Subject: Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
Date: Tue, 6 Apr 2021 16:08:21 +0100	[thread overview]
Message-ID: <YGx5Zfj9B7cO8Uso@C02ZJ1BRLVDN> (raw)
In-Reply-To: <20210330150312.GA284502@robh.at.kernel.org>

Hi Rob,

On Tue, Mar 30, 2021 at 10:03:12AM -0500, Rob Herring wrote:
> On Fri, Mar 26, 2021 at 05:26:52PM +0530, Sumit Garg wrote:
> > On Fri, 26 Mar 2021 at 16:25, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On Fri, Mar 26, 2021 at 10:35:23AM +0530, Sumit Garg wrote:
> > > > Hi Sudeep,
> > > >
> > > > Apologies for catching up late on this patch-set.
> > > >
> > > > On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > >
> > > > > Since the FF-A v1.0 specification doesn't list the UUID of all the
> > > > > partitions in the discovery API, we need to specify the UUID of the
> > > > > partitions that need to be accessed by drivers within the kernel.
> > > > >
> > > >
> > > > Wouldn't we be able to implement auto-discovery of ffa partitions? I
> > > > think enumeration of ffa partitions on FFA bus should be quite similar
> > > > to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
> > > > these redundant DT entries for every ffa partition which IMHO would
> > > > bloat up device trees for every platform.
> > > >
> > >
> > > Any suggestions on how to ? Clearly spec doesn't have that provision, I
> > > had raised this point in the past. Jens has similar concern and he did
> > > ask the same[1]. As I replied to him in that thread[2].
> > >
> > > I am open to suggestion on how to auto-discover, currently as I see spec
> > > doesn't support it.
> > >
> >
> > Thanks for sharing links to prior discussions and I can see that
> > currently spec doesn't support it. But from an implementation
> > perspective, I can't find any reason that we can't support
> > auto-discover. Have a look at below proposed simple FFA ABI:
> >
> > FFA_LIST_PARTITIONS
> >
> > - No input params.
> > - Returns an array of secure partition UUIDs to which this non-secure
> > virtual/physical FF-A instance is allowed to communicate with.
> >
> > I think with auto-discovery, one of the major benefits is that if the
> > OEM is using a common platform to cater to multiple use-cases which
> > rely on different secure partitions then OEM doesn't have to bother
> > about shipping separate DTs.
>
> +1
>
> DT should not be the dumping ground for everything forgotten to be made
> discoverable. There's not much we can do about h/w, but firmware is
> different and can be changed. In other threads (e.g. PCI config space
> SMC calls), fixing in firmware is the proposed answer. So let's do that
> here.
>
> Maybe if there are implementations shipping and changing is too late
> (yet not too late to use a new binding), then I'd feel differently. But
> being in a spec or not alone is not enough reason alone to accept this.
> It's obvious the spec did not have wide enough review.

The v1.0 specification (released almost one year back) was reviewed by a few
Trusted OS vendors amongst other partners. Nobody provided feedback that a
FFA_LIST_PARTITIONS like discovery mechanism is a gating factor. Hindsight is a
wonderful thing! The ability of discover UUIDs of partitions would indeed be a
very useful thing and this will be added to the next version of the
specification. I think it is worth providing some background in any case.

The usage model envisioned was that a partition would export services that would
be identified by a UUID. An FF-A client that wants to use these services would
query their presence by specifying this UUID. The FF-A discovery ABI returns a
16-bit ID of the partition that implements the services. This ID is more
user-friendly and space efficient than a 16 byte UUID for register and shared
memory based IPC between the client and the server.

For example, OP-TEE would have a UUID to represent the TEE service. The OP-TEE
driver would ask the FF-A driver if there is a partition with a UUID that
provides OP-TEE's TEE service.

The assumption was that a client must identify the service(s) it wants to access
to be able to use FF-A ABIs to talk to the partition that implements them. This
was more important than a discovery mechanism where the client says: I don't
know what I want but please list all the UUIDs of all partitions in the
system. I will then see if I can find anything that I like.

In this model, it was not envisioned that the FF-A driver would carry the UUIDs
of services that its clients could want to access. The client would know the
UUID and the FF-A driver would return the properties of the partition and its ID
to enable further communication.

We did envision the need for the FF-A driver to know the IDs of all the
partitions in the system and their properties e.g. number of vCPUs etc. The
FFA_PARTITION_INFO_GET ABI when invoked with a NULL UUID returns this
information. It is a bit like "grep *.*".

The implementation choice to have an FF-A bus with partitions as devices on it
was made after the specification was released. This was not obvious at all at
the time of writing the specification. Clearly, in this model the FF-A driver
needs to the know the UUIDs. But this does not change the fact that the clients
of the FF-A driver still need to know the UUID of the services they are
interested in.

For v1.0 we are left with no choice but to carry them in the DT. This will be
addressed in v1.1. I see this as a case where implementation choices drive the
evolution of a specification.

I hope you can understand that it was a bit difficult to foresee this usage
model for v1.0 when specification and implementation are temporally separated.

I also think that this issue is different from the cluster **** the PCIe ECAM
SMC discussion is.

I do hope this helps clarify the situation to some extent. Please let me know if
you need any more info or if I have missed anything. Also, apologies for not
getting back sooner as I was on holiday.

cheers,
Achin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-04-06 15:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-25 14:32 [PATCH v5 0/7] firmware: Add initial support for Arm FF-A Sudeep Holla
2021-03-25 14:32 ` [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding Sudeep Holla
2021-03-26  5:05   ` Sumit Garg
2021-03-26 10:55     ` Sudeep Holla
2021-03-26 11:56       ` Sumit Garg
2021-03-30 15:03         ` Rob Herring
2021-04-06 15:08           ` Achin Gupta [this message]
2021-03-25 14:32 ` [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers Sudeep Holla
2021-03-25 14:41   ` Mark Rutland
2021-03-26 12:18     ` Mark Rutland
2021-03-26 12:51       ` Sudeep Holla
2021-04-01 15:50         ` Michael Kelley
2021-03-25 14:32 ` [PATCH v5 3/7] firmware: arm_ffa: Add initial FFA bus support for device enumeration Sudeep Holla
2021-03-25 14:32 ` [PATCH v5 4/7] firmware: arm_ffa: Add initial Arm FFA driver support Sudeep Holla
2021-03-25 14:32 ` [PATCH v5 5/7] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver Sudeep Holla
2021-03-25 14:32 ` [PATCH v5 6/7] firmware: arm_ffa: Setup in-kernel users of FFA partitions Sudeep Holla
2021-04-09 15:45   ` Marc Bonnici
2021-03-25 14:32 ` [PATCH v5 7/7] firmware: arm_ffa: Add support for MEM_* interfaces Sudeep Holla

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YGx5Zfj9B7cO8Uso@C02ZJ1BRLVDN \
    --to=achin.gupta@arm.com \
    --cc=arunachalam.ganapathy@arm.com \
    --cc=arve@android.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dhh@qti.qualcomm.com \
    --cc=jens.wiklander@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marc.bonnici@arm.com \
    --cc=nd@arm.com \
    --cc=qwandor@google.com \
    --cc=robh@kernel.org \
    --cc=sudeep.holla@arm.com \
    --cc=sumit.garg@linaro.org \
    --cc=tsoni@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).