On 05/03/16 14:45, Ard Biesheuvel wrote:
> This reduces the amount of memory mapped as both writable and executable
> to the absolute minimum, by mapping all of memory non-executable by
> default, and using PermissionsPeCoffExtraActionLib to only map those
> regions executable that require it for execution. If possible, these
> regions are remapped read-only at the same time, but in some cases
> (runtime drivers, TE images, images with < 4 KB section alignment) we
> cannot avoid having to map the entire PE/COFF image RWX.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> ArmVirtPkg/ArmVirtQemu.dsc | 9
> ++++++++-
> ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 3 +++
> 2 files changed, 11 insertions(+), 1 deletion(-)
(1) Could you include a comprehensive table in the commit message that
describes what handling affects what driver type, and where each
treatment is established?
For example, you mention TE images as exempt. According to
"ArmVirtQemu.fdf", this means SEC, PEI_CORE, PEIM. However, below you
link the library into PeiMain.inf (i.e., the PEI_CORE) as well. Why is
that correct?
Also, why must e.g. runtime drivers be mapped fully RWX?
(2) Shouldn't ArmVirtQemuKernel.dsc be modified as well? (Maybe not
identically, since it runs entirely from DRAM, but still?)
(3) I think it's worth keeping in mind that on X64 and Ia32, some
drivers generate code dynamically. For example, in the implementation of
EFI_MP_SERVICES_PROTOCOL. I don't think it should block this series, but
perhaps make a note somewhere (code? commit message?) that such drivers
will have to massage their mappings individually on aarch64.
Thanks!
Laszlo
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 34323bf83d64..fbddd7fac7b5 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -82,6 +82,9 @@ [BuildOptions]
> GCC:*_*_ARM_PLATFORM_FLAGS == -mcpu=cortex-a15
> -I$(WORKSPACE)/ArmVirtPkg/Include
> *_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/ArmVirtPkg/Include
>
> +[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER]
> + GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x1000
> +
>
>
> ################################################################################
> #
> @@ -243,7 +246,10 @@ [Components.common]
> # PEI Phase modules
> #
> ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> - MdeModulePkg/Core/Pei/PeiMain.inf
> + MdeModulePkg/Core/Pei/PeiMain.inf {
> + <LibraryClasses>
> +
> PeCoffExtraActionLib|ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
> + }
> MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
> <LibraryClasses>
> PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> @@ -265,6 +271,7 @@ [Components.common]
> MdeModulePkg/Core/Dxe/DxeMain.inf {
> <LibraryClasses>
>
> NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
> +
> PeCoffExtraActionLib|ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
> }
> MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
> <LibraryClasses>
> diff --git
> a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> index f6c69152848e..c5899aa2ac3c 100644
> --- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> @@ -111,6 +111,9 @@ MemoryPeim (
> // Build Memory Allocation Hob
> InitMmu ();
>
> + ArmSetMemoryRegionNoExec ((EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemoryBase),
> + PcdGet64 (PcdSystemMemorySize));
> +
> if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
> // Optional feature that helps prevent EFI memory map fragmentation.
> BuildMemoryTypeInformationHob ();
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
