Re: [PATCH] MdeModulePkg/VariableSmmRuntimeDxe: Refactor locating Variable Arch Protocol

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

On Thu, 21 Feb 2019 at 10:33, Zeng, Star <star.zeng@intel.com> wrote:
>
> On 2019/2/21 17:11, Ard Biesheuvel wrote:
> > On Thu, 21 Feb 2019 at 10:04, Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> On 02/20/19 13:23, Ard Biesheuvel wrote:
> >>> On Wed, 20 Feb 2019 at 06:53, Jagadeesh Ujja <jagadeesh.ujja@arm.com> wrote:
> >>>>
> >>>> hi Ard,
> >>>> On Tue, Feb 19, 2019 at 6:55 PM Ard Biesheuvel
> >>>> <ard.biesheuvel@linaro.org> wrote:
> >>>>>
> >>>>> Hello Jagadeesh,
> >>>>>
> >>>>> On Tue, 19 Feb 2019 at 11:47, Jagadeesh Ujja <jagadeesh.ujja@arm.com> wrote:
> >>>>>>
> >>>>>> In preparation for providing a standalone MM based non-secure variable
> >>>>>> runtime driver, factor out some portions that are specific to the
> >>>>>> traditional driver, mainly related to locating variable arch protocol
> >>>>>> and variable write arch protocol, which are not required to be located
> >>>>>> when using standalone MM based secure variable implementation.
> >>>>>>
> >>>>>
> >>>>> While i think this change is correct from a technical perspective, I
> >>>>> don't think this is the right approach.
> >>>>>
> >>>> these changes are mandatory, this is one of the possible solution.
> >>>>
> >>>>> It was a deliberate decision to expose the MM services in a way that
> >>>>> only the producer of the communication protocol is aware of the
> >>>>> implementation details, i.e., whether it is backed by tradtional MM or
> >>>>> standalone MM.
> >>>>>
> >>>> can you please provide more details on how "exposing the MM services"
> >>>> will help to resolve the issue here. if this helps, definitely i will use that.
> >>>>
> >>>
> >>> Let me rephrase this for the benefit of the MdeModulePkg maintainers,
> >>> and ask them their opinion.
> >>>
> >>> Currently, the DXE runtime driver that produces the architectural
> >>> varstore protocols that are based on communication with MM components
> >>> living elsewhere, rely on the EFI protocol database for sequencing.
> >>> I.e., after dispatch, they wait for certain protocols to be installed
> >>> into the DXE protocol database by the SMM drivers before proceeding to
> >>> install the variable arch protocols.
> >>>
> >>> This does not work for standalone MM, since it has no access to the
> >>> DXE protocol database, nor is it needed, since it may be assumed that
> >>> the MM execution context is fully configured by the time the DXE phase
> >>> starts.
> >>>
> >>> Jagadeesh's proposal is to factor this out, and create two different
> >>> .INFs to build the same DXE runtime driver in two different ways. This
> >>> defeats the purpose of having an abstract MM communication protocol,
> >>> so it is something I would like to avoid. On the other hand, is it not
> >>> obvious how to parameterize this requirement in another way.
> >>>
> >>> For the moment, I could live with putting this into a library, and
> >>> leave it up to the platform to ensure the combination of the library
> >>> resolution with the driver that produces the MM communicate protocol
> >>> is a sane one.
> >>>
> >>> Any thoughts?
> >>
> >> I think I'm missing the gist of the library approach; still, would it be
> >> possible for affected platforms (i.e. those that depend on standalone
> >> MM) to procude the necessary DXE protocols (for unblocking the variable
> >> runtime driver) in a platform DXE driver?
> >>
> >
> > Yes, that is the other option: we could create a library that
> > unconditionally produces those protocols and hook it into the MM
> > communication driver via NULL library class resolution.
> >
>
> I am not familiar with standalone MM, either ARM. So may have no much
> valuable opinion.
>
> For this case, standalone MM could not install DXE protocols into DXE
> protocol database to notify the wrapper (VariableSmmRuntimeDxe), so need
> another way to install the DXE protocols, right?

Yes

> Could standalone MM assume the MM handler for variable is ready when MM
> communication driver runs?

Yes

> If yes, a NULL library instance should work (as a stub to install the
> DXE protocols in its constructor). :)
>

Yes, that was my suggestion.

So Jagadeesh, could you please take this approach instead?

- Create a library in StandaloneMmPkg with LIBRARY_CLASS = NULL and a
constructor that installs the two protocols.
- Update your platform so that the MM communication driver is included as

ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
  <LibraryClasses>
    NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
}

I don't think this will violate any ordering constraints, given that
the drivers that have a dependency on these protocols also have a
dependency on the MM communicate driver itself.

Thanks,
Ard.