Re: [PATCH v2 04/11] MdePkg/Include: Add StandaloneMmServicesTableLib library

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

On Mon, 7 Jan 2019 at 16:28, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 01/04/19 12:57, Ard Biesheuvel wrote:
> > On Thu, 3 Jan 2019 at 17:14, Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> On 01/03/19 12:03, Ard Biesheuvel wrote:
> >>> On Wed, 2 Jan 2019 at 14:14, Jagadeesh Ujja <jagadeesh.ujja@arm.com> wrote:
> >>>>
> >>>> Some of the existing DXE drivers can be refactored to execute within
> >>>> the Standalone MM execution environment as well. Allow such drivers to
> >>>> get access to the Standalone MM services tables.
> >>>>
> >>>> Add a mechanism to determine the execution mode is required.
> >>>> i.e, in MM or non-MM
> >>>>
> >>>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>>> Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> >>>> ---
> >>>>  MdePkg/Include/Library/StandaloneMmServicesTableLib.h                        | 43 ++++++++++++++++++++
> >>>>  MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.c   | 39 ++++++++++++++++++
> >>>>  MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf | 36 ++++++++++++++++
> >>>>  MdePkg/MdePkg.dec                                                            |  4 ++
> >>>>  4 files changed, 122 insertions(+)
> >>>>
> >>>
> >>> OK, so since the PI spec only refers to MM mode now, this library
> >>> class should be
> >>>
> >>> MmServicesTableLib|Include/Library/MmServicesTableLib.h
> >>>
> >>> with an implementation in MdeModulePkg that exposes the deprecated SMM
> >>> system table as the MM system table.
> >>>
> >>> In StandaloneMmPkg, we can add an implementation that exposes the
> >>> standalone MM system table.
> >>>
> >>> (They are binary compatible, so it is just a matter of casting one
> >>> pointer to the other)
> >>>
> >>> With this in place, we can go ahead and update FaultTolerantWrite and
> >>> Variable SMM driver to switch from SmmServicesTableLib to
> >>> MmServicesTableLib. This will require existing x86 platforms to define
> >>> a new library class resolution for MmServicesTableLib, referring to
> >>> the implementation in MdeModulePkg. This is unfortunate, but it is an
> >>> unavoidable consequence of the PI spec changes.
> >>
> >> It shouldn't be too intrusive or hard to review, I expect.
> >>
> >>>
> >>> Remaining question is what to do with InSmm() ...
> >>
> >> I'm lacking the context on this; on the other hand, I can refer back to
> >> at least one earlier discussion -- there had been multiple -- of the
> >> discrepancy between the PI spec and the edk2 code. See:
> >>
> >> - bullet (9) in
> >> <http://mid.mail-archive.com/aada511c-bdb9-d833-caa5-bee56cc47d27@redhat.com>,
> >> - and
> >> <http://mid.mail-archive.com/0C09AFA07DD0434D9E2A0C6AEB0483103BB55B46@shsmsx102.ccr.corp.intel.com>.
> >>
> >> Not sure how that can be applied to Arm.
> >>
> >
> > The code I posted yesterday does not use InMm() at all. For standalone
> > MM, it should always return TRUE anyway, and any code that a driver
> > would execute if it returned FALSE needs to be factored out anyway,
> > since it should not end up in standalone MM binaries as dead code.
> >
>
> OK. That seems to make sense. I've read up a bit on "standalone MM" in
> the PI v1.6 spec, vol 4. Having no access to UEFI protocols even in the
> entry point function, at driver init time, seems challenging to me. I
> guess I'll learn more about this as a part of the usual list traffic.
>
> What is the MODULE_TYPE that standalone MM drivers use, in place of
> DXE_SMM_DRIVER (= EFI_FV_FILETYPE_MM, 0x0A)?
>
> Hm... from the other patches, it seems to be MM_STANDALONE (=
> EFI_FV_FILETYPE_MM_STANDALONE, 0x0E). OK.
>
> If I'd like to see a short summary of standalone MM, relative to
> traditional MM, and why it is more suitable -- I presume -- for aarch64,
> which document should I look at, from
> <https://mantis.uefi.org/mantis/view.php?id=1390>, for example?
>

Perhaps Achin can answer this, since he has been driving the spec side
of this? (and maintains StandaloneMmPkg)