Re: SetMemorySpaceAttributes with EFI_MEMORY_XP

[Michael Zimmermann <sigmaepsilon92@gmail.com>]

Ard, why is SetMSetMemorySpaceAttributes being called in first place?
(ignoring the recent NX patch)
Looking at the initial GCD, it looks like unused memory usually
doesn't have any attributes set anyway.

Thanks
Michael

On Mon, Mar 20, 2017 at 12:04 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 20 March 2017 at 10:32, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote:
>> Hi,
>>
>> I didn't test ArmVirtQemuKernel but I'm trying to use some of the code
>> for another platform.
>> So does this call ever succeed with PcdDxeNxMemoryProtectionPolicy
>> being enabled?
>> https://github.com/tianocore/edk2/blob/76874be3d411bf8daac051718e20932e0bf97d70/ArmVirtPkg/HighMemDxe/HighMemDxe.c#L95
>> Status = gDS->SetMemorySpaceAttributes (CurBase, CurSize, Attributes);
>>
>> Neither the memory that was added by this Dxe nor the one added
>> automatically by GCD has the EFI_MEMORY_XP capability which causes
>> SetMemorySpaceAttributes to return EFI_UNSUPPORTED.
>>
>
> That is a very good point. I have been caught by this more than once
> already (and I did test this, but not as thoroughly as I should have,
> apparently)
>
> This is caused by the unfortunate situation in EDK2 that GCD
> permission attributes are ambiguous: it does not distinguish between
> 'the memory controller allows this range to be configured as
> non-executable' and 'the nature of the contents of this memory region
> allows it to be mapped without executable attributes', and therefore,
> RO/XP are never used in the GCD memory space map.
>
> The solution is to use the CPU_ARCH_PROTOCOL interface explicitly to
> set the XP attribute on the memory itself (but not on the descriptors
> in the GCD or UEFI memory maps). I will spin a patch to fix this.
>
> Thanks,
> Ard.