On Tue, 30 May 2023 at 18:51, Oliver Smith-Denny
<o...@linux.microsoft.com> wrote:
>
> On 5/25/2023 2:29 PM, Ard Biesheuvel wrote:
> > On Thu, 25 May 2023 at 19:21, Oliver Smith-Denny
> > <o...@linux.microsoft.com> wrote:
> >>
> >> On 5/25/2023 7:30 AM, Ard Biesheuvel wrote:
> >>> + if (Private->MemoryAttributePpi == NULL) {
> >>>
> >>> + return EFI_SUCCESS;
> >>
> >> We will have a gap here before the MemoryAttributePpi is installed,
> >> obviously, when CpuPei is installed. Is the expectation that only
> >> dependencies for CpuPei will be launched before and everything else
> >> will have a dependency on CpuPei?
> >>
> >> Or, is it that shadowed PEIMs won't happen before CpuPei? I am curious
> >> how big or small of a gap this really is.
> >>
> >
> > There are two different cases to consider here:
> > - First, there is the DxeIpl, which will rely on the PPI (via the
> > image loader and directly) to map the DXE core and the NX stack. The
> > DxeIpl will not proceed with that until all PEIMs are dispatched, so
> > if the PPI is going to be exposed, it will be available by that time.
> > - Then there are the shadowed PEIMs, and given that shadowed PEIMs
> > implicitly depend on PEI permanent memory having been installed, the
> > only requirement here is that, if the platform needs/wants this for
> > shadowed PEIMs as well, the PEIM that installs the PEI permanent
> > memory should depex on the PPI.
> >
>
> I see, thanks for the explanation. While this is not the main point
> of this patchset, I do think that the core should own the memory
> protections of the shadowed PEIMs, instead of relying on the platform
> to do the right thing (which it never does :). Sure, this is gated by
> PCDs that the platform sets, but that is a simpler interface than
> requiring a new depex.
>
> Perhaps this is not trivial, as I assume the permanent memory
> installation happens in a platform specific PEIM? And as such would
> require a PPI notification in the core for the memory attributes PPI,
> which would lead to additional undesirable complexity.
>
This is simply a matter of policy. If we decide that shadowed PEIMs
must always be remapped read-only, we should reorganize the code to
ensure that, and disallow shadowing otherwise. Shadowing is a
performance optimization in any case, although existing code may not
work, due to the way RegisterForShadow() is designed. But this is all
fixable.
But that doesn't change the fact that installing PEI permanent memory
is fundamentally something that is implemented in platform code. Each
platform that already implements this should be able to take advantage
of those core changes once they land.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105470): https://edk2.groups.io/g/devel/message/105470
Mute This Topic: https://groups.io/mt/99131192/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
