Re: [RFC PATCH 09/11] MdeModulePkg/DxeCore: Add PCD NX policy bit for default NX state

["Ard Biesheuvel" <ardb@kernel.org>]

On Tue, 30 May 2023 at 08:54, Ni, Ray <ray.ni@intel.com> wrote:
>
> 1. Do we want to catch a case that platform wrongly sets BIT61 but drivers run before CpuDxe are not XIP?
> 2. Why BIT61 set is the "Default state"?
>
> The setting of BIT61 is a bit confusing. Is there a way to avoid adding BIT61 through code optimization?
>

The idea is that we can inform the DXE core about whether the mappings
created during PEI are NX or not. So perhaps 'default state' should be
renamed to 'initial state'.

This is important for two reasons:
- The DXE drivers that execute in place are covered by
EfiBootServicesData allocations, and we should not remap those XP when
the CPU arch protocol is dispatched
- Going over all of memory to update the mappings is unnecessary if
the mappings are already XP.




>
> > -----Original Message-----
> > From: Ard Biesheuvel <ardb@kernel.org>
> > Sent: Monday, May 29, 2023 6:17 PM
> > To: devel@edk2.groups.io
> > Cc: Ard Biesheuvel <ardb@kernel.org>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor Beebe
> > <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>; Bi, Dandan
> > <dandan.bi@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kinney,
> > Michael D <michael.d.kinney@intel.com>; Leif Lindholm
> > <quic_llindhol@quicinc.com>; Michael Kubacki <mikuback@linux.microsoft.com>
> > Subject: [RFC PATCH 09/11] MdeModulePkg/DxeCore: Add PCD NX policy bit for
> > default NX state
> >
> > Introduce a new bit in the NX memory protection policy PCD mask that
> > specifies that the platform enters DXE with all unused and all non-code
> > regions mapped with non-execute permissions.
> >
> > This removes the need to do a pass over all memory regions to update
> > their NX memory attributes.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 7 +++++++
> >  MdeModulePkg/MdeModulePkg.dec                 | 3 +++
> >  2 files changed, 10 insertions(+)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > index 7cc829b17402c2bc..983ed450f143d62d 100644
> > --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > @@ -861,6 +861,13 @@ InitializeDxeNxMemoryProtectionPolicy (
> >      ASSERT (StackBase != 0);
> >
> >    }
> >
> >
> >
> > +  //
> >
> > +  // If the platform maps all DRAM non-execute by default, we are done here.
> >
> > +  //
> >
> > +  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT61) != 0) {
> >
> > +    return;
> >
> > +  }
> >
> > +
> >
> >    DEBUG ((
> >
> >      DEBUG_INFO,
> >
> >      "%a: applying strict permissions to active memory regions\n",
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > b/MdeModulePkg/MdeModulePkg.dec
> > index 2d72ac733d82195e..d2bd0cbb40300889 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -1416,12 +1416,15 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
> >    #  EfiMemoryMappedIOPortSpace     0x1000<BR>
> >
> >    #  EfiPalCode                     0x2000<BR>
> >
> >    #  EfiPersistentMemory            0x4000<BR>
> >
> > +  #  Default state      0x2000000000000000<BR>
> >
> >    #  OEM Reserved       0x4000000000000000<BR>
> >
> >    #  OS Reserved        0x8000000000000000<BR>
> >
> >    #
> >
> >    # NOTE: User must NOT set NX protection for EfiLoaderCode /
> > EfiBootServicesCode / EfiRuntimeServicesCode. <BR>
> >
> >    #       User MUST set the same NX protection for EfiBootServicesData and
> > EfiConventionalMemory. <BR>
> >
> >    #
> >
> > +  # If the platform enters DXE with all unused and non-code regions mapped NX,
> > bit 61 should be set.<BR>
> >
> > +  #
> >
> >    # e.g. 0x7FD5 can be used for all memory except Code. <BR>
> >
> >    # e.g. 0x7BD4 can be used for all memory except Code and ACPINVS/Reserved.
> > <BR>
> >
> >    #
> >
> > --
> > 2.39.2
>