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

Abdellatif El Khlifi
2023-07-31T11:46:49Z

Hi Ilias,

On Mon, Jul 31, 2023 at 12:38:16PM +0300, Ilias Apalodimas wrote:
> > > > > ...
> > > > > 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.
> >
>
> That's a fair point. FF-A in theory has APIs to discover memory.
> Abdellatif, why do we need the Kconfigs for shared memory on FF-A?

The statically carved out MM shared buffer address, size and offset cannot be discovered by FF-A ABIs.
The MM communication driver in U-Boot could allocate the buffer and share it with the MM SP but
we do not implement that support currently in either U-Boot or UEFI.

Simon suggested we use build configs to set the buffer address, size and offset since we don't want
a DT node for the MM firmware.

Kind regards
Abdellatif