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

[Simon Glass]

Hi Tom,

On Fri, 7 Jul 2023 at 11:44, Tom Rini <trini at konsulko.com> wrote:
>
> 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 at 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 at arm.com>
> > > Cc: Tom Rini <trini at konsulko.com>
> > > Cc: Simon Glass <sjg at chromium.org>
> > > Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > Cc: Jens Wiklander <jens.wiklander at 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 at linaro.org>
> > > - * Abdellatif El Khlifi <abdellatif.elkhlifi at arm.com>
> > > + * Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office at arm.com>
> > > + *
> > > + * Authors:
> > > + *   Abdellatif El Khlifi <abdellatif.elkhlifi at 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?

Yes let's not derail the series...it has gone on too long already.

But we do need to think about this. Addresses in #defines are not a
great look. It isn't even a CFG.

Regards,
Simon