Re: [PATCH 5/5] ArmVirtPkg: enable non-executable DXE stack for all platforms

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 1 March 2017 at 19:10, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/01/17 17:31, Ard Biesheuvel wrote:
>> Now that ARM has grown support for managing memory permissions in
>> ArmMmuLib, we can enable the non-executable DXE stack for all virt
>> platforms.
>>
>> Note that this is not [entirely] redundant: the non-executable stack
>> is configured before DxeCore is invoked. The image and memory protection
>> features configured during DXE only take affect when the CPU arch
>> protocol implementation is registered.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmVirtPkg/ArmVirt.dsc.inc       | 5 +++++
>>  ArmVirtPkg/ArmVirtQemu.dsc       | 2 --
>>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 --
>>  3 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index acfb71d3ff6c..e2d3dcce7945 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -386,6 +386,11 @@ [PcdsFixedAtBuild.common]
>>    #
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
>>
>> +  #
>> +  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
>> +  #
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>> +
>>  [PcdsFixedAtBuild.ARM]
>>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>>
>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>> index 615e1fca4877..477dfdcfc764 100644
>> --- a/ArmVirtPkg/ArmVirtQemu.dsc
>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
>> @@ -152,8 +152,6 @@ [PcdsFixedAtBuild.common]
>>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>>
>>  [PcdsFixedAtBuild.AARCH64]
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>> -
>>    # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
>>    # support anything bigger, even if the host hardware does
>>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> index e4902690123c..fd39c2802a85 100644
>> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> @@ -163,8 +163,6 @@ [PcdsFixedAtBuild.AARCH64]
>>    #
>>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>>
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>> -
>>    # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
>>    # support anything bigger, even if the host hardware does
>>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
>>
>
> This doesn't just extend PcdSetNxForStack from AARCH64 from ARM, but
> also from QEMU to Xen. Is that your intent? If so,
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>

Yes, it is, but I will mention that in the commit log

Thanks,
Ard.