Re: [PATCH v14 11/11] arm_ffa: efi: corstone1000: enable MM communication

Tom Rini
2023-07-07T17:44:15Z

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

On Fri, Jul 07, 2023 at 11:35:05AM -0600, Simon Glass wrote:
> Hi Abdellatif,
>
> On Fri, 7 Jul 2023 at 15:44, Abdellatif El Khlifi
> <abdellatif.elkhlifi@arm.com> wrote:
> >
> > turn on EFI MM communication
> >
> > On corstone1000 platform MM communication between u-boot
> > and the secure world (Optee) is done using the FF-A bus.
> >
> > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> >
> > ---
> >
> > Changelog:
> > ===============
> >
> > v13:
> >
> > * remove FF-A config in the defconfig
> > (because it's enabled automatically by CONFIG_EFI_MM_COMM_TEE)
> >
> > v9:
> >
> > * update copyright string
> >
> > v8:
> >
> > * drop OP-TEE configs from Corstone-1000 defconfig
> >
> > v7:
> >
> > * improve the definition of FFA_SHARED_MM_BUFFER_ADDR and
> > FFA_SHARED_MM_BUFFER_OFFSET
> > * update FFA_SHARED_MM_BUFFER_ADDR value
> >
> > v6:
> >
> > * corstone-1000: enable optee driver
> > * corstone-1000: remove CONFIG_ARM_FFA_EFI_RUNTIME_MODE from the defconfig
> >
> > v4:
> >
> > * corstone-1000: turn on EFI MM communication
> >
> > configs/corstone1000_defconfig | 1 +
> > include/configs/corstone1000.h | 15 +++++++++++++--
> > 2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig
> > index a8a79fd105..b57e2322c4 100644
> > --- a/configs/corstone1000_defconfig
> > +++ b/configs/corstone1000_defconfig
> > @@ -65,3 +65,4 @@ CONFIG_EFI_CAPSULE_ON_DISK=y
> > CONFIG_EFI_IGNORE_OSINDICATIONS=y
> > CONFIG_FWU_MULTI_BANK_UPDATE=y
> > CONFIG_ERRNO_STR=y
> > +CONFIG_EFI_MM_COMM_TEE=y
> > diff --git a/include/configs/corstone1000.h b/include/configs/corstone1000.h
> > index 3347c11792..4ef1f05e40 100644
> > --- a/include/configs/corstone1000.h
> > +++ b/include/configs/corstone1000.h
> > @@ -1,9 +1,11 @@
> > /* SPDX-License-Identifier: GPL-2.0+ */
> > /*
> > - * (C) Copyright 2022 ARM Limited
> > * (C) Copyright 2022 Linaro
> > * Rui Miguel Silva <rui.silva@linaro.org>
> > - * Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > + * Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> > + *
> > + * Authors:
> > + * Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > *
> > * Configuration for Corstone1000. Parts were derived from other ARM
> > * configurations.
> > @@ -14,6 +16,15 @@
> >
> > #include <linux/sizes.h>
> >
> > +#define FFA_SHARED_MM_BUFFER_SIZE SZ_4K /* 4 KB */
> > +
> > +/*
> > + * shared buffer physical address used for communication between
> > + * u-boot and the MM SP
> > + */
> > +#define FFA_SHARED_MM_BUFFER_ADDR 0x02000000UL
> > +#define FFA_SHARED_MM_BUFFER_OFFSET 0
> > +
>
> These should be in devicetree properties, shouldn't they? We don't
> want things in board config.h files anymore.

Wasn't one of the earlier debates on if the whole thing needs to be in
device tree, or not, because it's a "discoverable bus" and so Linux
wasn't needing one? I don't want to de-rail this series, which I think
we're just about otherwise ready to merge, over that debate again.

As these values have to match up (I assume) with the other side, are
these truly per-board, or SoC dependent?

--
Tom

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