[PATCH v12 09/10] arm_ffa: efi: introduce FF-A MM communication
Ilias Apalodimas
ilias.apalodimas at linaro.org
Fri May 19 16:07:43 CEST 2023
On Fri, May 19, 2023 at 02:36:55PM +0100, Abdellatif El Khlifi wrote:
> Hi Ilias,
>
> > Hi Abdellatif
> >
> > I still have some concerns on this
> >
> > In the past [0] I asking why this needs to be a Kconfig option. Since FF-A
> > is a mechanism we can use to discover SPs, in theory we dont need the
> > ifdefery. We might need something if the code difference grows too much
> > but I think we are fine for now.
>
> Is my understanding correct you prefer that CONFIG_EFI_MM_COMM_TEE implies
> CONFIG_ARM_FFA_TRANSPORT ? So, no ifdefs anymore in efi_variable_tee.c.
I wonder what it takes to autodiscover that and get rid of all ifdefs
(OPTEE and FF-A). In theory you could probe FF-A and if it's not there
switch to the smc calling no?
>
> >
> > On top of that I am nissing how was this tested? did you test the current
> > SMC to optee and the stmm for the RPMB backend?
>
> We use Corstone-1000 platform for testing FF-A.
> U-Boot communicates with smm-gateway (leight version of stmm) using FF-A SMC ABIs.
> In the secure world we use TF-A, Optee OS and Trusted Services providing smm-gateway SP.
> In U-Boot we don't use the Optee driver because we use FF-A communication only.
> I hope this clears all doubts.
Ok, but you will still need to load op-tee from BL2.
My problem here is that this is a bit confusing from an architectural point
of view and people need to be aware of what config options to choose, but if
my understanding is correct we can automate that.
Thanks
/Ilias
>
> Cheers
> Abdellatif
>
> >
> > > +++ b/include/mm_communication.h
> > > @@ -6,6 +6,9 @@
> > > * Copyright (c) 2017, Intel Corporation. All rights reserved.
> > > * Copyright (C) 2020 Linaro Ltd. <sughosh.ganu at linaro.org>
> > > * Copyright (C) 2020 Linaro Ltd. <ilias.apalodimas at linaro.org>
> > > + * Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office at arm.com>
> > > + * Authors:
> > > + * Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> > > */
> > >
> > > #ifndef _MM_COMMUNICATION_H_
> > > @@ -13,6 +16,9 @@
> > >
> > > #include <part_efi.h>
> > >
> > > +/* MM service UUID string (big-endian format). This UUID is common across all MM SPs */
> > > +#define MM_SP_UUID "33d532ed-e699-0942-c09c-a798d9cd722d"
> > > +
> > > /*
> > > * Interface to the pseudo Trusted Application (TA), which provides a
> > > * communication channel with the Standalone MM (Management Mode)
> > > @@ -248,4 +254,11 @@ struct smm_variable_var_check_property {
> > > u16 name[];
> > > };
> > >
> > > +/* supported MM transports */
> > > +enum mm_comms_select {
> > > + MM_COMMS_UNDEFINED,
> > > + MM_COMMS_FFA,
> > > + MM_COMMS_OPTEE
> > > +};
> > > +
> > > #endif /* _MM_COMMUNICATION_H_ */
> > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > index c5835e6ef6..08a6b84101 100644
> > > --- a/lib/efi_loader/Kconfig
> > > +++ b/lib/efi_loader/Kconfig
> > > @@ -55,13 +55,23 @@ 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
> > > help
> > > + Allowing access to the MM SP services (SPs such as StandAlonneMM, smm-gateway).
> > > + When using the u-boot OP-TEE driver, StandAlonneMM is supported.
> > > + When using the u-boot FF-A driver any MM SP is supported.
> > > +
> > > If OP-TEE is present and running StandAloneMM, dispatch all UEFI
> > > variable related operations to that. The application will verify,
> > > authenticate and store the variables on an RPMB.
> > >
> > > + When ARM_FFA_TRANSPORT is used, dispatch all UEFI variable related
> > > + operations to the MM SP running in the secure world.
> > > + A door bell mechanism is used to notify the SP when there is data in the shared
> > > + MM buffer. The data is copied by u-boot to the shared buffer before issuing
> > > + the door bell event.
> > > +
> > > config EFI_VARIABLE_NO_STORE
> > > bool "Don't persist non-volatile UEFI variables"
> > > help
> > > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> > > index dfef18435d..2effb705d7 100644
> > > --- a/lib/efi_loader/efi_variable_tee.c
> > > +++ b/lib/efi_loader/efi_variable_tee.c
> > > @@ -4,9 +4,14 @@
> > > *
> > > * Copyright (C) 2019 Linaro Ltd. <sughosh.ganu at linaro.org>
> > > * Copyright (C) 2019 Linaro Ltd. <ilias.apalodimas at linaro.org>
> > > + * Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office at arm.com>
> > > + *
> > > + * Authors:
> > > + * Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> > > */
> > >
> > > #include <common.h>
> > > +#include <dm.h>
> > > #include <efi.h>
> > > #include <efi_api.h>
> > > #include <efi_loader.h>
> > > @@ -15,6 +20,36 @@
> > > #include <malloc.h>
> > > #include <mm_communication.h>
> > >
> > > +#if IS_ENABLED(CONFIG_ARM_FFA_TRANSPORT)
> > > +
> > > +#include <arm_ffa.h>
> > > +#include <cpu_func.h>
> > > +#include <mapmem.h>
> > > +
> > > +#ifndef FFA_SHARED_MM_BUFFER_SIZE
> > > +#error "FFA_SHARED_MM_BUFFER_SIZE must be defined in include/configs/<board>.h"
> > > +#define FFA_SHARED_MM_BUFFER_SIZE 0
> > > +#endif
> > > +
> > > +#ifndef FFA_SHARED_MM_BUFFER_OFFSET
> > > +#error "FFA_SHARED_MM_BUFFER_OFFSET must be defined in include/configs/<board>.h"
> > > +#define FFA_SHARED_MM_BUFFER_OFFSET 0
> > > +#endif
> > > +
> > > +#ifndef FFA_SHARED_MM_BUFFER_ADDR
> > > +#error "FFA_SHARED_MM_BUFFER_ADDR must be defined in include/configs/<board>.h"
> > > +#define FFA_SHARED_MM_BUFFER_ADDR 0
> > > +#endif
> > > +
> > > +/* MM return codes */
> > > +#define MM_SUCCESS (0)
> > > +
> > > +static const char *mm_sp_svc_uuid = MM_SP_UUID;
> > > +
> > > +static u16 mm_sp_id;
> > > +
> > > +#endif
> > > +
> > > extern struct efi_var_file __efi_runtime_data *efi_var_buf;
> > > static efi_uintn_t max_buffer_size; /* comm + var + func + data */
> > > static efi_uintn_t max_payload_size; /* func + data */
> > > @@ -24,6 +59,7 @@ struct mm_connection {
> > > u32 session;
> > > };
> > >
> > > +#if (IS_ENABLED(CONFIG_OPTEE))
> >
> > > /**
> > > * get_connection() - Retrieve OP-TEE session for a specific UUID.
> > > *
> >
> > [...]
> >
> > Isn't this problematic? At the moment there's not > 8.4 hardware out
> > there. As a result the SPMC *is* implemented in OP-TEE. So how do we
> > expect FF-A to work?
> >
> > [...]
> >
> > [0] https://lore.kernel.org/u-boot/Y3NV991bKRgas1zZ@hera/
> >
> > Thanks
> > /Ilias
More information about the U-Boot
mailing list