Re: [PATCH] ArmPkg/UncachedMemoryAllocationLib: set XP bit via CPU arch protocol

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

On 15 March 2017 at 09:59, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> Hi Ard,
>
> Thanks for the quick turnaround as always.
>
> On 14 March 2017 at 20:20, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> On Tue, Mar 14, 2017 at 07:58:15PM +0000, Ard Biesheuvel wrote:
>>> Commit e7b24ec9785d ("ArmPkg/UncachedMemoryAllocationLib: map uncached
>>> allocations non-executable") adds code that manipulates the GCD memory
>>> space attributes of a newly allocated uncached region without checking
>>> whether this region expose these attributes in its capabilities mask.
>>>
>>> Given that the intent is to remove executable permissions from the region,
>>> this is a fairly pointless exercise to begin with, regardless of whether
>>> it is correct or not. The reason is that RO/XP memory attributes in the
>>> GCD memory space map or the UEFI memory map are completely disconnected
>>> from the actual mapping permissions used in the page tables.
>>>
>>> So instead, invoke the CPU arch protocol directly, and add the non-exec
>>> attributes in the page tables directly.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>
> FVPs and TC2 worked before this change, Juno was hanging on boot as
> per an earlier email thread.
>
> I tested this change on FVP Foundation & AEMv8, TC2 and Juno R0/1/2
> and they all work fine after it's applied to the current HEAD of EDK2
> (commit 8057622).
>

Thanks!