Hi Sayanta,

Thanks for confirming.

With that.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

From: Sayanta Pattanayak <Sayanta.Pattanayak@arm.com>
Date: Wednesday, 26 May 2021 at 19:15
To: Sami Mujawar <Sami.Mujawar@arm.com>, devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>, nd <nd@arm.com>
Subject: RE: [edk2-platforms][PATCH V1 3/3] Platform/Sgi: enable support for UEFI secure boot
Hi Sami,

Thanks for the review and suggestion. Please find my reply inline.

>
> Hi Sayanta,
>
> Thank you for this patch.
>
> Please find my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> On 24/05/2021 06:23 PM, Sayanta Pattanayak wrote:
> > Enable the use of UEFI secure boot for Arm's Neoverse reference design
> > platforms. The UEFI authenticated variable store uses NOR flash 2
> > which is accessible from Standalone MM context residing in a secure
> partition.
> >
> > Signed-off-by: Sayanta Pattanayak <sayanta.pattanayak@arm.com>
> > ---
> >   Platform/ARM/SgiPkg/SgiPlatform.dsc.inc       | 31
> +++++++++++++++++++
> >   Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc     | 32
> ++++++++++++++++++++
> >   Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc  | 15 +++++++++
> >   Platform/ARM/SgiPkg/PlatformStandaloneMm2.dsc | 15 +++++++++
> >   Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf  |  5 +++
> >   Platform/ARM/SgiPkg/SgiPlatform.fdf           |  9 +++++-
> >   6 files changed, 106 insertions(+), 1 deletion(-)
> >

<...>

> >     ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > +!endif
> > diff --git a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
> > b/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
> > index 3389ff676a91..6839ec35da8a 100644
> > --- a/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
> > +++ b/Platform/ARM/SgiPkg/SgiPlatformMm.dsc.inc
> > @@ -59,6 +59,19 @@
> >
> HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmH
> obLib.inf
> >
> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Stan
> daloneMmServicesTableLib.inf
> >
> >
> MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAll
> ocati
> > onLib/StandaloneMmMemoryAllocationLib.inf
> > +!if $(SECURE_STORAGE_ENABLE) == TRUE
> > +
> > +AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.i
> > +nf
> > +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> > +  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> > +
> >
> +NorFlashPlatformLib|Platform/ARM/SgiPkg/Library/NorFlashLib/Standalon
> > +eMmNorFlashLib.inf
> > +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > +  RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
> [SAMI] There is a recent patch series that adds ARMv8.5 FEAT_RNG support
> to BaseRngLib
>   see
> https://github.com/tianocore/edk2/commit/9301e5644cef5a5234f71b178373
> dd508cabb9ee.
> Can this be used instead of BaseRngLibTimerLib? BaseRngLibTimerLib is for
> non-production use so it would be good to avoid.
> Indeed, this would require that Sgi platforms are ARMv8.5 or above. If not,
> then can we conditionally use BaseRngLibTimerLib for platforms that do not
> support FEAT_RNG.
> [/SAMI]

Current SGI platforms with secureboot are of pre ARMv8.5.  For ARMv8.5 and above SGI platform, will follow the conditional approach.

Regards,
Sayanta

<...>