Re: [edk2-devel] [RFC] Remove PI spec TemporaryRamSupport PPI from PEI Core

["Nate DeSimone" <nathaniel.l.desimone@intel.com>]

Hi Jordan,

The TemporaryRamSupport PPI is mandatory for FSP. The reason why is the stack migration algorithm for FSP is quite different compared to the standard method implemented in PEI core.

During FspMemoryInit(), PEI core runs on top of the bootloader stack. PEI core is not the root of the stack during FSP-M, which violates a lot of assumptions in PEI core's implementation of shadowing to permanent memory. During the process of migrating FSP to permanent memory, we break the single stack into two stacks. The custom implementation for FSP is here:

https://github.com/tianocore/edk2/blob/master/IntelFsp2Pkg/FspSecCore/SecMain.c

Thanks,
Nate

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jordan Justen
Sent: Wednesday, April 24, 2019 12:51 AM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Andrew Fish <afish@apple.com>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>
Subject: [edk2-devel] [RFC] Remove PI spec TemporaryRamSupport PPI from PEI Core

Short summary: Despite being part of the Platorm Initialization Specificication, this proposal would remove support for the TemporaryRamSupport PPI from PEI Core. Arguably, this would mean the PEI Core does not support the PI spec, but we do not currently know if any platforms require this PPI.

Main question: Does anyone know of a platform that requires this PPI, or does anyone have major concerns with removing it before the PI specification can be updated to remove the PPI?

Alternatives: Continue to support the PPI, but we would need to merge a bug-fix patchset which adds some assembly code into the PEI Core.

More details:

I discussed this topic with the Tianocore Stewards, Vincent and Ray.
Although the path forward was not unanimous, it appears that the majority thought we should propose to remove support for the TemporaryRamSupport PPI from the PEI Core.

This PPI is defined in MdePkg/Include/Ppi/TemporaryRamSupport.h. I believe it has been present in all versions of the Platorm Initialization Specificication, including the latest, version 1.7.
(https://uefi.org/specsandtesttools)

The TemporaryRamSupport PPI is defined as "optional" in the PI spec, but I believe this only applied to producers of the PPI (PI platforms). The PEI Core (which is the consumer of this PPI) should arguably support the PPI in order to fully support PI spec based platforms.

But, there are some subtle issues with the PPI that make it difficult to implement for both the consumer (PEI Core) and the producer (the platform). In fact, there is a bug currently with the PEI Core's implementation, which I sent a patch series to address on April 10th, with the subject of "[PATCH v2 0/6] Fix PEI Core issue during TemporaryRamMigration".

Unfortunately the fix is not very simple, and requires adding assembly code to the PEI Core module. Based on this, we discussed if the TemporaryRamSupport PPI is no longer required by current PI platforms, despite being present in the PI spec. The only known example of it being required was a platform based on the IPF architecture, which is no longer supported by EDK II.

Several EDK II sample platforms produce this PPI in EDK II, but only as sample code. Clearly the removal would have to take this into account.

There are two replacements for the TemporaryRamSupport PPI, but they do not cover all types of potential hardware. If a platform cannot access main memory at the same time as the Temporary RAM, then it would not be supported by the alternatives. Nevertheless, the two replacements are:

1. Stop producing the PPI, and the PEI Core will automatically copy
   the required memory from Temporary RAM to the main memory. This
   will work for platforms that do not require special code to shut
   down Temporary RAM.

2. Produce the MdePkg/Include/Ppi/TemporaryRamDone.h PPI. This will
   cause the PEI Core to notify the platform after it has copied
   memory, and the platform can take special actions to disable
   Temporary RAM mode if required.

Since both of these options will cause the PEI Core to do a memory copy from Temporary RAM to the main memory, there might be platforms that they cannot work with as described above.

Thanks for your time,

-Jordan