From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C2EC7C77B75 for ; Fri, 19 May 2023 13:37:12 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 544FD84770; Fri, 19 May 2023 15:37:10 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id F29B984770; Fri, 19 May 2023 15:37:07 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by phobos.denx.de (Postfix) with ESMTP id 7EB978620C for ; Fri, 19 May 2023 15:37:04 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=abdellatif.elkhlifi@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id B55CB1FB; Fri, 19 May 2023 06:37:48 -0700 (PDT) Received: from e130802.arm.com (e130802.arm.com [10.1.33.37]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EB61E3F59C; Fri, 19 May 2023 06:37:02 -0700 (PDT) Date: Fri, 19 May 2023 14:36:55 +0100 From: Abdellatif El Khlifi To: Ilias Apalodimas Cc: u-boot@lists.denx.de, nd@arm.com Subject: Re: [PATCH v12 09/10] arm_ffa: efi: introduce FF-A MM communication Message-ID: <20230519133655.GA94049@e130802.arm.com> References: <20230412094245.44674-1-abdellatif.elkhlifi@arm.com> <20230512121044.111574-1-abdellatif.elkhlifi@arm.com> <20230512121044.111574-10-abdellatif.elkhlifi@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean 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. > > 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. Cheers Abdellatif > > > +++ b/include/mm_communication.h > > @@ -6,6 +6,9 @@ > > * Copyright (c) 2017, Intel Corporation. All rights reserved. > > * Copyright (C) 2020 Linaro Ltd. > > * Copyright (C) 2020 Linaro Ltd. > > + * Copyright 2022-2023 Arm Limited and/or its affiliates > > + * Authors: > > + * Abdellatif El Khlifi > > */ > > > > #ifndef _MM_COMMUNICATION_H_ > > @@ -13,6 +16,9 @@ > > > > #include > > > > +/* 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. > > * Copyright (C) 2019 Linaro Ltd. > > + * Copyright 2022-2023 Arm Limited and/or its affiliates > > + * > > + * Authors: > > + * Abdellatif El Khlifi > > */ > > > > #include > > +#include > > #include > > #include > > #include > > @@ -15,6 +20,36 @@ > > #include > > #include > > > > +#if IS_ENABLED(CONFIG_ARM_FFA_TRANSPORT) > > + > > +#include > > +#include > > +#include > > + > > +#ifndef FFA_SHARED_MM_BUFFER_SIZE > > +#error "FFA_SHARED_MM_BUFFER_SIZE must be defined in include/configs/.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/.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/.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