Re: [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Thu, 17 Jan 2019 at 12:04, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Jan 04, 2019 at 03:43:35PM +0100, Ard Biesheuvel wrote:
> > Create a pair of .DSC/.FDF files that describe the components and
> > the firmware volumes and flash device that will be dispatched into
> > a secure partition in the secure world to control the UEFI secure
> > variable store.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  Platform/Socionext/DeveloperBox/DeveloperBox.dsc     |   5 +-
> >  Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc |   7 +-
> >  Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc   | 103 +++++++++++++
> >  Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf   | 161 ++++++++++++++++++++
> >  4 files changed, 270 insertions(+), 6 deletions(-)
> >
> > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > index 56787a744157..666bd2716336 100644
> > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > @@ -250,10 +250,7 @@ [Components.common]
> >    #
> >    # Variable services
> >    #
> > -  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf {
> > -    <LibraryClasses>
> > -      NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
> > -  }
> > +  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
> >    MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> >    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
> >      <LibraryClasses>
> > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > index 56adc21d5caf..87b2094cb356 100644
> > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > @@ -125,6 +125,7 @@ [LibraryClasses.common]
> >    PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> >
> >    NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
> > +  NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
> >
> >  ################################################################################
> >  #
> > @@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common]
> >  !endif
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
> >
> > -  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
> > -  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
> > +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
>
> So, I can see why you add this hard-wired for the purpose of testing.
> But please, add a *very* conspicuous, and strongly worded, comment
> statement preceding it.
>

Well, I was talking to Peter about this the other day: according to
the spec, this setting should only matter before exit boot services,
and since this platform only supports serial and GOP consoles, one
could argue that only a physically present user could interact with it
before that time.

The obvious way of implementing this non-trivially on this platform is
to use a DIP switch, but that requires you to open the case to
enroll/delete the platform key. Perhaps that does not matter, and it
would in fact produce a less dangerous reference implementation.