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

["Ni, Ray" <ray.ni@intel.com>]

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?

Thanks,
Ray

> -----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