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

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

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.