public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Achin Gupta <Achin.Gupta@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Laszlo Ersek <lersek@redhat.com>,
	Jagadeesh Ujja <Jagadeesh.Ujja@arm.com>,
	 "Gao, Liming" <liming.gao@intel.com>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Zhang, Chao B" <chao.b.zhang@intel.com>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>,
	Jian J Wang <jian.j.wang@intel.com>, nd <nd@arm.com>
Subject: Re: [PATCH v2 04/11] MdePkg/Include: Add StandaloneMmServicesTableLib library
Date: Mon, 7 Jan 2019 19:21:33 +0000	[thread overview]
Message-ID: <20190107192137.GG14419@e104320-lin> (raw)
In-Reply-To: <CAKv+Gu-4tCuYt5hq=J6S1zvamaKz1S40rX7Lpa8rZmTcj=vyGA@mail.gmail.com>

On Mon, Jan 07, 2019 at 07:55:36PM +0100, Ard Biesheuvel wrote:
> On Mon, 7 Jan 2019 at 19:50, Achin Gupta <Achin.Gupta@arm.com> wrote:
> >
> > On Mon, Jan 07, 2019 at 06:33:26PM +0100, Ard Biesheuvel wrote:
> > > 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)
> >
> > The idea behind MM Standalone mode was to sandbox MM code in self sufficient
> > execution context. This was a step to avoid some of the vulnerabilities in
> > traditional SMM due to code and data sharing with DXE.
> >
> > On AArch64, the MM standalone mode is initialised during the SEC phase. This
> > corresponds to Trustzone initialisation. Furthermore, the MM standalone
> > execution context is placed in user mode (Secure EL0) instead of running it in a
> > privileged processor mode (S-EL1 or EL3 on AArch64, Ring -2 or SMM on x86). This
> > restricts what the MM standalone context can see and do. Lastly, after SEC no
> > more MM Standalone drivers can be initialized during PEI or DXE (in contrast to
> > the example in PI 1.6 Section 1.5.2).
> >
> > Hope that makes sense?
> >
> 
> I agree, but this is not what StandaloneMmPkg implements today: it
> implements a MmFvDispatchHandler that permits a FV to be brough into
> the MM environment, and the MM dispatcher will happily dispatch all
> the MM_STANDALONE modules it contains.

Umm! Not following why is that a problem. The FV on AArch64 must only contain
MM_STANDALONE drivers. These drivers rely on the MMST which is exclusively used
in the secure world. So the isolation is still achieved right? Are you saying
that additional checks are required to enforce this on AArch64?

> 
> Also, there are some other pieces missing (which I mentioned in one of
> the other threads but I suppose you may not have caught up yet):
> EndOfDxe (as well as some other PI defined events) needs to be
> signalled to the standalone MM context by some non-MM agent, and I
> think there are other parts of the traditional SMM IPL that have not
> been ported to standalone MM yet.

Yes! I am still catching up but saw the patches that Supreeth has reviewed and
they make sense. Could you please explain the need for End of DXE signalling and
the traditional SMM IPL. It is not obvious to me :o(

cheers,
Achin

> 
> > I have not seen all the patches in this and related series but the use of InMM()
> > to allow code to have a DXE driver or a MM Standalone driver personality seems
> > to defeat the entire purpose of Standalone MM. My concern is that any code that
> > is relevant only to DXE or PEI must not be a part of the MM Standalone
> > context. This could be achieved through proper refactoring + conditional
> > compilation. If the decision is taken at runtime then this is just traditional
> > MM.
> >


  reply	other threads:[~2019-01-07 19:21 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-02 13:13 [PATCH v2 00/11] Extend secure variable service to be usable from Standalone MM Jagadeesh Ujja
2019-01-02 13:13 ` [PATCH v2 01/11] StandaloneMmPkg: Remove MM_STANDALONE LIBRARY_CLASS from StandaloneMmCoreHobLib Jagadeesh Ujja
2019-01-02 13:13 ` [PATCH v2 02/11] StandaloneMmPkg: Adding the library packages used by MM_STANDALONE drivers Jagadeesh Ujja
2019-01-02 13:13 ` [PATCH v2 03/11] MdeModulePkg: Add a PCD to indicate Standalone MM supports secure variable Jagadeesh Ujja
2019-01-02 13:13 ` [PATCH v2 04/11] MdePkg/Include: Add StandaloneMmServicesTableLib library Jagadeesh Ujja
2019-01-03 11:03   ` Ard Biesheuvel
2019-01-03 16:14     ` Laszlo Ersek
2019-01-04 11:57       ` Ard Biesheuvel
2019-01-07 15:28         ` Laszlo Ersek
2019-01-07 17:33           ` Ard Biesheuvel
2019-01-07 18:50             ` Achin Gupta
2019-01-07 18:55               ` Ard Biesheuvel
2019-01-07 19:21                 ` Achin Gupta [this message]
2019-01-07 19:37                   ` Ard Biesheuvel
2019-01-08  1:11                     ` Laszlo Ersek
2019-01-08 13:27                       ` Ard Biesheuvel
2019-01-08 16:52                         ` Laszlo Ersek
2019-01-13 12:42                     ` Cohen, Eugene
2019-01-14 18:51                       ` Ard Biesheuvel
2019-01-02 13:13 ` [PATCH v2 05/11] MdeModulePkg/FaultTolerantWriteDxe: allow reusability as a MM driver Jagadeesh Ujja
2019-01-02 17:15   ` Ard Biesheuvel
2019-01-02 13:13 ` [PATCH v2 06/11] MdeModulePkg/Variable/RuntimeDxe: adapt for usability with MM Standalone Jagadeesh Ujja
2019-01-02 13:13 ` [PATCH v2 07/11] MdeModulePkg/Variable/RuntimeDxe: adapt as a MM Standalone driver Jagadeesh Ujja
2019-01-02 13:13 ` [PATCH v2 08/11] MdeModulePkg/VarCheckLib: allow MM_STANDALONE drivers to use this library Jagadeesh Ujja
2019-01-04 10:36   ` Ard Biesheuvel
2019-01-02 13:13 ` [PATCH v2 09/11] ArmPlatformPkg/NorFlashDxe: allow reusability as a MM driver Jagadeesh Ujja
2019-01-02 13:13 ` [PATCH v2 10/11] SecurityPkg/AuthVariableLib: allow MM_STANDALONE drivers to use this library Jagadeesh Ujja
2019-01-03  1:14   ` Zhang, Chao B
2019-01-03  6:15     ` Jagadeesh Ujja
2019-01-04 10:41       ` Ard Biesheuvel
2019-01-02 13:13 ` [PATCH v2 11/11] CryptoPkg/BaseCryptLib: " Jagadeesh Ujja
2019-01-04 10:35   ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190107192137.GG14419@e104320-lin \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox