Ray,
I think using MemoryAttribute PPI also looks good for X64 DxeIpl.
The only question that comes to my mind is the AMD sev feature. Since the
MemoryAttribute can't handle the AMD sev feature requirements(remapping ghcb
range from non-1:1 mapping to 1:1-mapping), we may need to find an appropriate
place to remap the Ghcb range.
Thanks,
Dun
-----Original Message-----
From: Ni, Ray <ray...@intel.com>
Sent: Tuesday, May 30, 2023 3:19 PM
To: Ard Biesheuvel <a...@kernel.org>; devel@edk2.groups.io; Tan, Dun
<dun....@intel.com>
Cc: Yao, Jiewen <jiewen....@intel.com>; Gerd Hoffmann <kra...@redhat.com>;
Taylor Beebe <t...@taylorbeebe.com>; Oliver Smith-Denny <o...@smith-denny.com>;
Bi, Dandan <dandan...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>;
Kinney, Michael D <michael.d.kin...@intel.com>; Leif Lindholm
<quic_llind...@quicinc.com>; Sunil V L <suni...@ventanamicro.com>; Warkentin,
Andrei <andrei.warken...@intel.com>
Subject: RE: [RFC PATCH 09/10] MdeModulePkg/DxeIpl: Use memory attribute PPI to
remap the stack NX
Looks good.
@Tan, Dun, can you please evaluate if using MemoryAttribute PPI, what opens
will there be for X64 DxeIpl?
> -----Original Message-----
> From: Ard Biesheuvel <a...@kernel.org>
> Sent: Thursday, May 25, 2023 10:31 PM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <a...@kernel.org>; Ni, Ray <ray...@intel.com>; Yao,
> Jiewen <jiewen....@intel.com>; Gerd Hoffmann <kra...@redhat.com>;
> Taylor Beebe <t...@taylorbeebe.com>; Oliver Smith-Denny
> <o...@smith-denny.com>; Bi, Dandan <dandan...@intel.com>; Gao, Liming
> <gaolim...@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kin...@intel.com>; Leif Lindholm
> <quic_llind...@quicinc.com>; Sunil V L <suni...@ventanamicro.com>;
> Warkentin, Andrei <andrei.warken...@intel.com>
> Subject: [RFC PATCH 09/10] MdeModulePkg/DxeIpl: Use memory attribute
> PPI to remap the stack NX
>
> If the associated PCD is set to TRUE, use the memory attribute PPI to
> remap the stack non-executable. This provides a generic method for
> doing so, which will be used by ARM and AArch64 as well once they move
> to the generic DxeIpl handoff implementation.
>
> Signed-off-by: Ard Biesheuvel <a...@kernel.org>
> ---
> MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c | 29 ++++++++++++++++++--
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 5 +++-
> 2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
> b/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
> index a0f85ebea56e6cba..22caabb02840ba88 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c
> @@ -2,12 +2,15 @@
> Generic version of arch-specific functionality for DxeLoad.
>
>
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights
> reserved.<BR>
>
> +Copyright (c) 2023, Google, LLC. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
>
>
> **/
>
>
>
> #include "DxeIpl.h"
>
>
>
> +#include <Ppi/MemoryAttribute.h>
>
> +
>
> /**
>
> Transfers control to DxeCore.
>
>
>
> @@ -25,9 +28,10 @@ HandOffToDxeCore (
> IN EFI_PEI_HOB_POINTERS HobList
>
> )
>
> {
>
> - VOID *BaseOfStack;
>
> - VOID *TopOfStack;
>
> - EFI_STATUS Status;
>
> + VOID *BaseOfStack;
>
> + VOID *TopOfStack;
>
> + EFI_STATUS Status;
>
> + EDKII_MEMORY_ATTRIBUTE_PPI *MemoryPpi;
>
>
>
> //
>
> // Allocate 128KB for the Stack
>
> @@ -35,6 +39,25 @@ HandOffToDxeCore (
> BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
>
> ASSERT (BaseOfStack != NULL);
>
>
>
> + if (PcdGetBool (PcdSetNxForStack)) {
>
> + Status = PeiServicesLocatePpi (
>
> + &gEdkiiMemoryAttributePpiGuid,
>
> + 0,
>
> + NULL,
>
> + (VOID **)&MemoryPpi
>
> + );
>
> + ASSERT_EFI_ERROR (Status);
>
> +
>
> + Status = MemoryPpi->SetPermissions (
>
> + MemoryPpi,
>
> + (UINTN)BaseOfStack,
>
> + STACK_SIZE,
>
> + EFI_MEMORY_XP,
>
> + 0
>
> + );
>
> + ASSERT_EFI_ERROR (Status);
>
> + }
>
> +
>
> //
>
> // Compute the top of the stack we were allocated. Pre-allocate a
> UINTN
>
> // for safety.
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index 60c998be6c1bad01..7126a96d8378d1f8 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -91,6 +91,7 @@ [Ppis]
> gEfiPeiMemoryDiscoveredPpiGuid ## SOMETIMES_CONSUMES
>
> gEdkiiPeiBootInCapsuleOnDiskModePpiGuid ## SOMETIMES_CONSUMES
>
> gEdkiiPeiCapsuleOnDiskPpiGuid ## SOMETIMES_CONSUMES # Consumed
> on firmware update boot path
>
> + gEdkiiMemoryAttributePpiGuid ## SOMETIMES_CONSUMES
>
>
>
> [Guids]
>
> ## SOMETIMES_CONSUMES ## Variable:L"MemoryTypeInformation"
>
> @@ -117,10 +118,12 @@ [Pcd.IA32,Pcd.X64]
> gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize ##
> CONSUMES
>
>
>
> [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
>
> - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ##
> SOMETIMES_CONSUMES
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ##
> SOMETIMES_CONSUMES
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ##
> SOMETIMES_CONSUMES
>
>
>
> +[Pcd]
>
> + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ##
> SOMETIMES_CONSUMES
>
> +
>
> [Depex]
>
> gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid
>
>
>
> --
> 2.39.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105446): https://edk2.groups.io/g/devel/message/105446
Mute This Topic: https://groups.io/mt/99131196/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
