Re: [edk2-devel] [PATCH v5 24/28] ArmVirtPkg: Apply Memory Protections via SetMemoryProtectionsLib

["Laszlo Ersek" <lersek@redhat.com>]

On 10/9/23 02:07, Taylor Beebe wrote:
> Set the memory protections on Arm virtual platforms. Because
> the QemuFg parser is not currently available in ArmVirtPkg, use
> the GrubCompat profile by default.
> 
> Signed-off-by: Taylor Beebe <taylor.d.beebe@gmail.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c   | 7 +++++++
>  ArmVirtPkg/ArmVirtPkg.dec                   | 7 +++++++
>  ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf | 3 +++
>  3 files changed, 17 insertions(+)
> 
> diff --git a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c
> index ef88a9df1d62..aaf2af9abccf 100644
> --- a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c
> +++ b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c
> @@ -11,6 +11,7 @@
>  #include <Library/ArmPlatformLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/HobLib.h>
> +#include <Library/SetMemoryProtectionsLib.h>
>  #include <Library/PeimEntryPoint.h>
>  #include <Library/PeiServicesLib.h>
>  #include <Library/PcdLib.h>
> @@ -100,5 +101,11 @@ InitializeMemory (
>               );
>    ASSERT_EFI_ERROR (Status);
>  
> +  ASSERT (FixedPcdGet8 (PcdDxeMemoryProtectionProfile) < DxeMemoryProtectionSettingsMax);
> +  SetDxeMemoryProtectionSettings (
> +    NULL,
> +    (DXE_MEMORY_PROTECTION_PROFILE_INDEX)FixedPcdGet8 (PcdDxeMemoryProtectionProfile)
> +    );
> +
>    return Status;
>  }
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index 4645c91a8375..d90e492fa56a 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -67,3 +67,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>    # Cloud Hypervisor has no other way to pass Rsdp address to the guest except use a PCD.
>    #
>    gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0x00000005
> +
> +  ##
> +  # This value will be used to determine the level of memory protection each boot.
> +  # See DXE_MEMORY_PROTECTION_PROFILE_INDEX in
> +  # MdeModulePkg/Include/Library/SetMemoryProtectionsLib.h for index definitions.
> +  #
> +  gArmVirtTokenSpaceGuid.PcdDxeMemoryProtectionProfile|0x3|UINT8|0x00000006
> diff --git a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
> index 2039f71a0ebe..9cfd10bc44ef 100644
> --- a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
> +++ b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
> @@ -26,6 +26,7 @@ [Packages]
>    EmbeddedPkg/EmbeddedPkg.dec
>    ArmPkg/ArmPkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
> +  ArmVirtPkg/ArmVirtPkg.dec
>  
>  [LibraryClasses]
>    PeimEntryPoint
> @@ -34,6 +35,7 @@ [LibraryClasses]
>    ArmLib
>    ArmPlatformLib
>    MemoryInitPeiLib
> +  SetMemoryProtectionsLib
>  
>  [Guids]
>    gEfiMemoryTypeInformationGuid
> @@ -44,6 +46,7 @@ [FeaturePcd]
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdSystemMemoryBase
>    gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize
> +  gArmVirtTokenSpaceGuid.PcdDxeMemoryProtectionProfile
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS

(1) I can't say whether setting the profile in MemoryInitPei is the
right (or optimal) place. For other reviewers to comment on.

(2) I think adding a dedicated PCD just for this seems overkill, but
then again I remember we want to allow platform builders to override
this simply with a --pcd option at build time, so it seems sensible.

(3) NB: are we sure this isn't needed for OVMF? (In which case the PCD
would be declared there, and ArmVirtPkg would consume it.) Because I
remember that even "nx stack" would break grub in some ancient debian
distro. Patch 23 ("OvmfPkg: Enable Choosing Memory Protection Profile
via QemuCfg") seems to default to the "release" profile. Just asking.

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109450): https://edk2.groups.io/g/devel/message/109450
Mute This Topic: https://groups.io/mt/101843368/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-