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

Taylor Beebe posted 28 patches 2 years, 4 months ago
Only 27 patches received!
[edk2-devel] [PATCH v5 24/28] ArmVirtPkg: Apply Memory Protections via SetMemoryProtectionsLib
Posted by Taylor Beebe 2 years, 4 months ago
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
-- 
2.42.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109429): https://edk2.groups.io/g/devel/message/109429
Mute This Topic: https://groups.io/mt/101843368/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v5 24/28] ArmVirtPkg: Apply Memory Protections via SetMemoryProtectionsLib
Posted by Laszlo Ersek 2 years, 4 months ago
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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v5 24/28] ArmVirtPkg: Apply Memory Protections via SetMemoryProtectionsLib
Posted by Gerd Hoffmann 2 years, 4 months ago
  Hi,

> > +  gArmVirtTokenSpaceGuid.PcdDxeMemoryProtectionProfile

> (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.)

Yes, I think both ovmf and armvirt should use this pcd.
ovmf would fallback to that in case nothing is set via QemuFwCfg.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109493): https://edk2.groups.io/g/devel/message/109493
Mute This Topic: https://groups.io/mt/101843368/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-