Re: [PATCH v17 09/10] arm_ffa: efi: introduce FF-A MM communication

Tom Rini
2023-07-28T13:54:28Z

[-- Attachment #1: Type: text/plain, Size: 3835 bytes --]

On Fri, Jul 28, 2023 at 02:00:25PM +0300, Ilias Apalodimas wrote:
> Hi Tom
>
> On Thu, 27 Jul 2023 at 19:43, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jul 27, 2023 at 05:07:11PM +0100, Abdellatif El Khlifi wrote:
> >
> > > Add MM communication support using FF-A transport
> > >
> > > This feature allows accessing MM partitions services through
> > > EFI MM communication protocol. MM partitions such as StandAlonneMM
> > > or smm-gateway secure partitions which reside in secure world.
> > >
> > > An MM shared buffer and a door bell event are used to exchange
> > > the data.
> > >
> > > The data is used by EFI services such as GetVariable()/SetVariable()
> > > and copied from the communication buffer to the MM shared buffer.
> > >
> > > The secure partition is notified about availability of data in the
> > > MM shared buffer by an FF-A message (door bell).
> > >
> > > On such event, MM SP can read the data and updates the MM shared
> > > buffer with the response data.
> > >
> > > The response data is copied back to the communication buffer and
> > > consumed by the EFI subsystem.
> > >
> > > MM communication protocol supports FF-A 64-bit direct messaging.
> > >
> > > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > > Tested-by: Gowtham Suresh Kumar <gowtham.sureshkumar@arm.com>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > Cc: Tom Rini <trini@konsulko.com>
> > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > >
> > > ---
> > >
> > > Changelog:
> > > ===============
> > >
> > > v17:
> > >
> > > * show a debug message rather than an error when FF-A is not detected
> > [snip]
> > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > index c5835e6ef6..8fbadb9201 100644
> > > --- a/lib/efi_loader/Kconfig
> > > +++ b/lib/efi_loader/Kconfig
> > > @@ -55,13 +55,53 @@ config EFI_VARIABLE_FILE_STORE
> > > stored as file /ubootefi.var on the EFI system partition.
> > >
> > > config EFI_MM_COMM_TEE
> > > - bool "UEFI variables storage service via OP-TEE"
> > > - depends on OPTEE
> > > + bool "UEFI variables storage service via the trusted world"
> > > + depends on OPTEE && ARM_FFA_TRANSPORT
> >
> > You didn't get my changes in here however. If you can do EFI_MM_COMM_TEE
> > without ARM_FFA_TRANSPORT (as lx2160ardb_tfa_stmm_defconfig does) then
> > you don't make this option depend on . If FF-A is only
> > for use here, you make FF-A depend on this, and the FF-A specific
> > variable depend on ARM_FFA_TRANSPORT.
>
> Abdellatif hinted at what's going on here. When I added this Kconfig
> option to lx2160 FF-A wasn't implemented yet.

The defconfig has existed since May 2020, which is when you added
EFI_MM_COMM_TEE itself too. So I think it's that no one did the check I
did until now and saw this series was disabling what was on the other
platform.

> Since FF-A isn't a new
> communication mechanism but builds upon the existing SMCs to build an
> easier API, I asked Abdellatif to hide this complexity.
> We had two options, either make Kconfig options for either FF-A or the
> traditional SMCs and remove the dependencies, or piggyback on FF-As
> discovery mechanism and make the choice at runtime. The latter has a
> small impact on code size, but imho makes developers' life a lot
> easier.

I'm not sure how much you can do a run-time option here since you're
setting a bunch of default values for FF-A to 0 in Kconfig. If we're
supposed to be able to get them at run time, we shouldn't need a Kconfig
option at all. I'm also not sure how valid a use case it is where we
won't know at build time what the rest of the firmware stack supports
here.

--
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]