On Mon, Feb 27, 2017 at 02:38:05PM +0000, Ard Biesheuvel wrote:
> To prevent the initial MMU->GCD memory space map synchronization from
> stripping permissions attributes [which we cannot use in the GCD memory
> space map, unfortunately], implement the same approach as x86, and ignore
> SetMemoryAttributes() calls during the time SyncCacheConfig() is in
> progress. This is a horrible hack, but is currently the only way we can
> implement strict permissions on arbitrary memory regions [as opposed to
> PE/COFF text/data sections only]
Sounds like another excellent argument for why this CpuDxe should be
cosying up with the UefiCpuPkg one longer-term.
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Reviewed-by: Jiewen Yao <jiewen....@intel.com>
> ---
> ArmPkg/Drivers/CpuDxe/CpuDxe.c | 3 +++
> ArmPkg/Drivers/CpuDxe/CpuDxe.h | 1 +
> ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 4 ++++
> 3 files changed, 8 insertions(+)
>
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
> index 5aa5b874144a..1955d1dece03 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c
> +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
> @@ -17,6 +17,7 @@
>
> #include <Guid/IdleLoopEvent.h>
>
> +BOOLEAN gIsFlushingGCD;
OK, I am unable to not bikeshed this:
The behaviour you're copying is implemented via a variable called
mIsFlushingGCD. Why change the prefix? Surely we're not looking to
export this variable any further in ARM?
>
> /**
> This function flushes the range of addresses from Start to Start+Length
> @@ -261,7 +262,9 @@ CpuDxeInitialize (
> // and that calls EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes, so this code
> needs to go
> // after the protocol is installed
> //
> + gIsFlushingGCD = TRUE;
> SyncCacheConfig (&mCpu);
> + gIsFlushingGCD = FALSE;
>
> // If the platform is a MPCore system then install the Configuration Table
> describing the
> // secondary core states
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.h b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
> index a00fc3064362..085e4cab2921 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuDxe.h
> +++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.h
> @@ -37,6 +37,7 @@
> #include <Protocol/DebugSupportPeriodicCallback.h>
> #include <Protocol/LoadedImage.h>
>
> +extern BOOLEAN gIsFlushingGCD;
Eew ... this suggest we are.
/
Leif
>
> /**
> This function registers and enables the handler specified by
> InterruptHandler for a processor
> diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> index ebe593d1c325..6dfec7e55888 100644
> --- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> +++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
> @@ -188,6 +188,10 @@ CpuSetMemoryAttributes (
> UINTN RegionLength;
> UINTN RegionArmAttributes;
>
> + if (gIsFlushingGCD) {
> + return EFI_SUCCESS;
> + }
> +
> if ((BaseAddress & (SIZE_4KB - 1)) != 0) {
> // Minimum granularity is SIZE_4KB (4KB on ARM)
> DEBUG ((EFI_D_PAGE, "CpuSetMemoryAttributes(%lx, %lx, %lx): Minimum
> ganularity is SIZE_4KB\n", BaseAddress, Length, EfiAttributes));
> --
> 2.7.4
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
